saml group mapping not working with groups with comma #2768

New Issue

OVERLORD · 2026-02-05T05:07:49+03:00

OVERLORD commented

2026-02-05 05:07:49 +03:00

Originally created by @GustavJer on GitHub (May 2, 2022).

Attempted Debugging

I have read the debugging page

Searched GitHub Issues

I have searched GitHub for the issue.

Describe the Scenario

Hi,

Im having some trouble getting SAML group-mapping to work when one of the groups contains a comma. This is while using External Authentication ID on a role to match with the groupname.

This is abit of a problem since our IDP sends the full DN(which contains commas) of the AD-groups that the users is a member of and we need to be able to match these DN´s with roles within Bookstack.

If I send a static string value from our IDP in the groups attribute like "test" and match that to a role in bookstack the group mapping works but if I change it to ",test" in our IDP and in the external auth id field it stops working.
I know that the documentation states that "BookStack will standardise the names of SAML groups to be lower-cased and spaces will be replaced with hyphens." so could it be something similar with commas?

Gustav

Exact BookStack Version

v22.02.3

Log Content

No response

PHP Version

No response

Hosting Environment

Ubuntu LTS 20.04 installed using official installation script.

Originally created by @GustavJer on GitHub (May 2, 2022). ### Attempted Debugging - [X] I have read the debugging page ### Searched GitHub Issues - [X] I have searched GitHub for the issue. ### Describe the Scenario Hi, Im having some trouble getting SAML group-mapping to work when one of the groups contains a comma. This is while using External Authentication ID on a role to match with the groupname. This is abit of a problem since our IDP sends the full DN(which contains commas) of the AD-groups that the users is a member of and we need to be able to match these DN´s with roles within Bookstack. If I send a static string value from our IDP in the groups attribute like "test" and match that to a role in bookstack the group mapping works but if I change it to ",test" in our IDP and in the external auth id field it stops working. I know that the documentation states that "BookStack will standardise the names of SAML groups to be lower-cased and spaces will be replaced with hyphens." so could it be something similar with commas? Gustav ### Exact BookStack Version v22.02.3 ### Log Content _No response_ ### PHP Version _No response_ ### Hosting Environment Ubuntu LTS 20.04 installed using official installation script.

OVERLORD added the 🐕 Support label 2026-02-05 05:07:49 +03:00

OVERLORD closed this issue

2026-02-05 05:07:52 +03:00

OVERLORD commented

2026-02-05 05:07:59 +03:00

@ssddanbrown commented on GitHub (May 2, 2022):

Yeah, This is an awkward scenario.
The values in our "External Authentication IDs" field get split on comma to allow mapping of multiple auth system groups to the role. Maybe we need to add some an escape option for commas so they can be treated literally.

@ssddanbrown commented on GitHub (May 2, 2022): Yeah, This is an awkward scenario. The values in our "External Authentication IDs" field get split on comma to allow mapping of multiple auth system groups to the role. Maybe we need to add some an escape option for commas so they can be treated literally.

OVERLORD commented

2026-02-05 05:08:02 +03:00

@GustavJer commented on GitHub (May 2, 2022):

Yep, that would be nice. Or if you could specify your group attribute delimiter in the .env file and choose between comma, semicolon and other options.

@GustavJer commented on GitHub (May 2, 2022): Yep, that would be nice. Or if you could specify your group attribute delimiter in the .env file and choose between comma, semicolon and other options.

OVERLORD commented

2026-02-05 05:08:04 +03:00

@ssddanbrown commented on GitHub (May 4, 2022):

Hi @GustavJer,
I've now added the code for handling escapes for commas within the external auth IDs: #3416

I'd often add this kind of change into a sooner patch release but since this could potentially be considered a breaking change (although very unlikely), and therefore would require an update notice, it's instead target for the next feature release (Likely end of month).

@ssddanbrown commented on GitHub (May 4, 2022): Hi @GustavJer, I've now added the code for handling escapes for commas within the external auth IDs: #3416 I'd often add this kind of change into a sooner patch release but since this could potentially be considered a breaking change (although very unlikely), and therefore would require an update notice, it's instead target for the next feature release (Likely end of month).

OVERLORD commented

2026-02-05 05:08:09 +03:00

@ssddanbrown commented on GitHub (May 30, 2022):

I'll close this off since https://github.com/BookStackApp/BookStack/pull/3416 has been merged.
This functionality will be part of the next feature release.

@ssddanbrown commented on GitHub (May 30, 2022): I'll close this off since https://github.com/BookStackApp/BookStack/pull/3416 has been merged. This functionality will be part of the next feature release.

OVERLORD referenced this issue

2026-02-05 10:23:37 +03:00

[PR #2768] [MERGED] [sec] Fixes a few minor vulnerabilies when using target="_blank" on links (RSPEC-5148) #6061

Sign in to join this conversation.

Branches Tags

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/BookStack#2768