Parameterize the Security plugin with the request-wise thread context supplier
#95040
Conversation
| @@ -510,7 +501,6 @@ private static void sendContentTypeErrorMessage(@Nullable List<String> contentTy | |||
|
|
|||
| private void tryAllHandlers(final RestRequest request, final RestChannel channel, final ThreadContext threadContext) throws Exception { | |||
| try { | |||
| copyRestHeaders(request, threadContext); | |||
There was a problem hiding this comment.
The RestController is not responsible to set (part of) the thread context anymore, mainly because the Security plugin cannot hook-in at this point. Instead, the Security plugin wraps the RestController before passing it to the Netty4HttpServerTransport that it builds (Security builds a HttpServerTransport in order to implement TLS).
| populateClientCertificate.accept(restRequest.getHttpChannel(), threadContext); | ||
| RemoteHostHeader.process(restRequest, threadContext); |
There was a problem hiding this comment.
This is the additional thread context that Security constructs, brought together in this single place. This context must be set before AuthenticationService#authenticate is attempted.
| if (extractClientCertificate) { | ||
| HttpChannel httpChannel = request.getHttpChannel(); | ||
| SSLEngineUtils.extractClientCertificates(logger, threadContext, httpChannel); | ||
| } | ||
|
|
||
| final RestRequest wrappedRequest = maybeWrapRestRequest(request); | ||
| RemoteHostHeader.process(request, threadContext); |
There was a problem hiding this comment.
These things moved.
| HttpServerTransport.Dispatcher dispatcherWithContext = HttpServerTransport.Dispatcher.dispatchWithThreadContextWrapper( | ||
| dispatcher, | ||
| (httpPreRequest -> dispatcherContext.accept(httpPreRequest, threadPool.getThreadContext())) | ||
| ); |
There was a problem hiding this comment.
Previously, the RestController was in charge to build the thread context from the request headers.
Now, the dispatcherContext does that, and it is exposed by the ActionModule, to be used by the plugins that build HttpServerTransport implementations (which take the RestController as the argument).
albertzaharovits
changed the title
Security plugin with the request-wise thread context supplier
albertzaharovits
added
:Security/Authentication
| @@ -1592,6 +1596,7 @@ public Map<String, Supplier<HttpServerTransport>> getHttpTransports( | |||
| NamedXContentRegistry xContentRegistry, | |||
| NetworkService networkService, | |||
| HttpServerTransport.Dispatcher dispatcher, | |||
| BiConsumer<HttpPreRequest, ThreadContext> dispatcherContext, | |||
There was a problem hiding this comment.
This is important. The Security plugin needs access to this supplier of thread context.
|
Pinging @elastic/es-security (Team:Security) |
server/src/main/java/org/elasticsearch/action/ActionModule.java
Outdated
Show resolved
Hide resolved
modules/transport-netty4/src/main/java/org/elasticsearch/transport/netty4/Netty4Plugin.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java
Outdated
Show resolved
Hide resolved
server/src/main/java/org/elasticsearch/rest/RestController.java
Outdated
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java
Show resolved
Hide resolved
server/src/test/java/org/elasticsearch/http/AbstractHttpServerTransportTests.java
Show resolved
Hide resolved
|
Thank you for the speedy reviews @jakelandis ! |
…xt supplier (elastic#95040) When `Security` authenticates a request (AuthenticationService#authenticate) it mostly looks in the thread context for all the request details that it needs. Consequently, there's code that populates the thread context before invoking authentication. This PR brings all the code that's responsible for populating the said thread context, as a parameter to the `Security` plugin. This is required in the follow-up that is going to involve setting up the thread context (given the aforementioned parameter), and running the authentication under that context, before the request is dispatched (rather than "while" it's being dispatched, currently).
When
Securityauthenticates a request (AuthenticationService#authenticate) it mostly looks in the thread context for all the request details that it needs.Consequently, there's code that populates the thread context before running authentication.
This PR brings all the code that's responsible for populating the said thread context, as a parameter to the
Securityplugin.This is required in the follow-up that is going to involve setting up the thread context (given the aforementioned parameter), and running the authentication under that context, before the request is dispatched (rather than "while" it's being dispatched, currently).