diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index 7f2c1e29cc..ccc77bd413 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -27,6 +27,7 @@ import { EPersonDataService } from '../eperson/eperson-data.service'; import { createSuccessfulRemoteDataObject$ } from '../../shared/remote-data.utils'; import { authMethodsMock } from '../../shared/testing/auth-service.stub'; import { AuthMethod } from './models/auth.method'; +import { HardRedirectService } from '../services/hard-redirect.service'; describe('AuthService test', () => { @@ -48,6 +49,7 @@ describe('AuthService test', () => { let authenticatedState; let unAuthenticatedState; let linkService; + let hardRedirectService; function init() { mockStore = jasmine.createSpyObj('store', { @@ -77,6 +79,7 @@ describe('AuthService test', () => { linkService = { resolveLinks: {} }; + hardRedirectService = jasmine.createSpyObj('hardRedirectService', ['redirect']); spyOn(linkService, 'resolveLinks').and.returnValue({ authenticated: true, eperson: observableOf({ payload: {} }) }); } @@ -104,6 +107,7 @@ describe('AuthService test', () => { { provide: ActivatedRoute, useValue: routeStub }, { provide: Store, useValue: mockStore }, { provide: EPersonDataService, useValue: mockEpersonDataService }, + { provide: HardRedirectService, useValue: hardRedirectService }, CookieService, AuthService ], @@ -210,7 +214,7 @@ describe('AuthService test', () => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store); + authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService); })); it('should return true when user is logged in', () => { @@ -289,7 +293,7 @@ describe('AuthService test', () => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store); + authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService); storage = (authService as any).storage; routeServiceMock = TestBed.get(RouteService); routerStub = TestBed.get(Router); @@ -320,34 +324,34 @@ describe('AuthService test', () => { it('should set redirect url to previous page', () => { spyOn(routeServiceMock, 'getHistory').and.callThrough(); - spyOn(routerStub, 'navigateByUrl'); authService.redirectAfterLoginSuccess(true); expect(routeServiceMock.getHistory).toHaveBeenCalled(); - expect(routerStub.navigateByUrl).toHaveBeenCalledWith('/collection/123'); + // Reload with redirect URL set to /collection/123 + expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('/reload/[0-9]*\\?redirect=' + encodeURIComponent('/collection/123')))); }); it('should set redirect url to current page', () => { spyOn(routeServiceMock, 'getHistory').and.callThrough(); - spyOn(routerStub, 'navigateByUrl'); authService.redirectAfterLoginSuccess(false); expect(routeServiceMock.getHistory).toHaveBeenCalled(); - expect(routerStub.navigateByUrl).toHaveBeenCalledWith('/home'); + // Reload with redirect URL set to /home + expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('/reload/[0-9]*\\?redirect=' + encodeURIComponent('/home')))); }); it('should redirect to / and not to /login', () => { spyOn(routeServiceMock, 'getHistory').and.returnValue(observableOf(['/login', '/login'])); - spyOn(routerStub, 'navigateByUrl'); authService.redirectAfterLoginSuccess(true); expect(routeServiceMock.getHistory).toHaveBeenCalled(); - expect(routerStub.navigateByUrl).toHaveBeenCalledWith('/'); + // Reload without a redirect URL + expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('/reload/[0-9]*(?!\\?)$'))); }); it('should redirect to / when no redirect url is found', () => { spyOn(routeServiceMock, 'getHistory').and.returnValue(observableOf([''])); - spyOn(routerStub, 'navigateByUrl'); authService.redirectAfterLoginSuccess(true); expect(routeServiceMock.getHistory).toHaveBeenCalled(); - expect(routerStub.navigateByUrl).toHaveBeenCalledWith('/'); + // Reload without a redirect URL + expect(hardRedirectService.redirect).toHaveBeenCalledWith(jasmine.stringMatching(new RegExp('/reload/[0-9]*(?!\\?)$'))); }); describe('impersonate', () => { @@ -464,6 +468,14 @@ describe('AuthService test', () => { }); }); }); + + describe('refreshAfterLogout', () => { + it('should call navigateToRedirectUrl with no url', () => { + spyOn(authService as any, 'navigateToRedirectUrl').and.stub(); + authService.refreshAfterLogout(); + expect((authService as any).navigateToRedirectUrl).toHaveBeenCalled(); + }); + }); }); describe('when user is not logged in', () => { @@ -496,7 +508,7 @@ describe('AuthService test', () => { (state as any).core = Object.create({}); (state as any).core.auth = unAuthenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store); + authService = new AuthService({}, window, undefined, authReqService, mockEpersonDataService, router, routeService, cookieService, store, hardRedirectService); })); it('should return null for the shortlived token', () => { diff --git a/src/app/core/reload/reload.guard.spec.ts b/src/app/core/reload/reload.guard.spec.ts new file mode 100644 index 0000000000..317245bafa --- /dev/null +++ b/src/app/core/reload/reload.guard.spec.ts @@ -0,0 +1,47 @@ +import { ReloadGuard } from './reload.guard'; +import { Router } from '@angular/router'; + +describe('ReloadGuard', () => { + let guard: ReloadGuard; + let router: Router; + + beforeEach(() => { + router = jasmine.createSpyObj('router', ['parseUrl', 'createUrlTree']); + guard = new ReloadGuard(router); + }); + + describe('canActivate', () => { + let route; + + describe('when the route\'s query params contain a redirect url', () => { + let redirectUrl; + + beforeEach(() => { + redirectUrl = '/redirect/url?param=extra'; + route = { + queryParams: { + redirect: redirectUrl + } + }; + }); + + it('should create a UrlTree with the redirect URL', () => { + guard.canActivate(route, undefined); + expect(router.parseUrl).toHaveBeenCalledWith(redirectUrl); + }); + }); + + describe('when the route\'s query params doesn\'t contain a redirect url', () => { + beforeEach(() => { + route = { + queryParams: {} + }; + }); + + it('should create a UrlTree to home', () => { + guard.canActivate(route, undefined); + expect(router.createUrlTree).toHaveBeenCalledWith(['home']); + }); + }); + }); +}); diff --git a/src/app/core/reload/reload.guard.ts b/src/app/core/reload/reload.guard.ts index 9880fabb69..78f9dcf642 100644 --- a/src/app/core/reload/reload.guard.ts +++ b/src/app/core/reload/reload.guard.ts @@ -2,11 +2,20 @@ import { ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot, UrlTr import { Injectable } from '@angular/core'; import { isNotEmpty } from '../../shared/empty.util'; +/** + * A guard redirecting the user to the URL provided in the route's query params + * When no redirect url is found, the user is redirected to the homepage + */ @Injectable() export class ReloadGuard implements CanActivate { constructor(private router: Router) { } + /** + * Get the UrlTree of the URL to redirect to + * @param route + * @param state + */ canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): UrlTree { if (isNotEmpty(route.queryParams.redirect)) { return this.router.parseUrl(route.queryParams.redirect); diff --git a/src/app/core/services/browser-hard-redirect.service.spec.ts b/src/app/core/services/browser-hard-redirect.service.spec.ts new file mode 100644 index 0000000000..b94b52d46e --- /dev/null +++ b/src/app/core/services/browser-hard-redirect.service.spec.ts @@ -0,0 +1,40 @@ +import {async, TestBed} from '@angular/core/testing'; +import {BrowserHardRedirectService} from './browser-hard-redirect.service'; + +describe('BrowserHardRedirectService', () => { + + const mockLocation = { + href: undefined, + origin: 'test origin', + } as Location; + + const service: BrowserHardRedirectService = new BrowserHardRedirectService(mockLocation); + + beforeEach(() => { + TestBed.configureTestingModule({}); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); + + describe('when performing a redirect', () => { + + const redirect = 'test redirect'; + + beforeEach(() => { + service.redirect(redirect); + }); + + it('should update the location', () => { + expect(mockLocation.href).toEqual(redirect); + }) + }); + + describe('when requesting the origin', () => { + + it('should return the location origin', () => { + expect(service.getOriginFromUrl()).toEqual('test origin'); + }); + }); +}); diff --git a/src/app/core/services/browser-hard-redirect.service.ts b/src/app/core/services/browser-hard-redirect.service.ts index 56c9054297..71ce7577d2 100644 --- a/src/app/core/services/browser-hard-redirect.service.ts +++ b/src/app/core/services/browser-hard-redirect.service.ts @@ -1,6 +1,9 @@ import {Inject, Injectable} from '@angular/core'; import {LocationToken} from '../../../modules/app/browser-app.module'; +/** + * Service for performing hard redirects within the browser app module + */ @Injectable() export class BrowserHardRedirectService { @@ -9,10 +12,17 @@ export class BrowserHardRedirectService { ) { } + /** + * Perform a hard redirect to URL + * @param url + */ redirect(url: string) { this.location.href = url; } + /** + * Get the origin of a request + */ getOriginFromUrl() { return this.location.origin; } diff --git a/src/app/core/services/server-hard-redirect.service.spec.ts b/src/app/core/services/server-hard-redirect.service.spec.ts new file mode 100644 index 0000000000..9704c64c74 --- /dev/null +++ b/src/app/core/services/server-hard-redirect.service.spec.ts @@ -0,0 +1,48 @@ +import { TestBed } from '@angular/core/testing'; +import { ServerHardRedirectService } from './server-hard-redirect.service'; + +describe('ServerHardRedirectService', () => { + + const mockRequest = jasmine.createSpyObj(['get']); + const mockResponse = jasmine.createSpyObj(['redirect', 'end']); + + const service: ServerHardRedirectService = new ServerHardRedirectService(mockRequest, mockResponse); + + beforeEach(() => { + TestBed.configureTestingModule({}); + }); + + it('should be created', () => { + expect(service).toBeTruthy(); + }); + + describe('when performing a redirect', () => { + + const redirect = 'test redirect'; + + beforeEach(() => { + service.redirect(redirect); + }); + + it('should update the response object', () => { + expect(mockResponse.redirect).toHaveBeenCalledWith(302, redirect); + expect(mockResponse.end).toHaveBeenCalled(); + }) + }); + + describe('when requesting the origin', () => { + + beforeEach(() => { + mockRequest.protocol = 'test-protocol'; + mockRequest.get.and.callFake((name) => { + if (name === 'hostname') { + return 'test-host'; + } + }); + }); + + it('should return the location origin', () => { + expect(service.getOriginFromUrl()).toEqual('test-protocol://test-host'); + }); + }); +}); diff --git a/src/app/core/services/server-hard-redirect.service.ts b/src/app/core/services/server-hard-redirect.service.ts index 4d58443cf4..69b6739445 100644 --- a/src/app/core/services/server-hard-redirect.service.ts +++ b/src/app/core/services/server-hard-redirect.service.ts @@ -2,6 +2,9 @@ import { Inject, Injectable } from '@angular/core'; import { Request, Response } from 'express'; import { REQUEST, RESPONSE } from '@nguniversal/express-engine/tokens'; +/** + * Service for performing hard redirects within the server app module + */ @Injectable() export class ServerHardRedirectService { @@ -11,6 +14,10 @@ export class ServerHardRedirectService { ) { } + /** + * Perform a hard redirect to URL + * @param url + */ redirect(url: string) { if (url === this.req.url) { @@ -45,6 +52,9 @@ export class ServerHardRedirectService { } } + /** + * Get the origin of a request + */ getOriginFromUrl() { return new URL(`${this.req.protocol}://${this.req.get('hostname')}`).toString();