fix(logout): Ensure we handle kerberos correctly - #1235
CarlSchwan wants to merge 1 commit into
Conversation
Where logout button doesn't make sense Signed-off-by: Carl Schwan <carl@carlschwan.eu>
| // Without the normal login form, /login redirects straight back into SAML, | ||
| // so a local logout would be pointless. | ||
| if ($slo === '' && !$this->appConfig->getAppValueBool('general-nextcloud_login_form')) { | ||
| return ''; |
There was a problem hiding this comment.
Maybe I am not seeing something, but this will still cause the same core.login.logout route, no? The server change does not, eh, change anything in this regard.
I also do not see any specific relation to Kerberos in the change.
There was a problem hiding this comment.
@blizzz server pr is updated now, to hide the login button if an empty string is returned from the user backend
(kerberos relation is that kerberos doesn't provide a logout url)
There was a problem hiding this comment.
(kerberos relation is that kerberos doesn't provide a logout url)
The logout URL is a configuration bit, a built-in SAML one does not have to provide one either.
There was a problem hiding this comment.
With a build-in SAML, we should be able to logout got the normal nextcloud login form and either login with another SAML provider or another login method
There was a problem hiding this comment.
With a build-in SAML, we should be able to logout got the normal nextcloud login form and either login with another SAML provider or another login method
With the own logout form, it redirects to the login page, which would redirect to the SSO page. When the session there is still active, it would redirect back into Nextcloud and the user would be logged in again. And if not they would stay with the SSO provider. All provided SAML is the only login option (so many conditions). Thus in some cases questionable user experience, but it would work. Tbh I do not recall the current behaviour when SLO is not configured 🙊
Having this not bound to kerberos is not an issue per se, the commit message is then misleading though and should be corrected. Think about your and our future selves!
3 participants
Where logout button doesn't make sense
Works best with nextcloud/server#64343
🤖 AI (if applicable)