[GitHub] [knox] zeroflag commented on a diff in pull request #681: KNOX-2839 - Identity assertion provider handles Hadoop ProxyUser auth using the 'doAs' query parameter

[GitBox <gi...@apache.org>]

zeroflag commented on code in PR #681:
URL: https://github.com/apache/knox/pull/681#discussion_r1045683325


##########
gateway-provider-security-jwt/src/main/java/org/apache/knox/gateway/provider/federation/jwt/filter/AccessTokenFederationFilter.java:
##########
@@ -176,6 +172,6 @@ private Subject createSubjectFromToken(JWTToken token) {
     // To modify the Principals Set, the caller must have AuthPermission("modifyPrincipals").
     // To modify the public credential Set, the caller must have AuthPermission("modifyPublicCredentials").
     // To modify the private credential Set, the caller must have AuthPermission("modifyPrivateCredentials").
-    return new javax.security.auth.Subject(true, principals, emptySet, emptySet);
+    return new javax.security.auth.Subject(true, principals, Collections.emptySet(), Collections.emptySet());

Review Comment:
   LGTM with one note. 
   I'm not sure if it's a real problem, but since we're using `Collections.emptySet()` here, this means that adding a new principal after this point (e.g.: `subject.getPrincipals().add()`) to the subject might fail because the `Collection.emptySet()` is unmodifiable. Unlike the `new HashSet<>();`.



-- 
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: dev-unsubscribe@knox.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org