From 71da07a7032e9dcd0d7111e034e8de138b4f9d3b Mon Sep 17 00:00:00 2001 From: vincbeck Date: Mon, 4 Mar 2024 10:35:47 -0500 Subject: [PATCH 1/3] Add "MENU" permission in auth manager --- airflow/auth/managers/base_auth_manager.py | 2 +- airflow/auth/managers/utils/fab.py | 2 +- .../providers/fab/auth_manager/fab_auth_manager.py | 11 +---------- airflow/www/security_manager.py | 4 +--- .../fab/auth_manager/test_fab_auth_manager.py | 4 ++-- 5 files changed, 6 insertions(+), 17 deletions(-) diff --git a/airflow/auth/managers/base_auth_manager.py b/airflow/auth/managers/base_auth_manager.py index 88164b644e81..a0176fc8337a 100644 --- a/airflow/auth/managers/base_auth_manager.py +++ b/airflow/auth/managers/base_auth_manager.py @@ -57,7 +57,7 @@ from airflow.www.extensions.init_appbuilder import AirflowAppBuilder from airflow.www.security_manager import AirflowSecurityManagerV2 -ResourceMethod = Literal["GET", "POST", "PUT", "DELETE"] +ResourceMethod = Literal["GET", "POST", "PUT", "DELETE", "MENU"] class BaseAuthManager(LoggingMixin): diff --git a/airflow/auth/managers/utils/fab.py b/airflow/auth/managers/utils/fab.py index 22b572e07f50..0f2bc7c46e69 100644 --- a/airflow/auth/managers/utils/fab.py +++ b/airflow/auth/managers/utils/fab.py @@ -36,6 +36,7 @@ "GET": ACTION_CAN_READ, "PUT": ACTION_CAN_EDIT, "DELETE": ACTION_CAN_DELETE, + "MENU": ACTION_CAN_ACCESS_MENU, } @@ -48,5 +49,4 @@ def get_method_from_fab_action_map(): """Return the map associating a FAB action to a method.""" return { **{v: k for k, v in _MAP_METHOD_NAME_TO_FAB_ACTION_NAME.items()}, - ACTION_CAN_ACCESS_MENU: "GET", } diff --git a/airflow/providers/fab/auth_manager/fab_auth_manager.py b/airflow/providers/fab/auth_manager/fab_auth_manager.py index 90109950b3ba..992c23112860 100644 --- a/airflow/providers/fab/auth_manager/fab_auth_manager.py +++ b/airflow/providers/fab/auth_manager/fab_auth_manager.py @@ -54,8 +54,6 @@ from airflow.providers.fab.auth_manager.models import Permission, Role, User from airflow.security import permissions from airflow.security.permissions import ( - ACTION_CAN_ACCESS_MENU, - ACTION_CAN_READ, RESOURCE_AUDIT_LOG, RESOURCE_CLUSTER_ACTIVITY, RESOURCE_CONFIG, @@ -463,18 +461,11 @@ def _get_user_permissions(user: BaseUser): """ Return the user permissions. - ACTION_CAN_READ and ACTION_CAN_ACCESS_MENU are merged into because they are very similar. - We can assume that if a user has permissions to read variables, they also have permissions to access - the menu "Variables". - :param user: the user to get permissions for :meta private: """ - perms = getattr(user, "perms") or [] - return [ - (ACTION_CAN_READ if perm[0] == ACTION_CAN_ACCESS_MENU else perm[0], perm[1]) for perm in perms - ] + return getattr(user, "perms") or [] def _get_root_dag_id(self, dag_id: str) -> str: """ diff --git a/airflow/www/security_manager.py b/airflow/www/security_manager.py index 5cd9c159238f..bd315359833e 100644 --- a/airflow/www/security_manager.py +++ b/airflow/www/security_manager.py @@ -39,8 +39,6 @@ from airflow.exceptions import AirflowException from airflow.models import Connection, DagRun, Pool, TaskInstance, Variable from airflow.security.permissions import ( - ACTION_CAN_ACCESS_MENU, - ACTION_CAN_READ, RESOURCE_ADMIN_MENU, RESOURCE_AUDIT_LOG, RESOURCE_BROWSE_MENU, @@ -340,7 +338,7 @@ def _get_auth_manager_is_authorized_method(self, fab_resource_name: str) -> Call # This means the page the user is trying to access is specific to the auth manager used # Example: the user list view in FabAuthManager return lambda action, resource_pk, user: get_auth_manager().is_authorized_custom_view( - fab_action_name=ACTION_CAN_READ if action == ACTION_CAN_ACCESS_MENU else action, + fab_action_name=action, fab_resource_name=fab_resource_name, user=user, ) diff --git a/tests/providers/fab/auth_manager/test_fab_auth_manager.py b/tests/providers/fab/auth_manager/test_fab_auth_manager.py index 65a6fb8f5603..8b16fedf8356 100644 --- a/tests/providers/fab/auth_manager/test_fab_auth_manager.py +++ b/tests/providers/fab/auth_manager/test_fab_auth_manager.py @@ -140,10 +140,10 @@ def test_is_logged_in(self, mock_get_user, auth_manager): [(ACTION_CAN_DELETE, resource_type), (ACTION_CAN_CREATE, "resource_test")], True, ), - # With permission (testing that ACTION_CAN_ACCESS_MENU gives GET permissions) + # With permission ( api_name, - "GET", + "MENU", [(ACTION_CAN_ACCESS_MENU, resource_type)], True, ), From e42e4aa0492bf4f91d7304142436f9a4caa33b0b Mon Sep 17 00:00:00 2001 From: vincbeck Date: Mon, 4 Mar 2024 16:03:39 -0500 Subject: [PATCH 2/3] Fix docs menu link --- .../providers/fab/auth_manager/fab_auth_manager.py | 4 +++- .../fab/auth_manager/test_fab_auth_manager.py | 13 +++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/airflow/providers/fab/auth_manager/fab_auth_manager.py b/airflow/providers/fab/auth_manager/fab_auth_manager.py index 992c23112860..7ee42c2f083b 100644 --- a/airflow/providers/fab/auth_manager/fab_auth_manager.py +++ b/airflow/providers/fab/auth_manager/fab_auth_manager.py @@ -261,8 +261,10 @@ def is_authorized_variable( return self._is_authorized(method=method, resource_type=RESOURCE_VARIABLE, user=user) def is_authorized_view(self, *, access_view: AccessView, user: BaseUser | None = None) -> bool: + # Docs are just links the menu and not pages + method: ResourceMethod = "MENU" if access_view == AccessView.DOCS else "GET" return self._is_authorized( - method="GET", resource_type=_MAP_ACCESS_VIEW_TO_FAB_RESOURCE_TYPE[access_view], user=user + method=method, resource_type=_MAP_ACCESS_VIEW_TO_FAB_RESOURCE_TYPE[access_view], user=user ) def is_authorized_custom_view( diff --git a/tests/providers/fab/auth_manager/test_fab_auth_manager.py b/tests/providers/fab/auth_manager/test_fab_auth_manager.py index 8b16fedf8356..89c63525432a 100644 --- a/tests/providers/fab/auth_manager/test_fab_auth_manager.py +++ b/tests/providers/fab/auth_manager/test_fab_auth_manager.py @@ -39,6 +39,7 @@ RESOURCE_DAG, RESOURCE_DAG_RUN, RESOURCE_DATASET, + RESOURCE_DOCS, RESOURCE_JOB, RESOURCE_PLUGIN, RESOURCE_PROVIDER, @@ -346,6 +347,18 @@ def test_is_authorized_dag( [(ACTION_CAN_READ, RESOURCE_TRIGGER)], False, ), + # Docs (positive) + ( + AccessView.DOCS, + [(ACTION_CAN_ACCESS_MENU, RESOURCE_DOCS)], + True, + ), + # Without permission + ( + AccessView.DOCS, + [(ACTION_CAN_READ, RESOURCE_DOCS)], + False, + ), ], ) def test_is_authorized_view(self, access_view, user_permissions, expected_result, auth_manager): From ddeab68f12e66ac7185aa5c8c8a0f2862b616ac2 Mon Sep 17 00:00:00 2001 From: vincbeck Date: Tue, 5 Mar 2024 10:29:14 -0500 Subject: [PATCH 3/3] Rephrase comment --- airflow/providers/fab/auth_manager/fab_auth_manager.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/airflow/providers/fab/auth_manager/fab_auth_manager.py b/airflow/providers/fab/auth_manager/fab_auth_manager.py index 7ee42c2f083b..1d3506c3df7a 100644 --- a/airflow/providers/fab/auth_manager/fab_auth_manager.py +++ b/airflow/providers/fab/auth_manager/fab_auth_manager.py @@ -261,7 +261,7 @@ def is_authorized_variable( return self._is_authorized(method=method, resource_type=RESOURCE_VARIABLE, user=user) def is_authorized_view(self, *, access_view: AccessView, user: BaseUser | None = None) -> bool: - # Docs are just links the menu and not pages + # "Docs" are only links in the menu, there is no page associated method: ResourceMethod = "MENU" if access_view == AccessView.DOCS else "GET" return self._is_authorized( method=method, resource_type=_MAP_ACCESS_VIEW_TO_FAB_RESOURCE_TYPE[access_view], user=user