mirror of
https://github.com/BookStackApp/BookStack.git
synced 2026-02-08 19:06:06 +03:00
SAML SP-initiated Single Logout (SLO) is not invalidating sessions prior to redirecting to the IdP #4355
Closed
opened 2026-02-05 08:38:55 +03:00 by OVERLORD
·
4 comments
No Branch/Tag Specified
development
further_theme_development
l10n_development
release
llm_only
vectors
v25-11
docker_env
drawio_rendering
user_permissions
ldap_host_failover
svg_image
prosemirror
captcha_example
fix/video-export
v25.12.3
v25.12.2
v25.12.1
v25.12
v25.11.6
v25.11.5
v25.11.4
v24.11.4
v25.11.3
v25.11.2
v25.11.1
v25.11
v25.07.3
v25.07.2
v25.07.1
v25.07
v25.05.2
v25.05.1
v25.05
v25.02.5
v25.02.4
v25.02.3
v25.02.2
v25.02.1
v25.02
v24.12.1
v24.12
v24.10.3
v24.10.2
v24.10.1
v24.10
v24.05.4
v24.05.3
v24.05.2
v24.05.1
v24.05
v24.02.3
v24.02.2
v24.02.1
v24.02
v23.12.3
v23.12.2
v23.12.1
v23.12
v23.10.4
v23.10.3
v23.10.2
v23.10.1
v23.10
v23.08.3
v23.08.2
v23.08.1
v23.08
v23.06.2
v23.06.1
v23.06
v23.05.2
v23.05.1
v23.05
v23.02.3
v23.02.2
v23.02.1
v23.02
v23.01.1
v23.01
v22.11.1
v22.11
v22.10.2
v22.10.1
v22.10
v22.09.1
v22.09
v22.07.3
v22.07.2
v22.07.1
v22.07
v22.06.2
v22.06.1
v22.06
v22.04.2
v22.04.1
v22.04
v22.03.1
v22.03
v22.02.3
v22.02.2
v22.02.1
v22.02
v21.12.5
v21.12.4
v21.12.3
v21.12.2
v21.12.1
v21.12
v21.11.3
v21.11.2
v21.11.1
v21.11
v21.10.3
v21.10.2
v21.10.1
v21.10
v21.08.6
v21.08.5
v21.08.4
v21.08.3
v21.08.2
v21.08.1
v21.08
v21.05.4
v21.05.3
v21.05.2
v21.05.1
v21.05
v21.04.6
v21.04.5
v21.04.4
v21.04.3
v21.04.2
v21.04.1
v21.04
v0.31.8
v0.31.7
v0.31.6
v0.31.5
v0.31.4
v0.31.3
v0.31.2
v0.31.1
v0.31.0
v0.30.7
v0.30.6
v0.30.5
v0.30.4
v0.30.3
v0.30.2
v0.30.1
v0.30.0
v0.29.3
v0.29.2
v0.29.1
v0.29.0
v0.28.3
v0.28.2
v0.28.1
v0.28.0
v0.27.5
v0.27.4
v0.27.3
v0.27.2
v0.27.1
v0.27
v0.26.4
v0.26.3
v0.26.2
v0.26.1
v0.26.0
v0.25.5
v0.25.4
v0.25.3
v0.25.2
v0.25.1
v0.25.0
v0.24.3
v0.24.2
v0.24.1
v0.24.0
v0.23.2
v0.23.1
v0.23.0
v0.22.0
v0.21.0
v0.20.3
v0.20.2
v0.20.1
v0.20.0
v0.19.0
v0.18.5
v0.18.4
v0.18.3
v0.18.2
v0.18.1
v0.18.0
v0.17.4
v0.17.3
v0.17.2
v0.17.1
v0.17.0
v0.16.3
v0.16.2
v0.16.1
v0.16.0
v0.15.3
v0.15.2
v0.15.1
v0.15.0
v0.14.3
v0.14.2
v0.14.1
v0.14.0
v0.13.1
v0.13.0
v0.12.2
v0.12.1
v0.12.0
v0.11.2
v0.11.1
v0.11.0
v0.10.0
v0.9.3
v0.9.2
v0.9.1
v0.9.0
v0.8.2
v0.8.1
v0.8.0
v0.7.6
v0.7.5
v0.7.4
v0.7.3
0.7.2
v.0.7.1
v0.7.0
v0.6.3
v0.6.2
v0.6.1
v0.6.0
v0.5.0
Labels
Clear labels
🎨 Design
📖 Docs Update
🐛 Bug
🐛 Bug
:cat2:🐈 Possible duplicate
💿 Database
☕ Open to discussion
💻 Front-End
🐕 Support
🚪 Authentication
🌍 Translations
🔌 API Task
🏭 Back-End
⛲ Upstream
🔨 Feature Request
🛠️ Enhancement
🛠️ Enhancement
🛠️ Enhancement
❤️ Happy feedback
🔒 Security
🔍 Pending Validation
💆 UX
📝 WYSIWYG Editor
🌔 Out of scope
🔩 API Request
:octocat: Admin/Meta
🖌️ View Customization
❓ Question
🚀 Priority
🛡️ Blocked
🚚 Export System
♿ A11y
🔧 Maintenance
> Markdown Editor
pull-request
Mirrored from GitHub Pull Request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: starred/BookStack#4355
Reference in New Issue
Block a user
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Delete Branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Originally created by @akkornel on GitHub (Dec 6, 2023).
Describe the Bug
Hello! I'd like to report an issue with SAML Logout: When initiating a logout from within Bookstack, Bookstack is not invalidating its session before sending the user to the SAML Identity Provider (IdP).
SAML Logout can work in a couple of different ways:
The logout can be initiated from outside of Bookstack. In this case, the SAML 2.0 IdP (Identity Provider) sends a request to Bookstack, asking for the user to be logged out. This is IdP-initiated Single Logout (SLO).
The logout can be initiated from inside of Bookstack. In this case, Bookstack needs to invalidate its session, and then redirect the user to the IdP to continue the logout process. This is SP-initiated Single Logout (SLO).
I'm reporting a problem the the second method, SP-initiated SLO.
Steps to Reproduce
Expected Behaviour
I expected to be prompted to log in to Bookstack. Instead, I was presented with the Bookstack main page; my Bookstack session was still valid.
Screenshots or Additional Context
I apologize in advance: This is a messy issue, with a lot of moving parts. If any part of my report is confusing, please let me know!
In the SAML V2.0 Technical Overview, SAML 2.0 Logout is defined in Section 5.3. Section 5.3.2 has a good diagram, showing both IdP-initiated and SP-initiated SLO.
In the diagram, Service Provider
sp1.example.comis going through SP-initiated SLO: The user (or, really, their web browser) has asked for a logout (Step 1), and the Service Provider (Bookstack) is redirecting the user to the IdP (Step 2). The redirect is working fine.What's missing is the session invalidation, as described in the text below the diagram:
The emphasized text is what I'm talking about: In case something goes wrong with the logout process (for example, maybe the IdP is down), the session (on the Bookstack side) should be "destroyed". That being said, Steps 5 and 6 make this more complicated:
The reason I say this is confusing is because, even though the authentication session state is supposed to be destroyed in Step 1, the SP (Bookstack) is still supposed to store enough information to be able to authenticate the message from the IdP in Step 5, and do something with the user in Step 6 (which I think, for Bookstack, is to return the user to the main page).
I understand that you're using php-saml, and looking through their documentation, I see there isn't any mention of the need to destroy local authentication session state (Step 1 from the top of this section). I'm wondering if there should also be an Issue raised with the php-saml folks.
Browser Details
No response
Exact BookStack Version
23.10.4
@ssddanbrown commented on GitHub (Dec 6, 2023):
Thanks for raising @akkornel, I agree this is currently not as per spec. I get the feeling I misunderstood the point of logout, by step 6 of the diagram specifically labelled as "Logged out" between the SP and browser.
I'll assign this to be addressed for the next BookStack feature release.
Thanks for the effort of the detailed explanation given in this issue.
@radiantwave commented on GitHub (Dec 7, 2023):
#2553 related?
Have the same issue right now.
This is pretty drastic for me, since users can't change accounts at all.
Authenticating with another user doesn't log you into the desired account because the old sessions never end.
@ssddanbrown commented on GitHub (Dec 7, 2023):
@radiantwave Related, but not the same. In a full logout flow you should be returned to BookStack eventually where you'll then be logged out of BookStack. This is the wrong point of logout within BookStack, which is what this issue addresses, but logout should work in most cases unless cancelled (or something goes wrong) on the IdP side of things.
#2553 was maybe more similar to your issue but likely specific to meeting Azure's requirements.
Your issue is likely similar, but specific to your IdP, although it would also be solved by this issue once addressed (although that would still leave a question if why you're not reaching the latter parts of the flow).
@ssddanbrown commented on GitHub (Dec 8, 2023):
This has now been addressed via
8cbaa3e27c, with testing added and manual testing performed via Keycloak, via both SP and IdP initiated logout, with various configuration changes and forced breaking of the full auth flow.These changes will be part of the next feature release.
Thanks again @akkornel for raising.