Remove RequestContext from Security (elastic/elasticsearch#4710)

RequestContext is a leftover from when we had no thread context. This
commit removes the last place where it was used and uses the thread context
instead.

Original commit: elastic/x-pack-elasticsearch@50a2bff400

elasticsearch/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -176,6 +176,7 @@ public class Security implements ActionPlugin, IngestPlugin, NetworkPlugin {
     private final SetOnce<IPFilter> ipFilter = new SetOnce<>();
     private final SetOnce<AuthenticationService> authcService = new SetOnce<>();
     private final SetOnce<SecurityContext> securityContext = new SetOnce<>();
+    private final SetOnce<ThreadContext> threadContext = new SetOnce<>();
 
     public Security(Settings settings, Environment env, XPackLicenseState licenseState, SSLService sslService) throws IOException {
         this.settings = settings;
@@ -246,7 +247,7 @@ public class Security implements ActionPlugin, IngestPlugin, NetworkPlugin {
         if (enabled == false) {
             return Collections.emptyList();
         }
-
+        threadContext.set(threadPool.getThreadContext());
         List<Object> components = new ArrayList<>();
         securityContext.set(new SecurityContext(settings, threadPool.getThreadContext(), cryptoService));
         components.add(securityContext.get());
@@ -472,7 +473,7 @@ public class Security implements ActionPlugin, IngestPlugin, NetworkPlugin {
              *  This impl. disabled the query cache if field level security is used for a particular request. If we wouldn't do
              *  forcefully overwrite the query cache implementation then we leave the system vulnerable to leakages of data to
              *  unauthorized users. */
-            module.forceQueryCacheProvider(OptOutQueryCache::new);
+            module.forceQueryCacheProvider((settings, cache) -> new OptOutQueryCache(settings, cache, threadContext.get()));
         }
     }
 

elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/OptOutQueryCache.java

@@ -8,15 +8,15 @@ package org.elasticsearch.xpack.security.authz.accesscontrol;
 import org.apache.lucene.search.QueryCachingPolicy;
 import org.apache.lucene.search.Weight;
 import org.elasticsearch.ElasticsearchException;
-import org.elasticsearch.action.support.broadcast.BroadcastShardRequest;
+import org.elasticsearch.common.util.concurrent.ThreadContext;
 import org.elasticsearch.index.AbstractIndexComponent;
 import org.elasticsearch.index.IndexSettings;
 import org.elasticsearch.index.cache.query.QueryCache;
 import org.elasticsearch.indices.IndicesQueryCache;
-import org.elasticsearch.search.internal.ShardSearchRequest;
 import org.elasticsearch.xpack.security.authz.AuthorizationService;
 
 import java.util.HashSet;
+import java.util.Objects;
 import java.util.Set;
 
 /**
@@ -25,11 +25,15 @@ import java.util.Set;
  */
 public final class OptOutQueryCache extends AbstractIndexComponent implements QueryCache {
 
-    final IndicesQueryCache indicesQueryCache;
+    private final IndicesQueryCache indicesQueryCache;
+    private final ThreadContext context;
+    private final String indexName;
 
-    public OptOutQueryCache(IndexSettings indexSettings, IndicesQueryCache indicesQueryCache) {
+    public OptOutQueryCache(IndexSettings indexSettings, IndicesQueryCache indicesQueryCache, ThreadContext context) {
         super(indexSettings);
         this.indicesQueryCache = indicesQueryCache;
+        this.context = Objects.requireNonNull(context, "threadContext must not be null");
+        this.indexName = indexSettings.getIndex().getName();
     }
 
     @Override
@@ -45,27 +49,13 @@ public final class OptOutQueryCache extends AbstractIndexComponent implements Qu
 
     @Override
     public Weight doCache(Weight weight, QueryCachingPolicy policy) {
-        final RequestContext context = RequestContext.current();
+        IndicesAccessControl indicesAccessControl = context.getTransient(
-        if (context == null) {
-            throw new IllegalStateException("opting out of the query cache. current request can't be found");
-        }
-        IndicesAccessControl indicesAccessControl = context.getThreadContext().getTransient(
                 AuthorizationService.INDICES_PERMISSIONS_KEY);
         if (indicesAccessControl == null) {
             logger.debug("opting out of the query cache. current request doesn't hold indices permissions");
             return weight;
         }
 
-        // At this level only IndicesRequest
-        final String indexName;
-        if (context.getRequest() instanceof ShardSearchRequest) {
-            indexName = ((ShardSearchRequest) context.getRequest()).shardId().getIndexName();
-        } else if (context.getRequest() instanceof BroadcastShardRequest) {
-            indexName = ((BroadcastShardRequest) context.getRequest()).shardId().getIndexName();
-        } else {
-            return weight;
-        }
-
         IndicesAccessControl.IndexAccessControl indexAccessControl = indicesAccessControl.getIndexPermissions(indexName);
         if (indexAccessControl != null && indexAccessControl.getFieldPermissions().hasFieldLevelSecurity()) {
             if (cachingIsSafe(weight, indexAccessControl)) {

elasticsearch/src/main/java/org/elasticsearch/xpack/security/authz/accesscontrol/RequestContext.java

@@ -1,61 +0,0 @@
-/*
- * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
- * or more contributor license agreements. Licensed under the Elastic License;
- * you may not use this file except in compliance with the Elastic License.
- */
-package org.elasticsearch.xpack.security.authz.accesscontrol;
-
-import org.elasticsearch.common.util.concurrent.ThreadContext;
-import org.elasticsearch.transport.TransportRequest;
-
-import java.util.Objects;
-
-/**
- * A thread local based holder of the currnet {@link TransportRequest} instance.
- */
-public final class RequestContext {
-
-    // Need thread local to make the current transport request available to places in the code that
-    // don't have direct access to the current transport request
-    private static final ThreadLocal<RequestContext> current = new ThreadLocal<>();
-
-    /**
-     * If set then this returns the current {@link RequestContext} with the current {@link TransportRequest}.
-     */
-    public static RequestContext current() {
-        return current.get();
-    }
-
-    /**
-     * Invoked by the transport service to set the current transport request in the thread local
-     */
-    public static void setCurrent(RequestContext value) {
-        current.set(value);
-    }
-
-    /**
-     * Invoked by the transport service to remove the current request from the thread local
-     */
-    public static void removeCurrent() {
-        current.remove();
-    }
-
-    private final ThreadContext threadContext;
-    private final TransportRequest request;
-
-    public RequestContext(TransportRequest request, ThreadContext threadContext) {
-        this.request = Objects.requireNonNull(request);
-        this.threadContext = Objects.requireNonNull(threadContext);
-    }
-
-    /**
-     * @return current {@link TransportRequest}
-     */
-    public TransportRequest getRequest() {
-        return request;
-    }
-
-    public ThreadContext getThreadContext() {
-        return threadContext;
-    }
-}

elasticsearch/src/main/java/org/elasticsearch/xpack/security/transport/SecurityServerTransportInterceptor.java

@@ -33,7 +33,6 @@ import org.elasticsearch.xpack.security.SecurityContext;
 import org.elasticsearch.xpack.security.authc.AuthenticationService;
 import org.elasticsearch.xpack.security.authz.AuthorizationService;
 import org.elasticsearch.xpack.security.authz.AuthorizationUtils;
-import org.elasticsearch.xpack.security.authz.accesscontrol.RequestContext;
 import org.elasticsearch.xpack.security.transport.netty4.SecurityNetty4Transport;
 import org.elasticsearch.xpack.security.user.KibanaUser;
 import org.elasticsearch.xpack.security.user.SystemUser;
@@ -217,15 +216,7 @@ public class SecurityServerTransportInterceptor extends AbstractComponent implem
 
                 @Override
                 protected void doRun() throws Exception {
-                    // FIXME we should remove the RequestContext completely since we have ThreadContext but cannot yet due to
+                    handler.messageReceived(request, channel, task);
-                    // the query cache
-                    RequestContext context = new RequestContext(request, threadContext);
-                    RequestContext.setCurrent(context);
-                    try {
-                        handler.messageReceived(request, channel, task);
-                    } finally {
-                        RequestContext.removeCurrent();
-                    }
                 }
             };
         }