nfantone edited a comment on issue #455: GUACAMOLE-361: CAS global logout URL: https://github.com/apache/guacamole-client/pull/455#issuecomment-566077663 @mike-jumper @siacali Interesting discussion. A few things that I'd like to point out: > Similar to #346, perhaps a new event (not repurposing `guacLogout`) which is (1) **reliably invoked after logout has succeeded** and (2) results in reattempting authentication only if `preventDefault()` is not invoked? Again, the problem with both of those statements, as far as I understand it, is that the reloading of the view -which always happens immediately after Guacamole logout- _destroys and re-instantiates Angular controllers_. As a consequence, the bubbling and handling of the logout event (or any other event, for what matters) might get interrupted. An event-based solution, within the current design, would trigger this race condition. Some workarounds I can think of: 1. Move reload logic to extensions - don't do it in core. `guacLogout` itself could serve this purpose. 2. Add some configurable property to _prevent_ reloading (may default to current behaviour) and leverage #346 to handle CAS redirect. 3. Similar to your `preventDefault()` suggestion, delay reloading to allow event handlers to be invoked and _only_ proceed with refreshing the view if no handler prevented it. We could pass a custom function/event to `$broadcast` for this purpose (or even a plain `new Event()`). ```js function createGuacLogoutEvent() { let reloadPrevented = false; function preventReload() { reloadPrevented = true; } const logoutEvent = { get reloadPrevented() { return reloadPrevented; }, preventReload }; return Object.seal(logoutEvent); } const guacLogoutEvent = createGuacLogoutEvent(); $rootScope.$broadcast('guacLogoutSuccess', guacLogoutEvent); setTimeout(() => !guacLogoutEvent.reloadPrevented && $route.reload(), 4000); ``` > I don’t think there’s any other context that needs to be exposed, at least not for CAS (@nfantone, do you agree?). I do. We don't need any extra data to trigger reloading - although depending on _where_ we decide to read the logout URI from, it might be convenient to expose it in the event context.
---------------------------------------------------------------- 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
