From e41e891f4458112c411fa59b1ee814052eb7ea71 Mon Sep 17 00:00:00 2001 From: LIU ZHE YOU Date: Fri, 28 Feb 2025 20:25:41 +0800 Subject: [PATCH 1/3] AIP-84 | Add Auth for Config --- .../core_api/openapi/v1-generated.yaml | 8 +++++++- .../core_api/routes/public/config.py | 5 ++++- airflow/api_fastapi/core_api/security.py | 20 +++++++++++++++++++ airflow/ui/openapi-gen/requests/types.gen.ts | 2 +- .../core_api/routes/public/test_config.py | 20 +++++++++++++++++++ 5 files changed, 52 insertions(+), 3 deletions(-) diff --git a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml index 3ae3391c1a4b0..7dd74a9396080 100644 --- a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml +++ b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml @@ -2832,6 +2832,8 @@ paths: - Config summary: Get Config operationId: get_config + security: + - OAuth2PasswordBearer: [] parameters: - name: section in: query @@ -2912,12 +2914,16 @@ paths: - Config summary: Get Config Value operationId: get_config_value + security: + - OAuth2PasswordBearer: [] parameters: - name: section in: path required: true schema: - type: string + anyOf: + - type: string + - type: 'null' title: Section - name: option in: path diff --git a/airflow/api_fastapi/core_api/routes/public/config.py b/airflow/api_fastapi/core_api/routes/public/config.py index 13edf5f821b4c..1df1582591581 100644 --- a/airflow/api_fastapi/core_api/routes/public/config.py +++ b/airflow/api_fastapi/core_api/routes/public/config.py @@ -18,7 +18,7 @@ import textwrap -from fastapi import HTTPException, status +from fastapi import Depends, HTTPException, status from fastapi.responses import Response from airflow.api_fastapi.common.headers import HeaderAcceptJsonOrText @@ -30,6 +30,7 @@ ConfigSection, ) from airflow.api_fastapi.core_api.openapi.exceptions import create_openapi_http_exception_doc +from airflow.api_fastapi.core_api.security import requires_access_configuration from airflow.configuration import conf text_example_response_for_get_config_value = { @@ -108,6 +109,7 @@ def _response_based_on_accept(accept: Mimetype, config: Config): }, }, response_model=Config, + dependencies=[Depends(requires_access_configuration("GET"))], ) def get_config( accept: HeaderAcceptJsonOrText, @@ -153,6 +155,7 @@ def get_config( }, }, response_model=Config, + dependencies=[Depends(requires_access_configuration("GET"))], ) def get_config_value( section: str, diff --git a/airflow/api_fastapi/core_api/security.py b/airflow/api_fastapi/core_api/security.py index 0c9be4be64667..3c09193a80932 100644 --- a/airflow/api_fastapi/core_api/security.py +++ b/airflow/api_fastapi/core_api/security.py @@ -27,6 +27,7 @@ from airflow.auth.managers.models.base_user import BaseUser from airflow.auth.managers.models.resource_details import ( AssetDetails, + ConfigurationDetails, ConnectionDetails, DagAccessEntity, DagDetails, @@ -123,6 +124,25 @@ def inner( return inner +def requires_access_configuration(method: ResourceMethod) -> Callable[[str | None, BaseUser | None], None]: + def inner( + section: str | None = None, + user: Annotated[BaseUser | None, Depends(get_user)] = None, + ) -> None: + def callback(): + return get_auth_manager().is_authorized_configuration( + method=method, + details=ConfigurationDetails(section=section), + user=user, + ) + + _requires_access( + is_authorized_callback=callback, + ) + + return inner + + def requires_access_variable(method: ResourceMethod) -> Callable[[Request, BaseUser | None], None]: def inner( request: Request, diff --git a/airflow/ui/openapi-gen/requests/types.gen.ts b/airflow/ui/openapi-gen/requests/types.gen.ts index f3578ab98a9c8..3abdcfae4fd65 100644 --- a/airflow/ui/openapi-gen/requests/types.gen.ts +++ b/airflow/ui/openapi-gen/requests/types.gen.ts @@ -1731,7 +1731,7 @@ export type GetConfigResponse = Config; export type GetConfigValueData = { accept?: "application/json" | "text/plain" | "*/*"; option: string; - section: string; + section: string | null; }; export type GetConfigValueResponse = Config; diff --git a/tests/api_fastapi/core_api/routes/public/test_config.py b/tests/api_fastapi/core_api/routes/public/test_config.py index 90009d1b54152..914cd5fcb3f0a 100644 --- a/tests/api_fastapi/core_api/routes/public/test_config.py +++ b/tests/api_fastapi/core_api/routes/public/test_config.py @@ -305,6 +305,14 @@ def test_get_config_non_sensitive_only( response = test_client.get("/public/config", headers=headers) self._validate_response(headers, expected_response, expected_status_code, response) + def test_get_config_should_response_401(self, unauthenticated_test_client): + response = unauthenticated_test_client.get("/public/config") + assert response.status_code == 401 + + def test_get_config_should_response_403(self, unauthorized_test_client): + response = unauthorized_test_client.get("/public/config") + assert response.status_code == 403 + class TestGetConfigValue(TestConfigEndpoint): @pytest.mark.parametrize( @@ -474,3 +482,15 @@ def test_get_config_value_non_sensitive_only( with conf_vars(AIRFLOW_CONFIG_NON_SENSITIVE_ONLY_CONFIG): response = test_client.get(f"/public/config/section/{section}/option/{option}", headers=headers) self._validate_response(headers, expected_response, expected_status_code, response) + + def test_get_config_value_should_response_401(self, unauthenticated_test_client): + response = unauthenticated_test_client.get( + f"/public/config/section/{SECTION_DATABASE}/option/{OPTION_KEY_SQL_ALCHEMY_CONN}" + ) + assert response.status_code == 401 + + def test_get_config_value_should_response_403(self, unauthorized_test_client): + response = unauthorized_test_client.get( + f"/public/config/section/{SECTION_DATABASE}/option/{OPTION_KEY_SQL_ALCHEMY_CONN}" + ) + assert response.status_code == 403 From 0e23e255d17ece10cc3dfb768e548adb065ddcd9 Mon Sep 17 00:00:00 2001 From: LIU ZHE YOU Date: Mon, 3 Mar 2025 20:59:00 +0800 Subject: [PATCH 2/3] Fix requires_access_configuration schema --- airflow/api_fastapi/core_api/openapi/v1-generated.yaml | 4 +--- airflow/api_fastapi/core_api/security.py | 6 ++++-- airflow/ui/openapi-gen/requests/types.gen.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml index 7dd74a9396080..1c73f2af0d104 100644 --- a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml +++ b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml @@ -2921,9 +2921,7 @@ paths: in: path required: true schema: - anyOf: - - type: string - - type: 'null' + type: string title: Section - name: option in: path diff --git a/airflow/api_fastapi/core_api/security.py b/airflow/api_fastapi/core_api/security.py index 3c09193a80932..8c1abf9046f84 100644 --- a/airflow/api_fastapi/core_api/security.py +++ b/airflow/api_fastapi/core_api/security.py @@ -124,11 +124,13 @@ def inner( return inner -def requires_access_configuration(method: ResourceMethod) -> Callable[[str | None, BaseUser | None], None]: +def requires_access_configuration(method: ResourceMethod) -> Callable[[Request, BaseUser | None], None]: def inner( - section: str | None = None, + request: Request, user: Annotated[BaseUser | None, Depends(get_user)] = None, ) -> None: + section: str | None = request.query_params.get("section") or request.path_params.get("section") + def callback(): return get_auth_manager().is_authorized_configuration( method=method, diff --git a/airflow/ui/openapi-gen/requests/types.gen.ts b/airflow/ui/openapi-gen/requests/types.gen.ts index 3abdcfae4fd65..f3578ab98a9c8 100644 --- a/airflow/ui/openapi-gen/requests/types.gen.ts +++ b/airflow/ui/openapi-gen/requests/types.gen.ts @@ -1731,7 +1731,7 @@ export type GetConfigResponse = Config; export type GetConfigValueData = { accept?: "application/json" | "text/plain" | "*/*"; option: string; - section: string | null; + section: string; }; export type GetConfigValueResponse = Config; From 12bdc654689a50612a21bb9202aec2fae2b3537e Mon Sep 17 00:00:00 2001 From: LIU ZHE YOU Date: Tue, 4 Mar 2025 21:02:00 +0800 Subject: [PATCH 3/3] Refactor requires_access_configuration with lambda --- airflow/api_fastapi/core_api/security.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/airflow/api_fastapi/core_api/security.py b/airflow/api_fastapi/core_api/security.py index 8c1abf9046f84..a8837a93c4225 100644 --- a/airflow/api_fastapi/core_api/security.py +++ b/airflow/api_fastapi/core_api/security.py @@ -131,15 +131,12 @@ def inner( ) -> None: section: str | None = request.query_params.get("section") or request.path_params.get("section") - def callback(): - return get_auth_manager().is_authorized_configuration( + _requires_access( + is_authorized_callback=lambda: get_auth_manager().is_authorized_configuration( method=method, details=ConfigurationDetails(section=section), user=user, ) - - _requires_access( - is_authorized_callback=callback, ) return inner