Skip to content

Commit

Permalink
UHF-8652: Merge remote-tracking branch 'origin/main' into UHF-8652
Browse files Browse the repository at this point in the history
  • Loading branch information
hyrsky committed Aug 27, 2024
2 parents 8099914 + e9d2c29 commit 4f06ec6
Show file tree
Hide file tree
Showing 12 changed files with 277 additions and 29 deletions.
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -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": {
Expand Down
13 changes: 13 additions & 0 deletions helfi_platform_config.install
Original file line number Diff line number Diff line change
Expand Up @@ -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']);
}
}
25 changes: 0 additions & 25 deletions helfi_platform_config.module
Original file line number Diff line number Diff line change
Expand Up @@ -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().
*/
Expand Down
2 changes: 0 additions & 2 deletions helfi_platform_config.permissions.yml

This file was deleted.

7 changes: 7 additions & 0 deletions modules/helfi_users/Readme.md
Original file line number Diff line number Diff line change
@@ -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)
7 changes: 7 additions & 0 deletions modules/helfi_users/helfi_users.info.yml
Original file line number Diff line number Diff line change
@@ -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
133 changes: 133 additions & 0 deletions modules/helfi_users/helfi_users.module
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
<?php

/**
* @file
* Helper functions related to deleting user accounts.
*
* This is in a separate module, so it can be turned off easier,
* and we can reassign module weights / run order if necessary.
*/

declare(strict_types=1);

use Drupal\Core\Form\FormStateInterface;
use Drupal\Core\Session\AccountInterface;
use Drupal\Core\StringTranslation\TranslatableMarkup;
use Drupal\user\Entity\User;
use Drupal\user\UserInterface;

/**
* Implements hook_module_implements_alter().
*/
function helfi_users_module_implements_alter(&$implementations, $hook) : void {
// Move helfi_users_user_cancel() implementation to the top of the
// list, so this is always run first before any other alter hooks, more
// specifically before 'node_user_cancel()' which causes issues when mass
// reassigning node revisions.
if ($hook === 'user_cancel') {
$group = $implementations['helfi_users'];
$implementations = ['helfi_users' => $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(),
]));
}
}
2 changes: 2 additions & 0 deletions modules/helfi_users/helfi_users.permissions.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
allow all user cancel methods:
title: All access to all user cancel methods
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ class UserCancelFormTest extends BrowserTestBase {
*/
protected static $modules = [
'user',
'helfi_platform_config',
'helfi_users',
];

/**
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
<?php

namespace Drupal\Tests\helfi_users\Functional;

use Drupal\Core\Database\Database;
use Drupal\KernelTests\KernelTestBase;
use Drupal\Tests\node\Traits\NodeCreationTrait;
use Drupal\Tests\user\Traits\UserCreationTrait;
use Drupal\user\UserInterface;

/**
* Tests for reassigning node revisions when canceling users.
*/
class UserCancelNodeRevisionsTest extends KernelTestBase {

use NodeCreationTrait;
use UserCreationTrait;

/**
* Uid 1 user.
*/
protected UserInterface $admin;

/**
* {@inheritdoc}
*/
protected $defaultTheme = 'stark';

/**
* {@inheritdoc}
*/
protected static $modules = [
'node',
'user',
'system',
'helfi_users',
];

/**
* {@inheritdoc}
*/
protected function setUp(): void {
parent::setUp();

foreach (['node', 'user'] as $entityTypeId) {
$this->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');
}

}
14 changes: 14 additions & 0 deletions modules/helfi_users/translations/fi.po
Original file line number Diff line number Diff line change
@@ -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."

0 comments on commit 4f06ec6

Please sign in to comment.