Repository: knox Updated Branches: refs/heads/master 061951bc9 -> 8f7247523
KNOX-954 - Properly handle parsing errors for JWT tokens Project: http://git-wip-us.apache.org/repos/asf/knox/repo Commit: http://git-wip-us.apache.org/repos/asf/knox/commit/8f724752 Tree: http://git-wip-us.apache.org/repos/asf/knox/tree/8f724752 Diff: http://git-wip-us.apache.org/repos/asf/knox/diff/8f724752 Branch: refs/heads/master Commit: 8f72475234be14397ae0f0ea03cb91d9d6d56d5a Parents: 061951b Author: Sandeep More <m...@apache.org> Authored: Fri Jun 2 13:24:36 2017 -0400 Committer: Sandeep More <m...@apache.org> Committed: Fri Jun 2 13:24:36 2017 -0400 ---------------------------------------------------------------------- .../jwt/filter/JWTFederationFilter.java | 13 ++++++--- .../jwt/filter/SSOCookieFederationFilter.java | 13 ++++++--- .../federation/AbstractJWTFilterTest.java | 29 ++++++++++++++++++++ .../federation/JWTFederationFilterTest.java | 5 ++++ .../federation/SSOCookieProviderTest.java | 5 ++++ .../services/security/token/impl/JWTToken.java | 3 +- 6 files changed, 59 insertions(+), 9 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/knox/blob/8f724752/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/JWTFederationFilter.java ---------------------------------------------------------------------- diff --git a/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/JWTFederationFilter.java b/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/JWTFederationFilter.java index 001b056..e61ff80 100644 --- a/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/JWTFederationFilter.java +++ b/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/JWTFederationFilter.java @@ -29,6 +29,7 @@ import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; import java.io.IOException; +import java.text.ParseException; public class JWTFederationFilter extends AbstractJWTFilter { @@ -72,10 +73,14 @@ public class JWTFederationFilter extends AbstractJWTFilter { } if (wireToken != null) { - JWTToken token = new JWTToken(wireToken); - if (validateToken((HttpServletRequest)request, (HttpServletResponse)response, chain, token)) { - Subject subject = createSubjectFromToken(token); - continueWithEstablishedSecurityContext(subject, (HttpServletRequest)request, (HttpServletResponse)response, chain); + try { + JWTToken token = new JWTToken(wireToken); + if (validateToken((HttpServletRequest)request, (HttpServletResponse)response, chain, token)) { + Subject subject = createSubjectFromToken(token); + continueWithEstablishedSecurityContext(subject, (HttpServletRequest)request, (HttpServletResponse)response, chain); + } + } catch (ParseException ex) { + ((HttpServletResponse) response).sendError(HttpServletResponse.SC_UNAUTHORIZED); } } else { http://git-wip-us.apache.org/repos/asf/knox/blob/8f724752/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java ---------------------------------------------------------------------- diff --git a/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java b/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java index 771592c..205010c 100644 --- a/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java +++ b/gateway-provider-security-jwt/src/main/java/org/apache/hadoop/gateway/provider/federation/jwt/filter/SSOCookieFederationFilter.java @@ -18,6 +18,7 @@ package org.apache.hadoop.gateway.provider.federation.jwt.filter; import java.io.IOException; +import java.text.ParseException; import javax.security.auth.Subject; import javax.servlet.FilterChain; @@ -92,10 +93,14 @@ public class SSOCookieFederationFilter extends AbstractJWTFilter { ((HttpServletResponse) response).sendRedirect(loginURL); } else { - JWTToken token = new JWTToken(wireToken); - if (validateToken((HttpServletRequest)request, (HttpServletResponse)response, chain, token)) { - Subject subject = createSubjectFromToken(token); - continueWithEstablishedSecurityContext(subject, (HttpServletRequest)request, (HttpServletResponse)response, chain); + try { + JWTToken token = new JWTToken(wireToken); + if (validateToken((HttpServletRequest)request, (HttpServletResponse)response, chain, token)) { + Subject subject = createSubjectFromToken(token); + continueWithEstablishedSecurityContext(subject, (HttpServletRequest)request, (HttpServletResponse)response, chain); + } + } catch (ParseException ex) { + ((HttpServletResponse) response).sendRedirect(loginURL); } } } http://git-wip-us.apache.org/repos/asf/knox/blob/8f724752/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java index 76c4e4e..ab6c4bc 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/AbstractJWTFilterTest.java @@ -71,6 +71,7 @@ public abstract class AbstractJWTFilterTest { protected RSAPrivateKey privateKey = null; protected abstract void setTokenOnRequest(HttpServletRequest request, SignedJWT jwt); + protected abstract void setGarbledTokenOnRequest(HttpServletRequest request, SignedJWT jwt); protected abstract String getAudienceProperty(); @Before @@ -236,6 +237,34 @@ public abstract class AbstractJWTFilterTest { } } + @Test + public void testUnableToParseJWT() throws Exception { + try { + Properties props = getProperties(); + handler.init(new TestFilterConfig(props)); + + SignedJWT jwt = getJWT("bob",new Date(new Date().getTime() + 5000), privateKey); + + HttpServletRequest request = EasyMock.createNiceMock(HttpServletRequest.class); + setGarbledTokenOnRequest(request, jwt); + + EasyMock.expect(request.getRequestURL()).andReturn( + new StringBuffer(SERVICE_URL)).anyTimes(); + EasyMock.expect(request.getQueryString()).andReturn(null); + HttpServletResponse response = EasyMock.createNiceMock(HttpServletResponse.class); + EasyMock.expect(response.encodeRedirectURL(SERVICE_URL)).andReturn( + SERVICE_URL).anyTimes(); + EasyMock.replay(request); + + TestFilterChain chain = new TestFilterChain(); + handler.doFilter(request, response, chain); + Assert.assertTrue("doFilterCalled should not be false.", chain.doFilterCalled == false); + Assert.assertTrue("No Subject should be returned.", chain.subject == null); + } catch (ServletException se) { + fail("Should NOT have thrown a ServletException."); + } + } + protected Properties getProperties() { Properties props = new Properties(); props.setProperty( http://git-wip-us.apache.org/repos/asf/knox/blob/8f724752/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java index 5da5d55..d1d9b56 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/JWTFederationFilterTest.java @@ -42,6 +42,11 @@ public class JWTFederationFilterTest extends AbstractJWTFilterTest { EasyMock.expect(request.getHeader("Authorization")).andReturn(token); } + protected void setGarbledTokenOnRequest(HttpServletRequest request, SignedJWT jwt) { + String token = "Bearer " + "ljm" + jwt.serialize(); + EasyMock.expect(request.getHeader("Authorization")).andReturn(token); + } + protected String getAudienceProperty() { return TestJWTFederationFilter.KNOX_TOKEN_AUDIENCES; } http://git-wip-us.apache.org/repos/asf/knox/blob/8f724752/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java ---------------------------------------------------------------------- diff --git a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java index 4006051..98d8207 100644 --- a/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java +++ b/gateway-provider-security-jwt/src/test/java/org/apache/hadoop/gateway/provider/federation/SSOCookieProviderTest.java @@ -57,6 +57,11 @@ public class SSOCookieProviderTest extends AbstractJWTFilterTest { EasyMock.expect(request.getCookies()).andReturn(new Cookie[] { cookie }); } + protected void setGarbledTokenOnRequest(HttpServletRequest request, SignedJWT jwt) { + Cookie cookie = new Cookie("hadoop-jwt", "ljm" + jwt.serialize()); + EasyMock.expect(request.getCookies()).andReturn(new Cookie[] { cookie }); + } + protected String getAudienceProperty() { return TestSSOCookieFederationProvider.SSO_EXPECTED_AUDIENCES; } http://git-wip-us.apache.org/repos/asf/knox/blob/8f724752/gateway-spi/src/main/java/org/apache/hadoop/gateway/services/security/token/impl/JWTToken.java ---------------------------------------------------------------------- diff --git a/gateway-spi/src/main/java/org/apache/hadoop/gateway/services/security/token/impl/JWTToken.java b/gateway-spi/src/main/java/org/apache/hadoop/gateway/services/security/token/impl/JWTToken.java index e0090c7..cc2ccfe 100644 --- a/gateway-spi/src/main/java/org/apache/hadoop/gateway/services/security/token/impl/JWTToken.java +++ b/gateway-spi/src/main/java/org/apache/hadoop/gateway/services/security/token/impl/JWTToken.java @@ -49,11 +49,12 @@ public class JWTToken implements JWT { } } - public JWTToken(String serializedJWT) { + public JWTToken(String serializedJWT) throws ParseException { try { jwt = SignedJWT.parse(serializedJWT); } catch (ParseException e) { log.unableToParseToken(e); + throw e; } }
