necouchman commented on a change in pull request #455: GUACAMOLE-361: CAS 
global logout
URL: https://github.com/apache/guacamole-client/pull/455#discussion_r354485740
 
 

 ##########
 File path: 
extensions/guacamole-auth-cas/src/main/java/org/apache/guacamole/auth/cas/ticket/TicketValidationService.java
 ##########
 @@ -134,7 +140,13 @@
 
         } 
         catch (TicketValidationException e) {
-            throw new GuacamoleException("Ticket validation failed.", e);
+            throw new GuacamoleInvalidCredentialsException("Ticket validation 
failed.",
+                new CredentialsInfo(Arrays.asList(new Field[] {
+                 // Will automatically redirect the user
+                 // to the CAS logout page
+                 new CASLogoutField(confService.getLogoutURI())
+
+             })));
 
 Review comment:
   Yeah, I'm not sure this is the way to go, unfortunately, and this may be 
where we run into issues that need to be resolved with GUACAMOLE-680.  Not 100% 
certain about that, but I don't think we should be completely re-using this 
`TicketValidationException` for the purposes of logging out, for a couple of 
reasons.  First, the exception may occur at other times that indicates an 
actual error that we want to know about and handle in some other way.  Second, 
Why call the logout URI when the ticket is already invalid?  This seems like it 
would be happening *after* we've actually logged out of CAS??
   
   We should be able to trigger that `GuacamoleInvalidCredentialsException` 
from somewhere else based on the logout action, with that `CASLogoutField` 
object, but I don't know that this is the place.

----------------------------------------------------------------
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

Reply via email to