You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "Subham-KRLX (via GitHub)" <gi...@apache.org> on 2026/03/08 17:27:16 UTC
[PR] Fix regression in security iframe navigation logic with basename [airflow]
Subham-KRLX opened a new pull request, #63141:
URL: https://github.com/apache/airflow/pull/63141
Fixes a regression where the security iframe ignores the configured `AIRFLOW__API__BASE_URL` (basename), causing valid navigation within the iframe to incorrectly redirect users to the root dashboard.
The fix updates [Security.tsx](cci:7://file:///Users/subhamsangwan/airflow/airflow-core/src/airflow/ui/src/pages/Security.tsx:0:0-0:0) to read the `<base>` tag's `href` (the same approach used by [router.tsx](cci:7://file:///Users/subhamsangwan/airflow/airflow-core/src/airflow/ui/src/router.tsx:0:0-0:0)) and safely strips the basename from the iframe's pathname before checking if it starts with `/auth/`.
closes: #63111
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix regression in security iframe navigation logic with basename [airflow]
Posted by "JC-comp (via GitHub)" <gi...@apache.org>.
JC-comp commented on code in PR #63141:
URL: https://github.com/apache/airflow/pull/63141#discussion_r2910120720
##########
airflow-core/src/airflow/ui/src/pages/Security.tsx:
##########
@@ -43,8 +43,18 @@ export const Security = () => {
const onLoad = () => {
const iframe: HTMLIFrameElement | null = document.querySelector("#security-iframe");
- if (iframe?.contentWindow && !iframe.contentWindow.location.pathname.startsWith("/auth/")) {
- void Promise.resolve(navigate("/"));
+ if (iframe?.contentWindow) {
+ const baseHref = document.querySelector("base")?.href;
+ const basename = baseHref === undefined ? "" : new URL(baseHref).pathname.replace(/\/$/u, "");
Review Comment:
Since we're already within a routing context, we can avoid manual DOM scraping of the `<base>` tag. Using [`useHref`](https://reactrouter.com/api/hooks/useHref) to resolve `/auth` automatically respects the `basename` configured in the Router provider. This makes the logic cleaner and removes the need to manually slice or parse the `iframePath`.
```js
const authPath = useHref("/auth");
...
const onLoad = () => {
if (!iframePath.startsWith(authPath)) {
...
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix regression in security iframe navigation logic with basename [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on code in PR #63141:
URL: https://github.com/apache/airflow/pull/63141#discussion_r2905536534
##########
airflow-core/src/airflow/ui/src/pages/Security.tsx:
##########
@@ -43,8 +43,18 @@ export const Security = () => {
const onLoad = () => {
const iframe: HTMLIFrameElement | null = document.querySelector("#security-iframe");
- if (iframe?.contentWindow && !iframe.contentWindow.location.pathname.startsWith("/auth/")) {
- void Promise.resolve(navigate("/"));
+ if (iframe?.contentWindow) {
+ const baseUrl = document.querySelector("base")?.href ?? "http://localhost:8080/";
Review Comment:
We shouldn't hardcode `localhost:8080` here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix regression in security iframe navigation logic with basename [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun merged PR #63141:
URL: https://github.com/apache/airflow/pull/63141
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix regression in security iframe navigation logic with basename [airflow]
Posted by "Subham-KRLX (via GitHub)" <gi...@apache.org>.
Subham-KRLX commented on PR #63141:
URL: https://github.com/apache/airflow/pull/63141#issuecomment-4023981237
I have addressed the feedback and removed the hardcoded `localhost:8080` fallback in . If the base tag href is missing, it now properly uses an empty string to avoid any incorrect routing. The static checks should also be green now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix regression in security iframe navigation logic with basename [airflow]
Posted by "Subham-KRLX (via GitHub)" <gi...@apache.org>.
Subham-KRLX commented on PR #63141:
URL: https://github.com/apache/airflow/pull/63141#issuecomment-4032649769
Simplified the iframe navigation logic using document.baseURI to handle the basename as suggested. I have also removed the newsfragment since it's a bugfix. All static checks are passing locally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix regression in security iframe navigation logic with basename [airflow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #63141:
URL: https://github.com/apache/airflow/pull/63141#issuecomment-4047540330
### Backport successfully created: v3-1-test
Note: As of [Merging PRs targeted for Airflow 3.X](https://github.com/apache/airflow/blob/main/dev/README_AIRFLOW3_DEV.md#merging-prs-targeted-for-airflow-3x)
the committer who merges the PR is responsible for backporting the PRs that are bug fixes (generally speaking) to the maintenance branches.
In matter of doubt please ask in [#release-management](https://apache-airflow.slack.com/archives/C03G9H97MM2) Slack channel.
<table>
<tr>
<th>Status</th>
<th>Branch</th>
<th>Result</th>
</tr>
<tr>
<td>✅</td>
<td>v3-1-test</td>
<td><a href="https://github.com/apache/airflow/pull/63453"><img src="https://img.shields.io/badge/PR-63453-blue" alt="PR Link"></a></td>
</tr>
</table>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix regression in security iframe navigation logic with basename [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #63141:
URL: https://github.com/apache/airflow/pull/63141#discussion_r2912672699
##########
airflow-core/newsfragments/63141.bugfix.rst:
##########
@@ -0,0 +1 @@
+Fix security iframe navigation when AIRFLOW__API__BASE_URL basename is configured
Review Comment:
We do not use newsframgnets for bugfixes. Only for 'significant' ones.
Can you remove this file please.
##########
airflow-core/src/airflow/ui/src/pages/Security.tsx:
##########
@@ -43,8 +43,18 @@ export const Security = () => {
const onLoad = () => {
const iframe: HTMLIFrameElement | null = document.querySelector("#security-iframe");
- if (iframe?.contentWindow && !iframe.contentWindow.location.pathname.startsWith("/auth/")) {
- void Promise.resolve(navigate("/"));
+ if (iframe?.contentWindow) {
+ const baseHref = document.querySelector("base")?.href;
+ const basename = baseHref === undefined ? "" : new URL(baseHref).pathname.replace(/\/$/u, "");
+
+ const iframePath = iframe.contentWindow.location.pathname;
+ const pathWithoutBase = iframePath.startsWith(basename)
+ ? iframePath.slice(basename.length)
+ : iframePath;
+
+ if (!pathWithoutBase.startsWith("/auth/")) {
+ void Promise.resolve(navigate("/"));
+ }
}
Review Comment:
This is overly complicated I believe:
```suggestion
if (iframe?.contentWindow) {
const base = new URL(document.baseURI).pathname.replace(/\/$/u, ""); // Remove trailing slash if exists
if (!iframe.contentWindow.location.pathname.startsWith(`${base}/auth/`)) {
void Promise.resolve(navigate("/"));
}
}
};
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Fix regression in security iframe navigation logic with basename [airflow]
Posted by "Subham-KRLX (via GitHub)" <gi...@apache.org>.
Subham-KRLX commented on code in PR #63141:
URL: https://github.com/apache/airflow/pull/63141#discussion_r2905703116
##########
airflow-core/src/airflow/ui/src/pages/Security.tsx:
##########
@@ -43,8 +43,18 @@ export const Security = () => {
const onLoad = () => {
const iframe: HTMLIFrameElement | null = document.querySelector("#security-iframe");
- if (iframe?.contentWindow && !iframe.contentWindow.location.pathname.startsWith("/auth/")) {
- void Promise.resolve(navigate("/"));
+ if (iframe?.contentWindow) {
+ const baseUrl = document.querySelector("base")?.href ?? "http://localhost:8080/";
Review Comment:
Removed the hardcoded localhost:8080 fallback. Now when no <base> tag is found, [basename](vscode-file://vscode-app/Applications/Visual%20Studio%20Code.app/Contents/Resources/app/out/vs/code/electron-browser/workbench/workbench.html) defaults to "" (empty string), so the path is checked as-is. Also added a newsfragment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org