From 93ed8d78bf5930fa190284867198ff10ef6c7777 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Mon, 13 Jul 2026 01:42:40 +0200 Subject: [PATCH 01/12] Add IMPORT_ERRORS_ALL permission for import errors of files with no registered Dag The Import Errors API authorizes each error per Dag defined in its file. Files with no registered Dag have no per-Dag key to authorize on. Gate the raw stack trace for those files on a dedicated, admin-by-default IMPORT_ERRORS_ALL view, scoped per team via the file's bundle where the auth manager supports multi-team isolation, and redact it for callers without the permission on both the list and single endpoints. Closes: #67461 Generated-by: Claude Opus 4.8 (1M context) following the guidelines at https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#gen-ai-assisted-contributions --- .../import-errors-all-permission.feature.rst | 1 + .../auth/managers/base_auth_manager.py | 22 ++ .../auth/managers/models/resource_details.py | 6 + .../managers/simple/simple_auth_manager.py | 17 +- .../core_api/routes/public/import_error.py | 59 ++++- .../auth/managers/test_base_auth_manager.py | 18 +- .../routes/public/test_import_error.py | 223 ++++++++++++++---- .../fab/auth_manager/fab_auth_manager.py | 2 + .../auth_manager/security_manager/override.py | 1 + .../providers/fab/www/security/permissions.py | 1 + .../auth_manager/keycloak_auth_manager.py | 6 + 11 files changed, 291 insertions(+), 65 deletions(-) create mode 100644 airflow-core/newsfragments/import-errors-all-permission.feature.rst diff --git a/airflow-core/newsfragments/import-errors-all-permission.feature.rst b/airflow-core/newsfragments/import-errors-all-permission.feature.rst new file mode 100644 index 0000000000000..8b11640159c78 --- /dev/null +++ b/airflow-core/newsfragments/import-errors-all-permission.feature.rst @@ -0,0 +1 @@ +Add a dedicated ``IMPORT_ERRORS_ALL`` permission (admin by default, team-scoped) gating visibility of import errors for files with no registered Dag; callers without it are denied those errors instead of receiving the raw stack trace. Auth managers gain an ``is_authorized_view_for_team`` method that defaults to ``is_authorized_view``, so managers that do not implement it (including out-of-tree ones) keep working unchanged. 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 d587ec7a68004..58db06b983538 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 @@ -365,6 +365,28 @@ def is_authorized_view( :param user: the user to performing the action """ + def is_authorized_view_for_team( + self, + *, + access_view: AccessView, + user: T, + team_name: str | None = None, + ) -> bool: + """ + Return whether the user is authorized to access a read-only view, scoped to a team. + + This is the team-aware variant of :meth:`is_authorized_view`. Auth managers that + support multi-team isolation override it to restrict access to users belonging to + ``team_name``. The default implementation ignores ``team_name`` and falls back to + :meth:`is_authorized_view`, so auth managers (including out-of-tree ones) that do + not implement it keep working unchanged. + + :param access_view: the specific read-only view/state the authorization request is about. + :param user: the user performing the action + :param team_name: team the view is scoped to, if any + """ + return self.is_authorized_view(access_view=access_view, user=user) + @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 41775d8043e55..9d7020fb9d6b1 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 @@ -101,6 +101,12 @@ class AccessView(Enum): CLUSTER_ACTIVITY = "CLUSTER_ACTIVITY" DOCS = "DOCS" IMPORT_ERRORS = "IMPORT_ERRORS" + # Visibility of import errors for files that have no registered Dag (parse + # failed before any Dag was defined, or all Dags were removed). There is no + # per-Dag key to authorize on for such files, so this is a dedicated, + # admin-by-default view, scoped per team via the file's bundle where the + # auth manager supports it. + IMPORT_ERRORS_ALL = "IMPORT_ERRORS_ALL" JOBS = "JOBS" PLUGINS = "PLUGINS" PROVIDERS = "PROVIDERS" 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 0559a388156d5..fc764090d0f50 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 @@ -37,7 +37,7 @@ from airflow.api_fastapi.app import AUTH_MANAGER_FASTAPI_APP_PREFIX from airflow.api_fastapi.auth.managers.base_auth_manager import BaseAuthManager -from airflow.api_fastapi.auth.managers.models.resource_details import TeamDetails +from airflow.api_fastapi.auth.managers.models.resource_details import AccessView, TeamDetails from airflow.api_fastapi.auth.managers.simple.user import SimpleAuthManagerUser from airflow.api_fastapi.common.types import MenuItem from airflow.configuration import AIRFLOW_HOME, conf @@ -47,7 +47,6 @@ from airflow.api_fastapi.auth.managers.base_auth_manager import ResourceMethod from airflow.api_fastapi.auth.managers.models.resource_details import ( - AccessView, AssetAliasDetails, AssetDetails, ConfigurationDetails, @@ -353,7 +352,19 @@ def is_authorized_variable( ) def is_authorized_view(self, *, access_view: AccessView, user: SimpleAuthManagerUser) -> bool: - return self._is_authorized(method="GET", allow_role=SimpleAuthManagerRole.VIEWER, user=user) + return self.is_authorized_view_for_team(access_view=access_view, user=user) + + def is_authorized_view_for_team( + self, *, access_view: AccessView, user: SimpleAuthManagerUser, team_name: str | None = None + ) -> bool: + # Import errors for files with no registered Dag are admin-only (there is no + # per-Dag key to authorize on); every other view stays readable by viewers. + allow_role = ( + SimpleAuthManagerRole.ADMIN + if access_view == AccessView.IMPORT_ERRORS_ALL + else SimpleAuthManagerRole.VIEWER + ) + return self._is_authorized(method="GET", allow_role=allow_role, user=user, team_name=team_name) def is_authorized_custom_view( self, *, method: ResourceMethod, resource_name: str, user: SimpleAuthManagerUser diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py index 63d97a9b7b946..dcc102d51cf4e 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py @@ -55,6 +55,7 @@ requires_access_view, ) from airflow.models import DagModel +from airflow.models.dagbundle import DagBundleModel from airflow.models.errors import ParseImportError REDACTED_STACKTRACE = "REDACTED - you do not have read permission on all Dags in the file" @@ -101,10 +102,23 @@ def get_import_error( # No Dags matched for this file -- either the file genuinely contains # no Dags (parse failed before any Dag was defined), or the name keys - # did not resolve. Return the raw error in this case; a proper - # admin-only path for unregistered files is tracked in a follow-up - # issue (see https://github.com/apache/airflow/issues/67461). + # did not resolve. There is no per-Dag key to authorize on, so gate + # visibility on the dedicated ``IMPORT_ERRORS_ALL`` view (admin by + # default), scoped to the file's team via its bundle. Deny access + # entirely for callers without it rather than failing open -- returning + # the row would leak the file's existence (and, for team-scoped bundles, + # cross-team repository structure). if not file_dag_ids: + team_name = ( + DagBundleModel.get_team_name(error.bundle_name, session=session) if error.bundle_name else None + ) + if not auth_manager.is_authorized_view_for_team( + access_view=AccessView.IMPORT_ERRORS_ALL, user=user, team_name=team_name + ): + raise HTTPException( + status.HTTP_403_FORBIDDEN, + "You do not have permission to view import errors for files with no registered Dag", + ) return error # Can the user read any Dags in the file? @@ -159,6 +173,31 @@ def get_import_errors( # Subquery for files that have any Dags files_with_any_dags = select(DagModel.relative_fileloc).distinct().subquery() + # Import errors for files that have **no** registered Dag have no per-Dag + # key to authorize on. Authorize them on the dedicated ``IMPORT_ERRORS_ALL`` + # view (admin by default), scoped to the file's team via its bundle, and + # keep only the ones the caller may see. Filtering here (rather than + # redacting in the loop) also excludes unauthorized rows from the count and + # pagination, so their existence -- and, for team-scoped bundles, the + # cross-team file/bundle names -- does not leak. + unregistered_errors = session.execute( + select(ParseImportError.id, ParseImportError.bundle_name) + .outerjoin(files_with_any_dags, ParseImportError.filename == files_with_any_dags.c.relative_fileloc) + .where(files_with_any_dags.c.relative_fileloc.is_(None)) + ).all() + team_name_by_bundle = DagBundleModel.get_team_names( + {bundle_name for _, bundle_name in unregistered_errors if bundle_name}, session=session + ) + authorized_unregistered_ids = { + error_id + for error_id, bundle_name in unregistered_errors + if auth_manager.is_authorized_view_for_team( + access_view=AccessView.IMPORT_ERRORS_ALL, + user=user, + team_name=team_name_by_bundle.get(bundle_name) if bundle_name else None, + ) + } + # Files (identified by ``(relative_fileloc, bundle_name)``) where the # user can read at least one Dag. Used to decide which import errors # the user is allowed to see at all. @@ -206,7 +245,9 @@ def get_import_errors( ) .where( or_( - files_with_any_dags.c.relative_fileloc.is_(None), + # Unregistered-file errors the caller is authorized to see. + ParseImportError.id.in_(authorized_unregistered_ids), + # Files where the caller can read at least one Dag. file_dags_cte.c.dag_id.isnot(None), ) ) @@ -259,12 +300,10 @@ def get_import_errors( for import_error, file_dag_ids_iter in import_errors_result: dag_ids = [dag_id for _, dag_id in file_dag_ids_iter if dag_id is not None] - # No Dags matched for this file -- either the file genuinely has - # no Dags yet (parse failed before any Dag was defined), or the - # name keys did not resolve. Append the raw error in this case; - # a proper admin-only path for unregistered files is tracked in - # a follow-up issue - # (see https://github.com/apache/airflow/issues/67461). + # No Dags matched for this file. Only unregistered-file errors the + # caller is authorized to see reach this point (unauthorized ones are + # excluded by the ``authorized_unregistered_ids`` filter above), so + # return the raw stacktrace. if not dag_ids: import_errors.append(import_error) continue 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 348bce778ec0c..5c929cbbe000e 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 @@ -26,6 +26,7 @@ from airflow.api_fastapi.auth.managers.base_auth_manager import BaseAuthManager, T from airflow.api_fastapi.auth.managers.models.base_user import BaseUser from airflow.api_fastapi.auth.managers.models.resource_details import ( + AccessView, ConnectionDetails, DagDetails, PoolDetails, @@ -41,7 +42,6 @@ if TYPE_CHECKING: from airflow.api_fastapi.auth.managers.base_auth_manager import ResourceMethod from airflow.api_fastapi.auth.managers.models.resource_details import ( - AccessView, AssetAliasDetails, AssetDetails, ConfigurationDetails, @@ -157,6 +157,22 @@ class TestBaseAuthManager: def test_init_non_multi_team_mode(self, auth_manager): assert auth_manager.init() is None + @patch.object(EmptyAuthManager, "is_authorized_view") + def test_is_authorized_view_for_team_defaults_to_is_authorized_view( + self, mock_is_authorized_view, auth_manager + ): + # Managers that do not override the team-aware variant (including + # out-of-tree ones) must keep working: the default ignores team_name and + # falls back to is_authorized_view. + mock_is_authorized_view.return_value = True + + result = auth_manager.is_authorized_view_for_team( + access_view=AccessView.DOCS, user=None, team_name="team_a" + ) + + assert result is True + mock_is_authorized_view.assert_called_once_with(access_view=AccessView.DOCS, user=None) + @conf_vars({("core", "multi_team"): "True"}) @pytest.mark.parametrize( ("auth_manager_teams", "db_teams", "expected"), diff --git a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py index e7be4fa3219ea..dcf941241076a 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py @@ -21,19 +21,22 @@ from unittest import mock import pytest +from sqlalchemy import delete -from airflow.api_fastapi.auth.managers.models.resource_details import DagDetails +from airflow.api_fastapi.auth.managers.models.resource_details import AccessView, DagDetails from airflow.api_fastapi.core_api.routes.public.import_error import REDACTED_STACKTRACE from airflow.models import DagModel from airflow.models.dagbundle import DagBundleModel from airflow.models.errors import ParseImportError -from airflow.utils.session import NEW_SESSION, provide_session +from airflow.utils.session import NEW_SESSION, create_session, provide_session from tests_common.test_utils.asserts import assert_queries_count from tests_common.test_utils.db import clear_db_dag_bundles, clear_db_dags, clear_db_import_errors from tests_common.test_utils.format_datetime import from_datetime_to_zulu_without_ms if TYPE_CHECKING: + from collections.abc import Generator + from sqlalchemy.orm import Session pytestmark = pytest.mark.db_test @@ -150,6 +153,14 @@ def set_mock_auth_manager__batch_is_authorized_dag( return mock_batch_is_authorized_dag +def set_mock_auth_manager__is_authorized_view_for_team( + mock_auth_manager: mock.Mock, is_authorized_view_for_team_return_value: bool = False +) -> mock.Mock: + mock_method = mock_auth_manager.return_value.is_authorized_view_for_team + mock_method.return_value = is_authorized_view_for_team_return_value + return mock_method + + class TestGetImportError: @pytest.mark.parametrize( ("prepared_import_error_idx", "expected_status_code", "expected_body"), @@ -259,28 +270,49 @@ def test_get_import_error__user_dont_have_read_permission_to_read_all_dags_in_fi "bundle_name": BUNDLE_NAME, } + @pytest.mark.parametrize( + ("can_view_all_import_errors", "expected_status_code"), + [ + pytest.param(True, 200, id="with-IMPORT_ERRORS_ALL-sees-raw"), + pytest.param(False, 403, id="without-IMPORT_ERRORS_ALL-forbidden"), + ], + ) @mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager") - def test_get_import_error__no_dag_in_dagmodel(self, mock_get_auth_manager, test_client, import_errors): - """Import error is returned with the raw stacktrace when no Dag exists - in ``DagModel`` for the file. - - Proper handling of unregistered files (separate admin-only permission, - multi-team isolation) is tracked as follow-up work; for now the endpoint - returns the raw error rather than redacting it. + def test_get_import_error__no_dag_in_dagmodel( + self, + mock_get_auth_manager, + test_client, + import_errors, + can_view_all_import_errors, + expected_status_code, + ): + """A file with no registered Dag has no per-Dag key to authorize on, so + visibility is gated on the dedicated ``IMPORT_ERRORS_ALL`` view: callers + holding it (admins by default) see the raw stacktrace, everyone else is + denied (403) rather than the endpoint failing open or leaking the row. """ import_error_id = import_errors[0].id set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) + mock_method = set_mock_auth_manager__is_authorized_view_for_team( + mock_get_auth_manager, can_view_all_import_errors + ) response = test_client.get(f"/importErrors/{import_error_id}") - assert response.status_code == 200 - assert response.json() == { - "import_error_id": import_error_id, - "timestamp": from_datetime_to_zulu_without_ms(TIMESTAMP1), - "filename": FILENAME1, - "stack_trace": STACKTRACE1, - "bundle_name": BUNDLE_NAME, - } + assert response.status_code == expected_status_code + if expected_status_code == 200: + assert response.json() == { + "import_error_id": import_error_id, + "timestamp": from_datetime_to_zulu_without_ms(TIMESTAMP1), + "filename": FILENAME1, + "stack_trace": STACKTRACE1, + "bundle_name": BUNDLE_NAME, + } + # The unregistered-file view is what gates access, scoped to the file's + # team (None for the un-teamed "testing" bundle). + mock_method.assert_called_once_with( + access_view=AccessView.IMPORT_ERRORS_ALL, user=mock.ANY, team_name=None + ) class TestGetImportErrors: @@ -373,7 +405,10 @@ def test_get_import_errors( set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, permitted_dag_model_all) set_mock_auth_manager__batch_is_authorized_dag(mock_get_auth_manager, True) - with assert_queries_count(5): + # One extra query vs. the has-Dag path: the list endpoint probes for + # import errors of files with no registered Dag so it can authorize them + # on the IMPORT_ERRORS_ALL view before building the main query. + with assert_queries_count(6): response = test_client.get("/importErrors", params=query_params) assert response.status_code == expected_status_code @@ -686,29 +721,45 @@ def test_bundle_name_join_condition_for_import_errors( assert response_json2["total_entries"] == 0 assert response_json2["import_errors"] == [] + @pytest.mark.parametrize( + ("can_view_all_import_errors", "expected_total_entries", "expected_stacktraces"), + [ + pytest.param( + True, + 3, + {FILENAME1: STACKTRACE1, FILENAME2: STACKTRACE2, FILENAME3: STACKTRACE3}, + id="with-IMPORT_ERRORS_ALL-sees-raw", + ), + pytest.param(False, 0, {}, id="without-IMPORT_ERRORS_ALL-hidden"), + ], + ) @mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager") - def test_get_import_errors__no_dag_in_dagmodel(self, mock_get_auth_manager, test_client, import_errors): - """Import errors are returned with their raw stacktraces when no Dag - exists in ``DagModel`` for the file. - - Proper handling of unregistered files is deferred to a follow-up issue - that introduces a dedicated permission and respects multi-team isolation. + def test_get_import_errors__no_dag_in_dagmodel( + self, + mock_get_auth_manager, + test_client, + import_errors, + can_view_all_import_errors, + expected_total_entries, + expected_stacktraces, + ): + """List endpoint gates unregistered-file import errors on the dedicated + ``IMPORT_ERRORS_ALL`` view: callers holding it see the raw stacktrace, + callers without it do not see the rows at all (excluded from both the + results and ``total_entries``, so the file's existence does not leak). """ set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) + set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get("/importErrors") assert response.status_code == 200 response_json = response.json() - assert response_json["total_entries"] == 3 + assert response_json["total_entries"] == expected_total_entries stacktrace_by_filename = { error["filename"]: error["stack_trace"] for error in response_json["import_errors"] } - assert stacktrace_by_filename == { - FILENAME1: STACKTRACE1, - FILENAME2: STACKTRACE2, - FILENAME3: STACKTRACE3, - } + assert stacktrace_by_filename == expected_stacktraces class TestImportErrorFileAuthorization: @@ -838,25 +889,35 @@ def test_single_endpoint_matches_file_via_relative_fileloc_not_fileloc( response = test_client.get(f"/importErrors/{lonely_file_import_error.id}") assert response.status_code == 403 + @pytest.mark.parametrize( + ("can_view_all_import_errors", "expected_status_code"), + [ + pytest.param(True, 200, id="with-IMPORT_ERRORS_ALL-sees-raw"), + pytest.param(False, 403, id="without-IMPORT_ERRORS_ALL-forbidden"), + ], + ) @mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager") - def test_single_endpoint_returns_raw_stacktrace_when_file_has_no_known_dags( + def test_single_endpoint_gates_unregistered_file_stacktrace_on_import_errors_all( self, mock_get_auth_manager, test_client, import_errors, + can_view_all_import_errors, + expected_status_code, ): - """Single endpoint returns the raw stacktrace when the - ``ParseImportError`` refers to a file with no matching ``DagModel`` - rows at all -- for example a file that failed to parse before any - Dag was defined. Proper handling of this case (dedicated permission, - multi-team isolation) is tracked as follow-up work. + """Single endpoint gates a file with no matching ``DagModel`` rows (for + example a file that failed to parse before any Dag was defined) on the + dedicated ``IMPORT_ERRORS_ALL`` view instead of failing open. Callers + holding it see the raw stacktrace; others are denied (403). """ set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) + set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get(f"/importErrors/{import_errors[0].id}") - assert response.status_code == 200 - body = response.json() - assert body["filename"] == FILENAME1 - assert body["stack_trace"] == STACKTRACE1 + assert response.status_code == expected_status_code + if expected_status_code == 200: + body = response.json() + assert body["filename"] == FILENAME1 + assert body["stack_trace"] == STACKTRACE1 @mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager") def test_list_endpoint_redacts_mixed_file_with_colocated_dag_outside_callers_scope( @@ -894,26 +955,86 @@ def permit_only_readable(requests, user): assert len(mixed_entries) == 1 assert mixed_entries[0]["stack_trace"] == REDACTED_STACKTRACE + @pytest.mark.parametrize( + ("can_view_all_import_errors", "expected_total_entries", "expected_stacktraces"), + [ + pytest.param( + True, + 3, + {FILENAME1: STACKTRACE1, FILENAME2: STACKTRACE2, FILENAME3: STACKTRACE3}, + id="with-IMPORT_ERRORS_ALL-sees-raw", + ), + pytest.param(False, 0, {}, id="without-IMPORT_ERRORS_ALL-hidden"), + ], + ) @mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager") - def test_list_endpoint_returns_raw_stacktrace_when_file_has_no_known_dags( + def test_list_endpoint_gates_unregistered_file_stacktrace_on_import_errors_all( self, mock_get_auth_manager, test_client, import_errors, + can_view_all_import_errors, + expected_total_entries, + expected_stacktraces, ): - """List endpoint returns the raw stacktrace for import errors whose - file has no matching ``DagModel`` rows. Proper handling of this case - (dedicated permission, multi-team isolation) is tracked as follow-up - work. + """List endpoint gates import errors whose file has no matching + ``DagModel`` rows on the dedicated ``IMPORT_ERRORS_ALL`` view instead of + failing open. Callers holding it see the raw stacktrace; callers without + it do not see the rows at all (excluded from results and + ``total_entries``). """ set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) + set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get("/importErrors") assert response.status_code == 200 body = response.json() - assert body["total_entries"] == 3 + assert body["total_entries"] == expected_total_entries stacktrace_by_filename = {entry["filename"]: entry["stack_trace"] for entry in body["import_errors"]} - assert stacktrace_by_filename == { - FILENAME1: STACKTRACE1, - FILENAME2: STACKTRACE2, - FILENAME3: STACKTRACE3, - } + assert stacktrace_by_filename == expected_stacktraces + + @pytest.fixture + def team_scoped_unregistered_import_error(self) -> Generator[ParseImportError, None, None]: + """An import error for a file with no registered Dag, whose bundle is + mapped to a team, so the endpoint must scope the ``IMPORT_ERRORS_ALL`` + check to that team.""" + from airflow.models.team import Team + + with create_session() as session: + team = Team(name="team_b") + bundle = DagBundleModel(name="team_b_bundle") + bundle.teams = [team] + session.add_all([team, bundle]) + error = ParseImportError( + bundle_name="team_b_bundle", + filename="team_b_unregistered.py", + stacktrace="team b stack trace", + timestamp=TIMESTAMP1, + ) + session.add(error) + session.commit() + session.refresh(error) + session.expunge(error) + yield error + with create_session() as cleanup_session: + cleanup_session.execute(delete(Team).where(Team.name == "team_b")) + cleanup_session.commit() + + @mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager") + def test_single_endpoint_scopes_unregistered_file_view_to_bundle_team( + self, + mock_get_auth_manager, + test_client, + team_scoped_unregistered_import_error, + ): + """The ``IMPORT_ERRORS_ALL`` check for an unregistered file is scoped to + the file's team, resolved from its bundle. A caller not in that team is + denied (403).""" + set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) + mock_method = set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, False) + + response = test_client.get(f"/importErrors/{team_scoped_unregistered_import_error.id}") + + assert response.status_code == 403 + mock_method.assert_called_once_with( + access_view=AccessView.IMPORT_ERRORS_ALL, user=mock.ANY, team_name="team_b" + ) 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 b6a79d032ae3c..8e164cf9ab662 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 @@ -77,6 +77,7 @@ RESOURCE_DAG_WARNING, RESOURCE_DOCS, RESOURCE_IMPORT_ERROR, + RESOURCE_IMPORT_ERROR_ALL, RESOURCE_JOB, RESOURCE_PLUGIN, RESOURCE_POOL, @@ -139,6 +140,7 @@ AccessView.CLUSTER_ACTIVITY: RESOURCE_CLUSTER_ACTIVITY, AccessView.DOCS: RESOURCE_DOCS, AccessView.IMPORT_ERRORS: RESOURCE_IMPORT_ERROR, + AccessView.IMPORT_ERRORS_ALL: RESOURCE_IMPORT_ERROR_ALL, AccessView.JOBS: RESOURCE_JOB, AccessView.PLUGINS: RESOURCE_PLUGIN, AccessView.PROVIDERS: RESOURCE_PROVIDER, 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 41ab5a9faac45..01029941bb5a3 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 @@ -331,6 +331,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_IMPORT_ERROR_ALL), (permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_RESCHEDULE), (permissions.ACTION_CAN_ACCESS_MENU, permissions.RESOURCE_TASK_RESCHEDULE), (permissions.ACTION_CAN_READ, permissions.RESOURCE_TRIGGER), 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 630a95ea541e5..10dfd6f45f29a 100644 --- a/providers/fab/src/airflow/providers/fab/www/security/permissions.py +++ b/providers/fab/src/airflow/providers/fab/www/security/permissions.py @@ -40,6 +40,7 @@ RESOURCE_DOCS_MENU = "Docs" RESOURCE_HITL_DETAIL = "HITL Detail" RESOURCE_IMPORT_ERROR = "ImportError" +RESOURCE_IMPORT_ERROR_ALL = "All Import Errors" RESOURCE_JOB = "Jobs" RESOURCE_MY_PASSWORD = "My Password" RESOURCE_MY_PROFILE = "My Profile" diff --git a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py index bef6632fe3d60..c6bd2c5156483 100644 --- a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py +++ b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py @@ -314,11 +314,17 @@ def is_authorized_team( ) def is_authorized_view(self, *, access_view: AccessView, user: KeycloakAuthManagerUser) -> bool: + return self.is_authorized_view_for_team(access_view=access_view, user=user) + + def is_authorized_view_for_team( + self, *, access_view: AccessView, user: KeycloakAuthManagerUser, team_name: str | None = None + ) -> bool: return self._is_authorized( method="GET", resource_type=KeycloakResource.VIEW, user=user, resource_id=access_view.value, + team_name=team_name, ) def is_authorized_custom_view( From 00b1b86f598f4b878d32248b851b93b916148b1d Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 18 Jul 2026 20:01:29 +0200 Subject: [PATCH 02/12] Keep FAB provider importable on core without IMPORT_ERRORS_ALL access view The provider is released independently of core and may run against an older Airflow that predates the AccessView.IMPORT_ERRORS_ALL member (added in 3.4.0). Referencing it directly in the resource map raised AttributeError at import time under the provider compatibility matrix. Resolve the member through a common.compat shim and map it only when the running core defines it. --- ...rmission.feature.rst => 69790.feature.rst} | 0 .../common/compat/security/access_view.py | 32 +++++++++++++++++++ .../fab/auth_manager/fab_auth_manager.py | 8 ++++- 3 files changed, 39 insertions(+), 1 deletion(-) rename airflow-core/newsfragments/{import-errors-all-permission.feature.rst => 69790.feature.rst} (100%) create mode 100644 providers/common/compat/src/airflow/providers/common/compat/security/access_view.py diff --git a/airflow-core/newsfragments/import-errors-all-permission.feature.rst b/airflow-core/newsfragments/69790.feature.rst similarity index 100% rename from airflow-core/newsfragments/import-errors-all-permission.feature.rst rename to airflow-core/newsfragments/69790.feature.rst 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 new file mode 100644 index 0000000000000..158d40e60dd58 --- /dev/null +++ b/providers/common/compat/src/airflow/providers/common/compat/security/access_view.py @@ -0,0 +1,32 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +# ``AccessView.IMPORT_ERRORS_ALL`` was added to Airflow core in 3.4.0. Auth-manager +# providers are released independently and may run against an older core that does +# not yet define the member (or, on Airflow 2.x, does not ship ``api_fastapi`` at +# all). Resolve it defensively so importing a provider that references the dedicated +# "all import errors" view does not raise on older core: ``None`` signals the view +# is unavailable 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) +except ImportError: + IMPORT_ERRORS_ALL_ACCESS_VIEW = None + +__all__ = ["IMPORT_ERRORS_ALL_ACCESS_VIEW"] 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 8e164cf9ab662..c76f341791a75 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,6 +56,7 @@ 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.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 @@ -140,7 +141,6 @@ AccessView.CLUSTER_ACTIVITY: RESOURCE_CLUSTER_ACTIVITY, AccessView.DOCS: RESOURCE_DOCS, AccessView.IMPORT_ERRORS: RESOURCE_IMPORT_ERROR, - AccessView.IMPORT_ERRORS_ALL: RESOURCE_IMPORT_ERROR_ALL, AccessView.JOBS: RESOURCE_JOB, AccessView.PLUGINS: RESOURCE_PLUGIN, AccessView.PROVIDERS: RESOURCE_PROVIDER, @@ -148,6 +148,12 @@ AccessView.WEBSITE: RESOURCE_WEBSITE, } +# ``AccessView.IMPORT_ERRORS_ALL`` only exists on Airflow core >= 3.4.0. Map it only +# when the running core defines it, so this provider keeps importing against older +# core (resolved via the common.compat shim, which yields ``None`` when absent). +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 + _MAP_MENU_ITEM_TO_FAB_RESOURCE_TYPE = { MenuItem.ASSETS: RESOURCE_ASSET, MenuItem.AUDIT_LOG: RESOURCE_AUDIT_LOG, From c7c4c471020a091c4eda64603af91c24119d40a3 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 18 Jul 2026 21:31:02 +0200 Subject: [PATCH 03/12] Mark fab and keycloak common-compat dependency as needing the next release The IMPORT_ERRORS_ALL access-view shim is added to the common.compat provider in this same PR, so fab and keycloak require the next common.compat release. The selective-check common-compat guard enforces the '# use next version' marker on those dependents; add it so the build-info check passes and the compatibility tests run. --- providers/fab/pyproject.toml | 2 +- providers/keycloak/pyproject.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/providers/fab/pyproject.toml b/providers/fab/pyproject.toml index 3833c6cdc5c4e..762201d13d7bf 100644 --- a/providers/fab/pyproject.toml +++ b/providers/fab/pyproject.toml @@ -67,7 +67,7 @@ requires-python = ">=3.10" # After you modify the dependencies, and rebuild your Breeze CI image with ``breeze ci-image build`` dependencies = [ "apache-airflow>=3.0.2", - "apache-airflow-providers-common-compat>=1.12.0", + "apache-airflow-providers-common-compat>=1.12.0", # use next version # Blinker use for signals in Flask, this is an optional dependency in Flask 2.2 and lower. # In Flask 2.3 it becomes a mandatory dependency, and flask signals are always available. "blinker>=1.6.2", diff --git a/providers/keycloak/pyproject.toml b/providers/keycloak/pyproject.toml index 5939a9ba847bd..19446155d9bd3 100644 --- a/providers/keycloak/pyproject.toml +++ b/providers/keycloak/pyproject.toml @@ -60,7 +60,7 @@ requires-python = ">=3.10" # After you modify the dependencies, and rebuild your Breeze CI image with ``breeze ci-image build`` dependencies = [ "apache-airflow>=3.0.0", - "apache-airflow-providers-common-compat>=1.12.0", + "apache-airflow-providers-common-compat>=1.12.0", # use next version "python-keycloak>=5.0.0", ] From 785f59b1daa9530b1f1f562c0b84e5410f131900 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 18 Jul 2026 22:03:34 +0200 Subject: [PATCH 04/12] Scope the has-any-Dag lookup for import errors to the file's bundle The same relative_fileloc can exist in more than one bundle, so matching only on the filename treated an import error whose file has no Dag in its own bundle as registered when another bundle happened to define a Dag for the same path. Join the has-any-Dag subquery on both filename and bundle_name so unregistered-file import errors are detected per bundle. --- .../api_fastapi/core_api/routes/public/import_error.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py index dcc102d51cf4e..78a771ff6c5df 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py @@ -171,7 +171,7 @@ def get_import_errors( readable_dag_ids = auth_manager.get_authorized_dag_ids(method="GET", user=user) # Subquery for files that have any Dags - files_with_any_dags = select(DagModel.relative_fileloc).distinct().subquery() + files_with_any_dags = select(DagModel.relative_fileloc, DagModel.bundle_name).distinct().subquery() # Import errors for files that have **no** registered Dag have no per-Dag # key to authorize on. Authorize them on the dedicated ``IMPORT_ERRORS_ALL`` @@ -182,7 +182,11 @@ def get_import_errors( # cross-team file/bundle names -- does not leak. unregistered_errors = session.execute( select(ParseImportError.id, ParseImportError.bundle_name) - .outerjoin(files_with_any_dags, ParseImportError.filename == files_with_any_dags.c.relative_fileloc) + .outerjoin( + files_with_any_dags, + (ParseImportError.filename == files_with_any_dags.c.relative_fileloc) + & (ParseImportError.bundle_name == files_with_any_dags.c.bundle_name), + ) .where(files_with_any_dags.c.relative_fileloc.is_(None)) ).all() team_name_by_bundle = DagBundleModel.get_team_names( From c6fd6f960e06f70863b55d16221e2958489f82c8 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 18 Jul 2026 22:50:24 +0200 Subject: [PATCH 05/12] Add test for the common.compat IMPORT_ERRORS_ALL access-view shim The shim module was added without a matching test, which the provider project-structure check flags. Cover both branches: it resolves to the enum member on a core that defines it, and to None on an older core that does not. --- .../compat/security/test_access_view.py | 49 +++++++++++++++++++ 1 file changed, 49 insertions(+) create mode 100644 providers/common/compat/tests/unit/common/compat/security/test_access_view.py 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 new file mode 100644 index 0000000000000..f8a91dd86d43c --- /dev/null +++ b/providers/common/compat/tests/unit/common/compat/security/test_access_view.py @@ -0,0 +1,49 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. +from __future__ import annotations + +import importlib +import sys +import types +from unittest import mock + +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_enum_member_when_core_defines_it(): + from airflow.api_fastapi.auth.managers.models.resource_details import AccessView + from airflow.providers.common.compat.security.access_view import IMPORT_ERRORS_ALL_ACCESS_VIEW + + assert IMPORT_ERRORS_ALL_ACCESS_VIEW is AccessView.IMPORT_ERRORS_ALL + + +def test_is_none_on_older_core_without_the_member(): + """On a core that predates ``AccessView.IMPORT_ERRORS_ALL`` the shim resolves to ``None``.""" + + class _AccessViewWithoutImportErrorsAll: + """Stand-in for an older core AccessView that lacks the new member.""" + + fake_resource_details = types.ModuleType(RESOURCE_DETAILS_MODULE) + fake_resource_details.AccessView = _AccessViewWithoutImportErrorsAll + + 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 + + # Restore the module against the real core so later imports see the real value. + importlib.reload(importlib.import_module(ACCESS_VIEW_SHIM_MODULE)) From 980ca61d429c885e43ff0458d75b12533aa76ba9 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sat, 18 Jul 2026 22:50:25 +0200 Subject: [PATCH 06/12] Scope the second has-any-Dag lookup for import errors per bundle The list endpoint joins the has-any-Dag subquery twice; only the first was scoped to the bundle. Since the subquery now yields one row per (file, bundle), the remaining filename-only join could duplicate an error across bundles and mis-detect registration. Join on bundle_name there too, add a regression test for a file registered only in another bundle, and update the existing bundle -join test, whose old expectation encoded the pre-fix cross-bundle leak. --- .../core_api/routes/public/import_error.py | 5 +- .../routes/public/test_import_error.py | 64 +++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py index 78a771ff6c5df..e17a4ca764895 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py @@ -238,7 +238,10 @@ def get_import_errors( select(ParseImportError, file_dags_cte.c.dag_id) .outerjoin( files_with_any_dags, - ParseImportError.filename == files_with_any_dags.c.relative_fileloc, + and_( + ParseImportError.filename == files_with_any_dags.c.relative_fileloc, + ParseImportError.bundle_name == files_with_any_dags.c.bundle_name, + ), ) .outerjoin( file_dags_cte, diff --git a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py index dcf941241076a..553dd94ebfd58 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py @@ -526,6 +526,63 @@ def test_get_import_errors_bundle_name_and_filename_filter( assert response_json["import_errors"][0]["bundle_name"] == BUNDLE_NAME assert response_json["import_errors"][0]["stack_trace"] == STACKTRACE1 + @mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager") + def test_get_import_errors__file_registered_only_in_other_bundle_is_unregistered( + self, + mock_get_auth_manager, + test_client, + session, + ): + """Regression: the has-any-Dag lookup is scoped per bundle. The same + ``relative_fileloc`` can exist in several bundles, so an import error + whose file has a registered Dag only in a *different* bundle must still + be treated as unregistered in its own bundle and gated on + ``IMPORT_ERRORS_ALL`` -- matching on the filename alone leaked it as + registered and could duplicate the row across bundles. + """ + clear_db_import_errors() + other_bundle = "other_bundle" + session.add(DagBundleModel(name=other_bundle)) + session.flush() + # FILENAME1 has a registered Dag only in ``other_bundle`` ... + session.add( + DagModel( + fileloc=f"/other/{FILENAME1}", + relative_fileloc=FILENAME1, + dag_id="dag_in_other_bundle", + is_paused=False, + bundle_name=other_bundle, + ) + ) + # ... while the import error for the same path is in BUNDLE_NAME, which + # has no Dag for it. + error = ParseImportError( + bundle_name=BUNDLE_NAME, + filename=FILENAME1, + stacktrace=STACKTRACE1, + timestamp=TIMESTAMP1, + ) + session.add(error) + session.commit() + + # Being able to read the other-bundle Dag must not grant visibility into + # the same-named file's error in BUNDLE_NAME. + set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, {"dag_in_other_bundle"}) + set_mock_auth_manager__batch_is_authorized_dag(mock_get_auth_manager, True) + mock_view = set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, True) + + response = test_client.get("/importErrors") + + assert response.status_code == 200 + body = response.json() + # Exactly one row (no filename-only-join duplication), admitted only via + # the IMPORT_ERRORS_ALL view. + assert body["total_entries"] == 1 + assert [e["bundle_name"] for e in body["import_errors"]] == [BUNDLE_NAME] + mock_view.assert_called_once_with( + access_view=AccessView.IMPORT_ERRORS_ALL, user=mock.ANY, team_name=None + ) + def test_should_raises_401_unauthenticated(self, unauthenticated_test_client): response = unauthenticated_test_client.get("/importErrors") assert response.status_code == 401 @@ -713,6 +770,13 @@ def test_bundle_name_join_condition_for_import_errors( session.merge(dag_model1) session.commit() + # FILENAME1 now has no Dag in BUNDLE_NAME (its only Dag moved to another + # bundle), so its error there is unregistered and gated on the dedicated + # IMPORT_ERRORS_ALL view -- which this caller does not hold, so the row + # is hidden. (Matching on filename alone would have wrongly treated it as + # registered via the same-named Dag in the other bundle.) + set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, False) + response2 = test_client.get("/importErrors") # Assert - should return 0 entries because bundle_name no longer matches From fdcf81839bf3172be6b61950b7c7bb138a86bdd6 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Sun, 19 Jul 2026 18:40:37 +0200 Subject: [PATCH 07/12] Make the access-view shim test version-agnostic The provider compatibility matrix runs this test against older cores that predate AccessView.IMPORT_ERRORS_ALL (or lack api_fastapi entirely), where referencing the member directly raised AttributeError. Compare the shim against whatever the running core exposes (the member, or None) so it holds everywhere. --- .../common/compat/security/test_access_view.py | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) 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 f8a91dd86d43c..1804ff96e24a6 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 @@ -25,11 +25,22 @@ ACCESS_VIEW_SHIM_MODULE = "airflow.providers.common.compat.security.access_view" -def test_resolves_to_enum_member_when_core_defines_it(): - from airflow.api_fastapi.auth.managers.models.resource_details import AccessView +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. + """ from airflow.providers.common.compat.security.access_view import IMPORT_ERRORS_ALL_ACCESS_VIEW - assert IMPORT_ERRORS_ALL_ACCESS_VIEW is AccessView.IMPORT_ERRORS_ALL + try: + from airflow.api_fastapi.auth.managers.models.resource_details import AccessView + + expected = getattr(AccessView, "IMPORT_ERRORS_ALL", None) + except ImportError: + expected = None + + assert IMPORT_ERRORS_ALL_ACCESS_VIEW is expected def test_is_none_on_older_core_without_the_member(): From 7f0cf5d9131b050335ba8e9ffc84ee85bcde4b01 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Mon, 20 Jul 2026 14:18:38 +0200 Subject: [PATCH 08/12] Add regression test for per-bundle scoping on single import error endpoint The has-any-Dag lookup in get_import_error is scoped per bundle. Without that scoping a same-named file registered in a different bundle resolved as a Dag match, so the endpoint returned the raw stacktrace on the strength of an unrelated bundle's read permission instead of gating on IMPORT_ERRORS_ALL. The list endpoint already had this coverage. --- .../routes/public/test_import_error.py | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py index 553dd94ebfd58..5a35edfd0a20c 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py @@ -314,6 +314,52 @@ def test_get_import_error__no_dag_in_dagmodel( access_view=AccessView.IMPORT_ERRORS_ALL, user=mock.ANY, team_name=None ) + @mock.patch("airflow.api_fastapi.core_api.routes.public.import_error.get_auth_manager") + def test_get_import_error__file_registered_only_in_other_bundle_is_unregistered( + self, + mock_get_auth_manager, + test_client, + import_errors, + session, + ): + """Regression: the has-any-Dag lookup is scoped per bundle. Matching on + ``relative_fileloc`` alone would resolve the same-named file's Dag in a + *different* bundle, treat this error as registered, and hand back the raw + stacktrace on the strength of an unrelated bundle's read permission -- + never consulting ``IMPORT_ERRORS_ALL``. + """ + import_error_id = import_errors[0].id + other_bundle = "other_bundle" + session.add(DagBundleModel(name=other_bundle)) + session.flush() + # FILENAME1 has a registered Dag only in ``other_bundle``, while the + # import error for the same path lives in BUNDLE_NAME, which has none. + session.add( + DagModel( + fileloc=f"/other/{FILENAME1}", + relative_fileloc=FILENAME1, + dag_id="dag_in_other_bundle", + is_paused=False, + bundle_name=other_bundle, + ) + ) + session.commit() + + # Readable other-bundle Dag must not grant visibility into the + # same-named file's error in BUNDLE_NAME. + set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, {"dag_in_other_bundle"}) + mock_method = set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, False) + + response = test_client.get(f"/importErrors/{import_error_id}") + + assert response.status_code == 403 + assert response.json() == { + "detail": "You do not have permission to view import errors for files with no registered Dag" + } + mock_method.assert_called_once_with( + access_view=AccessView.IMPORT_ERRORS_ALL, user=mock.ANY, team_name=None + ) + class TestGetImportErrors: @pytest.mark.parametrize( From 5d4ff6149d924067ce6192d6e0faa459f21c4581 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Tue, 21 Jul 2026 11:27:49 +0200 Subject: [PATCH 09/12] Fold team scoping into is_authorized_view instead of a separate method Per review, add the optional team_name to is_authorized_view directly rather than introducing a parallel is_authorized_view_for_team. Duplicating every authorization method for its team-aware variant would bloat the auth manager interface; the other is_authorized_* methods already carry team scoping through their details object, and this keeps is_authorized_view consistent with them. This is a breaking change for out-of-tree auth managers: an override that keeps the old (access_view, user) signature imports cleanly but raises TypeError at runtime the first time core authorizes a team-scoped view. Documented in a significant newsfragment with the one-line fix. --- .../docs/core-concepts/auth-manager/index.rst | 2 +- airflow-core/newsfragments/69790.feature.rst | 2 +- .../newsfragments/69790.significant.rst | 23 ++++++++++++++++ .../auth/managers/base_auth_manager.py | 26 ++++--------------- .../managers/simple/simple_auth_manager.py | 5 +--- .../core_api/routes/public/import_error.py | 4 +-- .../auth/managers/test_base_auth_manager.py | 22 ++++------------ .../routes/public/test_import_error.py | 24 ++++++++--------- .../aws/auth_manager/aws_auth_manager.py | 4 +++ .../fab/auth_manager/fab_auth_manager.py | 6 ++++- .../auth_manager/keycloak_auth_manager.py | 5 +--- 11 files changed, 60 insertions(+), 63 deletions(-) create mode 100644 airflow-core/newsfragments/69790.significant.rst diff --git a/airflow-core/docs/core-concepts/auth-manager/index.rst b/airflow-core/docs/core-concepts/auth-manager/index.rst index b821fbfa135bc..cdcd4bdd2cde6 100644 --- a/airflow-core/docs/core-concepts/auth-manager/index.rst +++ b/airflow-core/docs/core-concepts/auth-manager/index.rst @@ -140,7 +140,7 @@ These authorization methods are: * ``is_authorized_asset_alias``: Return whether the user is authorized to access Airflow asset aliases. Some details about the asset alias can be provided (e.g. the asset alias ID). * ``is_authorized_pool``: Return whether the user is authorized to access Airflow pools. Some details about the pool can be provided (e.g. the pool name). * ``is_authorized_variable``: Return whether the user is authorized to access Airflow variables. Some details about the variable can be provided (e.g. the variable key). -* ``is_authorized_view``: Return whether the user is authorized to access a specific view in Airflow. The view is specified through ``access_view`` (e.g. ``AccessView.CLUSTER_ACTIVITY``). +* ``is_authorized_view``: Return whether the user is authorized to access a specific view in Airflow. The view is specified through ``access_view`` (e.g. ``AccessView.CLUSTER_ACTIVITY``). An optional ``team_name`` scopes the check to a team; auth managers without multi-team support accept it and ignore it, which authorizes the view globally. * ``is_authorized_custom_view``: Return whether the user is authorized to access a specific view not defined in Airflow. This view can be provided by the auth manager itself or a plugin defined by the user. * ``filter_authorized_menu_items``: Given the list of menu items in the UI, return the list of menu items the user has access to. diff --git a/airflow-core/newsfragments/69790.feature.rst b/airflow-core/newsfragments/69790.feature.rst index 8b11640159c78..682849eca21b1 100644 --- a/airflow-core/newsfragments/69790.feature.rst +++ b/airflow-core/newsfragments/69790.feature.rst @@ -1 +1 @@ -Add a dedicated ``IMPORT_ERRORS_ALL`` permission (admin by default, team-scoped) gating visibility of import errors for files with no registered Dag; callers without it are denied those errors instead of receiving the raw stack trace. Auth managers gain an ``is_authorized_view_for_team`` method that defaults to ``is_authorized_view``, so managers that do not implement it (including out-of-tree ones) keep working unchanged. +Add a dedicated ``IMPORT_ERRORS_ALL`` permission (admin by default, team-scoped) gating visibility of import errors for files with no registered Dag; callers without it are denied those errors instead of receiving the raw stack trace. diff --git a/airflow-core/newsfragments/69790.significant.rst b/airflow-core/newsfragments/69790.significant.rst new file mode 100644 index 0000000000000..18a421c381578 --- /dev/null +++ b/airflow-core/newsfragments/69790.significant.rst @@ -0,0 +1,23 @@ +``BaseAuthManager.is_authorized_view`` takes an optional ``team_name`` + +The abstract ``is_authorized_view`` method on ``BaseAuthManager`` gained an optional +``team_name`` keyword argument, so that views can be authorized per team the same way the +other ``is_authorized_*`` methods already are through their ``details`` object. + +Auth managers bundled with Airflow (Simple, FAB, Keycloak, AWS) are updated. **Out-of-tree +auth managers must add the argument to their own override.** Python's ABC machinery only +checks that the method *name* is overridden, not its signature, so an override that keeps the +old ``(access_view, user)`` signature still imports and instantiates cleanly and then raises +``TypeError: is_authorized_view() got an unexpected keyword argument 'team_name'`` at runtime, +the first time Airflow authorizes a team-scoped view. + +Update the override to accept the argument: + +.. code-block:: python + + def is_authorized_view( + self, *, access_view: AccessView, user: MyUser, team_name: str | None = None + ) -> bool: ... + +Managers without multi-team support can accept ``team_name`` and ignore it -- that authorizes +the view globally, which is the behaviour they had before this change. 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 58db06b983538..71667a1dfdb9e 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 @@ -357,35 +357,19 @@ def is_authorized_view( *, access_view: AccessView, user: T, - ) -> bool: - """ - Return whether the user is authorized to access a read-only state of the installation. - - :param access_view: the specific read-only view/state the authorization request is about. - :param user: the user to performing the action - """ - - def is_authorized_view_for_team( - self, - *, - access_view: AccessView, - user: T, team_name: str | None = None, ) -> bool: """ - Return whether the user is authorized to access a read-only view, scoped to a team. + Return whether the user is authorized to access a read-only state of the installation. - This is the team-aware variant of :meth:`is_authorized_view`. Auth managers that - support multi-team isolation override it to restrict access to users belonging to - ``team_name``. The default implementation ignores ``team_name`` and falls back to - :meth:`is_authorized_view`, so auth managers (including out-of-tree ones) that do - not implement it keep working unchanged. + Auth managers that support multi-team isolation use ``team_name`` to restrict access + to users belonging to that team. Managers without multi-team support accept the + argument and ignore it, which authorizes the view globally. :param access_view: the specific read-only view/state the authorization request is about. - :param user: the user performing the action + :param user: the user to performing the action :param team_name: team the view is scoped to, if any """ - return self.is_authorized_view(access_view=access_view, user=user) @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/simple/simple_auth_manager.py b/airflow-core/src/airflow/api_fastapi/auth/managers/simple/simple_auth_manager.py index fc764090d0f50..3377a81be02b5 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 @@ -351,10 +351,7 @@ def is_authorized_variable( team_name=details.team_name if details else None, ) - def is_authorized_view(self, *, access_view: AccessView, user: SimpleAuthManagerUser) -> bool: - return self.is_authorized_view_for_team(access_view=access_view, user=user) - - def is_authorized_view_for_team( + 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 (there is no diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py index e17a4ca764895..d40c5832dccdf 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py @@ -112,7 +112,7 @@ def get_import_error( team_name = ( DagBundleModel.get_team_name(error.bundle_name, session=session) if error.bundle_name else None ) - if not auth_manager.is_authorized_view_for_team( + if not auth_manager.is_authorized_view( access_view=AccessView.IMPORT_ERRORS_ALL, user=user, team_name=team_name ): raise HTTPException( @@ -195,7 +195,7 @@ def get_import_errors( authorized_unregistered_ids = { error_id for error_id, bundle_name in unregistered_errors - if auth_manager.is_authorized_view_for_team( + if auth_manager.is_authorized_view( access_view=AccessView.IMPORT_ERRORS_ALL, user=user, team_name=team_name_by_bundle.get(bundle_name) if bundle_name else None, 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 5c929cbbe000e..cdeb0791c60bd 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 @@ -132,7 +132,11 @@ def is_authorized_variable( raise NotImplementedError() def is_authorized_view( - self, *, access_view: AccessView, user: BaseAuthManagerUserTest | None = None + self, + *, + access_view: AccessView, + user: BaseAuthManagerUserTest | None = None, + team_name: str | None = None, ) -> bool: raise NotImplementedError() @@ -157,22 +161,6 @@ class TestBaseAuthManager: def test_init_non_multi_team_mode(self, auth_manager): assert auth_manager.init() is None - @patch.object(EmptyAuthManager, "is_authorized_view") - def test_is_authorized_view_for_team_defaults_to_is_authorized_view( - self, mock_is_authorized_view, auth_manager - ): - # Managers that do not override the team-aware variant (including - # out-of-tree ones) must keep working: the default ignores team_name and - # falls back to is_authorized_view. - mock_is_authorized_view.return_value = True - - result = auth_manager.is_authorized_view_for_team( - access_view=AccessView.DOCS, user=None, team_name="team_a" - ) - - assert result is True - mock_is_authorized_view.assert_called_once_with(access_view=AccessView.DOCS, user=None) - @conf_vars({("core", "multi_team"): "True"}) @pytest.mark.parametrize( ("auth_manager_teams", "db_teams", "expected"), diff --git a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py index 5a35edfd0a20c..d53fbcaa41f8e 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py @@ -153,11 +153,11 @@ def set_mock_auth_manager__batch_is_authorized_dag( return mock_batch_is_authorized_dag -def set_mock_auth_manager__is_authorized_view_for_team( - mock_auth_manager: mock.Mock, is_authorized_view_for_team_return_value: bool = False +def set_mock_auth_manager__is_authorized_view( + mock_auth_manager: mock.Mock, is_authorized_view_return_value: bool = False ) -> mock.Mock: - mock_method = mock_auth_manager.return_value.is_authorized_view_for_team - mock_method.return_value = is_authorized_view_for_team_return_value + mock_method = mock_auth_manager.return_value.is_authorized_view + mock_method.return_value = is_authorized_view_return_value return mock_method @@ -293,7 +293,7 @@ def test_get_import_error__no_dag_in_dagmodel( """ import_error_id = import_errors[0].id set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - mock_method = set_mock_auth_manager__is_authorized_view_for_team( + mock_method = set_mock_auth_manager__is_authorized_view( mock_get_auth_manager, can_view_all_import_errors ) @@ -348,7 +348,7 @@ def test_get_import_error__file_registered_only_in_other_bundle_is_unregistered( # Readable other-bundle Dag must not grant visibility into the # same-named file's error in BUNDLE_NAME. set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, {"dag_in_other_bundle"}) - mock_method = set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, False) + mock_method = set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, False) response = test_client.get(f"/importErrors/{import_error_id}") @@ -615,7 +615,7 @@ def test_get_import_errors__file_registered_only_in_other_bundle_is_unregistered # the same-named file's error in BUNDLE_NAME. set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, {"dag_in_other_bundle"}) set_mock_auth_manager__batch_is_authorized_dag(mock_get_auth_manager, True) - mock_view = set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, True) + mock_view = set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, True) response = test_client.get("/importErrors") @@ -821,7 +821,7 @@ def test_bundle_name_join_condition_for_import_errors( # IMPORT_ERRORS_ALL view -- which this caller does not hold, so the row # is hidden. (Matching on filename alone would have wrongly treated it as # registered via the same-named Dag in the other bundle.) - set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, False) + set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, False) response2 = test_client.get("/importErrors") @@ -859,7 +859,7 @@ def test_get_import_errors__no_dag_in_dagmodel( results and ``total_entries``, so the file's existence does not leak). """ set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, can_view_all_import_errors) + set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get("/importErrors") @@ -1021,7 +1021,7 @@ def test_single_endpoint_gates_unregistered_file_stacktrace_on_import_errors_all holding it see the raw stacktrace; others are denied (403). """ set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, can_view_all_import_errors) + set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get(f"/importErrors/{import_errors[0].id}") assert response.status_code == expected_status_code if expected_status_code == 200: @@ -1094,7 +1094,7 @@ def test_list_endpoint_gates_unregistered_file_stacktrace_on_import_errors_all( ``total_entries``). """ set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, can_view_all_import_errors) + set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get("/importErrors") assert response.status_code == 200 body = response.json() @@ -1140,7 +1140,7 @@ def test_single_endpoint_scopes_unregistered_file_view_to_bundle_team( the file's team, resolved from its bundle. A caller not in that team is denied (403).""" set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - mock_method = set_mock_auth_manager__is_authorized_view_for_team(mock_get_auth_manager, False) + mock_method = set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, False) response = test_client.get(f"/importErrors/{team_scoped_unregistered_import_error.id}") diff --git a/providers/amazon/src/airflow/providers/amazon/aws/auth_manager/aws_auth_manager.py b/providers/amazon/src/airflow/providers/amazon/aws/auth_manager/aws_auth_manager.py index 03bd4c79b0bfb..50558d1e4b230 100644 --- a/providers/amazon/src/airflow/providers/amazon/aws/auth_manager/aws_auth_manager.py +++ b/providers/amazon/src/airflow/providers/amazon/aws/auth_manager/aws_auth_manager.py @@ -236,7 +236,11 @@ def is_authorized_view( *, access_view: AccessView, user: AwsAuthManagerUser, + team_name: str | None = None, ) -> bool: + # ``team_name`` is accepted for interface parity and ignored: views are authorized + # against the VIEW entity, which is not team-scoped in the Verified Permissions + # schema, so a team-scoped view authorizes exactly as the global one does. return self.avp_facade.is_authorized( method="GET", entity_type=AvpEntities.VIEW, 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 c76f341791a75..f5d0c57065af4 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 @@ -520,7 +520,11 @@ def is_authorized_variable( ) -> bool: return self._is_authorized(method=method, resource_type=RESOURCE_VARIABLE, user=user) - def is_authorized_view(self, *, access_view: AccessView, user: User) -> bool: + def is_authorized_view( + self, *, access_view: AccessView, user: User, team_name: str | None = None + ) -> bool: + # ``team_name`` is accepted for interface parity and ignored: FAB has no multi-team + # support, so a team-scoped view authorizes exactly as the global one does. # "Docs" are only links in the menu, there is no page associated method: ExtendedResourceMethod = "MENU" if access_view == AccessView.DOCS else "GET" return self._is_authorized( diff --git a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py index c6bd2c5156483..fd7bfaf899078 100644 --- a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py +++ b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py @@ -313,10 +313,7 @@ def is_authorized_team( team_name=team_name, ) - def is_authorized_view(self, *, access_view: AccessView, user: KeycloakAuthManagerUser) -> bool: - return self.is_authorized_view_for_team(access_view=access_view, user=user) - - def is_authorized_view_for_team( + def is_authorized_view( self, *, access_view: AccessView, user: KeycloakAuthManagerUser, team_name: str | None = None ) -> bool: return self._is_authorized( From c30e1cf475e7fb313976412782198364dca20eb5 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Tue, 21 Jul 2026 14:17:41 +0200 Subject: [PATCH 10/12] Degrade gracefully for auth managers without team-aware is_authorized_view Folding team_name into is_authorized_view made it a hard breaking change: an out-of-tree auth manager, or a provider released before the argument existed, keeps the old (access_view, user) signature and would raise TypeError the first time core authorized a team-scoped view -- a runtime 500 with no import- or startup-time signal. Route team-scoped authorization through a new BaseAuthManager.authorize_view, which detects (once, cached) whether the manager's override accepts team_name. A legacy manager is called without it and keeps working; team scoping is ignored so the view is authorized globally -- exactly what a manager with no multi-team support does anyway -- and a RemovedInAirflow4Warning tells the operator that some views (e.g. import errors for files with no registered Dag) are visible across teams until the auth manager is upgraded. The public interface stays a single is_authorized_view; the fallback is removed in Airflow 4. --- .../newsfragments/69790.significant.rst | 21 ++++++---- .../auth/managers/base_auth_manager.py | 37 +++++++++++++++- .../core_api/routes/public/import_error.py | 4 +- .../auth/managers/test_base_auth_manager.py | 42 +++++++++++++++++++ .../routes/public/test_import_error.py | 26 ++++++------ 5 files changed, 105 insertions(+), 25 deletions(-) diff --git a/airflow-core/newsfragments/69790.significant.rst b/airflow-core/newsfragments/69790.significant.rst index 18a421c381578..58d1109939086 100644 --- a/airflow-core/newsfragments/69790.significant.rst +++ b/airflow-core/newsfragments/69790.significant.rst @@ -4,14 +4,18 @@ The abstract ``is_authorized_view`` method on ``BaseAuthManager`` gained an opti ``team_name`` keyword argument, so that views can be authorized per team the same way the other ``is_authorized_*`` methods already are through their ``details`` object. -Auth managers bundled with Airflow (Simple, FAB, Keycloak, AWS) are updated. **Out-of-tree -auth managers must add the argument to their own override.** Python's ABC machinery only -checks that the method *name* is overridden, not its signature, so an override that keeps the -old ``(access_view, user)`` signature still imports and instantiates cleanly and then raises -``TypeError: is_authorized_view() got an unexpected keyword argument 'team_name'`` at runtime, -the first time Airflow authorizes a team-scoped view. +Auth managers bundled with Airflow (Simple, FAB, Keycloak, AWS) are updated. **Auth managers +that are not team-aware keep working but degrade gracefully.** Core no longer calls +``is_authorized_view`` directly on team-scoped paths; it goes through +``BaseAuthManager.authorize_view``, which detects whether the manager's override accepts +``team_name``. An override that keeps the old ``(access_view, user)`` signature -- an +out-of-tree manager, or a provider released before this change -- is still called (without +``team_name``), so it does not raise ``TypeError``. Instead it authorizes the view globally +and a ``RemovedInAirflow4Warning`` is emitted: some views that should be restricted to a +single team (such as import errors for files with no registered Dag) are visible across all +teams until the manager is upgraded. -Update the override to accept the argument: +To make an auth manager team-aware, accept the argument: .. code-block:: python @@ -20,4 +24,5 @@ Update the override to accept the argument: ) -> bool: ... Managers without multi-team support can accept ``team_name`` and ignore it -- that authorizes -the view globally, which is the behaviour they had before this change. +the view globally, which is the behaviour they had before this change. The graceful fallback +for legacy signatures is removed in Airflow 4, which will require the ``team_name`` argument. 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 71667a1dfdb9e..7a9fdc3f7b02c 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 @@ -17,12 +17,13 @@ # under the License. from __future__ import annotations +import inspect import logging import warnings from abc import ABCMeta, abstractmethod from collections import defaultdict from enum import Enum -from functools import cache +from functools import cache, cached_property from typing import TYPE_CHECKING, Any, Generic, Literal, TypeVar from jwt import InvalidTokenError @@ -44,6 +45,7 @@ ) from airflow.api_fastapi.common.types import ExtraMenuItem, MenuItem from airflow.configuration import conf +from airflow.exceptions import RemovedInAirflow4Warning from airflow.models import Connection, DagModel, Pool, Variable from airflow.models.dagbundle import DagBundleModel from airflow.models.revoked_token import RevokedToken @@ -371,6 +373,39 @@ def is_authorized_view( :param team_name: team the view is scoped to, if any """ + @cached_property + def _is_authorized_view_team_aware(self) -> bool: + """Whether this manager's ``is_authorized_view`` override accepts ``team_name``.""" + params = inspect.signature(self.is_authorized_view).parameters + return "team_name" in params or any(p.kind is inspect.Parameter.VAR_KEYWORD for p in params.values()) + + def authorize_view(self, *, access_view: AccessView, user: T, team_name: str | None = None) -> bool: + """ + Authorize a read-only view, tolerating auth managers that predate ``team_name``. + + Core calls this rather than :meth:`is_authorized_view` directly on any team-scoped + path. An auth manager whose ``is_authorized_view`` override still has the old + ``(access_view, user)`` signature — an out-of-tree manager, or a provider released + before ``team_name`` was added — would otherwise raise ``TypeError`` at call time. + Here it falls back to calling without ``team_name`` (global authorization, which is + exactly what a manager with no multi-team support does anyway) and warns. The + fallback is removed in Airflow 4. + """ + 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 some views that " + "should be restricted to a single team — such as import errors for files with no " + "registered Dag — are instead authorized across all teams and may be visible to " + "users of other teams. Upgrade to a version of this auth manager that supports team " + "scoping (for a provider auth manager, upgrade the provider; for a custom one, add " + "the 'team_name' argument to is_authorized_view). Airflow 4 will require team-aware " + "auth managers.", + RemovedInAirflow4Warning, + stacklevel=2, + ) + return self.is_authorized_view(access_view=access_view, user=user) + @abstractmethod def is_authorized_custom_view(self, *, method: ResourceMethod, resource_name: str, user: T) -> bool: """ diff --git a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py index d40c5832dccdf..842789274c56a 100644 --- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py +++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/import_error.py @@ -112,7 +112,7 @@ def get_import_error( team_name = ( DagBundleModel.get_team_name(error.bundle_name, session=session) if error.bundle_name else None ) - if not auth_manager.is_authorized_view( + if not auth_manager.authorize_view( access_view=AccessView.IMPORT_ERRORS_ALL, user=user, team_name=team_name ): raise HTTPException( @@ -195,7 +195,7 @@ def get_import_errors( authorized_unregistered_ids = { error_id for error_id, bundle_name in unregistered_errors - if auth_manager.is_authorized_view( + if auth_manager.authorize_view( access_view=AccessView.IMPORT_ERRORS_ALL, user=user, team_name=team_name_by_bundle.get(bundle_name) if bundle_name else None, 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 cdeb0791c60bd..5ca46908e3a69 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 @@ -35,6 +35,7 @@ ) from airflow.api_fastapi.auth.tokens import JWTGenerator, JWTValidator from airflow.api_fastapi.common.types import MenuItem +from airflow.exceptions import RemovedInAirflow4Warning from airflow.models.team import Team from tests_common.test_utils.config import conf_vars @@ -161,6 +162,47 @@ class TestBaseAuthManager: def test_init_non_multi_team_mode(self, auth_manager): assert auth_manager.init() is None + @patch.object(EmptyAuthManager, "is_authorized_view") + def test_authorize_view_passes_team_name_to_team_aware_manager( + self, mock_is_authorized_view, auth_manager + ): + mock_is_authorized_view.return_value = True + + result = auth_manager.authorize_view(access_view=AccessView.DOCS, user=None, team_name="team_a") + + assert result is True + mock_is_authorized_view.assert_called_once_with( + access_view=AccessView.DOCS, user=None, team_name="team_a" + ) + + def test_authorize_view_drops_team_name_and_warns_for_legacy_manager(self): + # A manager whose is_authorized_view predates team_name (out-of-tree, or a provider + # released before the argument existed) must keep working: authorize_view falls back + # to a global check and warns that some views are not team-restricted. + class LegacyAuthManager(EmptyAuthManager): + def is_authorized_view(self, *, access_view, user=None): # old signature, no team_name + return True + + manager = LegacyAuthManager() + + with pytest.warns(RemovedInAirflow4Warning, match="not team-aware"): + result = manager.authorize_view(access_view=AccessView.DOCS, user=None, team_name="team_a") + + assert result 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): + return kwargs.get("team_name") == "team_a" + + manager = KwargsAuthManager() + + with warnings.catch_warnings(): + warnings.simplefilter("error") # a warning here would fail the test + result = manager.authorize_view(access_view=AccessView.DOCS, user=None, team_name="team_a") + + assert result is True + @conf_vars({("core", "multi_team"): "True"}) @pytest.mark.parametrize( ("auth_manager_teams", "db_teams", "expected"), diff --git a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py index d53fbcaa41f8e..e703d98b2ad35 100644 --- a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py +++ b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_import_error.py @@ -153,11 +153,11 @@ def set_mock_auth_manager__batch_is_authorized_dag( return mock_batch_is_authorized_dag -def set_mock_auth_manager__is_authorized_view( - mock_auth_manager: mock.Mock, is_authorized_view_return_value: bool = False +def set_mock_auth_manager__authorize_view( + mock_auth_manager: mock.Mock, authorize_view_return_value: bool = False ) -> mock.Mock: - mock_method = mock_auth_manager.return_value.is_authorized_view - mock_method.return_value = is_authorized_view_return_value + mock_method = mock_auth_manager.return_value.authorize_view + mock_method.return_value = authorize_view_return_value return mock_method @@ -293,9 +293,7 @@ def test_get_import_error__no_dag_in_dagmodel( """ import_error_id = import_errors[0].id set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - mock_method = set_mock_auth_manager__is_authorized_view( - mock_get_auth_manager, can_view_all_import_errors - ) + mock_method = set_mock_auth_manager__authorize_view(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get(f"/importErrors/{import_error_id}") @@ -348,7 +346,7 @@ def test_get_import_error__file_registered_only_in_other_bundle_is_unregistered( # Readable other-bundle Dag must not grant visibility into the # same-named file's error in BUNDLE_NAME. set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, {"dag_in_other_bundle"}) - mock_method = set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, False) + mock_method = set_mock_auth_manager__authorize_view(mock_get_auth_manager, False) response = test_client.get(f"/importErrors/{import_error_id}") @@ -615,7 +613,7 @@ def test_get_import_errors__file_registered_only_in_other_bundle_is_unregistered # the same-named file's error in BUNDLE_NAME. set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, {"dag_in_other_bundle"}) set_mock_auth_manager__batch_is_authorized_dag(mock_get_auth_manager, True) - mock_view = set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, True) + mock_view = set_mock_auth_manager__authorize_view(mock_get_auth_manager, True) response = test_client.get("/importErrors") @@ -821,7 +819,7 @@ def test_bundle_name_join_condition_for_import_errors( # IMPORT_ERRORS_ALL view -- which this caller does not hold, so the row # is hidden. (Matching on filename alone would have wrongly treated it as # registered via the same-named Dag in the other bundle.) - set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, False) + set_mock_auth_manager__authorize_view(mock_get_auth_manager, False) response2 = test_client.get("/importErrors") @@ -859,7 +857,7 @@ def test_get_import_errors__no_dag_in_dagmodel( results and ``total_entries``, so the file's existence does not leak). """ set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, can_view_all_import_errors) + set_mock_auth_manager__authorize_view(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get("/importErrors") @@ -1021,7 +1019,7 @@ def test_single_endpoint_gates_unregistered_file_stacktrace_on_import_errors_all holding it see the raw stacktrace; others are denied (403). """ set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, can_view_all_import_errors) + set_mock_auth_manager__authorize_view(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get(f"/importErrors/{import_errors[0].id}") assert response.status_code == expected_status_code if expected_status_code == 200: @@ -1094,7 +1092,7 @@ def test_list_endpoint_gates_unregistered_file_stacktrace_on_import_errors_all( ``total_entries``). """ set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, can_view_all_import_errors) + set_mock_auth_manager__authorize_view(mock_get_auth_manager, can_view_all_import_errors) response = test_client.get("/importErrors") assert response.status_code == 200 body = response.json() @@ -1140,7 +1138,7 @@ def test_single_endpoint_scopes_unregistered_file_view_to_bundle_team( the file's team, resolved from its bundle. A caller not in that team is denied (403).""" set_mock_auth_manager__get_authorized_dag_ids(mock_get_auth_manager, set()) - mock_method = set_mock_auth_manager__is_authorized_view(mock_get_auth_manager, False) + mock_method = set_mock_auth_manager__authorize_view(mock_get_auth_manager, False) response = test_client.get(f"/importErrors/{team_scoped_unregistered_import_error.id}") From effcb0c370db7cd6e2766d3cb4398e160c7942d7 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Tue, 21 Jul 2026 14:32:48 +0200 Subject: [PATCH 11/12] Mark amazon common-compat dependency with '# use next version' This PR changes the common.compat provider alongside the amazon, fab and keycloak providers. When common.compat changes with other providers in the same PR, each provider depending on it must carry a '# use next version' comment so the release manager bumps the constraint to the not-yet-released common.compat at release time. fab and keycloak already had it; amazon was pulled into the changed set by the auth manager update and was missing it, which the selective-checks Build info job flagged. --- providers/amazon/pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/providers/amazon/pyproject.toml b/providers/amazon/pyproject.toml index eddb2645e6080..cd3a7e9d3a91a 100644 --- a/providers/amazon/pyproject.toml +++ b/providers/amazon/pyproject.toml @@ -60,7 +60,7 @@ requires-python = ">=3.10" # After you modify the dependencies, and rebuild your Breeze CI image with ``breeze ci-image build`` dependencies = [ "apache-airflow>=2.11.0", - "apache-airflow-providers-common-compat>=1.14.3", + "apache-airflow-providers-common-compat>=1.14.3", # use next version "apache-airflow-providers-common-sql>=1.32.0", "apache-airflow-providers-http", # We should update minimum version of boto3 and here regularly to avoid `pip` backtracking with the number From c82875fa7719e663b9ebc7bbb29567e6c35b0607 Mon Sep 17 00:00:00 2001 From: Jarek Potiuk Date: Tue, 21 Jul 2026 15:29:24 +0200 Subject: [PATCH 12/12] Document team_name auth-manager support and simplify the changelog note MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The team_name argument no longer forces a hard break — auth managers that predate it keep working with a deprecation warning — so it is a feature rather than a significant/breaking change. Replace the significant newsfragment with a single feature note that states when team_name became available (Airflow 3.4.0) and which bundled auth managers support it, and move the implementation detail (the authorize_view fallback, the deprecation, how to make a manager team-aware) into the auth manager documentation where implementers will look for it. --- .../docs/core-concepts/auth-manager/index.rst | 37 ++++++++++++++++++- airflow-core/newsfragments/69790.feature.rst | 2 +- .../newsfragments/69790.significant.rst | 28 -------------- 3 files changed, 37 insertions(+), 30 deletions(-) delete mode 100644 airflow-core/newsfragments/69790.significant.rst diff --git a/airflow-core/docs/core-concepts/auth-manager/index.rst b/airflow-core/docs/core-concepts/auth-manager/index.rst index cdcd4bdd2cde6..e72623fa6d6ad 100644 --- a/airflow-core/docs/core-concepts/auth-manager/index.rst +++ b/airflow-core/docs/core-concepts/auth-manager/index.rst @@ -140,13 +140,48 @@ These authorization methods are: * ``is_authorized_asset_alias``: Return whether the user is authorized to access Airflow asset aliases. Some details about the asset alias can be provided (e.g. the asset alias ID). * ``is_authorized_pool``: Return whether the user is authorized to access Airflow pools. Some details about the pool can be provided (e.g. the pool name). * ``is_authorized_variable``: Return whether the user is authorized to access Airflow variables. Some details about the variable can be provided (e.g. the variable key). -* ``is_authorized_view``: Return whether the user is authorized to access a specific view in Airflow. The view is specified through ``access_view`` (e.g. ``AccessView.CLUSTER_ACTIVITY``). An optional ``team_name`` scopes the check to a team; auth managers without multi-team support accept it and ignore it, which authorizes the view globally. +* ``is_authorized_view``: Return whether the user is authorized to access a specific view in Airflow. The view is specified through ``access_view`` (e.g. ``AccessView.CLUSTER_ACTIVITY``). An optional ``team_name`` scopes the check to a team -- see :ref:`team-scoped-view-authorization` below. * ``is_authorized_custom_view``: Return whether the user is authorized to access a specific view not defined in Airflow. This view can be provided by the auth manager itself or a plugin defined by the user. * ``filter_authorized_menu_items``: Given the list of menu items in the UI, return the list of menu items the user has access to. It should be noted that the ``method`` parameter listed above may only have relevance for a specific subset of the auth manager's authorization methods. For example, the ``configuration`` resource is by definition read-only, so only the ``GET`` parameter is relevant in the context of ``is_authorized_configuration``. +.. _team-scoped-view-authorization: + +Team-scoped view authorization +"""""""""""""""""""""""""""""" + +.. versionadded:: 3.4.0 + ``is_authorized_view`` accepts an optional ``team_name`` argument. + +In a multi-team deployment, access to a read-only view can be restricted to the users of a +specific team. ``is_authorized_view`` accepts an optional ``team_name`` for that purpose. An +auth manager that implements multi-team isolation honors it and only authorizes users who +belong to ``team_name``; an auth manager without multi-team support accepts the argument and +ignores it, which authorizes the view across all teams (the same behaviour it had before the +argument existed). + +Core never calls ``is_authorized_view`` with ``team_name`` directly. It goes through +``BaseAuthManager.authorize_view``, which first checks whether the auth manager's +``is_authorized_view`` accepts ``team_name``. This keeps auth managers that predate the +argument working: a custom or out-of-tree auth manager whose ``is_authorized_view`` still has +the old ``(access_view, user)`` signature is called *without* ``team_name`` -- it does not +raise -- and a ``RemovedInAirflow4Warning`` is emitted, warning that some views that should be +restricted to a single team are instead authorized across all teams until the auth manager is +upgraded. + +To make an auth manager team-aware, add ``team_name`` to the override (managers without +multi-team support may accept and ignore it): + +.. code-block:: python + + def is_authorized_view( + self, *, access_view: AccessView, user: MyUser, team_name: str | None = None + ) -> bool: ... + +The fallback for the older signature is removed in Airflow 4, which requires ``team_name``. + JWT token management by auth managers ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ The auth manager is responsible for creating the JWT token needed to interact with Airflow public API. diff --git a/airflow-core/newsfragments/69790.feature.rst b/airflow-core/newsfragments/69790.feature.rst index 682849eca21b1..04e9bde8794ab 100644 --- a/airflow-core/newsfragments/69790.feature.rst +++ b/airflow-core/newsfragments/69790.feature.rst @@ -1 +1 @@ -Add a dedicated ``IMPORT_ERRORS_ALL`` permission (admin by default, team-scoped) gating visibility of import errors for files with no registered Dag; callers without it are denied those errors instead of receiving the raw stack trace. +Auth managers can scope read-only view authorization to a team via an optional ``team_name`` argument on ``is_authorized_view``, added to the auth manager interface in Airflow 3.4.0. It is supported by the Simple auth manager (shipped in Airflow) from 3.4.0, and by the Amazon, FAB, and Keycloak providers from the releases after 9.32.0, 3.7.2, and 0.8.1 respectively; older provider releases and auth managers that have not adopted the argument keep working but cannot restrict views per team. See :doc:`the auth manager documentation ` for details. diff --git a/airflow-core/newsfragments/69790.significant.rst b/airflow-core/newsfragments/69790.significant.rst deleted file mode 100644 index 58d1109939086..0000000000000 --- a/airflow-core/newsfragments/69790.significant.rst +++ /dev/null @@ -1,28 +0,0 @@ -``BaseAuthManager.is_authorized_view`` takes an optional ``team_name`` - -The abstract ``is_authorized_view`` method on ``BaseAuthManager`` gained an optional -``team_name`` keyword argument, so that views can be authorized per team the same way the -other ``is_authorized_*`` methods already are through their ``details`` object. - -Auth managers bundled with Airflow (Simple, FAB, Keycloak, AWS) are updated. **Auth managers -that are not team-aware keep working but degrade gracefully.** Core no longer calls -``is_authorized_view`` directly on team-scoped paths; it goes through -``BaseAuthManager.authorize_view``, which detects whether the manager's override accepts -``team_name``. An override that keeps the old ``(access_view, user)`` signature -- an -out-of-tree manager, or a provider released before this change -- is still called (without -``team_name``), so it does not raise ``TypeError``. Instead it authorizes the view globally -and a ``RemovedInAirflow4Warning`` is emitted: some views that should be restricted to a -single team (such as import errors for files with no registered Dag) are visible across all -teams until the manager is upgraded. - -To make an auth manager team-aware, accept the argument: - -.. code-block:: python - - def is_authorized_view( - self, *, access_view: AccessView, user: MyUser, team_name: str | None = None - ) -> bool: ... - -Managers without multi-team support can accept ``team_name`` and ignore it -- that authorizes -the view globally, which is the behaviour they had before this change. The graceful fallback -for legacy signatures is removed in Airflow 4, which will require the ``team_name`` argument.