SAML 2.0 SLO is not provided a session index

[kompetenzlandkarte]

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.

[ssddanbrown]

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.

[aduthapa]

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.

[ssddanbrown]

This has now been added within commit 8c738ae.

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.