siacali commented on issue #454: GUACAMOLE-793: CAS Provider returns Group - like LDAP Provider URL: https://github.com/apache/guacamole-client/pull/454#issuecomment-560215725 Hi Nick, Thanks for the review! I'll get these cleaned up and resubmitted shortly. On the thinking behind both the creation of the TokensAndGroups class, and leaving the group attribute in Tokens: 1. I have no idea how folks may be using (or planning to use) Tokens, so I left their implementation alone. I didn't want to change the end-user apparent functionality of the existing code, especially since the Jira story was to "implement" not to clean up. Did I take that too literally? :-) I did briefly consider re-implementing the underlying library so it would return the group attributes in a more sensible/structured fashion...decided to "fix" that if/when there's a Jira story raised on it... 2. I had considered letting ValidateTicket just pass back Tokens and handle the parsing of groups there, but that would make it less obvious to someone reading the code (perhaps with intentions of modifying the "Token" code) that Groups depended upon that functionality. This way, dependencies are reduced and the dirty work of dealing with both is right out in the open. Normally I wouldn't make a class to use once, but given the choice of "hiding" dependencies in two separate modules or making a simple class to make things clearer, well...you know what I chose. Considering that thinking, would you still prefer I restructure away TokensAndGroups?
---------------------------------------------------------------- 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: [email protected] With regards, Apache Git Services
