From 252eb1e40ad09d520968a9307ebb7fe6e2c60b71 Mon Sep 17 00:00:00 2001 From: Jared Galanis Date: Tue, 24 Feb 2026 13:48:15 -0500 Subject: [PATCH 1/2] fix: hide admin sidebar when user has no admin permissions (#4770) Admin sidebar menu providers (New, Edit, Import, Export, Access Control) had their top-level sections set to visible: true unconditionally. This caused the sidebar to render for all authenticated users, even those with no admin permissions, showing an empty sidebar with only the pin/unpin toggle. Fix by gating each provider's top section visibility on the same authorization checks used by their subsections, matching the pattern already used by NotificationsMenuProvider and CoarNotifyMenuProvider. --- .../providers/access-control.menu.spec.ts | 25 +++++++++++++++++++ .../menu/providers/access-control.menu.ts | 22 +++++++++------- .../shared/menu/providers/edit.menu.spec.ts | 24 ++++++++++++++++++ src/app/shared/menu/providers/edit.menu.ts | 13 ++++++---- .../shared/menu/providers/export.menu.spec.ts | 25 +++++++++++++++++++ src/app/shared/menu/providers/export.menu.ts | 9 +++---- .../shared/menu/providers/import.menu.spec.ts | 25 +++++++++++++++++++ src/app/shared/menu/providers/import.menu.ts | 9 +++---- .../shared/menu/providers/new.menu.spec.ts | 24 ++++++++++++++++++ src/app/shared/menu/providers/new.menu.ts | 15 +++++++---- .../menu/providers/registries.menu.spec.ts | 25 +++++++++++++++++++ .../shared/menu/providers/registries.menu.ts | 9 +++---- 12 files changed, 191 insertions(+), 34 deletions(-) diff --git a/src/app/shared/menu/providers/access-control.menu.spec.ts b/src/app/shared/menu/providers/access-control.menu.spec.ts index eb10ab1492c..2ea246504a6 100644 --- a/src/app/shared/menu/providers/access-control.menu.spec.ts +++ b/src/app/shared/menu/providers/access-control.menu.spec.ts @@ -94,4 +94,29 @@ describe('AccessControlMenuProvider', () => { done(); }); }); + + describe('when user has no permissions', () => { + let noPermsProvider: AccessControlMenuProvider; + let noPermsAuthStub = new AuthorizationDataServiceStub(); + + beforeEach(() => { + spyOn(noPermsAuthStub, 'isAuthorized').and.returnValue(of(false)); + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + AccessControlMenuProvider, + { provide: AuthorizationDataService, useValue: noPermsAuthStub }, + { provide: ScriptDataService, useClass: ScriptServiceStub }, + ], + }); + noPermsProvider = TestBed.inject(AccessControlMenuProvider); + }); + + it('getTopSection should return visible false', (done) => { + noPermsProvider.getTopSection().subscribe((section) => { + expect(section.visible).toBeFalse(); + done(); + }); + }); + }); }); diff --git a/src/app/shared/menu/providers/access-control.menu.ts b/src/app/shared/menu/providers/access-control.menu.ts index 5737cc7f6f1..8b685ca1b43 100644 --- a/src/app/shared/menu/providers/access-control.menu.ts +++ b/src/app/shared/menu/providers/access-control.menu.ts @@ -15,7 +15,6 @@ import { combineLatest as observableCombineLatest, map, Observable, - of, } from 'rxjs'; import { MenuItemType } from '../menu-item-type.model'; @@ -37,14 +36,19 @@ export class AccessControlMenuProvider extends AbstractExpandableMenuProvider { } public getTopSection(): Observable { - return of({ - model: { - type: MenuItemType.TEXT, - text: 'menu.section.access_control', - }, - icon: 'key', - visible: true, - }); + return observableCombineLatest([ + this.authorizationService.isAuthorized(FeatureID.AdministratorOf), + this.authorizationService.isAuthorized(FeatureID.CanManageGroups), + ]).pipe( + map(([isSiteAdmin, canManageGroups]) => ({ + model: { + type: MenuItemType.TEXT, + text: 'menu.section.access_control', + }, + icon: 'key', + visible: isSiteAdmin || canManageGroups, + })), + ); } public getSubSections(): Observable { diff --git a/src/app/shared/menu/providers/edit.menu.spec.ts b/src/app/shared/menu/providers/edit.menu.spec.ts index e626face06a..e32a68ed893 100644 --- a/src/app/shared/menu/providers/edit.menu.spec.ts +++ b/src/app/shared/menu/providers/edit.menu.spec.ts @@ -93,4 +93,28 @@ describe('EditMenuProvider', () => { done(); }); }); + + describe('when user has no permissions', () => { + let noPermsProvider: EditMenuProvider; + let noPermsAuthStub = new AuthorizationDataServiceStub(); + + beforeEach(() => { + spyOn(noPermsAuthStub, 'isAuthorized').and.returnValue(of(false)); + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + EditMenuProvider, + { provide: AuthorizationDataService, useValue: noPermsAuthStub }, + ], + }); + noPermsProvider = TestBed.inject(EditMenuProvider); + }); + + it('getTopSection should return visible false', (done) => { + noPermsProvider.getTopSection().subscribe((section) => { + expect(section.visible).toBeFalse(); + done(); + }); + }); + }); }); diff --git a/src/app/shared/menu/providers/edit.menu.ts b/src/app/shared/menu/providers/edit.menu.ts index e8777b59b44..8871a217f39 100644 --- a/src/app/shared/menu/providers/edit.menu.ts +++ b/src/app/shared/menu/providers/edit.menu.ts @@ -14,7 +14,6 @@ import { combineLatest, map, Observable, - of, } from 'rxjs'; import { ThemedEditCollectionSelectorComponent } from '../../dso-selector/modal-wrappers/edit-collection-selector/themed-edit-collection-selector.component'; @@ -37,16 +36,20 @@ export class EditMenuProvider extends AbstractExpandableMenuProvider { } public getTopSection(): Observable { - return of( - { + return combineLatest([ + this.authorizationService.isAuthorized(FeatureID.IsCollectionAdmin), + this.authorizationService.isAuthorized(FeatureID.IsCommunityAdmin), + this.authorizationService.isAuthorized(FeatureID.CanEditItem), + ]).pipe( + map(([isCollectionAdmin, isCommunityAdmin, canEditItem]) => ({ accessibilityHandle: 'edit', model: { type: MenuItemType.TEXT, text: 'menu.section.edit', }, icon: 'pencil', - visible: true, - }, + visible: isCollectionAdmin || isCommunityAdmin || canEditItem, + })), ); } diff --git a/src/app/shared/menu/providers/export.menu.spec.ts b/src/app/shared/menu/providers/export.menu.spec.ts index e1ef6edeab4..955509d85e2 100644 --- a/src/app/shared/menu/providers/export.menu.spec.ts +++ b/src/app/shared/menu/providers/export.menu.spec.ts @@ -82,4 +82,29 @@ describe('ExportMenuProvider', () => { done(); }); }); + + describe('when user has no permissions', () => { + let noPermsProvider: ExportMenuProvider; + let noPermsAuthStub = new AuthorizationDataServiceStub(); + + beforeEach(() => { + spyOn(noPermsAuthStub, 'isAuthorized').and.returnValue(of(false)); + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + ExportMenuProvider, + { provide: AuthorizationDataService, useValue: noPermsAuthStub }, + { provide: ScriptDataService, useClass: ScriptServiceStub }, + ], + }); + noPermsProvider = TestBed.inject(ExportMenuProvider); + }); + + it('getTopSection should return visible false', (done) => { + noPermsProvider.getTopSection().subscribe((section) => { + expect(section.visible).toBeFalse(); + done(); + }); + }); + }); }); diff --git a/src/app/shared/menu/providers/export.menu.ts b/src/app/shared/menu/providers/export.menu.ts index 296e547eb6f..3adad14848c 100644 --- a/src/app/shared/menu/providers/export.menu.ts +++ b/src/app/shared/menu/providers/export.menu.ts @@ -18,7 +18,6 @@ import { combineLatest as observableCombineLatest, map, Observable, - of, } from 'rxjs'; import { ExportBatchSelectorComponent } from '../../dso-selector/modal-wrappers/export-batch-selector/export-batch-selector.component'; @@ -41,16 +40,16 @@ export class ExportMenuProvider extends AbstractExpandableMenuProvider { } public getTopSection(): Observable { - return of( - { + return this.authorizationService.isAuthorized(FeatureID.AdministratorOf).pipe( + map((isSiteAdmin) => ({ accessibilityHandle: 'export', model: { type: MenuItemType.TEXT, text: 'menu.section.export', }, icon: 'file-export', - visible: true, - }, + visible: isSiteAdmin, + })), ); } diff --git a/src/app/shared/menu/providers/import.menu.spec.ts b/src/app/shared/menu/providers/import.menu.spec.ts index e13c669fbf0..301211a1dcc 100644 --- a/src/app/shared/menu/providers/import.menu.spec.ts +++ b/src/app/shared/menu/providers/import.menu.spec.ts @@ -81,4 +81,29 @@ describe('ImportMenuProvider', () => { done(); }); }); + + describe('when user has no permissions', () => { + let noPermsProvider: ImportMenuProvider; + let noPermsAuthStub = new AuthorizationDataServiceStub(); + + beforeEach(() => { + spyOn(noPermsAuthStub, 'isAuthorized').and.returnValue(of(false)); + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + ImportMenuProvider, + { provide: AuthorizationDataService, useValue: noPermsAuthStub }, + { provide: ScriptDataService, useClass: ScriptServiceStub }, + ], + }); + noPermsProvider = TestBed.inject(ImportMenuProvider); + }); + + it('getTopSection should return visible false', (done) => { + noPermsProvider.getTopSection().subscribe((section) => { + expect(section.visible).toBeFalse(); + done(); + }); + }); + }); }); diff --git a/src/app/shared/menu/providers/import.menu.ts b/src/app/shared/menu/providers/import.menu.ts index 76614aa6c88..8faacba28fc 100644 --- a/src/app/shared/menu/providers/import.menu.ts +++ b/src/app/shared/menu/providers/import.menu.ts @@ -18,7 +18,6 @@ import { combineLatest as observableCombineLatest, map, Observable, - of, } from 'rxjs'; import { MenuItemType } from '../menu-item-type.model'; @@ -39,15 +38,15 @@ export class ImportMenuProvider extends AbstractExpandableMenuProvider { } public getTopSection(): Observable { - return of( - { + return this.authorizationService.isAuthorized(FeatureID.AdministratorOf).pipe( + map((isSiteAdmin) => ({ model: { type: MenuItemType.TEXT, text: 'menu.section.import', }, icon: 'file-import', - visible: true, - }, + visible: isSiteAdmin, + })), ); } diff --git a/src/app/shared/menu/providers/new.menu.spec.ts b/src/app/shared/menu/providers/new.menu.spec.ts index be900952c5b..2de24e0fc70 100644 --- a/src/app/shared/menu/providers/new.menu.spec.ts +++ b/src/app/shared/menu/providers/new.menu.spec.ts @@ -110,4 +110,28 @@ describe('NewMenuProvider', () => { done(); }); }); + + describe('when user has no permissions', () => { + let noPermsProvider: NewMenuProvider; + let noPermsAuthStub = new AuthorizationDataServiceStub(); + + beforeEach(() => { + spyOn(noPermsAuthStub, 'isAuthorized').and.returnValue(of(false)); + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + NewMenuProvider, + { provide: AuthorizationDataService, useValue: noPermsAuthStub }, + ], + }); + noPermsProvider = TestBed.inject(NewMenuProvider); + }); + + it('getTopSection should return visible false', (done) => { + noPermsProvider.getTopSection().subscribe((section) => { + expect(section.visible).toBeFalse(); + done(); + }); + }); + }); }); diff --git a/src/app/shared/menu/providers/new.menu.ts b/src/app/shared/menu/providers/new.menu.ts index e329fa64b52..4dba1fbd968 100644 --- a/src/app/shared/menu/providers/new.menu.ts +++ b/src/app/shared/menu/providers/new.menu.ts @@ -14,7 +14,6 @@ import { combineLatest, map, Observable, - of, } from 'rxjs'; import { ThemedCreateCollectionParentSelectorComponent } from '../../dso-selector/modal-wrappers/create-collection-parent-selector/themed-create-collection-parent-selector.component'; @@ -39,16 +38,22 @@ export class NewMenuProvider extends AbstractExpandableMenuProvider { } public getTopSection(): Observable { - return of( - { + return combineLatest([ + this.authorizationService.isAuthorized(FeatureID.IsCollectionAdmin), + this.authorizationService.isAuthorized(FeatureID.IsCommunityAdmin), + this.authorizationService.isAuthorized(FeatureID.AdministratorOf), + this.authorizationService.isAuthorized(FeatureID.CanSubmit), + this.authorizationService.isAuthorized(FeatureID.CoarNotifyEnabled), + ]).pipe( + map(([isCollectionAdmin, isCommunityAdmin, isSiteAdmin, canSubmit, isCoarNotifyEnabled]) => ({ accessibilityHandle: 'new', model: { type: MenuItemType.TEXT, text: 'menu.section.new', } as TextMenuItemModel, icon: 'plus', - visible: true, - }, + visible: isCollectionAdmin || isCommunityAdmin || isSiteAdmin || canSubmit || isCoarNotifyEnabled, + })), ); } diff --git a/src/app/shared/menu/providers/registries.menu.spec.ts b/src/app/shared/menu/providers/registries.menu.spec.ts index 8ff4f7da95f..fb2323cb087 100644 --- a/src/app/shared/menu/providers/registries.menu.spec.ts +++ b/src/app/shared/menu/providers/registries.menu.spec.ts @@ -82,4 +82,29 @@ describe('RegistriesMenuProvider', () => { done(); }); }); + + describe('when user has no permissions', () => { + let noPermsProvider: RegistriesMenuProvider; + let noPermsAuthStub = new AuthorizationDataServiceStub(); + + beforeEach(() => { + spyOn(noPermsAuthStub, 'isAuthorized').and.returnValue(of(false)); + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [ + RegistriesMenuProvider, + { provide: AuthorizationDataService, useValue: noPermsAuthStub }, + { provide: ScriptDataService, useClass: ScriptServiceStub }, + ], + }); + noPermsProvider = TestBed.inject(RegistriesMenuProvider); + }); + + it('getTopSection should return visible false', (done) => { + noPermsProvider.getTopSection().subscribe((section) => { + expect(section.visible).toBeFalse(); + done(); + }); + }); + }); }); diff --git a/src/app/shared/menu/providers/registries.menu.ts b/src/app/shared/menu/providers/registries.menu.ts index 8f87283f654..8a8ccf908a7 100644 --- a/src/app/shared/menu/providers/registries.menu.ts +++ b/src/app/shared/menu/providers/registries.menu.ts @@ -15,7 +15,6 @@ import { combineLatest, map, Observable, - of, } from 'rxjs'; import { MenuItemType } from '../menu-item-type.model'; @@ -36,15 +35,15 @@ export class RegistriesMenuProvider extends AbstractExpandableMenuProvider { } public getTopSection(): Observable { - return of( - { + return this.authorizationService.isAuthorized(FeatureID.AdministratorOf).pipe( + map((isSiteAdmin) => ({ model: { type: MenuItemType.TEXT, text: 'menu.section.registries', }, icon: 'list', - visible: true, - }, + visible: isSiteAdmin, + })), ); } From d13360c64e98f2df690b17e54d723f50e5d1775b Mon Sep 17 00:00:00 2001 From: Jared Galanis Date: Tue, 17 Mar 2026 15:00:34 -0400 Subject: [PATCH 2/2] chore: retrigger CI