Skip to content

Commit

Permalink
♻️(models) rename document/template access rights
Browse files Browse the repository at this point in the history
The "member" access right does not make sense for documents and templates.
What we really need are "editor" and "reader" access rights.
  • Loading branch information
sampaccoud committed May 29, 2024
1 parent 51325df commit 926fe37
Show file tree
Hide file tree
Showing 22 changed files with 601 additions and 265 deletions.
3 changes: 1 addition & 2 deletions src/backend/core/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ class DocumentAdmin(admin.ModelAdmin):
"""Document admin interface declaration."""

inlines = (DocumentAccessInline,)


@admin.register(models.Invitation)
class InvitationAdmin(admin.ModelAdmin):
Expand Down Expand Up @@ -119,4 +119,3 @@ class InvitationAdmin(admin.ModelAdmin):
def save_model(self, request, obj, form, change):
obj.issuer = request.user
obj.save()

15 changes: 7 additions & 8 deletions src/backend/core/api/viewsets.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
"""API endpoints"""
import json
from io import BytesIO

from django.contrib.postgres.aggregates import ArrayAgg
Expand Down Expand Up @@ -368,15 +367,15 @@ class DocumentAccessViewSet(
POST /api/v1.0/documents/<resource_id>/accesses/ with expected data:
- user: str
- role: str [owner|admin|member]
- role: str [administrator|editor|reader]
Return newly created document access
PUT /api/v1.0/documents/<resource_id>/accesses/<document_access_id>/ with expected data:
- role: str [owner|admin|member]
- role: str [owner|admin|editor|reader]
Return updated document access
PATCH /api/v1.0/documents/<resource_id>/accesses/<document_access_id>/ with expected data:
- role: str [owner|admin|member]
- role: str [owner|admin|editor|reader]
Return partially updated document access
DELETE /api/v1.0/documents/<resource_id>/accesses/<document_access_id>/
Expand Down Expand Up @@ -458,15 +457,15 @@ class TemplateAccessViewSet(
POST /api/v1.0/templates/<template_id>/accesses/ with expected data:
- user: str
- role: str [owner|admin|member]
- role: str [administrator|editor|reader]
Return newly created template access
PUT /api/v1.0/templates/<template_id>/accesses/<template_access_id>/ with expected data:
- role: str [owner|admin|member]
- role: str [owner|admin|editor|reader]
Return updated template access
PATCH /api/v1.0/templates/<template_id>/accesses/<template_access_id>/ with expected data:
- role: str [owner|admin|member]
- role: str [owner|admin|editor|reader]
Return partially updated template access
DELETE /api/v1.0/templates/<template_id>/accesses/<template_access_id>/
Expand Down Expand Up @@ -496,7 +495,7 @@ class InvitationViewset(
POST /api/v1.0/documents/<document_id>/invitations/ with expected data:
- email: str
- role: str [owner|admin|member]
- role: str [administrator|editor|reader]
Return newly created invitation (issuer and document are automatically set)
PUT / PATCH : Not permitted. Instead of updating your invitation,
Expand Down
27 changes: 24 additions & 3 deletions src/backend/core/migrations/0001_initial.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Generated by Django 5.0.3 on 2024-04-19 11:38
# Generated by Django 5.0.3 on 2024-05-28 20:29

import django.contrib.auth.models
import django.core.validators
Expand Down Expand Up @@ -89,7 +89,7 @@ class Migration(migrations.Migration):
('created_at', models.DateTimeField(auto_now_add=True, help_text='date and time at which a record was created', verbose_name='created on')),
('updated_at', models.DateTimeField(auto_now=True, help_text='date and time at which a record was last updated', verbose_name='updated on')),
('team', models.CharField(blank=True, max_length=100)),
('role', models.CharField(choices=[('member', 'Member'), ('administrator', 'Administrator'), ('owner', 'Owner')], default='member', max_length=20)),
('role', models.CharField(choices=[('reader', 'Reader'), ('editor', 'Editor'), ('administrator', 'Administrator'), ('owner', 'Owner')], default='reader', max_length=20)),
('document', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='accesses', to='core.document')),
('user', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
],
Expand All @@ -100,14 +100,31 @@ class Migration(migrations.Migration):
'ordering': ('-created_at',),
},
),
migrations.CreateModel(
name='Invitation',
fields=[
('id', models.UUIDField(default=uuid.uuid4, editable=False, help_text='primary key for the record as UUID', primary_key=True, serialize=False, verbose_name='id')),
('created_at', models.DateTimeField(auto_now_add=True, help_text='date and time at which a record was created', verbose_name='created on')),
('updated_at', models.DateTimeField(auto_now=True, help_text='date and time at which a record was last updated', verbose_name='updated on')),
('email', models.EmailField(max_length=254, verbose_name='email address')),
('role', models.CharField(choices=[('reader', 'Reader'), ('editor', 'Editor'), ('administrator', 'Administrator'), ('owner', 'Owner')], default='reader', max_length=20)),
('document', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='invitations', to='core.document')),
('issuer', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='invitations', to=settings.AUTH_USER_MODEL)),
],
options={
'verbose_name': 'Document invitation',
'verbose_name_plural': 'Document invitations',
'db_table': 'impress_invitation',
},
),
migrations.CreateModel(
name='TemplateAccess',
fields=[
('id', models.UUIDField(default=uuid.uuid4, editable=False, help_text='primary key for the record as UUID', primary_key=True, serialize=False, verbose_name='id')),
('created_at', models.DateTimeField(auto_now_add=True, help_text='date and time at which a record was created', verbose_name='created on')),
('updated_at', models.DateTimeField(auto_now=True, help_text='date and time at which a record was last updated', verbose_name='updated on')),
('team', models.CharField(blank=True, max_length=100)),
('role', models.CharField(choices=[('member', 'Member'), ('administrator', 'Administrator'), ('owner', 'Owner')], default='member', max_length=20)),
('role', models.CharField(choices=[('reader', 'Reader'), ('editor', 'Editor'), ('administrator', 'Administrator'), ('owner', 'Owner')], default='reader', max_length=20)),
('template', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='accesses', to='core.template')),
('user', models.ForeignKey(blank=True, null=True, on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)),
],
Expand All @@ -130,6 +147,10 @@ class Migration(migrations.Migration):
model_name='documentaccess',
constraint=models.CheckConstraint(check=models.Q(models.Q(('team', ''), ('user__isnull', False)), models.Q(('team__gt', ''), ('user__isnull', True)), _connector='OR'), name='check_document_access_either_user_or_team', violation_error_message='Either user or team must be set, not both.'),
),
migrations.AddConstraint(
model_name='invitation',
constraint=models.UniqueConstraint(fields=('email', 'document'), name='email_and_document_unique_together'),
),
migrations.AddConstraint(
model_name='templateaccess',
constraint=models.UniqueConstraint(condition=models.Q(('user__isnull', False)), fields=('user', 'template'), name='unique_template_user', violation_error_message='This user is already in this template.'),
Expand Down

This file was deleted.

34 changes: 21 additions & 13 deletions src/backend/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
Declare and configure the models for the impress core application
"""
import hashlib
import json
import smtplib
import textwrap
import uuid
Expand Down Expand Up @@ -55,8 +54,9 @@ def get_resource_roles(resource, user):
class RoleChoices(models.TextChoices):
"""Defines the possible roles a user can have in a template."""

MEMBER = "member", _("Member")
ADMIN = "administrator", _("Administrator")
READER = "reader", _("Reader") # Can read
EDITOR = "editor", _("Editor") # Can read and edit
ADMIN = "administrator", _("Administrator") # Can read, edit, delete and share
OWNER = "owner", _("Owner")


Expand Down Expand Up @@ -233,7 +233,7 @@ class BaseAccess(BaseModel):
)
team = models.CharField(max_length=100, blank=True)
role = models.CharField(
max_length=20, choices=RoleChoices.choices, default=RoleChoices.MEMBER
max_length=20, choices=RoleChoices.choices, default=RoleChoices.READER
)

class Meta:
Expand Down Expand Up @@ -265,14 +265,20 @@ def _get_abilities(self, resource, user):
RoleChoices.OWNER in roles
and resource.accesses.filter(role=RoleChoices.OWNER).count() > 1
)
set_role_to = [RoleChoices.ADMIN, RoleChoices.MEMBER] if can_delete else []
set_role_to = (
[RoleChoices.ADMIN, RoleChoices.EDITOR, RoleChoices.READER]
if can_delete
else []
)
else:
can_delete = is_owner_or_admin
set_role_to = []
if RoleChoices.OWNER in roles:
set_role_to.append(RoleChoices.OWNER)
if is_owner_or_admin:
set_role_to.extend([RoleChoices.ADMIN, RoleChoices.MEMBER])
set_role_to.extend(
[RoleChoices.ADMIN, RoleChoices.EDITOR, RoleChoices.READER]
)

# Remove the current role as we don't want to propose it as an option
try:
Expand Down Expand Up @@ -325,15 +331,15 @@ def content(self):
except (FileNotFoundError, ClientError):
pass
else:
self._content = response["Body"].read().decode('utf-8')
self._content = response["Body"].read().decode("utf-8")
return self._content

@content.setter
def content(self, content):
"""Cache the content, don't write to object storage yet"""
if not isinstance(content, str):
raise ValueError("content should be a string.")

self._content = content

def get_content_response(self, version_id=""):
Expand Down Expand Up @@ -447,6 +453,7 @@ def get_abilities(self, user):
is_owner_or_admin = bool(
set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN})
)
is_editor = bool(RoleChoices.EDITOR in roles)
can_get = self.is_public or bool(roles)
can_get_versions = bool(roles)

Expand All @@ -456,8 +463,8 @@ def get_abilities(self, user):
"versions_list": can_get_versions,
"versions_retrieve": can_get_versions,
"manage_accesses": is_owner_or_admin,
"update": is_owner_or_admin,
"partial_update": is_owner_or_admin,
"update": is_owner_or_admin or is_editor,
"partial_update": is_owner_or_admin or is_editor,
"retrieve": can_get,
}

Expand Down Expand Up @@ -537,14 +544,15 @@ def get_abilities(self, user):
is_owner_or_admin = bool(
set(roles).intersection({RoleChoices.OWNER, RoleChoices.ADMIN})
)
is_editor = bool(RoleChoices.EDITOR in roles)
can_get = self.is_public or bool(roles)

return {
"destroy": RoleChoices.OWNER in roles,
"generate_document": can_get,
"manage_accesses": is_owner_or_admin,
"update": is_owner_or_admin,
"partial_update": is_owner_or_admin,
"update": is_owner_or_admin or is_editor,
"partial_update": is_owner_or_admin or is_editor,
"retrieve": can_get,
}

Expand Down Expand Up @@ -631,7 +639,7 @@ class Invitation(BaseModel):
related_name="invitations",
)
role = models.CharField(
max_length=20, choices=RoleChoices.choices, default=RoleChoices.MEMBER
max_length=20, choices=RoleChoices.choices, default=RoleChoices.READER
)
issuer = models.ForeignKey(
User,
Expand Down
8 changes: 3 additions & 5 deletions src/backend/core/tests/documents/test_api_documents_delete.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,12 @@ def test_api_documents_delete_authenticated_unrelated():
assert models.Document.objects.count() == 1


@pytest.mark.parametrize("role", ["member", "administrator"])
@pytest.mark.parametrize("role", ["reader", "editor", "administrator"])
@pytest.mark.parametrize("via", VIA)
def test_api_documents_delete_authenticated_member_or_administrator(
via, role, mock_user_get_teams
):
def test_api_documents_delete_authenticated_not_owner(via, role, mock_user_get_teams):
"""
Authenticated users should not be allowed to delete a document for which they are
only a member or administrator.
only a reader, editor or administrator.
"""
user = factories.UserFactory()

Expand Down
Loading

0 comments on commit 926fe37

Please sign in to comment.