necouchman commented on code in PR #905:
URL: https://github.com/apache/guacamole-client/pull/905#discussion_r1313522024
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/ConfigurationService.java:
##########
@@ -125,6 +133,15 @@ public class ConfigurationService {
public String getName() { return "openid-groups-claim-type"; }
};
+ /**
+ * The claims within any valid JWT that should be mapped to
+ * the authenticated user's tokens, as configured with
guacamole.properties.
+ */
+ private static final StringListProperty OPENID_ATTRIBUTES_CLAIM_TYPE =
+ new StringListProperty() {
+ @Override
+ public String getName() { return
"openid-attributes-claim-type"; }
+ };
Review Comment:
This is a very minor thing, but please follow the established style
throughout the prior property declarations, particularly as it relates to
indentation.
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java:
##########
@@ -201,4 +209,53 @@ public Set<String> processGroups(JwtClaims claims) throws
GuacamoleException {
// Could not retrieve groups from JWT
return Collections.emptySet();
}
+
+ /**
+ * Parses the given JwtClaims, returning the attributes contained
+ * therein, as defined by the attributes claim type given in
+ * guacamole.properties. If the attributes claim type is missing or
+ * is invalid, an empty set is returned.
+ *
+ * @param claims
+ * A valid JwtClaims to extract attributes from.
+ *
+ * @return
+ * A Map of String,String representing the attributes and values
+ * from the OpenID provider point of view, or an empty Map if
+ * claim is not valid or the attributes claim type is missing.
+ *
+ * @throws GuacamoleException
+ * If guacamole.properties could not be parsed.
+ */
+ public Map<String, String> processAttributes(JwtClaims claims) throws
GuacamoleException {
+ List<String> attributesClaim = confService.getAttributesClaimType();
+
+ if (claims != null && !attributesClaim.isEmpty()) {
+ try {
+ logger.debug("Iterating over attributes claim list : {}",
attributesClaim);
+ Map<String, String> tokens = new HashMap<String,
String>(attributesClaim.size());
+ for (String key: attributesClaim) {
+ String oidcAttr = claims.getStringClaimValue(key);
+ if (oidcAttr != null && !oidcAttr.isEmpty()) {
+ String tokenName = TokenName.canonicalize(key,
OIDC_ATTRIBUTE_TOKEN_PREFIX);
+ tokens.put(tokenName, oidcAttr);
+ logger.debug("Claim {} found and set to {} as {}",
key, tokenName, oidcAttr);
+ }
+ else {
+ logger.debug("Claim {} not found in JWT.", key);
+ }
+ }
+ logger.debug("Processed attributes map : {}", tokens);
+ return Collections.unmodifiableMap(tokens);
+ }
+ catch (MalformedClaimException e) {
+ logger.info("Rejected OpenID token with malformed claim: {}",
e.getMessage());
+ logger.debug("Malformed claim within received JWT.", e);
+ }
+ }
Review Comment:
It might be good to have a few inline comments in this section to document
what you're doing. The `logger.debug()` statements kind of accomplish that, but
if you look at the methods above you'll see there are at least some one-line
comments to give hints for someone reading through the code.
--
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-unsubscr...@guacamole.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
