From 717e63415640580a5b9ca5c5b5e02340c590b152 Mon Sep 17 00:00:00 2001 From: Gian Merlino Date: Wed, 5 Jun 2024 15:31:02 -0700 Subject: [PATCH] Router: Authorize permissionless internal requests. (#16419) * Router: Authorize permissionless internal requests. Router-internal requests like /proxy/enabled and errors for invalid requests should not require permissions, but they still need to be authorized in order to satisfy the PreResponseAuthorizationCheckFilter. This patch adds authorization checks that do not require any particular permissions. * Fix tests. --- .../AsyncManagementForwardingServlet.java | 19 ++++++++++++++++++- .../AsyncManagementForwardingServletTest.java | 10 ++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/server/src/main/java/org/apache/druid/server/AsyncManagementForwardingServlet.java b/server/src/main/java/org/apache/druid/server/AsyncManagementForwardingServlet.java index b6264d923219..4c8db118d37b 100644 --- a/server/src/main/java/org/apache/druid/server/AsyncManagementForwardingServlet.java +++ b/server/src/main/java/org/apache/druid/server/AsyncManagementForwardingServlet.java @@ -32,6 +32,8 @@ import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.server.initialization.jetty.StandardResponseHeaderFilterHolder; import org.apache.druid.server.security.AuthConfig; +import org.apache.druid.server.security.AuthorizationUtils; +import org.apache.druid.server.security.AuthorizerMapper; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.client.api.Request; import org.eclipse.jetty.client.api.Response; @@ -41,6 +43,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.util.Collections; import java.util.concurrent.TimeUnit; public class AsyncManagementForwardingServlet extends AsyncProxyServlet @@ -71,6 +74,7 @@ public class AsyncManagementForwardingServlet extends AsyncProxyServlet private final DruidHttpClientConfig httpClientConfig; private final DruidLeaderSelector coordLeaderSelector; private final DruidLeaderSelector overlordLeaderSelector; + private final AuthorizerMapper authorizerMapper; @Inject public AsyncManagementForwardingServlet( @@ -78,7 +82,8 @@ public AsyncManagementForwardingServlet( @Global Provider httpClientProvider, @Global DruidHttpClientConfig httpClientConfig, @Coordinator DruidLeaderSelector coordLeaderSelector, - @IndexingService DruidLeaderSelector overlordLeaderSelector + @IndexingService DruidLeaderSelector overlordLeaderSelector, + AuthorizerMapper authorizerMapper ) { this.jsonMapper = jsonMapper; @@ -86,6 +91,7 @@ public AsyncManagementForwardingServlet( this.httpClientConfig = httpClientConfig; this.coordLeaderSelector = coordLeaderSelector; this.overlordLeaderSelector = overlordLeaderSelector; + this.authorizerMapper = authorizerMapper; } @Override @@ -110,9 +116,11 @@ protected void service(HttpServletRequest request, HttpServletResponse response) request.getRequestURI().substring(ARBITRARY_OVERLORD_BASE_PATH.length()) ); } else if (ENABLED_PATH.equals(requestURI)) { + authorizeNoPermissionsNeeded(request); handleEnabledRequest(response); return; } else { + authorizeNoPermissionsNeeded(request); handleInvalidRequest( response, StringUtils.format("Unsupported proxy destination[%s]", request.getRequestURI()), @@ -122,6 +130,7 @@ protected void service(HttpServletRequest request, HttpServletResponse response) } if (currentLeader == null) { + authorizeNoPermissionsNeeded(request); handleInvalidRequest( response, StringUtils.format( @@ -191,6 +200,14 @@ protected void onServerResponseHeaders( super.onServerResponseHeaders(clientRequest, proxyResponse, serverResponse); } + /** + * Authorizes router-internal requests that do not require any permissions. (But do require an authenticated user.) + */ + private void authorizeNoPermissionsNeeded(HttpServletRequest request) + { + AuthorizationUtils.authorizeAllResourceActions(request, Collections.emptyList(), authorizerMapper); + } + private void handleInvalidRequest(HttpServletResponse response, String errorMessage, int statusCode) throws IOException { if (!response.isCommitted()) { diff --git a/server/src/test/java/org/apache/druid/server/AsyncManagementForwardingServletTest.java b/server/src/test/java/org/apache/druid/server/AsyncManagementForwardingServletTest.java index b730860496dd..ba7c78b99b0a 100644 --- a/server/src/test/java/org/apache/druid/server/AsyncManagementForwardingServletTest.java +++ b/server/src/test/java/org/apache/druid/server/AsyncManagementForwardingServletTest.java @@ -39,6 +39,10 @@ import org.apache.druid.server.initialization.ServerConfig; import org.apache.druid.server.initialization.jetty.JettyServerInitUtils; import org.apache.druid.server.initialization.jetty.JettyServerInitializer; +import org.apache.druid.server.security.AllowAllAuthenticator; +import org.apache.druid.server.security.AllowAllAuthorizer; +import org.apache.druid.server.security.AuthenticationUtils; +import org.apache.druid.server.security.AuthorizerMapper; import org.eclipse.jetty.client.HttpClient; import org.eclipse.jetty.server.Handler; import org.eclipse.jetty.server.Server; @@ -321,7 +325,7 @@ public void testOverlordProxyLeader() throws Exception } @Test - public void testProxyEnebledCheck() throws Exception + public void testProxyEnabledCheck() throws Exception { HttpURLConnection connection = ((HttpURLConnection) new URL(StringUtils.format("http://localhost:%d/proxy/enabled", port)).openConnection()); @@ -491,7 +495,8 @@ public String getCurrentLeader() injector.getProvider(HttpClient.class), injector.getInstance(DruidHttpClientConfig.class), coordinatorLeaderSelector, - overlordLeaderSelector + overlordLeaderSelector, + new AuthorizerMapper(ImmutableMap.of("allowAll", new AllowAllAuthorizer())) ) ); @@ -502,6 +507,7 @@ public String getCurrentLeader() root.addServlet(holder, "/druid/indexer/*"); root.addServlet(holder, "/proxy/*"); + AuthenticationUtils.addAuthenticationFilterChain(root, ImmutableList.of(new AllowAllAuthenticator())); JettyServerInitUtils.addExtensionFilters(root, injector); final HandlerList handlerList = new HandlerList();