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 b6264d92321..4c8db118d37 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.guice.http.DruidHttpClientConfig; 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.ServletException; 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 class AsyncManagementForwardingServlet extends AsyncProxyServlet @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 class AsyncManagementForwardingServlet extends AsyncProxyServlet this.httpClientConfig = httpClientConfig; this.coordLeaderSelector = coordLeaderSelector; this.overlordLeaderSelector = overlordLeaderSelector; + this.authorizerMapper = authorizerMapper; } @Override @@ -110,9 +116,11 @@ public class AsyncManagementForwardingServlet extends AsyncProxyServlet 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 @@ public class AsyncManagementForwardingServlet extends AsyncProxyServlet } if (currentLeader == null) { + authorizeNoPermissionsNeeded(request); handleInvalidRequest( response, StringUtils.format( @@ -191,6 +200,14 @@ public class AsyncManagementForwardingServlet extends AsyncProxyServlet 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 b730860496d..ba7c78b99b0 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.BaseJettyTest; 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 class AsyncManagementForwardingServletTest extends BaseJettyTest } @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 class AsyncManagementForwardingServletTest extends BaseJettyTest injector.getProvider(HttpClient.class), injector.getInstance(DruidHttpClientConfig.class), coordinatorLeaderSelector, - overlordLeaderSelector + overlordLeaderSelector, + new AuthorizerMapper(ImmutableMap.of("allowAll", new AllowAllAuthorizer())) ) ); @@ -502,6 +507,7 @@ public class AsyncManagementForwardingServletTest extends BaseJettyTest 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();