From 0f8a13c92c99a3f0626b52189d5a9640e54ce85a Mon Sep 17 00:00:00 2001 From: Tim Vernum Date: Wed, 10 Oct 2018 09:55:40 +0100 Subject: [PATCH] Allow an AuthenticationResult to return metadata PR #34290 made it impossible to use thread-context values to pass authentication metadata out of a realm. The SAML realm used this technique to allow the SamlAuthenticateAction to process the parsed SAML token, and apply them to the access token that was generated. This new method adds metadata to the AuthenticationResult itself, and then the authentication service makes this result available on the thread context. Closes: #34332 --- .../security/authc/AuthenticationResult.java | 30 +++++++++++++++---- .../saml/TransportSamlAuthenticateAction.java | 8 ++++- .../security/authc/AuthenticationService.java | 3 ++ .../xpack/security/authc/saml/SamlRealm.java | 5 +++- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/AuthenticationResult.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/AuthenticationResult.java index 0f073ef4ae398..355a96dd19c39 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/AuthenticationResult.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/authc/AuthenticationResult.java @@ -8,6 +8,8 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.xpack.core.security.user.User; +import java.util.Collections; +import java.util.Map; import java.util.Objects; /** @@ -21,7 +23,9 @@ * */ public final class AuthenticationResult { - private static final AuthenticationResult NOT_HANDLED = new AuthenticationResult(Status.CONTINUE, null, null, null); + private static final AuthenticationResult NOT_HANDLED = new AuthenticationResult(Status.CONTINUE, null, null, null, null); + + public static String THREAD_CONTEXT_KEY = "_xpack_security_auth_result"; public enum Status { SUCCESS, @@ -33,12 +37,15 @@ public enum Status { private final User user; private final String message; private final Exception exception; + private final Map metadata; - private AuthenticationResult(Status status, @Nullable User user, @Nullable String message, @Nullable Exception exception) { + private AuthenticationResult(Status status, @Nullable User user, @Nullable String message, @Nullable Exception exception, + @Nullable Map metadata) { this.status = status; this.user = user; this.message = message; this.exception = exception; + this.metadata = metadata == null ? Collections.emptyMap() : Collections.unmodifiableMap(metadata); } public Status getStatus() { @@ -57,6 +64,10 @@ public Exception getException() { return exception; } + public Map getMetadata() { + return metadata; + } + /** * Creates an {@code AuthenticationResult} that indicates that the supplied {@link User} * has been successfully authenticated. @@ -69,7 +80,16 @@ public Exception getException() { */ public static AuthenticationResult success(User user) { Objects.requireNonNull(user); - return new AuthenticationResult(Status.SUCCESS, user, null, null); + return success(user, null); + } + + /** + * Creates a successful result, with optional metadata + * + * @see #success(User) + */ + public static AuthenticationResult success(User user, @Nullable Map metadata) { + return new AuthenticationResult(Status.SUCCESS, user, null, null, metadata); } /** @@ -96,7 +116,7 @@ public static AuthenticationResult notHandled() { */ public static AuthenticationResult unsuccessful(String message, @Nullable Exception cause) { Objects.requireNonNull(message); - return new AuthenticationResult(Status.CONTINUE, null, message, cause); + return new AuthenticationResult(Status.CONTINUE, null, message, cause, null); } /** @@ -110,7 +130,7 @@ public static AuthenticationResult unsuccessful(String message, @Nullable Except *

*/ public static AuthenticationResult terminate(String message, @Nullable Exception cause) { - return new AuthenticationResult(Status.TERMINATE, null, message, cause); + return new AuthenticationResult(Status.TERMINATE, null, message, cause, null); } public boolean isAuthenticated() { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlAuthenticateAction.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlAuthenticateAction.java index 9dd18be510f54..13fbe248bdc48 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlAuthenticateAction.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/action/saml/TransportSamlAuthenticateAction.java @@ -22,6 +22,7 @@ import org.elasticsearch.xpack.core.security.action.saml.SamlAuthenticateRequest; import org.elasticsearch.xpack.core.security.action.saml.SamlAuthenticateResponse; import org.elasticsearch.xpack.core.security.authc.Authentication; +import org.elasticsearch.xpack.core.security.authc.AuthenticationResult; import org.elasticsearch.xpack.security.authc.AuthenticationService; import org.elasticsearch.xpack.security.authc.TokenService; import org.elasticsearch.xpack.security.authc.saml.SamlRealm; @@ -54,7 +55,12 @@ protected void doExecute(Task task, SamlAuthenticateRequest request, ActionListe Authentication originatingAuthentication = Authentication.getAuthentication(threadContext); try (ThreadContext.StoredContext ignore = threadContext.stashContext()) { authenticationService.authenticate(SamlAuthenticateAction.NAME, request, saml, ActionListener.wrap(authentication -> { - final Map tokenMeta = threadContext.getTransient(SamlRealm.CONTEXT_TOKEN_DATA); + AuthenticationResult result = threadContext.getTransient(AuthenticationResult.THREAD_CONTEXT_KEY); + if (result == null) { + listener.onFailure(new IllegalStateException("Cannot find AuthenticationResult on thread context")); + return; + } + final Map tokenMeta = (Map) result.getMetadata().get(SamlRealm.CONTEXT_TOKEN_DATA); tokenService.createUserToken(authentication, originatingAuthentication, ActionListener.wrap(tuple -> { final String tokenString = tokenService.getUserTokenString(tuple.v1()); diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java index 037ed11ac1df2..d5242fab45fac 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java @@ -140,6 +140,7 @@ class Authenticator { private RealmRef authenticatedBy = null; private RealmRef lookedupBy = null; private AuthenticationToken authenticationToken = null; + private AuthenticationResult authenticationResult = null; Authenticator(RestRequest request, ActionListener listener) { this(new AuditableRestRequest(auditTrail, failureHandler, threadContext, request), null, listener); @@ -267,6 +268,7 @@ private void consumeToken(AuthenticationToken token) { if (result.getStatus() == AuthenticationResult.Status.SUCCESS) { // user was authenticated, populate the authenticated by information authenticatedBy = new RealmRef(realm.name(), realm.type(), nodeName); + authenticationResult = result; userListener.onResponse(result.getUser()); } else { // the user was not authenticated, call this so we can audit the correct event @@ -360,6 +362,7 @@ private void consumeUser(User user, Map> message }); listener.onFailure(request.authenticationFailed(authenticationToken)); } else { + threadContext.putTransient(AuthenticationResult.THREAD_CONTEXT_KEY, authenticationResult); if (runAsEnabled) { final String runAsUsername = threadContext.getHeader(AuthenticationServiceField.RUN_AS_USER_HEADER); if (runAsUsername != null && runAsUsername.isEmpty() == false) { diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java index 36ad208df2b33..7c982e6b1b396 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/saml/SamlRealm.java @@ -426,7 +426,10 @@ private void buildUser(SamlAttributes attributes, ActionListener tokenMetadata = createTokenMetadata(attributes.name(), attributes.session()); ActionListener wrappedListener = ActionListener.wrap(auth -> { if (auth.isAuthenticated()) { - config.threadContext().putTransient(CONTEXT_TOKEN_DATA, tokenMetadata); + // Add the SAML token details as metadata on the authentication + Map metadata = new HashMap<>(auth.getMetadata()); + metadata.put(CONTEXT_TOKEN_DATA, tokenMetadata); + auth = AuthenticationResult.success(auth.getUser(), metadata); } baseListener.onResponse(auth); }, baseListener::onFailure);