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

@@ -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 )

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] ) ) )

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 ) )
 
 
 # =============================================================================