Repository: openwisp/openwisp-users
Issue category: Security Vulnerability

Issue:
[bug/security] Org managers can escalate themselves to superusers
**Expected behavior:** org manager (non superuser) should not be able to edit permissions, nor flag themselves or others are superusers.

**Actual behavior**: org manager (non superuser) are able to flag themselves or others are superusers.

This is a regression, this was working fine before, but for some reason, the user permission section in the user management admin page got duplicated and the duplicated part allows users to flag themselves as superusers or assign themselves new permissions.

See the recording below for clarification:

![users-bug](https://user-images.githubusercontent.com/841044/137373951-d719b394-e445-414e-89d0-21cf011d5388.gif)

The second permission section is duplicated and shouldn't be there.

**We must add the following admin tests:**

- when accessing user change page as a non superuser who has permissions to edit an user:
  - ensure there's no mention of superuser at all (eg: `is_superuser`)
  - ensure user permissions are not editable
  - ensure there's only one is_active field, only one is_staff field, only one instnace of group editing 
[bug/security] Org managers can escalate themselves to superusers
**Expected behavior:** org manager (non superuser) should not be able to edit permissions, nor flag themselves or others are superusers.

**Actual behavior**: org manager (non superuser) are able to flag themselves or others are superusers.

This is a regression, this was working fine before, but for some reason, the user permission section in the user management admin page got duplicated and the duplicated part allows users to flag themselves as superusers or assign themselves new permissions.

See the recording below for clarification:

![users-bug](https://user-images.githubusercontent.com/841044/137373951-d719b394-e445-414e-89d0-21cf011d5388.gif)

The second permission section is duplicated and shouldn't be there.

**We must add the following admin tests:**

- when accessing user change page as a non superuser who has permissions to edit an user:
  - ensure there's no mention of superuser at all (eg: `is_superuser`)
  - ensure user permissions are not editable
  - ensure there's only one is_active field, only one is_staff field, only one instnace of group editing 


Hints:

Patch:
diff --git a/openwisp_users/admin.py b/openwisp_users/admin.py
index 8619e39d..f0cb45aa 100644
--- a/openwisp_users/admin.py
+++ b/openwisp_users/admin.py
@@ -318,7 +318,7 @@ def get_readonly_fields(self, request, obj=None):
         # do not allow operators to escalate their privileges
         if not request.user.is_superuser:
             # copy to avoid modifying reference
-            fields = fields[:] + ['user_permissions']
+            fields = fields[:] + ['user_permissions', 'is_superuser']
         return fields
 
     def has_change_permission(self, request, obj=None):
@@ -491,7 +491,7 @@ def queryset(self, request, queryset):
 base_fields = list(UserAdmin.fieldsets[1][1]['fields'])
 additional_fields = ['bio', 'url', 'company', 'location', 'phone_number', 'birth_date']
 UserAdmin.fieldsets[1][1]['fields'] = base_fields + additional_fields
-UserAdmin.fieldsets.insert(2, ('Internal', {'fields': ('notes',)}))
+UserAdmin.fieldsets.insert(3, ('Internal', {'fields': ('notes',)}))
 UserAdmin.add_fieldsets[0][1]['fields'] = (
     'username',
     'email',


Test patch:
diff --git a/openwisp_users/tests/test_admin.py b/openwisp_users/tests/test_admin.py
index f6e4e767..0c4157d3 100644
--- a/openwisp_users/tests/test_admin.py
+++ b/openwisp_users/tests/test_admin.py
@@ -261,6 +261,21 @@ def test_admin_change_non_superuser_readonly_fields(self):
             html = 'class="readonly"><img src="/static/admin/img/icon'
             self.assertContains(response, html)
 
+    def test_org_manager_privilege_escalation(self):
+        operator = self._create_operator()
+        options = {
+            'organization': self._get_org(),
+            'is_admin': True,
+            'user': self._get_operator(),
+        }
+        self._create_org_user(**options)
+        self.client.force_login(operator)
+        response = self.client.get(
+            reverse(f'admin:{self.app_label}_user_change', args=[operator.pk])
+        )
+        self.assertNotContains(response, 'superuser')
+        self.assertNotContains(response, 'id_user_permissions_from')
+
     def test_admin_changelist_user_superusers_hidden(self):
         self._create_admin()
         operator = self._create_operator()