From 595e8c4600e7299c7cd136fd281752105919749b Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Mon, 18 Oct 2021 10:55:41 +1100 Subject: [PATCH 1/3] Add name() to security extension 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 name() method is added which defaults to the class name of the extension, but can be customized by implementations --- .../core/security/SecurityExtension.java | 4 + .../xpack/security/Security.java | 74 +++++++++++-------- .../example/ExampleSecurityExtension.java | 5 ++ 3 files changed, 54 insertions(+), 29 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityExtension.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityExtension.java index c1a087061f99..2db295644553 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/SecurityExtension.java +++ b/x-pack/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 index bc1c62efbadd..e01424b420d5 100644 --- 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,7 @@ 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; @@ -651,37 +652,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>> headersSupplier = () -> { @@ -718,12 +697,49 @@ 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 findValueFromExtensions(String valueType, Function 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/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java b/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java index 7d02b82b60af..d09dc73c7b11 100644 --- a/x-pack/qa/security-example-spi-extension/src/main/java/org/elasticsearch/example/ExampleSecurityExtension.java +++ b/x-pack/qa/security-example-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 getRealms(SecurityComponents components) { return Map.ofEntries( From 3187ae359394b9c5116ba348cfc63a8387845fc9 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Mon, 18 Oct 2021 12:58:53 +1100 Subject: [PATCH 2/3] Use extensionName when dealing with role providers --- .../main/java/org/elasticsearch/xpack/security/Security.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index ceb4d7466b45..1ac3e97311e9 100644 --- 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 @@ -561,7 +561,7 @@ Collection createComponents(Client client, ThreadPool threadPool, Cluste extensionComponents ); if (providers != null && providers.isEmpty() == false) { - customRoleProviders.put(extension.toString(), providers); + customRoleProviders.put(extension.extensionName(), providers); } } From 25cc971a02fe8a8fff1ca77ce50b7b40c22b1af4 Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Mon, 18 Oct 2021 16:47:55 +1100 Subject: [PATCH 3/3] Update x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java Co-authored-by: Yang Wang --- .../main/java/org/elasticsearch/xpack/security/Security.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) 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 index 1ac3e97311e9..a1b1a4b97bf4 100644 --- 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 @@ -736,8 +736,7 @@ private T findValueFromExtensions(String valueType, Function