- Issue created by @pbattino
- Status changed to Postponed: needs info
over 1 year ago 9:46am 21 April 2023 - 🇵🇹Portugal jcnventura
I don't understand this report.. The code that adds or removes user groups based not on the groups claim, but on the configured role mapping.. If the role mapping is empty, it should do nothing. See the code in https://git.drupalcode.org/project/openid_connect/-/blob/2.x/src/OpenIDC...
Think of the role mapping as being the on/off switch you are requesting, but instead of having an explicit switch, it is simplified by the presence or not of a mapping. If there's a mapping, the feature is on, if there is no mapping the feature is off.
Of course, if the role mapping is not empty (i.e. there are groups being mapped to roles), and the groups claim is set, then the module will apply the expected configuration.
Can you please clarify how is it that the site removed all groups from the user role? There might be a bug on how the mapping logic is working that wasn't handled correctly in #3224128: Revoking group access does not reflect on applied roles → .
- 🇬🇧United Kingdom pbattino Reading
If the role mapping is empty, it should do nothing.
This is what I expected to happen, but unfortunately it seems that if you fill in at least one role, you can't go back. That is, resetting that fieldset with all empty fields, will not remove the mapping but rather map all roles to an empty string, meaning no role will be assigned and all roles will be removed.
In short, it seems that there is no way to "empty" the configuration of role_mappings. Even if you empty all fields in https://www.efas.local/en/admin/config/people/openid-connect/settings#ed... , the configuration will be:
role_mappings: editor: { } role1: { } role2: { } role3: { } role4: { } role5: { }
etc..
To cross-check that this was the problem, I edited the configuration removing altogether the "role_mappings" key and this restored the expected behaviour, i.e. no mapping at all / no removal of already assigned roles.So perhaps we should just better handle the case of when all mapping fields are empty, regardless of whether they were never used or used but then emptied.
(I stumbled upon this edge-case because errors after fresh install when no mapping is specified: 🐛 Trying to access array offset on value of type null in Drupal\openid_connect\Form\OpenIDConnectSettingsForm->buildForm() Fixed ) - 🇵🇹Portugal jcnventura
Yes, removing all maps should be the same as never having mapped anything. This should be fixed.
- 🇬🇧United Kingdom pbattino Reading
What about changing this part of submitForm() in openid_connect/src/Form/OpenIDConnectSettingsForm.php?
public function submitForm(array &$form, FormStateInterface $form_state) { parent::submitForm($form, $form_state); $role_mappings = []; foreach ($form_state->getValue('role_mappings') as $role => $mapping) { $values = array_values(array_filter(str_getcsv($mapping, ' '))); if (!empty($values)) { $role_mappings[$role] = $values; } }
It seems to work for me, I can submit a patch in a couple of hours.
- Status changed to Needs review
over 1 year ago 11:34am 24 April 2023 - last update
over 1 year ago 100 pass - 🇮🇳India Raveen Kumar
Hello Folks!!
I have reviewed & applied the patch on the module And I confirm the #6 patch has been applied successfully.
As you can see in my attached screenshot.
And Thank You. - Status changed to RTBC
over 1 year ago 6:40am 3 June 2023 -
jcnventura →
committed 56984b1f on 3.x
Issue #3355560 by pbattino, Raveen Thakur, jcnventura: Better handling...
-
jcnventura →
committed 56984b1f on 3.x
-
jcnventura →
committed 11c32010 on 2.x
Issue #3355560 by pbattino, Raveen Thakur, jcnventura: Better handling...
-
jcnventura →
committed 11c32010 on 2.x
- Status changed to Fixed
over 1 year ago 9:26pm 24 July 2023 Automatically closed - issue fixed for 2 weeks with no activity.
- Status changed to Fixed
about 1 month ago 1:45am 13 November 2024 - 🇩🇪Germany webflo
I got bitten by this issue today. Roles for over 100 users have been deleted. I think needs an update hook, or should have an additional empty array condition during runtime.
Update in 🐛 Update role_mappings to avoid data loss Active