Cleanup user permissions check, fix bug to allow admin to change own username, first_name and last_name.

awx/main/access.py

@@ -130,10 +130,11 @@ class UserAccess(BaseAccess):
     def can_change(self, obj, data):
         # A user can be changed if they are themselves, or by org admins or
         # superusers.
+        if self.user.is_superuser:
+            return True
         if self.user == obj:
             return 'partial'
-        return bool(self.user.is_superuser or 
+        return bool(obj.organizations.filter(admins__in=[self.user]).count())
-                    obj.organizations.filter(admins__in=[self.user]).count())
 
     def can_delete(self, obj):
         return bool(self.user.is_superuser or 

awx/main/models/__init__.py

@@ -78,39 +78,6 @@ PERMISSION_TYPE_CHOICES = [
     (PERM_INVENTORY_CHECK, _('Deploy To Inventory (Dry Run)')),
 ]
 
-class EditHelper(object):
-
-    @classmethod
-    def illegal_changes(cls, request, obj, model_class):
-        ''' have any illegal changes been made (for a PUT request)? '''
-        from awx.main.access import check_user_access
-        #can_admin = model_class.can_user_administrate(request.user, obj, request.DATA)
-        can_admin = check_user_access(request.user, User, 'change', obj, request.DATA)
-        if (not can_admin) or (can_admin == 'partial'):
-            check_fields = model_class.admin_only_edit_fields
-            changed = cls.fields_changed(check_fields, obj, request.DATA)
-            if len(changed.keys()) > 0:
-                return True
-        return False
-
-    @classmethod
-    def fields_changed(cls, fields, obj, data):
-        ''' return the fields that would be changed by a prospective PUT operation '''
-        changed = {}
-        for f in fields:
-            left = getattr(obj, f, None)
-            if left is None:
-                raise Exception("internal error, %s is not a member of %s" % (f, obj))
-            right = data.get(f, None)
-            if (right is not None) and (left != right):
-                changed[f] = (left, right)
-        return changed
-
-class UserHelper(object):
-
-    # fields that the user themselves cannot edit, but are not actually read only
-    admin_only_edit_fields = ('last_name', 'first_name', 'username', 'is_active', 'is_superuser')
-
 class PrimordialModel(models.Model):
     '''
     common model for all object types that have these standard fields

awx/main/tests/users.py

@@ -178,6 +178,17 @@ class UsersTest(BaseTest):
         self.assertEquals(data['results'][0]['username'], 'admin')
         self.assertEquals(data['count'], 1)
 
+    def test_superuser_can_change_admin_only_fields_about_himself(self):
+        url = reverse('main:user_detail', args=(self.super_django_user.pk,))
+        data = self.get(url, expect=200, auth=self.get_super_credentials())
+        data['username'] += '2'
+        data['first_name'] += ' Awesome'
+        data['last_name'] += ', Jr.'
+        response = self.put(url, data, expect=200,
+                            auth=self.get_super_credentials())
+        # FIXME: Test if super user mark himself as no longer a super user, or
+        # delete himself.
+
     def test_user_related_resources(self):
 
         # organizations the user is a member of, should be 1

awx/main/views.py

@@ -609,8 +609,19 @@ class UserDetail(BaseDetail):
     def put_filter(self, request, *args, **kwargs):
         ''' make sure non-read-only fields that can only be edited by admins, are only edited by admins '''
         obj = User.objects.get(pk=kwargs['pk'])
-        if EditHelper.illegal_changes(request, obj, UserHelper):
+        can_admin = check_user_access(request.user, User, 'admin', obj, request.DATA)
-            raise PermissionDenied()
+        if not can_admin or can_admin == 'partial':
+            admin_only_edit_fields = ('last_name', 'first_name', 'username',
+                                      'is_active', 'is_superuser')
+            changed = {}
+            for field in admin_only_edit_fields:
+                left = getattr(obj, field, None)
+                right = request.DATA.get(field, None)
+                if left is not None and right is not None and left != right:
+                    changed[field] = (left, right)
+            if changed:
+                raise PermissionDenied('Cannot change %s' % ', '.join(changed.keys()))
+
         if 'password' in request.DATA:
             obj.set_password(request.DATA['password'])
             obj.save()