diff --git a/lib/galaxy/managers/rbac_secured.py b/lib/galaxy/managers/rbac_secured.py index 2a053b57545..e90a50e2959 100644 --- a/lib/galaxy/managers/rbac_secured.py +++ b/lib/galaxy/managers/rbac_secured.py @@ -173,6 +173,10 @@ class ManageDatasetRBACPermission( DatasetRBACPermission ): # anonymous users cannot manage permissions on datasets if self.user_manager.is_anonymous( user ): return False + # 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(): if self._role_is_permitted( dataset, role ): return True @@ -225,7 +229,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 is always permitted + self.user_manager.is_admin( user ) or self._user_has_all_roles( user, current_roles ) ) def grant( self, item, user ): @@ -241,14 +247,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 +265,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 5b1349995e5..def81bb8684 100644 --- a/test/unit/managers/test_DatasetManager.py +++ b/test/unit/managers/test_DatasetManager.py @@ -112,6 +112,16 @@ 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 ) ) + + 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" ) @@ -135,6 +145,16 @@ 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 ) ) + + 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 ) @@ -162,6 +182,16 @@ 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 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 ) ) + + 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 ): @@ -248,7 +278,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 +288,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() @@ -350,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 } ) @@ -374,6 +412,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