Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adds/enables msqrole module #721

Open
wants to merge 2 commits into
base: 3.x
Choose a base branch
from
Open

Conversation

markconroy
Copy link
Member

@markconroy markconroy commented May 27, 2024

Closes #674

This PR is to add and enable msqrole module only. It does not add a dedicated role for the purpose.

If we want a dedicated role, let's create a new (generic) role for these type things.

===
Thanks to Big Blue Door for sponsoring my time to work on this.

@ekes
Copy link
Member

ekes commented May 27, 2024

Saw the discussion about this. And I didn't get that this would need to be installed on prod. Maybe only on stage and dev sites?

@markconroy
Copy link
Member Author

@ekes I think this is probably worth chatting about at Merge Tuesday and/or Tech Drop-in.

@andybroomfield
Copy link
Contributor

Leaving for the disuccion in #674

@andybroomfield
Copy link
Contributor

Just updated to fix merge conflict pending discussion.
It would be good to get either MSQrole or masquerade installed.
From BHCC viewpoint we also install msqrole on prod to help diagnose issues with roles.
We have a mild preference for msrole as

  • It has security support
  • Lets us test role coverage without having to find existing user
  • Has an overlay to indicate using masquraded roles and easy to remove them (the masqureade block conflicts with Gin theme during tests).

Pros for masqureade

  • Lets you take over an actual user account, so if someone says they are having a problem, you view the site as if you are logged in as them.

@@ -80,9 +87,25 @@ function localgov_update_9502() {
}

/**
* Enable msqrole module.
* Update existing sites to use entity_usage for media.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm a bit confused as to what's happened here?

This is still the MR for enabling some masquerade system?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ekes I think it was just the merge confict as the Update existing sites to use entity_usage for media PR went in first and they both originally used the same update hook number 9503.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actual change (move this to update 9504:

localgov/localgov.install

Lines 89 to 112 in 33c491c

/**
* Update existing sites to use entity_usage for media.
*/
function localgov_update_9503() {
if (\Drupal::service('module_handler')->moduleExists('entity_usage')) {
$config_factory = \Drupal::configFactory();
$entity_usage_config = $config_factory->getEditable('entity_usage.settings');
$local_task_enabled_entity_types = $entity_usage_config->get('local_task_enabled_entity_types');
if (!in_array('media', $local_task_enabled_entity_types)) {
$local_task_enabled_entity_types[] = 'media';
$entity_usage_config->set('local_task_enabled_entity_types', $local_task_enabled_entity_types);
$entity_usage_config->save(TRUE);
}
}
}
/**
* Enable msqrole module.
*/
function localgov_update_9504() {
if (!\Drupal::service('module_handler')->moduleExists('msqrole')) {
\Drupal::service('module_installer')->install(['msqrole']);
}
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Install 'Masquerade role' and create a dedicated role for it
3 participants