Add extensionName() to security extension (#79329)

Extension loading code needs to know how to refer to an extension at runtime. It previously used "toString()", but there was no contract that required that this method be implemented in a meaningful way. A new extensionName() method is added which defaults to the class name of the extension, but can be customized by implementations
elastic · Oct 19, 2021 · c369f97 · c369f97
1 parent 4ea349d
commit c369f97
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 30 deletions.
diff --git a/...ck/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityExtension.java b/...ck/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityExtension.java
@@ -115,4 +115,8 @@ default AuthenticationFailureHandler getAuthenticationFailureHandler(SecurityCom
     default AuthorizationEngine getAuthorizationEngine(Settings settings) {
         return null;
     }
+
+    default String extensionName() {
+        return getClass().getName();
+    }
 }
diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java
@@ -43,6 +43,9 @@
 import org.elasticsearch.common.util.concurrent.EsExecutors;
 import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.common.util.set.Sets;
+import org.elasticsearch.core.Nullable;
+import org.elasticsearch.xcontent.NamedXContentRegistry;
+import org.elasticsearch.xcontent.XContentBuilder;
 import org.elasticsearch.env.Environment;
 import org.elasticsearch.env.NodeEnvironment;
 import org.elasticsearch.http.HttpServerTransport;
@@ -567,7 +570,7 @@ Collection<Object> createComponents(Client client, ThreadPool threadPool, Cluste
                 extensionComponents
             );
             if (providers != null && providers.isEmpty() == false) {
-                customRoleProviders.put(extension.toString(), providers);
+                customRoleProviders.put(extension.extensionName(), providers);
             }
         }
 
@@ -676,37 +679,15 @@ auditTrailService, failureHandler, threadPool, anonymousUser, getAuthorizationEn
     }
 
     private AuthorizationEngine getAuthorizationEngine() {
-        AuthorizationEngine authorizationEngine = null;
-        String extensionName = null;
-        for (SecurityExtension extension : securityExtensions) {
-            final AuthorizationEngine extensionEngine = extension.getAuthorizationEngine(settings);
-            if (extensionEngine != null && authorizationEngine != null) {
-                throw new IllegalStateException("Extensions [" + extensionName + "] and [" + extension.toString() + "] "
-                    + "both set an authorization engine");
-            }
-            authorizationEngine = extensionEngine;
-            extensionName = extension.toString();
-        }
-
-        if (authorizationEngine != null) {
-            logger.debug("Using authorization engine from extension [" + extensionName + "]");
-        }
-        return authorizationEngine;
+        return findValueFromExtensions("authorization engine", extension -> extension.getAuthorizationEngine(settings));
     }
 
     private AuthenticationFailureHandler createAuthenticationFailureHandler(final Realms realms,
                                                                             final SecurityExtension.SecurityComponents components) {
-        AuthenticationFailureHandler failureHandler = null;
-        String extensionName = null;
-        for (SecurityExtension extension : securityExtensions) {
-            AuthenticationFailureHandler extensionFailureHandler = extension.getAuthenticationFailureHandler(components);
-            if (extensionFailureHandler != null && failureHandler != null) {
-                throw new IllegalStateException("Extensions [" + extensionName + "] and [" + extension.toString() + "] "
-                        + "both set an authentication failure handler");
-            }
-            failureHandler = extensionFailureHandler;
-            extensionName = extension.toString();
-        }
+        AuthenticationFailureHandler failureHandler = findValueFromExtensions(
+            "authentication failure handler",
+            extension -> extension.getAuthenticationFailureHandler(components)
+        );
         if (failureHandler == null) {
             logger.debug("Using default authentication failure handler");
             Supplier<Map<String, List<String>>> headersSupplier = () -> {
@@ -743,12 +724,48 @@ private AuthenticationFailureHandler createAuthenticationFailureHandler(final Re
             getLicenseState().addListener(() -> {
                 finalDefaultFailureHandler.setHeaders(headersSupplier.get());
             });
-        } else {
-            logger.debug("Using authentication failure handler from extension [" + extensionName + "]");
         }
         return failureHandler;
     }
 
+    /**
+     * Calls the provided function for each configured extension and return the value that was generated by the extensions.
+     * If multiple extensions provide a value, throws {@link IllegalStateException}.
+     * If no extensions provide a value (or if there are no extensions) returns {@code null}.
+     */
+    @Nullable
+    private <T> T findValueFromExtensions(String valueType, Function<SecurityExtension, T> method) {
+        T foundValue = null;
+        String fromExtension = null;
+        for (SecurityExtension extension : securityExtensions) {
+            final T extensionValue = method.apply(extension);
+            if (extensionValue == null) {
+                continue;
+            }
+            if (foundValue == null) {
+                foundValue = extensionValue;
+                fromExtension = extension.extensionName();
+            } else {
+                throw new IllegalStateException(
+                    "Extensions ["
+                        + fromExtension
+                        + "] and ["
+                        + extension.extensionName()
+                        + "] "
+                        + " both attempted to provide a value for ["
+                        + valueType
+                        + "]"
+                );
+            }
+        }
+        if (foundValue == null) {
+            return null;
+        } else {
+            logger.debug("Using [{}] [{}] from extension [{}]", valueType, foundValue, fromExtension);
+            return foundValue;
+        }
+    }
+
     @Override
     public Settings additionalSettings() {
         return additionalSettings(settings, enabled);

diff --git a/...ample-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java b/...ample-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java
@@ -42,6 +42,11 @@ public class ExampleSecurityExtension implements SecurityExtension {
         });
     }
 
+    @Override
+    public String extensionName() {
+        return "example";
+    }
+
     @Override
     public Map<String, Realm.Factory> getRealms(SecurityComponents components) {
         return Map.ofEntries(