117 better control of user deletion in user administration page#140
Open
amadulhaxxani wants to merge 12 commits into
Open
117 better control of user deletion in user administration page#140amadulhaxxani wants to merge 12 commits into
amadulhaxxani wants to merge 12 commits into
Conversation
Prevent administrators from deleting their own account by disabling the delete button for the current authenticated user and showing a tooltip. Add a warningLabel input to the confirmation modal and render it when present. Implement logic in EPeopleRegistryComponent to determine the current user, compute a contextual delete warning (submitter, admin, or both) by querying workspace/workflow submissions, search, and group membership, and open the confirmation modal with that warning. Handle delete responses to show a friendly notification for backend 400 self-delete errors and use a generic failure notification otherwise. Update templates, component imports and helper methods (isCurrentUser, getDeleteWarningLabel, hasSubmittedItems, isAdministrator, isSelfDeletionError, showSelfDeleteNotification), and extend unit tests to cover disabled self-delete UI, composed warning labels, and notification behavior.
Disable direct self-deletion and surface contextual warnings when deleting an EPerson. Template: render the delete button as disabled for the currently authenticated user and show a tooltip explaining self-delete restrictions; otherwise show the normal delete button. Component: track the current authenticated user, compute a combined warning label based on whether the EPerson is an administrator and/or has submitted items (checks workspace, workflow and archived submissions), present that warning in the confirmation modal, and handle deletion results including a friendly notification for backend self-delete errors. Added helper methods (isCurrentUser, getDeleteWarningLabel, hasSubmittedItems, isAdministrator, isSelfDeletionError, showSelfDeleteNotification) and adjusted delete flow to update canDelete$ appropriately. Tests: updated and added specs and mocks to cover the disabled self-delete UI, combined warning label usage, and the friendly self-delete error notification; added required service mocks and test setup changes.
Add four i18n keys to en.json5 and cs.json5 for EPerson deletion flows: a forbidden self-delete message and warnings for deleting users who are submitters, administrators, or both. Provides localized English and Czech strings to surface these messages in the admin access-control UI.
There was a problem hiding this comment.
Pull request overview
Adds stronger frontend safeguards around EPerson deletion in the admin UI, so self-deletion is blocked earlier and delete confirmations can warn about high-impact accounts.
Changes:
- Added self-delete handling in the EPeople registry and EPerson form flows.
- Added contextual delete warnings for submitters and administrator accounts.
- Extended confirmation modal support and updated English/Czech translations and unit tests.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
src/assets/i18n/en.json5 |
Adds new English delete-warning and self-delete strings. |
src/assets/i18n/cs.json5 |
Adds Czech translations for the new delete-warning strings. |
src/app/shared/confirmation-modal/confirmation-modal.component.ts |
Adds optional warning-label input to the shared modal. |
src/app/shared/confirmation-modal/confirmation-modal.component.spec.ts |
Tests warning rendering in the confirmation modal. |
src/app/shared/confirmation-modal/confirmation-modal.component.html |
Renders the optional warning text inside the modal body. |
src/app/access-control/epeople-registry/eperson-form/eperson-form.component.ts |
Adds self-delete checks, warning lookup logic, and updated delete notifications in the form flow. |
src/app/access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts |
Adds form tests for warning labels, self-delete handling, and disabled delete button behavior. |
src/app/access-control/epeople-registry/eperson-form/eperson-form.component.html |
Updates the form delete button to show a disabled self-delete state with tooltip. |
src/app/access-control/epeople-registry/epeople-registry.component.ts |
Adds self-delete checks, warning lookup logic, and updated delete notifications in the registry flow. |
src/app/access-control/epeople-registry/epeople-registry.component.spec.ts |
Adds registry tests for warning composition, self-delete handling, and delete failure behavior. |
src/app/access-control/epeople-registry/epeople-registry.component.html |
Updates registry delete actions to show a disabled self-delete state with tooltip. |
Prevent delete actions from running before the current authenticated user id is available. Add a template condition to hide the delete button until currentAuthenticatedUserId is set, add an early-return guard in deleteEPerson when the id is missing, and add a unit test ensuring the modal is not opened and delete is not called before the id is resolved.
Prevent delete actions before the current authenticated user ID is available. Add *ngIf to the delete button in the template, add an early return guard in the component's delete flow when currentAuthenticatedUserId is not set, and include unit tests verifying the button is hidden and the delete/modal are not invoked until the ID is resolved.
Template: Wrap self-delete tooltip/button in a guard that checks epersonDto.ableToDelete and currentAuthenticatedUserId so delete UI is only rendered when deletion is allowed; simplify the enabledDeleteButton markup by removing a duplicated *ngIf on the inner button. Spec: remove unused DebugElement import, rename the test from 'should be disabled' to 'should be hidden', and update assertions to expect no delete buttons to be present (reflecting the new visibility behavior).
Add support for detecting Administrator group membership across paginated group lists and a unit test for it. - Introduce hasAdministratorGroupOnPage(groupsHref, currentPage) which fetches group pages (elementsPerPage=100) and recursively checks subsequent pages when the Administrator group is not found on the current page. - isAdministrator now delegates to hasAdministratorGroupOnPage starting at page 1. - Preserve error handling to return false on failures. - Add a unit test that simulates a two-page group response (administrator present on the second page) and verifies the component detects administrator membership and displays the correct warning label.
Refactor group membership check to traverse paginated group lists until an "Administrator" group is found or pages are exhausted. Introduces a recursive hasAdministratorGroupOnPage that requests pages, validates payload/pageInfo, checks for the admin group, and requests the next page when needed. Adds a unit test covering detection on later pages and a minor spec setup tweak (setting currentAuthenticatedUserId) to correctly exercise deletion behavior.
Corrects a spelling mistake in src/assets/i18n/cs.json5 for key admin.access-control.epeople.delete.warning.submitter: changed 'repositáři' to 'repozitáři' to improve Czech translation accuracy.
Add tabindex="-1" to the disabled delete button in the epeople registry template so it cannot receive keyboard focus. This ensures the parent span (which provides the tooltip) remains the sole focus target for the current-user case, improving accessibility and preventing duplicate focusable elements.
Add tabindex="-1" to the disabled delete button in eperson-form.component.html so the button cannot receive keyboard focus when showing the current user. The tooltip remains on the parent span, improving keyboard navigation and accessibility for the self-delete warning.
Remove an unintended "after" attribute from a <span> in eperson-form.component.html. The change cleans up the template by leaving only the intended *ngIf binding for conditional rendering of the delete/impersonation UI, preventing potential template parsing or linting issues.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem description
The EPeople administration UI did not adequately guard the delete flow for high-risk cases. This PR adds better handling for self-deletion and surfaces contextual warnings when deleting users with elevated impact.
Analysis
This PR:
Copilot review