github-advanced-security[bot] commented on code in PR #19520:
URL: https://github.com/apache/druid/pull/19520#discussion_r3309261856
##########
extensions-core/druid-kerberos/src/test/java/org/apache/druid/security/kerberos/KerberosAuthenticatorTest.java:
##########
@@ -76,15 +179,18 @@
DruidException exception = Assert.assertThrows(
DruidException.class,
- () -> new KerberosAuthenticator(
- TEST_SERVER_PRINCIPAL,
- TEST_SERVER_KEYTAB,
- TEST_AUTH_TO_LOCAL,
- "", // empty cookie signature secret
- TEST_AUTHORIZER_NAME,
- TEST_NAME,
- node
- )
+ () -> {
+ @SuppressWarnings("unused")
+ KerberosAuthenticator authenticator = new KerberosAuthenticator(
+ TEST_SERVER_PRINCIPAL,
+ TEST_SERVER_KEYTAB,
+ TEST_AUTH_TO_LOCAL,
+ "", // empty cookie signature secret
+ TEST_AUTHORIZER_NAME,
+ TEST_NAME,
+ node
+ );
Review Comment:
## CodeQL / Unread local variable
Variable 'KerberosAuthenticator authenticator' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/11254)
##########
extensions-core/druid-kerberos/src/test/java/org/apache/druid/security/kerberos/KerberosAuthenticatorTest.java:
##########
@@ -39,22 +54,110 @@
}
+ /**
+ * 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);
+ Assert.assertNull("Empty hadoop.auth cookie should be treated as no
cookie", token);
+
+ // 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);
+ Assert.assertNull("Missing hadoop.auth cookie should return null",
tokenForNoCookie);
+ }
+
+ 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 = Assert.assertThrows(
DruidException.class,
- () -> new KerberosAuthenticator(
- TEST_SERVER_PRINCIPAL,
- TEST_SERVER_KEYTAB,
- TEST_AUTH_TO_LOCAL,
- null, // null cookie signature secret
- TEST_AUTHORIZER_NAME,
- TEST_NAME,
- node
- )
+ () -> {
+ @SuppressWarnings("unused")
+ KerberosAuthenticator authenticator = new KerberosAuthenticator(
+ TEST_SERVER_PRINCIPAL,
+ TEST_SERVER_KEYTAB,
+ TEST_AUTH_TO_LOCAL,
+ null, // null cookie signature secret
+ TEST_AUTHORIZER_NAME,
+ TEST_NAME,
+ node
+ );
Review Comment:
## CodeQL / Unread local variable
Variable 'KerberosAuthenticator authenticator' is never read.
[Show more
details](https://github.com/apache/druid/security/code-scanning/11253)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
