From 09307ab3bdd9ce27f746c6c1fa8018f807388182 Mon Sep 17 00:00:00 2001 From: Nizamudeen A Date: Tue, 18 May 2021 14:18:38 +0530 Subject: [PATCH] mgr/dashboard: Fix for query params resetting on change-password Fixes: https://tracker.ceph.com/issues/50857 Signed-off-by: Nizamudeen A --- .../src/app/shared/api/auth.service.ts | 9 ++++++--- .../change-password-guard.service.spec.ts | 18 +++++++++++------- .../services/change-password-guard.service.ts | 16 +++++++++++----- 3 files changed, 28 insertions(+), 15 deletions(-) diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/auth.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/auth.service.ts index 6c8356af87395..8a291799235b3 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/api/auth.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/api/auth.service.ts @@ -1,7 +1,8 @@ import { HttpClient } from '@angular/common/http'; import { Injectable } from '@angular/core'; -import { Router } from '@angular/router'; +import { ActivatedRoute, Router } from '@angular/router'; +import * as _ from 'lodash'; import { Observable } from 'rxjs'; import { tap } from 'rxjs/operators'; @@ -16,7 +17,8 @@ export class AuthService { constructor( private authStorageService: AuthStorageService, private http: HttpClient, - private router: Router + private router: Router, + private route: ActivatedRoute ) {} check(token: string) { @@ -40,7 +42,8 @@ export class AuthService { logout(callback: Function = null) { return this.http.post('api/auth/logout', null).subscribe((resp: any) => { this.authStorageService.remove(); - this.router.navigate(['/login'], { skipLocationChange: true }); + const url = _.get(this.route.snapshot.queryParams, 'returnUrl', '/login'); + this.router.navigate([url], { skipLocationChange: true }); if (callback) { callback(); } diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/change-password-guard.service.spec.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/change-password-guard.service.spec.ts index 5bc8d751fd4d9..12800d112207b 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/change-password-guard.service.spec.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/change-password-guard.service.spec.ts @@ -1,6 +1,6 @@ import { Component, NgZone } from '@angular/core'; import { fakeAsync, TestBed, tick } from '@angular/core/testing'; -import { Router, Routes } from '@angular/router'; +import { ActivatedRouteSnapshot, Router, RouterStateSnapshot, Routes } from '@angular/router'; import { RouterTestingModule } from '@angular/router/testing'; import { configureTestBed } from '~/testing/unit-test-helper'; @@ -11,6 +11,8 @@ describe('ChangePasswordGuardService', () => { let service: ChangePasswordGuardService; let authStorageService: AuthStorageService; let ngZone: NgZone; + let route: ActivatedRouteSnapshot; + let state: RouterStateSnapshot; @Component({ selector: 'cd-login-password-form', template: '' }) class LoginPasswordFormComponent {} @@ -35,30 +37,32 @@ describe('ChangePasswordGuardService', () => { it('should do nothing (not logged in)', () => { spyOn(authStorageService, 'isLoggedIn').and.returnValue(false); - expect(service.canActivate()).toBeTruthy(); + expect(service.canActivate(route, state)).toBeTruthy(); }); it('should do nothing (SSO enabled)', () => { spyOn(authStorageService, 'isLoggedIn').and.returnValue(true); spyOn(authStorageService, 'isSSO').and.returnValue(true); - expect(service.canActivate()).toBeTruthy(); + expect(service.canActivate(route, state)).toBeTruthy(); }); it('should do nothing (no update pwd required)', () => { spyOn(authStorageService, 'isLoggedIn').and.returnValue(true); spyOn(authStorageService, 'getPwdUpdateRequired').and.returnValue(false); - expect(service.canActivate()).toBeTruthy(); + expect(service.canActivate(route, state)).toBeTruthy(); }); - it('should redirect to change password page', fakeAsync(() => { + it('should redirect to change password page by preserving the query params', fakeAsync(() => { + route = null; + state = { url: '/host', root: null }; spyOn(authStorageService, 'isLoggedIn').and.returnValue(true); spyOn(authStorageService, 'isSSO').and.returnValue(false); spyOn(authStorageService, 'getPwdUpdateRequired').and.returnValue(true); const router = TestBed.inject(Router); ngZone.run(() => { - expect(service.canActivate()).toBeFalsy(); + expect(service.canActivate(route, state)).toBeFalsy(); }); tick(); - expect(router.url).toBe('/login-change-password'); + expect(router.url).toBe('/login-change-password?returnUrl=%2Fhost'); })); }); diff --git a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/change-password-guard.service.ts b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/change-password-guard.service.ts index 7a3332bc6f50e..d97160f922a27 100644 --- a/src/pybind/mgr/dashboard/frontend/src/app/shared/services/change-password-guard.service.ts +++ b/src/pybind/mgr/dashboard/frontend/src/app/shared/services/change-password-guard.service.ts @@ -1,5 +1,11 @@ import { Injectable } from '@angular/core'; -import { CanActivate, CanActivateChild, Router } from '@angular/router'; +import { + ActivatedRouteSnapshot, + CanActivate, + CanActivateChild, + Router, + RouterStateSnapshot +} from '@angular/router'; import { AuthStorageService } from './auth-storage.service'; @@ -13,7 +19,7 @@ import { AuthStorageService } from './auth-storage.service'; export class ChangePasswordGuardService implements CanActivate, CanActivateChild { constructor(private router: Router, private authStorageService: AuthStorageService) {} - canActivate() { + canActivate(_route: ActivatedRouteSnapshot, state: RouterStateSnapshot) { // Redirect to '/login-change-password' when the following constraints // are fulfilled: // - The user must be logged in. @@ -24,13 +30,13 @@ export class ChangePasswordGuardService implements CanActivate, CanActivateChild !this.authStorageService.isSSO() && this.authStorageService.getPwdUpdateRequired() ) { - this.router.navigate(['/login-change-password']); + this.router.navigate(['/login-change-password'], { queryParams: { returnUrl: state.url } }); return false; } return true; } - canActivateChild(): boolean { - return this.canActivate(); + canActivateChild(childRoute: ActivatedRouteSnapshot, state: RouterStateSnapshot): boolean { + return this.canActivate(childRoute, state); } } -- 2.39.5