From 4088ea567ca8553ffb52df699c59b59628d95695 Mon Sep 17 00:00:00 2001 From: David Burke Date: Thu, 22 Jun 2023 15:57:55 -0400 Subject: [PATCH 1/7] API can be used to create/modify organizations --- kobo/apps/organizations/models.py | 18 +++++++++++++----- kobo/apps/organizations/serializers.py | 7 ++++++- .../tests/test_organizations_api.py | 5 +++++ kobo/apps/organizations/views.py | 15 ++++++++------- 4 files changed, 32 insertions(+), 13 deletions(-) diff --git a/kobo/apps/organizations/models.py b/kobo/apps/organizations/models.py index afe168a057..eed6d0d248 100644 --- a/kobo/apps/organizations/models.py +++ b/kobo/apps/organizations/models.py @@ -1,13 +1,17 @@ import uuid +from functools import partial from django.db import models -from kpi.fields import KpiUidField from django.forms.fields import EmailField +from organizations.abstract import ( + AbstractOrganization, + AbstractOrganizationInvitation, + AbstractOrganizationOwner, + AbstractOrganizationUser, +) +from organizations.utils import create_organization as create_organization_base -from organizations.abstract import (AbstractOrganization, - AbstractOrganizationInvitation, - AbstractOrganizationOwner, - AbstractOrganizationUser) +from kpi.fields import KpiUidField class Organization(AbstractOrganization): @@ -21,6 +25,7 @@ def email(self): """ return self.owner.organization_user.user.email + class OrganizationUser(AbstractOrganizationUser): pass @@ -31,3 +36,6 @@ class OrganizationOwner(AbstractOrganizationOwner): class OrganizationInvitation(AbstractOrganizationInvitation): pass + + +create_organization = partial(create_organization_base, model=Organization) diff --git a/kobo/apps/organizations/serializers.py b/kobo/apps/organizations/serializers.py index 58f666310a..c5ef754e5f 100644 --- a/kobo/apps/organizations/serializers.py +++ b/kobo/apps/organizations/serializers.py @@ -1,9 +1,14 @@ from rest_framework import serializers -from kobo.apps.organizations.models import Organization +from kobo.apps.organizations.models import Organization, create_organization class OrganizationSerializer(serializers.ModelSerializer): class Meta: model = Organization fields = ['id', 'name', 'is_active', 'created', 'modified', 'slug'] + read_only_fields = ["id", "slug"] + + def create(self, validated_data): + user = self.context['request'].user + return create_organization(user, validated_data["name"]) diff --git a/kobo/apps/organizations/tests/test_organizations_api.py b/kobo/apps/organizations/tests/test_organizations_api.py index 08d4477041..f45bf56bf6 100644 --- a/kobo/apps/organizations/tests/test_organizations_api.py +++ b/kobo/apps/organizations/tests/test_organizations_api.py @@ -34,6 +34,11 @@ def test_anonymous_user(self): assert response_list.status_code == status.HTTP_403_FORBIDDEN response_detail = self.client.get(self.url_detail) assert response_detail.status_code == status.HTTP_403_FORBIDDEN + + def test_create(self): + data = {"name": "my org"} + res = self.client.post(self.url_list, data) + self.assertContains(res, data["name"], status_code=201) def test_api_creates_org(self): self.assertFalse(self.user.organizations_organization.all()) diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py index 2e42b5dfad..86be5129b3 100644 --- a/kobo/apps/organizations/views.py +++ b/kobo/apps/organizations/views.py @@ -1,24 +1,25 @@ from django.contrib.auth.models import User from django.db.models import QuerySet - -from organizations.utils import create_organization from rest_framework import viewsets from rest_framework.permissions import IsAuthenticated -from kobo.apps.organizations.models import Organization +from kobo.apps.organizations.models import Organization, create_organization from kobo.apps.organizations.serializers import OrganizationSerializer -class OrganizationViewSet(viewsets.ReadOnlyModelViewSet): +class OrganizationViewSet(viewsets.ModelViewSet): """ - todo: create documentation + Organizations are groups of users with assigned permissions and configurations + + - Organization admins can manage the organization and it's membership + - Connect to authentication mechanisms and enforce policy + - Create teams and projects under the organization """ queryset = Organization.objects.all() serializer_class = OrganizationSerializer lookup_field = 'id' permission_classes = (IsAuthenticated,) - extra_context = None def get_queryset(self) -> QuerySet: user = self.request.user @@ -26,6 +27,6 @@ def get_queryset(self) -> QuerySet: if not queryset: # Very inefficient get or create queryset. # It's temporary and should be removed later. - create_organization(user, f"{user.username}'s organization", model=Organization) + create_organization(user, f"{user.username}'s organization") queryset = queryset.all() # refresh return queryset From 3ec76d6ffa5b015b6e7a566f9c0d187aa3124435 Mon Sep 17 00:00:00 2001 From: David Burke Date: Thu, 22 Jun 2023 16:27:19 -0400 Subject: [PATCH 2/7] Add org edit permissions --- ...1_squashed_0004_remove_organization_uid.py | 103 ++++++++++++++++++ kobo/apps/organizations/permissions.py | 17 +++ kobo/apps/organizations/serializers.py | 2 +- .../tests/test_organizations_api.py | 25 +++-- kobo/apps/organizations/views.py | 12 +- 5 files changed, 141 insertions(+), 18 deletions(-) create mode 100644 kobo/apps/organizations/migrations/0001_squashed_0004_remove_organization_uid.py create mode 100644 kobo/apps/organizations/permissions.py diff --git a/kobo/apps/organizations/migrations/0001_squashed_0004_remove_organization_uid.py b/kobo/apps/organizations/migrations/0001_squashed_0004_remove_organization_uid.py new file mode 100644 index 0000000000..f1cf49c260 --- /dev/null +++ b/kobo/apps/organizations/migrations/0001_squashed_0004_remove_organization_uid.py @@ -0,0 +1,103 @@ +# Generated by Django 3.2.15 on 2023-06-22 20:25 + +from django.conf import settings +from django.db import migrations, models +import django.db.models.deletion +import django.utils.timezone +import kpi.fields.kpi_uid +import organizations.fields + + +class Migration(migrations.Migration): + + replaces = [('organizations', '0001_initial'), ('organizations', '0002_alter_organization_id_to_kpiuidfield'), ('organizations', '0003_copy_organization_uid_to_id'), ('organizations', '0004_remove_organization_uid')] + + initial = True + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.CreateModel( + name='Organization', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('name', models.CharField(help_text='The name of the organization', max_length=200)), + ('is_active', models.BooleanField(default=True)), + ('created', organizations.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False)), + ('modified', organizations.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False)), + ('slug', organizations.fields.SlugField(blank=True, editable=False, help_text='The name in all lowercase, suitable for URL identification', max_length=200, populate_from='name', unique=True)), + ('uid', kpi.fields.kpi_uid.KpiUidField(_null=False, uid_prefix='org')), + ], + options={ + 'verbose_name': 'organization', + 'verbose_name_plural': 'organizations', + 'ordering': ['name'], + 'abstract': False, + }, + ), + migrations.CreateModel( + name='OrganizationUser', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', organizations.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False)), + ('modified', organizations.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False)), + ('is_admin', models.BooleanField(default=False)), + ('organization', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='organization_users', to='organizations.organization')), + ('user', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='organizations_organizationuser', to=settings.AUTH_USER_MODEL)), + ], + options={ + 'verbose_name': 'organization user', + 'verbose_name_plural': 'organization users', + 'ordering': ['organization', 'user'], + 'abstract': False, + 'unique_together': {('user', 'organization')}, + }, + ), + migrations.CreateModel( + name='OrganizationOwner', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('created', organizations.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False)), + ('modified', organizations.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False)), + ('organization', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='owner', to='organizations.organization')), + ('organization_user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to='organizations.organizationuser')), + ], + options={ + 'verbose_name': 'organization owner', + 'verbose_name_plural': 'organization owners', + 'abstract': False, + }, + ), + migrations.CreateModel( + name='OrganizationInvitation', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('guid', models.UUIDField(editable=False)), + ('invitee_identifier', models.CharField(help_text='The contact identifier for the invitee, email, phone number, social media handle, etc.', max_length=1000)), + ('created', organizations.fields.AutoCreatedField(default=django.utils.timezone.now, editable=False)), + ('modified', organizations.fields.AutoLastModifiedField(default=django.utils.timezone.now, editable=False)), + ('invited_by', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='organizations_organizationinvitation_sent_invitations', to=settings.AUTH_USER_MODEL)), + ('invitee', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, related_name='organizations_organizationinvitation_invitations', to=settings.AUTH_USER_MODEL)), + ('organization', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='organization_invites', to='organizations.organization')), + ], + options={ + 'abstract': False, + }, + ), + migrations.AddField( + model_name='organization', + name='users', + field=models.ManyToManyField(related_name='organizations_organization', through='organizations.OrganizationUser', to=settings.AUTH_USER_MODEL), + ), + migrations.AlterField( + model_name='organization', + name='id', + field=kpi.fields.kpi_uid.KpiUidField(_null=False, primary_key=True, uid_prefix='org'), + ), + migrations.RemoveField( + model_name='organization', + name='uid', + ), + ] diff --git a/kobo/apps/organizations/permissions.py b/kobo/apps/organizations/permissions.py new file mode 100644 index 0000000000..bc0e924d19 --- /dev/null +++ b/kobo/apps/organizations/permissions.py @@ -0,0 +1,17 @@ +from rest_framework import permissions + + +class IsOrgAdminOrReadOnly(permissions.BasePermission): + """ + Object-level permission to only allow admin members of an object to edit it. + Assumes the model instance has an `is_admin` attribute. + """ + + def has_object_permission(self, request, view, obj): + # Read permissions are allowed to any request, + # so we'll always allow GET, HEAD or OPTIONS requests. + if request.method in permissions.SAFE_METHODS: + return True + + # Instance must have an attribute named `owner`. + return obj.is_admin(request.user) diff --git a/kobo/apps/organizations/serializers.py b/kobo/apps/organizations/serializers.py index c5ef754e5f..5fd261e037 100644 --- a/kobo/apps/organizations/serializers.py +++ b/kobo/apps/organizations/serializers.py @@ -11,4 +11,4 @@ class Meta: def create(self, validated_data): user = self.context['request'].user - return create_organization(user, validated_data["name"]) + return create_organization(user, validated_data['name']) diff --git a/kobo/apps/organizations/tests/test_organizations_api.py b/kobo/apps/organizations/tests/test_organizations_api.py index f45bf56bf6..1eb6068b36 100644 --- a/kobo/apps/organizations/tests/test_organizations_api.py +++ b/kobo/apps/organizations/tests/test_organizations_api.py @@ -10,7 +10,6 @@ class OrganizationTestCase(BaseTestCase): - fixtures = ['test_data'] URL_NAMESPACE = URL_NAMESPACE @@ -34,16 +33,11 @@ def test_anonymous_user(self): assert response_list.status_code == status.HTTP_403_FORBIDDEN response_detail = self.client.get(self.url_detail) assert response_detail.status_code == status.HTTP_403_FORBIDDEN - + def test_create(self): - data = {"name": "my org"} + data = {'name': 'my org'} res = self.client.post(self.url_list, data) - self.assertContains(res, data["name"], status_code=201) - - def test_api_creates_org(self): - self.assertFalse(self.user.organizations_organization.all()) - self.client.get(self.url_list) - self.assertTrue(self.user.organizations_organization.all()) + self.assertContains(res, data['name'], status_code=201) def test_api_returns_org_data(self): self._insert_data() @@ -51,3 +45,16 @@ def test_api_returns_org_data(self): self.assertContains(response, self.organization.slug) self.assertContains(response, self.organization.id) self.assertContains(response, self.organization.name) + + def test_update(self): + self._insert_data() + data = {'name': 'edit'} + res = self.client.patch(self.url_detail, data) + self.assertContains(res, data['name']) + + user = baker.make(User) + self.client.force_login(user) + org_user = self.organization.add_user(user=user) + with self.assertNumQueries(3): + res = self.client.patch(self.url_detail, data) + self.assertEqual(res.status_code, 403) diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py index 86be5129b3..dc21f740dd 100644 --- a/kobo/apps/organizations/views.py +++ b/kobo/apps/organizations/views.py @@ -3,8 +3,9 @@ from rest_framework import viewsets from rest_framework.permissions import IsAuthenticated -from kobo.apps.organizations.models import Organization, create_organization -from kobo.apps.organizations.serializers import OrganizationSerializer +from .models import Organization, create_organization +from .serializers import OrganizationSerializer +from .permissions import IsOrgAdminOrReadOnly class OrganizationViewSet(viewsets.ModelViewSet): @@ -19,14 +20,9 @@ class OrganizationViewSet(viewsets.ModelViewSet): queryset = Organization.objects.all() serializer_class = OrganizationSerializer lookup_field = 'id' - permission_classes = (IsAuthenticated,) + permission_classes = (IsAuthenticated, IsOrgAdminOrReadOnly) def get_queryset(self) -> QuerySet: user = self.request.user queryset = super().get_queryset().filter(users=user) - if not queryset: - # Very inefficient get or create queryset. - # It's temporary and should be removed later. - create_organization(user, f"{user.username}'s organization") - queryset = queryset.all() # refresh return queryset From 3aa32f786e1f3d0ef08ba1d77cf037487d492dc7 Mon Sep 17 00:00:00 2001 From: David Burke Date: Fri, 23 Jun 2023 14:15:22 -0400 Subject: [PATCH 3/7] Add back org get queryset hack --- kobo/apps/organizations/tests/test_organizations_api.py | 5 +++++ kobo/apps/organizations/views.py | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/kobo/apps/organizations/tests/test_organizations_api.py b/kobo/apps/organizations/tests/test_organizations_api.py index 1eb6068b36..a60c14bc79 100644 --- a/kobo/apps/organizations/tests/test_organizations_api.py +++ b/kobo/apps/organizations/tests/test_organizations_api.py @@ -38,6 +38,11 @@ def test_create(self): data = {'name': 'my org'} res = self.client.post(self.url_list, data) self.assertContains(res, data['name'], status_code=201) + + def test_list_creates_org(self): + self.assertFalse(self.user.organizations_organization.all()) + self.client.get(self.url_list) + self.assertTrue(self.user.organizations_organization.all()) def test_api_returns_org_data(self): self._insert_data() diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py index dc21f740dd..f129427209 100644 --- a/kobo/apps/organizations/views.py +++ b/kobo/apps/organizations/views.py @@ -25,4 +25,9 @@ class OrganizationViewSet(viewsets.ModelViewSet): def get_queryset(self) -> QuerySet: user = self.request.user queryset = super().get_queryset().filter(users=user) + if self.action == "list": + # Very inefficient get or create queryset. + # It's temporary and should be removed later. + create_organization(user, f"{user.username}'s organization", model=Organization) + queryset = queryset.all() # refresh return queryset From 33568e60ae96e7fe6b6cc03aeade18db107e8b0b Mon Sep 17 00:00:00 2001 From: David Burke Date: Fri, 23 Jun 2023 14:20:14 -0400 Subject: [PATCH 4/7] sort and format --- kobo/apps/organizations/admin.py | 21 ++++++++++++------- .../tests/test_organizations_api.py | 3 +-- kobo/apps/organizations/views.py | 6 ++++-- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/kobo/apps/organizations/admin.py b/kobo/apps/organizations/admin.py index f04fc0e6a2..0221892897 100644 --- a/kobo/apps/organizations/admin.py +++ b/kobo/apps/organizations/admin.py @@ -1,12 +1,17 @@ from django.contrib import admin - -from organizations.base_admin import (BaseOrganizationAdmin, - BaseOrganizationOwnerAdmin, - BaseOrganizationUserAdmin, - BaseOwnerInline) - -from .models import (Organization, OrganizationInvitation, OrganizationOwner, - OrganizationUser) +from organizations.base_admin import ( + BaseOrganizationAdmin, + BaseOrganizationOwnerAdmin, + BaseOrganizationUserAdmin, + BaseOwnerInline, +) + +from .models import ( + Organization, + OrganizationInvitation, + OrganizationOwner, + OrganizationUser, +) class OwnerInline(BaseOwnerInline): diff --git a/kobo/apps/organizations/tests/test_organizations_api.py b/kobo/apps/organizations/tests/test_organizations_api.py index a60c14bc79..6954aed06b 100644 --- a/kobo/apps/organizations/tests/test_organizations_api.py +++ b/kobo/apps/organizations/tests/test_organizations_api.py @@ -1,6 +1,5 @@ from django.contrib.auth.models import User from django.urls import reverse - from model_bakery import baker from rest_framework import status @@ -38,7 +37,7 @@ def test_create(self): data = {'name': 'my org'} res = self.client.post(self.url_list, data) self.assertContains(res, data['name'], status_code=201) - + def test_list_creates_org(self): self.assertFalse(self.user.organizations_organization.all()) self.client.get(self.url_list) diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py index f129427209..edb635b995 100644 --- a/kobo/apps/organizations/views.py +++ b/kobo/apps/organizations/views.py @@ -4,8 +4,8 @@ from rest_framework.permissions import IsAuthenticated from .models import Organization, create_organization -from .serializers import OrganizationSerializer from .permissions import IsOrgAdminOrReadOnly +from .serializers import OrganizationSerializer class OrganizationViewSet(viewsets.ModelViewSet): @@ -28,6 +28,8 @@ def get_queryset(self) -> QuerySet: if self.action == "list": # Very inefficient get or create queryset. # It's temporary and should be removed later. - create_organization(user, f"{user.username}'s organization", model=Organization) + create_organization( + user, f"{user.username}'s organization", model=Organization + ) queryset = queryset.all() # refresh return queryset From 7e5985878367e6381610034578ed8901dff0686c Mon Sep 17 00:00:00 2001 From: David Burke Date: Fri, 23 Jun 2023 14:21:18 -0400 Subject: [PATCH 5/7] Fix create org on list logic --- kobo/apps/organizations/views.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py index edb635b995..cf7e3f341a 100644 --- a/kobo/apps/organizations/views.py +++ b/kobo/apps/organizations/views.py @@ -25,7 +25,7 @@ class OrganizationViewSet(viewsets.ModelViewSet): def get_queryset(self) -> QuerySet: user = self.request.user queryset = super().get_queryset().filter(users=user) - if self.action == "list": + if self.action == "list" and not queryset: # Very inefficient get or create queryset. # It's temporary and should be removed later. create_organization( From 6f8bc38fa5b851880ab2175588af58cddbbaf054 Mon Sep 17 00:00:00 2001 From: David Burke Date: Fri, 23 Jun 2023 14:22:20 -0400 Subject: [PATCH 6/7] Remove unnecessary create_organization param --- kobo/apps/organizations/views.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/kobo/apps/organizations/views.py b/kobo/apps/organizations/views.py index cf7e3f341a..51b53eab2a 100644 --- a/kobo/apps/organizations/views.py +++ b/kobo/apps/organizations/views.py @@ -28,8 +28,6 @@ def get_queryset(self) -> QuerySet: if self.action == "list" and not queryset: # Very inefficient get or create queryset. # It's temporary and should be removed later. - create_organization( - user, f"{user.username}'s organization", model=Organization - ) + create_organization(user, f"{user.username}'s organization") queryset = queryset.all() # refresh return queryset From b2003595e9610b9dc62de4f7b537ac6acba23a3a Mon Sep 17 00:00:00 2001 From: David Burke Date: Wed, 28 Jun 2023 16:37:36 -0400 Subject: [PATCH 7/7] Add org api list test --- .../organizations/tests/test_organizations_api.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/kobo/apps/organizations/tests/test_organizations_api.py b/kobo/apps/organizations/tests/test_organizations_api.py index 6954aed06b..ab0b7245f6 100644 --- a/kobo/apps/organizations/tests/test_organizations_api.py +++ b/kobo/apps/organizations/tests/test_organizations_api.py @@ -38,6 +38,14 @@ def test_create(self): res = self.client.post(self.url_list, data) self.assertContains(res, data['name'], status_code=201) + def test_list(self): + self._insert_data() + organization2 = baker.make(Organization, id='org_abcd123') + organization2.add_user(user=self.user, is_admin=True) + with self.assertNumQueries(2): + res = self.client.get(self.url_list) + self.assertContains(res, organization2.name) + def test_list_creates_org(self): self.assertFalse(self.user.organizations_organization.all()) self.client.get(self.url_list) @@ -53,12 +61,12 @@ def test_api_returns_org_data(self): def test_update(self): self._insert_data() data = {'name': 'edit'} - res = self.client.patch(self.url_detail, data) + with self.assertNumQueries(4): + res = self.client.patch(self.url_detail, data) self.assertContains(res, data['name']) user = baker.make(User) self.client.force_login(user) org_user = self.organization.add_user(user=user) - with self.assertNumQueries(3): - res = self.client.patch(self.url_detail, data) + res = self.client.patch(self.url_detail, data) self.assertEqual(res.status_code, 403)