mike-jumper commented on code in PR #905:
URL: https://github.com/apache/guacamole-client/pull/905#discussion_r1290301491
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/ConfigurationService.java:
##########
@@ -126,6 +128,12 @@ public class ConfigurationService {
};
+ private static final StringGuacamoleProperty OPENID_ATTRIBUTES_CLAIM_TYPE =
+ new StringGuacamoleProperty() {
+ @Override
+ public String getName() { return
"openid-attributes-claim-type"; }
+ };
Review Comment:
It would be better to use `StringListProperty` for this, rather than
manually parse the string into an array each time a claim is being handled.
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/token/TokenValidationService.java:
##########
@@ -201,4 +214,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 groups 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 {
+ String attributesClaim = confService.getAttributesClaimType();
+
+ if (claims != null && !attributesClaim.isEmpty()) {
+ try {
+ String[] attrs = DELIMITER_PATTERN.split(attributesClaim);
+ logger.debug("Iterating over attributes claim list : {}",
attrs.toString());
+ Map<String, String> tokens = new HashMap<String,
String>(attrs.length);
+ for (String key: attrs) {
+ String oidcAttr = claims.getStringClaimValue(key);
+ if (oidcAttr != null && !oidcAttr.isEmpty()) {
+ String tokenName = TokenName.canonicalize(key,
OIDC_ATTRIBUTE_TOKEN_PREFIX);
+ tokens.put(tokenName, oidcAttr.toString());
+ logger.debug("Claim {} found and set to {} as {}",
key, tokenName, oidcAttr.toString());
+ } else {
Review Comment:
Please do not cuddle the `else`. See established code and:
https://guacamole.apache.org/guac-style/#braces
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/ConfigurationService.java:
##########
@@ -326,6 +334,10 @@ public String getGroupsClaimType() throws
GuacamoleException {
return environment.getProperty(OPENID_GROUPS_CLAIM_TYPE,
DEFAULT_GROUPS_CLAIM_TYPE);
}
+ public String getAttributesClaimType() throws GuacamoleException {
Review Comment:
Same here - please document.
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/ConfigurationService.java:
##########
@@ -45,6 +45,8 @@ public class ConfigurationService {
*/
private static final String DEFAULT_GROUPS_CLAIM_TYPE = "groups";
+ private static final String DEFAULT_ATTRIBUTES_CLAIM_TYPE = "";
Review Comment:
And here (JavaDoc).
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-openid/src/main/java/org/apache/guacamole/auth/openid/conf/ConfigurationService.java:
##########
@@ -126,6 +128,12 @@ public class ConfigurationService {
};
+ private static final StringGuacamoleProperty OPENID_ATTRIBUTES_CLAIM_TYPE =
Review Comment:
All newly-added functions, member variables, etc. need to be documented with
JavaDoc comments (see surrounding 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
