Problem/Motivation

Currently the simplesamlphp_auth.settings change when a new role is inserted even though the module's configuration has not changed. This is due to the configuration saving all available roles, rather than only the configured roles.

Proposed resolution

Only save the configured roles in simplesamlphp_auth.settings.allow.default_login_roles, so the module's configuration is unchanged when a new user role is added.

Remaining tasks

  1. Write a patch
  2. Review
  3. Commit

User interface changes

None.

API changes

None.

Data model changes

simplesamlphp_auth.settings.allow.default_login_roles now only saves configured roles.

Release notes snippet

simplesamlphp_auth.settings.allow.default_login_roles now only saves configured roles.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

idebr created an issue. See original summary.

idebr’s picture

Issue summary: View changes
Status: Active » Needs review
FileSize
1.29 KB

Attached patch only saves the configured roles in simplesamlphp_auth.settings.allow.default_login_roles, so the module's configuration is unchanged when a new user role is added.

Before:

allow:
  set_drupal_pwd: false
  default_login: true
  default_login_roles:
    authenticated: '0'
    fb: '0'
    news_editor: '0'
    editor: '0'
    corpwebteam: '0'
    vc: '0'
    programme_editor: '0'
  default_login_users: '1'
...

After:

allow:
  set_drupal_pwd: false
  default_login: true
  default_login_roles: {  }
  default_login_users: '1'
...
Berdir’s picture

+++ b/src/Form/LocalSettingsForm.php
@@ -81,7 +81,7 @@ class LocalSettingsForm extends ConfigFormBase {
     $config->set('allow.set_drupal_pwd', $form_state->getValue('allow_set_drupal_pwd'));
-    $config->set('allow.default_login_roles', $form_state->getValue('allow_default_login_roles'));
+    $config->set('allow.default_login_roles', array_values(array_filter($form_state->getValue('allow_default_login_roles'))));
     $config->set('allow.default_login_users', $form_state->getValue('allow_default_login_users'));
     $config->set('logout_goto_url', $form_state->getValue('logout_goto_url'));

Hm, the array_values() does change the structure to not be keyed by the role id anymore.

As far as I see, the only use for that setting does an array_intersect() and only cares about the values, but strictly speaking, it's still a change, in case someone had custom logic for some reason with that setting?

Maybe drop the array_values()? a little bit more verbose then, but should be safe for whatever people did with that setting.

idebr’s picture

FileSize
872 bytes
1.28 KB

#3 Dropped the array_values() call in order to maintain the current array structure: keyed by role id.

  • Berdir committed 9eee0de on 8.x-3.x authored by idebr
    Issue #3097390 by idebr: Only save configured roles in...
Berdir’s picture

Status: Needs review » Fixed

Thanks, it's unlikely that a problem would have occurred for someone, but better to be careful.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.