set DRUID_AUTHORIZATION_CHECKED attribute for router endpoints (#8026)

* add state resource filter to router endpoints * add RouterResource to ResourceFilter test framework
apache · Jul 9, 2019 · 027291a · 027291a
1 parent 12f1267
commit 027291a
Show file tree

Hide file tree

Showing 9 changed files with 30 additions and 23 deletions.
diff --git a/server/src/main/java/org/apache/druid/server/http/RouterResource.java b/server/src/main/java/org/apache/druid/server/http/RouterResource.java
@@ -20,7 +20,9 @@
 package org.apache.druid.server.http;
 
 import com.google.inject.Inject;
+import com.sun.jersey.spi.container.ResourceFilters;
 import org.apache.druid.client.selector.Server;
+import org.apache.druid.server.http.security.StateResourceFilter;
 import org.apache.druid.server.router.TieredBrokerHostSelector;
 
 import javax.ws.rs.GET;
@@ -47,6 +49,7 @@ public RouterResource(TieredBrokerHostSelector tieredBrokerHostSelector)
 
   @GET
   @Path("/brokers")
+  @ResourceFilters(StateResourceFilter.class)
   @Produces(MediaType.APPLICATION_JSON)
   public Map<String, List<String>> getBrokers()
   {

diff --git a/server/src/main/java/org/apache/druid/server/security/AuthenticationUtils.java b/server/src/main/java/org/apache/druid/server/security/AuthenticationUtils.java
@@ -57,7 +57,7 @@ public static void addAuthenticationFilterChain(
     }
   }
 
-  public static void addNoopAuthorizationFilters(ServletContextHandler root, List<String> unsecuredPaths)
+  public static void addNoopAuthenticationAndAuthorizationFilters(ServletContextHandler root, List<String> unsecuredPaths)
   {
     for (String unsecuredPath : unsecuredPaths) {
       root.addFilter(new FilterHolder(new UnsecuredResourceFilter()), unsecuredPath, null);

diff --git a/server/src/main/java/org/apache/druid/server/security/UnsecuredResourceFilter.java b/server/src/main/java/org/apache/druid/server/security/UnsecuredResourceFilter.java
@@ -47,9 +47,13 @@ public void doFilter(ServletRequest servletRequest, ServletResponse servletRespo
     // but the value doesn't matter since we skip authorization checks for requests that go through this filter
     servletRequest.setAttribute(
         AuthConfig.DRUID_AUTHENTICATION_RESULT,
-        new AuthenticationResult(AuthConfig.ALLOW_ALL_NAME, AuthConfig.ALLOW_ALL_NAME, AuthConfig.ALLOW_ALL_NAME, null)
+        new AuthenticationResult(
+            AuthConfig.ALLOW_ALL_NAME,
+            AuthConfig.ALLOW_ALL_NAME,
+            AuthConfig.ALLOW_ALL_NAME,
+            null
+        )
     );
-
     // This request will not go to an Authorizer, so we need to set this for PreResponseAuthorizationCheckFilter
     servletRequest.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true);
     servletRequest.setAttribute(AuthConfig.DRUID_ALLOW_UNSECURED_PATH, true);

diff --git a/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java b/server/src/test/java/org/apache/druid/server/http/security/SecurityResourceFilterTest.java
@@ -34,6 +34,7 @@
 import org.apache.druid.server.http.HistoricalResource;
 import org.apache.druid.server.http.IntervalsResource;
 import org.apache.druid.server.http.MetadataResource;
+import org.apache.druid.server.http.RouterResource;
 import org.apache.druid.server.http.RulesResource;
 import org.apache.druid.server.http.ServersResource;
 import org.apache.druid.server.http.TiersResource;
@@ -46,14 +47,12 @@
 import org.junit.runners.Parameterized;
 
 import java.util.Collection;
-import java.util.regex.Pattern;
 
 @RunWith(Parameterized.class)
 public class SecurityResourceFilterTest extends ResourceFilterTestHelper
 {
-  private static final Pattern WORD = Pattern.compile("\\w+");
 
-  @Parameterized.Parameters
+  @Parameterized.Parameters(name = "{index}: requestPath={0}, requestMethod={1}, resourceFilter={2}")
   public static Collection<Object[]> data()
   {
     return ImmutableList.copyOf(
@@ -71,7 +70,8 @@ public static Collection<Object[]> data()
             getRequestPathsWithAuthorizer(CoordinatorDynamicConfigsResource.class),
             getRequestPathsWithAuthorizer(QueryResource.class),
             getRequestPathsWithAuthorizer(StatusResource.class),
-            getRequestPathsWithAuthorizer(BrokerQueryResource.class)
+            getRequestPathsWithAuthorizer(BrokerQueryResource.class),
+            getRequestPathsWithAuthorizer(RouterResource.class)
         )
     );
   }

diff --git a/services/src/main/java/org/apache/druid/cli/CliOverlord.java b/services/src/main/java/org/apache/druid/cli/CliOverlord.java
@@ -379,9 +379,9 @@ public void initialize(Server server, Injector injector)
 
       AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper);
 
-      // perform no-op authorization for these resources
-      AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS);
-      AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths());
+      // perform no-op authorization/authentication for these resources
+      AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, UNSECURED_PATHS);
+      AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, authConfig.getUnsecuredPaths());
 
       final List<Authenticator> authenticators = authenticatorMapper.getAuthenticatorChain();
       AuthenticationUtils.addAuthenticationFilterChain(root, authenticators);

diff --git a/services/src/main/java/org/apache/druid/cli/CoordinatorJettyServerInitializer.java b/services/src/main/java/org/apache/druid/cli/CoordinatorJettyServerInitializer.java
@@ -101,12 +101,12 @@ public void initialize(Server server, Injector injector)
 
     AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper);
 
-    // perform no-op authorization for these resources
-    AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS);
-    AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths());
+    // perform no-op authorization/authentication for these resources
+    AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, UNSECURED_PATHS);
+    AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, authConfig.getUnsecuredPaths());
 
     if (beOverlord) {
-      AuthenticationUtils.addNoopAuthorizationFilters(root, CliOverlord.UNSECURED_PATHS);
+      AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, CliOverlord.UNSECURED_PATHS);
     }
 
     List<Authenticator> authenticators = authenticatorMapper.getAuthenticatorChain();

diff --git a/services/src/main/java/org/apache/druid/cli/MiddleManagerJettyServerInitializer.java b/services/src/main/java/org/apache/druid/cli/MiddleManagerJettyServerInitializer.java
@@ -74,9 +74,9 @@ public void initialize(Server server, Injector injector)
 
     AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper);
 
-    // perform no-op authorization for these resources
-    AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS);
-    AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths());
+    // perform no-op authorization/authentication for these resources
+    AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, UNSECURED_PATHS);
+    AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, authConfig.getUnsecuredPaths());
 
     final List<Authenticator> authenticators = authenticatorMapper.getAuthenticatorChain();
     AuthenticationUtils.addAuthenticationFilterChain(root, authenticators);

diff --git a/services/src/main/java/org/apache/druid/cli/QueryJettyServerInitializer.java b/services/src/main/java/org/apache/druid/cli/QueryJettyServerInitializer.java
@@ -96,8 +96,8 @@ public void initialize(Server server, Injector injector)
     AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper);
 
     // perform no-op authorization for these resources
-    AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS);
-    AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths());
+    AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, UNSECURED_PATHS);
+    AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, authConfig.getUnsecuredPaths());
 
     List<Authenticator> authenticators = authenticatorMapper.getAuthenticatorChain();
     AuthenticationUtils.addAuthenticationFilterChain(root, authenticators);

diff --git a/services/src/main/java/org/apache/druid/cli/RouterJettyServerInitializer.java b/services/src/main/java/org/apache/druid/cli/RouterJettyServerInitializer.java
@@ -138,12 +138,12 @@ public void initialize(Server server, Injector injector)
 
     AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper);
 
-    // perform no-op authorization for these resources
-    AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS);
+    // perform no-op authorization/authentication for these resources
+    AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, UNSECURED_PATHS);
     if (managementProxyConfig.isEnabled()) {
-      AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS_FOR_UI);
+      AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, UNSECURED_PATHS_FOR_UI);
     }
-    AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths());
+    AuthenticationUtils.addNoopAuthenticationAndAuthorizationFilters(root, authConfig.getUnsecuredPaths());
 
     final List<Authenticator> authenticators = authenticatorMapper.getAuthenticatorChain();
     AuthenticationUtils.addAuthenticationFilterChain(root, authenticators);