diff --git a/config/install/oe_authentication.settings.yml b/config/install/oe_authentication.settings.yml index 227ac511..5616f2b9 100644 --- a/config/install/oe_authentication.settings.yml +++ b/config/install/oe_authentication.settings.yml @@ -4,3 +4,4 @@ validation_path: 'TicketValidationService' assurance_level: TOP ticket_types: SERVICE,PROXY force_2fa: false +restrict_user_delete_cancel_methods: true diff --git a/config/schema/oe_authentication.schema.yml b/config/schema/oe_authentication.schema.yml index d1b6f94b..2a39713f 100644 --- a/config/schema/oe_authentication.schema.yml +++ b/config/schema/oe_authentication.schema.yml @@ -20,3 +20,6 @@ oe_authentication.settings: force_2fa: type: boolean label: 'Force two factor authentication' + restrict_user_delete_cancel_methods: + type: boolean + label: 'Restrict access to user cancel methods that permanently delete the account' diff --git a/oe_authentication.module b/oe_authentication.module index 462bcd44..71e0a1c1 100644 --- a/oe_authentication.module +++ b/oe_authentication.module @@ -10,10 +10,17 @@ declare(strict_types=1); /** * Implements hook_user_cancel_methods_alter(). * - * Remove the option to delete users while keeping the option to block them. + * Conditionally removes the option to delete users. */ function oe_authentication_user_cancel_methods_alter(&$methods) { - if (\Drupal::currentUser()->id() == 1) { + $current_user = \Drupal::currentUser(); + $restrict_user_delete = \Drupal::configFactory() + ->get('oe_authentication.settings') + ->get('restrict_user_delete_cancel_methods') ?? TRUE; + // @todo The restriction should not check for UID 1 as it's bad practice. It + // should either check for the user being an admin, or use a permission + // based approach like the module drupal/user_cancel_methods_permissions. + if ($current_user->id() == 1 || $restrict_user_delete !== TRUE) { return; } $restricted_options = [ @@ -21,6 +28,8 @@ function oe_authentication_user_cancel_methods_alter(&$methods) { 'user_cancel_delete', ]; foreach ($restricted_options as $restricted_option) { + // @todo the 'access' property of the cancel methods should be used, rather + // than removing the method altogether. unset($methods[$restricted_option]); } } diff --git a/oe_authentication.post_update.php b/oe_authentication.post_update.php index 7fa0799e..52e4b140 100644 --- a/oe_authentication.post_update.php +++ b/oe_authentication.post_update.php @@ -42,3 +42,12 @@ function oe_authentication_post_update_00004(): void { ->set('user_accounts.auto_register_follow_registration_policy', TRUE) ->save(); } + +/** + * Set the restrict_user_delete_cancel_methods to TRUE as default. + */ +function oe_authentication_post_update_00005(): void { + \Drupal::configFactory()->getEditable('oe_authentication.settings') + ->set('restrict_user_delete_cancel_methods', TRUE) + ->save(); +} diff --git a/src/Form/AuthenticationSettingsForm.php b/src/Form/AuthenticationSettingsForm.php index a8b3d14c..8e21dd87 100644 --- a/src/Form/AuthenticationSettingsForm.php +++ b/src/Form/AuthenticationSettingsForm.php @@ -58,6 +58,12 @@ public function buildForm(array $form, FormStateInterface $form_state) { '#title' => $this->t('Force two factor authentication'), '#default_value' => $this->config(static::CONFIG_NAME)->get('force_2fa'), ]; + $form['restrict_user_delete_cancel_methods'] = [ + '#type' => 'checkbox', + '#title' => $this->t('Restrict access to user cancel methods that permanently delete the account'), + '#description' => $this->t('If enabled, only the super admin user (uid 1) will have access to these methods.'), + '#default_value' => $this->config(static::CONFIG_NAME)->get('restrict_user_delete_cancel_methods'), + ]; return parent::buildForm($form, $form_state); } @@ -72,6 +78,7 @@ public function submitForm(array &$form, FormStateInterface $form_state) { ->set('assurance_level', $form_state->getValue('assurance_level')) ->set('ticket_types', $form_state->getValue('ticket_types')) ->set('force_2fa', (bool) $form_state->getValue('force_2fa')) + ->set('restrict_user_delete_cancel_methods', (bool) $form_state->getValue('restrict_user_delete_cancel_methods')) ->save(); parent::submitForm($form, $form_state); } diff --git a/tests/Behat/AuthenticationContext.php b/tests/Behat/AuthenticationContext.php index 166baac5..4e2967a6 100644 --- a/tests/Behat/AuthenticationContext.php +++ b/tests/Behat/AuthenticationContext.php @@ -115,6 +115,27 @@ public function visitOwnUserPage(): void { $this->visitPath($url->getInternalPath()); } + /** + * Navigates to other user's page. + * + * @var string $username + * The name of the user whose page is to be visited. + * + * @Given I visit the :user_name user page + * + * @throws \Exception + * Thrown when the user with the given name does not exist. + */ + public function visitUserPage(string $user_name): void { + $user = user_load_by_name($user_name); + if ($user === FALSE) { + throw new \Exception(sprintf('User with name %s could not be found.', $user_name)); + } + /** @var \Drupal\Core\Url $url */ + $url = $user->toUrl(); + $this->visitPath($url->getInternalPath()); + } + /** * Configures the the Drupal site so that users are active on creation. * @@ -143,4 +164,28 @@ public function allowExternalUsers(): void { $this->configContext->setConfig('oe_authentication.settings', 'assurance_level', 'LOW'); } + /** + * Configures oe_authentication module to allow to use user delete options. + * + * @Given the site is configured to not restrict user delete options + */ + public function setConfigEnableDeleteUsersOptions(): void { + $this->configContext->setConfig('oe_authentication.settings', 'restrict_user_delete_cancel_methods', FALSE); + } + + /** + * Logs in as superuser user using default credentials. + * + * @Given I am logged in as the superuser user + */ + public function loginSuperuser(): void { + $user = (object) [ + 'uid' => 1, + 'name' => 'admin', + 'pass' => 'admin', + ]; + // Login. + $this->login($user); + } + } diff --git a/tests/features/configure_authentication.feature b/tests/features/configure_authentication.feature index 405589ea..a313e1b6 100644 --- a/tests/features/configure_authentication.feature +++ b/tests/features/configure_authentication.feature @@ -17,6 +17,7 @@ Feature: Authentication And the "Application assurance levels" field should contain "TOP" And the "Application available ticket types" field should contain "SERVICE,PROXY" And the "Force two factor authentication" checkbox should not be checked + And the "Restrict access to user cancel methods that permanently delete the account" checkbox should be checked # Change the configuration values. When I fill in "Application authentication protocol" with "something" @@ -25,6 +26,7 @@ Feature: Authentication And I fill in "Application assurance levels" with "assurance" And I fill in "Application available ticket types" with "ticket.test" And I check the box "Force two factor authentication" + And I uncheck the box "Restrict access to user cancel methods that permanently delete the account" And I press "Save configuration" Then I should see the message "The configuration options have been saved." And the "Application authentication protocol" field should contain "something" @@ -33,3 +35,4 @@ Feature: Authentication And the "Application assurance levels" field should contain "assurance" And the "Application available ticket types" field should contain "ticket.test" And the "Force two factor authentication" checkbox should be checked + And the "Restrict access to user cancel methods that permanently delete the account" checkbox should not be checked diff --git a/tests/features/user-management.feature b/tests/features/user-management.feature index 2e7850d4..90492e25 100644 --- a/tests/features/user-management.feature +++ b/tests/features/user-management.feature @@ -1,13 +1,40 @@ @api +@DrupalLogin Feature: Manage users Because users are managed externally I should not be able to delete users on the site + Unless I am superuser user + Or module configuration to restrict the user delete options is not enabled - @DrupalLogin Scenario: Users should not be able to delete other users Given I am logged in as a user with the "administer users" permissions When I visit my user page And I click "Edit" And I click "Cancel account" Then I should see "Disable the account and keep its content." - And I should not see "Delete the account and its content." + And I should see "Disable the account and unpublish its content." + And I should not see "Delete the account" + + Scenario: As the superuser I can access the delete options + Given users: + | name | mail | roles | + | foo | foo@bar.com | | + And I am logged in as the superuser user + When I visit the foo user page + And I click "Edit" + And I click "Cancel account" + Then I should see "Disable the account and keep its content." + And I should see "Disable the account and unpublish its content." + And I should see "Delete the account and its content." + And I should see "Delete the account and make its content belong to the Anonymous user." + + Scenario: Users should be able to delete other users when the configuration to restrict the user delete options is not enabled. + Given the site is configured to not restrict user delete options + And I am logged in as a user with the "administer users" permissions + When I visit my user page + And I click "Edit" + And I click "Cancel account" + Then I should see "Disable the account and keep its content." + And I should see "Disable the account and unpublish its content." + And I should see "Delete the account and its content." + And I should see "Delete the account and make its content belong to the Anonymous user."