Add session status check to classic view

- Check session when moving between classic views
-- Update backend session-status entry point to also check legacy session
- Check if classic view iframe was re-directed to login page
- Logout user in the above
- Add conditional redirect to auth service logout
- Add key based loading to fix issues with loading on logout.
- Update jasmine tests

core/app/src/app/app-routing.module.ts

@@ -17,6 +17,7 @@ const routes: Routes = [
     {
         path: 'Login',
         loadChildren: () => import('../components/login/login.module').then(m => m.LoginUiModule),
+        runGuardsAndResolvers: 'always',
         resolve: {
             metadata: BaseMetadataResolver
         },
@@ -37,7 +38,8 @@ const routes: Routes = [
             legacyUrl: ClassicViewResolver,
         },
         data: {
-            reuseRoute: false
+            reuseRoute: false,
+            checkSession: true
         }
     },
     {
@@ -49,7 +51,8 @@ const routes: Routes = [
             legacyUrl: ClassicViewResolver,
         },
         data: {
-            reuseRoute: false
+            reuseRoute: false,
+            checkSession: true
         }
     },
     {
@@ -61,7 +64,8 @@ const routes: Routes = [
             legacyUrl: ClassicViewResolver,
         },
         data: {
-            reuseRoute: false
+            reuseRoute: false,
+            checkSession: true
         }
     },
     {path: '**', redirectTo: 'Login'},

core/app/src/app/app.component.ts

@@ -21,13 +21,13 @@ export class AppComponent implements OnInit {
 
     private checkRouterEvent(routerEvent: Event) {
         if (routerEvent instanceof NavigationStart) {
-            this.appStateFacade.updateLoading(true);
+            this.appStateFacade.updateLoading('router-navigation',true);
         }
 
         if (routerEvent instanceof NavigationEnd ||
             routerEvent instanceof NavigationCancel ||
             routerEvent instanceof NavigationError) {
-            this.appStateFacade.updateLoading(false);
+            this.appStateFacade.updateLoading('router-navigation', false);
         }
     }
 }

core/app/src/components/classic-view/classic-view.component.spec.ts

@@ -7,6 +7,7 @@ import {ActivatedRoute} from '@angular/router';
 import {ApolloTestingModule} from 'apollo-angular/testing';
 import {IframePageChangeObserver} from '@services/classic-view/iframe-page-change-observer.service';
 import {IframeResizeHandlerHandler} from '@services/classic-view/iframe-resize-handler.service';
+import {AuthService} from '@services/auth/auth.service';
 
 class ClassicViewUiComponentMock extends ClassicViewUiComponent {
     protected buildIframePageChangeObserver(): IframePageChangeObserver {
@@ -38,7 +39,16 @@ describe('ClassicViewUiComponent', () => {
                 RouterTestingModule,
                 ApolloTestingModule
             ],
-            providers: [{provide: ActivatedRoute, useValue: route}],
+            providers: [
+                {
+                    provide: ActivatedRoute,
+                    useValue: route
+                },
+                {
+                    provide: AuthService,
+                    useValue: jasmine.createSpyObj('AuthService', ['logout'])
+                }
+            ],
             declarations: [ClassicViewUiComponentMock]
         })
             .compileComponents();

core/app/src/components/classic-view/classic-view.component.ts

@@ -4,6 +4,7 @@ import {DomSanitizer} from '@angular/platform-browser';
 import {RouteConverter} from '@services/navigation/route-converter/route-converter.service';
 import {IframePageChangeObserver} from '@services/classic-view/iframe-page-change-observer.service';
 import {IframeResizeHandlerHandler} from '@services/classic-view/iframe-resize-handler.service';
+import {AuthService} from '@services/auth/auth.service';
 
 @Component({
     selector: 'scrm-classic-view-ui',
@@ -24,7 +25,9 @@ export class ClassicViewUiComponent implements OnInit, OnDestroy, AfterViewInit
         private router: Router,
         private sanitizer: DomSanitizer,
         private routeConverter: RouteConverter,
-        private ngZone: NgZone) {
+        private auth: AuthService,
+        private ngZone: NgZone
+    ) {
     }
 
     ngOnInit(): void {
@@ -85,6 +88,11 @@ export class ClassicViewUiComponent implements OnInit, OnDestroy, AfterViewInit
     protected onPageChange(newLocation): void {
         const location = this.routeConverter.toFrontEnd(newLocation);
 
+        if (location === '/users/login'){
+            this.auth.logout('LBL_SESSION_EXPIRED');
+            return;
+        }
+
         this.ngZone.run(() => this.router.navigateByUrl(location).then()).then();
     }
 

core/app/src/directives/button-loading/button-loading.directive.spec.ts

@@ -46,14 +46,14 @@ describe('ButtonLoadingDirective', () => {
     it('button should get disabled when app loading is active', () => {
 
         expect(testHostComponent).toBeTruthy();
-        appState.updateLoading(true);
+        appState.updateLoading('button-loading',true);
         testHostFixture.detectChanges();
         let button = testHostFixture.nativeElement.querySelector('button');
 
         expect(button).toBeTruthy();
         expect(button.disabled).toEqual(true);
 
-        appState.updateLoading(false);
+        appState.updateLoading('button-loading',false);
 
         testHostFixture.detectChanges();
         button = testHostFixture.nativeElement.querySelector('button');
@@ -65,7 +65,7 @@ describe('ButtonLoadingDirective', () => {
     it('button should get disabled when loading input is active', () => {
 
         expect(testHostComponent).toBeTruthy();
-        appState.updateLoading(false);
+        appState.updateLoading('button-loading',false);
         testHostComponent.setLoading(true);
         testHostFixture.detectChanges();
         let button = testHostFixture.nativeElement.querySelector('button');

core/app/src/facades/app-state/app-state.facade.spec.ts

@@ -15,7 +15,7 @@ describe('AppState Facade', () => {
 
     it('#updateLoading',
         (done: DoneFn) => {
-            service.updateLoading(true);
+            service.updateLoading('test', true);
             service.loading$.pipe(take(1)).subscribe(loading => {
                 expect(loading).toEqual(true);
                 done();

core/app/src/facades/app-state/app-state.facade.ts

@@ -1,6 +1,6 @@
 import {Injectable} from '@angular/core';
-import {BehaviorSubject, Observable, combineLatest} from 'rxjs';
-import {map, distinctUntilChanged} from 'rxjs/operators';
+import {BehaviorSubject, combineLatest, Observable} from 'rxjs';
+import {distinctUntilChanged, map} from 'rxjs/operators';
 import {deepClone} from '@base/utils/object-utils';
 import {StateFacade} from '@base/facades/state';
 
@@ -21,6 +21,7 @@ export class AppStateFacade implements StateFacade {
 
     protected store = new BehaviorSubject<AppState>(internalState);
     protected state$ = this.store.asObservable();
+    protected loadingQueue = {};
 
     loading$ = this.state$.pipe(map(state => state.loading), distinctUntilChanged());
 
@@ -39,11 +40,62 @@ export class AppStateFacade implements StateFacade {
      * Clear state
      */
     public clear(): void {
+        this.loadingQueue = {};
         this.updateState(deepClone(initialState));
     }
 
-    public updateLoading(loading: boolean) {
-        this.updateState({...internalState, loading});
+    /**
+     * Update loading status for given key
+     *
+     * @param {string} key to update
+     * @param {string} loading status to set
+     */
+    public updateLoading(key: string, loading: boolean): void {
+
+        if (loading === true) {
+            this.addToLoadingQueue(key);
+            this.updateState({...internalState, loading});
+            return;
+        }
+
+        this.removeFromLoadingQueue(key);
+
+        if (this.hasActiveLoading()) {
+            this.updateState({...internalState, loading});
+        }
+    }
+
+    /**
+     * Internal API
+     */
+
+    /**
+     *  Check if there are still active loadings
+     *
+     *  @returns {boolean} active loading
+     */
+    protected hasActiveLoading(): boolean {
+        return Object.keys(this.loadingQueue).length < 1;
+    }
+
+    /**
+     * Remove key from loading queue
+     *
+     * @param {string} key to remove
+     */
+    protected removeFromLoadingQueue(key: string): void {
+        if (this.loadingQueue[key]) {
+            delete this.loadingQueue[key];
+        }
+    }
+
+    /**
+     * Add key to loading queue
+     *
+     * @param {string} key to add
+     */
+    protected addToLoadingQueue(key: string): void {
+        this.loadingQueue[key] = true;
     }
 
     /**

core/app/src/facades/language/language.facade.ts

@@ -133,10 +133,10 @@ export class LanguageFacade implements StateFacade {
 
         internalState.hasChanged = true;
 
-        this.appStateFacade.updateLoading(true);
+        this.appStateFacade.updateLoading('change-language', true);
 
         this.load(languageKey, types).pipe(
-            tap(() => this.appStateFacade.updateLoading(false))
+            tap(() => this.appStateFacade.updateLoading('change-language',false))
         ).subscribe();
     }
 

core/app/src/facades/state-manager.ts

@@ -5,7 +5,7 @@ import {NavigationFacade} from '@base/facades/navigation/navigation.facade';
 import {SystemConfigFacade} from '@base/facades/system-config/system-config.facade';
 import {ThemeImagesFacade} from '@base/facades/theme-images/theme-images.facade';
 import {UserPreferenceFacade} from '@base/facades/user-preference/user-preference.facade';
-import {StateFacadeMap} from '@base/facades/state';
+import {StateFacade, StateFacadeMap, StateFacadeMapEntry} from '@base/facades/state';
 
 @Injectable({
     providedIn: 'root',
@@ -21,12 +21,12 @@ export class StateManager {
         protected themeImagesFacade: ThemeImagesFacade,
         protected userPreferenceFacade: UserPreferenceFacade
     ) {
-        this.stateFacades.appFacade = appFacade;
-        this.stateFacades.languageFacade = languageFacade;
-        this.stateFacades.navigationFacade = navigationFacade;
-        this.stateFacades.systemConfigFacade = systemConfigFacade;
-        this.stateFacades.themeImagesFacade = themeImagesFacade;
-        this.stateFacades.userPreferenceFacade = userPreferenceFacade;
+        this.stateFacades.appFacade = this.buildMapEntry(appFacade, false);
+        this.stateFacades.languageFacade = this.buildMapEntry(languageFacade, false)
+        this.stateFacades.navigationFacade = this.buildMapEntry(navigationFacade, true);
+        this.stateFacades.systemConfigFacade = this.buildMapEntry(systemConfigFacade, false);
+        this.stateFacades.themeImagesFacade = this.buildMapEntry(themeImagesFacade, false);
+        this.stateFacades.userPreferenceFacade =  this.buildMapEntry(userPreferenceFacade, true);
     }
 
     /**
@@ -38,7 +38,36 @@ export class StateManager {
      */
     public clear(): void {
         Object.keys(this.stateFacades).forEach((key) => {
-            this.stateFacades[key].clear();
+            this.stateFacades[key].facade.clear();
         });
     }
+
+    /**
+     * Clear all state
+     */
+    public clearAuthBased(): void {
+        Object.keys(this.stateFacades).forEach((key) => {
+            if (this.stateFacades[key].authBased){
+                this.stateFacades[key].facade.clear();
+            }
+        });
+    }
+
+    /**
+     * Internal api
+     */
+
+    /**
+     * Build Map entry
+     *
+     * @param {{}} facade to use
+     * @param {boolean} authBased flag
+     * @returns {{}} StateFacadeMapEntry
+     */
+    protected buildMapEntry(facade: StateFacade, authBased: boolean): StateFacadeMapEntry {
+        return {
+            facade,
+            authBased
+        };
+    }
 }

core/app/src/facades/state.ts

@@ -1,5 +1,10 @@
 export interface StateFacadeMap {
-    [key: string]: StateFacade;
+    [key: string]: StateFacadeMapEntry;
+}
+
+export interface StateFacadeMapEntry {
+    facade: StateFacade;
+    authBased: boolean;
 }
 
 export interface StateFacade {

core/app/src/facades/theme-images/theme-images.facade.ts

@@ -78,10 +78,10 @@ export class ThemeImagesFacade implements StateFacade {
      */
     public changeTheme(theme: string): void {
 
-        this.appStateFacade.updateLoading(true);
+        this.appStateFacade.updateLoading('change-theme', true);
 
         this.load(theme).pipe(
-            tap(() => this.appStateFacade.updateLoading(false))
+            tap(() => this.appStateFacade.updateLoading('change-theme', false))
         ).subscribe();
     }
 

core/app/src/services/auth/auth-guard.service.ts

@@ -1,11 +1,10 @@
 import {Injectable} from '@angular/core';
-import {ActivatedRouteSnapshot, CanActivate, Router, RouterStateSnapshot, UrlTree} from '@angular/router';
-import {MessageService} from '../message/message.service';
-import {AuthService} from '../auth/auth.service';
-import {Observable, of} from "rxjs";
-import {HttpClient, HttpHeaders} from "@angular/common/http";
-import {catchError, map, take, tap} from "rxjs/operators";
-import {StateManager} from "@base/facades/state-manager";
+import {ActivatedRouteSnapshot, CanActivate, Router, UrlTree} from '@angular/router';
+import {HttpClient, HttpHeaders} from '@angular/common/http';
+import {Observable, of} from 'rxjs';
+import {catchError, map, take, tap} from 'rxjs/operators';
+import {MessageService} from '@services/message/message.service';
+import {AuthService} from './auth.service';
 
 @Injectable({
     providedIn: 'root'
@@ -16,14 +15,17 @@ export class AuthGuard implements CanActivate {
         protected router: Router,
         protected http: HttpClient,
         private authService: AuthService,
-        protected stateManager: StateManager,
     ) {
     }
 
-    canActivate(route: ActivatedRouteSnapshot, state: RouterStateSnapshot): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree {
-        if (this.authService.isUserLoggedIn.value) {
+    canActivate(
+        route: ActivatedRouteSnapshot
+    ): Observable<boolean | UrlTree> | Promise<boolean | UrlTree> | boolean | UrlTree {
+
+        if (this.authService.isUserLoggedIn.value && route.data.checkSession !== true) {
             return true;
         }
+
         const loginUrl = 'Login';
         const tree: UrlTree = this.router.parseUrl(loginUrl);
 
@@ -38,10 +40,14 @@ export class AuthGuard implements CanActivate {
                         this.authService.setCurrentUser(user);
                         return true;
                     }
+                    this.authService.logout('LBL_SESSION_EXPIRED', false);
                     // Re-direct to login
                     return tree;
                 }),
-                catchError(() => of(tree)),
+                catchError(() => {
+                    this.authService.logout('LBL_SESSION_EXPIRED', false);
+                    return of(tree);
+                }),
                 tap((result: boolean | UrlTree) => {
                     if (result === true) {
                         this.authService.isUserLoggedIn.next(true);

core/app/src/services/auth/auth.service.spec.ts

@@ -11,7 +11,7 @@ describe('Auth Service', () => {
     let languageMock = null;
     let service = null;
     let IdleMock = null;
-    let systemConfigMock = null;
+    let appStateMock = null;
 
     beforeEach(() => {
         TestBed.configureTestingModule({});
@@ -34,7 +34,13 @@ describe('Auth Service', () => {
         });
 
 
-        stateManagerMock = jasmine.createSpyObj('StateManager', ['clear']);
+        stateManagerMock = jasmine.createSpyObj(
+            'StateManager',
+            [
+                'clear',
+                'clearAuthBased'
+            ]
+        );
         stateManagerMock.clear.and.callFake(() => {
         });
 
@@ -42,9 +48,9 @@ describe('Auth Service', () => {
         languageMock.getAppString.and.callFake((key: string) => key);
 
         IdleMock = jasmine.createSpyObj('bnIdle', ['doLogin']);
-        systemConfigMock = jasmine.createSpyObj('systemConfigFacade', ['doLogin']);
+        appStateMock = jasmine.createSpyObj('AppStateFacade', ['updateLoading']);
 
-        service = new AuthService(httpMock, routerMock, messageMock, stateManagerMock, languageMock, IdleMock, systemConfigMock);
+        service = new AuthService(httpMock, routerMock, messageMock, stateManagerMock, languageMock, IdleMock, appStateMock);
 
     });
 
@@ -56,7 +62,7 @@ describe('Auth Service', () => {
 
         expect(httpMock.post).toHaveBeenCalledWith('logout', body.toString(), {headers, responseType: 'text'});
 
-        expect(stateManagerMock.clear).toHaveBeenCalledWith();
+        expect(stateManagerMock.clearAuthBased).toHaveBeenCalledWith();
         expect(languageMock.getAppString).toHaveBeenCalledWith('LBL_LOGOUT_SUCCESS');
         expect(messageMock.addSuccessMessage).toHaveBeenCalledWith('LBL_LOGOUT_SUCCESS');
         expect(messageMock.log).toHaveBeenCalledWith('Logout success');

core/app/src/services/auth/auth.service.ts

@@ -1,7 +1,7 @@
 import {Injectable} from '@angular/core';
 import {Router} from '@angular/router';
 import {HttpClient, HttpErrorResponse, HttpHeaders, HttpParams} from '@angular/common/http';
-import {BehaviorSubject, throwError} from 'rxjs';
+import {BehaviorSubject, Subscription, throwError} from 'rxjs';
 import {catchError, distinctUntilChanged, finalize, take} from 'rxjs/operators';
 import {LoginUiComponent} from '@components/login/login.component';
 import {User} from '@services/user/user';
@@ -9,7 +9,7 @@ import {MessageService} from '@services/message/message.service';
 import {StateManager} from '@base/facades/state-manager';
 import {LanguageFacade} from '@base/facades/language/language.facade';
 import {BnNgIdleService} from 'bn-ng-idle';
-import {SystemConfigFacade} from "@base/facades/system-config/system-config.facade";
+import {AppStateFacade} from '@base/facades/app-state/app-state.facade';
 
 @Injectable({
     providedIn: 'root'
@@ -18,7 +18,7 @@ export class AuthService {
     private currentUserSubject = new BehaviorSubject<User>({} as User);
     public currentUser$ = this.currentUserSubject.asObservable().pipe(distinctUntilChanged());
     public isUserLoggedIn: BehaviorSubject<boolean> = new BehaviorSubject<boolean>(false);
-    defaultTimeout: string = '3600';
+    defaultTimeout = '3600';
 
     constructor(
         private http: HttpClient,
@@ -27,7 +27,7 @@ export class AuthService {
         protected stateManager: StateManager,
         protected languageFacade: LanguageFacade,
         private bnIdle: BnNgIdleService,
-        protected systemConfigFacade: SystemConfigFacade
+        protected appStateFacade: AppStateFacade
     ) {
     }
 
@@ -35,7 +35,7 @@ export class AuthService {
         return this.currentUserSubject.value;
     }
 
-    setCurrentUser(data) {
+    setCurrentUser(data): void {
         this.currentUserSubject.next(data);
     }
 
@@ -45,7 +45,7 @@ export class AuthService {
         password: string,
         onSuccess: (caller: LoginUiComponent, response: string) => void,
         onError: (caller: LoginUiComponent, error: HttpErrorResponse) => void
-    ) {
+    ): Subscription {
         const loginUrl = 'login';
 
         const headers = new HttpHeaders({
@@ -64,7 +64,7 @@ export class AuthService {
             this.isUserLoggedIn.next(true);
             this.setCurrentUser(response);
 
-            let duration = response.duration;
+            const duration = response.duration;
 
             if (duration === 0 || duration === '0') {
                 return;
@@ -80,23 +80,28 @@ export class AuthService {
                     this.message.removeMessages();
                     this.message.addDangerMessage('Session Expired');
                 }
-            })
+            });
         }, (error: HttpErrorResponse) => {
             onError(caller, error);
-        })
+        });
     }
 
     /**
      * Logout user
      *
-     * @param messageKey of message to display
+     * @param {string} messageKey of message to display
+     * @param {boolean} redirect to home
      */
-    logout(messageKey = 'LBL_LOGOUT_SUCCESS'): void {
-        const logoutUrl = 'logout';
+    public logout(messageKey = 'LBL_LOGOUT_SUCCESS', redirect = true): void {
+        this.appStateFacade.updateLoading('logout', true);
 
+        const logoutUrl = 'logout';
         const body = new HttpParams();
+
         const headers = new HttpHeaders().set('Content-Type', 'text/plain; charset=utf-8');
 
+        this.resetState();
+
         this.http.post(logoutUrl, body.toString(), {headers, responseType: 'text'})
             .pipe(
                 take(1),
@@ -105,15 +110,24 @@ export class AuthService {
                     return throwError(err);
                 }),
                 finalize(() => {
-                    this.stateManager.clear();
-                    this.router.navigate(['/Login']).finally();
+                    this.appStateFacade.updateLoading('logout', false);
+                    if (redirect === true) {
+                        this.router.navigate(['/Login']).finally();
+                    }
                 })
             )
             .subscribe(() => {
                 this.message.log('Logout success');
                 const label = this.languageFacade.getAppString(messageKey);
                 this.message.addSuccessMessage(label);
-                this.isUserLoggedIn.next(false);
             });
     }
+
+    /**
+     * On logout state reset
+     */
+    public resetState(): void {
+        this.stateManager.clearAuthBased();
+        this.isUserLoggedIn.next(false);
+    }
 }

core/app/src/services/process/processes/recover-password/recover-password.ts

@@ -17,8 +17,9 @@ export class RecoverPasswordService {
     /**
      * Send recover password request
      *
-     * @param userName
-     * @param email
+     * @param {string} userName to check
+     * @param {string} email to check
+     * @returns {{}} Observable<Process>
      */
     public run(userName: string, email: string): Observable<Process> {
         const options = {
@@ -26,14 +27,14 @@ export class RecoverPasswordService {
             useremail: email
         };
 
-        this.appStateFacade.updateLoading(true);
+        this.appStateFacade.updateLoading('recover-password', true);
 
         return this.processService
             .submit(this.processType, options)
             .pipe(
-                tap(() => this.appStateFacade.updateLoading(false)),
+                tap(() => this.appStateFacade.updateLoading('recover-password',false)),
                 catchError(err => {
-                    this.appStateFacade.updateLoading(false);
+                    this.appStateFacade.updateLoading('recover-password',false);
                     throw err;
                 }),
             );

core/src/Controller/SecurityController.php

@@ -4,9 +4,11 @@ namespace App\Controller;
 
 use Exception;
 use RuntimeException;
+use SuiteCRM\Core\Legacy\Authentication;
 use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
 use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Response;
+use Symfony\Component\HttpFoundation\Session\SessionInterface;
 use Symfony\Component\Routing\Annotation\Route;
 use Symfony\Component\Security\Core\Security;
 use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
@@ -17,6 +19,27 @@ use Symfony\Component\Security\Http\Authentication\AuthenticationUtils;
  */
 class SecurityController extends AbstractController
 {
+    /**
+     * @var Authentication
+     */
+    private $authentication;
+
+    /**
+     * @var SessionInterface
+     */
+    private $session;
+
+    /**
+     * SecurityController constructor.
+     * @param Authentication $authentication
+     * @param SessionInterface $session
+     */
+    public function __construct(Authentication $authentication, SessionInterface $session)
+    {
+        $this->authentication = $authentication;
+        $this->session = $session;
+    }
+
     /**
      * @Route("/login", name="app_login")
      * @param AuthenticationUtils $authenticationUtils
@@ -49,18 +72,30 @@ class SecurityController extends AbstractController
      */
     public function sessionStatus(Security $security): JsonResponse
     {
+
+        $isActive = $this->authentication->checkSession();
+
+        if ($isActive !== true) {
+            $this->session->invalidate();
+            return new JsonResponse(['active' => false], Response::HTTP_OK);
+        }
+
         $user = $security->getUser();
+        if ($user === null) {
+            $this->session->invalidate();
+            return new JsonResponse(['active' => false], Response::HTTP_OK);
+        }
+
         $id = $user->getId();
         $firstName = $user->getFirstName();
         $lastName = $user->getLastName();
 
-        $data =
-            [
-                'active' => true,
-                'id' => $id,
-                'firstName' => $firstName,
-                'lastName' => $lastName
-            ];
+        $data = [
+            'active' => true,
+            'id' => $id,
+            'firstName' => $firstName,
+            'lastName' => $lastName
+        ];
 
         return new JsonResponse($data, Response::HTTP_OK);
     }