necouchman commented on code in PR #989:
URL: https://github.com/apache/guacamole-client/pull/989#discussion_r1620734322
##########
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:
Instead of doing this, here, with a `getUser()` method, what if the
`AssertedIdentity` object gets initialized with what we expect to be the
username from the outset? The `AssertedIdentity` class pulls the `NameId` value
out of the SAML response and assigns it to the `username` variable - I'm
thinking a better approach would be to somehow let `AssertedIdentity` know what
attribute should be used for the username. This may require `@Inject`ing the
configuration service into that class, but would make sure that the `username`
actually contains what we expect it to contain - and then there wouldn't be a
need to adjust calls to anything else.
I don't know of the `NameId` value needs to be kept for any other reason,
but, if so, a separate variable could be added to `AssertedIdentity` for that.
This would also allow you, in the configuration service, to specify `NameId`
as the default and not have to return `null`, check for `null`, etc.
--
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]
