diff --git a/src/app/lookup-by-id/lookup-guard.spec.ts b/src/app/lookup-by-id/lookup-guard.spec.ts index b3a5ad6ccdc..d4b6d5509cb 100644 --- a/src/app/lookup-by-id/lookup-guard.spec.ts +++ b/src/app/lookup-by-id/lookup-guard.spec.ts @@ -1,18 +1,60 @@ -import { of } from 'rxjs'; +import { TestBed } from '@angular/core/testing'; +import { + Router, + UrlTree, +} from '@angular/router'; +import { + BehaviorSubject, + Observable, + of, +} from 'rxjs'; +import { take } from 'rxjs/operators'; +import { AuthService } from '../core/auth/auth.service'; +import { DsoRedirectService } from '../core/data/dso-redirect.service'; import { IdentifierType } from '../core/data/request.models'; +import { ServerResponseService } from '../core/services/server-response.service'; +import { + createFailedRemoteDataObject, + createSuccessfulRemoteDataObject, +} from '../shared/remote-data.utils'; import { lookupGuard } from './lookup-guard'; describe('lookupGuard', () => { let dsoService: any; + let authService: any; + let router: any; + let serverResponseService: any; + // the guard is typed as CanActivateFn, so its injected parameters can only be passed positionally through `any` let guard: any; + let forbiddenUrlTree: UrlTree; + let loginUrlTree: UrlTree; + + const state: any = { url: '/handle/123456789/1234' }; + const handleRoute: any = { + params: { + id: '1234', + idType: '123456789', + }, + }; beforeEach(() => { dsoService = { - findByIdAndIDType: jasmine.createSpy('findByIdAndIDType').and.returnValue(of({ hasFailed: false, - hasSucceeded: true })), + findByIdAndIDType: jasmine.createSpy('findByIdAndIDType') + .and.returnValue(of(createSuccessfulRemoteDataObject(undefined))), }; - guard = lookupGuard; + authService = jasmine.createSpyObj('authService', { + // the real AuthService returns a store selector, which never completes + isAuthenticated: new BehaviorSubject(false), + setRedirectUrl: {}, + }); + forbiddenUrlTree = new UrlTree(); + loginUrlTree = new UrlTree(); + router = jasmine.createSpyObj('router', ['parseUrl']); + router.parseUrl.and.callFake((url: string) => url === '/403' ? forbiddenUrlTree : loginUrlTree); + serverResponseService = jasmine.createSpyObj('serverResponseService', ['setStatus']); + guard = (route: any, routerState: any): Observable => + (lookupGuard as any)(route, routerState, dsoService, authService, router, serverResponseService); }); it('should call findByIdAndIDType with handle params', () => { @@ -22,18 +64,18 @@ describe('lookupGuard', () => { idType: '123456789', }, }; - guard(scopedRoute as any, undefined, dsoService); + guard(scopedRoute, state); expect(dsoService.findByIdAndIDType).toHaveBeenCalledWith('hdl:123456789/1234', IdentifierType.HANDLE); }); - it('should call findByIdAndIDType with handle params', () => { + it('should call findByIdAndIDType with encoded handle params', () => { const scopedRoute = { params: { id: '123456789%2F1234', idType: 'handle', }, }; - guard(scopedRoute as any, undefined, dsoService); + guard(scopedRoute, state); expect(dsoService.findByIdAndIDType).toHaveBeenCalledWith('hdl:123456789%2F1234', IdentifierType.HANDLE); }); @@ -44,8 +86,193 @@ describe('lookupGuard', () => { idType: 'uuid', }, }; - guard(scopedRoute as any, undefined, dsoService); + guard(scopedRoute, state); expect(dsoService.findByIdAndIDType).toHaveBeenCalledWith('34cfed7c-f597-49ef-9cbe-ea351f0023c2', IdentifierType.UUID); }); + it('should resolve its dependencies from the injector when they are not passed in', (done) => { + TestBed.configureTestingModule({ + providers: [ + { provide: DsoRedirectService, useValue: dsoService }, + { provide: AuthService, useValue: authService }, + { provide: Router, useValue: router }, + { provide: ServerResponseService, useValue: serverResponseService }, + ], + }); + + const result = TestBed.runInInjectionContext(() => lookupGuard(handleRoute, state)) as Observable; + + expect(dsoService.findByIdAndIDType).toHaveBeenCalledWith('hdl:123456789/1234', IdentifierType.HANDLE); + result.subscribe((activate) => { + expect(activate).toBeFalse(); + done(); + }); + }); + + describe('when the object was found', () => { + it('should return false so the ObjectNotFound page is not shown', (done) => { + guard(handleRoute, state).subscribe((result) => { + expect(result).toBeFalse(); + expect(serverResponseService.setStatus).not.toHaveBeenCalled(); + done(); + }); + }); + }); + + describe('when the lookup fails with a 404', () => { + beforeEach(() => { + dsoService.findByIdAndIDType.and.returnValue(of(createFailedRemoteDataObject('Not found', 404))); + }); + + it('should return true so the ObjectNotFound page is shown', (done) => { + guard(handleRoute, state).subscribe((result) => { + expect(result).toBeTrue(); + expect(authService.setRedirectUrl).not.toHaveBeenCalled(); + expect(router.parseUrl).not.toHaveBeenCalled(); + expect(serverResponseService.setStatus).not.toHaveBeenCalled(); + done(); + }); + }); + }); + + // 501 is what the identifier endpoint answers for an unresolvable identifier type; 422 never + // reaches this guard, but the fallback must treat every non-401/403 status the same way + [501, 422, 500].forEach((statusCode: number) => { + describe(`when the lookup fails with a ${statusCode}`, () => { + beforeEach(() => { + dsoService.findByIdAndIDType.and.returnValue(of(createFailedRemoteDataObject('Failed', statusCode))); + }); + + it('should return true so the ObjectNotFound page is shown', (done) => { + guard(handleRoute, state).subscribe((result) => { + expect(result).toBeTrue(); + expect(authService.setRedirectUrl).not.toHaveBeenCalled(); + expect(router.parseUrl).not.toHaveBeenCalled(); + expect(serverResponseService.setStatus).not.toHaveBeenCalled(); + done(); + }); + }); + }); + }); + + describe('when the lookup fails without a status code', () => { + beforeEach(() => { + dsoService.findByIdAndIDType.and.returnValue(of(createFailedRemoteDataObject('Network error', undefined))); + }); + + it('should return true so the ObjectNotFound page is shown', (done) => { + guard(handleRoute, state).subscribe((result) => { + expect(result).toBeTrue(); + expect(router.parseUrl).not.toHaveBeenCalled(); + done(); + }); + }); + }); + + describe('when the lookup fails with a 401 and the user is not authenticated', () => { + beforeEach(() => { + dsoService.findByIdAndIDType.and.returnValue(of(createFailedRemoteDataObject('Unauthorized', 401))); + authService.isAuthenticated.and.returnValue(new BehaviorSubject(false)); + }); + + it('should store the requested url and return a UrlTree to the login page', (done) => { + guard(handleRoute, state).subscribe((result) => { + expect(authService.setRedirectUrl).toHaveBeenCalledWith(state.url); + expect(router.parseUrl).toHaveBeenCalledWith('login'); + expect(result).toBe(loginUrlTree); + done(); + }); + }); + + it('should set the server response status so the page is not cached as a 200', (done) => { + guard(handleRoute, state).subscribe(() => { + expect(serverResponseService.setStatus).toHaveBeenCalledWith(401); + done(); + }); + }); + + it('should emit exactly once and complete even though isAuthenticated() never completes', (done) => { + let emissions = 0; + guard(handleRoute, state).pipe(take(2)).subscribe({ + next: (result) => { + emissions++; + expect(result).toBe(loginUrlTree); + }, + complete: () => { + expect(emissions).toBe(1); + done(); + }, + }); + }); + }); + + describe('when the lookup fails with a 401 and the user is authenticated', () => { + beforeEach(() => { + dsoService.findByIdAndIDType.and.returnValue(of(createFailedRemoteDataObject('Unauthorized', 401))); + authService.isAuthenticated.and.returnValue(new BehaviorSubject(true)); + }); + + it('should return a UrlTree to the forbidden page', (done) => { + guard(handleRoute, state).subscribe((result) => { + expect(authService.setRedirectUrl).not.toHaveBeenCalled(); + expect(router.parseUrl).toHaveBeenCalledWith('/403'); + expect(result).toBe(forbiddenUrlTree); + done(); + }); + }); + + it('should set the server response status so the page is not cached as a 200', (done) => { + guard(handleRoute, state).subscribe(() => { + expect(serverResponseService.setStatus).toHaveBeenCalledWith(401); + done(); + }); + }); + }); + + describe('when the lookup fails with a 403 and the user is not authenticated', () => { + beforeEach(() => { + dsoService.findByIdAndIDType.and.returnValue(of(createFailedRemoteDataObject('Forbidden', 403))); + authService.isAuthenticated.and.returnValue(new BehaviorSubject(false)); + }); + + it('should store the requested url and return a UrlTree to the login page', (done) => { + guard(handleRoute, state).subscribe((result) => { + expect(authService.setRedirectUrl).toHaveBeenCalledWith(state.url); + expect(router.parseUrl).toHaveBeenCalledWith('login'); + expect(result).toBe(loginUrlTree); + done(); + }); + }); + + it('should set the server response status so the page is not cached as a 200', (done) => { + guard(handleRoute, state).subscribe(() => { + expect(serverResponseService.setStatus).toHaveBeenCalledWith(403); + done(); + }); + }); + }); + + describe('when the lookup fails with a 403 and the user is authenticated', () => { + beforeEach(() => { + dsoService.findByIdAndIDType.and.returnValue(of(createFailedRemoteDataObject('Forbidden', 403))); + authService.isAuthenticated.and.returnValue(new BehaviorSubject(true)); + }); + + it('should return a UrlTree to the forbidden page and not touch the redirect url', (done) => { + guard(handleRoute, state).subscribe((result) => { + expect(authService.setRedirectUrl).not.toHaveBeenCalled(); + expect(router.parseUrl).toHaveBeenCalledWith('/403'); + expect(result).toBe(forbiddenUrlTree); + done(); + }); + }); + + it('should set the server response status so the page is not cached as a 200', (done) => { + guard(handleRoute, state).subscribe(() => { + expect(serverResponseService.setStatus).toHaveBeenCalledWith(403); + done(); + }); + }); + }); + }); diff --git a/src/app/lookup-by-id/lookup-guard.ts b/src/app/lookup-by-id/lookup-guard.ts index 25813d28ff5..6ea42567540 100644 --- a/src/app/lookup-by-id/lookup-guard.ts +++ b/src/app/lookup-by-id/lookup-guard.ts @@ -2,14 +2,25 @@ import { inject } from '@angular/core'; import { ActivatedRouteSnapshot, CanActivateFn, + Router, RouterStateSnapshot, + UrlTree, } from '@angular/router'; -import { Observable } from 'rxjs'; -import { map } from 'rxjs/operators'; +import { + Observable, + of, +} from 'rxjs'; +import { + switchMap, + take, +} from 'rxjs/operators'; +import { AuthService } from '../core/auth/auth.service'; import { DsoRedirectService } from '../core/data/dso-redirect.service'; import { RemoteData } from '../core/data/remote-data'; import { IdentifierType } from '../core/data/request.models'; +import { ServerResponseService } from '../core/services/server-response.service'; +import { returnForbiddenUrlTreeOrLoginOnFalse } from '../core/shared/authorized.operators'; import { DSpaceObject } from '../core/shared/dspace-object.model'; interface LookupParams { @@ -21,10 +32,27 @@ export const lookupGuard: CanActivateFn = ( route: ActivatedRouteSnapshot, state: RouterStateSnapshot, dsoService: DsoRedirectService = inject(DsoRedirectService), -): Observable => { + authService: AuthService = inject(AuthService), + router: Router = inject(Router), + serverResponseService: ServerResponseService = inject(ServerResponseService), +): Observable => { const params = getLookupParams(route); return dsoService.findByIdAndIDType(params.id, params.type).pipe( - map((response: RemoteData) => response.hasFailed), + switchMap((response: RemoteData) => { + // A restricted object, which REST reports as 401/403 rather than 404 + if (response.hasFailed && (response.statusCode === 401 || response.statusCode === 403)) { + // or SSR would cache the login page as HTTP 200 under the identifier's URL. No-op in the browser. + serverResponseService.setStatus(response.statusCode); + // `false` = not authorized: login page for anonymous users, /403 for authenticated ones, the + // same split /items/:id makes. take(1) because isAuthenticated() never completes. + return of(false).pipe( + returnForbiddenUrlTreeOrLoginOnFalse(router, authService, state.url), + take(1), + ); + } + // Any other failure (404, 501, 5xx) activates the route so ObjectNotFoundComponent renders + return of(response.hasFailed); + }), ); };