From 4b1f0864696fb06a8a10457184b90dbe0dc15d75 Mon Sep 17 00:00:00 2001 From: Marie Verdonck Date: Fri, 18 Jun 2021 13:18:17 +0200 Subject: [PATCH] 79700: Feedback 2021-06-15 applied --- src/app/app.component.ts | 37 +++++++++++++++-- src/app/core/auth/auth.reducer.ts | 12 ++---- src/app/core/auth/auth.service.ts | 13 +++--- src/app/root/root.component.ts | 40 ++----------------- .../idle-modal/idle-modal.component.spec.ts | 12 +++++- .../shared/idle-modal/idle-modal.component.ts | 7 +++- 6 files changed, 64 insertions(+), 57 deletions(-) diff --git a/src/app/app.component.ts b/src/app/app.component.ts index 2c01bf637b..48e1e6f3d1 100644 --- a/src/app/app.component.ts +++ b/src/app/app.component.ts @@ -1,4 +1,4 @@ -import { delay, distinctUntilChanged, filter, take } from 'rxjs/operators'; +import { delay, distinctUntilChanged, filter, take, withLatestFrom } from 'rxjs/operators'; import { AfterViewInit, ChangeDetectionStrategy, @@ -11,7 +11,7 @@ import { } from '@angular/core'; import { NavigationCancel, NavigationEnd, NavigationStart, Router } from '@angular/router'; -import { BehaviorSubject, Observable, of } from 'rxjs'; +import { BehaviorSubject, combineLatest as observableCombineLatest, Observable, of } from 'rxjs'; import { select, Store } from '@ngrx/store'; import { TranslateService } from '@ngx-translate/core'; import { Angulartics2GoogleAnalytics } from 'angulartics2/ga'; @@ -38,6 +38,8 @@ import { ThemeService } from './shared/theme-support/theme.service'; import { BASE_THEME_NAME } from './shared/theme-support/theme.constants'; import { DEFAULT_THEME_CONFIG } from './shared/theme-support/theme.effects'; import { BreadcrumbsService } from './breadcrumbs/breadcrumbs.service'; +import { IdleModalComponent } from './shared/idle-modal/idle-modal.component'; +import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; @Component({ selector: 'ds-app', @@ -70,6 +72,11 @@ export class AppComponent implements OnInit, AfterViewInit { isThemeLoading$: BehaviorSubject = new BehaviorSubject(false); + /** + * Whether or not the idle modal is is currently open + */ + idleModalOpen: boolean; + constructor( @Inject(NativeWindowService) private _window: NativeWindowRef, @Inject(DOCUMENT) private document: any, @@ -87,6 +94,7 @@ export class AppComponent implements OnInit, AfterViewInit { private windowService: HostWindowService, private localeService: LocaleService, private breadcrumbsService: BreadcrumbsService, + private modalService: NgbModal, @Optional() private cookiesService: KlaroService, @Optional() private googleAnalyticsService: GoogleAnalyticsService, ) { @@ -108,6 +116,11 @@ export class AppComponent implements OnInit, AfterViewInit { } }); + if (isPlatformBrowser(this.platformId)) { + this.authService.trackTokenExpiration(); + this.trackIdleModal(); + } + // Load all the languages that are defined as active from the config file translate.addLangs(environment.languages.filter((LangConfig) => LangConfig.active === true).map((a) => a.code)); @@ -130,7 +143,6 @@ export class AppComponent implements OnInit, AfterViewInit { console.info(environment); } this.storeCSSVariables(); - this.authService.trackTokenExpiration(); } ngOnInit() { @@ -229,4 +241,23 @@ export class AppComponent implements OnInit, AfterViewInit { }; head.appendChild(link); } + + private trackIdleModal() { + const isIdle$ = this.authService.isUserIdle(); + const isAuthenticated$ = this.authService.isAuthenticated(); + isIdle$.pipe(withLatestFrom(isAuthenticated$)) + .subscribe(([userIdle, authenticated]) => { + if (userIdle && authenticated) { + if (!this.idleModalOpen) { + const modalRef = this.modalService.open(IdleModalComponent); + this.idleModalOpen = true; + modalRef.componentInstance.response.pipe(take(1)).subscribe((closed: boolean) => { + if (closed) { + this.idleModalOpen = false; + } + }); + } + } + }); + } } diff --git a/src/app/core/auth/auth.reducer.ts b/src/app/core/auth/auth.reducer.ts index f26ddb0182..ef803503c8 100644 --- a/src/app/core/auth/auth.reducer.ts +++ b/src/app/core/auth/auth.reducer.ts @@ -240,15 +240,9 @@ export function authReducer(state: any = initialState, action: AuthActions): Aut }); case AuthActionTypes.SET_USER_AS_IDLE: - if (state.authenticated) { - return Object.assign({}, state, { - idle: true, - }); - } else { - return Object.assign({}, state, { - idle: false, - }); - } + return Object.assign({}, state, { + idle: true, + }); case AuthActionTypes.UNSET_USER_AS_IDLE: return Object.assign({}, state, { diff --git a/src/app/core/auth/auth.service.ts b/src/app/core/auth/auth.service.ts index 7b7c61f741..a5b5d70704 100644 --- a/src/app/core/auth/auth.service.ts +++ b/src/app/core/auth/auth.service.ts @@ -282,7 +282,7 @@ export class AuthService { // Send a request that sign end the session let headers = new HttpHeaders(); headers = headers.append('Content-Type', 'application/x-www-form-urlencoded'); - const options: HttpOptions = Object.create({headers, responseType: 'text'}); + const options: HttpOptions = Object.create({ headers, responseType: 'text' }); return this.authRequestService.postToEndpoint('logout', options).pipe( map((rd: RemoteData) => { const status = rd.payload; @@ -447,11 +447,14 @@ export class AuthService { * @param redirectUrl */ public navigateToRedirectUrl(redirectUrl: string) { - let url = `/reload/${new Date().getTime()}`; - if (isNotEmpty(redirectUrl) && !redirectUrl.startsWith(LOGIN_ROUTE)) { - url += `?redirect=${encodeURIComponent(redirectUrl)}`; + // Don't do redirect if already on reload url + if (!hasValue(redirectUrl) || !redirectUrl.includes('/reload/')) { + let url = `/reload/${new Date().getTime()}`; + if (isNotEmpty(redirectUrl) && !redirectUrl.startsWith(LOGIN_ROUTE)) { + url += `?redirect=${encodeURIComponent(redirectUrl)}`; + } + this.hardRedirectService.redirect(url); } - this.hardRedirectService.redirect(url); } /** diff --git a/src/app/root/root.component.ts b/src/app/root/root.component.ts index 81ae1a745c..209f17b520 100644 --- a/src/app/root/root.component.ts +++ b/src/app/root/root.component.ts @@ -1,13 +1,8 @@ -import { map, take } from 'rxjs/operators'; -import { Component, Inject, OnInit, Optional, Input } from '@angular/core'; +import { map } from 'rxjs/operators'; +import { Component, Inject, OnInit, Input } from '@angular/core'; import { Router } from '@angular/router'; -import { - combineLatest as observableCombineLatest, - combineLatest as combineLatestObservable, - Observable, - of -} from 'rxjs'; +import { combineLatest as combineLatestObservable, Observable, of } from 'rxjs'; import { Store } from '@ngrx/store'; import { TranslateService } from '@ngx-translate/core'; import { Angulartics2GoogleAnalytics } from 'angulartics2/ga'; @@ -23,11 +18,7 @@ import { HostWindowService } from '../shared/host-window.service'; import { ThemeConfig } from '../../config/theme.model'; import { Angulartics2DSpace } from '../statistics/angulartics/dspace-provider'; import { environment } from '../../environments/environment'; -import { LocaleService } from '../core/locale/locale.service'; -import { KlaroService } from '../shared/cookies/klaro.service'; import { slideSidebarPadding } from '../shared/animations/slide'; -import { NgbModal } from '@ng-bootstrap/ng-bootstrap'; -import { IdleModalComponent } from '../shared/idle-modal/idle-modal.component'; @Component({ selector: 'ds-root', @@ -54,11 +45,6 @@ export class RootComponent implements OnInit { */ @Input() shouldShowRouteLoader: boolean; - /** - * Whether or not the idle modal is is currently open - */ - idleModalOpen: boolean; - constructor( @Inject(NativeWindowService) private _window: NativeWindowRef, private translate: TranslateService, @@ -70,10 +56,7 @@ export class RootComponent implements OnInit { private router: Router, private cssService: CSSVariableService, private menuService: MenuService, - private windowService: HostWindowService, - private localeService: LocaleService, - @Optional() private cookiesService: KlaroService, - private modalService: NgbModal + private windowService: HostWindowService ) { } @@ -88,20 +71,5 @@ export class RootComponent implements OnInit { .pipe( map(([collapsed, mobile]) => collapsed || mobile) ); - - observableCombineLatest([this.authService.isUserIdle(), this.authService.isAuthenticated()]) - .subscribe(([userIdle, authenticated]) => { - if (userIdle && authenticated) { - if (!this.idleModalOpen) { - const modalRef = this.modalService.open(IdleModalComponent); - this.idleModalOpen = true; - modalRef.componentInstance.response.pipe(take(1)).subscribe((closed: boolean) => { - if (closed) { - this.idleModalOpen = false; - } - }); - } - } - }); } } diff --git a/src/app/shared/idle-modal/idle-modal.component.spec.ts b/src/app/shared/idle-modal/idle-modal.component.spec.ts index 639cbd6ad1..552315d1a4 100644 --- a/src/app/shared/idle-modal/idle-modal.component.spec.ts +++ b/src/app/shared/idle-modal/idle-modal.component.spec.ts @@ -5,6 +5,7 @@ import { TranslateModule } from '@ngx-translate/core'; import { IdleModalComponent } from './idle-modal.component'; import { AuthService } from '../../core/auth/auth.service'; import { By } from '@angular/platform-browser'; +import { HardRedirectService } from '../../core/services/hard-redirect.service'; describe('IdleModalComponent', () => { let component: IdleModalComponent; @@ -12,15 +13,18 @@ describe('IdleModalComponent', () => { let debugElement: DebugElement; const modalStub = jasmine.createSpyObj('modalStub', ['close']); - const authServiceStub = jasmine.createSpyObj('authService', ['setIdle', 'logout']); + const authServiceStub = jasmine.createSpyObj('authService', ['setIdle', 'logout', 'navigateToRedirectUrl']); + let hardRedirectService; beforeEach(waitForAsync(() => { + hardRedirectService = jasmine.createSpyObj('hardRedirectService', ['getCurrentRoute']); TestBed.configureTestingModule({ imports: [TranslateModule.forRoot()], declarations: [IdleModalComponent], providers: [ { provide: NgbActiveModal, useValue: modalStub }, - { provide: AuthService, useValue: authServiceStub } + { provide: AuthService, useValue: authServiceStub }, + { provide: HardRedirectService, useValue: hardRedirectService } ], schemas: [NO_ERRORS_SCHEMA] }).compileComponents(); @@ -63,6 +67,10 @@ describe('IdleModalComponent', () => { it('should close the modal', () => { expect(modalStub.close).toHaveBeenCalled(); }); + it('should reload', () => { + expect(hardRedirectService.getCurrentRoute).toHaveBeenCalled(); + expect(authServiceStub.navigateToRedirectUrl).toHaveBeenCalled(); + }); }); describe('closePressed', () => { diff --git a/src/app/shared/idle-modal/idle-modal.component.ts b/src/app/shared/idle-modal/idle-modal.component.ts index 750657c2e4..d812d3ffc1 100644 --- a/src/app/shared/idle-modal/idle-modal.component.ts +++ b/src/app/shared/idle-modal/idle-modal.component.ts @@ -4,6 +4,7 @@ import { environment } from '../../../environments/environment'; import { AuthService } from '../../core/auth/auth.service'; import { Subject } from 'rxjs'; import { hasValue } from '../empty.util'; +import { HardRedirectService } from '../../core/services/hard-redirect.service'; @Component({ selector: 'ds-idle-modal', @@ -29,7 +30,8 @@ export class IdleModalComponent implements OnInit { response: Subject = new Subject(); constructor(private activeModal: NgbActiveModal, - private authService: AuthService) { + private authService: AuthService, + protected hardRedirectService: HardRedirectService) { this.timeToExpire = (environment.auth.ui.timeUntilIdle + environment.auth.ui.idleGracePeriod) / 1000 / 60; // ms => min } @@ -53,8 +55,9 @@ export class IdleModalComponent implements OnInit { * Close modal and logout */ logOutPressed() { - this.authService.logout(); this.closeModal(); + this.authService.logout(); + this.authService.navigateToRedirectUrl(this.hardRedirectService.getCurrentRoute()); } /**