From 39e0c035939d661b02ba3680492d4a80cec80a28 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 17 Jun 2020 16:27:25 +0200 Subject: [PATCH 01/14] 71429: Feature and Authentication Models and Services --- src/app/core/core.module.ts | 10 +- .../authorization-data.service.ts | 126 ++++++++++++++++++ .../authorization-search-params.ts | 11 ++ .../authorization-utils.ts | 53 ++++++++ .../feature-data.service.ts | 72 ++++++++++ src/app/core/shared/authorization.model.ts | 54 ++++++++ .../shared/authorization.resource-type.ts | 9 ++ src/app/core/shared/feature.model.ts | 31 +++++ src/app/core/shared/feature.resource-type.ts | 9 ++ 9 files changed, 374 insertions(+), 1 deletion(-) create mode 100644 src/app/core/data/feature-authorization/authorization-data.service.ts create mode 100644 src/app/core/data/feature-authorization/authorization-search-params.ts create mode 100644 src/app/core/data/feature-authorization/authorization-utils.ts create mode 100644 src/app/core/data/feature-authorization/feature-data.service.ts create mode 100644 src/app/core/shared/authorization.model.ts create mode 100644 src/app/core/shared/authorization.resource-type.ts create mode 100644 src/app/core/shared/feature.model.ts create mode 100644 src/app/core/shared/feature.resource-type.ts diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index 715f7a5cc0..af394ea71a 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -145,6 +145,10 @@ import { Version } from './shared/version.model'; import { VersionHistory } from './shared/version-history.model'; import { WorkflowActionDataService } from './data/workflow-action-data.service'; import { WorkflowAction } from './tasks/models/workflow-action-object.model'; +import { Feature } from './shared/feature.model'; +import { Authorization } from './shared/authorization.model'; +import { FeatureDataService } from './data/feature-authorization/feature-data.service'; +import { AuthorizationDataService } from './data/feature-authorization/authorization-data.service'; /** * When not in production, endpoint responses can be mocked for testing purposes @@ -264,6 +268,8 @@ const PROVIDERS = [ LicenseDataService, ItemTypeDataService, WorkflowActionDataService, + FeatureDataService, + AuthorizationDataService, // register AuthInterceptor as HttpInterceptor { provide: HTTP_INTERCEPTORS, @@ -314,7 +320,9 @@ export const models = ExternalSourceEntry, Version, VersionHistory, - WorkflowAction + WorkflowAction, + Feature, + Authorization ]; @NgModule({ diff --git a/src/app/core/data/feature-authorization/authorization-data.service.ts b/src/app/core/data/feature-authorization/authorization-data.service.ts new file mode 100644 index 0000000000..57a02435dc --- /dev/null +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -0,0 +1,126 @@ +import { of as observableOf } from 'rxjs'; +import { Injectable } from '@angular/core'; +import { AUTHORIZATION } from '../../shared/authorization.resource-type'; +import { dataService } from '../../cache/builders/build-decorators'; +import { DataService } from '../data.service'; +import { Authorization } from '../../shared/authorization.model'; +import { RequestService } from '../request.service'; +import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; +import { Store } from '@ngrx/store'; +import { CoreState } from '../../core.reducers'; +import { ObjectCacheService } from '../../cache/object-cache.service'; +import { HALEndpointService } from '../../shared/hal-endpoint.service'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { HttpClient } from '@angular/common/http'; +import { DSOChangeAnalyzer } from '../dso-change-analyzer.service'; +import { AuthService } from '../../auth/auth.service'; +import { SiteDataService } from '../site-data.service'; +import { FindListOptions, FindListRequest } from '../request.models'; +import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model'; +import { Observable } from 'rxjs/internal/Observable'; +import { RemoteData } from '../remote-data'; +import { PaginatedList } from '../paginated-list'; +import { find, skipWhile, switchMap, tap } from 'rxjs/operators'; +import { hasValue, isNotEmpty } from '../../../shared/empty.util'; +import { RequestParam } from '../../cache/models/request-param.model'; +import { AuthorizationSearchParams } from './authorization-search-params'; +import { addAuthenticatedUserUuidIfEmpty, addSiteObjectUrlIfEmpty } from './authorization-utils'; + +/** + * A service to retrieve {@link Authorization}s from the REST API + */ +@Injectable() +@dataService(AUTHORIZATION) +export class AuthorizationDataService extends DataService { + protected linkPath = 'authorizations'; + protected searchByObjectPath = 'object'; + + constructor( + protected requestService: RequestService, + protected rdbService: RemoteDataBuildService, + protected store: Store, + protected objectCache: ObjectCacheService, + protected halService: HALEndpointService, + protected notificationsService: NotificationsService, + protected http: HttpClient, + protected comparator: DSOChangeAnalyzer, + protected authService: AuthService, + protected siteService: SiteDataService + ) { + super(); + } + + /** + * Search for a list of {@link Authorization}s using the "object" search endpoint and providing optional object url, + * {@link EPerson} uuid and/or {@link Feature} id + * @param objectUrl URL to the object to search {@link Authorization}s for. + * If not provided, the repository's {@link Site} will be used. + * @param ePersonUuid UUID of the {@link EPerson} to search {@link Authorization}s for. + * If not provided, the UUID of the currently authenticated {@link EPerson} will be used. + * @param featureId ID of the {@link Feature} to search {@link Authorization}s for + * @param options {@link FindListOptions} to provide pagination and/or additional arguments + * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved + */ + searchByObject(objectUrl?: string, ePersonUuid?: string, featureId?: 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); + }) + ); + } + + /** + * Make a new FindListRequest with given search method + * + * @param searchMethod The search method for the object + * @param options The [[FindListOptions]] object + * @param linksToFollow The array of [[FollowLinkConfig]] + * @return {Observable>} + * Return an observable that emits response from the server + */ + searchBy(searchMethod: string, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { + const hrefObs = this.getSearchByHref(searchMethod, options, ...linksToFollow); + + return hrefObs.pipe( + find((href: string) => hasValue(href)), + tap((href: string) => { + this.requestService.removeByHrefSubstring(href); + const request = new FindListRequest(this.requestService.generateRequestId(), href, options); + + this.requestService.configure(request); + } + ), + switchMap((href) => this.requestService.getByHref(href)), + skipWhile((requestEntry) => hasValue(requestEntry) && requestEntry.completed), + switchMap((href) => + this.rdbService.buildList(hrefObs, ...linksToFollow) as Observable>> + ) + ); + } + + /** + * Create {@link FindListOptions} with {@link RequestParam}s containing a "uri", "feature" and/or "eperson" parameter + * @param objectUrl Required parameter value to add to {@link RequestParam} "uri" + * @param options Optional initial {@link FindListOptions} to add parameters to + * @param ePersonUuid Optional parameter value to add to {@link RequestParam} "eperson" + * @param featureId Optional parameter value to add to {@link RequestParam} "feature" + */ + private createSearchOptions(objectUrl: string, options: FindListOptions = {}, ePersonUuid?: string, featureId?: string): FindListOptions { + let params = []; + if (isNotEmpty(options.searchParams)) { + params = [...options.searchParams]; + } + params.push(new RequestParam('uri', objectUrl)) + if (hasValue(featureId)) { + params.push(new RequestParam('feature', featureId)); + } + if (hasValue(ePersonUuid)) { + params.push(new RequestParam('eperson', ePersonUuid)); + } + return Object.assign(new FindListOptions(), options, { + searchParams: [...params] + }); + } +} diff --git a/src/app/core/data/feature-authorization/authorization-search-params.ts b/src/app/core/data/feature-authorization/authorization-search-params.ts new file mode 100644 index 0000000000..8d545039ba --- /dev/null +++ b/src/app/core/data/feature-authorization/authorization-search-params.ts @@ -0,0 +1,11 @@ +export class AuthorizationSearchParams { + objectUrl: string; + ePersonUuid: string; + featureId: string; + + constructor(objectUrl?: string, ePersonUuid?: string, featureId?: string) { + this.objectUrl = objectUrl; + this.ePersonUuid = ePersonUuid; + this.featureId = featureId; + } +} diff --git a/src/app/core/data/feature-authorization/authorization-utils.ts b/src/app/core/data/feature-authorization/authorization-utils.ts new file mode 100644 index 0000000000..6cc5a3d2ef --- /dev/null +++ b/src/app/core/data/feature-authorization/authorization-utils.ts @@ -0,0 +1,53 @@ +import { map, switchMap } from 'rxjs/operators'; +import { Observable } from 'rxjs/internal/Observable'; +import { AuthorizationSearchParams } from './authorization-search-params'; +import { SiteDataService } from '../site-data.service'; +import { hasNoValue } from '../../../shared/empty.util'; +import { of as observableOf } from 'rxjs'; +import { AuthService } from '../../auth/auth.service'; + +/** + * Operator accepting {@link AuthorizationSearchParams} and adding the current {@link Site}'s selflink to the parameter's + * objectUrl property, if this property is empty + * @param siteService The {@link SiteDataService} used for retrieving the repository's {@link Site} + */ +export const addSiteObjectUrlIfEmpty = (siteService: SiteDataService) => + (source: Observable): Observable => + source.pipe( + switchMap((params: AuthorizationSearchParams) => { + if (hasNoValue(params.objectUrl)) { + return siteService.find().pipe( + map((site) => Object.assign({}, params, { objectUrl: site.self })) + ); + } else { + return observableOf(params); + } + }) + ); + +/** + * Operator accepting {@link AuthorizationSearchParams} and adding the authenticated user's uuid to the parameter's + * ePersonUuid property, if this property is empty and an {@link EPerson} is currently authenticated + * @param authService The {@link AuthService} used for retrieving the currently authenticated {@link EPerson} + */ +export const addAuthenticatedUserUuidIfEmpty = (authService: AuthService) => + (source: Observable): Observable => + source.pipe( + switchMap((params: AuthorizationSearchParams) => { + if (hasNoValue(params.ePersonUuid)) { + return authService.isAuthenticated().pipe( + switchMap((authenticated) => { + if (authenticated) { + return authService.getAuthenticatedUserFromStore().pipe( + map((ePerson) => Object.assign({}, params, { ePersonUuid: ePerson.uuid })) + ); + } else { + observableOf(params) + } + }) + ); + } else { + return observableOf(params); + } + }) + ); diff --git a/src/app/core/data/feature-authorization/feature-data.service.ts b/src/app/core/data/feature-authorization/feature-data.service.ts new file mode 100644 index 0000000000..8003b6c31d --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-data.service.ts @@ -0,0 +1,72 @@ +import { Observable } from 'rxjs/internal/Observable'; +import { Injectable } from '@angular/core'; +import { FEATURE } from '../../shared/feature.resource-type'; +import { dataService } from '../../cache/builders/build-decorators'; +import { DataService } from '../data.service'; +import { Feature } from '../../shared/feature.model'; +import { RequestService } from '../request.service'; +import { RemoteDataBuildService } from '../../cache/builders/remote-data-build.service'; +import { Store } from '@ngrx/store'; +import { CoreState } from '../../core.reducers'; +import { ObjectCacheService } from '../../cache/object-cache.service'; +import { HALEndpointService } from '../../shared/hal-endpoint.service'; +import { NotificationsService } from '../../../shared/notifications/notifications.service'; +import { HttpClient } from '@angular/common/http'; +import { DSOChangeAnalyzer } from '../dso-change-analyzer.service'; +import { FindListOptions, FindListRequest } from '../request.models'; +import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model'; +import { RemoteData } from '../remote-data'; +import { PaginatedList } from '../paginated-list'; +import { find, skipWhile, switchMap, tap } from 'rxjs/operators'; +import { hasValue } from '../../../shared/empty.util'; + +/** + * A service to retrieve {@link Feature}s from the REST API + */ +@Injectable() +@dataService(FEATURE) +export class FeatureDataService extends DataService { + protected linkPath = 'features'; + + constructor( + protected requestService: RequestService, + protected rdbService: RemoteDataBuildService, + protected store: Store, + protected objectCache: ObjectCacheService, + protected halService: HALEndpointService, + protected notificationsService: NotificationsService, + protected http: HttpClient, + protected comparator: DSOChangeAnalyzer + ) { + super(); + } + + /** + * Make a new FindListRequest with given search method + * + * @param searchMethod The search method for the object + * @param options The [[FindListOptions]] object + * @param linksToFollow The array of [[FollowLinkConfig]] + * @return {Observable>} + * Return an observable that emits response from the server + */ + searchBy(searchMethod: string, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { + const hrefObs = this.getSearchByHref(searchMethod, options, ...linksToFollow); + + return hrefObs.pipe( + find((href: string) => hasValue(href)), + tap((href: string) => { + this.requestService.removeByHrefSubstring(href); + const request = new FindListRequest(this.requestService.generateRequestId(), href, options); + + this.requestService.configure(request); + } + ), + switchMap((href) => this.requestService.getByHref(href)), + skipWhile((requestEntry) => hasValue(requestEntry) && requestEntry.completed), + switchMap((href) => + this.rdbService.buildList(hrefObs, ...linksToFollow) as Observable>> + ) + ); + } +} diff --git a/src/app/core/shared/authorization.model.ts b/src/app/core/shared/authorization.model.ts new file mode 100644 index 0000000000..0dfb7fd631 --- /dev/null +++ b/src/app/core/shared/authorization.model.ts @@ -0,0 +1,54 @@ +import { link, typedObject } from '../cache/builders/build-decorators'; +import { AUTHORIZATION } from './authorization.resource-type'; +import { autoserialize, deserialize, inheritSerialization } from 'cerialize'; +import { HALLink } from './hal-link.model'; +import { Observable } from 'rxjs/internal/Observable'; +import { RemoteData } from '../data/remote-data'; +import { EPerson } from '../eperson/models/eperson.model'; +import { EPERSON } from '../eperson/models/eperson.resource-type'; +import { FEATURE } from './feature.resource-type'; +import { DSpaceObject } from './dspace-object.model'; +import { Feature } from './feature.model'; +import { ITEM } from './item.resource-type'; + +/** + * Class representing a DSpace Authorization + */ +@typedObject +@inheritSerialization(DSpaceObject) +export class Authorization extends DSpaceObject { + static type = AUTHORIZATION; + + /** + * Unique identifier for this authorization + */ + @autoserialize + id: string; + + @deserialize + _links: { + self: HALLink; + eperson: HALLink; + feature: HALLink; + object: HALLink; + }; + + /** + * The EPerson this Authorization belongs to + * Null if the authorization grants access to anonymous users + */ + @link(EPERSON) + eperson?: Observable>; + + /** + * The Feature enabled by this Authorization + */ + @link(FEATURE) + feature?: Observable>; + + /** + * The Object this authorization applies to + */ + @link(ITEM) + object?: Observable>; +} diff --git a/src/app/core/shared/authorization.resource-type.ts b/src/app/core/shared/authorization.resource-type.ts new file mode 100644 index 0000000000..e5547314b3 --- /dev/null +++ b/src/app/core/shared/authorization.resource-type.ts @@ -0,0 +1,9 @@ +import { ResourceType } from './resource-type'; + +/** + * The resource type for Authorization + * + * Needs to be in a separate file to prevent circular + * dependencies in webpack. + */ +export const AUTHORIZATION = new ResourceType('authorization'); diff --git a/src/app/core/shared/feature.model.ts b/src/app/core/shared/feature.model.ts new file mode 100644 index 0000000000..cc0fd2db87 --- /dev/null +++ b/src/app/core/shared/feature.model.ts @@ -0,0 +1,31 @@ +import { typedObject } from '../cache/builders/build-decorators'; +import { autoserialize, inheritSerialization } from 'cerialize'; +import { FEATURE } from './feature.resource-type'; +import { DSpaceObject } from './dspace-object.model'; + +/** + * Class representing a DSpace Feature + */ +@typedObject +@inheritSerialization(DSpaceObject) +export class Feature extends DSpaceObject { + static type = FEATURE; + + /** + * Unique identifier for this feature + */ + @autoserialize + id: string; + + /** + * A human readable description of the feature's purpose + */ + @autoserialize + description: string; + + /** + * A list of resource types this feature applies to + */ + @autoserialize + resourcetypes: string[]; +} diff --git a/src/app/core/shared/feature.resource-type.ts b/src/app/core/shared/feature.resource-type.ts new file mode 100644 index 0000000000..98ee526ce3 --- /dev/null +++ b/src/app/core/shared/feature.resource-type.ts @@ -0,0 +1,9 @@ +import { ResourceType } from './resource-type'; + +/** + * The resource type for Feature + * + * Needs to be in a separate file to prevent circular + * dependencies in webpack. + */ +export const FEATURE = new ResourceType('feature'); From ec998fce303e67898822578e286cff827aeea865 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 18 Jun 2020 10:04:18 +0200 Subject: [PATCH 02/14] 71429: isAuthenticated and canImpersonate$ --- .../eperson-form/eperson-form.component.ts | 15 ++++++++++----- .../authorization-data.service.ts | 16 +++++++++++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts index 9e3bcc88c0..10c4dd5bfc 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts @@ -7,9 +7,9 @@ import { DynamicInputModel } from '@ng-dynamic-forms/core'; import { TranslateService } from '@ngx-translate/core'; -import { Subscription, combineLatest, of } from 'rxjs'; +import { Subscription, combineLatest, of, interval } from 'rxjs'; import { Observable } from 'rxjs/internal/Observable'; -import { take } from 'rxjs/operators'; +import { flatMap, map, switchMap, take } from 'rxjs/operators'; import { RestResponse } from '../../../../core/cache/response.models'; import { PaginatedList } from '../../../../core/data/paginated-list'; import { RemoteData } from '../../../../core/data/remote-data'; @@ -23,6 +23,8 @@ import { FormBuilderService } from '../../../../shared/form/builder/form-builder import { NotificationsService } from '../../../../shared/notifications/notifications.service'; import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model'; import { AuthService } from '../../../../core/auth/auth.service'; +import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; +import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; @Component({ selector: 'ds-eperson-form', @@ -120,9 +122,8 @@ export class EPersonFormComponent implements OnInit, OnDestroy { /** * Observable whether or not the admin is allowed to impersonate the EPerson - * TODO: Initialize the observable once the REST API supports this (currently hardcoded to return true) */ - canImpersonate$: Observable = of(true); + canImpersonate$: Observable; /** * List of subscriptions @@ -158,7 +159,8 @@ export class EPersonFormComponent implements OnInit, OnDestroy { private formBuilderService: FormBuilderService, private translateService: TranslateService, private notificationsService: NotificationsService, - private authService: AuthService) { + private authService: AuthService, + private authorizationService: AuthorizationDataService) { this.subs.push(this.epersonService.getActiveEPerson().subscribe((eperson: EPerson) => { this.epersonInitial = eperson; if (hasValue(eperson)) { @@ -242,6 +244,9 @@ export class EPersonFormComponent implements OnInit, OnDestroy { requireCertificate: eperson != null ? eperson.requireCertificate : false }); })); + this.canImpersonate$ = this.epersonService.getActiveEPerson().pipe( + switchMap((eperson) => this.authorizationService.isAuthenticated(eperson.self, undefined, 'loginOnBehalfOf')) + ); }); } 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 57a02435dc..1a827c413e 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -20,7 +20,7 @@ import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model import { Observable } from 'rxjs/internal/Observable'; import { RemoteData } from '../remote-data'; import { PaginatedList } from '../paginated-list'; -import { find, skipWhile, switchMap, tap } from 'rxjs/operators'; +import { find, map, skipWhile, switchMap, tap } from 'rxjs/operators'; import { hasValue, isNotEmpty } from '../../../shared/empty.util'; import { RequestParam } from '../../cache/models/request-param.model'; import { AuthorizationSearchParams } from './authorization-search-params'; @@ -50,6 +50,20 @@ export class AuthorizationDataService extends DataService { super(); } + /** + * Checks if an {@link EPerson} (or anonymous) has access to a specific object within a {@link Feature} + * @param objectUrl URL to the object to search {@link Authorization}s for. + * If not provided, the repository's {@link Site} will be used. + * @param ePersonUuid UUID of the {@link EPerson} to search {@link Authorization}s for. + * If not provided, the UUID of the currently authenticated {@link EPerson} will be used. + * @param featureId ID of the {@link Feature} to check {@link Authorization} for + */ + isAuthenticated(objectUrl?: string, ePersonUuid?: string, featureId?: string): Observable { + return this.searchByObject(objectUrl, ePersonUuid, featureId).pipe( + map((authorizationRD) => (authorizationRD.statusCode !== 401 && hasValue(authorizationRD.payload) && isNotEmpty(authorizationRD.payload.page))) + ); + } + /** * Search for a list of {@link Authorization}s using the "object" search endpoint and providing optional object url, * {@link EPerson} uuid and/or {@link Feature} id From 8350833b61ab46d0e79c77dcdb06ea814187ed32 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 18 Jun 2020 14:47:59 +0200 Subject: [PATCH 03/14] 71429: FeatureType enum and searchBy override --- .../eperson-form/eperson-form.component.ts | 4 +-- .../authorization-data.service.ts | 7 +++-- .../authorization-search-params.ts | 6 ++-- .../feature-authorization/feature-type.ts | 6 ++++ src/app/core/eperson/eperson-data.service.ts | 31 ++++++++++++++++++- 5 files changed, 46 insertions(+), 8 deletions(-) create mode 100644 src/app/core/data/feature-authorization/feature-type.ts diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts index 10c4dd5bfc..e9e446ed34 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts @@ -24,7 +24,7 @@ import { NotificationsService } from '../../../../shared/notifications/notificat import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model'; import { AuthService } from '../../../../core/auth/auth.service'; import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; -import { BehaviorSubject } from 'rxjs/internal/BehaviorSubject'; +import { FeatureType } from '../../../../core/data/feature-authorization/feature-type'; @Component({ selector: 'ds-eperson-form', @@ -245,7 +245,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy { }); })); this.canImpersonate$ = this.epersonService.getActiveEPerson().pipe( - switchMap((eperson) => this.authorizationService.isAuthenticated(eperson.self, undefined, 'loginOnBehalfOf')) + switchMap((eperson) => this.authorizationService.isAuthenticated(eperson.self, undefined, FeatureType.LoginOnBehalfOf)) ); }); } 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 1a827c413e..686ff94b19 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -25,6 +25,7 @@ import { hasValue, isNotEmpty } from '../../../shared/empty.util'; import { RequestParam } from '../../cache/models/request-param.model'; import { AuthorizationSearchParams } from './authorization-search-params'; import { addAuthenticatedUserUuidIfEmpty, addSiteObjectUrlIfEmpty } from './authorization-utils'; +import { FeatureType } from './feature-type'; /** * A service to retrieve {@link Authorization}s from the REST API @@ -58,7 +59,7 @@ export class AuthorizationDataService extends DataService { * If not provided, the UUID of the currently authenticated {@link EPerson} will be used. * @param featureId ID of the {@link Feature} to check {@link Authorization} for */ - isAuthenticated(objectUrl?: string, ePersonUuid?: string, featureId?: string): Observable { + isAuthenticated(objectUrl?: string, ePersonUuid?: string, featureId?: FeatureType): Observable { return this.searchByObject(objectUrl, ePersonUuid, featureId).pipe( map((authorizationRD) => (authorizationRD.statusCode !== 401 && hasValue(authorizationRD.payload) && isNotEmpty(authorizationRD.payload.page))) ); @@ -75,7 +76,7 @@ export class AuthorizationDataService extends DataService { * @param options {@link FindListOptions} to provide pagination and/or additional arguments * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ - searchByObject(objectUrl?: string, ePersonUuid?: string, featureId?: string, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { + searchByObject(objectUrl?: string, ePersonUuid?: string, featureId?: FeatureType, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { return observableOf(new AuthorizationSearchParams(objectUrl, ePersonUuid, featureId)).pipe( addSiteObjectUrlIfEmpty(this.siteService), addAuthenticatedUserUuidIfEmpty(this.authService), @@ -121,7 +122,7 @@ export class AuthorizationDataService extends DataService { * @param ePersonUuid Optional parameter value to add to {@link RequestParam} "eperson" * @param featureId Optional parameter value to add to {@link RequestParam} "feature" */ - private createSearchOptions(objectUrl: string, options: FindListOptions = {}, ePersonUuid?: string, featureId?: string): FindListOptions { + private createSearchOptions(objectUrl: string, options: FindListOptions = {}, ePersonUuid?: string, featureId?: FeatureType): FindListOptions { let params = []; if (isNotEmpty(options.searchParams)) { params = [...options.searchParams]; diff --git a/src/app/core/data/feature-authorization/authorization-search-params.ts b/src/app/core/data/feature-authorization/authorization-search-params.ts index 8d545039ba..dc045add84 100644 --- a/src/app/core/data/feature-authorization/authorization-search-params.ts +++ b/src/app/core/data/feature-authorization/authorization-search-params.ts @@ -1,9 +1,11 @@ +import { FeatureType } from './feature-type'; + export class AuthorizationSearchParams { objectUrl: string; ePersonUuid: string; - featureId: string; + featureId: FeatureType; - constructor(objectUrl?: string, ePersonUuid?: string, featureId?: string) { + constructor(objectUrl?: string, ePersonUuid?: string, featureId?: FeatureType) { this.objectUrl = objectUrl; this.ePersonUuid = ePersonUuid; this.featureId = featureId; diff --git a/src/app/core/data/feature-authorization/feature-type.ts b/src/app/core/data/feature-authorization/feature-type.ts new file mode 100644 index 0000000000..a4cbc23d15 --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-type.ts @@ -0,0 +1,6 @@ +/** + * Enum object for all possible {@link Feature} types + */ +export enum FeatureType { + LoginOnBehalfOf = 'loginOnBehalfOf' +} diff --git a/src/app/core/eperson/eperson-data.service.ts b/src/app/core/eperson/eperson-data.service.ts index 86e53178a0..3dccbcb338 100644 --- a/src/app/core/eperson/eperson-data.service.ts +++ b/src/app/core/eperson/eperson-data.service.ts @@ -3,7 +3,7 @@ import { Injectable } from '@angular/core'; import { createSelector, select, Store } from '@ngrx/store'; import { Operation } from 'fast-json-patch/lib/core'; import { Observable } from 'rxjs'; -import { filter, map, take } from 'rxjs/operators'; +import { filter, find, map, skipWhile, switchMap, take, tap } from 'rxjs/operators'; import { EPeopleRegistryCancelEPersonAction, EPeopleRegistryEditEPersonAction @@ -249,4 +249,33 @@ export class EPersonDataService extends DataService { return '/admin/access-control/epeople'; } + /** + * Make a new FindListRequest with given search method + * + * @param searchMethod The search method for the object + * @param options The [[FindListOptions]] object + * @param linksToFollow The array of [[FollowLinkConfig]] + * @return {Observable>} + * Return an observable that emits response from the server + */ + searchBy(searchMethod: string, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { + const hrefObs = this.getSearchByHref(searchMethod, options, ...linksToFollow); + + return hrefObs.pipe( + find((href: string) => hasValue(href)), + tap((href: string) => { + this.requestService.removeByHrefSubstring(href); + const request = new FindListRequest(this.requestService.generateRequestId(), href, options); + + this.requestService.configure(request); + } + ), + switchMap((href) => this.requestService.getByHref(href)), + skipWhile((requestEntry) => hasValue(requestEntry) && requestEntry.completed), + switchMap((href) => + this.rdbService.buildList(hrefObs, ...linksToFollow) as Observable>> + ) + ); + } + } From bbbeddc875774d712c2df08a110aa7396eddab2a Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 18 Jun 2020 17:13:37 +0200 Subject: [PATCH 04/14] 71429: SiteAdministratorGuard on admin routes + authorization check on visibility of admin sidebar sections --- .../eperson-form/eperson-form.component.ts | 2 +- .../admin-sidebar/admin-sidebar.component.ts | 65 ++++++++++--------- src/app/app-routing.module.ts | 3 +- src/app/core/core.module.ts | 2 + .../authorization-data.service.ts | 6 +- .../authorization-utils.ts | 2 +- .../feature-authorization/feature-type.ts | 3 +- .../site-administrator.guard.ts | 31 +++++++++ 8 files changed, 77 insertions(+), 37 deletions(-) create mode 100644 src/app/core/data/feature-authorization/site-administrator.guard.ts diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts index e9e446ed34..6d1efd956c 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts @@ -245,7 +245,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy { }); })); this.canImpersonate$ = this.epersonService.getActiveEPerson().pipe( - switchMap((eperson) => this.authorizationService.isAuthenticated(eperson.self, undefined, FeatureType.LoginOnBehalfOf)) + switchMap((eperson) => this.authorizationService.isAuthenticated(FeatureType.LoginOnBehalfOf, eperson.self)) ); }); } diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index f11313fb6d..b009aacb33 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts @@ -2,7 +2,7 @@ import { Component, Injector, OnInit } from '@angular/core'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { combineLatest as combineLatestObservable } from 'rxjs'; import { Observable } from 'rxjs/internal/Observable'; -import { first, map } from 'rxjs/operators'; +import { first, map, take } from 'rxjs/operators'; import { AuthService } from '../../core/auth/auth.service'; import { slideHorizontal, slideSidebar } from '../../shared/animations/slide'; import { CreateCollectionParentSelectorComponent } from '../../shared/dso-selector/modal-wrappers/create-collection-parent-selector/create-collection-parent-selector.component'; @@ -18,6 +18,8 @@ import { TextMenuItemModel } from '../../shared/menu/menu-item/models/text.model import { MenuComponent } from '../../shared/menu/menu.component'; import { MenuService } from '../../shared/menu/menu.service'; import { CSSVariableService } from '../../shared/sass-helper/sass-helper.service'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; +import { FeatureType } from '../../core/data/feature-authorization/feature-type'; /** * Component representing the admin sidebar @@ -61,7 +63,8 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { protected injector: Injector, private variableService: CSSVariableService, private authService: AuthService, - private modalService: NgbModal + private modalService: NgbModal, + private authorizationService: AuthorizationDataService ) { super(menuService, injector); } @@ -70,30 +73,32 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { * Set and calculate all initial values of the instance variables */ ngOnInit(): void { - this.createMenu(); - super.ngOnInit(); - this.sidebarWidth = this.variableService.getVariable('sidebarItemsWidth'); - this.authService.isAuthenticated() - .subscribe((loggedIn: boolean) => { - if (loggedIn) { - this.menuService.showMenu(this.menuID); - } - }); - this.menuCollapsed.pipe(first()) - .subscribe((collapsed: boolean) => { - this.sidebarOpen = !collapsed; - this.sidebarClosed = collapsed; - }); - this.sidebarExpanded = combineLatestObservable(this.menuCollapsed, this.menuPreviewCollapsed) - .pipe( - map(([collapsed, previewCollapsed]) => (!collapsed || !previewCollapsed)) - ); + this.authorizationService.isAuthenticated(FeatureType.AdministratorOf).pipe(take(1)).subscribe((authorized) => { + this.createMenu(authorized); + super.ngOnInit(); + this.sidebarWidth = this.variableService.getVariable('sidebarItemsWidth'); + this.authService.isAuthenticated() + .subscribe((loggedIn: boolean) => { + if (loggedIn) { + this.menuService.showMenu(this.menuID); + } + }); + this.menuCollapsed.pipe(first()) + .subscribe((collapsed: boolean) => { + this.sidebarOpen = !collapsed; + this.sidebarClosed = collapsed; + }); + this.sidebarExpanded = combineLatestObservable(this.menuCollapsed, this.menuPreviewCollapsed) + .pipe( + map(([collapsed, previewCollapsed]) => (!collapsed || !previewCollapsed)) + ); + }); } /** * Initialize all menu sections and items for this menu */ - createMenu() { + createMenu(isAdministratorOfSite: boolean) { const menuList = [ /* News */ { @@ -309,7 +314,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { { id: 'access_control', active: false, - visible: true, + visible: isAdministratorOfSite, model: { type: MenuItemType.TEXT, text: 'menu.section.access_control' @@ -321,7 +326,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { id: 'access_control_people', parentID: 'access_control', active: false, - visible: true, + visible: isAdministratorOfSite, model: { type: MenuItemType.LINK, text: 'menu.section.access_control_people', @@ -332,7 +337,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { id: 'access_control_groups', parentID: 'access_control', active: false, - visible: true, + visible: isAdministratorOfSite, model: { type: MenuItemType.LINK, text: 'menu.section.access_control_groups', @@ -343,7 +348,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { id: 'access_control_authorizations', parentID: 'access_control', active: false, - visible: true, + visible: isAdministratorOfSite, model: { type: MenuItemType.LINK, text: 'menu.section.access_control_authorizations', @@ -354,7 +359,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { { id: 'admin_search', active: false, - visible: true, + visible: isAdministratorOfSite, model: { type: MenuItemType.LINK, text: 'menu.section.admin_search', @@ -367,7 +372,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { { id: 'registries', active: false, - visible: true, + visible: isAdministratorOfSite, model: { type: MenuItemType.TEXT, text: 'menu.section.registries' @@ -379,7 +384,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { id: 'registries_metadata', parentID: 'registries', active: false, - visible: true, + visible: isAdministratorOfSite, model: { type: MenuItemType.LINK, text: 'menu.section.registries_metadata', @@ -390,7 +395,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { id: 'registries_format', parentID: 'registries', active: false, - visible: true, + visible: isAdministratorOfSite, model: { type: MenuItemType.LINK, text: 'menu.section.registries_format', @@ -443,7 +448,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { { id: 'workflow', active: false, - visible: true, + visible: isAdministratorOfSite, model: { type: MenuItemType.LINK, text: 'menu.section.workflow', diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index 0ba0851e4e..0acec5e728 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -10,6 +10,7 @@ import { Collection } from './core/shared/collection.model'; import { Item } from './core/shared/item.model'; 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/site-administrator.guard'; const ITEM_MODULE_PATH = 'items'; @@ -82,7 +83,7 @@ export function getDSOPath(dso: DSpaceObject): string { }, { path: 'search', loadChildren: './+search-page/search-page-routing.module#SearchPageRoutingModule' }, { path: 'browse', loadChildren: './+browse-by/browse-by.module#BrowseByModule'}, - { path: ADMIN_MODULE_PATH, loadChildren: './+admin/admin.module#AdminModule', canActivate: [AuthenticatedGuard] }, + { path: ADMIN_MODULE_PATH, loadChildren: './+admin/admin.module#AdminModule', canActivate: [SiteAdministratorGuard] }, { path: 'login', loadChildren: './+login-page/login-page.module#LoginPageModule' }, { path: 'logout', loadChildren: './+logout-page/logout-page.module#LogoutPageModule' }, { path: 'submit', loadChildren: './+submit-page/submit-page.module#SubmitPageModule' }, diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index af394ea71a..eaa384d8a5 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -149,6 +149,7 @@ import { Feature } from './shared/feature.model'; import { Authorization } from './shared/authorization.model'; import { FeatureDataService } from './data/feature-authorization/feature-data.service'; import { AuthorizationDataService } from './data/feature-authorization/authorization-data.service'; +import { SiteAdministratorGuard } from './data/feature-authorization/site-administrator.guard'; /** * When not in production, endpoint responses can be mocked for testing purposes @@ -270,6 +271,7 @@ const PROVIDERS = [ WorkflowActionDataService, FeatureDataService, AuthorizationDataService, + SiteAdministratorGuard, // register AuthInterceptor as HttpInterceptor { provide: HTTP_INTERCEPTORS, 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 686ff94b19..94e49a149f 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -59,8 +59,8 @@ export class AuthorizationDataService extends DataService { * If not provided, the UUID of the currently authenticated {@link EPerson} will be used. * @param featureId ID of the {@link Feature} to check {@link Authorization} for */ - isAuthenticated(objectUrl?: string, ePersonUuid?: string, featureId?: FeatureType): Observable { - return this.searchByObject(objectUrl, ePersonUuid, featureId).pipe( + isAuthenticated(featureId?: FeatureType, objectUrl?: string, ePersonUuid?: string): Observable { + return this.searchByObject(featureId, objectUrl, ePersonUuid).pipe( map((authorizationRD) => (authorizationRD.statusCode !== 401 && hasValue(authorizationRD.payload) && isNotEmpty(authorizationRD.payload.page))) ); } @@ -76,7 +76,7 @@ export class AuthorizationDataService extends DataService { * @param options {@link FindListOptions} to provide pagination and/or additional arguments * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ - searchByObject(objectUrl?: string, ePersonUuid?: string, featureId?: FeatureType, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { + searchByObject(featureId?: FeatureType, objectUrl?: string, ePersonUuid?: string, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { return observableOf(new AuthorizationSearchParams(objectUrl, ePersonUuid, featureId)).pipe( addSiteObjectUrlIfEmpty(this.siteService), addAuthenticatedUserUuidIfEmpty(this.authService), diff --git a/src/app/core/data/feature-authorization/authorization-utils.ts b/src/app/core/data/feature-authorization/authorization-utils.ts index 6cc5a3d2ef..352d9f06cb 100644 --- a/src/app/core/data/feature-authorization/authorization-utils.ts +++ b/src/app/core/data/feature-authorization/authorization-utils.ts @@ -42,7 +42,7 @@ export const addAuthenticatedUserUuidIfEmpty = (authService: AuthService) => map((ePerson) => Object.assign({}, params, { ePersonUuid: ePerson.uuid })) ); } else { - observableOf(params) + return observableOf(params) } }) ); diff --git a/src/app/core/data/feature-authorization/feature-type.ts b/src/app/core/data/feature-authorization/feature-type.ts index a4cbc23d15..eaeaeca94f 100644 --- a/src/app/core/data/feature-authorization/feature-type.ts +++ b/src/app/core/data/feature-authorization/feature-type.ts @@ -2,5 +2,6 @@ * Enum object for all possible {@link Feature} types */ export enum FeatureType { - LoginOnBehalfOf = 'loginOnBehalfOf' + LoginOnBehalfOf = 'loginOnBehalfOf', + AdministratorOf = 'administratorOf' } diff --git a/src/app/core/data/feature-authorization/site-administrator.guard.ts b/src/app/core/data/feature-authorization/site-administrator.guard.ts new file mode 100644 index 0000000000..43208fba20 --- /dev/null +++ b/src/app/core/data/feature-authorization/site-administrator.guard.ts @@ -0,0 +1,31 @@ +import { Injectable } from '@angular/core'; +import { ActivatedRouteSnapshot, CanActivate, CanLoad, Route, RouterStateSnapshot, UrlSegment } from '@angular/router'; +import { Observable } from 'rxjs'; +import { AuthorizationDataService } from './authorization-data.service'; +import { FeatureType } from './feature-type'; + +/** + * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have administrator + * rights to the {@link Site} + */ +@Injectable({ + providedIn: 'root' +}) +export class SiteAdministratorGuard implements CanActivate, CanLoad { + constructor(private authorizationService: AuthorizationDataService) { + } + + /** + * True when user has administrator rights to the {@link Site} + */ + canActivate(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return this.authorizationService.isAuthenticated(FeatureType.AdministratorOf); + } + + /** + * True when user has administrator rights to the {@link Site} + */ + canLoad(route: Route, segments: UrlSegment[]): Observable { + return this.authorizationService.isAuthenticated(FeatureType.AdministratorOf); + } +} From 89afaaa4a53e6993236fb506d7f1dd2fa60ef6f0 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 18 Jun 2020 17:39:11 +0200 Subject: [PATCH 05/14] 71429: Abstract FeatureAuthorizationGuard + small fix --- .../admin-sidebar/admin-sidebar.component.ts | 2 +- src/app/app-routing.module.ts | 2 +- src/app/core/core.module.ts | 2 +- .../feature-authorization.guard.ts | 52 +++++++++++++++++++ .../site-administrator.guard.ts | 24 +++++++++ .../site-administrator.guard.ts | 31 ----------- 6 files changed, 79 insertions(+), 34 deletions(-) create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts delete mode 100644 src/app/core/data/feature-authorization/site-administrator.guard.ts diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index b009aacb33..d18b1ba90f 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts @@ -73,7 +73,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { * Set and calculate all initial values of the instance variables */ ngOnInit(): void { - this.authorizationService.isAuthenticated(FeatureType.AdministratorOf).pipe(take(1)).subscribe((authorized) => { + this.authorizationService.isAuthenticated(FeatureType.AdministratorOf).subscribe((authorized) => { this.createMenu(authorized); super.ngOnInit(); this.sidebarWidth = this.variableService.getVariable('sidebarItemsWidth'); diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index 0acec5e728..b10ae4df55 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -10,7 +10,7 @@ import { Collection } from './core/shared/collection.model'; import { Item } from './core/shared/item.model'; 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/site-administrator.guard'; +import { SiteAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; const ITEM_MODULE_PATH = 'items'; diff --git a/src/app/core/core.module.ts b/src/app/core/core.module.ts index eaa384d8a5..43babd0147 100644 --- a/src/app/core/core.module.ts +++ b/src/app/core/core.module.ts @@ -149,7 +149,7 @@ import { Feature } from './shared/feature.model'; import { Authorization } from './shared/authorization.model'; import { FeatureDataService } from './data/feature-authorization/feature-data.service'; import { AuthorizationDataService } from './data/feature-authorization/authorization-data.service'; -import { SiteAdministratorGuard } from './data/feature-authorization/site-administrator.guard'; +import { SiteAdministratorGuard } from './data/feature-authorization/feature-authorization-guard/site-administrator.guard'; /** * When not in production, endpoint responses can be mocked for testing purposes 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 new file mode 100644 index 0000000000..62576030a6 --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.ts @@ -0,0 +1,52 @@ +import { ActivatedRouteSnapshot, CanActivate, CanLoad, Route, RouterStateSnapshot, UrlSegment } from '@angular/router'; +import { AuthorizationDataService } from '../authorization-data.service'; +import { FeatureType } from '../feature-type'; +import { Observable } from 'rxjs/internal/Observable'; + +/** + * Abstract Guard for preventing unauthorized activating and loading of routes when a user + * doesn't have authorized rights on a specific feature and/or object. + * Override the desired getters in the parent class for checking specific authorization on a feature and/or object. + */ +export abstract class FeatureAuthorizationGuard implements CanActivate, CanLoad { + constructor(protected authorizationService: AuthorizationDataService) { + } + + /** + * True when user has authorization rights for the feature and object provided + */ + canActivate(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return this.authorizationService.isAuthenticated(this.getFeatureType(), this.getObjectUrl(), this.getEPersonUuid()); + } + + /** + * True when user has authorization rights for the feature and object provided + */ + canLoad(route: Route, segments: UrlSegment[]): Observable { + return this.authorizationService.isAuthenticated(this.getFeatureType(), this.getObjectUrl(), this.getEPersonUuid()); + } + + /** + * The type of feature to check authorization for + * Override this method to define a feature + */ + getFeatureType(): FeatureType { + return undefined; + } + + /** + * 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; + } + + /** + * 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; + } +} 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 new file mode 100644 index 0000000000..6034bc2f95 --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/site-administrator.guard.ts @@ -0,0 +1,24 @@ +import { Injectable } from '@angular/core'; +import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { FeatureType } from '../feature-type'; +import { AuthorizationDataService } from '../authorization-data.service'; + +/** + * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have administrator + * rights to the {@link Site} + */ +@Injectable({ + providedIn: 'root' +}) +export class SiteAdministratorGuard extends FeatureAuthorizationGuard { + constructor(protected authorizationService: AuthorizationDataService) { + super(authorizationService); + } + + /** + * Check administrator authorization rights + */ + getFeatureType(): FeatureType { + return FeatureType.AdministratorOf; + } +} diff --git a/src/app/core/data/feature-authorization/site-administrator.guard.ts b/src/app/core/data/feature-authorization/site-administrator.guard.ts deleted file mode 100644 index 43208fba20..0000000000 --- a/src/app/core/data/feature-authorization/site-administrator.guard.ts +++ /dev/null @@ -1,31 +0,0 @@ -import { Injectable } from '@angular/core'; -import { ActivatedRouteSnapshot, CanActivate, CanLoad, Route, RouterStateSnapshot, UrlSegment } from '@angular/router'; -import { Observable } from 'rxjs'; -import { AuthorizationDataService } from './authorization-data.service'; -import { FeatureType } from './feature-type'; - -/** - * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have administrator - * rights to the {@link Site} - */ -@Injectable({ - providedIn: 'root' -}) -export class SiteAdministratorGuard implements CanActivate, CanLoad { - constructor(private authorizationService: AuthorizationDataService) { - } - - /** - * True when user has administrator rights to the {@link Site} - */ - canActivate(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return this.authorizationService.isAuthenticated(FeatureType.AdministratorOf); - } - - /** - * True when user has administrator rights to the {@link Site} - */ - canLoad(route: Route, segments: UrlSegment[]): Observable { - return this.authorizationService.isAuthenticated(FeatureType.AdministratorOf); - } -} From 72381f40831c75d9a4f55aa76e266d6589f0844f Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Fri, 19 Jun 2020 15:55:25 +0200 Subject: [PATCH 06/14] 71429: Feature and Authorization tests --- .../eperson-form.component.spec.ts | 14 ++ .../eperson-form/eperson-form.component.ts | 2 +- .../admin-sidebar.component.spec.ts | 6 + .../authorization-data.service.spec.ts | 162 ++++++++++++++++++ .../feature-authorization.guard.spec.ts | 67 ++++++++ 5 files changed, 250 insertions(+), 1 deletion(-) create mode 100644 src/app/core/data/feature-authorization/authorization-data.service.spec.ts create mode 100644 src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts index 693f3cf916..51eb9d0690 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts @@ -33,6 +33,9 @@ import { NotificationsServiceStub } from '../../../../shared/testing/notificatio import { TranslateLoaderMock } from '../../../../shared/mocks/translate-loader.mock'; import { AuthService } from '../../../../core/auth/auth.service'; import { AuthServiceStub } from '../../../../shared/testing/auth-service.stub'; +import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; +import { GroupDataService } from '../../../../core/eperson/group-data.service'; +import { createPaginatedList } from '../../../../shared/testing/utils.test'; describe('EPersonFormComponent', () => { let component: EPersonFormComponent; @@ -43,6 +46,8 @@ describe('EPersonFormComponent', () => { let mockEPeople; let ePersonDataServiceStub: any; let authService: AuthServiceStub; + let authorizationService: AuthorizationDataService; + let groupsDataService: GroupDataService; beforeEach(async(() => { mockEPeople = [EPersonMock, EPersonMock2]; @@ -108,6 +113,13 @@ describe('EPersonFormComponent', () => { builderService = getMockFormBuilderService(); translateService = getMockTranslateService(); authService = new AuthServiceStub(); + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthenticated: observableOf(true) + }); + groupsDataService = jasmine.createSpyObj('groupsDataService', { + findAllByHref: createSuccessfulRemoteDataObject$(createPaginatedList([])), + getGroupRegistryRouterLink: '' + }); TestBed.configureTestingModule({ imports: [CommonModule, NgbModule, FormsModule, ReactiveFormsModule, BrowserModule, TranslateModule.forRoot({ @@ -130,6 +142,8 @@ describe('EPersonFormComponent', () => { { provide: RemoteDataBuildService, useValue: {} }, { provide: HALEndpointService, useValue: {} }, { provide: AuthService, useValue: authService }, + { provide: AuthorizationDataService, useValue: authorizationService }, + { provide: GroupDataService, useValue: groupsDataService }, EPeopleRegistryComponent ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts index 6d1efd956c..4a52125957 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts @@ -245,7 +245,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy { }); })); this.canImpersonate$ = this.epersonService.getActiveEPerson().pipe( - switchMap((eperson) => this.authorizationService.isAuthenticated(FeatureType.LoginOnBehalfOf, eperson.self)) + switchMap((eperson) => this.authorizationService.isAuthenticated(FeatureType.LoginOnBehalfOf, hasValue(eperson) ? eperson.self : undefined)) ); }); } diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts index e3e8d08753..fd76f0293e 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts @@ -15,13 +15,18 @@ import { By } from '@angular/platform-browser'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { RouterTestingModule } from '@angular/router/testing'; import { ActivatedRoute } from '@angular/router'; +import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; describe('AdminSidebarComponent', () => { let comp: AdminSidebarComponent; let fixture: ComponentFixture; const menuService = new MenuServiceStub(); + let authorizationService: AuthorizationDataService; beforeEach(async(() => { + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthenticated: observableOf(true) + }); TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule], declarations: [AdminSidebarComponent], @@ -31,6 +36,7 @@ describe('AdminSidebarComponent', () => { { provide: CSSVariableService, useClass: CSSVariableServiceStub }, { provide: AuthService, useClass: AuthServiceStub }, { provide: ActivatedRoute, useValue: {} }, + { provide: AuthorizationDataService, useValue: authorizationService }, { provide: NgbModal, useValue: { open: () => {/*comment*/} 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 new file mode 100644 index 0000000000..bd1fb20ac3 --- /dev/null +++ b/src/app/core/data/feature-authorization/authorization-data.service.spec.ts @@ -0,0 +1,162 @@ +import { AuthorizationDataService } from './authorization-data.service'; +import { SiteDataService } from '../site-data.service'; +import { AuthService } from '../../auth/auth.service'; +import { Site } from '../../shared/site.model'; +import { EPerson } from '../../eperson/models/eperson.model'; +import { of as observableOf } from 'rxjs'; +import { FindListOptions } from '../request.models'; +import { FeatureType } from './feature-type'; +import { hasValue } from '../../../shared/empty.util'; +import { RequestParam } from '../../cache/models/request-param.model'; +import { Authorization } from '../../shared/authorization.model'; +import { RemoteData } from '../remote-data'; +import { createSuccessfulRemoteDataObject$ } from '../../../shared/remote-data.utils'; +import { createPaginatedList } from '../../../shared/testing/utils.test'; + +describe('AuthorizationDataService', () => { + let service: AuthorizationDataService; + let siteService: SiteDataService; + let authService: AuthService; + + let site: Site; + let ePerson: EPerson; + + function init() { + site = Object.assign(new Site(), { + id: 'test-site', + _links: { + self: { href: 'test-site-href' } + } + }); + ePerson = Object.assign(new EPerson(), { + id: 'test-eperson', + uuid: 'test-eperson' + }); + siteService = jasmine.createSpyObj('siteService', { + find: observableOf(site) + }); + authService = { + isAuthenticated: () => observableOf(true), + getAuthenticatedUserFromStore: () => observableOf(ePerson) + } as AuthService; + service = new AuthorizationDataService(undefined, undefined, undefined, undefined, undefined, undefined, undefined, undefined, authService, siteService); + } + + beforeEach(() => { + init(); + spyOn(service, 'searchBy').and.returnValue(observableOf(undefined)); + }); + + describe('searchByObject', () => { + const objectUrl = 'fake-object-url'; + const ePersonUuid = 'fake-eperson-uuid'; + + function createExpected(objectUrl: string, ePersonUuid?: string, featureId?: FeatureType): FindListOptions { + const searchParams = [new RequestParam('uri', objectUrl)]; + if (hasValue(featureId)) { + searchParams.push(new RequestParam('feature', featureId)); + } + if (hasValue(ePersonUuid)) { + searchParams.push(new RequestParam('eperson', ePersonUuid)); + } + return Object.assign(new FindListOptions(), { searchParams }); + } + + describe('when no arguments are provided and a user is authenticated', () => { + 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)); + }); + }); + + describe('when no arguments except for a feature are provided and a user is authenticated', () => { + beforeEach(() => { + service.searchByObject(FeatureType.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, FeatureType.LoginOnBehalfOf)); + }); + }); + + describe('when a feature and object url are provided, but no user uuid and a user is authenticated', () => { + beforeEach(() => { + service.searchByObject(FeatureType.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, FeatureType.LoginOnBehalfOf)); + }); + }); + + describe('when all arguments are provided', () => { + beforeEach(() => { + service.searchByObject(FeatureType.LoginOnBehalfOf, objectUrl, ePersonUuid).subscribe(); + }); + + it('should call searchBy with the object\'s url, user\'s uuid and the feature', () => { + expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, ePersonUuid, FeatureType.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('isAuthenticated', () => { + const validPayload = [ + Object.assign(new Authorization()) + ] + const emptyPayload = []; + + describe('when searchByObject returns a 401', () => { + beforeEach(() => { + spyOn(service, 'searchByObject').and.returnValue(observableOf(new RemoteData(false, false, true, undefined, undefined, 401))); + }); + + it('should return false', (done) => { + service.isAuthenticated().subscribe((result) => { + expect(result).toEqual(false); + done(); + }); + }); + }); + + describe('when searchByObject returns an empty list', () => { + beforeEach(() => { + spyOn(service, 'searchByObject').and.returnValue(createSuccessfulRemoteDataObject$(createPaginatedList(emptyPayload))); + }); + + it('should return false', (done) => { + service.isAuthenticated().subscribe((result) => { + expect(result).toEqual(false); + done(); + }); + }); + }); + + describe('when searchByObject returns a valid list', () => { + beforeEach(() => { + spyOn(service, 'searchByObject').and.returnValue(createSuccessfulRemoteDataObject$(createPaginatedList(validPayload))); + }); + + it('should return true', (done) => { + service.isAuthenticated().subscribe((result) => { + expect(result).toEqual(true); + done(); + }); + }); + }); + }); +}); 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 new file mode 100644 index 0000000000..5af0f42bf7 --- /dev/null +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/feature-authorization.guard.spec.ts @@ -0,0 +1,67 @@ +import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { AuthorizationDataService } from '../authorization-data.service'; +import { FeatureType } from '../feature-type'; +import { of as observableOf } from 'rxjs'; + +/** + * Test implementation of abstract class FeatureAuthorizationGuard + * Provide the return values of the overwritten getters as constructor arguments + */ +class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard { + constructor(protected authorizationService: AuthorizationDataService, + protected featureType: FeatureType, + protected objectUrl: string, + protected ePersonUuid: string) { + super(authorizationService); + } + + getFeatureType(): FeatureType { + return this.featureType; + } + + getObjectUrl(): string { + return this.objectUrl; + } + + getEPersonUuid(): string { + return this.ePersonUuid; + } +} + +describe('FeatureAuthorizationGuard', () => { + let guard: FeatureAuthorizationGuard; + let authorizationService: AuthorizationDataService; + + let featureType: FeatureType; + let objectUrl: string; + let ePersonUuid: string; + + function init() { + featureType = FeatureType.LoginOnBehalfOf; + objectUrl = 'fake-object-url'; + ePersonUuid = 'fake-eperson-uuid'; + + authorizationService = jasmine.createSpyObj('authorizationService', { + isAuthenticated: observableOf(true) + }); + guard = new FeatureAuthorizationGuardImpl(authorizationService, featureType, objectUrl, ePersonUuid); + } + + beforeEach(() => { + init(); + }); + + describe('canActivate', () => { + it('should call authorizationService.isAuthenticated with the appropriate arguments', () => { + guard.canActivate(undefined, undefined).subscribe(); + expect(authorizationService.isAuthenticated).toHaveBeenCalledWith(featureType, objectUrl, ePersonUuid); + }); + }); + + describe('canLoad', () => { + it('should call authorizationService.isAuthenticated with the appropriate arguments', () => { + guard.canLoad(undefined, undefined).subscribe(); + expect(authorizationService.isAuthenticated).toHaveBeenCalledWith(featureType, objectUrl, ePersonUuid); + }); + }); +}); From 0cef62ff480f07d2c7f590ab618f8f6abf3a27b8 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 24 Jun 2020 13:05:58 +0200 Subject: [PATCH 07/14] 71429: Feedback 2020-06-23 --- .../eperson-form/eperson-form.component.ts | 4 +- .../admin-sidebar/admin-sidebar.component.ts | 268 +++++++++--------- .../authorization-data.service.spec.ts | 16 +- .../authorization-data.service.ts | 10 +- .../authorization-search-params.ts | 9 +- .../feature-authorization.guard.spec.ts | 18 +- .../feature-authorization.guard.ts | 10 +- .../site-administrator.guard.ts | 6 +- .../{feature-type.ts => feature-id.ts} | 4 +- src/app/core/shared/feature.model.ts | 8 +- 10 files changed, 185 insertions(+), 168 deletions(-) rename src/app/core/data/feature-authorization/{feature-type.ts => feature-id.ts} (52%) diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts index 4a52125957..41f360d0a7 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts @@ -24,7 +24,7 @@ import { NotificationsService } from '../../../../shared/notifications/notificat import { PaginationComponentOptions } from '../../../../shared/pagination/pagination-component-options.model'; import { AuthService } from '../../../../core/auth/auth.service'; import { AuthorizationDataService } from '../../../../core/data/feature-authorization/authorization-data.service'; -import { FeatureType } from '../../../../core/data/feature-authorization/feature-type'; +import { FeatureID } from '../../../../core/data/feature-authorization/feature-id'; @Component({ selector: 'ds-eperson-form', @@ -245,7 +245,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy { }); })); this.canImpersonate$ = this.epersonService.getActiveEPerson().pipe( - switchMap((eperson) => this.authorizationService.isAuthenticated(FeatureType.LoginOnBehalfOf, hasValue(eperson) ? eperson.self : undefined)) + switchMap((eperson) => this.authorizationService.isAuthenticated(FeatureID.LoginOnBehalfOf, hasValue(eperson) ? eperson.self : undefined)) ); }); } diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index d18b1ba90f..30cbf89aaf 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts @@ -19,7 +19,7 @@ import { MenuComponent } from '../../shared/menu/menu.component'; import { MenuService } from '../../shared/menu/menu.service'; import { CSSVariableService } from '../../shared/sass-helper/sass-helper.service'; import { AuthorizationDataService } from '../../core/data/feature-authorization/authorization-data.service'; -import { FeatureType } from '../../core/data/feature-authorization/feature-type'; +import { FeatureID } from '../../core/data/feature-authorization/feature-id'; /** * Component representing the admin sidebar @@ -73,32 +73,31 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { * Set and calculate all initial values of the instance variables */ ngOnInit(): void { - this.authorizationService.isAuthenticated(FeatureType.AdministratorOf).subscribe((authorized) => { - this.createMenu(authorized); - super.ngOnInit(); - this.sidebarWidth = this.variableService.getVariable('sidebarItemsWidth'); - this.authService.isAuthenticated() - .subscribe((loggedIn: boolean) => { - if (loggedIn) { - this.menuService.showMenu(this.menuID); - } - }); - this.menuCollapsed.pipe(first()) - .subscribe((collapsed: boolean) => { - this.sidebarOpen = !collapsed; - this.sidebarClosed = collapsed; - }); - this.sidebarExpanded = combineLatestObservable(this.menuCollapsed, this.menuPreviewCollapsed) - .pipe( - map(([collapsed, previewCollapsed]) => (!collapsed || !previewCollapsed)) - ); - }); + this.createMenu(); + this.createSiteAdministratorMenuSections(); + super.ngOnInit(); + this.sidebarWidth = this.variableService.getVariable('sidebarItemsWidth'); + this.authService.isAuthenticated() + .subscribe((loggedIn: boolean) => { + if (loggedIn) { + this.menuService.showMenu(this.menuID); + } + }); + this.menuCollapsed.pipe(first()) + .subscribe((collapsed: boolean) => { + this.sidebarOpen = !collapsed; + this.sidebarClosed = collapsed; + }); + this.sidebarExpanded = combineLatestObservable(this.menuCollapsed, this.menuPreviewCollapsed) + .pipe( + map(([collapsed, previewCollapsed]) => (!collapsed || !previewCollapsed)) + ); } /** * Initialize all menu sections and items for this menu */ - createMenu(isAdministratorOfSite: boolean) { + createMenu() { const menuList = [ /* News */ { @@ -310,99 +309,6 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { } as LinkMenuItemModel, }, - /* Access Control */ - { - id: 'access_control', - active: false, - visible: isAdministratorOfSite, - model: { - type: MenuItemType.TEXT, - text: 'menu.section.access_control' - } as TextMenuItemModel, - icon: 'key', - index: 4 - }, - { - id: 'access_control_people', - parentID: 'access_control', - active: false, - visible: isAdministratorOfSite, - model: { - type: MenuItemType.LINK, - text: 'menu.section.access_control_people', - link: '/admin/access-control/epeople' - } as LinkMenuItemModel, - }, - { - id: 'access_control_groups', - parentID: 'access_control', - active: false, - visible: isAdministratorOfSite, - model: { - type: MenuItemType.LINK, - text: 'menu.section.access_control_groups', - link: '/admin/access-control/groups' - } as LinkMenuItemModel, - }, - { - id: 'access_control_authorizations', - parentID: 'access_control', - active: false, - visible: isAdministratorOfSite, - model: { - type: MenuItemType.LINK, - text: 'menu.section.access_control_authorizations', - link: '' - } as LinkMenuItemModel, - }, - /* Admin Search */ - { - id: 'admin_search', - active: false, - visible: isAdministratorOfSite, - model: { - type: MenuItemType.LINK, - text: 'menu.section.admin_search', - link: '/admin/search' - } as LinkMenuItemModel, - icon: 'search', - index: 5 - }, - /* Registries */ - { - id: 'registries', - active: false, - visible: isAdministratorOfSite, - model: { - type: MenuItemType.TEXT, - text: 'menu.section.registries' - } as TextMenuItemModel, - icon: 'list', - index: 6 - }, - { - id: 'registries_metadata', - parentID: 'registries', - active: false, - visible: isAdministratorOfSite, - model: { - type: MenuItemType.LINK, - text: 'menu.section.registries_metadata', - link: 'admin/registries/metadata' - } as LinkMenuItemModel, - }, - { - id: 'registries_format', - parentID: 'registries', - active: false, - visible: isAdministratorOfSite, - model: { - type: MenuItemType.LINK, - text: 'menu.section.registries_format', - link: 'admin/registries/bitstream-formats' - } as LinkMenuItemModel, - }, - /* Curation tasks */ { id: 'curation_tasks', @@ -444,24 +350,130 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { icon: 'cogs', index: 9 }, - /* Workflow */ - { - id: 'workflow', - active: false, - visible: isAdministratorOfSite, - model: { - type: MenuItemType.LINK, - text: 'menu.section.workflow', - link: '/admin/workflow' - } as LinkMenuItemModel, - icon: 'user-check', - index: 10 - }, ]; menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, Object.assign(menuSection, { shouldPersistOnRouteChange: true }))); + } + /** + * Create menu sections dependent on whether or not the current user is a site administrator + */ + createSiteAdministratorMenuSections() { + this.authorizationService.isAuthenticated(FeatureID.AdministratorOf).subscribe((authorized) => { + const menuList = [ + /* Access Control */ + { + id: 'access_control', + active: false, + visible: authorized, + model: { + type: MenuItemType.TEXT, + text: 'menu.section.access_control' + } as TextMenuItemModel, + icon: 'key', + index: 4 + }, + { + id: 'access_control_people', + parentID: 'access_control', + active: false, + visible: authorized, + model: { + type: MenuItemType.LINK, + text: 'menu.section.access_control_people', + link: '/admin/access-control/epeople' + } as LinkMenuItemModel, + }, + { + id: 'access_control_groups', + parentID: 'access_control', + active: false, + visible: authorized, + model: { + type: MenuItemType.LINK, + text: 'menu.section.access_control_groups', + link: '/admin/access-control/groups' + } as LinkMenuItemModel, + }, + { + id: 'access_control_authorizations', + parentID: 'access_control', + active: false, + visible: authorized, + model: { + type: MenuItemType.LINK, + text: 'menu.section.access_control_authorizations', + link: '' + } as LinkMenuItemModel, + }, + /* Admin Search */ + { + id: 'admin_search', + active: false, + visible: authorized, + model: { + type: MenuItemType.LINK, + text: 'menu.section.admin_search', + link: '/admin/search' + } as LinkMenuItemModel, + icon: 'search', + index: 5 + }, + /* Registries */ + { + id: 'registries', + active: false, + visible: authorized, + model: { + type: MenuItemType.TEXT, + text: 'menu.section.registries' + } as TextMenuItemModel, + icon: 'list', + index: 6 + }, + { + id: 'registries_metadata', + parentID: 'registries', + active: false, + visible: authorized, + model: { + type: MenuItemType.LINK, + text: 'menu.section.registries_metadata', + link: 'admin/registries/metadata' + } as LinkMenuItemModel, + }, + { + id: 'registries_format', + parentID: 'registries', + active: false, + visible: authorized, + model: { + type: MenuItemType.LINK, + text: 'menu.section.registries_format', + link: 'admin/registries/bitstream-formats' + } as LinkMenuItemModel, + }, + + /* Workflow */ + { + id: 'workflow', + active: false, + visible: authorized, + model: { + type: MenuItemType.LINK, + text: 'menu.section.workflow', + link: '/admin/workflow' + } as LinkMenuItemModel, + icon: 'user-check', + index: 10 + }, + ]; + + menuList.forEach((menuSection) => this.menuService.addSection(this.menuID, Object.assign(menuSection, { + shouldPersistOnRouteChange: true + }))); + }); } /** 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 bd1fb20ac3..9bb6a5b17e 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 @@ -5,7 +5,7 @@ import { Site } from '../../shared/site.model'; import { EPerson } from '../../eperson/models/eperson.model'; import { of as observableOf } from 'rxjs'; import { FindListOptions } from '../request.models'; -import { FeatureType } from './feature-type'; +import { FeatureID } from './feature-id'; import { hasValue } from '../../../shared/empty.util'; import { RequestParam } from '../../cache/models/request-param.model'; import { Authorization } from '../../shared/authorization.model'; @@ -51,7 +51,7 @@ describe('AuthorizationDataService', () => { const objectUrl = 'fake-object-url'; const ePersonUuid = 'fake-eperson-uuid'; - function createExpected(objectUrl: string, ePersonUuid?: string, featureId?: FeatureType): FindListOptions { + function createExpected(objectUrl: string, ePersonUuid?: string, featureId?: FeatureID): FindListOptions { const searchParams = [new RequestParam('uri', objectUrl)]; if (hasValue(featureId)) { searchParams.push(new RequestParam('feature', featureId)); @@ -74,31 +74,31 @@ describe('AuthorizationDataService', () => { describe('when no arguments except for a feature are provided and a user is authenticated', () => { beforeEach(() => { - service.searchByObject(FeatureType.LoginOnBehalfOf).subscribe(); + 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, FeatureType.LoginOnBehalfOf)); + expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(site.self, ePerson.uuid, FeatureID.LoginOnBehalfOf)); }); }); describe('when a feature and object url are provided, but no user uuid and a user is authenticated', () => { beforeEach(() => { - service.searchByObject(FeatureType.LoginOnBehalfOf, objectUrl).subscribe(); + 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, FeatureType.LoginOnBehalfOf)); + expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, ePerson.uuid, FeatureID.LoginOnBehalfOf)); }); }); describe('when all arguments are provided', () => { beforeEach(() => { - service.searchByObject(FeatureType.LoginOnBehalfOf, objectUrl, ePersonUuid).subscribe(); + service.searchByObject(FeatureID.LoginOnBehalfOf, objectUrl, ePersonUuid).subscribe(); }); it('should call searchBy with the object\'s url, user\'s uuid and the feature', () => { - expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, ePersonUuid, FeatureType.LoginOnBehalfOf)); + expect(service.searchBy).toHaveBeenCalledWith('object', createExpected(objectUrl, ePersonUuid, FeatureID.LoginOnBehalfOf)); }); }); 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 94e49a149f..f997ee250d 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,7 @@ import { hasValue, isNotEmpty } from '../../../shared/empty.util'; import { RequestParam } from '../../cache/models/request-param.model'; import { AuthorizationSearchParams } from './authorization-search-params'; import { addAuthenticatedUserUuidIfEmpty, addSiteObjectUrlIfEmpty } from './authorization-utils'; -import { FeatureType } from './feature-type'; +import { FeatureID } from './feature-id'; /** * A service to retrieve {@link Authorization}s from the REST API @@ -59,7 +59,7 @@ export class AuthorizationDataService extends DataService { * If not provided, the UUID of the currently authenticated {@link EPerson} will be used. * @param featureId ID of the {@link Feature} to check {@link Authorization} for */ - isAuthenticated(featureId?: FeatureType, objectUrl?: string, ePersonUuid?: string): Observable { + isAuthenticated(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string): Observable { return this.searchByObject(featureId, objectUrl, ePersonUuid).pipe( map((authorizationRD) => (authorizationRD.statusCode !== 401 && hasValue(authorizationRD.payload) && isNotEmpty(authorizationRD.payload.page))) ); @@ -76,7 +76,7 @@ export class AuthorizationDataService extends DataService { * @param options {@link FindListOptions} to provide pagination and/or additional arguments * @param linksToFollow List of {@link FollowLinkConfig} that indicate which {@link HALLink}s should be automatically resolved */ - searchByObject(featureId?: FeatureType, objectUrl?: string, ePersonUuid?: string, options: FindListOptions = {}, ...linksToFollow: Array>): Observable>> { + 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), @@ -101,14 +101,12 @@ export class AuthorizationDataService extends DataService { return hrefObs.pipe( find((href: string) => hasValue(href)), tap((href: string) => { - this.requestService.removeByHrefSubstring(href); const request = new FindListRequest(this.requestService.generateRequestId(), href, options); this.requestService.configure(request); } ), switchMap((href) => this.requestService.getByHref(href)), - skipWhile((requestEntry) => hasValue(requestEntry) && requestEntry.completed), switchMap((href) => this.rdbService.buildList(hrefObs, ...linksToFollow) as Observable>> ) @@ -122,7 +120,7 @@ export class AuthorizationDataService extends DataService { * @param ePersonUuid Optional parameter value to add to {@link RequestParam} "eperson" * @param featureId Optional parameter value to add to {@link RequestParam} "feature" */ - private createSearchOptions(objectUrl: string, options: FindListOptions = {}, ePersonUuid?: string, featureId?: FeatureType): FindListOptions { + private createSearchOptions(objectUrl: string, options: FindListOptions = {}, ePersonUuid?: string, featureId?: FeatureID): FindListOptions { let params = []; if (isNotEmpty(options.searchParams)) { params = [...options.searchParams]; diff --git a/src/app/core/data/feature-authorization/authorization-search-params.ts b/src/app/core/data/feature-authorization/authorization-search-params.ts index dc045add84..3c281804fb 100644 --- a/src/app/core/data/feature-authorization/authorization-search-params.ts +++ b/src/app/core/data/feature-authorization/authorization-search-params.ts @@ -1,11 +1,14 @@ -import { FeatureType } from './feature-type'; +import { FeatureID } from './feature-id'; +/** + * Search parameters for retrieving authorizations from the REST API + */ export class AuthorizationSearchParams { objectUrl: string; ePersonUuid: string; - featureId: FeatureType; + featureId: FeatureID; - constructor(objectUrl?: string, ePersonUuid?: string, featureId?: FeatureType) { + constructor(objectUrl?: string, ePersonUuid?: string, featureId?: FeatureID) { this.objectUrl = objectUrl; this.ePersonUuid = ePersonUuid; this.featureId = featureId; 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 5af0f42bf7..fca665862f 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 @@ -1,6 +1,6 @@ import { FeatureAuthorizationGuard } from './feature-authorization.guard'; import { AuthorizationDataService } from '../authorization-data.service'; -import { FeatureType } from '../feature-type'; +import { FeatureID } from '../feature-id'; import { of as observableOf } from 'rxjs'; /** @@ -9,14 +9,14 @@ import { of as observableOf } from 'rxjs'; */ class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, - protected featureType: FeatureType, + protected featureId: FeatureID, protected objectUrl: string, protected ePersonUuid: string) { super(authorizationService); } - getFeatureType(): FeatureType { - return this.featureType; + getFeatureID(): FeatureID { + return this.featureId; } getObjectUrl(): string { @@ -32,19 +32,19 @@ describe('FeatureAuthorizationGuard', () => { let guard: FeatureAuthorizationGuard; let authorizationService: AuthorizationDataService; - let featureType: FeatureType; + let featureId: FeatureID; let objectUrl: string; let ePersonUuid: string; function init() { - featureType = FeatureType.LoginOnBehalfOf; + featureId = FeatureID.LoginOnBehalfOf; objectUrl = 'fake-object-url'; ePersonUuid = 'fake-eperson-uuid'; authorizationService = jasmine.createSpyObj('authorizationService', { isAuthenticated: observableOf(true) }); - guard = new FeatureAuthorizationGuardImpl(authorizationService, featureType, objectUrl, ePersonUuid); + guard = new FeatureAuthorizationGuardImpl(authorizationService, featureId, objectUrl, ePersonUuid); } beforeEach(() => { @@ -54,14 +54,14 @@ describe('FeatureAuthorizationGuard', () => { describe('canActivate', () => { it('should call authorizationService.isAuthenticated with the appropriate arguments', () => { guard.canActivate(undefined, undefined).subscribe(); - expect(authorizationService.isAuthenticated).toHaveBeenCalledWith(featureType, objectUrl, ePersonUuid); + expect(authorizationService.isAuthenticated).toHaveBeenCalledWith(featureId, objectUrl, ePersonUuid); }); }); describe('canLoad', () => { it('should call authorizationService.isAuthenticated with the appropriate arguments', () => { guard.canLoad(undefined, undefined).subscribe(); - expect(authorizationService.isAuthenticated).toHaveBeenCalledWith(featureType, objectUrl, ePersonUuid); + expect(authorizationService.isAuthenticated).toHaveBeenCalledWith(featureId, objectUrl, 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 62576030a6..bbee3abff2 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 @@ -1,6 +1,6 @@ import { ActivatedRouteSnapshot, CanActivate, CanLoad, Route, RouterStateSnapshot, UrlSegment } from '@angular/router'; import { AuthorizationDataService } from '../authorization-data.service'; -import { FeatureType } from '../feature-type'; +import { FeatureID } from '../feature-id'; import { Observable } from 'rxjs/internal/Observable'; /** @@ -16,23 +16,21 @@ export abstract class FeatureAuthorizationGuard implements CanActivate, CanLoad * True when user has authorization rights for the feature and object provided */ canActivate(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { - return this.authorizationService.isAuthenticated(this.getFeatureType(), this.getObjectUrl(), this.getEPersonUuid()); + return this.authorizationService.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()); } /** * True when user has authorization rights for the feature and object provided */ canLoad(route: Route, segments: UrlSegment[]): Observable { - return this.authorizationService.isAuthenticated(this.getFeatureType(), this.getObjectUrl(), this.getEPersonUuid()); + return this.authorizationService.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()); } /** * The type of feature to check authorization for * Override this method to define a feature */ - getFeatureType(): FeatureType { - return undefined; - } + abstract getFeatureID(): FeatureID; /** * The URL of the object to check if the user has authorized rights for 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 6034bc2f95..91f47fa19c 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 @@ -1,6 +1,6 @@ import { Injectable } from '@angular/core'; import { FeatureAuthorizationGuard } from './feature-authorization.guard'; -import { FeatureType } from '../feature-type'; +import { FeatureID } from '../feature-id'; import { AuthorizationDataService } from '../authorization-data.service'; /** @@ -18,7 +18,7 @@ export class SiteAdministratorGuard extends FeatureAuthorizationGuard { /** * Check administrator authorization rights */ - getFeatureType(): FeatureType { - return FeatureType.AdministratorOf; + getFeatureID(): FeatureID { + return FeatureID.AdministratorOf; } } diff --git a/src/app/core/data/feature-authorization/feature-type.ts b/src/app/core/data/feature-authorization/feature-id.ts similarity index 52% rename from src/app/core/data/feature-authorization/feature-type.ts rename to src/app/core/data/feature-authorization/feature-id.ts index eaeaeca94f..4731e92d6c 100644 --- a/src/app/core/data/feature-authorization/feature-type.ts +++ b/src/app/core/data/feature-authorization/feature-id.ts @@ -1,7 +1,7 @@ /** - * Enum object for all possible {@link Feature} types + * Enum object for all possible {@link Feature} IDs */ -export enum FeatureType { +export enum FeatureID { LoginOnBehalfOf = 'loginOnBehalfOf', AdministratorOf = 'administratorOf' } diff --git a/src/app/core/shared/feature.model.ts b/src/app/core/shared/feature.model.ts index cc0fd2db87..2ba5d8d4e1 100644 --- a/src/app/core/shared/feature.model.ts +++ b/src/app/core/shared/feature.model.ts @@ -1,7 +1,8 @@ import { typedObject } from '../cache/builders/build-decorators'; -import { autoserialize, inheritSerialization } from 'cerialize'; +import { autoserialize, deserialize, inheritSerialization } from 'cerialize'; import { FEATURE } from './feature.resource-type'; import { DSpaceObject } from './dspace-object.model'; +import { HALLink } from './hal-link.model'; /** * Class representing a DSpace Feature @@ -28,4 +29,9 @@ export class Feature extends DSpaceObject { */ @autoserialize resourcetypes: string[]; + + @deserialize + _links: { + self: HALLink; + }; } From 78a5bd5fce289e2e95ec09eb46a8bfdd10577c84 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 24 Jun 2020 14:22:28 +0200 Subject: [PATCH 08/14] 71429: Unauthorized component --- src/app/app-routing.module.ts | 2 ++ src/app/app.module.ts | 2 ++ .../authorization-data.service.ts | 1 + .../feature-authorization.guard.ts | 20 +++++++++--- .../site-administrator.guard.ts | 5 +-- .../core/services/server-response.service.ts | 4 +++ src/app/core/shared/operators.ts | 13 ++++++++ .../unauthorized/unauthorized.component.html | 10 ++++++ .../unauthorized/unauthorized.component.scss | 0 .../unauthorized/unauthorized.component.ts | 32 +++++++++++++++++++ src/assets/i18n/en.json5 | 8 +++++ 11 files changed, 91 insertions(+), 6 deletions(-) create mode 100644 src/app/unauthorized/unauthorized.component.html create mode 100644 src/app/unauthorized/unauthorized.component.scss create mode 100644 src/app/unauthorized/unauthorized.component.ts diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index b10ae4df55..9694ed42b0 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -11,6 +11,7 @@ import { Item } from './core/shared/item.model'; 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'; const ITEM_MODULE_PATH = 'items'; @@ -99,6 +100,7 @@ export function getDSOPath(dso: DSpaceObject): string { path: PROFILE_MODULE_PATH, loadChildren: './profile-page/profile-page.module#ProfilePageModule', canActivate: [AuthenticatedGuard] }, + { path: '401', component: UnauthorizedComponent }, { path: '**', pathMatch: 'full', component: PageNotFoundComponent }, ], { diff --git a/src/app/app.module.ts b/src/app/app.module.ts index ced019a6f9..33454ed6c5 100755 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -40,6 +40,7 @@ import { SharedModule } from './shared/shared.module'; import { BreadcrumbsComponent } from './breadcrumbs/breadcrumbs.component'; import { environment } from '../environments/environment'; import { BrowserModule } from '@angular/platform-browser'; +import { UnauthorizedComponent } from './unauthorized/unauthorized.component'; export function getBase() { return environment.ui.nameSpace; @@ -123,6 +124,7 @@ const EXPORTS = [ declarations: [ ...DECLARATIONS, BreadcrumbsComponent, + UnauthorizedComponent, ], exports: [ ...EXPORTS 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 f997ee250d..92a56e7bcf 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -26,6 +26,7 @@ import { RequestParam } from '../../cache/models/request-param.model'; import { AuthorizationSearchParams } from './authorization-search-params'; import { addAuthenticatedUserUuidIfEmpty, addSiteObjectUrlIfEmpty } from './authorization-utils'; import { FeatureID } from './feature-id'; +import { of } from 'rxjs/internal/observable/of'; /** * A service to retrieve {@link Authorization}s from the REST API 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 bbee3abff2..708b197441 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 @@ -1,7 +1,16 @@ -import { ActivatedRouteSnapshot, CanActivate, CanLoad, Route, RouterStateSnapshot, UrlSegment } from '@angular/router'; +import { + ActivatedRouteSnapshot, + CanActivate, + CanLoad, + Route, + Router, + RouterStateSnapshot, + UrlSegment +} from '@angular/router'; import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; import { Observable } from 'rxjs/internal/Observable'; +import { redirectToUnauthorizedOnFalse } from '../../../shared/operators'; /** * Abstract Guard for preventing unauthorized activating and loading of routes when a user @@ -9,21 +18,24 @@ import { Observable } from 'rxjs/internal/Observable'; * Override the desired getters in the parent class for checking specific authorization on a feature and/or object. */ export abstract class FeatureAuthorizationGuard implements CanActivate, CanLoad { - constructor(protected authorizationService: AuthorizationDataService) { + constructor(protected authorizationService: AuthorizationDataService, + protected router: Router) { } /** * 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.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()); + return this.authorizationService.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()).pipe(redirectToUnauthorizedOnFalse(this.router)); } /** * 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 */ canLoad(route: Route, segments: UrlSegment[]): Observable { - return this.authorizationService.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()); + return this.authorizationService.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()).pipe(redirectToUnauthorizedOnFalse(this.router)); } /** 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 91f47fa19c..a64e40468d 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,6 +2,7 @@ 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'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have administrator @@ -11,8 +12,8 @@ import { AuthorizationDataService } from '../authorization-data.service'; providedIn: 'root' }) export class SiteAdministratorGuard extends FeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService) { - super(authorizationService); + constructor(protected authorizationService: AuthorizationDataService, protected router: Router) { + super(authorizationService, router); } /** diff --git a/src/app/core/services/server-response.service.ts b/src/app/core/services/server-response.service.ts index f143c56ffb..10da2a3379 100644 --- a/src/app/core/services/server-response.service.ts +++ b/src/app/core/services/server-response.service.ts @@ -20,6 +20,10 @@ export class ServerResponseService { return this; } + setUnauthorized(message = 'Unauthorized'): this { + return this.setStatus(401, message) + } + setNotFound(message = 'Not found'): this { return this.setStatus(404, message) } diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index a307b144d2..ca1394005e 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -180,6 +180,19 @@ export const redirectToPageNotFoundOn404 = (router: Router) => } })); +/** + * Operator that redirects the user to the unauthorized page when the boolean received is false + * @param router + */ +export const redirectToUnauthorizedOnFalse = (router: Router) => + (source: Observable): Observable => + source.pipe( + tap((authorized: boolean) => { + if (!authorized) { + router.navigateByUrl('/401', { skipLocationChange: true }); + } + })); + export const getFinishedRemoteData = () => (source: Observable>): Observable> => source.pipe(find((rd: RemoteData) => !rd.isLoading)); diff --git a/src/app/unauthorized/unauthorized.component.html b/src/app/unauthorized/unauthorized.component.html new file mode 100644 index 0000000000..a25e271b77 --- /dev/null +++ b/src/app/unauthorized/unauthorized.component.html @@ -0,0 +1,10 @@ +
+

401

+

{{"401.unauthorized" | translate}}

+
+

{{"401.help" | translate}}

+
+

+ {{"401.link.home-page" | translate}} +

+
diff --git a/src/app/unauthorized/unauthorized.component.scss b/src/app/unauthorized/unauthorized.component.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/app/unauthorized/unauthorized.component.ts b/src/app/unauthorized/unauthorized.component.ts new file mode 100644 index 0000000000..280a1ae947 --- /dev/null +++ b/src/app/unauthorized/unauthorized.component.ts @@ -0,0 +1,32 @@ +import { Component, OnInit } from '@angular/core'; +import { AuthService } from '../core/auth/auth.service'; +import { ServerResponseService } from '../core/services/server-response.service'; + +/** + * This component representing the `Unauthorized` DSpace page. + */ +@Component({ + selector: 'ds-unauthorized', + templateUrl: './unauthorized.component.html', + styleUrls: ['./unauthorized.component.scss'] +}) +export class UnauthorizedComponent implements OnInit { + + /** + * Initialize instance variables + * + * @param {AuthService} authservice + * @param {ServerResponseService} responseService + */ + constructor(private authservice: AuthService, private responseService: ServerResponseService) { + this.responseService.setUnauthorized(); + } + + /** + * Remove redirect url from the state + */ + ngOnInit(): void { + this.authservice.clearRedirectUrl(); + } + +} diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index 4173fa1cf2..08a411fcf8 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -1,5 +1,13 @@ { + "401.help": "You're not authorized to access this page. You can use the button below to get back to the home page.", + + "401.link.home-page": "Take me to the home page", + + "401.unauthorized": "unauthorized", + + + "404.help": "We can't find the page you're looking for. The page may have been moved or deleted. You can use the button below to get back to the home page. ", "404.link.home-page": "Take me to the home page", From 080ddf8a1fabdffb392895e27436cca6a6b0c216 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 24 Jun 2020 15:05:24 +0200 Subject: [PATCH 09/14] 71429: Unauthorized page fixes --- .../eperson-form/eperson-form.component.ts | 2 +- .../admin-sidebar/admin-sidebar.component.ts | 2 +- src/app/app-routing.module.ts | 8 +++++++- .../authorization-data.service.spec.ts | 20 +++++++++---------- .../authorization-data.service.ts | 2 +- .../feature-authorization.guard.spec.ts | 19 +++++++++--------- .../feature-authorization.guard.ts | 20 +++++-------------- src/app/core/shared/operators.ts | 15 +++++++------- 8 files changed, 41 insertions(+), 47 deletions(-) diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts index 41f360d0a7..6838b8d803 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts @@ -245,7 +245,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy { }); })); this.canImpersonate$ = this.epersonService.getActiveEPerson().pipe( - switchMap((eperson) => this.authorizationService.isAuthenticated(FeatureID.LoginOnBehalfOf, hasValue(eperson) ? eperson.self : undefined)) + switchMap((eperson) => this.authorizationService.isAuthorized(FeatureID.LoginOnBehalfOf, hasValue(eperson) ? eperson.self : undefined)) ); }); } diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index 30cbf89aaf..114a3e903d 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts @@ -360,7 +360,7 @@ export class AdminSidebarComponent extends MenuComponent implements OnInit { * Create menu sections dependent on whether or not the current user is a site administrator */ createSiteAdministratorMenuSections() { - this.authorizationService.isAuthenticated(FeatureID.AdministratorOf).subscribe((authorized) => { + this.authorizationService.isAuthorized(FeatureID.AdministratorOf).subscribe((authorized) => { const menuList = [ /* Access Control */ { diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index 9694ed42b0..0ae3d356a3 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -64,6 +64,12 @@ export function getDSOPath(dso: DSpaceObject): string { } } +const UNAUTHORIZED_PATH = 'unauthorized'; + +export function getUnauthorizedPath() { + return `/${UNAUTHORIZED_PATH}`; +} + @NgModule({ imports: [ RouterModule.forRoot([ @@ -100,7 +106,7 @@ export function getDSOPath(dso: DSpaceObject): string { path: PROFILE_MODULE_PATH, loadChildren: './profile-page/profile-page.module#ProfilePageModule', canActivate: [AuthenticatedGuard] }, - { path: '401', component: UnauthorizedComponent }, + { path: UNAUTHORIZED_PATH, component: UnauthorizedComponent }, { path: '**', pathMatch: 'full', component: PageNotFoundComponent }, ], { 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 9bb6a5b17e..a7dd8414f1 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 @@ -51,13 +51,13 @@ describe('AuthorizationDataService', () => { const objectUrl = 'fake-object-url'; const ePersonUuid = 'fake-eperson-uuid'; - function createExpected(objectUrl: string, ePersonUuid?: string, featureId?: FeatureID): FindListOptions { - const searchParams = [new RequestParam('uri', objectUrl)]; - if (hasValue(featureId)) { - searchParams.push(new RequestParam('feature', featureId)); + function createExpected(providedObjectUrl: string, providedEPersonUuid?: string, providedFeatureId?: FeatureID): FindListOptions { + const searchParams = [new RequestParam('uri', providedObjectUrl)]; + if (hasValue(providedFeatureId)) { + searchParams.push(new RequestParam('feature', providedFeatureId)); } - if (hasValue(ePersonUuid)) { - searchParams.push(new RequestParam('eperson', ePersonUuid)); + if (hasValue(providedEPersonUuid)) { + searchParams.push(new RequestParam('eperson', providedEPersonUuid)); } return Object.assign(new FindListOptions(), { searchParams }); } @@ -114,7 +114,7 @@ describe('AuthorizationDataService', () => { }); }); - describe('isAuthenticated', () => { + describe('isAuthorized', () => { const validPayload = [ Object.assign(new Authorization()) ] @@ -126,7 +126,7 @@ describe('AuthorizationDataService', () => { }); it('should return false', (done) => { - service.isAuthenticated().subscribe((result) => { + service.isAuthorized().subscribe((result) => { expect(result).toEqual(false); done(); }); @@ -139,7 +139,7 @@ describe('AuthorizationDataService', () => { }); it('should return false', (done) => { - service.isAuthenticated().subscribe((result) => { + service.isAuthorized().subscribe((result) => { expect(result).toEqual(false); done(); }); @@ -152,7 +152,7 @@ describe('AuthorizationDataService', () => { }); it('should return true', (done) => { - service.isAuthenticated().subscribe((result) => { + service.isAuthorized().subscribe((result) => { expect(result).toEqual(true); done(); }); 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 92a56e7bcf..fa7eee0681 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -60,7 +60,7 @@ export class AuthorizationDataService extends DataService { * If not provided, the UUID of the currently authenticated {@link EPerson} will be used. * @param featureId ID of the {@link Feature} to check {@link Authorization} for */ - isAuthenticated(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string): Observable { + isAuthorized(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string): Observable { return this.searchByObject(featureId, objectUrl, ePersonUuid).pipe( map((authorizationRD) => (authorizationRD.statusCode !== 401 && hasValue(authorizationRD.payload) && isNotEmpty(authorizationRD.payload.page))) ); 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 fca665862f..f69bd9363a 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,6 +2,7 @@ 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'; /** * Test implementation of abstract class FeatureAuthorizationGuard @@ -9,10 +10,11 @@ import { of as observableOf } from 'rxjs'; */ class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, + protected router: Router, protected featureId: FeatureID, protected objectUrl: string, protected ePersonUuid: string) { - super(authorizationService); + super(authorizationService, router); } getFeatureID(): FeatureID { @@ -31,6 +33,7 @@ class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard { describe('FeatureAuthorizationGuard', () => { let guard: FeatureAuthorizationGuard; let authorizationService: AuthorizationDataService; + let router: Router; let featureId: FeatureID; let objectUrl: string; @@ -44,7 +47,10 @@ describe('FeatureAuthorizationGuard', () => { authorizationService = jasmine.createSpyObj('authorizationService', { isAuthenticated: observableOf(true) }); - guard = new FeatureAuthorizationGuardImpl(authorizationService, featureId, objectUrl, ePersonUuid); + router = jasmine.createSpyObj('router', { + navigateByUrl: {} + }); + guard = new FeatureAuthorizationGuardImpl(authorizationService, router, featureId, objectUrl, ePersonUuid); } beforeEach(() => { @@ -54,14 +60,7 @@ describe('FeatureAuthorizationGuard', () => { describe('canActivate', () => { it('should call authorizationService.isAuthenticated with the appropriate arguments', () => { guard.canActivate(undefined, undefined).subscribe(); - expect(authorizationService.isAuthenticated).toHaveBeenCalledWith(featureId, objectUrl, ePersonUuid); - }); - }); - - describe('canLoad', () => { - it('should call authorizationService.isAuthenticated with the appropriate arguments', () => { - guard.canLoad(undefined, undefined).subscribe(); - expect(authorizationService.isAuthenticated).toHaveBeenCalledWith(featureId, objectUrl, ePersonUuid); + expect(authorizationService.isAuthorized).toHaveBeenCalledWith(featureId, objectUrl, 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 708b197441..7806d87b0c 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 @@ -1,23 +1,21 @@ import { ActivatedRouteSnapshot, CanActivate, - CanLoad, - Route, Router, RouterStateSnapshot, - UrlSegment + UrlTree } from '@angular/router'; import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; import { Observable } from 'rxjs/internal/Observable'; -import { redirectToUnauthorizedOnFalse } from '../../../shared/operators'; +import { returnUnauthorizedUrlTreeOnFalse } from '../../../shared/operators'; /** * Abstract Guard for preventing unauthorized activating and loading of routes when a user * doesn't have authorized rights on a specific feature and/or object. * Override the desired getters in the parent class for checking specific authorization on a feature and/or object. */ -export abstract class FeatureAuthorizationGuard implements CanActivate, CanLoad { +export abstract class FeatureAuthorizationGuard implements CanActivate { constructor(protected authorizationService: AuthorizationDataService, protected router: Router) { } @@ -26,16 +24,8 @@ export abstract class FeatureAuthorizationGuard implements CanActivate, CanLoad * 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.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()).pipe(redirectToUnauthorizedOnFalse(this.router)); - } - - /** - * 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 - */ - canLoad(route: Route, segments: UrlSegment[]): Observable { - return this.authorizationService.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()).pipe(redirectToUnauthorizedOnFalse(this.router)); + canActivate(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { + return this.authorizationService.isAuthorized(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()).pipe(returnUnauthorizedUrlTreeOnFalse(this.router)); } /** diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index ca1394005e..4abb71350b 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -1,4 +1,4 @@ -import { Router } from '@angular/router'; +import { Router, UrlTree } from '@angular/router'; import { Observable } from 'rxjs'; import { filter, find, flatMap, map, take, tap } from 'rxjs/operators'; import { hasValue, hasValueOperator, isNotEmpty } from '../../shared/empty.util'; @@ -11,6 +11,7 @@ import { RequestEntry } from '../data/request.reducer'; import { RequestService } from '../data/request.service'; import { BrowseDefinition } from './browse-definition.model'; import { DSpaceObject } from './dspace-object.model'; +import { getUnauthorizedPath } from '../../app-routing.module'; /** * This file contains custom RxJS operators that can be used in multiple places @@ -181,16 +182,14 @@ export const redirectToPageNotFoundOn404 = (router: Router) => })); /** - * Operator that redirects the user to the unauthorized page when the boolean received is false + * Operator that returns a UrlTree to the unauthorized page when the boolean received is false * @param router */ -export const redirectToUnauthorizedOnFalse = (router: Router) => - (source: Observable): Observable => +export const returnUnauthorizedUrlTreeOnFalse = (router: Router) => + (source: Observable): Observable => source.pipe( - tap((authorized: boolean) => { - if (!authorized) { - router.navigateByUrl('/401', { skipLocationChange: true }); - } + map((authorized: boolean) => { + return authorized ? authorized : router.parseUrl(getUnauthorizedPath()) })); export const getFinishedRemoteData = () => From 882ff3ea180121e7fca3b6b430674968d2f50cb9 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 24 Jun 2020 16:08:10 +0200 Subject: [PATCH 10/14] 71429: Test fixes --- .../eperson-form/eperson-form.component.spec.ts | 2 +- src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts | 3 +-- .../feature-authorization.guard.spec.ts | 4 ++-- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts index 51eb9d0690..7d8be4fad3 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.spec.ts @@ -114,7 +114,7 @@ describe('EPersonFormComponent', () => { translateService = getMockTranslateService(); authService = new AuthServiceStub(); authorizationService = jasmine.createSpyObj('authorizationService', { - isAuthenticated: observableOf(true) + isAuthorized: observableOf(true) }); groupsDataService = jasmine.createSpyObj('groupsDataService', { findAllByHref: createSuccessfulRemoteDataObject$(createPaginatedList([])), diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts index fd76f0293e..40539d3e13 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.spec.ts @@ -9,7 +9,6 @@ import { CSSVariableService } from '../../shared/sass-helper/sass-helper.service import { CSSVariableServiceStub } from '../../shared/testing/css-variable-service.stub'; import { AuthServiceStub } from '../../shared/testing/auth-service.stub'; import { AuthService } from '../../core/auth/auth.service'; - import { of as observableOf } from 'rxjs'; import { By } from '@angular/platform-browser'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; @@ -25,7 +24,7 @@ describe('AdminSidebarComponent', () => { beforeEach(async(() => { authorizationService = jasmine.createSpyObj('authorizationService', { - isAuthenticated: observableOf(true) + isAuthorized: observableOf(true) }); TestBed.configureTestingModule({ imports: [TranslateModule.forRoot(), NoopAnimationsModule, RouterTestingModule], 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 f69bd9363a..bfd161bad2 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 @@ -45,10 +45,10 @@ describe('FeatureAuthorizationGuard', () => { ePersonUuid = 'fake-eperson-uuid'; authorizationService = jasmine.createSpyObj('authorizationService', { - isAuthenticated: observableOf(true) + isAuthorized: observableOf(true) }); router = jasmine.createSpyObj('router', { - navigateByUrl: {} + parseUrl: {} }); guard = new FeatureAuthorizationGuardImpl(authorizationService, router, featureId, objectUrl, ePersonUuid); } From fcdd1a8ef2cd2d5878f9e9f3a23cd81c0807aeee Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 24 Jun 2020 16:40:50 +0200 Subject: [PATCH 11/14] 71429: Test fix --- src/app/core/metadata/metadata.service.spec.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/app/core/metadata/metadata.service.spec.ts b/src/app/core/metadata/metadata.service.spec.ts index c21cf50512..74c4522bbd 100644 --- a/src/app/core/metadata/metadata.service.spec.ts +++ b/src/app/core/metadata/metadata.service.spec.ts @@ -168,7 +168,8 @@ describe('MetadataService', () => { { provide: BitstreamDataService, useValue: mockBitstreamDataService }, Meta, Title, - ItemDataService, + // tslint:disable-next-line:no-empty + { provide: ItemDataService, useValue: { findById: () => {} } }, BrowseService, MetadataService ], From d562b19d57c119eecbf56c9b87a41db34eaa64b7 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 2 Jul 2020 14:28:35 +0200 Subject: [PATCH 12/14] 71429: LGTM fixes --- .../epeople-registry/eperson-form/eperson-form.component.ts | 4 ++-- src/app/+admin/admin-sidebar/admin-sidebar.component.ts | 2 +- .../data/feature-authorization/authorization-data.service.ts | 3 +-- src/app/shared/menu/menu.component.ts | 4 ++-- 4 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts index 6838b8d803..0031271313 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.ts @@ -7,9 +7,9 @@ import { DynamicInputModel } from '@ng-dynamic-forms/core'; import { TranslateService } from '@ngx-translate/core'; -import { Subscription, combineLatest, of, interval } from 'rxjs'; +import { Subscription, combineLatest, of } from 'rxjs'; import { Observable } from 'rxjs/internal/Observable'; -import { flatMap, map, switchMap, take } from 'rxjs/operators'; +import { switchMap, take } from 'rxjs/operators'; import { RestResponse } from '../../../../core/cache/response.models'; import { PaginatedList } from '../../../../core/data/paginated-list'; import { RemoteData } from '../../../../core/data/remote-data'; diff --git a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts index 114a3e903d..a2818b699a 100644 --- a/src/app/+admin/admin-sidebar/admin-sidebar.component.ts +++ b/src/app/+admin/admin-sidebar/admin-sidebar.component.ts @@ -2,7 +2,7 @@ import { Component, Injector, OnInit } from '@angular/core'; import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; import { combineLatest as combineLatestObservable } from 'rxjs'; import { Observable } from 'rxjs/internal/Observable'; -import { first, map, take } from 'rxjs/operators'; +import { first, map } from 'rxjs/operators'; import { AuthService } from '../../core/auth/auth.service'; import { slideHorizontal, slideSidebar } from '../../shared/animations/slide'; import { CreateCollectionParentSelectorComponent } from '../../shared/dso-selector/modal-wrappers/create-collection-parent-selector/create-collection-parent-selector.component'; 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 fa7eee0681..c75decccef 100644 --- a/src/app/core/data/feature-authorization/authorization-data.service.ts +++ b/src/app/core/data/feature-authorization/authorization-data.service.ts @@ -20,13 +20,12 @@ import { FollowLinkConfig } from '../../../shared/utils/follow-link-config.model import { Observable } from 'rxjs/internal/Observable'; import { RemoteData } from '../remote-data'; import { PaginatedList } from '../paginated-list'; -import { find, map, skipWhile, switchMap, tap } from 'rxjs/operators'; +import { find, map, switchMap, tap } from 'rxjs/operators'; import { hasValue, isNotEmpty } from '../../../shared/empty.util'; import { RequestParam } from '../../cache/models/request-param.model'; import { AuthorizationSearchParams } from './authorization-search-params'; import { addAuthenticatedUserUuidIfEmpty, addSiteObjectUrlIfEmpty } from './authorization-utils'; import { FeatureID } from './feature-id'; -import { of } from 'rxjs/internal/observable/of'; /** * A service to retrieve {@link Authorization}s from the REST API diff --git a/src/app/shared/menu/menu.component.ts b/src/app/shared/menu/menu.component.ts index 329fabf703..28e906c456 100644 --- a/src/app/shared/menu/menu.component.ts +++ b/src/app/shared/menu/menu.component.ts @@ -3,9 +3,9 @@ import { Observable } from 'rxjs/internal/Observable'; import { MenuService } from './menu.service'; import { MenuID } from './initial-menus-state'; import { MenuSection } from './menu.reducer'; -import { distinctUntilChanged, filter, first, map, tap } from 'rxjs/operators'; +import { distinctUntilChanged, first, map } from 'rxjs/operators'; import { GenericConstructor } from '../../core/shared/generic-constructor'; -import { hasNoValue, hasValue } from '../empty.util'; +import { hasValue } from '../empty.util'; import { MenuSectionComponent } from './menu-section/menu-section.component'; import { getComponentForMenu } from './menu-section.decorator'; import { Subscription } from 'rxjs/internal/Subscription'; From 3560bc60aa8cff1f928585924c7c713f52e0214b Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 9 Jul 2020 15:46:14 +0200 Subject: [PATCH 13/14] 71429: isAuthorized check on feature ID + hide impersonate button when unauthorized --- .../eperson-form/eperson-form.component.html | 2 +- .../authorization-data.service.spec.ts | 46 +++++++++++++++++-- .../authorization-data.service.ts | 19 ++++++-- .../authorization-utils.ts | 35 +++++++++++++- .../form/process-form.component.spec.ts | 2 +- 5 files changed, 91 insertions(+), 13 deletions(-) diff --git a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.html b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.html index b87b3e0848..34fdef89bf 100644 --- a/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.html +++ b/src/app/+admin/admin-access-control/epeople-registry/eperson-form/eperson-form.component.html @@ -20,7 +20,7 @@ -