Browse Source

Security: Use settings from Environment object (#98770)

Settings have a lifecycle and may evolve between when plugins are
constructed and when `createComponents` is called. This commit fixes the
Security plugin to use Environment.settings() once it is available
Tim Vernum 2 years ago
parent
commit
adee9fadeb

+ 11 - 6
x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -526,7 +526,7 @@ public class Security extends Plugin
 
     private static final Logger logger = LogManager.getLogger(Security.class);
 
-    private final Settings settings;
+    private Settings settings;
     private final boolean enabled;
     private final SecuritySystemIndices systemIndices;
     private final ListenableFuture<Void> nodeStartedListenable;
@@ -563,7 +563,10 @@ public class Security extends Plugin
     }
 
     Security(Settings settings, List<SecurityExtension> extensions) {
-        // TODO This is wrong. Settings can change after this. We should use the settings from createComponents
+        // Note: The settings that are passed in here might not be the final values - things like Plugin.additionalSettings()
+        // will be called after the plugins are constructed, and may introduce new setting values.
+        // Accordingly we should avoid using this settings object for very much and mostly rely on Environment.setting() as provided
+        // to createComponents.
         this.settings = settings;
         // TODO this is wrong, we should only use the environment that is provided to createComponents
         this.enabled = XPackSettings.SECURITY_ENABLED.get(settings);
@@ -667,6 +670,10 @@ public class Security extends Plugin
             return Collections.singletonList(new SecurityUsageServices(null, null, null, null, null, null));
         }
 
+        // The settings in `environment` may have additional values over what was provided during construction
+        // See Plugin#additionalSettings()
+        this.settings = environment.settings();
+
         systemIndices.init(client, clusterService);
 
         scriptServiceReference.set(scriptService);
@@ -780,9 +787,7 @@ public class Security extends Plugin
         );
         components.add(privilegeStore);
 
-        final ReservedRolesStore reservedRolesStore = new ReservedRolesStore(
-            Set.copyOf(INCLUDED_RESERVED_ROLES_SETTING.get(environment.settings()))
-        );
+        final ReservedRolesStore reservedRolesStore = new ReservedRolesStore(Set.copyOf(INCLUDED_RESERVED_ROLES_SETTING.get(settings)));
         dlsBitsetCache.set(new DocumentSubsetBitsetCache(settings, threadPool));
         final FieldPermissionsCache fieldPermissionsCache = new FieldPermissionsCache(settings);
         final FileRolesStore fileRolesStore = new FileRolesStore(
@@ -905,7 +910,7 @@ public class Security extends Plugin
         final AuthenticationFailureHandler failureHandler = createAuthenticationFailureHandler(realms, extensionComponents);
 
         // operator privileges are enabled either explicitly via the setting or if running serverless
-        final boolean operatorPrivilegesEnabled = OPERATOR_PRIVILEGES_ENABLED.get(environment.settings()) || DiscoveryNode.isServerless();
+        final boolean operatorPrivilegesEnabled = OPERATOR_PRIVILEGES_ENABLED.get(settings) || DiscoveryNode.isServerless();
 
         if (operatorPrivilegesEnabled) {
             logger.info("operator privileges are enabled");