diff --git a/airflow-core/docs/core-concepts/auth-manager/index.rst b/airflow-core/docs/core-concepts/auth-manager/index.rst index b821fbfa135bc..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``). +* ``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 new file mode 100644 index 0000000000000..04e9bde8794ab --- /dev/null +++ b/airflow-core/newsfragments/69790.feature.rst @@ -0,0 +1 @@ +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/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..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 @@ -357,13 +359,52 @@ def is_authorized_view( *, access_view: AccessView, user: T, + team_name: str | None = None, ) -> bool: """ Return whether the user is authorized to access a read-only state of the installation. + 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 to performing the action - """ + :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/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..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 @@ -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, @@ -352,8 +351,17 @@ 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(method="GET", allow_role=SimpleAuthManagerRole.VIEWER, user=user) + 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 + # 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..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 @@ -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.authorize_view( + 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? @@ -157,7 +171,36 @@ 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`` + # 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) + & (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( + {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.authorize_view( + 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 @@ -195,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, @@ -206,7 +252,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 +307,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..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 @@ -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, @@ -34,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 @@ -41,7 +43,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, @@ -132,7 +133,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,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 e7be4fa3219ea..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 @@ -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__authorize_view( + mock_auth_manager: mock.Mock, authorize_view_return_value: bool = False +) -> mock.Mock: + mock_method = mock_auth_manager.return_value.authorize_view + mock_method.return_value = authorize_view_return_value + return mock_method + + class TestGetImportError: @pytest.mark.parametrize( ("prepared_import_error_idx", "expected_status_code", "expected_body"), @@ -259,28 +270,93 @@ 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__authorize_view(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.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 + ) + + @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__authorize_view(mock_get_auth_manager, False) + + response = test_client.get(f"/importErrors/{import_error_id}") + + assert response.status_code == 403 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, + "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: @@ -373,7 +449,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 @@ -491,6 +570,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__authorize_view(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 @@ -678,6 +814,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__authorize_view(mock_get_auth_manager, False) + response2 = test_client.get("/importErrors") # Assert - should return 0 entries because bundle_name no longer matches @@ -686,29 +829,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__authorize_view(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 +997,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__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 == 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 +1063,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__authorize_view(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__authorize_view(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/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 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/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/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..1804ff96e24a6 --- /dev/null +++ b/providers/common/compat/tests/unit/common/compat/security/test_access_view.py @@ -0,0 +1,60 @@ +# 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_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 + + 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(): + """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)) 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/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..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 @@ -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 @@ -77,6 +78,7 @@ RESOURCE_DAG_WARNING, RESOURCE_DOCS, RESOURCE_IMPORT_ERROR, + RESOURCE_IMPORT_ERROR_ALL, RESOURCE_JOB, RESOURCE_PLUGIN, RESOURCE_POOL, @@ -146,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, @@ -512,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/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/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", ] 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..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,12 +313,15 @@ def is_authorized_team( team_name=team_name, ) - def is_authorized_view(self, *, access_view: AccessView, user: KeycloakAuthManagerUser) -> bool: + def is_authorized_view( + 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(