Skip to content

Commit e08d6b1

Browse files
authored
feat: pass the last visited route on OIDC login (#1753)
## Description This PR adds a service to track the last visited route, and passes it to /oidc backend endpoint as a query parameter, so that after successful login (i.e. when browser calls /oidc/callback), the user is redirected to the page he started from. Related backend PR: SciCatProject/scicat-backend-next#1714 ## Motivation Previously, after OIDC login, the user would always land on "/" (or "/datasets" or some hardcoded route configured in backend). A better user experience is to return the user to the page she was on (e.g. if the user clicks login from /datasets/pid123/datafiles, she should come back to this page after logging in). This PR utilizes the backend support added in SciCatProject/scicat-backend-next#1714 to enable this behavior. ## Fixes: * None ## Changes: - Add RotueTrackerService, to keep track of last navigated route, and start it on Application startup. - Modify redirectOIDC in login component to pass returnUrl (retrieved from above service) to the backend as query param - Don't pass redirectUrl in AuthGuard to /login component, as this is already handled by RouteTrackerService ## Tests included - [x] Included for each change/fix? - [x] Passing? (Merge will not be approved unless this is checked) ## Documentation - [ ] swagger documentation updated \[required\] - [ ] official documentation updated \[nice-to-have\] ### official documentation info If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included ## Backend version - [x] Does it require a specific version of the backend - which version of the backend is required: v4 ## Summary by Sourcery This pull request enhances the user experience by redirecting users to their last visited page after OIDC login. It introduces a RouteTrackerService to keep track of the last route and passes it to the backend as a query parameter during the OIDC authentication process. It also adds basic exception handling in /auth-callback to ensure returnUrl is a valid path. New Features: - Implements a RouteTrackerService to track the last visited route before OIDC login. - Passes the last visited route to the backend's /oidc endpoint as a query parameter. Enhancements: - After successful OIDC login, users are now redirected to their last visited page instead of a hardcoded route. Tests: - Added a unit test for the RouteTrackerService to verify that it saves the last route navigated from.
1 parent 2eaf4d5 commit e08d6b1

File tree

8 files changed

+93
-7
lines changed

8 files changed

+93
-7
lines changed

cypress/e2e/datasets/datasets-general.cy.js

+4
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ describe("Datasets general", () => {
3838

3939
cy.finishedLoading();
4040

41+
cy.reload();
42+
// Without reloading, the user will land on last visited page before logout
43+
// i.e. the dataset detail page, because the login page "remembers" the previousRoute.
44+
4145
cy.url().should("include", "/login");
4246

4347
cy.get('mat-tab-group [role="tab"]').contains("Local").click();

src/app/_layout/app-header/app-header.component.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,9 @@ export class AppHeaderComponent implements OnInit {
4646

4747
login(): void {
4848
if (this.config.skipSciCatLoginPageEnabled) {
49+
const returnURL = encodeURIComponent(this.router.url);
4950
for (const endpoint of this.oAuth2Endpoints) {
50-
this.document.location.href = `${this.config.lbBaseURL}/${endpoint.authURL}`;
51+
this.document.location.href = `${this.config.lbBaseURL}/${endpoint.authURL}?returnURL=${returnURL}`;
5152
}
5253
} else {
5354
this.router.navigateByUrl("/login");

src/app/app-routing/auth.guard.ts

+1-3
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,7 @@ export class AuthGuard implements CanActivate {
3434
.usersControllerGetMyUser()
3535
.toPromise()
3636
.catch(() => {
37-
this.router.navigate(["/login"], {
38-
queryParams: { returnUrl: state.url },
39-
});
37+
this.router.navigate(["/login"]);
4038
return false;
4139
})
4240
.then(() => true);

src/app/app.module.ts

+7
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ import { CookieService } from "ngx-cookie-service";
3131
import { TranslateLoader, TranslateModule } from "@ngx-translate/core";
3232
import { CustomTranslateLoader } from "shared/loaders/custom-translate.loader";
3333
import { DATE_PIPE_DEFAULT_OPTIONS } from "@angular/common";
34+
import { RouteTrackerService } from "shared/services/route-tracker.service";
3435

3536
const appConfigInitializerFn = (appConfig: AppConfigService) => {
3637
return () => appConfig.loadAppConfig();
@@ -103,6 +104,12 @@ const apiConfigurationFn = (
103104
multi: true,
104105
deps: [AppThemeService],
105106
},
107+
{
108+
provide: APP_INITIALIZER,
109+
useFactory: () => () => {},
110+
multi: true,
111+
deps: [RouteTrackerService],
112+
},
106113
{
107114
provide: HTTP_INTERCEPTORS,
108115
useClass: SnackbarInterceptor,
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,46 @@
1+
import { TestBed } from "@angular/core/testing";
2+
3+
import { RouteTrackerService } from "./route-tracker.service";
4+
import { provideRouter, Router } from "@angular/router";
5+
import { Component } from "@angular/core";
6+
7+
@Component({})
8+
class DummyComponent {}
9+
10+
describe("RouteTrackerService", () => {
11+
let service: RouteTrackerService;
12+
let router: Router;
13+
14+
beforeEach(() => {
15+
TestBed.configureTestingModule({
16+
providers: [
17+
provideRouter([
18+
{ path: "datasets/:pid", component: DummyComponent },
19+
{ path: "login", component: DummyComponent },
20+
]),
21+
],
22+
});
23+
service = TestBed.inject(RouteTrackerService);
24+
router = TestBed.inject(Router);
25+
});
26+
27+
it("should be created", () => {
28+
expect(service).toBeTruthy();
29+
});
30+
31+
it("should save the last route navigated from", async () => {
32+
expect(router).toBeTruthy();
33+
await router.navigateByUrl("/datasets/123");
34+
await router.navigateByUrl("/login");
35+
const previousRoute = service.getPreviousRoute();
36+
expect(previousRoute).toBe("/datasets/123");
37+
});
38+
39+
it("should preserve query parameters in the previous route", async () => {
40+
expect(router).toBeTruthy();
41+
await router.navigateByUrl("/datasets/123?query=value");
42+
await router.navigateByUrl("/login");
43+
const previousRoute = service.getPreviousRoute();
44+
expect(previousRoute).toBe("/datasets/123?query=value");
45+
});
46+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
import { Injectable } from "@angular/core";
2+
import { Router, NavigationStart } from "@angular/router";
3+
import { filter } from "rxjs";
4+
5+
@Injectable({
6+
providedIn: "root",
7+
})
8+
export class RouteTrackerService {
9+
private currentRoute: string | null = null;
10+
private previousRoute: string | null = null;
11+
12+
constructor(private router: Router) {
13+
this.router.events
14+
.pipe(filter((event) => event instanceof NavigationStart))
15+
.subscribe((event) => {
16+
this.previousRoute = this.currentRoute;
17+
this.currentRoute = (event as NavigationStart).url;
18+
});
19+
}
20+
21+
getPreviousRoute(): string | null {
22+
return this.previousRoute;
23+
}
24+
}

src/app/users/auth-callback/auth-callback.component.ts

+2-1
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,8 @@ export class AuthCallbackComponent implements OnInit {
6363
);
6464

6565
// After the user is authenticated, we will redirect to the home page
66-
const returnUrl = params["returnUrl"];
66+
// or the value of returnUrl query param
67+
const returnUrl: string = params["returnUrl"];
6768
this.router.navigateByUrl(returnUrl || "/");
6869
}
6970
});

src/app/users/login/login.component.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
AppConfigService,
2121
OAuth2Endpoint,
2222
} from "app-config.service";
23+
import { RouteTrackerService } from "shared/services/route-tracker.service";
2324

2425
interface LoginForm {
2526
username: string;
@@ -74,13 +75,17 @@ export class LoginComponent implements OnInit, OnDestroy {
7475
private router: Router,
7576
private route: ActivatedRoute,
7677
private store: Store,
78+
private routeTrackerService: RouteTrackerService,
7779
@Inject(DOCUMENT) public document: Document,
7880
) {
79-
this.returnUrl = this.route.snapshot.queryParams["returnUrl"] || "";
81+
this.returnUrl = this.routeTrackerService.getPreviousRoute() || "";
8082
}
8183

8284
redirectOIDC(authURL: string) {
83-
this.document.location.href = `${this.appConfig.lbBaseURL}/${authURL}`;
85+
const returnURL = this.returnUrl
86+
? encodeURIComponent(this.returnUrl)
87+
: "/datasets";
88+
this.document.location.href = `${this.appConfig.lbBaseURL}/${authURL}?returnURL=${returnURL}`;
8489
}
8590

8691
openPrivacyDialog() {

0 commit comments

Comments
 (0)