From 93ac36c67e1f53ceaedf8809b0626a6bbee415d3 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Mon, 9 Apr 2018 13:37:26 -0700 Subject: [PATCH 1/5] Fix HTTP OPTIONS request auth handling --- docs/content/configuration/auth.md | 1 + .../ITBasicAuthConfigurationTest.java | 12 +++ .../security/AllowOptionsResourceFilter.java | 77 +++++++++++++++++++ .../io/druid/server/security/AuthConfig.java | 45 +++++++---- .../server/security/AuthenticationUtils.java | 10 +++ .../main/java/io/druid/cli/CliOverlord.java | 2 + .../CoordinatorJettyServerInitializer.java | 3 +- .../MiddleManagerJettyServerInitializer.java | 3 +- .../cli/QueryJettyServerInitializer.java | 2 + .../cli/RouterJettyServerInitializer.java | 2 + 10 files changed, 142 insertions(+), 15 deletions(-) create mode 100644 server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java diff --git a/docs/content/configuration/auth.md b/docs/content/configuration/auth.md index 296d9ba2b1e8..53ea321b5b0c 100644 --- a/docs/content/configuration/auth.md +++ b/docs/content/configuration/auth.md @@ -10,6 +10,7 @@ layout: doc_page |`druid.escalator.type`|String|Type of the Escalator that should be used for internal Druid communications. This Escalator must use an authentication scheme that is supported by an Authenticator in `druid.auth.authenticationChain`.|"noop"|no| |`druid.auth.authorizers`|JSON List of Strings|List of Authorizer type names |["allowAll"]|no| |`druid.auth.unsecuredPaths`| List of Strings|List of paths for which security checks will not be performed. All requests to these paths will be allowed.|[]|no| +|`druid.auth.allowUnauthenticatedHttpOptions`|Boolean|If true, skip authentication checks for HTTP OPTIONS requests. This is needed for certain use cases, such as supporting CORS pre-flight requests. Note that disabling authentication checks for OPTIONS requests will allow unauthenticated users to determine what Druid endpoints are valid (by checking if the OPTIONS request returns a 200 instead of 404), so the authentication checks should not be disabled unless necessary. |false|no| ## Enabling Authentication/Authorization diff --git a/integration-tests/src/test/java/io/druid/tests/security/ITBasicAuthConfigurationTest.java b/integration-tests/src/test/java/io/druid/tests/security/ITBasicAuthConfigurationTest.java index 17c87efa0fd2..c1758967cdd1 100644 --- a/integration-tests/src/test/java/io/druid/tests/security/ITBasicAuthConfigurationTest.java +++ b/integration-tests/src/test/java/io/druid/tests/security/ITBasicAuthConfigurationTest.java @@ -224,6 +224,18 @@ public void testAuthConfiguration() throws Exception LOG.info("Testing Avatica query on router with incorrect credentials."); testAvaticaAuthFailure(routerUrl); + + LOG.info("Checking OPTIONS requests on services..."); + testOptionsRequests(adminClient); + } + + private void testOptionsRequests(HttpClient httpClient) + { + makeRequest(httpClient, HttpMethod.OPTIONS, config.getCoordinatorUrl() + "/status", null); + makeRequest(httpClient, HttpMethod.OPTIONS, config.getIndexerUrl() + "/status", null); + makeRequest(httpClient, HttpMethod.OPTIONS, config.getBrokerUrl() + "/status", null); + makeRequest(httpClient, HttpMethod.OPTIONS, config.getHistoricalUrl() + "/status", null); + makeRequest(httpClient, HttpMethod.OPTIONS, config.getRouterUrl() + "/status", null); } private void checkUnsecuredCoordinatorLoadQueuePath(HttpClient client) diff --git a/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java b/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java new file mode 100644 index 000000000000..fc2597b2d887 --- /dev/null +++ b/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java @@ -0,0 +1,77 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.server.security; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import javax.ws.rs.HttpMethod; +import java.io.IOException; + +public class AllowOptionsResourceFilter implements Filter +{ + private final boolean allowUnauthenticatedHttpOptions; + + public AllowOptionsResourceFilter( + boolean allowUnauthenticatedHttpOptions + ) + { + this.allowUnauthenticatedHttpOptions = allowUnauthenticatedHttpOptions; + } + + @Override + public void init(FilterConfig filterConfig) throws ServletException + { + + } + + @Override + public void doFilter( + ServletRequest request, ServletResponse response, FilterChain chain + ) throws IOException, ServletException + { + HttpServletRequest httpReq = (HttpServletRequest) request; + + // Druid itself doesn't explictly handle OPTIONS requests, no resource handler will authorize such requests. + // so this filter catches all OPTIONS requests and authorizes them. + if (HttpMethod.OPTIONS.equals(httpReq.getMethod())) { + if (allowUnauthenticatedHttpOptions) { + httpReq.setAttribute( + AuthConfig.DRUID_AUTHENTICATION_RESULT, + new AuthenticationResult(AuthConfig.ALLOW_ALL_NAME, AuthConfig.ALLOW_ALL_NAME, null) + ); + } + + httpReq.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true); + } + + chain.doFilter(request, response); + } + + @Override + public void destroy() + { + + } +} diff --git a/server/src/main/java/io/druid/server/security/AuthConfig.java b/server/src/main/java/io/druid/server/security/AuthConfig.java index 95f67c9ee0fc..d1b6d0932197 100644 --- a/server/src/main/java/io/druid/server/security/AuthConfig.java +++ b/server/src/main/java/io/druid/server/security/AuthConfig.java @@ -44,19 +44,23 @@ public class AuthConfig public AuthConfig() { - this(null, null, null); + this(null, null, null, null); } @JsonCreator public AuthConfig( @JsonProperty("authenticatorChain") List authenticationChain, @JsonProperty("authorizers") List authorizers, - @JsonProperty("unsecuredPaths") List unsecuredPaths + @JsonProperty("unsecuredPaths") List unsecuredPaths, + @JsonProperty("allowUnauthenticatedHttpOptions") Boolean allowUnauthenticatedHttpOptions ) { this.authenticatorChain = authenticationChain; this.authorizers = authorizers; this.unsecuredPaths = unsecuredPaths == null ? Collections.emptyList() : unsecuredPaths; + this.allowUnauthenticatedHttpOptions = allowUnauthenticatedHttpOptions == null + ? false + : allowUnauthenticatedHttpOptions; } @JsonProperty @@ -68,6 +72,9 @@ public AuthConfig( @JsonProperty private final List unsecuredPaths; + @JsonProperty + private final boolean allowUnauthenticatedHttpOptions; + public List getAuthenticatorChain() { return authenticatorChain; @@ -83,14 +90,9 @@ public List getUnsecuredPaths() return unsecuredPaths; } - @Override - public String toString() + public boolean isAllowUnauthenticatedHttpOptions() { - return "AuthConfig{" + - "authenticatorChain='" + authenticatorChain + '\'' + - ", authorizers='" + authorizers + '\'' + - ", unsecuredPaths='" + unsecuredPaths + '\'' + - '}'; + return allowUnauthenticatedHttpOptions; } @Override @@ -103,14 +105,31 @@ public boolean equals(Object o) return false; } AuthConfig that = (AuthConfig) o; - return Objects.equals(authenticatorChain, that.authenticatorChain) && - Objects.equals(authorizers, that.authorizers) && - Objects.equals(unsecuredPaths, that.unsecuredPaths); + return isAllowUnauthenticatedHttpOptions() == that.isAllowUnauthenticatedHttpOptions() && + Objects.equals(getAuthenticatorChain(), that.getAuthenticatorChain()) && + Objects.equals(getAuthorizers(), that.getAuthorizers()) && + Objects.equals(getUnsecuredPaths(), that.getUnsecuredPaths()); } @Override public int hashCode() { - return Objects.hash(authenticatorChain, authorizers, unsecuredPaths); + return Objects.hash( + getAuthenticatorChain(), + getAuthorizers(), + getUnsecuredPaths(), + isAllowUnauthenticatedHttpOptions() + ); + } + + @Override + public String toString() + { + return "AuthConfig{" + + "authenticatorChain=" + authenticatorChain + + ", authorizers=" + authorizers + + ", unsecuredPaths=" + unsecuredPaths + + ", allowUnauthenticatedHttpOptions=" + allowUnauthenticatedHttpOptions + + '}'; } } diff --git a/server/src/main/java/io/druid/server/security/AuthenticationUtils.java b/server/src/main/java/io/druid/server/security/AuthenticationUtils.java index cabaa828274c..bd6b2be7680f 100644 --- a/server/src/main/java/io/druid/server/security/AuthenticationUtils.java +++ b/server/src/main/java/io/druid/server/security/AuthenticationUtils.java @@ -27,6 +27,16 @@ public class AuthenticationUtils { + public static void addAllowOptionsFilter(ServletContextHandler root, boolean allowUnauthenticatedHttpOptions) + { + FilterHolder holder = new FilterHolder(new AllowOptionsResourceFilter(allowUnauthenticatedHttpOptions)); + root.addFilter( + holder, + "/*", + null + ); + } + public static void addAuthenticationFilterChain( ServletContextHandler root, List authenticators diff --git a/services/src/main/java/io/druid/cli/CliOverlord.java b/services/src/main/java/io/druid/cli/CliOverlord.java index 33f842c2e4e6..83d0030ac852 100644 --- a/services/src/main/java/io/druid/cli/CliOverlord.java +++ b/services/src/main/java/io/druid/cli/CliOverlord.java @@ -344,6 +344,8 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths()); + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); diff --git a/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java b/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java index c9bc725e5e95..6b15cb611662 100644 --- a/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java @@ -125,12 +125,13 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, CliOverlord.UNSECURED_PATHS); } + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); JettyServerInitUtils.addExtensionFilters(root, injector); - // Check that requests were authorized before sending responses AuthenticationUtils.addPreResponseAuthorizationCheckFilter( root, diff --git a/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java b/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java index 9408acc588bf..f4314d8d119d 100644 --- a/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java @@ -78,10 +78,11 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths()); + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); - JettyServerInitUtils.addExtensionFilters(root, injector); // Check that requests were authorized before sending responses diff --git a/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java b/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java index b079041b2ad9..80372b49cd50 100644 --- a/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java @@ -102,6 +102,8 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths()); + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + 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 ec0ca471dfef..729e4d1191f9 100644 --- a/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java @@ -111,6 +111,8 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths()); + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + final List authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); From b6a6509bb3cdd92159c810556bb25e482e617df2 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Thu, 12 Apr 2018 22:03:02 -0700 Subject: [PATCH 2/5] PR comment --- docs/content/configuration/auth.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/content/configuration/auth.md b/docs/content/configuration/auth.md index 53ea321b5b0c..4044fadd6dd5 100644 --- a/docs/content/configuration/auth.md +++ b/docs/content/configuration/auth.md @@ -10,7 +10,7 @@ layout: doc_page |`druid.escalator.type`|String|Type of the Escalator that should be used for internal Druid communications. This Escalator must use an authentication scheme that is supported by an Authenticator in `druid.auth.authenticationChain`.|"noop"|no| |`druid.auth.authorizers`|JSON List of Strings|List of Authorizer type names |["allowAll"]|no| |`druid.auth.unsecuredPaths`| List of Strings|List of paths for which security checks will not be performed. All requests to these paths will be allowed.|[]|no| -|`druid.auth.allowUnauthenticatedHttpOptions`|Boolean|If true, skip authentication checks for HTTP OPTIONS requests. This is needed for certain use cases, such as supporting CORS pre-flight requests. Note that disabling authentication checks for OPTIONS requests will allow unauthenticated users to determine what Druid endpoints are valid (by checking if the OPTIONS request returns a 200 instead of 404), so the authentication checks should not be disabled unless necessary. |false|no| +|`druid.auth.allowUnauthenticatedHttpOptions`|Boolean|If true, skip authentication checks for HTTP OPTIONS requests. This is needed for certain use cases, such as supporting CORS pre-flight requests. Note that disabling authentication checks for OPTIONS requests will allow unauthenticated users to determine what Druid endpoints are valid (by checking if the OPTIONS request returns a 200 instead of 404), so enabling this option may reveal information about server configuration, including information about what extensions are loaded (if those extensions add endpoints).|false|no| ## Enabling Authentication/Authorization From 8ec0b06f1551c65b25f907814221979ee8e70412 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Fri, 13 Apr 2018 19:04:57 -0700 Subject: [PATCH 3/5] More PR comments --- .../security/AllowOptionsResourceFilter.java | 15 +++++++++++---- .../src/main/java/io/druid/cli/CliOverlord.java | 4 ++-- .../cli/CoordinatorJettyServerInitializer.java | 4 ++-- .../cli/MiddleManagerJettyServerInitializer.java | 4 ++-- .../io/druid/cli/QueryJettyServerInitializer.java | 4 ++-- .../druid/cli/RouterJettyServerInitializer.java | 4 ++-- 6 files changed, 21 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java b/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java index fc2597b2d887..65884d4a8902 100644 --- a/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java +++ b/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java @@ -26,6 +26,7 @@ import javax.servlet.ServletRequest; import javax.servlet.ServletResponse; import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import javax.ws.rs.HttpMethod; import java.io.IOException; @@ -57,10 +58,16 @@ public void doFilter( // so this filter catches all OPTIONS requests and authorizes them. if (HttpMethod.OPTIONS.equals(httpReq.getMethod())) { if (allowUnauthenticatedHttpOptions) { - httpReq.setAttribute( - AuthConfig.DRUID_AUTHENTICATION_RESULT, - new AuthenticationResult(AuthConfig.ALLOW_ALL_NAME, AuthConfig.ALLOW_ALL_NAME, null) - ); + // If the request already had credentials and authenticated successfully, keep the authenticated identity. + // Otherwise, allow the unauthenticated request. + if (httpReq.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT) == null) { + httpReq.setAttribute( + AuthConfig.DRUID_AUTHENTICATION_RESULT, + new AuthenticationResult(AuthConfig.ALLOW_ALL_NAME, AuthConfig.ALLOW_ALL_NAME, null) + ); + } + } else { + ((HttpServletResponse) response).sendError(HttpServletResponse.SC_UNAUTHORIZED); } httpReq.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true); diff --git a/services/src/main/java/io/druid/cli/CliOverlord.java b/services/src/main/java/io/druid/cli/CliOverlord.java index 83d0030ac852..1faba1f4f328 100644 --- a/services/src/main/java/io/druid/cli/CliOverlord.java +++ b/services/src/main/java/io/druid/cli/CliOverlord.java @@ -344,11 +344,11 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths()); - AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); - authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + JettyServerInitUtils.addExtensionFilters(root, injector); diff --git a/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java b/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java index 6b15cb611662..f896d82ad9f1 100644 --- a/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/CoordinatorJettyServerInitializer.java @@ -125,11 +125,11 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, CliOverlord.UNSECURED_PATHS); } - AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); - authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + JettyServerInitUtils.addExtensionFilters(root, injector); // Check that requests were authorized before sending responses diff --git a/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java b/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java index f4314d8d119d..3636f26a83de 100644 --- a/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/MiddleManagerJettyServerInitializer.java @@ -78,11 +78,11 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths()); - AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); - authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + JettyServerInitUtils.addExtensionFilters(root, injector); // Check that requests were authorized before sending responses diff --git a/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java b/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java index 80372b49cd50..51021457b973 100644 --- a/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/QueryJettyServerInitializer.java @@ -102,11 +102,11 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths()); - AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); - authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + JettyServerInitUtils.addExtensionFilters(root, injector); // Check that requests were authorized before sending responses diff --git a/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java b/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java index 729e4d1191f9..2f65ac3595b1 100644 --- a/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java +++ b/services/src/main/java/io/druid/cli/RouterJettyServerInitializer.java @@ -111,11 +111,11 @@ public void initialize(Server server, Injector injector) AuthenticationUtils.addNoopAuthorizationFilters(root, UNSECURED_PATHS); AuthenticationUtils.addNoopAuthorizationFilters(root, authConfig.getUnsecuredPaths()); - AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); - final List authenticators = authenticatorMapper.getAuthenticatorChain(); AuthenticationUtils.addAuthenticationFilterChain(root, authenticators); + AuthenticationUtils.addAllowOptionsFilter(root, authConfig.isAllowUnauthenticatedHttpOptions()); + JettyServerInitUtils.addExtensionFilters(root, injector); // Check that requests were authorized before sending responses From 3ee07aa6e58141bd8f0befad76e469b8855b4407 Mon Sep 17 00:00:00 2001 From: jon-wei Date: Mon, 16 Apr 2018 12:46:24 -0700 Subject: [PATCH 4/5] Fix --- .../druid/server/security/AllowOptionsResourceFilter.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java b/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java index 65884d4a8902..9ec87415593a 100644 --- a/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java +++ b/server/src/main/java/io/druid/server/security/AllowOptionsResourceFilter.java @@ -57,17 +57,17 @@ public void doFilter( // Druid itself doesn't explictly handle OPTIONS requests, no resource handler will authorize such requests. // so this filter catches all OPTIONS requests and authorizes them. if (HttpMethod.OPTIONS.equals(httpReq.getMethod())) { - if (allowUnauthenticatedHttpOptions) { + if (httpReq.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT) == null) { // If the request already had credentials and authenticated successfully, keep the authenticated identity. // Otherwise, allow the unauthenticated request. - if (httpReq.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT) == null) { + if (allowUnauthenticatedHttpOptions) { httpReq.setAttribute( AuthConfig.DRUID_AUTHENTICATION_RESULT, new AuthenticationResult(AuthConfig.ALLOW_ALL_NAME, AuthConfig.ALLOW_ALL_NAME, null) ); + } else { + ((HttpServletResponse) response).sendError(HttpServletResponse.SC_UNAUTHORIZED); } - } else { - ((HttpServletResponse) response).sendError(HttpServletResponse.SC_UNAUTHORIZED); } httpReq.setAttribute(AuthConfig.DRUID_AUTHORIZATION_CHECKED, true); From c93e616ee55ddde138e8aa88eb79eb03ba78635d Mon Sep 17 00:00:00 2001 From: jon-wei Date: Mon, 16 Apr 2018 12:50:59 -0700 Subject: [PATCH 5/5] PR comment --- .../main/java/io/druid/server/security/AuthConfig.java | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/io/druid/server/security/AuthConfig.java b/server/src/main/java/io/druid/server/security/AuthConfig.java index d1b6d0932197..312f52e2c2c0 100644 --- a/server/src/main/java/io/druid/server/security/AuthConfig.java +++ b/server/src/main/java/io/druid/server/security/AuthConfig.java @@ -44,7 +44,7 @@ public class AuthConfig public AuthConfig() { - this(null, null, null, null); + this(null, null, null, false); } @JsonCreator @@ -52,15 +52,13 @@ public AuthConfig( @JsonProperty("authenticatorChain") List authenticationChain, @JsonProperty("authorizers") List authorizers, @JsonProperty("unsecuredPaths") List unsecuredPaths, - @JsonProperty("allowUnauthenticatedHttpOptions") Boolean allowUnauthenticatedHttpOptions + @JsonProperty("allowUnauthenticatedHttpOptions") boolean allowUnauthenticatedHttpOptions ) { this.authenticatorChain = authenticationChain; this.authorizers = authorizers; this.unsecuredPaths = unsecuredPaths == null ? Collections.emptyList() : unsecuredPaths; - this.allowUnauthenticatedHttpOptions = allowUnauthenticatedHttpOptions == null - ? false - : allowUnauthenticatedHttpOptions; + this.allowUnauthenticatedHttpOptions = allowUnauthenticatedHttpOptions; } @JsonProperty