exceptionfactory commented on code in PR #9566:
URL: https://github.com/apache/nifi/pull/9566#discussion_r1911093053
##########
nifi-registry/nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/security/authentication/jwt/JwtService.java:
##########
@@ -74,14 +79,24 @@ public String getUserIdentityFromToken(final String
base64EncodedToken) throws J
if (StringUtils.isEmpty(jws.getPayload().getIssuer())) {
throw new JwtException("No issuer available in token");
}
- return jws.getPayload().getSubject();
+
+ return jws;
} catch (JwtException e) {
- final String errorMessage = "There was an error validating the
JWT";
- logger.error(errorMessage, e);
- throw e;
+ throw new JwtException("There was an error validating the JWT", e);
}
}
+ public String getUserIdentityFromToken(final Jws<Claims> jws) throws
JwtException {
+ return jws.getPayload().getSubject();
+ }
+
+ public Set<String> getUserGroupsFromToken(final Jws<Claims> jws) throws
JwtException {
+ @SuppressWarnings("unchecked")
+ ArrayList<String> groupsString = jws.getPayload().get(GROUPS_CLAIM,
ArrayList.class);
Review Comment:
This should be declared as `List` instead of `ArrayList`:
```suggestion
final List<String> groupsString = jws.getPayload().get(GROUPS_CLAIM,
ArrayList.class);
```
##########
nifi-registry/nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/security/authentication/jwt/JwtIdentityProvider.java:
##########
@@ -61,16 +64,19 @@ public AuthenticationResponse
authenticate(AuthenticationRequest authenticationR
}
final Object credentials = authenticationRequest.getCredentials();
- String jwtAuthToken = credentials != null && credentials instanceof
String ? (String) credentials : null;
-
if (credentials == null) {
logger.info("JWT not found in authenticationRequest credentials,
returning null.");
return null;
}
try {
- final String jwtPrincipal =
jwtService.getUserIdentityFromToken(jwtAuthToken);
- return new AuthenticationResponse(jwtPrincipal, jwtPrincipal,
expiration, issuer);
+ String jwtAuthToken = credentials instanceof String ? (String)
credentials : null;
Review Comment:
With the null check for `credentials` on line 67, it looks like the
intermediate `jwtAuthToken` can be declared as `credentials.toString()`
##########
nifi-registry/nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/security/authentication/jwt/JwtService.java:
##########
@@ -74,14 +79,24 @@ public String getUserIdentityFromToken(final String
base64EncodedToken) throws J
if (StringUtils.isEmpty(jws.getPayload().getIssuer())) {
throw new JwtException("No issuer available in token");
}
- return jws.getPayload().getSubject();
+
+ return jws;
} catch (JwtException e) {
- final String errorMessage = "There was an error validating the
JWT";
- logger.error(errorMessage, e);
- throw e;
+ throw new JwtException("There was an error validating the JWT", e);
}
}
+ public String getUserIdentityFromToken(final Jws<Claims> jws) throws
JwtException {
+ return jws.getPayload().getSubject();
+ }
+
+ public Set<String> getUserGroupsFromToken(final Jws<Claims> jws) throws
JwtException {
+ @SuppressWarnings("unchecked")
+ ArrayList<String> groupsString = jws.getPayload().get(GROUPS_CLAIM,
ArrayList.class);
Review Comment:
It looks like this could return null, so the groups need to be null-checked
before being passed to `new HashSet<>()`
##########
nifi-registry/nifi-registry-core/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/security/authentication/oidc/StandardOidcIdentityProvider.java:
##########
@@ -401,6 +401,10 @@ private String
convertOIDCTokenToNiFiToken(OIDCTokenResponse response) throws Ba
String identityClaim = properties.getOidcClaimIdentifyingUser();
String identity = claimsSet.getStringClaim(identityClaim);
+ // Attempt to extract groups from the configured claim; default is
'groups'
+ String groupsClaim = properties.getOidcClaimGroups();
+ List<String> groups = claimsSet.getStringListClaim(groupsClaim);
Review Comment:
```suggestion
final String groupsClaim = properties.getOidcClaimGroups();
final List<String> groups =
claimsSet.getStringListClaim(groupsClaim);
```
--
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]
