[admin] allow administrator role access organization admin #143

CHANGES.rst

@@ -9,6 +9,8 @@ WIP
 - `organization owners <https://github.com/openwisp/openwisp-users#organization-owners>`_:
   [admin] Added support for organization owners
 - [admin] Made organization owners read-only to non-superusers
+- `organization permissions <https://github.com/openwisp/openwisp-users#organization-permissions>`_:
+  [admin] Allowed administrator role to access organization admin
 
 Version 0.2.2 [2020-05-04]
 --------------------------

README.rst

@@ -299,6 +299,18 @@ the token in the ``Authorization`` header:
     # send bearer token
     http GET localhost:8000/api/v1/firmware/build/ "Authorization: Bearer $TOKEN"
 
+Organization permissions
+------------------------
+
+Here's a summary of the default permissions:
+
+- All users who belong to the Administrators group and are organization
+  managers (``OrganizationUser.is_admin=True``), have the permission to edit
+  the organizations details which they administrate.
+- Only super users have the permission to add and delete organizations.
+- Only super users and organization owners have the permission to change
+  the ``OrganizationOwner`` inline or delete the relation.
+
 Organization membership helpers
 -------------------------------
 

openwisp_users/admin.py

@@ -84,6 +84,11 @@ class OrganizationOwnerInline(admin.StackedInline):
     model = OrganizationOwner
     extra = 0
 
+    def has_change_permission(self, request, obj=None):
+        if obj and not request.user.is_superuser and not request.user.is_owner(obj):
+            return False
+        return super().has_change_permission(request, obj)
+
 
 class OrganizationUserInline(admin.StackedInline):
     model = OrganizationUser
@@ -456,7 +461,9 @@ class GroupAdmin(BaseGroupAdmin, BaseAdmin):
     pass
 
 
-class OrganizationAdmin(BaseOrganizationAdmin, BaseAdmin, UUIDAdmin):
+class OrganizationAdmin(
+    MultitenantAdminMixin, BaseOrganizationAdmin, BaseAdmin, UUIDAdmin
+):
     view_on_site = False
     inlines = [OrganizationOwnerInline]
     readonly_fields = ['uuid']
@@ -476,8 +483,7 @@ def get_inline_instances(self, request, obj=None):
 
     def has_change_permission(self, request, obj=None):
         """
-        Allow operator to change an organization only if
-        they is an admin of that organization
+        Allow only managers and superuser to change organization
         """
         if obj and not request.user.is_superuser and not request.user.is_manager(obj):
             return False

openwisp_users/migrations/0010_allow_admins_change_organization.py

@@ -0,0 +1,18 @@
+# Generated by Django 3.0.8 on 2020-07-14 15:28
+
+from django.db import migrations
+
+from . import allow_admins_change_organization
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('openwisp_users', '0009_create_organization_owners'),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            allow_admins_change_organization, reverse_code=migrations.RunPython.noop
+        )
+    ]

openwisp_users/migrations/__init__.py

@@ -134,3 +134,22 @@ def create_organization_owners(apps, schema_editor):
             OrganizationOwner.objects.create(
                 organization_user=org_user, organization=org
             )
+
+
+def allow_admins_change_organization(apps, schema_editor):
+    Group = get_model(apps, 'Group')
+    try:
+        admins = Group.objects.get(name='Administrator')
+        permissions = [
+            Permission.objects.get(
+                content_type__app_label=Group._meta.app_label,
+                codename='change_organization',
+            ).pk,
+            Permission.objects.get(
+                content_type__app_label=Group._meta.app_label,
+                codename='change_organizationowner',
+            ).pk,
+        ]
+        admins.permissions.add(*permissions)
+    except ObjectDoesNotExist:
+        pass

openwisp_users/multitenancy.py

@@ -43,6 +43,8 @@ def get_queryset(self, request):
             return qs
         if hasattr(self.model, 'organization'):
             return qs.filter(organization__in=user.organizations_dict.keys())
+        if self.model.__name__ == 'Organization':
+            return qs.filter(pk__in=user.organizations_dict.keys())
         elif not self.multitenant_parent:
             return qs
         else:

openwisp_users/tests/test_admin.py

@@ -317,10 +317,7 @@ def test_operator_change_organization(self):
                 f'admin:{self.app_label}_organization_change', args=[default_org.pk]
             )
         )
-        self.assertNotContains(
-            response,
-            '<input type="text" name="name" value="{0}"'.format(default_org.name),
-        )
+        self.assertEqual(response.status_code, 302)
         response = self.client.get(
             reverse(f'admin:{self.app_label}_organization_change', args=[org2.pk])
         )
@@ -1108,6 +1105,192 @@ def test_superuser_bulk_delete(self):
         self.assertContains(r, 'Successfully deleted 2 users')
         self.assertEqual(user_qs.count(), 2)
 
+    def test_admin_user_has_change_org_perm(self):
+        user = self._get_user()
+        group = Group.objects.filter(name='Administrator')
+        user.groups.set(group)
+        self.assertIn(
+            f'{self.app_label}.change_organization', user.get_all_permissions()
+        )
+
+    def test_can_change_org(self):
+        org = self._get_org()
+        user = self._create_user(
+            username='change', password='change', email='email@email', is_staff=True
+        )
+        group = Group.objects.filter(name='Administrator')
+        user.groups.set(group)
+        org_user = self._create_org_user(user=user, organization=org, is_admin=True)
+        path = reverse(f'admin:{self.app_label}_organization_change', args=[org.pk])
+
+        with self.subTest('org owner can change org'):
+            self.client.force_login(user)
+            r = self.client.get(path)
+            self.assertEqual(r.status_code, 200)
+            self.assertContains(r, f'<input type="text" name="name" value="{org.name}"')
+
+        with self.subTest('managers can change org'):
+            OrganizationOwner.objects.all().delete()
+            self.client.force_login(user)
+            r = self.client.get(path)
+            self.assertEqual(r.status_code, 200)
+            self.assertContains(r, f'<input type="text" name="name" value="{org.name}"')
+
+        with self.subTest('member can not edit org'):
+            OrganizationOwner.objects.all().delete()
+            org_user.is_admin = False
+            org_user.save()
+            self.client.force_login(user)
+            r = self.client.get(path)
+            self.assertEqual(r.status_code, 200)
+            self.assertContains(r, f'class="readonly">{org.name}')
+
+    def test_only_superuser_has_add_delete_org_perm(self):
+        user = self._create_user(
+            username='change', password='change', email='email@email', is_staff=True
+        )
+        group = Group.objects.filter(name='Administrator')
+        user.groups.set(group)
+        org = self._get_org()
+        add_params = {
+            'name': 'new org',
+            'slug': 'new',
+            'owner-TOTAL_FORMS': '0',
+            'owner-INITIAL_FORMS': '0',
+            'owner-MIN_NUM_FORMS': '0',
+            'owner-MAX_NUM_FORMS': '1',
+        }
+        delete_params = {
+            'action': 'delete_selected',
+            '_selected_action': [org.pk],
+            'post': 'yes',
+        }
+        add_path = reverse(f'admin:{self.app_label}_organization_add')
+        delete_path = reverse(f'admin:{self.app_label}_organization_changelist')
+
+        with self.subTest('Administrators can not add org'):
+            self.client.force_login(user)
+            r = self.client.post(add_path, add_params, follow=True)
+            self.assertEqual(r.status_code, 403)
+            orgs = Organization.objects.filter(slug='new')
+            self.assertEqual(orgs.count(), 0)
+
+        with self.subTest('Administrators can not delete org'):
+            self.client.force_login(user)
+            r = self.client.post(delete_path, delete_params, follow=True)
+            self.assertEqual(r.status_code, 200)
+            orgs = Organization.objects.filter(pk=org.pk)
+            self.assertEqual(orgs.count(), 1)
+
+        with self.subTest('superuser can add org'):
+            self.client.force_login(self._get_admin())
+            r = self.client.post(add_path, add_params, follow=True)
+            self.assertEqual(r.status_code, 200)
+            orgs = Organization.objects.get(name='new org')
+            self.assertEqual(orgs.name, 'new org')
+
+        with self.subTest('superuser can delete org'):
+            self.client.force_login(self._get_admin())
+            r = self.client.post(delete_path, delete_params, follow=True)
+            self.assertEqual(r.status_code, 200)
+            self.assertContains(r, 'Successfully deleted 1 organization')
+            orgs = Organization.objects.filter(pk=org.pk)
+            self.assertEqual(orgs.count(), 0)
+
+    def test_can_change_inline_org_owner(self):
+        user1 = self._create_user(
+            username='user1', password='user1', email='email1@email', is_staff=True
+        )
+        user2 = self._create_user(
+            username='user2', password='user2', email='email2@email', is_staff=True
+        )
+        group = Group.objects.filter(name='Administrator')
+        user1.groups.set(group)
+        user2.groups.set(group)
+        org = self._get_org()
+        org_user = self._create_org_user(user=user1, organization=org, is_admin=True)
+        org_owner = OrganizationOwner.objects.get(organization_user=org_user)
+        org_user2 = self._create_org_user(organization=org, user=user2, is_admin=True)
+        params = {
+            'name': org.name,
+            'slug': org.slug,
+            'owner-TOTAL_FORMS': '1',
+            'owner-INITIAL_FORMS': '1',
+            'owner-MIN_NUM_FORMS': '0',
+            'owner-MAX_NUM_FORMS': '1',
+            'owner-0-organization_user': f'{org_user.pk}',
+            'owner-0-organization': f'{org.pk}',
+            'owner-0-id': f'{org_owner.pk}',
+        }
+        path = reverse(f'admin:{self.app_label}_organization_change', args=[org.pk])
+
+        with self.subTest('manager can not edit inline org owner'):
+            self.client.force_login(user2)
+            params.update({'owner-0-organization_user': f'{org_user2.pk}'})
+            r = self.client.post(path, params, follow=True)
+            self.assertEqual(r.status_code, 200)
+            org_owners = OrganizationOwner.objects.filter(organization_user=org_user2)
+            self.assertEqual(org_owners.count(), 0)
+
+        with self.subTest('owner can edit inline org owner'):
+            self.client.force_login(user1)
+            params.update({'owner-0-organization_user': f'{org_user2.pk}'})
+            r = self.client.post(path, params, follow=True)
+            self.assertEqual(r.status_code, 200)
+            org_owners = OrganizationOwner.objects.filter(organization_user=org_user2)
+            self.assertEqual(org_owners.count(), 1)
+
+        with self.subTest('superuser can edit inline org owner'):
+            self.client.force_login(self._get_admin())
+            user3 = self._create_user(
+                username='user3', password='user3', email='email3@email', is_staff=True
+            )
+            user3.groups.set(group)
+            org_user3 = self._create_org_user(
+                organization=org, user=user3, is_admin=True
+            )
+            params.update({'owner-0-organization_user': f'{org_user3.pk}'})
+            r = self.client.post(path, params, follow=True)
+            self.assertEqual(r.status_code, 200)
+            org_owners = OrganizationOwner.objects.filter(organization_user=org_user3)
+            self.assertEqual(org_owners.count(), 1)
+
+    def test_only_superuser_can_delete_inline_org_owner(self):
+        org = self._get_org()
+        user = self._create_user(
+            username='change', password='change', email='email@email', is_staff=True
+        )
+        group = Group.objects.filter(name='Administrator')
+        user.groups.set(group)
+        self._create_org_user(organization=org, user=user, is_admin=True)
+        path = reverse(f'admin:{self.app_label}_organization_change', args=[org.pk])
+
+        with self.subTest('org owners can not delete inline org owner'):
+            self.client.force_login(user)
+            r = self.client.get(path)
+            self.assertEqual(r.status_code, 200)
+            self.assertNotContains(r, '-DELETE">Delete')
+
+        with self.subTest('managers can not delete inline org owner'):
+            user1 = self._create_user(
+                username='change1',
+                password='change1',
+                email='email1@email',
+                is_staff=True,
+            )
+            user1.groups.set(group)
+            self._create_org_user(organization=org, user=user1, is_admin=True)
+            self.client.force_login(user1)
+            r = self.client.get(path)
+            self.assertEqual(r.status_code, 200)
+            self.assertNotContains(r, '-DELETE">Delete')
+
+        with self.subTest('superuser can delete inline org owner'):
+            self.client.force_login(self._get_admin())
+            r = self.client.get(path)
+            self.assertEqual(r.status_code, 200)
+            self.assertContains(r, '-DELETE">Delete')
+
     @patch('sys.stdout', devnull)
     @patch('sys.stderr', devnull)
     def test_admin_add_user_with_invalid_email(self):

tests/testapp/migrations/0004_allow_admins_access_organization.py

@@ -0,0 +1,13 @@
+# Generated by Django 3.0.8 on 2020-07-23 17:55
+
+from django.db import migrations
+from openwisp_users.migrations import allow_admins_change_organization
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('testapp', '0003_multitenancy'),
+    ]
+
+    operations = [migrations.RunPython(allow_admins_change_organization)]