From fc3dcf3b875cbadf75a1eb34ce1395e29dbe0baf Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Tue, 13 Sep 2016 13:00:58 -0400 Subject: [PATCH] Permissions, datasets: allow admin in RBAC - Admin are considered permitted when calling ** dataset_manager.permssions.access and ** dataset_manager.permissions.manage - Update bad test - Add admin test to other permission checks --- lib/galaxy/managers/datasets.py | 3 +-- lib/galaxy/managers/rbac_secured.py | 13 +++++++++---- test/unit/managers/test_DatasetManager.py | 18 ++++++++++++++---- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/lib/galaxy/managers/datasets.py b/lib/galaxy/managers/datasets.py index 526ab620a58..44e5dbe4b21 100644 --- a/lib/galaxy/managers/datasets.py +++ b/lib/galaxy/managers/datasets.py @@ -199,8 +199,7 @@ class DatasetSerializer( base.ModelSerializer, deletable.PurgableSerializerMixin def serialize_permissions( self, dataset, key, user=None, **context ): """ """ - is_admin = self.user_manager.is_admin( user ) - if not is_admin and not self.dataset_manager.permissions.manage.is_permitted( dataset, user ): + if not self.dataset_manager.permissions.manage.is_permitted( dataset, user ): self.skip() management_permissions = self.dataset_manager.permissions.manage.by_dataset( dataset ) diff --git a/lib/galaxy/managers/rbac_secured.py b/lib/galaxy/managers/rbac_secured.py index 2a053b57545..2420e2cccfb 100644 --- a/lib/galaxy/managers/rbac_secured.py +++ b/lib/galaxy/managers/rbac_secured.py @@ -173,6 +173,9 @@ class ManageDatasetRBACPermission( DatasetRBACPermission ): # anonymous users cannot manage permissions on datasets if self.user_manager.is_anonymous( user ): return False + # admin can always manager permissions + if self.user_manager.is_admin( user ): + return True for role in user.all_roles(): if self._role_is_permitted( dataset, role ): return True @@ -225,7 +228,9 @@ class AccessDatasetRBACPermission( DatasetRBACPermission ): current_roles = self._roles( dataset ) # NOTE: that because of short circuiting this allows # anonymous access to public datasets - return ( self._is_public_from_roles( current_roles ) or + return ( self._is_public_based_on_roles( current_roles ) or + # admin can always manager permissions + self.user_manager.is_admin( user ) or self._user_has_all_roles( user, current_roles ) ) def grant( self, item, user ): @@ -241,14 +246,14 @@ class AccessDatasetRBACPermission( DatasetRBACPermission ): # TODO: these are a lil off message def is_public( self, dataset ): current_roles = self._roles( dataset ) - return self._is_public_from_roles( current_roles ) + return self._is_public_based_on_roles( current_roles ) def set_private( self, dataset, user, flush=True ): private_role = self.user_manager.private_role( user ) return self.set( dataset, [ private_role ], flush=flush ) # ---- private - def _is_public_from_roles( self, roles ): + def _is_public_based_on_roles( self, roles ): return len( roles ) == 0 def _user_has_all_roles( self, user, roles ): @@ -259,6 +264,6 @@ class AccessDatasetRBACPermission( DatasetRBACPermission ): def _role_is_permitted( self, dataset, role ): current_roles = self._roles( dataset ) - return ( self._is_public_from_roles( current_roles ) or + return ( self._is_public_based_on_roles( current_roles ) or # if there's only one role and this is it, let em in ( ( len( current_roles ) == 1 ) and ( role == current_roles[0] ) ) ) diff --git a/test/unit/managers/test_DatasetManager.py b/test/unit/managers/test_DatasetManager.py index a0c4e1c320d..0cc0a7b2b47 100644 --- a/test/unit/managers/test_DatasetManager.py +++ b/test/unit/managers/test_DatasetManager.py @@ -112,6 +112,11 @@ class DatasetManagerTestCase( BaseTestCase ): self.log( "a dataset without permissions should be accessible" ) self.assertTrue( self.dataset_manager.permissions.access.is_permitted( dataset, user3 ) ) + self.log( "a dataset without permissions should be manageable by an admin" ) + self.assertTrue( self.dataset_manager.permissions.manage.is_permitted( dataset, self.admin_user ) ) + self.log( "a dataset without permissions should be accessible by an admin" ) + self.assertTrue( self.dataset_manager.permissions.access.is_permitted( dataset, self.admin_user ) ) + def test_create_public_dataset( self ): self.log( "should be able to create a new Dataset and give it some permissions that actually, you know, " "might work if there's any justice in this universe" ) @@ -135,6 +140,11 @@ class DatasetManagerTestCase( BaseTestCase ): self.log( "a public dataset should be accessible" ) self.assertTrue( self.dataset_manager.permissions.access.is_permitted( dataset, user3 ) ) + self.log( "a public dataset should be manageable by an admin" ) + self.assertTrue( self.dataset_manager.permissions.manage.is_permitted( dataset, self.admin_user ) ) + self.log( "a public dataset should be accessible by an admin" ) + self.assertTrue( self.dataset_manager.permissions.access.is_permitted( dataset, self.admin_user ) ) + def test_create_private_dataset( self ): self.log( "should be able to create a new Dataset and give it private permissions" ) owner = self.user_manager.create( **user2_data ) @@ -162,10 +172,10 @@ class DatasetManagerTestCase( BaseTestCase ): self.log( "a private dataset shouldn't be accessible to just anyone" ) self.assertFalse( self.dataset_manager.permissions.access.is_permitted( dataset, user3 ) ) - self.log( "a private dataset shouldn be manageable by an admin" ) - self.assertFalse( self.dataset_manager.permissions.manage.is_permitted( dataset, self.admin_user ) ) - self.log( "a private dataset shouldn be accessible by an admin" ) - self.assertFalse( self.dataset_manager.permissions.access.is_permitted( dataset, self.admin_user ) ) + self.log( "a private dataset should be manageable by an admin" ) + self.assertTrue( self.dataset_manager.permissions.manage.is_permitted( dataset, self.admin_user ) ) + self.log( "a private dataset should be accessible by an admin" ) + self.assertTrue( self.dataset_manager.permissions.access.is_permitted( dataset, self.admin_user ) ) # =============================================================================