From 989ed8d0c206b95d793efa005830722f4cc4f8be Mon Sep 17 00:00:00 2001 From: Atul Mohan Date: Wed, 23 Aug 2023 02:09:23 -0700 Subject: [PATCH] Fix null check for JWT claims (#14872) --- .../druid/security/pac4j/JwtAuthFilter.java | 2 +- .../security/pac4j/JwtAuthenticatorTest.java | 96 +++++++++++++++++++ 2 files changed, 97 insertions(+), 1 deletion(-) diff --git a/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/JwtAuthFilter.java b/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/JwtAuthFilter.java index 826e2a479ef..a0132615ff5 100644 --- a/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/JwtAuthFilter.java +++ b/extensions-core/druid-pac4j/src/main/java/org/apache/druid/security/pac4j/JwtAuthFilter.java @@ -83,7 +83,7 @@ public class JwtAuthFilter implements Filter // Parses the JWT and performs the ID Token validation specified in the OpenID spec: https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation IDTokenClaimsSet claims = tokenValidator.validate(JWTParser.parse(idToken.get()), null); if (claims != null) { - Optional claim = Optional.of(claims.getStringClaim(oidcConfig.getOidcClaim())); + Optional claim = Optional.ofNullable(claims.getStringClaim(oidcConfig.getOidcClaim())); if (claim.isPresent()) { LOG.debug("Authentication successful for " + oidcConfig.getClientID()); diff --git a/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/JwtAuthenticatorTest.java b/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/JwtAuthenticatorTest.java index fee99139091..73704731f2a 100644 --- a/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/JwtAuthenticatorTest.java +++ b/extensions-core/druid-pac4j/src/test/java/org/apache/druid/security/pac4j/JwtAuthenticatorTest.java @@ -20,7 +20,13 @@ package org.apache.druid.security.pac4j; import com.google.common.collect.ImmutableMap; +import com.nimbusds.jose.JOSEException; +import com.nimbusds.jose.proc.BadJOSEException; +import com.nimbusds.oauth2.sdk.id.Issuer; +import com.nimbusds.oauth2.sdk.id.Subject; +import com.nimbusds.openid.connect.sdk.claims.IDTokenClaimsSet; import org.apache.druid.server.security.AuthConfig; +import org.apache.druid.server.security.AuthenticationResult; import org.easymock.EasyMock; import org.junit.Assert; import org.junit.Test; @@ -31,9 +37,12 @@ import javax.servlet.ServletException; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.util.Collections; public class JwtAuthenticatorTest { + private static final String DUMMY_BEARER_TOKEN_HEADER = "Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJzdWIiOiIxMjM0NTY3ODkwIiwibmFtZSI6IkpvaG4gRG9lIiwiaWF0IjoxNTE2MjM5MDIyfQ.SflKxwRJSMeKKF2QT4fwpMeJf36POk6yJV_adQssw5c"; + @Test public void testBearerToken() throws IOException, ServletException @@ -87,4 +96,91 @@ public class JwtAuthenticatorTest EasyMock.verify(req, resp, filterChain); } + + @Test + public void testValidClaim() + throws IOException, ServletException, BadJOSEException, JOSEException + { + HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class); + EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(null); + EasyMock.expect(req.getHeader("Authorization")).andReturn(DUMMY_BEARER_TOKEN_HEADER); + req.setAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT, new AuthenticationResult("foo", "allowAll", "jwt", null)); + EasyMock.expectLastCall().times(1); + EasyMock.replay(req); + + OIDCConfig configuration = EasyMock.createMock(OIDCConfig.class); + EasyMock.expect(configuration.getOidcClaim()).andReturn("iss"); + EasyMock.expect(configuration.getClientID()).andReturn("testClient"); + EasyMock.replay(configuration); + + TokenValidator tokenValidator = EasyMock.createMock(TokenValidator.class); + EasyMock.expect(tokenValidator.validate(EasyMock.anyObject(), EasyMock.anyObject())) + .andReturn(new IDTokenClaimsSet(new Issuer("foo"), + new Subject("testsub"), + Collections.emptyList(), + null, + null + )); + EasyMock.replay(tokenValidator); + + 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); + + + JwtAuthenticator jwtAuthenticator = new JwtAuthenticator("jwt", "allowAll", configuration); + JwtAuthFilter authFilter = new JwtAuthFilter("allowAll", "jwt", configuration, tokenValidator); + authFilter.doFilter(req, resp, filterChain); + + EasyMock.verify(req, resp, filterChain); + Assert.assertEquals(jwtAuthenticator.getFilterClass(), JwtAuthFilter.class); + Assert.assertNull(jwtAuthenticator.getInitParameters()); + Assert.assertNull(jwtAuthenticator.authenticateJDBCContext(ImmutableMap.of())); + } + + @Test + public void testEmptyClaim() + throws IOException, ServletException, BadJOSEException, JOSEException + { + OIDCConfig configuration = EasyMock.createMock(OIDCConfig.class); + + HttpServletRequest req = EasyMock.createMock(HttpServletRequest.class); + EasyMock.expect(req.getAttribute(AuthConfig.DRUID_AUTHENTICATION_RESULT)).andReturn(null); + EasyMock.expect(req.getHeader("Authorization")).andReturn(DUMMY_BEARER_TOKEN_HEADER); + + EasyMock.replay(req); + + TokenValidator tokenValidator = EasyMock.createMock(TokenValidator.class); + // This doesn't return any claims for the default scope + EasyMock.expect(tokenValidator.validate(EasyMock.anyObject(), EasyMock.anyObject())) + .andReturn(new IDTokenClaimsSet(new Issuer("test"), + new Subject("testsub"), + Collections.emptyList(), + null, + null + )); + EasyMock.replay(tokenValidator); + + 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); + + + JwtAuthenticator jwtAuthenticator = new JwtAuthenticator("jwt", "allowAll", configuration); + JwtAuthFilter authFilter = new JwtAuthFilter("allowAll", "jwt", configuration, tokenValidator); + authFilter.doFilter(req, resp, filterChain); + + EasyMock.verify(req, resp, filterChain); + Assert.assertEquals(jwtAuthenticator.getFilterClass(), JwtAuthFilter.class); + Assert.assertNull(jwtAuthenticator.getInitParameters()); + Assert.assertNull(jwtAuthenticator.authenticateJDBCContext(ImmutableMap.of())); + } }