Skip to content

Commit 06c9ae5

Browse files
Jonattan Infanteclaude
andcommitted
WEB-956: preserve deep-link after login redirect
Currently when an unauthenticated user opens a deep link (e.g. /clients/1 from a bookmark, a shared URL or an email), AuthenticationGuard logs the user out and navigates to /login with replaceUrl:true. Because the guard does not propagate the originally requested URL and LoginComponent unconditionally navigates to "/" after a successful authentication, the user always lands on the home dashboard instead of the page they were trying to reach. This change: - AuthenticationGuard.canActivate now accepts RouterStateSnapshot and forwards state.url as a returnUrl query parameter when the target is a meaningful route (i.e. not "/" nor "/login", which would otherwise cause noise or 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, preserving the previous behavior for the ordinary login flow. - Adds unit tests covering: authenticated pass-through, deep-link capture, "/" and "/login" being skipped to avoid loops/noise, and complex URLs with query params and 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 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent a5bafdf commit 06c9ae5

3 files changed

Lines changed: 113 additions & 7 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/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)