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

Implement OIDC session handling #5278

Open
1 task done
timhallmann opened this issue Oct 20, 2024 · 2 comments
Open
1 task done

Implement OIDC session handling #5278

timhallmann opened this issue Oct 20, 2024 · 2 comments

Comments

@timhallmann
Copy link

timhallmann commented Oct 20, 2024

Describe the feature you'd like

Once a user successfully logs into BookStack, their session becomes largely independent from the session at the OpenID Provider. While BookStack does support RP-Initiated Logout, it doesn't utilize the OPs session lifetime.

In the event that the OP sets a session lifetime, BookStack should deem a user as unauthenticated once the session expires. However, if the OP supplies a refresh_token, BookStack should use this token to refresh the session in the background, thus allowing the users session to persist. This refresh should be transparent to the user.

Describe the benefits this would bring to existing BookStack users

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

No. (While it might technically be possible to accomplish this through the logical theme system, that's likely to get quite ugly.)

Have you searched for an existing open/closed issue?

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

How long have you been using BookStack?

Not using yet, just scoping

Additional context

Prior discussion in #3715 (RP-Initiated Logout).

Related feature request: #5279 (Implement OIDC Front-Channel / Back-Channel Logout).

I'm open to implementing this feature myself, but I lack prior experience with Laravel and PHP, so I have some questions about the applications architecture and Laravel.

Here's my preliminary plan for managing the session lifetime:

  • In OidcService::processAccessTokenCallback: Store the token response in the session.
    • Is there a size limit on the session? Keycloaks tokens can be quite long.
  • In a new middleware: Check if it's an OIDC session. If so, validate the lifetime and decide whether the session is valid, a refresh is possible and was successful or whether a logout is necessary.
    • Would a middleware be the best place for this?
  • Potentially add an option to enable / disable session management.

Alternatively, implement custom sessions.

Notes / Considerations:

  • If a user remains idle long enough for the OP's session to expire, they'll be logged out (as intended), but this might occur while editing a page. Currently, if you edit something, log out in another tab, and then try to save the edit, BookStack shows a generic error message and the edit is lost. We should mitigate this, perhaps by checking a new endpoint like /ajax/logged-in before saving and prompting the user to reauthenticate if necessary.
  • There's little benefit in implementing the iframe polling described in OpenID Connect Session Management 1.0 as it doesn't work for Back-Channel Logouts.
@ssddanbrown
Copy link
Member

Thanks for the suggestion @timhallmann, and for the offer to contribute.
As you've touched on, some of this would require a more transparent/accessible view of sessions since their access/use is a little limited by default in Laravel. This would maybe be part of a wider, non-oidc-specific, look as sessions as some other issues/requests are related to this.

Overall though, I'm not keen to expand the maintenance & support surface area of OIDC without some proven demand for such additions.

There's essentially two feature requests in here (OIDC session handling and OIDC logout support), can you split this into separate feature requests to allow for each to gain support and discussion individually?

@timhallmann
Copy link
Author

As you've touched on, some of this would require a more transparent/accessible view of sessions since their access/use is a little limited by default in Laravel. This would maybe be part of a wider, non-oidc-specific, look as sessions as some other issues/requests are related to this.

I decided against proposing to implement custom sessions (on top of Laravels) as I didn't want to broaden the scope even further. However, if other issues also run into similar limitations, it would definitely be a better solution.

Overall though, I'm not keen to expand the maintenance & support surface area of OIDC without some proven demand for such additions.

Our use case: My department is looking into using BookStack for our internal user-facing documentation. We'd prefer consistent behavior across all user-facing applications and for the logout to be as simple as the login, especially on shared laboratory computers. Also, it makes our IT security happy. :)

Obviously, that might not be enough to justify the maintenance and support cost. If that's the case, I'll probably have to implement and maintain this myself.

There's essentially two feature requests in here (OIDC session handling and OIDC logout support), can you split this into separate feature requests to allow for each to gain support and discussion individually?

Sure, will do.

@timhallmann timhallmann changed the title Improve OIDC session handling and logout Implement OIDC session handling Oct 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants
@ssddanbrown @timhallmann and others