WEB-956: share auth session across tabs and preserve deep links

playwright/auth.setup.ts

@@ -29,16 +29,23 @@ setup('authenticate', async ({ page, browser }) => {
   await loginPage.navigate();
   await loginPage.loginAndWaitForDashboard(username, password);
 
-  console.log('Auth setup: copying mifosXCredentials from sessionStorage → localStorage');
-  const credsCopied = await page.evaluate(() => {
-    const creds = sessionStorage.getItem('mifosXCredentials');
-    if (!creds) return false;
-    localStorage.setItem('mifosXCredentials', creds);
-    return true;
+  // Ensure mifosXCredentials is persisted in localStorage so storageState
+  // captures a shareable, cross-tab session. AuthenticationService now
+  // writes to localStorage directly, but we still accept sessionStorage
+  // as a legacy source while older branches/builds are around.
+  const credsAvailable = await page.evaluate(() => {
+    const fromLocal = localStorage.getItem('mifosXCredentials');
+    if (fromLocal) return true;
+    const fromSession = sessionStorage.getItem('mifosXCredentials');
+    if (fromSession) {
+      localStorage.setItem('mifosXCredentials', fromSession);
+      return true;
+    }
+    return false;
   });
 
-  if (!credsCopied) {
-    throw new Error('CRITICAL: mifosXCredentials not found in sessionStorage. ' + 'Did the auth storage key change?');
+  if (!credsAvailable) {
+    throw new Error('CRITICAL: mifosXCredentials not found in either storage after login.');
   }
 
   await page.context().storageState({ path: authFile });

src/app/core/authentication/authentication.guard.spec.ts

@@ -0,0 +1,95 @@
+/**
+ * Copyright since 2025 Mifos Initiative
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+import { TestBed } from '@angular/core/testing';
+import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
+
+import { AuthenticationGuard } from './authentication.guard';
+import { AuthenticationService } from './authentication.service';
+
+describe('AuthenticationGuard', () => {
+  let guard: AuthenticationGuard;
+  let authServiceMock: jest.Mocked<Pick<AuthenticationService, 'isAuthenticated' | 'logout'>>;
+  let routerMock: jest.Mocked<Pick<Router, 'navigate'>>;
+
+  const buildState = (url: string): RouterStateSnapshot => ({ url }) as RouterStateSnapshot;
+  const emptyRoute = {} as ActivatedRouteSnapshot;
+
+  beforeEach(() => {
+    authServiceMock = {
+      isAuthenticated: jest.fn(),
+      logout: jest.fn()
+    };
+    routerMock = { navigate: jest.fn() };
+
+    TestBed.configureTestingModule({
+      providers: [
+        AuthenticationGuard,
+        { provide: AuthenticationService, useValue: authServiceMock },
+        { provide: Router, useValue: routerMock }]
+    });
+
+    guard = TestBed.inject(AuthenticationGuard);
+  });
+
+  it('allows the route when user is authenticated', () => {
+    authServiceMock.isAuthenticated.mockReturnValue(true);
+    const result = guard.canActivate(emptyRoute, buildState('/clients'));
+    expect(result).toBe(true);
+    expect(routerMock.navigate).not.toHaveBeenCalled();
+    expect(authServiceMock.logout).not.toHaveBeenCalled();
+  });
+
+  it('redirects to /login with returnUrl when target is a deep link', () => {
+    authServiceMock.isAuthenticated.mockReturnValue(false);
+    const result = guard.canActivate(emptyRoute, buildState('/clients/1/general'));
+    expect(result).toBe(false);
+    expect(authServiceMock.logout).toHaveBeenCalledTimes(1);
+    expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], {
+      queryParams: { returnUrl: '/clients/1/general' },
+      replaceUrl: true
+    });
+  });
+
+  it('redirects to /login WITHOUT returnUrl when target is "/" (default landing)', () => {
+    authServiceMock.isAuthenticated.mockReturnValue(false);
+    guard.canActivate(emptyRoute, buildState('/'));
+    expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], { queryParams: {}, replaceUrl: true });
+  });
+
+  it('redirects to /login WITHOUT returnUrl when target is already /login (avoids loops)', () => {
+    authServiceMock.isAuthenticated.mockReturnValue(false);
+    guard.canActivate(emptyRoute, buildState('/login'));
+    expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], { queryParams: {}, replaceUrl: true });
+  });
+
+  it('redirects to /login WITHOUT returnUrl when target is /login with stale params', () => {
+    authServiceMock.isAuthenticated.mockReturnValue(false);
+    guard.canActivate(emptyRoute, buildState('/login?returnUrl=/foo'));
+    expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], { queryParams: {}, replaceUrl: true });
+  });
+
+  it('preserves complex URLs with query params and fragments', () => {
+    authServiceMock.isAuthenticated.mockReturnValue(false);
+    const target = '/loans/42?tab=schedule#repayment';
+    guard.canActivate(emptyRoute, buildState(target));
+    expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], {
+      queryParams: { returnUrl: target },
+      replaceUrl: true
+    });
+  });
+
+  it('preserves /login-history as a returnUrl (does not match the exact /login route)', () => {
+    authServiceMock.isAuthenticated.mockReturnValue(false);
+    guard.canActivate(emptyRoute, buildState('/login-history'));
+    expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], {
+      queryParams: { returnUrl: '/login-history' },
+      replaceUrl: true
+    });
+  });
+});

src/app/core/authentication/authentication.guard.ts

@@ -8,7 +8,7 @@
 
 /** Angular Imports */
 import { Injectable, inject } from '@angular/core';
-import { Router } from '@angular/router';
+import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
 
 /** Custom Services */
 import { Logger } from '../logger/logger.service';
@@ -26,18 +26,37 @@ export class AuthenticationGuard {
   private authenticationService = inject(AuthenticationService);
 
   /**
-   * Ensures route access is authorized only when user is authenticated, otherwise redirects to login.
+   * Ensures route access is authorized only when user is authenticated.
    *
+   * If unauthenticated, redirects to /login while preserving the originally
+   * requested URL in the `returnUrl` query param so the LoginComponent can
+   * restore it after a successful authentication.
+   *
+   * @param _route Activated route snapshot (unused, kept for guard signature).
+   * @param state  Router state — provides the URL the user was trying to reach.
    * @returns {boolean} True if user is authenticated.
    */
-  canActivate(): boolean {
+  canActivate(_route: ActivatedRouteSnapshot, state: RouterStateSnapshot): boolean {
     if (this.authenticationService.isAuthenticated()) {
       return true;
     }
 
-    log.debug('User not authenticated, redirecting to login...');
+    log.debug(`User not authenticated, redirecting to login (target was: ${state.url})...`);
     this.authenticationService.logout();
-    this.router.navigate(['/login'], { replaceUrl: true });
+
+    // Preserve the originally requested URL so the user can be sent back
+    // there after authenticating. We only forward non-trivial targets
+    // (avoid carrying "/" or "/login" as the returnUrl). The login check
+    // matches the exact /login path (with optional query/fragment) so
+    // unrelated routes like /login-history keep their deep link.
+    const targetUrl = state.url;
+    const isLoginTarget = targetUrl === '/login' || targetUrl.startsWith('/login?') || targetUrl.startsWith('/login#');
+    const isMeaningfulTarget = !!targetUrl && targetUrl !== '/' && !isLoginTarget;
+
+    this.router.navigate(['/login'], {
+      queryParams: isMeaningfulTarget ? { returnUrl: targetUrl } : {},
+      replaceUrl: true
+    });
     return false;
   }
 }

src/app/core/authentication/authentication.service.spec.ts

@@ -0,0 +1,191 @@
+/**
+ * Copyright since 2025 Mifos Initiative
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+import { TestBed } from '@angular/core/testing';
+import { HttpClient } from '@angular/common/http';
+import { OAuthService } from 'angular-oauth2-oidc';
+import { TranslateService } from '@ngx-translate/core';
+import { BehaviorSubject } from 'rxjs';
+
+import { AuthenticationService } from './authentication.service';
+import { AuthenticationInterceptor } from './authentication.interceptor';
+import { AlertService } from '../alert/alert.service';
+import { AuthMode } from './oauth.config';
+import { environment } from '../../../environments/environment';
+
+/**
+ * Cross-cutting tests for the storage / cross-tab behaviour added by
+ * WEB-956. These exercise the code paths that differ per `AuthMode`
+ * (Basic, OAuth2, OIDC, +2FA) without requiring a live backend.
+ *
+ * The cross-tab logout listener and the `initializeOAuthService()`
+ * storage choice are the parts of the fix that only fire in non-Basic
+ * modes; testing them here lets us prove correctness for OAuth2/OIDC
+ * without needing a Keycloak / Zitadel / mock-oauth2-server.
+ */
+describe('AuthenticationService — cross-mode storage & cross-tab logout', () => {
+  let oauthService: jest.Mocked<
+    Pick<
+      OAuthService,
+      | 'configure'
+      | 'setStorage'
+      | 'logOut'
+      | 'loadDiscoveryDocumentAndTryLogin'
+      | 'setupAutomaticSilentRefresh'
+      | 'events'
+      | 'hasValidAccessToken'
+      | 'getAccessToken'
+      | 'getRefreshToken'
+      | 'refreshToken'
+    >
+  >;
+  let interceptor: jest.Mocked<
+    Pick<
+      AuthenticationInterceptor,
+      'setAuthorizationToken' | 'removeAuthorization' | 'removeTwoFactorAuthorization' | 'setTwoFactorAccessToken'
+    >
+  >;
+  let alertService: jest.Mocked<Pick<AlertService, 'alert' | 'alertEvent'>>;
+  let translate: jest.Mocked<Pick<TranslateService, 'instant'>>;
+  let http: jest.Mocked<Pick<HttpClient, 'post' | 'put'>>;
+
+  beforeEach(() => {
+    localStorage.clear();
+    sessionStorage.clear();
+    // Reset auth mode flags before each test so they don't leak between cases.
+    (environment.OIDC as any).oidcServerEnabled = false;
+    (environment.oauth as any).enabled = false;
+
+    oauthService = {
+      configure: jest.fn(),
+      setStorage: jest.fn(),
+      logOut: jest.fn(),
+      loadDiscoveryDocumentAndTryLogin: jest.fn().mockResolvedValue(true),
+      setupAutomaticSilentRefresh: jest.fn(),
+      events: new BehaviorSubject({ type: 'idle' }) as any,
+      hasValidAccessToken: jest.fn().mockReturnValue(false),
+      getAccessToken: jest.fn().mockReturnValue(''),
+      getRefreshToken: jest.fn().mockReturnValue(''),
+      refreshToken: jest.fn().mockResolvedValue({})
+    } as any;
+
+    interceptor = {
+      setAuthorizationToken: jest.fn(),
+      removeAuthorization: jest.fn(),
+      removeTwoFactorAuthorization: jest.fn(),
+      setTwoFactorAccessToken: jest.fn()
+    } as any;
+
+    alertService = {
+      alert: jest.fn(),
+      alertEvent: new BehaviorSubject({} as any)
+    } as any;
+
+    translate = {
+      instant: jest.fn((k: string) => k)
+    } as any;
+
+    http = {
+      post: jest.fn(),
+      put: jest.fn()
+    } as any;
+
+    TestBed.configureTestingModule({
+      providers: [
+        AuthenticationService,
+        { provide: OAuthService, useValue: oauthService },
+        { provide: AuthenticationInterceptor, useValue: interceptor },
+        { provide: AlertService, useValue: alertService },
+        { provide: TranslateService, useValue: translate },
+        { provide: HttpClient, useValue: http }]
+    });
+  });
+
+  describe('initializeOAuthService — storage choice', () => {
+    it('uses localStorage as OAuth tokens store (independent of enableRememberMe)', async () => {
+      // Force non-Basic mode so initializeOAuthService runs in the constructor.
+      (environment.OIDC as any).oidcServerEnabled = true;
+      // Instantiate the service (TestBed.inject does this lazily).
+      const svc = TestBed.inject(AuthenticationService);
+      // setStorage should be called once with localStorage in init.
+      const calls = oauthService.setStorage.mock.calls;
+      expect(calls.length).toBeGreaterThanOrEqual(1);
+      expect(calls[0][0]).toBe(localStorage);
+    });
+  });
+
+  describe('listenForCrossTabAuthEvents — Basic mode logout', () => {
+    it('clears credentials + 2FA token + headers when a logout broadcast arrives (Basic)', () => {
+      // Basic mode is the default — no environment overrides needed.
+      const svc = TestBed.inject(AuthenticationService);
+      // Seed both stores to simulate a logged-in tab with 2FA.
+      localStorage.setItem('mifosXCredentials', JSON.stringify({ username: 'mifos', rememberMe: true }));
+      sessionStorage.setItem('mifosXCredentials', JSON.stringify({ username: 'mifos', rememberMe: false }));
+      localStorage.setItem('mifosXTwoFactorAuthenticationToken', JSON.stringify({ token: 'abc' }));
+      sessionStorage.setItem('mifosXTwoFactorAuthenticationToken', JSON.stringify({ token: 'abc' }));
+      // Mark this tab as logged-in so the listener acts on the broadcast.
+      (svc as any).userLoggedIn$.next(true);
+      (svc as any).handleCrossTabAuthEvent({ data: { type: 'logout' } });
+
+      expect(interceptor.removeAuthorization).toHaveBeenCalledTimes(1);
+      expect(interceptor.removeTwoFactorAuthorization).toHaveBeenCalledTimes(1);
+      expect(localStorage.getItem('mifosXCredentials')).toBeFalsy();
+      expect(sessionStorage.getItem('mifosXCredentials')).toBeFalsy();
+      expect(localStorage.getItem('mifosXTwoFactorAuthenticationToken')).toBeFalsy();
+      expect(sessionStorage.getItem('mifosXTwoFactorAuthenticationToken')).toBeFalsy();
+      // Basic mode → MUST NOT call oauthService.logOut (no OAuth tokens to clear).
+      expect(oauthService.logOut).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('listenForCrossTabAuthEvents — OAuth2 / OIDC mode logout', () => {
+    it('additionally clears OAuth library tokens via oauthService.logOut(true) (OIDC)', () => {
+      (environment.OIDC as any).oidcServerEnabled = true;
+      const svc = TestBed.inject(AuthenticationService);
+      (svc as any).userLoggedIn$.next(true);
+
+      (svc as any).handleCrossTabAuthEvent({ data: { type: 'logout' } });
+
+      // Critical: passive tab must call logOut(true) so library-managed tokens
+      // (access_token / id_token / refresh_token) don't linger.
+      expect(oauthService.logOut).toHaveBeenCalledWith(true);
+    });
+
+    it('additionally clears OAuth library tokens via oauthService.logOut(true) (OAuth2)', () => {
+      (environment.oauth as any).enabled = true;
+      const svc = TestBed.inject(AuthenticationService);
+      (svc as any).userLoggedIn$.next(true);
+
+      (svc as any).handleCrossTabAuthEvent({ data: { type: 'logout' } });
+
+      expect(oauthService.logOut).toHaveBeenCalledWith(true);
+    });
+  });
+
+  describe('listenForCrossTabAuthEvents — login broadcast', () => {
+    it('does nothing when current tab is already logged-in (avoids re-broadcast loops)', () => {
+      // Basic mode is the default — no environment overrides needed.
+      const svc = TestBed.inject(AuthenticationService);
+      (svc as any).userLoggedIn$.next(true);
+
+      (svc as any).handleCrossTabAuthEvent({ data: { type: 'login' } });
+
+      // No restoreSession side-effects when we're already logged-in.
+      expect(interceptor.setAuthorizationToken).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('storage envelope', () => {
+    it('default storage field is localStorage (no per-tab fragmentation)', () => {
+      // Basic mode is the default — no environment overrides needed.
+      const svc = TestBed.inject(AuthenticationService);
+      // Direct private field access via `any` — explicit cross-mode invariant.
+      expect((svc as any).storage).toBe(localStorage);
+    });
+  });
+});