starred/BookStack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2026-02-05 00:29:48 +03:00
Code Issues 305 Packages Projects Releases 10 Wiki Activity

SAML SingleLogoutService Issues with ADFS #1563

New Issue
Closed
opened 2026-02-05 01:16:12 +03:00 by OVERLORD · 6 comments
OVERLORD commented 2026-02-05 01:16:12 +03:00
Owner
Copy Link

Originally created by @SoarinFerret on GitHub (Mar 1, 2020).

Describe the bug
I believe I have uncovered two issues when setting up SAML with ADFS, not sure if I need to supply multiple bug reports or not.

  1. ADFS requires both requests and responses for SLS to be signed by the SP. I was able to get around this by updating the saml config file located in app/config/saml2.php and providing the certificate as an environment variable.
...
        // sp config under onelogin
        'sp' => [
...
            'x509cert' => env('SAML2_SP_x509',''),
            'privateKey' => env('SAML2_SP_PRIVATEKEY',''),
        ],
...
        // New section under onelogin called 'security', below idp
        'security' => [
            'logoutRequestSigned' => true,
            'logoutResponseSigned' => true,
        ],
  1. The probably bigger issue is that the SLS request sent to ADFS does not seem to make much sense. The nameid format should be returned the same as Bookstack requests it, which is urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress. However, Bookstack is sending back urn:oasis:names:tc:SAML:2.0:nameid-format:entity. In addition, the NameID should be my UPN / E-Mail address, but instead it appears to be sending an ADFS URL.

Here is the sample response:

<samlp:LogoutRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol"
                     xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion"
                     ID="ONELOGIN_c83ad4ce9dab4d3a2f510f1ca01a8f78ac0ed4a2"
                     Version="2.0"
                     IssueInstant="2020-02-22T17:37:07Z"
                     Destination="https://sts.example.com/adfs/ls/"
                     >
    <saml:Issuer>https://bookstack.example.com/saml2/metadata</saml:Issuer>
    <saml:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">http://sts.example.com/adfs/services/trust</saml:NameID>
</samlp:LogoutRequest>

Here is the error generated on ADFS after receiving that:

The SAML Single Logout request does not correspond to the logged-in session participant. 
Requestor: https://bookstack.example.com/saml2/metadata 
Request name identifier: Format: urn:oasis:names:tc:SAML:2.0:nameid-format:entity, NameQualifier:  SPNameQualifier: , SPProvidedId:  
Logged-in session participants: 
Count: 1, [Issuer: https://bookstack.example.com/saml2/metadata, NameID: (Format: urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress, NameQualifier:  SPNameQualifier: , SPProvidedId: )]  

This request failed.

Steps To Reproduce
Steps to reproduce the behavior:

  1. Sign in with ADFS
  2. Click on Logout

Expected behavior
I believe I should be expecting 2 things:

  • The ability to specify options to require signed logout requests (via an env var or something)
  • For the nameid format during the SLS to be the same format and return the correct var.

Your Configuration (please complete the following information):

  • Exact BookStack Version (Found in settings): v0.28.2
  • PHP Version: 7.3.14
  • Hosting Method (Nginx/Apache/Docker): Linuxserver.io Docker Container

Additional context
Add any other context about the problem here.

Here is my .env relevant SAML config:

## SAML Config
# Set authentication method to be saml2
AUTH_METHOD=saml2

# Set the display name to be shown on the login button.
# (Login with <name>)
SAML2_NAME=ADFS

# Name of the attribute which provides the user's email address
SAML2_EMAIL_ATTRIBUTE=mail

# Name of the attribute to use as an ID for the SAML user.
SAML2_EXTERNAL_ID_ATTRIBUTE=http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn

# Name of the attribute(s) to use for the user's display name
# Can have mulitple attributes listed, separated with a '|' in which
# case those values will be joined with a space.
# Example: SAML2_DISPLAY_NAME_ATTRIBUTES=firstName|lastName
# Defaults to the ID value if not found.
SAML2_DISPLAY_NAME_ATTRIBUTES=displayName

# Identity Provider entityID URL
SAML2_IDP_ENTITYID=https://sts.example.com/federationmetadata/2007-06/federationmetadata.xml

# Auto-load metatadata from the IDP
# Setting this to true negates the need to specify the next three options
SAML2_AUTOLOAD_METADATA=true

# Custom values I added on my own
SAML2_SP_x509="-----BEGIN CERTIFICATE-----..."
SAML2_SP_PRIVATEKEY="-----BEGIN PRIVATE KEY-----..."

And here are my ADFS claims rules:

@RuleTemplate = "LdapClaims"
@RuleName = "User Attributes"
c:[Type == "http://schemas.microsoft.com/ws/2008/06/identity/claims/windowsaccountname", Issuer == "AD AUTHORITY"]
 => issue(store = "Active Directory", types = ("http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn", "mail", "groups", "displayName"), query = ";userPrincipalName,otherMailbox,tokenGroups,displayName;{0}", param = c.Value);

@RuleName = "Transform UPN to Name ID"
c:[Type == "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn"]
 => issue(Type = "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier", Issuer = c.Issuer, OriginalIssuer = c.OriginalIssuer, Value = c.Value, ValueType = c.ValueType, Properties["http://schemas.xmlsoap.org/ws/2005/05/identity/claimproperties/format"] = "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress");
Originally created by @SoarinFerret on GitHub (Mar 1, 2020). **Describe the bug** I believe I have uncovered two issues when setting up SAML with ADFS, not sure if I need to supply multiple bug reports or not. 1) ADFS requires both requests and responses for SLS to be signed by the SP. I was able to get around this by updating the saml config file located in `app/config/saml2.php` and providing the certificate as an environment variable. ```php ... // sp config under onelogin 'sp' => [ ... 'x509cert' => env('SAML2_SP_x509',''), 'privateKey' => env('SAML2_SP_PRIVATEKEY',''), ], ... // New section under onelogin called 'security', below idp 'security' => [ 'logoutRequestSigned' => true, 'logoutResponseSigned' => true, ], ``` 2) The probably bigger issue is that the SLS request sent to ADFS does not seem to make much sense. The nameid format should be returned the same as Bookstack requests it, which is `urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress`. However, Bookstack is sending back `urn:oasis:names:tc:SAML:2.0:nameid-format:entity`. In addition, the NameID should be my UPN / E-Mail address, but instead it appears to be sending an ADFS URL. Here is the sample response: ```xml <samlp:LogoutRequest xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="ONELOGIN_c83ad4ce9dab4d3a2f510f1ca01a8f78ac0ed4a2" Version="2.0" IssueInstant="2020-02-22T17:37:07Z" Destination="https://sts.example.com/adfs/ls/" > <saml:Issuer>https://bookstack.example.com/saml2/metadata</saml:Issuer> <saml:NameID Format="urn:oasis:names:tc:SAML:2.0:nameid-format:entity">http://sts.example.com/adfs/services/trust</saml:NameID> </samlp:LogoutRequest> ``` Here is the error generated on ADFS after receiving that: ``` The SAML Single Logout request does not correspond to the logged-in session participant. Requestor: https://bookstack.example.com/saml2/metadata Request name identifier: Format: urn:oasis:names:tc:SAML:2.0:nameid-format:entity, NameQualifier: SPNameQualifier: , SPProvidedId: Logged-in session participants: Count: 1, [Issuer: https://bookstack.example.com/saml2/metadata, NameID: (Format: urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress, NameQualifier: SPNameQualifier: , SPProvidedId: )] This request failed. ``` **Steps To Reproduce** Steps to reproduce the behavior: 1. Sign in with ADFS 2. Click on Logout **Expected behavior** I believe I should be expecting 2 things: - The ability to specify options to require signed logout requests (via an env var or something) - For the nameid format during the SLS to be the same format and return the correct var. **Your Configuration (please complete the following information):** - Exact BookStack Version (Found in settings): v0.28.2 - PHP Version: 7.3.14 - Hosting Method (Nginx/Apache/Docker): Linuxserver.io Docker Container **Additional context** Add any other context about the problem here. Here is my `.env` relevant SAML config: ``` ## SAML Config # Set authentication method to be saml2 AUTH_METHOD=saml2 # Set the display name to be shown on the login button. # (Login with <name>) SAML2_NAME=ADFS # Name of the attribute which provides the user's email address SAML2_EMAIL_ATTRIBUTE=mail # Name of the attribute to use as an ID for the SAML user. SAML2_EXTERNAL_ID_ATTRIBUTE=http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn # Name of the attribute(s) to use for the user's display name # Can have mulitple attributes listed, separated with a '|' in which # case those values will be joined with a space. # Example: SAML2_DISPLAY_NAME_ATTRIBUTES=firstName|lastName # Defaults to the ID value if not found. SAML2_DISPLAY_NAME_ATTRIBUTES=displayName # Identity Provider entityID URL SAML2_IDP_ENTITYID=https://sts.example.com/federationmetadata/2007-06/federationmetadata.xml # Auto-load metatadata from the IDP # Setting this to true negates the need to specify the next three options SAML2_AUTOLOAD_METADATA=true # Custom values I added on my own SAML2_SP_x509="-----BEGIN CERTIFICATE-----..." SAML2_SP_PRIVATEKEY="-----BEGIN PRIVATE KEY-----..." ``` And here are my ADFS claims rules: ``` @RuleTemplate = "LdapClaims" @RuleName = "User Attributes" c:[Type == "http://schemas.microsoft.com/ws/2008/06/identity/claims/windowsaccountname", Issuer == "AD AUTHORITY"] => issue(store = "Active Directory", types = ("http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn", "mail", "groups", "displayName"), query = ";userPrincipalName,otherMailbox,tokenGroups,displayName;{0}", param = c.Value); @RuleName = "Transform UPN to Name ID" c:[Type == "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn"] => issue(Type = "http://schemas.xmlsoap.org/ws/2005/05/identity/claims/nameidentifier", Issuer = c.Issuer, OriginalIssuer = c.OriginalIssuer, Value = c.Value, ValueType = c.ValueType, Properties["http://schemas.xmlsoap.org/ws/2005/05/identity/claimproperties/format"] = "urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress"); ```
OVERLORD commented 2026-02-05 01:16:21 +03:00
Author
Owner
Copy Link

@SoarinFerret commented on GitHub (Feb 15, 2021):

I was recently reached out to for help with this, I figured I would update this issue. I actually have gotten this configuration working without using SLS. More details can be found here: https://blog.kanto.cloud/bookstack-adfs-setup/

The gist of of it is choosing to manually define your parameters for ADFS instead of using the autoload from the federation metadata. This prevents Bookstack from automatically trying to use SLS. Here is the relevant .env for my ADFS SAML config:

## SAML Config
AUTH_METHOD=saml2
SAML2_NAME=ADFS
SAML2_EMAIL_ATTRIBUTE=mail
SAML2_EXTERNAL_ID_ATTRIBUTE=http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn
SAML2_DISPLAY_NAME_ATTRIBUTES=displayName
## Important - do not set set do the FederationMetadata URL
SAML2_IDP_ENTITYID=http://sts.example.com/adfs/services/trust
## Important
SAML2_AUTOLOAD_METADATA=false
SAML2_IDP_SSO=https://sts.example.com/adfs/ls/
## Important, make sure the following is commented out
#SAML2_IDP_SLO=null
## This is just your signing certificate on ADFS
SAML2_IDP_x509="MIIC2...."

I still have intentions of one day finishing my other patch, because SLS is cool, but it is not a necessity for me at the moment.

@SoarinFerret commented on GitHub (Feb 15, 2021): I was recently reached out to for help with this, I figured I would update this issue. I actually have gotten this configuration working without using SLS. More details can be found here: https://blog.kanto.cloud/bookstack-adfs-setup/ The gist of of it is choosing to manually define your parameters for ADFS instead of using the autoload from the federation metadata. This prevents Bookstack from automatically trying to use SLS. Here is the relevant `.env` for my ADFS SAML config: ```env ## SAML Config AUTH_METHOD=saml2 SAML2_NAME=ADFS SAML2_EMAIL_ATTRIBUTE=mail SAML2_EXTERNAL_ID_ATTRIBUTE=http://schemas.xmlsoap.org/ws/2005/05/identity/claims/upn SAML2_DISPLAY_NAME_ATTRIBUTES=displayName ## Important - do not set set do the FederationMetadata URL SAML2_IDP_ENTITYID=http://sts.example.com/adfs/services/trust ## Important SAML2_AUTOLOAD_METADATA=false SAML2_IDP_SSO=https://sts.example.com/adfs/ls/ ## Important, make sure the following is commented out #SAML2_IDP_SLO=null ## This is just your signing certificate on ADFS SAML2_IDP_x509="MIIC2...." ``` I still have intentions of one day finishing my other patch, because SLS is cool, but it is not a necessity for me at the moment.
OVERLORD commented 2026-02-05 01:16:30 +03:00
Author
Owner
Copy Link

@dani commented on GitHub (Mar 17, 2021):

I do have a similar issue with Lemonldap::NG as SAML IDP. SLO can't work because the the NameID isn't correct. Lemonldap::NG can't make sense of the request so respond with a code 400

@dani commented on GitHub (Mar 17, 2021): I do have a similar issue with Lemonldap::NG as SAML IDP. SLO can't work because the the NameID isn't correct. Lemonldap::NG can't make sense of the request so respond with a code 400
OVERLORD commented 2026-02-05 01:16:34 +03:00
Author
Owner
Copy Link

@coudot commented on GitHub (Mar 17, 2021):

The issue is in this part of the code: 65ddd16532/app/Auth/Access/Saml2Service.php (L60)

The logout method should be called with correct arguments, see https://github.com/onelogin/php-saml#initiate-slo

At least the correct NameID and SessionIndex that should be registered in user session when he authenticates.

@coudot commented on GitHub (Mar 17, 2021): The issue is in this part of the code: https://github.com/BookStackApp/BookStack/blob/65ddd16532e0329cc862f069064181593ad83253/app/Auth/Access/Saml2Service.php#L60 The logout method should be called with correct arguments, see https://github.com/onelogin/php-saml#initiate-slo At least the correct NameID and SessionIndex that should be registered in user session when he authenticates.
OVERLORD commented 2026-02-05 01:16:37 +03:00
Author
Owner
Copy Link

@theodor-franke commented on GitHub (Aug 30, 2021):

Is there a solution?

@theodor-franke commented on GitHub (Aug 30, 2021): Is there a solution?
OVERLORD commented 2026-02-05 01:16:40 +03:00
Author
Owner
Copy Link

@theodor-franke commented on GitHub (Aug 30, 2021):

#2902 I created a PR that should resolve this Issue

@theodor-franke commented on GitHub (Aug 30, 2021): #2902 I created a PR that should resolve this Issue
OVERLORD commented 2026-02-05 01:16:44 +03:00
Author
Owner
Copy Link

@ssddanbrown commented on GitHub (Oct 25, 2021):

As per #2902 a range of changes have now been made for BookStack v21.10.

The new SAML2_SP_x509 and SAML2_SP_x509_KEY options, which enable SP SLS signing, can be seen in the updated documentation:
https://www.bookstackapp.com/docs/admin/saml2-auth/

Will therefore close this off but please open a new issue if there are problems with the updated implementation.

@ssddanbrown commented on GitHub (Oct 25, 2021): As per #2902 a range of changes have now been made for BookStack v21.10. The new `SAML2_SP_x509` and `SAML2_SP_x509_KEY` options, which enable SP SLS signing, can be seen in the updated documentation: https://www.bookstackapp.com/docs/admin/saml2-auth/ Will therefore close this off but please open a new issue if there are problems with the updated implementation.
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
No Label
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#1563
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?