Merge pull request elastic/elasticsearch#2860 from rjernst/deguice12

Internal: Simplify SecurityContext dependencies

Original commit: elastic/x-pack-elasticsearch@74d0036e80

elasticsearch/x-pack/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -248,6 +248,8 @@ public class Security implements ActionPlugin, IngestPlugin {
             return Collections.emptyList();
         }
         List<Object> 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);

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);
-
-    <V> V executeAs(User user, Callable<V> callable);
-
-    User getUser();
-
-    Authentication getAuthentication();
-
-    default boolean hasAuthentication() {
-        return getAuthentication() != null;
-    }
-
-    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> V executeAs(User user, Callable<V> callable) {
-            try {
-                return callable.call();
-            } catch (Exception e) {
-                throw new ElasticsearchException(e);
-            }
-        }
-
-        @Override
-        public User getUser() {
-            return null;
-        }
-
-        @Override
-        public Authentication getAuthentication() {
-            return null;
-        }
-    }
-
-    class Secure implements SecurityContext {
 
+    private final ESLogger logger;
     private final ThreadContext threadContext;
-        private final AuthenticationService authcService;
+    private final CryptoService cryptoService;
+    private final boolean signUserHeader;
 
-        @Inject
+    /**
-        public Secure(ThreadPool threadPool, AuthenticationService authcService) {
+     * 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.authcService = authcService;
+        this.cryptoService = cryptoService;
+        this.signUserHeader = InternalAuthenticationService.SIGN_USER_HEADER.get(settings);
     }
 
-        public void executeAs(User user, Runnable runnable) {
+    /** Returns the current user information, or null if the current request has no authentication info. */
-            try (ThreadContext.StoredContext ctx = threadContext.stashContext()) {
-                setUser(user);
-                runnable.run();
-            }
-        }
-
-        public <V> V executeAs(User user, Callable<V> 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();
+        Authentication authentication = getAuthentication();
         return authentication == null ? null : authentication.getUser();
     }
 
-        @Override
+    /** Returns the authentication information, or null if the current request has no authentication info. */
     public Authentication getAuthentication() {
-            return authcService.getCurrentAuthentication();
+        if (cryptoService == null) {
+            return null;
         }
-
-        private void setUser(User user) {
         try {
-                authcService.attachUserIfMissing(user);
+            return Authentication.readFromContext(threadContext, cryptoService, signUserHeader);
-            } catch (IOException | IllegalArgumentException e) {
+        } catch (IOException e) {
-                throw new ElasticsearchException("failed to attach user to request", 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;
         }
     }
 }

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);
         }
     }
 

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)) {

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();
 }

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);
     }