saml group mapping not working with groups with comma

[GustavJer]

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.

[ssddanbrown]

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.

[GustavJer]

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.

[ssddanbrown]

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]

I'll close this off since #3416 has been merged.
This functionality will be part of the next feature release.