[
https://issues.apache.org/jira/browse/HADOOP-11717?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14365135#comment-14365135
]
Kai Zheng commented on HADOOP-11717:
------------------------------------
More comments to complete the review, mainly about tests.
1. The new handler looks little heavier to me. One thing we can do for now is
to have a utility class like {{JwtTokenUtil}} and remove at least
{{getPublicKey}} related logic and variables there. So some related tests like
{{testValidPEM}} will not need a handler instance. How about having
{{parsePublicKey}} (instead of {{getPublicKey}}), {{parseAudiences}}, and
{{parseJwtToken}} (even trivial, restore JWT from a string like from cookie)
for the new utility class if it sounds better to have ?
2. It would be good not to couple the new test with
{{KerberosSecurityTestcase}} since all the test cases won't relate to Kerberos
at all.
3. For all the handler really logic tests, better to move the following to
{{setup}} or {{teardown}}.
{code}
JWTRedirectAuthenticationHandler handler = new
JWTRedirectAuthenticationHandler();
...
handler.destroy();
{code}
4. All the handler logic tests are different in token preparing. It's possible
to have the following in a function where token is a parameter to avoid
repeating.
{code}
+ Properties props = getProperties();
+ handler.init(props);
+
+ SignedJWT jwt = getJWT("bob", new Date(new Date().getTime() + 5000),
+ privateKey);
+
+ Cookie cookie = new Cookie("hadoop-jwt", jwt.serialize());
+ HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+ Mockito.when(request.getCookies()).thenReturn(new Cookie[] { cookie });
+ Mockito.when(request.getRequestURL()).thenReturn(
+ new StringBuffer(SERVICE_URL));
+ HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+ Mockito.when(response.encodeRedirectURL(SERVICE_URL)).thenReturn(
+ SERVICE_URL);
+
+ AuthenticationToken token = handler.alternateAuthenticate(request,
+ response);
{code}
5. In the tests, we have repeated values like "bar", "bob" here and there. How
about having variables for them ?
6. In the following codes, {{aud}} and {{sigInput}} aren't really used.
{code}
+ List<String> aud = new ArrayList<String>();
+ aud.add("bar");
+ claimsSet.setAudience("bar");
+
+ JWSHeader header = new JWSHeader.Builder(JWSAlgorithm.RS256).build();
+
+ SignedJWT signedJWT = new SignedJWT(header, claimsSet);
+ Base64URL sigInput = Base64URL.encode(signedJWT.getSigningInput());
+ JWSSigner signer = new RSASSASigner(privateKey);
+
+ signedJWT.sign(signer);
+
+ return signedJWT;
{code}
> Add Redirecting WebSSO behavior with JWT Token in Hadoop Auth
> -------------------------------------------------------------
>
> Key: HADOOP-11717
> URL: https://issues.apache.org/jira/browse/HADOOP-11717
> Project: Hadoop Common
> Issue Type: Improvement
> Components: security
> Reporter: Larry McCay
> Assignee: Larry McCay
> Attachments: HADOOP-11717-1.patch, HADOOP-11717-2.patch,
> HADOOP-11717-3.patch, HADOOP-11717-4.patch, HADOOP-11717-5.patch
>
>
> Extend AltKerberosAuthenticationHandler to provide WebSSO flow for UIs.
> The actual authentication is done by some external service that the handler
> will redirect to when there is no hadoop.auth cookie and no JWT token found
> in the incoming request.
> Using JWT provides a number of benefits:
> * It is not tied to any specific authentication mechanism - so buys us many
> SSO integrations
> * It is cryptographically verifiable for determining whether it can be trusted
> * Checking for expiration allows for a limited lifetime and window for
> compromised use
> This will introduce the use of nimbus-jose-jwt library for processing,
> validating and parsing JWT tokens.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
