From a76a6b4e54aaa7266796c44d58b38b2082f68ea3 Mon Sep 17 00:00:00 2001 From: Ryan Ernst Date: Mon, 18 Jul 2016 17:00:55 -0700 Subject: [PATCH] Internal: Simplify SecurityContext dependencies Currently the security context is an object passed around to code needing to check the user for the current request. Like recent InternalClient changes, it current depends on the AuthenticationService, but can be simplified by only knowing about the thread context and crypto service. This change makes SecurityContext a class, instead of an interface, and removes the dependency on AuthenticationService. Original commit: elastic/x-pack-elasticsearch@b8af75e8cbfa4a665da3f1527985fd7ec8fd2ff6 --- .../xpack/security/Security.java | 2 + .../xpack/security/SecurityContext.java | 134 ++++++------------ .../xpack/security/SecurityModule.java | 4 - .../action/filter/SecurityActionFilter.java | 2 +- .../security/authc/AuthenticationService.java | 2 - .../authc/InternalAuthenticationService.java | 11 -- 6 files changed, 44 insertions(+), 111 deletions(-) diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 343c392ec35..9e988294667 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -245,6 +245,8 @@ public class Security implements ActionPlugin { return Collections.emptyList(); } List components = new ArrayList<>(); + final SecurityContext securityContext = new SecurityContext(settings, threadPool, cryptoService); + components.add(securityContext); final SSLConfiguration.Global globalSslConfig = new SSLConfiguration.Global(settings); final ClientSSLService clientSSLService = new ClientSSLService(settings, env, globalSslConfig, resourceWatcherService); diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityContext.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityContext.java index 090cdebc455..6b5ebcb2cc6 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityContext.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityContext.java @@ -5,110 +5,58 @@ */ package org.elasticsearch.xpack.security; -import org.elasticsearch.ElasticsearchException; -import org.elasticsearch.common.inject.Inject; -import org.elasticsearch.common.util.concurrent.ThreadContext; -import org.elasticsearch.xpack.security.authc.Authentication; -import org.elasticsearch.xpack.security.authc.AuthenticationService; -import org.elasticsearch.xpack.security.user.User; -import org.elasticsearch.threadpool.ThreadPool; - import java.io.IOException; -import java.util.concurrent.Callable; + +import org.elasticsearch.common.logging.ESLogger; +import org.elasticsearch.common.logging.Loggers; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.common.util.concurrent.ThreadContext; +import org.elasticsearch.threadpool.ThreadPool; +import org.elasticsearch.xpack.security.authc.Authentication; +import org.elasticsearch.xpack.security.authc.InternalAuthenticationService; +import org.elasticsearch.xpack.security.crypto.CryptoService; +import org.elasticsearch.xpack.security.user.User; /** - * + * A lightweight utility that can find the current user and authentication information for the local thread. */ -public interface SecurityContext { +public class SecurityContext { - void executeAs(User user, Runnable runnable); + private final ESLogger logger; + private final ThreadContext threadContext; + private final CryptoService cryptoService; + private final boolean signUserHeader; - V executeAs(User user, Callable callable); - - User getUser(); - - Authentication getAuthentication(); - - default boolean hasAuthentication() { - return getAuthentication() != null; + /** + * Creates a new security context. + * If cryptoService is null, security is disabled and {@link #getUser()} + * and {@link #getAuthentication()} will always return null. + */ + public SecurityContext(Settings settings, ThreadPool threadPool, CryptoService cryptoService) { + this.logger = Loggers.getLogger(getClass(), settings); + this.threadContext = threadPool.getThreadContext(); + this.cryptoService = cryptoService; + this.signUserHeader = InternalAuthenticationService.SIGN_USER_HEADER.get(settings); } - class Insecure implements SecurityContext { - - public static final Insecure INSTANCE = new Insecure(); - - private Insecure() { - } - - @Override - public void executeAs(User user, Runnable runnable) { - runnable.run(); - } - - @Override - public V executeAs(User user, Callable callable) { - try { - return callable.call(); - } catch (Exception e) { - throw new ElasticsearchException(e); - } - } - - @Override - public User getUser() { - return null; - } - - @Override - public Authentication getAuthentication() { - return null; - } + /** Returns the current user information, or null if the current request has no authentication info. */ + public User getUser() { + Authentication authentication = getAuthentication(); + return authentication == null ? null : authentication.getUser(); } - class Secure implements SecurityContext { - - private final ThreadContext threadContext; - private final AuthenticationService authcService; - - @Inject - public Secure(ThreadPool threadPool, AuthenticationService authcService) { - this.threadContext = threadPool.getThreadContext(); - this.authcService = authcService; + /** Returns the authentication information, or null if the current request has no authentication info. */ + public Authentication getAuthentication() { + if (cryptoService == null) { + return null; } - - public void executeAs(User user, Runnable runnable) { - try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { - setUser(user); - runnable.run(); - } - } - - public V executeAs(User user, Callable callable) { - try (ThreadContext.StoredContext ctx = threadContext.stashContext()) { - setUser(user); - return callable.call(); - } catch (Exception e) { - throw new ElasticsearchException(e); - } - } - - @Override - public User getUser() { - Authentication authentication = authcService.getCurrentAuthentication(); - return authentication == null ? null : authentication.getUser(); - } - - @Override - public Authentication getAuthentication() { - return authcService.getCurrentAuthentication(); - } - - private void setUser(User user) { - try { - authcService.attachUserIfMissing(user); - } catch (IOException | IllegalArgumentException e) { - throw new ElasticsearchException("failed to attach user to request", e); - } + try { + return Authentication.readFromContext(threadContext, cryptoService, signUserHeader); + } catch (IOException e) { + // TODO: this seems bogus, the only way to get an ioexception here is from a corrupt or tampered + // auth header, which should be be audited? + logger.error("failed to read authentication", e); + return null; } } } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityModule.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityModule.java index df37eda635e..1ba6ff33542 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityModule.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/SecurityModule.java @@ -27,11 +27,7 @@ public class SecurityModule extends AbstractSecurityModule { XPackPlugin.bindFeatureSet(binder(), SecurityFeatureSet.class); if (securityEnabled) { - bind(SecurityContext.Secure.class).asEagerSingleton(); - bind(SecurityContext.class).to(SecurityContext.Secure.class); bind(SecurityLifecycleService.class).asEagerSingleton(); - } else { - bind(SecurityContext.class).toInstance(SecurityContext.Insecure.INSTANCE); } } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java index 74e8bdda4c1..0b562dd2a69 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/action/filter/SecurityActionFilter.java @@ -92,7 +92,7 @@ public class SecurityActionFilter extends AbstractComponent implements ActionFil // only restore the context if it is not empty. This is needed because sometimes a response is sent to the user // and then a cleanup action is executed (like for search without a scroll) final ThreadContext.StoredContext original = threadContext.newStoredContext(); - final boolean restoreOriginalContext = securityContext.hasAuthentication(); + final boolean restoreOriginalContext = securityContext.getAuthentication() != null; try { if (licenseState.authenticationAndAuthorizationEnabled()) { if (AuthorizationUtils.shouldReplaceUserWithSystem(threadContext, action)) { diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java index bab60b48076..ec6d2e49e0b 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/AuthenticationService.java @@ -58,6 +58,4 @@ public interface AuthenticationService { * @param user The user to be attached if the header is missing */ void attachUserIfMissing(User user) throws IOException, IllegalArgumentException; - - Authentication getCurrentAuthentication(); } diff --git a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/InternalAuthenticationService.java b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/InternalAuthenticationService.java index 67c3e379722..56f53c7bf70 100644 --- a/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/InternalAuthenticationService.java +++ b/elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/authc/InternalAuthenticationService.java @@ -82,17 +82,6 @@ public class InternalAuthenticationService extends AbstractComponent implements authentication.writeToContextIfMissing(threadContext, cryptoService, signUserHeader); } - @Override - public Authentication getCurrentAuthentication() { - try { - Authentication authentication = Authentication.readFromContext(threadContext, cryptoService, signUserHeader); - return authentication == null ? null : authentication; - } catch (IOException e) { - logger.error("failed to read authentication", e); - return null; - } - } - Authenticator createAuthenticator(RestRequest request) { return new Authenticator(request); }