[GitHub] [guacamole-client] necouchman commented on a change in pull request #454: GUACAMOLE-793: CAS Provider returns Group - like LDAP Provider

[GitBox <gi...@apache.org>]

necouchman commented on a change in pull request #454: GUACAMOLE-793: CAS Provider returns Group - like LDAP Provider
URL: https://github.com/apache/guacamole-client/pull/454#discussion_r384883052
 
 

 ##########
 File path: extensions/guacamole-auth-cas/src/main/java/org/apache/guacamole/auth/cas/AuthenticationProviderService.java
 ##########
 @@ -81,11 +86,13 @@ public CASAuthenticatedUser authenticateUser(Credentials credentials)
         if (request != null) {
             String ticket = request.getParameter(CASTicketField.PARAMETER_NAME);
             if (ticket != null) {
-                Map<String, String> tokens = ticketService.validateTicket(ticket, credentials);
+                TokensAndGroups tokensAndGroups = ticketService.validateTicket(ticket, credentials);
 
 Review comment:
   I think my record will show that I like classes as much as the next guy, as I've implemented plenty of them! ;-)
   
   In this case, though, it just seems like a lot of overhead to essentially accomplish splitting out the contents of an array into groups and other stuff.  To me it is simpler and more maintainable to have a private method within the existing class accomplish the same thing.
   
   Just my two bits...okay, I've put in way more than that on this topic...

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services