Assign role at authentication based on SAML attributes

Please enable the submodule 'samlauth_user_roles' and check the admin UI for what options it provides. I believe it's extensive enough that it may do just what you need.

The submodule has no readme, I figured that its existence plus its description 'SAML Auth User Role Assignment' would be enough, together with the main readme's note

Synchronizing other fields and/or roles is done with optional modules, so that
their behavior can be more easily replaced with custom code. See the modules/
subdirectory and enable the shipped submodules as desired; their configuration
is exposed in extra tabs next to the "Configuration" tab.

...altough the main readme is a swamp of info, so I see how you can miss that. (Suggestions welcome.)

Thank you so much for this guidance. I did miss that in the readme. Very helpful.

I have configured the Value conversions this way:

IT Staff|Member

I have double-checked that the SAML attribute is exactly "IT Staff" and that the Drupal role is exactly "Member." Nevertheless, no role is assigned to new users on login.

Not much I can do from here.

1)
Check the config value for 'samlauth_user_roles.mapping'. (Using e.g. drush cget or config inspector?)

- 'saml_attribute' sub-value should match exactly the name of the attribute that you see in your IdP response, containing "IT Staff". (See "SAML" main config screen to debug/log Idp response, if necessary.)

- 'only_first_login' should likely not be on, in your case

- 'value_map' should contain array ( 'attribute_value' => 'IT STaff' , 'role_machine_name' => [ Your machine name, i.e. likely not "Member"] )

2)
If that all is correct, then debug UserRolesEventSubscriber::onUserSync(). I have no idea what's going wrong there, then. The code is... fairly readable.

Everything looks OK in the config, and the saml attribute is the same as in the response from the IdP. I'm going to do some debugging in the UserRolesEventSubscriber class like you mentioned, in fact I'm already looking at it.

only_first_login: false
unassign_roles: {  }
default_roles: {  }
saml_attribute: userrole
saml_attribute_separator: ''
value_map:
  -
    attribute_value: 'IT Staff'
    role_machine_name: member

On line 220 of the UserRolesEventSubscriber class, the array_diff($account_role_ids, $changed_role_ids) will return an empty array, if $account_role_ids = [ "anonymous" ] and $changed_role_ids = [ "anonymous", "member" ], because "anonymous" is in both arrays, so not a difference. If we use array_intersect($account_role_ids, $changed_role_ids), then we get the desired role(s) that should be removed with $account->removeRole($role_id), in this case the "anonymous" role. In fact, maybe there's even a better way to always at least remove the "anonymous" role, since if we're assigning a role to a user, that user is certainly no longer anonymous.

Lines from 220:

      foreach (array_diff($account_role_ids, $changed_role_ids) as $role_id) {
        $account->removeRole($role_id);
      }
      foreach (array_diff($changed_role_ids, $account_role_ids) as $role_id) {
        $account->addRole($role_id);
      }

So with these values,

• line 220 returns an empty array, so no roles are removed from the existing user
• line 222 returns ['member'], which will be added to the existing user

so that's good, or no?

$event->markAccountChanged() is a signal to the caller that the account should be saved (because otherwise every event subscriber would re-save the account). Perhaps something is going wrong or some exception gets thrown somewhere after this, so the account isn't actually saved?

On second thought, using array_intersect there is also not correct, what we want to make sure of is that the "anonymous" role is not added to the $changed_role_ids array, that way if the current user is anonymous, the role will be removed after using array_diff on the two arrays.

oooooh.... now I understand.

Removing the "anonymous" role.

I guess my mistake here is assuming that $account->getRoles() is actually representative of the roles saved in the database. When "anonymous" is returned when the actual roles are zero.

Yes, that would mean that the fix is


if ( array_diff($changed_role_ids, $account_role_ids) )
{
if (getRoles() contains "anonymous" (which is equivalent to == ["anonymous"] , because it never occurs together with another role ) )
{ remove "anonymous" }

foreach (array_diff($changed_role_ids, $account_role_ids) as $role_id) {
$account->addRole($role_id);
}
}

yeah, I think we were cross-posting while responding to ourselves :-)

Maybe line 89 could be replaced with

$account_role_ids = $account->getRoles();
$account_role_ids == ["anonymous"] ? $changed_role_ids = [] : $changed_role_ids = $account_role_ids;

I've noticed that $account->removeRole("anonymous") does not remove the role from the account, I'm still seeing both the "anonymous" and "member" roles on the account afterwards, so the issue isn't with the arrays, apparently.

@stephenplatz I'm looking at Core now rather than just replying immediately:

line 89 could change to
$changed_role_ids = $account_role_ids = $account->getRoles(TRUE);

However... I'm not sure if the code is actually wrong for usual cases, if I look into the definition of User:;getRoles() and User::addRole(). then what I am reading (and I'm not trying it at the moment), feels to me like:

$account->getRoles()  // returns [ 'anonymous' ]
$account->get('roles')  // SHOULD return []
$account->addRole('member');
$account->getRoles()  // SHOULD return [ 'member' ]
$account->get('roles') // SHOULD return [ 'member' ]

... and I'd check, just in case, if I am saying something wrong here and/or if your database / saved configuration contains any role mappings to role 'anonymous'. It should not.

If I am right, then I guess

1)
$changed_role_ids = $account_role_ids = $account->getRoles(TRUE); isn't actually good for the module, because it does nothing except introduce a possibility to add "anonymous" (which throws an exception)

2)
your suggestion for a change is good for always calling $account->removeRole('anonymous'). which doesn't throw an exception, but I'm not super sure I'm happy with that as-is, because it will re-save the account on every login.

The ways it's written currently is not working at all in my case, even if I assign a default role in config, the account still retains both the "anonymous" and the "member" roles. I'm going to try to understand better why $account->removeRole('anonymous') doesn't remove that role, there must be another way for that specific case.

I think that the issue is more fundamental, since Drupal hasn't yet assigned a uid to the account, so it will be anonymous until the account is saved. We may need to use another method here, like postSave() to set roles on new account creation.