From 73370fa00dcee4dd124edc6863fb036b4b931e16 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 9 Jul 2020 17:56:35 +0200 Subject: [PATCH 1/9] 71764: DsoPageAdministratorGuard --- .../collection-page-administrator.guard.ts | 20 +++++++ .../collection-page-routing.module.ts | 6 +- .../community-page-administrator.guard.ts | 20 +++++++ .../community-page-routing.module.ts | 6 +- .../item-page-administrator.guard.ts | 20 +++++++ .../+item-page/item-page-routing.module.ts | 6 +- .../dso-page-administrator.guard.spec.ts | 56 +++++++++++++++++++ .../dso-page-administrator.guard.ts | 39 +++++++++++++ .../feature-authorization.guard.spec.ts | 15 ++--- .../feature-authorization.guard.ts | 19 ++++--- .../site-administrator.guard.ts | 8 ++- 11 files changed, 192 insertions(+), 23 deletions(-) create mode 100644 src/app/+collection-page/collection-page-administrator.guard.ts create mode 100644 src/app/+community-page/community-page-administrator.guard.ts create mode 100644 src/app/+item-page/item-page-administrator.guard.ts create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.spec.ts create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.ts diff --git a/src/app/+collection-page/collection-page-administrator.guard.ts b/src/app/+collection-page/collection-page-administrator.guard.ts new file mode 100644 index 0000000000..dcb9f545a9 --- /dev/null +++ b/src/app/+collection-page/collection-page-administrator.guard.ts @@ -0,0 +1,20 @@ +import { Injectable } from '@angular/core'; +import { Router } from '@angular/router'; +import { DsoPageAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard'; +import { Collection } from '../core/shared/collection.model'; +import { CollectionPageResolver } from './collection-page.resolver'; +import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Collection} pages requiring administrator rights + */ +export class CollectionPageAdministratorGuard extends DsoPageAdministratorGuard { + constructor(protected resolver: CollectionPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router) { + super(resolver, authorizationService, router); + } +} diff --git a/src/app/+collection-page/collection-page-routing.module.ts b/src/app/+collection-page/collection-page-routing.module.ts index 1ebd9b3630..ebe086375f 100644 --- a/src/app/+collection-page/collection-page-routing.module.ts +++ b/src/app/+collection-page/collection-page-routing.module.ts @@ -16,6 +16,7 @@ import { CollectionBreadcrumbResolver } from '../core/breadcrumbs/collection-bre import { DSOBreadcrumbsService } from '../core/breadcrumbs/dso-breadcrumbs.service'; import { LinkService } from '../core/cache/builders/link.service'; import { I18nBreadcrumbResolver } from '../core/breadcrumbs/i18n-breadcrumb.resolver'; +import { CollectionPageAdministratorGuard } from './collection-page-administrator.guard'; export const COLLECTION_PARENT_PARAMETER = 'parent'; @@ -54,7 +55,7 @@ const ITEMTEMPLATE_PATH = 'itemtemplate'; { path: COLLECTION_EDIT_PATH, loadChildren: './edit-collection-page/edit-collection-page.module#EditCollectionPageModule', - canActivate: [AuthenticatedGuard] + canActivate: [CollectionPageAdministratorGuard] }, { path: 'delete', @@ -93,7 +94,8 @@ const ITEMTEMPLATE_PATH = 'itemtemplate'; CollectionBreadcrumbResolver, DSOBreadcrumbsService, LinkService, - CreateCollectionPageGuard + CreateCollectionPageGuard, + CollectionPageAdministratorGuard ] }) export class CollectionPageRoutingModule { diff --git a/src/app/+community-page/community-page-administrator.guard.ts b/src/app/+community-page/community-page-administrator.guard.ts new file mode 100644 index 0000000000..886a449951 --- /dev/null +++ b/src/app/+community-page/community-page-administrator.guard.ts @@ -0,0 +1,20 @@ +import { Injectable } from '@angular/core'; +import { Router } from '@angular/router'; +import { DsoPageAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard'; +import { Community } from '../core/shared/community.model'; +import { CommunityPageResolver } from './community-page.resolver'; +import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Community} pages requiring administrator rights + */ +export class CommunityPageAdministratorGuard extends DsoPageAdministratorGuard { + constructor(protected resolver: CommunityPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router) { + super(resolver, authorizationService, router); + } +} diff --git a/src/app/+community-page/community-page-routing.module.ts b/src/app/+community-page/community-page-routing.module.ts index 9922bc2c01..384574d9be 100644 --- a/src/app/+community-page/community-page-routing.module.ts +++ b/src/app/+community-page/community-page-routing.module.ts @@ -12,6 +12,7 @@ import { getCommunityModulePath } from '../app-routing.module'; import { CommunityBreadcrumbResolver } from '../core/breadcrumbs/community-breadcrumb.resolver'; import { DSOBreadcrumbsService } from '../core/breadcrumbs/dso-breadcrumbs.service'; import { LinkService } from '../core/cache/builders/link.service'; +import { CommunityPageAdministratorGuard } from './community-page-administrator.guard'; export const COMMUNITY_PARENT_PARAMETER = 'parent'; @@ -49,7 +50,7 @@ const COMMUNITY_EDIT_PATH = 'edit'; { path: COMMUNITY_EDIT_PATH, loadChildren: './edit-community-page/edit-community-page.module#EditCommunityPageModule', - canActivate: [AuthenticatedGuard] + canActivate: [CommunityPageAdministratorGuard] }, { path: 'delete', @@ -71,7 +72,8 @@ const COMMUNITY_EDIT_PATH = 'edit'; CommunityBreadcrumbResolver, DSOBreadcrumbsService, LinkService, - CreateCommunityPageGuard + CreateCommunityPageGuard, + CommunityPageAdministratorGuard ] }) export class CommunityPageRoutingModule { diff --git a/src/app/+item-page/item-page-administrator.guard.ts b/src/app/+item-page/item-page-administrator.guard.ts new file mode 100644 index 0000000000..507f8fcdbe --- /dev/null +++ b/src/app/+item-page/item-page-administrator.guard.ts @@ -0,0 +1,20 @@ +import { Injectable } from '@angular/core'; +import { Router } from '@angular/router'; +import { DsoPageAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard'; +import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; +import { ItemPageResolver } from './item-page.resolver'; +import { Item } from '../core/shared/item.model'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring administrator rights + */ +export class ItemPageAdministratorGuard extends DsoPageAdministratorGuard { + constructor(protected resolver: ItemPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router) { + super(resolver, authorizationService, router); + } +} diff --git a/src/app/+item-page/item-page-routing.module.ts b/src/app/+item-page/item-page-routing.module.ts index 52faf96236..fc5cfa3522 100644 --- a/src/app/+item-page/item-page-routing.module.ts +++ b/src/app/+item-page/item-page-routing.module.ts @@ -11,6 +11,7 @@ import { ItemBreadcrumbResolver } from '../core/breadcrumbs/item-breadcrumb.reso import { DSOBreadcrumbsService } from '../core/breadcrumbs/dso-breadcrumbs.service'; import { LinkService } from '../core/cache/builders/link.service'; import { UploadBitstreamComponent } from './bitstreams/upload/upload-bitstream.component'; +import { ItemPageAdministratorGuard } from './item-page-administrator.guard'; export function getItemPageRoute(itemId: string) { return new URLCombiner(getItemModulePath(), itemId).toString(); @@ -46,7 +47,7 @@ const UPLOAD_BITSTREAM_PATH = 'bitstreams/new'; { path: ITEM_EDIT_PATH, loadChildren: './edit-item-page/edit-item-page.module#EditItemPageModule', - canActivate: [AuthenticatedGuard] + canActivate: [ItemPageAdministratorGuard] }, { path: UPLOAD_BITSTREAM_PATH, @@ -61,7 +62,8 @@ const UPLOAD_BITSTREAM_PATH = 'bitstreams/new'; ItemPageResolver, ItemBreadcrumbResolver, DSOBreadcrumbsService, - LinkService + LinkService, + ItemPageAdministratorGuard ] }) diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.spec.ts new file mode 100644 index 0000000000..7fef2e5d4c --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.spec.ts @@ -0,0 +1,56 @@ +import { DsoPageAdministratorGuard } from './dso-page-administrator.guard'; +import { AuthorizationDataService } from '../authorization-data.service'; +import { Resolve, Router } from '@angular/router'; +import { RemoteData } from '../../remote-data'; +import { of as observableOf } from 'rxjs'; +import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; +import { DSpaceObject } from '../../../shared/dspace-object.model'; + +/** + * Test implementation of abstract class DsoPageAdministratorGuard + */ +class DsoPageAdministratorGuardImpl extends DsoPageAdministratorGuard { + constructor(protected resolver: Resolve>, + protected authorizationService: AuthorizationDataService, + protected router: Router) { + super(resolver, authorizationService, router); + } +} + +describe('DsoPageAdministratorGuard', () => { + let guard: DsoPageAdministratorGuard; + let authorizationService: AuthorizationDataService; + let router: Router; + let resolver: Resolve>; + let object: DSpaceObject; + + function init() { + object = { + self: 'test-selflink' + } as DSpaceObject; + + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthorized: observableOf(true) + }); + router = jasmine.createSpyObj('router', { + parseUrl: {} + }); + resolver = jasmine.createSpyObj('resolver', { + resolve: createSuccessfulRemoteDataObject$(object) + }); + guard = new DsoPageAdministratorGuardImpl(resolver, authorizationService, router); + } + + beforeEach(() => { + init(); + }); + + describe('getObjectUrl', () => { + it('should return the resolved object\'s selflink', (done) => { + guard.getObjectUrl(undefined, undefined).subscribe((selflink) => { + expect(selflink).toEqual(object.self); + done(); + }); + }); + }); +}); diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.ts new file mode 100644 index 0000000000..868510eb4e --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.ts @@ -0,0 +1,39 @@ +import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { AuthorizationDataService } from '../authorization-data.service'; +import { ActivatedRouteSnapshot, Resolve, Router, RouterStateSnapshot } from '@angular/router'; +import { DSpaceObject } from '../../../shared/dspace-object.model'; +import { FeatureID } from '../feature-id'; +import { of as observableOf } from 'rxjs'; +import { Observable } from 'rxjs/internal/Observable'; +import { RemoteData } from '../../remote-data'; +import { getAllSucceededRemoteDataPayload } from '../../../shared/operators'; +import { map } from 'rxjs/operators'; + +/** + * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require administrator rights + * This guard utilizes a resolver to retrieve the relevant object to check authorizations for + */ +export abstract class DsoPageAdministratorGuard extends FeatureAuthorizationGuard { + constructor(protected resolver: Resolve>, + protected authorizationService: AuthorizationDataService, + protected router: Router) { + super(authorizationService, router); + } + + /** + * Check administrator authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.AdministratorOf); + } + + /** + * 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( + getAllSucceededRemoteDataPayload(), + map((dso) => dso.self) + ); + } +} diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts index bfd161bad2..829a246dcc 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts @@ -2,7 +2,8 @@ import { FeatureAuthorizationGuard } from './feature-authorization.guard'; import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; import { of as observableOf } from 'rxjs'; -import { Router } from '@angular/router'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { Observable } from 'rxjs/internal/Observable'; /** * Test implementation of abstract class FeatureAuthorizationGuard @@ -17,16 +18,16 @@ class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard { super(authorizationService, router); } - getFeatureID(): FeatureID { - return this.featureId; + getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(this.featureId); } - getObjectUrl(): string { - return this.objectUrl; + getObjectUrl(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(this.objectUrl); } - getEPersonUuid(): string { - return this.ePersonUuid; + getEPersonUuid(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(this.ePersonUuid); } } diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts index 7806d87b0c..d53e71e289 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts @@ -9,6 +9,8 @@ import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; import { Observable } from 'rxjs/internal/Observable'; import { returnUnauthorizedUrlTreeOnFalse } from '../../../shared/operators'; +import { combineLatest as observableCombineLatest, of as observableOf } from 'rxjs'; +import { switchMap } from 'rxjs/operators'; /** * Abstract Guard for preventing unauthorized activating and loading of routes when a user @@ -24,29 +26,32 @@ export abstract class FeatureAuthorizationGuard implements CanActivate { * True when user has authorization rights for the feature and object provided * Redirect the user to the unauthorized page when he/she's not authorized for the given feature */ - canActivate(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return this.authorizationService.isAuthorized(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()).pipe(returnUnauthorizedUrlTreeOnFalse(this.router)); + canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableCombineLatest(this.getFeatureID(route, state), this.getObjectUrl(route, state), this.getEPersonUuid(route, state)).pipe( + switchMap(([featureID, objectUrl, ePersonUuid]) => this.authorizationService.isAuthorized(featureID, objectUrl, ePersonUuid)), + returnUnauthorizedUrlTreeOnFalse(this.router) + ); } /** * The type of feature to check authorization for * Override this method to define a feature */ - abstract getFeatureID(): FeatureID; + abstract getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable; /** * The URL of the object to check if the user has authorized rights for * Override this method to define an object URL. If not provided, the {@link Site}'s URL will be used */ - getObjectUrl(): string { - return undefined; + getObjectUrl(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(undefined); } /** * The UUID of the user to check authorization rights for * Override this method to define an {@link EPerson} UUID. If not provided, the authenticated user's UUID will be used. */ - getEPersonUuid(): string { - return undefined; + getEPersonUuid(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(undefined); } } diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts index a64e40468d..a45049645a 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts @@ -2,7 +2,9 @@ import { Injectable } from '@angular/core'; import { FeatureAuthorizationGuard } from './feature-authorization.guard'; import { FeatureID } from '../feature-id'; import { AuthorizationDataService } from '../authorization-data.service'; -import { Router } from '@angular/router'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { of as observableOf } from 'rxjs'; +import { Observable } from 'rxjs/internal/Observable'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have administrator @@ -19,7 +21,7 @@ export class SiteAdministratorGuard extends FeatureAuthorizationGuard { /** * Check administrator authorization rights */ - getFeatureID(): FeatureID { - return FeatureID.AdministratorOf; + getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.AdministratorOf); } } From 9a666731e67ce460a2af9c997b870a3bbde4d50e Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 10 Jul 2020 11:47:29 +0200 Subject: [PATCH 2/9] 71764: Hide withdraw and reinstate button depending on authorization --- .../item-status/item-status.component.html | 4 +- .../item-status/item-status.component.ts | 40 ++++++++++++++++--- .../data/feature-authorization/feature-id.ts | 4 +- 3 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-status/item-status.component.html b/src/app/+item-page/edit-item-page/item-status/item-status.component.html index 83662c9d7c..a619d4a576 100644 --- a/src/app/+item-page/edit-item-page/item-status/item-status.component.html +++ b/src/app/+item-page/edit-item-page/item-status/item-status.component.html @@ -15,7 +15,7 @@ {{getItemPage((itemRD$ | async)?.payload)}} -
- +
+
diff --git a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts index 1be13e3a7a..04e0e28645 100644 --- a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts +++ b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts @@ -1,17 +1,20 @@ -import { ChangeDetectionStrategy, Component, OnInit } from '@angular/core'; +import { ChangeDetectionStrategy, ChangeDetectorRef, Component, OnInit } from '@angular/core'; import { fadeIn, fadeInOut } from '../../../shared/animations/fade'; import { Item } from '../../../core/shared/item.model'; import { ActivatedRoute } from '@angular/router'; import { ItemOperation } from '../item-operation/itemOperation.model'; -import { first, map } from 'rxjs/operators'; +import { distinctUntilChanged, first, map } from 'rxjs/operators'; import { Observable } from 'rxjs'; import { RemoteData } from '../../../core/data/remote-data'; import { getItemEditPath, getItemPageRoute } from '../../item-page-routing.module'; +import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; +import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; +import { hasValue } from '../../../shared/empty.util'; @Component({ selector: 'ds-item-status', templateUrl: './item-status.component.html', - changeDetection: ChangeDetectionStrategy.OnPush, + changeDetection: ChangeDetectionStrategy.Default, animations: [ fadeIn, fadeInOut @@ -47,7 +50,9 @@ export class ItemStatusComponent implements OnInit { */ actionsKeys; - constructor(private route: ActivatedRoute) { + constructor(private route: ActivatedRoute, + private authorizationService: AuthorizationDataService, + private changeDetection: ChangeDetectorRef) { } ngOnInit(): void { @@ -70,10 +75,29 @@ export class ItemStatusComponent implements OnInit { this.operations = []; this.operations.push(new ItemOperation('authorizations', this.getCurrentUrl(item) + '/authorizations')); this.operations.push(new ItemOperation('mappedCollections', this.getCurrentUrl(item) + '/mapper')); + this.operations.push(undefined); + // Store the index of the "withdraw" or "reinstate" operation, because it's added asynchronously + const index = this.operations.length - 1; if (item.isWithdrawn) { - this.operations.push(new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate')); + this.authorizationService.isAuthorized(FeatureID.ReinstateItem, item.self).pipe(distinctUntilChanged()).subscribe((authorized) => { + if (authorized) { + console.log('added reinstate'); + this.operations[index] = new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate'); + } else { + this.operations[index] = undefined; + } + this.changeDetection.detectChanges(); + }); } else { - this.operations.push(new ItemOperation('withdraw', this.getCurrentUrl(item) + '/withdraw')); + this.authorizationService.isAuthorized(FeatureID.WithdrawItem, item.self).pipe(distinctUntilChanged()).subscribe((authorized) => { + if (authorized) { + console.log('added withdraw'); + this.operations[index] = new ItemOperation('withdraw', this.getCurrentUrl(item) + '/withdraw'); + } else { + this.operations[index] = undefined; + } + this.changeDetection.detectChanges(); + }); } if (item.isDiscoverable) { this.operations.push(new ItemOperation('private', this.getCurrentUrl(item) + '/private')); @@ -102,4 +126,8 @@ export class ItemStatusComponent implements OnInit { return getItemEditPath(item.id); } + trackOperation(index: number, operation: ItemOperation) { + return hasValue(operation) ? operation.operationKey : undefined; + } + } diff --git a/src/app/core/data/feature-authorization/feature-id.ts b/src/app/core/data/feature-authorization/feature-id.ts index 4731e92d6c..e3eb9cd668 100644 --- a/src/app/core/data/feature-authorization/feature-id.ts +++ b/src/app/core/data/feature-authorization/feature-id.ts @@ -3,5 +3,7 @@ */ export enum FeatureID { LoginOnBehalfOf = 'loginOnBehalfOf', - AdministratorOf = 'administratorOf' + AdministratorOf = 'administratorOf', + WithdrawItem = 'withdrawItem', + ReinstateItem = 'reinstateItem', } From 02fb4a4e4e2c645c6c8e442a3ed69cd7c09512b3 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 10 Jul 2020 12:08:48 +0200 Subject: [PATCH 3/9] 71764: Refactor DsoPageAdministratorGuard to more abstract DsoPageFeatureGuard and add implementations for WithdrawItem / ReinstateItem guards --- .../collection-page-administrator.guard.ts | 16 ++++++++-- .../community-page-administrator.guard.ts | 16 ++++++++-- .../edit-item-page.routing.module.ts | 8 ++++- .../item-page-reinstate.guard.ts | 30 +++++++++++++++++++ .../item-page-withdraw.guard.ts | 30 +++++++++++++++++++ .../item-status/item-status.component.ts | 2 -- .../item-page-administrator.guard.ts | 16 ++++++++-- ...spec.ts => dso-page-feature.guard.spec.ts} | 19 ++++++++---- ...tor.guard.ts => dso-page-feature.guard.ts} | 21 ++++--------- 9 files changed, 125 insertions(+), 33 deletions(-) create mode 100644 src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts create mode 100644 src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts rename src/app/core/data/feature-authorization/feature-authorization-guard/{dso-page-administrator.guard.spec.ts => dso-page-feature.guard.spec.ts} (68%) rename src/app/core/data/feature-authorization/feature-authorization-guard/{dso-page-administrator.guard.ts => dso-page-feature.guard.ts} (73%) diff --git a/src/app/+collection-page/collection-page-administrator.guard.ts b/src/app/+collection-page/collection-page-administrator.guard.ts index dcb9f545a9..4d2f246689 100644 --- a/src/app/+collection-page/collection-page-administrator.guard.ts +++ b/src/app/+collection-page/collection-page-administrator.guard.ts @@ -1,9 +1,12 @@ import { Injectable } from '@angular/core'; -import { Router } from '@angular/router'; -import { DsoPageAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; import { Collection } from '../core/shared/collection.model'; import { CollectionPageResolver } from './collection-page.resolver'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; +import { of as observableOf } from 'rxjs'; +import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { Observable } from 'rxjs/internal/Observable'; +import { FeatureID } from '../core/data/feature-authorization/feature-id'; @Injectable({ providedIn: 'root' @@ -11,10 +14,17 @@ import { AuthorizationDataService } from '../core/data/feature-authorization/aut /** * Guard for preventing unauthorized access to certain {@link Collection} pages requiring administrator rights */ -export class CollectionPageAdministratorGuard extends DsoPageAdministratorGuard { +export class CollectionPageAdministratorGuard extends DsoPageFeatureGuard { constructor(protected resolver: CollectionPageResolver, protected authorizationService: AuthorizationDataService, protected router: Router) { super(resolver, authorizationService, router); } + + /** + * Check administrator authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.AdministratorOf); + } } diff --git a/src/app/+community-page/community-page-administrator.guard.ts b/src/app/+community-page/community-page-administrator.guard.ts index 886a449951..c5e58ddb1a 100644 --- a/src/app/+community-page/community-page-administrator.guard.ts +++ b/src/app/+community-page/community-page-administrator.guard.ts @@ -1,9 +1,12 @@ import { Injectable } from '@angular/core'; -import { Router } from '@angular/router'; -import { DsoPageAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; import { Community } from '../core/shared/community.model'; import { CommunityPageResolver } from './community-page.resolver'; import { AuthorizationDataService } from '../core/data/feature-authorization/authorization-data.service'; +import { of as observableOf } from 'rxjs'; +import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { Observable } from 'rxjs/internal/Observable'; +import { FeatureID } from '../core/data/feature-authorization/feature-id'; @Injectable({ providedIn: 'root' @@ -11,10 +14,17 @@ import { AuthorizationDataService } from '../core/data/feature-authorization/aut /** * Guard for preventing unauthorized access to certain {@link Community} pages requiring administrator rights */ -export class CommunityPageAdministratorGuard extends DsoPageAdministratorGuard { +export class CommunityPageAdministratorGuard extends DsoPageFeatureGuard { constructor(protected resolver: CommunityPageResolver, protected authorizationService: AuthorizationDataService, protected router: Router) { super(resolver, authorizationService, router); } + + /** + * Check administrator authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.AdministratorOf); + } } 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 87b4b7a592..ff1b79a247 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 @@ -20,6 +20,8 @@ import { ResourcePolicyResolver } from '../../shared/resource-policies/resolvers import { ResourcePolicyCreateComponent } from '../../shared/resource-policies/create/resource-policy-create.component'; import { ResourcePolicyEditComponent } from '../../shared/resource-policies/edit/resource-policy-edit.component'; import { I18nBreadcrumbsService } from '../../core/breadcrumbs/i18n-breadcrumbs.service'; +import { ItemPageReinstateGuard } from './item-page-reinstate.guard'; +import { ItemPageWithdrawGuard } from './item-page-withdraw.guard'; export const ITEM_EDIT_WITHDRAW_PATH = 'withdraw'; export const ITEM_EDIT_REINSTATE_PATH = 'reinstate'; @@ -97,10 +99,12 @@ export const ITEM_EDIT_AUTHORIZATIONS_PATH = 'authorizations'; { path: ITEM_EDIT_WITHDRAW_PATH, component: ItemWithdrawComponent, + canActivate: [ItemPageWithdrawGuard] }, { path: ITEM_EDIT_REINSTATE_PATH, component: ItemReinstateComponent, + canActivate: [ItemPageReinstateGuard] }, { path: ITEM_EDIT_PRIVATE_PATH, @@ -153,7 +157,9 @@ export const ITEM_EDIT_AUTHORIZATIONS_PATH = 'authorizations'; I18nBreadcrumbResolver, I18nBreadcrumbsService, ResourcePolicyResolver, - ResourcePolicyTargetResolver + ResourcePolicyTargetResolver, + ItemPageReinstateGuard, + ItemPageWithdrawGuard ] }) export class EditItemPageRoutingModule { diff --git a/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts b/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts new file mode 100644 index 0000000000..061705619a --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts @@ -0,0 +1,30 @@ +import { Injectable } from '@angular/core'; +import { DsoPageFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { Item } from '../../core/shared/item.model'; +import { ItemPageResolver } from '../item-page.resolver'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { Observable } from 'rxjs/internal/Observable'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; +import { of as observableOf } from 'rxjs'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring reinstate rights + */ +export class ItemPageReinstateGuard extends DsoPageFeatureGuard { + constructor(protected resolver: ItemPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router) { + super(resolver, authorizationService, router); + } + + /** + * Check reinstate authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.ReinstateItem); + } +} diff --git a/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts b/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts new file mode 100644 index 0000000000..60576bcdb8 --- /dev/null +++ b/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts @@ -0,0 +1,30 @@ +import { DsoPageFeatureGuard } from '../../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; +import { Item } from '../../core/shared/item.model'; +import { Injectable } from '@angular/core'; +import { ItemPageResolver } from '../item-page.resolver'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { Observable } from 'rxjs/internal/Observable'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; +import { of as observableOf } from 'rxjs'; + +@Injectable({ + providedIn: 'root' +}) +/** + * Guard for preventing unauthorized access to certain {@link Item} pages requiring withdraw rights + */ +export class ItemPageWithdrawGuard extends DsoPageFeatureGuard { + constructor(protected resolver: ItemPageResolver, + protected authorizationService: AuthorizationDataService, + protected router: Router) { + super(resolver, authorizationService, router); + } + + /** + * Check withdraw authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.WithdrawItem); + } +} diff --git a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts index 04e0e28645..00a03e18c1 100644 --- a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts +++ b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts @@ -81,7 +81,6 @@ export class ItemStatusComponent implements OnInit { if (item.isWithdrawn) { this.authorizationService.isAuthorized(FeatureID.ReinstateItem, item.self).pipe(distinctUntilChanged()).subscribe((authorized) => { if (authorized) { - console.log('added reinstate'); this.operations[index] = new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate'); } else { this.operations[index] = undefined; @@ -91,7 +90,6 @@ export class ItemStatusComponent implements OnInit { } else { this.authorizationService.isAuthorized(FeatureID.WithdrawItem, item.self).pipe(distinctUntilChanged()).subscribe((authorized) => { if (authorized) { - console.log('added withdraw'); this.operations[index] = new ItemOperation('withdraw', this.getCurrentUrl(item) + '/withdraw'); } else { this.operations[index] = undefined; diff --git a/src/app/+item-page/item-page-administrator.guard.ts b/src/app/+item-page/item-page-administrator.guard.ts index 507f8fcdbe..eae76348ad 100644 --- a/src/app/+item-page/item-page-administrator.guard.ts +++ b/src/app/+item-page/item-page-administrator.guard.ts @@ -1,9 +1,12 @@ import { Injectable } from '@angular/core'; -import { Router } from '@angular/router'; -import { DsoPageAdministratorGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard'; +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 } from 'rxjs/internal/Observable'; +import { FeatureID } from '../core/data/feature-authorization/feature-id'; +import { of as observableOf } from 'rxjs'; @Injectable({ providedIn: 'root' @@ -11,10 +14,17 @@ import { Item } from '../core/shared/item.model'; /** * Guard for preventing unauthorized access to certain {@link Item} pages requiring administrator rights */ -export class ItemPageAdministratorGuard extends DsoPageAdministratorGuard { +export class ItemPageAdministratorGuard extends DsoPageFeatureGuard { constructor(protected resolver: ItemPageResolver, protected authorizationService: AuthorizationDataService, protected router: Router) { super(resolver, authorizationService, router); } + + /** + * Check administrator authorization rights + */ + getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.AdministratorOf); + } } diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts similarity index 68% rename from src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.spec.ts rename to src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts index 7fef2e5d4c..1f5efd1329 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.spec.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts @@ -1,24 +1,31 @@ -import { DsoPageAdministratorGuard } from './dso-page-administrator.guard'; import { AuthorizationDataService } from '../authorization-data.service'; -import { Resolve, Router } from '@angular/router'; +import { ActivatedRouteSnapshot, Resolve, Router, RouterStateSnapshot } from '@angular/router'; import { RemoteData } from '../../remote-data'; import { of as observableOf } from 'rxjs'; import { createSuccessfulRemoteDataObject$ } from '../../../../shared/remote-data.utils'; import { DSpaceObject } from '../../../shared/dspace-object.model'; +import { DsoPageFeatureGuard } from './dso-page-feature.guard'; +import { FeatureID } from '../feature-id'; +import { Observable } from 'rxjs/internal/Observable'; /** * Test implementation of abstract class DsoPageAdministratorGuard */ -class DsoPageAdministratorGuardImpl extends DsoPageAdministratorGuard { +class DsoPageFeatureGuardImpl extends DsoPageFeatureGuard { constructor(protected resolver: Resolve>, protected authorizationService: AuthorizationDataService, - protected router: Router) { + protected router: Router, + protected featureID: FeatureID) { super(resolver, authorizationService, router); } + + getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(this.featureID); + } } describe('DsoPageAdministratorGuard', () => { - let guard: DsoPageAdministratorGuard; + let guard: DsoPageFeatureGuard; let authorizationService: AuthorizationDataService; let router: Router; let resolver: Resolve>; @@ -38,7 +45,7 @@ describe('DsoPageAdministratorGuard', () => { resolver = jasmine.createSpyObj('resolver', { resolve: createSuccessfulRemoteDataObject$(object) }); - guard = new DsoPageAdministratorGuardImpl(resolver, authorizationService, router); + guard = new DsoPageFeatureGuardImpl(resolver, authorizationService, router, undefined); } beforeEach(() => { diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts similarity index 73% rename from src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.ts rename to src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts index 868510eb4e..ed2590b521 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-administrator.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts @@ -1,32 +1,23 @@ -import { FeatureAuthorizationGuard } from './feature-authorization.guard'; -import { AuthorizationDataService } from '../authorization-data.service'; import { ActivatedRouteSnapshot, Resolve, Router, RouterStateSnapshot } from '@angular/router'; -import { DSpaceObject } from '../../../shared/dspace-object.model'; -import { FeatureID } from '../feature-id'; -import { of as observableOf } from 'rxjs'; -import { Observable } from 'rxjs/internal/Observable'; import { RemoteData } from '../../remote-data'; +import { AuthorizationDataService } from '../authorization-data.service'; +import { Observable } from 'rxjs/internal/Observable'; import { getAllSucceededRemoteDataPayload } from '../../../shared/operators'; import { map } from 'rxjs/operators'; +import { DSpaceObject } from '../../../shared/dspace-object.model'; +import { FeatureAuthorizationGuard } from './feature-authorization.guard'; /** - * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require administrator rights + * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require rights for a specific feature * This guard utilizes a resolver to retrieve the relevant object to check authorizations for */ -export abstract class DsoPageAdministratorGuard extends FeatureAuthorizationGuard { +export abstract class DsoPageFeatureGuard extends FeatureAuthorizationGuard { constructor(protected resolver: Resolve>, protected authorizationService: AuthorizationDataService, protected router: Router) { super(authorizationService, router); } - /** - * Check administrator authorization rights - */ - getFeatureID(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return observableOf(FeatureID.AdministratorOf); - } - /** * Check authorization rights for the object resolved using the provided resolver */ From 7787a2d0e9387819d799e0bdfa98a354088c7835 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 10 Jul 2020 13:07:31 +0200 Subject: [PATCH 4/9] 71764: SiteRegisterGuard + hide register link when unauthorized --- src/app/app-routing.module.ts | 3 ++- src/app/core/core.module.ts | 2 ++ .../site-register.guard.ts | 27 +++++++++++++++++++ .../data/feature-authorization/feature-id.ts | 1 + src/app/shared/log-in/log-in.component.html | 2 +- src/app/shared/log-in/log-in.component.ts | 11 +++++++- 6 files changed, 43 insertions(+), 3 deletions(-) create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index c8ee6ecd8b..5842e8b06b 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -12,6 +12,7 @@ import { getItemPageRoute } from './+item-page/item-page-routing.module'; import { getCollectionPageRoute } from './+collection-page/collection-page-routing.module'; import { SiteAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; import { UnauthorizedComponent } from './unauthorized/unauthorized.component'; +import { SiteRegisterGuard } from './core/data/feature-authorization/feature-authorization-guard/site-register.guard'; const ITEM_MODULE_PATH = 'items'; @@ -93,7 +94,7 @@ export function getUnauthorizedPath() { { path: 'community-list', loadChildren: './community-list-page/community-list-page.module#CommunityListPageModule' }, { path: 'id', loadChildren: './+lookup-by-id/lookup-by-id.module#LookupIdModule' }, { path: 'handle', loadChildren: './+lookup-by-id/lookup-by-id.module#LookupIdModule' }, - { path: REGISTER_PATH, loadChildren: './register-page/register-page.module#RegisterPageModule' }, + { path: REGISTER_PATH, loadChildren: './register-page/register-page.module#RegisterPageModule', canActivate: [SiteRegisterGuard] }, { path: FORGOT_PASSWORD_PATH, loadChildren: './forgot-password/forgot-password.module#ForgotPasswordModule' }, { path: COMMUNITY_MODULE_PATH, loadChildren: './+community-page/community-page.module#CommunityPageModule' }, { path: COLLECTION_MODULE_PATH, loadChildren: './+collection-page/collection-page.module#CollectionPageModule' }, diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index 4b2a842bca..6c8c1ec1dc 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -159,6 +159,7 @@ import { SubmissionCcLicenseDataService } from './submission/submission-cc-licen import { SubmissionCcLicence } from './submission/models/submission-cc-license.model'; import { SubmissionCcLicenceUrl } from './submission/models/submission-cc-license-url.model'; import { SubmissionCcLicenseUrlDataService } from './submission/submission-cc-license-url-data.service'; +import { SiteRegisterGuard } from './data/feature-authorization/feature-authorization-guard/site-register.guard'; /** * When not in production, endpoint responses can be mocked for testing purposes @@ -282,6 +283,7 @@ const PROVIDERS = [ FeatureDataService, AuthorizationDataService, SiteAdministratorGuard, + SiteRegisterGuard, MetadataSchemaDataService, MetadataFieldDataService, TokenResponseParsingService, diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts new file mode 100644 index 0000000000..18397cf71e --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts @@ -0,0 +1,27 @@ +import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { Injectable } from '@angular/core'; +import { AuthorizationDataService } from '../authorization-data.service'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; +import { Observable } from 'rxjs/internal/Observable'; +import { FeatureID } from '../feature-id'; +import { of as observableOf } from 'rxjs'; + +/** + * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have registration + * rights to the {@link Site} + */ +@Injectable({ + providedIn: 'root' +}) +export class SiteRegisterGuard extends FeatureAuthorizationGuard { + constructor(protected authorizationService: AuthorizationDataService, protected router: Router) { + super(authorizationService, router); + } + + /** + * Check registration authorization rights + */ + getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return observableOf(FeatureID.EPersonRegistration); + } +} diff --git a/src/app/core/data/feature-authorization/feature-id.ts b/src/app/core/data/feature-authorization/feature-id.ts index e3eb9cd668..27d6618e44 100644 --- a/src/app/core/data/feature-authorization/feature-id.ts +++ b/src/app/core/data/feature-authorization/feature-id.ts @@ -6,4 +6,5 @@ export enum FeatureID { AdministratorOf = 'administratorOf', WithdrawItem = 'withdrawItem', ReinstateItem = 'reinstateItem', + EPersonRegistration = 'epersonRegistration', } diff --git a/src/app/shared/log-in/log-in.component.html b/src/app/shared/log-in/log-in.component.html index 5285bc65e4..0b63ce0304 100644 --- a/src/app/shared/log-in/log-in.component.html +++ b/src/app/shared/log-in/log-in.component.html @@ -8,6 +8,6 @@
- {{"login.form.new-user" | translate}} + {{"login.form.new-user" | translate}} {{"login.form.forgot-password" | translate}} diff --git a/src/app/shared/log-in/log-in.component.ts b/src/app/shared/log-in/log-in.component.ts index 32e10fef45..91d0ea97a8 100644 --- a/src/app/shared/log-in/log-in.component.ts +++ b/src/app/shared/log-in/log-in.component.ts @@ -9,6 +9,8 @@ import { getAuthenticationMethods, isAuthenticated, isAuthenticationLoading } fr import { CoreState } from '../../core/core.reducers'; import { AuthService } from '../../core/auth/auth.service'; import { getForgotPasswordPath, getRegisterPath } from '../../app-routing.module'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; /** * /users/sign-in @@ -51,8 +53,14 @@ export class LogInComponent implements OnInit, OnDestroy { */ private alive = true; + /** + * Whether or not the current user (or anonymous) is authorized to register an account + */ + canRegister$: Observable; + constructor(private store: Store, - private authService: AuthService,) { + private authService: AuthService, + private authorizationService: AuthorizationDataService) { } ngOnInit(): void { @@ -77,6 +85,7 @@ export class LogInComponent implements OnInit, OnDestroy { } ); + this.canRegister$ = this.authorizationService.isAuthorized(FeatureID.EPersonRegistration); } ngOnDestroy(): void { From 7f6bd680b2f62d9e5c26913ac4bd5d3f7c3c8148 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 10 Jul 2020 14:18:01 +0200 Subject: [PATCH 5/9] 71764: Test fixes --- .../item-status/item-status.component.spec.ts | 15 +++++++++++++-- .../form/process-form.component.spec.ts | 2 +- src/app/shared/log-in/log-in.component.spec.ts | 9 +++++++++ 3 files changed, 23 insertions(+), 3 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-status/item-status.component.spec.ts b/src/app/+item-page/edit-item-page/item-status/item-status.component.spec.ts index abb2839551..9c28f097a4 100644 --- a/src/app/+item-page/edit-item-page/item-status/item-status.component.spec.ts +++ b/src/app/+item-page/edit-item-page/item-status/item-status.component.spec.ts @@ -12,6 +12,7 @@ import { By } from '@angular/platform-browser'; import { CUSTOM_ELEMENTS_SCHEMA } from '@angular/core'; import { of as observableOf } from 'rxjs'; import { createSuccessfulRemoteDataObject } from '../../../shared/remote-data.utils'; +import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; describe('ItemStatusComponent', () => { let comp: ItemStatusComponent; @@ -20,7 +21,10 @@ describe('ItemStatusComponent', () => { const mockItem = Object.assign(new Item(), { id: 'fake-id', handle: 'fake/handle', - lastModified: '2018' + lastModified: '2018', + _links: { + self: { href: 'test-item-selflink' } + } }); const itemPageUrl = `items/${mockItem.id}`; @@ -31,13 +35,20 @@ describe('ItemStatusComponent', () => { } }; + let authorizationService: AuthorizationDataService; + beforeEach(async(() => { + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthorized: observableOf(true) + }); + TestBed.configureTestingModule({ imports: [CommonModule, RouterTestingModule.withRoutes([]), TranslateModule.forRoot(), NgbModule], declarations: [ItemStatusComponent], providers: [ { provide: ActivatedRoute, useValue: routeStub }, - { provide: HostWindowService, useValue: new HostWindowServiceStub(0) } + { provide: HostWindowService, useValue: new HostWindowServiceStub(0) }, + { provide: AuthorizationDataService, useValue: authorizationService }, ], schemas: [CUSTOM_ELEMENTS_SCHEMA] }).compileComponents(); })); diff --git a/src/app/process-page/form/process-form.component.spec.ts b/src/app/process-page/form/process-form.component.spec.ts index 938aa56a88..95f266ed6f 100644 --- a/src/app/process-page/form/process-form.component.spec.ts +++ b/src/app/process-page/form/process-form.component.spec.ts @@ -60,7 +60,7 @@ describe('ProcessFormComponent', () => { { provide: ScriptDataService, useValue: scriptService }, { provide: NotificationsService, useClass: NotificationsServiceStub }, { provide: RequestService, useValue: jasmine.createSpyObj('requestService', ['removeBySubstring', 'removeByHrefSubstring']) }, - { provide: Router, useValue: {} }, + { provide: Router, useValue: jasmine.createSpyObj('router', ['navigateByUrl']) }, ], schemas: [NO_ERRORS_SCHEMA] }) diff --git a/src/app/shared/log-in/log-in.component.spec.ts b/src/app/shared/log-in/log-in.component.spec.ts index a9a42bf3dd..5ff9754fec 100644 --- a/src/app/shared/log-in/log-in.component.spec.ts +++ b/src/app/shared/log-in/log-in.component.spec.ts @@ -18,6 +18,8 @@ import { NativeWindowService } from '../../core/services/window.service'; import { provideMockStore } from '@ngrx/store/testing'; import { createTestComponent } from '../testing/utils.test'; import { RouterTestingModule } from '@angular/router/testing'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { of } from 'rxjs/internal/observable/of'; describe('LogInComponent', () => { @@ -34,7 +36,13 @@ describe('LogInComponent', () => { } }; + let authorizationService: AuthorizationDataService; + beforeEach(async(() => { + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthorized: of(true) + }); + // refine the test module by declaring the test component TestBed.configureTestingModule({ imports: [ @@ -58,6 +66,7 @@ describe('LogInComponent', () => { { provide: NativeWindowService, useFactory: NativeWindowMockFactory }, // { provide: Router, useValue: new RouterStub() }, { provide: ActivatedRoute, useValue: new ActivatedRouteStub() }, + { provide: AuthorizationDataService, useValue: authorizationService }, provideMockStore({ initialState }), LogInComponent ], From 3afe8c0b1dfb4ef7fc95677109db94594aa8464a Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 14 Jul 2020 15:13:21 +0200 Subject: [PATCH 6/9] 71764: ItemStatusComponent BehaviorSubject for updating operations --- .../item-status/item-status.component.html | 2 +- .../item-status/item-status.component.ts | 47 ++++++++++--------- 2 files changed, 27 insertions(+), 22 deletions(-) diff --git a/src/app/+item-page/edit-item-page/item-status/item-status.component.html b/src/app/+item-page/edit-item-page/item-status/item-status.component.html index a619d4a576..3fcf10a2f5 100644 --- a/src/app/+item-page/edit-item-page/item-status/item-status.component.html +++ b/src/app/+item-page/edit-item-page/item-status/item-status.component.html @@ -15,7 +15,7 @@ {{getItemPage((itemRD$ | async)?.payload)}} -
+
diff --git a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts index 00a03e18c1..93792acb35 100644 --- a/src/app/+item-page/edit-item-page/item-status/item-status.component.ts +++ b/src/app/+item-page/edit-item-page/item-status/item-status.component.ts @@ -10,6 +10,7 @@ import { getItemEditPath, getItemPageRoute } from '../../item-page-routing.modul import { AuthorizationDataService } from '../../../core/data/feature-authorization/authorization-data.service'; import { FeatureID } from '../../../core/data/feature-authorization/feature-id'; import { hasValue } from '../../../shared/empty.util'; +import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; @Component({ selector: 'ds-item-status', @@ -43,7 +44,7 @@ export class ItemStatusComponent implements OnInit { * The possible actions that can be performed on the item * key: id value: url to action's component */ - operations: ItemOperation[]; + operations$: BehaviorSubject = new BehaviorSubject([]); /** * The keys of the actions (to loop over) @@ -51,8 +52,7 @@ export class ItemStatusComponent implements OnInit { actionsKeys; constructor(private route: ActivatedRoute, - private authorizationService: AuthorizationDataService, - private changeDetection: ChangeDetectorRef) { + private authorizationService: AuthorizationDataService) { } ngOnInit(): void { @@ -72,38 +72,43 @@ export class ItemStatusComponent implements OnInit { i18n example: 'item.edit.tabs.status.buttons..label' The value is supposed to be a href for the button */ - this.operations = []; - this.operations.push(new ItemOperation('authorizations', this.getCurrentUrl(item) + '/authorizations')); - this.operations.push(new ItemOperation('mappedCollections', this.getCurrentUrl(item) + '/mapper')); - this.operations.push(undefined); + const operations = []; + operations.push(new ItemOperation('authorizations', this.getCurrentUrl(item) + '/authorizations')); + operations.push(new ItemOperation('mappedCollections', this.getCurrentUrl(item) + '/mapper')); + operations.push(undefined); // Store the index of the "withdraw" or "reinstate" operation, because it's added asynchronously - const index = this.operations.length - 1; + const indexOfWithdrawReinstate = operations.length - 1; + if (item.isDiscoverable) { + operations.push(new ItemOperation('private', this.getCurrentUrl(item) + '/private')); + } else { + operations.push(new ItemOperation('public', this.getCurrentUrl(item) + '/public')); + } + operations.push(new ItemOperation('delete', this.getCurrentUrl(item) + '/delete')); + operations.push(new ItemOperation('move', this.getCurrentUrl(item) + '/move')); + + this.operations$.next(operations); + if (item.isWithdrawn) { this.authorizationService.isAuthorized(FeatureID.ReinstateItem, item.self).pipe(distinctUntilChanged()).subscribe((authorized) => { + const newOperations = [...this.operations$.value]; if (authorized) { - this.operations[index] = new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate'); + newOperations[indexOfWithdrawReinstate] = new ItemOperation('reinstate', this.getCurrentUrl(item) + '/reinstate'); } else { - this.operations[index] = undefined; + newOperations[indexOfWithdrawReinstate] = undefined; } - this.changeDetection.detectChanges(); + this.operations$.next(newOperations); }); } else { this.authorizationService.isAuthorized(FeatureID.WithdrawItem, item.self).pipe(distinctUntilChanged()).subscribe((authorized) => { + const newOperations = [...this.operations$.value]; if (authorized) { - this.operations[index] = new ItemOperation('withdraw', this.getCurrentUrl(item) + '/withdraw'); + newOperations[indexOfWithdrawReinstate] = new ItemOperation('withdraw', this.getCurrentUrl(item) + '/withdraw'); } else { - this.operations[index] = undefined; + newOperations[indexOfWithdrawReinstate] = undefined; } - this.changeDetection.detectChanges(); + this.operations$.next(newOperations); }); } - if (item.isDiscoverable) { - this.operations.push(new ItemOperation('private', this.getCurrentUrl(item) + '/private')); - } else { - this.operations.push(new ItemOperation('public', this.getCurrentUrl(item) + '/public')); - } - this.operations.push(new ItemOperation('delete', this.getCurrentUrl(item) + '/delete')); - this.operations.push(new ItemOperation('move', this.getCurrentUrl(item) + '/move')); }); } From adb51bc06c6ea6ee044a6fd7619f0ee996ebcc11 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 30 Jul 2020 11:09:16 +0200 Subject: [PATCH 7/9] 71764: authorization-data-service remove eperson param unless provided --- .../data/feature-authorization/authorization-data.service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/core/data/feature-authorization/authorization-data.service.ts b/src/app/core/data/feature-authorization/authorization-data.service.ts index 2d32b26efa..e8ee7a9d65 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -90,7 +90,6 @@ export class AuthorizationDataService extends DataService { searchByObject(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { return observableOf(new AuthorizationSearchParams(objectUrl, ePersonUuid, featureId)).pipe( addSiteObjectUrlIfEmpty(this.siteService), - addAuthenticatedUserUuidIfEmpty(this.authService), switchMap((params: AuthorizationSearchParams) => { return this.searchBy(this.searchByObjectPath, this.createSearchOptions(params.objectUrl, options, params.ePersonUuid, params.featureId), ...linksToFollow); }) From 23354b45c189e61bd688e6ecae43c29af6635a51 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 30 Jul 2020 11:24:58 +0200 Subject: [PATCH 8/9] 71764: Test changes --- .../authorization-data.service.spec.ts | 29 ++++++------------- 1 file changed, 9 insertions(+), 20 deletions(-) diff --git a/src/app/core/data/feature-authorization/authorization-data.service.spec.ts b/src/app/core/data/feature-authorization/authorization-data.service.spec.ts index 29db1a086b..7db7c27c29 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.spec.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.spec.ts @@ -63,33 +63,33 @@ describe('AuthorizationDataService', () => { return Object.assign(new FindListOptions(), { searchParams }); } - describe('when no arguments are provided and a user is authenticated', () => { + describe('when no arguments are provided', () => { beforeEach(() => { service.searchByObject().subscribe(); }); - it('should call searchBy with the site\'s url and authenticated user\'s uuid', () => { - expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(site.self, ePerson.uuid)); + it('should call searchBy with the site\'s url', () => { + expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(site.self)); }); }); - describe('when no arguments except for a feature are provided and a user is authenticated', () => { + describe('when no arguments except for a feature are provided', () => { beforeEach(() => { service.searchByObject(FeatureID.LoginOnBehalfOf).subscribe(); }); - it('should call searchBy with the site\'s url, authenticated user\'s uuid and the feature', () => { - expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(site.self, ePerson.uuid, FeatureID.LoginOnBehalfOf)); + it('should call searchBy with the site\'s url and the feature', () => { + expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(site.self, null, FeatureID.LoginOnBehalfOf)); }); }); - describe('when a feature and object url are provided, but no user uuid and a user is authenticated', () => { + describe('when a feature and object url are provided', () => { beforeEach(() => { service.searchByObject(FeatureID.LoginOnBehalfOf, objectUrl).subscribe(); }); - it('should call searchBy with the object\'s url, authenticated user\'s uuid and the feature', () => { - expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, ePerson.uuid, FeatureID.LoginOnBehalfOf)); + it('should call searchBy with the object\'s url and the feature', () => { + expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, null, FeatureID.LoginOnBehalfOf)); }); }); @@ -102,17 +102,6 @@ describe('AuthorizationDataService', () => { expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, ePersonUuid, FeatureID.LoginOnBehalfOf)); }); }); - - describe('when no arguments are provided and no user is authenticated', () => { - beforeEach(() => { - spyOn(authService, 'isAuthenticated').and.returnValue(observableOf(false)); - service.searchByObject().subscribe(); - }); - - it('should call searchBy with the site\'s url', () => { - expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(site.self)); - }); - }); }); describe('isAuthorized', () => { From 124ed4aff2ef99f5f5b952afa2670874310e9d82 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 22 Sep 2020 09:14:07 +0200 Subject: [PATCH 9/9] 71764: LGTM Issue fix --- .../data/feature-authorization/authorization-data.service.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/core/data/feature-authorization/authorization-data.service.ts b/src/app/core/data/feature-authorization/authorization-data.service.ts index e8ee7a9d65..4dfa89cde6 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -25,7 +25,6 @@ import { hasValue, isNotEmpty } from '../../../shared/empty.util'; import { RequestParam } from '../../cache/models/request-param.model'; import { AuthorizationSearchParams } from './authorization-search-params'; import { - addAuthenticatedUserUuidIfEmpty, addSiteObjectUrlIfEmpty, oneAuthorizationMatchesFeature } from './authorization-utils';