scpcom commented on code in PR #989:
URL: https://github.com/apache/guacamole-client/pull/989#discussion_r1620867014
##########
extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/user/SAMLAuthenticatedUser.java:
##########
@@ -104,6 +104,23 @@ private Set<String> getGroups(AssertedIdentity identity)
}
+ private String getUser(AssertedIdentity identity)
+ throws GuacamoleException {
+
+ String samlUserAttribute = confService.getUserAttribute();
+ List<String> samlUser = null;
+
+ if (samlUserAttribute == null || samlUserAttribute.isEmpty())
+ return identity.getUsername();
+
+ samlUser = identity.getAttributes().get(samlUserAttribute);
+ if (samlUser == null || samlUser.isEmpty())
+ return identity.getUsername();
+
+ return samlUser.get(0);
+
+ }
+
Review Comment:
I agree, we could place the username assignment here:
https://github.com/scpcom/guacamole-client/blob/guacamole-auth-sso-saml-user-attribute/extensions/guacamole-auth-sso/modules/guacamole-auth-sso-saml/src/main/java/org/apache/guacamole/auth/saml/acs/AssertedIdentity.java#L93
But since "NameId" is not an attribute it may not be a good idea to set this
as default in the configuration (response.getNameId() is not the same as
response.getAttributes().get("NameId")).
Instead of checking for null with still would ned to check for "NameId".
May be we could use "" as default, so we only need to check for isEmpty()
--
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
