This is an automated email from the ASF dual-hosted git repository.
ephraimanierobi pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new b594a8dcc0 Fix custom actions in security manager `has_access` (#39421)
b594a8dcc0 is described below
commit b594a8dcc0ec1a3bd8a07ab5828ead62ee14b015
Author: Jed Cunningham <66968678+jedcunning...@users.noreply.github.com>
AuthorDate: Mon May 6 03:37:59 2024 -0400
Fix custom actions in security manager `has_access` (#39421)
---
airflow/www/security_manager.py | 2 +-
tests/www/test_security_manager.py | 36 +++++++++++++++++++++++++++++-------
2 files changed, 30 insertions(+), 8 deletions(-)
diff --git a/airflow/www/security_manager.py b/airflow/www/security_manager.py
index 88c2caa32a..316daace7d 100644
--- a/airflow/www/security_manager.py
+++ b/airflow/www/security_manager.py
@@ -337,7 +337,7 @@ class AirflowSecurityManagerV2(LoggingMixin):
# The user is trying to access a page specific to the auth manager
# (e.g. the user list view in FabAuthManager) or a page defined in
a plugin
return lambda action, resource_pk, user:
get_auth_manager().is_authorized_custom_view(
- method=get_method_from_fab_action_map()[action],
+ method=get_method_from_fab_action_map().get(action, action),
resource_name=fab_resource_name,
user=user,
)
diff --git a/tests/www/test_security_manager.py
b/tests/www/test_security_manager.py
index 81a05e5fd0..2051ccef09 100644
--- a/tests/www/test_security_manager.py
+++ b/tests/www/test_security_manager.py
@@ -50,13 +50,14 @@ def security_manager(app_builder):
@pytest.mark.db_test
class TestAirflowSecurityManagerV2:
@pytest.mark.parametrize(
- "resource_name, auth_manager_methods, expected",
+ "action_name, resource_name, auth_manager_methods, expected",
[
- (RESOURCE_VARIABLE, {"is_authorized_variable": True}, True),
- (RESOURCE_VARIABLE, {"is_authorized_variable": False}, False),
- (RESOURCE_DOCS_MENU, {"is_authorized_view": True}, True),
- (RESOURCE_DOCS_MENU, {"is_authorized_view": False}, False),
+ (ACTION_CAN_READ, RESOURCE_VARIABLE, {"is_authorized_variable":
True}, True),
+ (ACTION_CAN_READ, RESOURCE_VARIABLE, {"is_authorized_variable":
False}, False),
+ (ACTION_CAN_READ, RESOURCE_DOCS_MENU, {"is_authorized_view":
True}, True),
+ (ACTION_CAN_READ, RESOURCE_DOCS_MENU, {"is_authorized_view":
False}, False),
(
+ ACTION_CAN_READ,
RESOURCE_ADMIN_MENU,
{
"is_authorized_view": False,
@@ -69,6 +70,7 @@ class TestAirflowSecurityManagerV2:
True,
),
(
+ ACTION_CAN_READ,
RESOURCE_ADMIN_MENU,
{
"is_authorized_view": False,
@@ -81,6 +83,7 @@ class TestAirflowSecurityManagerV2:
False,
),
(
+ ACTION_CAN_READ,
RESOURCE_BROWSE_MENU,
{
"is_authorized_dag": False,
@@ -89,6 +92,7 @@ class TestAirflowSecurityManagerV2:
False,
),
(
+ ACTION_CAN_READ,
RESOURCE_BROWSE_MENU,
{
"is_authorized_dag": False,
@@ -96,11 +100,29 @@ class TestAirflowSecurityManagerV2:
},
True,
),
+ (
+ "can_not_a_default_action",
+ "custom_resource",
+ {"is_authorized_custom_view": False},
+ False,
+ ),
+ (
+ "can_not_a_default_action",
+ "custom_resource",
+ {"is_authorized_custom_view": True},
+ True,
+ ),
],
)
@mock.patch("airflow.www.security_manager.get_auth_manager")
def test_has_access(
- self, mock_get_auth_manager, security_manager, resource_name,
auth_manager_methods, expected
+ self,
+ mock_get_auth_manager,
+ security_manager,
+ action_name,
+ resource_name,
+ auth_manager_methods,
+ expected,
):
user = Mock()
auth_manager = Mock()
@@ -108,7 +130,7 @@ class TestAirflowSecurityManagerV2:
method = Mock(return_value=method_return)
getattr(auth_manager, method_name).side_effect = method
mock_get_auth_manager.return_value = auth_manager
- result = security_manager.has_access(ACTION_CAN_READ, resource_name,
user=user)
+ result = security_manager.has_access(action_name, resource_name,
user=user)
assert result == expected
if len(auth_manager_methods) > 1 and not expected:
for method_name in auth_manager_methods:
