From af44d1142bab259d5011a331df77c8e632c79d46 Mon Sep 17 00:00:00 2001 From: Jonathan Wei Date: Wed, 15 Nov 2017 12:41:30 -0800 Subject: [PATCH] Add unsecured /health endpoint, remove auth checks from isLeader (#5087) * Add unsecured /health endpoint, remove auth checks from isLeader * PR comments --- .../indexing/overlord/http/OverlordResource.java | 4 +++- .../main/java/io/druid/server/StatusResource.java | 14 +++++++++++++- .../io/druid/server/http/CoordinatorResource.java | 7 ++++++- .../server/security/UnsecuredResourceFilter.java | 12 ++++++++++++ .../src/main/java/io/druid/cli/CliOverlord.java | 10 +++++++--- .../cli/CoordinatorJettyServerInitializer.java | 10 +++++++--- .../cli/MiddleManagerJettyServerInitializer.java | 9 +++++++++ .../io/druid/cli/QueryJettyServerInitializer.java | 8 ++++++++ .../io/druid/cli/RouterJettyServerInitializer.java | 9 +++++++++ 9 files changed, 74 insertions(+), 9 deletions(-) diff --git a/indexing-service/src/main/java/io/druid/indexing/overlord/http/OverlordResource.java b/indexing-service/src/main/java/io/druid/indexing/overlord/http/OverlordResource.java index 96872eee4af..bf905677f29 100644 --- a/indexing-service/src/main/java/io/druid/indexing/overlord/http/OverlordResource.java +++ b/indexing-service/src/main/java/io/druid/indexing/overlord/http/OverlordResource.java @@ -179,9 +179,11 @@ public class OverlordResource return Response.ok(taskMaster.getCurrentLeader()).build(); } + /** + * This is an unsecured endpoint, defined as such in UNSECURED_PATHS in CliOverlord + */ @GET @Path("/isLeader") - @ResourceFilters(StateResourceFilter.class) @Produces(MediaType.APPLICATION_JSON) public Response isLeader() { diff --git a/server/src/main/java/io/druid/server/StatusResource.java b/server/src/main/java/io/druid/server/StatusResource.java index 77e00844687..432e5b58a59 100644 --- a/server/src/main/java/io/druid/server/StatusResource.java +++ b/server/src/main/java/io/druid/server/StatusResource.java @@ -38,16 +38,28 @@ import java.util.List; /** */ @Path("/status") -@ResourceFilters(StateResourceFilter.class) public class StatusResource { @GET + @ResourceFilters(StateResourceFilter.class) @Produces(MediaType.APPLICATION_JSON) public Status doGet() { return new Status(Initialization.getLoadedImplementations(DruidModule.class)); } + /** + * This is an unsecured endpoint, defined as such in UNSECURED_PATHS in the service initiailization files + * (e.g. CliOverlord, CoordinatorJettyServerInitializer) + */ + @GET + @Path("/health") + @Produces(MediaType.APPLICATION_JSON) + public boolean getHealth() + { + return true; + } + public static class Status { final String version; diff --git a/server/src/main/java/io/druid/server/http/CoordinatorResource.java b/server/src/main/java/io/druid/server/http/CoordinatorResource.java index eea70723635..45b6767793f 100644 --- a/server/src/main/java/io/druid/server/http/CoordinatorResource.java +++ b/server/src/main/java/io/druid/server/http/CoordinatorResource.java @@ -41,7 +41,6 @@ import java.util.Map; /** */ @Path("/druid/coordinator/v1") -@ResourceFilters(StateResourceFilter.class) public class CoordinatorResource { private final DruidCoordinator coordinator; @@ -56,12 +55,16 @@ public class CoordinatorResource @GET @Path("/leader") + @ResourceFilters(StateResourceFilter.class) @Produces(MediaType.APPLICATION_JSON) public Response getLeader() { return Response.ok(coordinator.getCurrentLeader()).build(); } + /** + * This is an unsecured endpoint, defined as such in UNSECURED_PATHS in CoordinatorJettyServerInitializer + */ @GET @Path("/isLeader") @Produces(MediaType.APPLICATION_JSON) @@ -78,6 +81,7 @@ public class CoordinatorResource @GET @Path("/loadstatus") + @ResourceFilters(StateResourceFilter.class) @Produces(MediaType.APPLICATION_JSON) public Response getLoadStatus( @QueryParam("simple") String simple, @@ -96,6 +100,7 @@ public class CoordinatorResource @GET @Path("/loadqueue") + @ResourceFilters(StateResourceFilter.class) @Produces(MediaType.APPLICATION_JSON) public Response getLoadQueue( @QueryParam("simple") String simple, diff --git a/server/src/main/java/io/druid/server/security/UnsecuredResourceFilter.java b/server/src/main/java/io/druid/server/security/UnsecuredResourceFilter.java index 1fbbc7013c8..dda859c1af9 100644 --- a/server/src/main/java/io/druid/server/security/UnsecuredResourceFilter.java +++ b/server/src/main/java/io/druid/server/security/UnsecuredResourceFilter.java @@ -27,6 +27,10 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import java.io.IOException; +/** + * Sets necessary request attributes for requests sent to endpoints that don't need authentication or + * authorization checks. This Filter is placed before all Authenticators in the filter chain. + */ public class UnsecuredResourceFilter implements Filter { @Override @@ -40,6 +44,14 @@ public class UnsecuredResourceFilter implements Filter ServletRequest servletRequest, ServletResponse servletResponse, FilterChain filterChain ) throws IOException, ServletException { + // PreResponseAuthorizationCheckFilter checks that this attribute is set, + // 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, null) + ); + + // This request will not go to an Authorizer, so we need to set this for PreResponseAuthorizationCheckFilter servletRequest.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true); filterChain.doFilter(servletRequest, servletResponse); } diff --git a/services/src/main/java/io/druid/cli/CliOverlord.java b/services/src/main/java/io/druid/cli/CliOverlord.java index e976ffd7ae8..7bba5f4e2b2 100644 --- a/services/src/main/java/io/druid/cli/CliOverlord.java +++ b/services/src/main/java/io/druid/cli/CliOverlord.java @@ -121,7 +121,9 @@ public class CliOverlord extends ServerRunnable "/console.html", "/old-console/*", "/images/*", - "/js/*" + "/js/*", + "/druid/indexer/v1/isLeader", + "/status/health" ); public CliOverlord() @@ -320,13 +322,15 @@ public class CliOverlord extends ServerRunnable List authenticators = null; AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper); + + // perform no-op authorization for these resources + AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); + authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); JettyServerInitUtils.addExtensionFilters(root, injector); - // perform no-op authorization for these static resources - AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); // Check that requests were authorized before sending responses AuthenticationUtils.addPreResponseAuthorizationCheckFilter( diff --git a/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java b/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java index 6f3e96cb2e6..1174bf4b9b9 100644 --- a/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java @@ -62,7 +62,9 @@ class CoordinatorJettyServerInitializer implements JettyServerInitializer "/fonts/*", "/old-console/*", "/coordinator/false", - "/overlord/false" + "/overlord/false", + "/status/health", + "/druid/coordinator/v1/isLeader" ); private static Logger log = new Logger(CoordinatorJettyServerInitializer.class); @@ -112,13 +114,15 @@ class CoordinatorJettyServerInitializer implements JettyServerInitializer List authenticators = null; AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper); + + // perform no-op authorization for these resources + AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); + authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); JettyServerInitUtils.addExtensionFilters(root, injector); - // perform no-op authorization for these static resources - AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); // Check that requests were authorized before sending responses AuthenticationUtils.addPreResponseAuthorizationCheckFilter( diff --git a/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java b/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java index f66676b9205..2eaa4d154fe 100644 --- a/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java @@ -20,6 +20,7 @@ package io.druid.cli; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Lists; import com.google.inject.Injector; import com.google.inject.Key; import com.google.inject.servlet.GuiceFilter; @@ -47,6 +48,10 @@ class MiddleManagerJettyServerInitializer implements JettyServerInitializer { private static Logger log = new Logger(MiddleManagerJettyServerInitializer.class); + private static List UNSECURED_PATHS = Lists.newArrayList( + "/status/health" + ); + @Override public void initialize(Server server, Injector injector) { @@ -59,6 +64,10 @@ class MiddleManagerJettyServerInitializer implements JettyServerInitializer List authenticators = null; AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper); + + // perform no-op authorization for these resources + AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); + authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); diff --git a/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java b/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java index e3988a29cae..0bc34f17fee 100644 --- a/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java @@ -22,6 +22,7 @@ package io.druid.cli; import com.fasterxml.jackson.databind.ObjectMapper; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Lists; import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Key; @@ -51,6 +52,9 @@ import java.util.Set; public class QueryJettyServerInitializer implements JettyServerInitializer { private static final Logger log = new Logger(QueryJettyServerInitializer.class); + private static List UNSECURED_PATHS = Lists.newArrayList( + "/status/health" + ); private final List extensionHandlers; @@ -88,6 +92,10 @@ public class QueryJettyServerInitializer implements JettyServerInitializer List authenticators = null; AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper); + + // perform no-op authorization for these resources + AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); + authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); diff --git a/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java b/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java index f7acecaa102..3ea97097a13 100644 --- a/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java @@ -20,6 +20,7 @@ package io.druid.cli; import com.fasterxml.jackson.databind.ObjectMapper; +import com.google.common.collect.Lists; import com.google.inject.Inject; import com.google.inject.Injector; import com.google.inject.Key; @@ -50,6 +51,10 @@ public class RouterJettyServerInitializer implements JettyServerInitializer { private static Logger log = new Logger(RouterJettyServerInitializer.class); + private static List UNSECURED_PATHS = Lists.newArrayList( + "/status/health" + ); + private final AsyncQueryForwardingServlet asyncQueryForwardingServlet; private final DruidHttpClientConfig httpClientConfig; @@ -88,6 +93,10 @@ public class RouterJettyServerInitializer implements JettyServerInitializer List authenticators = null; AuthenticationUtils.addSecuritySanityCheckFilter(root, jsonMapper); + + // perform no-op authorization for these resources + AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); + authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators);