starred/BookStack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2026-05-04 18:08:46 +03:00
Code Issues 784 Packages Projects Releases 15 Wiki Activity

SAML 2.0 SLO is not provided a session index #3418

New Issue
Closed
opened 2026-02-05 06:39:30 +03:00 by OVERLORD · 3 comments
OVERLORD commented 2026-02-05 06:39:30 +03:00
Owner
Copy Link

Originally created by @kompetenzlandkarte on GitHub (Dec 28, 2022).

Describe the Bug

Trying using the SAML SLO functionality I get this error:

<samlp:StatusMessage>Missing SessionIndex: session participants MUST include at least one <SessionIndex> element in the logout request</samlp:StatusMessage>

Steps to Reproduce

Configure SAML Authentication with SLO functionality based on the documentation

Expected Behaviour

<SessionIndex> Object included in SLO Request

Screenshots or Additional Context

Currently the SLO Request looks like this:

<samlp:LogoutRequest
    xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
    xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
    ID="redacted"
    Version="2.0"
    IssueInstant="2022-12-28T10:00:34Z"
    Destination="redacted">
    <saml:Issuer>https://redacted/saml2/metadata</saml:Issuer>
    <saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">redacted</saml:NameID>
</samlp:LogoutRequest>

SAML Login Response (mostly truncated) - Includes SessionIndex

<samlp:Response Version="2.0" ID="redacted" IssueInstant="2022-12-28T10:08:33.861Z" InResponseTo="redacted" Destination="https://redacted/saml2/acs" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol">
    <saml:Assertion ID="redacted" IssueInstant="2022-12-28T10:08:33.894Z" Version="2.0" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion">
        <saml:AuthnStatement SessionIndex="redacted" AuthnInstant="2022-12-28T10:08:33.885Z">
            <saml:AuthnContext>
                <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified</saml:AuthnContextClassRef>
            </saml:AuthnContext>
        </saml:AuthnStatement>
    </saml:Assertion>
</samlp:Response>

Browser Details

No response

Exact BookStack Version

v22.11

PHP Version

No response

Hosting Environment

Inside official Docker Container on our Kubernetes Instance.

Originally created by @kompetenzlandkarte on GitHub (Dec 28, 2022). ### Describe the Bug Trying using the SAML SLO functionality I get this error: ```xml <samlp:StatusMessage>Missing SessionIndex: session participants MUST include at least one <SessionIndex> element in the logout request</samlp:StatusMessage> ``` ### Steps to Reproduce Configure SAML Authentication with SLO functionality based on the documentation ### Expected Behaviour `<SessionIndex>` Object included in SLO Request ### Screenshots or Additional Context Currently the SLO Request looks like this: ```xml <samlp:LogoutRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="redacted" Version="2.0" IssueInstant="2022-12-28T10:00:34Z" Destination="redacted"> <saml:Issuer>https://redacted/saml2/metadata</saml:Issuer> <saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">redacted</saml:NameID> </samlp:LogoutRequest> ``` SAML Login Response (mostly truncated) - Includes SessionIndex ```xml <samlp:Response Version="2.0" ID="redacted" IssueInstant="2022-12-28T10:08:33.861Z" InResponseTo="redacted" Destination="https://redacted/saml2/acs" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"> <saml:Assertion ID="redacted" IssueInstant="2022-12-28T10:08:33.894Z" Version="2.0" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"> <saml:AuthnStatement SessionIndex="redacted" AuthnInstant="2022-12-28T10:08:33.885Z"> <saml:AuthnContext> <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:unspecified</saml:AuthnContextClassRef> </saml:AuthnContext> </saml:AuthnStatement> </saml:Assertion> </samlp:Response> ``` ### Browser Details _No response_ ### Exact BookStack Version v22.11 ### PHP Version _No response_ ### Hosting Environment Inside official Docker Container on our Kubernetes Instance.
OVERLORD added the 🐛 Bug📖 Docs Update🚪 Authentication labels 2026-02-05 06:39:30 +03:00
OVERLORD commented 2026-02-05 06:39:34 +03:00
Author
Owner
Copy Link

@ssddanbrown commented on GitHub (Dec 28, 2022):

Thanks for reporting. I'm going to update the title slightly since we don't target specific auth providers, but instead the standards used.

Findings

  • We are currently not passing a sessionindex to the logout process, although it can be added here.
  • Pretty sure the session index would originate from the login, so would need to carry it for later use. Could probably just use the session for this since the login should always be persistent to the session for current SAML auth.
  • Line 1208 of this doc requires a sessionindex. Line 1302 can counter this but likely not applies in our case as a SP.
  • Need to re-educate on SAML sessions to understand exact implications of adding. Probably good to at least add note to update notes to indicate usage of sessions on SLO, since IDPs could act differently.
@ssddanbrown commented on GitHub (Dec 28, 2022): Thanks for reporting. I'm going to update the title slightly since we don't target specific auth providers, but instead the standards used. ### Findings - We are currently not passing a sessionindex to the logout process, although [it can be added here](https://github.com/BookStackApp/BookStack/blob/f0ac454be1e234ad157d3a400f593495b7a35727/app/Auth/Access/Saml2Service.php#L70). - Pretty sure the session index would originate from the login, so would need to carry it for later use. Could probably just use the session for this since the login should always be persistent to the session for current SAML auth. - [Line 1208 of this doc](https://docs.oasis-open.org/security/saml/v2.0/saml-profiles-2.0-os.pdf) requires a sessionindex. Line 1302 can counter this but likely not applies in our case as a SP. - Need to re-educate on SAML sessions to understand exact implications of adding. Probably good to at least add note to update notes to indicate usage of sessions on SLO, since IDPs could act differently.
OVERLORD commented 2026-02-05 06:39:41 +03:00
Author
Owner
Copy Link

@aduthapa commented on GitHub (Apr 8, 2023):

I have tried the SAML using Auth0 and the SAML works. But the issue is of SAML session index.
@ssddanbrown, what would you specifically suggest that could be added.
Can we have a code to add the specific line to remove this session issue so that SLO can process.

@aduthapa commented on GitHub (Apr 8, 2023): I have tried the SAML using Auth0 and the SAML works. But the issue is of SAML session index. @ssddanbrown, what would you specifically suggest that could be added. Can we have a code to add the specific line to remove this session issue so that SLO can process.
OVERLORD commented 2026-02-05 06:39:42 +03:00
Author
Owner
Copy Link

@ssddanbrown commented on GitHub (Apr 28, 2023):

This has now been added within commit 8c738aedee.

This has been very tricky to test, and understand the implications of.
I've added as a non-optional/configuration part of the flow, since it technically adding better alignment to the spec, but it's tricky to predict if this will affect existing flows for people, since it's heavily IdP dependant.
Will need to note as an update advisory (So folks using SLO can check the flow upon update).

The trouble is I didn't have an active platform with SLO enabled, that requires the session ID, so could not functionally validate, just observe that the SLO request contains the session index as required.
I tested on Auth0, and the changes work, but I found that the logout flow would work without these changes also, as long as things were configured up correctly to use HTTP redirect for SLO.
Tried to test on athentik, but not sure that's doing SLO to spec as we expect to even get SLO working.

@ssddanbrown commented on GitHub (Apr 28, 2023): This has now been added within commit 8c738aedee343826c3be7efc8b130d7e9966216f. This has been very tricky to test, and understand the implications of. I've added as a non-optional/configuration part of the flow, since it technically adding better alignment to the spec, but it's tricky to predict if this will affect existing flows for people, since it's heavily IdP dependant. **Will need to note as an update advisory (So folks using SLO can check the flow upon update).** The trouble is I didn't have an active platform with SLO enabled, that requires the session ID, so could not functionally validate, just observe that the SLO request contains the session index as required. I tested on Auth0, and the changes work, but I found that the logout flow would work without these changes also, as long as things were configured up correctly to use HTTP redirect for SLO. Tried to test on athentik, but not sure that's doing SLO to spec as we expect to even get SLO working.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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
No Label 📖 Docs Update 🐛 Bug 🚪 Authentication
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
OVERLORD
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: starred/BookStack#3418
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?