hazza/dspace-angular
1
0
Fork 1
mirror of https://github.com/DSpace/dspace-angular.git synced 2025-10-07 01:54:15 +00:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #1237 from atmire/w2p-bugfix-issue-1124-access-control-section-visible-to-non-admin-users

Browse Source 
Access Control - Groups only editable by Com/Col admin if group related to com/col & Only site admin access to access control epeople
This commit is contained in:
Tim Donohue
2021-06-24 13:47:20 -05:00
committed by GitHub
parent 1187edb486 511a2e18ab
commit 980d6a6154
11 changed files with 306 additions and 45 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
src/app/+admin/admin-sidebar/admin-sidebar.component.ts
View File

@@ -531,14 +531,17 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
* Create menu sections dependent on whether or not the current user can manage access control groups
*/
createAccessControlMenuSections() {
this.authorizationService.isAuthorized(FeatureID.CanManageGroups).subscribe((authorized) => {
observableCombineLatest(
this.authorizationService.isAuthorized(FeatureID.AdministratorOf),
this.authorizationService.isAuthorized(FeatureID.CanManageGroups)
).subscribe(([isSiteAdmin, canManageGroups]) => {
const menuList = [
/* Access Control */
{
id: 'access_control_people',
parentID: 'access_control',
active: false,
visible: authorized,
visible: isSiteAdmin,
model: {
type: MenuItemType.LINK,
text: 'menu.section.access_control_people',
@@ -549,7 +552,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
id: 'access_control_groups',
parentID: 'access_control',
active: false,
visible: authorized,
visible: canManageGroups,
model: {
type: MenuItemType.LINK,
text: 'menu.section.access_control_groups',
@@ -571,7 +574,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit {
{
id: 'access_control',
active: false,
visible: authorized,
visible: canManageGroups || isSiteAdmin,
model: {
type: MenuItemType.TEXT,
text: 'menu.section.access_control'

4
src/app/access-control/access-control-routing-paths.ts
View File

@@ -3,6 +3,10 @@ import { getAccessControlModuleRoute } from '../app-routing-paths';
export const GROUP_EDIT_PATH = 'groups';
export function getGroupsRoute() {
return new URLCombiner(getAccessControlModuleRoute(), GROUP_EDIT_PATH).toString();
}
export function getGroupEditRoute(id: string) {
return new URLCombiner(getAccessControlModuleRoute(), GROUP_EDIT_PATH, id).toString();
}

15
src/app/access-control/access-control-routing.module.ts
View File

@@ -5,6 +5,9 @@ import { GroupFormComponent } from './group-registry/group-form/group-form.compo
import { GroupsRegistryComponent } from './group-registry/groups-registry.component';
import { GROUP_EDIT_PATH } from './access-control-routing-paths';
import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver';
import { GroupPageGuard } from './group-registry/group-page.guard';
import { GroupAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/group-administrator.guard';
import { SiteAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/site-administrator.guard';
@NgModule({
imports: [
@@ -15,7 +18,8 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso
resolve: {
breadcrumb: I18nBreadcrumbResolver
},
data: { title: 'admin.access-control.epeople.title', breadcrumbKey: 'admin.access-control.epeople' }
data: { title: 'admin.access-control.epeople.title', breadcrumbKey: 'admin.access-control.epeople' },
canActivate: [SiteAdministratorGuard]
},
{
path: GROUP_EDIT_PATH,
@@ -23,7 +27,8 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso
resolve: {
breadcrumb: I18nBreadcrumbResolver
},
data: { title: 'admin.access-control.groups.title', breadcrumbKey: 'admin.access-control.groups' }
data: { title: 'admin.access-control.groups.title', breadcrumbKey: 'admin.access-control.groups' },
canActivate: [GroupAdministratorGuard]
},
{
path: `${GROUP_EDIT_PATH}/newGroup`,
@@ -31,7 +36,8 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso
resolve: {
breadcrumb: I18nBreadcrumbResolver
},
data: { title: 'admin.access-control.groups.title.addGroup', breadcrumbKey: 'admin.access-control.groups.addGroup' }
data: { title: 'admin.access-control.groups.title.addGroup', breadcrumbKey: 'admin.access-control.groups.addGroup' },
canActivate: [GroupAdministratorGuard]
},
{
path: `${GROUP_EDIT_PATH}/:groupId`,
@@ -39,7 +45,8 @@ import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.reso
resolve: {
breadcrumb: I18nBreadcrumbResolver
},
data: { title: 'admin.access-control.groups.title.singleGroup', breadcrumbKey: 'admin.access-control.groups.singleGroup' }
data: { title: 'admin.access-control.groups.title.singleGroup', breadcrumbKey: 'admin.access-control.groups.singleGroup' },
canActivate: [GroupPageGuard]
}
])
]

83
src/app/access-control/group-registry/group-page.guard.spec.ts Normal file
View File

@@ -0,0 +1,83 @@
import { GroupPageGuard } from './group-page.guard';
import { HALEndpointService } from '../../core/shared/hal-endpoint.service';
import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
import { ActivatedRouteSnapshot, Router } from '@angular/router';
import { of as observableOf } from 'rxjs';
import { AuthService } from '../../core/auth/auth.service';
import { FeatureID } from '../../core/data/feature-authorization/feature-id';
describe('GroupPageGuard', () => {
const groupsEndpointUrl = 'https://test.org/api/eperson/groups';
const groupUuid = '0d6f89df-f95a-4829-943c-f21f434fb892';
const groupEndpointUrl = `${groupsEndpointUrl}/${groupUuid}`;
const routeSnapshotWithGroupId = {
params: {
groupId: groupUuid,
}
} as unknown as ActivatedRouteSnapshot;
let guard: GroupPageGuard;
let halEndpointService: HALEndpointService;
let authorizationService: AuthorizationDataService;
let router: Router;
let authService: AuthService;
beforeEach(() => {
halEndpointService = jasmine.createSpyObj(['getEndpoint']);
(halEndpointService as any).getEndpoint.and.returnValue(observableOf(groupsEndpointUrl));
authorizationService = jasmine.createSpyObj(['isAuthorized']);
// NOTE: value is set in beforeEach
router = jasmine.createSpyObj(['parseUrl']);
(router as any).parseUrl.and.returnValue = {};
authService = jasmine.createSpyObj(['isAuthenticated']);
(authService as any).isAuthenticated.and.returnValue(observableOf(true));
guard = new GroupPageGuard(halEndpointService, authorizationService, router, authService);
});
it('should be created', () => {
expect(guard).toBeTruthy();
});
describe('canActivate', () => {
describe('when the current user can manage the group', () => {
beforeEach(() => {
(authorizationService as any).isAuthorized.and.returnValue(observableOf(true));
});
it('should return true', (done) => {
guard.canActivate(
routeSnapshotWithGroupId, { url: 'current-url'} as any
).subscribe((result) => {
expect(authorizationService.isAuthorized).toHaveBeenCalledWith(
FeatureID.CanManageGroup, groupEndpointUrl, undefined
);
expect(result).toBeTrue();
done();
});
});
});
describe('when the current user can not manage the group', () => {
beforeEach(() => {
(authorizationService as any).isAuthorized.and.returnValue(observableOf(false));
});
it('should not return true', (done) => {
guard.canActivate(
routeSnapshotWithGroupId, { url: 'current-url'} as any
).subscribe((result) => {
expect(authorizationService.isAuthorized).toHaveBeenCalledWith(
FeatureID.CanManageGroup, groupEndpointUrl, undefined
);
expect(result).not.toBeTrue();
done();
});
});
});
});
});

35
src/app/access-control/group-registry/group-page.guard.ts Normal file
View File

@@ -0,0 +1,35 @@
import { Injectable } from '@angular/core';
import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
import { Observable, of as observableOf } from 'rxjs';
import { FeatureID } from '../../core/data/feature-authorization/feature-id';
import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
import { AuthService } from '../../core/auth/auth.service';
import { SomeFeatureAuthorizationGuard } from '../../core/data/feature-authorization/feature-authorization-guard/some-feature-authorization.guard';
import { HALEndpointService } from '../../core/shared/hal-endpoint.service';
import { map } from 'rxjs/operators';
@Injectable({
providedIn: 'root'
})
export class GroupPageGuard extends SomeFeatureAuthorizationGuard {
protected groupsEndpoint = 'groups';
constructor(protected halEndpointService: HALEndpointService,
protected authorizationService: AuthorizationDataService,
protected router: Router,
protected authService: AuthService) {
super(authorizationService, router, authService);
}
getFeatureIDs(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<FeatureID[]> {
return observableOf([FeatureID.CanManageGroup]);
}
getObjectUrl(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<string> {
return this.halEndpointService.getEndpoint(this.groupsEndpoint).pipe(
map(groupsUrl => `${groupsUrl}/${route?.params?.groupId}`)
);
}
}

22
src/app/access-control/group-registry/groups-registry.component.html
View File

@@ -33,9 +33,9 @@
</div>
</form>
<ds-loading *ngIf="searching$ | async"></ds-loading>
<ds-loading *ngIf="loading$ | async"></ds-loading>
<ds-pagination
*ngIf="(pageInfoState$ | async)?.totalElements > 0 && !(searching$ | async)"
*ngIf="(pageInfoState$ | async)?.totalElements > 0 && !(loading$ | async)"
[paginationOptions]="config"
[pageInfoState]="pageInfoState$"
[collectionSize]="(pageInfoState$ | async)?.totalElements"
@@ -59,11 +59,23 @@
<td>{{groupDto.epersons?.totalElements + groupDto.subgroups?.totalElements}}</td>
<td>
<div class="btn-group edit-field">
<button [routerLink]="groupService.getGroupEditPageRouterLink(groupDto.group)"
class="btn btn-outline-primary btn-sm"
title="{{messagePrefix + 'table.edit.buttons.edit' | translate: {name: groupDto.group.name} }}">
<ng-container [ngSwitch]="groupDto.ableToEdit">
<button *ngSwitchCase="true"
[routerLink]="groupService.getGroupEditPageRouterLink(groupDto.group)"
class="btn btn-outline-primary btn-sm btn-edit"
title="{{messagePrefix + 'table.edit.buttons.edit' | translate: {name: groupDto.group.name} }}"
>
<i class="fas fa-edit fa-fw"></i>
</button>
<button *ngSwitchCase="false"
[disabled]="true"
class="btn btn-outline-primary btn-sm btn-edit"
placement="left"
[ngbTooltip]="'admin.access-control.epeople.table.edit.buttons.edit-disabled' | translate"
>
<i class="fas fa-edit fa-fw"></i>
</button>
</ng-container>
<button *ngIf="!groupDto.group?.permanent && groupDto.ableToDelete"
(click)="deleteGroup(groupDto)" class="btn btn-outline-danger btn-sm"
title="{{messagePrefix + 'table.edit.buttons.remove' | translate: {name: groupDto.group.name} }}">

101
src/app/access-control/group-registry/groups-registry.component.spec.ts
View File

@@ -30,6 +30,7 @@ import { routeServiceStub } from '../../shared/testing/route-service.stub';
import { RouterMock } from '../../shared/mocks/router.mock';
import { PaginationService } from '../../core/pagination/pagination.service';
import { PaginationServiceStub } from '../../shared/testing/pagination-service.stub';
import { FeatureID } from '../../core/data/feature-authorization/feature-id';
describe('GroupRegistryComponent', () => {
let component: GroupsRegistryComponent;
@@ -43,6 +44,26 @@ describe('GroupRegistryComponent', () => {
let mockEPeople;
let paginationService;
/**
* Set authorizationService.isAuthorized to return the following values.
* @param isAdmin whether or not the current user is an admin.
* @param canManageGroup whether or not the current user can manage all groups.
*/
const setIsAuthorized = (isAdmin: boolean, canManageGroup: boolean) => {
(authorizationService as any).isAuthorized.and.callFake((featureId?: FeatureID) => {
switch (featureId) {
case FeatureID.AdministratorOf:
return observableOf(isAdmin);
case FeatureID.CanManageGroup:
return observableOf(canManageGroup);
case FeatureID.CanDelete:
return observableOf(true);
default:
throw new Error(`setIsAuthorized: this fake implementation does not support ${featureId}.`);
}
});
};
beforeEach(waitForAsync(() => {
mockGroups = [GroupMock, GroupMock2];
mockEPeople = [EPersonMock, EPersonMock2];
@@ -131,9 +152,8 @@ describe('GroupRegistryComponent', () => {
return createSuccessfulRemoteDataObject$(undefined);
}
};
authorizationService = jasmine.createSpyObj('authorizationService', {
isAuthorized: observableOf(true)
});
authorizationService = jasmine.createSpyObj('authorizationService', ['isAuthorized']);
setIsAuthorized(true, true);
paginationService = new PaginationServiceStub();
TestBed.configureTestingModule({
imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule,
@@ -180,6 +200,81 @@ describe('GroupRegistryComponent', () => {
});
});
describe('edit buttons', () => {
describe('when the user is a general admin', () => {
beforeEach(fakeAsync(() => {
// NOTE: setting canManageGroup to false should not matter, since isAdmin takes priority
setIsAuthorized(true, false);
// force rerender after setup changes
component.search({ query: '' });
tick();
fixture.detectChanges();
}));
it('should be active', () => {
const editButtonsFound = fixture.debugElement.queryAll(By.css('#groups tr td:nth-child(4) button.btn-edit'));
expect(editButtonsFound.length).toEqual(2);
editButtonsFound.forEach((editButtonFound) => {
expect(editButtonFound.nativeElement.disabled).toBeFalse();
});
});
it('should not check the canManageGroup permissions', () => {
expect(authorizationService.isAuthorized).not.toHaveBeenCalledWith(
FeatureID.CanManageGroup, mockGroups[0].self
);
expect(authorizationService.isAuthorized).not.toHaveBeenCalledWith(
FeatureID.CanManageGroup, mockGroups[0].self, undefined // treated differently
);
expect(authorizationService.isAuthorized).not.toHaveBeenCalledWith(
FeatureID.CanManageGroup, mockGroups[1].self
);
expect(authorizationService.isAuthorized).not.toHaveBeenCalledWith(
FeatureID.CanManageGroup, mockGroups[1].self, undefined // treated differently
);
});
});
describe('when the user can edit the groups', () => {
beforeEach(fakeAsync(() => {
setIsAuthorized(false, true);
// force rerender after setup changes
component.search({ query: '' });
tick();
fixture.detectChanges();
}));
it('should be active', () => {
const editButtonsFound = fixture.debugElement.queryAll(By.css('#groups tr td:nth-child(4) button.btn-edit'));
expect(editButtonsFound.length).toEqual(2);
editButtonsFound.forEach((editButtonFound) => {
expect(editButtonFound.nativeElement.disabled).toBeFalse();
});
});
});
describe('when the user can not edit the groups', () => {
beforeEach(fakeAsync(() => {
setIsAuthorized(false, false);
// force rerender after setup changes
component.search({ query: '' });
tick();
fixture.detectChanges();
}));
it('should not be active', () => {
const editButtonsFound = fixture.debugElement.queryAll(By.css('#groups tr td:nth-child(4) button.btn-edit'));
expect(editButtonsFound.length).toEqual(2);
editButtonsFound.forEach((editButtonFound) => {
expect(editButtonFound.nativeElement.disabled).toBeTrue();
});
});
});
});
describe('search', () => {
describe('when searching with query', () => {
let groupIdsFound;

32
src/app/access-control/group-registry/groups-registry.component.ts
View File

@@ -9,7 +9,7 @@ import {
of as observableOf,
Subscription
} from 'rxjs';
import { catchError, map, switchMap, take } from 'rxjs/operators';
import { catchError, map, switchMap, take, tap } from 'rxjs/operators';
import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service';
import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service';
import { FeatureID } from '../../core/data/feature-authorization/feature-id';
@@ -75,7 +75,7 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy {
/**
* A boolean representing if a search is pending
*/
searching$: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false);
loading$: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false);
// Current search in groups registry
currentSearchQuery: string;
@@ -118,12 +118,12 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy {
* @param data Contains query param
*/
search(data: any) {
this.searching$.next(true);
if (hasValue(this.searchSub)) {
this.searchSub.unsubscribe();
this.subs = this.subs.filter((sub: Subscription) => sub !== this.searchSub);
}
this.searchSub = this.paginationService.getCurrentPagination(this.config.id, this.config).pipe(
tap(() => this.loading$.next(true)),
switchMap((paginationOptions) => {
const query: string = data.query;
if (query != null && this.currentSearchQuery !== query) {
@@ -141,18 +141,22 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy {
if (groups.page.length === 0) {
return observableOf(buildPaginatedList(groups.pageInfo, []));
}
return this.authorizationService.isAuthorized(FeatureID.AdministratorOf).pipe(
switchMap((isSiteAdmin: boolean) => {
return observableCombineLatest(groups.page.map((group: Group) => {
if (!this.deletedGroupsIds.includes(group.id)) {
if (hasValue(group) && !this.deletedGroupsIds.includes(group.id)) {
return observableCombineLatest([
this.authorizationService.isAuthorized(FeatureID.CanDelete, hasValue(group) ? group.self : undefined),
this.authorizationService.isAuthorized(FeatureID.CanDelete, group.self),
this.canManageGroup$(isSiteAdmin, group),
this.hasLinkedDSO(group),
this.getSubgroups(group),
this.getMembers(group)
]).pipe(
map(([isAuthorized, hasLinkedDSO, subgroups, members]:
[boolean, boolean, RemoteData<PaginatedList<Group>>, RemoteData<PaginatedList<EPerson>>]) => {
map(([canDelete, canManageGroup, hasLinkedDSO, subgroups, members]:
[boolean, boolean, boolean, RemoteData<PaginatedList<Group>>, RemoteData<PaginatedList<EPerson>>]) => {
const groupDtoModel: GroupDtoModel = new GroupDtoModel();
groupDtoModel.ableToDelete = isAuthorized && !hasLinkedDSO;
groupDtoModel.ableToDelete = canDelete && !hasLinkedDSO;
groupDtoModel.ableToEdit = canManageGroup;
groupDtoModel.group = group;
groupDtoModel.subgroups = subgroups.payload;
groupDtoModel.epersons = members.payload;
@@ -165,15 +169,25 @@ export class GroupsRegistryComponent implements OnInit, OnDestroy {
return buildPaginatedList(groups.pageInfo, dtos);
}));
})
);
})
).subscribe((value: PaginatedList<GroupDtoModel>) => {
this.groupsDto$.next(value);
this.pageInfoState$.next(value.pageInfo);
this.searching$.next(false);
this.loading$.next(false);
});
this.subs.push(this.searchSub);
}
canManageGroup$(isSiteAdmin: boolean, group: Group): Observable<boolean> {
if (isSiteAdmin) {
return observableOf(true);
} else {
return this.authorizationService.isAuthorized(FeatureID.CanManageGroup, group.self);
}
}
/**
* Delete Group
*/

1
src/app/core/data/feature-authorization/feature-id.ts
View File

@@ -10,6 +10,7 @@ export enum FeatureID {
ReinstateItem = 'reinstateItem',
EPersonRegistration = 'epersonRegistration',
CanManageGroups = 'canManageGroups',
CanManageGroup = 'canManageGroup',
IsCollectionAdmin = 'isCollectionAdmin',
IsCommunityAdmin = 'isCommunityAdmin',
CanDownload = 'canDownload',

5
src/app/core/eperson/models/group-dto.model.ts
View File

@@ -17,6 +17,11 @@ export class GroupDtoModel {
*/
public ableToDelete: boolean;
/**
* Whether or not the current user is able to edit the linked group
*/
public ableToEdit: boolean;
/**
* List of subgroups of this group
*/

2
src/assets/i18n/en.json5
View File

@@ -238,6 +238,8 @@
"admin.access-control.epeople.table.edit.buttons.edit": "Edit \"{{name}}\"",
"admin.access-control.epeople.table.edit.buttons.edit-disabled": "You are not authorized to edit this group",
"admin.access-control.epeople.table.edit.buttons.remove": "Delete \"{{name}}\"",
"admin.access-control.epeople.no-items": "No EPeople to show.",