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 18d0513892bec..e8569383ab7c7 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 @@ -568,8 +568,8 @@ def _is_authorized_dag( # Check whether the user has permissions to access a specific DAG resource_dag_name = self._resource_name(details.id, RESOURCE_DAG) return self._is_authorized(method=method, resource_type=resource_dag_name, user=user) - - return False + authorized_dags = self.get_authorized_dag_ids(user=user, method=method) + return len(authorized_dags) > 0 def _is_authorized_dag_run( self, @@ -594,8 +594,8 @@ def _is_authorized_dag_run( # Check whether the user has permissions to access a specific DAG Run permission on a DAG Level resource_dag_name = self._resource_name(details.id, RESOURCE_DAG_RUN) return self._is_authorized(method=method, resource_type=resource_dag_name, user=user) - - return False + authorized_dags = self.get_authorized_dag_ids(user=user, method=method) + return len(authorized_dags) > 0 @staticmethod def _get_fab_action(method: ResourceMethod) -> str: 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 5e517be3efe30..b4d99b6928409 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 @@ -279,6 +279,62 @@ def test_is_authorized(self, api_name, method, user_permissions, expected_result [(ACTION_CAN_READ, "resource_test")], False, ), + # With specific DAG permissions but no specific DAG requested + ( + "GET", + None, + None, + [(ACTION_CAN_READ, "DAG:test_dag_id")], + True, + ), + # With multiple specific DAG permissions, no specific DAG requested + ( + "GET", + None, + None, + [(ACTION_CAN_READ, "DAG:test_dag_id"), (ACTION_CAN_READ, "DAG:test_dag_id2")], + True, + ), + # With specific DAG permissions but wrong method + ( + "POST", + None, + None, + [(ACTION_CAN_READ, "DAG:test_dag_id")], + True, + ), + # With correct method permissions for specific DAG + ( + "POST", + None, + None, + [(ACTION_CAN_CREATE, "DAG:test_dag_id")], + True, + ), + # With EDIT permission on specific DAG, no specific DAG requested + ( + "PUT", + None, + None, + [(ACTION_CAN_EDIT, "DAG:test_dag_id")], + True, + ), + # With DELETE permission on specific DAG, no specific DAG requested + ( + "DELETE", + None, + None, + [(ACTION_CAN_DELETE, "DAG:test_dag_id")], + True, + ), + # Mixed permissions - some DAG, some non-DAG + ( + "GET", + None, + None, + [(ACTION_CAN_READ, "DAG:test_dag_id"), (ACTION_CAN_READ, "resource_test")], + True, + ), # Scenario 2 # # With global permissions on DAGs ( @@ -348,10 +404,52 @@ def test_is_authorized(self, api_name, method, user_permissions, expected_result [(ACTION_CAN_READ, RESOURCE_TASK_INSTANCE)], False, ), + # DAG sub-entity with specific DAG permissions but no specific DAG requested + ( + "GET", + DagAccessEntity.RUN, + None, + [(ACTION_CAN_READ, "DAG:test_dag_id"), (ACTION_CAN_READ, RESOURCE_DAG_RUN)], + True, + ), + # DAG sub-entity access with no DAG permissions, no specific DAG requested + ( + "GET", + DagAccessEntity.RUN, + None, + [(ACTION_CAN_READ, RESOURCE_DAG_RUN)], + True, + ), + # DAG sub-entity with specific DAG permissions but missing sub-entity permission + ( + "GET", + DagAccessEntity.TASK_INSTANCE, + None, + [(ACTION_CAN_READ, "DAG:test_dag_id")], + False, + ), + # Multiple DAG access entities with proper permissions + ( + "DELETE", + DagAccessEntity.TASK, + None, + [(ACTION_CAN_EDIT, "DAG:test_dag_id"), (ACTION_CAN_DELETE, RESOURCE_TASK_INSTANCE)], + True, + ), + # User with specific DAG permissions but wrong method for sub-entity + ( + "POST", + DagAccessEntity.RUN, + None, + [(ACTION_CAN_READ, "DAG:test_dag_id"), (ACTION_CAN_READ, RESOURCE_DAG_RUN)], + True, + ), ], ) + @mock.patch.object(FabAuthManager, "get_authorized_dag_ids") def test_is_authorized_dag( self, + mock_get_authorized_dag_ids, method, dag_access_entity, dag_details, @@ -359,8 +457,13 @@ def test_is_authorized_dag( expected_result, auth_manager_with_appbuilder, ): + dag_permissions = [perm[1] for perm in user_permissions if perm[1].startswith("DAG:")] + dag_ids = {perm.replace("DAG:", "") for perm in dag_permissions} + mock_get_authorized_dag_ids.return_value = dag_ids + user = Mock() user.perms = user_permissions + user.id = 1 result = auth_manager_with_appbuilder.is_authorized_dag( method=method, access_entity=dag_access_entity, details=dag_details, user=user )