From 46ea50736dc83643aef8e678ee8919e00756cade Mon Sep 17 00:00:00 2001 From: James Timmins Date: Mon, 25 Jan 2021 06:00:57 -0800 Subject: [PATCH] Don't add User role perms to custom roles. (#13856) closes: #9245 #13511 (cherry picked from commit 35b5a38313ea8ea74d86ac1643d944c468b49669) --- airflow/www/security.py | 108 ++++++++++++------------------------- airflow/www/views.py | 2 +- tests/www/test_security.py | 8 +-- tests/www/test_views.py | 4 ++ 4 files changed, 43 insertions(+), 79 deletions(-) diff --git a/airflow/www/security.py b/airflow/www/security.py index c3efc64ca1..a1253a2895 100644 --- a/airflow/www/security.py +++ b/airflow/www/security.py @@ -43,7 +43,7 @@ EXISTING_ROLES = { } -class AirflowSecurityManager(SecurityManager, LoggingMixin): +class AirflowSecurityManager(SecurityManager, LoggingMixin): # pylint: disable=too-many-public-methods """Custom security manager, which introduces an permission model adapted to Airflow""" ########################################################################### @@ -220,8 +220,8 @@ class AirflowSecurityManager(SecurityManager, LoggingMixin): return [current_app.appbuilder.sm.find_role(public_role)] if public_role else [] return user.roles - def get_all_permissions_views(self): - """Returns a set of tuples with the perm name and view menu name""" + def get_current_user_permissions(self): + """Returns permissions for logged in user as a set of tuples with the perm name and view menu name""" perms_views = set() for role in self.get_user_roles(): perms_views.update( @@ -363,7 +363,7 @@ class AirflowSecurityManager(SecurityManager, LoggingMixin): def _get_and_cache_perms(self): """Cache permissions-views""" - self.perms = self.get_all_permissions_views() + self.perms = self.get_current_user_permissions() def _has_role(self, role_name_or_list): """Whether the user has this role name""" @@ -439,89 +439,48 @@ class AirflowSecurityManager(SecurityManager, LoggingMixin): if not permission_view and permission_name and view_menu_name: self.add_permission_view_menu(permission_name, view_menu_name) - @provide_session - def create_custom_dag_permission_view(self, session=None): + def add_homepage_access_to_custom_roles(self): """ - Workflow: - 1. Fetch all the existing (permissions, view-menu) from Airflow DB. - 2. Fetch all the existing dag models that are either active or paused. - 3. Create both read and write permission view-menus relation for every dags from step 2 - 4. Find out all the dag specific roles(excluded pubic, admin, viewer, op, user) - 5. Get all the permission-vm owned by the user role. - 6. Grant all the user role's permission-vm except the all-dag view-menus to the dag roles. - 7. Commit the updated permission-vm-role into db + Add Website.can_read access to all roles. :return: None. """ - self.log.debug('Fetching a set of all permission, view_menu from FAB meta-table') + website_permission = self.add_permission_view_menu( + permissions.ACTION_CAN_READ, permissions.RESOURCE_WEBSITE + ) + for role in self.get_all_roles(): + self.add_permission_role(role, website_permission) - def merge_pv(perm, view_menu): - """Create permission view menu only if it doesn't exist""" - if view_menu and perm and (view_menu, perm) not in all_permission_views: - self._merge_perm(perm, view_menu) + self.get_session.commit() - all_permission_views = set() + def get_all_permissions(self): + """Returns all permissions as a set of tuples with the perm name and view menu name""" + perms = set() for permission_view in self.get_session.query(self.permissionview_model).all(): if permission_view.permission and permission_view.view_menu: - all_permission_views.add((permission_view.permission.name, permission_view.view_menu.name)) + perms.add((permission_view.permission.name, permission_view.view_menu.name)) - # Get all the active / paused dags and insert them into a set - all_dags_models = ( + return perms + + @provide_session + def create_dag_specific_permissions(self, session=None): + """ + Creates 'can_read' and 'can_edit' permissions for all active and paused DAGs. + + :return: None. + """ + perms = self.get_all_permissions() + dag_models = ( session.query(models.DagModel) .filter(or_(models.DagModel.is_active, models.DagModel.is_paused)) .all() ) - # create can_edit and can_read permissions for every dag(vm) - for dag in all_dags_models: - for perm in self.DAG_PERMS: - merge_pv(perm, self.prefixed_dag_id(dag.dag_id)) - - # for all the dag-level role, add the permission of viewer - # with the dag view to ab_permission_view - all_roles = self.get_all_roles() - user_role = self.find_role('User') - - dag_role = [role for role in all_roles if role.name not in EXISTING_ROLES] - update_perm_views = [] - - # need to remove all_dag vm from all the existing view-menus - dag_vm = self.find_view_menu(permissions.RESOURCE_DAG) - ab_perm_view_role = sqla_models.assoc_permissionview_role - perm_view = self.permissionview_model - view_menu = self.viewmenu_model - - all_perm_view_by_user = ( - session.query(ab_perm_view_role) - .join( - perm_view, - perm_view.id == ab_perm_view_role.columns.permission_view_id, # pylint: disable=no-member - ) - .filter(ab_perm_view_role.columns.role_id == user_role.id) # pylint: disable=no-member - .join(view_menu) - .filter(perm_view.view_menu_id != dag_vm.id) - ) - all_perm_views = {role.permission_view_id for role in all_perm_view_by_user} - - for role in dag_role: - # pylint: disable=no-member - # Get all the perm-view of the role - - existing_perm_view_by_user = self.get_session.query(ab_perm_view_role).filter( - ab_perm_view_role.columns.role_id == role.id - ) - - existing_perms_views = {pv.permission_view_id for pv in existing_perm_view_by_user} - missing_perm_views = all_perm_views - existing_perms_views - - for perm_view_id in missing_perm_views: - update_perm_views.append({'permission_view_id': perm_view_id, 'role_id': role.id}) - - if update_perm_views: - self.get_session.execute( - ab_perm_view_role.insert(), update_perm_views # pylint: disable=no-value-for-parameter - ) - self.get_session.commit() + for dag in dag_models: + for perm_name in self.DAG_PERMS: + dag_resource_name = self.prefixed_dag_id(dag.dag_id) + if dag_resource_name and perm_name and (dag_resource_name, perm_name) not in perms: + self._merge_perm(perm_name, dag_resource_name) def update_admin_perm_view(self): """ @@ -560,13 +519,14 @@ class AirflowSecurityManager(SecurityManager, LoggingMixin): """ # Create global all-dag VM self.create_perm_vm_for_all_dag() + self.create_dag_specific_permissions() # Create default user role. for config in self.ROLE_CONFIGS: role = config['role'] perms = config['perms'] self.init_role(role, perms) - self.create_custom_dag_permission_view() + self.add_homepage_access_to_custom_roles() # init existing roles, the rest role could be created through UI. self.update_admin_perm_view() self.clean_perms() diff --git a/airflow/www/views.py b/airflow/www/views.py index d3d57f9f7a..e5b732d289 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -528,7 +528,7 @@ class Airflow(AirflowBaseView): # noqa: D101 pylint: disable=too-many-public-m .all() ) - user_permissions = current_app.appbuilder.sm.get_all_permissions_views() + user_permissions = current_app.appbuilder.sm.get_current_user_permissions() all_dags_editable = (permissions.ACTION_CAN_EDIT, permissions.RESOURCE_DAG) in user_permissions for dag in dags: diff --git a/tests/www/test_security.py b/tests/www/test_security.py index a8ec939a49..27b0aba68b 100644 --- a/tests/www/test_security.py +++ b/tests/www/test_security.py @@ -226,11 +226,11 @@ class TestSecurity(unittest.TestCase): assert perms_views == viewer_role_perms @mock.patch('airflow.www.security.AirflowSecurityManager.get_user_roles') - def test_get_all_permissions_views(self, mock_get_user_roles): + def test_get_current_user_permissions(self, mock_get_user_roles): role_name = 'MyRole5' role_perm = 'can_some_action' role_vm = 'SomeBaseView' - username = 'get_all_permissions_views' + username = 'get_current_user_permissions' with self.app.app_context(): user = fab_utils.create_user( @@ -244,10 +244,10 @@ class TestSecurity(unittest.TestCase): role = user.roles[0] mock_get_user_roles.return_value = [role] - assert self.security_manager.get_all_permissions_views() == {(role_perm, role_vm)} + assert self.security_manager.get_current_user_permissions() == {(role_perm, role_vm)} mock_get_user_roles.return_value = [] - assert len(self.security_manager.get_all_permissions_views()) == 0 + assert len(self.security_manager.get_current_user_permissions()) == 0 def test_get_accessible_dag_ids(self): role_name = 'MyRole1' diff --git a/tests/www/test_views.py b/tests/www/test_views.py index c101eb6c87..0a56b067d7 100644 --- a/tests/www/test_views.py +++ b/tests/www/test_views.py @@ -1824,6 +1824,10 @@ class TestDagACLView(TestBase): permissions.ACTION_CAN_READ, permissions.RESOURCE_DAG ) self.appbuilder.sm.add_permission_role(all_dag_role, read_perm_on_all_dag) + read_perm_on_task_instance = self.appbuilder.sm.find_permission_view_menu( + permissions.ACTION_CAN_READ, permissions.RESOURCE_TASK_INSTANCE + ) + self.appbuilder.sm.add_permission_role(all_dag_role, read_perm_on_task_instance) self.appbuilder.sm.add_permission_role(all_dag_role, website_permission) role_user = self.appbuilder.sm.find_role('User')