This is an automated email from the ASF dual-hosted git repository.

pierrejeambrun pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/main by this push:
     new cc70fce5bed AIP-38 Move token handling to axios interceptor (#47562)
cc70fce5bed is described below

commit cc70fce5bed792f41cc981fdc94636c434c34b9f
Author: Pierre Jeambrun <[email protected]>
AuthorDate: Mon Mar 10 17:23:02 2025 +0100

    AIP-38 Move token handling to axios interceptor (#47562)
---
 airflow/ui/src/layouts/BaseLayout.tsx              | 22 +----------
 airflow/ui/src/main.tsx                            | 14 +------
 .../tokenHandler.test.ts}                          | 28 ++++++--------
 airflow/ui/src/utils/tokenHandler.ts               | 45 ++++++++++++++++++++++
 .../providers/fab/auth_manager/fab_auth_manager.py |  2 +-
 .../unit/fab/auth_manager/test_fab_auth_manager.py |  2 +-
 6 files changed, 63 insertions(+), 50 deletions(-)

diff --git a/airflow/ui/src/layouts/BaseLayout.tsx 
b/airflow/ui/src/layouts/BaseLayout.tsx
index 61c1780eb17..769009ca01a 100644
--- a/airflow/ui/src/layouts/BaseLayout.tsx
+++ b/airflow/ui/src/layouts/BaseLayout.tsx
@@ -17,18 +17,13 @@
  * under the License.
  */
 import { Box } from "@chakra-ui/react";
-import { useEffect, type PropsWithChildren } from "react";
-import { Outlet, useSearchParams } from "react-router-dom";
-import { useLocalStorage } from "usehooks-ts";
+import type { PropsWithChildren } from "react";
+import { Outlet } from "react-router-dom";
 
 import { useConfig } from "src/queries/useConfig";
 
 import { Nav } from "./Nav";
 
-export const TOKEN_STORAGE_KEY = "token";
-
-export const TOKEN_QUERY_PARAM_NAME = "token";
-
 export const BaseLayout = ({ children }: PropsWithChildren) => {
   const instanceName = useConfig("instance_name");
 
@@ -36,19 +31,6 @@ export const BaseLayout = ({ children }: PropsWithChildren) 
=> {
     document.title = instanceName;
   }
 
-  const [searchParams, setSearchParams] = useSearchParams();
-  const paramToken = searchParams.get(TOKEN_QUERY_PARAM_NAME);
-
-  const [, setToken] = useLocalStorage<string | null>(TOKEN_STORAGE_KEY, 
paramToken);
-
-  useEffect(() => {
-    if (paramToken !== null) {
-      setToken(paramToken);
-      searchParams.delete(TOKEN_QUERY_PARAM_NAME);
-      setSearchParams(searchParams);
-    }
-  }, [paramToken, searchParams, setSearchParams, setToken]);
-
   return (
     <>
       <Nav />
diff --git a/airflow/ui/src/main.tsx b/airflow/ui/src/main.tsx
index 2315a8666c3..5d40f493223 100644
--- a/airflow/ui/src/main.tsx
+++ b/airflow/ui/src/main.tsx
@@ -27,9 +27,9 @@ import { ColorModeProvider } from "src/context/colorMode";
 import { TimezoneProvider } from "src/context/timezone";
 import { router } from "src/router";
 
-import { TOKEN_STORAGE_KEY } from "./layouts/BaseLayout";
 import { queryClient } from "./queryClient";
 import { system } from "./theme";
+import { tokenHandler } from "./utils/tokenHandler";
 
 // redirect to login page if the API responds with unauthorized or forbidden 
errors
 axios.interceptors.response.use(
@@ -46,17 +46,7 @@ axios.interceptors.response.use(
   },
 );
 
-axios.interceptors.request.use((config) => {
-  const token: string | null = localStorage.getItem(TOKEN_STORAGE_KEY);
-
-  if (token !== null) {
-    // usehooks-ts stores a JSON.stringified version of values, we cannot use 
usehooks-ts here because we are outside of
-    // a react component. Therefore using bare localStorage.getItem and 
manually parsing the value.
-    config.headers.Authorization = `Bearer ${JSON.parse(token)}`;
-  }
-
-  return config;
-});
+axios.interceptors.request.use(tokenHandler);
 
 createRoot(document.querySelector("#root") as HTMLDivElement).render(
   <StrictMode>
diff --git a/airflow/ui/src/layouts/BaseLayout.test.tsx 
b/airflow/ui/src/utils/tokenHandler.test.ts
similarity index 71%
rename from airflow/ui/src/layouts/BaseLayout.test.tsx
rename to airflow/ui/src/utils/tokenHandler.test.ts
index 90f4694d65b..b0073f9560d 100644
--- a/airflow/ui/src/layouts/BaseLayout.test.tsx
+++ b/airflow/ui/src/utils/tokenHandler.test.ts
@@ -16,45 +16,41 @@
  * specific language governing permissions and limitations
  * under the License.
  */
-import { render } from "@testing-library/react";
-import { URLSearchParams } from "node:url";
-import * as reactRouterDom from "react-router-dom";
+import type { InternalAxiosRequestConfig } from "axios";
 import { afterEach, describe, it, vi, expect } from "vitest";
 
-import { Wrapper } from "src/utils/Wrapper";
-
-import { BaseLayout, TOKEN_QUERY_PARAM_NAME, TOKEN_STORAGE_KEY } from 
"./BaseLayout";
+import { TOKEN_QUERY_PARAM_NAME, TOKEN_STORAGE_KEY, tokenHandler } from 
"./tokenHandler";
 
 describe.each([
   { searchParams: new URLSearchParams({ token: "something" }) },
   { searchParams: new URLSearchParams({ param2: "someParam2", token: "else" }) 
},
   { searchParams: new URLSearchParams({}) },
-])("BaseLayout", ({ searchParams }) => {
+])("TokenFlow Interceptor", ({ searchParams }) => {
   it("Should read from the SearchParams, persist to the localStorage and 
remove from the SearchParams", () => {
-    const useSearchParamMock = vi.spyOn(reactRouterDom, "useSearchParams");
-
-    const setSearchParamsMock = vi.fn();
-
     const token = searchParams.get(TOKEN_QUERY_PARAM_NAME);
 
-    useSearchParamMock.mockImplementation(() => [searchParams, 
setSearchParamsMock]);
-
     const setItemMock = vi.spyOn(localStorage, "setItem");
 
-    render(<BaseLayout />, { wrapper: Wrapper });
+    vi.stubGlobal("location", { search: searchParams.toString() });
+
+    const headers = {};
+
+    const config = { headers };
 
-    expect(useSearchParamMock).toHaveBeenCalled();
+    const { headers: updatedHeaders } = tokenHandler(config as 
InternalAxiosRequestConfig);
 
     if (token === null) {
       expect(setItemMock).toHaveBeenCalledTimes(0);
     } else {
       expect(setItemMock).toHaveBeenCalledOnce();
-      expect(setItemMock).toHaveBeenCalledWith(TOKEN_STORAGE_KEY, 
JSON.stringify(token));
+      expect(setItemMock).toHaveBeenCalledWith(TOKEN_STORAGE_KEY, token);
       expect(searchParams).not.to.contains.keys(TOKEN_QUERY_PARAM_NAME);
+      expect(updatedHeaders).toEqual({ Authorization: `Bearer ${token}` });
     }
   });
 });
 
 afterEach(() => {
   vi.restoreAllMocks();
+  vi.unstubAllGlobals();
 });
diff --git a/airflow/ui/src/utils/tokenHandler.ts 
b/airflow/ui/src/utils/tokenHandler.ts
new file mode 100644
index 00000000000..ab267cf96ec
--- /dev/null
+++ b/airflow/ui/src/utils/tokenHandler.ts
@@ -0,0 +1,45 @@
+/*!
+ * 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 type { InternalAxiosRequestConfig } from "axios";
+
+export const TOKEN_STORAGE_KEY = "token";
+export const TOKEN_QUERY_PARAM_NAME = "token";
+
+export const tokenHandler = (config: InternalAxiosRequestConfig) => {
+  const searchParams = new URLSearchParams(globalThis.location.search);
+
+  const tokenUrl = searchParams.get(TOKEN_QUERY_PARAM_NAME);
+
+  let token: string | null;
+
+  if (tokenUrl === null) {
+    token = localStorage.getItem(TOKEN_STORAGE_KEY);
+  } else {
+    localStorage.setItem(TOKEN_QUERY_PARAM_NAME, tokenUrl);
+    searchParams.delete(TOKEN_QUERY_PARAM_NAME);
+    globalThis.location.search = searchParams.toString();
+    token = tokenUrl;
+  }
+
+  if (token !== null) {
+    config.headers.Authorization = `Bearer ${token}`;
+  }
+
+  return config;
+};
diff --git 
a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py 
b/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py
index 0c721e713a1..e3dd384df21 100644
--- a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py
+++ b/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py
@@ -411,7 +411,7 @@ class FabAuthManager(BaseAuthManager[User]):
 
     def get_url_login(self, **kwargs) -> str:
         """Return the login page url."""
-        return f"{self.apiserver_endpoint}/auth/login"
+        return f"{self.apiserver_endpoint}/auth/login/"
 
     def get_url_logout(self):
         """Return the logout page url."""
diff --git a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py 
b/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py
index a50489b1610..a9f7ba1275b 100644
--- a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py
+++ b/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py
@@ -561,7 +561,7 @@ class TestFabAuthManager:
 
     def test_get_url_login(self, auth_manager):
         result = auth_manager.get_url_login()
-        assert result == "http://localhost:8080/auth/login";
+        assert result == "http://localhost:8080/auth/login/";
 
     @pytest.mark.db_test
     def test_get_url_logout_when_auth_view_not_defined(self, 
auth_manager_with_appbuilder):

Reply via email to