From f7f36e6e83c5a260455b8122fee68cc91c6daeb3 Mon Sep 17 00:00:00 2001 From: kyounghoonJang Date: Mon, 3 Nov 2025 17:53:10 +0900 Subject: [PATCH] Migrate FAB DELETE /roles to FastAPI --- .../v2-fab-auth-manager-generated.yaml | 48 +++++++++++ .../auth_manager/api_fastapi/routes/roles.py | 19 ++++- .../api_fastapi/services/roles.py | 12 +++ .../api_fastapi/routes/test_roles.py | 80 +++++++++++++++++++ .../api_fastapi/services/test_roles.py | 20 +++++ 5 files changed, 178 insertions(+), 1 deletion(-) diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/openapi/v2-fab-auth-manager-generated.yaml b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/openapi/v2-fab-auth-manager-generated.yaml index 565e2e3d308d7..ac496dfa556f6 100644 --- a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/openapi/v2-fab-auth-manager-generated.yaml +++ b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/openapi/v2-fab-auth-manager-generated.yaml @@ -215,6 +215,54 @@ paths: application/json: schema: $ref: '#/components/schemas/HTTPValidationError' + /auth/fab/v1/roles/{name}: + delete: + tags: + - FabAuthManager + summary: Delete Role + description: Delete an existing role. + operationId: delete_role + security: + - OAuth2PasswordBearer: [] + - HTTPBearer: [] + parameters: + - name: name + in: path + required: true + schema: + type: string + minLength: 1 + title: Name + responses: + '200': + description: Successful Response + content: + application/json: + schema: {} + '401': + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPExceptionResponse' + description: Unauthorized + '403': + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPExceptionResponse' + description: Forbidden + '404': + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPExceptionResponse' + description: Not Found + '422': + description: Validation Error + content: + application/json: + schema: + $ref: '#/components/schemas/HTTPValidationError' components: schemas: ActionResourceResponse: diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/routes/roles.py b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/routes/roles.py index d2223e0e8de5d..e733b1bbc836a 100644 --- a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/routes/roles.py +++ b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/routes/roles.py @@ -18,7 +18,7 @@ from typing import TYPE_CHECKING -from fastapi import Depends, Query, status +from fastapi import Depends, Path, Query, status from airflow.api_fastapi.common.router import AirflowRouter from airflow.api_fastapi.core_api.openapi.exceptions import create_openapi_http_exception_doc @@ -83,3 +83,20 @@ def get_roles( """List roles with pagination and ordering.""" with get_application_builder(): return FABAuthManagerRoles.get_roles(order_by=order_by, limit=limit, offset=offset) + + +@roles_router.delete( + "/roles/{name}", + responses=create_openapi_http_exception_doc( + [ + status.HTTP_401_UNAUTHORIZED, + status.HTTP_403_FORBIDDEN, + status.HTTP_404_NOT_FOUND, + ] + ), + dependencies=[Depends(requires_fab_custom_view("DELETE", permissions.RESOURCE_ROLE))], +) +def delete_role(name: str = Path(..., min_length=1)) -> None: + """Delete an existing role.""" + with get_application_builder(): + return FABAuthManagerRoles.delete_role(name=name) diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py index 3a3ae6b5c2dea..350a99185f80d 100644 --- a/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py +++ b/providers/fab/src/airflow/providers/fab/auth_manager/api_fastapi/services/roles.py @@ -96,3 +96,15 @@ def get_roles(cls, *, order_by: str, limit: int, offset: int) -> RoleCollectionR roles=[RoleResponse.model_validate(r) for r in roles], total_entries=total_entries, ) + + @classmethod + def delete_role(cls, name: str) -> None: + security_manager = get_fab_auth_manager().security_manager + + existing = security_manager.find_role(name=name) + if not existing: + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail=f"Role with name {name!r} does not exist.", + ) + security_manager.delete_role(existing) diff --git a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/routes/test_roles.py b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/routes/test_roles.py index a2deb8bd9c74f..97d0f1345966a 100644 --- a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/routes/test_roles.py +++ b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/routes/test_roles.py @@ -21,6 +21,7 @@ from unittest.mock import MagicMock, patch import pytest +from fastapi import HTTPException, status from airflow.providers.fab.auth_manager.api_fastapi.datamodels.roles import ( RoleCollectionResponse, @@ -252,3 +253,82 @@ def test_get_roles_validation_422_negative_offset( resp = test_client.get("/fab/v1/roles", params={"offset": -1}) assert resp.status_code == 422 mock_roles.get_roles.assert_not_called() + + # DELETE /roles/{name} + + @patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles") + @patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager") + @patch( + "airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder", + return_value=_noop_cm(), + ) + def test_delete_role( + self, mock_get_application_builder, mock_get_auth_manager, mock_roles, test_client, as_user + ): + mgr = MagicMock() + mgr.is_authorized_custom_view.return_value = True + mock_roles.delete_role.return_value = None + mock_get_auth_manager.return_value = mgr + + with as_user(): + resp = test_client.delete("/fab/v1/roles/roleA") + assert resp.status_code == 200 + mock_roles.delete_role.assert_called_once_with(name="roleA") + + @patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles") + @patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager") + @patch( + "airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder", + return_value=_noop_cm(), + ) + def test_delete_role_forbidden( + self, mock_get_application_builder, mock_get_auth_manager, mock_roles, test_client, as_user + ): + mgr = MagicMock() + mgr.is_authorized_custom_view.return_value = False + mock_get_auth_manager.return_value = mgr + + with as_user(): + resp = test_client.delete("/fab/v1/roles/roleA") + assert resp.status_code == 403 + mock_roles.delete_role.assert_not_called() + + @patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles") + @patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager") + @patch( + "airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder", + return_value=_noop_cm(), + ) + def test_delete_role_validation_404_not_found( + self, mock_get_application_builder, mock_get_auth_manager, mock_roles, test_client, as_user + ): + mgr = MagicMock() + mgr.is_authorized_custom_view.return_value = True + mock_get_auth_manager.return_value = mgr + mock_roles.delete_role.side_effect = HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Role with name 'non_existent_role' does not exist.", + ) + + with as_user(): + resp = test_client.delete("/fab/v1/roles/non_existent_role") + assert resp.status_code == 404 + mock_roles.delete_role.assert_called_once_with(name="non_existent_role") + + @patch("airflow.providers.fab.auth_manager.api_fastapi.routes.roles.FABAuthManagerRoles") + @patch("airflow.providers.fab.auth_manager.api_fastapi.security.get_auth_manager") + @patch( + "airflow.providers.fab.auth_manager.api_fastapi.routes.roles.get_application_builder", + return_value=_noop_cm(), + ) + def test_delete_role_validation_404_empty_name( + self, mock_get_application_builder, mock_get_auth_manager, mock_roles, test_client, as_user + ): + mgr = MagicMock() + mgr.is_authorized_custom_view.return_value = True + mock_get_auth_manager.return_value = mgr + + with as_user(): + resp = test_client.delete("/fab/v1/roles/") + assert resp.status_code == 404 + mock_roles.delete_role.assert_not_called() diff --git a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py index cc105bb05d52b..c8adbc2c5b4b0 100644 --- a/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py +++ b/providers/fab/tests/unit/fab/auth_manager/api_fastapi/services/test_roles.py @@ -209,3 +209,23 @@ def test_get_roles_invalid_order_by_bubbles_400(self, build_ordering, get_fab_au with pytest.raises(HTTPException) as ex: FABAuthManagerRoles.get_roles(order_by="nope", limit=10, offset=0) assert ex.value.status_code == 400 + + # DELETE /roles/{name} + + def test_delete_role_success(self, get_fab_auth_manager, fab_auth_manager, security_manager): + security_manager.find_role.return_value = _make_role_obj("roleA", []) + fab_auth_manager.security_manager = security_manager + get_fab_auth_manager.return_value = fab_auth_manager + + FABAuthManagerRoles.delete_role(name="roleA") + + security_manager.delete_role.assert_called_once() + + def test_delete_role_not_found(self, get_fab_auth_manager, fab_auth_manager, security_manager): + security_manager.find_role.return_value = None + fab_auth_manager.security_manager = security_manager + get_fab_auth_manager.return_value = fab_auth_manager + + with pytest.raises(HTTPException) as ex: + FABAuthManagerRoles.delete_role(name="roleA") + assert ex.value.status_code == 404