From 97073e03aebaabcb17eadf5229fd95e7aebd9c15 Mon Sep 17 00:00:00 2001 From: Sebastian Stehle Date: Tue, 6 Sep 2022 18:40:57 +0200 Subject: [PATCH] Redirect login. (#919) --- frontend/src/app/framework/configurations.ts | 2 +- .../must-be-authenticated.guard.spec.ts | 23 ++++++++------ .../guards/must-be-authenticated.guard.ts | 18 ++++++----- .../must-be-not-authenticated.guard.spec.ts | 23 ++++++++------ .../guards/must-be-not-authenticated.guard.ts | 19 +++++++----- .../interceptors/auth.interceptor.spec.ts | 12 ++++++-- .../shared/interceptors/auth.interceptor.ts | 6 ++-- .../src/app/shared/services/auth.service.ts | 30 +++++++++---------- .../shell/pages/home/home-page.component.ts | 23 +++++++++----- .../pages/internal/profile-menu.component.ts | 2 +- .../shell/pages/login/login-page.component.ts | 6 ++-- .../pages/logout/logout-page.component.ts | 6 ++-- 12 files changed, 105 insertions(+), 65 deletions(-) diff --git a/frontend/src/app/framework/configurations.ts b/frontend/src/app/framework/configurations.ts index 5d39ffc7a..4b1fd7fd3 100644 --- a/frontend/src/app/framework/configurations.ts +++ b/frontend/src/app/framework/configurations.ts @@ -7,7 +7,7 @@ export class UIOptions { constructor( - private readonly value: any, + public readonly value: any, ) { } diff --git a/frontend/src/app/shared/guards/must-be-authenticated.guard.spec.ts b/frontend/src/app/shared/guards/must-be-authenticated.guard.spec.ts index 8d851cc8a..9d80beb4e 100644 --- a/frontend/src/app/shared/guards/must-be-authenticated.guard.spec.ts +++ b/frontend/src/app/shared/guards/must-be-authenticated.guard.spec.ts @@ -5,6 +5,7 @@ * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved. */ +import { Location } from '@angular/common'; import { Router } from '@angular/router'; import { firstValueFrom, of } from 'rxjs'; import { IMock, It, Mock, Times } from 'typemoq'; @@ -13,19 +14,25 @@ import { MustBeAuthenticatedGuard } from './must-be-authenticated.guard'; describe('MustBeAuthenticatedGuard', () => { let router: IMock; + let location: IMock; let authService: IMock; - const uiOptions = new UIOptions({ map: { type: 'OSM' } }); - const uiOptionsRedirect = new UIOptions({ map: { type: 'OSM' }, redirectToLogin: true }); + let authGuard: MustBeAuthenticatedGuard; + + const uiOptions = new UIOptions({}); beforeEach(() => { + location = Mock.ofType(); + + location.setup(x => x.path(true)) + .returns(() => '/my-path'); + router = Mock.ofType(); authService = Mock.ofType(); + authGuard = new MustBeAuthenticatedGuard(authService.object, location.object, router.object, uiOptions); }); it('should navigate to default page if not authenticated', async () => { - const authGuard = new MustBeAuthenticatedGuard(uiOptions, authService.object, router.object); - authService.setup(x => x.userChanges) .returns(() => of(null)); @@ -33,12 +40,10 @@ describe('MustBeAuthenticatedGuard', () => { expect(result).toBeFalsy(); - router.verify(x => x.navigate(['']), Times.once()); + router.verify(x => x.navigate([''], { queryParams: { redirectPath: '/my-path' } }), Times.once()); }); it('should return true if authenticated', async () => { - const authGuard = new MustBeAuthenticatedGuard(uiOptions, authService.object, router.object); - authService.setup(x => x.userChanges) .returns(() => of({})); @@ -50,7 +55,7 @@ describe('MustBeAuthenticatedGuard', () => { }); it('should login redirect if redirect enabled', async () => { - const authGuard = new MustBeAuthenticatedGuard(uiOptionsRedirect, authService.object, router.object); + uiOptions.value.redirectToLogin = true; authService.setup(x => x.userChanges) .returns(() => of(null)); @@ -59,6 +64,6 @@ describe('MustBeAuthenticatedGuard', () => { expect(result!).toBeFalsy(); - authService.verify(x => x.loginRedirect(), Times.once()); + authService.verify(x => x.loginRedirect('/my-path'), Times.once()); }); }); diff --git a/frontend/src/app/shared/guards/must-be-authenticated.guard.ts b/frontend/src/app/shared/guards/must-be-authenticated.guard.ts index 2e35129d1..73c3f20b4 100644 --- a/frontend/src/app/shared/guards/must-be-authenticated.guard.ts +++ b/frontend/src/app/shared/guards/must-be-authenticated.guard.ts @@ -5,6 +5,7 @@ * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved. */ +import { Location } from '@angular/common'; import { Injectable } from '@angular/core'; import { CanActivate, Router } from '@angular/router'; import { Observable } from 'rxjs'; @@ -14,24 +15,27 @@ import { AuthService } from './../services/auth.service'; @Injectable() export class MustBeAuthenticatedGuard implements CanActivate { - private readonly redirect: boolean; - - constructor(uiOptions: UIOptions, + constructor( private readonly authService: AuthService, + private readonly location: Location, private readonly router: Router, + private readonly uiOptions: UIOptions, ) { - this.redirect = uiOptions.get('redirectToLogin'); } public canActivate(): Observable { + const redirect = this.uiOptions.get('redirectToLogin'); + return this.authService.userChanges.pipe( take(1), tap(user => { if (!user) { - if (this.redirect) { - this.authService.loginRedirect(); + const redirectPath = this.location.path(true); + + if (redirect) { + this.authService.loginRedirect(redirectPath); } else { - this.router.navigate(['']); + this.router.navigate([''], { queryParams: { redirectPath } }); } } }), diff --git a/frontend/src/app/shared/guards/must-be-not-authenticated.guard.spec.ts b/frontend/src/app/shared/guards/must-be-not-authenticated.guard.spec.ts index 2e9af64e2..1bfa95923 100644 --- a/frontend/src/app/shared/guards/must-be-not-authenticated.guard.spec.ts +++ b/frontend/src/app/shared/guards/must-be-not-authenticated.guard.spec.ts @@ -5,6 +5,7 @@ * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved. */ +import { Location } from '@angular/common'; import { Router } from '@angular/router'; import { firstValueFrom, of } from 'rxjs'; import { IMock, It, Mock, Times } from 'typemoq'; @@ -13,19 +14,25 @@ import { MustBeNotAuthenticatedGuard } from './must-be-not-authenticated.guard'; describe('MustBeNotAuthenticatedGuard', () => { let router: IMock; + let location: IMock; let authService: IMock; - const uiOptions = new UIOptions({ map: { type: 'OSM' } }); - const uiOptionsRedirect = new UIOptions({ map: { type: 'OSM' }, redirectToLogin: true }); + let authGuard: MustBeNotAuthenticatedGuard; + + const uiOptions = new UIOptions({}); beforeEach(() => { + location = Mock.ofType(); + + location.setup(x => x.path(true)) + .returns(() => '/my-path'); + router = Mock.ofType(); authService = Mock.ofType(); + authGuard = new MustBeNotAuthenticatedGuard(authService.object, location.object, router.object, uiOptions); }); it('should navigate to app page if authenticated', async () => { - const authGuard = new MustBeNotAuthenticatedGuard(uiOptions, authService.object, router.object); - authService.setup(x => x.userChanges) .returns(() => of({})); @@ -33,12 +40,10 @@ describe('MustBeNotAuthenticatedGuard', () => { expect(result!).toBeFalsy(); - router.verify(x => x.navigate(['app']), Times.once()); + router.verify(x => x.navigate(['app'], { queryParams: { redirectPath: '/my-path' } }), Times.once()); }); it('should return true if not authenticated', async () => { - const authGuard = new MustBeNotAuthenticatedGuard(uiOptions, authService.object, router.object); - authService.setup(x => x.userChanges) .returns(() => of(null)); @@ -50,7 +55,7 @@ describe('MustBeNotAuthenticatedGuard', () => { }); it('should login redirect and return false if redirect enabled', async () => { - const authGuard = new MustBeNotAuthenticatedGuard(uiOptionsRedirect, authService.object, router.object); + uiOptions.value.redirectToLogin = true; authService.setup(x => x.userChanges) .returns(() => of(null)); @@ -59,6 +64,6 @@ describe('MustBeNotAuthenticatedGuard', () => { expect(result).toBeFalsy(); - authService.verify(x => x.loginRedirect(), Times.once()); + authService.verify(x => x.loginRedirect('/my-path'), Times.once()); }); }); diff --git a/frontend/src/app/shared/guards/must-be-not-authenticated.guard.ts b/frontend/src/app/shared/guards/must-be-not-authenticated.guard.ts index d7f54e255..da647c97f 100644 --- a/frontend/src/app/shared/guards/must-be-not-authenticated.guard.ts +++ b/frontend/src/app/shared/guards/must-be-not-authenticated.guard.ts @@ -5,6 +5,7 @@ * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved. */ +import { Location } from '@angular/common'; import { Injectable } from '@angular/core'; import { CanActivate, Router } from '@angular/router'; import { Observable } from 'rxjs'; @@ -14,25 +15,29 @@ import { AuthService } from './../services/auth.service'; @Injectable() export class MustBeNotAuthenticatedGuard implements CanActivate { - private readonly redirect: boolean; - constructor(uiOptions: UIOptions, + constructor( private readonly authService: AuthService, + private readonly location: Location, private readonly router: Router, + private readonly uiOptions: UIOptions, ) { - this.redirect = uiOptions.get('redirectToLogin'); } public canActivate(): Observable { + const redirect = this.uiOptions.get('redirectToLogin'); + return this.authService.userChanges.pipe( take(1), tap(user => { - if (this.redirect) { - this.authService.loginRedirect(); + const redirectPath = this.location.path(true); + + if (redirect) { + this.authService.loginRedirect(redirectPath); } else if (user) { - this.router.navigate(['app']); + this.router.navigate(['app'], { queryParams: { redirectPath } }); } }), - map(user => !user && !this.redirect)); + map(user => !user && !redirect)); } } diff --git a/frontend/src/app/shared/interceptors/auth.interceptor.spec.ts b/frontend/src/app/shared/interceptors/auth.interceptor.spec.ts index 28dd2cdcb..7ce74c10b 100644 --- a/frontend/src/app/shared/interceptors/auth.interceptor.spec.ts +++ b/frontend/src/app/shared/interceptors/auth.interceptor.spec.ts @@ -7,6 +7,7 @@ /* eslint-disable deprecation/deprecation */ +import { Location } from '@angular/common'; import { HTTP_INTERCEPTORS, HttpClient, HttpHeaders } from '@angular/common/http'; import { HttpClientTestingModule, HttpTestingController } from '@angular/common/http/testing'; import { inject, TestBed } from '@angular/core/testing'; @@ -19,9 +20,15 @@ import { AuthInterceptor } from './auth.interceptor'; describe('AuthInterceptor', () => { let authService: IMock; + let location: IMock; let router: IMock; beforeEach(() => { + location = Mock.ofType(); + + location.setup(x => x.path()) + .returns(() => '/my-path'); + authService = Mock.ofType(AuthService); router = Mock.ofType(); @@ -32,6 +39,7 @@ describe('AuthInterceptor', () => { ], providers: [ { provide: Router, useFactory: () => router.object }, + { provide: Location, useFactory: () => location.object }, { provide: AuthService, useValue: authService.object }, { provide: ApiUrlConfig, useValue: new ApiUrlConfig('http://service/p/') }, { @@ -100,7 +108,7 @@ describe('AuthInterceptor', () => { expect().nothing(); - authService.verify(x => x.logoutRedirect(), Times.once()); + authService.verify(x => x.logoutRedirect('/my-path'), Times.once()); })); const AUTH_ERRORS = [403]; @@ -137,7 +145,7 @@ describe('AuthInterceptor', () => { expect().nothing(); - authService.verify(x => x.logoutRedirect(), Times.never()); + authService.verify(x => x.logoutRedirect('/my-path'), Times.never()); })); }); }); diff --git a/frontend/src/app/shared/interceptors/auth.interceptor.ts b/frontend/src/app/shared/interceptors/auth.interceptor.ts index 92c71d6cc..e7b52ed14 100644 --- a/frontend/src/app/shared/interceptors/auth.interceptor.ts +++ b/frontend/src/app/shared/interceptors/auth.interceptor.ts @@ -5,6 +5,7 @@ * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved. */ +import { Location } from '@angular/common'; import { HttpErrorResponse, HttpEvent, HttpHandler, HttpInterceptor, HttpRequest } from '@angular/common/http'; import { Injectable } from '@angular/core'; import { Router } from '@angular/router'; @@ -19,6 +20,7 @@ export class AuthInterceptor implements HttpInterceptor { constructor(apiUrlConfig: ApiUrlConfig, private readonly authService: AuthService, + private readonly location: Location, private readonly router: Router, ) { this.baseUrl = apiUrlConfig.buildUrl(''); @@ -50,7 +52,7 @@ export class AuthInterceptor implements HttpInterceptor { if (error.status === 401 && renew) { return this.authService.loginSilent().pipe( catchError(() => { - this.authService.logoutRedirect(); + this.authService.logoutRedirect(this.location.path()); return EMPTY; }), @@ -58,7 +60,7 @@ export class AuthInterceptor implements HttpInterceptor { } else if (error.status === 401 || error.status === 403) { if (req.method === 'GET') { if (error.status === 401) { - this.authService.logoutRedirect(); + this.authService.logoutRedirect(this.location.path()); } else { this.router.navigate(['/forbidden'], { replaceUrl: true }); } diff --git a/frontend/src/app/shared/services/auth.service.ts b/frontend/src/app/shared/services/auth.service.ts index d9c23f52f..ce1998410 100644 --- a/frontend/src/app/shared/services/auth.service.ts +++ b/frontend/src/app/shared/services/auth.service.ts @@ -7,7 +7,7 @@ import { Injectable } from '@angular/core'; import { Log, User, UserManager, WebStorageStateStore } from 'oidc-client'; -import { concat, Observable, Observer, of, ReplaySubject, throwError, TimeoutError } from 'rxjs'; +import { concat, Observable, of, ReplaySubject, throwError, TimeoutError } from 'rxjs'; import { delay, mergeMap, retryWhen, take, timeout } from 'rxjs/operators'; import { ApiUrlConfig, Types } from '@app/framework'; @@ -124,19 +124,19 @@ export class AuthService { this.checkState(this.userManager.getUser()); } - public logoutRedirect() { - this.userManager.signoutRedirect(); + public logoutRedirect(redirectPath: string) { + this.userManager.signoutRedirect({ state: { redirectPath } }); } - public loginRedirect() { - this.userManager.signinRedirect(); + public loginRedirect(redirectPath: string) { + this.userManager.signinRedirect({ state: { redirectPath } }); } - public logoutRedirectComplete(): Observable { - return new Observable((observer: Observer) => { + public logoutRedirectComplete(): Observable { + return new Observable(observer => { this.userManager.signoutRedirectCallback() .then(x => { - observer.next(x); + observer.next(x.state?.redirectPath); observer.complete(); }, err => { observer.error(err); @@ -145,11 +145,11 @@ export class AuthService { }); } - public loginPopup(): Observable { - return new Observable(observer => { - this.userManager.signinPopup() + public loginPopup(redirectPath: string): Observable { + return new Observable(observer => { + this.userManager.signinPopup({ state: { redirectPath } }) .then(x => { - observer.next(AuthService.createProfile(x)); + observer.next(x.state?.redirectPath); observer.complete(); }, err => { observer.error(err); @@ -158,11 +158,11 @@ export class AuthService { }); } - public loginRedirectComplete(): Observable { - return new Observable(observer => { + public loginRedirectComplete(): Observable { + return new Observable(observer => { this.userManager.signinRedirectCallback() .then(x => { - observer.next(AuthService.createProfile(x)); + observer.next(x.state?.redirectPath); observer.complete(); }, err => { observer.error(err); diff --git a/frontend/src/app/shell/pages/home/home-page.component.ts b/frontend/src/app/shell/pages/home/home-page.component.ts index 0159e29c7..e86c6c7c0 100644 --- a/frontend/src/app/shell/pages/home/home-page.component.ts +++ b/frontend/src/app/shell/pages/home/home-page.component.ts @@ -5,8 +5,9 @@ * Copyright (c) Squidex UG (haftungsbeschränkt). All rights reserved. */ +import { Location } from '@angular/common'; import { Component } from '@angular/core'; -import { Router } from '@angular/router'; +import { ActivatedRoute, Router } from '@angular/router'; import { AuthService } from '@app/shared'; @Component({ @@ -19,18 +20,26 @@ export class HomePageComponent { constructor( private readonly authService: AuthService, + private readonly location: Location, + private readonly route: ActivatedRoute, private readonly router: Router, ) { } public login() { + const redirectPath = + this.route.snapshot.queryParams.redirectPath || + this.location.path(); + if (this.isIE()) { - this.authService.loginRedirect(); + this.authService.loginRedirect(redirectPath); } else { - this.authService.loginPopup() + this.authService.loginPopup(redirectPath) .subscribe({ - next: () => { - this.router.navigate(['/app']); + next: path => { + path ||= '/app'; + + this.router.navigateByUrl(path); }, error: () => { this.showLoginError = true; @@ -40,8 +49,6 @@ export class HomePageComponent { } public isIE() { - const isIE = !!navigator.userAgent.match(/Trident/g) || !!navigator.userAgent.match(/MSIE/g); - - return isIE; + return !!navigator.userAgent.match(/Trident/g) || !!navigator.userAgent.match(/MSIE/g); } } diff --git a/frontend/src/app/shell/pages/internal/profile-menu.component.ts b/frontend/src/app/shell/pages/internal/profile-menu.component.ts index e59708c79..bfd251809 100644 --- a/frontend/src/app/shell/pages/internal/profile-menu.component.ts +++ b/frontend/src/app/shell/pages/internal/profile-menu.component.ts @@ -93,6 +93,6 @@ export class ProfileMenuComponent extends StatefulComponent implements On } public logout() { - this.authService.logoutRedirect(); + this.authService.logoutRedirect('/'); } } diff --git a/frontend/src/app/shell/pages/login/login-page.component.ts b/frontend/src/app/shell/pages/login/login-page.component.ts index 81149e8e2..fbb00da25 100644 --- a/frontend/src/app/shell/pages/login/login-page.component.ts +++ b/frontend/src/app/shell/pages/login/login-page.component.ts @@ -23,8 +23,10 @@ export class LoginPageComponent implements OnInit { public ngOnInit() { this.authService.loginRedirectComplete() .subscribe({ - next: () => { - this.router.navigate(['/app'], { replaceUrl: true }); + next: path => { + path ||= '/app'; + + this.router.navigateByUrl(path, { replaceUrl: true }); }, error: () => { this.router.navigate(['/'], { replaceUrl: true }); diff --git a/frontend/src/app/shell/pages/logout/logout-page.component.ts b/frontend/src/app/shell/pages/logout/logout-page.component.ts index bdb7fa7c9..d4357f537 100644 --- a/frontend/src/app/shell/pages/logout/logout-page.component.ts +++ b/frontend/src/app/shell/pages/logout/logout-page.component.ts @@ -23,8 +23,10 @@ export class LogoutPageComponent implements OnInit { public ngOnInit() { this.authService.logoutRedirectComplete() .subscribe({ - next: () => { - this.router.navigate(['/'], { replaceUrl: true }); + next: path => { + path ||= '/'; + + this.router.navigateByUrl(path, { replaceUrl: true }); }, error: () => { this.router.navigate(['/'], { replaceUrl: true });