Issue #2953111 by quietone, yogeshmpawar, ravi.shankar, benjifisher, alexpott, mikelutz, catch, andypost, larowlan, danflanagan8: Only migrate role permissions that exist on the destination

(cherry picked from commit 247bd31efd)
merge-requests/1766/head^2
Alex Pott 2022-09-12 15:30:44 +01:00
parent 4a67a410b0
commit 1002797d91
No known key found for this signature in database
GPG Key ID: BDA67E7EE836E5CE
8 changed files with 582 additions and 99 deletions

View File

@ -35,13 +35,15 @@ process:
- plugin: node_update_7008
- plugin: flatten
- plugin: filter_format_permission
# A special flag so we can migrate permissions that do not exist yet.
# @todo Remove in https://www.drupal.org/project/drupal/issues/2953111.
skip_missing_permission_deprecation:
plugin: default_value
default_value: true
destination:
plugin: entity:user_role
migration_dependencies:
required:
- d6_filter_format
optional:
- block_content_type
- contact_category
- d6_comment_type
- d6_node_type
- d6_taxonomy_vocabulary
- d6_taxonomy_vocabulary_translation

View File

@ -33,13 +33,15 @@ process:
'edit own forum topics': 'edit own forum content'
- plugin: flatten
weight: weight
# A special flag so we can migrate permissions that do not exist yet.
# @todo Remove in https://www.drupal.org/project/drupal/issues/2953111.
skip_missing_permission_deprecation:
plugin: default_value
default_value: true
destination:
plugin: entity:user_role
migration_dependencies:
optional:
- block_content_type
- contact_category
- d7_comment_type
- d7_filter_format
- d7_node_type
- d7_shortcut_set
- d7_taxonomy_vocabulary
- d7_taxonomy_vocabulary_translation

View File

@ -202,7 +202,7 @@ class Role extends ConfigEntityBase implements RoleInterface {
$permission_definitions = \Drupal::service('user.permissions')->getPermissions();
$valid_permissions = array_intersect($this->permissions, array_keys($permission_definitions));
$invalid_permissions = array_diff($this->permissions, $valid_permissions);
if (!empty($invalid_permissions) && !$this->get('skip_missing_permission_deprecation')) {
if (!empty($invalid_permissions)) {
throw new \RuntimeException('Adding non-existent permissions to a role is not allowed. The incorrect permissions are "' . implode('", "', $invalid_permissions) . '".');
}
foreach ($valid_permissions as $permission) {

View File

@ -0,0 +1,95 @@
<?php
namespace Drupal\user\Plugin\migrate\destination;
use Drupal\Core\Config\ConfigFactoryInterface;
use Drupal\Core\Entity\EntityStorageInterface;
use Drupal\Core\Language\LanguageManagerInterface;
use Drupal\migrate\Plugin\migrate\destination\EntityConfigBase;
use Drupal\migrate\Plugin\MigrationInterface;
use Drupal\migrate\Row;
use Symfony\Component\DependencyInjection\ContainerInterface;
/**
* Provides a destination plugin for migrating user role entities.
*
* @MigrateDestination(
* id = "entity:user_role"
* )
*/
class EntityUserRole extends EntityConfigBase {
/**
* All permissions on the destination site.
*
* @var string[]
*/
protected $destinationPermissions = [];
/**
* Builds a user role entity destination.
*
* @param array $configuration
* A configuration array containing information about the plugin instance.
* @param string $plugin_id
* The plugin_id for the plugin instance.
* @param mixed $plugin_definition
* The plugin implementation definition.
* @param \Drupal\migrate\Plugin\MigrationInterface $migration
* The migration.
* @param \Drupal\Core\Entity\EntityStorageInterface $storage
* The storage for this entity type.
* @param array $bundles
* The list of bundles this entity type has.
* @param \Drupal\Core\Language\LanguageManagerInterface $language_manager
* The language manager.
* @param \Drupal\Core\Config\ConfigFactoryInterface $config_factory
* The configuration factory.
* @param array $destination_permissions
* All available permissions.
*/
public function __construct(array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration, EntityStorageInterface $storage, array $bundles, LanguageManagerInterface $language_manager, ConfigFactoryInterface $config_factory, array $destination_permissions) {
parent::__construct($configuration, $plugin_id, $plugin_definition, $migration, $storage, $bundles, $language_manager, $config_factory);
$this->destinationPermissions = $destination_permissions;
}
/**
* {@inheritdoc}
*/
public static function create(ContainerInterface $container, array $configuration, $plugin_id, $plugin_definition, MigrationInterface $migration = NULL) {
$entity_type_id = static::getEntityTypeId($plugin_id);
return new static(
$configuration,
$plugin_id,
$plugin_definition,
$migration,
$container->get('entity_type.manager')->getStorage($entity_type_id),
array_keys($container->get('entity_type.bundle.info')->getBundleInfo($entity_type_id)),
$container->get('language_manager'),
$container->get('config.factory'),
array_keys($container->get('user.permissions')->getPermissions()),
);
}
/**
* {@inheritdoc}
*/
public function import(Row $row, array $old_destination_id_values = []): array|bool {
$permissions = $row->getDestinationProperty('permissions') ?? [];
// Get permissions that do not exist on the destination.
$invalid_permissions = array_diff($permissions, $this->destinationPermissions);
if ($invalid_permissions) {
sort($invalid_permissions);
// Log the message in the migration message table.
$message = "Permission(s) '" . implode("', '", $invalid_permissions) . "' not found.";
$this->migration->getIdMap()
->saveMessage($row->getSourceIdValues(), $message, MigrationInterface::MESSAGE_WARNING);
}
$valid_permissions = array_intersect($permissions, $this->destinationPermissions);
$row->setDestinationProperty('permissions', $valid_permissions);
return parent::import($row, $old_destination_id_values);
}
}

View File

@ -0,0 +1,65 @@
<?php
namespace Drupal\Tests\user\Functional\Update;
use Drupal\FunctionalTests\Update\UpdatePathTestBase;
use Drupal\user\Entity\Role;
/**
* Tests user_post_update_update_roles_followup() upgrade path.
*
* @group Update
* @group legacy
*/
class UserUpdateRoleMigrateTest extends UpdatePathTestBase {
/**
* {@inheritdoc}
*/
protected function setDatabaseDumpFiles() {
$this->databaseDumpFiles = [
__DIR__ . '/../../../../../system/tests/fixtures/update/drupal-9.4.0.bare.standard.php.gz',
];
}
/**
* Tests that roles have only existing permissions.
*/
public function testRolePermissions() {
/** @var \Drupal\Core\Database\Connection $connection */
$connection = \Drupal::service('database');
// Edit the authenticated role to have a non-existent permission.
$authenticated = $connection->select('config')
->fields('config', ['data'])
->condition('collection', '')
->condition('name', 'user.role.authenticated')
->execute()
->fetchField();
$authenticated = unserialize($authenticated);
$authenticated['permissions'][] = 'does_not_exist';
$connection->update('config')
->fields([
'data' => serialize($authenticated),
])
->condition('collection', '')
->condition('name', 'user.role.authenticated')
->execute();
$authenticated = Role::load('authenticated');
$this->assertTrue($authenticated->hasPermission('does_not_exist'), 'Authenticated role has a permission that does not exist');
$this->runUpdates();
$this->assertSession()->pageTextContains('The role Authenticated user has had non-existent permissions removed. Check the logs for details.');
$authenticated = Role::load('authenticated');
$this->assertFalse($authenticated->hasPermission('does_not_exist'), 'Authenticated role does not have a permission that does not exist');
$this->drupalLogin($this->createUser(['access site reports']));
$this->drupalGet('admin/reports/dblog', ['query' => ['type[]' => 'update']]);
$this->clickLink('The role Authenticated user has had the following non-…');
$this->assertSession()->pageTextContains('The role Authenticated user has had the following non-existent permission(s) removed: does_not_exist.');
}
}

View File

@ -2,6 +2,7 @@
namespace Drupal\Tests\user\Kernel\Migrate\d6;
use Drupal\migrate\Plugin\MigrationInterface;
use Drupal\user\Entity\Role;
use Drupal\user\RoleInterface;
use Drupal\Tests\migrate_drupal\Kernel\d6\MigrateDrupal6TestBase;
@ -19,7 +20,43 @@ class MigrateUserRoleTest extends MigrateDrupal6TestBase {
*/
protected function setUp(): void {
parent::setUp();
$this->executeMigrations(['d6_filter_format', 'd6_user_role']);
$this->startCollectingMessages();
}
/**
* Assert the logged migrate messages.
*
* @param string[][] $role_data
* An array of role data keyed by the destination role id. The role data
* contains the source role id, an array of valid permissions and an array
* of invalid permissions.
* @param \Drupal\migrate\Plugin\MigrateIdMapInterface $id_map
* The migration ID map plugin.
*/
public function assertMessages(array $role_data, MigrateIdMapInterface $id_map) {
foreach ($id_map->getMessages() as $message) {
$permissions = implode("', '", $role_data[$message->dest_id]['invalid']);
$expected_message = "Permission(s) '" . $permissions . "' not found.";
$this->assertSame($expected_message, $message->message);
$this->assertSame(MigrationInterface::MESSAGE_WARNING, (int) $message->level);
}
}
/**
* Asserts there are no duplicate roles.
*/
public function assertNoDuplicateRoles() {
$roles = [
'anonymous1',
'authenticated1',
'administrator1',
'migrate_test_role_11',
'migrate_test_role_21',
'migrate_test_role_3_that_is_longer_than_thirty_two_characters1',
'migrate_test_role_41',
];
$this->assertEmpty(Role::loadMultiple($roles));
}
/**
@ -40,7 +77,6 @@ class MigrateUserRoleTest extends MigrateDrupal6TestBase {
/** @var \Drupal\user\RoleInterface $role */
$role = Role::load($id);
$this->assertInstanceOf(RoleInterface::class, $role);
sort($permissions);
$this->assertSame($permissions, $role->getPermissions());
$this->assertSame([[$id]], $id_map->lookupDestinationIds(['rid' => $lookupId]));
}
@ -49,81 +85,217 @@ class MigrateUserRoleTest extends MigrateDrupal6TestBase {
* Helper function to test the migration of the user roles. The user roles
* will be re-imported and the tests here will be repeated.
*
* @param array $permissions
* Contains the valid and invalid permissions.
* @param \Drupal\migrate\Plugin\MigrateIdMapInterface $id_map
* The map table plugin.
*
* @internal
*/
protected function assertRoles(MigrateIdMapInterface $id_map): void {
protected function assertRoles(array $permissions, MigrateIdMapInterface $id_map): void {
foreach ($permissions as $rid => $datum) {
$this->assertRole($rid, $datum['valid'], $datum['rid'], $id_map);
}
}
// The permissions for each role are found in the two tables in the Drupal 6
// source database. One is the permission table and the other is the
// filter_format table.
$permissions = [
// From permission table.
'access content',
'migrate test anonymous permission',
// From filter_format tables.
'use text format filtered_html',
/**
* Data provider for user role migration tests.
*/
public function providerTestUserRole() {
return [
'filter only' => [
'modules' => [],
'migrations' => [
'd6_filter_format',
'd6_user_role',
],
'role_data' => [
'anonymous' => [
'rid' => '1',
'valid' => [
'access content',
'use text format filtered_html',
],
'invalid' => [
'migrate test anonymous permission',
],
],
'authenticated' => [
'rid' => '2',
'valid' => [
'access content',
'use text format filtered_html',
],
'invalid' => [
'access comments',
'migrate test authenticated permission',
'post comments',
'skip comment approval',
],
],
'migrate_test_role_1' => [
'rid' => '3',
'valid' => [
'use text format full_html',
'use text format php_code',
],
'invalid' => [
'migrate test role 1 test permission',
],
],
'migrate_test_role_2' => [
'rid' => '4',
'valid' => [
'access content overview',
'administer nodes',
'use text format php_code',
],
'invalid' => [
'administer contact forms',
'create forum content',
'delete any blog content',
'delete any forum content',
'delete own blog content',
'delete own forum content',
'edit any blog content',
'edit any forum content',
'edit own blog content',
'edit own forum content',
'migrate test role 2 test permission',
'skip comment approval',
'use PHP for settings',
],
],
'migrate_test_role_3_that_is_longer_than_thirty_two_characters' => [
'rid' => '5',
'valid' => [
'use text format php_code',
],
'invalid' => [],
],
],
],
'all dependent migrations' => [
'modules' => [
'block',
'block_content',
'comment',
'contact',
'config_translation',
'language',
'link',
'menu_ui',
'node',
'taxonomy',
'text',
],
'migrations' => [
'language',
'd6_comment_type',
'block_content_type',
'contact_category',
'd6_filter_format',
'd6_taxonomy_vocabulary',
'd6_taxonomy_vocabulary_translation',
'd6_user_role',
],
'role_data' => [
'anonymous' => [
'rid' => '1',
'valid' => [
'access content',
'use text format filtered_html',
],
'invalid' => [
'migrate test anonymous permission',
],
],
'authenticated' => [
'rid' => '2',
'valid' => [
'access comments',
'access content',
'post comments',
'skip comment approval',
'use text format filtered_html',
],
'invalid' => [
'migrate test authenticated permission',
],
],
'migrate_test_role_1' => [
'rid' => '3',
'valid' => [
'use text format full_html',
'use text format php_code',
],
'invalid' => [
'migrate test role 1 test permission',
],
],
'migrate_test_role_2' => [
'rid' => '4',
'valid' => [
'access content overview',
'administer contact forms',
'administer nodes',
'create forum content',
'delete any forum content',
'delete own forum content',
'edit any forum content',
'edit own forum content',
'skip comment approval',
'use text format php_code',
],
'invalid' => [
'delete any blog content',
'delete own blog content',
'edit any blog content',
'edit own blog content',
'migrate test role 2 test permission',
'use PHP for settings',
],
],
'migrate_test_role_3_that_is_longer_than_thirty_two_characters' => [
'rid' => '5',
'valid' => [
'use text format php_code',
],
'invalid' => [],
],
],
],
];
$this->assertRole('anonymous', $permissions, 1, $id_map);
$permissions = [
// From permission table.
'access comments',
'access content',
'migrate test authenticated permission',
'post comments',
'skip comment approval',
// From filter_format.
'use text format filtered_html',
];
$this->assertRole('authenticated', $permissions, 2, $id_map);
$permissions = [
// From permission table.
'migrate test role 1 test permission',
// From filter format.
'use text format full_html',
'use text format php_code',
];
$this->assertRole('migrate_test_role_1', $permissions, 3, $id_map);
$permissions = [
// From permission table.
'migrate test role 2 test permission',
'use PHP for settings',
'administer contact forms',
'skip comment approval',
'edit own blog content',
'edit any blog content',
'delete own blog content',
'delete any blog content',
'create forum content',
'delete any forum content',
'delete own forum content',
'edit any forum content',
'edit own forum content',
'administer nodes',
'access content overview',
// From filter format.
'use text format php_code',
];
$this->assertRole('migrate_test_role_2', $permissions, 4, $id_map);
// The only permission for this role is a filter format.
$permissions = ['use text format php_code'];
$this->assertRole('migrate_test_role_3_that_is_longer_than_thirty_two_characters', $permissions, 5, $id_map);
}
/**
* Tests user role migration.
*
* @param string[] $modules
* A list of modules to install.
* @param string[] $migrations
* A list of migrations to execute.
* @param string[][] $role_data
* An array of role data keyed by the destination role id. The role data
* contains the source role id, an array of valid permissions and an array
* of invalid permissions.
*
* @dataProvider providerTestUserRole()
*/
public function testUserRole() {
public function testUserRole(array $modules, array $migrations, array $role_data) {
if ($modules) {
// Install modules that have migrations that may provide permissions.
\Drupal::service('module_installer')->install($modules);
$this->installEntitySchema('block_content');
$this->installConfig(['block_content', 'comment']);
$this->migrateContentTypes();
}
$this->executeMigrations($migrations);
$id_map = $this->getMigration('d6_user_role')->getIdMap();
$this->assertRoles($id_map);
// Test there are no duplicated roles.
// After all the migrations are run, there are changes to the permissions.
$this->assertRoles($role_data, $id_map);
$roles = [
'anonymous1',
'authenticated1',
@ -134,6 +306,9 @@ class MigrateUserRoleTest extends MigrateDrupal6TestBase {
];
$this->assertEmpty(Role::loadMultiple($roles));
$this->assertMessages($role_data, $id_map);
$this->assertSame(4, $id_map->messageCount());
// Remove the map row for the migrate_test_role_1 role and rerun the
// migration. This will re-import the migrate_test_role_1 role migration
// again.
@ -157,11 +332,10 @@ class MigrateUserRoleTest extends MigrateDrupal6TestBase {
$this->executeMigration('d6_user_role');
// Test there are no duplicated roles.
$roles[] = 'migrate_test_role_41';
$this->assertEmpty(Role::loadMultiple($roles));
$this->assertNoDuplicateRoles();
// Test that the existing roles have not changed.
$this->assertRoles($id_map);
$this->assertRoles($role_data, $id_map);
// Test the migration of the new role, migrate_test_role_4.
$permissions = ['access content'];

View File

@ -2,7 +2,7 @@
namespace Drupal\Tests\user\Kernel\Migrate\d7;
use Drupal\Core\Database\Database;
use Drupal\migrate\Plugin\MigrationInterface;
use Drupal\Tests\migrate_drupal\Kernel\d7\MigrateDrupal7TestBase;
use Drupal\user\Entity\Role;
use Drupal\user\RoleInterface;
@ -29,36 +29,46 @@ class MigrateUserRoleTest extends MigrateDrupal7TestBase {
* The role ID.
* @param string $label
* The role's expected label.
* @param int $original_rid
* The original (integer) ID of the role, to check permissions.
* @param string[] $permissions
* The expected permissions.
*
* @internal
*/
protected function assertEntity(string $id, string $label, int $original_rid): void {
protected function assertEntity(string $id, string $label, array $permissions): void {
/** @var \Drupal\user\RoleInterface $entity */
$entity = Role::load($id);
$this->assertInstanceOf(RoleInterface::class, $entity);
$this->assertSame($label, $entity->label());
if (isset($original_rid)) {
$permissions = Database::getConnection('default', 'migrate')
->select('role_permission', 'rp')
->fields('rp', ['permission'])
->condition('rid', $original_rid)
->execute()
->fetchCol();
sort($permissions);
$this->assertSame($permissions, $entity->getPermissions());
}
$this->assertSame($permissions, $entity->getPermissions());
}
/**
* Tests user role migration.
*/
public function testUserRole() {
$this->assertEntity('anonymous', 'anonymous user', 1);
$this->assertEntity('authenticated', 'authenticated user', 2);
$this->assertEntity('administrator', 'administrator', 3);
$anonymous_permissions = ['access content'];
$this->assertEntity('anonymous', 'anonymous user', $anonymous_permissions);
$this->assertEntity('authenticated', 'authenticated user', $anonymous_permissions);
$admin_permissions = [
'access administration pages',
'access content',
'access site in maintenance mode',
'access site reports',
'access user profiles',
'administer menu',
'administer modules',
'administer permissions',
'administer site configuration',
'administer software updates',
'administer themes',
'administer users',
'cancel account',
'change own username',
'select account cancellation method',
'view the administration theme',
];
$this->assertEntity('administrator', 'administrator', $admin_permissions);
// Test there are no duplicated roles.
$roles = [
'anonymous1',
@ -97,12 +107,107 @@ class MigrateUserRoleTest extends MigrateDrupal7TestBase {
$this->assertEmpty(Role::loadMultiple($roles));
// Test that the existing roles have not changed.
$this->assertEntity('administrator', 'administrator', 3);
$this->assertEntity('anonymous', 'anonymous user', 1);
$this->assertEntity('authenticated', 'authenticated user', 2);
$this->assertEntity('administrator', 'administrator', $admin_permissions);
$this->assertEntity('anonymous', 'anonymous user', $anonymous_permissions);
$this->assertEntity('authenticated', 'authenticated user', $anonymous_permissions);
// Test the migration of the new role, test role.
$this->assertEntity('test_role', 'test role', 4);
$this->assertEntity('test_role', 'test role', $anonymous_permissions);
// Tests the migration log contains an error message.
// User role Authenticated.
$permissions[1] = [
'access comments',
'use text format filtered_html',
];
// User role test_role.
$permissions[2] = [
'access comments',
'post comments',
'skip comment approval',
'use text format custom_text_format',
'use text format filtered_html',
];
// User role administrator.
$permissions[3] = [
'access all views',
'access comments',
'access content overview',
'access contextual links',
'access dashboard',
'access news feeds',
'access overlay',
'access printer-friendly version',
'access site-wide contact form',
'access statistics',
'access toolbar',
'access user contact forms',
'add content to books',
'administer actions',
'administer blocks',
'administer book outlines',
'administer comments',
'administer contact forms',
'administer content types',
'administer filters',
'administer forums',
'administer image styles',
'administer languages',
'administer news feeds',
'administer nodes',
'administer search',
'administer shortcuts',
'administer statistics',
'administer taxonomy',
'administer unit tests',
'administer url aliases',
'administer views',
'block IP addresses',
'bypass node access',
'create article content',
'create new books',
'create page content',
'create url aliases',
'customize shortcut links',
'delete any article content',
'delete any page content',
'delete own article content',
'delete own page content',
'delete revisions',
'delete terms in 1',
'edit any article content',
'edit any page content',
'edit own article content',
'edit own comments',
'edit own page content',
'edit terms in 1',
'post comments',
'revert revisions',
'search content',
'skip comment approval',
'switch shortcut sets',
'translate admin strings',
'translate blocks',
'translate content',
'translate interface',
'translate user-defined strings',
'use PHP for settings',
'use advanced search',
'use text format custom_text_format',
'use text format filtered_html',
'use text format full_html',
'view own unpublished content',
'view post access counter',
'view revisions',
];
foreach ($id_map->getMessages() as $message) {
$expected_permissions = implode("', '", $permissions[$message->src_rid]);
$expected_message = "Permission(s) '" . $expected_permissions . "' not found.";
$this->assertSame($expected_message, $message->message);
$this->assertSame(MigrationInterface::MESSAGE_WARNING, (int) $message->level);
}
$this->assertSame(3, $id_map->messageCount());
}
}

View File

@ -5,6 +5,10 @@
* Post update functions for User module.
*/
use Drupal\Core\Config\Entity\ConfigEntityUpdater;
use Drupal\Core\StringTranslation\PluralTranslatableMarkup;
use Drupal\user\Entity\Role;
/**
* Implements hook_removed_post_updates().
*/
@ -14,3 +18,39 @@ function user_removed_post_updates() {
'user_post_update_update_roles' => '10.0.0',
];
}
/**
* Remove non-existent permissions created by migrations.
*/
function user_post_update_update_migrated_roles_followup(&$sandbox = NULL) {
$cleaned_roles = [];
$existing_permissions = array_keys(\Drupal::service('user.permissions')
->getPermissions());
\Drupal::classResolver(ConfigEntityUpdater::class)
->update($sandbox, 'user_role', function (Role $role) use ($existing_permissions, &$cleaned_roles) {
$removed_permissions = array_diff($role->getPermissions(), $existing_permissions);
if (!empty($removed_permissions)) {
$cleaned_roles[] = $role->label();
\Drupal::logger('update')->notice(
'The role %role has had the following non-existent permission(s) removed: %permissions.',
[
'%role' => $role->label(),
'%permissions' => implode(', ', $removed_permissions),
]
);
$permissions = array_intersect($role->getPermissions(), $existing_permissions);
$role->set('permissions', $permissions);
return TRUE;
}
});
if (!empty($cleaned_roles)) {
return new PluralTranslatableMarkup(
count($cleaned_roles),
'The role %role_list has had non-existent permissions removed. Check the logs for details.',
'The roles %role_list have had non-existent permissions removed. Check the logs for details.',
['%role_list' => implode(', ', $cleaned_roles)]
);
}
}