From 91a190c6eb2f37e041575548bfae85d61b64ce7e Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Mon, 12 Sep 2016 10:50:57 -0400 Subject: [PATCH 1/6] Datasets: allow admin to serialize permissions And updates tests. --- lib/galaxy/managers/datasets.py | 3 ++- test/unit/managers/test_DatasetManager.py | 11 ++++++++++- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/managers/datasets.py b/lib/galaxy/managers/datasets.py index 44e5dbe4b21..526ab620a58 100644 --- a/lib/galaxy/managers/datasets.py +++ b/lib/galaxy/managers/datasets.py @@ -199,7 +199,8 @@ class DatasetSerializer( base.ModelSerializer, deletable.PurgableSerializerMixin def serialize_permissions( self, dataset, key, user=None, **context ): """ """ - if not self.dataset_manager.permissions.manage.is_permitted( dataset, user ): + is_admin = self.user_manager.is_admin( user ) + if not is_admin and 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/test/unit/managers/test_DatasetManager.py b/test/unit/managers/test_DatasetManager.py index 5b1349995e5..7ed0446ff5e 100644 --- a/test/unit/managers/test_DatasetManager.py +++ b/test/unit/managers/test_DatasetManager.py @@ -162,6 +162,11 @@ 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 ) ) + # ============================================================================= class DatasetRBACPermissionsTestCase( BaseTestCase ): @@ -248,7 +253,6 @@ class DatasetSerializerTestCase( BaseTestCase ): role_id = self.app.security.decode_id( role_id ) role = self.role_manager.get( self.trans, role_id ) self.assertTrue( who_manages in [ user_role.user for user_role in role.users ]) - # wat self.log( 'permissions should be not returned for non-managing users' ) not_my_supervisor = self.user_manager.create( **user3_data ) @@ -259,6 +263,11 @@ class DatasetSerializerTestCase( BaseTestCase ): self.assertRaises( SkipAttribute, self.dataset_serializer.serialize_permissions, dataset, 'perms', user=None ) + self.log( 'permissions should be returned for admin users' ) + permissions = self.dataset_serializer.serialize_permissions( dataset, 'perms', user=self.admin_user ) + self.assertIsInstance( permissions, dict ) + self.assertKeys( permissions, [ 'manage', 'access' ] ) + def test_serializers( self ): # self.user_manager.create( **user2_data ) dataset = self.dataset_manager.create() From b88035955d7a81ff43858f092924b60bef1bca9c Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Mon, 12 Sep 2016 11:04:39 -0400 Subject: [PATCH 2/6] Datasets: confirm admin ability to set permissions The power was inside you all along. --- test/unit/managers/test_DatasetManager.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/test/unit/managers/test_DatasetManager.py b/test/unit/managers/test_DatasetManager.py index 7ed0446ff5e..a0c4e1c320d 100644 --- a/test/unit/managers/test_DatasetManager.py +++ b/test/unit/managers/test_DatasetManager.py @@ -383,6 +383,28 @@ class DatasetDeserializerTestCase( BaseTestCase ): permissions = self.dataset_serializer.serialize_permissions( dataset, 'perms', user=who_manages ) self.assertEqual( new_manage_permissions, permissions[ 'manage' ] ) + def test_deserialize_permissions_with_admin( self ): + dataset = self.dataset_manager.create() + who_manages = self.user_manager.create( **user2_data ) + self.dataset_manager.permissions.manage.grant( dataset, who_manages ) + existing_permissions = self.dataset_serializer.serialize_permissions( dataset, 'permissions', user=who_manages ) + existing_manage_permissions = existing_permissions[ 'manage' ] + + user3 = self.user_manager.create( **user3_data ) + self.assertRaises( rbac_secured.DatasetManagePermissionFailedException, self.dataset_deserializer.deserialize, + dataset, user=user3, data={ 'permissions': existing_permissions }) + + self.log( 'deserializing permissions using an admin user should not error' ) + private_role = self.user_manager.private_role( who_manages ) + private_role = private_role.to_dict( value_mapper={ 'id' : self.app.security.encode_id } ) + permissions = dict( manage=existing_manage_permissions, access=[ private_role[ 'id' ] ] ) + self.dataset_deserializer.deserialize( dataset, user=who_manages, data={ + 'permissions': permissions + }) + + self.assertRaises( rbac_secured.DatasetManagePermissionFailedException, self.dataset_deserializer.deserialize, + dataset, user=user3, data={ 'permissions': existing_permissions }) + # ============================================================================= # NOTE: that we test the DatasetAssociation* classes in either test_HDAManager or test_LDAManager From fc3dcf3b875cbadf75a1eb34ce1395e29dbe0baf Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Tue, 13 Sep 2016 13:00:58 -0400 Subject: [PATCH 3/6] 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 ) ) # ============================================================================= From 9636ea1b6cb1283ef9e5d8cb210a962eff45bbef Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Tue, 13 Sep 2016 13:09:11 -0400 Subject: [PATCH 4/6] Datasets: test for anon access/manage --- test/unit/managers/test_DatasetManager.py | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/test/unit/managers/test_DatasetManager.py b/test/unit/managers/test_DatasetManager.py index 0cc0a7b2b47..ed2d8737513 100644 --- a/test/unit/managers/test_DatasetManager.py +++ b/test/unit/managers/test_DatasetManager.py @@ -117,6 +117,11 @@ class DatasetManagerTestCase( BaseTestCase ): 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 ) ) + self.log( "a dataset without permissions shouldn't be manageable by an anonymous user" ) + self.assertFalse( self.dataset_manager.permissions.manage.is_permitted( dataset, None ) ) + self.log( "a dataset without permissions should be accessible by an anonymous user" ) + self.assertTrue( self.dataset_manager.permissions.access.is_permitted( dataset, None ) ) + 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" ) @@ -145,6 +150,11 @@ class DatasetManagerTestCase( BaseTestCase ): self.log( "a public dataset should be accessible by an admin" ) self.assertTrue( self.dataset_manager.permissions.access.is_permitted( dataset, self.admin_user ) ) + self.log( "a public dataset shouldn't be manageable by an anonymous user" ) + self.assertFalse( self.dataset_manager.permissions.manage.is_permitted( dataset, None ) ) + self.log( "a public dataset should be accessible by an anonymous user" ) + self.assertTrue( self.dataset_manager.permissions.access.is_permitted( dataset, None ) ) + 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 ) @@ -177,6 +187,11 @@ class DatasetManagerTestCase( BaseTestCase ): self.log( "a private dataset should be accessible by an admin" ) self.assertTrue( self.dataset_manager.permissions.access.is_permitted( dataset, self.admin_user ) ) + self.log( "a private dataset shouldn't be manageable by an anonymous user" ) + self.assertFalse( self.dataset_manager.permissions.manage.is_permitted( dataset, None ) ) + self.log( "a private dataset shouldn't be accessible by an anonymous user" ) + self.assertFalse( self.dataset_manager.permissions.access.is_permitted( dataset, None ) ) + # ============================================================================= class DatasetRBACPermissionsTestCase( BaseTestCase ): From e7cb0ac2a19f0141336285fdd8fcf9aeb32221f2 Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Tue, 13 Sep 2016 13:10:24 -0400 Subject: [PATCH 5/6] Datasets: test for anon deserialization --- test/unit/managers/test_DatasetManager.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/unit/managers/test_DatasetManager.py b/test/unit/managers/test_DatasetManager.py index ed2d8737513..def81bb8684 100644 --- a/test/unit/managers/test_DatasetManager.py +++ b/test/unit/managers/test_DatasetManager.py @@ -384,6 +384,10 @@ class DatasetDeserializerTestCase( BaseTestCase ): self.assertRaises( rbac_secured.DatasetManagePermissionFailedException, self.dataset_deserializer.deserialize, dataset, user=user3, data={ 'permissions': existing_permissions }) + self.log( 'deserializing permissions using an anon user should error' ) + self.assertRaises( rbac_secured.DatasetManagePermissionFailedException, self.dataset_deserializer.deserialize, + dataset, user=None, data={ 'permissions': existing_permissions }) + self.log( 'deserializing permissions with a single access should make the dataset private' ) private_role = self.user_manager.private_role( who_manages ) private_role = private_role.to_dict( value_mapper={ 'id' : self.app.security.encode_id } ) From b301fd42021245f5c3c7291207510659aa66b5ac Mon Sep 17 00:00:00 2001 From: carlfeberhard Date: Wed, 14 Sep 2016 13:18:46 -0400 Subject: [PATCH 6/6] Dataset perms: correct comment --- lib/galaxy/managers/rbac_secured.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/managers/rbac_secured.py b/lib/galaxy/managers/rbac_secured.py index 2420e2cccfb..e90a50e2959 100644 --- a/lib/galaxy/managers/rbac_secured.py +++ b/lib/galaxy/managers/rbac_secured.py @@ -173,7 +173,8 @@ class ManageDatasetRBACPermission( DatasetRBACPermission ): # anonymous users cannot manage permissions on datasets if self.user_manager.is_anonymous( user ): return False - # admin can always manager permissions + # admin is always permitted + # TODO: could probably move this into RBACPermission and call that first if self.user_manager.is_admin( user ): return True for role in user.all_roles(): @@ -229,7 +230,7 @@ class AccessDatasetRBACPermission( DatasetRBACPermission ): # NOTE: that because of short circuiting this allows # anonymous access to public datasets return ( self._is_public_based_on_roles( current_roles ) or - # admin can always manager permissions + # admin is always permitted self.user_manager.is_admin( user ) or self._user_has_all_roles( user, current_roles ) )