Skip to content

Commit 30a36b4

Browse files
UI: Preserve proxied URL on login redirect (#66690) (#67091)
When the UI is reached through a proxy (Gitpod, Codespaces, ngrok, reverse proxies), the auth-failure interceptor sent the API server's absolute URL as the 'next' parameter, so post-login redirects went to e.g. http://localhost:29091 instead of the URL the browser is on. Send a same-origin path+search+hash instead, so the browser stays on whatever origin it is currently using. Add regression coverage for proxied subpaths so redirects also preserve base paths such as /team-a/. closes: #46533 (cherry picked from commit 25ef835) Co-authored-by: Sai Teja Desu <31831475+desusaiteja@users.noreply.github.com>
1 parent 2f77c47 commit 30a36b4

3 files changed

Lines changed: 60 additions & 3 deletions

File tree

airflow-core/src/airflow/ui/src/main.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ import { ChakraCustomProvider } from "src/context/ChakraCustomProvider";
3434
import { ColorModeProvider } from "src/context/colorMode";
3535
import { TimezoneProvider } from "src/context/timezone";
3636
import { router } from "src/router";
37-
import { getRedirectPath } from "src/utils/links.ts";
37+
import { getNextHref, getRedirectPath } from "src/utils/links.ts";
3838

3939
import i18n from "./i18n/config";
4040
import { client } from "./queryClient";
@@ -75,7 +75,7 @@ axios.interceptors.response.use(
7575
) {
7676
const params = new URLSearchParams();
7777

78-
params.set("next", globalThis.location.href);
78+
params.set("next", getNextHref(globalThis.location));
7979
const loginPath = getRedirectPath("api/v2/auth/login");
8080

8181
globalThis.location.replace(`${loginPath}?${params.toString()}`);

airflow-core/src/airflow/ui/src/utils/links.test.ts

Lines changed: 51 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
/* eslint-disable max-lines */
2+
13
/*!
24
* Licensed to the Apache Software Foundation (ASF) under one
35
* or more contributor license agreements. See the NOTICE file
@@ -20,7 +22,12 @@ import { describe, it, expect } from "vitest";
2022

2123
import type { TaskInstanceResponse } from "openapi/requests/types.gen";
2224

23-
import { buildTaskInstanceUrl, getTaskInstanceAdditionalPath, getTaskInstanceLink } from "./links";
25+
import {
26+
buildTaskInstanceUrl,
27+
getNextHref,
28+
getTaskInstanceAdditionalPath,
29+
getTaskInstanceLink,
30+
} from "./links";
2431

2532
describe("getTaskInstanceLink", () => {
2633
const testCases = [
@@ -284,3 +291,46 @@ describe("buildTaskInstanceUrl", () => {
284291
).toBe("/dags/new_dag/runs/new_run/tasks/new_task/mapped");
285292
});
286293
});
294+
295+
describe("getNextHref", () => {
296+
// Regression tests for https://github.com/apache/airflow/issues/46533 — the
297+
// "next" parameter sent to the login redirect must be a same-origin relative
298+
// URL so that proxied deployments (e.g. Gitpod) don't bounce the browser
299+
// back to the API server's reported origin (e.g. http://localhost:29091).
300+
it.each([
301+
{
302+
description: "preserves pathname only",
303+
expected: "/dags/my_dag",
304+
input: { hash: "", pathname: "/dags/my_dag", search: "" },
305+
},
306+
{
307+
description: "preserves pathname and search",
308+
expected: "/dags/my_dag?tab=graph",
309+
input: { hash: "", pathname: "/dags/my_dag", search: "?tab=graph" },
310+
},
311+
{
312+
description: "preserves pathname, search, and hash",
313+
expected: "/dags/my_dag?tab=graph#section",
314+
input: { hash: "#section", pathname: "/dags/my_dag", search: "?tab=graph" },
315+
},
316+
{
317+
description: "preserves proxied base path, search, and hash",
318+
expected: "/team-a/dags/my_dag?tab=graph#section",
319+
input: { hash: "#section", pathname: "/team-a/dags/my_dag", search: "?tab=graph" },
320+
},
321+
{
322+
description: "handles root path",
323+
expected: "/",
324+
input: { hash: "", pathname: "/", search: "" },
325+
},
326+
])("$description", ({ expected, input }) => {
327+
expect(getNextHref(input)).toBe(expected);
328+
});
329+
330+
it("does not include the origin (no http(s) prefix)", () => {
331+
const result = getNextHref({ hash: "", pathname: "/dags/my_dag", search: "" });
332+
333+
expect(result.startsWith("http://")).toBe(false);
334+
expect(result.startsWith("https://")).toBe(false);
335+
});
336+
});

airflow-core/src/airflow/ui/src/utils/links.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,13 @@ export const getRedirectPath = (targetPath: string): string => {
4747
return new URL(targetPath, baseUrl).pathname;
4848
};
4949

50+
// Build a same-origin "next" target (path + query + hash) from a Location.
51+
// Using a relative URL ensures redirects work correctly when the UI is
52+
// reached through a proxy or a different origin than the API server reports
53+
// (e.g. Gitpod port-based domains, see #46533).
54+
export const getNextHref = (location: Pick<Location, "hash" | "pathname" | "search">): string =>
55+
`${location.pathname}${location.search}${location.hash}`;
56+
5057
export const getTaskInstanceAdditionalPath = (pathname: string): string => {
5158
const subRoutes = taskInstanceRoutes.filter((route) => route.path !== undefined).map((route) => route.path);
5259
// Look for patterns like /tasks/{taskId}/mapped/{mapIndex}/{sub-route}

0 commit comments

Comments
 (0)