Skip to content

Commit

Permalink
Add "MENU" permission in auth manager (#37881)
Browse files Browse the repository at this point in the history
  • Loading branch information
vincbeck committed Mar 5, 2024
1 parent 1b12120 commit 89e7f3e
Show file tree
Hide file tree
Showing 5 changed files with 22 additions and 18 deletions.
2 changes: 1 addition & 1 deletion airflow/auth/managers/base_auth_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
2 changes: 1 addition & 1 deletion airflow/auth/managers/utils/fab.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
"GET": ACTION_CAN_READ,
"PUT": ACTION_CAN_EDIT,
"DELETE": ACTION_CAN_DELETE,
"MENU": ACTION_CAN_ACCESS_MENU,
}


Expand All @@ -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",
}
15 changes: 4 additions & 11 deletions airflow/providers/fab/auth_manager/fab_auth_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -263,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 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="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(
Expand Down Expand Up @@ -463,18 +463,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:
"""
Expand Down
4 changes: 1 addition & 3 deletions airflow/www/security_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
)
Expand Down
17 changes: 15 additions & 2 deletions tests/providers/fab/auth_manager/test_fab_auth_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
RESOURCE_DAG,
RESOURCE_DAG_RUN,
RESOURCE_DATASET,
RESOURCE_DOCS,
RESOURCE_JOB,
RESOURCE_PLUGIN,
RESOURCE_PROVIDER,
Expand Down Expand Up @@ -140,10 +141,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,
),
Expand Down Expand Up @@ -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):
Expand Down

0 comments on commit 89e7f3e

Please sign in to comment.