diff --git a/src/app/+item-page/edit-item-page/edit-item-page.component.html b/src/app/+item-page/edit-item-page/edit-item-page.component.html index ca1c809cd9..6b6f74309c 100644 --- a/src/app/+item-page/edit-item-page/edit-item-page.component.html +++ b/src/app/+item-page/edit-item-page/edit-item-page.component.html @@ -5,11 +5,18 @@
diff --git a/src/app/+item-page/edit-item-page/edit-item-page.component.spec.ts b/src/app/+item-page/edit-item-page/edit-item-page.component.spec.ts new file mode 100644 index 0000000000..cff9104c01 --- /dev/null +++ b/src/app/+item-page/edit-item-page/edit-item-page.component.spec.ts @@ -0,0 +1,107 @@ +import { ComponentFixture, fakeAsync, TestBed, waitForAsync } from '@angular/core/testing'; +import { TranslateLoader, TranslateModule } from '@ngx-translate/core'; +import { TranslateLoaderMock } from '../../shared/mocks/translate-loader.mock'; +import { ChangeDetectionStrategy, NO_ERRORS_SCHEMA } from '@angular/core'; +import { ActivatedRoute, ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot, UrlTree } from '@angular/router'; +import { EditItemPageComponent } from './edit-item-page.component'; +import { Observable, of as observableOf } from 'rxjs'; +import { By } from '@angular/platform-browser'; +import { createSuccessfulRemoteDataObject } from '../../shared/remote-data.utils'; +import { Item } from '../../core/shared/item.model'; + +describe('ItemPageComponent', () => { + let comp: EditItemPageComponent; + let fixture: ComponentFixture; + + class AcceptAllGuard implements CanActivate { + canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable | Promise | boolean | UrlTree { + return observableOf(true); + } + } + + // tslint:disable-next-line:max-classes-per-file + class AcceptNoneGuard implements CanActivate { + canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable | Promise | boolean | UrlTree { + console.log('BLA'); + return observableOf(false); + } + } + + const accesiblePages = ['accessible']; + const inaccesiblePages = ['inaccessible', 'inaccessibleDoubleGuard']; + const mockRoute = { + snapshot: { + firstChild: { + routeConfig: { + path: accesiblePages[0] + } + }, + routerState: { + snapshot: undefined + } + }, + routeConfig: { + children: [ + { + path: accesiblePages[0], + canActivate: [AcceptAllGuard] + }, { + path: inaccesiblePages[0], + canActivate: [AcceptNoneGuard] + }, { + path: inaccesiblePages[1], + canActivate: [AcceptAllGuard, AcceptNoneGuard] + }, + ] + }, + data: observableOf({dso: createSuccessfulRemoteDataObject(new Item())}) + }; + + const mockRouter = { + routerState: { + snapshot: undefined + }, + events: observableOf(undefined) + }; + + beforeEach(waitForAsync(() => { + TestBed.configureTestingModule({ + imports: [TranslateModule.forRoot({ + loader: { + provide: TranslateLoader, + useClass: TranslateLoaderMock + } + })], + declarations: [EditItemPageComponent], + providers: [ + { provide: ActivatedRoute, useValue: mockRoute }, + { provide: Router, useValue: mockRouter }, + AcceptAllGuard, + AcceptNoneGuard, + ], + + schemas: [NO_ERRORS_SCHEMA] + }).overrideComponent(EditItemPageComponent, { + set: { changeDetection: ChangeDetectionStrategy.Default } + }).compileComponents(); + })); + + beforeEach(waitForAsync(() => { + fixture = TestBed.createComponent(EditItemPageComponent); + comp = fixture.componentInstance; + spyOn((comp as any).injector, 'get').and.callFake((a) => new a()); + fixture.detectChanges(); + })); + + describe('ngOnInit', () => { + it('should enable tabs that the user can activate', fakeAsync(() => { + const enabledItems = fixture.debugElement.queryAll(By.css('a.nav-link')); + expect(enabledItems.length).toBe(accesiblePages.length); + })); + + it('should disable tabs that the user can not activate', () => { + const disabledItems = fixture.debugElement.queryAll(By.css('button.nav-link.disabled')); + expect(disabledItems.length).toBe(inaccesiblePages.length); + }); + }); +}); diff --git a/src/app/+item-page/edit-item-page/edit-item-page.component.ts b/src/app/+item-page/edit-item-page/edit-item-page.component.ts index ec7cdb022d..5f1889a404 100644 --- a/src/app/+item-page/edit-item-page/edit-item-page.component.ts +++ b/src/app/+item-page/edit-item-page/edit-item-page.component.ts @@ -1,12 +1,13 @@ import { fadeIn, fadeInOut } from '../../shared/animations/fade'; -import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core'; -import { ActivatedRoute, Router } from '@angular/router'; +import { ChangeDetectionStrategy, Component, Injector, OnInit } from '@angular/core'; +import { ActivatedRoute, CanActivate, Route, Router } from '@angular/router'; import { RemoteData } from '../../core/data/remote-data'; import { Item } from '../../core/shared/item.model'; -import { Observable } from 'rxjs'; +import { combineLatest as observableCombineLatest, Observable, of as observableOf } from 'rxjs'; import { map } from 'rxjs/operators'; import { isNotEmpty } from '../../shared/empty.util'; import { getItemPageRoute } from '../item-page-routing-paths'; +import { GenericConstructor } from '../../core/shared/generic-constructor'; @Component({ selector: 'ds-edit-item-page', @@ -35,9 +36,9 @@ export class EditItemPageComponent implements OnInit { /** * All possible page outlet strings */ - pages: string[]; + pages: { page: string, enabled: Observable }[]; - constructor(private route: ActivatedRoute, private router: Router) { + constructor(private route: ActivatedRoute, private router: Router, private injector: Injector) { this.router.events.subscribe(() => { this.currentPage = this.route.snapshot.firstChild.routeConfig.path; }); @@ -45,8 +46,20 @@ export class EditItemPageComponent implements OnInit { ngOnInit(): void { this.pages = this.route.routeConfig.children - .map((child: any) => child.path) - .filter((path: string) => isNotEmpty(path)); // ignore reroutes + .filter((child: Route) => isNotEmpty(child.path)) + .map((child: Route) => { + let enabled = observableOf(true); + if (isNotEmpty(child.canActivate)) { + enabled = observableCombineLatest(child.canActivate.map((guardConstructor: GenericConstructor) => { + const guard: CanActivate = this.injector.get(guardConstructor); + return guard.canActivate(this.route.snapshot, this.router.routerState.snapshot); + }) + ).pipe( + map((canActivateOutcomes: any[]) => canActivateOutcomes.every((e) => e === true)) + ); + } + return { page: child.path, enabled: enabled }; + }); // ignore reroutes this.itemRD$ = this.route.data.pipe(map((data) => data.dso)); } diff --git a/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts b/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts index da10d33add..b7d650d8c3 100644 --- a/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts +++ b/src/app/+item-page/edit-item-page/edit-item-page.routing.module.ts @@ -22,15 +22,17 @@ import { ResourcePolicyEditComponent } from '../../shared/resource-policies/edit import { I18nBreadcrumbsService } from '../../core/breadcrumbs/i18n-breadcrumbs.service'; import { ITEM_EDIT_AUTHORIZATIONS_PATH, - ITEM_EDIT_MOVE_PATH, ITEM_EDIT_DELETE_PATH, - ITEM_EDIT_PUBLIC_PATH, + ITEM_EDIT_MOVE_PATH, ITEM_EDIT_PRIVATE_PATH, + ITEM_EDIT_PUBLIC_PATH, ITEM_EDIT_REINSTATE_PATH, ITEM_EDIT_WITHDRAW_PATH } from './edit-item-page.routing-paths'; import { ItemPageReinstateGuard } from './item-page-reinstate.guard'; import { ItemPageWithdrawGuard } from './item-page-withdraw.guard'; +import { ItemPageEditMetadataGuard } from '../item-page-edit-metadata.guard'; +import { ItemPageAdministratorGuard } from '../item-page-administrator.guard'; /** * Routing module that handles the routing for the Edit Item page administrator functionality @@ -57,22 +59,26 @@ import { ItemPageWithdrawGuard } from './item-page-withdraw.guard'; { path: 'status', component: ItemStatusComponent, - data: { title: 'item.edit.tabs.status.title', showBreadcrumbs: true } + data: { title: 'item.edit.tabs.status.title', showBreadcrumbs: true }, + canActivate: [ItemPageAdministratorGuard] }, { path: 'bitstreams', component: ItemBitstreamsComponent, - data: { title: 'item.edit.tabs.bitstreams.title', showBreadcrumbs: true } + data: { title: 'item.edit.tabs.bitstreams.title', showBreadcrumbs: true }, + canActivate: [ItemPageAdministratorGuard] }, { path: 'metadata', component: ItemMetadataComponent, - data: { title: 'item.edit.tabs.metadata.title', showBreadcrumbs: true } + data: { title: 'item.edit.tabs.metadata.title', showBreadcrumbs: true }, + canActivate: [ItemPageEditMetadataGuard] }, { path: 'relationships', component: ItemRelationshipsComponent, - data: { title: 'item.edit.tabs.relationships.title', showBreadcrumbs: true } + data: { title: 'item.edit.tabs.relationships.title', showBreadcrumbs: true }, + canActivate: [ItemPageEditMetadataGuard] }, /* TODO - uncomment & fix when view page exists { @@ -89,12 +95,14 @@ import { ItemPageWithdrawGuard } from './item-page-withdraw.guard'; { path: 'versionhistory', component: ItemVersionHistoryComponent, - data: { title: 'item.edit.tabs.versionhistory.title', showBreadcrumbs: true } + data: { title: 'item.edit.tabs.versionhistory.title', showBreadcrumbs: true }, + canActivate: [ItemPageAdministratorGuard] }, { path: 'mapper', component: ItemCollectionMapperComponent, - data: { title: 'item.edit.tabs.item-mapper.title', showBreadcrumbs: true } + data: { title: 'item.edit.tabs.item-mapper.title', showBreadcrumbs: true }, + canActivate: [ItemPageAdministratorGuard] } ] }, @@ -165,7 +173,9 @@ import { ItemPageWithdrawGuard } from './item-page-withdraw.guard'; ResourcePolicyResolver, ResourcePolicyTargetResolver, ItemPageReinstateGuard, - ItemPageWithdrawGuard + ItemPageWithdrawGuard, + ItemPageAdministratorGuard, + ItemPageEditMetadataGuard, ] }) export class EditItemPageRoutingModule { diff --git a/src/app/+item-page/item-page-edit-metadata.guard.ts b/src/app/+item-page/item-page-edit-metadata.guard.ts new file mode 100644 index 0000000000..a9b870b1cd --- /dev/null +++ b/src/app/+item-page/item-page-edit-metadata.guard.ts @@ -0,0 +1,31 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; +import { ItemPageResolver } from './item-page.resolver'; +import { Item } from '../core/shared/item.model'; +import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { Observable, of as observableOf } from 'rxjs'; +import { FeatureID } from '../core/data/feature-authorization/feature-id'; +import { AuthService } from '../core/auth/auth.service'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring edit metadata rights + */ +export class ItemPageEditMetadataGuard extends DsoPageFeatureGuard { + constructor(protected resolver: ItemPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); + } + + /** + * Check edit metadata authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.CanEditMetadata); + } +} diff --git a/src/app/+item-page/item-page-routing.module.ts b/src/app/+item-page/item-page-routing.module.ts index c9f2c402bb..f2d0a23935 100644 --- a/src/app/+item-page/item-page-routing.module.ts +++ b/src/app/+item-page/item-page-routing.module.ts @@ -37,7 +37,6 @@ import { ThemedFullItemPageComponent } from './full/themed-full-item-page.compon path: ITEM_EDIT_PATH, loadChildren: () => import('./edit-item-page/edit-item-page.module') .then((m) => m.EditItemPageModule), - canActivate: [ItemPageAdministratorGuard] }, { path: UPLOAD_BITSTREAM_PATH, @@ -67,7 +66,7 @@ import { ThemedFullItemPageComponent } from './full/themed-full-item-page.compon ItemBreadcrumbResolver, DSOBreadcrumbsService, LinkService, - ItemPageAdministratorGuard + ItemPageAdministratorGuard, ] }) diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts index 4041e588ed..f98e3f1837 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts @@ -32,6 +32,8 @@ describe('DsoPageAdministratorGuard', () => { let authService: AuthService; let resolver: Resolve>; let object: DSpaceObject; + let route; + let parentRoute; function init() { object = { @@ -50,6 +52,16 @@ describe('DsoPageAdministratorGuard', () => { authService = jasmine.createSpyObj('authService', { isAuthenticated: observableOf(true) }); + parentRoute = { + params: { + id: '3e1a5327-dabb-41ff-af93-e6cab9d032f0' + } + }; + route = { + params: { + }, + parent: parentRoute + }; guard = new DsoPageFeatureGuardImpl(resolver, authorizationService, router, authService, undefined); } @@ -59,10 +71,17 @@ describe('DsoPageAdministratorGuard', () => { describe('getObjectUrl', () => { it('should return the resolved object\'s selflink', (done) => { - guard.getObjectUrl(undefined, undefined).subscribe((selflink) => { + guard.getObjectUrl(route, undefined).subscribe((selflink) => { expect(selflink).toEqual(object.self); done(); }); }); }); + + describe('getRouteWithDSOId', () => { + it('should return the route that has the UUID of the DSO', () => { + const foundRoute = (guard as any).getRouteWithDSOId(route); + expect(foundRoute).toBe(parentRoute); + }); + }); }); diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts index c9ac7155d4..c50dd7f95d 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts @@ -7,6 +7,7 @@ import { map } from 'rxjs/operators'; import { DSpaceObject } from '../../../shared/dspace-object.model'; import { FeatureAuthorizationGuard } from './feature-authorization.guard'; import { AuthService } from '../../../auth/auth.service'; +import { hasNoValue, hasValue } from '../../../../shared/empty.util'; /** * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require rights for a specific feature @@ -24,9 +25,22 @@ export abstract class DsoPageFeatureGuard extends Featur * Check authorization rights for the object resolved using the provided resolver */ getObjectUrl(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return (this.resolver.resolve(route, state) as Observable>).pipe( + const routeWithObjectID = this.getRouteWithDSOId(route); + return (this.resolver.resolve(routeWithObjectID, state) as Observable>).pipe( getAllSucceededRemoteDataPayload(), map((dso) => dso.self) ); } + + /** + * Method to resolve resolve (parent) route that contains the UUID of the DSO + * @param route The current route + */ + protected getRouteWithDSOId(route: ActivatedRouteSnapshot): ActivatedRouteSnapshot { + let routeWithDSOId = route; + while (hasNoValue(routeWithDSOId.params.id) && hasValue(routeWithDSOId.parent)) { + routeWithDSOId = routeWithDSOId.parent; + } + return routeWithDSOId; + } } diff --git a/src/app/shared/dso-page/dso-page-edit-button/dso-page-edit-button.component.html b/src/app/shared/dso-page/dso-page-edit-button/dso-page-edit-button.component.html index 36661c895a..d845f852c8 100644 --- a/src/app/shared/dso-page/dso-page-edit-button/dso-page-edit-button.component.html +++ b/src/app/shared/dso-page/dso-page-edit-button/dso-page-edit-button.component.html @@ -1,5 +1,5 @@ diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index fa9e220f7c..84c0b27962 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1475,6 +1475,8 @@ "item.edit.breadcrumbs": "Edit Item", + "item.edit.tabs.disabled.tooltip": "You don't have permission to access this tab", + "item.edit.tabs.mapper.head": "Collection Mapper",