Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem
- Install Drupal with Minimal profile.
- Create a new user role.
- Go to /admin/config/people/accounts
- Bogus default value:
Cause
- The default value in user.settings is
admin_role: ''
user_admin_settings()
contains:
$roles = user_role_names(); unset($roles[DRUPAL_ANONYMOUS_RID]); unset($roles[DRUPAL_AUTHENTICATED_RID]); $roles[0] = t('disabled'); $form['admin_role']['user_admin_role'] = array( '#type' => 'select', '#title' => t('Administrator role'), '#default_value' => $config->get('admin_role'), '#options' => $roles,
- The default value is
0
(integer), whereas the actual default value is''
(empty string).
Proposed solution
- Replace this:
$roles[0] = t('disabled');
With this:
'#empty_value' => '', '#empty_option' => t('- Disabled -'),
Comment | File | Size | Author |
---|---|---|---|
#11 | drupal8.user-admin-role-test.11.patch | 760 bytes | pplantinga |
#4 | drupal8.user-admin-role-test.4.patch | 3.73 KB | sun |
#2 | drupal8.user-admin-role-default-value.1-before.png | 47.81 KB | andymartha |
#2 | drupal8.user-admin-role-default-value.1-after.png | 41.06 KB | andymartha |
#1 | drupal8.user-admin-role-default-value.1.patch | 1.01 KB | sun |
Comments
Comment #1
sunAttached patch fixes the bug.
Comment #2
andymartha CreditAttribution: andymartha commentedWhen pursuing these steps on Drupal 8.x-dev downloaded on March 6, 2013:
Install Drupal with Minimal profile.
Create a new user role.
Go to /admin/config/people/accounts
The default admin role is verified as a random role, often the newest created role. See attached "before" screenshot.
After applying patch drupal8.user-admin-role-default-value.1.patch in #1 from Sun, the default admin role is "none". See attached "after" screenshot.
Comment #3
Dries CreditAttribution: Dries commentedI committed it to 8.x.
Do we need tests for this?
Comment #4
sunYeah, since such a bogus default value - if overlooked/missed - could have a pretty negative security impact, we should add tests.
Apparently, there's no dedicated test for the admin role feature already, so I've created one and moved the only test into it.
Comment #6
sun#4: drupal8.user-admin-role-test.4.patch queued for re-testing.
Comment #7
Aleksey Zubko CreditAttribution: Aleksey Zubko commented#4: drupal8.user-admin-role-test.4.patch queued for re-testing.
Comment #8
benjifisher#4: drupal8.user-admin-role-test.4.patch queued for re-testing.
Comment #9
pplantinga CreditAttribution: pplantinga commented#4: drupal8.user-admin-role-test.4.patch queued for re-testing.
Comment #11
pplantinga CreditAttribution: pplantinga commentedI don't know if we necessarily need a dedicated test for admin role. If so, maybe we should create a followup.
Comment #12
sunI wasn't aware that this was still open for adding a test.
Just adding an assertion to the existing (as opposed to my larger test changes earlier) is fine with me.
Patch still applies cleanly for me locally.
Comment #13
catchCommitted/pushed to 8.x, thanks!