diff --git a/src/app/+login-page/login-page.component.html b/src/app/+login-page/login-page.component.html index 6dcb11fbb0..84059877f4 100644 --- a/src/app/+login-page/login-page.component.html +++ b/src/app/+login-page/login-page.component.html @@ -3,7 +3,8 @@

{{"login.form.header" | translate}}

- +
diff --git a/src/app/core/auth/auth.service.spec.ts b/src/app/core/auth/auth.service.spec.ts index ab2e6fd86b..5084dc8596 100644 --- a/src/app/core/auth/auth.service.spec.ts +++ b/src/app/core/auth/auth.service.spec.ts @@ -23,11 +23,14 @@ import { AppState } from '../../app.reducer'; import { ClientCookieService } from '../services/client-cookie.service'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; import { getMockRemoteDataBuildService } from '../../shared/mocks/mock-remote-data-build.service'; +import { routeServiceStub } from '../../shared/testing/route-service-stub'; +import { RouteService } from '../services/route.service'; describe('AuthService test', () => { let mockStore: Store; let authService: AuthService; + let routeServiceMock: RouteService; let authRequest; let window; let routerStub; @@ -74,6 +77,7 @@ describe('AuthService test', () => { { provide: NativeWindowService, useValue: window }, { provide: REQUEST, useValue: {} }, { provide: Router, useValue: routerStub }, + { provide: RouteService, useValue: routeServiceStub }, { provide: ActivatedRoute, useValue: routeStub }, { provide: Store, useValue: mockStore }, { provide: RemoteDataBuildService, useValue: rdbService }, @@ -138,6 +142,7 @@ describe('AuthService test', () => { { provide: AuthRequestService, useValue: authRequest }, { provide: REQUEST, useValue: {} }, { provide: Router, useValue: routerStub }, + { provide: RouteService, useValue: routeServiceStub }, { provide: RemoteDataBuildService, useValue: rdbService }, CookieService, AuthService @@ -145,13 +150,13 @@ describe('AuthService test', () => { }).compileComponents(); })); - beforeEach(inject([CookieService, AuthRequestService, Store, Router], (cookieService: CookieService, authReqService: AuthRequestService, store: Store, router: Router) => { + beforeEach(inject([CookieService, AuthRequestService, Store, Router, RouteService], (cookieService: CookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService) => { store .subscribe((state) => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, router, cookieService, store, rdbService); + authService = new AuthService({}, window, undefined, authReqService, router, routeService, cookieService, store, rdbService); })); it('should return true when user is logged in', () => { @@ -189,6 +194,7 @@ describe('AuthService test', () => { { provide: AuthRequestService, useValue: authRequest }, { provide: REQUEST, useValue: {} }, { provide: Router, useValue: routerStub }, + { provide: RouteService, useValue: routeServiceStub }, { provide: RemoteDataBuildService, useValue: rdbService }, ClientCookieService, CookieService, @@ -197,7 +203,7 @@ describe('AuthService test', () => { }).compileComponents(); })); - beforeEach(inject([ClientCookieService, AuthRequestService, Store, Router], (cookieService: ClientCookieService, authReqService: AuthRequestService, store: Store, router: Router) => { + beforeEach(inject([ClientCookieService, AuthRequestService, Store, Router, RouteService], (cookieService: ClientCookieService, authReqService: AuthRequestService, store: Store, router: Router, routeService: RouteService) => { const expiredToken: AuthTokenInfo = new AuthTokenInfo('test_token'); expiredToken.expires = Date.now() - (1000 * 60 * 60); authenticatedState = { @@ -212,11 +218,14 @@ describe('AuthService test', () => { (state as any).core = Object.create({}); (state as any).core.auth = authenticatedState; }); - authService = new AuthService({}, window, undefined, authReqService, router, cookieService, store, rdbService); + authService = new AuthService({}, window, undefined, authReqService, router, routeService, cookieService, store, rdbService); storage = (authService as any).storage; + routeServiceMock = TestBed.get(RouteService); + routerStub = TestBed.get(Router); spyOn(storage, 'get'); spyOn(storage, 'remove'); spyOn(storage, 'set'); + })); it('should throw false when token is not valid', () => { @@ -238,5 +247,32 @@ describe('AuthService test', () => { expect(storage.remove).toHaveBeenCalled(); }); + it ('should set redirect url to previous page', () => { + spyOn(routeServiceMock, 'getHistory').and.callThrough(); + authService.redirectAfterLoginSuccess(true); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(routerStub.navigate).toHaveBeenCalledWith(['/collection/123']); + }); + + it ('should set redirect url to current page', () => { + spyOn(routeServiceMock, 'getHistory').and.callThrough(); + authService.redirectAfterLoginSuccess(false); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(routerStub.navigate).toHaveBeenCalledWith(['/home']); + }); + + it ('should redirect to / and not to /login', () => { + spyOn(routeServiceMock, 'getHistory').and.returnValue(observableOf(['/login', '/login'])); + authService.redirectAfterLoginSuccess(true); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(routerStub.navigate).toHaveBeenCalledWith(['/']); + }); + + it ('should redirect to / when no redirect url is found', () => { + spyOn(routeServiceMock, 'getHistory').and.returnValue(observableOf([''])); + authService.redirectAfterLoginSuccess(true); + expect(routeServiceMock.getHistory).toHaveBeenCalled(); + expect(routerStub.navigate).toHaveBeenCalledWith(['/']); + }); }); }); diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 08c94b02f2..5287e537ee 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -22,6 +22,7 @@ import { ResetAuthenticationMessagesAction, SetRedirectUrlAction } from './auth. import { NativeWindowRef, NativeWindowService } from '../services/window.service'; import { Base64EncodeUrl } from '../../shared/utils/encode-decode.util'; import { RemoteDataBuildService } from '../cache/builders/remote-data-build.service'; +import {RouteService} from '../services/route.service'; export const LOGIN_ROUTE = '/login'; export const LOGOUT_ROUTE = '/logout'; @@ -45,6 +46,7 @@ export class AuthService { protected authRequestService: AuthRequestService, @Optional() @Inject(RESPONSE) private response: any, protected router: Router, + protected routeService: RouteService, protected storage: CookieService, protected store: Store, protected rdbService: RemoteDataBuildService @@ -337,7 +339,7 @@ export class AuthService { /** * Redirect to the route navigated before the login */ - public redirectToPreviousUrl() { + public redirectAfterLoginSuccess(isStandalonePage: boolean) { this.getRedirectUrl().pipe( take(1)) .subscribe((redirectUrl) => { @@ -346,18 +348,39 @@ export class AuthService { this.clearRedirectUrl(); this.router.onSameUrlNavigation = 'reload'; const url = decodeURIComponent(redirectUrl); - this.router.navigateByUrl(url); - /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ - // this._window.nativeWindow.location.href = url; + this.navigateToRedirectUrl(url); } else { - this.router.navigate(['/']); - /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ - // this._window.nativeWindow.location.href = '/'; + // If redirectUrl is empty use history. + this.routeService.getHistory().pipe( + take(1) + ).subscribe((history) => { + let redirUrl; + if (isStandalonePage) { + // For standalone login pages, use the previous route. + redirUrl = history[history.length - 2] || ''; + } else { + redirUrl = history[history.length - 1] || ''; + } + this.navigateToRedirectUrl(redirUrl); + }); } - }) + }); } + protected navigateToRedirectUrl(url: string) { + // in case the user navigates directly to /login (via bookmark, etc), or the route history is not found. + if (isEmpty(url) || url.startsWith(LOGIN_ROUTE)) { + this.router.navigate(['/']); + /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ + // this._window.nativeWindow.location.href = '/'; + } else { + /* TODO Reenable hard redirect when REST API can handle x-forwarded-for, see https://github.com/DSpace/DSpace/pull/2207 */ + // this._window.nativeWindow.location.href = url; + this.router.navigate([url]); + } + } + /** * Refresh route navigated */ @@ -400,4 +423,5 @@ export class AuthService { this.store.dispatch(new SetRedirectUrlAction('')); this.storage.remove(REDIRECT_COOKIE); } + } diff --git a/src/app/core/auth/server-auth.service.ts b/src/app/core/auth/server-auth.service.ts index c344683e38..cf4d4a658e 100644 --- a/src/app/core/auth/server-auth.service.ts +++ b/src/app/core/auth/server-auth.service.ts @@ -1,12 +1,12 @@ -import { map, switchMap, take } from 'rxjs/operators'; +import { filter, map, switchMap, take } from 'rxjs/operators'; import { Injectable } from '@angular/core'; import { Observable } from 'rxjs'; import { HttpHeaders } from '@angular/common/http'; import { HttpOptions } from '../dspace-rest-v2/dspace-rest-v2.service'; import { AuthStatus } from './models/auth-status.model'; -import { isNotEmpty } from '../../shared/empty.util'; -import { AuthService } from './auth.service'; +import { isEmpty, isNotEmpty } from '../../shared/empty.util'; +import { AuthService, LOGIN_ROUTE } from './auth.service'; import { AuthTokenInfo } from './models/auth-token-info.model'; import { CheckAuthenticationTokenAction } from './auth.actions'; import { EPerson } from '../eperson/models/eperson.model'; @@ -54,7 +54,7 @@ export class ServerAuthService extends AuthService { /** * Redirect to the route navigated before the login */ - public redirectToPreviousUrl() { + public redirectAfterLoginSuccess(isStandalonePage: boolean) { this.getRedirectUrl().pipe( take(1)) .subscribe((redirectUrl) => { @@ -67,10 +67,15 @@ export class ServerAuthService extends AuthService { const url = decodeURIComponent(redirectUrl); this.router.navigateByUrl(url); } else { - this.router.navigate(['/']); + // If redirectUrl is empty use history. For ssr the history array should contain the requested url. + this.routeService.getHistory().pipe( + filter((history) => history.length > 0), + take(1) + ).subscribe((history) => { + this.navigateToRedirectUrl(history[history.length - 1] || ''); + }); } }) - } } diff --git a/src/app/shared/auth-nav-menu/auth-nav-menu.component.html b/src/app/shared/auth-nav-menu/auth-nav-menu.component.html index b560283ad5..4df07880d8 100644 --- a/src/app/shared/auth-nav-menu/auth-nav-menu.component.html +++ b/src/app/shared/auth-nav-menu/auth-nav-menu.component.html @@ -3,7 +3,8 @@ diff --git a/src/app/shared/log-in/log-in.component.spec.ts b/src/app/shared/log-in/log-in.component.spec.ts index dd2aea35d5..13f9e5369a 100644 --- a/src/app/shared/log-in/log-in.component.spec.ts +++ b/src/app/shared/log-in/log-in.component.spec.ts @@ -95,6 +95,7 @@ describe('LogInComponent', () => { // verify Store.dispatch() is invoked expect(page.navigateSpy.calls.any()).toBe(true, 'Store.dispatch not invoked'); }); + }); /** diff --git a/src/app/shared/log-in/log-in.component.ts b/src/app/shared/log-in/log-in.component.ts index 1291e6aa4c..b6b97230dd 100644 --- a/src/app/shared/log-in/log-in.component.ts +++ b/src/app/shared/log-in/log-in.component.ts @@ -1,5 +1,5 @@ import { filter, map, takeWhile } from 'rxjs/operators'; -import { Component, OnDestroy, OnInit } from '@angular/core'; +import { Component, Input, OnDestroy, OnInit } from '@angular/core'; import { FormBuilder, FormGroup, Validators } from '@angular/forms'; import { select, Store } from '@ngrx/store'; @@ -20,6 +20,7 @@ import { CoreState } from '../../core/core.reducers'; import { isNotEmpty } from '../empty.util'; import { fadeOut } from '../animations/fade'; import { AuthService } from '../../core/auth/auth.service'; +import { Router } from '@angular/router'; /** * /users/sign-in @@ -81,10 +82,13 @@ export class LogInComponent implements OnDestroy, OnInit { */ private alive = true; + @Input() isStandalonePage: boolean; + /** * @constructor * @param {AuthService} authService * @param {FormBuilder} formBuilder + * @param {Router} router * @param {Store} store */ constructor( @@ -135,7 +139,7 @@ export class LogInComponent implements OnDestroy, OnInit { takeWhile(() => this.alive), filter((authenticated) => authenticated)) .subscribe(() => { - this.authService.redirectToPreviousUrl(); + this.authService.redirectAfterLoginSuccess(this.isStandalonePage); } ); } @@ -188,4 +192,5 @@ export class LogInComponent implements OnDestroy, OnInit { // clear form this.form.reset(); } + } diff --git a/src/app/shared/testing/auth-service-stub.ts b/src/app/shared/testing/auth-service-stub.ts index a65923dcab..a6d24d5c8b 100644 --- a/src/app/shared/testing/auth-service-stub.ts +++ b/src/app/shared/testing/auth-service-stub.ts @@ -10,6 +10,7 @@ export class AuthServiceStub { token: AuthTokenInfo = new AuthTokenInfo('token_test'); private _tokenExpired = false; + private redirectUrl; constructor() { this.token.expires = Date.now() + (1000 * 60 * 60); @@ -88,7 +89,11 @@ export class AuthServiceStub { } setRedirectUrl(url: string) { - return; + this.redirectUrl = url; + } + + getRedirectUrl() { + return observableOf(this.redirectUrl); } public storeToken(token: AuthTokenInfo) { diff --git a/src/app/shared/testing/route-service-stub.ts b/src/app/shared/testing/route-service-stub.ts index e3cdd9d8c6..a493f10a13 100644 --- a/src/app/shared/testing/route-service-stub.ts +++ b/src/app/shared/testing/route-service-stub.ts @@ -27,6 +27,9 @@ export const routeServiceStub: any = { }, getRouteDataValue: (param) => { return observableOf({}) + }, + getHistory: () => { + return observableOf(['/home','/collection/123','/home']) } /* tslint:enable:no-empty */ }; diff --git a/src/modules/app/server-app.module.ts b/src/modules/app/server-app.module.ts index 0c3046ba68..02abf6449b 100644 --- a/src/modules/app/server-app.module.ts +++ b/src/modules/app/server-app.module.ts @@ -64,7 +64,7 @@ export function createTranslateLoader() { { provide: SubmissionService, useClass: ServerSubmissionService - }, + } ] }) export class ServerAppModule {