Skip to content

Commit c6a651b

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 c6a651b

4 files changed

Lines changed: 198 additions & 19 deletions

File tree

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
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+
87+
it('preserves /login-history as a returnUrl (does not match the exact /login route)', () => {
88+
authServiceMock.isAuthenticated.mockReturnValue(false);
89+
guard.canActivate(emptyRoute, buildState('/login-history'));
90+
expect(routerMock.navigate).toHaveBeenCalledWith(['/login'], {
91+
queryParams: { returnUrl: '/login-history' },
92+
replaceUrl: true
93+
});
94+
});
95+
});

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

Lines changed: 24 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,37 @@ 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). The login check
50+
// matches the exact /login path (with optional query/fragment) so
51+
// unrelated routes like /login-history keep their deep link.
52+
const targetUrl = state.url;
53+
const isLoginTarget = targetUrl === '/login' || targetUrl.startsWith('/login?') || targetUrl.startsWith('/login#');
54+
const isMeaningfulTarget = !!targetUrl && targetUrl !== '/' && !isLoginTarget;
55+
56+
this.router.navigate(['/login'], {
57+
queryParams: isMeaningfulTarget ? { returnUrl: targetUrl } : {},
58+
replaceUrl: true
59+
});
4160
return false;
4261
}
4362
}

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

Lines changed: 73 additions & 12 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
/**
@@ -182,12 +226,12 @@ export class AuthenticationService {
182226
}
183227

184228
/**
185-
* Reads the cached credentials from session or local storage, if present.
229+
* Reads the cached credentials from localStorage, the single source of
230+
* truth for the authenticated session (see class doc).
186231
* @returns {Credentials | null} Stored credentials or null when absent.
187232
*/
188233
private getSavedCredentials(): Credentials | null {
189-
const stored =
190-
sessionStorage.getItem(this.credentialsStorageKey) || localStorage.getItem(this.credentialsStorageKey);
234+
const stored = localStorage.getItem(this.credentialsStorageKey);
191235
return stored ? JSON.parse(stored) : null;
192236
}
193237

@@ -220,7 +264,10 @@ export class AuthenticationService {
220264
}
221265

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

225272
// Basic Auth: Direct authentication with Fineract
226273
return this.http
@@ -374,6 +421,7 @@ export class AuthenticationService {
374421
this.setCredentials();
375422
this.resetDialog();
376423
this.userLoggedIn$.next(false);
424+
this.broadcastChannel?.postMessage({ type: 'logout' });
377425

378426
if (this.authMode === AuthMode.OIDC) {
379427
// OIDC: Use library to handle logout (redirects to OIDC provider)
@@ -435,11 +483,19 @@ export class AuthenticationService {
435483
*/
436484
private setCredentials(credentials?: Credentials): void {
437485
if (credentials) {
486+
// Capture whether credentials were already persisted BEFORE we write
487+
// them. We cannot rely on userLoggedIn$ here because onLoginSuccess()
488+
// sets it to `true` before calling setCredentials(), which would make
489+
// every login look like a no-op refresh and skip the cross-tab broadcast.
490+
const hadPersistedCredentials = !!this.getSavedCredentials();
438491
credentials.rememberMe = this.rememberMe;
439-
// Make sure we're using the correct storage based on rememberMe value
440-
this.storage = credentials.rememberMe ? localStorage : sessionStorage;
492+
// Credentials are always written to localStorage so the session is
493+
// shared across tabs/windows of the same origin (see class doc).
494+
this.storage = localStorage;
441495
this.oauthService.setStorage(this.storage);
442496
this.storage.setItem(this.credentialsStorageKey, JSON.stringify(credentials));
497+
// Notify other tabs only on a NEW login (not on every credential refresh).
498+
if (!hadPersistedCredentials) this.broadcastChannel?.postMessage({ type: 'login' });
443499
} else {
444500
// Clear credentials from both storage types to ensure complete logout
445501
[
@@ -533,13 +589,18 @@ export class AuthenticationService {
533589
message: this.translateService.instant('errors.auth.passwordExpired.message')
534590
});
535591
} else {
592+
// Persist the 2FA token in shared storage BEFORE setCredentials so
593+
// any other tab waking up from the cross-tab login broadcast (fired
594+
// by setCredentials) rehydrates with the matching 2FA header. Using
595+
// localStorage directly here keeps the order explicit and avoids
596+
// depending on the current value of `this.storage`.
597+
localStorage.setItem(this.twoFactorAuthenticationTokenStorageKey, JSON.stringify(response));
536598
this.setCredentials(this.credentials);
537599
this.alertService.alert({
538600
type: this.translateService.instant('errors.auth.success.type'),
539601
message: this.translateService.instant('errors.auth.success.message', { username: this.credentials.username })
540602
});
541603
delete this.credentials;
542-
this.storage.setItem(this.twoFactorAuthenticationTokenStorageKey, JSON.stringify(response));
543604
}
544605
}
545606

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)