Fix for Issue elastic/x-pack-elasticsearch#3403 - Predictable ordering of security realms (elastic/x-pack-elasticsearch#3533)

* Security Realms: Predictable ordering for realms

To have predictable ordering of realms, by having secondary
sorting on realm name resulting in stable and consistent documentation.
Documentation update describing how ordering of realms is determined.
Testing done by adding unit test for the change, ran gradle clean check locally.

relates elastic/x-pack-elasticsearch#3403

Original commit: elastic/x-pack-elasticsearch@98c42a8c51

docs/en/security/authentication.asciidoc

@@ -209,8 +209,11 @@ see <<custom-realms, Integrating with Other Authentication Systems>>.
 
 Realms live within a _realm chain_. It is essentially a prioritized list of
 configured realms (typically of various types). The order of the list determines
-the order in which the realms will be consulted. During the authentication process,
-{security} will consult and try to authenticate the request one realm at a time.
+the order in which the realms will be consulted. 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 `name` order.
+During the authentication process, {security} will consult and try to
+authenticate the request one realm at a time.
 Once one of the realms successfully authenticates the request, the authentication
 is considered to be successful and the authenticated user will be associated
 with the request (which will then proceed to the authorization phase). If a realm

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

@@ -98,7 +98,9 @@ under the `xpack.security.authc.realms` namespace. The options you can set depen
 on the settings exposed by the custom realm. At a minimum, you must set the realm
 `type` to the type defined by the extension. 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.
+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.
 +
 IMPORTANT: When you configure realms in `elasticsearch.yml`, only the
 realms you specify are used for authentication. If you also want to use the

plugin/src/main/java/org/elasticsearch/xpack/security/authc/Realm.java

@@ -53,7 +53,12 @@ public abstract class Realm implements Comparable<Realm> {
 
     @Override
     public int compareTo(Realm other) {
-        return Integer.compare(config.order, other.config.order);
+        int result = Integer.compare(config.order, other.config.order);
+        if (result == 0) {
+            // If same order, compare based on the realm name
+            result = config.name.compareTo(other.config.name);
+        }
+        return result;
     }
 
     /**

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

@@ -20,6 +20,7 @@ import org.elasticsearch.xpack.security.authc.ldap.LdapRealm;
 import org.elasticsearch.xpack.security.user.User;
 import org.junit.Before;
 
+import java.math.BigInteger;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -27,6 +28,7 @@ import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 import java.util.Map.Entry;
+import java.util.TreeMap;
 
 import static org.hamcrest.Matchers.contains;
 import static org.hamcrest.Matchers.containsString;
@@ -95,6 +97,45 @@ public class RealmsTests extends ESTestCase {
         }
     }
 
+    public void testWithSettingsWhereDifferentRealmsHaveSameOrder() throws Exception {
+        Settings.Builder builder = Settings.builder()
+                .put("path.home", createTempDir());
+        List<Integer> randomSeq = new ArrayList<>(factories.size() - 2);
+        for (int i = 0; i < factories.size() - 2; i++) {
+            randomSeq.add(i);
+        }
+        Collections.shuffle(randomSeq, random());
+
+        TreeMap<String, Integer> nameToRealmId = new TreeMap<>();
+        for (int i = 0; i < factories.size() - 2; i++) {
+            int randomizedRealmId = randomSeq.get(i);
+            String randomizedRealmName = randomAlphaOfLengthBetween(12,32);
+            nameToRealmId.put("realm_" + randomizedRealmName, randomizedRealmId);
+            builder.put("xpack.security.authc.realms.realm_" + randomizedRealmName + ".type", "type_" + randomizedRealmId);
+            // set same order for all realms
+            builder.put("xpack.security.authc.realms.realm_" + randomizedRealmName + ".order", 1);
+        }
+        Settings settings = builder.build();
+        Environment env = TestEnvironment.newEnvironment(settings);
+        Realms realms = new Realms(settings, env, factories, licenseState, threadContext, reservedRealm);
+
+        Iterator<Realm> iterator = realms.iterator();
+        assertThat(iterator.hasNext(), is(true));
+        Realm realm = iterator.next();
+        assertThat(realm, is(reservedRealm));
+
+        // As order is same for all realms, it should fall back secondary comparison on name
+        // Verify that realms are iterated in order based on name
+        Iterator<String> expectedSortedOrderNames = nameToRealmId.keySet().iterator();
+        while (iterator.hasNext()) {
+            realm = iterator.next();
+            String expectedRealmName = expectedSortedOrderNames.next();
+            assertThat(realm.order(), equalTo(1));
+            assertThat(realm.type(), equalTo("type_" + nameToRealmId.get(expectedRealmName)));
+            assertThat(realm.name(), equalTo(expectedRealmName));
+        }
+    }
+
     public void testWithSettingsWithMultipleInternalRealmsOfSameType() throws Exception {
         Settings settings = Settings.builder()
                 .put("xpack.security.authc.realms.realm_1.type", FileRealm.TYPE)