Make order setting mandatory for Realm config

docs/reference/migration/migrate_8_0/security.asciidoc

@@ -6,6 +6,29 @@
 //Installation and Upgrade Guide
 
 //tag::notable-breaking-changes[]
+[float]
+==== The realm `order` setting is required
+
+The `xpack.security.authc.realms.*.*.order` setting is now required and must be
+specified for each explicitly configured realm. Each value must be unique.
+The cluster will fail to start if the requirements are not met.
+
+For example, the following configuration is invalid:
+[source,yaml]
+--------------------------------------------------
+xpack.security.authc.realms.kerberos.kerb1:
+  keytab.path: es.keytab
+  remove_realm_name: false
+--------------------------------------------------
+
+And must be configured as:
+[source,yaml]
+--------------------------------------------------
+xpack.security.authc.realms.kerberos.kerb1:
+  order: 0
+  keytab.path: es.keytab
+  remove_realm_name: false
+--------------------------------------------------
 
 // end::notable-breaking-changes[]
 
@@ -79,17 +102,17 @@ It is now an error to configure any SSL settings for
 For example, the following configuration is invalid:
 [source,yaml]
 --------------------------------------------------
-xpack.security.http.ssl.certificate: elasticsearch.crt 
-xpack.security.http.ssl.key: elasticsearch.key 
+xpack.security.http.ssl.certificate: elasticsearch.crt
+xpack.security.http.ssl.key: elasticsearch.key
 xpack.security.http.ssl.certificate_authorities: [ "corporate-ca.crt" ]
 --------------------------------------------------
 
 And must be configured as either:
 [source,yaml]
 --------------------------------------------------
 xpack.security.http.ssl.enabled: true <1>
-xpack.security.http.ssl.certificate: elasticsearch.crt 
-xpack.security.http.ssl.key: elasticsearch.key 
+xpack.security.http.ssl.certificate: elasticsearch.crt
+xpack.security.http.ssl.key: elasticsearch.key
 xpack.security.http.ssl.certificate_authorities: [ "corporate-ca.crt" ]
 --------------------------------------------------
 <1> or `false`.
@@ -110,4 +133,3 @@ a certificate and key through use of the `xpack.security.http.ssl.keystore.path`
 setting or the `xpack.security.http.ssl.certificate` and
 `xpack.security.http.ssl.key` settings.
 
-

docs/reference/settings/security-settings.asciidoc

@@ -189,7 +189,7 @@ namespace in `elasticsearch.yml`. For example:
 xpack.security.authc.realms:
 
     native.realm1: <1>
-        order: 0
+        order: 0 <2>
         ...
 
     ldap.realm2:
@@ -204,6 +204,8 @@ xpack.security.authc.realms:
 <1> Specifies the type of realm (for example, `native`, `ldap`,
 `active_directory`, `pki`, `file`, `kerberos`, `saml`) and the realm name. This
 information is required.
+<2> Specifies priority of a realm in the realm chain. This information
+is required.
 
 The valid settings vary depending on the realm type. For more
 information, see <<setting-up-authentication>>.
@@ -214,8 +216,7 @@ information, see <<setting-up-authentication>>.
 
 `order`::
 The priority of the realm within the realm chain. Realms with a lower order are
-consulted first. Although not required, use of this setting is strongly
-recommended when you configure multiple realms. Defaults to `Integer.MAX_VALUE`.
+consulted first. The value must be unique for each realm. This setting is required.
 
 `enabled`::
 Indicates whether a realm is enabled. You can use this setting to disable a

x-pack/docs/en/security/authentication/configuring-active-directory-realm.asciidoc

@@ -3,10 +3,7 @@ realm and map Active Directory users and groups to roles in the role mapping fil
 
 . Add a realm configuration of type `active_directory` to `elasticsearch.yml`
 under the `xpack.security.authc.realms.active_directory` namespace.
-At a minimum, you must specify the Active Directory `domain_name`.
-If you are configuring multiple realms, you should also 
-explicitly set the `order` attribute to control the order in which the realms 
-are consulted during authentication. 
+At a minimum, you must specify the Active Directory `domain_name` and `order`.
 +
 --
 See <<ref-ad-settings>> for all of the options you can set for an 

x-pack/docs/en/security/authentication/configuring-ldap-realm.asciidoc

@@ -21,10 +21,9 @@ However, multiple bind operations might be needed to find the correct user DN.
 
 .. Add a realm configuration of to `elasticsearch.yml` under the
 `xpack.security.authc.realms.ldap` namespace. At a minimum, you must specify
-the `url` of the LDAP server, and set `user_search.base_dn` to the container DN
-where the users are searched for.
-If you are configuring multiple realms, you should also explicitly set the
-`order` attribute to control the order in which the realms are consulted during 
+the `url` and `order` of the LDAP server, and set `user_search.base_dn` to the
+container DN where the users are searched for.
+The `order` attribute to control the order in which the realms are consulted during 
 authentication. See <<ref-ldap-settings>> for all of the options you can set for 
 an `ldap` realm.
 +
@@ -73,7 +72,7 @@ realms you specify are used for authentication. If you also want to use the
 .. Add a realm configuration to `elasticsearch.yml` in the
 `xpack.security.authc.realms.ldap` namespace. At a minimum, you must specify
 the `url` of the LDAP server, and specify at least one template with the
-`user_dn_templates` option. If you are configuring multiple realms, you should
+`user_dn_templates` option. If you are configuring multiple realms, you must
 also explicitly set the `order` attribute to control the order in which the
 realms are consulted during authentication.
 See <<ref-ldap-settings>> for all of the options you can set for an `ldap` realm.
@@ -206,6 +205,7 @@ xpack:
       realms:
         ldap:
           ldap1:
+            order: 0
             metadata: cn
 --------------------------------------------------
 --

x-pack/docs/en/security/authentication/configuring-pki-realm.asciidoc

@@ -21,9 +21,8 @@ clients connect directly to {es}.
 
 . Add a realm configuration for a `pki` realm to `elasticsearch.yml` under the
 `xpack.security.authc.realms.pki` namespace.
-If you are configuring multiple realms, you should 
-explicitly set the `order` attribute. See <<ref-pki-settings>> for all of the 
-options you can set for a `pki` realm.
+You must explicitly set the `order` attribute. See <<ref-pki-settings>> for all
+of the options you can set for a `pki` realm.
 +
 --
 For example, the following snippet shows the most basic `pki` realm configuration:
@@ -61,6 +60,7 @@ xpack:
       realms:
         pki:
           pki1:
+            order: 1
             username_pattern: "EMAILADDRESS=(.*?)(?:,|$)"
 ------------------------------------------------------------
 
@@ -118,6 +118,7 @@ xpack:
       realms:
         pki:
           pki1:
+            order: 1
             truststore:
               path: "pki1_truststore.jks"
 ------------------------------------------------------------

x-pack/docs/en/security/authentication/custom-realm.asciidoc

@@ -89,11 +89,10 @@ under the `xpack.security.authc.realms` namespace.
 You must define your realm within the namespace that matchesto the type defined
 by the extension.
 The options you can set depend on the settings exposed by the custom realm.
-If you are configuring multiple realms, you should also explicitly set the
-`order` attribute to control the order in which the realms are consulted during
-authentication. You should make sure each configured realm has a distinct
-`order` setting. In the event that two or more realms have the same `order`,
-they will be processed in realm `name` order.
+At a minimum, you must explicitly set the `order` attribute to control the
+order in which the realms are consulted during authentication. You must also
+make sure each configured realm has a distinct `order` setting. In the event
+that two or more realms have the same `order`, the cluster will fail to start.
 +
 IMPORTANT: When you configure realms in `elasticsearch.yml`, only the
 realms you specify are used for authentication. If you also want to use the

x-pack/docs/en/security/authentication/realm-chains.asciidoc

@@ -5,9 +5,9 @@
 <<realms,Realms>> live within a _realm chain_. It is essentially a prioritized
 list of configured realms (typically of various types). Realms are consulted in
 ascending order (that is to say, the realm with the lowest `order` value is
-consulted first). You should make sure each configured realm has a distinct
+consulted first). You must make sure each configured realm has a distinct
 `order` setting. In the event that two or more realms have the same `order`,
-they are processed in `name` order.
+the cluster will fail to start.
 
 During the authentication process, {stack} {security-features} consult and try
 to authenticate the request one realm at a time. Once one of the realms

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

@@ -27,9 +27,15 @@ public RealmConfig(RealmIdentifier identifier, Settings settings, Environment en
         this.identifier = identifier;
         this.settings = settings;
         this.env = env;
+        this.threadContext = threadContext;
         this.enabled = getSetting(RealmSettings.ENABLED_SETTING);
+        if (false == hasSetting(RealmSettings.ORDER_SETTING.apply(type()))) {
+            throw new IllegalArgumentException("'order' is a mandatory parameter for realm config. " +
+                "Found invalid config for realm: '" + identifier.name + "'\n" +
+                "Please see the breaking changes documentation."
+            );
+        }
         this.order = getSetting(RealmSettings.ORDER_SETTING);
-        this.threadContext = threadContext;
     }
 
     public RealmIdentifier identifier() {

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

@@ -0,0 +1,51 @@
+/*
+ *
+ *  * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ *  * or more contributor license agreements. Licensed under the Elastic License;
+ *  * you may not use this file except in compliance with the Elastic License.
+ *
+ */
+
+package org.elasticsearch.xpack.core.security.authc;
+
+import org.elasticsearch.common.settings.Settings;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
+import org.elasticsearch.env.Environment;
+import org.elasticsearch.test.ESTestCase;
+import org.junit.Before;
+import org.mockito.Mockito;
+
+import static org.hamcrest.Matchers.containsString;
+
+public class RealmConfigTests extends ESTestCase {
+
+    private RealmConfig.RealmIdentifier realmIdentifier;
+    private Settings globalSettings;
+    private Environment environment;
+    private ThreadContext threadContext;
+
+    @Before
+    public void setUp() throws Exception {
+        realmIdentifier = new RealmConfig.RealmIdentifier(randomAlphaOfLengthBetween(4, 12), randomAlphaOfLengthBetween(4,12));
+        environment = Mockito.mock(Environment.class);
+        globalSettings = Settings.builder().put("path.home", createTempDir()).build();
+        threadContext = new ThreadContext(globalSettings);
+        super.setUp();
+    }
+
+    public void testWillPassWhenOrderSettingIsConfigured() {
+        Settings settings = Settings.builder()
+                .put(globalSettings)
+                .put(RealmSettings.realmSettingPrefix(realmIdentifier) + "order", 0)
+                .build();
+
+        RealmConfig realmConfig = new RealmConfig(realmIdentifier, settings, environment, threadContext);
+        assertEquals(0, realmConfig.order);
+    }
+
+    public void testWillFailWhenOrderSettingIsMissing() {
+        Settings settings = Settings.builder().put(globalSettings).build();
+        var e = expectThrows(IllegalArgumentException.class, () -> new RealmConfig(realmIdentifier, settings, environment, threadContext));
+        assertThat(e.getMessage(), containsString("'order' is a mandatory parameter for realm config"));
+    }
+}

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

@@ -186,6 +186,7 @@ protected List<Realm> initRealms() throws Exception {
         List<Realm> realms = new ArrayList<>();
         List<String> kerberosRealmNames = new ArrayList<>();
         Map<String, Set<String>> nameToRealmIdentifier = new HashMap<>();
+        Map<Integer, Set<String>> orderToRealmName = new HashMap<>();
         for (RealmConfig.RealmIdentifier identifier: realmsSettings.keySet()) {
             Realm.Factory factory = factories.get(identifier.getType());
             if (factory == null) {
@@ -218,9 +219,13 @@ protected List<Realm> initRealms() throws Exception {
             Realm realm = factory.create(config);
             nameToRealmIdentifier.computeIfAbsent(realm.name(), k ->
                 new HashSet<>()).add(RealmSettings.realmSettingPrefix(realm.type()) + realm.name());
+            orderToRealmName.computeIfAbsent(realm.order(), k -> new HashSet<>())
+                .add(realm.name());
             realms.add(realm);
         }
 
+        checkUniqueOrders(orderToRealmName);
+
         if (!realms.isEmpty()) {
             Collections.sort(realms);
         } else {
@@ -305,15 +310,34 @@ public void usageStats(ActionListener<Map<String, Object>> listener) {
     private void addNativeRealms(List<Realm> realms) throws Exception {
         Realm.Factory fileRealm = factories.get(FileRealmSettings.TYPE);
         if (fileRealm != null) {
+            var realmIdentifier = new RealmConfig.RealmIdentifier(FileRealmSettings.TYPE, "default_" + FileRealmSettings.TYPE);
             realms.add(fileRealm.create(new RealmConfig(
-                    new RealmConfig.RealmIdentifier(FileRealmSettings.TYPE, "default_" + FileRealmSettings.TYPE),
-                    settings, env, threadContext)));
+                realmIdentifier,
+                ensureOrderSetting(settings, realmIdentifier, Integer.MIN_VALUE + 1),
+                env, threadContext)));
         }
         Realm.Factory indexRealmFactory = factories.get(NativeRealmSettings.TYPE);
         if (indexRealmFactory != null) {
+            var realmIdentifier = new RealmConfig.RealmIdentifier(NativeRealmSettings.TYPE, "default_" + NativeRealmSettings.TYPE);
             realms.add(indexRealmFactory.create(new RealmConfig(
-                    new RealmConfig.RealmIdentifier(NativeRealmSettings.TYPE, "default_" + NativeRealmSettings.TYPE),
-                    settings, env, threadContext)));
+                realmIdentifier,
+                ensureOrderSetting(settings, realmIdentifier, Integer.MIN_VALUE + 2),
+                env, threadContext)));
+        }
+    }
+
+    private Settings ensureOrderSetting(Settings settings, RealmConfig.RealmIdentifier realmIdentifier, int order) {
+        String orderSettingKey = RealmSettings.realmSettingPrefix(realmIdentifier) + "order";
+        return Settings.builder().put(settings).put(orderSettingKey, order).build();
+    }
+
+    private void checkUniqueOrders(Map<Integer, Set<String>> orderToRealmName) {
+        String duplicateOrders = orderToRealmName.entrySet().stream()
+            .filter(entry -> entry.getValue().size() > 1)
+            .map(entry -> entry.getKey() + ": " + entry.getValue())
+            .collect(Collectors.joining("; "));
+        if (Strings.hasText(duplicateOrders)) {
+            throw new IllegalArgumentException("Found multiple realms configured with the same order: " + duplicateOrders + "");
         }
     }
 

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

@@ -18,6 +18,7 @@
 import org.elasticsearch.xpack.core.XPackSettings;
 import org.elasticsearch.xpack.core.security.authc.AuthenticationResult;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
+import org.elasticsearch.xpack.core.security.authc.RealmSettings;
 import org.elasticsearch.xpack.core.security.authc.esnative.ClientReservedRealm;
 import org.elasticsearch.xpack.core.security.authc.support.Hasher;
 import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken;
@@ -63,7 +64,11 @@ public class ReservedRealm extends CachingUsernamePasswordRealm {
 
     public ReservedRealm(Environment env, Settings settings, NativeUsersStore nativeUsersStore, AnonymousUser anonymousUser,
                          SecurityIndexManager securityIndex, ThreadPool threadPool) {
-        super(new RealmConfig(new RealmConfig.RealmIdentifier(TYPE, TYPE), settings, env, threadPool.getThreadContext()), threadPool);
+        super(new RealmConfig(new RealmConfig.RealmIdentifier(TYPE, TYPE),
+            Settings.builder()
+                .put(settings)
+                .put(RealmSettings.realmSettingPrefix(new RealmConfig.RealmIdentifier(TYPE, TYPE)) + "order", Integer.MIN_VALUE)
+                .build(), env, threadPool.getThreadContext()), threadPool);
         this.nativeUsersStore = nativeUsersStore;
         this.realmEnabled = XPackSettings.RESERVED_REALM_ENABLED_SETTING.get(settings);
         this.anonymousUser = anonymousUser;

x-pack/plugin/security/src/test/java/org/elasticsearch/test/SecuritySettingsSource.java

@@ -185,7 +185,7 @@ protected String nodeClientUsername() {
     protected SecureString nodeClientPassword() {
         return new SecureString(TEST_PASSWORD.toCharArray());
     }
-    
+
     public static void addSSLSettingsForNodePEMFiles(Settings.Builder builder, String prefix, boolean hostnameVerificationEnabled) {
         addSSLSettingsForPEMFiles(builder, prefix,
             "/org/elasticsearch/xpack/security/transport/ssl/certs/simple/testnode.pem", "testnode",

x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/action/oidc/TransportOpenIdConnectLogoutActionTests.java

@@ -44,6 +44,7 @@
 import org.elasticsearch.xpack.core.security.action.oidc.OpenIdConnectLogoutResponse;
 import org.elasticsearch.xpack.core.security.authc.Authentication;
 import org.elasticsearch.xpack.core.security.authc.RealmConfig;
+import org.elasticsearch.xpack.core.security.authc.RealmSettings;
 import org.elasticsearch.xpack.core.security.authc.oidc.OpenIdConnectRealmSettings;
 import org.elasticsearch.xpack.core.security.user.User;
 import org.elasticsearch.xpack.core.ssl.SSLService;
@@ -88,9 +89,11 @@ public class TransportOpenIdConnectLogoutActionTests extends OpenIdConnectTestCa
 
     @Before
     public void setup() throws Exception {
+        final RealmConfig.RealmIdentifier realmIdentifier = new RealmConfig.RealmIdentifier("oidc", REALM_NAME);
         final Settings settings = getBasicRealmSettings()
             .put(XPackSettings.TOKEN_SERVICE_ENABLED_SETTING.getKey(), true)
             .put("path.home", createTempDir())
+            .put(RealmSettings.getFullSettingKey(realmIdentifier, RealmSettings.ORDER_SETTING), 0)
             .build();
         final Settings sslSettings = Settings.builder()
             .put("xpack.security.authc.realms.oidc.oidc-realm.ssl.verification_mode", "certificate")
@@ -179,8 +182,6 @@ public void setup() throws Exception {
 
         final Environment env = TestEnvironment.newEnvironment(settings);
 
-        final RealmConfig.RealmIdentifier realmIdentifier = new RealmConfig.RealmIdentifier("oidc", REALM_NAME);
-
         final RealmConfig realmConfig = new RealmConfig(realmIdentifier, settings, env, threadContext);
         oidcRealm = new OpenIdConnectRealm(realmConfig, new SSLService(TestEnvironment.newEnvironment(sslSettings)),
             mock(UserRoleMapper.class), mock(ResourceWatcherService.class));