Add warnings for invalid realm order config (#51195)

docs/reference/migration/index.asciidoc

@@ -11,6 +11,7 @@ For information about how to upgrade your cluster, see <<setup-upgrade>>.
 
 --
 
+include::migrate_7_7.asciidoc[]
 include::migrate_7_6.asciidoc[]
 include::migrate_7_5.asciidoc[]
 include::migrate_7_4.asciidoc[]

docs/reference/migration/migrate_7_7.asciidoc

@@ -31,3 +31,22 @@ loggers in the `org.elasticsearch.action.*` hierarchy emitted log messages at
 log noise. From 7.7 onwards the default log level for logger in this hierarchy
 is now `INFO`, in line with most other loggers. If needed, you can recover the
 pre-7.7 default behaviour by adjusting your <<logging>>.
+
+[discrete]
+[[breaking_77_settings_changes]]
+=== Settings changes
+
+[discrete]
+[[deprecate-missing-realm-order]]
+==== Authentication realm `order` will be a required config in version 8.0.0.
+
+The `order` config will be required in version 8.0.0 for authentication realm
+configuration of any type. If the `order` config is missing for a realm, the node
+will fail to start.
+
+[discrete]
+[[deprecate-duplicated-realm-orders]]
+==== Authentication realm `order` uniqueness will be enforced in version 8.0.0.
+
+The `order` config of authentication realms must be unique in version 8.0.0.
+If you configure more than one realm of any type with the same order, the node will fail to start.

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/RealmSettings.java

@@ -29,10 +29,11 @@
 public class RealmSettings {
 
     public static final String PREFIX = "xpack.security.authc.realms.";
+    public static final String ORDER_SETTING_KEY = "order";
 
     public static final Function<String, Setting.AffixSetting<Boolean>> ENABLED_SETTING = affixSetting("enabled",
             key -> Setting.boolSetting(key, true, Setting.Property.NodeScope));
-    public static final Function<String, Setting.AffixSetting<Integer>> ORDER_SETTING = affixSetting("order",
+    public static final Function<String, Setting.AffixSetting<Integer>> ORDER_SETTING = affixSetting(ORDER_SETTING_KEY,
             key -> Setting.intSetting(key, Integer.MAX_VALUE, Setting.Property.NodeScope));
 
     public static String realmSettingPrefix(String type) {

x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/DeprecationChecks.java

@@ -43,7 +43,9 @@ private DeprecationChecks() {
     static List<BiFunction<Settings, PluginsAndModules, DeprecationIssue>> NODE_SETTINGS_CHECKS =
         Collections.unmodifiableList(Arrays.asList(
             NodeDeprecationChecks::checkPidfile,
-            NodeDeprecationChecks::checkProcessors
+            NodeDeprecationChecks::checkProcessors,
+            NodeDeprecationChecks::checkMissingRealmOrders,
+            NodeDeprecationChecks::checkUniqueRealmOrders
         ));
 
     static List<Function<IndexMetaData, DeprecationIssue>> INDEX_SETTINGS_CHECKS =

x-pack/plugin/deprecation/src/main/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecks.java

@@ -12,8 +12,13 @@
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
+import org.elasticsearch.xpack.core.security.authc.RealmSettings;
 
+import java.util.List;
 import java.util.Locale;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
 
 class NodeDeprecationChecks {
 
@@ -35,6 +40,62 @@ static DeprecationIssue checkProcessors(final Settings settings , final PluginsA
             "https://www.elastic.co/guide/en/elasticsearch/reference/7.4/breaking-changes-7.4.html#deprecate-processors");
     }
 
+    static DeprecationIssue checkMissingRealmOrders(final Settings settings, final PluginsAndModules pluginsAndModules) {
+        final Set<String> orderNotConfiguredRealms = RealmSettings.getRealmSettings(settings).entrySet()
+                .stream()
+                .filter(e -> false == e.getValue().hasValue(RealmSettings.ORDER_SETTING_KEY))
+                .map(e -> RealmSettings.realmSettingPrefix(e.getKey()) + RealmSettings.ORDER_SETTING_KEY)
+                .collect(Collectors.toSet());
+
+        if (orderNotConfiguredRealms.isEmpty()) {
+            return null;
+        }
+
+        final String details = String.format(
+            Locale.ROOT,
+            "Found realms without order config: [%s]. In next major release, node will fail to start with missing realm order.",
+            String.join("; ", orderNotConfiguredRealms));
+        return new DeprecationIssue(
+            DeprecationIssue.Level.CRITICAL,
+            "Realm order will be required in next major release.",
+            "https://www.elastic.co/guide/en/elasticsearch/reference/7.7/breaking-changes-7.7.html#deprecate-missing-realm-order",
+            details
+        );
+    }
+
+    static DeprecationIssue checkUniqueRealmOrders(final Settings settings, final PluginsAndModules pluginsAndModules) {
+        final Map<String, List<String>> orderToRealmSettings =
+            RealmSettings.getRealmSettings(settings).entrySet()
+                .stream()
+                .filter(e -> e.getValue().hasValue(RealmSettings.ORDER_SETTING_KEY))
+                .collect(Collectors.groupingBy(
+                    e -> e.getValue().get(RealmSettings.ORDER_SETTING_KEY),
+                    Collectors.mapping(e -> RealmSettings.realmSettingPrefix(e.getKey()) + RealmSettings.ORDER_SETTING_KEY,
+                        Collectors.toList())));
+
+        Set<String> duplicateOrders = orderToRealmSettings.entrySet().stream()
+            .filter(entry -> entry.getValue().size() > 1)
+            .map(entry -> entry.getKey() + ": " + entry.getValue())
+            .collect(Collectors.toSet());
+
+        if (duplicateOrders.isEmpty()) {
+            return null;
+        }
+
+        final String details = String.format(
+            Locale.ROOT,
+            "Found multiple realms configured with the same order: [%s]. " +
+                "In next major release, node will fail to start with duplicated realm order.",
+            String.join("; ", duplicateOrders));
+
+        return new DeprecationIssue(
+            DeprecationIssue.Level.CRITICAL,
+            "Realm orders must be unique in next major release.",
+            "https://www.elastic.co/guide/en/elasticsearch/reference/7.7/breaking-changes-7.7.html#deprecate-duplicated-realm-orders",
+            details
+        );
+    }
+
     private static DeprecationIssue checkDeprecatedSetting(
         final Settings settings,
         final PluginsAndModules pluginsAndModules,

x-pack/plugin/deprecation/src/test/java/org/elasticsearch/xpack/deprecation/NodeDeprecationChecksTests.java

@@ -13,12 +13,18 @@
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.core.deprecation.DeprecationIssue;
+import org.elasticsearch.xpack.core.security.authc.RealmConfig;
+import org.elasticsearch.xpack.core.security.authc.RealmSettings;
 
 import java.util.Collections;
 import java.util.List;
+import java.util.Locale;
 
 import static org.hamcrest.Matchers.contains;
+import static org.hamcrest.Matchers.containsString;
 import static org.hamcrest.Matchers.empty;
+import static org.hamcrest.Matchers.not;
+import static org.hamcrest.Matchers.startsWith;
 
 public class NodeDeprecationChecksTests extends ESTestCase {
 
@@ -60,4 +66,81 @@ public void testCheckProcessors() {
         assertSettingDeprecationsAndWarnings(new Setting<?>[]{EsExecutors.PROCESSORS_SETTING});
     }
 
+    public void testCheckMissingRealmOrders() {
+        final RealmConfig.RealmIdentifier invalidRealm =
+            new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
+        final RealmConfig.RealmIdentifier validRealm =
+            new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
+        final Settings settings =
+            Settings.builder()
+                .put("xpack.security.authc.realms." + invalidRealm.getType() + "." + invalidRealm.getName() + ".enabled", "true")
+                .put("xpack.security.authc.realms." + validRealm.getType() + "." + validRealm.getName() + ".order", randomInt())
+                .build();
+
+        final PluginsAndModules pluginsAndModules = new PluginsAndModules(Collections.emptyList(), Collections.emptyList());
+        final List<DeprecationIssue> deprecationIssues =
+            DeprecationChecks.filterChecks(DeprecationChecks.NODE_SETTINGS_CHECKS, c -> c.apply(settings, pluginsAndModules));
+
+        assertEquals(1, deprecationIssues.size());
+        assertEquals(new DeprecationIssue(
+            DeprecationIssue.Level.CRITICAL,
+            "Realm order will be required in next major release.",
+            "https://www.elastic.co/guide/en/elasticsearch/reference/7.7/breaking-changes-7.7.html#deprecate-missing-realm-order",
+            String.format(
+                Locale.ROOT,
+                "Found realms without order config: [%s]. In next major release, node will fail to start with missing realm order.",
+                RealmSettings.realmSettingPrefix(invalidRealm) + RealmSettings.ORDER_SETTING_KEY
+            )
+        ), deprecationIssues.get(0));
+    }
+
+    public void testCheckUniqueRealmOrders() {
+        final int order = randomInt(9999);
+
+        final RealmConfig.RealmIdentifier invalidRealm1 =
+            new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
+        final RealmConfig.RealmIdentifier invalidRealm2 =
+            new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
+        final RealmConfig.RealmIdentifier validRealm =
+            new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4, 12));
+        final Settings settings = Settings.builder()
+            .put("xpack.security.authc.realms."
+                + invalidRealm1.getType() + "." + invalidRealm1.getName() + ".order", order)
+            .put("xpack.security.authc.realms."
+                + invalidRealm2.getType() + "." + invalidRealm2.getName() + ".order", order)
+            .put("xpack.security.authc.realms."
+                + validRealm.getType() + "." + validRealm.getName() + ".order", order + 1)
+            .build();
+
+        final PluginsAndModules pluginsAndModules = new PluginsAndModules(Collections.emptyList(), Collections.emptyList());
+        final List<DeprecationIssue> deprecationIssues =
+            DeprecationChecks.filterChecks(DeprecationChecks.NODE_SETTINGS_CHECKS, c -> c.apply(settings, pluginsAndModules));
+
+        assertEquals(1, deprecationIssues.size());
+        assertEquals(DeprecationIssue.Level.CRITICAL, deprecationIssues.get(0).getLevel());
+        assertEquals(
+            "https://www.elastic.co/guide/en/elasticsearch/reference/7.7/breaking-changes-7.7.html#deprecate-duplicated-realm-orders",
+            deprecationIssues.get(0).getUrl());
+        assertEquals("Realm orders must be unique in next major release.", deprecationIssues.get(0).getMessage());
+        assertThat(deprecationIssues.get(0).getDetails(), startsWith("Found multiple realms configured with the same order:"));
+        assertThat(deprecationIssues.get(0).getDetails(), containsString(invalidRealm1.getType() + "." + invalidRealm1.getName()));
+        assertThat(deprecationIssues.get(0).getDetails(), containsString(invalidRealm2.getType() + "." + invalidRealm2.getName()));
+        assertThat(deprecationIssues.get(0).getDetails(), not(containsString(validRealm.getType() + "." + validRealm.getName())));
+    }
+
+    public void testCorrectRealmOrders() {
+        final int order = randomInt(9999);
+        final Settings settings = Settings.builder()
+            .put("xpack.security.authc.realms."
+                + randomAlphaOfLengthBetween(4, 12) + "." + randomAlphaOfLengthBetween(4, 12) + ".order", order)
+            .put("xpack.security.authc.realms."
+                + randomAlphaOfLengthBetween(4, 12) + "." + randomAlphaOfLengthBetween(4, 12) + ".order", order + 1)
+            .build();
+
+        final PluginsAndModules pluginsAndModules = new PluginsAndModules(Collections.emptyList(), Collections.emptyList());
+        final List<DeprecationIssue> deprecationIssues =
+            DeprecationChecks.filterChecks(DeprecationChecks.NODE_SETTINGS_CHECKS, c -> c.apply(settings, pluginsAndModules));
+
+        assertEquals(0, deprecationIssues.size());
+    }
 }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Realms.java

@@ -10,6 +10,7 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.collect.MapBuilder;
+import org.elasticsearch.common.logging.DeprecationLogger;
 import org.elasticsearch.common.settings.Settings;
 import org.elasticsearch.common.util.concurrent.CountDown;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
@@ -34,6 +35,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.Set;
+import java.util.TreeSet;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -45,6 +47,7 @@
 public class Realms implements Iterable<Realm> {
 
     private static final Logger logger = LogManager.getLogger(Realms.class);
+    private static final DeprecationLogger deprecationLogger = new DeprecationLogger(logger);
 
     private final Settings settings;
     private final Environment env;
@@ -186,7 +189,16 @@ protected List<Realm> initRealms() throws Exception {
         List<Realm> realms = new ArrayList<>();
         List<String> kerberosRealmNames = new ArrayList<>();
         Map<String, Set<String>> nameToRealmIdentifier = new HashMap<>();
-        for (RealmConfig.RealmIdentifier identifier: realmsSettings.keySet()) {
+        Set<String> missingOrderRealmSettingKeys = new TreeSet<>();
+        Map<String, Set<String>> orderToRealmOrderSettingKeys = new HashMap<>();
+        for (final Map.Entry<RealmConfig.RealmIdentifier, Settings> entry: realmsSettings.entrySet()) {
+            final RealmConfig.RealmIdentifier identifier = entry.getKey();
+            if (false == entry.getValue().hasValue(RealmSettings.ORDER_SETTING_KEY)) {
+                missingOrderRealmSettingKeys.add(RealmSettings.getFullSettingKey(identifier, RealmSettings.ORDER_SETTING));
+            } else {
+                orderToRealmOrderSettingKeys.computeIfAbsent(entry.getValue().get(RealmSettings.ORDER_SETTING_KEY), k -> new TreeSet<>())
+                    .add(RealmSettings.getFullSettingKey(identifier, RealmSettings.ORDER_SETTING));
+            }
             Realm.Factory factory = factories.get(identifier.getType());
             if (factory == null) {
                 throw new IllegalArgumentException("unknown realm type [" + identifier.getType() + "] for realm [" + identifier + "]");
@@ -236,6 +248,9 @@ protected List<Realm> initRealms() throws Exception {
         if (Strings.hasText(duplicateRealms)) {
             throw new IllegalArgumentException("Found multiple realms configured with the same name: " + duplicateRealms + "");
         }
+
+        logDeprecationIfFound(missingOrderRealmSettingKeys, orderToRealmOrderSettingKeys);
+
         return realms;
     }
 
@@ -354,4 +369,24 @@ public static boolean isRealmTypeAvailable(AllowedRealmType enabledRealmType, St
         }
     }
 
+    private void logDeprecationIfFound(Set<String> missingOrderRealmSettingKeys, Map<String, Set<String>> orderToRealmOrderSettingKeys) {
+        if (missingOrderRealmSettingKeys.size() > 0) {
+            deprecationLogger.deprecated("Found realms without order config: [{}]. " +
+                    "In next major release, node will fail to start with missing realm order.",
+                String.join("; ", missingOrderRealmSettingKeys)
+            );
+        }
+        final List<String> duplicatedRealmOrderSettingKeys = orderToRealmOrderSettingKeys.entrySet()
+            .stream()
+            .filter(e -> e.getValue().size() > 1)
+            .map(e -> e.getKey() + ": " + String.join(",", e.getValue()))
+            .sorted()
+            .collect(Collectors.toList());
+        if (false == duplicatedRealmOrderSettingKeys.isEmpty()) {
+            deprecationLogger.deprecated("Found multiple realms configured with the same order: [{}]. " +
+                    "In next major release, node will fail to start with duplicated realm order.",
+                String.join("; ", duplicatedRealmOrderSettingKeys));
+        }
+    }
+
 }

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/authc/RealmsTests.java

@@ -37,6 +37,7 @@
 import java.util.Map;
 import java.util.Map.Entry;
 import java.util.TreeMap;
+import java.util.stream.Collectors;
 
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsString;
@@ -65,7 +66,7 @@ public void init() throws Exception {
         factories.put(FileRealmSettings.TYPE, config -> new DummyRealm(FileRealmSettings.TYPE, config));
         factories.put(NativeRealmSettings.TYPE, config -> new DummyRealm(NativeRealmSettings.TYPE, config));
         factories.put(KerberosRealmSettings.TYPE, config -> new DummyRealm(KerberosRealmSettings.TYPE, config));
-        randomRealmTypesCount = randomIntBetween(1, 5);
+        randomRealmTypesCount = randomIntBetween(2, 5);
         for (int i = 0; i < randomRealmTypesCount; i++) {
             String name = "type_" + i;
             factories.put(name, config -> new DummyRealm(name, config));
@@ -154,6 +155,13 @@ public void testWithSettingsWhereDifferentRealmsHaveSameOrder() throws Exception
 
         assertThat(realms.getUnlicensedRealms(), empty());
         assertThat(realms.getUnlicensedRealms(), sameInstance(realms.getUnlicensedRealms()));
+
+        assertWarnings("Found multiple realms configured with the same order: [1: "
+            + nameToRealmId.entrySet().stream()
+                .map(e -> "xpack.security.authc.realms.type_" + e.getValue() + "." + e.getKey() + ".order")
+                .sorted().collect(Collectors.joining(","))
+            + "]. "
+            + "In next major release, node will fail to start with duplicated realm order.");
     }
 
     public void testWithSettingsWithMultipleInternalRealmsOfSameType() throws Exception {
@@ -614,6 +622,20 @@ public void testInitRealmsFailsForMultipleKerberosRealms() throws IOException {
                 "multiple realms [realm_1, realm_2] configured of type [kerberos], [kerberos] can only have one such realm configured")));
     }
 
+    public void testWarningForMissingRealmOrder() throws Exception {
+        final int realmTypeId = randomIntBetween(0, randomRealmTypesCount - 1);
+        final String realmName = randomAlphaOfLengthBetween(4, 12);
+        final Settings settings = Settings.builder()
+                .put("path.home", createTempDir())
+                .put("xpack.security.authc.realms.type_" + realmTypeId + ".realm_" + realmName + ".enabled", true)
+                .build();
+
+        new Realms(settings, TestEnvironment.newEnvironment(settings), factories, licenseState, threadContext, reservedRealm);
+        assertWarnings("Found realms without order config: [xpack.security.authc.realms.type_"
+            + realmTypeId + ".realm_" + realmName + ".order]. "
+            + "In next major release, node will fail to start with missing realm order.");
+    }
+
     static class DummyRealm extends Realm {
 
         DummyRealm(String type, RealmConfig config) {