User role cleared after log in

This sounds like a similar issue as #3487116, which should have corrected this.

- What OpenID Client are you using?
- Can you provide the plugin's configuration before and after the upgrade (obfuscating the sensitive parts)?

Did you run the database updates after updating to alpha5?

I encountered the same problem, and re-saving the OpenID Connect settings resolved it for me.

@pfrilling
Below is the config in question. It seems to stay the same after upgrading from alpha3 to alpha4.

@dcam
I did run drush deploy after upgrading so I assume database update was successful.

@gokul.jayan
I tried creating a new Google profile with the same configuration. But it still wipes away my user roles after switching to that profile (on alpha4).

uuid: 0aax...cae
langcode: en
status: true
dependencies: {  }
id: webmail
label: 'Webmail'
plugin: google
settings:
  client_id: 5032...o1.apps.googleusercontent.com
  client_secret: GOC....ZFd
  iss_allowed_domains: "xxx.or.jp\r\nddev.site\r\npantheonsite.io"

Perhaps this is the culprit? If we don't have any mappings, we should skip role syncing, right?

   $role_mappings = $this->configFactory->get('openid_connect.settings')->get('role_mappings') ?? [];
   $user_groups = $userinfo['groups'] ?? [];
   foreach ($role_mappings as $role => $mappings) {
     if (empty(array_intersect($mappings, $user_groups))) {
       // User doesn't have a mapped role. Remove it from their account.
       $account->removeRole($role);
     }
     else {
       // User has a mapped role. Add it to their account.
       $account->addRole($role);

alpha5 has the fix for this issue.

Hi @dcam, I tried upgrading from alpha4 to alpha5 and still have the same issue.

Update to alpha5, and there are configuration changes for the module settings that needs to be exported. After the export the configuration looks something like this.

_core:
  default_config_hash: NFnOZU6VCOfURCAR4-O3V7eMHTx2LNqnCmDKRSNjjns
always_save_userinfo: true
connect_existing_users: true
override_registration_settings: false
end_session_enabled: true
user_login_display: above
redirect_login: /user
redirect_logout: /
userinfo_mappings: {  }
role_mappings: {  }
 

gokul.jayan → changed the visibility of the branch 3497559-user-role-cleared to hidden.

I did run drush deploy after upgrading so I assume database update was successful.

I'd need to know more about your deployment workflow. Did you update the module on a dev environment, update the database (without using drush deploy), export the configuration, commit the configuration, push your changes to production, install the update, and then run drush deploy?

Hi @dcam:

I was performing a minor Drupal upgrade alongside other module upgrades .

1. On dev environment, composer require the latest 3.0.0-alpha version of OIDC. (from alpha3 to alpha4 at the time of upgrading).
2. Push changes to dev/test/live, without any changes made to OIDC settings. I cannot remember if I did a config export.. but both the module setting and the google plugin setting remained the same, i.e. as copied above.
3. Drush cr and drush deploy was run when deployed, so any database update should have run already.

Hope this helps!

Hi @gokul.jayan,

I don't know why but my role mapping has empty items with my Drupal site roles. (see #7).
I added a quick fix but hopefully that doesn't break other functionalities.

hktang → changed the visibility of the branch 3497559-user-role-cleared to active.

hktang → changed the visibility of the branch 3497559-user-role-cleared to hidden.

I asked about the workflow because drush deploy runs database updates and then imports your configuration. If a person did that on an environment without following a strict procedure for configuration management, then their database would be updated and then immediately overwritten with the old configuration. That would result in no changes being apparent.

In any case, if the database updates were run but your configuration remains in a broken state, then it's probably going to need to be manually repaired at this point. @gokul.jayan gave the solution. The role_mappings key needs to be reduced to an empty array, role_mappings: { }. Personally, I would just edit the config file to do it and then import the configuration.

Hi @hktang
I also encountered the same problem with alpha5. In my case, when I resaved the module settings, there were changes to the role mappings—specifically, the removal of all role mappings and their replacement with only the role_mapping key containing an empty array. I believe resaving the module settings should resolve the issue.

Yeah. That should work too.

Hi @dcam and @gokul.jayan, thank you for your feedback!

I tried renaming the database upgrade script and quickly rerun it as 30003.

Then I did a config export just to make sure the role_mappings is emptied.

Still, it seems the upgrade script didn't empty the role_mappings config item as expected. My config stays the same as #7.

Here's the log for drush deploy and drush cex -y.

ddev drush deploy
 [notice] Database updates start.
 ---------------- ----------- --------------- ------------------------------------------------------------ 
  Module           Update ID   Type            Description                                                 
 ---------------- ----------- --------------- ------------------------------------------------------------ 
  openid_connect   30003       hook_update_n   30003 - Clean-up role_mappings in openid_connect.settings.  
 ---------------- ----------- --------------- ------------------------------------------------------------ 


 Do you wish to run the specified pending updates? (yes/no) [yes]:
 > >  [notice] Module twig_xdebug has an entry in the system.schema key/value storage, but is not installed. <a href="https://www.drupal.org/node/3137656">More information about this error</a>.
>  [notice] Module workflow_buttons has an entry in the system.schema key/value storage, but is missing from your site. <a href="https://www.drupal.org/node/3137656">More information about this error</a>.
> >  [notice] Update started: openid_connect_update_30003
> >  [notice] Update completed: openid_connect_update_30003
>  [success] Finished performing updates.
 [success] Config import start.
+------------+-------------------------+-----------+
| Collection | Config                  | Operation |
+------------+-------------------------+-----------+
|            | openid_connect.settings | Update    |
+------------+-------------------------+-----------+

 Import the listed configuration changes? (yes/no) [yes]:
 > >  [notice] Synchronized configuration: update openid_connect.settings.
>  [notice] Finalizing configuration synchronization.
>  [success] The configuration was imported successfully.
 [success] Cache rebuild start.
>  [success] Cache rebuild complete.
 [success] Deploy hook start.
>  [success] No pending deploy hooks.
❯ ddev drush cr
 [success] Cache rebuild complete.
❯ ddev drush cex
 [notice] The active configuration is identical to the configuration in the export directory (/var/www/html/config/sync).
/var/www/html/config/sync

Also, I agree with you that we should follow deployment best practices.

Manually updating the config yaml file might have the risk of it being overwritten by other processes. Wouldn't it be better if we just have a simple check and skip the dangerous role removal, as suggested in the merge request?

Hi guys, I also confirm the update script runs correctly but drush still thinks there's no difference.

This is the config right after running the update script.

Array
 (
     [_core] => Array
         (
             [default_config_hash] => NFnOZU...SNjjns
         )
 
     [always_save_userinfo] => 1
     [connect_existing_users] => 1
     [override_registration_settings] => 
     [end_session_enabled] => 1
     [user_login_display] => above
     [redirect_login] => /user
     [redirect_logout] => /
     [userinfo_mappings] => Array
         (
         )
 
     [role_mappings] => Array
         (
         )
 
 )

But my config, after running drush cex, is still the same as #7. i.e. different content, but considered same by Drush.

Hi @hktang
During the Drush deployment process, changes were made to the openid_connect.settings, and the new changes were overridden. Try resaving the module settings and then run drush cex.

Hi there, I finally confirmed what happened. It's caused by my deployment script which uses "drush deploy".

According to documentation, drush deploy imports existing config immediately after running database update. It's a combination of:

drush updatedb --no-cache-clear
drush cache:rebuild
drush config:import
drush cache:rebuild
drush deploy:hook

This sequence of actions effectively ignores the $config-save() action made by the update script added via https://www.drupal.org/node/3487116 → . When I do `drush cex` after deployment, I am actually exporting the old configuration instead of the new one.

I wonder if drush deploy is the standard deploy action?

+1 We have also run into this issue where our custom Drupal roles are getting removed on user log in. We aren't using the role mapping feature built into the module and the experimental fields are blank. We do use drush deploy, and I agree with @hktang's comment #26 that it should work either way with drush deploy or drush cim.

+1 Also ran into this issue where we don't have role mapping enabled, but the role_mappings config not being an empty array and overriding on login.

Saving the form does not help, but after changing the config manually it works again.

Thank you your your feedback!

I am moving this post to Needs Review as the change should do what it's expected to do. Thanks in advance for community review.

hi,
We were facing this issue with openid_connect. As the users was logging in the role was removed for the users. The fix provided helped us resolve the issue.

Thanks

I am not moving the issue to RTBC as the pipeline for the MR is failing. Once the pipeline passes it will be ready for RTBC.

Thank you. I propose we update the test case to reflect the same expected behavior: we don't remove roles if the groups does not exist in the first place. Please see the commit above.

It is also possible to run into this situation if you are using config_split to only enable certain modules on upper environments (eg. Prod) and forget to account for an update hook for those modules.

I think there are situations where a site would prefer to have the roles cleared if the groups array was empty (as is the current behavior). We should probably add a configuration option that would allow a site to choose the behavior. Then, we need to decide which option should be the default. I'm leaning to making the default to _not_ clear the roles, as that feels like the least intrusive, but am open to other opinions.

Fully agree that we should not clear the empty roles by default.

Regarding the configuration option, shall we address it in a separate issue? I am happy to start a new ticket for it.

I am suggesting this because I feel this issue should be patched with some urgency, and the configuration helper text could need some refining, which could take time.

We are facing the issue after updating the roles after each re-login is cleared. (our mapping is default, email)

patch https://www.drupal.org/project/openid_connect/issues/3497559#comment-159... 🐛 User role cleared after log in Active didn't change that.

So also looking for a solution to that.

Patch from the MR. It works for me.

I started a new branch and merge request from the patch of MR 133 (I couldn't figure out how to rename the branch 🤷‍♂️). See MR #157 for the updated code.

The new MR includes the aforementioned configuration option that would allow sites reliant on the current default behavior to continue working. I set the default value to be off. Any sites that want this behavior to continue will need to manually enable that configuration option. I don't see a method to enable that for some sites and not others. I'll need to over communicate that breaking change in the release notes after this merges.

pfrilling → closed merge request !133

Thanks for the review @hktang! I resolved all the threads in the MR. Can you review again?

Thank you very much for addressing my comments.

Am I right, that this behavior to remove all roles if no "groups" is set came in with 🐛 Mapped user roles are not always revoked Active