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:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
