From ae840002dec008ecaebbf10ed219446d4b8be98d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 27 Aug 2021 12:21:16 -0700 Subject: [PATCH 001/111] Only show update option when there's an active readthrough --- .../templates/snippets/shelve_button/shelve_button_options.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/templates/snippets/shelve_button/shelve_button_options.html b/bookwyrm/templates/snippets/shelve_button/shelve_button_options.html index 5cce1477c..0bda58602 100644 --- a/bookwyrm/templates/snippets/shelve_button/shelve_button_options.html +++ b/bookwyrm/templates/snippets/shelve_button/shelve_button_options.html @@ -42,7 +42,7 @@ {% endfor %} {% if dropdown %} -{% if readthrough and active_shelf.shelf.identifier != 'read' %} +{% if readthrough and not readthrough.finish_date and active_shelf.shelf.identifier != 'read' %} From 0f57a43bfbed7d4df779e357585e7c67122addac Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 13:31:35 -0700 Subject: [PATCH 029/111] Separate access controls for delete and re-order --- bookwyrm/templates/lists/list.html | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bookwyrm/templates/lists/list.html b/bookwyrm/templates/lists/list.html index 241bc4080..dbee2f1f5 100644 --- a/bookwyrm/templates/lists/list.html +++ b/bookwyrm/templates/lists/list.html @@ -66,15 +66,14 @@

{% blocktrans with username=item.user.display_name user_path=item.user.local_path %}Added by {{ username }}{% endblocktrans %}

- {% if list.user != request.user %} - {% if list.curation == 'open' and item.user == request.user %} + {% if list.user == request.user %}
+ {% csrf_token %}
- {% csrf_token %}
@@ -84,13 +83,14 @@
-
+ {% endif %} + {% if list.user == request.user or list.curation == 'open' and item.user == request.user %} + {% csrf_token %}
{% endif %} - {% endif %} From ab317989313ed8b0448cff3117ea0f0cb018d2ee Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 14:02:34 -0700 Subject: [PATCH 030/111] Adds model function to check perms --- bookwyrm/models/base_model.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index aa174a143..afec2271d 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -1,6 +1,8 @@ """ base model with default fields """ import base64 from Crypto import Random + +from django.core.exceptions import PermissionDenied from django.db import models from django.dispatch import receiver from django.utils.translation import gettext_lazy as _ @@ -78,6 +80,31 @@ class BookWyrmModel(models.Model): return True return False + def raise_not_editable(self, viewer): + """does this user have permission to edit this object? liable to be overwritten + by models that inherit this base model class""" + if not hasattr(self, "user"): + return + + # generally moderators shouldn't be able to edit other people's stuff + if self.user == viewer: + return + + raise PermissionDenied + + def raise_not_deletable(self, viewer): + """does this user have permission to delete this object? liable to be + overwritten by models that inherit this base model class""" + if not hasattr(self, "user"): + return + + # but generally moderators can delete other people's stuff + if self.user == viewer or viewer.has_perm("moderate_post"): + return + + raise PermissionDenied + + @receiver(models.signals.post_save) # pylint: disable=unused-argument From 556ae0726b4d47de451c4d3ad35967f05fea143c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 14:03:06 -0700 Subject: [PATCH 031/111] Check perms in list views --- bookwyrm/models/list.py | 6 ++++ bookwyrm/views/list.py | 64 +++++++++++++++++++---------------------- 2 files changed, 36 insertions(+), 34 deletions(-) diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py index 49fb53757..022a0d981 100644 --- a/bookwyrm/models/list.py +++ b/bookwyrm/models/list.py @@ -92,6 +92,12 @@ class ListItem(CollectionItemMixin, BookWyrmModel): notification_type="ADD", ) + def raise_not_deletable(self, viewer): + """the associated user OR the list owner can delete""" + if self.book_list.user == viewer: + return + super().raise_not_deletable(viewer) + class Meta: """A book may only be placed into a list once, and each order in the list may be used only once""" diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index 695302041..c043e4830 100644 --- a/bookwyrm/views/list.py +++ b/bookwyrm/views/list.py @@ -3,7 +3,6 @@ from typing import Optional from urllib.parse import urlencode from django.contrib.auth.decorators import login_required -from django.core.exceptions import PermissionDenied from django.core.paginator import Paginator from django.db import IntegrityError, transaction from django.db.models import Avg, Count, DecimalField, Q, Max @@ -270,8 +269,7 @@ def delete_list(request, list_id): book_list = get_object_or_404(models.List, id=list_id) # only the owner or a moderator can delete a list - if book_list.user != request.user and not request.user.has_perm("moderate_post"): - raise PermissionDenied + book_list.raise_not_editable(request.user) book_list.delete() return redirect("lists") @@ -334,13 +332,11 @@ def remove_book(request, list_id): with transaction.atomic(): book_list = get_object_or_404(models.List, id=list_id) item = get_object_or_404(models.ListItem, id=request.POST.get("item")) - - if not book_list.user == request.user and not item.user == request.user: - return HttpResponseNotFound() + item.raise_not_deletable(request.user) deleted_order = item.order item.delete() - normalize_book_list_ordering(book_list.id, start=deleted_order) + normalize_book_list_ordering(book_list.id, start=deleted_order) return redirect("list", list_id) @@ -351,34 +347,34 @@ def set_book_position(request, list_item_id): Action for when the list user manually specifies a list position, takes special care with the unique ordering per list. """ + list_item = get_object_or_404(models.ListItem, id=list_item_id) + list_item.list.raise_not_editable(request.user) + try: + int_position = int(request.POST.get("position")) + except ValueError: + return HttpResponseBadRequest( + "bad value for position. should be an integer" + ) + + if int_position < 1: + return HttpResponseBadRequest("position cannot be less than 1") + + book_list = list_item.book_list + + # the max position to which a book may be set is the highest order for + # books which are approved + order_max = book_list.listitem_set.filter(approved=True).aggregate( + Max("order") + )["order__max"] + + int_position = min(int_position, order_max) + + original_order = list_item.order + if original_order == int_position: + # no change + return HttpResponse(status=204) + with transaction.atomic(): - list_item = get_object_or_404(models.ListItem, id=list_item_id) - try: - int_position = int(request.POST.get("position")) - except ValueError: - return HttpResponseBadRequest( - "bad value for position. should be an integer" - ) - - if int_position < 1: - return HttpResponseBadRequest("position cannot be less than 1") - - book_list = list_item.book_list - - # the max position to which a book may be set is the highest order for - # books which are approved - order_max = book_list.listitem_set.filter(approved=True).aggregate( - Max("order") - )["order__max"] - - int_position = min(int_position, order_max) - - if request.user not in (book_list.user, list_item.user): - return HttpResponseNotFound() - - original_order = list_item.order - if original_order == int_position: - return HttpResponse(status=204) if original_order > int_position: list_item.order = -1 list_item.save() From 3657f9e0df8701f6c80f7571d0311b9ea4a3917e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 14:03:17 -0700 Subject: [PATCH 032/111] Check perms in status views --- bookwyrm/models/status.py | 9 +++++++++ bookwyrm/views/status.py | 9 ++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 3a0fad5e5..e4f28e000 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -3,6 +3,7 @@ from dataclasses import MISSING import re from django.apps import apps +from django.core.exceptions import PermissionDenied from django.core.validators import MaxValueValidator, MinValueValidator from django.db import models from django.dispatch import receiver @@ -187,6 +188,14 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): """json serialized activitypub class""" return self.to_activity_dataclass(pure=pure).serialize() + def raise_not_editable(self, viewer): + """certain types of status aren't editable""" + # first, the standard raise + super().raise_not_editable(viewer) + if isinstance(self, (GeneratedNote, ReviewRating)): + raise PermissionDenied + + class GeneratedNote(Status): """these are app-generated messages about user activity""" diff --git a/bookwyrm/views/status.py b/bookwyrm/views/status.py index a8b3ab0a2..3da8c7258 100644 --- a/bookwyrm/views/status.py +++ b/bookwyrm/views/status.py @@ -98,8 +98,7 @@ class DeleteStatus(View): status = get_object_or_404(models.Status, id=status_id) # don't let people delete other people's statuses - if status.user != request.user and not request.user.has_perm("moderate_post"): - return HttpResponseBadRequest() + status.raise_not_deletable(request.user) # perform deletion status.delete() @@ -115,12 +114,8 @@ class DeleteAndRedraft(View): status = get_object_or_404( models.Status.objects.select_subclasses(), id=status_id ) - if isinstance(status, (models.GeneratedNote, models.ReviewRating)): - return HttpResponseBadRequest() - # don't let people redraft other people's statuses - if status.user != request.user: - return HttpResponseBadRequest() + status.raise_not_editable(request.user) status_type = status.status_type.lower() if status.reply_parent: From 3f10ae248abd1caba3a9456a289ddbc424dd7b46 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 15:54:58 -0700 Subject: [PATCH 033/111] Changes visiblity function to raise --- bookwyrm/models/base_model.py | 19 +++++++------- bookwyrm/tests/models/test_base_model.py | 32 ++++++++++++++---------- 2 files changed, 29 insertions(+), 22 deletions(-) diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index afec2271d..dcb92f1f6 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -5,6 +5,7 @@ from Crypto import Random from django.core.exceptions import PermissionDenied from django.db import models from django.dispatch import receiver +from django.http import Http404 from django.utils.translation import gettext_lazy as _ from bookwyrm.settings import DOMAIN @@ -50,26 +51,26 @@ class BookWyrmModel(models.Model): """how to link to this object in the local app""" return self.get_remote_id().replace(f"https://{DOMAIN}", "") - def visible_to_user(self, viewer): + def raise_visible_to_user(self, viewer): """is a user authorized to view an object?""" # make sure this is an object with privacy owned by a user if not hasattr(self, "user") or not hasattr(self, "privacy"): - return None + return # viewer can't see it if the object's owner blocked them if viewer in self.user.blocks.all(): - return False + raise Http404() # you can see your own posts and any public or unlisted posts if viewer == self.user or self.privacy in ["public", "unlisted"]: - return True + return # you can see the followers only posts of people you follow if ( self.privacy == "followers" and self.user.followers.filter(id=viewer.id).first() ): - return True + return # you can see dms you are tagged in if hasattr(self, "mention_users"): @@ -77,8 +78,8 @@ class BookWyrmModel(models.Model): self.privacy == "direct" and self.mention_users.filter(id=viewer.id).first() ): - return True - return False + return + raise Http404() def raise_not_editable(self, viewer): """does this user have permission to edit this object? liable to be overwritten @@ -90,7 +91,7 @@ class BookWyrmModel(models.Model): if self.user == viewer: return - raise PermissionDenied + raise PermissionDenied() def raise_not_deletable(self, viewer): """does this user have permission to delete this object? liable to be @@ -102,7 +103,7 @@ class BookWyrmModel(models.Model): if self.user == viewer or viewer.has_perm("moderate_post"): return - raise PermissionDenied + raise PermissionDenied() diff --git a/bookwyrm/tests/models/test_base_model.py b/bookwyrm/tests/models/test_base_model.py index 285647405..dc857a044 100644 --- a/bookwyrm/tests/models/test_base_model.py +++ b/bookwyrm/tests/models/test_base_model.py @@ -1,5 +1,6 @@ """ testing models """ from unittest.mock import patch +from django.http import Http404 from django.test import TestCase from bookwyrm import models @@ -39,14 +40,14 @@ class BaseModel(TestCase): """these should be generated""" self.test_model.id = 1 expected = self.test_model.get_remote_id() - self.assertEqual(expected, "https://%s/bookwyrmtestmodel/1" % DOMAIN) + self.assertEqual(expected, f"https://{DOMAIN}/bookwyrmtestmodel/1") def test_remote_id_with_user(self): """format of remote id when there's a user object""" self.test_model.user = self.local_user self.test_model.id = 1 expected = self.test_model.get_remote_id() - self.assertEqual(expected, "https://%s/user/mouse/bookwyrmtestmodel/1" % DOMAIN) + self.assertEqual(expected, f"https://{DOMAIN}/user/mouse/bookwyrmtestmodel/1") def test_set_remote_id(self): """this function sets remote ids after creation""" @@ -56,7 +57,7 @@ class BaseModel(TestCase): instance.remote_id = None base_model.set_remote_id(None, instance, True) self.assertEqual( - instance.remote_id, "https://%s/book/%d" % (DOMAIN, instance.id) + instance.remote_id, f"https://{DOMAIN}/book/{instance.id}" ) # shouldn't set remote_id if it's not created @@ -70,28 +71,30 @@ class BaseModel(TestCase): obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="public" ) - self.assertTrue(obj.visible_to_user(self.local_user)) + self.assertIsNone(obj.raise_visible_to_user(self.local_user)) obj = models.Shelf.objects.create( name="test", user=self.remote_user, privacy="unlisted" ) - self.assertTrue(obj.visible_to_user(self.local_user)) + self.assertIsNone(obj.raise_visible_to_user(self.local_user)) obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="followers" ) - self.assertFalse(obj.visible_to_user(self.local_user)) + with self.assertRaise(Http404): + obj.raise_visible_to_user(self.local_user) obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="direct" ) - self.assertFalse(obj.visible_to_user(self.local_user)) + with self.assertRaise(Http404): + obj.raise_visible_to_user(self.local_user) obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="direct" ) obj.mention_users.add(self.local_user) - self.assertTrue(obj.visible_to_user(self.local_user)) + self.assertIsNone(obj.raise_visible_to_user(self.local_user)) @patch("bookwyrm.activitystreams.add_status_task.delay") def test_object_visible_to_user_follower(self, _): @@ -100,18 +103,19 @@ class BaseModel(TestCase): obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="followers" ) - self.assertTrue(obj.visible_to_user(self.local_user)) + self.assertIsNone(obj.raise_visible_to_user(self.local_user)) obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="direct" ) - self.assertFalse(obj.visible_to_user(self.local_user)) + with self.assertRaise(Http404): + obj.raise_visible_to_user(self.local_user) obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="direct" ) obj.mention_users.add(self.local_user) - self.assertTrue(obj.visible_to_user(self.local_user)) + self.assertIsNone(obj.raise_visible_to_user(self.local_user)) @patch("bookwyrm.activitystreams.add_status_task.delay") def test_object_visible_to_user_blocked(self, _): @@ -120,9 +124,11 @@ class BaseModel(TestCase): obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="public" ) - self.assertFalse(obj.visible_to_user(self.local_user)) + with self.assertRaise(Http404): + obj.raise_visible_to_user(self.local_user) obj = models.Shelf.objects.create( name="test", user=self.remote_user, privacy="unlisted" ) - self.assertFalse(obj.visible_to_user(self.local_user)) + with self.assertRaise(Http404): + obj.raise_visible_to_user(self.local_user) From 84443c7f81caff5a3811169ad7972d4e1c3cabb5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 15:55:55 -0700 Subject: [PATCH 034/111] Custom perms function for shelf view --- bookwyrm/models/shelf.py | 7 ++++ bookwyrm/views/shelf.py | 79 +++++++++++++++++++--------------------- 2 files changed, 44 insertions(+), 42 deletions(-) diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py index 6f1344022..89ea9471d 100644 --- a/bookwyrm/models/shelf.py +++ b/bookwyrm/models/shelf.py @@ -1,5 +1,6 @@ """ puttin' books on shelves """ import re +from django.core.exceptions import PermissionDenied from django.db import models from django.utils import timezone @@ -57,6 +58,12 @@ class Shelf(OrderedCollectionMixin, BookWyrmModel): identifier = self.identifier or self.get_identifier() return f"{base_path}/books/{identifier}" + def raise_not_deletable(self, viewer): + """don't let anyone delete a default shelf""" + super().raise_not_deletable(viewer) + if not self.editable: + raise PermissionDenied() + class Meta: """user/shelf unqiueness""" diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index 414ace39e..2cce84d9d 100644 --- a/bookwyrm/views/shelf.py +++ b/bookwyrm/views/shelf.py @@ -1,11 +1,11 @@ -""" shelf views""" +""" shelf views """ from collections import namedtuple -from django.db import IntegrityError +from django.db import IntegrityError, transaction from django.db.models import OuterRef, Subquery, F from django.contrib.auth.decorators import login_required from django.core.paginator import Paginator -from django.http import HttpResponseBadRequest, HttpResponseNotFound +from django.http import HttpResponseBadRequest from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse from django.utils.decorators import method_decorator @@ -16,7 +16,7 @@ from django.views.decorators.http import require_POST from bookwyrm import forms, models from bookwyrm.activitypub import ActivitypubResponse from bookwyrm.settings import PAGE_LENGTH -from .helpers import is_api_request, get_edition, get_user_from_username +from .helpers import is_api_request, get_user_from_username from .helpers import privacy_filter @@ -37,15 +37,11 @@ class Shelf(View): # get the shelf and make sure the logged in user should be able to see it if shelf_identifier: - try: - shelf = user.shelf_set.get(identifier=shelf_identifier) - except models.Shelf.DoesNotExist: - return HttpResponseNotFound() - if not shelf.visible_to_user(request.user): - return HttpResponseNotFound() + shelf = get_object_or_404(user.shelf_set, identifier=shelf_identifier) + shelf.raise_visible_to_user(request.user) books = shelf.books - # this is a constructed "all books" view, with a fake "shelf" obj else: + # this is a constructed "all books" view, with a fake "shelf" obj FakeShelf = namedtuple( "Shelf", ("identifier", "name", "user", "books", "privacy") ) @@ -100,13 +96,11 @@ class Shelf(View): # pylint: disable=unused-argument def post(self, request, username, shelf_identifier): """edit a shelf""" - try: - shelf = request.user.shelf_set.get(identifier=shelf_identifier) - except models.Shelf.DoesNotExist: - return HttpResponseNotFound() + user = get_user_from_username(request.user, username) + shelf = get_object_or_404(user.shelf_set, identifier=shelf_identifier) + shelf.raise_not_editable(request.user) - if request.user != shelf.user: - return HttpResponseBadRequest() + # you can't change the name of the default shelves if not shelf.editable and request.POST.get("name") != shelf.name: return HttpResponseBadRequest() @@ -134,8 +128,7 @@ def create_shelf(request): def delete_shelf(request, shelf_id): """user generated shelves""" shelf = get_object_or_404(models.Shelf, id=shelf_id) - if request.user != shelf.user or not shelf.editable: - return HttpResponseBadRequest() + shelf.raise_not_deletable() shelf.delete() return redirect("user-shelves", request.user.localname) @@ -143,25 +136,28 @@ def delete_shelf(request, shelf_id): @login_required @require_POST +@transaction.atomic def shelve(request): """put a book on a user's shelf""" - book = get_edition(request.POST.get("book")) - - desired_shelf = models.Shelf.objects.filter( - identifier=request.POST.get("shelf"), user=request.user - ).first() - if not desired_shelf: - return HttpResponseNotFound() + book = get_object_or_404(models.Edition, id=request.POST.get("book")) + desired_shelf = get_object_or_404( + request.user.shelf_set, identifier=request.POST.get("shelf") + ) + # first we need to remove from the specified shelf change_from_current_identifier = request.POST.get("change-shelf-from") - if change_from_current_identifier is not None: - current_shelf = models.Shelf.objects.get( - user=request.user, identifier=change_from_current_identifier - ) - handle_unshelve(book, current_shelf) + if not change_from_current_identifier: + #find the shelfbook obj and delete it + get_object_or_404( + models.ShelfBook, + book=book, + user=request.user, + shelf__identifier=change_from_current_identifier + ).delete() # A book can be on multiple shelves, but only on one read status shelf at a time if desired_shelf.identifier in models.Shelf.READ_STATUS_IDENTIFIERS: + # figure out where state shelf it's currently on (if any) current_read_status_shelfbook = ( models.ShelfBook.objects.select_related("shelf") .filter( @@ -176,14 +172,16 @@ def shelve(request): current_read_status_shelfbook.shelf.identifier != desired_shelf.identifier ): - handle_unshelve(book, current_read_status_shelfbook.shelf) + current_read_status_shelfbook.delete() else: # It is already on the shelf return redirect(request.headers.get("Referer", "/")) + # create the new shelf-book entry models.ShelfBook.objects.create( book=book, shelf=desired_shelf, user=request.user ) else: + # we're putting it on a custom shelf try: models.ShelfBook.objects.create( book=book, shelf=desired_shelf, user=request.user @@ -198,15 +196,12 @@ def shelve(request): @login_required @require_POST def unshelve(request): - """put a on a user's shelf""" - book = models.Edition.objects.get(id=request.POST["book"]) - current_shelf = models.Shelf.objects.get(id=request.POST["shelf"]) + """put a on a user's shelf""" + book = get_object_or_404(models.Edition, id=request.POST.get("book")) + shelf_book = get_object_or_404( + models.ShelfBook, book=book, shelf__id=request.POST["shelf"] + ) + shelf_book.raise_not_deletable(request.user) - handle_unshelve(book, current_shelf) + shelf_book.delete() return redirect(request.headers.get("Referer", "/")) - - -def handle_unshelve(book, shelf): - """unshelve a book""" - row = models.ShelfBook.objects.get(book=book, shelf=shelf) - row.delete() From e6ae5005698dc02c0d6221152c8853f9e15edc4f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 15:57:22 -0700 Subject: [PATCH 035/111] Fixes syntax problem on http raises --- bookwyrm/forms.py | 2 +- bookwyrm/models/status.py | 3 +-- bookwyrm/urls.py | 2 +- bookwyrm/views/admin/reports.py | 2 +- bookwyrm/views/books.py | 2 +- bookwyrm/views/import_data.py | 2 +- bookwyrm/views/password.py | 4 ++-- bookwyrm/views/register.py | 4 ++-- 8 files changed, 10 insertions(+), 11 deletions(-) diff --git a/bookwyrm/forms.py b/bookwyrm/forms.py index 23063ff7c..b9b93694c 100644 --- a/bookwyrm/forms.py +++ b/bookwyrm/forms.py @@ -228,7 +228,7 @@ class ExpiryWidget(widgets.Select): elif selected_string == "forever": return None else: - return selected_string # "This will raise + return selected_string # This will raise return timezone.now() + interval diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index e4f28e000..b62036788 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -193,8 +193,7 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): # first, the standard raise super().raise_not_editable(viewer) if isinstance(self, (GeneratedNote, ReviewRating)): - raise PermissionDenied - + raise PermissionDenied() class GeneratedNote(Status): diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index f55051812..7d9bd8da3 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -276,7 +276,7 @@ urlpatterns = [ # User books re_path(rf"{USER_PATH}/books/?$", views.Shelf.as_view(), name="user-shelves"), re_path( - rf"^{USER_PATH}/(helf|books)/(?P[\w-]+)(.json)?/?$", + rf"^{USER_PATH}/(shelf|books)/(?P[\w-]+)(.json)?/?$", views.Shelf.as_view(), name="shelf", ), diff --git a/bookwyrm/views/admin/reports.py b/bookwyrm/views/admin/reports.py index 72b7f4db9..23bb6fa3d 100644 --- a/bookwyrm/views/admin/reports.py +++ b/bookwyrm/views/admin/reports.py @@ -105,7 +105,7 @@ def moderator_delete_user(request, user_id): # we can't delete users on other instances if not user.local: - raise PermissionDenied + raise PermissionDenied() form = forms.DeleteUserForm(request.POST, instance=user) diff --git a/bookwyrm/views/books.py b/bookwyrm/views/books.py index 6ab938db2..e2878aca7 100644 --- a/bookwyrm/views/books.py +++ b/bookwyrm/views/books.py @@ -50,7 +50,7 @@ class Book(View): ) if not book or not book.parent_work: - raise Http404 + raise Http404() # all reviews for all editions of the book reviews = privacy_filter( diff --git a/bookwyrm/views/import_data.py b/bookwyrm/views/import_data.py index 2232e39c2..fe54c39a9 100644 --- a/bookwyrm/views/import_data.py +++ b/bookwyrm/views/import_data.py @@ -80,7 +80,7 @@ class ImportStatus(View): """status of an import job""" job = get_object_or_404(models.ImportJob, id=job_id) if job.user != request.user: - raise PermissionDenied + raise PermissionDenied() try: task = app.AsyncResult(job.task_id) diff --git a/bookwyrm/views/password.py b/bookwyrm/views/password.py index 4f0bd50dc..b9cb99ded 100644 --- a/bookwyrm/views/password.py +++ b/bookwyrm/views/password.py @@ -54,9 +54,9 @@ class PasswordReset(View): try: reset_code = models.PasswordReset.objects.get(code=code) if not reset_code.valid(): - raise PermissionDenied + raise PermissionDenied() except models.PasswordReset.DoesNotExist: - raise PermissionDenied + raise PermissionDenied() return TemplateResponse(request, "password_reset.html", {"code": code}) diff --git a/bookwyrm/views/register.py b/bookwyrm/views/register.py index bf927e682..dd8249203 100644 --- a/bookwyrm/views/register.py +++ b/bookwyrm/views/register.py @@ -29,11 +29,11 @@ class Register(View): invite_code = request.POST.get("invite_code") if not invite_code: - raise PermissionDenied + raise PermissionDenied() invite = get_object_or_404(models.SiteInvite, code=invite_code) if not invite.valid(): - raise PermissionDenied + raise PermissionDenied() else: invite = None From dfa8bafe18c39d210c916d9ec9a278238a063438 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 16:04:40 -0700 Subject: [PATCH 036/111] Uses new visible/editable/deleteable functions --- bookwyrm/views/block.py | 13 +++++-------- bookwyrm/views/feed.py | 17 +++++++---------- bookwyrm/views/follow.py | 13 +++++-------- bookwyrm/views/get_started.py | 6 ++---- bookwyrm/views/goal.py | 19 +++++++++---------- bookwyrm/views/list.py | 18 +++++++++--------- bookwyrm/views/reading.py | 19 +++++-------------- bookwyrm/views/user.py | 9 +++++++-- bookwyrm/views/wellknown.py | 6 ++---- 9 files changed, 51 insertions(+), 69 deletions(-) diff --git a/bookwyrm/views/block.py b/bookwyrm/views/block.py index 0b80c327d..90b3be90c 100644 --- a/bookwyrm/views/block.py +++ b/bookwyrm/views/block.py @@ -1,6 +1,5 @@ """ views for actions you can take in the application """ from django.contrib.auth.decorators import login_required -from django.http import HttpResponseNotFound from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse from django.utils.decorators import method_decorator @@ -32,12 +31,10 @@ class Block(View): def unblock(request, user_id): """undo a block""" to_unblock = get_object_or_404(models.User, id=user_id) - try: - block = models.UserBlocks.objects.get( - user_subject=request.user, - user_object=to_unblock, - ) - except models.UserBlocks.DoesNotExist: - return HttpResponseNotFound() + block = get_object_or_404( + models.UserBlocks, + user_subject=request.user, + user_object=to_unblock, + ) block.delete() return redirect("prefs-block") diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index 09d45065b..ea3bb8f8b 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -3,6 +3,7 @@ from django.contrib.auth.decorators import login_required from django.core.paginator import Paginator from django.db.models import Q from django.http import HttpResponseNotFound, Http404 +from django.shortcuts import get_object_or_404 from django.template.response import TemplateResponse from django.utils import timezone from django.utils.decorators import method_decorator @@ -93,17 +94,12 @@ class Status(View): def get(self, request, username, status_id): """display a particular status (and replies, etc)""" - try: - user = get_user_from_username(request.user, username) - status = models.Status.objects.select_subclasses().get( - user=user, id=status_id, deleted=False - ) - except (ValueError, models.Status.DoesNotExist): - return HttpResponseNotFound() - + user = get_user_from_username(request.user, username) + status = get_object_or_404(models.Status.objects.select_subclasses(), + user=user, id=status_id, deleted=False + ) # make sure the user is authorized to see the status - if not status.visible_to_user(request.user): - return HttpResponseNotFound() + status.raise_visible_to_user(request.user) if is_api_request(request): return ActivitypubResponse( @@ -133,6 +129,7 @@ class Replies(View): status = models.Status.objects.get(id=status_id) if status.user.localname != username: return HttpResponseNotFound() + status.raise_visible_to_user(request.user) return ActivitypubResponse(status.to_replies(**request.GET)) diff --git a/bookwyrm/views/follow.py b/bookwyrm/views/follow.py index 0710a70c5..8cbb1d253 100644 --- a/bookwyrm/views/follow.py +++ b/bookwyrm/views/follow.py @@ -1,8 +1,7 @@ """ views for actions you can take in the application """ from django.contrib.auth.decorators import login_required from django.db import IntegrityError -from django.http import HttpResponseBadRequest -from django.shortcuts import redirect +from django.shortcuts import get_object_or_404, redirect from django.views.decorators.http import require_POST from bookwyrm import models @@ -78,12 +77,10 @@ def delete_follow_request(request): username = request.POST["user"] requester = get_user_from_username(request.user, username) - try: - follow_request = models.UserFollowRequest.objects.get( - user_subject=requester, user_object=request.user - ) - except models.UserFollowRequest.DoesNotExist: - return HttpResponseBadRequest() + follow_request = get_object_or_404(models.UserFollowRequest, + user_subject=requester, user_object=request.user + ) + follow_request.raise_not_deletable(request.user) follow_request.delete() return redirect(f"/user/{request.user.localname}") diff --git a/bookwyrm/views/get_started.py b/bookwyrm/views/get_started.py index 3de88e104..4a6b55c58 100644 --- a/bookwyrm/views/get_started.py +++ b/bookwyrm/views/get_started.py @@ -5,7 +5,6 @@ from django.contrib.auth.decorators import login_required from django.contrib.postgres.search import TrigramSimilarity from django.db.models.functions import Greatest from django.db.models import Count, Q -from django.http import HttpResponseNotFound from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse from django.utils.decorators import method_decorator @@ -91,9 +90,8 @@ class GetStartedBooks(View): for (book_id, shelf_id) in shelve_actions: book = get_object_or_404(models.Edition, id=book_id) shelf = get_object_or_404(models.Shelf, id=shelf_id) - if shelf.user != request.user: - # hmmmmm - return HttpResponseNotFound() + shelf.raise_not_editable(request.user) + models.ShelfBook.objects.create(book=book, shelf=shelf, user=request.user) return redirect(self.next_view) diff --git a/bookwyrm/views/goal.py b/bookwyrm/views/goal.py index 122245179..1a0a99bf5 100644 --- a/bookwyrm/views/goal.py +++ b/bookwyrm/views/goal.py @@ -31,8 +31,7 @@ class Goal(View): if not goal and year != timezone.now().year: return redirect("user-goal", username, current_year) - if goal and not goal.visible_to_user(request.user): - return HttpResponseNotFound() + goal.raise_visible_to_user(request.user) data = { "goal_form": forms.GoalForm(instance=goal), @@ -45,12 +44,12 @@ class Goal(View): def post(self, request, username, year): """update or create an annual goal""" - user = get_user_from_username(request.user, username) - if user != request.user: - return HttpResponseNotFound() - year = int(year) - goal = models.AnnualGoal.objects.filter(year=year, user=request.user).first() + user = get_user_from_username(request.user, username) + goal = models.AnnualGoal.objects.filter(year=year, user=user).first() + if goal: + goal.raise_not_editable(request.user) + form = forms.GoalForm(request.POST, instance=goal) if not form.is_valid(): data = { @@ -62,11 +61,11 @@ class Goal(View): goal = form.save() if request.POST.get("post-status"): - # create status, if appropraite + # create status, if appropriate template = get_template("snippets/generated_status/goal.html") create_generated_note( request.user, - template.render({"goal": goal, "user": request.user}).strip(), + template.render({"goal": goal, "user": user}).strip(), privacy=goal.privacy, ) @@ -78,5 +77,5 @@ class Goal(View): def hide_goal(request): """don't keep bugging people to set a goal""" request.user.show_goal = False - request.user.save(broadcast=False) + request.user.save(broadcast=False, update_fields=["show_goal"]) return redirect(request.headers.get("Referer", "/")) diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index c043e4830..99fee3f86 100644 --- a/bookwyrm/views/list.py +++ b/bookwyrm/views/list.py @@ -7,7 +7,7 @@ from django.core.paginator import Paginator from django.db import IntegrityError, transaction from django.db.models import Avg, Count, DecimalField, Q, Max from django.db.models.functions import Coalesce -from django.http import HttpResponseNotFound, HttpResponseBadRequest, HttpResponse +from django.http import HttpResponseBadRequest, HttpResponse from django.shortcuts import get_object_or_404, redirect from django.template.response import TemplateResponse from django.urls import reverse @@ -110,8 +110,7 @@ class List(View): def get(self, request, list_id): """display a book list""" book_list = get_object_or_404(models.List, id=list_id) - if not book_list.visible_to_user(request.user): - return HttpResponseNotFound() + book_list.raise_visible_to_user(request.user) if is_api_request(request): return ActivitypubResponse(book_list.to_activity(**request.GET)) @@ -192,6 +191,8 @@ class List(View): def post(self, request, list_id): """edit a list""" book_list = get_object_or_404(models.List, id=list_id) + book_list.raise_not_editable(request.user) + form = forms.ListForm(request.POST, instance=book_list) if not form.is_valid(): return redirect("list", book_list.id) @@ -206,9 +207,7 @@ class Curate(View): def get(self, request, list_id): """display a pending list""" book_list = get_object_or_404(models.List, id=list_id) - if not book_list.user == request.user: - # only the creater can curate the list - return HttpResponseNotFound() + book_list.raise_not_editable(request.user) data = { "list": book_list, @@ -222,6 +221,8 @@ class Curate(View): def post(self, request, list_id): """edit a book_list""" book_list = get_object_or_404(models.List, id=list_id) + book_list.raise_not_editable(request.user) + suggestion = get_object_or_404(models.ListItem, id=request.POST.get("item")) approved = request.POST.get("approved") == "true" if approved: @@ -269,7 +270,7 @@ def delete_list(request, list_id): book_list = get_object_or_404(models.List, id=list_id) # only the owner or a moderator can delete a list - book_list.raise_not_editable(request.user) + book_list.raise_not_deletable(request.user) book_list.delete() return redirect("lists") @@ -280,8 +281,7 @@ def delete_list(request, list_id): def add_book(request): """put a book on a list""" book_list = get_object_or_404(models.List, id=request.POST.get("list")) - if not book_list.visible_to_user(request.user): - return HttpResponseNotFound() + book_list.raise_visible_to_user(request.user) book = get_object_or_404(models.Edition, id=request.POST.get("book")) # do you have permission to add to the list? diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index 09b4b2bf8..8d0020c20 100644 --- a/bookwyrm/views/reading.py +++ b/bookwyrm/views/reading.py @@ -45,9 +45,9 @@ class ReadingStatus(View): if not identifier: return HttpResponseBadRequest() - desired_shelf = models.Shelf.objects.filter( + desired_shelf = get_object_or_404(models.Shelf, identifier=identifier, user=request.user - ).first() + ) book = ( models.Edition.viewer_aware_objects(request.user) @@ -138,10 +138,7 @@ def update_readthrough_on_shelve( def edit_readthrough(request): """can't use the form because the dates are too finnicky""" readthrough = get_object_or_404(models.ReadThrough, id=request.POST.get("id")) - - # don't let people edit other people's data - if request.user != readthrough.user: - return HttpResponseBadRequest() + readthrough.raise_not_editable(request.user) readthrough.start_date = load_date_in_user_tz_as_utc( request.POST.get("start_date"), request.user @@ -178,10 +175,7 @@ def edit_readthrough(request): def delete_readthrough(request): """remove a readthrough""" readthrough = get_object_or_404(models.ReadThrough, id=request.POST.get("id")) - - # don't let people edit other people's data - if request.user != readthrough.user: - return HttpResponseBadRequest() + readthrough.raise_not_deletable(request.user) readthrough.delete() return redirect(request.headers.get("Referer", "/")) @@ -225,10 +219,7 @@ def load_date_in_user_tz_as_utc(date_str: str, user: models.User) -> datetime: def delete_progressupdate(request): """remove a progress update""" update = get_object_or_404(models.ProgressUpdate, id=request.POST.get("id")) - - # don't let people edit other people's data - if request.user != update.user: - return HttpResponseBadRequest() + update.raise_not_deletable(request.user) update.delete() return redirect(request.headers.get("Referer", "/")) diff --git a/bookwyrm/views/user.py b/bookwyrm/views/user.py index ca6eb0a52..fb553228b 100644 --- a/bookwyrm/views/user.py +++ b/bookwyrm/views/user.py @@ -1,6 +1,7 @@ """ non-interactive pages """ from django.contrib.auth.decorators import login_required from django.core.paginator import Paginator +from django.http import Http404 from django.shortcuts import redirect from django.template.response import TemplateResponse from django.utils import timezone @@ -77,8 +78,12 @@ class User(View): goal = models.AnnualGoal.objects.filter( user=user, year=timezone.now().year ).first() - if goal and not goal.visible_to_user(request.user): - goal = None + if goal: + try: + goal.raise_visible_to_user(request.user) + except Http404: + goal = None + data = { "user": user, "is_self": is_self, diff --git a/bookwyrm/views/wellknown.py b/bookwyrm/views/wellknown.py index 660a45244..3350ae31e 100644 --- a/bookwyrm/views/wellknown.py +++ b/bookwyrm/views/wellknown.py @@ -3,6 +3,7 @@ from dateutil.relativedelta import relativedelta from django.http import HttpResponseNotFound from django.http import JsonResponse +from django.shortcuts import get_object_or_404 from django.template.response import TemplateResponse from django.utils import timezone from django.views.decorators.http import require_GET @@ -19,10 +20,7 @@ def webfinger(request): return HttpResponseNotFound() username = resource.replace("acct:", "") - try: - user = models.User.objects.get(username__iexact=username) - except models.User.DoesNotExist: - return HttpResponseNotFound("No account found") + user = get_object_or_404(models.User, username__iexact=username) return JsonResponse( { From af2f78095e20e536fe5e666eb33064e29f8ad571 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 16:06:01 -0700 Subject: [PATCH 037/111] Fixes assert syntax --- bookwyrm/tests/models/test_base_model.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bookwyrm/tests/models/test_base_model.py b/bookwyrm/tests/models/test_base_model.py index dc857a044..b94dd331d 100644 --- a/bookwyrm/tests/models/test_base_model.py +++ b/bookwyrm/tests/models/test_base_model.py @@ -81,13 +81,13 @@ class BaseModel(TestCase): obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="followers" ) - with self.assertRaise(Http404): + with self.assertRaises(Http404): obj.raise_visible_to_user(self.local_user) obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="direct" ) - with self.assertRaise(Http404): + with self.assertRaises(Http404): obj.raise_visible_to_user(self.local_user) obj = models.Status.objects.create( @@ -108,7 +108,7 @@ class BaseModel(TestCase): obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="direct" ) - with self.assertRaise(Http404): + with self.assertRaises(Http404): obj.raise_visible_to_user(self.local_user) obj = models.Status.objects.create( @@ -124,11 +124,11 @@ class BaseModel(TestCase): obj = models.Status.objects.create( content="hi", user=self.remote_user, privacy="public" ) - with self.assertRaise(Http404): + with self.assertRaises(Http404): obj.raise_visible_to_user(self.local_user) obj = models.Shelf.objects.create( name="test", user=self.remote_user, privacy="unlisted" ) - with self.assertRaise(Http404): + with self.assertRaises(Http404): obj.raise_visible_to_user(self.local_user) From 62ff9d6199922ea0a6b416075e0e383b6f545e85 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 16:08:52 -0700 Subject: [PATCH 038/111] Python formatting --- bookwyrm/models/base_model.py | 1 - bookwyrm/tests/models/test_base_model.py | 4 +--- bookwyrm/views/feed.py | 7 +++++-- bookwyrm/views/follow.py | 4 ++-- bookwyrm/views/list.py | 10 ++++------ bookwyrm/views/reading.py | 4 ++-- bookwyrm/views/shelf.py | 4 ++-- 7 files changed, 16 insertions(+), 18 deletions(-) diff --git a/bookwyrm/models/base_model.py b/bookwyrm/models/base_model.py index dcb92f1f6..ca32521a3 100644 --- a/bookwyrm/models/base_model.py +++ b/bookwyrm/models/base_model.py @@ -106,7 +106,6 @@ class BookWyrmModel(models.Model): raise PermissionDenied() - @receiver(models.signals.post_save) # pylint: disable=unused-argument def set_remote_id(sender, instance, created, *args, **kwargs): diff --git a/bookwyrm/tests/models/test_base_model.py b/bookwyrm/tests/models/test_base_model.py index b94dd331d..7c7b48de1 100644 --- a/bookwyrm/tests/models/test_base_model.py +++ b/bookwyrm/tests/models/test_base_model.py @@ -56,9 +56,7 @@ class BaseModel(TestCase): instance = models.Work.objects.create(title="work title") instance.remote_id = None base_model.set_remote_id(None, instance, True) - self.assertEqual( - instance.remote_id, f"https://{DOMAIN}/book/{instance.id}" - ) + self.assertEqual(instance.remote_id, f"https://{DOMAIN}/book/{instance.id}") # shouldn't set remote_id if it's not created instance.remote_id = None diff --git a/bookwyrm/views/feed.py b/bookwyrm/views/feed.py index ea3bb8f8b..0c2647aea 100644 --- a/bookwyrm/views/feed.py +++ b/bookwyrm/views/feed.py @@ -95,8 +95,11 @@ class Status(View): def get(self, request, username, status_id): """display a particular status (and replies, etc)""" user = get_user_from_username(request.user, username) - status = get_object_or_404(models.Status.objects.select_subclasses(), - user=user, id=status_id, deleted=False + status = get_object_or_404( + models.Status.objects.select_subclasses(), + user=user, + id=status_id, + deleted=False, ) # make sure the user is authorized to see the status status.raise_visible_to_user(request.user) diff --git a/bookwyrm/views/follow.py b/bookwyrm/views/follow.py index 8cbb1d253..7d91ce5b4 100644 --- a/bookwyrm/views/follow.py +++ b/bookwyrm/views/follow.py @@ -77,8 +77,8 @@ def delete_follow_request(request): username = request.POST["user"] requester = get_user_from_username(request.user, username) - follow_request = get_object_or_404(models.UserFollowRequest, - user_subject=requester, user_object=request.user + follow_request = get_object_or_404( + models.UserFollowRequest, user_subject=requester, user_object=request.user ) follow_request.raise_not_deletable(request.user) diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index 99fee3f86..332a5d1d6 100644 --- a/bookwyrm/views/list.py +++ b/bookwyrm/views/list.py @@ -352,9 +352,7 @@ def set_book_position(request, list_item_id): try: int_position = int(request.POST.get("position")) except ValueError: - return HttpResponseBadRequest( - "bad value for position. should be an integer" - ) + return HttpResponseBadRequest("bad value for position. should be an integer") if int_position < 1: return HttpResponseBadRequest("position cannot be less than 1") @@ -363,9 +361,9 @@ def set_book_position(request, list_item_id): # the max position to which a book may be set is the highest order for # books which are approved - order_max = book_list.listitem_set.filter(approved=True).aggregate( - Max("order") - )["order__max"] + order_max = book_list.listitem_set.filter(approved=True).aggregate(Max("order"))[ + "order__max" + ] int_position = min(int_position, order_max) diff --git a/bookwyrm/views/reading.py b/bookwyrm/views/reading.py index 8d0020c20..02ee0a189 100644 --- a/bookwyrm/views/reading.py +++ b/bookwyrm/views/reading.py @@ -45,8 +45,8 @@ class ReadingStatus(View): if not identifier: return HttpResponseBadRequest() - desired_shelf = get_object_or_404(models.Shelf, - identifier=identifier, user=request.user + desired_shelf = get_object_or_404( + models.Shelf, identifier=identifier, user=request.user ) book = ( diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index 2cce84d9d..5d1008d09 100644 --- a/bookwyrm/views/shelf.py +++ b/bookwyrm/views/shelf.py @@ -147,12 +147,12 @@ def shelve(request): # first we need to remove from the specified shelf change_from_current_identifier = request.POST.get("change-shelf-from") if not change_from_current_identifier: - #find the shelfbook obj and delete it + # find the shelfbook obj and delete it get_object_or_404( models.ShelfBook, book=book, user=request.user, - shelf__identifier=change_from_current_identifier + shelf__identifier=change_from_current_identifier, ).delete() # A book can be on multiple shelves, but only on one read status shelf at a time From c874a762dd707bb325d3d85b2889907ec9f5ce5f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 17:27:17 -0700 Subject: [PATCH 039/111] Updates inbox view --- bookwyrm/tests/views/inbox/test_inbox.py | 25 ++++++++------- bookwyrm/views/inbox.py | 40 +++++++++++------------- 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/bookwyrm/tests/views/inbox/test_inbox.py b/bookwyrm/tests/views/inbox/test_inbox.py index a84458ab2..47e6a86e7 100644 --- a/bookwyrm/tests/views/inbox/test_inbox.py +++ b/bookwyrm/tests/views/inbox/test_inbox.py @@ -3,6 +3,7 @@ import json import pathlib from unittest.mock import patch +from django.core.exceptions import PermissionDenied from django.http import HttpResponseNotAllowed, HttpResponseNotFound from django.test import TestCase, Client from django.test.client import RequestFactory @@ -130,22 +131,24 @@ class Inbox(TestCase): "", HTTP_USER_AGENT="http.rb/4.4.1 (Mastodon/3.3.0; +https://mastodon.social/)", ) - self.assertFalse(views.inbox.is_blocked_user_agent(request)) + self.assertIsNone(views.inbox.raise_is_blocked_user_agent(request)) models.FederatedServer.objects.create( server_name="mastodon.social", status="blocked" ) - self.assertTrue(views.inbox.is_blocked_user_agent(request)) + with self.assertRaises(PermissionDenied): + views.inbox.raise_is_blocked_user_agent(request) def test_is_blocked_activity(self): """check for blocked servers""" activity = {"actor": "https://mastodon.social/user/whaatever/else"} - self.assertFalse(views.inbox.is_blocked_activity(activity)) + self.assertIsNone(views.inbox.raise_is_blocked_activity(activity)) models.FederatedServer.objects.create( server_name="mastodon.social", status="blocked" ) - self.assertTrue(views.inbox.is_blocked_activity(activity)) + with self.assertRaises(PermissionDenied): + views.inbox.raise_is_blocked_activity(activity) @patch("bookwyrm.suggested_users.remove_user_task.delay") def test_create_by_deactivated_user(self, _): @@ -157,11 +160,11 @@ class Inbox(TestCase): activity = self.create_json activity["actor"] = self.remote_user.remote_id activity["object"] = status_data + activity["type"] = "Create" - with patch("bookwyrm.views.inbox.has_valid_signature") as mock_valid: - mock_valid.return_value = True - - result = self.client.post( - "/inbox", json.dumps(activity), content_type="application/json" - ) - self.assertEqual(result.status_code, 403) + response = self.client.post( + "/inbox", + json.dumps(activity), + content_type="application/json", + ) + self.assertEqual(response.status_code, 403) diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index ff01cfb2f..239824958 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -3,8 +3,9 @@ import json import re from urllib.parse import urldefrag -from django.http import HttpResponse, HttpResponseNotFound -from django.http import HttpResponseBadRequest, HttpResponseForbidden +from django.http import HttpResponse, Http404 +from django.core.exceptions import BadRequest, PermissionDenied +from django.shortcuts import get_object_or_404 from django.utils.decorators import method_decorator from django.views import View from django.views.decorators.csrf import csrf_exempt @@ -21,36 +22,30 @@ from bookwyrm.utils import regex class Inbox(View): """requests sent by outside servers""" - # pylint: disable=too-many-return-statements def post(self, request, username=None): """only works as POST request""" # first check if this server is on our shitlist - if is_blocked_user_agent(request): - return HttpResponseForbidden() + raise_is_blocked_user_agent(request) # make sure the user's inbox even exists if username: - try: - models.User.objects.get(localname=username) - except models.User.DoesNotExist: - return HttpResponseNotFound() + get_object_or_404(models.User, localname=username, is_active=True) # is it valid json? does it at least vaguely resemble an activity? try: activity_json = json.loads(request.body) except json.decoder.JSONDecodeError: - return HttpResponseBadRequest() + raise BadRequest() # let's be extra sure we didn't block this domain - if is_blocked_activity(activity_json): - return HttpResponseForbidden() + raise_is_blocked_activity(activity_json) if ( not "object" in activity_json or not "type" in activity_json or not activity_json["type"] in activitypub.activity_objects ): - return HttpResponseNotFound() + raise Http404() # verify the signature if not has_valid_signature(request, activity_json): @@ -65,32 +60,35 @@ class Inbox(View): return HttpResponse() -def is_blocked_user_agent(request): +def raise_is_blocked_user_agent(request): """check if a request is from a blocked server based on user agent""" # check user agent user_agent = request.headers.get("User-Agent") if not user_agent: - return False + return url = re.search(rf"https?://{regex.DOMAIN}/?", user_agent) if not url: - return False + return url = url.group() - return models.FederatedServer.is_blocked(url) + if models.FederatedServer.is_blocked(url): + raise PermissionDenied() -def is_blocked_activity(activity_json): +def raise_is_blocked_activity(activity_json): """get the sender out of activity json and check if it's blocked""" actor = activity_json.get("actor") # check if the user is banned/deleted existing = models.User.find_existing_by_remote_id(actor) if existing and existing.deleted: - return True + raise PermissionDenied() if not actor: # well I guess it's not even a valid activity so who knows - return False - return models.FederatedServer.is_blocked(actor) + return + + if models.FederatedServer.is_blocked(actor): + raise PermissionDenied() @app.task(queue="medium_priority") From 6c0b1da83b4201458c7303ecb3c2a322c0ec3827 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 17:32:54 -0700 Subject: [PATCH 040/111] Fixes feed tests --- bookwyrm/tests/views/test_feed.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/bookwyrm/tests/views/test_feed.py b/bookwyrm/tests/views/test_feed.py index 526cb0f99..a6f220b58 100644 --- a/bookwyrm/tests/views/test_feed.py +++ b/bookwyrm/tests/views/test_feed.py @@ -5,6 +5,7 @@ import pathlib from PIL import Image from django.core.files.base import ContentFile +from django.http import Http404 from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory @@ -81,9 +82,8 @@ class FeedViews(TestCase): request.user = self.local_user with patch("bookwyrm.views.feed.is_api_request") as is_api: is_api.return_value = False - result = view(request, "mouse", 12345) - - self.assertEqual(result.status_code, 404) + with self.assertRaises(Http404): + view(request, "mouse", 12345) def test_status_page_not_found_wrong_user(self, *_): """there are so many views, this just makes sure it LOADS""" @@ -102,9 +102,8 @@ class FeedViews(TestCase): request.user = self.local_user with patch("bookwyrm.views.feed.is_api_request") as is_api: is_api.return_value = False - result = view(request, "mouse", status.id) - - self.assertEqual(result.status_code, 404) + with self.assertRaises(Http404): + view(request, "mouse", status.id) def test_status_page_with_image(self, *_): """there are so many views, this just makes sure it LOADS""" From 56bf8d923f729288e29fab7890c5e370394ba0ac Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 17:52:10 -0700 Subject: [PATCH 041/111] Updates goal and list tests --- bookwyrm/tests/views/test_goal.py | 5 +++-- bookwyrm/tests/views/test_list_actions.py | 3 ++- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/bookwyrm/tests/views/test_goal.py b/bookwyrm/tests/views/test_goal.py index 2ecce0ac6..cb8c5d8e8 100644 --- a/bookwyrm/tests/views/test_goal.py +++ b/bookwyrm/tests/views/test_goal.py @@ -3,6 +3,7 @@ from unittest.mock import patch from django.utils import timezone from django.contrib.auth.models import AnonymousUser +from django.http import Http404 from django.template.response import TemplateResponse from django.test import TestCase from django.test.client import RequestFactory @@ -103,8 +104,8 @@ class GoalViews(TestCase): request = self.factory.get("") request.user = self.rat - result = view(request, self.local_user.localname, self.year) - self.assertEqual(result.status_code, 404) + with self.assertRaises(Http404): + view(request, self.local_user.localname, self.year) @patch("bookwyrm.activitystreams.add_status_task.delay") def test_create_goal(self, _): diff --git a/bookwyrm/tests/views/test_list_actions.py b/bookwyrm/tests/views/test_list_actions.py index 585156816..f7775d19f 100644 --- a/bookwyrm/tests/views/test_list_actions.py +++ b/bookwyrm/tests/views/test_list_actions.py @@ -568,5 +568,6 @@ class ListActionViews(TestCase): ) request.user = self.rat - views.list.remove_book(request, self.list.id) + with self.assertRaises(PermissionDenied): + views.list.remove_book(request, self.list.id) self.assertTrue(self.list.listitem_set.exists()) From 99c1a670f416400cf599c3833d26ee75f13e2538 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 17:52:20 -0700 Subject: [PATCH 042/111] Fixes checking goal perms --- bookwyrm/views/goal.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/bookwyrm/views/goal.py b/bookwyrm/views/goal.py index 1a0a99bf5..105c8b90b 100644 --- a/bookwyrm/views/goal.py +++ b/bookwyrm/views/goal.py @@ -31,7 +31,8 @@ class Goal(View): if not goal and year != timezone.now().year: return redirect("user-goal", username, current_year) - goal.raise_visible_to_user(request.user) + if goal: + goal.raise_visible_to_user(request.user) data = { "goal_form": forms.GoalForm(instance=goal), From f13f3d460c50458468bcf6591dea0803855ab35a Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 17:52:27 -0700 Subject: [PATCH 043/111] Fixing checking list perms --- bookwyrm/views/list.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bookwyrm/views/list.py b/bookwyrm/views/list.py index 332a5d1d6..904e5f1cd 100644 --- a/bookwyrm/views/list.py +++ b/bookwyrm/views/list.py @@ -329,11 +329,11 @@ def add_book(request): @login_required def remove_book(request, list_id): """remove a book from a list""" - with transaction.atomic(): - book_list = get_object_or_404(models.List, id=list_id) - item = get_object_or_404(models.ListItem, id=request.POST.get("item")) - item.raise_not_deletable(request.user) + book_list = get_object_or_404(models.List, id=list_id) + item = get_object_or_404(models.ListItem, id=request.POST.get("item")) + item.raise_not_deletable(request.user) + with transaction.atomic(): deleted_order = item.order item.delete() normalize_book_list_ordering(book_list.id, start=deleted_order) @@ -348,7 +348,7 @@ def set_book_position(request, list_item_id): special care with the unique ordering per list. """ list_item = get_object_or_404(models.ListItem, id=list_item_id) - list_item.list.raise_not_editable(request.user) + list_item.book_list.raise_not_editable(request.user) try: int_position = int(request.POST.get("position")) except ValueError: From e0aa8a7fdf5fc046bc817410c1e65b6a4a880ea5 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 17:58:21 -0700 Subject: [PATCH 044/111] Fixes shelf view boolean logic --- bookwyrm/tests/views/test_shelf.py | 2 +- bookwyrm/views/shelf.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/views/test_shelf.py b/bookwyrm/tests/views/test_shelf.py index d873edc6a..0d7d01cbc 100644 --- a/bookwyrm/tests/views/test_shelf.py +++ b/bookwyrm/tests/views/test_shelf.py @@ -105,7 +105,7 @@ class ShelfViews(TestCase): shelf.refresh_from_db() self.assertEqual(shelf.name, "cool name") - self.assertEqual(shelf.identifier, "testshelf-%d" % shelf.id) + self.assertEqual(shelf.identifier, f"testshelf-{shelf.id}") def test_edit_shelf_name_not_editable(self, *_): """can't change the name of an non-editable shelf""" diff --git a/bookwyrm/views/shelf.py b/bookwyrm/views/shelf.py index 5d1008d09..6e75cc999 100644 --- a/bookwyrm/views/shelf.py +++ b/bookwyrm/views/shelf.py @@ -146,7 +146,7 @@ def shelve(request): # first we need to remove from the specified shelf change_from_current_identifier = request.POST.get("change-shelf-from") - if not change_from_current_identifier: + if change_from_current_identifier: # find the shelfbook obj and delete it get_object_or_404( models.ShelfBook, From 767d37817032dbd33161d18e0fdf44f73af09ef2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 18:52:45 -0700 Subject: [PATCH 045/111] Update status tests --- bookwyrm/tests/views/test_status.py | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index 592d1f5c7..6a09807c7 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -1,6 +1,7 @@ """ test for app action functionality """ import json from unittest.mock import patch +from django.core.exceptions import PermissionDenied from django.test import TestCase from django.test.client import RequestFactory @@ -196,9 +197,9 @@ class StatusViews(TestCase): ) with patch("bookwyrm.activitystreams.remove_status_task.delay") as mock: - result = view(request, status.id) + with self.assertRaises(PermissionDenied): + view(request, status.id) self.assertFalse(mock.called) - self.assertEqual(result.status_code, 400) status.refresh_from_db() self.assertFalse(status.deleted) @@ -214,9 +215,9 @@ class StatusViews(TestCase): ) with patch("bookwyrm.activitystreams.remove_status_task.delay") as mock: - result = view(request, status.id) + with self.assertRaises(PermissionDenied): + view(request, status.id) self.assertFalse(mock.called) - self.assertEqual(result.status_code, 400) status.refresh_from_db() self.assertFalse(status.deleted) @@ -375,7 +376,8 @@ http://www.fish.com/""" request = self.factory.post("") request.user = self.remote_user - view(request, status.id) + with self.assertRaises(PermissionDenied): + view(request, status.id) status.refresh_from_db() self.assertFalse(status.deleted) From 99ef81be9b471602b58c6bba9d0813b444c3e1aa Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 19:05:13 -0700 Subject: [PATCH 046/111] Linter fixes --- bookwyrm/templates/opensearch.xml | 18 +++++++++++++++--- bookwyrm/views/wellknown.py | 1 + 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/bookwyrm/templates/opensearch.xml b/bookwyrm/templates/opensearch.xml index 50167144a..9582cdb79 100644 --- a/bookwyrm/templates/opensearch.xml +++ b/bookwyrm/templates/opensearch.xml @@ -1,11 +1,23 @@ {% load i18n %} {% load static %} - + BW - {% blocktrans with site_name=site.name %}{{ site_name }} search{% endblocktrans %} + {% blocktrans trimmed with site_name=site.name %} + {{ site_name }} search + {% endblocktrans %} UTF-8 - {% if site.favicon %}{% get_media_prefix %}{{ site.favicon }}{% else %}{% static "images/favicon.ico" %}{% endif %} + {% if site.favicon %} + {% get_media_prefix %}{{ site.favicon }}{% else %}{% static "images/favicon.ico" %} + {% endif %} diff --git a/bookwyrm/views/wellknown.py b/bookwyrm/views/wellknown.py index 6ece669ef..a30cfb9f0 100644 --- a/bookwyrm/views/wellknown.py +++ b/bookwyrm/views/wellknown.py @@ -131,6 +131,7 @@ def host_meta(request): """meta of the host""" return TemplateResponse(request, "host_meta.xml", {"DOMAIN": DOMAIN}) + @require_GET def opensearch(request): """Open Search xml spec""" From c32f975a671b44efac8e04e4aaa0be798199227d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 19:28:50 -0700 Subject: [PATCH 047/111] Working in firefox --- bookwyrm/templates/opensearch.xml | 25 ++++++++----------------- bookwyrm/views/wellknown.py | 5 ++++- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/bookwyrm/templates/opensearch.xml b/bookwyrm/templates/opensearch.xml index 9582cdb79..651958ab4 100644 --- a/bookwyrm/templates/opensearch.xml +++ b/bookwyrm/templates/opensearch.xml @@ -1,25 +1,16 @@ -{% load i18n %} -{% load static %} - +{% load i18n %}{% load static %} BW {% blocktrans trimmed with site_name=site.name %} {{ site_name }} search {% endblocktrans %} - UTF-8 - {% if site.favicon %} - {% get_media_prefix %}{{ site.favicon }}{% else %}{% static "images/favicon.ico" %} - {% endif %} - - - - {% url 'search' %} + {{ image }} + diff --git a/bookwyrm/views/wellknown.py b/bookwyrm/views/wellknown.py index a30cfb9f0..8b55eaa39 100644 --- a/bookwyrm/views/wellknown.py +++ b/bookwyrm/views/wellknown.py @@ -135,4 +135,7 @@ def host_meta(request): @require_GET def opensearch(request): """Open Search xml spec""" - return TemplateResponse(request, "opensearch.xml") + site = models.SiteSettings.get() + logo_path = site.favicon or "images/favicon.png" + logo = f"{MEDIA_FULL_URL}{logo_path}" + return TemplateResponse(request, "opensearch.xml", {"image": logo, "DOMAIN": DOMAIN}) From 95cdaae4d4d31416d5ccbbfbaaedae990fd96e85 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 27 Sep 2021 19:38:54 -0700 Subject: [PATCH 048/111] Python formatting --- bookwyrm/views/wellknown.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/bookwyrm/views/wellknown.py b/bookwyrm/views/wellknown.py index 8b55eaa39..0f562b8a4 100644 --- a/bookwyrm/views/wellknown.py +++ b/bookwyrm/views/wellknown.py @@ -138,4 +138,6 @@ def opensearch(request): site = models.SiteSettings.get() logo_path = site.favicon or "images/favicon.png" logo = f"{MEDIA_FULL_URL}{logo_path}" - return TemplateResponse(request, "opensearch.xml", {"image": logo, "DOMAIN": DOMAIN}) + return TemplateResponse( + request, "opensearch.xml", {"image": logo, "DOMAIN": DOMAIN} + ) From 9db75cc5b7c70516aa8aea364173e56db1a0a704 Mon Sep 17 00:00:00 2001 From: Levi Bard Date: Mon, 27 Sep 2021 13:04:30 +0200 Subject: [PATCH 049/111] Add test for creating a book with a cover url --- bookwyrm/tests/views/test_book.py | 55 +++++++++++++++++++++++-------- 1 file changed, 41 insertions(+), 14 deletions(-) diff --git a/bookwyrm/tests/views/test_book.py b/bookwyrm/tests/views/test_book.py index 99022ec5e..d3bec6f01 100644 --- a/bookwyrm/tests/views/test_book.py +++ b/bookwyrm/tests/views/test_book.py @@ -282,6 +282,46 @@ class BookViews(TestCase): self.assertEqual(book.authors.first().name, "Sappho") self.assertEqual(book.authors.first(), book.parent_work.authors.first()) + def _setup_cover_url(self): + cover_url = "http://example.com" + image_file = pathlib.Path(__file__).parent.joinpath( + "../../static/images/default_avi.jpg" + ) + image = Image.open(image_file) + output = BytesIO() + image.save(output, format=image.format) + responses.add( + responses.GET, + cover_url, + body=output.getvalue(), + status=200, + ) + return cover_url + + @responses.activate + def test_create_book_upload_cover_url(self): + """create an entirely new book and work with cover url""" + self.assertFalse(self.book.cover) + view = views.ConfirmEditBook.as_view() + self.local_user.groups.add(self.group) + cover_url = self._setup_cover_url() + + form = forms.EditionForm() + form.data["title"] = "New Title" + form.data["last_edited_by"] = self.local_user.id + form.data["cover-url"] = cover_url + request = self.factory.post("", form.data) + request.user = self.local_user + + with patch( + "bookwyrm.models.activitypub_mixin.broadcast_task.delay" + ) as delay_mock: + views.upload_cover(request, self.book.id) + self.assertEqual(delay_mock.call_count, 1) + + self.book.refresh_from_db() + self.assertTrue(self.book.cover) + def test_upload_cover_file(self): """add a cover via file upload""" self.assertFalse(self.book.cover) @@ -310,21 +350,8 @@ class BookViews(TestCase): def test_upload_cover_url(self): """add a cover via url""" self.assertFalse(self.book.cover) - image_file = pathlib.Path(__file__).parent.joinpath( - "../../static/images/default_avi.jpg" - ) - image = Image.open(image_file) - output = BytesIO() - image.save(output, format=image.format) - responses.add( - responses.GET, - "http://example.com", - body=output.getvalue(), - status=200, - ) - form = forms.CoverForm(instance=self.book) - form.data["cover-url"] = "http://example.com" + form.data["cover-url"] = self._setup_cover_url() request = self.factory.post("", form.data) request.user = self.local_user From 72cbc1cb1b818b568146593a6baf12588768a652 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 28 Sep 2021 09:23:50 -0700 Subject: [PATCH 050/111] Adds label --- .../snippets/reading_modals/progress_update_modal.html | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bookwyrm/templates/snippets/reading_modals/progress_update_modal.html b/bookwyrm/templates/snippets/reading_modals/progress_update_modal.html index 6fefae010..fb52432c4 100644 --- a/bookwyrm/templates/snippets/reading_modals/progress_update_modal.html +++ b/bookwyrm/templates/snippets/reading_modals/progress_update_modal.html @@ -7,8 +7,10 @@ {% block modal-form-open %}
+{% csrf_token %} {% endblock %} {% block reading-dates %} + {% include "snippets/progress_field.html" with progress_required=True %} {% endblock %} From 9a06b7d4936b20f05c4dd0d9fd89daad741aba38 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 28 Sep 2021 09:24:09 -0700 Subject: [PATCH 051/111] Fixes incorrect label in edit user panel --- bookwyrm/templates/preferences/edit_user.html | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/templates/preferences/edit_user.html b/bookwyrm/templates/preferences/edit_user.html index 4dc792f9b..a0c7a79ab 100644 --- a/bookwyrm/templates/preferences/edit_user.html +++ b/bookwyrm/templates/preferences/edit_user.html @@ -46,7 +46,7 @@ {% trans "Show reading goal prompt in feed:" %} {{ form.show_goal }} -