Browse Source

Fix broken short-circuit in getUnlicensedRealms (#44399)

The existing equals check was broken, and would always be false.

The correct behaviour is to return "Collections.emptyList()" whenever
the the active(licensed)-realms equals the configured-realms.
Tim Vernum 6 years ago
parent
commit
2b6548905d

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

@@ -135,7 +135,7 @@ public class Realms implements Iterable<Realm> {
 
 
         final List<Realm> allowedRealms = this.asList();
         final List<Realm> allowedRealms = this.asList();
         // Shortcut for the typical case, all the configured realms are allowed
         // Shortcut for the typical case, all the configured realms are allowed
-        if (allowedRealms.equals(this.realms.size())) {
+        if (allowedRealms.equals(this.realms)) {
             return Collections.emptyList();
             return Collections.emptyList();
         }
         }
 
 

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

@@ -48,6 +48,7 @@ import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.is;
 import static org.hamcrest.Matchers.iterableWithSize;
 import static org.hamcrest.Matchers.iterableWithSize;
 import static org.hamcrest.Matchers.notNullValue;
 import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.sameInstance;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 import static org.mockito.Mockito.when;
 
 
@@ -110,6 +111,7 @@ public class RealmsTests extends ESTestCase {
         }
         }
 
 
         assertThat(realms.getUnlicensedRealms(), empty());
         assertThat(realms.getUnlicensedRealms(), empty());
+        assertThat(realms.getUnlicensedRealms(), sameInstance(realms.getUnlicensedRealms()));
     }
     }
 
 
     public void testWithSettingsWhereDifferentRealmsHaveSameOrder() throws Exception {
     public void testWithSettingsWhereDifferentRealmsHaveSameOrder() throws Exception {
@@ -150,6 +152,7 @@ public class RealmsTests extends ESTestCase {
         }
         }
 
 
         assertThat(realms.getUnlicensedRealms(), empty());
         assertThat(realms.getUnlicensedRealms(), empty());
+        assertThat(realms.getUnlicensedRealms(), sameInstance(realms.getUnlicensedRealms()));
     }
     }
 
 
     public void testWithSettingsWithMultipleInternalRealmsOfSameType() throws Exception {
     public void testWithSettingsWithMultipleInternalRealmsOfSameType() throws Exception {
@@ -185,6 +188,7 @@ public class RealmsTests extends ESTestCase {
         assertThat(iter.hasNext(), is(false));
         assertThat(iter.hasNext(), is(false));
 
 
         assertThat(realms.getUnlicensedRealms(), empty());
         assertThat(realms.getUnlicensedRealms(), empty());
+        assertThat(realms.getUnlicensedRealms(), sameInstance(realms.getUnlicensedRealms()));
     }
     }
 
 
     public void testUnlicensedWithOnlyCustomRealms() throws Exception {
     public void testUnlicensedWithOnlyCustomRealms() throws Exception {
@@ -220,6 +224,7 @@ public class RealmsTests extends ESTestCase {
         }
         }
 
 
         assertThat(realms.getUnlicensedRealms(), empty());
         assertThat(realms.getUnlicensedRealms(), empty());
+        assertThat(realms.getUnlicensedRealms(), sameInstance(realms.getUnlicensedRealms()));
 
 
         when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.DEFAULT);
         when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.DEFAULT);
 
 
@@ -303,6 +308,7 @@ public class RealmsTests extends ESTestCase {
         }
         }
         assertThat(types, contains("ldap", "type_0"));
         assertThat(types, contains("ldap", "type_0"));
         assertThat(realms.getUnlicensedRealms(), empty());
         assertThat(realms.getUnlicensedRealms(), empty());
+        assertThat(realms.getUnlicensedRealms(), sameInstance(realms.getUnlicensedRealms()));
 
 
         when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.DEFAULT);
         when(licenseState.allowedRealmType()).thenReturn(AllowedRealmType.DEFAULT);
         iter = realms.iterator();
         iter = realms.iterator();