This is an automated email from the ASF dual-hosted git repository.
potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new dfaff482594 Gate audit log rows not tied to a Dag on a dedicated
AccessView (#70759)
dfaff482594 is described below
commit dfaff4825944d8dce1b5c40e83f28706a2354984
Author: Jarek Potiuk <[email protected]>
AuthorDate: Sat Aug 1 02:47:09 2026 +0200
Gate audit log rows not tied to a Dag on a dedicated AccessView (#70759)
* Gate audit log rows not tied to a Dag on a dedicated AccessView
`PermittedEventLogFilter` returned every `dag_id IS NULL` row
unconditionally,
and the detail endpoint's guard resolved such a row to `dag_id=None` and
then
authorized it as Dag-level `DagAccessEntity.AUDIT_LOG` access. Those rows
record
Connection, Variable and Pool operations, so they carry no per-Dag key to
authorize on -- and Dag-level audit access is held by any viewer under the
default auth manager, which is not the audience for them.
Add `AccessView.AUDIT_LOGS_ALL` and gate both read paths on it, mirroring
the
`IMPORT_ERRORS_ALL` view that covers the same shape of problem for import
errors
of files with no registered Dag. The list endpoint filters the rows out in
the
query, so they stay out of `total_entries` and pagination too; the detail
endpoint answers 403. `SimpleAuthManager` grants the view to admins only,
and the
FAB auth manager maps it to a new `All Audit Logs` resource that lands in
`ADMIN_PERMISSIONS`.
The detail guard now selects `Log.id` alongside `Log.dag_id`: a missing row
and a
NULL `dag_id` both read back as `None` from a bare scalar, and they
authorize
differently. A missing row keeps the Dag-level check so the route can still
answer 404 rather than turning an unknown id into a permission error.
`authorize_view` also stops propagating a `KeyError` from an auth manager
that
cannot map an `AccessView`. `AccessView` members are added by core, but auth
managers ship as separately released providers, so a core newer than the
installed manager can name a view the manager has never heard of -- the FAB
manager translates the enum through a lookup table and would have raised,
which
surfaces as a 500 on the endpoint. It now denies and warns instead, which
keeps
the endpoint working and fails closed.
Generated-by: Claude Opus 5 (1M context) following the guidelines at
https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions
* Add newsfragment for the audit log view gate
* Update event log guard tests for the row lookup shape
The guard now selects Log.id alongside Log.dag_id so a missing row and a
NULL
dag_id can be told apart. These tests mocked session.scalar, so with a Mock
session the new call returned a truthy Mock whose dag_id was itself a Mock.
Mock the execute(...).one_or_none() shape instead, and add the two cases the
unit tests did not cover: a row that exists with a NULL dag_id authorizes on
AUDIT_LOGS_ALL rather than the Dag check, and is Forbidden without it.
---
airflow-core/newsfragments/70759.significant.rst | 24 ++++++
.../api_fastapi/auth/managers/base_auth_manager.py | 48 +++++++++---
.../auth/managers/models/resource_details.py | 4 +
.../auth/managers/simple/simple_auth_manager.py | 7 +-
.../src/airflow/api_fastapi/core_api/security.py | 64 ++++++++++++++--
.../managers/simple/test_simple_auth_manager.py | 27 +++++++
.../auth/managers/test_base_auth_manager.py | 20 +++++
.../core_api/routes/public/test_event_logs.py | 89 +++++++++++++++++++++-
.../unit/api_fastapi/core_api/test_security.py | 57 +++++++++++++-
.../common/compat/security/access_view.py | 10 ++-
.../common/compat/security/test_access_view.py | 28 ++++---
providers/fab/docs/auth-manager/access-control.rst | 6 +-
.../providers/fab/auth_manager/fab_auth_manager.py | 13 +++-
.../fab/auth_manager/security_manager/override.py | 1 +
.../providers/fab/www/security/permissions.py | 1 +
15 files changed, 354 insertions(+), 45 deletions(-)
diff --git a/airflow-core/newsfragments/70759.significant.rst
b/airflow-core/newsfragments/70759.significant.rst
new file mode 100644
index 00000000000..3b3c864d53b
--- /dev/null
+++ b/airflow-core/newsfragments/70759.significant.rst
@@ -0,0 +1,24 @@
+Audit log rows not tied to a Dag now require the ``AUDIT_LOGS_ALL`` view
+
+Rows in the audit log whose ``dag_id`` is ``NULL`` record operations that are
not tied to a
+Dag, such as Connection, Variable and Pool changes. They were returned to any
caller that
+could read event logs at all, because there is no per-Dag key to authorize
them on and both
+read paths fell back to a Dag-level audit log check.
+
+They are now gated on a dedicated ``AccessView.AUDIT_LOGS_ALL``, in the same
way
+``AccessView.IMPORT_ERRORS_ALL`` gates import errors for files with no
registered Dag. Rows
+bound to a Dag keep their existing per-Dag check.
+
+**Behaviour changes:**
+
+- ``GET /eventLogs`` no longer returns rows with a ``NULL`` ``dag_id`` to
callers without the
+ new view, and excludes them from ``total_entries`` and pagination.
+ ``GET /eventLogs/{event_log_id}`` returns ``403`` for such a row. An id
matching no row
+ still returns ``404``.
+- With the simple auth manager the view is granted to the ``ADMIN`` role only.
With the FAB
+ auth manager it maps to a new ``All Audit Logs`` resource, which ships in
the ``Admin``
+ role; custom roles that need to read these rows must be granted
+ ``All Audit Logs.can_read`` explicitly.
+- An auth manager that cannot map a requested ``AccessView`` now denies that
view and emits
+ a ``UserWarning`` instead of raising ``KeyError``. This affects auth
managers older than
+ the core they run against, which previously failed the request with a server
error.
diff --git
a/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py
b/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py
index e0d56258148..f780097cea4 100644
--- a/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py
+++ b/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py
@@ -383,18 +383,48 @@ class BaseAuthManager(Generic[T], LoggingMixin,
metaclass=ABCMeta):
Core calls this instead of :meth:`is_authorized_view` on team-scoped
paths: an
override still on the old ``(access_view, user)`` signature would
otherwise raise
``TypeError``. Removed in Airflow 4.
+
+ A manager that does not recognise ``access_view`` at all is also
tolerated, and
+ denied -- see :meth:`_authorize_view_unmapped_denied`.
+ """
+ try:
+ if self._is_authorized_view_team_aware:
+ return self.is_authorized_view(access_view=access_view,
user=user, team_name=team_name)
+ warnings.warn(
+ f"The '{type(self).__name__}' auth manager is not team-aware,
so team-scoped views are "
+ "authorized across all teams and may be visible to users of
other teams. Add the "
+ "'team_name' argument to its is_authorized_view (or upgrade
the provider). Airflow 4 "
+ "will require team-aware auth managers.",
+ RemovedInAirflow4Warning,
+ stacklevel=2,
+ )
+ return self.is_authorized_view(access_view=access_view, user=user)
+ except KeyError:
+ return self._authorize_view_unmapped_denied(access_view)
+
+ def _authorize_view_unmapped_denied(self, access_view: AccessView) -> bool:
+ """
+ Deny an ``AccessView`` the auth manager cannot map, instead of raising.
+
+ ``AccessView`` members are added by core, but auth managers ship as
separately
+ released providers, so a core newer than the installed auth manager
can name a
+ view the manager has never heard of. Managers that translate the enum
through a
+ lookup table (the FAB auth manager, for one) raise ``KeyError`` on
such a member,
+ which would surface as a 500 on the endpoint that consults it.
+
+ Denying keeps the endpoint working and fails closed: an unmappable
view means the
+ manager cannot express who may see the records, and these views gate
records with
+ no other authorization key. Upgrading the auth manager provider to a
version that
+ maps the view restores access.
"""
- if self._is_authorized_view_team_aware:
- return self.is_authorized_view(access_view=access_view, user=user,
team_name=team_name)
warnings.warn(
- f"The '{type(self).__name__}' auth manager is not team-aware, so
team-scoped views are "
- "authorized across all teams and may be visible to users of other
teams. Add the "
- "'team_name' argument to its is_authorized_view (or upgrade the
provider). Airflow 4 "
- "will require team-aware auth managers.",
- RemovedInAirflow4Warning,
- stacklevel=2,
+ f"The '{type(self).__name__}' auth manager cannot map the
'{access_view.name}' view, so "
+ "access to it is denied. This usually means the auth manager
provider is older than "
+ "Airflow core; upgrade it to a version that supports this view.",
+ UserWarning,
+ stacklevel=3,
)
- return self.is_authorized_view(access_view=access_view, user=user)
+ return False
@abstractmethod
def is_authorized_custom_view(self, *, method: ResourceMethod,
resource_name: str, user: T) -> bool:
diff --git
a/airflow-core/src/airflow/api_fastapi/auth/managers/models/resource_details.py
b/airflow-core/src/airflow/api_fastapi/auth/managers/models/resource_details.py
index e4222603058..89319b9237b 100644
---
a/airflow-core/src/airflow/api_fastapi/auth/managers/models/resource_details.py
+++
b/airflow-core/src/airflow/api_fastapi/auth/managers/models/resource_details.py
@@ -98,6 +98,10 @@ class VariableDetails:
class AccessView(Enum):
"""Enum of specific views the user tries to access."""
+ # Audit log rows not tied to a Dag -- Connection, Variable, Pool, …
operations:
+ # there is no per-Dag key to authorize on, so they get their own
admin-by-default
+ # view rather than riding on Dag-level ``DagAccessEntity.AUDIT_LOG``
access.
+ AUDIT_LOGS_ALL = "AUDIT_LOGS_ALL"
CLUSTER_ACTIVITY = "CLUSTER_ACTIVITY"
DOCS = "DOCS"
IMPORT_ERRORS = "IMPORT_ERRORS"
diff --git
a/airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py
b/airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py
index ae8aafdb037..a9858081cb5 100644
---
a/airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py
+++
b/airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py
@@ -354,11 +354,12 @@ class
SimpleAuthManager(BaseAuthManager[SimpleAuthManagerUser]):
def is_authorized_view(
self, *, access_view: AccessView, user: SimpleAuthManagerUser,
team_name: str | None = None
) -> bool:
- # Import errors for files with no registered Dag are admin-only; every
other
- # view stays readable by viewers.
+ # Views covering records that have no per-Dag key to authorize on are
admin-only --
+ # import errors for files with no registered Dag, and audit log rows
not tied to a
+ # Dag. Every other view stays readable by viewers.
allow_role = (
SimpleAuthManagerRole.ADMIN
- if access_view == AccessView.IMPORT_ERRORS_ALL
+ if access_view in (AccessView.IMPORT_ERRORS_ALL,
AccessView.AUDIT_LOGS_ALL)
else SimpleAuthManagerRole.VIEWER
)
return self._is_authorized(method="GET", allow_role=allow_role,
user=user, team_name=team_name)
diff --git a/airflow-core/src/airflow/api_fastapi/core_api/security.py
b/airflow-core/src/airflow/api_fastapi/core_api/security.py
index 720423797b5..96478eb5a14 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/security.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/security.py
@@ -265,12 +265,22 @@ class PermittedDagWarningFilter(PermittedDagFilter):
class PermittedEventLogFilter(PermittedDagFilter):
- """A parameter that filters the permitted even logs for the user."""
+ """A parameter that filters the permitted event logs for the user."""
+
+ def __init__(self, value: set[str] | None = None, *, include_non_dag_logs:
bool = False):
+ super().__init__(value)
+ self.include_non_dag_logs = include_non_dag_logs
def to_orm(self, select: Select) -> Select:
- # Event Logs not related to Dags have dag_id as None and are always
returned.
- # return select.where(Log.dag_id.in_(self.value or set()) or
Log.dag_id.is_(None))
- return select.where(or_(Log.dag_id.in_(self.value or set()),
Log.dag_id.is_(None)))
+ permitted_dag_logs = Log.dag_id.in_(self.value or set())
+ if not self.include_non_dag_logs:
+ return select.where(permitted_dag_logs)
+ # Event logs not related to a Dag have dag_id as None. They record
Connection,
+ # Variable, Pool, … operations, so they carry no per-Dag key to
authorize on and
+ # are gated on ``AccessView.AUDIT_LOGS_ALL`` instead of Dag-level
audit access.
+ # Filtering here rather than after the fact keeps unauthorized rows
out of the
+ # count and pagination too, so their existence does not leak either.
+ return select.where(or_(permitted_dag_logs, Log.dag_id.is_(None)))
class PermittedTIFilter(PermittedDagFilter):
@@ -339,9 +349,32 @@ ReadableDagWarningsFilterDep = Annotated[
ReadableTIFilterDep = Annotated[
PermittedTIFilter, Depends(permitted_dag_filter_factory("GET",
PermittedTIFilter))
]
-ReadableEventLogsFilterDep = Annotated[
- PermittedTIFilter, Depends(permitted_dag_filter_factory("GET",
PermittedEventLogFilter))
-]
+
+
+def readable_event_logs_filter_factory() -> Callable[[BaseUser,
BaseAuthManager], PermittedEventLogFilter]:
+ """
+ Create a callable for Depends in FastAPI that returns the event-log filter
for the user.
+
+ Event logs need their own factory rather than
``permitted_dag_filter_factory``: besides
+ the readable Dag ids, the filter needs to know whether the user may read
audit rows that
+ are not tied to a Dag, which is a separate authorization decision.
+ """
+
+ def depends_readable_event_logs_filter(
+ user: GetUserDep,
+ auth_manager: AuthManagerDep,
+ ) -> PermittedEventLogFilter:
+ return PermittedEventLogFilter(
+ auth_manager.get_authorized_dag_ids(user=user, method="GET"),
+ include_non_dag_logs=auth_manager.authorize_view(
+ access_view=AccessView.AUDIT_LOGS_ALL, user=user
+ ),
+ )
+
+ return depends_readable_event_logs_filter
+
+
+ReadableEventLogsFilterDep = Annotated[PermittedEventLogFilter,
Depends(readable_event_logs_filter_factory())]
ReadableXComFilterDep = Annotated[
PermittedXComFilter, Depends(permitted_dag_filter_factory("GET",
PermittedXComFilter))
]
@@ -423,7 +456,22 @@ def requires_access_event_log(
status_code=status.HTTP_400_BAD_REQUEST,
detail="'event_log_id' must be an integer",
)
- dag_id = session.scalar(select(Log.dag_id).where(Log.id ==
event_log_id))
+ # Select the id alongside dag_id: a NULL dag_id and a missing row
both come back
+ # as None from a bare ``Log.dag_id`` scalar, and they authorize
differently.
+ row = session.execute(select(Log.id, Log.dag_id).where(Log.id ==
event_log_id)).one_or_none()
+ if row is not None and row.dag_id is None:
+ # The row records an operation that is not tied to a Dag, so
there is no
+ # per-Dag key to authorize on: gate it on
``AccessView.AUDIT_LOGS_ALL``,
+ # the same view the list endpoint's filter uses for these rows.
+ _requires_access(
+ is_authorized_callback=lambda:
get_auth_manager().authorize_view(
+ access_view=AccessView.AUDIT_LOGS_ALL, user=user
+ ),
+ )
+ return
+ # A missing row keeps the Dag-level check so the route can answer
404 rather
+ # than turning an unknown id into a permission error.
+ dag_id = row.dag_id if row is not None else None
requires_access_dag(method, DagAccessEntity.AUDIT_LOG, dag_id)(
request,
diff --git
a/airflow-core/tests/unit/api_fastapi/auth/managers/simple/test_simple_auth_manager.py
b/airflow-core/tests/unit/api_fastapi/auth/managers/simple/test_simple_auth_manager.py
index c10e1273d16..b51329006d4 100644
---
a/airflow-core/tests/unit/api_fastapi/auth/managers/simple/test_simple_auth_manager.py
+++
b/airflow-core/tests/unit/api_fastapi/auth/managers/simple/test_simple_auth_manager.py
@@ -347,6 +347,33 @@ class TestSimpleAuthManager:
is result
)
+ @pytest.mark.parametrize(
+ "access_view",
+ [AccessView.IMPORT_ERRORS_ALL, AccessView.AUDIT_LOGS_ALL],
+ )
+ @pytest.mark.parametrize(
+ ("role", "result"),
+ [
+ ("ADMIN", True),
+ ("OP", False),
+ ("USER", False),
+ ("VIEWER", False),
+ ],
+ )
+ def test_is_authorized_view_admin_only_views(self, auth_manager,
access_view, role, result):
+ """The views covering records with no per-Dag key to authorize on are
admin-only.
+
+ Every other view is readable by a viewer (asserted above); these two
gate records
+ that carry no other authorization key -- import errors for files with
no registered
+ Dag, and audit log rows not tied to a Dag -- so they must not ride on
viewer access.
+ """
+ assert (
+ auth_manager.is_authorized_view(
+ access_view=access_view,
user=SimpleAuthManagerUser(username="test", role=role)
+ )
+ is result
+ )
+
@pytest.mark.parametrize(
"api",
[
diff --git
a/airflow-core/tests/unit/api_fastapi/auth/managers/test_base_auth_manager.py
b/airflow-core/tests/unit/api_fastapi/auth/managers/test_base_auth_manager.py
index 5ca46908e3a..d7f2286aa15 100644
---
a/airflow-core/tests/unit/api_fastapi/auth/managers/test_base_auth_manager.py
+++
b/airflow-core/tests/unit/api_fastapi/auth/managers/test_base_auth_manager.py
@@ -190,6 +190,26 @@ class TestBaseAuthManager:
assert result is True
+ def test_authorize_view_denies_a_view_the_manager_cannot_map(self):
+ # AccessView members are added by core, but auth managers ship as
separately
+ # released providers, so a core newer than the installed manager can
name a view
+ # the manager has never heard of. Managers that translate the enum
through a lookup
+ # table raise KeyError on such a member, which would surface as a 500
on the
+ # endpoint. authorize_view denies instead -- the endpoint keeps
working and the
+ # records these views gate stay closed.
+ class LookupTableAuthManager(EmptyAuthManager):
+ def is_authorized_view(self, *, access_view, user=None,
team_name=None):
+ return {AccessView.WEBSITE: True}[access_view]
+
+ manager = LookupTableAuthManager()
+
+ with pytest.warns(UserWarning, match="cannot map the 'DOCS' view"):
+ result = manager.authorize_view(access_view=AccessView.DOCS,
user=None)
+
+ assert result is False
+ # A view the manager does map is unaffected.
+ assert manager.authorize_view(access_view=AccessView.WEBSITE,
user=None) is True
+
def test_authorize_view_treats_kwargs_override_as_team_aware(self):
class KwargsAuthManager(EmptyAuthManager):
def is_authorized_view(self, *, access_view, user=None, **kwargs):
diff --git
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_event_logs.py
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_event_logs.py
index 871692f98d2..75fd6229d5d 100644
---
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_event_logs.py
+++
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_event_logs.py
@@ -22,7 +22,11 @@ from unittest import mock
import pytest
from sqlalchemy.orm import Session
-from airflow.api_fastapi.auth.managers.models.resource_details import
DagAccessEntity, DagDetails
+from airflow.api_fastapi.auth.managers.models.resource_details import (
+ AccessView,
+ DagAccessEntity,
+ DagDetails,
+)
from airflow.models.log import Log
from airflow.utils.session import NEW_SESSION, provide_session
@@ -236,6 +240,49 @@ class TestGetEventLog(TestEventLogsEndpoint):
user=mock.ANY,
)
+ @pytest.mark.parametrize(
+ ("can_view_all_audit_logs", "expected_status_code"),
+ [
+ pytest.param(True, 200, id="with-AUDIT_LOGS_ALL-sees-row"),
+ pytest.param(False, 403, id="without-AUDIT_LOGS_ALL-forbidden"),
+ ],
+ )
+ def test_non_dag_row_is_gated_on_audit_logs_all(
+ self, test_client, setup, can_view_all_audit_logs, expected_status_code
+ ):
+ """A row with a NULL dag_id records an operation that is not tied to a
Dag -- a
+ Connection, Variable or Pool change -- so it has no per-Dag key to
authorize on.
+ Visibility is gated on the dedicated ``AUDIT_LOGS_ALL`` view rather
than riding on
+ Dag-level audit log access, which every viewer holds.
+ """
+ event_log_id = setup[EVENT_NORMAL].id
+ with mock.patch(
+
"airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager.is_authorized_view",
+ return_value=can_view_all_audit_logs,
+ ) as mock_is_authorized_view:
+ response = test_client.get(f"/eventLogs/{event_log_id}")
+
+ assert response.status_code == expected_status_code
+ mock_is_authorized_view.assert_called_once_with(
+ access_view=AccessView.AUDIT_LOGS_ALL, user=mock.ANY,
team_name=None
+ )
+
+ def test_unknown_id_stays_404_and_does_not_consult_audit_logs_all(self,
test_client, setup):
+ """An id that matches no row must answer 404, not 403.
+
+ A missing row and a NULL dag_id both read back as ``None``, so the
guard has to tell
+ them apart: turning an unknown id into a permission error would change
the documented
+ contract of the endpoint for callers that are allowed to use it.
+ """
+ with mock.patch(
+
"airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager.is_authorized_view",
+ return_value=False,
+ ) as mock_is_authorized_view:
+ response = test_client.get(f"/eventLogs/{EVENT_NON_EXISTED_ID}")
+
+ assert response.status_code == 404
+ mock_is_authorized_view.assert_not_called()
+
@provide_session
def test_should_return_404_for_log_without_dttm(self, test_client, *,
session: Session = NEW_SESSION): # noqa: PT028
event_log = Log(event=EVENT_WITHOUT_DTTM)
@@ -463,3 +510,43 @@ class TestGetEventLogs(TestEventLogsEndpoint):
def test_should_raises_403_forbidden(self, unauthorized_test_client):
response = unauthorized_test_client.get("/eventLogs")
assert response.status_code == 403
+
+ @pytest.mark.parametrize(
+ ("can_view_all_audit_logs", "expected_events"),
+ [
+ pytest.param(
+ True,
+ [EVENT_NORMAL, EVENT_WITH_OWNER, TASK_INSTANCE_EVENT,
EVENT_WITH_OWNER_AND_TASK_INSTANCE],
+ id="with-AUDIT_LOGS_ALL-sees-non-dag-rows",
+ ),
+ pytest.param(
+ False,
+ [TASK_INSTANCE_EVENT, EVENT_WITH_OWNER_AND_TASK_INSTANCE],
+ id="without-AUDIT_LOGS_ALL-only-dag-rows",
+ ),
+ ],
+ )
+ def test_non_dag_rows_are_gated_on_audit_logs_all(
+ self, test_client, can_view_all_audit_logs, expected_events
+ ):
+ """Rows with a NULL dag_id are returned only to callers holding
``AUDIT_LOGS_ALL``.
+
+ Before this gate every caller that could read event logs at all
received them, which
+ for the default auth manager is any viewer. ``EVENT_NORMAL`` and
``EVENT_WITH_OWNER``
+ carry no dag_id; the other two are bound to a Dag and stay visible
either way.
+ """
+ with mock.patch(
+
"airflow.api_fastapi.auth.managers.simple.simple_auth_manager.SimpleAuthManager.is_authorized_view",
+ return_value=can_view_all_audit_logs,
+ ) as mock_is_authorized_view:
+ response = test_client.get("/eventLogs")
+
+ assert response.status_code == 200
+ resp_json = response.json()
+ # Filtered in the query, so the excluded rows are absent from the
count and
+ # pagination too -- their existence does not leak through
total_entries.
+ assert resp_json["total_entries"] == len(expected_events)
+ assert {event_log["event"] for event_log in resp_json["event_logs"]}
== set(expected_events)
+ mock_is_authorized_view.assert_called_once_with(
+ access_view=AccessView.AUDIT_LOGS_ALL, user=mock.ANY,
team_name=None
+ )
diff --git a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
index 8824969f976..37750e30da9 100644
--- a/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
+++ b/airflow-core/tests/unit/api_fastapi/core_api/test_security.py
@@ -27,6 +27,7 @@ from airflow import settings
from airflow.api_fastapi.app import create_app
from airflow.api_fastapi.auth.managers.base_auth_manager import
COOKIE_NAME_JWT_TOKEN
from airflow.api_fastapi.auth.managers.models.resource_details import (
+ AccessView,
ConnectionDetails,
DagAccessEntity,
DagDetails,
@@ -468,7 +469,7 @@ class TestFastApiSecurity:
mock_get_team_name.return_value = "team1"
session = Mock()
- session.scalar.return_value = "event_log_dag_id"
+ session.execute.return_value.one_or_none.return_value = Mock(id=42,
dag_id="event_log_dag_id")
request = Mock()
request.path_params = {"event_log_id": "42"}
@@ -496,7 +497,7 @@ class TestFastApiSecurity:
mock_get_team_name.return_value = None
session = Mock()
- session.scalar.return_value = "unauthorized_dag"
+ session.execute.return_value.one_or_none.return_value = Mock(id=1,
dag_id="unauthorized_dag")
request = Mock()
request.path_params = {"event_log_id": "1"}
@@ -524,7 +525,7 @@ class TestFastApiSecurity:
mock_get_auth_manager.return_value = auth_manager
session = Mock()
- session.scalar.return_value = None
+ session.execute.return_value.one_or_none.return_value = None
request = Mock()
request.path_params = {"event_log_id": "999"}
@@ -542,6 +543,56 @@ class TestFastApiSecurity:
)
mock_get_team_name.assert_not_called()
+ @pytest.mark.db_test
+ @pytest.mark.asyncio
+ @patch.object(DagModel, "get_team_name")
+ @patch("airflow.api_fastapi.core_api.security.get_auth_manager")
+ async def test_requires_access_event_log_non_dag_row_uses_audit_logs_all(
+ self, mock_get_auth_manager, mock_get_team_name
+ ):
+ """A row that exists with a NULL dag_id is gated on AUDIT_LOGS_ALL,
not the Dag check.
+
+ Such a row records an operation that is not tied to a Dag, so there is
no per-Dag key
+ to authorize on. It must not fall through to an unscoped
``DagDetails(id=None)`` check,
+ which any caller holding Dag-level audit access would pass.
+ """
+ auth_manager = Mock()
+ auth_manager.authorize_view.return_value = True
+ mock_get_auth_manager.return_value = auth_manager
+
+ session = Mock()
+ session.execute.return_value.one_or_none.return_value = Mock(id=7,
dag_id=None)
+
+ request = Mock()
+ request.path_params = {"event_log_id": "7"}
+ request.query_params = {}
+ user = Mock()
+
+ await requires_access_event_log("GET")(request, user, session)
+
+
auth_manager.authorize_view.assert_called_once_with(access_view=AccessView.AUDIT_LOGS_ALL,
user=user)
+ auth_manager.is_authorized_dag.assert_not_called()
+ mock_get_team_name.assert_not_called()
+
+ @pytest.mark.db_test
+ @pytest.mark.asyncio
+ @patch("airflow.api_fastapi.core_api.security.get_auth_manager")
+ async def test_requires_access_event_log_non_dag_row_denied(self,
mock_get_auth_manager):
+ """Without AUDIT_LOGS_ALL, a non-Dag row is Forbidden rather than
returned."""
+ auth_manager = Mock()
+ auth_manager.authorize_view.return_value = False
+ mock_get_auth_manager.return_value = auth_manager
+
+ session = Mock()
+ session.execute.return_value.one_or_none.return_value = Mock(id=7,
dag_id=None)
+
+ request = Mock()
+ request.path_params = {"event_log_id": "7"}
+ request.query_params = {}
+
+ with pytest.raises(HTTPException, match="Forbidden"):
+ await requires_access_event_log("GET")(request, Mock(), session)
+
@pytest.mark.db_test
@pytest.mark.parametrize("bad_event_log_id", ["abc", "1.5", "1,2", ""])
@patch("airflow.api_fastapi.core_api.security.requires_access_dag")
diff --git
a/providers/common/compat/src/airflow/providers/common/compat/security/access_view.py
b/providers/common/compat/src/airflow/providers/common/compat/security/access_view.py
index 4c79c5a139c..23209b68ad9 100644
---
a/providers/common/compat/src/airflow/providers/common/compat/security/access_view.py
+++
b/providers/common/compat/src/airflow/providers/common/compat/security/access_view.py
@@ -16,14 +16,16 @@
# under the License.
from __future__ import annotations
-# ``AccessView.IMPORT_ERRORS_ALL`` was added in Airflow 3.4.0, and providers
are
-# released independently of core. ``None`` signals the view is unavailable on
the
-# running core and callers should skip mapping it.
+# ``AccessView.IMPORT_ERRORS_ALL`` and ``AccessView.AUDIT_LOGS_ALL`` were
added in
+# Airflow 3.4.0, and providers are released independently of core. ``None``
signals the
+# view is unavailable on the running core and callers should skip mapping it.
try:
from airflow.api_fastapi.auth.managers.models.resource_details import
AccessView
IMPORT_ERRORS_ALL_ACCESS_VIEW: AccessView | None = getattr(AccessView,
"IMPORT_ERRORS_ALL", None)
+ AUDIT_LOGS_ALL_ACCESS_VIEW: AccessView | None = getattr(AccessView,
"AUDIT_LOGS_ALL", None)
except ImportError:
IMPORT_ERRORS_ALL_ACCESS_VIEW = None
+ AUDIT_LOGS_ALL_ACCESS_VIEW = None
-__all__ = ["IMPORT_ERRORS_ALL_ACCESS_VIEW"]
+__all__ = ["AUDIT_LOGS_ALL_ACCESS_VIEW", "IMPORT_ERRORS_ALL_ACCESS_VIEW"]
diff --git
a/providers/common/compat/tests/unit/common/compat/security/test_access_view.py
b/providers/common/compat/tests/unit/common/compat/security/test_access_view.py
index 1804ff96e24..9798ff9330a 100644
---
a/providers/common/compat/tests/unit/common/compat/security/test_access_view.py
+++
b/providers/common/compat/tests/unit/common/compat/security/test_access_view.py
@@ -21,40 +21,44 @@ import sys
import types
from unittest import mock
+import pytest
+
RESOURCE_DETAILS_MODULE =
"airflow.api_fastapi.auth.managers.models.resource_details"
ACCESS_VIEW_SHIM_MODULE =
"airflow.providers.common.compat.security.access_view"
-def test_resolves_to_the_core_access_view_member_or_none():
- """The shim mirrors the running core: the ``AccessView.IMPORT_ERRORS_ALL``
- member on a core that defines it (>= 3.4.0), otherwise ``None``. Kept
- version-agnostic so it holds across the whole provider compatibility
matrix,
- including cores that predate the member or lack ``api_fastapi`` entirely.
[email protected]("member_name", ["IMPORT_ERRORS_ALL",
"AUDIT_LOGS_ALL"])
+def test_resolves_to_the_core_access_view_member_or_none(member_name):
+ """The shim mirrors the running core: the ``AccessView`` member on a core
that
+ defines it (>= 3.4.0), otherwise ``None``. Kept version-agnostic so it
holds
+ across the whole provider compatibility matrix, including cores that
predate the
+ member or lack ``api_fastapi`` entirely.
"""
- from airflow.providers.common.compat.security.access_view import
IMPORT_ERRORS_ALL_ACCESS_VIEW
+ shim = importlib.import_module(ACCESS_VIEW_SHIM_MODULE)
try:
from airflow.api_fastapi.auth.managers.models.resource_details import
AccessView
- expected = getattr(AccessView, "IMPORT_ERRORS_ALL", None)
+ expected = getattr(AccessView, member_name, None)
except ImportError:
expected = None
- assert IMPORT_ERRORS_ALL_ACCESS_VIEW is expected
+ assert getattr(shim, f"{member_name}_ACCESS_VIEW") is expected
def test_is_none_on_older_core_without_the_member():
- """On a core that predates ``AccessView.IMPORT_ERRORS_ALL`` the shim
resolves to ``None``."""
+ """On a core that predates the new ``AccessView`` members the shim
resolves to ``None``."""
- class _AccessViewWithoutImportErrorsAll:
- """Stand-in for an older core AccessView that lacks the new member."""
+ class _AccessViewWithoutNewMembers:
+ """Stand-in for an older core AccessView that lacks the new members."""
fake_resource_details = types.ModuleType(RESOURCE_DETAILS_MODULE)
- fake_resource_details.AccessView = _AccessViewWithoutImportErrorsAll
+ fake_resource_details.AccessView = _AccessViewWithoutNewMembers
with mock.patch.dict(sys.modules, {RESOURCE_DETAILS_MODULE:
fake_resource_details}):
reloaded =
importlib.reload(importlib.import_module(ACCESS_VIEW_SHIM_MODULE))
assert reloaded.IMPORT_ERRORS_ALL_ACCESS_VIEW is None
+ assert reloaded.AUDIT_LOGS_ALL_ACCESS_VIEW is None
# Restore the module against the real core so later imports see the real
value.
importlib.reload(importlib.import_module(ACCESS_VIEW_SHIM_MODULE))
diff --git a/providers/fab/docs/auth-manager/access-control.rst
b/providers/fab/docs/auth-manager/access-control.rst
index da6ad74648e..52658bd6e80 100644
--- a/providers/fab/docs/auth-manager/access-control.rst
+++ b/providers/fab/docs/auth-manager/access-control.rst
@@ -172,8 +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
Viewer
-/eventLogs/{event_log_id}
GET Audit Logs.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)
/importErrors
GET ImportError.can_read
Viewer
/importErrors/{import_error_id}
GET ImportError.can_read
Viewer
/health
GET None
Public
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 046278a0408..09e58424302 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
@@ -56,7 +56,10 @@ from airflow.api_fastapi.common.types import ExtraMenuItem,
MenuItem
from airflow.exceptions import AirflowConfigException,
AirflowProviderDeprecationWarning
from airflow.models import Connection, DagModel, Pool, Variable
from airflow.providers.common.compat.sdk import AirflowException, conf
-from airflow.providers.common.compat.security.access_view import
IMPORT_ERRORS_ALL_ACCESS_VIEW
+from airflow.providers.common.compat.security.access_view import (
+ AUDIT_LOGS_ALL_ACCESS_VIEW,
+ IMPORT_ERRORS_ALL_ACCESS_VIEW,
+)
from airflow.providers.fab.auth_manager.models import Permission, Role, User
from airflow.providers.fab.auth_manager.models.anonymous_user import
AnonymousUser
from airflow.providers.fab.version_compat import AIRFLOW_V_3_1_PLUS
@@ -65,6 +68,7 @@ from airflow.providers.fab.www.security import permissions
from airflow.providers.fab.www.security.permissions import (
ACTION_CAN_READ,
RESOURCE_AUDIT_LOG,
+ RESOURCE_AUDIT_LOG_ALL,
RESOURCE_BACKFILL,
RESOURCE_CLUSTER_ACTIVITY,
RESOURCE_CONFIG,
@@ -148,10 +152,13 @@ _MAP_ACCESS_VIEW_TO_FAB_RESOURCE_TYPE = {
AccessView.WEBSITE: RESOURCE_WEBSITE,
}
-# ``AccessView.IMPORT_ERRORS_ALL`` only exists on core >= 3.4.0; the compat
shim
-# yields ``None`` on older core so this provider still imports there.
+# ``AccessView.IMPORT_ERRORS_ALL`` and ``AccessView.AUDIT_LOGS_ALL`` only
exist on
+# core >= 3.4.0; the compat shim yields ``None`` on older core so this
provider still
+# imports there.
if IMPORT_ERRORS_ALL_ACCESS_VIEW is not None:
_MAP_ACCESS_VIEW_TO_FAB_RESOURCE_TYPE[IMPORT_ERRORS_ALL_ACCESS_VIEW] =
RESOURCE_IMPORT_ERROR_ALL
+if AUDIT_LOGS_ALL_ACCESS_VIEW is not None:
+ _MAP_ACCESS_VIEW_TO_FAB_RESOURCE_TYPE[AUDIT_LOGS_ALL_ACCESS_VIEW] =
RESOURCE_AUDIT_LOG_ALL
_MAP_MENU_ITEM_TO_FAB_RESOURCE_TYPE = {
MenuItem.ASSETS: RESOURCE_ASSET,
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 05a72235f7d..22ba8353119 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
@@ -332,6 +332,7 @@ class
FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
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),
(permissions.ACTION_CAN_ACCESS_MENU,
permissions.RESOURCE_TASK_RESCHEDULE),
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 10dfd6f45f2..7fcf1ea357a 100644
--- a/providers/fab/src/airflow/providers/fab/www/security/permissions.py
+++ b/providers/fab/src/airflow/providers/fab/www/security/permissions.py
@@ -22,6 +22,7 @@ from typing import TypedDict
RESOURCE_ACTION = "Permissions"
RESOURCE_ADMIN_MENU = "Admin"
RESOURCE_AUDIT_LOG = "Audit Logs"
+RESOURCE_AUDIT_LOG_ALL = "All Audit Logs"
RESOURCE_BACKFILL = "Backfills"
RESOURCE_BROWSE_MENU = "Browse"
RESOURCE_CONFIG = "Configurations"
