[GitHub] [guacamole-client] mildis commented on a change in pull request #564: GUACAMOLE-1172: add logic to retrieve groups from OIDC token

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

mildis commented on a change in pull request #564:
URL: https://github.com/apache/guacamole-client/pull/564#discussion_r484381427



##########
File path: extensions/guacamole-auth-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java
##########
@@ -132,9 +108,79 @@ public String processUsername(String token) throws GuacamoleException {
             logger.debug("Malformed claim within received JWT.", e);
         }
 
+
+	return claims;
+    }
+
+    /**
+     * Validates and parses the given ID token, returning the username contained
+     * therein, as defined by the username claim type given in
+     * guacamole.properties. If the username claim type is missing or the ID
+     * token is invalid, null is returned.
+     *
+     * @param token
+     *     The ID token to validate and parse.
+     *
+     * @return
+     *     The username contained within the given ID token, or null if the ID
+     *     token is not valid or the username claim type is missing,
+     *
+     * @throws GuacamoleException
+     *     If guacamole.properties could not be parsed.
+     */
+    public String processUsername(String token) throws GuacamoleException {
+        String usernameClaim = confService.getUsernameClaimType();
+
+        JwtClaims claims = validateToken(token);

Review comment:
       At first I tough of maintaining a `synchronizedSet` of valid JWT (via `claims.getJwtId()`) in `validateToken(String)` and bypassing the whole validation is the JWT id was part of the Set.
   However, as every JWT has its own lifespan, it would involved lots of housekeeping of a time-based cache.
   And being able to call `getJwtId()` to search in the already validated Set means that most of the validation process has been done (create the `JwtConsumer` and calling `processToClaims()` the be able to get the `jit` claim).
   My bet is that it's not worth it.




----------------------------------------------------------------
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.

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