From 4a3bdec634d6a6704c557d97cf215961a6e9c24c Mon Sep 17 00:00:00 2001 From: pierrejeambrun Date: Thu, 13 Mar 2025 14:16:39 +0100 Subject: [PATCH 1/4] AIP-84 Add logout endpoint --- .../core_api/openapi/v1-generated.yaml | 36 ++++++++++++++++++- .../core_api/routes/public/__init__.py | 4 +-- .../routes/public/{login.py => auth.py} | 19 ++++++++-- airflow/ui/openapi-gen/queries/common.ts | 14 ++++++++ airflow/ui/openapi-gen/queries/prefetch.ts | 20 +++++++++++ airflow/ui/openapi-gen/queries/queries.ts | 26 ++++++++++++++ airflow/ui/openapi-gen/queries/suspense.ts | 26 ++++++++++++++ .../ui/openapi-gen/requests/services.gen.ts | 26 +++++++++++++- airflow/ui/openapi-gen/requests/types.gen.ts | 27 +++++++++++++- airflow/ui/src/main.tsx | 2 +- .../public/{test_login.py => test_auth.py} | 20 ++++++++--- 11 files changed, 207 insertions(+), 13 deletions(-) rename airflow/api_fastapi/core_api/routes/public/{login.py => auth.py} (78%) rename tests/api_fastapi/core_api/routes/public/{test_login.py => test_auth.py} (77%) diff --git a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml index fca067f9a3c18..cb984528e7e62 100644 --- a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml +++ b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml @@ -7216,7 +7216,7 @@ paths: application/json: schema: $ref: '#/components/schemas/VersionInfo' - /public/login: + /public/auth/login: get: tags: - Login @@ -7250,6 +7250,40 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPValidationError' + /public/auth/logout: + get: + tags: + - Login + summary: Logout + description: Logout the user. + operationId: logout + parameters: + - name: next + in: query + required: false + schema: + anyOf: + - type: string + - type: 'null' + title: Next + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + '307': + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPExceptionResponse' + description: Temporary Redirect + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' components: schemas: AppBuilderMenuItemResponse: diff --git a/airflow/api_fastapi/core_api/routes/public/__init__.py b/airflow/api_fastapi/core_api/routes/public/__init__.py index fb1ba41603454..114c851a775ad 100644 --- a/airflow/api_fastapi/core_api/routes/public/__init__.py +++ b/airflow/api_fastapi/core_api/routes/public/__init__.py @@ -22,6 +22,7 @@ from airflow.api_fastapi.common.router import AirflowRouter from airflow.api_fastapi.core_api.openapi.exceptions import create_openapi_http_exception_doc from airflow.api_fastapi.core_api.routes.public.assets import assets_router +from airflow.api_fastapi.core_api.routes.public.auth import auth_router from airflow.api_fastapi.core_api.routes.public.backfills import backfills_router from airflow.api_fastapi.core_api.routes.public.config import config_router from airflow.api_fastapi.core_api.routes.public.connections import connections_router @@ -39,7 +40,6 @@ from airflow.api_fastapi.core_api.routes.public.import_error import import_error_router from airflow.api_fastapi.core_api.routes.public.job import job_router from airflow.api_fastapi.core_api.routes.public.log import task_instances_log_router -from airflow.api_fastapi.core_api.routes.public.login import login_router from airflow.api_fastapi.core_api.routes.public.monitor import monitor_router from airflow.api_fastapi.core_api.routes.public.plugins import plugins_router from airflow.api_fastapi.core_api.routes.public.pools import pools_router @@ -90,4 +90,4 @@ # Following routers are not included in common router, for now we don't expect it to have authentication public_router.include_router(monitor_router) public_router.include_router(version_router) -public_router.include_router(login_router) +public_router.include_router(auth_router) diff --git a/airflow/api_fastapi/core_api/routes/public/login.py b/airflow/api_fastapi/core_api/routes/public/auth.py similarity index 78% rename from airflow/api_fastapi/core_api/routes/public/login.py rename to airflow/api_fastapi/core_api/routes/public/auth.py index 75ef49c7c4f74..589294d71a0eb 100644 --- a/airflow/api_fastapi/core_api/routes/public/login.py +++ b/airflow/api_fastapi/core_api/routes/public/auth.py @@ -23,11 +23,11 @@ from airflow.api_fastapi.core_api.openapi.exceptions import create_openapi_http_exception_doc from airflow.api_fastapi.core_api.security import is_safe_url -login_router = AirflowRouter(tags=["Login"], prefix="/login") +auth_router = AirflowRouter(tags=["Login"], prefix="/auth") -@login_router.get( - "", +@auth_router.get( + "/login", responses=create_openapi_http_exception_doc([status.HTTP_307_TEMPORARY_REDIRECT]), ) def login(request: Request, next: None | str = None) -> RedirectResponse: @@ -40,3 +40,16 @@ def login(request: Request, next: None | str = None) -> RedirectResponse: if next: login_url += f"?next={next}" return RedirectResponse(login_url) + + +@auth_router.get( + "/logout", + responses=create_openapi_http_exception_doc([status.HTTP_307_TEMPORARY_REDIRECT]), +) +def logout(request: Request, next: None | str = None) -> RedirectResponse: + """Logout the user.""" + request.app.state.auth_manager.logout() + + login_url = request.app.state.auth_manager.get_url_login() + + return RedirectResponse(login_url) diff --git a/airflow/ui/openapi-gen/queries/common.ts b/airflow/ui/openapi-gen/queries/common.ts index f3f17b7da7713..be7a9fbf9497e 100644 --- a/airflow/ui/openapi-gen/queries/common.ts +++ b/airflow/ui/openapi-gen/queries/common.ts @@ -1778,6 +1778,20 @@ export const UseLoginServiceLoginKeyFn = ( } = {}, queryKey?: Array, ) => [useLoginServiceLoginKey, ...(queryKey ?? [{ next }])]; +export type LoginServiceLogoutDefaultResponse = Awaited>; +export type LoginServiceLogoutQueryResult< + TData = LoginServiceLogoutDefaultResponse, + TError = unknown, +> = UseQueryResult; +export const useLoginServiceLogoutKey = "LoginServiceLogout"; +export const UseLoginServiceLogoutKeyFn = ( + { + next, + }: { + next?: string; + } = {}, + queryKey?: Array, +) => [useLoginServiceLogoutKey, ...(queryKey ?? [{ next }])]; export type AssetServiceCreateAssetEventMutationResult = Awaited< ReturnType >; diff --git a/airflow/ui/openapi-gen/queries/prefetch.ts b/airflow/ui/openapi-gen/queries/prefetch.ts index 447a3dbbff78c..ac4778250df22 100644 --- a/airflow/ui/openapi-gen/queries/prefetch.ts +++ b/airflow/ui/openapi-gen/queries/prefetch.ts @@ -2486,3 +2486,23 @@ export const prefetchUseLoginServiceLogin = ( queryKey: Common.UseLoginServiceLoginKeyFn({ next }), queryFn: () => LoginService.login({ next }), }); +/** + * Logout + * Logout the user. + * @param data The data for the request. + * @param data.next + * @returns unknown Successful Response + * @throws ApiError + */ +export const prefetchUseLoginServiceLogout = ( + queryClient: QueryClient, + { + next, + }: { + next?: string; + } = {}, +) => + queryClient.prefetchQuery({ + queryKey: Common.UseLoginServiceLogoutKeyFn({ next }), + queryFn: () => LoginService.logout({ next }), + }); diff --git a/airflow/ui/openapi-gen/queries/queries.ts b/airflow/ui/openapi-gen/queries/queries.ts index 32cb6d467e3a8..bd78200aaa73c 100644 --- a/airflow/ui/openapi-gen/queries/queries.ts +++ b/airflow/ui/openapi-gen/queries/queries.ts @@ -2953,6 +2953,32 @@ export const useLoginServiceLogin = < queryFn: () => LoginService.login({ next }) as TData, ...options, }); +/** + * Logout + * Logout the user. + * @param data The data for the request. + * @param data.next + * @returns unknown Successful Response + * @throws ApiError + */ +export const useLoginServiceLogout = < + TData = Common.LoginServiceLogoutDefaultResponse, + TError = unknown, + TQueryKey extends Array = unknown[], +>( + { + next, + }: { + next?: string; + } = {}, + queryKey?: TQueryKey, + options?: Omit, "queryKey" | "queryFn">, +) => + useQuery({ + queryKey: Common.UseLoginServiceLogoutKeyFn({ next }, queryKey), + queryFn: () => LoginService.logout({ next }) as TData, + ...options, + }); /** * Create Asset Event * Create asset events. diff --git a/airflow/ui/openapi-gen/queries/suspense.ts b/airflow/ui/openapi-gen/queries/suspense.ts index b9806dc9070db..bb643b21329c6 100644 --- a/airflow/ui/openapi-gen/queries/suspense.ts +++ b/airflow/ui/openapi-gen/queries/suspense.ts @@ -2930,3 +2930,29 @@ export const useLoginServiceLoginSuspense = < queryFn: () => LoginService.login({ next }) as TData, ...options, }); +/** + * Logout + * Logout the user. + * @param data The data for the request. + * @param data.next + * @returns unknown Successful Response + * @throws ApiError + */ +export const useLoginServiceLogoutSuspense = < + TData = Common.LoginServiceLogoutDefaultResponse, + TError = unknown, + TQueryKey extends Array = unknown[], +>( + { + next, + }: { + next?: string; + } = {}, + queryKey?: TQueryKey, + options?: Omit, "queryKey" | "queryFn">, +) => + useSuspenseQuery({ + queryKey: Common.UseLoginServiceLogoutKeyFn({ next }, queryKey), + queryFn: () => LoginService.logout({ next }) as TData, + ...options, + }); diff --git a/airflow/ui/openapi-gen/requests/services.gen.ts b/airflow/ui/openapi-gen/requests/services.gen.ts index 700ab409409c1..ce55dfb254342 100644 --- a/airflow/ui/openapi-gen/requests/services.gen.ts +++ b/airflow/ui/openapi-gen/requests/services.gen.ts @@ -213,6 +213,8 @@ import type { GetVersionResponse, LoginData, LoginResponse, + LogoutData, + LogoutResponse, } from "./types.gen"; export class AuthLinksService { @@ -3546,7 +3548,29 @@ export class LoginService { public static login(data: LoginData = {}): CancelablePromise { return __request(OpenAPI, { method: "GET", - url: "/public/login", + url: "/public/auth/login", + query: { + next: data.next, + }, + errors: { + 307: "Temporary Redirect", + 422: "Validation Error", + }, + }); + } + + /** + * Logout + * Logout the user. + * @param data The data for the request. + * @param data.next + * @returns unknown Successful Response + * @throws ApiError + */ + public static logout(data: LogoutData = {}): CancelablePromise { + return __request(OpenAPI, { + method: "GET", + url: "/public/auth/logout", query: { next: data.next, }, diff --git a/airflow/ui/openapi-gen/requests/types.gen.ts b/airflow/ui/openapi-gen/requests/types.gen.ts index 3c0016a6349b9..84ccba08f7d81 100644 --- a/airflow/ui/openapi-gen/requests/types.gen.ts +++ b/airflow/ui/openapi-gen/requests/types.gen.ts @@ -2593,6 +2593,12 @@ export type LoginData = { export type LoginResponse = unknown; +export type LogoutData = { + next?: string | null; +}; + +export type LogoutResponse = unknown; + export type $OpenApiTs = { "/ui/auth/links": { get: { @@ -5381,7 +5387,7 @@ export type $OpenApiTs = { }; }; }; - "/public/login": { + "/public/auth/login": { get: { req: LoginData; res: { @@ -5400,4 +5406,23 @@ export type $OpenApiTs = { }; }; }; + "/public/auth/logout": { + get: { + req: LogoutData; + res: { + /** + * Successful Response + */ + 200: unknown; + /** + * Temporary Redirect + */ + 307: HTTPExceptionResponse; + /** + * Validation Error + */ + 422: HTTPValidationError; + }; + }; + }; }; diff --git a/airflow/ui/src/main.tsx b/airflow/ui/src/main.tsx index 5d40f493223a6..e80a74ef192c0 100644 --- a/airflow/ui/src/main.tsx +++ b/airflow/ui/src/main.tsx @@ -39,7 +39,7 @@ axios.interceptors.response.use( const params = new URLSearchParams(); params.set("next", globalThis.location.href); - globalThis.location.replace(`/public/login?${params.toString()}`); + globalThis.location.replace(`/public/auth/login?${params.toString()}`); } return Promise.reject(error); diff --git a/tests/api_fastapi/core_api/routes/public/test_login.py b/tests/api_fastapi/core_api/routes/public/test_auth.py similarity index 77% rename from tests/api_fastapi/core_api/routes/public/test_login.py rename to tests/api_fastapi/core_api/routes/public/test_auth.py index f3e407db0e9bc..f016781f3038c 100644 --- a/tests/api_fastapi/core_api/routes/public/test_login.py +++ b/tests/api_fastapi/core_api/routes/public/test_auth.py @@ -27,7 +27,7 @@ pytestmark = pytest.mark.db_test -class TestLoginEndpoint: +class TestAuthEndpoint: @pytest.fixture(autouse=True) def setup(self, test_client) -> None: auth_manager_mock = MagicMock() @@ -35,7 +35,7 @@ def setup(self, test_client) -> None: test_client.app.state.auth_manager = auth_manager_mock -class TestGetLogin(TestLoginEndpoint): +class TestGetLogin(TestAuthEndpoint): @pytest.mark.parametrize( "params", [ @@ -46,7 +46,7 @@ class TestGetLogin(TestLoginEndpoint): ], ) def test_should_respond_307(self, test_client, params): - response = test_client.get("/public/login", follow_redirects=False, params=params) + response = test_client.get("/public/auth/login", follow_redirects=False, params=params) assert response.status_code == 307 assert ( @@ -64,6 +64,18 @@ def test_should_respond_307(self, test_client, params): ) @conf_vars({("api", "base_url"): "http://localhost:8080/prefix"}) def test_should_respond_400(self, test_client, params): - response = test_client.get("/public/login", follow_redirects=False, params=params) + response = test_client.get("/public/auth/login", follow_redirects=False, params=params) assert response.status_code == 400 + + +class TestLogout(TestAuthEndpoint): + def test_should_respond_307( + self, + test_client, + ): + response = test_client.get("/public/auth/logout", follow_redirects=False) + test_client.app.state.auth_manager.logout.assert_called_once_with() + + assert response.status_code == 307 + assert response.headers["location"] == AUTH_MANAGER_LOGIN_URL From b9109f40b4f65e722d0ef1142692eef9ae84d016 Mon Sep 17 00:00:00 2001 From: pierrejeambrun Date: Thu, 13 Mar 2025 17:54:28 +0100 Subject: [PATCH 2/4] Adjust following AuthManager update --- .../api_fastapi/core_api/routes/public/auth.py | 6 ++++-- .../core_api/routes/public/test_auth.py | 15 +++++++++++++-- 2 files changed, 17 insertions(+), 4 deletions(-) diff --git a/airflow/api_fastapi/core_api/routes/public/auth.py b/airflow/api_fastapi/core_api/routes/public/auth.py index 589294d71a0eb..f17bd61c027fe 100644 --- a/airflow/api_fastapi/core_api/routes/public/auth.py +++ b/airflow/api_fastapi/core_api/routes/public/auth.py @@ -48,8 +48,10 @@ def login(request: Request, next: None | str = None) -> RedirectResponse: ) def logout(request: Request, next: None | str = None) -> RedirectResponse: """Logout the user.""" - request.app.state.auth_manager.logout() + logout_url = request.app.state.auth_manager.get_url_logout() - login_url = request.app.state.auth_manager.get_url_login() + if logout_url: + return RedirectResponse(logout_url) + login_url = request.app.state.auth_manager.get_url_login() return RedirectResponse(login_url) diff --git a/tests/api_fastapi/core_api/routes/public/test_auth.py b/tests/api_fastapi/core_api/routes/public/test_auth.py index f016781f3038c..3dec469d7fd43 100644 --- a/tests/api_fastapi/core_api/routes/public/test_auth.py +++ b/tests/api_fastapi/core_api/routes/public/test_auth.py @@ -70,12 +70,23 @@ def test_should_respond_400(self, test_client, params): class TestLogout(TestAuthEndpoint): + @pytest.mark.parametrize( + "mock_logout_url, expected_redirection", + [ + # logout_url is None, should redirect to the login page directly. + (None, AUTH_MANAGER_LOGIN_URL), + # logout_url is defined, should redirect to the logout_url. + ("http://localhost/auth/some_logout_url", "http://localhost/auth/some_logout_url"), + ], + ) def test_should_respond_307( self, test_client, + mock_logout_url, + expected_redirection, ): + test_client.app.state.auth_manager.get_url_logout.return_value = mock_logout_url response = test_client.get("/public/auth/logout", follow_redirects=False) - test_client.app.state.auth_manager.logout.assert_called_once_with() assert response.status_code == 307 - assert response.headers["location"] == AUTH_MANAGER_LOGIN_URL + assert response.headers["location"] == expected_redirection From a2e436a4f8a1571d4c9e3c46ff97c5fa808081ec Mon Sep 17 00:00:00 2001 From: pierrejeambrun Date: Thu, 13 Mar 2025 18:20:12 +0100 Subject: [PATCH 3/4] Address PR comment --- airflow/api_fastapi/core_api/routes/public/auth.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/airflow/api_fastapi/core_api/routes/public/auth.py b/airflow/api_fastapi/core_api/routes/public/auth.py index f17bd61c027fe..cd1f87d766c71 100644 --- a/airflow/api_fastapi/core_api/routes/public/auth.py +++ b/airflow/api_fastapi/core_api/routes/public/auth.py @@ -50,8 +50,7 @@ def logout(request: Request, next: None | str = None) -> RedirectResponse: """Logout the user.""" logout_url = request.app.state.auth_manager.get_url_logout() - if logout_url: - return RedirectResponse(logout_url) + if not logout_url: + logout_url = request.app.state.auth_manager.get_url_login() - login_url = request.app.state.auth_manager.get_url_login() - return RedirectResponse(login_url) + return RedirectResponse(logout_url) From ea1afd4f2fb8567a35ebc0902ae21aead01ae804 Mon Sep 17 00:00:00 2001 From: pierrejeambrun Date: Thu, 13 Mar 2025 19:21:14 +0100 Subject: [PATCH 4/4] Fix CI --- tests/api_fastapi/core_api/routes/test_routes.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/api_fastapi/core_api/routes/test_routes.py b/tests/api_fastapi/core_api/routes/test_routes.py index a67e300031c1c..0b42af3379c06 100644 --- a/tests/api_fastapi/core_api/routes/test_routes.py +++ b/tests/api_fastapi/core_api/routes/test_routes.py @@ -20,7 +20,8 @@ # Set of paths that are allowed to be accessible without authentication NO_AUTH_PATHS = { - "/public/login", + "/public/auth/login", + "/public/auth/logout", "/public/version", "/public/monitor/health", }