Skip to content

feat: session expired interceptor with login redirection#2379

Open
abdellah257 wants to merge 1 commit into
SciCatProject:masterfrom
abdellah257:session-expired-redirect
Open

feat: session expired interceptor with login redirection#2379
abdellah257 wants to merge 1 commit into
SciCatProject:masterfrom
abdellah257:session-expired-redirect

Conversation

@abdellah257
Copy link
Copy Markdown
Contributor

@abdellah257 abdellah257 commented May 18, 2026

Description

Redirect to login page at session expiration if an authentication with an expired token is attempted. then redirect back in case of successful login.

Motivation

  • Informs the user of session expiration by redirecting to login page, avoiding silent 403 failures returning empty results (e.i: empty datafiles, no datasets at filtering, ...)

Changes:

  • new interceptor: auth.interceptor.ts
  • new sessionTimeout user action
  • update the returnUrl for login in case of session timeout

Backend changes:

  • Requires 401 HTTP status errors with SESSION_EXPIRED message to be thrown from the backend yo perform the redirection.
  • See Backend PR 2737

Tests included

  • Included for each change/fix?
  • 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

Summary by Sourcery

Handle expired authentication sessions by redirecting users to the login page and back to their original route after successful re-authentication.

New Features:

  • Add an HTTP auth interceptor that detects SESSION_EXPIRED 401 responses and triggers a session-timeout flow with post-login redirection.
  • Introduce a session timeout user action and effect to clear user-related state and complete logout with a redirect to the login page.
  • Restore a stored post-login redirect URL in the login component so users return to the page they were on before session expiration.

Enhancements:

  • Register the new auth interceptor in the application module HTTP interceptor chain.

@abdellah257 abdellah257 requested a review from a team as a code owner May 18, 2026 11:40
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've found 3 issues, and left some high level feedback:

  • In the sessionTimeout$ effect, the various clear*StateAction() calls are just being invoked and their return values ignored; instead, return or dispatch these actions from the effect so the relevant slices actually get cleared (e.g., by returning a concat/merge of multiple actions or using map to an array).
  • The AuthInterceptor currently imports Subscription, selectProfile, and injects NgZone without using them; removing these unused imports/dependencies will keep the interceptor simpler and avoid confusion.
  • There are several console.log statements in the login component and AuthInterceptor; consider removing them or switching to the existing logging mechanism to avoid noisy console output in production.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- In the `sessionTimeout$` effect, the various `clear*StateAction()` calls are just being invoked and their return values ignored; instead, return or dispatch these actions from the effect so the relevant slices actually get cleared (e.g., by returning a `concat`/`merge` of multiple actions or using `map` to an array).
- The `AuthInterceptor` currently imports `Subscription`, `selectProfile`, and injects `NgZone` without using them; removing these unused imports/dependencies will keep the interceptor simpler and avoid confusion.
- There are several `console.log` statements in the login component and `AuthInterceptor`; consider removing them or switching to the existing logging mechanism to avoid noisy console output in production.

## Individual Comments

### Comment 1
<location path="src/app/state-management/effects/user.effects.ts" line_range="253-262" />
<code_context>
+    return this.actions$.pipe(
+      ofType(fromActions.sessionTimeoutAction),
+      filter(() => this.authService.isAuthenticated()),
+      switchMap(() => {
+        this.authService.clear();
+        clearDatasetsStateAction();
+        clearInstrumentsStateAction();
+        clearJobsStateAction();
+        clearLogbooksStateAction();
+        clearPoliciesStateAction();
+        clearProposalsStateAction();
+        clearPublishedDataStateAction();
+        clearSamplesStateAction();
+        return of(fromActions.logoutCompleteAction({ logoutURL: "/login" }));
+      }),
+    );
</code_context>
<issue_to_address>
**issue (bug_risk):** The clear*StateAction calls are never dispatched, so the feature states will not actually be cleared on session timeout.

Within an effect, simply calling these action creators has no effect—they must be returned from the stream or dispatched. Currently only `logoutCompleteAction` is emitted, so the other slices are never reset on timeout. Please update the effect to emit all the clear*StateAction actions (e.g. `concatMap`/`mergeMap` to `of(clearDatasetsStateAction(), ...)`) or dispatch them via the store so the state is actually cleared.
</issue_to_address>

### Comment 2
<location path="src/app/users/login/login.component.ts" line_range="134-135" />
<code_context>
+      sessionStorage.removeItem("postLoginRedirect");
+    }
+
+    console.log("[LOGIN] return url: ", this.returnUrl);
+
     this.proceedSubscription = this.vm$
</code_context>
<issue_to_address>
**suggestion:** Consider removing or guarding this console.log to avoid noisy production logs.

Since this component runs on every login, logging here will generate a lot of noise in production. If you still need this during development, consider removing it before release or wrapping it in an environment check or configurable logger.

```suggestion
```
</issue_to_address>

### Comment 3
<location path="src/app/shared/interceptors/auth.interceptor.ts" line_range="31-44" />
<code_context>
+        const isSessionExpired =
+          error.status === 401 && error.error?.message === "SESSION_EXPIRED";
+
+        console.log("isSessionExpired", isSessionExpired);
+
+        if (isSessionExpired) {
+          const currentUrl = this.router.url;
+          sessionStorage.setItem("postLoginRedirect", currentUrl);
+
+          this.store.dispatch(sessionTimeoutAction());
+
+          console.log("currentUrl", currentUrl);
+        }
+
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Debugging console logs in the interceptor may be too noisy and potentially leak sensitive routing information.

Since this interceptor runs on every failed HTTP request, these `console.log` statements can both flood the console and expose internal URLs. Please remove them or replace them with structured logging that’s appropriately filtered/disabled in production.

```suggestion
      catchError((error: HttpErrorResponse) => {
        const isSessionExpired =
          error.status === 401 && error.error?.message === "SESSION_EXPIRED";

        if (isSessionExpired) {
          const currentUrl = this.router.url;
          sessionStorage.setItem("postLoginRedirect", currentUrl);

          this.store.dispatch(sessionTimeoutAction());
        }
```
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

Comment thread src/app/state-management/effects/user.effects.ts Outdated
Comment thread src/app/users/login/login.component.ts Outdated
Comment thread src/app/shared/interceptors/auth.interceptor.ts
@abdellah257 abdellah257 force-pushed the session-expired-redirect branch from 8356562 to c9ff0a3 Compare May 18, 2026 11:46
@abdellah257 abdellah257 force-pushed the session-expired-redirect branch from c9ff0a3 to aecf848 Compare May 18, 2026 12:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant