Merge pull request #2257 from bookwyrm-social/group-perms

Fixes perms checks for groups

bookwyrm/tests/views/test_group.py

@@ -2,6 +2,7 @@
 from unittest.mock import patch
 
 from django.contrib.auth.models import AnonymousUser
+from django.core.exceptions import PermissionDenied
 from django.http import Http404
 from django.template.response import TemplateResponse
 from django.test import TestCase
@@ -15,7 +16,7 @@ from bookwyrm.tests.validate_html import validate_html
 class GroupViews(TestCase):
     """view group and edit details"""
 
-    def setUp(self):
+    def setUp(self):  # pylint: disable=invalid-name
         """we need basic test data and mocks"""
         self.factory = RequestFactory()
         with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
@@ -129,6 +130,23 @@ class GroupViews(TestCase):
             ).exists()
         )
 
+    def test_group_create_permission_denied(self, _):
+        """create group view"""
+        view = views.UserGroups.as_view()
+        request = self.factory.post(
+            "",
+            {
+                "name": "A group",
+                "description": "wowzers",
+                "privacy": "unlisted",
+                "user": self.local_user.id,
+            },
+        )
+        request.user = self.rat
+
+        with self.assertRaises(PermissionDenied):
+            view(request, "username")
+
     def test_group_edit(self, _):
         """test editing a "group" database entry"""
         view = views.Group.as_view()

bookwyrm/views/group.py

@@ -1,7 +1,7 @@
 """group views"""
 from django.apps import apps
 from django.contrib.auth.decorators import login_required
-from django.db import IntegrityError
+from django.db import IntegrityError, transaction
 from django.core.paginator import Paginator
 from django.http import HttpResponseBadRequest
 from django.shortcuts import get_object_or_404, redirect
@@ -112,9 +112,13 @@ class UserGroups(View):
         form = forms.GroupForm(request.POST)
         if not form.is_valid():
             return redirect(request.user.local_path + "/groups")
-        group = form.save()
-        # add the creator as a group member
-        models.GroupMember.objects.create(group=group, user=request.user)
+
+        group = form.save(commit=False)
+        group.raise_not_editable(request.user)
+        with transaction.atomic():
+            group.save()
+            # add the creator as a group member
+            models.GroupMember.objects.create(group=group, user=request.user)
         return redirect("group", group.id)
 
 
@@ -128,6 +132,7 @@ class FindUsers(View):
         """basic profile info"""
         user_query = request.GET.get("user_query")
         group = get_object_or_404(models.Group, id=group_id)
+        group.raise_not_editable(request.user)
         lists = (
             models.List.privacy_filter(request.user)
             .filter(group=group)
@@ -183,10 +188,11 @@ def delete_group(request, group_id):
     # only the owner can delete a group
     group.raise_not_deletable(request.user)
 
-    # deal with any group lists
-    models.List.objects.filter(group=group).update(curation="closed", group=None)
+    with transaction.atomic():
+        # deal with any group lists
+        models.List.objects.filter(group=group).update(curation="closed", group=None)
 
-    group.delete()
+        group.delete()
     return redirect(request.user.local_path + "/groups")