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

SHS:5892: Site managers can perform actions on user accounts that don't make sense with SSO #1682

Merged
merged 6 commits into from
Dec 2, 2024

Conversation

cienvaras
Copy link
Collaborator

@cienvaras cienvaras commented Nov 7, 2024

READY FOR REVIEW

Summary

  • Remove "Cancel the selected user account(s)" and "Update URL Alias" bulk actions from the user list view
  • In the user edit form, hide the URL alias related fields for all users and the "Cancel User" button for non admin users

Need Review By (Date)

12/04

Urgency

medium

Steps to Test

  1. Log into the tugboat test site
  2. Visit the People view
  3. Select one or more users. Confirm one or more users and confirm that the following actions are not available anymore on the "Action" selector that appears at the bottom of the list:
    • Update URL alias
    • Cancel the selected user account(s)
  4. For any user, click on the "Edit" button in the "Operations" column at the right. Confirm that:
    • There are no fields related to the URL alias visible
    • The "Cancel account" button is present at the bottom of the form
  5. Masquerade as an user with the "Site Manager" role (Lindsey)
  6. Repeat steps 2 through 4. Everything should be the same as for the administrator user, except that the "Cancel account" button in the user edit form shouldn't be present.

PR Checklist


@cienvaras cienvaras self-assigned this Nov 7, 2024
@ahughes3 ahughes3 temporarily deployed to Tugboat November 7, 2024 20:02 Destroyed
@ahughes3 ahughes3 temporarily deployed to Tugboat November 7, 2024 20:07 Destroyed
@cienvaras
Copy link
Collaborator Author

@joegl Before passing this to @ahughes3 I wanted a quick check on our approach. The goal of the ticket is to remove some user actions that don't make sense in a site with SSO. We removed them from the bulk operations in the user list, but also want to remove them from the user edit form. The easiest way we found is to use a form alter and set the #access to FALSE, but I wanted to check if you see any potential issue or if you consider that a different approach is better. Thanks!

@joegl
Copy link
Contributor

joegl commented Nov 8, 2024

@cienvaras The approach seems fine but a little broad. I'd like to talk H&S about this one though, because there is at least one site where Drupal accounts are created without SSO and users need to be deleted regularly. I think adding a custom permission to control this would help, and then we could still allow Administrators (those with the Developer role) to delete users.

Base automatically changed from 11.5.1-release to develop November 13, 2024 15:35
@ahughes3 ahughes3 temporarily deployed to Tugboat November 21, 2024 00:42 Destroyed
@cienvaras cienvaras changed the base branch from develop to 11.6.1-release November 21, 2024 00:43
@cienvaras
Copy link
Collaborator Author

@joegl checked with @ahughes3 and confirmed the requirements:

Only the developer role needs the ability to delete users, and no role needs to have the ability to update the URL alias of a user.

Based on this, we don't need a new permission, I just updated the form alter to check for the user roles when hiding the "Cancel account" button.

@ahughes3 This one will be ready for you to test as soon as Tugboat is working again.

@cienvaras cienvaras marked this pull request as ready for review November 21, 2024 00:48
@ahughes3 ahughes3 temporarily deployed to Tugboat November 21, 2024 15:56 Destroyed
@joegl
Copy link
Contributor

joegl commented Nov 21, 2024

@ahughes3 When we talked about this on Tuesday we said it should only remove the bulk cancel action and not the cancel button on the user edit form. This PR currently removes both. I just want to confirm that's what you want.

@ahughes3
Copy link
Collaborator

@joegl In discussing with the web team, we have removed users manually on the Women's leadership site as a support request for the unit in the past and will continue that process. We want to restrict the ability to Cancel Accounts to just the Developer role as Site Managers do not need the ability to cancel any user accounts. In testing on Tugboat it looks like as a Developer I can still Cancel an Account from the User's edit page and as a Site Manager I no longer have that ability, which meets the requirements.

@cienvaras cienvaras requested a review from ahughes3 November 21, 2024 19:20
@cienvaras cienvaras assigned ahughes3 and unassigned cienvaras Nov 21, 2024
@joegl
Copy link
Contributor

joegl commented Nov 22, 2024

@joegl In discussing with the web team, we have removed users manually on the Women's leadership site as a support request for the unit in the past and will continue that process. We want to restrict the ability to Cancel Accounts to just the Developer role as Site Managers do not need the ability to cancel any user accounts. In testing on Tugboat it looks like as a Developer I can still Cancel an Account from the User's edit page and as a Site Manager I no longer have that ability, which meets the requirements.

@ahughes3 Sounds good, thanks 👍 I'm ready to merge this one once you're done reviewing it 🚀

Copy link
Collaborator

@ahughes3 ahughes3 left a comment

Choose a reason for hiding this comment

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

looks good!

@ahughes3 ahughes3 requested a review from pookmish November 26, 2024 19:38
@ahughes3 ahughes3 assigned pookmish and unassigned ahughes3 Nov 26, 2024
@ahughes3
Copy link
Collaborator

@pookmish this PR was approved by Joe and ready to merge but was waiting for my review approval - #1682 (comment)

Copy link
Member

@pookmish pookmish left a comment

Choose a reason for hiding this comment

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

Simple enough. An alternative would've been to create a custom permission, but that's probably unnecessary here.

@pookmish pookmish merged commit f320398 into 11.6.1-release Dec 2, 2024
17 checks passed
@pookmish pookmish deleted the shs-5892--update-user-actions branch December 2, 2024 23:53
@pookmish
Copy link
Member

pookmish commented Dec 2, 2024

Oh sorry @ahughes3 , i didn't notice your comment. Let me know if we need to revert or revise it.

@ahughes3
Copy link
Collaborator

ahughes3 commented Dec 3, 2024

@pookmish it was approved by me as well, we are good to go, thank you!

pookmish added a commit that referenced this pull request Jan 8, 2025
* 11.6.1

* Update lock file.

* SHS-5911: Add indicator of 3rd level shortcut menu (#1681)

* SHS-5911: Add indicator of 3rd level shortcut menu.

* fix(shs-5912): change layout of ckeditor styles dropdown list (#1688)

Co-authored-by: joegl <[email protected]>

* SHS:5892: Site managers can perform actions on user accounts that don't make sense with SSO (#1682)

* feat(shs-5892): remove actions from people view

* feat(shs-5892): remove elements from used edit form

* fix(shs-5892): fix issues in user edit form alter

* fix(shs-5892): remove delete actions only for non admin users

* Updated dependencies

* fix(shs-5911): use absolute path for image url to prevent error when drupal aggregates image (#1693)

* HSD8-1700: Increase Tugboat Max Allowed Packet size to 512MB. (#1695)

Co-authored-by: Seth Hendrick <[email protected]>

* SHS-5808: Uninstall Webform module (#1625)

* SHS-5905: Rework social media footer (#1662)

* feat(SHS-5905): Recreate social media block as a custom block plugin

* feat(SHS-5905): Add composer.lock

* feat(SHS-5905): Enable multivalue form element module

* feat(SHS-5905): Add update hook for permissions change

* fix(SHS-5905): Fix error for default config and add cache context for user

* fix(SHS-5905): Contextual menu still not rendering on Tugboat for non-admins

* fix(SHS-5905): Fix attached library for contextual menu

* fix(SHS-5909): Strict checking of the array seemed to fix the contextual links on local

* fix(SHS-5905): Remove strict checking

* fix(SHS-5905): Remove old permissions before adding social media block permissions

* fix(SHS-5905): Linting fixes

* chore(SHS-5905): WIP TEST ONLY - see if removing the code block in the profile works on tugboat

* fix(SHS-5905): Fix contextual menu for non-admins

* feat(SHS-5905): Remove permission update hook and add dependency on one in profile

* feat(shs-5906): icon logic, templates and styles for social media footer block (#1673)

* chore(SHS-5905): Temp remove code to test other contextual menus

* feat(SHS-5905): Add custom contextual link

* fix(SHS-5905): Get contextual links working (finally) for Social Media block

* fix(shs-5905): fixes in social media block

* fix(shs-5905): replace multivalue_form_element with custom version in social media block

* fix(shs-5905): remove multivalue_form_element module

* fix(shs-5905): update social media block links help text

* fix(shs-5905): social media footer block fixes

---------

Co-authored-by: Andrés Díaz Soto <[email protected]>

* Update hook to delete webform submissions

* SHS-5958: Bug: Inconsistent "Add Above" feature (#1692)

* fix(SHS-5958): Fix issue with paragraphs features to allow add buttons on non gin/claro themes

* feat(SHS-5958): Add config and update hook to allow add paragraphs in between

* refactor(SHS-5958): Change how we are updating the add in between to config

* test(SHS-5958): Update test to refelect new buttons

* test(SHS-5958): Update testPostCard test for new add in between buttons

* test(SHS-5958): Update other tests for new add in between buttons

* test(SHS-5958): Add a wait in test to make sure JS add in between buttons load

* test(SHS-5958): Fix photo album test - photo album can come before or after text component, making tests work sometimes but not all the time

* fix(shs-5958): fix video embed test

* fix(shs-5958): fix private page tests

* fix(shs-5958): fix video embed test

* fix(shs-5958): fix private page tests

* fix(shs-5958): fixes in tests

* fix(shs-5958): fix private page tests

* fix(shs-5958): fix private page tests

* chore(SHS-5958): Remove unnecessary layout builder config

* chore(SHS-5958): Revert hidden layout builder layout

* chore(SHS-5958): Revert composer.lock to 11.6.1 version

---------

Co-authored-by: Andrés Díaz Soto <[email protected]>

* Removed unintended underscore in update hook name

* fixup update function name

* HSD8-1707: Removed edit button from default people view to resolve button display to anonymous users. (#1701)

* SHS-5904: Add new "log out" block to existing sites (#1696)

* HSD8-1664: Updated PR template (#1685)

* Updated PR template.

* Updated sanitiztion line.

* dropped ready for review title. added review tasks heading.

---------

Co-authored-by: joegl <[email protected]>

* Revoke any webform permissions before uninstall module

* Removed assert_options from settings files

* HSD8-1702: delete mediterraneanstudies

---------

Co-authored-by: Github Actions <[email protected]>
Co-authored-by: joegl <[email protected]>
Co-authored-by: Mariana Núñez <[email protected]>
Co-authored-by: Andrés Díaz Soto <[email protected]>
Co-authored-by: Mike Decker <[email protected]>
Co-authored-by: Seth <[email protected]>
Co-authored-by: Seth Hendrick <[email protected]>
Co-authored-by: Marc Berger <[email protected]>
Co-authored-by: Joe Gilliland-Lloyd <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants