siacali commented on issue #454: GUACAMOLE-793: CAS Provider returns Group - like LDAP Provider URL: https://github.com/apache/guacamole-client/pull/454#issuecomment-564255506 > > I have no idea how folks may be using (or planning to use) Tokens, so I left their implementation alone. > > That's fine - I have no issue with these remaining available as a token. > > > 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. > > My current view is that this just adds more complexity than it's worth, and that doing a private method within the current class to parse it out is probably the way to go. I'd appreciate feedback from some of the other project members, though - @mike-jumper @jmuehlner @ceharris, any thoughts on this? I don't want to be the single voice pushing in one direction or another... @necouchman , I revisited this with the intentions of accepting your guidance and implementing it the way you wished to see it done. In getting into it, I discovered that he returned token names are all capitalized (and group names are case sensitive), so parsing those to create groups won't work quite right. It looks like the extra class is actually not a bad way to get it done. Also has the benefit of being much more efficient than the other method would be in terms of fewer loops, no string operations to undo combines that were just done, etc... Can we agree to keep it or do you have other guidance?
---------------------------------------------------------------- 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
