From 6935f69f7b27a1fe57fad39b0a77d5d6b34902f5 Mon Sep 17 00:00:00 2001 From: kimyenac Date: Mon, 7 Sep 2026 18:56:46 +0900 Subject: [PATCH] [ZEPPELIN-6643] Log out on session expiry instead of throwing in the interceptor The 405 branch in AppHttpInterceptor guarded logout with event.url.contains('logout'). JavaScript strings have no contains method, so the guard threw a TypeError inside catchError before ticketService.logout was ever reached. The New UI therefore never logged out on session expiry: the caller observed a TypeError instead of the 405, and the expired session stayed in place until the user reloaded the page by hand. The check becomes event.url?.includes('logout'). includes is the method that exists, and the optional chain covers HttpErrorResponse.url being null, which it is whenever the failure carries no resolved url. A 405 with no url cannot be identified as the logout call, so it falls through to logout, which is the same conclusion the branch already draws for every other request. The 401 redirect branch is untouched. Tightening the substring match and the wider typing of this interceptor belong to ZEPPELIN-6469, which waits on this behaviour being correct first. The spec constructs the interceptor directly with a logout stub rather than starting TestBed, since no Angular wiring is involved. It pins the three things the branch has to get right: a non-logout 405 calls logout exactly once, that 405 is rethrown to the caller unchanged rather than replaced by a TypeError, and a 405 from the logout request itself does not call logout again. Reverting the source line fails three of the four. --- .../src/app/app-http.interceptor.spec.ts | 80 +++++++++++++++++++ .../src/app/app-http.interceptor.ts | 2 +- 2 files changed, 81 insertions(+), 1 deletion(-) create mode 100644 zeppelin-web-angular/src/app/app-http.interceptor.spec.ts diff --git a/zeppelin-web-angular/src/app/app-http.interceptor.spec.ts b/zeppelin-web-angular/src/app/app-http.interceptor.spec.ts new file mode 100644 index 00000000000..8e46833f64a --- /dev/null +++ b/zeppelin-web-angular/src/app/app-http.interceptor.spec.ts @@ -0,0 +1,80 @@ +/* + * Licensed 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, HttpHandler, HttpRequest } from '@angular/common/http'; +import { of, throwError } from 'rxjs'; + +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { TicketService } from '@zeppelin/services'; + +import { AppHttpInterceptor } from './app-http.interceptor'; + +const REST_BASE = 'http://localhost:8080/api'; + +/** + * The server answers an expired session with 405 on the REST base rather than 401, so the 405 + * branch is the only path that reaches logout. The response carries no Location header, which + * is what keeps the 401 branch out of the way. + */ +function sessionExpired(url: string | undefined): HttpErrorResponse { + return new HttpErrorResponse({ status: 405, url }); +} + +describe('AppHttpInterceptor', () => { + let logout: ReturnType; + let interceptor: AppHttpInterceptor; + + /** Drives one request through the interceptor and returns whatever the caller would observe. */ + function intercept(failure: HttpErrorResponse, url = REST_BASE): Promise { + const next: HttpHandler = { handle: () => throwError(() => failure) }; + return new Promise(resolve => { + interceptor.intercept(new HttpRequest('GET', url), next).subscribe({ + next: resolve, + error: resolve + }); + }); + } + + beforeEach(() => { + logout = vi.fn(() => of({})); + interceptor = new AppHttpInterceptor({ logout } as unknown as TicketService); + }); + + it('logs out once when a non-logout request is answered with 405', async () => { + await intercept(sessionExpired(`${REST_BASE}/notebook`), `${REST_BASE}/notebook`); + + // `String.prototype.contains` does not exist, so this branch used to throw before reaching logout + expect(logout).toHaveBeenCalledTimes(1); + }); + + it('rethrows the 405 it logged out on instead of a TypeError', async () => { + const failure = sessionExpired(`${REST_BASE}/notebook`); + + const observed = await intercept(failure, `${REST_BASE}/notebook`); + + expect(observed).toBe(failure); + }); + + it('does not log out again when the logout request itself is answered with 405', async () => { + await intercept(sessionExpired(`${REST_BASE}/login/logout`), `${REST_BASE}/login/logout`); + + expect(logout).not.toHaveBeenCalled(); + }); + + it('logs out on a 405 that reports no url', async () => { + // an XHR that never resolved a url cannot be identified as the logout call, so the session is gone + await intercept(sessionExpired(undefined)); + + expect(logout).toHaveBeenCalledTimes(1); + }); +}); diff --git a/zeppelin-web-angular/src/app/app-http.interceptor.ts b/zeppelin-web-angular/src/app/app-http.interceptor.ts index 6a6a4a18533..8b46262b308 100644 --- a/zeppelin-web-angular/src/app/app-http.interceptor.ts +++ b/zeppelin-web-angular/src/app/app-http.interceptor.ts @@ -48,7 +48,7 @@ export class AppHttpInterceptor implements HttpInterceptor { if (event.status === 401 && !isNil(redirect)) { // Handle page redirect window.location.href = redirect; - } else if (event.status === 405 && !event.url.contains('logout')) { + } else if (event.status === 405 && !event.url?.includes('logout')) { this.ticketService.logout().subscribe(); } return throwError(event);