From ab755d3fd99c2ef9b0e1095e7573102a12684519 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Wed, 18 Nov 2020 14:05:36 +0100 Subject: [PATCH 1/4] 74612: 403 page + redirect to login on unauthorized --- .../collection-page.component.ts | 8 +-- .../community-page.component.ts | 8 +-- .../full/full-item-page.component.spec.ts | 11 +++- .../full/full-item-page.component.ts | 5 +- .../simple/item-page.component.spec.ts | 10 +++- .../+item-page/simple/item-page.component.ts | 6 ++- src/app/app-routing-paths.ts | 6 +++ src/app/app-routing.module.ts | 4 +- src/app/app.module.ts | 6 ++- .../core/services/server-response.service.ts | 4 ++ src/app/core/shared/operators.spec.ts | 51 ++++++++++++++++--- src/app/core/shared/operators.ts | 24 ++++++--- src/app/forbidden/forbidden.component.html | 10 ++++ src/app/forbidden/forbidden.component.scss | 0 src/app/forbidden/forbidden.component.ts | 32 ++++++++++++ .../detail/process-detail.component.spec.ts | 9 +++- .../detail/process-detail.component.ts | 8 +-- ...llection-statistics-page.component.spec.ts | 7 +++ .../collection-statistics-page.component.ts | 3 ++ ...ommunity-statistics-page.component.spec.ts | 7 +++ .../community-statistics-page.component.ts | 3 ++ .../item-statistics-page.component.spec.ts | 7 +++ .../item-statistics-page.component.ts | 3 ++ .../site-statistics-page.component.spec.ts | 7 +++ .../site-statistics-page.component.ts | 3 ++ .../statistics-page.component.ts | 6 ++- src/assets/i18n/en.json5 | 8 +++ 27 files changed, 221 insertions(+), 35 deletions(-) create mode 100644 src/app/forbidden/forbidden.component.html create mode 100644 src/app/forbidden/forbidden.component.scss create mode 100644 src/app/forbidden/forbidden.component.ts diff --git a/src/app/+collection-page/collection-page.component.ts b/src/app/+collection-page/collection-page.component.ts index a390bda4cb..c99ba34936 100644 --- a/src/app/+collection-page/collection-page.component.ts +++ b/src/app/+collection-page/collection-page.component.ts @@ -17,13 +17,14 @@ import { DSpaceObjectType } from '../core/shared/dspace-object-type.model'; import { Item } from '../core/shared/item.model'; import { getSucceededRemoteData, - redirectOn404Or401, + redirectOn4xx, toDSpaceObjectListRD } from '../core/shared/operators'; import { fadeIn, fadeInOut } from '../shared/animations/fade'; import { hasNoValue, hasValue, isNotEmpty } from '../shared/empty.util'; import { PaginationComponentOptions } from '../shared/pagination/pagination-component-options.model'; +import { AuthService } from '../core/auth/auth.service'; @Component({ selector: 'ds-collection-page', @@ -51,7 +52,8 @@ export class CollectionPageComponent implements OnInit { private searchService: SearchService, private metadata: MetadataService, private route: ActivatedRoute, - private router: Router + private router: Router, + private authService: AuthService, ) { this.paginationConfig = new PaginationComponentOptions(); this.paginationConfig.id = 'collection-page-pagination'; @@ -63,7 +65,7 @@ export class CollectionPageComponent implements OnInit { ngOnInit(): void { this.collectionRD$ = this.route.data.pipe( map((data) => data.dso as RemoteData), - redirectOn404Or401(this.router), + redirectOn4xx(this.router, this.authService), take(1) ); this.logoRD$ = this.collectionRD$.pipe( diff --git a/src/app/+community-page/community-page.component.ts b/src/app/+community-page/community-page.component.ts index 7c93b1bf4e..f65da14644 100644 --- a/src/app/+community-page/community-page.component.ts +++ b/src/app/+community-page/community-page.component.ts @@ -13,7 +13,8 @@ import { MetadataService } from '../core/metadata/metadata.service'; import { fadeInOut } from '../shared/animations/fade'; import { hasValue } from '../shared/empty.util'; -import { redirectOn404Or401 } from '../core/shared/operators'; +import { redirectOn4xx } from '../core/shared/operators'; +import { AuthService } from '../core/auth/auth.service'; @Component({ selector: 'ds-community-page', @@ -39,7 +40,8 @@ export class CommunityPageComponent implements OnInit { private communityDataService: CommunityDataService, private metadata: MetadataService, private route: ActivatedRoute, - private router: Router + private router: Router, + private authService: AuthService, ) { } @@ -47,7 +49,7 @@ export class CommunityPageComponent implements OnInit { ngOnInit(): void { this.communityRD$ = this.route.data.pipe( map((data) => data.dso as RemoteData), - redirectOn404Or401(this.router) + redirectOn4xx(this.router, this.authService) ); this.logoRD$ = this.communityRD$.pipe( map((rd: RemoteData) => rd.payload), diff --git a/src/app/+item-page/full/full-item-page.component.spec.ts b/src/app/+item-page/full/full-item-page.component.spec.ts index 0512ea2fef..6cba9cb8ff 100644 --- a/src/app/+item-page/full/full-item-page.component.spec.ts +++ b/src/app/+item-page/full/full-item-page.component.spec.ts @@ -21,6 +21,7 @@ import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { AuthService } from '../../core/auth/auth.service'; const mockItem: Item = Object.assign(new Item(), { bundles: createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), [])), @@ -46,7 +47,14 @@ describe('FullItemPageComponent', () => { let comp: FullItemPageComponent; let fixture: ComponentFixture; + let authService: AuthService; + beforeEach(async(() => { + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + setRedirectUrl: {} + }); + TestBed.configureTestingModule({ imports: [TranslateModule.forRoot({ loader: { @@ -58,7 +66,8 @@ describe('FullItemPageComponent', () => { providers: [ {provide: ActivatedRoute, useValue: routeStub}, {provide: ItemDataService, useValue: {}}, - {provide: MetadataService, useValue: metadataServiceStub} + {provide: MetadataService, useValue: metadataServiceStub}, + { provide: AuthService, useValue: authService }, ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/+item-page/full/full-item-page.component.ts b/src/app/+item-page/full/full-item-page.component.ts index b2a42b7c6f..741f1e76a7 100644 --- a/src/app/+item-page/full/full-item-page.component.ts +++ b/src/app/+item-page/full/full-item-page.component.ts @@ -15,6 +15,7 @@ import { MetadataService } from '../../core/metadata/metadata.service'; import { fadeInOut } from '../../shared/animations/fade'; import { hasValue } from '../../shared/empty.util'; +import { AuthService } from '../../core/auth/auth.service'; /** * This component renders a simple item page. @@ -35,8 +36,8 @@ export class FullItemPageComponent extends ItemPageComponent implements OnInit { metadata$: Observable; - constructor(route: ActivatedRoute, router: Router, items: ItemDataService, metadataService: MetadataService) { - super(route, router, items, metadataService); + constructor(route: ActivatedRoute, router: Router, items: ItemDataService, metadataService: MetadataService, authService: AuthService) { + super(route, router, items, metadataService, authService); } /*** AoT inheritance fix, will hopefully be resolved in the near future **/ diff --git a/src/app/+item-page/simple/item-page.component.spec.ts b/src/app/+item-page/simple/item-page.component.spec.ts index 6c26e75908..1695b0d3b9 100644 --- a/src/app/+item-page/simple/item-page.component.spec.ts +++ b/src/app/+item-page/simple/item-page.component.spec.ts @@ -20,6 +20,7 @@ import { createFailedRemoteDataObject$, createPendingRemoteDataObject$, createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; +import { AuthService } from '../../core/auth/auth.service'; const mockItem: Item = Object.assign(new Item(), { bundles: createSuccessfulRemoteDataObject$(new PaginatedList(new PageInfo(), [])), @@ -30,6 +31,7 @@ const mockItem: Item = Object.assign(new Item(), { describe('ItemPageComponent', () => { let comp: ItemPageComponent; let fixture: ComponentFixture; + let authService: AuthService; const mockMetadataService = { /* tslint:disable:no-empty */ @@ -41,6 +43,11 @@ describe('ItemPageComponent', () => { }); beforeEach(async(() => { + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + setRedirectUrl: {} + }); + TestBed.configureTestingModule({ imports: [TranslateModule.forRoot({ loader: { @@ -53,7 +60,8 @@ describe('ItemPageComponent', () => { {provide: ActivatedRoute, useValue: mockRoute}, {provide: ItemDataService, useValue: {}}, {provide: MetadataService, useValue: mockMetadataService}, - {provide: Router, useValue: {}} + {provide: Router, useValue: {}}, + { provide: AuthService, useValue: authService }, ], schemas: [NO_ERRORS_SCHEMA] diff --git a/src/app/+item-page/simple/item-page.component.ts b/src/app/+item-page/simple/item-page.component.ts index 87d2294ff9..03912d258e 100644 --- a/src/app/+item-page/simple/item-page.component.ts +++ b/src/app/+item-page/simple/item-page.component.ts @@ -11,8 +11,9 @@ import { Item } from '../../core/shared/item.model'; import { MetadataService } from '../../core/metadata/metadata.service'; import { fadeInOut } from '../../shared/animations/fade'; -import { redirectOn404Or401 } from '../../core/shared/operators'; +import { redirectOn4xx } from '../../core/shared/operators'; import { ViewMode } from '../../core/shared/view-mode.model'; +import { AuthService } from '../../core/auth/auth.service'; /** * This component renders a simple item page. @@ -48,6 +49,7 @@ export class ItemPageComponent implements OnInit { private router: Router, private items: ItemDataService, private metadataService: MetadataService, + private authService: AuthService, ) { } /** @@ -56,7 +58,7 @@ export class ItemPageComponent implements OnInit { ngOnInit(): void { this.itemRD$ = this.route.data.pipe( map((data) => data.item as RemoteData), - redirectOn404Or401(this.router) + redirectOn4xx(this.router, this.authService) ); this.metadataService.processRemoteData(this.itemRD$); } diff --git a/src/app/app-routing-paths.ts b/src/app/app-routing-paths.ts index 2b57a1957c..39f3da8ed3 100644 --- a/src/app/app-routing-paths.ts +++ b/src/app/app-routing-paths.ts @@ -61,6 +61,12 @@ export function getUnauthorizedRoute() { return `/${UNAUTHORIZED_PATH}`; } +export const FORBIDDEN_PATH = '403'; + +export function getForbiddenRoute() { + return `/${FORBIDDEN_PATH}`; +} + export const PAGE_NOT_FOUND_PATH = '404'; export function getPageNotFoundRoute() { diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index ecb27efbb3..8eb380e364 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -14,7 +14,7 @@ import { PROFILE_MODULE_PATH, ADMIN_MODULE_PATH, BITSTREAM_MODULE_PATH, - INFO_MODULE_PATH + INFO_MODULE_PATH, FORBIDDEN_PATH } from './app-routing-paths'; import { COLLECTION_MODULE_PATH } from './+collection-page/collection-page-routing-paths'; import { COMMUNITY_MODULE_PATH } from './+community-page/community-page-routing-paths'; @@ -22,6 +22,7 @@ import { ITEM_MODULE_PATH } from './+item-page/item-page-routing-paths'; import { ReloadGuard } from './core/reload/reload.guard'; import { EndUserAgreementCurrentUserGuard } from './core/end-user-agreement/end-user-agreement-current-user.guard'; import { SiteRegisterGuard } from './core/data/feature-authorization/feature-authorization-guard/site-register.guard'; +import { ForbiddenComponent } from './forbidden/forbidden.component'; @NgModule({ imports: [ @@ -69,6 +70,7 @@ import { SiteRegisterGuard } from './core/data/feature-authorization/feature-aut { path: 'processes', loadChildren: './process-page/process-page.module#ProcessPageModule', canActivate: [AuthenticatedGuard, EndUserAgreementCurrentUserGuard] }, { path: INFO_MODULE_PATH, loadChildren: './info/info.module#InfoModule' }, { path: UNAUTHORIZED_PATH, component: UnauthorizedComponent }, + { path: FORBIDDEN_PATH, component: ForbiddenComponent }, { path: 'statistics', loadChildren: './statistics-page/statistics-page-routing.module#StatisticsPageRoutingModule', diff --git a/src/app/app.module.ts b/src/app/app.module.ts index f1cdd5f2e5..013336a506 100755 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -42,6 +42,7 @@ import { BreadcrumbsComponent } from './breadcrumbs/breadcrumbs.component'; import { environment } from '../environments/environment'; import { BrowserModule } from '@angular/platform-browser'; import { UnauthorizedComponent } from './unauthorized/unauthorized.component'; +import { ForbiddenComponent } from './forbidden/forbidden.component'; export function getBase() { return environment.ui.nameSpace; @@ -116,6 +117,9 @@ const DECLARATIONS = [ NotificationComponent, NotificationsBoardComponent, SearchNavbarComponent, + BreadcrumbsComponent, + UnauthorizedComponent, + ForbiddenComponent, ]; const EXPORTS = [ @@ -133,8 +137,6 @@ const EXPORTS = [ ], declarations: [ ...DECLARATIONS, - BreadcrumbsComponent, - UnauthorizedComponent, ], exports: [ ...EXPORTS diff --git a/src/app/core/services/server-response.service.ts b/src/app/core/services/server-response.service.ts index 10da2a3379..adf2ecf4d2 100644 --- a/src/app/core/services/server-response.service.ts +++ b/src/app/core/services/server-response.service.ts @@ -24,6 +24,10 @@ export class ServerResponseService { return this.setStatus(401, message) } + setForbidden(message = 'Forbidden'): this { + return this.setStatus(403, message) + } + setNotFound(message = 'Not found'): this { return this.setStatus(404, message) } diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index 8acf5ea860..cc25aeec10 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -14,7 +14,7 @@ import { getResourceLinksFromResponse, getResponseFromEntry, getSucceededRemoteData, - redirectOn404Or401 + redirectOn4xx } from './operators'; import { RemoteData } from '../data/remote-data'; import { RemoteDataError } from '../data/remote-data-error'; @@ -200,39 +200,74 @@ describe('Core Module - RxJS Operators', () => { }); }); - describe('redirectOn404Or401', () => { + describe('redirectOn4xx', () => { let router; + let authService; + beforeEach(() => { router = jasmine.createSpyObj('router', ['navigateByUrl']); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + setRedirectUrl: {} + }); }); it('should call navigateByUrl to a 404 page, when the remote data contains a 404 error', () => { const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(404, 'Not Found', 'Object was not found')); - observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); + observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); expect(router.navigateByUrl).toHaveBeenCalledWith('/404', { skipLocationChange: true }); }); + it('should call navigateByUrl to a 403 page, when the remote data contains a 403 error', () => { + const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(403, 'Forbidden', 'Forbidden access')); + + observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); + expect(router.navigateByUrl).toHaveBeenCalledWith('/403', { skipLocationChange: true }); + }); + it('should call navigateByUrl to a 401 page, when the remote data contains a 401 error', () => { const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(401, 'Unauthorized', 'The current user is unauthorized')); - observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); + observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); expect(router.navigateByUrl).toHaveBeenCalledWith('/401', { skipLocationChange: true }); }); - it('should not call navigateByUrl to a 404 or 401 page, when the remote data contains another error than a 404 or 401', () => { + it('should not call navigateByUrl to a 404, 403 or 401 page, when the remote data contains another error than a 404, 403 or 401', () => { const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(500, 'Server Error', 'Something went wrong')); - observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); + observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); expect(router.navigateByUrl).not.toHaveBeenCalled(); }); - it('should not call navigateByUrl to a 404 or 401 page, when the remote data contains no error', () => { + it('should not call navigateByUrl to a 404, 403 or 401 page, when the remote data contains no error', () => { const testRD = createSuccessfulRemoteDataObject(undefined); - observableOf(testRD).pipe(redirectOn404Or401(router)).subscribe(); + observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); expect(router.navigateByUrl).not.toHaveBeenCalled(); }); + + describe('when the user is not authenticated', () => { + beforeEach(() => { + (authService.isAuthenticated as jasmine.Spy).and.returnValue(observableOf(false)); + }); + + it('should set the redirect url and navigate to login when the remote data contains a 401 error', () => { + const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(401, 'Unauthorized', 'The current user is unauthorized')); + + observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); + expect(authService.setRedirectUrl).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('login'); + }); + + it('should set the redirect url and navigate to login when the remote data contains a 403 error', () => { + const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(403, 'Forbidden', 'Forbidden access')); + + observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); + expect(authService.setRedirectUrl).toHaveBeenCalled(); + expect(router.navigateByUrl).toHaveBeenCalledWith('login'); + }); + }); }); describe('getResponseFromEntry', () => { diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 29e41907e1..5d8d3609a2 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -13,8 +13,9 @@ import { MetadataField } from '../metadata/metadata-field.model'; import { MetadataSchema } from '../metadata/metadata-schema.model'; import { BrowseDefinition } from './browse-definition.model'; import { DSpaceObject } from './dspace-object.model'; -import { getPageNotFoundRoute, getUnauthorizedRoute } from '../../app-routing-paths'; +import { getForbiddenRoute, getPageNotFoundRoute, getUnauthorizedRoute } from '../../app-routing-paths'; import { getEndUserAgreementPath } from '../../info/info-routing-paths'; +import { AuthService } from '../auth/auth.service'; /** * This file contains custom RxJS operators that can be used in multiple places @@ -174,18 +175,29 @@ export const getAllSucceededRemoteListPayload = () => * Operator that checks if a remote data object returned a 401 or 404 error * When it does contain such an error, it will redirect the user to the related error page, without altering the current URL * @param router The router used to navigate to a new page + * @param authService Service to check if the user is authenticated */ -export const redirectOn404Or401 = (router: Router) => +export const redirectOn4xx = (router: Router, authService: AuthService) => (source: Observable>): Observable> => - source.pipe( - tap((rd: RemoteData) => { + observableCombineLatest(source, authService.isAuthenticated()).pipe( + map(([rd, isAuthenticated]: [RemoteData, boolean]) => { if (rd.hasFailed) { if (rd.error.statusCode === 404) { router.navigateByUrl(getPageNotFoundRoute(), {skipLocationChange: true}); - } else if (rd.error.statusCode === 401) { - router.navigateByUrl(getUnauthorizedRoute(), {skipLocationChange: true}); + } else if (rd.error.statusCode === 403 || rd.error.statusCode === 401) { + if (isAuthenticated) { + if (rd.error.statusCode === 403) { + router.navigateByUrl(getForbiddenRoute(), {skipLocationChange: true}); + } else if (rd.error.statusCode === 401) { + router.navigateByUrl(getUnauthorizedRoute(), {skipLocationChange: true}); + } + } else { + authService.setRedirectUrl(router.url); + router.navigateByUrl('login'); + } } } + return rd; })); /** diff --git a/src/app/forbidden/forbidden.component.html b/src/app/forbidden/forbidden.component.html new file mode 100644 index 0000000000..067aad0048 --- /dev/null +++ b/src/app/forbidden/forbidden.component.html @@ -0,0 +1,10 @@ +
+

403

+

{{"403.forbidden" | translate}}

+
+

{{"403.help" | translate}}

+
+

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

+
diff --git a/src/app/forbidden/forbidden.component.scss b/src/app/forbidden/forbidden.component.scss new file mode 100644 index 0000000000..e69de29bb2 diff --git a/src/app/forbidden/forbidden.component.ts b/src/app/forbidden/forbidden.component.ts new file mode 100644 index 0000000000..b622a0f066 --- /dev/null +++ b/src/app/forbidden/forbidden.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 `Forbidden` DSpace page. + */ +@Component({ + selector: 'ds-forbidden', + templateUrl: './forbidden.component.html', + styleUrls: ['./forbidden.component.scss'] +}) +export class ForbiddenComponent implements OnInit { + + /** + * Initialize instance variables + * + * @param {AuthService} authService + * @param {ServerResponseService} responseService + */ + constructor(private authService: AuthService, private responseService: ServerResponseService) { + this.responseService.setForbidden(); + } + + /** + * Remove redirect url from the state + */ + ngOnInit(): void { + this.authService.clearRedirectUrl(); + } + +} diff --git a/src/app/process-page/detail/process-detail.component.spec.ts b/src/app/process-page/detail/process-detail.component.spec.ts index dff481fdc6..2006609f01 100644 --- a/src/app/process-page/detail/process-detail.component.spec.ts +++ b/src/app/process-page/detail/process-detail.component.spec.ts @@ -15,6 +15,7 @@ import { ProcessDataService } from '../../core/data/processes/process-data.servi import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { createSuccessfulRemoteDataObject, createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { createPaginatedList } from '../../shared/testing/utils.test'; +import { AuthService } from '../../core/auth/auth.service'; describe('ProcessDetailComponent', () => { let component: ProcessDetailComponent; @@ -22,6 +23,7 @@ describe('ProcessDetailComponent', () => { let processService: ProcessDataService; let nameService: DSONameService; + let authService: AuthService; let process: Process; let fileName: string; @@ -65,6 +67,10 @@ describe('ProcessDetailComponent', () => { nameService = jasmine.createSpyObj('nameService', { getName: fileName }); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + setRedirectUrl: {} + }); } beforeEach(async(() => { @@ -75,7 +81,8 @@ describe('ProcessDetailComponent', () => { providers: [ { provide: ActivatedRoute, useValue: { data: observableOf({ process: createSuccessfulRemoteDataObject(process) }) } }, { provide: ProcessDataService, useValue: processService }, - { provide: DSONameService, useValue: nameService } + { provide: DSONameService, useValue: nameService }, + { provide: AuthService, useValue: authService }, ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); diff --git a/src/app/process-page/detail/process-detail.component.ts b/src/app/process-page/detail/process-detail.component.ts index c4610b70e9..dd89724f0e 100644 --- a/src/app/process-page/detail/process-detail.component.ts +++ b/src/app/process-page/detail/process-detail.component.ts @@ -4,12 +4,13 @@ import { Observable } from 'rxjs/internal/Observable'; import { RemoteData } from '../../core/data/remote-data'; import { Process } from '../processes/process.model'; import { map, switchMap } from 'rxjs/operators'; -import { getFirstSucceededRemoteDataPayload, redirectOn404Or401 } from '../../core/shared/operators'; +import { getFirstSucceededRemoteDataPayload, redirectOn4xx } from '../../core/shared/operators'; import { AlertType } from '../../shared/alert/aletr-type'; import { ProcessDataService } from '../../core/data/processes/process-data.service'; import { PaginatedList } from '../../core/data/paginated-list'; import { Bitstream } from '../../core/shared/bitstream.model'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; +import { AuthService } from '../../core/auth/auth.service'; @Component({ selector: 'ds-process-detail', @@ -39,7 +40,8 @@ export class ProcessDetailComponent implements OnInit { constructor(protected route: ActivatedRoute, protected router: Router, protected processService: ProcessDataService, - protected nameService: DSONameService) { + protected nameService: DSONameService, + protected authService: AuthService) { } /** @@ -49,7 +51,7 @@ export class ProcessDetailComponent implements OnInit { ngOnInit(): void { this.processRD$ = this.route.data.pipe( map((data) => data.process as RemoteData), - redirectOn404Or401(this.router) + redirectOn4xx(this.router, this.authService) ); this.filesRD$ = this.processRD$.pipe( diff --git a/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts b/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts index 110757670c..cf6a8b7cff 100644 --- a/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts +++ b/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.spec.ts @@ -14,6 +14,7 @@ import { SharedModule } from '../../shared/shared.module'; import { CommonModule } from '@angular/common'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; +import { AuthService } from '../../core/auth/auth.service'; describe('CollectionStatisticsPageComponent', () => { @@ -59,6 +60,11 @@ describe('CollectionStatisticsPageComponent', () => { getName: () => observableOf('test dso name'), }; + const authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + setRedirectUrl: {} + }); + TestBed.configureTestingModule({ imports: [ TranslateModule.forRoot(), @@ -75,6 +81,7 @@ describe('CollectionStatisticsPageComponent', () => { { provide: UsageReportService, useValue: usageReportService }, { provide: DSpaceObjectDataService, useValue: {} }, { provide: DSONameService, useValue: nameService }, + { provide: AuthService, useValue: authService }, ], }) .compileComponents(); diff --git a/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.ts b/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.ts index 05f4641d81..41ee47ff88 100644 --- a/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.ts +++ b/src/app/statistics-page/collection-statistics-page/collection-statistics-page.component.ts @@ -4,6 +4,7 @@ import { UsageReportService } from '../../core/statistics/usage-report-data.serv import { ActivatedRoute , Router} from '@angular/router'; import { Collection } from '../../core/shared/collection.model'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; +import { AuthService } from '../../core/auth/auth.service'; /** * Component representing the statistics page for a collection. @@ -30,12 +31,14 @@ export class CollectionStatisticsPageComponent extends StatisticsPageComponent { @@ -59,6 +60,11 @@ describe('CommunityStatisticsPageComponent', () => { getName: () => observableOf('test dso name'), }; + const authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + setRedirectUrl: {} + }); + TestBed.configureTestingModule({ imports: [ TranslateModule.forRoot(), @@ -75,6 +81,7 @@ describe('CommunityStatisticsPageComponent', () => { { provide: UsageReportService, useValue: usageReportService }, { provide: DSpaceObjectDataService, useValue: {} }, { provide: DSONameService, useValue: nameService }, + { provide: AuthService, useValue: authService }, ], }) .compileComponents(); diff --git a/src/app/statistics-page/community-statistics-page/community-statistics-page.component.ts b/src/app/statistics-page/community-statistics-page/community-statistics-page.component.ts index 65d5fe88e5..1a1c8fdf0e 100644 --- a/src/app/statistics-page/community-statistics-page/community-statistics-page.component.ts +++ b/src/app/statistics-page/community-statistics-page/community-statistics-page.component.ts @@ -4,6 +4,7 @@ import { UsageReportService } from '../../core/statistics/usage-report-data.serv import { ActivatedRoute, Router } from '@angular/router'; import { Community } from '../../core/shared/community.model'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; +import { AuthService } from '../../core/auth/auth.service'; /** * Component representing the statistics page for a community. @@ -30,12 +31,14 @@ export class CommunityStatisticsPageComponent extends StatisticsPageComponent { @@ -59,6 +60,11 @@ describe('ItemStatisticsPageComponent', () => { getName: () => observableOf('test dso name'), }; + const authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + setRedirectUrl: {} + }); + TestBed.configureTestingModule({ imports: [ TranslateModule.forRoot(), @@ -75,6 +81,7 @@ describe('ItemStatisticsPageComponent', () => { { provide: UsageReportService, useValue: usageReportService }, { provide: DSpaceObjectDataService, useValue: {} }, { provide: DSONameService, useValue: nameService }, + { provide: AuthService, useValue: authService }, ], }) .compileComponents(); diff --git a/src/app/statistics-page/item-statistics-page/item-statistics-page.component.ts b/src/app/statistics-page/item-statistics-page/item-statistics-page.component.ts index fb9ced4520..69a8361a46 100644 --- a/src/app/statistics-page/item-statistics-page/item-statistics-page.component.ts +++ b/src/app/statistics-page/item-statistics-page/item-statistics-page.component.ts @@ -4,6 +4,7 @@ import { UsageReportService } from '../../core/statistics/usage-report-data.serv import { ActivatedRoute, Router } from '@angular/router'; import { Item } from '../../core/shared/item.model'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; +import { AuthService } from '../../core/auth/auth.service'; /** * Component representing the statistics page for an item. @@ -31,12 +32,14 @@ export class ItemStatisticsPageComponent extends StatisticsPageComponent { protected router: Router, protected usageReportService: UsageReportService, protected nameService: DSONameService, + protected authService: AuthService ) { super( route, router, usageReportService, nameService, + authService, ); } } diff --git a/src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts b/src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts index 6f2247b433..1a3babc570 100644 --- a/src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts +++ b/src/app/statistics-page/site-statistics-page/site-statistics-page.component.spec.ts @@ -14,6 +14,7 @@ import { CommonModule } from '@angular/common'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { DSpaceObjectDataService } from '../../core/data/dspace-object-data.service'; import { SiteDataService } from '../../core/data/site-data.service'; +import { AuthService } from '../../core/auth/auth.service'; describe('SiteStatisticsPageComponent', () => { @@ -55,6 +56,11 @@ describe('SiteStatisticsPageComponent', () => { })) }; + const authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true), + setRedirectUrl: {} + }); + TestBed.configureTestingModule({ imports: [ TranslateModule.forRoot(), @@ -72,6 +78,7 @@ describe('SiteStatisticsPageComponent', () => { { provide: DSpaceObjectDataService, useValue: {} }, { provide: DSONameService, useValue: nameService }, { provide: SiteDataService, useValue: siteService }, + { provide: AuthService, useValue: authService }, ], }) .compileComponents(); diff --git a/src/app/statistics-page/site-statistics-page/site-statistics-page.component.ts b/src/app/statistics-page/site-statistics-page/site-statistics-page.component.ts index fd1319723c..59f1be330c 100644 --- a/src/app/statistics-page/site-statistics-page/site-statistics-page.component.ts +++ b/src/app/statistics-page/site-statistics-page/site-statistics-page.component.ts @@ -6,6 +6,7 @@ import { ActivatedRoute, Router } from '@angular/router'; import { Site } from '../../core/shared/site.model'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; import { switchMap } from 'rxjs/operators'; +import { AuthService } from '../../core/auth/auth.service'; /** * Component representing the site-wide statistics page. @@ -30,12 +31,14 @@ export class SiteStatisticsPageComponent extends StatisticsPageComponent { protected usageReportService: UsageReportService, protected nameService: DSONameService, protected siteService: SiteDataService, + protected authService: AuthService, ) { super( route, router, usageReportService, nameService, + authService, ); } diff --git a/src/app/statistics-page/statistics-page/statistics-page.component.ts b/src/app/statistics-page/statistics-page/statistics-page.component.ts index 31cb74eb08..6fa10b437c 100644 --- a/src/app/statistics-page/statistics-page/statistics-page.component.ts +++ b/src/app/statistics-page/statistics-page/statistics-page.component.ts @@ -4,10 +4,11 @@ import { UsageReportService } from '../../core/statistics/usage-report-data.serv import { map, switchMap } from 'rxjs/operators'; import { UsageReport } from '../../core/statistics/models/usage-report.model'; import { RemoteData } from '../../core/data/remote-data'; -import { getRemoteDataPayload, getSucceededRemoteData, redirectOn404Or401 } from '../../core/shared/operators'; +import { getRemoteDataPayload, getSucceededRemoteData, redirectOn4xx } from '../../core/shared/operators'; import { DSpaceObject } from '../../core/shared/dspace-object.model'; import { ActivatedRoute, Router } from '@angular/router'; import { DSONameService } from '../../core/breadcrumbs/dso-name.service'; +import { AuthService } from '../../core/auth/auth.service'; /** * Class representing an abstract statistics page component. @@ -36,6 +37,7 @@ export abstract class StatisticsPageComponent implements protected router: Router, protected usageReportService: UsageReportService, protected nameService: DSONameService, + protected authService: AuthService, ) { } @@ -55,7 +57,7 @@ export abstract class StatisticsPageComponent implements protected getScope$(): Observable { return this.route.data.pipe( map((data) => data.scope as RemoteData), - redirectOn404Or401(this.router), + redirectOn4xx(this.router, this.authService), getSucceededRemoteData(), getRemoteDataPayload(), ); diff --git a/src/assets/i18n/en.json5 b/src/assets/i18n/en.json5 index de4c3eb552..fb14806734 100644 --- a/src/assets/i18n/en.json5 +++ b/src/assets/i18n/en.json5 @@ -8,6 +8,14 @@ + "403.help": "You don't have permission to access this page. You can use the button below to get back to the home page.", + + "403.link.home-page": "Take me to the home page", + + "403.forbidden": "forbidden", + + + "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 b2369b237e841e4345cc85aeb80612c9b214b4da Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 24 Nov 2020 10:36:50 +0100 Subject: [PATCH 2/4] 74612: Redirect bugfix --- src/app/core/auth/auth.service.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 06906346ed..8eea9f8938 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -453,7 +453,7 @@ export class AuthService { * Clear redirect url */ clearRedirectUrl() { - this.store.dispatch(new SetRedirectUrlAction('')); + this.store.dispatch(new SetRedirectUrlAction(undefined)); this.storage.remove(REDIRECT_COOKIE); } From 8486fecc17235e0d54dd7f1b73dc20a5e906673a Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Tue, 24 Nov 2020 12:03:34 +0100 Subject: [PATCH 3/4] 74612: Post-merge test fixes --- .../process-page/detail/process-detail.component.spec.ts | 6 ------ src/app/shared/mocks/auth.service.mock.ts | 7 +++++++ 2 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/app/process-page/detail/process-detail.component.spec.ts b/src/app/process-page/detail/process-detail.component.spec.ts index 94282f886e..b81eedabad 100644 --- a/src/app/process-page/detail/process-detail.component.spec.ts +++ b/src/app/process-page/detail/process-detail.component.spec.ts @@ -37,7 +37,6 @@ describe('ProcessDetailComponent', () => { let nameService: DSONameService; let bitstreamDataService: BitstreamDataService; let httpClient: HttpClient; - let authService: AuthService; let process: Process; let fileName: string; @@ -105,10 +104,6 @@ describe('ProcessDetailComponent', () => { httpClient = jasmine.createSpyObj('httpClient', { get: observableOf(processOutput) }); - authService = jasmine.createSpyObj('authService', { - isAuthenticated: observableOf(true), - setRedirectUrl: {} - }); } beforeEach(async(() => { @@ -126,7 +121,6 @@ describe('ProcessDetailComponent', () => { { provide: DSONameService, useValue: nameService }, { provide: AuthService, useValue: new AuthServiceMock() }, { provide: HttpClient, useValue: httpClient }, - { provide: AuthService, useValue: authService }, ], schemas: [CUSTOM_ELEMENTS_SCHEMA] }).compileComponents(); diff --git a/src/app/shared/mocks/auth.service.mock.ts b/src/app/shared/mocks/auth.service.mock.ts index 0f3a7d13d1..064af7dade 100644 --- a/src/app/shared/mocks/auth.service.mock.ts +++ b/src/app/shared/mocks/auth.service.mock.ts @@ -12,4 +12,11 @@ export class AuthServiceMock { public getShortlivedToken(): Observable { return observableOf('token'); } + + public isAuthenticated(): Observable { + return observableOf(true); + } + + public setRedirectUrl(url: string) { + } } From 411e2bd7461e25985e7d5c359c88660e3c1f9470 Mon Sep 17 00:00:00 2001 From: Kristof De Langhe Date: Thu, 3 Dec 2020 14:38:27 +0100 Subject: [PATCH 4/4] 74612: Remove 401 pages and add login redirect to feature-authorization guards --- .../collection-page-administrator.guard.ts | 6 ++-- .../community-page-administrator.guard.ts | 6 ++-- .../item-page-reinstate.guard.ts | 6 ++-- .../item-page-withdraw.guard.ts | 6 ++-- .../item-page-administrator.guard.ts | 6 ++-- src/app/app-routing-paths.ts | 6 ---- src/app/app-routing.module.ts | 6 ++-- src/app/app.module.ts | 2 -- .../dso-page-feature.guard.spec.ts | 10 ++++-- .../dso-page-feature.guard.ts | 6 ++-- .../feature-authorization.guard.spec.ts | 12 +++++-- .../feature-authorization.guard.ts | 8 +++-- .../site-administrator.guard.ts | 5 +-- .../site-register.guard.ts | 5 +-- src/app/core/shared/operators.spec.ts | 7 ---- src/app/core/shared/operators.ts | 31 +++++++++++------- .../unauthorized/unauthorized.component.html | 10 ------ .../unauthorized/unauthorized.component.scss | 0 .../unauthorized/unauthorized.component.ts | 32 ------------------- 19 files changed, 73 insertions(+), 97 deletions(-) delete mode 100644 src/app/unauthorized/unauthorized.component.html delete mode 100644 src/app/unauthorized/unauthorized.component.scss delete mode 100644 src/app/unauthorized/unauthorized.component.ts diff --git a/src/app/+collection-page/collection-page-administrator.guard.ts b/src/app/+collection-page/collection-page-administrator.guard.ts index 4d2f246689..4f7c6f985b 100644 --- a/src/app/+collection-page/collection-page-administrator.guard.ts +++ b/src/app/+collection-page/collection-page-administrator.guard.ts @@ -7,6 +7,7 @@ import { of as observableOf } from 'rxjs'; import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; import { Observable } from 'rxjs/internal/Observable'; import { FeatureID } from '../core/data/feature-authorization/feature-id'; +import { AuthService } from '../core/auth/auth.service'; @Injectable({ providedIn: 'root' @@ -17,8 +18,9 @@ import { FeatureID } from '../core/data/feature-authorization/feature-id'; export class CollectionPageAdministratorGuard extends DsoPageFeatureGuard { constructor(protected resolver: CollectionPageResolver, protected authorizationService: AuthorizationDataService, - protected router: Router) { - super(resolver, authorizationService, router); + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); } /** diff --git a/src/app/+community-page/community-page-administrator.guard.ts b/src/app/+community-page/community-page-administrator.guard.ts index c5e58ddb1a..bb296a26fd 100644 --- a/src/app/+community-page/community-page-administrator.guard.ts +++ b/src/app/+community-page/community-page-administrator.guard.ts @@ -7,6 +7,7 @@ import { of as observableOf } from 'rxjs'; import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard'; import { Observable } from 'rxjs/internal/Observable'; import { FeatureID } from '../core/data/feature-authorization/feature-id'; +import { AuthService } from '../core/auth/auth.service'; @Injectable({ providedIn: 'root' @@ -17,8 +18,9 @@ import { FeatureID } from '../core/data/feature-authorization/feature-id'; export class CommunityPageAdministratorGuard extends DsoPageFeatureGuard { constructor(protected resolver: CommunityPageResolver, protected authorizationService: AuthorizationDataService, - protected router: Router) { - super(resolver, authorizationService, router); + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); } /** diff --git a/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts b/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts index 061705619a..9bb99feaa2 100644 --- a/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts +++ b/src/app/+item-page/edit-item-page/item-page-reinstate.guard.ts @@ -7,6 +7,7 @@ import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/ro import { Observable } from 'rxjs/internal/Observable'; import { FeatureID } from '../../core/data/feature-authorization/feature-id'; import { of as observableOf } from 'rxjs'; +import { AuthService } from '../../core/auth/auth.service'; @Injectable({ providedIn: 'root' @@ -17,8 +18,9 @@ import { of as observableOf } from 'rxjs'; export class ItemPageReinstateGuard extends DsoPageFeatureGuard { constructor(protected resolver: ItemPageResolver, protected authorizationService: AuthorizationDataService, - protected router: Router) { - super(resolver, authorizationService, router); + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); } /** diff --git a/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts b/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts index 60576bcdb8..b0bd1820bb 100644 --- a/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts +++ b/src/app/+item-page/edit-item-page/item-page-withdraw.guard.ts @@ -7,6 +7,7 @@ import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/ro import { Observable } from 'rxjs/internal/Observable'; import { FeatureID } from '../../core/data/feature-authorization/feature-id'; import { of as observableOf } from 'rxjs'; +import { AuthService } from '../../core/auth/auth.service'; @Injectable({ providedIn: 'root' @@ -17,8 +18,9 @@ import { of as observableOf } from 'rxjs'; export class ItemPageWithdrawGuard extends DsoPageFeatureGuard { constructor(protected resolver: ItemPageResolver, protected authorizationService: AuthorizationDataService, - protected router: Router) { - super(resolver, authorizationService, router); + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); } /** diff --git a/src/app/+item-page/item-page-administrator.guard.ts b/src/app/+item-page/item-page-administrator.guard.ts index eae76348ad..692300a871 100644 --- a/src/app/+item-page/item-page-administrator.guard.ts +++ b/src/app/+item-page/item-page-administrator.guard.ts @@ -7,6 +7,7 @@ import { DsoPageFeatureGuard } from '../core/data/feature-authorization/feature- import { Observable } from 'rxjs/internal/Observable'; import { FeatureID } from '../core/data/feature-authorization/feature-id'; import { of as observableOf } from 'rxjs'; +import { AuthService } from '../core/auth/auth.service'; @Injectable({ providedIn: 'root' @@ -17,8 +18,9 @@ import { of as observableOf } from 'rxjs'; export class ItemPageAdministratorGuard extends DsoPageFeatureGuard { constructor(protected resolver: ItemPageResolver, protected authorizationService: AuthorizationDataService, - protected router: Router) { - super(resolver, authorizationService, router); + protected router: Router, + protected authService: AuthService) { + super(resolver, authorizationService, router, authService); } /** diff --git a/src/app/app-routing-paths.ts b/src/app/app-routing-paths.ts index 39f3da8ed3..8db4ba5aa7 100644 --- a/src/app/app-routing-paths.ts +++ b/src/app/app-routing-paths.ts @@ -55,12 +55,6 @@ export function getDSORoute(dso: DSpaceObject): string { } } -export const UNAUTHORIZED_PATH = '401'; - -export function getUnauthorizedRoute() { - return `/${UNAUTHORIZED_PATH}`; -} - export const FORBIDDEN_PATH = '403'; export function getForbiddenRoute() { diff --git a/src/app/app-routing.module.ts b/src/app/app-routing.module.ts index 8eb380e364..00a0e94054 100644 --- a/src/app/app-routing.module.ts +++ b/src/app/app-routing.module.ts @@ -5,16 +5,15 @@ import { AuthBlockingGuard } from './core/auth/auth-blocking.guard'; import { PageNotFoundComponent } from './pagenotfound/pagenotfound.component'; import { AuthenticatedGuard } from './core/auth/authenticated.guard'; import { SiteAdministratorGuard } from './core/data/feature-authorization/feature-authorization-guard/site-administrator.guard'; -import { UnauthorizedComponent } from './unauthorized/unauthorized.component'; import { - UNAUTHORIZED_PATH, WORKFLOW_ITEM_MODULE_PATH, FORGOT_PASSWORD_PATH, REGISTER_PATH, PROFILE_MODULE_PATH, ADMIN_MODULE_PATH, BITSTREAM_MODULE_PATH, - INFO_MODULE_PATH, FORBIDDEN_PATH + INFO_MODULE_PATH, + FORBIDDEN_PATH, } from './app-routing-paths'; import { COLLECTION_MODULE_PATH } from './+collection-page/collection-page-routing-paths'; import { COMMUNITY_MODULE_PATH } from './+community-page/community-page-routing-paths'; @@ -69,7 +68,6 @@ import { ForbiddenComponent } from './forbidden/forbidden.component'; }, { path: 'processes', loadChildren: './process-page/process-page.module#ProcessPageModule', canActivate: [AuthenticatedGuard, EndUserAgreementCurrentUserGuard] }, { path: INFO_MODULE_PATH, loadChildren: './info/info.module#InfoModule' }, - { path: UNAUTHORIZED_PATH, component: UnauthorizedComponent }, { path: FORBIDDEN_PATH, component: ForbiddenComponent }, { path: 'statistics', diff --git a/src/app/app.module.ts b/src/app/app.module.ts index 013336a506..fcb6ef0ebc 100755 --- a/src/app/app.module.ts +++ b/src/app/app.module.ts @@ -41,7 +41,6 @@ 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'; import { ForbiddenComponent } from './forbidden/forbidden.component'; export function getBase() { @@ -118,7 +117,6 @@ const DECLARATIONS = [ NotificationsBoardComponent, SearchNavbarComponent, BreadcrumbsComponent, - UnauthorizedComponent, ForbiddenComponent, ]; diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts index 1f5efd1329..d08fdec39f 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.spec.ts @@ -7,6 +7,7 @@ import { DSpaceObject } from '../../../shared/dspace-object.model'; import { DsoPageFeatureGuard } from './dso-page-feature.guard'; import { FeatureID } from '../feature-id'; import { Observable } from 'rxjs/internal/Observable'; +import { AuthService } from '../../../auth/auth.service'; /** * Test implementation of abstract class DsoPageAdministratorGuard @@ -15,8 +16,9 @@ class DsoPageFeatureGuardImpl extends DsoPageFeatureGuard { constructor(protected resolver: Resolve>, protected authorizationService: AuthorizationDataService, protected router: Router, + protected authService: AuthService, protected featureID: FeatureID) { - super(resolver, authorizationService, router); + super(resolver, authorizationService, router, authService); } getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { @@ -28,6 +30,7 @@ describe('DsoPageAdministratorGuard', () => { let guard: DsoPageFeatureGuard; let authorizationService: AuthorizationDataService; let router: Router; + let authService: AuthService; let resolver: Resolve>; let object: DSpaceObject; @@ -45,7 +48,10 @@ describe('DsoPageAdministratorGuard', () => { resolver = jasmine.createSpyObj('resolver', { resolve: createSuccessfulRemoteDataObject$(object) }); - guard = new DsoPageFeatureGuardImpl(resolver, authorizationService, router, undefined); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true) + }); + guard = new DsoPageFeatureGuardImpl(resolver, authorizationService, router, authService, undefined); } beforeEach(() => { diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts index ed2590b521..0c40b6f016 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/dso-page-feature.guard.ts @@ -6,6 +6,7 @@ import { getAllSucceededRemoteDataPayload } from '../../../shared/operators'; import { map } from 'rxjs/operators'; import { DSpaceObject } from '../../../shared/dspace-object.model'; import { FeatureAuthorizationGuard } from './feature-authorization.guard'; +import { AuthService } from '../../../auth/auth.service'; /** * Abstract Guard for preventing unauthorized access to {@link DSpaceObject} pages that require rights for a specific feature @@ -14,8 +15,9 @@ import { FeatureAuthorizationGuard } from './feature-authorization.guard'; export abstract class DsoPageFeatureGuard extends FeatureAuthorizationGuard { constructor(protected resolver: Resolve>, protected authorizationService: AuthorizationDataService, - protected router: Router) { - super(authorizationService, router); + protected router: Router, + protected authService: AuthService) { + super(authorizationService, router, authService); } /** 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 829a246dcc..1bfc8ffa2e 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 @@ -4,6 +4,7 @@ import { FeatureID } from '../feature-id'; import { of as observableOf } from 'rxjs'; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; import { Observable } from 'rxjs/internal/Observable'; +import { AuthService } from '../../../auth/auth.service'; /** * Test implementation of abstract class FeatureAuthorizationGuard @@ -12,10 +13,11 @@ import { Observable } from 'rxjs/internal/Observable'; class FeatureAuthorizationGuardImpl extends FeatureAuthorizationGuard { constructor(protected authorizationService: AuthorizationDataService, protected router: Router, + protected authService: AuthService, protected featureId: FeatureID, protected objectUrl: string, protected ePersonUuid: string) { - super(authorizationService, router); + super(authorizationService, router, authService); } getFeatureID(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { @@ -35,6 +37,7 @@ describe('FeatureAuthorizationGuard', () => { let guard: FeatureAuthorizationGuard; let authorizationService: AuthorizationDataService; let router: Router; + let authService: AuthService; let featureId: FeatureID; let objectUrl: string; @@ -51,7 +54,10 @@ describe('FeatureAuthorizationGuard', () => { router = jasmine.createSpyObj('router', { parseUrl: {} }); - guard = new FeatureAuthorizationGuardImpl(authorizationService, router, featureId, objectUrl, ePersonUuid); + authService = jasmine.createSpyObj('authService', { + isAuthenticated: observableOf(true) + }); + guard = new FeatureAuthorizationGuardImpl(authorizationService, router, authService, featureId, objectUrl, ePersonUuid); } beforeEach(() => { @@ -60,7 +66,7 @@ describe('FeatureAuthorizationGuard', () => { describe('canActivate', () => { it('should call authorizationService.isAuthenticated with the appropriate arguments', () => { - guard.canActivate(undefined, undefined).subscribe(); + guard.canActivate(undefined, { url: 'current-url' } as any).subscribe(); 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 d53e71e289..851c4127d8 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 @@ -8,9 +8,10 @@ import { import { AuthorizationDataService } from '../authorization-data.service'; import { FeatureID } from '../feature-id'; import { Observable } from 'rxjs/internal/Observable'; -import { returnUnauthorizedUrlTreeOnFalse } from '../../../shared/operators'; +import { returnForbiddenUrlTreeOrLoginOnFalse } from '../../../shared/operators'; import { combineLatest as observableCombineLatest, of as observableOf } from 'rxjs'; import { switchMap } from 'rxjs/operators'; +import { AuthService } from '../../../auth/auth.service'; /** * Abstract Guard for preventing unauthorized activating and loading of routes when a user @@ -19,7 +20,8 @@ import { switchMap } from 'rxjs/operators'; */ export abstract class FeatureAuthorizationGuard implements CanActivate { constructor(protected authorizationService: AuthorizationDataService, - protected router: Router) { + protected router: Router, + protected authService: AuthService) { } /** @@ -29,7 +31,7 @@ export abstract class FeatureAuthorizationGuard implements CanActivate { canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable { return observableCombineLatest(this.getFeatureID(route, state), this.getObjectUrl(route, state), this.getEPersonUuid(route, state)).pipe( switchMap(([featureID, objectUrl, ePersonUuid]) => this.authorizationService.isAuthorized(featureID, objectUrl, ePersonUuid)), - returnUnauthorizedUrlTreeOnFalse(this.router) + returnForbiddenUrlTreeOrLoginOnFalse(this.router, this.authService, state.url) ); } 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 a45049645a..41b4f06b54 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 @@ -5,6 +5,7 @@ import { AuthorizationDataService } from '../authorization-data.service'; import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router'; import { of as observableOf } from 'rxjs'; import { Observable } from 'rxjs/internal/Observable'; +import { AuthService } from '../../../auth/auth.service'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have administrator @@ -14,8 +15,8 @@ import { Observable } from 'rxjs/internal/Observable'; providedIn: 'root' }) export class SiteAdministratorGuard extends FeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, protected router: Router) { - super(authorizationService, router); + constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { + super(authorizationService, router, authService); } /** diff --git a/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts b/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts index 18397cf71e..147e0617ca 100644 --- a/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts +++ b/src/app/core/data/feature-authorization/feature-authorization-guard/site-register.guard.ts @@ -5,6 +5,7 @@ import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/ro import { Observable } from 'rxjs/internal/Observable'; import { FeatureID } from '../feature-id'; import { of as observableOf } from 'rxjs'; +import { AuthService } from '../../../auth/auth.service'; /** * Prevent unauthorized activating and loading of routes when the current authenticated user doesn't have registration @@ -14,8 +15,8 @@ import { of as observableOf } from 'rxjs'; providedIn: 'root' }) export class SiteRegisterGuard extends FeatureAuthorizationGuard { - constructor(protected authorizationService: AuthorizationDataService, protected router: Router) { - super(authorizationService, router); + constructor(protected authorizationService: AuthorizationDataService, protected router: Router, protected authService: AuthService) { + super(authorizationService, router, authService); } /** diff --git a/src/app/core/shared/operators.spec.ts b/src/app/core/shared/operators.spec.ts index cc25aeec10..91a8029617 100644 --- a/src/app/core/shared/operators.spec.ts +++ b/src/app/core/shared/operators.spec.ts @@ -226,13 +226,6 @@ describe('Core Module - RxJS Operators', () => { expect(router.navigateByUrl).toHaveBeenCalledWith('/403', { skipLocationChange: true }); }); - it('should call navigateByUrl to a 401 page, when the remote data contains a 401 error', () => { - const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(401, 'Unauthorized', 'The current user is unauthorized')); - - observableOf(testRD).pipe(redirectOn4xx(router, authService)).subscribe(); - expect(router.navigateByUrl).toHaveBeenCalledWith('/401', { skipLocationChange: true }); - }); - it('should not call navigateByUrl to a 404, 403 or 401 page, when the remote data contains another error than a 404, 403 or 401', () => { const testRD = createFailedRemoteDataObject(undefined, new RemoteDataError(500, 'Server Error', 'Something went wrong')); diff --git a/src/app/core/shared/operators.ts b/src/app/core/shared/operators.ts index 78fef701a1..e98a0cee7b 100644 --- a/src/app/core/shared/operators.ts +++ b/src/app/core/shared/operators.ts @@ -13,7 +13,7 @@ import { MetadataField } from '../metadata/metadata-field.model'; import { MetadataSchema } from '../metadata/metadata-schema.model'; import { BrowseDefinition } from './browse-definition.model'; import { DSpaceObject } from './dspace-object.model'; -import { getForbiddenRoute, getPageNotFoundRoute, getUnauthorizedRoute } from '../../app-routing-paths'; +import { getForbiddenRoute, getPageNotFoundRoute } from '../../app-routing-paths'; import { getEndUserAgreementPath } from '../../info/info-routing-paths'; import { AuthService } from '../auth/auth.service'; @@ -190,11 +190,7 @@ export const redirectOn4xx = (router: Router, authService: AuthService) => router.navigateByUrl(getPageNotFoundRoute(), {skipLocationChange: true}); } else if (rd.error.statusCode === 403 || rd.error.statusCode === 401) { if (isAuthenticated) { - if (rd.error.statusCode === 403) { - router.navigateByUrl(getForbiddenRoute(), {skipLocationChange: true}); - } else if (rd.error.statusCode === 401) { - router.navigateByUrl(getUnauthorizedRoute(), {skipLocationChange: true}); - } + router.navigateByUrl(getForbiddenRoute(), {skipLocationChange: true}); } else { authService.setRedirectUrl(router.url); router.navigateByUrl('login'); @@ -205,14 +201,25 @@ export const redirectOn4xx = (router: Router, authService: AuthService) => })); /** - * Operator that returns a UrlTree to the unauthorized page when the boolean received is false - * @param router + * Operator that returns a UrlTree to a forbidden page or the login page when the boolean received is false + * @param router The router used to navigate to a forbidden page + * @param authService The AuthService used to determine whether or not the user is logged in + * @param redirectUrl The URL to redirect back to after logging in */ -export const returnUnauthorizedUrlTreeOnFalse = (router: Router) => +export const returnForbiddenUrlTreeOrLoginOnFalse = (router: Router, authService: AuthService, redirectUrl: string) => (source: Observable): Observable => - source.pipe( - map((authorized: boolean) => { - return authorized ? authorized : router.parseUrl(getUnauthorizedRoute()) + observableCombineLatest(source, authService.isAuthenticated()).pipe( + map(([authorized, authenticated]: [boolean, boolean]) => { + if (authorized) { + return authorized; + } else { + if (authenticated) { + return router.parseUrl(getForbiddenRoute()); + } else { + authService.setRedirectUrl(redirectUrl); + return router.parseUrl('login'); + } + } })); /** diff --git a/src/app/unauthorized/unauthorized.component.html b/src/app/unauthorized/unauthorized.component.html deleted file mode 100644 index a25e271b77..0000000000 --- a/src/app/unauthorized/unauthorized.component.html +++ /dev/null @@ -1,10 +0,0 @@ -
-

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 deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/src/app/unauthorized/unauthorized.component.ts b/src/app/unauthorized/unauthorized.component.ts deleted file mode 100644 index 280a1ae947..0000000000 --- a/src/app/unauthorized/unauthorized.component.ts +++ /dev/null @@ -1,32 +0,0 @@ -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(); - } - -}