Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Logout of OIDC provider when logging out of Bookstack #3715

Closed
1 task done
hadiahmed098 opened this issue Sep 10, 2022 · 12 comments
Closed
1 task done

Logout of OIDC provider when logging out of Bookstack #3715

hadiahmed098 opened this issue Sep 10, 2022 · 12 comments

Comments

@hadiahmed098
Copy link

Describe the feature you'd like

When I press "Logout" on Bookstack, I only get logged out of Bookstack. If the authentication is through an OIDC provider, I'd want to be logged out of the provider as well.

There is an end_session_endpoint given in the OIDC discovery endpoint, so this shouldn't be too difficult to add.

Describe the benefits this would bring to existing BookStack users

This feature is how the SAML method works, and is how SSO is supposed to work - single sign on, single logout.

If another person wants to log onto the Bookstack service and we are using OIDC, I need to logout of Bookstack and manually clear my OIDC tokens before they can log on.

Can the goal of this request already be achieved via other means?

Yes? Technically, using SAML would allow this to work, but I was unable to successfully get SAML setup, whereas the OIDC setup was quick and painless.

Have you searched for an existing open/closed issue?

  • I have searched for existing issues and none cover my fundemental request

How long have you been using BookStack?

0 to 6 months

Additional context

No response

@ssddanbrown
Copy link
Member

@hadiahmed098 Thanks for the suggestion.
As usual with auth features, I'm hesitant to grow the scope.


Relevant specs:

All in implementers draft standard, support will vary across identity providers. Existing OIDC work has been done to final spec standards.
Not really sure on the general support expectation in regards to depth of support and support of the different specs above.
From a glance of the specs, back-channel looks quite technically ideal and compatible with BookStack's server model, but would require a fair amount of extra logic and looks like maybe it's a younger/lesser-used spec. Has the ability for the auth provide to logout all active RP sessions.
Session spec looks more established, but it has an iframe-session-polling element which looks generally not required but somewhat expected for a useful implementation, which is not ideal.

@hadiahmed098
Copy link
Author

I found this spec on the OpenID website, it is also in implementers draft.

(Note that the OpenID specification list doesn't include it under implementers draft, but this press release mentions it.

The RP-initiated logout would be much simpler to implement, and is exactly what I had in mind. Keycloak, a fairly popular SSO IdP does support this, and since it's just a GET or POST request, it should be simple to ignore if the IdP does not support it.

I would work on a PR, but my PHP is rather weak and I don't know how much help I could be. What do you think?

@hadiahmed098
Copy link
Author

The voting period to finalize the specification closes on Monday, so there should be an update on the specification after Tuesday, September 13. I'll keep an eye on it.

@hadiahmed098
Copy link
Author

@ssddanbrown The RP-Initiated logout procedure was promoted to a final specification.

https://openid.net/specs/openid-connect-rpinitiated-1_0.html

@nurradityam
Copy link

hi, i just recently tried OIDC method, then found this logout problem too.

keycloak 22.0.0
bookstack 23.06.1

@ssddanbrown
Copy link
Member

@hadiahmed098 Do you still desire this feature? If so, Would you be able to confirm the software/service you're using for OIDC? Just to help me understand the OIDC platform landscape, and logout support, of those that desire this.

@fuchsi3010
Copy link

hi, i'd desire this feature. i use a selfhosted keycloak instance for auth management.

@hadiahmed098
Copy link
Author

@ssddanbrown yes, this feature is still desired. I am running a self hosted Keycloak server v19.0.3 with Bookstack 23.06.1. I have successfully used the RP-initiated logout functionality across other self hosted sites.

@ssddanbrown
Copy link
Member

Okay, so only really demanded for keycloak so far.
Felt nervous with only a single platform in the mix, but checking support (list below) on some of my test OIDC provider systems makes me feel more comfortable, think it'd be fine to move forward with, especially as I should have a range of platforms to validate/test against.

Follow up question to all for feedback:

I'm thinking we'd enable logout automatically if the auth provider indicates support from autodiscovery.
There will still be the option to disable this (or define endpoint via setting if autodiscovery is not active).
There would still be an upgrade advisory in our update notes since it's a change to auth functionality.

Does that sound sensible? Or could you see that being a nuisance for new and existing setups using OIDC?
Or could there be security implications of enabling by default?

Sounds okay to me but I don't live in the realm of enterprise administration.


Platform Support Checks

Likely RP-Initiated logout support based upon end_session_endpoint existing via discovery:

Note: It could be there are in-auth-platform settings which can affect this.

  • Okta - Yes
  • Auth0 - No
  • Keycloak - Yes (Not checked but based upon feedback above)
  • Azure - Yes

@thespad
Copy link

thespad commented Oct 4, 2023

Would be useful for Authentik as well.

@hadiahmed098
Copy link
Author

Sorry for my late response, your plan sounds sensible.

Getting the endpoint via autodiscovery with options to define/disable this makes sense to me, and keeps the flow of the other OIDC settings. I can't imagine there are any security implications with this, as long as the Bookstack session is ended before we send the request to the IdP. As for nuisances, I can't speak to that unless admins/users were relying on staying logged into an IdP while logging out of Bookstack.

Also, after looking into it a bit Auth0 does support RP initiated endpoints as well.

@ssddanbrown ssddanbrown added this to the Next Feature Release milestone Dec 7, 2023
@ssddanbrown
Copy link
Member

This functionality has now been added via @joancyho in PR #4467, with further changes via PR #4714.
This will be part of the next feature release.

In contrary my previous comment, this will not be enabled by default via auto-discovery.
While testing I found that, as per the spec, some auth systems will be strict on the return path, which often requires configuration on the auth system side. We could omit that, but then the user journey presents quite a break relative to existing use.
Therefore, to avoid issues, this will be inactive by default but with option to load via autodiscovery, or specify a URL.

I successfully tested the added functionality across the following auth systems:

  • Okta
  • KeyCloak
  • Azure
  • Authentik
  • Auth0

Thanks again @hadiahmed098 for the original request and for pointing me to (and watching progress of) the spec.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

5 participants