This is an automated email from the ASF dual-hosted git repository.
voidmatcha pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zeppelin.git
The following commit(s) were added to refs/heads/master by this push:
new b8b24593be [ZEPPELIN-6765] Fix returnUrl loss during initial login
navigation
b8b24593be is described below
commit b8b24593be736d3ffcb18a6db60c40d2750c2e83
Author: Minho Jang <[email protected]>
AuthorDate: Sun Nov 1 10:43:47 2026 +0900
[ZEPPELIN-6765] Fix returnUrl loss during initial login navigation
### What is this PR for?
Unauthenticated ticket checks can reach the HTTP 405 handler during initial
navigation. That handler starts logout and redirects to `/login`, discarding
`returnUrl` before the user submits credentials.
This PR marks initial ticket checks so their 405 errors reach the route
guards without triggering logout. Successful login can then return to the
requested destination. Established-session failure handling and SSO redirects
remain unchanged.
### What type of PR is it?
Bug Fix
### What is the Jira issue?
* https://issues.apache.org/jira/browse/ZEPPELIN-6765
### How should this be tested?
With frontend dependencies installed and a Shiro-enabled Zeppelin backend
running at `http://localhost:8080`, run from `zeppelin-web-angular`. The local
`conf/shiro.ini` should match the server configuration and include an admin
account for the credential-page scenarios.
```bash
npm run test:shell -- src/app/app-http.interceptor.spec.ts
npm run e2e:fast -- tests/login/login.spec.ts --workers=1
```
The regression tests check that direct login entry and unauthenticated
access to a protected route preserve `returnUrl` through successful login,
including the destination query parameters and authenticated browser session.
### Questions:
* Does the license files need to update? No
* Is there breaking changes for older versions? No
* Does this needs documentation? No, this restores the existing login
return-destination behavior
Closes #5600 from miinhho/fix/login-returnurl-initial-navigation.
Signed-off-by: YONGJAE LEE <[email protected]>
---
zeppelin-web-angular/e2e/tests/login/login.spec.ts | 39 ++++++++++
.../src/app/app-http.interceptor.spec.ts | 86 +++++++++++++++++++++-
.../src/app/app-http.interceptor.ts | 6 +-
.../src/app/services/ticket.service.ts | 9 ++-
4 files changed, 135 insertions(+), 5 deletions(-)
diff --git a/zeppelin-web-angular/e2e/tests/login/login.spec.ts
b/zeppelin-web-angular/e2e/tests/login/login.spec.ts
index e7d07c649e..d76faf4496 100644
--- a/zeppelin-web-angular/e2e/tests/login/login.spec.ts
+++ b/zeppelin-web-angular/e2e/tests/login/login.spec.ts
@@ -98,6 +98,45 @@ test.describe('Login Page', () => {
await expect(loginPage.formContainer).not.toBeVisible();
});
+ for (const entry of ['login', 'protected route']) {
+ test(`should preserve returnUrl through a cold ${entry} entry and
successful authentication`, async ({ page }) => {
+ const destination = '/credential?probe=returnurl';
+ const credentials = Object.values(testCredentials).find(account =>
account.roles?.includes('admin'));
+ if (!credentials) {
+ throw new Error('The returnUrl regression requires an admin account
for the credential destination');
+ }
+ const entryUrl = entry === 'login' ?
`/#/login?returnUrl=${encodeURIComponent(destination)}` : `/#${destination}`;
+
+ // Force a new document, rather than updating query params on an already
mounted login component.
+ await page.goto('about:blank');
+ await page.goto(entryUrl);
+ await expect(loginPage.userNameInput).toBeVisible();
+ await expect(page).toHaveURL(url => {
+ const [path, query] = url.hash.split('?');
+ return path === '#/login' && new
URLSearchParams(query).get('returnUrl') === destination;
+ });
+
+ const responsePromise = page.waitForResponse(
+ response => response.request().method() === 'POST' && new
URL(response.url()).pathname === '/api/login'
+ );
+ await loginPage.login(credentials.username, credentials.password);
+ const response = await responsePromise;
+ expect(response.status()).toBe(200);
+ expect((await
response.json()).body.principal).toBe(credentials.username);
+ await expect(page).toHaveURL(url => url.hash === `#${destination}`);
+ await expect(
+ page.getByText('Manage your credentials. You can add new credential
information.', { exact: true })
+ ).toBeVisible();
+ await expect(loginPage.userNameInput).toBeHidden();
+
+ const cookies = await page.context().cookies(page.url());
+ expect(cookies.some(cookie => cookie.name === 'JSESSIONID' &&
cookie.value !== '' && cookie.httpOnly)).toBe(true);
+ const ticket = await page.context().request.get('/api/security/ticket',
{ maxRedirects: 0 });
+ expect(ticket.status()).toBe(200);
+ expect((await ticket.json()).body.principal).toBe(credentials.username);
+ });
+ }
+
test('should maintain form state after failed login', async () => {
const invalidCreds = testCredentials.INVALID_USER;
await loginPage.login(invalidCreds.username, invalidCreds.password);
diff --git a/zeppelin-web-angular/src/app/app-http.interceptor.spec.ts
b/zeppelin-web-angular/src/app/app-http.interceptor.spec.ts
index 53bcd93f04..b42dce345e 100644
--- a/zeppelin-web-angular/src/app/app-http.interceptor.spec.ts
+++ b/zeppelin-web-angular/src/app/app-http.interceptor.spec.ts
@@ -10,7 +10,15 @@
* limitations under the License.
*/
-import { HttpClient, HttpErrorResponse, HttpEvent, HttpHandler, HttpHeaders,
HttpRequest } from '@angular/common/http';
+import {
+ HttpClient,
+ HttpErrorResponse,
+ HttpEvent,
+ HttpHandler,
+ HttpHeaders,
+ HttpRequest,
+ HttpResponse
+} from '@angular/common/http';
import { Router } from '@angular/router';
import { defer, firstValueFrom, of, Subject, throwError } from 'rxjs';
@@ -254,3 +262,79 @@ describe('AppHttpInterceptor', () => {
});
});
});
+
+describe('Ticket authentication checks through AppHttpInterceptor', () => {
+ let service: TicketService;
+ let failure: HttpErrorResponse;
+ let requestedUrls: string[];
+ let navigate: ReturnType<typeof vi.fn>;
+ let redirects: string[];
+
+ beforeEach(() => {
+ failure = sessionExpired(`${REST_BASE}/login`);
+ requestedUrls = [];
+ redirects = [];
+ navigate = vi.fn(() => Promise.resolve(true));
+ vi.spyOn(window, 'location', 'get').mockReturnValue({
+ set href(value: string) {
+ redirects.push(value);
+ }
+ } as Location);
+ let interceptor: AppHttpInterceptor;
+ const backend: HttpHandler = {
+ handle: request => {
+ requestedUrls.push(request.url);
+ if (request.url === `${REST_BASE}/version`) {
+ return of(new HttpResponse({ body: { body: { version: 'test' } } }));
+ }
+ return throwError(() => failure);
+ }
+ };
+ const client = new HttpClient({ handle: request =>
interceptor.intercept(request, backend) });
+ service = new TicketService(
+ client,
+ createBaseUrl(),
+ { navigate } as unknown as Router,
+ { success: vi.fn() } as unknown as NzMessageService
+ );
+ interceptor = new AppHttpInterceptor(service);
+ });
+
+ afterEach(() => vi.restoreAllMocks());
+
+ it('leaves an initial unauthenticated check to its guard without logging
out', async () => {
+ await expect(firstValueFrom(service.getTicket())).rejects.toBe(failure);
+
+ expect(requestedUrls).not.toContain(`${REST_BASE}/login/logout`);
+ expect(navigate).not.toHaveBeenCalled();
+ expect(service.ticket.init).toBe(false);
+ expect(service.logout$.value).toBe(false);
+ });
+
+ it('still clears an initialized session when its ticket check fails with
405', async () => {
+ service.ticket.init = true;
+ service.ticket.principal = 'user1';
+
+ await expect(firstValueFrom(service.getTicket())).rejects.toBe(failure);
+
+ expect(requestedUrls.filter(url => url ===
`${REST_BASE}/login/logout`)).toHaveLength(1);
+ expect(navigate).toHaveBeenCalledExactlyOnceWith(['/login']);
+ expect(service.ticket.init).toBe(false);
+ expect(service.ticket.principal).toBe('');
+ expect(service.logout$.value).toBe(false);
+ });
+
+ it('still follows an SSO 401 Location during an initial authentication
check', async () => {
+ failure = new HttpErrorResponse({
+ status: 401,
+ url: `${REST_BASE}/security/ticket`,
+ headers: new HttpHeaders({ Location: '/sso/login' })
+ });
+
+ await
expect(firstValueFrom(service.isAuthenticated())).rejects.toBe(failure);
+
+ expect(redirects).toEqual(['/sso/login']);
+ expect(requestedUrls).not.toContain(`${REST_BASE}/login/logout`);
+ expect(navigate).not.toHaveBeenCalled();
+ });
+});
diff --git a/zeppelin-web-angular/src/app/app-http.interceptor.ts
b/zeppelin-web-angular/src/app/app-http.interceptor.ts
index 2b94e9a4b4..051d61536e 100644
--- a/zeppelin-web-angular/src/app/app-http.interceptor.ts
+++ b/zeppelin-web-angular/src/app/app-http.interceptor.ts
@@ -18,7 +18,7 @@ import { catchError, map } from 'rxjs/operators';
import { isNil } from 'lodash';
import { environment } from '@zeppelin/environment';
-import { TicketService } from '@zeppelin/services';
+import { INITIAL_AUTHENTICATION_CHECK, TicketService } from
'@zeppelin/services';
@Injectable()
export class AppHttpInterceptor implements HttpInterceptor {
@@ -44,8 +44,10 @@ export class AppHttpInterceptor implements HttpInterceptor {
}
}),
catchError(event => {
+ // Let guards handle a missing initial session without a competing
logout navigation.
+ const initialCheck = event.status === 405 &&
httpRequest.context.get(INITIAL_AUTHENTICATION_CHECK);
// A 405 from the logout request itself must not start another logout.
- if (!(event.status === 405 && httpRequest.url.includes('logout'))) {
+ if (!initialCheck && !(event.status === 405 &&
httpRequest.url.includes('logout'))) {
this.ticketService.handleAuthFailure(event.status,
event.headers.get('Location'));
}
return throwError(event);
diff --git a/zeppelin-web-angular/src/app/services/ticket.service.ts
b/zeppelin-web-angular/src/app/services/ticket.service.ts
index 505a93be0b..b345c16023 100644
--- a/zeppelin-web-angular/src/app/services/ticket.service.ts
+++ b/zeppelin-web-angular/src/app/services/ticket.service.ts
@@ -10,7 +10,7 @@
* limitations under the License.
*/
-import { HttpClient } from '@angular/common/http';
+import { HttpClient, HttpContext, HttpContextToken } from
'@angular/common/http';
import { Injectable } from '@angular/core';
import { Router } from '@angular/router';
import { forkJoin, BehaviorSubject, EMPTY, Subject } from 'rxjs';
@@ -22,6 +22,9 @@ import { ITicket, ITicketWrapped, IZeppelinVersion } from
'@zeppelin/interfaces'
import { BaseUrlService } from './base-url.service';
+// Initial authentication failures belong to route guards, not
established-session logout handling.
+export const INITIAL_AUTHENTICATION_CHECK = new HttpContextToken<boolean>(()
=> false);
+
@Injectable({
providedIn: 'root'
})
@@ -35,7 +38,9 @@ export class TicketService {
getTicket() {
return forkJoin([
-
this.httpClient.get<ITicket>(`${this.baseUrlService.getRestApiBase()}/security/ticket`),
+
this.httpClient.get<ITicket>(`${this.baseUrlService.getRestApiBase()}/security/ticket`,
{
+ context: new HttpContext().set(INITIAL_AUTHENTICATION_CHECK,
!this.ticket.init)
+ }),
this.getZeppelinVersion()
]).pipe(
tap(data => {
