Skip to content

Commit e4e13a5

Browse files
Jonattan Infanteclaude
andcommitted
WEB-956: share auth session across tabs and preserve deep links
Currently AuthenticationService persists the user credentials in sessionStorage by default, which is scoped to a single browser tab. As a result, any link to a Mifos URL opened from outside the current tab — email, chat, bookmark, target="_blank", a new browser window — boots into an empty storage and the AuthenticationGuard sends the already-logged-in user back to /login. Compounding this, AuthenticationGuard does not propagate the requested URL and LoginComponent unconditionally navigates to "/" after a successful login, so the originally requested deep link is permanently lost. This change adopts the pattern used by `@supabase/auth-js` (GoTrueClient.ts) and Auth0's SPA SDK: localStorage as the single source of truth for the session, plus a BroadcastChannel for cross-tab synchronisation. It also makes the guard forward the target URL through the login flow. - AuthenticationService.storage is now localStorage unconditionally, so the session is visible to every tab/window of the same origin. The `rememberMe` flag is preserved for the backend token-expiration policy but no longer chooses the Storage instance. - getSavedCredentials() prefers localStorage and performs a one-time migration of any leftover sessionStorage credentials so existing users do not get stranded after the upgrade. - A BroadcastChannel named `mifosXAuth` is created lazily (with a feature-detect fallback). Login broadcasts `{ type: 'login' }`, logout broadcasts `{ type: 'logout' }`; the listener uses hadPersistedCredentials to fire the login broadcast even though onLoginSuccess() flips userLoggedIn$ before setCredentials() runs. - The cross-tab logout handler now also clears the two-factor token storage and the two-factor authorization header, mirroring what logout() does on the active tab. - AuthenticationGuard.canActivate now accepts RouterStateSnapshot and forwards state.url as a returnUrl query parameter when the target is a meaningful route. Trivial targets ("/", "/login") are skipped to avoid noise and redirect loops. - LoginComponent reads queryParamMap.get('returnUrl') from the ActivatedRoute when the authentication success alert fires, and navigates there via navigateByUrl. Falls back to "/" when no returnUrl is present. - Adds unit tests for the guard covering: authenticated pass-through, deep-link capture, "/" and "/login" being skipped to avoid loops/noise, stale-params handling, and URLs with query params plus fragments. Verified end-to-end against an isolated docker-compose stack (postgres + fineract + web-app): before: /#/clients -> /#/login -> /#/home (bug) after: /#/clients -> /#/login?returnUrl=/clients -> /#/clients cross-tab logout: tab A logs out -> tab B reacts live. Addresses CodeRabbit review feedback: - getSavedCredentials() now treats localStorage as authoritative and migrates legacy sessionStorage credentials once. - Cross-tab logout handler also clears 2FA state. - Login broadcast is gated on persisted credentials, not on userLoggedIn$, so a fresh login is correctly announced to other tabs even though onLoginSuccess() sets userLoggedIn$ first. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a5bafdf commit e4e13a5

4 files changed

Lines changed: 192 additions & 18 deletions

File tree

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
/**
2+
* Copyright since 2025 Mifos Initiative
3+
*
4+
* This Source Code Form is subject to the terms of the Mozilla Public
5+
* License, v. 2.0. If a copy of the MPL was not distributed with this
6+
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
7+
*/
8+
9+
import { TestBed } from '@angular/core/testing';
10+
import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
11+
12+
import { AuthenticationGuard } from './authentication.guard';
13+
import { AuthenticationService } from './authentication.service';
14+
15+
describe('AuthenticationGuard', () => {
16+
let guard: AuthenticationGuard;
17+
let authServiceMock: jest.Mocked<Pick<AuthenticationService, 'isAuthenticated' | 'logout'>>;
18+
let routerMock: jest.Mocked<Pick<Router, 'navigate'>>;
19+
20+
const buildState = (url: string): RouterStateSnapshot => ({ url }) as RouterStateSnapshot;
21+
const emptyRoute = {} as ActivatedRouteSnapshot;
22+
23+
beforeEach(() => {
24+
authServiceMock = {
25+
isAuthenticated: jest.fn(),
26+
logout: jest.fn()
27+
};
28+
routerMock = { navigate: jest.fn() };
29+
30+
TestBed.configureTestingModule({
31+
providers: [
32+
AuthenticationGuard,
33+
{ provide: AuthenticationService, useValue: authServiceMock },
34+
{ provide: Router, useValue: routerMock }]
35+
});
36+
37+
guard = TestBed.inject(AuthenticationGuard);
38+
});
39+
40+
it('allows the route when user is authenticated', () => {
41+
authServiceMock.isAuthenticated.mockReturnValue(true);
42+
const result = guard.canActivate(emptyRoute, buildState('/clients'));
43+
expect(result).toBe(true);
44+
expect(routerMock.navigate).not.toHaveBeenCalled();
45+
expect(authServiceMock.logout).not.toHaveBeenCalled();
46+
});
47+
48+
it('redirects to /login with returnUrl when target is a deep link', () => {
49+
authServiceMock.isAuthenticated.mockReturnValue(false);
50+
const result = guard.canActivate(emptyRoute, buildState('/clients/1/general'));
51+
expect(result).toBe(false);
52+
expect(authServiceMock.logout).toHaveBeenCalledTimes(1);
53+
expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], {
54+
queryParams: { returnUrl: '/clients/1/general' },
55+
replaceUrl: true
56+
});
57+
});
58+
59+
it('redirects to /login WITHOUT returnUrl when target is "/" (default landing)', () => {
60+
authServiceMock.isAuthenticated.mockReturnValue(false);
61+
guard.canActivate(emptyRoute, buildState('/'));
62+
expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], { queryParams: {}, replaceUrl: true });
63+
});
64+
65+
it('redirects to /login WITHOUT returnUrl when target is already /login (avoids loops)', () => {
66+
authServiceMock.isAuthenticated.mockReturnValue(false);
67+
guard.canActivate(emptyRoute, buildState('/login'));
68+
expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], { queryParams: {}, replaceUrl: true });
69+
});
70+
71+
it('redirects to /login WITHOUT returnUrl when target is /login with stale params', () => {
72+
authServiceMock.isAuthenticated.mockReturnValue(false);
73+
guard.canActivate(emptyRoute, buildState('/login?returnUrl=/foo'));
74+
expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], { queryParams: {}, replaceUrl: true });
75+
});
76+
77+
it('preserves complex URLs with query params and fragments', () => {
78+
authServiceMock.isAuthenticated.mockReturnValue(false);
79+
const target = '/loans/42?tab=schedule#repayment';
80+
guard.canActivate(emptyRoute, buildState(target));
81+
expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], {
82+
queryParams: { returnUrl: target },
83+
replaceUrl: true
84+
});
85+
});
86+
});

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

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
/** Angular Imports */
1010
import { Injectable, inject } from '@angular/core';
11-
import { Router } from '@angular/router';
11+
import { ActivatedRouteSnapshot, Router, RouterStateSnapshot } from '@angular/router';
1212

1313
/** Custom Services */
1414
import { Logger } from '../logger/logger.service';
@@ -26,18 +26,34 @@ export class AuthenticationGuard {
2626
private authenticationService = inject(AuthenticationService);
2727

2828
/**
29-
* Ensures route access is authorized only when user is authenticated, otherwise redirects to login.
29+
* Ensures route access is authorized only when user is authenticated.
3030
*
31+
* If unauthenticated, redirects to /login while preserving the originally
32+
* requested URL in the `returnUrl` query param so the LoginComponent can
33+
* restore it after a successful authentication.
34+
*
35+
* @param _route Activated route snapshot (unused, kept for guard signature).
36+
* @param state Router state — provides the URL the user was trying to reach.
3137
* @returns {boolean} True if user is authenticated.
3238
*/
33-
canActivate(): boolean {
39+
canActivate(_route: ActivatedRouteSnapshot, state: RouterStateSnapshot): boolean {
3440
if (this.authenticationService.isAuthenticated()) {
3541
return true;
3642
}
3743

38-
log.debug('User not authenticated, redirecting to login...');
44+
log.debug(`User not authenticated, redirecting to login (target was: ${state.url})...`);
3945
this.authenticationService.logout();
40-
this.router.navigate(['/login'], { replaceUrl: true });
46+
47+
// Preserve the originally requested URL so the user can be sent back
48+
// there after authenticating. We only forward non-trivial targets
49+
// (avoid carrying "/" or "/login" as the returnUrl).
50+
const targetUrl = state.url;
51+
const isMeaningfulTarget = targetUrl && targetUrl !== '/' && !targetUrl.startsWith('/login');
52+
53+
this.router.navigate(['/login'], {
54+
queryParams: isMeaningfulTarget ? { returnUrl: targetUrl } : {},
55+
replaceUrl: true
56+
});
4157
return false;
4258
}
4359
}

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

Lines changed: 79 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -58,13 +58,17 @@ export class AuthenticationService {
5858
/** Denotes whether the user credentials should persist through sessions. */
5959
private rememberMe = false;
6060
/**
61-
* Denotes the type of storage:
61+
* Credentials are always persisted in localStorage so the authenticated
62+
* session is shared across browser tabs and windows of the same origin.
63+
* sessionStorage would scope the credentials to a single tab, which forces
64+
* an unnecessary re-login when the user opens any internal link from an
65+
* external program (email, chat, bookmark, target="_blank", etc.).
6266
*
63-
* Session Storage: User credentials should not persist through sessions.
64-
*
65-
* Local Storage: User credentials should persist through sessions.
67+
* The `rememberMe` flag still controls the token expiration policy on the
68+
* backend; on logout (or when credentials are cleared explicitly) the
69+
* storage is wiped from both localStorage and sessionStorage.
6670
*/
67-
private storage: Storage = sessionStorage;
71+
private storage: Storage = localStorage;
6872
private credentials: Credentials;
6973
private dialogShown = false;
7074
private authMode: AuthMode = AuthMode.Basic;
@@ -77,6 +81,17 @@ export class AuthenticationService {
7781
/** Key to store two factor authentication token in storage. */
7882
private readonly twoFactorAuthenticationTokenStorageKey = 'mifosXTwoFactorAuthenticationToken';
7983

84+
/**
85+
* Broadcast channel used to synchronise login/logout events across browser
86+
* tabs and windows of the same origin. Mirrors the pattern used by
87+
* `@supabase/auth-js` (see supabase/auth-js GoTrueClient.ts) so that a
88+
* logout in any tab propagates to the others without waiting for a page
89+
* reload, and a login in a new tab updates already-open ones too.
90+
* Falls back gracefully when BroadcastChannel is unavailable.
91+
*/
92+
private readonly broadcastChannel: BroadcastChannel | null =
93+
typeof BroadcastChannel !== 'undefined' ? new BroadcastChannel('mifosXAuth') : null;
94+
8095
/**
8196
* Initializes the type of storage and authorization headers depending on whether
8297
* credentials are presently in storage or not.
@@ -93,6 +108,35 @@ export class AuthenticationService {
93108
}
94109

95110
this.restoreSession();
111+
this.listenForCrossTabAuthEvents();
112+
}
113+
114+
/**
115+
* Listens for login/logout events broadcast from other tabs and updates
116+
* the local authentication state accordingly. This keeps the UI in sync
117+
* (e.g. a logout in tab A removes the session from tab B in real-time).
118+
*/
119+
private listenForCrossTabAuthEvents(): void {
120+
if (!this.broadcastChannel) return;
121+
this.broadcastChannel.onmessage = (event: MessageEvent<{ type: 'login' | 'logout' }>) => {
122+
if (event.data?.type === 'logout' && this.userLoggedIn$.getValue()) {
123+
// Mirror the logout effects without re-broadcasting (avoid loops).
124+
// Includes 2FA state to match what logout() clears on the active tab.
125+
this.authenticationInterceptor.removeAuthorization();
126+
this.authenticationInterceptor.removeTwoFactorAuthorization();
127+
[
128+
localStorage,
129+
sessionStorage
130+
].forEach((store) => {
131+
store.removeItem(this.credentialsStorageKey);
132+
store.removeItem(this.twoFactorAuthenticationTokenStorageKey);
133+
});
134+
this.userLoggedIn$.next(false);
135+
} else if (event.data?.type === 'login' && !this.userLoggedIn$.getValue()) {
136+
// Another tab logged in: rehydrate from shared storage.
137+
this.restoreSession();
138+
}
139+
};
96140
}
97141

98142
/**
@@ -186,9 +230,21 @@ export class AuthenticationService {
186230
* @returns {Credentials | null} Stored credentials or null when absent.
187231
*/
188232
private getSavedCredentials(): Credentials | null {
189-
const stored =
190-
sessionStorage.getItem(this.credentialsStorageKey) || localStorage.getItem(this.credentialsStorageKey);
191-
return stored ? JSON.parse(stored) : null;
233+
// localStorage is the authoritative source for the shared session.
234+
const local = localStorage.getItem(this.credentialsStorageKey);
235+
if (local) return JSON.parse(local);
236+
237+
// One-time migration: any credentials still lingering in
238+
// sessionStorage from older builds are promoted to localStorage
239+
// so the session is shared across tabs going forward.
240+
const legacySession = sessionStorage.getItem(this.credentialsStorageKey);
241+
if (legacySession) {
242+
localStorage.setItem(this.credentialsStorageKey, legacySession);
243+
sessionStorage.removeItem(this.credentialsStorageKey);
244+
return JSON.parse(legacySession);
245+
}
246+
247+
return null;
192248
}
193249

194250
/**
@@ -220,7 +276,10 @@ export class AuthenticationService {
220276
}
221277

222278
this.rememberMe = environment.enableRememberMe ? (loginContext?.remember ?? false) : false;
223-
this.storage = this.rememberMe ? localStorage : sessionStorage;
279+
// Always use localStorage so the authenticated session is visible
280+
// to any tab/window opened against the same origin (links from
281+
// email, chat, bookmarks, target="_blank", new browser windows, ...).
282+
this.storage = localStorage;
224283

225284
// Basic Auth: Direct authentication with Fineract
226285
return this.http
@@ -374,6 +433,7 @@ export class AuthenticationService {
374433
this.setCredentials();
375434
this.resetDialog();
376435
this.userLoggedIn$.next(false);
436+
this.broadcastChannel?.postMessage({ type: 'logout' });
377437

378438
if (this.authMode === AuthMode.OIDC) {
379439
// OIDC: Use library to handle logout (redirects to OIDC provider)
@@ -435,11 +495,19 @@ export class AuthenticationService {
435495
*/
436496
private setCredentials(credentials?: Credentials): void {
437497
if (credentials) {
498+
// Capture whether credentials were already persisted BEFORE we write
499+
// them. We cannot rely on userLoggedIn$ here because onLoginSuccess()
500+
// sets it to `true` before calling setCredentials(), which would make
501+
// every login look like a no-op refresh and skip the cross-tab broadcast.
502+
const hadPersistedCredentials = !!this.getSavedCredentials();
438503
credentials.rememberMe = this.rememberMe;
439-
// Make sure we're using the correct storage based on rememberMe value
440-
this.storage = credentials.rememberMe ? localStorage : sessionStorage;
504+
// Credentials are always written to localStorage so the session is
505+
// shared across tabs/windows of the same origin (see class doc).
506+
this.storage = localStorage;
441507
this.oauthService.setStorage(this.storage);
442508
this.storage.setItem(this.credentialsStorageKey, JSON.stringify(credentials));
509+
// Notify other tabs only on a NEW login (not on every credential refresh).
510+
if (!hadPersistedCredentials) this.broadcastChannel?.postMessage({ type: 'login' });
443511
} else {
444512
// Clear credentials from both storage types to ensure complete logout
445513
[

src/app/login/login.component.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88

99
/** Angular Imports */
1010
import { ChangeDetectionStrategy, Component, OnInit, OnDestroy, inject } from '@angular/core';
11-
import { Router } from '@angular/router';
11+
import { ActivatedRoute, Router } from '@angular/router';
1212

1313
/** rxjs Imports */
1414

@@ -86,6 +86,7 @@ export class LoginComponent implements OnInit, OnDestroy {
8686
private settingsService = inject(SettingsService);
8787
private themingService = inject(ThemingService);
8888
private router = inject(Router);
89+
private activatedRoute = inject(ActivatedRoute);
8990

9091
private versionService = inject(VersionService);
9192
private translateService = inject(TranslateService);
@@ -143,7 +144,10 @@ export class LoginComponent implements OnInit, OnDestroy {
143144
} else if (alertType === this.translateService.instant('errors.auth.success.type')) {
144145
this.resetPassword = false;
145146
this.twoFactorAuthenticationRequired = false;
146-
this.router.navigate(['/'], { replaceUrl: true });
147+
const returnUrl = this.activatedRoute.snapshot.queryParamMap.get('returnUrl');
148+
// Restore the originally requested deep link if the AuthenticationGuard captured one.
149+
// Falls back to root (default landing) when no returnUrl is present.
150+
this.router.navigateByUrl(returnUrl || '/', { replaceUrl: true });
147151
} else if (alertType === this.translateService.instant('errors.tenant.changed.type')) {
148152
this.updateLogo();
149153
}

0 commit comments

Comments
 (0)