SAML attributes for users can contain commas in the the values. Right now the role sync rule logic does not allow for this.

EXAMPLES:

3:group,=,"thing=stuff,another=this"
Add role 1 when group value is "thing=stuff,another=that" not for example: "thing=nono,another=that"

5:company,=,"Acme, Inc."
Add role 5 when user's company is a string value containing a comma.

CommentFileSizeAuthor
#6 2134581-allow-commas-2.x.patch1.03 KBStevel
#2 2134581-allow-commas.patch1.05 KBStevel

Comments

doublejosh’s picture

doublejosh commented

As of PHP 5.3 you can forgo this explode(',', $my_val); for the super handy str_getcsv() function.

The fix is a one line change to the _simplesamlphp_auth_rolepopulation() function...

REPLACE:

$roleruleevaluationdc = explode(',', $roleruleevaluation);

WITH TIHS:

$roleruleevaluationdc = str_getcsv($roleruleevaluation);

WITHIN: simplesamlphp_auth.module

Stevel’s picture

Stevel commented
Version: 7.x-2.x-dev » 7.x-3.x-dev
StatusFileSize
new 2134581-allow-commas.patch1.05 KB

Attached a proper patch. A note of warning: this patch makes the module require php 5.3, as opposed to 5.2.5 for Drupal core. PHP 5.3 is already pretty old though.

snufkin’s picture

snufkin commented

I don't really mind the explicit PHP dependence, for the 3.x release we can make it required.

snufkin’s picture

snufkin commented
Status: Needs review » Fixed

Committed, thanks!

  • snufkin committed 63f8709 on 7.x-3.x authored by Stevel 
    Issue #2134581 by Stevel: Allow role sync rules with commas in values
Stevel’s picture

Stevel commented
Version: 7.x-3.x-dev » 7.x-2.x-dev
Status: Fixed » Needs review
StatusFileSize
new 2134581-allow-commas-2.x.patch1.03 KB

Backported patch for the 2.x branch.

  • snufkin committed 63f8709 on 8.x-3.x authored by Stevel 
    Issue #2134581 by Stevel: Allow role sync rules with commas in values

  • snufkin committed 63f8709 on 8.x-3.x authored by Stevel 
    Issue #2134581 by Stevel: Allow role sync rules with commas in values
kingdutch’s picture

kingdutch
he/him
Primary language English
commented
Status: Needs review » Fixed

Came across this while searching for some other issue. This has commits so it looks like this should be "Fixed"

kingdutch’s picture

kingdutch
he/him
Primary language English
commented
Status: Fixed » Needs review

Whoops, noticed that the 2.x version is still maintained but hasn't landed yet. Undoing my mistake.