You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@druid.apache.org by GitBox <gi...@apache.org> on 2018/07/09 20:24:47 UTC
[GitHub] jihoonson closed pull request #5966: [Backport] Immediately send
401 on basic HTTP authentication failure
jihoonson closed pull request #5966: [Backport] Immediately send 401 on basic HTTP authentication failure
URL: https://github.com/apache/incubator-druid/pull/5966
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/extensions-core/druid-basic-security/src/main/java/io/druid/security/basic/BasicAuthUtils.java b/extensions-core/druid-basic-security/src/main/java/io/druid/security/basic/BasicAuthUtils.java
index 9b1f92d1603..5bc80f8ed81 100644
--- a/extensions-core/druid-basic-security/src/main/java/io/druid/security/basic/BasicAuthUtils.java
+++ b/extensions-core/druid-basic-security/src/main/java/io/druid/security/basic/BasicAuthUtils.java
@@ -117,7 +117,7 @@ public static String getEncodedCredentials(final String unencodedCreds)
}
@Nullable
- public static String getBasicUserSecretFromHttpReq(HttpServletRequest httpReq)
+ public static String getEncodedUserSecretFromHttpReq(HttpServletRequest httpReq)
{
String authHeader = httpReq.getHeader("Authorization");
@@ -133,8 +133,12 @@ public static String getBasicUserSecretFromHttpReq(HttpServletRequest httpReq)
return null;
}
- String encodedUserSecret = authHeader.substring(6);
+ return authHeader.substring(6);
+ }
+ @Nullable
+ public static String decodeUserSecret(String encodedUserSecret)
+ {
try {
return StringUtils.fromUtf8(Base64.getDecoder().decode(encodedUserSecret));
}
diff --git a/extensions-core/druid-basic-security/src/main/java/io/druid/security/basic/authentication/BasicHTTPAuthenticator.java b/extensions-core/druid-basic-security/src/main/java/io/druid/security/basic/authentication/BasicHTTPAuthenticator.java
index eeb406ee681..e895b8c6fc5 100644
--- a/extensions-core/druid-basic-security/src/main/java/io/druid/security/basic/authentication/BasicHTTPAuthenticator.java
+++ b/extensions-core/druid-basic-security/src/main/java/io/druid/security/basic/authentication/BasicHTTPAuthenticator.java
@@ -155,15 +155,22 @@ public void doFilter(
) throws IOException, ServletException
{
HttpServletResponse httpResp = (HttpServletResponse) servletResponse;
- String userSecret = BasicAuthUtils.getBasicUserSecretFromHttpReq((HttpServletRequest) servletRequest);
- if (userSecret == null) {
+ String encodedUserSecret = BasicAuthUtils.getEncodedUserSecretFromHttpReq((HttpServletRequest) servletRequest);
+ if (encodedUserSecret == null) {
// Request didn't have HTTP Basic auth credentials, move on to the next filter
filterChain.doFilter(servletRequest, servletResponse);
return;
}
- String[] splits = userSecret.split(":");
+ String decodedUserSecret = BasicAuthUtils.decodeUserSecret(encodedUserSecret);
+ if (decodedUserSecret == null) {
+ // we recognized a Basic auth header, but could not decode the user secret
+ httpResp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ return;
+ }
+
+ String[] splits = decodedUserSecret.split(":");
if (splits.length != 2) {
httpResp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
return;
@@ -175,6 +182,9 @@ public void doFilter(
if (checkCredentials(user, password)) {
AuthenticationResult authenticationResult = new AuthenticationResult(user, authorizerName, name, null);
servletRequest.setAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT, authenticationResult);
+ } else {
+ httpResp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ return;
}
filterChain.doFilter(servletRequest, servletResponse);
diff --git a/extensions-core/druid-basic-security/src/test/java/io/druid/security/authentication/BasicHTTPAuthenticatorTest.java b/extensions-core/druid-basic-security/src/test/java/io/druid/security/authentication/BasicHTTPAuthenticatorTest.java
new file mode 100644
index 00000000000..e78b424b8fc
--- /dev/null
+++ b/extensions-core/druid-basic-security/src/test/java/io/druid/security/authentication/BasicHTTPAuthenticatorTest.java
@@ -0,0 +1,261 @@
+/*
+ * 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.security.authentication;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.inject.Provider;
+import com.google.inject.util.Providers;
+import io.druid.java.util.common.StringUtils;
+import io.druid.security.basic.authentication.BasicHTTPAuthenticator;
+import io.druid.security.basic.authentication.db.cache.BasicAuthenticatorCacheManager;
+import io.druid.security.basic.authentication.entity.BasicAuthenticatorCredentialUpdate;
+import io.druid.security.basic.authentication.entity.BasicAuthenticatorCredentials;
+import io.druid.security.basic.authentication.entity.BasicAuthenticatorUser;
+import io.druid.server.security.AuthConfig;
+import io.druid.server.security.AuthenticationResult;
+import org.asynchttpclient.util.Base64;
+import org.easymock.EasyMock;
+import org.junit.Test;
+
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletException;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.io.IOException;
+import java.util.Map;
+
+public class BasicHTTPAuthenticatorTest
+{
+ public static BasicAuthenticatorCredentials USER_A_CREDENTIALS = new BasicAuthenticatorCredentials(
+ new BasicAuthenticatorCredentialUpdate("helloworld", 20)
+ );
+
+ public static Provider<BasicAuthenticatorCacheManager> CACHE_MANAGER_PROVIDER = Providers.of(
+ new BasicAuthenticatorCacheManager()
+ {
+ @Override
+ public void handleAuthenticatorUpdate(String authenticatorPrefix, byte[] serializedUserMap)
+ {
+
+ }
+
+ @Override
+ public Map<String, BasicAuthenticatorUser> getUserMap(String authenticatorPrefix)
+ {
+ return ImmutableMap.of(
+ "userA", new BasicAuthenticatorUser("userA", USER_A_CREDENTIALS)
+ );
+ }
+ }
+ );
+
+ public static BasicHTTPAuthenticator AUTHENTICATOR = new BasicHTTPAuthenticator(
+ CACHE_MANAGER_PROVIDER,
+ "basic",
+ "basic",
+ "a",
+ "a",
+ false,
+ null,
+ null
+ );
+
+ @Test
+ public void testGoodPassword() throws IOException, ServletException
+ {
+ String header = Base64.encode(
+ StringUtils.toUtf8("userA:helloworld")
+ );
+ header = StringUtils.format("Basic %s", header);
+
+ HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
+ EasyMock.expect(req.getHeader("Authorization")).andReturn(header);
+ req.setAttribute(
+ AuthConfig.DRUID_AUTHENTICATION_RESULT,
+ new AuthenticationResult("userA", "basic", "basic", null)
+ );
+ EasyMock.expectLastCall().times(1);
+ EasyMock.replay(req);
+
+ HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
+ EasyMock.replay(resp);
+
+ FilterChain filterChain = EasyMock.createMock(FilterChain.class);
+ filterChain.doFilter(req, resp);
+ EasyMock.expectLastCall().times(1);
+ EasyMock.replay(filterChain);
+
+ Filter authenticatorFilter = AUTHENTICATOR.getFilter();
+ authenticatorFilter.doFilter(req, resp, filterChain);
+
+ EasyMock.verify(req, resp, filterChain);
+ }
+
+ @Test
+ public void testBadPassword() throws IOException, ServletException
+ {
+ String header = Base64.encode(
+ StringUtils.toUtf8("userA:badpassword")
+ );
+ header = StringUtils.format("Basic %s", header);
+
+ HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
+ EasyMock.expect(req.getHeader("Authorization")).andReturn(header);
+ EasyMock.replay(req);
+
+ HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
+ resp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ EasyMock.expectLastCall().times(1);
+ EasyMock.replay(resp);
+
+ FilterChain filterChain = EasyMock.createMock(FilterChain.class);
+ EasyMock.replay(filterChain);
+
+ Filter authenticatorFilter = AUTHENTICATOR.getFilter();
+ authenticatorFilter.doFilter(req, resp, filterChain);
+
+ EasyMock.verify(req, resp, filterChain);
+ }
+
+ @Test
+ public void testUnknownUser() throws IOException, ServletException
+ {
+ String header = Base64.encode(
+ StringUtils.toUtf8("userB:helloworld")
+ );
+ header = StringUtils.format("Basic %s", header);
+
+ HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
+ EasyMock.expect(req.getHeader("Authorization")).andReturn(header);
+ EasyMock.replay(req);
+
+ HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
+ resp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ EasyMock.expectLastCall().times(1);
+ EasyMock.replay(resp);
+
+ FilterChain filterChain = EasyMock.createMock(FilterChain.class);
+ EasyMock.replay(filterChain);
+
+ Filter authenticatorFilter = AUTHENTICATOR.getFilter();
+ authenticatorFilter.doFilter(req, resp, filterChain);
+
+ EasyMock.verify(req, resp, filterChain);
+ }
+
+ @Test
+ public void testRecognizedButMalformedBasicAuthHeader() throws IOException, ServletException
+ {
+ String header = Base64.encode(
+ StringUtils.toUtf8("malformed decoded header data")
+ );
+ header = StringUtils.format("Basic %s", header);
+
+ HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
+ EasyMock.expect(req.getHeader("Authorization")).andReturn(header);
+ EasyMock.replay(req);
+
+ HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
+ resp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ EasyMock.expectLastCall().times(1);
+ EasyMock.replay(resp);
+
+ FilterChain filterChain = EasyMock.createMock(FilterChain.class);
+ EasyMock.replay(filterChain);
+
+ Filter authenticatorFilter = AUTHENTICATOR.getFilter();
+ authenticatorFilter.doFilter(req, resp, filterChain);
+
+ EasyMock.verify(req, resp, filterChain);
+ }
+
+ @Test
+ public void testRecognizedButNotBase64BasicAuthHeader() throws IOException, ServletException
+ {
+ String header = "Basic this_is_not_base64";
+
+ HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
+ EasyMock.expect(req.getHeader("Authorization")).andReturn(header);
+ EasyMock.replay(req);
+
+ HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
+ resp.sendError(HttpServletResponse.SC_UNAUTHORIZED);
+ EasyMock.expectLastCall().times(1);
+ EasyMock.replay(resp);
+
+ FilterChain filterChain = EasyMock.createMock(FilterChain.class);
+ EasyMock.replay(filterChain);
+
+ Filter authenticatorFilter = AUTHENTICATOR.getFilter();
+ authenticatorFilter.doFilter(req, resp, filterChain);
+
+ EasyMock.verify(req, resp, filterChain);
+ }
+
+ @Test
+ public void testUnrecognizedHeader() throws IOException, ServletException
+ {
+ String header = Base64.encode(
+ StringUtils.toUtf8("userA:helloworld")
+ );
+ header = StringUtils.format("NotBasic %s", header);
+
+ HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
+ EasyMock.expect(req.getHeader("Authorization")).andReturn(header);
+ EasyMock.replay(req);
+
+ HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
+ EasyMock.replay(resp);
+
+ // Authentication filter should move on to the next filter in the chain without sending a response
+ FilterChain filterChain = EasyMock.createMock(FilterChain.class);
+ filterChain.doFilter(req, resp);
+ EasyMock.expectLastCall().times(1);
+ EasyMock.replay(filterChain);
+
+ Filter authenticatorFilter = AUTHENTICATOR.getFilter();
+ authenticatorFilter.doFilter(req, resp, filterChain);
+
+ EasyMock.verify(req, resp, filterChain);
+ }
+
+ @Test
+ public void testMissingHeader() throws IOException, ServletException
+ {
+ HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class);
+ EasyMock.expect(req.getHeader("Authorization")).andReturn(null);
+ EasyMock.replay(req);
+
+ HttpServletResponse resp = EasyMock.createMock(HttpServletResponse.class);
+ EasyMock.replay(resp);
+
+ // Authentication filter should move on to the next filter in the chain without sending a response
+ FilterChain filterChain = EasyMock.createMock(FilterChain.class);
+ filterChain.doFilter(req, resp);
+ EasyMock.expectLastCall().times(1);
+ EasyMock.replay(filterChain);
+
+ Filter authenticatorFilter = AUTHENTICATOR.getFilter();
+ authenticatorFilter.doFilter(req, resp, filterChain);
+
+ EasyMock.verify(req, resp, filterChain);
+ }
+}
diff --git a/server/src/main/java/io/druid/server/security/AuthenticationResult.java b/server/src/main/java/io/druid/server/security/AuthenticationResult.java
index a1a7d5d966e..f409af41c8f 100644
--- a/server/src/main/java/io/druid/server/security/AuthenticationResult.java
+++ b/server/src/main/java/io/druid/server/security/AuthenticationResult.java
@@ -21,6 +21,7 @@
import javax.annotation.Nullable;
import java.util.Map;
+import java.util.Objects;
/**
* An AuthenticationResult contains information about a successfully authenticated request.
@@ -84,4 +85,26 @@ public String getAuthenticatedBy()
{
return authenticatedBy;
}
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ AuthenticationResult that = (AuthenticationResult) o;
+ return Objects.equals(getIdentity(), that.getIdentity()) &&
+ Objects.equals(getAuthorizerName(), that.getAuthorizerName()) &&
+ Objects.equals(getAuthenticatedBy(), that.getAuthenticatedBy()) &&
+ Objects.equals(getContext(), that.getContext());
+ }
+
+ @Override
+ public int hashCode()
+ {
+ return Objects.hash(getIdentity(), getAuthorizerName(), getAuthenticatedBy(), getContext());
+ }
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@druid.apache.org
For additional commands, e-mail: dev-help@druid.apache.org