Recompute active realms when license changes

This commit changes the implementation of the Realms class to listen
for license changes, and recompute the set of actively licensed realms
only when the license changes rather than each time the "asList" method
is called.

This is primarily a performance optimisation, but it also allows us to
turn off the "in use" license tracking for realms when they are
disabled by a change in license.

Relates: elastic#76476

Author: tvernum

x-pack/plugin/core/src/test/java/org/elasticsearch/license/MockLicenseState.java

@@ -25,4 +25,9 @@ public boolean isAllowed(LicensedFeature feature) {
     public void enableUsageTracking(LicensedFeature feature, String contextName) {
         super.enableUsageTracking(feature, contextName);
     }
+
+    @Override
+    public void disableUsageTracking(LicensedFeature feature, String contextName) {
+        super.disableUsageTracking(feature, contextName);
+    }
 }

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

@@ -8,7 +8,6 @@
 
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
-import org.elasticsearch.Assertions;
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.collect.MapBuilder;
@@ -30,7 +29,6 @@
 import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
 
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
@@ -59,12 +57,11 @@ public class Realms implements Iterable<Realm> {
     private final ThreadContext threadContext;
     private final ReservedRealm reservedRealm;
 
-    protected List<Realm> realms;
-    // a list of realms that are considered standard in that they are provided by x-pack and
-    // interact with a 3rd party source on a limited basis
-    List<Realm> standardRealmsOnly;
-    // a list of realms that are considered native, that is they only interact with x-pack and no 3rd party auth sources
-    List<Realm> nativeRealmsOnly;
+    // All realms that were explicitly configured in the settings, some of these may not be enabled due to licensing
+    private final List<Realm> allConfiguredRealms;
+
+    // the realms in current use. This list will change dynamically as the license changes
+    private volatile List<Realm> activeRealms;
 
     public Realms(Settings settings, Environment env, Map<String, Realm.Factory> factories, XPackLicenseState licenseState,
                   ThreadContext threadContext, ReservedRealm reservedRealm) throws Exception {
@@ -74,35 +71,40 @@ public Realms(Settings settings, Environment env, Map<String, Realm.Factory> fac
         this.licenseState = licenseState;
         this.threadContext = threadContext;
         this.reservedRealm = reservedRealm;
+
         assert XPackSettings.SECURITY_ENABLED.get(settings) : "security must be enabled";
         assert factories.get(ReservedRealm.TYPE) == null;
+
         final List<RealmConfig> realmConfigs = buildRealmConfigs();
-        this.realms = initRealms(realmConfigs);
-        assert realms.get(0) == reservedRealm : "the first realm must be reserved realm";
-        // pre-computing a list of internal only realms allows us to have much cheaper iteration than a custom iterator
-        // and is also simpler in terms of logic. These lists are small, so the duplication should not be a real issue here
-        List<Realm> standardRealms = new ArrayList<>(List.of(reservedRealm));
-        List<Realm> basicRealms = new ArrayList<>(List.of(reservedRealm));
-        for (Realm realm : realms) {
-            // don't add the reserved realm here otherwise we end up with only this realm...
-            if (InternalRealms.isStandardRealm(realm.type())) {
-                standardRealms.add(realm);
-            }
+        this.allConfiguredRealms = initRealms(realmConfigs);
+        this.allConfiguredRealms.forEach(r -> r.initialize(allConfiguredRealms, licenseState));
+        assert allConfiguredRealms.get(0) == reservedRealm : "the first realm must be reserved realm";
 
-            if (InternalRealms.isBuiltinRealm(realm.type())) {
-                basicRealms.add(realm);
-            }
-        }
+        recomputeActiveRealms();
+        licenseState.addListener(this::recomputeActiveRealms);
+    }
 
-        if (Assertions.ENABLED) {
-            for (List<Realm> realmList : Arrays.asList(standardRealms, basicRealms)) {
-                assert realmList.get(0) == reservedRealm : "the first realm must be reserved realm";
-            }
+    protected void recomputeActiveRealms() {
+        final XPackLicenseState licenseStateSnapshot = licenseState.copyCurrentLicenseState();
+        final List<Realm> licensedRealms = calculateLicensedRealms(licenseStateSnapshot);
+        logger.info(
+            "license mode is [{}], currently licensed security realms are [{}]",
+            licenseStateSnapshot.getOperationMode().description(),
+            Strings.collectionToCommaDelimitedString(licensedRealms)
+        );
+
+        // Stop license-tracking for any previously-active realms that are no longer allowed
+        if (activeRealms != null) {
+            activeRealms.stream().filter(r -> licensedRealms.contains(r) == false).forEach(realm -> {
+                if (InternalRealms.isStandardRealm(realm.type())) {
+                    Security.STANDARD_REALMS_FEATURE.stopTracking(licenseStateSnapshot, realm.name());
+                } else {
+                    Security.ALL_REALMS_FEATURE.stopTracking(licenseStateSnapshot, realm.name());
+                }
+            });
         }
 
-        this.standardRealmsOnly = Collections.unmodifiableList(standardRealms);
-        this.nativeRealmsOnly = Collections.unmodifiableList(basicRealms);
-        realms.forEach(r -> r.initialize(this, licenseState));
+        activeRealms = licensedRealms;
     }
 
     @Override
@@ -114,37 +116,33 @@ public Iterator<Realm> iterator() {
      * Returns a list of realms that are configured, but are not permitted under the current license.
      */
     public List<Realm> getUnlicensedRealms() {
-        final XPackLicenseState licenseStateSnapshot = licenseState.copyCurrentLicenseState();
-
-        // If all realms are allowed, then nothing is unlicensed
-        if (Security.ALL_REALMS_FEATURE.checkWithoutTracking(licenseStateSnapshot)) {
-            return Collections.emptyList();
-        }
-
-        final List<Realm> allowedRealms = this.asList();
-        // Shortcut for the typical case, all the configured realms are allowed
-        if (allowedRealms.equals(this.realms)) {
+        final List<Realm> activeSnapshot = activeRealms;
+        if (activeSnapshot.equals(allConfiguredRealms)) {
             return Collections.emptyList();
         }
 
         // Otherwise, we return anything in "all realms" that is not in the allowed realm list
-        return realms.stream().filter(r -> allowedRealms.contains(r) == false).collect(Collectors.toUnmodifiableList());
+        return allConfiguredRealms.stream().filter(r -> activeSnapshot.contains(r) == false).collect(Collectors.toUnmodifiableList());
     }
 
     public Stream<Realm> stream() {
         return StreamSupport.stream(this.spliterator(), false);
     }
 
     public List<Realm> asList() {
-        // TODO : Recalculate this when the license changes rather than on every call
-        return realms.stream().filter(r -> checkLicense(r, licenseState)).collect(Collectors.toUnmodifiableList());
+        assert activeRealms != null : "Active realms not configured";
+        return activeRealms;
+    }
+
+    // Protected for testing
+    protected List<Realm> calculateLicensedRealms(XPackLicenseState licenseStateSnapshot) {
+        return allConfiguredRealms.stream()
+            .filter(r -> checkLicense(r, licenseStateSnapshot))
+            .collect(Collectors.toUnmodifiableList());
     }
 
     private static boolean checkLicense(Realm realm, XPackLicenseState licenseState) {
-        if (ReservedRealm.TYPE.equals(realm.type())) {
-            return true;
-        }
-        if (InternalRealms.isBuiltinRealm(realm.type())) {
+        if (isBasicLicensedRealm(realm)) {
             return true;
         }
         if (InternalRealms.isStandardRealm(realm.type())) {
@@ -153,8 +151,12 @@ private static boolean checkLicense(Realm realm, XPackLicenseState licenseState)
         return Security.ALL_REALMS_FEATURE.checkAndStartTracking(licenseState, realm.name());
     }
 
+    private static boolean isBasicLicensedRealm(Realm realm) {
+        return ReservedRealm.TYPE.equals(realm.type()) || InternalRealms.isBuiltinRealm(realm.type());
+    }
+
     public Realm realm(String name) {
-        for (Realm realm : realms) {
+        for (Realm realm : activeRealms) {
             if (name.equals(realm.name())) {
                 return realm;
             }

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

@@ -51,6 +51,7 @@
 import org.elasticsearch.index.get.GetResult;
 import org.elasticsearch.index.seqno.SequenceNumbers;
 import org.elasticsearch.index.shard.ShardId;
+import org.elasticsearch.license.License;
 import org.elasticsearch.license.MockLicenseState;
 import org.elasticsearch.license.XPackLicenseState;
 import org.elasticsearch.license.XPackLicenseState.Feature;
@@ -220,16 +221,23 @@ public void init() throws Exception {
             .build();
         MockLicenseState licenseState = mock(MockLicenseState.class);
         when(licenseState.isAllowed(Security.ALL_REALMS_FEATURE)).thenReturn(true);
+        when(licenseState.isAllowed(Security.STANDARD_REALMS_FEATURE)).thenReturn(true);
         when(licenseState.checkFeature(Feature.SECURITY_TOKEN_SERVICE)).thenReturn(true);
         when(licenseState.copyCurrentLicenseState()).thenReturn(licenseState);
         when(licenseState.checkFeature(Feature.SECURITY_AUDITING)).thenReturn(true);
+        when(licenseState.getOperationMode()).thenReturn(randomFrom(License.OperationMode.ENTERPRISE, License.OperationMode.PLATINUM));
+
         ReservedRealm reservedRealm = mock(ReservedRealm.class);
         when(reservedRealm.type()).thenReturn("reserved");
         when(reservedRealm.name()).thenReturn("reserved_realm");
         realms = spy(new TestRealms(Settings.EMPTY, TestEnvironment.newEnvironment(settings),
             Map.of(FileRealmSettings.TYPE, config -> mock(FileRealm.class), NativeRealmSettings.TYPE, config -> mock(NativeRealm.class)),
             licenseState, threadContext, reservedRealm, Arrays.asList(firstRealm, secondRealm),
-            Collections.singletonList(firstRealm)));
+            Arrays.asList(firstRealm)));
+
+        // Needed because this is calculated in the constructor, which means the override doesn't get called correctly
+        realms.recomputeActiveRealms();
+        assertThat(realms.asList(), contains(firstRealm, secondRealm));
 
         auditTrail = mock(AuditTrail.class);
         auditTrailService = new AuditTrailService(Collections.singletonList(auditTrail), licenseState);
@@ -388,7 +396,10 @@ public void testAuthenticateBothSupportSecondSucceeds() throws Exception {
         }, this::logAndFail));
         assertTrue(completed.get());
         verify(auditTrail).authenticationFailed(reqId.get(), firstRealm.name(), token, "_action", transportRequest);
-        verify(realms).asList();
+        verify(realms, atLeastOnce()).recomputeActiveRealms();
+        verify(realms, atLeastOnce()).calculateLicensedRealms(any(XPackLicenseState.class));
+        verify(realms, atLeastOnce()).asList();
+        // ^^ We don't care how many times these methods are called, we just check it here so that we can verify no more interactions below.
         verifyNoMoreInteractions(realms);
     }
 
@@ -447,9 +458,8 @@ public void testAuthenticateSmartRealmOrdering() {
 
         verify(auditTrail).authenticationFailed(reqId.get(), firstRealm.name(), token, "_action", transportRequest);
         verify(firstRealm, times(2)).name(); // used above one time
-        verify(firstRealm, atLeastOnce()).type();
-        verify(secondRealm, Mockito.atLeast(3)).name(); // also used in license tracking
-        verify(secondRealm, Mockito.atLeast(3)).type(); // used to create realm ref, and license tracking
+        verify(secondRealm, Mockito.atLeast(2)).name(); // also used in license tracking
+        verify(secondRealm, Mockito.atLeast(2)).type(); // used to create realm ref, and license tracking
         verify(firstRealm, times(2)).token(threadContext);
         verify(secondRealm, times(2)).token(threadContext);
         verify(firstRealm).supports(token);
@@ -573,9 +583,8 @@ public void testAuthenticateSmartRealmOrderingDisabled() {
         }, this::logAndFail));
         verify(auditTrail, times(2)).authenticationFailed(reqId.get(), firstRealm.name(), token, "_action", transportRequest);
         verify(firstRealm, times(3)).name(); // used above one time
-        verify(firstRealm, atLeastOnce()).type();
-        verify(secondRealm, Mockito.atLeast(3)).name();
-        verify(secondRealm, Mockito.atLeast(3)).type(); // used to create realm ref
+        verify(secondRealm, Mockito.atLeast(2)).name();
+        verify(secondRealm, Mockito.atLeast(2)).type(); // used to create realm ref
         verify(firstRealm, times(2)).token(threadContext);
         verify(secondRealm, times(2)).token(threadContext);
         verify(firstRealm, times(2)).supports(token);
@@ -638,10 +647,7 @@ public void testAuthenticateCached() throws Exception {
         assertThat(result.v1(), is(authentication));
         assertThat(result.v1().getAuthenticationType(), is(AuthenticationType.REALM));
         verifyZeroInteractions(auditTrail);
-        verify(firstRealm, atLeastOnce()).type();
-        verify(secondRealm, atLeastOnce()).type();
-        verify(secondRealm, atLeastOnce()).name(); // This realm is license-tracked, which uses the name
-        verifyNoMoreInteractions(firstRealm, secondRealm);
+        verifyZeroInteractions(firstRealm, secondRealm);
         verifyZeroInteractions(operatorPrivilegesService);
     }
 
@@ -920,8 +926,6 @@ public void testAuthenticateTransportContextAndHeader() throws Exception {
                     verifyZeroInteractions(operatorPrivilegesService);
                 }, this::logAndFail));
             assertTrue(completed.compareAndSet(true, false));
-            verify(firstRealm, atLeastOnce()).type();
-            verify(firstRealm, atLeastOnce()).name();
             verifyNoMoreInteractions(firstRealm);
             reset(firstRealm);
         } finally {
@@ -970,8 +974,6 @@ public void testAuthenticateTransportContextAndHeader() throws Exception {
                     verifyZeroInteractions(operatorPrivilegesService);
                 }, this::logAndFail));
             assertTrue(completed.get());
-            verify(firstRealm, atLeastOnce()).type();
-            verify(firstRealm, atLeastOnce()).name();
             verifyNoMoreInteractions(firstRealm);
         } finally {
             terminate(threadPool2);
@@ -2106,12 +2108,40 @@ private static void mockRealmLookupReturnsNull(Realm realm, String username) {
 
     static class TestRealms extends Realms {
 
-        TestRealms(Settings settings, Environment env, Map<String, Factory> factories, XPackLicenseState licenseState,
-                   ThreadContext threadContext, ReservedRealm reservedRealm, List<Realm> realms, List<Realm> internalRealms)
-                throws Exception {
+        private final List<Realm> allRealms;
+        private final List<Realm> internalRealms;
+
+        TestRealms(
+            Settings settings,
+            Environment env,
+            Map<String, Factory> factories,
+            XPackLicenseState licenseState,
+            ThreadContext threadContext,
+            ReservedRealm reservedRealm,
+            List<Realm> realms,
+            List<Realm> internalRealms
+        ) throws Exception {
             super(settings, env, factories, licenseState, threadContext, reservedRealm);
-            this.realms = realms;
-            this.standardRealmsOnly = internalRealms;
+            this.allRealms = realms;
+            this.internalRealms = internalRealms;
+        }
+
+        @Override
+        protected List<Realm> calculateLicensedRealms(XPackLicenseState licenseState) {
+            if (allRealms == null) {
+                // This can happen because the realms are recalculated during construction
+                return super.calculateLicensedRealms(licenseState);
+            }
+            if (Security.STANDARD_REALMS_FEATURE.checkWithoutTracking(licenseState)) {
+                return allRealms;
+            } else {
+                return internalRealms;
+            }
+        }
+
+        // Make public for testing
+        public void recomputeActiveRealms() {
+            super.recomputeActiveRealms();
         }
     }