[enchancement] hide other org content from operators #32

- operator is able to see and edit users of the organizations
in which they are admin
- `is_superuser` column hidden in list view
- `is_superuser` column hidden in add/change view
- operator is only able to add/change users in
their own organization organization

Closes #32

openwisp_users/admin.py

@@ -1,3 +1,5 @@
+from copy import deepcopy
+
 from allauth.account.models import EmailAddress
 from django import forms
 from django.apps import apps
@@ -53,6 +55,21 @@ class OrganizationUserInline(admin.StackedInline):
     model = OrganizationUser
     formset = RequiredInlineFormSet
 
+    def get_formset(self, request, obj=None, **kwargs):
+        """
+        In form dropdowns, display only organizations
+        in which operator `is_admin` and for superusers
+        display all organizations
+        """
+        formset = super(OrganizationUserInline, self).get_formset(request, obj=obj, **kwargs)
+        if not request.user.is_superuser:
+            operator_orgs = OrganizationUser.objects.values_list('organization') \
+                                                    .filter(user=request.user,
+                                                            is_admin=True)
+            formset.form.base_fields['organization'].queryset = \
+                Organization.objects.filter(pk__in=operator_orgs)
+        return formset
+
     def get_extra(self, request, obj=None, **kwargs):
         if not obj:
             return 1
@@ -64,7 +81,7 @@ class EmailRequiredMixin(forms.ModelForm):
 
     def _clean_email(self, email):
         if User.objects.filter(email=email).count() > 0 or \
-         EmailAddress.objects.filter(email=email).exclude(user=self.instance.pk).count() > 0:
+                EmailAddress.objects.filter(email=email).exclude(user=self.instance.pk).count() > 0:
             raise ValidationError({'email': ['User with this email already exists.']})
 
     def clean(self):
@@ -113,22 +130,43 @@ class UserAdmin(BaseUserAdmin, BaseAdmin):
     inlines = [EmailAddressInline, OrganizationUserInline]
     save_on_top = True
 
+    def get_list_display(self, request):
+        """
+        Hide `is_superuser` from column from operators
+        """
+        default_list_display = super(UserAdmin, self).get_list_display(request)
+        if (not request.user.is_superuser and
+                'is_superuser' in default_list_display):
+            # avoid editing the default_list_display
+            operators_list_display = default_list_display[:]
+            operators_list_display.remove('is_superuser')
+            return operators_list_display
+        return default_list_display
+
     def get_fieldsets(self, request, obj=None):
         # add form fields for staff users
         if not obj and not request.user.is_superuser:
             return self.add_form.Meta.fieldsets
         # add form fields for superusers
         if not obj and request.user.is_superuser:
             return self.add_form.Meta.fieldsets_superuser
-        return super(UserAdmin, self).get_fieldsets(request, obj)
+        # return fieldsets according to user
+        fieldsets = super(UserAdmin, self).get_fieldsets(request, obj)
+        if not request.user.is_superuser:
+            # copy to avoid modifying reference
+            non_superuser_fieldsets = deepcopy(fieldsets)
+            non_superuser_fieldsets[2][1]['fields'] = tuple(
+                ('is_active', 'is_staff', 'groups', 'user_permissions'))
+            return non_superuser_fieldsets
+        return fieldsets
 
     def get_readonly_fields(self, request, obj=None):
         # retrieve readonly fields
         fields = super(UserAdmin, self).get_readonly_fields(request, obj)
         # do not allow operators to escalate their privileges
         if not request.user.is_superuser:
             # copy to avoid modifying reference
-            fields = fields[:] + ['is_superuser', 'user_permissions']
+            fields = fields[:] + ['user_permissions']
         return fields
 
     def has_change_permission(self, request, obj=None):
@@ -139,23 +177,21 @@ def has_change_permission(self, request, obj=None):
         return super(UserAdmin, self).has_change_permission(request, obj)
 
     def get_queryset(self, request):
-        # if not superuser, show only members of the same org
+        """
+        if operator is logged in - show only users
+        from same organization and hide superusers
+        if superuser is logged in - show all users
+        """
         if not request.user.is_superuser:
             user = request.user
             org_users = OrganizationUser.objects.filter(user=user) \
                                                 .select_related('organization')
-            qs = None
+            qs = User.objects.none()
             for org_user in org_users:
-                add_qs = org_user.organization.users.all()
-                if qs:
-                    # if merging an existing queryset
-                    # avoid including the current user twice
-                    qs = qs | add_qs.exclude(pk=user.id)
-                else:
-                    qs = add_qs
-                # hide superusers from organization operators
-                # so they can't edit nor delete them
-                qs = qs.filter(is_superuser=False)
+                qs = qs | org_user.organization.users.all().distinct()
+            # hide superusers from organization operators
+            # so they can't edit nor delete them
+            qs = qs.filter(is_superuser=False)
         else:
             qs = super(UserAdmin, self).get_queryset(request)
         return qs
@@ -193,7 +229,8 @@ def save_model(self, request, obj, form, change):
 base_fields = list(UserAdmin.fieldsets[1][1]['fields'])
 additional_fields = ['bio', 'url', 'company', 'location']
 UserAdmin.fieldsets[1][1]['fields'] = base_fields + additional_fields
-UserAdmin.add_fieldsets[0][1]['fields'] = ('username', 'email', 'password1', 'password2')
+UserAdmin.add_fieldsets[0][1]['fields'] = ('username', 'email',
+                                           'password1', 'password2')
 
 
 class GroupAdmin(BaseGroupAdmin, BaseAdmin):

openwisp_users/tests/__init__.py

@@ -0,0 +1,75 @@
+from django.contrib.auth.models import Permission
+from ..models import Organization, User, OrganizationUser
+
+
+class TestOrganizationMixin(object):
+    def _create_user(self, **kwargs):
+        opts = dict(username='tester',
+                    password='tester',
+                    first_name='Tester',
+                    last_name='Tester',
+                    email='[email protected]')
+        opts.update(kwargs)
+        user = User.objects.create_user(**opts)
+        return user
+
+    def _create_admin(self, **kwargs):
+        opts = dict(username='admin',
+                    email='[email protected]',
+                    is_superuser=True,
+                    is_staff=True)
+        opts.update(kwargs)
+        return self._create_user(**opts)
+
+    def _create_org(self, **kwargs):
+        options = {
+            'name': 'test org',
+            'is_active': True,
+            'slug': 'test-org'
+        }
+        options.update(kwargs)
+        org = Organization.objects.create(**options)
+        return org
+
+    def _create_operator(self):
+        operator = User.objects.create_user(username='operator',
+                                            password='tester',
+                                            email='[email protected]',
+                                            is_staff=True)
+        operator.user_permissions.add(
+            *Permission.objects.filter(codename__endswith='user'))
+        return operator
+
+    def _get_org(self, org_name='test org'):
+        try:
+            return Organization.objects.get(name=org_name)
+        except Organization.DoesNotExist:
+            return self._create_org()
+
+    def _get_user(self, username='tester'):
+        try:
+            return User.objects.get(username=username)
+        except User.DoesNotExist:
+            return self._create_user()
+
+    def _get_admin(self, username='admin'):
+        try:
+            return User.objects.get(username=username)
+        except User.DoesNotExist:
+            return self._create_admin()
+
+    def _get_operator(self, username='operator'):
+        try:
+            return User.objects.get(username=username)
+        except User.DoesNotExist:
+            return self._create_operator()
+
+    def _create_org_user(self, **kwargs):
+        options = {
+            'organization': self._get_org(),
+            'is_admin': False,
+            'user': self._get_user()
+        }
+        options.update(kwargs)
+        org = OrganizationUser.objects.create(**options)
+        return org

openwisp_users/tests/test_admin.py

@@ -1,10 +1,9 @@
-from django.contrib.auth.models import Permission
 from django.core import mail
 from django.test import TestCase
 from django.urls import reverse
+from openwisp_users.models import User
 
-from ..models import User
-from .utils import TestOrganizationMixin
+from . import TestOrganizationMixin
 
 
 class TestUsersAdmin(TestOrganizationMixin, TestCase):
@@ -20,14 +19,6 @@ class TestUsersAdmin(TestOrganizationMixin, TestCase):
         'openwisp_users_organizationuser-MAX_NUM_FORMS': 0
     }
 
-    def _create_operator(self, organizations=[]):
-        operator = User.objects.create_user(username='operator',
-                                            password='tester',
-                                            email='[email protected]',
-                                            is_staff=True)
-        operator.user_permissions.add(*Permission.objects.filter(codename__endswith='user'))
-        return operator
-
     def test_admin_add_user_auto_email(self):
         admin = self._create_admin()
         self.client.force_login(admin)
@@ -124,8 +115,14 @@ def test_admin_change_user_is_superuser_editable(self):
         html = '<input type="checkbox" name="is_superuser"'
         self.assertContains(response, html)
 
-    def test_admin_change_user_is_superuser_readonly(self):
+    def test_admin_change_user_is_superuser_absent(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('admin:openwisp_users_user_change', args=[operator.pk]))
         html = '<input type="checkbox" name="is_superuser" checked id="id_is_superuser">'
@@ -140,6 +137,12 @@ def test_admin_change_user_permissions_editable(self):
 
     def test_admin_change_user_permissions_readonly(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('admin:openwisp_users_user_change', args=[operator.pk]))
         html = '<div class="readonly">openwisp_users'
@@ -152,9 +155,48 @@ def test_admin_changelist_user_superusers_hidden(self):
         response = self.client.get(reverse('admin:openwisp_users_user_changelist'))
         self.assertNotContains(response, 'admin</a>')
 
+    def test_admin_changelist_operator_org_users_visible(self):
+        # Check with operator in same organization and is_admin
+        self._create_org_user()
+        operator = self._create_operator()
+        options = {
+            'organization': self._get_org(),
+            'is_admin': True,
+            'user': operator
+        }
+        self._create_org_user(**options)
+        self.client.force_login(operator)
+        response = self.client.get(reverse('admin:openwisp_users_user_changelist'))
+        self.assertContains(response, 'tester</a>')
+        self.assertContains(response, 'operator</a>')
+
+    def test_operator_changelist_superuser_column_hidden(self):
+        operator = self._create_operator()
+        options = {
+            'organization': self._get_org(),
+            'is_admin': True,
+            'user': operator
+        }
+        self._create_org_user(**options)
+        self.client.force_login(operator)
+        response = self.client.get(reverse('admin:openwisp_users_user_changelist'))
+        self.assertNotContains(response, 'Superuser status</a>')
+
+    def test_admin_changelist_superuser_column_visible(self):
+        admin = self._create_admin()
+        self.client.force_login(admin)
+        response = self.client.get(reverse('admin:openwisp_users_user_changelist'))
+        self.assertContains(response, 'Superuser status</a>')
+
     def test_admin_operator_change_superuser_forbidden(self):
         admin = self._create_admin()
         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('admin:openwisp_users_user_change', args=[operator.pk]))
         self.assertEqual(response.status_code, 200)

openwisp_users/tests/test_models.py

@@ -1,7 +1,7 @@
 from django.test import TestCase
+from openwisp_users.models import OrganizationUser, User
 
-from ..models import OrganizationUser, User
-from .utils import TestOrganizationMixin
+from . import TestOrganizationMixin
 
 
 class TestUsers(TestOrganizationMixin, TestCase):

runflake8

@@ -1,4 +1,4 @@
 #!/bin/bash
 set -e
 flake8 --max-line-length=110 \
-       --exclude=migrations,./tests/*settings*.py || exit 1
+       --exclude=migrations,./tests/*settings*.py,./setup.py || exit 1

tests/testapp/tests.py

@@ -1,7 +1,7 @@
 from django.core.exceptions import ValidationError
 from django.test import TestCase
 from django.urls import resolve, reverse
-from openwisp_users.tests.utils import TestOrganizationMixin
+from openwisp_users.tests import TestOrganizationMixin
 
 from .models import Config, Template