71429: Unauthorized page fixes

This commit is contained in:
Kristof De Langhe
2020-06-24 15:05:24 +02:00
parent 78a5bd5fce
commit 080ddf8a1f
8 changed files with 41 additions and 47 deletions

View File

@@ -245,7 +245,7 @@ export class EPersonFormComponent implements OnInit, OnDestroy {
}); });
})); }));
this.canImpersonate$ = this.epersonService.getActiveEPerson().pipe( 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))
); );
}); });
} }

View File

@@ -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 * Create menu sections dependent on whether or not the current user is a site administrator
*/ */
createSiteAdministratorMenuSections() { createSiteAdministratorMenuSections() {
this.authorizationService.isAuthenticated(FeatureID.AdministratorOf).subscribe((authorized) => { this.authorizationService.isAuthorized(FeatureID.AdministratorOf).subscribe((authorized) => {
const menuList = [ const menuList = [
/* Access Control */ /* Access Control */
{ {

View File

@@ -64,6 +64,12 @@ export function getDSOPath(dso: DSpaceObject): string {
} }
} }
const UNAUTHORIZED_PATH = 'unauthorized';
export function getUnauthorizedPath() {
return `/${UNAUTHORIZED_PATH}`;
}
@NgModule({ @NgModule({
imports: [ imports: [
RouterModule.forRoot([ RouterModule.forRoot([
@@ -100,7 +106,7 @@ export function getDSOPath(dso: DSpaceObject): string {
path: PROFILE_MODULE_PATH, path: PROFILE_MODULE_PATH,
loadChildren: './profile-page/profile-page.module#ProfilePageModule', canActivate: [AuthenticatedGuard] loadChildren: './profile-page/profile-page.module#ProfilePageModule', canActivate: [AuthenticatedGuard]
}, },
{ path: '401', component: UnauthorizedComponent }, { path: UNAUTHORIZED_PATH, component: UnauthorizedComponent },
{ path: '**', pathMatch: 'full', component: PageNotFoundComponent }, { path: '**', pathMatch: 'full', component: PageNotFoundComponent },
], ],
{ {

View File

@@ -51,13 +51,13 @@ describe('AuthorizationDataService', () => {
const objectUrl = 'fake-object-url'; const objectUrl = 'fake-object-url';
const ePersonUuid = 'fake-eperson-uuid'; const ePersonUuid = 'fake-eperson-uuid';
function createExpected(objectUrl: string, ePersonUuid?: string, featureId?: FeatureID): FindListOptions { function createExpected(providedObjectUrl: string, providedEPersonUuid?: string, providedFeatureId?: FeatureID): FindListOptions {
const searchParams = [new RequestParam('uri', objectUrl)]; const searchParams = [new RequestParam('uri', providedObjectUrl)];
if (hasValue(featureId)) { if (hasValue(providedFeatureId)) {
searchParams.push(new RequestParam('feature', featureId)); searchParams.push(new RequestParam('feature', providedFeatureId));
} }
if (hasValue(ePersonUuid)) { if (hasValue(providedEPersonUuid)) {
searchParams.push(new RequestParam('eperson', ePersonUuid)); searchParams.push(new RequestParam('eperson', providedEPersonUuid));
} }
return Object.assign(new FindListOptions(), { searchParams }); return Object.assign(new FindListOptions(), { searchParams });
} }
@@ -114,7 +114,7 @@ describe('AuthorizationDataService', () => {
}); });
}); });
describe('isAuthenticated', () => { describe('isAuthorized', () => {
const validPayload = [ const validPayload = [
Object.assign(new Authorization()) Object.assign(new Authorization())
] ]
@@ -126,7 +126,7 @@ describe('AuthorizationDataService', () => {
}); });
it('should return false', (done) => { it('should return false', (done) => {
service.isAuthenticated().subscribe((result) => { service.isAuthorized().subscribe((result) => {
expect(result).toEqual(false); expect(result).toEqual(false);
done(); done();
}); });
@@ -139,7 +139,7 @@ describe('AuthorizationDataService', () => {
}); });
it('should return false', (done) => { it('should return false', (done) => {
service.isAuthenticated().subscribe((result) => { service.isAuthorized().subscribe((result) => {
expect(result).toEqual(false); expect(result).toEqual(false);
done(); done();
}); });
@@ -152,7 +152,7 @@ describe('AuthorizationDataService', () => {
}); });
it('should return true', (done) => { it('should return true', (done) => {
service.isAuthenticated().subscribe((result) => { service.isAuthorized().subscribe((result) => {
expect(result).toEqual(true); expect(result).toEqual(true);
done(); done();
}); });

View File

@@ -60,7 +60,7 @@ export class AuthorizationDataService extends DataService<Authorization> {
* If not provided, the UUID of the currently authenticated {@link EPerson} will be used. * 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 * @param featureId ID of the {@link Feature} to check {@link Authorization} for
*/ */
isAuthenticated(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string): Observable<boolean> { isAuthorized(featureId?: FeatureID, objectUrl?: string, ePersonUuid?: string): Observable<boolean> {
return this.searchByObject(featureId, objectUrl, ePersonUuid).pipe( return this.searchByObject(featureId, objectUrl, ePersonUuid).pipe(
map((authorizationRD) => (authorizationRD.statusCode !== 401 && hasValue(authorizationRD.payload) && isNotEmpty(authorizationRD.payload.page))) map((authorizationRD) => (authorizationRD.statusCode !== 401 && hasValue(authorizationRD.payload) && isNotEmpty(authorizationRD.payload.page)))
); );

View File

@@ -2,6 +2,7 @@ import { FeatureAuthorizationGuard } from './feature-authorization.guard';
import { AuthorizationDataService } from '../authorization-data.service'; import { AuthorizationDataService } from '../authorization-data.service';
import { FeatureID } from '../feature-id'; import { FeatureID } from '../feature-id';
import { of as observableOf } from 'rxjs'; import { of as observableOf } from 'rxjs';
import { Router } from '@angular/router';
/** /**
* Test implementation of abstract class FeatureAuthorizationGuard * Test implementation of abstract class FeatureAuthorizationGuard
@@ -9,10 +10,11 @@ import { of as observableOf } from 'rxjs';
*/ */
class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard { class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard {
constructor(protected authorizationService: AuthorizationDataService, constructor(protected authorizationService: AuthorizationDataService,
protected router: Router,
protected featureId: FeatureID, protected featureId: FeatureID,
protected objectUrl: string, protected objectUrl: string,
protected ePersonUuid: string) { protected ePersonUuid: string) {
super(authorizationService); super(authorizationService, router);
} }
getFeatureID(): FeatureID { getFeatureID(): FeatureID {
@@ -31,6 +33,7 @@ class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard {
describe('FeatureAuthorizationGuard', () => { describe('FeatureAuthorizationGuard', () => {
let guard: FeatureAuthorizationGuard; let guard: FeatureAuthorizationGuard;
let authorizationService: AuthorizationDataService; let authorizationService: AuthorizationDataService;
let router: Router;
let featureId: FeatureID; let featureId: FeatureID;
let objectUrl: string; let objectUrl: string;
@@ -44,7 +47,10 @@ describe('FeatureAuthorizationGuard', () => {
authorizationService = jasmine.createSpyObj('authorizationService', { authorizationService = jasmine.createSpyObj('authorizationService', {
isAuthenticated: observableOf(true) isAuthenticated: observableOf(true)
}); });
guard = new FeatureAuthorizationGuardImpl(authorizationService, featureId, objectUrl, ePersonUuid); router = jasmine.createSpyObj('router', {
navigateByUrl: {}
});
guard = new FeatureAuthorizationGuardImpl(authorizationService, router, featureId, objectUrl, ePersonUuid);
} }
beforeEach(() => { beforeEach(() => {
@@ -54,14 +60,7 @@ describe('FeatureAuthorizationGuard', () => {
describe('canActivate', () => { describe('canActivate', () => {
it('should call authorizationService.isAuthenticated with the appropriate arguments', () => { it('should call authorizationService.isAuthenticated with the appropriate arguments', () => {
guard.canActivate(undefined, undefined).subscribe(); guard.canActivate(undefined, undefined).subscribe();
expect(authorizationService.isAuthenticated).toHaveBeenCalledWith(featureId, objectUrl, ePersonUuid); expect(authorizationService.isAuthorized).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);
}); });
}); });
}); });

View File

@@ -1,23 +1,21 @@
import { import {
ActivatedRouteSnapshot, ActivatedRouteSnapshot,
CanActivate, CanActivate,
CanLoad,
Route,
Router, Router,
RouterStateSnapshot, RouterStateSnapshot,
UrlSegment UrlTree
} from '@angular/router'; } from '@angular/router';
import { AuthorizationDataService } from '../authorization-data.service'; import { AuthorizationDataService } from '../authorization-data.service';
import { FeatureID } from '../feature-id'; import { FeatureID } from '../feature-id';
import { Observable } from 'rxjs/internal/Observable'; 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 * 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. * 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. * 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, constructor(protected authorizationService: AuthorizationDataService,
protected router: Router) { 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 * 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 * Redirect the user to the unauthorized page when he/she's not authorized for the given feature
*/ */
canActivate(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean> { canActivate(next: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean | UrlTree> {
return this.authorizationService.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()).pipe(redirectToUnauthorizedOnFalse(this.router)); return this.authorizationService.isAuthorized(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()).pipe(returnUnauthorizedUrlTreeOnFalse(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<boolean> {
return this.authorizationService.isAuthenticated(this.getFeatureID(), this.getObjectUrl(), this.getEPersonUuid()).pipe(redirectToUnauthorizedOnFalse(this.router));
} }
/** /**

View File

@@ -1,4 +1,4 @@
import { Router } from '@angular/router'; import { Router, UrlTree } from '@angular/router';
import { Observable } from 'rxjs'; import { Observable } from 'rxjs';
import { filter, find, flatMap, map, take, tap } from 'rxjs/operators'; import { filter, find, flatMap, map, take, tap } from 'rxjs/operators';
import { hasValue, hasValueOperator, isNotEmpty } from '../../shared/empty.util'; 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 { RequestService } from '../data/request.service';
import { BrowseDefinition } from './browse-definition.model'; import { BrowseDefinition } from './browse-definition.model';
import { DSpaceObject } from './dspace-object.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 * 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 * @param router
*/ */
export const redirectToUnauthorizedOnFalse = (router: Router) => export const returnUnauthorizedUrlTreeOnFalse = (router: Router) =>
(source: Observable<boolean>): Observable<boolean> => (source: Observable<boolean>): Observable<boolean | UrlTree> =>
source.pipe( source.pipe(
tap((authorized: boolean) => { map((authorized: boolean) => {
if (!authorized) { return authorized ? authorized : router.parseUrl(getUnauthorizedPath())
router.navigateByUrl('/401', { skipLocationChange: true });
}
})); }));
export const getFinishedRemoteData = () => export const getFinishedRemoteData = () =>