diff --git a/README.md b/README.md index 4649a7c28..c63e5927f 100644 --- a/README.md +++ b/README.md @@ -11,6 +11,7 @@ This repository holds configuration for the Hel.fi platform. - [Update instructions (2.x to 3.x)](documentation/update.md) - [Two-factor authentication/TFA/MFA](/modules/helfi_tfa/README.md) - [JSON:API remote entities](/modules/helfi_etusivu_entities/README.md) +- [Users](/modules/helfi_users/README.md) ## Contact diff --git a/composer.json b/composer.json index 5305be1e3..f2868651e 100644 --- a/composer.json +++ b/composer.json @@ -75,6 +75,7 @@ "drupal/ctools": "<3.11 || ^4.0.1", "drupal/gin_toolbar": ">1.0.0-rc6", "drupal/helfi_media_map": "*", + "drupal/default_content": ">2.0.0-alpha2", "drush/drush": "<12" }, "extra": { diff --git a/helfi_platform_config.install b/helfi_platform_config.install index 72cc3ac42..7a7adeeb1 100644 --- a/helfi_platform_config.install +++ b/helfi_platform_config.install @@ -168,3 +168,16 @@ function helfi_platform_config_update_9311(): void { } } } + +/** + * UHF-9708: Enable helfi_users module. + */ +function helfi_platform_config_update_9312() : void { + helfi_platform_config_remove_permissions_from_all_roles([ + 'delete user accounts', + ]); + + if (!\Drupal::moduleHandler()->moduleExists('helfi_users')) { + \Drupal::service('module_installer')->install(['helfi_users']); + } +} diff --git a/helfi_platform_config.module b/helfi_platform_config.module index 2f9a32dc7..22c7ee45b 100644 --- a/helfi_platform_config.module +++ b/helfi_platform_config.module @@ -496,31 +496,6 @@ function helfi_platform_config_config_ignore_settings_alter(array &$settings) { } } -/** - * Implements hook_user_cancel_methods_alter(). - */ -function helfi_platform_config_user_cancel_methods_alter(array &$methods): void { - /** @var \Drupal\Core\Session\AccountInterface $account */ - $account = \Drupal::currentUser(); - - // Only allow user to disable user accounts if the user doesn't have - // a permission to delete user accounts. - $white_listed_methods = [ - 'user_cancel_block', - 'user_cancel_block_unpublish', - ]; - - // Deny access to all non-whitelisted methods if user doesn't have - // the 'delete user accounts' permission. - if (!$account->hasPermission('delete user accounts')) { - foreach ($methods as $name => &$method) { - if (!in_array($name, $white_listed_methods)) { - $method['access'] = FALSE; - } - } - } -} - /** * Implements hook_config_schema_info_alter(). */ diff --git a/helfi_platform_config.permissions.yml b/helfi_platform_config.permissions.yml deleted file mode 100644 index 65cb8132e..000000000 --- a/helfi_platform_config.permissions.yml +++ /dev/null @@ -1,2 +0,0 @@ -delete user accounts: - title: Delete user accounts diff --git a/modules/helfi_users/Readme.md b/modules/helfi_users/Readme.md new file mode 100644 index 000000000..4643be34b --- /dev/null +++ b/modules/helfi_users/Readme.md @@ -0,0 +1,7 @@ +# HELfi users + +Fixes related to deleting or banning users. + +Other user account related fixes and features: + - [Expired users](https://github.com/City-of-Helsinki/drupal-module-helfi-api-base/blob/main/documentation/user-expire.md) + - [Infofinland user sanitization](https://github.com/City-of-Helsinki/drupal-infofinland/blob/dev/public/modules/custom/infofinland_user_cancel/README.md) diff --git a/modules/helfi_users/helfi_users.info.yml b/modules/helfi_users/helfi_users.info.yml new file mode 100644 index 000000000..79569281f --- /dev/null +++ b/modules/helfi_users/helfi_users.info.yml @@ -0,0 +1,7 @@ +name: HELfi users +description: 'Fixes related to deleting or canceling users.' +package: HELfi +type: module +core_version_requirement: ^10 || ^11 +'interface translation project': helfi_users +'interface translation server pattern': modules/contrib/helfi_platform_config/modules/helfi_users/translations/%language.po diff --git a/modules/helfi_users/helfi_users.module b/modules/helfi_users/helfi_users.module new file mode 100644 index 000000000..6e40656d6 --- /dev/null +++ b/modules/helfi_users/helfi_users.module @@ -0,0 +1,133 @@ + $group] + $implementations; + } +} + +/** + * Implements hook_form_alter(). + */ +function helfi_users_form_alter(&$form, FormStateInterface $form_state, $form_id): void { + if (!in_array($form_id, ["user_multiple_cancel_confirm", "user_cancel_form"])) { + return; + } + + // Hide email confirmation checkbox. + $form['user_cancel_confirm']['#access'] = FALSE; + $form['user_cancel_method']['#description'] = new TranslatableMarkup( + "Banning accounts prevents them from logging in. If the account is no longer needed, it should be deleted." + ); +} + +/** + * Implements hook_user_cancel_methods_alter(). + */ +function helfi_users_user_cancel_methods_alter(array &$methods): void { + // User can only access allowed methods. User must also have + // 'administer users' permission from core to be able to cancel users. + $allowed_methods = [ + 'user_cancel_block' => new TranslatableMarkup("Ban the account and keep their content."), + 'user_cancel_block_unpublish' => new TranslatableMarkup("Ban the account and unpublish their content."), + 'user_cancel_reassign' => new TranslatableMarkup("Delete the account and make their content belong to %uid1. This action cannot be undone.", [ + '%uid1' => User::load(1)->getAccountName(), + ]), + ]; + + foreach ($allowed_methods as $name => $title) { + $methods[$name]['title'] = $title; + } + + // Without special permission, user is not allowed to access all methods. + if (!\Drupal::currentUser()->hasPermission('allow all user cancel methods')) { + foreach ($methods as $name => &$method) { + $method['access'] = array_key_exists($name, $allowed_methods); + } + } +} + +/** + * Implements hook_user_cancel(). + * + * We have encountered crashes/timeout issues with reassign batch api + * implementation from node module. This Optimizes hook_user_cancel by + * reassigning nodes with direct database query. + * + * This has to run before node module's user_cancel hook. + */ +function helfi_users_user_cancel($edit, UserInterface $account, $method): void { + // Reassign nodes for the old account. + if ($method === 'user_cancel_reassign') { + _helfi_users_reassign_nodes($account, User::load(1)); + } +} + +/** + * Reassigns all node revisions from $source to $target. + * + * Prevents crashes and timeouts when revisions are handled by node_mass_update. + * + * @param \Drupal\Core\Session\AccountInterface $source + * Source user. + * @param \Drupal\Core\Session\AccountInterface $target + * Target user. + */ +function _helfi_users_reassign_nodes(AccountInterface $source, AccountInterface $target): void { + $database = \Drupal::database(); + $tables = [ + 'node_field_data' => 'uid', + 'node_field_revision' => 'uid', + 'node_revision' => 'revision_uid', + ]; + + foreach ($tables as $table => $uid_field) { + $matches = $database->select($table) + ->condition($uid_field, $source->id()) + ->countQuery() + ->execute() + ->fetchField(); + + if ((int) $matches < 1) { + continue; + } + + // Notice: this does not invalidate any caches. This should be fine for + // HELfi, where user information is not rendered on public pages. + $database->update($table) + ->fields([$uid_field => $target->id()]) + ->condition($uid_field, $source->id()) + ->execute(); + + \Drupal::logger('helfi_users')->notice(t('Set @count rows from @table to @target from @source', [ + '@count' => $matches, + '@table' => $table, + '@target' => $target->id(), + '@source' => $source->id(), + ])); + } +} diff --git a/modules/helfi_users/helfi_users.permissions.yml b/modules/helfi_users/helfi_users.permissions.yml new file mode 100644 index 000000000..d605b1dad --- /dev/null +++ b/modules/helfi_users/helfi_users.permissions.yml @@ -0,0 +1,2 @@ +allow all user cancel methods: + title: All access to all user cancel methods diff --git a/tests/src/Functional/UserCancelFormTest.php b/modules/helfi_users/tests/src/Functional/UserCancelFormTest.php similarity index 95% rename from tests/src/Functional/UserCancelFormTest.php rename to modules/helfi_users/tests/src/Functional/UserCancelFormTest.php index 1aad023ac..ebc95b00b 100644 --- a/tests/src/Functional/UserCancelFormTest.php +++ b/modules/helfi_users/tests/src/Functional/UserCancelFormTest.php @@ -18,7 +18,7 @@ class UserCancelFormTest extends BrowserTestBase { */ protected static $modules = [ 'user', - 'helfi_platform_config', + 'helfi_users', ]; /** @@ -60,7 +60,7 @@ public function testUserCancelForm(): void { $this->assertSession()->statusCodeEquals(200); $this->assertSession()->elementExists('xpath', '//input[@value="user_cancel_block"]'); $this->assertSession()->elementExists('xpath', '//input[@value="user_cancel_block_unpublish"]'); - $this->assertSession()->elementNotExists('xpath', '//input[@value="user_cancel_reassign"]'); + $this->assertSession()->elementExists('xpath', '//input[@value="user_cancel_reassign"]'); $this->assertSession()->elementNotExists('xpath', '//input[@value="user_cancel_delete"]'); // Test that the editorUser cannot access the cancel page. diff --git a/modules/helfi_users/tests/src/Kernel/UserCancelNodeRevisionsTest.php b/modules/helfi_users/tests/src/Kernel/UserCancelNodeRevisionsTest.php new file mode 100644 index 000000000..0f0adac52 --- /dev/null +++ b/modules/helfi_users/tests/src/Kernel/UserCancelNodeRevisionsTest.php @@ -0,0 +1,97 @@ +installEntitySchema($entityTypeId); + } + + $this->installSchema('node', 'node_access'); + + $this->admin = $this->createUser(name: 'admin', values: [ + 'uid' => 1, + ]); + } + + /** + * Tests revision reassign function. + */ + public function testRevisionAnonymization(): void { + $testUser = $this->createUser(name: 'testuser'); + $node = $this->createNode([ + 'uid' => $testUser->id(), + ]); + + // Create few revisions for a total of 4 including the original. + $revision_count = 3; + for ($i = 0; $i < $revision_count; $i++) { + $node->setNewRevision(); + $node + ->setTitle($this->randomMachineName()) + ->save(); + } + + // Run function for test user, assign content to . + _helfi_users_reassign_nodes($testUser, $this->admin); + + // Test that revisions for this user were anonymized correctly. + $connection = Database::getConnection(); + $revision_count = $connection->select('node_revision') + ->condition('revision_uid', $testUser->id()) + ->condition('nid', $node->id()) + ->countQuery() + ->execute() + ->fetchField(); + $this->assertEquals(0, (int) $revision_count, 'Found revisions after running anonymization function.'); + + // Test that the revisions were correctly assigned to target user. + $anon_revision_count = $connection->select('node_revision') + ->condition('revision_uid', $this->admin->id()) + ->condition('nid', $node->id()) + ->countQuery() + ->execute() + ->fetchField(); + $this->assertEquals(4, (int) $anon_revision_count, 'Amount of anonymized revisions does not match'); + } + +} diff --git a/modules/helfi_users/translations/fi.po b/modules/helfi_users/translations/fi.po new file mode 100644 index 000000000..bf1eb9146 --- /dev/null +++ b/modules/helfi_users/translations/fi.po @@ -0,0 +1,14 @@ +msgid "" +msgstr "" + +msgid "Ban the account and keep their content." +msgstr "Estä käyttäjä ja säilytä luotu sisältö." + +msgid "Ban the account and unpublish their content." +msgstr "Estä käyttäjä ja piilota luotu sisältö." + +msgid "Delete the account and make their content belong to %uid1. This action cannot be undone." +msgstr "Poista käyttäjä ja siirrä luotu sisältö käyttäjälle %uid1. Tätä toimintoa ei voi peruuttaa." + +msgid "Banning accounts prevents them from logging in. If the account is no longer needed, it should be deleted." +msgstr "Käyttäjän estäminen estää kirjautumisen käyttäjällä. Käyttäjä tulisi poistaa jos sitä ei enää tarvita."