From d4c44e06b686d4f5e2267b8d1cbefeeb4a71614a Mon Sep 17 00:00:00 2001 From: vatsrahul1001 Date: Mon, 10 Mar 2025 08:24:41 +0530 Subject: [PATCH 1/3] add auth to dag versions endpoints --- .../core_api/openapi/v1-generated.yaml | 12 +++++++++-- .../core_api/routes/public/dag_versions.py | 6 +++++- airflow/ui/openapi-gen/requests/types.gen.ts | 4 ++-- .../routes/public/test_dag_versions.py | 20 +++++++++++++++++++ 4 files changed, 37 insertions(+), 5 deletions(-) diff --git a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml index fb2e3c79d45fe..55e4754354eda 100644 --- a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml +++ b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml @@ -7036,12 +7036,16 @@ paths: summary: Get Dag Version description: Get one Dag Version. operationId: get_dag_version + security: + - OAuth2PasswordBearer: [] parameters: - name: dag_id in: path required: true schema: - type: string + anyOf: + - type: string + - type: 'null' title: Dag Id - name: version_number in: path @@ -7091,12 +7095,16 @@ paths: This endpoint allows specifying `~` as the dag_id to retrieve DAG Versions for all DAGs.' operationId: get_dag_versions + security: + - OAuth2PasswordBearer: [] parameters: - name: dag_id in: path required: true schema: - type: string + anyOf: + - type: string + - type: 'null' title: Dag Id - name: limit in: query diff --git a/airflow/api_fastapi/core_api/routes/public/dag_versions.py b/airflow/api_fastapi/core_api/routes/public/dag_versions.py index 3e39374d40eb2..bb0e0037394ed 100644 --- a/airflow/api_fastapi/core_api/routes/public/dag_versions.py +++ b/airflow/api_fastapi/core_api/routes/public/dag_versions.py @@ -21,6 +21,7 @@ from fastapi import Depends, HTTPException, Request, status from sqlalchemy import select +from airflow.api_fastapi.auth.managers.models.resource_details import DagAccessEntity from airflow.api_fastapi.common.db.common import SessionDep, paginated_select from airflow.api_fastapi.common.parameters import ( FilterParam, @@ -35,6 +36,7 @@ DagVersionResponse, ) from airflow.api_fastapi.core_api.openapi.exceptions import create_openapi_http_exception_doc +from airflow.api_fastapi.core_api.security import requires_access_dag from airflow.models.dag import DAG from airflow.models.dag_version import DagVersion @@ -48,6 +50,7 @@ status.HTTP_404_NOT_FOUND, ] ), + dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.CODE))], ) def get_dag_version( dag_id: str, @@ -71,8 +74,9 @@ def get_dag_version( responses=create_openapi_http_exception_doc( [ status.HTTP_404_NOT_FOUND, - ] + ], ), + dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.CODE))], ) def get_dag_versions( dag_id: str, diff --git a/airflow/ui/openapi-gen/requests/types.gen.ts b/airflow/ui/openapi-gen/requests/types.gen.ts index ce1b31a13eabf..ea2eeedf19d74 100644 --- a/airflow/ui/openapi-gen/requests/types.gen.ts +++ b/airflow/ui/openapi-gen/requests/types.gen.ts @@ -2547,7 +2547,7 @@ export type ReparseDagFileData = { export type ReparseDagFileResponse = null; export type GetDagVersionData = { - dagId: string; + dagId: string | null; versionNumber: number; }; @@ -2556,7 +2556,7 @@ export type GetDagVersionResponse = DagVersionResponse; export type GetDagVersionsData = { bundleName?: string; bundleVersion?: string | null; - dagId: string; + dagId: string | null; limit?: number; offset?: number; orderBy?: string; diff --git a/tests/api_fastapi/core_api/routes/public/test_dag_versions.py b/tests/api_fastapi/core_api/routes/public/test_dag_versions.py index 7398eb3c876f7..bb0c0a3898ba2 100644 --- a/tests/api_fastapi/core_api/routes/public/test_dag_versions.py +++ b/tests/api_fastapi/core_api/routes/public/test_dag_versions.py @@ -117,6 +117,18 @@ def test_get_dag_version_404(self, test_client): "detail": "The DagVersion with dag_id: `dag_with_multiple_versions` and version_number: `99` was not found", } + def test_should_respond_401(self, unauthenticated_test_client): + response = unauthenticated_test_client.get( + "/public/dags/dag_with_multiple_versions/dagVersions/99", params={} + ) + assert response.status_code == 401 + + def test_should_respond_403(self, unauthorized_test_client): + response = unauthorized_test_client.get( + "/public/dags/dag_with_multiple_versions/dagVersions/99", params={} + ) + assert response.status_code == 403 + class TestGetDagVersions(TestDagVersionEndpoint): @pytest.mark.parametrize( @@ -298,3 +310,11 @@ def test_get_dag_versions_should_return_404_for_missing_dag(self, test_client): assert response.json() == { "detail": "The DAG with dag_id: `MISSING_ID` was not found", } + + def test_should_respond_401(self, unauthenticated_test_client): + response = unauthenticated_test_client.get("/public/dags/~/dagVersions", params={}) + assert response.status_code == 401 + + def test_should_respond_403(self, unauthorized_test_client): + response = unauthorized_test_client.get("/public/dags/~/dagVersions", params={}) + assert response.status_code == 403 From 610c4b46f4409454f0554f25c16851519f6a403d Mon Sep 17 00:00:00 2001 From: vatsrahul1001 Date: Wed, 12 Mar 2025 20:09:15 +0530 Subject: [PATCH 2/3] adding new DagAccessEntity for DAG Version --- airflow/api_fastapi/auth/managers/models/resource_details.py | 1 + airflow/api_fastapi/core_api/routes/public/dag_versions.py | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/airflow/api_fastapi/auth/managers/models/resource_details.py b/airflow/api_fastapi/auth/managers/models/resource_details.py index 6d93acd772c31..2341daf625da8 100644 --- a/airflow/api_fastapi/auth/managers/models/resource_details.py +++ b/airflow/api_fastapi/auth/managers/models/resource_details.py @@ -104,3 +104,4 @@ class DagAccessEntity(Enum): TASK_LOGS = "TASK_LOGS" WARNING = "WARNING" XCOM = "XCOM" + DAG_VERSION = "DAG_VERSION" diff --git a/airflow/api_fastapi/core_api/routes/public/dag_versions.py b/airflow/api_fastapi/core_api/routes/public/dag_versions.py index bb0e0037394ed..393f62decb435 100644 --- a/airflow/api_fastapi/core_api/routes/public/dag_versions.py +++ b/airflow/api_fastapi/core_api/routes/public/dag_versions.py @@ -50,7 +50,7 @@ status.HTTP_404_NOT_FOUND, ] ), - dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.CODE))], + dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.DAG_VERSION))], ) def get_dag_version( dag_id: str, @@ -76,7 +76,7 @@ def get_dag_version( status.HTTP_404_NOT_FOUND, ], ), - dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.CODE))], + dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.DAG_VERSION))], ) def get_dag_versions( dag_id: str, From aaebc9d86a4754664744a2030bfbbb394a93c271 Mon Sep 17 00:00:00 2001 From: Wei Lee Date: Thu, 13 Mar 2025 18:56:45 +0800 Subject: [PATCH 3/3] feat(security): add DagAccessEntity.VERSION --- .../auth/managers/models/resource_details.py | 4 +-- .../core_api/openapi/v1-generated.yaml | 8 ++--- .../core_api/routes/public/dag_versions.py | 4 +-- airflow/assets/manager.py | 2 +- airflow/security/permissions.py | 1 + airflow/ui/openapi-gen/requests/types.gen.ts | 4 +-- .../aws/auth_manager/test_aws_auth_manager.py | 29 ++++++++++++++----- .../common/compat/security/permissions.py | 16 ++++++++-- .../fab/auth_manager/fab_auth_manager.py | 2 ++ .../auth_manager/security_manager/override.py | 3 +- .../providers/fab/www/security/permissions.py | 1 + .../unit/fab/auth_manager/test_security.py | 3 +- 12 files changed, 52 insertions(+), 25 deletions(-) diff --git a/airflow/api_fastapi/auth/managers/models/resource_details.py b/airflow/api_fastapi/auth/managers/models/resource_details.py index 2341daf625da8..7aefccab703bb 100644 --- a/airflow/api_fastapi/auth/managers/models/resource_details.py +++ b/airflow/api_fastapi/auth/managers/models/resource_details.py @@ -100,8 +100,8 @@ class DagAccessEntity(Enum): SLA_MISS = "SLA_MISS" TASK = "TASK" TASK_INSTANCE = "TASK_INSTANCE" - TASK_RESCHEDULE = "TASK_RESCHEDULE" TASK_LOGS = "TASK_LOGS" + TASK_RESCHEDULE = "TASK_RESCHEDULE" + VERSION = "VERSION" WARNING = "WARNING" XCOM = "XCOM" - DAG_VERSION = "DAG_VERSION" diff --git a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml index 55e4754354eda..b81da9d47a6a1 100644 --- a/airflow/api_fastapi/core_api/openapi/v1-generated.yaml +++ b/airflow/api_fastapi/core_api/openapi/v1-generated.yaml @@ -7043,9 +7043,7 @@ paths: in: path required: true schema: - anyOf: - - type: string - - type: 'null' + type: string title: Dag Id - name: version_number in: path @@ -7102,9 +7100,7 @@ paths: in: path required: true schema: - anyOf: - - type: string - - type: 'null' + type: string title: Dag Id - name: limit in: query diff --git a/airflow/api_fastapi/core_api/routes/public/dag_versions.py b/airflow/api_fastapi/core_api/routes/public/dag_versions.py index 393f62decb435..b41f203a7df88 100644 --- a/airflow/api_fastapi/core_api/routes/public/dag_versions.py +++ b/airflow/api_fastapi/core_api/routes/public/dag_versions.py @@ -50,7 +50,7 @@ status.HTTP_404_NOT_FOUND, ] ), - dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.DAG_VERSION))], + dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.VERSION))], ) def get_dag_version( dag_id: str, @@ -76,7 +76,7 @@ def get_dag_version( status.HTTP_404_NOT_FOUND, ], ), - dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.DAG_VERSION))], + dependencies=[Depends(requires_access_dag(method="GET", access_entity=DagAccessEntity.VERSION))], ) def get_dag_versions( dag_id: str, diff --git a/airflow/assets/manager.py b/airflow/assets/manager.py index ca6cb345dd4e9..4e5dd37a66b8d 100644 --- a/airflow/assets/manager.py +++ b/airflow/assets/manager.py @@ -17,7 +17,7 @@ # under the License. from __future__ import annotations -from collections.abc import Collection, Iterable +from collections.abc import Collection from typing import TYPE_CHECKING from sqlalchemy import exc, or_, select diff --git a/airflow/security/permissions.py b/airflow/security/permissions.py index ee0454b85fdc9..647bcf0b0c6e3 100644 --- a/airflow/security/permissions.py +++ b/airflow/security/permissions.py @@ -32,6 +32,7 @@ RESOURCE_DAG_PREFIX = "DAG:" RESOURCE_DAG_RUN = "DAG Runs" RESOURCE_DAG_RUN_PREFIX = "DAG Run:" +RESOURCE_DAG_VERSION = "DAG Versions" RESOURCE_DAG_WARNING = "DAG Warnings" RESOURCE_CLUSTER_ACTIVITY = "Cluster Activity" RESOURCE_ASSET = "Assets" diff --git a/airflow/ui/openapi-gen/requests/types.gen.ts b/airflow/ui/openapi-gen/requests/types.gen.ts index ea2eeedf19d74..ce1b31a13eabf 100644 --- a/airflow/ui/openapi-gen/requests/types.gen.ts +++ b/airflow/ui/openapi-gen/requests/types.gen.ts @@ -2547,7 +2547,7 @@ export type ReparseDagFileData = { export type ReparseDagFileResponse = null; export type GetDagVersionData = { - dagId: string | null; + dagId: string; versionNumber: number; }; @@ -2556,7 +2556,7 @@ export type GetDagVersionResponse = DagVersionResponse; export type GetDagVersionsData = { bundleName?: string; bundleVersion?: string | null; - dagId: string | null; + dagId: string; limit?: number; offset?: number; orderBy?: string; diff --git a/providers/amazon/tests/unit/amazon/aws/auth_manager/test_aws_auth_manager.py b/providers/amazon/tests/unit/amazon/aws/auth_manager/test_aws_auth_manager.py index 2f3b1e6df74e6..861b7a3d3df43 100644 --- a/providers/amazon/tests/unit/amazon/aws/auth_manager/test_aws_auth_manager.py +++ b/providers/amazon/tests/unit/amazon/aws/auth_manager/test_aws_auth_manager.py @@ -412,7 +412,10 @@ def test_batch_is_authorized_dag( requests=[ {"method": "GET"}, {"method": "GET", "details": DagDetails(id="dag_1")}, - {"method": "GET", "details": DagDetails(id="dag_1"), "access_entity": DagAccessEntity.CODE}, + ] + + [ + {"method": "GET", "details": DagDetails(id="dag_1"), "access_entity": dag_access_entity} + for dag_access_entity in DagAccessEntity ], user=mock, ) @@ -431,16 +434,28 @@ def test_batch_is_authorized_dag( "entity_id": "dag_1", "context": None, }, + ] + + [ { "method": "GET", "entity_type": AvpEntities.DAG, "entity_id": "dag_1", - "context": { - "dag_entity": { - "string": DagAccessEntity.CODE.value, - }, - }, - }, + "context": {"dag_entity": {"string": dag_entity}}, + } + for dag_entity in ( + DagAccessEntity.AUDIT_LOG.value, + DagAccessEntity.CODE.value, + DagAccessEntity.DEPENDENCIES.value, + DagAccessEntity.RUN.value, + DagAccessEntity.SLA_MISS.value, + DagAccessEntity.TASK.value, + DagAccessEntity.TASK_INSTANCE.value, + DagAccessEntity.TASK_LOGS.value, + DagAccessEntity.TASK_RESCHEDULE.value, + DagAccessEntity.VERSION.value, + DagAccessEntity.WARNING.value, + DagAccessEntity.XCOM.value, + ) ], user=ANY, ) diff --git a/providers/common/compat/src/airflow/providers/common/compat/security/permissions.py b/providers/common/compat/src/airflow/providers/common/compat/security/permissions.py index 200cd24ab44ed..1a2d774510e93 100644 --- a/providers/common/compat/src/airflow/providers/common/compat/security/permissions.py +++ b/providers/common/compat/src/airflow/providers/common/compat/security/permissions.py @@ -19,12 +19,22 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from airflow.security.permissions import RESOURCE_ASSET, RESOURCE_ASSET_ALIAS, RESOURCE_BACKFILL + from airflow.security.permissions import ( + RESOURCE_ASSET, + RESOURCE_ASSET_ALIAS, + RESOURCE_BACKFILL, + RESOURCE_DAG_VERSION, + ) else: try: - from airflow.security.permissions import RESOURCE_ASSET, RESOURCE_ASSET_ALIAS, RESOURCE_BACKFILL + from airflow.security.permissions import ( + RESOURCE_ASSET, + RESOURCE_ASSET_ALIAS, + RESOURCE_BACKFILL, + RESOURCE_DAG_VERSION, + ) except ImportError: from airflow.security.permissions import RESOURCE_DATASET as RESOURCE_ASSET -__all__ = ["RESOURCE_ASSET", "RESOURCE_ASSET_ALIAS", "RESOURCE_BACKFILL"] +__all__ = ["RESOURCE_ASSET", "RESOURCE_ASSET_ALIAS", "RESOURCE_BACKFILL", "RESOURCE_DAG_VERSION"] 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 a793a80d0e637..67915837e4ae6 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 @@ -74,6 +74,7 @@ RESOURCE_DAG_CODE, RESOURCE_DAG_DEPENDENCIES, RESOURCE_DAG_RUN, + RESOURCE_DAG_VERSION, RESOURCE_DAG_WARNING, RESOURCE_DOCS, RESOURCE_IMPORT_ERROR, @@ -129,6 +130,7 @@ DagAccessEntity.TASK_INSTANCE: (RESOURCE_DAG_RUN, RESOURCE_TASK_INSTANCE), DagAccessEntity.TASK_LOGS: (RESOURCE_TASK_LOG,), DagAccessEntity.TASK_RESCHEDULE: (RESOURCE_TASK_RESCHEDULE,), + DagAccessEntity.VERSION: (RESOURCE_DAG_VERSION,), DagAccessEntity.WARNING: (RESOURCE_DAG_WARNING,), DagAccessEntity.XCOM: (RESOURCE_XCOM,), } diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py b/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py index 97eca2a858077..c3f00bd0f9e20 100644 --- a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py +++ b/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py @@ -246,13 +246,14 @@ class FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2): (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_DEPENDENCIES), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_CODE), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_VERSION), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_WARNING), (permissions.ACTION_CAN_READ, RESOURCE_ASSET), (permissions.ACTION_CAN_READ, RESOURCE_ASSET_ALIAS), (permissions.ACTION_CAN_READ, RESOURCE_BACKFILL), (permissions.ACTION_CAN_READ, permissions.RESOURCE_CLUSTER_ACTIVITY), (permissions.ACTION_CAN_READ, permissions.RESOURCE_POOL), (permissions.ACTION_CAN_READ, permissions.RESOURCE_IMPORT_ERROR), - (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_WARNING), (permissions.ACTION_CAN_READ, permissions.RESOURCE_JOB), (permissions.ACTION_CAN_READ, permissions.RESOURCE_MY_PASSWORD), (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_MY_PASSWORD), diff --git a/providers/fab/src/airflow/providers/fab/www/security/permissions.py b/providers/fab/src/airflow/providers/fab/www/security/permissions.py index ee0454b85fdc9..647bcf0b0c6e3 100644 --- a/providers/fab/src/airflow/providers/fab/www/security/permissions.py +++ b/providers/fab/src/airflow/providers/fab/www/security/permissions.py @@ -32,6 +32,7 @@ RESOURCE_DAG_PREFIX = "DAG:" RESOURCE_DAG_RUN = "DAG Runs" RESOURCE_DAG_RUN_PREFIX = "DAG Run:" +RESOURCE_DAG_VERSION = "DAG Versions" RESOURCE_DAG_WARNING = "DAG Warnings" RESOURCE_CLUSTER_ACTIVITY = "Cluster Activity" RESOURCE_ASSET = "Assets" diff --git a/providers/fab/tests/unit/fab/auth_manager/test_security.py b/providers/fab/tests/unit/fab/auth_manager/test_security.py index 874a748a8f772..2b27e1098d2a1 100644 --- a/providers/fab/tests/unit/fab/auth_manager/test_security.py +++ b/providers/fab/tests/unit/fab/auth_manager/test_security.py @@ -426,12 +426,13 @@ def test_get_user_roles_for_anonymous_user(app, security_manager): (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_DEPENDENCIES), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_CODE), (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_RUN), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_VERSION), + (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_WARNING), (permissions.ACTION_CAN_READ, RESOURCE_ASSET), (permissions.ACTION_CAN_READ, RESOURCE_ASSET_ALIAS), (permissions.ACTION_CAN_READ, RESOURCE_BACKFILL), (permissions.ACTION_CAN_READ, permissions.RESOURCE_CLUSTER_ACTIVITY), (permissions.ACTION_CAN_READ, permissions.RESOURCE_IMPORT_ERROR), - (permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG_WARNING), (permissions.ACTION_CAN_READ, permissions.RESOURCE_JOB), (permissions.ACTION_CAN_READ, permissions.RESOURCE_POOL), (permissions.ACTION_CAN_READ, permissions.RESOURCE_SLA_MISS),