From 11cf6f3ef4f7e39c3a634cb17dc50c190dbee582 Mon Sep 17 00:00:00 2001 From: James Timmins Date: Thu, 10 Jun 2021 14:19:39 -0700 Subject: [PATCH] Swap out calls to find_permission_view_menu for get_permission wrapper. (#16377) --- .../2c6edca13270_resource_based_permissions.py | 4 ++-- ...8c147f_remove_can_read_permission_on_config_.py | 4 ++-- ...ad25_resource_based_permissions_for_default_.py | 4 ++-- airflow/www/security.py | 2 +- tests/test_utils/api_connexion_utils.py | 2 +- tests/www/test_security.py | 8 +++----- tests/www/views/test_views_acl.py | 14 +++++++------- 7 files changed, 18 insertions(+), 20 deletions(-) diff --git a/airflow/migrations/versions/2c6edca13270_resource_based_permissions.py b/airflow/migrations/versions/2c6edca13270_resource_based_permissions.py index fdba3f945079f..54b397ac0f024 100644 --- a/airflow/migrations/versions/2c6edca13270_resource_based_permissions.py +++ b/airflow/migrations/versions/2c6edca13270_resource_based_permissions.py @@ -289,7 +289,7 @@ def remap_permissions(): appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder for old, new in mapping.items(): (old_view_name, old_perm_name) = old - old_pvm = appbuilder.sm.find_permission_view_menu(old_perm_name, old_view_name) + old_pvm = appbuilder.sm.get_permission(old_perm_name, old_view_name) if not old_pvm: continue for new_perm_name, new_view_name in new: @@ -303,7 +303,7 @@ def remap_permissions(): if not appbuilder.sm.find_permission(old_perm_name): continue view_menus = appbuilder.sm.get_all_view_menu() - if not any(appbuilder.sm.find_permission_view_menu(old_perm_name, view.name) for view in view_menus): + if not any(appbuilder.sm.get_permission(old_perm_name, view.name) for view in view_menus): appbuilder.sm.del_permission(old_perm_name) diff --git a/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py b/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py index 85d0872704570..8803c871b0fed 100644 --- a/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py +++ b/airflow/migrations/versions/82b7c48c147f_remove_can_read_permission_on_config_.py @@ -42,7 +42,7 @@ def upgrade(): appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder roles_to_modify = [role for role in appbuilder.sm.get_all_roles() if role.name in ["User", "Viewer"]] - can_read_on_config_perm = appbuilder.sm.find_permission_view_menu( + can_read_on_config_perm = appbuilder.sm.get_permission( permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG ) @@ -59,7 +59,7 @@ def downgrade(): """Add can_read permission on config resource for User and Viewer role""" appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder roles_to_modify = [role for role in appbuilder.sm.get_all_roles() if role.name in ["User", "Viewer"]] - can_read_on_config_perm = appbuilder.sm.find_permission_view_menu( + can_read_on_config_perm = appbuilder.sm.get_permission( permissions.ACTION_CAN_READ, permissions.RESOURCE_CONFIG ) diff --git a/airflow/migrations/versions/a13f7613ad25_resource_based_permissions_for_default_.py b/airflow/migrations/versions/a13f7613ad25_resource_based_permissions_for_default_.py index bf868390dc573..c918b30ab1d2b 100644 --- a/airflow/migrations/versions/a13f7613ad25_resource_based_permissions_for_default_.py +++ b/airflow/migrations/versions/a13f7613ad25_resource_based_permissions_for_default_.py @@ -141,7 +141,7 @@ def remap_permissions(): appbuilder = create_app(config={'FAB_UPDATE_PERMS': False}).appbuilder for old, new in mapping.items(): (old_view_name, old_perm_name) = old - old_pvm = appbuilder.sm.find_permission_view_menu(old_perm_name, old_view_name) + old_pvm = appbuilder.sm.get_permission(old_perm_name, old_view_name) if not old_pvm: continue for new_perm_name, new_view_name in new: @@ -155,7 +155,7 @@ def remap_permissions(): if not appbuilder.sm.find_permission(old_perm_name): continue view_menus = appbuilder.sm.get_all_view_menu() - if not any(appbuilder.sm.find_permission_view_menu(old_perm_name, view.name) for view in view_menus): + if not any(appbuilder.sm.get_permission(old_perm_name, view.name) for view in view_menus): appbuilder.sm.del_permission(old_perm_name) diff --git a/airflow/www/security.py b/airflow/www/security.py index 3dfa49d408875..67d7fea81fd7a 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -686,7 +686,7 @@ def create_dag_specific_permissions(self) -> None: self._merge_perm(action_name, dag_resource_name) if dag.access_control: - self._sync_dag_view_permissions(dag_resource_name, dag.access_control) + self.sync_perm_for_dag(dag_resource_name, dag.access_control) def update_admin_permission(self): """ diff --git a/tests/test_utils/api_connexion_utils.py b/tests/test_utils/api_connexion_utils.py index 46a428bcd2a23..1f20b4f723a00 100644 --- a/tests/test_utils/api_connexion_utils.py +++ b/tests/test_utils/api_connexion_utils.py @@ -44,7 +44,7 @@ def create_role(app, name, permissions=None): if not permissions: permissions = [] for permission in permissions: - perm_object = appbuilder.sm.find_permission_view_menu(*permission) + perm_object = appbuilder.sm.get_permission(*permission) appbuilder.sm.add_permission_role(role, perm_object) return role diff --git a/tests/www/test_security.py b/tests/www/test_security.py index c563a61ddd237..b230b867942c1 100644 --- a/tests/www/test_security.py +++ b/tests/www/test_security.py @@ -190,9 +190,7 @@ def test_update_and_verify_permission_role(self): self.security_manager.bulk_sync_roles(mock_roles) role = self.security_manager.find_role(role_name) - perm = self.security_manager.find_permission_view_menu( - permissions.ACTION_CAN_EDIT, permissions.RESOURCE_ROLE - ) + perm = self.security_manager.get_permission(permissions.ACTION_CAN_EDIT, permissions.RESOURCE_ROLE) self.security_manager.add_permission_role(role, perm) role_perms_len = len(role.permissions) @@ -410,11 +408,11 @@ def test_sync_perm_for_dag_creates_permissions_on_view_menus(self): prefixed_test_dag_id = f'DAG:{test_dag_id}' self.security_manager.sync_perm_for_dag(test_dag_id, access_control=None) assert ( - self.security_manager.find_permission_view_menu(permissions.ACTION_CAN_READ, prefixed_test_dag_id) + self.security_manager.get_permission(permissions.ACTION_CAN_READ, prefixed_test_dag_id) is not None ) assert ( - self.security_manager.find_permission_view_menu(permissions.ACTION_CAN_EDIT, prefixed_test_dag_id) + self.security_manager.get_permission(permissions.ACTION_CAN_EDIT, prefixed_test_dag_id) is not None ) diff --git a/tests/www/views/test_views_acl.py b/tests/www/views/test_views_acl.py index 07bb980c1584f..5964081bef376 100644 --- a/tests/www/views/test_views_acl.py +++ b/tests/www/views/test_views_acl.py @@ -85,31 +85,31 @@ def acl_app(app): # FIXME: Clean up this block of code..... - website_permission = security_manager.find_permission_view_menu( + website_permission = security_manager.get_permission( permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE ) dag_tester_role = security_manager.find_role('dag_acl_tester') - edit_perm_on_dag = security_manager.find_permission_view_menu( + edit_perm_on_dag = security_manager.get_permission( permissions.ACTION_CAN_EDIT, 'DAG:example_bash_operator' ) security_manager.add_permission_role(dag_tester_role, edit_perm_on_dag) - read_perm_on_dag = security_manager.find_permission_view_menu( + read_perm_on_dag = security_manager.get_permission( permissions.ACTION_CAN_READ, 'DAG:example_bash_operator' ) security_manager.add_permission_role(dag_tester_role, read_perm_on_dag) security_manager.add_permission_role(dag_tester_role, website_permission) all_dag_role = security_manager.find_role('all_dag_role') - edit_perm_on_all_dag = security_manager.find_permission_view_menu( + edit_perm_on_all_dag = security_manager.get_permission( permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG ) security_manager.add_permission_role(all_dag_role, edit_perm_on_all_dag) - read_perm_on_all_dag = security_manager.find_permission_view_menu( + read_perm_on_all_dag = security_manager.get_permission( permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG ) security_manager.add_permission_role(all_dag_role, read_perm_on_all_dag) - read_perm_on_task_instance = security_manager.find_permission_view_menu( + read_perm_on_task_instance = security_manager.get_permission( permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE ) security_manager.add_permission_role(all_dag_role, read_perm_on_task_instance) @@ -120,7 +120,7 @@ def acl_app(app): security_manager.add_permission_role(role_user, edit_perm_on_all_dag) security_manager.add_permission_role(role_user, website_permission) - read_only_perm_on_dag = security_manager.find_permission_view_menu( + read_only_perm_on_dag = security_manager.get_permission( permissions.ACTION_CAN_READ, 'DAG:example_bash_operator' ) dag_read_only_role = security_manager.find_role('dag_acl_read_only')