This is an automated email from the ASF dual-hosted git repository.
FrankChen021 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/druid.git
The following commit(s) were added to refs/heads/master by this push:
new 32c3dc4a85e fix: Treat empty string in hadoop.auth cookie as no cookie
(#20234)
32c3dc4a85e is described below
commit 32c3dc4a85ee8686f6d679d9570d0a84f61b70e8
Author: Jiang Wu <[email protected]>
AuthorDate: Thu Sep 10 08:06:18 2026 +0200
fix: Treat empty string in hadoop.auth cookie as no cookie (#20234)
* fix: Treat empty string in hadoop.auth cookie as no cookie
* fix: Improve test coverage for KerberosAuthenticator
* fix: Missing dependency declaration
* test: migrate KerberosAuthenticatorTest to JUnit Jupiter
---------
Co-authored-by: Paweł Rzepiński <[email protected]>
---
extensions-core/druid-kerberos/pom.xml | 10 +
.../security/kerberos/KerberosAuthenticator.java | 41 ++-
.../kerberos/KerberosAuthenticatorTest.java | 344 ++++++++++++++++++++-
3 files changed, 368 insertions(+), 27 deletions(-)
diff --git a/extensions-core/druid-kerberos/pom.xml
b/extensions-core/druid-kerberos/pom.xml
index 282679ae1c3..2c59c7eeb3f 100644
--- a/extensions-core/druid-kerberos/pom.xml
+++ b/extensions-core/druid-kerberos/pom.xml
@@ -46,6 +46,11 @@
<version>${project.parent.version}</version>
<scope>provided</scope>
</dependency>
+ <dependency>
+ <groupId>com.google.code.findbugs</groupId>
+ <artifactId>jsr305</artifactId>
+ <scope>provided</scope>
+ </dependency>
<dependency>
<groupId>org.apache.hadoop</groupId>
<artifactId>hadoop-common</artifactId>
@@ -374,6 +379,11 @@
</dependency>
<!-- Tests -->
+ <dependency>
+ <groupId>org.mockito</groupId>
+ <artifactId>mockito-core</artifactId>
+ <scope>test</scope>
+ </dependency>
<dependency>
<groupId>org.apache.druid</groupId>
<artifactId>druid-processing</artifactId>
diff --git
a/extensions-core/druid-kerberos/src/main/java/org/apache/druid/security/kerberos/KerberosAuthenticator.java
b/extensions-core/druid-kerberos/src/main/java/org/apache/druid/security/kerberos/KerberosAuthenticator.java
index 6ef391610c3..6c2e9afec39 100644
---
a/extensions-core/druid-kerberos/src/main/java/org/apache/druid/security/kerberos/KerberosAuthenticator.java
+++
b/extensions-core/druid-kerberos/src/main/java/org/apache/druid/security/kerberos/KerberosAuthenticator.java
@@ -44,6 +44,7 @@ import
org.apache.hadoop.security.authentication.util.SignerSecretProvider;
import org.eclipse.jetty.client.Request;
import org.eclipse.jetty.http.HttpCookie;
+import javax.annotation.Nullable;
import javax.security.auth.Subject;
import javax.security.auth.kerberos.KerberosPrincipal;
import javax.security.auth.login.AppConfigurationEntry;
@@ -188,11 +189,15 @@ public class KerberosAuthenticator implements
Authenticator
for (Cookie cookie : cookies) {
if (cookie.getName().equals(AuthenticatedURL.AUTH_COOKIE)) {
tokenStr = cookie.getValue();
- try {
- tokenStr = mySigner.verifyAndExtract(tokenStr);
- }
- catch (SignerException ex) {
- throw new AuthenticationException(ex);
+ if (tokenStr == null || tokenStr.isEmpty()) {
+ tokenStr = null;
+ } else {
+ try {
+ tokenStr = mySigner.verifyAndExtract(tokenStr);
+ }
+ catch (SignerException ex) {
+ throw new AuthenticationException(ex);
+ }
}
break;
}
@@ -266,7 +271,7 @@ public class KerberosAuthenticator implements Authenticator
}
token = getAuthenticationHandler().authenticate(httpRequest,
httpResponse);
if (token != null && token.getExpires() != 0 &&
- token != AuthenticationToken.ANONYMOUS) {
+ !AuthenticationToken.ANONYMOUS.equals(token)) {
token.setExpires(System.currentTimeMillis() + getValidity() *
1000);
}
newToken = true;
@@ -293,12 +298,13 @@ public class KerberosAuthenticator implements
Authenticator
}
@Override
+ @Nullable
public Principal getUserPrincipal()
{
- return (authToken != AuthenticationToken.ANONYMOUS) ?
authToken : null;
+ return (!AuthenticationToken.ANONYMOUS.equals(authToken)) ?
authToken : null;
}
};
- if (newToken && !token.isExpired() && token !=
AuthenticationToken.ANONYMOUS) {
+ if (newToken && !token.isExpired() &&
!AuthenticationToken.ANONYMOUS.equals(token)) {
String signedToken = mySigner.sign(token.toString());
tokenToAuthCookie(
httpResponse,
@@ -374,6 +380,7 @@ public class KerberosAuthenticator implements Authenticator
}
@Override
+ @Nullable
public Class<? extends Filter> getFilterClass()
{
return null;
@@ -398,6 +405,7 @@ public class KerberosAuthenticator implements Authenticator
}
@Override
+ @Nullable
public EnumSet<DispatcherType> getDispatcherType()
{
return null;
@@ -423,9 +431,8 @@ public class KerberosAuthenticator implements Authenticator
)
{
Object cookieToken = clientRequest.getAttribute(SIGNED_TOKEN_ATTRIBUTE);
- if (cookieToken != null && cookieToken instanceof String) {
+ if (cookieToken instanceof String authResult) {
log.debug("Found cookie token will attache it to proxyRequest as
cookie");
- String authResult = (String) cookieToken;
proxyRequest.cookie(HttpCookie.from(SIGNED_TOKEN_ATTRIBUTE, authResult));
}
}
@@ -435,8 +442,8 @@ public class KerberosAuthenticator implements Authenticator
*/
public static class DruidKerberosConfiguration extends Configuration
{
- private String keytab;
- private String principal;
+ private final String keytab;
+ private final String principal;
public DruidKerberosConfiguration(String keytab, String principal)
{
@@ -497,11 +504,11 @@ public class KerberosAuthenticator implements
Authenticator
String keytab;
try {
- if (serverPrincipal == null || serverPrincipal.trim().length() == 0) {
+ if (serverPrincipal == null || serverPrincipal.trim().isEmpty()) {
throw new ServletException("Principal not defined in configuration");
}
keytab = serverKeytab;
- if (keytab == null || keytab.trim().length() == 0) {
+ if (keytab == null || keytab.trim().isEmpty()) {
throw new ServletException("Keytab not defined in configuration");
}
if (!new File(keytab).exists()) {
@@ -568,7 +575,7 @@ public class KerberosAuthenticator implements Authenticator
{
StringBuilder sb = new StringBuilder(AuthenticatedURL.AUTH_COOKIE)
.append("=");
- if (token != null && token.length() > 0) {
+ if (token != null && !token.isEmpty()) {
sb.append("\"").append(token).append("\"");
}
@@ -580,7 +587,9 @@ public class KerberosAuthenticator implements Authenticator
sb.append("; Domain=").append(domain);
}
- if (expires >= 0 && isCookiePersistent) {
+ if (expires == 0) {
+ sb.append("; Max-Age=0");
+ } else if (expires > 0 && isCookiePersistent) {
Date date = new Date(expires);
SimpleDateFormat df = new SimpleDateFormat("EEE, dd-MMM-yyyy HH:mm:ss
zzz", Locale.ENGLISH);
df.setTimeZone(TimeZone.getTimeZone("GMT"));
diff --git
a/extensions-core/druid-kerberos/src/test/java/org/apache/druid/security/kerberos/KerberosAuthenticatorTest.java
b/extensions-core/druid-kerberos/src/test/java/org/apache/druid/security/kerberos/KerberosAuthenticatorTest.java
index c433b525eb4..56c9313f380 100644
---
a/extensions-core/druid-kerberos/src/test/java/org/apache/druid/security/kerberos/KerberosAuthenticatorTest.java
+++
b/extensions-core/druid-kerberos/src/test/java/org/apache/druid/security/kerberos/KerberosAuthenticatorTest.java
@@ -21,8 +21,35 @@ package org.apache.druid.security.kerberos;
import org.apache.druid.error.DruidException;
import org.apache.druid.server.DruidNode;
-import org.junit.jupiter.api.Assertions;
+import org.apache.druid.server.security.AuthConfig;
+import org.apache.hadoop.security.authentication.client.AuthenticatedURL;
+import org.apache.hadoop.security.authentication.server.AuthenticationFilter;
+import org.apache.hadoop.security.authentication.server.AuthenticationToken;
+import org.apache.hadoop.security.authentication.util.Signer;
+import org.apache.hadoop.security.authentication.util.SignerSecretProvider;
+import org.eclipse.jetty.client.Request;
+import org.eclipse.jetty.http.HttpCookie;
import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentMatchers;
+import org.mockito.Mockito;
+
+import javax.servlet.Filter;
+import javax.servlet.FilterChain;
+import javax.servlet.ServletContext;
+import javax.servlet.ServletException;
+import javax.servlet.http.Cookie;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+import java.lang.reflect.Field;
+import java.lang.reflect.Method;
+import java.nio.charset.StandardCharsets;
+import java.util.Properties;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
public class KerberosAuthenticatorTest
{
@@ -39,12 +66,97 @@ public class KerberosAuthenticatorTest
}
+ /**
+ * Verifies that an empty hadoop.auth cookie value is treated as "no cookie"
rather than
+ * causing a SignerException. An empty cookie results from a prior session
expiry where
+ * Druid cleared the cookie. Without this fix, the empty value would be
passed to
+ * Signer.verifyAndExtract("") which throws SignerException, setting
authenticationEx
+ * and causing the entire auth chain to short-circuit with a 403.
+ */
+ @Test
+ public void testGetTokenWithEmptyCookieReturnsNull() throws Exception
+ {
+ final Filter filter = createFilterWithSigner();
+ final Method getToken = findGetTokenMethod();
+
+ // Empty cookie value - the real-world scenario after session expiry
clears the cookie.
+ // Without the fix, Signer.verifyAndExtract("") throws SignerException.
+ final HttpServletRequest requestWithEmptyCookie =
mockRequestWithEmptyCookie();
+ final AuthenticationToken token = (AuthenticationToken)
getToken.invoke(filter, requestWithEmptyCookie);
+ assertNull(token, "Empty hadoop.auth cookie should be treated as no
cookie");
+
+ // No cookie at all - baseline, should return null
+ final HttpServletRequest requestWithNoCookie =
Mockito.mock(HttpServletRequest.class);
+ Mockito.when(requestWithNoCookie.getCookies()).thenReturn(null);
+ final AuthenticationToken tokenForNoCookie = (AuthenticationToken)
getToken.invoke(filter, requestWithNoCookie);
+ assertNull(tokenForNoCookie, "Missing hadoop.auth cookie should return
null");
+ }
+
+ private Filter createFilterWithSigner() throws Exception
+ {
+ final Filter filter = new KerberosAuthenticator(
+ TEST_SERVER_PRINCIPAL,
+ TEST_SERVER_KEYTAB,
+ TEST_AUTH_TO_LOCAL,
+ TEST_COOKIE_SECRET,
+ TEST_AUTHORIZER_NAME,
+ TEST_NAME,
+ createTestNode()
+ ).getFilter();
+
+ final SignerSecretProvider secretProvider = new SignerSecretProvider()
+ {
+ @Override
+ public void init(Properties config, ServletContext servletContext, long
tokenValidity)
+ {
+ }
+
+ @Override
+ public byte[] getCurrentSecret()
+ {
+ return TEST_COOKIE_SECRET.getBytes(StandardCharsets.UTF_8);
+ }
+
+ @Override
+ public byte[][] getAllSecrets()
+ {
+ return new
byte[][]{TEST_COOKIE_SECRET.getBytes(StandardCharsets.UTF_8)};
+ }
+ };
+ final Signer signer = new Signer(secretProvider);
+
+ // Inject mySigner into the anonymous AuthenticationFilter subclass via
reflection
+ for (Field field : filter.getClass().getDeclaredFields()) {
+ if (field.getType().equals(Signer.class)) {
+ field.setAccessible(true);
+ field.set(filter, signer);
+ break;
+ }
+ }
+ return filter;
+ }
+
+ private Method findGetTokenMethod() throws Exception
+ {
+ final Method method =
AuthenticationFilter.class.getDeclaredMethod("getToken",
HttpServletRequest.class);
+ method.setAccessible(true);
+ return method;
+ }
+
+ private HttpServletRequest mockRequestWithEmptyCookie()
+ {
+ final HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+ final Cookie cookie = new Cookie(AuthenticatedURL.AUTH_COOKIE, "");
+ Mockito.when(request.getCookies()).thenReturn(new Cookie[]{cookie});
+ return request;
+ }
+
@Test
public void testConstructorWithNullCookieSignatureSecret()
{
DruidNode node = createTestNode();
- DruidException exception = Assertions.assertThrows(
+ DruidException exception = assertThrows(
DruidException.class,
() -> new KerberosAuthenticator(
TEST_SERVER_PRINCIPAL,
@@ -57,13 +169,13 @@ public class KerberosAuthenticatorTest
)
);
- Assertions.assertEquals(DruidException.Persona.OPERATOR,
exception.getTargetPersona());
- Assertions.assertEquals(DruidException.Category.INVALID_INPUT,
exception.getCategory());
- Assertions.assertTrue(
+ assertEquals(DruidException.Persona.OPERATOR,
exception.getTargetPersona());
+ assertEquals(DruidException.Category.INVALID_INPUT,
exception.getCategory());
+ assertTrue(
exception.getMessage().contains("cookieSignatureSecret"),
"Exception message should mention cookieSignatureSecret"
);
- Assertions.assertTrue(
+ assertTrue(
exception.getMessage().contains("is not set"),
"Exception message should mention 'is not set'"
);
@@ -74,7 +186,7 @@ public class KerberosAuthenticatorTest
{
DruidNode node = createTestNode();
- DruidException exception = Assertions.assertThrows(
+ DruidException exception = assertThrows(
DruidException.class,
() -> new KerberosAuthenticator(
TEST_SERVER_PRINCIPAL,
@@ -87,15 +199,225 @@ public class KerberosAuthenticatorTest
)
);
- Assertions.assertEquals(DruidException.Persona.OPERATOR,
exception.getTargetPersona());
- Assertions.assertEquals(DruidException.Category.INVALID_INPUT,
exception.getCategory());
- Assertions.assertTrue(
+ assertEquals(DruidException.Persona.OPERATOR,
exception.getTargetPersona());
+ assertEquals(DruidException.Category.INVALID_INPUT,
exception.getCategory());
+ assertTrue(
exception.getMessage().contains("cookieSignatureSecret"),
"Exception message should mention cookieSignatureSecret"
);
- Assertions.assertTrue(
+ assertTrue(
exception.getMessage().contains("is not set"),
"Exception message should mention 'is not set'"
);
}
+
+ @Test
+ public void testTokenToCookieStringWithZeroExpiresIncludesMaxAge() throws
Exception
+ {
+ final Method method = KerberosAuthenticator.class.getDeclaredMethod(
+ "tokenToCookieString",
+ String.class,
+ String.class,
+ String.class,
+ long.class,
+ boolean.class,
+ boolean.class
+ );
+ method.setAccessible(true);
+
+ // Test case: expires = 0 (intended for cookie deletion)
+ final String cookieString = (String) method.invoke(
+ null,
+ "", // token
+ "localhost", // domain
+ "/", // path
+ 0, // expires
+ false, // isCookiePersistent
+ false // isSecure
+ );
+
+ assertTrue(cookieString.contains("Max-Age=0"), "Cookie string should
contain 'Max-Age=0'");
+ assertFalse(cookieString.contains("Expires="), "Cookie string should not
contain 'Expires=' when expires is 0");
+
+ // Test case: expires > 0 and persistent
+ final String persistentCookieString = (String) method.invoke(
+ null,
+ "some-token",
+ "localhost",
+ "/",
+ System.currentTimeMillis() + 3600,
+ true,
+ false
+ );
+ assertTrue(persistentCookieString.contains("Expires="), "Persistent cookie
should contain 'Expires='");
+ assertFalse(persistentCookieString.contains("Max-Age=0"), "Persistent
cookie should not contain 'Max-Age=0'");
+ }
+
+ @Test
+ public void testTokenToCookieStringWithNullToken() throws Exception
+ {
+ final Method method = KerberosAuthenticator.class.getDeclaredMethod(
+ "tokenToCookieString",
+ String.class,
+ String.class,
+ String.class,
+ long.class,
+ boolean.class,
+ boolean.class
+ );
+ method.setAccessible(true);
+
+ final String cookieString = (String) method.invoke(null, null,
"localhost", "/", 0, false, false);
+
+ assertFalse(cookieString.contains("\""), "Null token should not add quoted
value");
+ assertTrue(cookieString.contains("Max-Age=0"), "Cookie string should
contain Max-Age=0");
+ }
+
+ @Test
+ public void testTokenToCookieStringWithNonPersistentPositiveExpires() throws
Exception
+ {
+ final Method method = KerberosAuthenticator.class.getDeclaredMethod(
+ "tokenToCookieString",
+ String.class,
+ String.class,
+ String.class,
+ long.class,
+ boolean.class,
+ boolean.class
+ );
+ method.setAccessible(true);
+
+ final String cookieString = (String) method.invoke(
+ null,
+ "some-token",
+ "localhost",
+ "/",
+ System.currentTimeMillis() + 3600,
+ false, // isCookiePersistent = false
+ false
+ );
+
+ assertFalse(cookieString.contains("Expires="), "Non-persistent cookie
should not contain 'Expires='");
+ assertFalse(cookieString.contains("Max-Age=0"), "Non-persistent cookie
should not contain 'Max-Age=0'");
+ }
+
+ @Test
+ public void testDecorateProxyRequestWithStringToken()
+ {
+ final KerberosAuthenticator authenticator = new KerberosAuthenticator(
+ TEST_SERVER_PRINCIPAL,
+ TEST_SERVER_KEYTAB,
+ TEST_AUTH_TO_LOCAL,
+ TEST_COOKIE_SECRET,
+ TEST_AUTHORIZER_NAME,
+ TEST_NAME,
+ createTestNode()
+ );
+ final HttpServletRequest clientRequest =
Mockito.mock(HttpServletRequest.class);
+ final HttpServletResponse proxyResponse =
Mockito.mock(HttpServletResponse.class);
+ final Request proxyRequest = Mockito.mock(Request.class);
+
Mockito.when(clientRequest.getAttribute(KerberosAuthenticator.SIGNED_TOKEN_ATTRIBUTE)).thenReturn("signed-token-value");
+
+ authenticator.decorateProxyRequest(clientRequest, proxyResponse,
proxyRequest);
+
+
Mockito.verify(proxyRequest).cookie(ArgumentMatchers.any(HttpCookie.class));
+ }
+
+ @Test
+ public void testDecorateProxyRequestWithoutToken()
+ {
+ final KerberosAuthenticator authenticator = new KerberosAuthenticator(
+ TEST_SERVER_PRINCIPAL,
+ TEST_SERVER_KEYTAB,
+ TEST_AUTH_TO_LOCAL,
+ TEST_COOKIE_SECRET,
+ TEST_AUTHORIZER_NAME,
+ TEST_NAME,
+ createTestNode()
+ );
+ final HttpServletRequest clientRequest =
Mockito.mock(HttpServletRequest.class);
+ final HttpServletResponse proxyResponse =
Mockito.mock(HttpServletResponse.class);
+ final Request proxyRequest = Mockito.mock(Request.class);
+
Mockito.when(clientRequest.getAttribute(KerberosAuthenticator.SIGNED_TOKEN_ATTRIBUTE)).thenReturn(null);
+
+ authenticator.decorateProxyRequest(clientRequest, proxyResponse,
proxyRequest);
+
+ Mockito.verify(proxyRequest,
Mockito.never()).cookie(ArgumentMatchers.any());
+ }
+
+ @Test
+ public void testDoFilterWithNullPrincipalThrowsServletException()
+ {
+ // null serverPrincipal causes initializeKerberosLogin to throw "Principal
not defined"
+ final KerberosAuthenticator authenticator = new KerberosAuthenticator(
+ null,
+ TEST_SERVER_KEYTAB,
+ TEST_AUTH_TO_LOCAL,
+ TEST_COOKIE_SECRET,
+ TEST_AUTHORIZER_NAME,
+ TEST_NAME,
+ createTestNode()
+ );
+ final Filter filter = authenticator.getFilter();
+ final HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+ final HttpServletResponse response =
Mockito.mock(HttpServletResponse.class);
+ final FilterChain chain = Mockito.mock(FilterChain.class);
+
Mockito.when(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).thenReturn(null);
+
+ final ServletException ex = assertThrows(
+ ServletException.class,
+ () -> filter.doFilter(request, response, chain)
+ );
+ assertTrue(ex.getMessage().contains("Principal not defined"));
+ }
+
+ @Test
+ public void testDoFilterWithNullKeytabThrowsServletException()
+ {
+ final KerberosAuthenticator authenticator = new KerberosAuthenticator(
+ TEST_SERVER_PRINCIPAL,
+ null, // null keytab
+ TEST_AUTH_TO_LOCAL,
+ TEST_COOKIE_SECRET,
+ TEST_AUTHORIZER_NAME,
+ TEST_NAME,
+ createTestNode()
+ );
+ final Filter filter = authenticator.getFilter();
+ final HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+ final HttpServletResponse response =
Mockito.mock(HttpServletResponse.class);
+ final FilterChain chain = Mockito.mock(FilterChain.class);
+
Mockito.when(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).thenReturn(null);
+
+ final ServletException ex = assertThrows(
+ ServletException.class,
+ () -> filter.doFilter(request, response, chain)
+ );
+ assertTrue(ex.getMessage().contains("Keytab not defined"));
+ }
+
+ @Test
+ public void testDoFilterWithEmptyKeytabThrowsServletException()
+ {
+ final KerberosAuthenticator authenticator = new KerberosAuthenticator(
+ TEST_SERVER_PRINCIPAL,
+ "", // empty keytab
+ TEST_AUTH_TO_LOCAL,
+ TEST_COOKIE_SECRET,
+ TEST_AUTHORIZER_NAME,
+ TEST_NAME,
+ createTestNode()
+ );
+ final Filter filter = authenticator.getFilter();
+ final HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+ final HttpServletResponse response =
Mockito.mock(HttpServletResponse.class);
+ final FilterChain chain = Mockito.mock(FilterChain.class);
+
Mockito.when(request.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).thenReturn(null);
+
+ final ServletException ex = assertThrows(
+ ServletException.class,
+ () -> filter.doFilter(request, response, chain)
+ );
+ assertTrue(ex.getMessage().contains("Keytab not defined"));
+ }
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
