Skip to content

Commit 73ba21c

Browse files
KasinhouMatus Kasak
Kasinhou
and
Matus Kasak
authored
JCU/Prevent an admin from deleting their own account (#1447)
* Prevent an admin from deleting their own account Backport of the UFAL self-delete guard from dtq-dev (dspace-angular #1335, #1357, #1373) to this customer branch. The EPeople registry and the EPerson form now hide/disable the delete action for the currently authenticated user (with an explanatory tooltip), show a contextual warning in the confirmation modal when the target is a submitter and/or an administrator, and surface a friendly notification when the backend rejects a self-delete. Shared logic lives in the new EPersonDeleteGuardService so both call sites stay in sync. Refs dataquest-dev/dspace-customers#855 * Use the failure i18n key when a delete fails The error branch of the EPeople registry delete handler was reusing 'notification.deleted.success', so any non-self-delete failure showed a red toast reading "Successfully deleted EPerson" with an empty name. Switch to 'notification.deleted.failure' and pass restResponse, which the failure string interpolates as {{restResponse.errorMessage}}. This matches what dtq-dev already does (07957d8) — the hunk was missed when the change was translated to the 9.x component. Raised by Copilot review on PR #1447. Refs dataquest-dev/dspace-customers#855 * Reword the Czech self-delete / delete-warning messages Rewrites the four Czech strings for the self-delete guard so they read naturally rather than as literal translations, keeping the repository's established Czech terminology (uživatel / správce / záznamy / smazat) and active phrasing ("Jeho smazáním odeberete…" instead of the nominal "Smazání tohoto uživatele odebere…"). Wording is identical across all customer branches. Raised in review on PR #1447. Refs dataquest-dev/dspace-customers#855 --------- Co-authored-by: Matus Kasak <matus.kasak@dataquest.sk>
1 parent 382a721 commit 73ba21c

13 files changed

Lines changed: 551 additions & 47 deletions

‎src/app/access-control/epeople-registry/epeople-registry.component.html‎

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,12 +74,24 @@ <h2 id="search" class="border-bottom pb-2">
7474
title="{{labelPrefix + 'table.edit.buttons.edit' | translate: { name: dsoNameService.getName(epersonDto.eperson) } }}">
7575
<i class="fas fa-edit fa-fw"></i>
7676
</button>
77-
@if (epersonDto.ableToDelete) {
78-
<button (click)="deleteEPerson(epersonDto.eperson)"
79-
class="delete-button btn btn-outline-danger btn-sm access-control-deleteEPersonButton"
80-
title="{{labelPrefix + 'table.edit.buttons.remove' | translate: { name: dsoNameService.getName(epersonDto.eperson) } }}">
81-
<i class="fas fa-trash-alt fa-fw"></i>
82-
</button>
77+
@if (epersonDto.ableToDelete && currentAuthenticatedUserId) {
78+
@if (isCurrentUser(epersonDto.eperson)) {
79+
<span tabindex="0" [ngbTooltip]="selfDeleteWarningLabel | translate" container="body">
80+
<button [dsBtnDisabled]="true"
81+
tabindex="-1"
82+
[attr.aria-label]="selfDeleteWarningLabel | translate"
83+
class="delete-button btn btn-outline-danger btn-sm access-control-deleteEPersonButton"
84+
type="button">
85+
<i class="fas fa-trash-alt fa-fw"></i>
86+
</button>
87+
</span>
88+
} @else {
89+
<button (click)="deleteEPerson(epersonDto.eperson)"
90+
class="delete-button btn btn-outline-danger btn-sm access-control-deleteEPersonButton"
91+
title="{{labelPrefix + 'table.edit.buttons.remove' | translate: { name: dsoNameService.getName(epersonDto.eperson) } }}">
92+
<i class="fas fa-trash-alt fa-fw"></i>
93+
</button>
94+
}
8395
}
8496
</div>
8597
</td>

‎src/app/access-control/epeople-registry/epeople-registry.component.spec.ts‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
of,
3131
} from 'rxjs';
3232

33+
import { AuthService } from '../../core/auth/auth.service';
3334
import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
3435
import { FindListOptions } from '../../core/data/find-list-options.model';
3536
import {
@@ -57,6 +58,7 @@ import {
5758
import { NotificationsServiceStub } from '../../shared/testing/notifications-service.stub';
5859
import { PaginationServiceStub } from '../../shared/testing/pagination-service.stub';
5960
import { EPeopleRegistryComponent } from './epeople-registry.component';
61+
import { EPersonDeleteGuardService } from './eperson-delete-guard.service';
6062
import { EPersonFormComponent } from './eperson-form/eperson-form.component';
6163

6264
describe('EPeopleRegistryComponent', () => {
@@ -67,6 +69,8 @@ describe('EPeopleRegistryComponent', () => {
6769
let mockEPeople: EPerson[];
6870
let ePersonDataServiceStub: any;
6971
let authorizationService: AuthorizationDataService;
72+
let authService: jasmine.SpyObj<AuthService>;
73+
let deleteGuard: jasmine.SpyObj<EPersonDeleteGuardService>;
7074
let modalService: NgbModal;
7175
let paginationService: PaginationServiceStub;
7276

@@ -149,6 +153,12 @@ describe('EPeopleRegistryComponent', () => {
149153
});
150154
builderService = getMockFormBuilderService();
151155

156+
authService = jasmine.createSpyObj('authService', ['getAuthenticatedUserFromStore']);
157+
authService.getAuthenticatedUserFromStore.and.returnValue(of(Object.assign(new EPerson(), { id: 'different-user-id' })));
158+
deleteGuard = jasmine.createSpyObj('deleteGuard', ['isCurrentUser', 'getDeleteWarningLabel', 'isSelfDeletionError', 'showSelfDeleteNotification']);
159+
deleteGuard.isCurrentUser.and.callFake((ePerson: EPerson, currentId: string) => !!ePerson?.id && ePerson.id === currentId);
160+
deleteGuard.getDeleteWarningLabel.and.returnValue(of(undefined));
161+
deleteGuard.isSelfDeletionError.and.returnValue(false);
152162
paginationService = new PaginationServiceStub();
153163
TestBed.configureTestingModule({
154164
imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule, RouterTestingModule.withRoutes([]),
@@ -157,6 +167,8 @@ describe('EPeopleRegistryComponent', () => {
157167
{ provide: EPersonDataService, useValue: ePersonDataServiceStub },
158168
{ provide: NotificationsService, useValue: new NotificationsServiceStub() },
159169
{ provide: AuthorizationDataService, useValue: authorizationService },
170+
{ provide: AuthService, useValue: authService },
171+
{ provide: EPersonDeleteGuardService, useValue: deleteGuard },
160172
{ provide: FormBuilderService, useValue: builderService },
161173
{ provide: Router, useValue: new RouterMock() },
162174
{ provide: RequestService, useValue: jasmine.createSpyObj('requestService', ['setStaleByHrefSubstring']) },
@@ -257,6 +269,25 @@ describe('EPeopleRegistryComponent', () => {
257269
});
258270
});
259271
});
272+
273+
describe('when the ePerson is the currently authenticated user', () => {
274+
beforeEach(() => {
275+
component.currentAuthenticatedUserId = EPersonMock.id;
276+
fixture.detectChanges();
277+
});
278+
279+
it('renders the delete button for that row as disabled', () => {
280+
const deleteButtons = fixture.debugElement.queryAll(By.css('.access-control-deleteEPersonButton'));
281+
const disabled = deleteButtons.filter((button) => button.nativeElement.getAttribute('aria-disabled') === 'true');
282+
expect(disabled.length).toBe(1);
283+
});
284+
285+
it('notifies instead of opening the confirmation modal', () => {
286+
component.deleteEPerson(EPersonMock);
287+
expect(deleteGuard.showSelfDeleteNotification).toHaveBeenCalled();
288+
expect(modalService.open).not.toHaveBeenCalled();
289+
});
290+
});
260291
});
261292

262293

‎src/app/access-control/epeople-registry/epeople-registry.component.ts‎

Lines changed: 55 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ import {
1515
Router,
1616
RouterModule,
1717
} from '@angular/router';
18-
import { NgbModal } from '@ng-bootstrap/ng-bootstrap';
18+
import {
19+
NgbModal,
20+
NgbTooltipModule,
21+
} from '@ng-bootstrap/ng-bootstrap';
1922
import {
2023
TranslateModule,
2124
TranslateService,
@@ -32,6 +35,7 @@ import {
3235
take,
3336
} from 'rxjs/operators';
3437

38+
import { AuthService } from '../../core/auth/auth.service';
3539
import { DSONameService } from '../../core/breadcrumbs/dso-name.service';
3640
import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
3741
import { FeatureID } from '../../core/data/feature-authorization/feature-id';
@@ -51,6 +55,7 @@ import {
5155
getFirstCompletedRemoteData,
5256
} from '../../core/shared/operators';
5357
import { PageInfo } from '../../core/shared/page-info.model';
58+
import { BtnDisabledDirective } from '../../shared/btn-disabled.directive';
5459
import { ConfirmationModalComponent } from '../../shared/confirmation-modal/confirmation-modal.component';
5560
import { hasValue } from '../../shared/empty.util';
5661
import { ThemedLoadingComponent } from '../../shared/loading/themed-loading.component';
@@ -61,12 +66,18 @@ import {
6166
getEPersonEditRoute,
6267
getEPersonsRoute,
6368
} from '../access-control-routing-paths';
69+
import {
70+
EPersonDeleteGuardService,
71+
SELF_DELETE_WARNING_LABEL,
72+
} from './eperson-delete-guard.service';
6473

6574
@Component({
6675
selector: 'ds-epeople-registry',
6776
templateUrl: './epeople-registry.component.html',
6877
imports: [
6978
AsyncPipe,
79+
BtnDisabledDirective,
80+
NgbTooltipModule,
7081
NgClass,
7182
PaginationComponent,
7283
ReactiveFormsModule,
@@ -82,6 +93,9 @@ import {
8293
export class EPeopleRegistryComponent implements OnInit, OnDestroy {
8394

8495
labelPrefix = 'admin.access-control.epeople.';
96+
selfDeleteWarningLabel = SELF_DELETE_WARNING_LABEL;
97+
98+
currentAuthenticatedUserId: string;
8599

86100
/**
87101
* A list of all the current EPeople within the repository or the result of the search
@@ -135,6 +149,8 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy {
135149
private translateService: TranslateService,
136150
private notificationsService: NotificationsService,
137151
private authorizationService: AuthorizationDataService,
152+
private authService: AuthService,
153+
private deleteGuard: EPersonDeleteGuardService,
138154
private formBuilder: UntypedFormBuilder,
139155
private router: Router,
140156
private modalService: NgbModal,
@@ -161,6 +177,9 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy {
161177
this.searching$.next(true);
162178
this.search({ scope: this.currentSearchScope, query: this.currentSearchQuery });
163179
this.activeEPerson$ = this.epersonService.getActiveEPerson();
180+
this.subs.push(this.authService.getAuthenticatedUserFromStore().subscribe((currentUser: EPerson) => {
181+
this.currentAuthenticatedUserId = currentUser?.id;
182+
}));
164183
this.subs.push(this.ePeople$.pipe(
165184
switchMap((epeople: PaginatedList<EPerson>) => {
166185
if (epeople.pageInfo.totalElements > 0) {
@@ -233,30 +252,52 @@ export class EPeopleRegistryComponent implements OnInit, OnDestroy {
233252
*/
234253
deleteEPerson(ePerson: EPerson) {
235254
if (hasValue(ePerson.id)) {
236-
const modalRef = this.modalService.open(ConfirmationModalComponent);
237-
modalRef.componentInstance.name = this.dsoNameService.getName(ePerson);
238-
modalRef.componentInstance.headerLabel = 'confirmation-modal.delete-eperson.header';
239-
modalRef.componentInstance.infoLabel = 'confirmation-modal.delete-eperson.info';
240-
modalRef.componentInstance.cancelLabel = 'confirmation-modal.delete-eperson.cancel';
241-
modalRef.componentInstance.confirmLabel = 'confirmation-modal.delete-eperson.confirm';
242-
modalRef.componentInstance.brandColor = 'danger';
243-
modalRef.componentInstance.confirmIcon = 'fas fa-trash';
244-
modalRef.componentInstance.response.pipe(take(1)).subscribe((confirm: boolean) => {
245-
if (confirm) {
246-
if (hasValue(ePerson.id)) {
255+
if (!hasValue(this.currentAuthenticatedUserId)) {
256+
return;
257+
}
258+
259+
if (this.isCurrentUser(ePerson)) {
260+
this.deleteGuard.showSelfDeleteNotification();
261+
return;
262+
}
263+
264+
this.deleteGuard.getDeleteWarningLabel(ePerson).pipe(take(1)).subscribe((warningLabel: string | undefined) => {
265+
const modalRef = this.modalService.open(ConfirmationModalComponent);
266+
modalRef.componentInstance.name = this.dsoNameService.getName(ePerson);
267+
modalRef.componentInstance.headerLabel = 'confirmation-modal.delete-eperson.header';
268+
modalRef.componentInstance.infoLabel = 'confirmation-modal.delete-eperson.info';
269+
modalRef.componentInstance.warningLabel = warningLabel;
270+
modalRef.componentInstance.cancelLabel = 'confirmation-modal.delete-eperson.cancel';
271+
modalRef.componentInstance.confirmLabel = 'confirmation-modal.delete-eperson.confirm';
272+
modalRef.componentInstance.brandColor = 'danger';
273+
modalRef.componentInstance.confirmIcon = 'fas fa-trash';
274+
modalRef.componentInstance.response.pipe(take(1)).subscribe((confirm: boolean) => {
275+
if (confirm) {
247276
this.epersonService.deleteEPerson(ePerson).pipe(getFirstCompletedRemoteData()).subscribe((restResponse: RemoteData<NoContent>) => {
248277
if (restResponse.hasSucceeded) {
249278
this.notificationsService.success(this.translateService.get(this.labelPrefix + 'notification.deleted.success', { name: this.dsoNameService.getName(ePerson) }));
279+
} else if (this.isCurrentUser(ePerson) || this.deleteGuard.isSelfDeletionError(restResponse)) {
280+
this.deleteGuard.showSelfDeleteNotification();
250281
} else {
251-
this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.deleted.success', { id: ePerson.id, statusCode: restResponse.statusCode, errorMessage: restResponse.errorMessage }));
282+
this.notificationsService.error(this.translateService.get(this.labelPrefix + 'notification.deleted.failure', {
283+
name: this.dsoNameService.getName(ePerson),
284+
id: ePerson.id,
285+
statusCode: restResponse.statusCode,
286+
errorMessage: restResponse.errorMessage,
287+
restResponse,
288+
}));
252289
}
253290
});
254291
}
255-
}
292+
});
256293
});
257294
}
258295
}
259296

297+
isCurrentUser(ePerson: EPerson): boolean {
298+
return this.deleteGuard.isCurrentUser(ePerson, this.currentAuthenticatedUserId);
299+
}
300+
260301
/**
261302
* Unsub all subscriptions
262303
*/

0 commit comments

Comments
 (0)