This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/texera.git
The following commit(s) were added to refs/heads/main by this push:
new 749798d821 fix(frontend): handle 401 by clearing session, skip sending
expired JWTs (#5392)
749798d821 is described below
commit 749798d8214a5999b2870ebcf3f62fefd609edfa
Author: Yicong Huang <[email protected]>
AuthorDate: Sat Jun 6 00:15:12 2026 -0700
fix(frontend): handle 401 by clearing session, skip sending expired JWTs
(#5392)
### What changes were proposed in this PR?
Adds `UnauthorizedHttpInterceptor` that watches every HTTP response:
when the server returns 401 to a request that carried an `Authorization`
header, it routes the logout through `UserService.logout()` (which
clears the JWT and broadcasts `userChanged(undefined)` so header /
dashboard / in-memory state stay in sync), surfaces a "session expired"
notification, and routes to `/about` with `returnUrl`. 401 responses to
anonymous or auth-endpoint requests pass through unchanged so a 401 on a
public endpoint never wipes a freshly-stored token, and a wrong-password
401 on `/auth/login` cannot kick an already-authenticated user out.
Two adversarial-review findings (caught during self-review, fixed in
commit 2):
1. **Auth-endpoint allowlist.** `JwtModule` blanket-attaches
`Authorization` to every request, including `POST /api/auth/login`. A
wrong-password 401 there carries the user's existing token and would be
misread as "session invalid", kicking an already-authenticated user out
the moment they fat-finger a re-login. Allowlist regex
`\/auth\/(login|register|refresh|google\/login)` short-circuits the
interceptor for these URLs.
2. **Concurrent 401 dedup.** A burst of 8-10 parallel dashboard
requests, all 401'd by a server-side revoke, would fire 8 toasts and 8
`router.navigate` calls. Routing the logout through
`UserService.logout()` (which flips `isLogin()` from true to false)
gives natural dedup: only the first 401 sees `isLogin() === true` and
fires side effects; the rest of the burst short-circuits. The flag
re-arms automatically after a successful re-login.
Flips `JwtModule.forRoot`'s `skipWhenExpired` from `false` to `true`, so
a stale `localStorage` token is no longer auto-appended to public
pre-login endpoints (`/api/config/gui`, `/api/config/user-system`). That
auto-append is the exact mechanism that caused the revert of #4903 in
#5025 — without it, an expired token reaches public endpoints, gets
rejected, and breaks the login page before the user can re-authenticate.
`UserService` is resolved lazily via `Injector` inside `catchError`
because it transitively depends on `HttpClient`, and direct injection
would form a DI cycle through the interceptor chain.
### Any related issues, documentation, discussions?
Closes #5391. Prerequisite for a re-attempt of #4901 (whose first try,
#4903, was reverted by #5025 because of #5026). Discussion:
https://lists.apache.org/thread/w5jmczrffxd5doc5hnss2zm7dbhnbhyy
### How was this PR tested?
Added `unauthorized-http-interceptor.service.spec.ts` with 12 cases
covering: 401 with Authorization → logout + notify + navigate; 401
without Authorization → no-op; non-401 with Authorization → no-op; root
path → omits `returnUrl`; auth-endpoint URLs (login / register / refresh
/ google/login, with and without query strings) → no-op; over-broad URL
match guard (`recent-logins` still logs out); concurrent 401 burst →
exactly one side-effect set; re-arms after re-login.
Tested manually as well.
### Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 4.7)
---
frontend/src/app/app.module.ts | 8 +-
.../unauthorized-http-interceptor.service.spec.ts | 193 +++++++++++++++++++++
.../unauthorized-http-interceptor.service.ts | 87 ++++++++++
3 files changed, 287 insertions(+), 1 deletion(-)
diff --git a/frontend/src/app/app.module.ts b/frontend/src/app/app.module.ts
index 78fc75d7cb..485b3c2e4a 100644
--- a/frontend/src/app/app.module.ts
+++ b/frontend/src/app/app.module.ts
@@ -78,6 +78,7 @@ import { NzCardModule } from "ng-zorro-antd/card";
import { NzTagModule } from "ng-zorro-antd/tag";
import { NzAvatarModule } from "ng-zorro-antd/avatar";
import { BlobErrorHttpInterceptor } from
"./common/service/blob-error-http-interceptor.service";
+import { UnauthorizedHttpInterceptor } from
"./common/service/unauthorized-http-interceptor.service";
import { ConsoleFrameComponent } from
"./workspace/component/result-panel/console-frame/console-frame.component";
import { ResultTableFrameComponent } from
"./workspace/component/result-panel/result-table-frame/result-table-frame.component";
import { RowModalComponent } from
"./workspace/component/result-panel/result-panel-modal.component";
@@ -203,7 +204,7 @@ registerLocaleData(en);
JwtModule.forRoot({
config: {
tokenGetter: AuthService.getAccessToken,
- skipWhenExpired: false,
+ skipWhenExpired: true,
throwNoTokenError: false,
disallowedRoutes: ["forum/api/users"],
},
@@ -375,6 +376,11 @@ registerLocaleData(en);
useClass: BlobErrorHttpInterceptor,
multi: true,
},
+ {
+ provide: HTTP_INTERCEPTORS,
+ useClass: UnauthorizedHttpInterceptor,
+ multi: true,
+ },
{
provide: "SocialAuthServiceConfig",
useFactory: (googleAuthService: GoogleAuthService, userService:
UserService) => {
diff --git
a/frontend/src/app/common/service/unauthorized-http-interceptor.service.spec.ts
b/frontend/src/app/common/service/unauthorized-http-interceptor.service.spec.ts
new file mode 100644
index 0000000000..1b11d9128b
--- /dev/null
+++
b/frontend/src/app/common/service/unauthorized-http-interceptor.service.spec.ts
@@ -0,0 +1,193 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { HTTP_INTERCEPTORS, HttpClient } from "@angular/common/http";
+import { HttpClientTestingModule, HttpTestingController } from
"@angular/common/http/testing";
+import { TestBed } from "@angular/core/testing";
+import { Router } from "@angular/router";
+import { ABOUT } from "../../app-routing.constant";
+import { NotificationService } from "./notification/notification.service";
+import { UserService } from "./user/user.service";
+import { UnauthorizedHttpInterceptor } from
"./unauthorized-http-interceptor.service";
+
+describe("UnauthorizedHttpInterceptor", () => {
+ let http: HttpClient;
+ let httpMock: HttpTestingController;
+ let routerSpy: { navigate: ReturnType<typeof vi.fn>; url: string };
+ let notificationSpy: { error: ReturnType<typeof vi.fn> };
+ let userServiceSpy: { logout: ReturnType<typeof vi.fn>; isLogin:
ReturnType<typeof vi.fn> };
+
+ beforeEach(() => {
+ routerSpy = { navigate: vi.fn(), url: "/user/workflow/42" };
+ notificationSpy = { error: vi.fn() };
+ userServiceSpy = { logout: vi.fn(), isLogin: vi.fn().mockReturnValue(true)
};
+
+ TestBed.configureTestingModule({
+ imports: [HttpClientTestingModule],
+ providers: [
+ { provide: HTTP_INTERCEPTORS, useClass: UnauthorizedHttpInterceptor,
multi: true },
+ { provide: Router, useValue: routerSpy },
+ { provide: NotificationService, useValue: notificationSpy },
+ { provide: UserService, useValue: userServiceSpy },
+ ],
+ });
+
+ http = TestBed.inject(HttpClient);
+ httpMock = TestBed.inject(HttpTestingController);
+ });
+
+ afterEach(() => {
+ httpMock.verify();
+ });
+
+ function authedGet(url: string) {
+ return http.get(url, { headers: { Authorization: "Bearer stale-token" } });
+ }
+
+ it("logs out, notifies, and redirects to ABOUT on 401 for an authenticated
request", () => {
+ // The decision to log out hinges on whether *this* request was
authenticated.
+ // A 401 from an anonymous request is the server saying "you need to log
in",
+ // not "your session is invalid" — clearing the session there would wipe a
+ // freshly-stored token (e.g. mid-login race).
+ authedGet("/api/secret").subscribe({ error: () => {} });
+ httpMock.expectOne("/api/secret").flush(null, { status: 401, statusText:
"Unauthorized" });
+
+ expect(userServiceSpy.logout).toHaveBeenCalledTimes(1);
+ expect(notificationSpy.error).toHaveBeenCalledTimes(1);
+
expect(notificationSpy.error.mock.calls[0][0]).toMatch(/session.*expired|log
in/i);
+ expect(routerSpy.navigate).toHaveBeenCalledWith([ABOUT], {
+ queryParams: { returnUrl: "/user/workflow/42" },
+ });
+ });
+
+ it("leaves the session untouched when 401 comes back for an anonymous
request", () => {
+ // Reproduces the #5026 / #4903-revert scenario: a public endpoint (or one
+ // whose token JwtModule skipped because it was expired) returning 401
+ // must NOT trigger a logout — the user may not even be logged in yet.
+ http.get("/api/public").subscribe({ error: () => {} });
+ httpMock.expectOne("/api/public").flush(null, { status: 401, statusText:
"Unauthorized" });
+
+ expect(userServiceSpy.logout).not.toHaveBeenCalled();
+ expect(notificationSpy.error).not.toHaveBeenCalled();
+ expect(routerSpy.navigate).not.toHaveBeenCalled();
+ });
+
+ it("does not log out on non-401 errors even when Authorization was sent", ()
=> {
+ authedGet("/api/oops").subscribe({ error: () => {} });
+ httpMock.expectOne("/api/oops").flush(null, { status: 500, statusText:
"Server Error" });
+
+ expect(userServiceSpy.logout).not.toHaveBeenCalled();
+ expect(notificationSpy.error).not.toHaveBeenCalled();
+ expect(routerSpy.navigate).not.toHaveBeenCalled();
+ });
+
+ it("omits returnUrl when the current route is the root", () => {
+ routerSpy.url = "/";
+ authedGet("/api/secret").subscribe({ error: () => {} });
+ httpMock.expectOne("/api/secret").flush(null, { status: 401, statusText:
"Unauthorized" });
+
+ expect(routerSpy.navigate).toHaveBeenCalledWith([ABOUT], { queryParams: {
returnUrl: null } });
+ });
+
+ // Adversarial-review fix #1: a stale token gets auto-attached to /auth/login
+ // by JwtModule. A wrong-password 401 from login must NOT be misread as
+ // "your session is invalid", or we'd kick an already-authenticated user
+ // out the moment they fat-finger a re-login.
+ describe("auth-endpoint allowlist", () => {
+ const authPaths = [
+ "/api/auth/login",
+ "/api/auth/register",
+ "/api/auth/refresh",
+ "/api/auth/google/login",
+ "/api/auth/login?next=/dashboard",
+ ];
+
+ authPaths.forEach(path => {
+ it(`does not log out on 401 from ${path}`, () => {
+ authedGet(path).subscribe({ error: () => {} });
+ httpMock.expectOne(path).flush(null, { status: 401, statusText:
"Unauthorized" });
+
+ expect(userServiceSpy.logout).not.toHaveBeenCalled();
+ expect(notificationSpy.error).not.toHaveBeenCalled();
+ expect(routerSpy.navigate).not.toHaveBeenCalled();
+ });
+ });
+
+ it("still logs out on 401 from a URL that merely contains 'login' in a
non-auth segment", () => {
+ // Guards against an over-broad regex like /login/ that would skip e.g.
+ // /api/dashboard/recent-logins.
+ authedGet("/api/dashboard/recent-logins").subscribe({ error: () => {} });
+ httpMock.expectOne("/api/dashboard/recent-logins").flush(null, { status:
401, statusText: "Unauthorized" });
+
+ expect(userServiceSpy.logout).toHaveBeenCalledTimes(1);
+ });
+ });
+
+ // Adversarial-review fix #2: dashboard load fires 8-10 parallel requests; a
+ // server-side revoke makes them all 401 within microseconds of each other.
+ // Side effects must fire exactly once, not once per request — otherwise we
+ // get a toast pile-up and redundant navigations.
+ describe("concurrent 401 deduplication", () => {
+ it("fires logout side effects exactly once for a burst of 401s", () => {
+ // Simulate UserService transitioning from logged-in → logged-out as soon
+ // as logout() is called; this is what UserService.changeUser(undefined)
+ // does in production and is the discriminator the interceptor reads.
+ userServiceSpy.logout.mockImplementation(() => {
+ userServiceSpy.isLogin.mockReturnValue(false);
+ });
+
+ authedGet("/api/a").subscribe({ error: () => {} });
+ authedGet("/api/b").subscribe({ error: () => {} });
+ authedGet("/api/c").subscribe({ error: () => {} });
+
+ httpMock.expectOne("/api/a").flush(null, { status: 401, statusText:
"Unauthorized" });
+ httpMock.expectOne("/api/b").flush(null, { status: 401, statusText:
"Unauthorized" });
+ httpMock.expectOne("/api/c").flush(null, { status: 401, statusText:
"Unauthorized" });
+
+ expect(userServiceSpy.logout).toHaveBeenCalledTimes(1);
+ expect(notificationSpy.error).toHaveBeenCalledTimes(1);
+ expect(routerSpy.navigate).toHaveBeenCalledTimes(1);
+ });
+
+ it("re-arms after a successful re-login so a later 401 triggers logout
again", () => {
+ // After logout, isLogin() is false. Dedup keys off isLogin(), so when
the
+ // user re-logs in (isLogin() flips back to true) and a fresh request
401s,
+ // the interceptor must fire side effects again. Otherwise the user gets
+ // stuck in a half-logged-in state after a transient session error.
+ userServiceSpy.logout.mockImplementationOnce(() => {
+ userServiceSpy.isLogin.mockReturnValue(false);
+ });
+
+ authedGet("/api/a").subscribe({ error: () => {} });
+ httpMock.expectOne("/api/a").flush(null, { status: 401, statusText:
"Unauthorized" });
+ expect(userServiceSpy.logout).toHaveBeenCalledTimes(1);
+
+ // Simulate re-login: isLogin() back to true.
+ userServiceSpy.isLogin.mockReturnValue(true);
+ userServiceSpy.logout.mockImplementationOnce(() => {
+ userServiceSpy.isLogin.mockReturnValue(false);
+ });
+
+ authedGet("/api/b").subscribe({ error: () => {} });
+ httpMock.expectOne("/api/b").flush(null, { status: 401, statusText:
"Unauthorized" });
+
+ expect(userServiceSpy.logout).toHaveBeenCalledTimes(2);
+ });
+ });
+});
diff --git
a/frontend/src/app/common/service/unauthorized-http-interceptor.service.ts
b/frontend/src/app/common/service/unauthorized-http-interceptor.service.ts
new file mode 100644
index 0000000000..2ff0cbc5db
--- /dev/null
+++ b/frontend/src/app/common/service/unauthorized-http-interceptor.service.ts
@@ -0,0 +1,87 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+import { HttpErrorResponse, HttpEvent, HttpHandler, HttpInterceptor,
HttpRequest } from "@angular/common/http";
+import { Injectable, Injector } from "@angular/core";
+import { Router } from "@angular/router";
+import { Observable, throwError } from "rxjs";
+import { catchError } from "rxjs/operators";
+import { ABOUT } from "../../app-routing.constant";
+import { NotificationService } from "./notification/notification.service";
+import { UserService } from "./user/user.service";
+
+// Endpoints whose own 401 means "wrong credentials", not "your session is
invalid".
+// JwtModule blanket-attaches Authorization to every request, so a stale token
+// gets piggy-backed on login/register/refresh attempts even when the user is
+// trying to *acquire* a session. Treating those 401s as session-invalidation
+// would kick out an already-authenticated user who fat-fingers a re-login.
+const AUTH_ENDPOINT_PATTERN =
/\/auth\/(login|register|refresh|google\/login)(?:\?|$)/;
+
+/**
+ * Globally handles 401 responses that come back for *authenticated* requests:
+ * routes the logout through UserService (which clears the token and broadcasts
+ * `userChanged(undefined)` so header / dashboard / in-memory state stay in
+ * sync), surfaces a notification, and routes to the landing page with
+ * returnUrl. 401s on anonymous, auth-endpoint, or already-logged-out paths
+ * pass through unchanged. See #5391 / #4901 / #5026.
+ *
+ * UserService is resolved lazily via Injector because it (transitively)
+ * depends on HttpClient, and HttpClient depends on the interceptor chain —
+ * direct injection here would form a DI cycle.
+ */
+@Injectable()
+export class UnauthorizedHttpInterceptor implements HttpInterceptor {
+ constructor(
+ private injector: Injector,
+ private router: Router,
+ private notificationService: NotificationService
+ ) {}
+
+ public intercept(req: HttpRequest<unknown>, next: HttpHandler):
Observable<HttpEvent<unknown>> {
+ return next.handle(req).pipe(
+ catchError((err: unknown) => {
+ if (this.shouldHandleAsSessionExpired(req, err)) {
+ const userService = this.injector.get(UserService);
+ // Dedup: if a burst of 401s arrives, only the first one through here
+ // sees isLogin()=true. logout() flips it to false (via changeUser),
+ // so the rest of the burst skip the side effects. After a real
+ // re-login isLogin() flips back to true and the interceptor re-arms.
+ if (userService.isLogin()) {
+ userService.logout();
+ this.notificationService.error("Your session has expired. Please
log in again.");
+ const currentUrl = this.router.url;
+ this.router.navigate([ABOUT], {
+ queryParams: { returnUrl: currentUrl === "/" ? null : currentUrl
},
+ });
+ }
+ }
+ return throwError(() => err);
+ })
+ );
+ }
+
+ private shouldHandleAsSessionExpired(req: HttpRequest<unknown>, err:
unknown): boolean {
+ return (
+ err instanceof HttpErrorResponse &&
+ err.status === 401 &&
+ req.headers.has("Authorization") &&
+ !AUTH_ENDPOINT_PATTERN.test(req.url)
+ );
+ }
+}
