diff --git a/providers/fab/docs/auth-manager/access-control.rst b/providers/fab/docs/auth-manager/access-control.rst index 52658bd6e80dc..d3ee73d76a40c 100644 --- a/providers/fab/docs/auth-manager/access-control.rst +++ b/providers/fab/docs/auth-manager/access-control.rst @@ -172,10 +172,10 @@ Endpoint /assets GET Assets.can_read Viewer /assets/{uri} GET Assets.can_read Viewer /assets/events GET Assets.can_read Viewer -/eventLogs GET Audit Logs.can_read Admin - All Audit Logs.can_read (for rows not tied to a Dag) -/eventLogs/{event_log_id} GET Audit Logs.can_read Admin - All Audit Logs.can_read (for rows not tied to a Dag) +/eventLogs GET Dags.can_read, Audit Logs.can_read (rows tied to a Dag) Viewer +/eventLogs GET All Audit Logs.can_read (rows not tied to a Dag) Admin +/eventLogs/{event_log_id} GET Dags.can_read, Audit Logs.can_read (rows tied to a Dag) Viewer +/eventLogs/{event_log_id} GET All Audit Logs.can_read (rows not tied to a Dag) Admin /importErrors GET ImportError.can_read Viewer /importErrors/{import_error_id} GET ImportError.can_read Viewer /health GET None Public diff --git a/providers/fab/docs/changelog.rst b/providers/fab/docs/changelog.rst index f9cc22aa12826..b2d0fec518cf5 100644 --- a/providers/fab/docs/changelog.rst +++ b/providers/fab/docs/changelog.rst @@ -20,6 +20,16 @@ Changelog --------- +.. warning:: + On Airflow 3.4.0 and newer the ``Audit Logs.can_read`` and ``Audit Logs.menu_access`` + permissions move from the ``Admin`` role to the ``Viewer`` role, so ``Viewer``, ``User`` and + ``Op`` now reach the audit log of the Dags they may read. The audit rows that are not tied to + a Dag -- Connection, Variable and Pool operations -- stay behind ``All Audit Logs.can_read``, + which only ``Admin`` holds. On an older Airflow those rows are not gated separately, so both + permissions stay with ``Admin`` there. Upgrading syncs the new permissions onto the built-in + roles; a deployment that wants the previous behaviour has to revoke them from the ``Viewer`` + role explicitly. + 3.8.1 ..... 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 2bb4d6ea53cdb..20c59938e2c03 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 @@ -73,6 +73,7 @@ from werkzeug.security import check_password_hash, generate_password_hash from airflow.providers.common.compat.sdk import conf +from airflow.providers.common.compat.security.access_view import AUDIT_LOGS_ALL_ACCESS_VIEW from airflow.providers.fab.auth_manager.models import ( Action, Group, @@ -158,6 +159,16 @@ def _iter_dags() -> Iterable[DAG | SerializedDAG]: # purging the old sessions by using `airflow db clean` command. MAX_NUM_DATABASE_USER_SESSIONS = 50000 +# Airflow 3.4.0 moved the audit rows that carry no Dag -- Connection, Variable and Pool +# operations -- behind ``AccessView.AUDIT_LOGS_ALL``, which leaves ``Audit Logs.can_read`` +# covering only the rows the event log endpoints already narrow to the caller's readable Dags. +# An older core returns those Dag-less rows to anyone holding ``Audit Logs.can_read``, so there +# the permission has to stay admin-only. +_DAG_AUDIT_LOG_PERMISSIONS = [ + (permissions.ACTION_CAN_READ, permissions.RESOURCE_AUDIT_LOG), + (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_AUDIT_LOG), +] + class FabException(Exception): """Custom exception for FAB security manager.""" @@ -269,6 +280,7 @@ class FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2): (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, permissions.RESOURCE_AUDIT_LOG), (permissions.ACTION_CAN_READ, permissions.RESOURCE_BACKFILL), (permissions.ACTION_CAN_READ, permissions.RESOURCE_CLUSTER_ACTIVITY), (permissions.ACTION_CAN_READ, permissions.RESOURCE_POOL), @@ -289,6 +301,7 @@ class FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2): (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_DAG_DEPENDENCIES), (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_DAG_RUN), (permissions.ACTION_CAN_ACCESS_MENU, RESOURCE_ASSET), + (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_AUDIT_LOG), (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_CLUSTER_ACTIVITY), (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_DOCS), (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_DOCS_MENU), @@ -352,8 +365,6 @@ class FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2): # [START security_admin_perms] ADMIN_PERMISSIONS = [ - (permissions.ACTION_CAN_READ, permissions.RESOURCE_AUDIT_LOG), - (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_AUDIT_LOG), (permissions.ACTION_CAN_READ, permissions.RESOURCE_AUDIT_LOG_ALL), (permissions.ACTION_CAN_READ, permissions.RESOURCE_IMPORT_ERROR_ALL), (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_RESCHEDULE), @@ -367,6 +378,12 @@ class FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2): ] # [END security_admin_perms] + if AUDIT_LOGS_ALL_ACCESS_VIEW is None: + VIEWER_PERMISSIONS = [ + permission for permission in VIEWER_PERMISSIONS if permission not in _DAG_AUDIT_LOG_PERMISSIONS + ] + ADMIN_PERMISSIONS = ADMIN_PERMISSIONS + _DAG_AUDIT_LOG_PERMISSIONS + ########################################################################### # DEFAULT ROLE CONFIGURATIONS ########################################################################### 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 f2520fbd88461..b93491629faea 100644 --- a/providers/fab/tests/unit/fab/auth_manager/test_security.py +++ b/providers/fab/tests/unit/fab/auth_manager/test_security.py @@ -47,9 +47,11 @@ class _TestBase(DeclarativeBase): Mapped = Any # type: ignore[assignment,misc] from airflow.api_fastapi.app import get_auth_manager +from airflow.api_fastapi.auth.managers.models.resource_details import DagAccessEntity, DagDetails from airflow.models import DagModel from airflow.models.dag import DAG from airflow.models.dagbundle import DagBundleModel +from airflow.providers.common.compat.security.access_view import AUDIT_LOGS_ALL_ACCESS_VIEW from airflow.providers.common.compat.sqlalchemy.orm import mapped_column from airflow.providers.fab.auth_manager.fab_auth_manager import FabAuthManager from airflow.providers.fab.auth_manager.models.anonymous_user import AnonymousUser @@ -512,6 +514,11 @@ def test_get_user_roles_for_anonymous_user(app, security_manager): (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_DOCS_MENU), (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_DOCS), } + if AUDIT_LOGS_ALL_ACCESS_VIEW is not None: + viewer_role_perms |= { + (permissions.ACTION_CAN_READ, permissions.RESOURCE_AUDIT_LOG), + (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_AUDIT_LOG), + } app.config["AUTH_ROLE_PUBLIC"] = "Viewer" with app.app_context(): @@ -523,6 +530,30 @@ def test_get_user_roles_for_anonymous_user(app, security_manager): assert perms_views == viewer_role_perms +@pytest.mark.skipif( + AUDIT_LOGS_ALL_ACCESS_VIEW is None, + reason="Viewers are granted audit log access only on a core that gates the Dag-less rows separately", +) +@pytest.mark.parametrize( + ("role_name", "expected_all_audit_logs"), + [("Viewer", False), ("User", False), ("Op", False), ("Admin", True)], +) +def test_default_roles_read_dag_audit_logs(app, role_name, expected_all_audit_logs): + """Every default role reads the audit rows of a Dag it can see; only Admin reads the rows with no Dag.""" + with app.app_context(): + with create_user_scope(app, username="audit_log_user", role_name=role_name) as user: + assert get_auth_manager().is_authorized_dag( + method="GET", + access_entity=DagAccessEntity.AUDIT_LOG, + details=DagDetails(id="example_dag"), + user=user, + ) + assert ( + get_auth_manager().is_authorized_view(access_view=AUDIT_LOGS_ALL_ACCESS_VIEW, user=user) + is expected_all_audit_logs + ) + + def test_get_current_user_permissions(app): action = "can_some_action" resource = "SomeBaseView"