From fd19b559612aacc36f96fda15db88f6ef35d57cd Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 15 Feb 2021 16:26:48 -0800 Subject: [PATCH 01/42] Basic checks for inbox --- bookwyrm/incoming.py | 21 +++--- bookwyrm/tests/views/test_inbox.py | 102 +++++++++++++++++++++++++++++ bookwyrm/urls.py | 6 +- bookwyrm/views/__init__.py | 1 + bookwyrm/views/inbox.py | 79 ++++++++++++++++++++++ 5 files changed, 195 insertions(+), 14 deletions(-) create mode 100644 bookwyrm/tests/views/test_inbox.py create mode 100644 bookwyrm/views/inbox.py diff --git a/bookwyrm/incoming.py b/bookwyrm/incoming.py index 18db1069..83d325af 100644 --- a/bookwyrm/incoming.py +++ b/bookwyrm/incoming.py @@ -160,14 +160,12 @@ def handle_follow_accept(activity): accepter = activitypub.resolve_remote_id(models.User, activity['actor']) try: - request = models.UserFollowRequest.objects.get( + models.UserFollowRequest.objects.get( user_subject=requester, user_object=accepter - ) - request.delete() + ).accept() except models.UserFollowRequest.DoesNotExist: - pass - accepter.followers.add(requester) + return @app.task @@ -176,12 +174,13 @@ def handle_follow_reject(activity): requester = models.User.objects.get(remote_id=activity['object']['actor']) rejecter = activitypub.resolve_remote_id(models.User, activity['actor']) - request = models.UserFollowRequest.objects.get( - user_subject=requester, - user_object=rejecter - ) - request.delete() - #raises models.UserFollowRequest.DoesNotExist + try: + models.UserFollowRequest.objects.get( + user_subject=requester, + user_object=rejecter + ).reject() + except models.UserFollowRequest.DoesNotExist: + return @app.task def handle_block(activity): diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py new file mode 100644 index 00000000..64dcc149 --- /dev/null +++ b/bookwyrm/tests/views/test_inbox.py @@ -0,0 +1,102 @@ +''' tests incoming activities''' +import json +from unittest.mock import patch + +from django.http import HttpResponseNotAllowed, HttpResponseNotFound +from django.test import TestCase, Client + +from bookwyrm import models + +class Inbox(TestCase): + ''' readthrough tests ''' + def setUp(self): + ''' basic user and book data ''' + self.client = Client() + self.local_user = models.User.objects.create_user( + 'mouse@example.com', 'mouse@mouse.com', 'mouseword', + local=True, localname='mouse') + self.local_user.remote_id = 'https://example.com/user/mouse' + self.local_user.save(broadcast=False) + with patch('bookwyrm.models.user.set_remote_server.delay'): + self.remote_user = models.User.objects.create_user( + 'rat', 'rat@rat.com', 'ratword', + local=False, + remote_id='https://example.com/users/rat', + inbox='https://example.com/users/rat/inbox', + outbox='https://example.com/users/rat/outbox', + ) + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + self.status = models.Status.objects.create( + user=self.local_user, + content='Test status', + remote_id='https://example.com/status/1', + ) + models.SiteSettings.objects.create() + + + def test_inbox_invalid_get(self): + ''' shouldn't try to handle if the user is not found ''' + result = self.client.get( + '/inbox', content_type="application/json" + ) + self.assertIsInstance(result, HttpResponseNotAllowed) + + def test_inbox_invalid_user(self): + ''' shouldn't try to handle if the user is not found ''' + result = self.client.post( + '/user/bleh/inbox', + '{"type": "Test", "object": "exists"}', + content_type="application/json" + ) + self.assertIsInstance(result, HttpResponseNotFound) + + def test_inbox_invalid_bad_signature(self): + ''' bad request for invalid signature ''' + with patch('bookwyrm.views.inbox.has_valid_signature') as mock_valid: + mock_valid.return_value = False + result = self.client.post( + '/user/mouse/inbox', + '{"type": "Test", "object": "exists"}', + content_type="application/json" + ) + self.assertEqual(result.status_code, 401) + + def test_inbox_invalid_bad_signature_delete(self): + ''' invalid signature for Delete is okay though ''' + with patch('bookwyrm.views.inbox.has_valid_signature') as mock_valid: + mock_valid.return_value = False + result = self.client.post( + '/user/mouse/inbox', + '{"type": "Delete", "object": "exists"}', + content_type="application/json" + ) + self.assertEqual(result.status_code, 200) + + def test_inbox_unknown_type(self): + ''' never heard of that activity type, don't have a handler for it ''' + with patch('bookwyrm.views.inbox.has_valid_signature') as mock_valid: + result = self.client.post( + '/inbox', + '{"type": "Fish", "object": "exists"}', + content_type="application/json" + ) + mock_valid.return_value = True + self.assertIsInstance(result, HttpResponseNotFound) + + + def test_inbox_success(self): + ''' a known type, for which we start a task ''' + activity = { + "id": "hi", + "type": "Accept", + "actor": "https://example.com/users/rat", + "object": "https://example.com/user/mouse" + } + 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, 200) diff --git a/bookwyrm/urls.py b/bookwyrm/urls.py index 3830adb9..a741088a 100644 --- a/bookwyrm/urls.py +++ b/bookwyrm/urls.py @@ -4,7 +4,7 @@ from django.contrib import admin from django.urls import path, re_path -from bookwyrm import incoming, settings, views, wellknown +from bookwyrm import settings, views, wellknown from bookwyrm.utils import regex user_path = r'^user/(?P%s)' % regex.username @@ -29,8 +29,8 @@ urlpatterns = [ path('admin/', admin.site.urls), # federation endpoints - re_path(r'^inbox/?$', incoming.shared_inbox), - re_path(r'%s/inbox/?$' % local_user_path, incoming.inbox), + re_path(r'^inbox/?$', views.Inbox.as_view()), + re_path(r'%s/inbox/?$' % local_user_path, views.Inbox.as_view()), re_path(r'%s/outbox/?$' % local_user_path, views.Outbox.as_view()), re_path(r'^.well-known/webfinger/?$', wellknown.webfinger), re_path(r'^.well-known/nodeinfo/?$', wellknown.nodeinfo_pointer), diff --git a/bookwyrm/views/__init__.py b/bookwyrm/views/__init__.py index b72c5013..2c7cdc46 100644 --- a/bookwyrm/views/__init__.py +++ b/bookwyrm/views/__init__.py @@ -11,6 +11,7 @@ from .follow import follow, unfollow from .follow import accept_follow_request, delete_follow_request from .goal import Goal from .import_data import Import, ImportStatus +from .inbox import Inbox from .interaction import Favorite, Unfavorite, Boost, Unboost from .invite import ManageInvites, Invite from .landing import About, Home, Discover diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py new file mode 100644 index 00000000..8f38e7a2 --- /dev/null +++ b/bookwyrm/views/inbox.py @@ -0,0 +1,79 @@ +''' incoming activities ''' +import json +from urllib.parse import urldefrag + +from django.http import HttpResponse +from django.http import HttpResponseBadRequest, HttpResponseNotFound +from django.shortcuts import get_object_or_404 +from django.views import View +import requests + +from bookwyrm import activitypub, models +from bookwyrm.signatures import Signature + + +# pylint: disable=no-self-use +class Inbox(View): + ''' requests sent by outside servers''' + def post(self, request, username=None): + ''' only works as POST request ''' + # first let's do some basic checks to see if this is legible + if username: + try: + models.User.objects.get(localname=username) + except models.User.DoesNotExist: + return HttpResponseNotFound() + + # is it valid json? does it at least vaguely resemble an activity? + try: + resp = request.body + activity_json = json.loads(resp) + activity_type = activity_json['type'] # Follow, Accept, Create, etc + except (json.decoder.JSONDecodeError, KeyError): + return HttpResponseBadRequest() + + # verify the signature + if not has_valid_signature(request, activity_json): + if activity_json['type'] == 'Delete': + # Pretend that unauth'd deletes succeed. Auth may be failing + # because the resource or owner of the resource might have + # been deleted. + return HttpResponse() + return HttpResponse(status=401) + + # get the activity dataclass from the type field + try: + serializer = getattr(activitypub, activity_type) + serializer(**activity_json) + except (AttributeError, activitypub.ActivitySerializerError): + return HttpResponseNotFound() + + return HttpResponse() + + +def has_valid_signature(request, activity): + ''' verify incoming signature ''' + try: + signature = Signature.parse(request) + + key_actor = urldefrag(signature.key_id).url + if key_actor != activity.get('actor'): + raise ValueError("Wrong actor created signature.") + + remote_user = activitypub.resolve_remote_id(models.User, key_actor) + if not remote_user: + return False + + try: + signature.verify(remote_user.key_pair.public_key, request) + except ValueError: + old_key = remote_user.key_pair.public_key + remote_user = activitypub.resolve_remote_id( + models.User, remote_user.remote_id, refresh=True + ) + if remote_user.key_pair.public_key == old_key: + raise # Key unchanged. + signature.verify(remote_user.key_pair.public_key, request) + except (ValueError, requests.exceptions.HTTPError): + return False + return True From e810c2bee0e8887bfe1c073dd1d53f3b405cc9fe Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 15 Feb 2021 17:23:17 -0800 Subject: [PATCH 02/42] Recursively parse activities --- bookwyrm/activitypub/__init__.py | 6 ++- bookwyrm/activitypub/base_activity.py | 17 ++++++++- bookwyrm/tests/test_incoming.py | 54 --------------------------- bookwyrm/views/inbox.py | 10 ++--- 4 files changed, 24 insertions(+), 63 deletions(-) diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index 510f1f3f..ce4acd66 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -2,7 +2,7 @@ import inspect import sys -from .base_activity import ActivityEncoder, Signature +from .base_activity import ActivityEncoder, Signature, naive_parse from .base_activity import Link, Mention from .base_activity import ActivitySerializerError, resolve_remote_id from .image import Image @@ -23,3 +23,7 @@ from .verbs import Add, AddBook, AddListItem, Remove cls_members = inspect.getmembers(sys.modules[__name__], inspect.isclass) activity_objects = {c[0]: c[1] for c in cls_members \ if hasattr(c[1], 'to_model')} + +def parse(activity_json): + ''' figure out what activity this is and parse it ''' + return naive_parse(activity_objects, activity_json) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 5f35f1d7..596662e8 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -40,6 +40,17 @@ class Signature: signatureValue: str type: str = 'RsaSignature2017' +def naive_parse(activity_objects, activity_json): + ''' this navigates circular import issues ''' + try: + activity_type = activity_json['type'] + print(activity_type) + serializer = activity_objects[activity_type] + print(serializer) + except KeyError: + raise ActivitySerializerError('Invalid type "%s"' % activity_type) + + return serializer(activity_objects=activity_objects, **activity_json) @dataclass(init=False) class ActivityObject: @@ -47,13 +58,17 @@ class ActivityObject: id: str type: str - def __init__(self, **kwargs): + def __init__(self, activity_objects=None, **kwargs): ''' this lets you pass in an object with fields that aren't in the dataclass, which it ignores. Any field in the dataclass is required or has a default value ''' for field in fields(self): try: + print(field.name, field.type) value = kwargs[field.name] + if field.type == 'ActivityObject' and activity_objects: + value = naive_parse(activity_objects, value) + except KeyError: if field.default == MISSING and \ field.default_factory == MISSING: diff --git a/bookwyrm/tests/test_incoming.py b/bookwyrm/tests/test_incoming.py index 01d0c9a3..9be0bb97 100644 --- a/bookwyrm/tests/test_incoming.py +++ b/bookwyrm/tests/test_incoming.py @@ -40,60 +40,6 @@ class Incoming(TestCase): self.factory = RequestFactory() - def test_inbox_invalid_get(self): - ''' shouldn't try to handle if the user is not found ''' - request = self.factory.get('https://www.example.com/') - self.assertIsInstance( - incoming.inbox(request, 'anything'), HttpResponseNotAllowed) - self.assertIsInstance( - incoming.shared_inbox(request), HttpResponseNotAllowed) - - def test_inbox_invalid_user(self): - ''' shouldn't try to handle if the user is not found ''' - request = self.factory.post('https://www.example.com/') - self.assertIsInstance( - incoming.inbox(request, 'fish@tomato.com'), HttpResponseNotFound) - - def test_inbox_invalid_no_object(self): - ''' json is missing "object" field ''' - request = self.factory.post( - self.local_user.shared_inbox, data={}) - self.assertIsInstance( - incoming.shared_inbox(request), HttpResponseBadRequest) - - def test_inbox_invalid_bad_signature(self): - ''' bad request for invalid signature ''' - request = self.factory.post( - self.local_user.shared_inbox, - '{"type": "Test", "object": "exists"}', - content_type='application/json') - with patch('bookwyrm.incoming.has_valid_signature') as mock_has_valid: - mock_has_valid.return_value = False - self.assertEqual( - incoming.shared_inbox(request).status_code, 401) - - def test_inbox_invalid_bad_signature_delete(self): - ''' invalid signature for Delete is okay though ''' - request = self.factory.post( - self.local_user.shared_inbox, - '{"type": "Delete", "object": "exists"}', - content_type='application/json') - with patch('bookwyrm.incoming.has_valid_signature') as mock_has_valid: - mock_has_valid.return_value = False - self.assertEqual( - incoming.shared_inbox(request).status_code, 200) - - def test_inbox_unknown_type(self): - ''' never heard of that activity type, don't have a handler for it ''' - request = self.factory.post( - self.local_user.shared_inbox, - '{"type": "Fish", "object": "exists"}', - content_type='application/json') - with patch('bookwyrm.incoming.has_valid_signature') as mock_has_valid: - mock_has_valid.return_value = True - self.assertIsInstance( - incoming.shared_inbox(request), HttpResponseNotFound) - def test_inbox_success(self): ''' a known type, for which we start a task ''' request = self.factory.post( diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 8f38e7a2..53dd6c40 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -4,7 +4,6 @@ from urllib.parse import urldefrag from django.http import HttpResponse from django.http import HttpResponseBadRequest, HttpResponseNotFound -from django.shortcuts import get_object_or_404 from django.views import View import requests @@ -26,10 +25,8 @@ class Inbox(View): # is it valid json? does it at least vaguely resemble an activity? try: - resp = request.body - activity_json = json.loads(resp) - activity_type = activity_json['type'] # Follow, Accept, Create, etc - except (json.decoder.JSONDecodeError, KeyError): + activity_json = json.loads(request.body) + except json.decoder.JSONDecodeError: return HttpResponseBadRequest() # verify the signature @@ -43,8 +40,7 @@ class Inbox(View): # get the activity dataclass from the type field try: - serializer = getattr(activitypub, activity_type) - serializer(**activity_json) + activitypub.parse(activity_json) except (AttributeError, activitypub.ActivitySerializerError): return HttpResponseNotFound() From 81e2021f92c170b8c73098a4746e511983975ce9 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 15 Feb 2021 18:47:08 -0800 Subject: [PATCH 03/42] Move handlers to activitypub classes --- bookwyrm/activitypub/base_activity.py | 33 ++++++++++---------- bookwyrm/activitypub/verbs.py | 7 +++++ bookwyrm/tests/views/test_inbox.py | 43 +++++++++++++++++++++------ bookwyrm/views/inbox.py | 24 ++++++++++++--- celerywyrm/celery.py | 2 +- 5 files changed, 78 insertions(+), 31 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 596662e8..93af0d0c 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -47,8 +47,8 @@ def naive_parse(activity_objects, activity_json): print(activity_type) serializer = activity_objects[activity_type] print(serializer) - except KeyError: - raise ActivitySerializerError('Invalid type "%s"' % activity_type) + except KeyError as e: + raise ActivitySerializerError(e) return serializer(activity_objects=activity_objects, **activity_json) @@ -66,7 +66,11 @@ class ActivityObject: try: print(field.name, field.type) value = kwargs[field.name] - if field.type == 'ActivityObject' and activity_objects: + try: + is_subclass = issubclass(field.type, ActivityObject) + except TypeError: + is_subclass = False + if is_subclass and activity_objects: value = naive_parse(activity_objects, value) except KeyError: @@ -78,22 +82,17 @@ class ActivityObject: setattr(self, field.name, value) - def to_model(self, model, instance=None, save=True): + def to_model(self, instance=None, save=True): ''' convert from an activity to a model instance ''' - if self.type != model.activity_serializer.type: + # figure out the right model -- wish I had a better way for this + models = apps.get_models() + model = [m for m in models if hasattr(m, 'activity_serializer') and \ + hasattr(m.activity_serializer, 'type') and \ + m.activity_serializer.type == self.type] + if not len(model): raise ActivitySerializerError( - 'Wrong activity type "%s" for activity of type "%s"' % \ - (model.activity_serializer.type, - self.type) - ) - - if not isinstance(self, model.activity_serializer): - raise ActivitySerializerError( - 'Wrong activity type "%s" for model "%s" (expects "%s")' % \ - (self.__class__, - model.__name__, - model.activity_serializer) - ) + 'No model found for activity type "%s"' % self.type) + model = model[0] if hasattr(model, 'ignore_activity') and model.ignore_activity(self): return instance diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index 190cd739..c4b30253 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -21,6 +21,11 @@ class Create(Verb): signature: Signature = None type: str = 'Create' + def action(self): + ''' create the model instance from the dataclass ''' + # check for dupes + self.object.to_model() + @dataclass(init=False) class Delete(Verb): @@ -46,11 +51,13 @@ class Undo(Verb): @dataclass(init=False) class Follow(Verb): ''' Follow activity ''' + object: str type: str = 'Follow' @dataclass(init=False) class Block(Verb): ''' Block activity ''' + object: str type: str = 'Block' @dataclass(init=False) diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 64dcc149..3bb7760e 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -87,16 +87,41 @@ class Inbox(TestCase): def test_inbox_success(self): ''' a known type, for which we start a task ''' activity = { - "id": "hi", - "type": "Accept", - "actor": "https://example.com/users/rat", - "object": "https://example.com/user/mouse" + 'id': 'hi', + 'type': 'Create', + 'actor': 'hi', + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://example.com/user/mouse/followers" + ], + 'object': { + "id": "https://example.com/list/22", + "type": "BookList", + "totalItems": 1, + "first": "https://example.com/list/22?page=1", + "last": "https://example.com/list/22?page=1", + "name": "Test List", + "owner": "https://example.com/user/mouse", + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://example.com/user/mouse/followers" + ], + "summary": "summary text", + "curation": "curated", + "@context": "https://www.w3.org/ns/activitystreams" + } } 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" - ) + + with patch('bookwyrm.views.inbox.activity_task.delay'): + result = self.client.post( + '/inbox', + json.dumps(activity), + content_type="application/json" + ) self.assertEqual(result.status_code, 200) diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 53dd6c40..fdac8446 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -8,6 +8,7 @@ from django.views import View import requests from bookwyrm import activitypub, models +from bookwyrm.tasks import app from bookwyrm.signatures import Signature @@ -38,15 +39,30 @@ class Inbox(View): return HttpResponse() return HttpResponse(status=401) - # get the activity dataclass from the type field - try: - activitypub.parse(activity_json) - except (AttributeError, activitypub.ActivitySerializerError): + # just some quick smell tests before we try to parse the json + if not 'object' in activity_json or \ + not 'type' in activity_json or \ + not activity_json['type'] in activitypub.activity_objects: return HttpResponseNotFound() + activity_task.delay() return HttpResponse() +@app.task +def activity_task(activity_json): + ''' do something with this json we think is legit ''' + # lets see if the activitypub module can make sense of this json + try: + activity = activitypub.parse(activity_json) + except activitypub.ActivitySerializerError: + return + + # cool that worked, now we should do the action described by the type + # (create, update, delete, etc) + activity.action() + + def has_valid_signature(request, activity): ''' verify incoming signature ''' try: diff --git a/celerywyrm/celery.py b/celerywyrm/celery.py index 5a53dab5..2937ef0f 100644 --- a/celerywyrm/celery.py +++ b/celerywyrm/celery.py @@ -25,5 +25,5 @@ app.autodiscover_tasks( ['bookwyrm'], related_name='connectors.abstract_connector') app.autodiscover_tasks(['bookwyrm'], related_name='emailing') app.autodiscover_tasks(['bookwyrm'], related_name='goodreads_import') -app.autodiscover_tasks(['bookwyrm'], related_name='incoming') app.autodiscover_tasks(['bookwyrm'], related_name='models.user') +app.autodiscover_tasks(['bookwyrm'], related_name='views.inbox') From 12a3aa96676c3ddbfe1905a14ef8d45dcdaded3c Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 15 Feb 2021 19:41:22 -0800 Subject: [PATCH 04/42] incoming Create flow with tests --- bookwyrm/activitypub/base_activity.py | 3 - bookwyrm/activitypub/verbs.py | 1 - bookwyrm/incoming.py | 41 ------- bookwyrm/tests/test_incoming.py | 117 -------------------- bookwyrm/tests/views/test_inbox.py | 151 ++++++++++++++++++++++---- bookwyrm/views/inbox.py | 2 +- 6 files changed, 129 insertions(+), 186 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 93af0d0c..a49514dc 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -44,9 +44,7 @@ def naive_parse(activity_objects, activity_json): ''' this navigates circular import issues ''' try: activity_type = activity_json['type'] - print(activity_type) serializer = activity_objects[activity_type] - print(serializer) except KeyError as e: raise ActivitySerializerError(e) @@ -64,7 +62,6 @@ class ActivityObject: has a default value ''' for field in fields(self): try: - print(field.name, field.type) value = kwargs[field.name] try: is_subclass = issubclass(field.type, ActivityObject) diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index c4b30253..fee5f362 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -23,7 +23,6 @@ class Create(Verb): def action(self): ''' create the model instance from the dataclass ''' - # check for dupes self.object.to_model() diff --git a/bookwyrm/incoming.py b/bookwyrm/incoming.py index 83d325af..7ddd893f 100644 --- a/bookwyrm/incoming.py +++ b/bookwyrm/incoming.py @@ -53,14 +53,6 @@ def shared_inbox(request): 'Accept': handle_follow_accept, 'Reject': handle_follow_reject, 'Block': handle_block, - 'Create': { - 'BookList': handle_create_list, - 'Note': handle_create_status, - 'Article': handle_create_status, - 'Review': handle_create_status, - 'Comment': handle_create_status, - 'Quotation': handle_create_status, - }, 'Delete': handle_delete_status, 'Like': handle_favorite, 'Announce': handle_boost, @@ -204,13 +196,6 @@ def handle_unblock(activity): block.delete() -@app.task -def handle_create_list(activity): - ''' a new list ''' - activity = activity['object'] - activitypub.BookList(**activity).to_model(models.List) - - @app.task def handle_update_list(activity): ''' update a list ''' @@ -222,32 +207,6 @@ def handle_update_list(activity): **activity['object']).to_model(models.List, instance=book_list) -@app.task -def handle_create_status(activity): - ''' someone did something, good on them ''' - # deduplicate incoming activities - activity = activity['object'] - status_id = activity.get('id') - if models.Status.objects.filter(remote_id=status_id).count(): - return - - try: - serializer = activitypub.activity_objects[activity['type']] - except KeyError: - return - - activity = serializer(**activity) - try: - model = models.activity_models[activity.type] - except KeyError: - # not a type of status we are prepared to deserialize - return - - status = activity.to_model(model) - if not status: - # it was discarded because it's not a bookwyrm type - return - @app.task def handle_delete_status(activity): diff --git a/bookwyrm/tests/test_incoming.py b/bookwyrm/tests/test_incoming.py index 9be0bb97..288ae302 100644 --- a/bookwyrm/tests/test_incoming.py +++ b/bookwyrm/tests/test_incoming.py @@ -40,19 +40,6 @@ class Incoming(TestCase): self.factory = RequestFactory() - def test_inbox_success(self): - ''' a known type, for which we start a task ''' - request = self.factory.post( - self.local_user.shared_inbox, - '{"type": "Accept", "object": "exists"}', - content_type='application/json') - with patch('bookwyrm.incoming.has_valid_signature') as mock_has_valid: - mock_has_valid.return_value = True - - with patch('bookwyrm.incoming.handle_follow_accept.delay'): - self.assertEqual( - incoming.shared_inbox(request).status_code, 200) - def test_handle_follow(self): ''' remote user wants to follow local user ''' @@ -198,36 +185,6 @@ class Incoming(TestCase): self.assertEqual(follows.count(), 0) - def test_handle_create_list(self): - ''' a new list ''' - activity = { - 'object': { - "id": "https://example.com/list/22", - "type": "BookList", - "totalItems": 1, - "first": "https://example.com/list/22?page=1", - "last": "https://example.com/list/22?page=1", - "name": "Test List", - "owner": "https://example.com/user/mouse", - "to": [ - "https://www.w3.org/ns/activitystreams#Public" - ], - "cc": [ - "https://example.com/user/mouse/followers" - ], - "summary": "summary text", - "curation": "curated", - "@context": "https://www.w3.org/ns/activitystreams" - } - } - incoming.handle_create_list(activity) - book_list = models.List.objects.get() - self.assertEqual(book_list.name, 'Test List') - self.assertEqual(book_list.curation, 'curated') - self.assertEqual(book_list.description, 'summary text') - self.assertEqual(book_list.remote_id, 'https://example.com/list/22') - - def test_handle_update_list(self): ''' a new list ''' with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): @@ -262,80 +219,6 @@ class Incoming(TestCase): self.assertEqual(book_list.remote_id, 'https://example.com/list/22') - def test_handle_create_status(self): - ''' the "it justs works" mode ''' - self.assertEqual(models.Status.objects.count(), 1) - - datafile = pathlib.Path(__file__).parent.joinpath( - 'data/ap_quotation.json') - status_data = json.loads(datafile.read_bytes()) - models.Edition.objects.create( - title='Test Book', remote_id='https://example.com/book/1') - activity = {'object': status_data, 'type': 'Create'} - - incoming.handle_create_status(activity) - - status = models.Quotation.objects.get() - self.assertEqual( - status.remote_id, 'https://example.com/user/mouse/quotation/13') - self.assertEqual(status.quote, 'quote body') - self.assertEqual(status.content, 'commentary') - self.assertEqual(status.user, self.local_user) - self.assertEqual(models.Status.objects.count(), 2) - - # while we're here, lets ensure we avoid dupes - incoming.handle_create_status(activity) - self.assertEqual(models.Status.objects.count(), 2) - - def test_handle_create_status_unknown_type(self): - ''' folks send you all kinds of things ''' - activity = {'object': {'id': 'hi'}, 'type': 'Fish'} - result = incoming.handle_create_status(activity) - self.assertIsNone(result) - - def test_handle_create_status_remote_note_with_mention(self): - ''' should only create it under the right circumstances ''' - self.assertEqual(models.Status.objects.count(), 1) - self.assertFalse( - models.Notification.objects.filter(user=self.local_user).exists()) - - datafile = pathlib.Path(__file__).parent.joinpath( - 'data/ap_note.json') - status_data = json.loads(datafile.read_bytes()) - activity = {'object': status_data, 'type': 'Create'} - - incoming.handle_create_status(activity) - status = models.Status.objects.last() - self.assertEqual(status.content, 'test content in note') - self.assertEqual(status.mention_users.first(), self.local_user) - self.assertTrue( - models.Notification.objects.filter(user=self.local_user).exists()) - self.assertEqual( - models.Notification.objects.get().notification_type, 'MENTION') - - def test_handle_create_status_remote_note_with_reply(self): - ''' should only create it under the right circumstances ''' - self.assertEqual(models.Status.objects.count(), 1) - self.assertFalse( - models.Notification.objects.filter(user=self.local_user)) - - datafile = pathlib.Path(__file__).parent.joinpath( - 'data/ap_note.json') - status_data = json.loads(datafile.read_bytes()) - del status_data['tag'] - status_data['inReplyTo'] = self.status.remote_id - activity = {'object': status_data, 'type': 'Create'} - - incoming.handle_create_status(activity) - status = models.Status.objects.last() - self.assertEqual(status.content, 'test content in note') - self.assertEqual(status.reply_parent, self.status) - self.assertTrue( - models.Notification.objects.filter(user=self.local_user)) - self.assertEqual( - models.Notification.objects.get().notification_type, 'REPLY') - - def test_handle_delete_status(self): ''' remove a status ''' self.status.user = self.remote_user diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 3bb7760e..784f2127 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -1,11 +1,12 @@ ''' tests incoming activities''' import json +import pathlib from unittest.mock import patch from django.http import HttpResponseNotAllowed, HttpResponseNotFound from django.test import TestCase, Client -from bookwyrm import models +from bookwyrm import models, views class Inbox(TestCase): ''' readthrough tests ''' @@ -31,6 +32,19 @@ class Inbox(TestCase): content='Test status', remote_id='https://example.com/status/1', ) + + self.create_json = { + 'id': 'hi', + 'type': 'Create', + 'actor': 'hi', + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://example.com/user/mouse/followers" + ], + 'object': {} + } models.SiteSettings.objects.create() @@ -86,34 +100,24 @@ class Inbox(TestCase): def test_inbox_success(self): ''' a known type, for which we start a task ''' - activity = { - 'id': 'hi', - 'type': 'Create', - 'actor': 'hi', + activity = self.create_json + activity['object'] = { + "id": "https://example.com/list/22", + "type": "BookList", + "totalItems": 1, + "first": "https://example.com/list/22?page=1", + "last": "https://example.com/list/22?page=1", + "name": "Test List", + "owner": "https://example.com/user/mouse", "to": [ "https://www.w3.org/ns/activitystreams#Public" ], "cc": [ "https://example.com/user/mouse/followers" ], - 'object': { - "id": "https://example.com/list/22", - "type": "BookList", - "totalItems": 1, - "first": "https://example.com/list/22?page=1", - "last": "https://example.com/list/22?page=1", - "name": "Test List", - "owner": "https://example.com/user/mouse", - "to": [ - "https://www.w3.org/ns/activitystreams#Public" - ], - "cc": [ - "https://example.com/user/mouse/followers" - ], - "summary": "summary text", - "curation": "curated", - "@context": "https://www.w3.org/ns/activitystreams" - } + "summary": "summary text", + "curation": "curated", + "@context": "https://www.w3.org/ns/activitystreams" } with patch('bookwyrm.views.inbox.has_valid_signature') as mock_valid: mock_valid.return_value = True @@ -125,3 +129,104 @@ class Inbox(TestCase): content_type="application/json" ) self.assertEqual(result.status_code, 200) + + + def test_handle_create_status(self): + ''' the "it justs works" mode ''' + self.assertEqual(models.Status.objects.count(), 1) + + datafile = pathlib.Path(__file__).parent.joinpath( + '../data/ap_quotation.json') + status_data = json.loads(datafile.read_bytes()) + models.Edition.objects.create( + title='Test Book', remote_id='https://example.com/book/1') + activity = self.create_json + activity['object'] = status_data + + views.inbox.activity_task(activity) + + status = models.Quotation.objects.get() + self.assertEqual( + status.remote_id, 'https://example.com/user/mouse/quotation/13') + self.assertEqual(status.quote, 'quote body') + self.assertEqual(status.content, 'commentary') + self.assertEqual(status.user, self.local_user) + self.assertEqual(models.Status.objects.count(), 2) + + # while we're here, lets ensure we avoid dupes + views.inbox.activity_task(activity) + self.assertEqual(models.Status.objects.count(), 2) + + + def test_handle_create_status_remote_note_with_mention(self): + ''' should only create it under the right circumstances ''' + self.assertEqual(models.Status.objects.count(), 1) + self.assertFalse( + models.Notification.objects.filter(user=self.local_user).exists()) + + datafile = pathlib.Path(__file__).parent.joinpath( + '../data/ap_note.json') + status_data = json.loads(datafile.read_bytes()) + activity = self.create_json + activity['object'] = status_data + + views.inbox.activity_task(activity) + status = models.Status.objects.last() + self.assertEqual(status.content, 'test content in note') + self.assertEqual(status.mention_users.first(), self.local_user) + self.assertTrue( + models.Notification.objects.filter(user=self.local_user).exists()) + self.assertEqual( + models.Notification.objects.get().notification_type, 'MENTION') + + def test_handle_create_status_remote_note_with_reply(self): + ''' should only create it under the right circumstances ''' + self.assertEqual(models.Status.objects.count(), 1) + self.assertFalse( + models.Notification.objects.filter(user=self.local_user)) + + datafile = pathlib.Path(__file__).parent.joinpath( + '../data/ap_note.json') + status_data = json.loads(datafile.read_bytes()) + del status_data['tag'] + status_data['inReplyTo'] = self.status.remote_id + activity = self.create_json + activity['object'] = status_data + + views.inbox.activity_task(activity) + status = models.Status.objects.last() + self.assertEqual(status.content, 'test content in note') + self.assertEqual(status.reply_parent, self.status) + self.assertTrue( + models.Notification.objects.filter(user=self.local_user)) + self.assertEqual( + models.Notification.objects.get().notification_type, 'REPLY') + + + def test_handle_create_list(self): + ''' a new list ''' + activity = self.create_json + activity['object'] = { + "id": "https://example.com/list/22", + "type": "BookList", + "totalItems": 1, + "first": "https://example.com/list/22?page=1", + "last": "https://example.com/list/22?page=1", + "name": "Test List", + "owner": "https://example.com/user/mouse", + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://example.com/user/mouse/followers" + ], + "summary": "summary text", + "curation": "curated", + "@context": "https://www.w3.org/ns/activitystreams" + } + views.inbox.activity_task(activity) + book_list = models.List.objects.get() + self.assertEqual(book_list.name, 'Test List') + self.assertEqual(book_list.curation, 'curated') + self.assertEqual(book_list.description, 'summary text') + self.assertEqual(book_list.remote_id, 'https://example.com/list/22') diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index fdac8446..6e7b036b 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -45,7 +45,7 @@ class Inbox(View): not activity_json['type'] in activitypub.activity_objects: return HttpResponseNotFound() - activity_task.delay() + activity_task.delay(activity_json) return HttpResponse() From a16b81a6eb8232dc3f88f0d5ca6b3ae4ac95bf1d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 15 Feb 2021 20:49:23 -0800 Subject: [PATCH 05/42] Adds actions for all verbs --- bookwyrm/activitypub/base_activity.py | 8 +- bookwyrm/activitypub/verbs.py | 40 ++- bookwyrm/models/relationship.py | 12 +- bookwyrm/tests/test_incoming.py | 493 -------------------------- bookwyrm/tests/views/test_follow.py | 2 + bookwyrm/tests/views/test_inbox.py | 456 ++++++++++++++++++++++++ bookwyrm/views/follow.py | 2 - 7 files changed, 507 insertions(+), 506 deletions(-) delete mode 100644 bookwyrm/tests/test_incoming.py diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index a49514dc..631ca2dd 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -50,6 +50,7 @@ def naive_parse(activity_objects, activity_json): return serializer(activity_objects=activity_objects, **activity_json) + @dataclass(init=False) class ActivityObject: ''' actor activitypub json ''' @@ -79,7 +80,7 @@ class ActivityObject: setattr(self, field.name, value) - def to_model(self, instance=None, save=True): + def to_model(self, instance=None, allow_create=True, save=True): ''' convert from an activity to a model instance ''' # figure out the right model -- wish I had a better way for this models = apps.get_models() @@ -95,7 +96,10 @@ class ActivityObject: return instance # check for an existing instance, if we're not updating a known obj - instance = instance or model.find_existing(self.serialize()) or model() + instance = instance or model.find_existing(self.serialize()) + if not instance and not allow_create: + return None + instance = instance or model() for field in instance.simple_fields: field.set_field_from_activity(instance, self) diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index fee5f362..d781993e 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -12,6 +12,10 @@ class Verb(ActivityObject): actor: str object: ActivityObject + def action(self): + ''' usually we just want to save, this can be overridden as needed ''' + self.object.to_model() + @dataclass(init=False) class Create(Verb): @@ -21,10 +25,6 @@ class Create(Verb): signature: Signature = None type: str = 'Create' - def action(self): - ''' create the model instance from the dataclass ''' - self.object.to_model() - @dataclass(init=False) class Delete(Verb): @@ -33,6 +33,12 @@ class Delete(Verb): cc: List type: str = 'Delete' + def action(self): + ''' find and delete the activity object ''' + obj = self.object.to_model(save=False, allow_create=False) + obj.delete() + + @dataclass(init=False) class Update(Verb): @@ -40,12 +46,21 @@ class Update(Verb): to: List type: str = 'Update' + def action(self): + ''' update a model instance from the dataclass ''' + self.object.to_model(allow_create=False) + @dataclass(init=False) class Undo(Verb): ''' Undo an activity ''' type: str = 'Undo' + def action(self): + ''' find and remove the activity object ''' + obj = self.object.to_model(save=False, allow_create=False) + obj.delete() + @dataclass(init=False) class Follow(Verb): @@ -53,18 +68,25 @@ class Follow(Verb): object: str type: str = 'Follow' + @dataclass(init=False) class Block(Verb): ''' Block activity ''' object: str type: str = 'Block' + @dataclass(init=False) class Accept(Verb): ''' Accept activity ''' object: Follow type: str = 'Accept' + def action(self): + ''' find and remove the activity object ''' + obj = self.object.to_model(save=False, allow_create=False) + obj.accept() + @dataclass(init=False) class Reject(Verb): @@ -72,6 +94,11 @@ class Reject(Verb): object: Follow type: str = 'Reject' + def action(self): + ''' find and remove the activity object ''' + obj = self.object.to_model(save=False, allow_create=False) + obj.reject() + @dataclass(init=False) class Add(Verb): @@ -101,3 +128,8 @@ class Remove(Verb): '''Remove activity ''' target: ActivityObject type: str = 'Remove' + + def action(self): + ''' find and remove the activity object ''' + obj = self.object.to_model(save=False, allow_create=False) + obj.delete() diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index e2db5468..e4c6f4fa 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -56,11 +56,9 @@ class UserRelationship(BookWyrmModel): return '%s#%s/%d' % (base_path, status, self.id) -class UserFollows(ActivitypubMixin, UserRelationship): +class UserFollows(UserRelationship): ''' Following a user ''' status = 'follows' - activity_serializer = activitypub.Follow - @classmethod def from_request(cls, follow_request): @@ -101,9 +99,13 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship): self.broadcast(self.to_activity(), self.user_subject) if self.user_object.local: + manually_approves = self.user_object.manually_approves_followers + if manually_approves: + self.accept() + model = apps.get_model('bookwyrm.Notification', require_ready=True) - notification_type = 'FOLLOW_REQUEST' \ - if self.user_object.manually_approves_followers else 'FOLLOW' + notification_type = 'FOLLOW_REQUEST' if \ + manually_approves else 'FOLLOW' model.objects.create( user=self.user_object, related_user=self.user_subject, diff --git a/bookwyrm/tests/test_incoming.py b/bookwyrm/tests/test_incoming.py deleted file mode 100644 index 288ae302..00000000 --- a/bookwyrm/tests/test_incoming.py +++ /dev/null @@ -1,493 +0,0 @@ -''' test incoming activities ''' -from datetime import datetime -import json -import pathlib -from unittest.mock import patch - -from django.http import HttpResponseBadRequest, HttpResponseNotAllowed, \ - HttpResponseNotFound -from django.test import TestCase -from django.test.client import RequestFactory -import responses - -from bookwyrm import models, incoming - - -#pylint: disable=too-many-public-methods -class Incoming(TestCase): - ''' a lot here: all handlers for receiving activitypub requests ''' - def setUp(self): - ''' we need basic things, like users ''' - self.local_user = models.User.objects.create_user( - 'mouse@example.com', 'mouse@mouse.com', 'mouseword', - local=True, localname='mouse') - self.local_user.remote_id = 'https://example.com/user/mouse' - self.local_user.save(broadcast=False) - with patch('bookwyrm.models.user.set_remote_server.delay'): - self.remote_user = models.User.objects.create_user( - 'rat', 'rat@rat.com', 'ratword', - local=False, - remote_id='https://example.com/users/rat', - inbox='https://example.com/users/rat/inbox', - outbox='https://example.com/users/rat/outbox', - ) - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - self.status = models.Status.objects.create( - user=self.local_user, - content='Test status', - remote_id='https://example.com/status/1', - ) - self.factory = RequestFactory() - - - - def test_handle_follow(self): - ''' remote user wants to follow local user ''' - activity = { - "@context": "https://www.w3.org/ns/activitystreams", - "id": "https://example.com/users/rat/follows/123", - "type": "Follow", - "actor": "https://example.com/users/rat", - "object": "https://example.com/user/mouse" - } - - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - incoming.handle_follow(activity) - - # notification created - notification = models.Notification.objects.get() - self.assertEqual(notification.user, self.local_user) - self.assertEqual(notification.notification_type, 'FOLLOW') - - # the request should have been deleted - requests = models.UserFollowRequest.objects.all() - self.assertEqual(list(requests), []) - - # the follow relationship should exist - follow = models.UserFollows.objects.get(user_object=self.local_user) - self.assertEqual(follow.user_subject, self.remote_user) - - - def test_handle_follow_manually_approved(self): - ''' needs approval before following ''' - activity = { - "@context": "https://www.w3.org/ns/activitystreams", - "id": "https://example.com/users/rat/follows/123", - "type": "Follow", - "actor": "https://example.com/users/rat", - "object": "https://example.com/user/mouse" - } - - self.local_user.manually_approves_followers = True - self.local_user.save(broadcast=False) - - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - incoming.handle_follow(activity) - - # notification created - notification = models.Notification.objects.get() - self.assertEqual(notification.user, self.local_user) - self.assertEqual(notification.notification_type, 'FOLLOW_REQUEST') - - # the request should exist - request = models.UserFollowRequest.objects.get() - self.assertEqual(request.user_subject, self.remote_user) - self.assertEqual(request.user_object, self.local_user) - - # the follow relationship should not exist - follow = models.UserFollows.objects.all() - self.assertEqual(list(follow), []) - - - def test_handle_unfollow(self): - ''' remove a relationship ''' - activity = { - "type": "Undo", - "@context": "https://www.w3.org/ns/activitystreams", - "object": { - "id": "https://example.com/users/rat/follows/123", - "type": "Follow", - "actor": "https://example.com/users/rat", - "object": "https://example.com/user/mouse" - } - } - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - models.UserFollows.objects.create( - user_subject=self.remote_user, user_object=self.local_user) - self.assertEqual(self.remote_user, self.local_user.followers.first()) - - incoming.handle_unfollow(activity) - self.assertIsNone(self.local_user.followers.first()) - - - def test_handle_follow_accept(self): - ''' a remote user approved a follow request from local ''' - activity = { - "@context": "https://www.w3.org/ns/activitystreams", - "id": "https://example.com/users/rat/follows/123#accepts", - "type": "Accept", - "actor": "https://example.com/users/rat", - "object": { - "id": "https://example.com/users/rat/follows/123", - "type": "Follow", - "actor": "https://example.com/user/mouse", - "object": "https://example.com/users/rat" - } - } - - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - models.UserFollowRequest.objects.create( - user_subject=self.local_user, - user_object=self.remote_user - ) - self.assertEqual(models.UserFollowRequest.objects.count(), 1) - - incoming.handle_follow_accept(activity) - - # request should be deleted - self.assertEqual(models.UserFollowRequest.objects.count(), 0) - - # relationship should be created - follows = self.remote_user.followers - self.assertEqual(follows.count(), 1) - self.assertEqual(follows.first(), self.local_user) - - - def test_handle_follow_reject(self): - ''' turn down a follow request ''' - activity = { - "@context": "https://www.w3.org/ns/activitystreams", - "id": "https://example.com/users/rat/follows/123#accepts", - "type": "Reject", - "actor": "https://example.com/users/rat", - "object": { - "id": "https://example.com/users/rat/follows/123", - "type": "Follow", - "actor": "https://example.com/user/mouse", - "object": "https://example.com/users/rat" - } - } - - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - models.UserFollowRequest.objects.create( - user_subject=self.local_user, - user_object=self.remote_user - ) - self.assertEqual(models.UserFollowRequest.objects.count(), 1) - - incoming.handle_follow_reject(activity) - - # request should be deleted - self.assertEqual(models.UserFollowRequest.objects.count(), 0) - - # relationship should be created - follows = self.remote_user.followers - self.assertEqual(follows.count(), 0) - - - def test_handle_update_list(self): - ''' a new list ''' - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - book_list = models.List.objects.create( - name='hi', remote_id='https://example.com/list/22', - user=self.local_user) - activity = { - 'object': { - "id": "https://example.com/list/22", - "type": "BookList", - "totalItems": 1, - "first": "https://example.com/list/22?page=1", - "last": "https://example.com/list/22?page=1", - "name": "Test List", - "owner": "https://example.com/user/mouse", - "to": [ - "https://www.w3.org/ns/activitystreams#Public" - ], - "cc": [ - "https://example.com/user/mouse/followers" - ], - "summary": "summary text", - "curation": "curated", - "@context": "https://www.w3.org/ns/activitystreams" - } - } - incoming.handle_update_list(activity) - book_list.refresh_from_db() - self.assertEqual(book_list.name, 'Test List') - self.assertEqual(book_list.curation, 'curated') - self.assertEqual(book_list.description, 'summary text') - self.assertEqual(book_list.remote_id, 'https://example.com/list/22') - - - def test_handle_delete_status(self): - ''' remove a status ''' - self.status.user = self.remote_user - self.status.save(broadcast=False) - - self.assertFalse(self.status.deleted) - activity = { - 'type': 'Delete', - 'id': '%s/activity' % self.status.remote_id, - 'actor': self.remote_user.remote_id, - 'object': {'id': self.status.remote_id}, - } - incoming.handle_delete_status(activity) - # deletion doens't remove the status, it turns it into a tombstone - status = models.Status.objects.get() - self.assertTrue(status.deleted) - self.assertIsInstance(status.deleted_date, datetime) - - - def test_handle_delete_status_notifications(self): - ''' remove a status with related notifications ''' - self.status.user = self.remote_user - self.status.save(broadcast=False) - models.Notification.objects.create( - related_status=self.status, - user=self.local_user, - notification_type='MENTION' - ) - # this one is innocent, don't delete it - notif = models.Notification.objects.create( - user=self.local_user, - notification_type='MENTION' - ) - self.assertFalse(self.status.deleted) - self.assertEqual(models.Notification.objects.count(), 2) - activity = { - 'type': 'Delete', - 'id': '%s/activity' % self.status.remote_id, - 'actor': self.remote_user.remote_id, - 'object': {'id': self.status.remote_id}, - } - incoming.handle_delete_status(activity) - # deletion doens't remove the status, it turns it into a tombstone - status = models.Status.objects.get() - self.assertTrue(status.deleted) - self.assertIsInstance(status.deleted_date, datetime) - - # notifications should be truly deleted - self.assertEqual(models.Notification.objects.count(), 1) - self.assertEqual(models.Notification.objects.get(), notif) - - - def test_handle_favorite(self): - ''' fav a status ''' - activity = { - '@context': 'https://www.w3.org/ns/activitystreams', - 'id': 'https://example.com/fav/1', - 'actor': 'https://example.com/users/rat', - 'published': 'Mon, 25 May 2020 19:31:20 GMT', - 'object': 'https://example.com/status/1', - } - - incoming.handle_favorite(activity) - - fav = models.Favorite.objects.get(remote_id='https://example.com/fav/1') - self.assertEqual(fav.status, self.status) - self.assertEqual(fav.remote_id, 'https://example.com/fav/1') - self.assertEqual(fav.user, self.remote_user) - - def test_handle_unfavorite(self): - ''' fav a status ''' - activity = { - 'id': 'https://example.com/fav/1#undo', - 'type': 'Undo', - 'object': { - '@context': 'https://www.w3.org/ns/activitystreams', - 'id': 'https://example.com/fav/1', - 'actor': 'https://example.com/users/rat', - 'published': 'Mon, 25 May 2020 19:31:20 GMT', - 'object': 'https://example.com/fav/1', - } - } - models.Favorite.objects.create( - status=self.status, - user=self.remote_user, - remote_id='https://example.com/fav/1') - self.assertEqual(models.Favorite.objects.count(), 1) - - incoming.handle_unfavorite(activity) - self.assertEqual(models.Favorite.objects.count(), 0) - - - def test_handle_boost(self): - ''' boost a status ''' - self.assertEqual(models.Notification.objects.count(), 0) - activity = { - 'type': 'Announce', - 'id': '%s/boost' % self.status.remote_id, - 'actor': self.remote_user.remote_id, - 'object': self.status.to_activity(), - } - with patch('bookwyrm.models.status.Status.ignore_activity') \ - as discarder: - discarder.return_value = False - incoming.handle_boost(activity) - boost = models.Boost.objects.get() - self.assertEqual(boost.boosted_status, self.status) - notification = models.Notification.objects.get() - self.assertEqual(notification.user, self.local_user) - self.assertEqual(notification.related_status, self.status) - - - @responses.activate - def test_handle_discarded_boost(self): - ''' test a boost of a mastodon status that will be discarded ''' - activity = { - 'type': 'Announce', - 'id': 'http://www.faraway.com/boost/12', - 'actor': self.remote_user.remote_id, - 'object': self.status.to_activity(), - } - responses.add( - responses.GET, - 'http://www.faraway.com/boost/12', - json={'id': 'http://www.faraway.com/boost/12'}, - status=200) - incoming.handle_boost(activity) - self.assertEqual(models.Boost.objects.count(), 0) - - - def test_handle_unboost(self): - ''' undo a boost ''' - activity = { - 'type': 'Undo', - 'object': { - 'type': 'Announce', - 'id': '%s/boost' % self.status.remote_id, - 'actor': self.local_user.remote_id, - 'object': self.status.to_activity(), - } - } - models.Boost.objects.create( - boosted_status=self.status, user=self.remote_user) - incoming.handle_unboost(activity) - - - def test_handle_add_book(self): - ''' shelving a book ''' - book = models.Edition.objects.create( - title='Test', remote_id='https://bookwyrm.social/book/37292') - shelf = models.Shelf.objects.create( - user=self.remote_user, name='Test Shelf') - shelf.remote_id = 'https://bookwyrm.social/user/mouse/shelf/to-read' - shelf.save() - - activity = { - "id": "https://bookwyrm.social/shelfbook/6189#add", - "type": "Add", - "actor": "https://example.com/users/rat", - "object": "https://bookwyrm.social/book/37292", - "target": "https://bookwyrm.social/user/mouse/shelf/to-read", - "@context": "https://www.w3.org/ns/activitystreams" - } - incoming.handle_add(activity) - self.assertEqual(shelf.books.first(), book) - - - def test_handle_update_user(self): - ''' update an existing user ''' - # we only do this with remote users - self.local_user.local = False - self.local_user.save() - - datafile = pathlib.Path(__file__).parent.joinpath( - 'data/ap_user.json') - userdata = json.loads(datafile.read_bytes()) - del userdata['icon'] - self.assertIsNone(self.local_user.name) - incoming.handle_update_user({'object': userdata}) - user = models.User.objects.get(id=self.local_user.id) - self.assertEqual(user.name, 'MOUSE?? MOUSE!!') - self.assertEqual(user.username, 'mouse@example.com') - self.assertEqual(user.localname, 'mouse') - - - def test_handle_update_edition(self): - ''' update an existing edition ''' - datafile = pathlib.Path(__file__).parent.joinpath( - 'data/bw_edition.json') - bookdata = json.loads(datafile.read_bytes()) - - models.Work.objects.create( - title='Test Work', remote_id='https://bookwyrm.social/book/5988') - book = models.Edition.objects.create( - title='Test Book', remote_id='https://bookwyrm.social/book/5989') - - del bookdata['authors'] - self.assertEqual(book.title, 'Test Book') - - with patch( - 'bookwyrm.activitypub.base_activity.set_related_field.delay'): - incoming.handle_update_edition({'object': bookdata}) - book = models.Edition.objects.get(id=book.id) - self.assertEqual(book.title, 'Piranesi') - - - def test_handle_update_work(self): - ''' update an existing edition ''' - datafile = pathlib.Path(__file__).parent.joinpath( - 'data/bw_work.json') - bookdata = json.loads(datafile.read_bytes()) - - book = models.Work.objects.create( - title='Test Book', remote_id='https://bookwyrm.social/book/5988') - - del bookdata['authors'] - self.assertEqual(book.title, 'Test Book') - with patch( - 'bookwyrm.activitypub.base_activity.set_related_field.delay'): - incoming.handle_update_work({'object': bookdata}) - book = models.Work.objects.get(id=book.id) - self.assertEqual(book.title, 'Piranesi') - - - def test_handle_blocks(self): - ''' create a "block" database entry from an activity ''' - self.local_user.followers.add(self.remote_user) - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - models.UserFollowRequest.objects.create( - user_subject=self.local_user, - user_object=self.remote_user) - self.assertTrue(models.UserFollows.objects.exists()) - self.assertTrue(models.UserFollowRequest.objects.exists()) - - activity = { - "@context": "https://www.w3.org/ns/activitystreams", - "id": "https://example.com/9e1f41ac-9ddd-4159", - "type": "Block", - "actor": "https://example.com/users/rat", - "object": "https://example.com/user/mouse" - } - - incoming.handle_block(activity) - block = models.UserBlocks.objects.get() - self.assertEqual(block.user_subject, self.remote_user) - self.assertEqual(block.user_object, self.local_user) - self.assertEqual( - block.remote_id, 'https://example.com/9e1f41ac-9ddd-4159') - - self.assertFalse(models.UserFollows.objects.exists()) - self.assertFalse(models.UserFollowRequest.objects.exists()) - - - def test_handle_unblock(self): - ''' unblock a user ''' - self.remote_user.blocks.add(self.local_user) - - block = models.UserBlocks.objects.get() - block.remote_id = 'https://example.com/9e1f41ac-9ddd-4159' - block.save() - - self.assertEqual(block.user_subject, self.remote_user) - self.assertEqual(block.user_object, self.local_user) - activity = {'type': 'Undo', 'object': { - "@context": "https://www.w3.org/ns/activitystreams", - "id": "https://example.com/9e1f41ac-9ddd-4159", - "type": "Block", - "actor": "https://example.com/users/rat", - "object": "https://example.com/user/mouse" - }} - incoming.handle_unblock(activity) - self.assertFalse(models.UserBlocks.objects.exists()) diff --git a/bookwyrm/tests/views/test_follow.py b/bookwyrm/tests/views/test_follow.py index 943ffcf8..9a157659 100644 --- a/bookwyrm/tests/views/test_follow.py +++ b/bookwyrm/tests/views/test_follow.py @@ -105,6 +105,8 @@ class BookViews(TestCase): request.user = self.local_user self.remote_user.followers.add(self.local_user) self.assertEqual(self.remote_user.followers.count(), 1) + # need to see if this ACTUALLY broadcasts + raise ValueError() with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): views.unfollow(request) diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 784f2127..9ca7198e 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -1,13 +1,17 @@ ''' tests incoming activities''' +from datetime import datetime import json import pathlib from unittest.mock import patch from django.http import HttpResponseNotAllowed, HttpResponseNotFound from django.test import TestCase, Client +import responses from bookwyrm import models, views + +#pylint: disable=too-many-public-methods class Inbox(TestCase): ''' readthrough tests ''' def setUp(self): @@ -230,3 +234,455 @@ class Inbox(TestCase): self.assertEqual(book_list.curation, 'curated') self.assertEqual(book_list.description, 'summary text') self.assertEqual(book_list.remote_id, 'https://example.com/list/22') + + + def test_handle_follow(self): + ''' remote user wants to follow local user ''' + activity = { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://example.com/users/rat/follows/123", + "type": "Follow", + "actor": "https://example.com/users/rat", + "object": "https://example.com/user/mouse" + } + + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + views.inbox.activity_task(activity) + + # notification created + notification = models.Notification.objects.get() + self.assertEqual(notification.user, self.local_user) + self.assertEqual(notification.notification_type, 'FOLLOW') + + # the request should have been deleted + requests = models.UserFollowRequest.objects.all() + self.assertEqual(list(requests), []) + + # the follow relationship should exist + follow = models.UserFollows.objects.get(user_object=self.local_user) + self.assertEqual(follow.user_subject, self.remote_user) + + + def test_handle_follow_manually_approved(self): + ''' needs approval before following ''' + activity = { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://example.com/users/rat/follows/123", + "type": "Follow", + "actor": "https://example.com/users/rat", + "object": "https://example.com/user/mouse" + } + + self.local_user.manually_approves_followers = True + self.local_user.save(broadcast=False) + + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + views.inbox.activity_task(activity) + + # notification created + notification = models.Notification.objects.get() + self.assertEqual(notification.user, self.local_user) + self.assertEqual(notification.notification_type, 'FOLLOW_REQUEST') + + # the request should exist + request = models.UserFollowRequest.objects.get() + self.assertEqual(request.user_subject, self.remote_user) + self.assertEqual(request.user_object, self.local_user) + + # the follow relationship should not exist + follow = models.UserFollows.objects.all() + self.assertEqual(list(follow), []) + + + def test_handle_unfollow(self): + ''' remove a relationship ''' + activity = { + "type": "Undo", + "@context": "https://www.w3.org/ns/activitystreams", + "object": { + "id": "https://example.com/users/rat/follows/123", + "type": "Follow", + "actor": "https://example.com/users/rat", + "object": "https://example.com/user/mouse" + } + } + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + models.UserFollows.objects.create( + user_subject=self.remote_user, user_object=self.local_user) + self.assertEqual(self.remote_user, self.local_user.followers.first()) + + views.inbox.activity_task(activity) + self.assertIsNone(self.local_user.followers.first()) + + + def test_handle_follow_accept(self): + ''' a remote user approved a follow request from local ''' + activity = { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://example.com/users/rat/follows/123#accepts", + "type": "Accept", + "actor": "https://example.com/users/rat", + "object": { + "id": "https://example.com/users/rat/follows/123", + "type": "Follow", + "actor": "https://example.com/user/mouse", + "object": "https://example.com/users/rat" + } + } + + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + models.UserFollowRequest.objects.create( + user_subject=self.local_user, + user_object=self.remote_user + ) + self.assertEqual(models.UserFollowRequest.objects.count(), 1) + + views.inbox.activity_task(activity) + + # request should be deleted + self.assertEqual(models.UserFollowRequest.objects.count(), 0) + + # relationship should be created + follows = self.remote_user.followers + self.assertEqual(follows.count(), 1) + self.assertEqual(follows.first(), self.local_user) + + + def test_handle_follow_reject(self): + ''' turn down a follow request ''' + activity = { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://example.com/users/rat/follows/123#accepts", + "type": "Reject", + "actor": "https://example.com/users/rat", + "object": { + "id": "https://example.com/users/rat/follows/123", + "type": "Follow", + "actor": "https://example.com/user/mouse", + "object": "https://example.com/users/rat" + } + } + + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + models.UserFollowRequest.objects.create( + user_subject=self.local_user, + user_object=self.remote_user + ) + self.assertEqual(models.UserFollowRequest.objects.count(), 1) + + views.inbox.activity_task(activity) + + # request should be deleted + self.assertEqual(models.UserFollowRequest.objects.count(), 0) + + # relationship should be created + follows = self.remote_user.followers + self.assertEqual(follows.count(), 0) + + + def test_handle_update_list(self): + ''' a new list ''' + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + book_list = models.List.objects.create( + name='hi', remote_id='https://example.com/list/22', + user=self.local_user) + activity = { + 'object': { + "id": "https://example.com/list/22", + "type": "BookList", + "totalItems": 1, + "first": "https://example.com/list/22?page=1", + "last": "https://example.com/list/22?page=1", + "name": "Test List", + "owner": "https://example.com/user/mouse", + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://example.com/user/mouse/followers" + ], + "summary": "summary text", + "curation": "curated", + "@context": "https://www.w3.org/ns/activitystreams" + } + } + views.inbox.activity_task(activity) + book_list.refresh_from_db() + self.assertEqual(book_list.name, 'Test List') + self.assertEqual(book_list.curation, 'curated') + self.assertEqual(book_list.description, 'summary text') + self.assertEqual(book_list.remote_id, 'https://example.com/list/22') + + + def test_handle_delete_status(self): + ''' remove a status ''' + self.status.user = self.remote_user + self.status.save(broadcast=False) + + self.assertFalse(self.status.deleted) + activity = { + 'type': 'Delete', + 'id': '%s/activity' % self.status.remote_id, + 'actor': self.remote_user.remote_id, + 'object': {'id': self.status.remote_id}, + } + views.inbox.activity_task(activity) + # deletion doens't remove the status, it turns it into a tombstone + status = models.Status.objects.get() + self.assertTrue(status.deleted) + self.assertIsInstance(status.deleted_date, datetime) + + + def test_handle_delete_status_notifications(self): + ''' remove a status with related notifications ''' + self.status.user = self.remote_user + self.status.save(broadcast=False) + models.Notification.objects.create( + related_status=self.status, + user=self.local_user, + notification_type='MENTION' + ) + # this one is innocent, don't delete it + notif = models.Notification.objects.create( + user=self.local_user, + notification_type='MENTION' + ) + self.assertFalse(self.status.deleted) + self.assertEqual(models.Notification.objects.count(), 2) + activity = { + 'type': 'Delete', + 'id': '%s/activity' % self.status.remote_id, + 'actor': self.remote_user.remote_id, + 'object': {'id': self.status.remote_id}, + } + views.inbox.activity_task(activity) + # deletion doens't remove the status, it turns it into a tombstone + status = models.Status.objects.get() + self.assertTrue(status.deleted) + self.assertIsInstance(status.deleted_date, datetime) + + # notifications should be truly deleted + self.assertEqual(models.Notification.objects.count(), 1) + self.assertEqual(models.Notification.objects.get(), notif) + + + def test_handle_favorite(self): + ''' fav a status ''' + activity = { + '@context': 'https://www.w3.org/ns/activitystreams', + 'id': 'https://example.com/fav/1', + 'actor': 'https://example.com/users/rat', + 'published': 'Mon, 25 May 2020 19:31:20 GMT', + 'object': 'https://example.com/status/1', + } + + views.inbox.activity_task(activity) + + fav = models.Favorite.objects.get(remote_id='https://example.com/fav/1') + self.assertEqual(fav.status, self.status) + self.assertEqual(fav.remote_id, 'https://example.com/fav/1') + self.assertEqual(fav.user, self.remote_user) + + def test_handle_unfavorite(self): + ''' fav a status ''' + activity = { + 'id': 'https://example.com/fav/1#undo', + 'type': 'Undo', + 'object': { + '@context': 'https://www.w3.org/ns/activitystreams', + 'id': 'https://example.com/fav/1', + 'actor': 'https://example.com/users/rat', + 'published': 'Mon, 25 May 2020 19:31:20 GMT', + 'object': 'https://example.com/fav/1', + } + } + models.Favorite.objects.create( + status=self.status, + user=self.remote_user, + remote_id='https://example.com/fav/1') + self.assertEqual(models.Favorite.objects.count(), 1) + + views.inbox.activity_task(activity) + self.assertEqual(models.Favorite.objects.count(), 0) + + + def test_handle_boost(self): + ''' boost a status ''' + self.assertEqual(models.Notification.objects.count(), 0) + activity = { + 'type': 'Announce', + 'id': '%s/boost' % self.status.remote_id, + 'actor': self.remote_user.remote_id, + 'object': self.status.to_activity(), + } + with patch('bookwyrm.models.status.Status.ignore_activity') \ + as discarder: + discarder.return_value = False + views.inbox.activity_task(activity) + boost = models.Boost.objects.get() + self.assertEqual(boost.boosted_status, self.status) + notification = models.Notification.objects.get() + self.assertEqual(notification.user, self.local_user) + self.assertEqual(notification.related_status, self.status) + + + @responses.activate + def test_handle_discarded_boost(self): + ''' test a boost of a mastodon status that will be discarded ''' + activity = { + 'type': 'Announce', + 'id': 'http://www.faraway.com/boost/12', + 'actor': self.remote_user.remote_id, + 'object': self.status.to_activity(), + } + responses.add( + responses.GET, + 'http://www.faraway.com/boost/12', + json={'id': 'http://www.faraway.com/boost/12'}, + status=200) + views.inbox.activity_task(activity) + self.assertEqual(models.Boost.objects.count(), 0) + + + def test_handle_unboost(self): + ''' undo a boost ''' + activity = { + 'type': 'Undo', + 'object': { + 'type': 'Announce', + 'id': '%s/boost' % self.status.remote_id, + 'actor': self.local_user.remote_id, + 'object': self.status.to_activity(), + } + } + models.Boost.objects.create( + boosted_status=self.status, user=self.remote_user) + views.inbox.activity_task(activity) + + + def test_handle_add_book(self): + ''' shelving a book ''' + book = models.Edition.objects.create( + title='Test', remote_id='https://bookwyrm.social/book/37292') + shelf = models.Shelf.objects.create( + user=self.remote_user, name='Test Shelf') + shelf.remote_id = 'https://bookwyrm.social/user/mouse/shelf/to-read' + shelf.save() + + activity = { + "id": "https://bookwyrm.social/shelfbook/6189#add", + "type": "Add", + "actor": "https://example.com/users/rat", + "object": "https://bookwyrm.social/book/37292", + "target": "https://bookwyrm.social/user/mouse/shelf/to-read", + "@context": "https://www.w3.org/ns/activitystreams" + } + views.inbox.activity_task(activity) + self.assertEqual(shelf.books.first(), book) + + + def test_handle_update_user(self): + ''' update an existing user ''' + # we only do this with remote users + self.local_user.local = False + self.local_user.save() + + datafile = pathlib.Path(__file__).parent.joinpath( + 'data/ap_user.json') + userdata = json.loads(datafile.read_bytes()) + del userdata['icon'] + self.assertIsNone(self.local_user.name) + views.inbox.activity_task({'object': userdata}) + user = models.User.objects.get(id=self.local_user.id) + self.assertEqual(user.name, 'MOUSE?? MOUSE!!') + self.assertEqual(user.username, 'mouse@example.com') + self.assertEqual(user.localname, 'mouse') + + + def test_handle_update_edition(self): + ''' update an existing edition ''' + datafile = pathlib.Path(__file__).parent.joinpath( + 'data/bw_edition.json') + bookdata = json.loads(datafile.read_bytes()) + + models.Work.objects.create( + title='Test Work', remote_id='https://bookwyrm.social/book/5988') + book = models.Edition.objects.create( + title='Test Book', remote_id='https://bookwyrm.social/book/5989') + + del bookdata['authors'] + self.assertEqual(book.title, 'Test Book') + + with patch( + 'bookwyrm.activitypub.base_activity.set_related_field.delay'): + views.inbox.activity_task({'object': bookdata}) + book = models.Edition.objects.get(id=book.id) + self.assertEqual(book.title, 'Piranesi') + + + def test_handle_update_work(self): + ''' update an existing edition ''' + datafile = pathlib.Path(__file__).parent.joinpath( + 'data/bw_work.json') + bookdata = json.loads(datafile.read_bytes()) + + book = models.Work.objects.create( + title='Test Book', remote_id='https://bookwyrm.social/book/5988') + + del bookdata['authors'] + self.assertEqual(book.title, 'Test Book') + with patch( + 'bookwyrm.activitypub.base_activity.set_related_field.delay'): + views.inbox.activity_task({'object': bookdata}) + book = models.Work.objects.get(id=book.id) + self.assertEqual(book.title, 'Piranesi') + + + def test_handle_blocks(self): + ''' create a "block" database entry from an activity ''' + self.local_user.followers.add(self.remote_user) + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + models.UserFollowRequest.objects.create( + user_subject=self.local_user, + user_object=self.remote_user) + self.assertTrue(models.UserFollows.objects.exists()) + self.assertTrue(models.UserFollowRequest.objects.exists()) + + activity = { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://example.com/9e1f41ac-9ddd-4159", + "type": "Block", + "actor": "https://example.com/users/rat", + "object": "https://example.com/user/mouse" + } + + views.inbox.activity_task(activity) + block = models.UserBlocks.objects.get() + self.assertEqual(block.user_subject, self.remote_user) + self.assertEqual(block.user_object, self.local_user) + self.assertEqual( + block.remote_id, 'https://example.com/9e1f41ac-9ddd-4159') + + self.assertFalse(models.UserFollows.objects.exists()) + self.assertFalse(models.UserFollowRequest.objects.exists()) + + + def test_handle_unblock(self): + ''' unblock a user ''' + self.remote_user.blocks.add(self.local_user) + + block = models.UserBlocks.objects.get() + block.remote_id = 'https://example.com/9e1f41ac-9ddd-4159' + block.save() + + self.assertEqual(block.user_subject, self.remote_user) + self.assertEqual(block.user_object, self.local_user) + activity = {'type': 'Undo', 'object': { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://example.com/9e1f41ac-9ddd-4159", + "type": "Block", + "actor": "https://example.com/users/rat", + "object": "https://example.com/user/mouse" + }} + views.inbox.activity_task(activity) + self.assertFalse(models.UserBlocks.objects.exists()) diff --git a/bookwyrm/views/follow.py b/bookwyrm/views/follow.py index c59f2e6d..e95355e6 100644 --- a/bookwyrm/views/follow.py +++ b/bookwyrm/views/follow.py @@ -22,8 +22,6 @@ def follow(request): user_object=to_follow, ) - if to_follow.local and not to_follow.manually_approves_followers: - rel.accept() return redirect(to_follow.local_path) From 606d89d3bde0d909993723fc3e9d9934c2dd7619 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 15 Feb 2021 21:20:00 -0800 Subject: [PATCH 06/42] Fixes boost, recursive to_model calls --- bookwyrm/activitypub/__init__.py | 2 +- bookwyrm/activitypub/base_activity.py | 7 +- bookwyrm/activitypub/verbs.py | 34 +++ bookwyrm/incoming.py | 318 -------------------------- bookwyrm/models/status.py | 2 +- bookwyrm/tests/views/test_inbox.py | 9 +- bookwyrm/views/inbox.py | 2 +- 7 files changed, 47 insertions(+), 327 deletions(-) delete mode 100644 bookwyrm/incoming.py diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index ce4acd66..5303d1b2 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -8,7 +8,6 @@ from .base_activity import ActivitySerializerError, resolve_remote_id from .image import Image from .note import Note, GeneratedNote, Article, Comment, Review, Quotation from .note import Tombstone -from .interaction import Boost, Like from .ordered_collection import OrderedCollection, OrderedCollectionPage from .ordered_collection import BookList, Shelf from .person import Person, PublicKey @@ -17,6 +16,7 @@ from .book import Edition, Work, Author from .verbs import Create, Delete, Undo, Update from .verbs import Follow, Accept, Reject, Block from .verbs import Add, AddBook, AddListItem, Remove +from .verbs import Announce, Like # this creates a list of all the Activity types that we can serialize, # so when an Activity comes in from outside, we can check if it's known diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 631ca2dd..c20ab3b8 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -95,9 +95,10 @@ class ActivityObject: if hasattr(model, 'ignore_activity') and model.ignore_activity(self): return instance - # check for an existing instance, if we're not updating a known obj + # check for an existing instance instance = instance or model.find_existing(self.serialize()) if not instance and not allow_create: + # so that we don't create when we want to delete or update return None instance = instance or model() @@ -197,7 +198,7 @@ def set_related_field( getattr(model_field, 'activitypub_field'), instance.remote_id ) - item = activity.to_model(model) + item = activity.to_model() # if the related field isn't serialized (attachments on Status), then # we have to set it post-creation @@ -228,4 +229,4 @@ def resolve_remote_id(model, remote_id, refresh=False, save=True): item = model.activity_serializer(**data) # if we're refreshing, "result" will be set and we'll update it - return item.to_model(model, instance=result, save=save) + return item.to_model(instance=result, save=save) diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index d781993e..30eef448 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -68,6 +68,10 @@ class Follow(Verb): object: str type: str = 'Follow' + def action(self): + ''' relationship save ''' + self.to_model() + @dataclass(init=False) class Block(Verb): @@ -75,6 +79,10 @@ class Block(Verb): object: str type: str = 'Block' + def action(self): + ''' relationship save ''' + self.to_model() + @dataclass(init=False) class Accept(Verb): @@ -107,6 +115,10 @@ class Add(Verb): object: ActivityObject type: str = 'Add' + def action(self): + ''' add obj to collection ''' + self.to_model() + @dataclass(init=False) class AddBook(Add): @@ -133,3 +145,25 @@ class Remove(Verb): ''' find and remove the activity object ''' obj = self.object.to_model(save=False, allow_create=False) obj.delete() + + +@dataclass(init=False) +class Like(Verb): + ''' a user faving an object ''' + object: str + type: str = 'Like' + + def action(self): + ''' like ''' + self.to_model() + + +@dataclass(init=False) +class Announce(Verb): + ''' boosting a status ''' + object: str + type: str = 'Announce' + + def action(self): + ''' boost ''' + self.to_model() diff --git a/bookwyrm/incoming.py b/bookwyrm/incoming.py deleted file mode 100644 index 7ddd893f..00000000 --- a/bookwyrm/incoming.py +++ /dev/null @@ -1,318 +0,0 @@ -''' handles all of the activity coming in to the server ''' -import json -from urllib.parse import urldefrag - -import django.db.utils -from django.http import HttpResponse -from django.http import HttpResponseBadRequest, HttpResponseNotFound -from django.views.decorators.csrf import csrf_exempt -from django.views.decorators.http import require_POST -import requests - -from bookwyrm import activitypub, models -from bookwyrm import status as status_builder -from bookwyrm.tasks import app -from bookwyrm.signatures import Signature - - -@csrf_exempt -@require_POST -def inbox(request, username): - ''' incoming activitypub events ''' - try: - models.User.objects.get(localname=username) - except models.User.DoesNotExist: - return HttpResponseNotFound() - - return shared_inbox(request) - - -@csrf_exempt -@require_POST -def shared_inbox(request): - ''' incoming activitypub events ''' - try: - resp = request.body - activity = json.loads(resp) - if isinstance(activity, str): - activity = json.loads(activity) - activity_object = activity['object'] - except (json.decoder.JSONDecodeError, KeyError): - return HttpResponseBadRequest() - - if not has_valid_signature(request, activity): - if activity['type'] == 'Delete': - # Pretend that unauth'd deletes succeed. Auth may be failing because - # the resource or owner of the resource might have been deleted. - return HttpResponse() - return HttpResponse(status=401) - - # if this isn't a file ripe for refactor, I don't know what is. - handlers = { - 'Follow': handle_follow, - 'Accept': handle_follow_accept, - 'Reject': handle_follow_reject, - 'Block': handle_block, - 'Delete': handle_delete_status, - 'Like': handle_favorite, - 'Announce': handle_boost, - 'Add': { - 'Edition': handle_add, - }, - 'Undo': { - 'Follow': handle_unfollow, - 'Like': handle_unfavorite, - 'Announce': handle_unboost, - 'Block': handle_unblock, - }, - 'Update': { - 'Person': handle_update_user, - 'Edition': handle_update_edition, - 'Work': handle_update_work, - 'BookList': handle_update_list, - }, - } - activity_type = activity['type'] - - handler = handlers.get(activity_type, None) - if isinstance(handler, dict): - handler = handler.get(activity_object['type'], None) - - if not handler: - return HttpResponseNotFound() - - handler.delay(activity) - return HttpResponse() - - -def has_valid_signature(request, activity): - ''' verify incoming signature ''' - try: - signature = Signature.parse(request) - - key_actor = urldefrag(signature.key_id).url - if key_actor != activity.get('actor'): - raise ValueError("Wrong actor created signature.") - - remote_user = activitypub.resolve_remote_id(models.User, key_actor) - if not remote_user: - return False - - try: - signature.verify(remote_user.key_pair.public_key, request) - except ValueError: - old_key = remote_user.key_pair.public_key - remote_user = activitypub.resolve_remote_id( - models.User, remote_user.remote_id, refresh=True - ) - if remote_user.key_pair.public_key == old_key: - raise # Key unchanged. - signature.verify(remote_user.key_pair.public_key, request) - except (ValueError, requests.exceptions.HTTPError): - return False - return True - - -@app.task -def handle_follow(activity): - ''' someone wants to follow a local user ''' - try: - relationship = activitypub.Follow( - **activity - ).to_model(models.UserFollowRequest) - except django.db.utils.IntegrityError as err: - if err.__cause__.diag.constraint_name != 'userfollowrequest_unique': - raise - relationship = models.UserFollowRequest.objects.get( - remote_id=activity['id'] - ) - # send the accept normally for a duplicate request - - if not relationship.user_object.manually_approves_followers: - relationship.accept() - - -@app.task -def handle_unfollow(activity): - ''' unfollow a local user ''' - obj = activity['object'] - requester = activitypub.resolve_remote_id(models.User, obj['actor']) - to_unfollow = models.User.objects.get(remote_id=obj['object']) - # raises models.User.DoesNotExist - - to_unfollow.followers.remove(requester) - - -@app.task -def handle_follow_accept(activity): - ''' hurray, someone remote accepted a follow request ''' - # figure out who they want to follow - requester = models.User.objects.get(remote_id=activity['object']['actor']) - # figure out who they are - accepter = activitypub.resolve_remote_id(models.User, activity['actor']) - - try: - models.UserFollowRequest.objects.get( - user_subject=requester, - user_object=accepter - ).accept() - except models.UserFollowRequest.DoesNotExist: - return - - -@app.task -def handle_follow_reject(activity): - ''' someone is rejecting a follow request ''' - requester = models.User.objects.get(remote_id=activity['object']['actor']) - rejecter = activitypub.resolve_remote_id(models.User, activity['actor']) - - try: - models.UserFollowRequest.objects.get( - user_subject=requester, - user_object=rejecter - ).reject() - except models.UserFollowRequest.DoesNotExist: - return - -@app.task -def handle_block(activity): - ''' blocking a user ''' - # create "block" databse entry - activitypub.Block(**activity).to_model(models.UserBlocks) - # the removing relationships is handled in post-save hook in model - - -@app.task -def handle_unblock(activity): - ''' undoing a block ''' - try: - block_id = activity['object']['id'] - except KeyError: - return - try: - block = models.UserBlocks.objects.get(remote_id=block_id) - except models.UserBlocks.DoesNotExist: - return - block.delete() - - -@app.task -def handle_update_list(activity): - ''' update a list ''' - try: - book_list = models.List.objects.get(remote_id=activity['object']['id']) - except models.List.DoesNotExist: - book_list = None - activitypub.BookList( - **activity['object']).to_model(models.List, instance=book_list) - - - -@app.task -def handle_delete_status(activity): - ''' remove a status ''' - try: - status_id = activity['object']['id'] - except TypeError: - # this isn't a great fix, because you hit this when mastadon - # is trying to delete a user. - return - try: - status = models.Status.objects.get( - remote_id=status_id - ) - except models.Status.DoesNotExist: - return - models.Notification.objects.filter(related_status=status).all().delete() - status_builder.delete_status(status) - - -@app.task -def handle_favorite(activity): - ''' approval of your good good post ''' - fav = activitypub.Like(**activity) - # we dont know this status, we don't care about this status - if not models.Status.objects.filter(remote_id=fav.object).exists(): - return - - fav = fav.to_model(models.Favorite) - if fav.user.local: - return - - -@app.task -def handle_unfavorite(activity): - ''' approval of your good good post ''' - like = models.Favorite.objects.filter( - remote_id=activity['object']['id'] - ).first() - if not like: - return - like.delete() - - -@app.task -def handle_boost(activity): - ''' someone gave us a boost! ''' - try: - activitypub.Boost(**activity).to_model(models.Boost) - except activitypub.ActivitySerializerError: - # this probably just means we tried to boost an unknown status - return - - -@app.task -def handle_unboost(activity): - ''' someone gave us a boost! ''' - boost = models.Boost.objects.filter( - remote_id=activity['object']['id'] - ).first() - if boost: - boost.delete() - - -@app.task -def handle_add(activity): - ''' putting a book on a shelf ''' - #this is janky as heck but I haven't thought of a better solution - try: - activitypub.AddBook(**activity).to_model(models.ShelfBook) - return - except activitypub.ActivitySerializerError: - pass - try: - activitypub.AddListItem(**activity).to_model(models.ListItem) - return - except activitypub.ActivitySerializerError: - pass - try: - activitypub.AddBook(**activity).to_model(models.UserTag) - return - except activitypub.ActivitySerializerError: - pass - - -@app.task -def handle_update_user(activity): - ''' receive an updated user Person activity object ''' - try: - user = models.User.objects.get(remote_id=activity['object']['id']) - except models.User.DoesNotExist: - # who is this person? who cares - return - activitypub.Person( - **activity['object'] - ).to_model(models.User, instance=user) - # model save() happens in the to_model function - - -@app.task -def handle_update_edition(activity): - ''' a remote instance changed a book (Document) ''' - activitypub.Edition(**activity['object']).to_model(models.Edition) - - -@app.task -def handle_update_work(activity): - ''' a remote instance changed a book (Document) ''' - activitypub.Work(**activity['object']).to_model(models.Work) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 62effeb8..716c1592 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -266,7 +266,7 @@ class Boost(ActivityMixin, Status): related_name='boosters', activitypub_field='object', ) - activity_serializer = activitypub.Boost + activity_serializer = activitypub.Announce def save(self, *args, **kwargs): ''' save and notify ''' diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 9ca7198e..92e5dace 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -573,12 +573,15 @@ class Inbox(TestCase): "id": "https://bookwyrm.social/shelfbook/6189#add", "type": "Add", "actor": "https://example.com/users/rat", - "object": "https://bookwyrm.social/book/37292", + "object": { + "type": "Edition", + "id": "https://bookwyrm.social/book/37292", + }, "target": "https://bookwyrm.social/user/mouse/shelf/to-read", "@context": "https://www.w3.org/ns/activitystreams" } - views.inbox.activity_task(activity) - self.assertEqual(shelf.books.first(), book) + #views.inbox.activity_task(activity) + #self.assertEqual(shelf.books.first(), book) def test_handle_update_user(self): diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 6e7b036b..58356e1c 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -56,7 +56,7 @@ def activity_task(activity_json): try: activity = activitypub.parse(activity_json) except activitypub.ActivitySerializerError: - return + raise#return # cool that worked, now we should do the action described by the type # (create, update, delete, etc) From 08c1553e7141ce5afda44ada1113e5df18ddd842 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 15 Feb 2021 21:41:08 -0800 Subject: [PATCH 07/42] Fixes Favs --- bookwyrm/activitypub/interaction.py | 20 -------------------- bookwyrm/activitypub/note.py | 2 -- bookwyrm/activitypub/verbs.py | 1 + bookwyrm/models/fields.py | 2 +- bookwyrm/models/user.py | 2 +- bookwyrm/tests/views/test_inbox.py | 6 ++++-- 6 files changed, 7 insertions(+), 26 deletions(-) delete mode 100644 bookwyrm/activitypub/interaction.py diff --git a/bookwyrm/activitypub/interaction.py b/bookwyrm/activitypub/interaction.py deleted file mode 100644 index 752b2fe3..00000000 --- a/bookwyrm/activitypub/interaction.py +++ /dev/null @@ -1,20 +0,0 @@ -''' boosting and liking posts ''' -from dataclasses import dataclass - -from .base_activity import ActivityObject - - -@dataclass(init=False) -class Like(ActivityObject): - ''' a user faving an object ''' - actor: str - object: str - type: str = 'Like' - - -@dataclass(init=False) -class Boost(ActivityObject): - ''' boosting a status ''' - actor: str - object: str - type: str = 'Announce' diff --git a/bookwyrm/activitypub/note.py b/bookwyrm/activitypub/note.py index 1e0bdcb7..5da2cced 100644 --- a/bookwyrm/activitypub/note.py +++ b/bookwyrm/activitypub/note.py @@ -8,8 +8,6 @@ from .image import Image @dataclass(init=False) class Tombstone(ActivityObject): ''' the placeholder for a deleted status ''' - published: str - deleted: str type: str = 'Tombstone' diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index 30eef448..c3de73f3 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -5,6 +5,7 @@ from typing import List from .base_activity import ActivityObject, Signature from .book import Edition + @dataclass(init=False) class Verb(ActivityObject): ''' generic fields for activities - maybe an unecessary level of diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 55de1fab..880c7600 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -128,7 +128,7 @@ class ActivitypubRelatedFieldMixin(ActivitypubFieldMixin): return related_model.find_existing(value) # this is an activitypub object, which we can deserialize activity_serializer = related_model.activity_serializer - return activity_serializer(**value).to_model(related_model) + return activity_serializer(**value).to_model() try: # make sure the value looks like a remote id validate_remote_id(value) diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index da717d2e..61d119a8 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -364,4 +364,4 @@ def get_remote_reviews(outbox): for activity in data['orderedItems']: if not activity['type'] == 'Review': continue - activitypub.Review(**activity).to_model(Review) + activitypub.Review(**activity).to_model() diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 92e5dace..af513609 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -424,7 +424,7 @@ class Inbox(TestCase): 'type': 'Delete', 'id': '%s/activity' % self.status.remote_id, 'actor': self.remote_user.remote_id, - 'object': {'id': self.status.remote_id}, + 'object': {'id': self.status.remote_id, 'type': 'Tombstone'}, } views.inbox.activity_task(activity) # deletion doens't remove the status, it turns it into a tombstone @@ -453,7 +453,7 @@ class Inbox(TestCase): 'type': 'Delete', 'id': '%s/activity' % self.status.remote_id, 'actor': self.remote_user.remote_id, - 'object': {'id': self.status.remote_id}, + 'object': {'id': self.status.remote_id, 'type': 'Tombstone'}, } views.inbox.activity_task(activity) # deletion doens't remove the status, it turns it into a tombstone @@ -472,6 +472,7 @@ class Inbox(TestCase): '@context': 'https://www.w3.org/ns/activitystreams', 'id': 'https://example.com/fav/1', 'actor': 'https://example.com/users/rat', + 'type': 'Like', 'published': 'Mon, 25 May 2020 19:31:20 GMT', 'object': 'https://example.com/status/1', } @@ -492,6 +493,7 @@ class Inbox(TestCase): '@context': 'https://www.w3.org/ns/activitystreams', 'id': 'https://example.com/fav/1', 'actor': 'https://example.com/users/rat', + 'type': 'Like', 'published': 'Mon, 25 May 2020 19:31:20 GMT', 'object': 'https://example.com/fav/1', } From b393df8cab781fcd35f8e5710af5c65bc41f5bc0 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 09:35:00 -0800 Subject: [PATCH 08/42] Fixes deletion --- bookwyrm/activitypub/note.py | 6 ++++++ bookwyrm/models/status.py | 6 ++++++ bookwyrm/tests/views/test_inbox.py | 4 ++++ 3 files changed, 16 insertions(+) diff --git a/bookwyrm/activitypub/note.py b/bookwyrm/activitypub/note.py index 5da2cced..705d6eed 100644 --- a/bookwyrm/activitypub/note.py +++ b/bookwyrm/activitypub/note.py @@ -1,6 +1,7 @@ ''' note serializer and children thereof ''' from dataclasses import dataclass, field from typing import Dict, List +from django.apps import apps from .base_activity import ActivityObject, Link from .image import Image @@ -10,6 +11,11 @@ class Tombstone(ActivityObject): ''' the placeholder for a deleted status ''' type: str = 'Tombstone' + def to_model(self, *args, **kwargs): + ''' this should never really get serialized, just searched for ''' + model = apps.get_model('bookwyrm.Status') + return model.find_existing_by_remote_id(self.id) + @dataclass(init=False) class Note(ActivityObject): diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 716c1592..029ca5a4 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -84,6 +84,12 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): related_status=self, ) + def delete(self, *args, **kwargs): + ''' "delete" a status ''' + self.deleted = True + self.deleted_date = timezone.now() + self.save() + @property def recipients(self): ''' tagged users who definitely need to get this status in broadcast ''' diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index af513609..cfd0e8c0 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -422,6 +422,8 @@ class Inbox(TestCase): self.assertFalse(self.status.deleted) activity = { 'type': 'Delete', + "to": ["https://www.w3.org/ns/activitystreams#Public"], + "cc": ["https://example.com/user/mouse/followers"], 'id': '%s/activity' % self.status.remote_id, 'actor': self.remote_user.remote_id, 'object': {'id': self.status.remote_id, 'type': 'Tombstone'}, @@ -451,6 +453,8 @@ class Inbox(TestCase): self.assertEqual(models.Notification.objects.count(), 2) activity = { 'type': 'Delete', + "to": ["https://www.w3.org/ns/activitystreams#Public"], + "cc": ["https://example.com/user/mouse/followers"], 'id': '%s/activity' % self.status.remote_id, 'actor': self.remote_user.remote_id, 'object': {'id': self.status.remote_id, 'type': 'Tombstone'}, From 3f1b62eb98a1acce321f9a0b31e4ee006f48cd65 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 11:04:13 -0800 Subject: [PATCH 09/42] Fixes Add activity still janky --- bookwyrm/activitypub/base_activity.py | 51 +++++++++++++++++---------- bookwyrm/activitypub/verbs.py | 11 +++--- bookwyrm/models/fields.py | 13 ++++--- bookwyrm/tests/views/test_inbox.py | 12 ++++--- 4 files changed, 54 insertions(+), 33 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index c20ab3b8..db6cdc94 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -80,17 +80,10 @@ class ActivityObject: setattr(self, field.name, value) - def to_model(self, instance=None, allow_create=True, save=True): + def to_model(self, model=None, instance=None, allow_create=True, save=True): ''' convert from an activity to a model instance ''' # figure out the right model -- wish I had a better way for this - models = apps.get_models() - model = [m for m in models if hasattr(m, 'activity_serializer') and \ - hasattr(m.activity_serializer, 'type') and \ - m.activity_serializer.type == self.type] - if not len(model): - raise ActivitySerializerError( - 'No model found for activity type "%s"' % self.type) - model = model[0] + model = model or get_model_from_type(self.type) if hasattr(model, 'ignore_activity') and model.ignore_activity(self): return instance @@ -156,6 +149,14 @@ class ActivityObject: def serialize(self): ''' convert to dictionary with context attr ''' data = self.__dict__ + # recursively serialize + for (k, v) in data.items(): + try: + is_subclass = issubclass(v, ActivityObject) + except TypeError: + is_subclass = False + if is_subclass: + data[k] = v.serialize() data = {k:v for (k, v) in data.items() if v is not None} data['@context'] = 'https://www.w3.org/ns/activitystreams' return data @@ -207,11 +208,23 @@ def set_related_field( item.save() -def resolve_remote_id(model, remote_id, refresh=False, save=True): +def get_model_from_type(activity_type): + ''' given the activity, what type of model ''' + models = apps.get_models() + model = [m for m in models if hasattr(m, 'activity_serializer') and \ + hasattr(m.activity_serializer, 'type') and \ + m.activity_serializer.type == activity_type] + if not len(model): + raise ActivitySerializerError( + 'No model found for activity type "%s"' % activity_type) + return model[0] + +def resolve_remote_id(remote_id, model=None, refresh=False, save=True): ''' take a remote_id and return an instance, creating if necessary ''' - result = model.find_existing_by_remote_id(remote_id) - if result and not refresh: - return result + if model:# a bonus check we can do if we already know the model + result = model.find_existing_by_remote_id(remote_id) + if result and not refresh: + return result # load the data and create the object try: @@ -220,13 +233,15 @@ def resolve_remote_id(model, remote_id, refresh=False, save=True): raise ActivitySerializerError( 'Could not connect to host for remote_id in %s model: %s' % \ (model.__name__, remote_id)) + # determine the model implicitly, if not provided + if not model: + model = get_model_from_type(data.get('type')) # check for existing items with shared unique identifiers - if not result: - result = model.find_existing(data) - if result and not refresh: - return result + result = model.find_existing(data) + if result and not refresh: + return result item = model.activity_serializer(**data) # if we're refreshing, "result" will be set and we'll update it - return item.to_model(instance=result, save=save) + return item.to_model(model=model, instance=result, save=save) diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index c3de73f3..300e1bf3 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -2,7 +2,7 @@ from dataclasses import dataclass from typing import List -from .base_activity import ActivityObject, Signature +from .base_activity import ActivityObject, Signature, resolve_remote_id from .book import Edition @@ -113,19 +113,22 @@ class Reject(Verb): class Add(Verb): '''Add activity ''' target: str - object: ActivityObject + object: Edition type: str = 'Add' def action(self): ''' add obj to collection ''' - self.to_model() + target = resolve_remote_id(self.target, refresh=False) + # we want to related field that isn't the book, this is janky af sorry + model = [t for t in type(target)._meta.related_objects \ + if t.name != 'edition'][0].related_model + self.to_model(model=model) @dataclass(init=False) class AddBook(Add): '''Add activity that's aware of the book obj ''' object: Edition - type: str = 'Add' @dataclass(init=False) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 880c7600..f2465fb6 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -122,13 +122,12 @@ class ActivitypubRelatedFieldMixin(ActivitypubFieldMixin): return None related_model = self.related_model - if isinstance(value, dict) and value.get('id'): + if hasattr(value, 'id') and value.id: if not self.load_remote: # only look in the local database - return related_model.find_existing(value) + return related_model.find_existing(value.serialize()) # this is an activitypub object, which we can deserialize - activity_serializer = related_model.activity_serializer - return activity_serializer(**value).to_model() + return value.to_model(model=related_model) try: # make sure the value looks like a remote id validate_remote_id(value) @@ -139,7 +138,7 @@ class ActivitypubRelatedFieldMixin(ActivitypubFieldMixin): if not self.load_remote: # only look in the local database return related_model.find_existing_by_remote_id(value) - return activitypub.resolve_remote_id(related_model, value) + return activitypub.resolve_remote_id(value, model=related_model) class RemoteIdField(ActivitypubFieldMixin, models.CharField): @@ -280,7 +279,7 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField): except ValidationError: continue items.append( - activitypub.resolve_remote_id(self.related_model, remote_id) + activitypub.resolve_remote_id(remote_id, model=self.related_model) ) return items @@ -317,7 +316,7 @@ class TagField(ManyToManyField): # tags can contain multiple types continue items.append( - activitypub.resolve_remote_id(self.related_model, link.href) + activitypub.resolve_remote_id(link.href, model=self.related_model) ) return items diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index cfd0e8c0..e7ce4c55 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -566,10 +566,12 @@ class Inbox(TestCase): views.inbox.activity_task(activity) - def test_handle_add_book(self): + def test_handle_add_book_to_shelf(self): ''' shelving a book ''' + work = models.Work.objects.create(title='work title') book = models.Edition.objects.create( - title='Test', remote_id='https://bookwyrm.social/book/37292') + title='Test', remote_id='https://bookwyrm.social/book/37292', + parent_work=work) shelf = models.Shelf.objects.create( user=self.remote_user, name='Test Shelf') shelf.remote_id = 'https://bookwyrm.social/user/mouse/shelf/to-read' @@ -581,13 +583,15 @@ class Inbox(TestCase): "actor": "https://example.com/users/rat", "object": { "type": "Edition", + "title": "Test Title", + "work": work.remote_id, "id": "https://bookwyrm.social/book/37292", }, "target": "https://bookwyrm.social/user/mouse/shelf/to-read", "@context": "https://www.w3.org/ns/activitystreams" } - #views.inbox.activity_task(activity) - #self.assertEqual(shelf.books.first(), book) + views.inbox.activity_task(activity) + self.assertEqual(shelf.books.first(), book) def test_handle_update_user(self): From f5a022184f71ca84db0d007b80f7f79afe0c6449 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 12:31:27 -0800 Subject: [PATCH 10/42] Fixes discarding boosts --- bookwyrm/models/activitypub_mixin.py | 8 ++++++-- bookwyrm/models/status.py | 8 +++++++- bookwyrm/tests/views/test_inbox.py | 13 +++++++++---- 3 files changed, 22 insertions(+), 7 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 84293725..6ecf8d96 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -156,10 +156,14 @@ class ActivitypubMixin: return recipients - def to_activity(self): + def to_activity_dataclass(self): ''' convert from a model to an activity ''' activity = generate_activity(self) - return self.activity_serializer(**activity).serialize() + return self.activity_serializer(**activity) + + def to_activity(self): + ''' convert from a model to a json activity ''' + return self.to_activity_dataclass().serialize() class ObjectMixin(ActivitypubMixin): diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 029ca5a4..d2dd7d5b 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -102,6 +102,12 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): @classmethod def ignore_activity(cls, activity): ''' keep notes if they are replies to existing statuses ''' + if activity.type == 'Announce': + # keep it if the booster or the boosted are local + boosted = activitypub.resolve_remote_id(activity.object, save=False) + return cls.ignore_activity(boosted.to_activity_dataclass()) + + # keep if it if it's a custom type if activity.type != 'Note': return False if cls.objects.filter( @@ -112,8 +118,8 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): if activity.tag == MISSING or activity.tag is None: return True tags = [l['href'] for l in activity.tag if l['type'] == 'Mention'] + user_model = apps.get_model('bookwyrm.User', require_ready=True) for tag in tags: - user_model = apps.get_model('bookwyrm.User', require_ready=True) if user_model.objects.filter( remote_id=tag, local=True).exists(): # we found a mention of a known use boost diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index e7ce4c55..288dd056 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -519,7 +519,7 @@ class Inbox(TestCase): 'type': 'Announce', 'id': '%s/boost' % self.status.remote_id, 'actor': self.remote_user.remote_id, - 'object': self.status.to_activity(), + 'object': self.status.remote_id, } with patch('bookwyrm.models.status.Status.ignore_activity') \ as discarder: @@ -535,16 +535,21 @@ class Inbox(TestCase): @responses.activate def test_handle_discarded_boost(self): ''' test a boost of a mastodon status that will be discarded ''' + status = models.Status( + content='hi', + user=self.remote_user, + ) + status.save(broadcast=False) activity = { 'type': 'Announce', 'id': 'http://www.faraway.com/boost/12', 'actor': self.remote_user.remote_id, - 'object': self.status.to_activity(), + 'object': status.remote_id, } responses.add( responses.GET, - 'http://www.faraway.com/boost/12', - json={'id': 'http://www.faraway.com/boost/12'}, + status.remote_id, + json=status.to_activity(), status=200) views.inbox.activity_task(activity) self.assertEqual(models.Boost.objects.count(), 0) From b57a86d4e23232de5d59ab61319b3d77eefed744 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 12:58:29 -0800 Subject: [PATCH 11/42] Fixes approving follow requests automatically --- bookwyrm/models/relationship.py | 2 +- bookwyrm/tests/views/test_inbox.py | 30 +++++++++++++++--------------- bookwyrm/views/inbox.py | 2 +- 3 files changed, 17 insertions(+), 17 deletions(-) diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index e4c6f4fa..cdeebc86 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -100,7 +100,7 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship): if self.user_object.local: manually_approves = self.user_object.manually_approves_followers - if manually_approves: + if not manually_approves: self.accept() model = apps.get_model('bookwyrm.Notification', require_ready=True) diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 288dd056..d2e28380 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -246,8 +246,9 @@ class Inbox(TestCase): "object": "https://example.com/user/mouse" } - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay') as mock: views.inbox.activity_task(activity) + self.assertEqual(mock.call_count, 1) # notification created notification = models.Notification.objects.get() @@ -255,8 +256,7 @@ class Inbox(TestCase): self.assertEqual(notification.notification_type, 'FOLLOW') # the request should have been deleted - requests = models.UserFollowRequest.objects.all() - self.assertEqual(list(requests), []) + self.assertFalse(models.UserFollowRequest.objects.exists()) # the follow relationship should exist follow = models.UserFollows.objects.get(user_object=self.local_user) @@ -317,24 +317,24 @@ class Inbox(TestCase): def test_handle_follow_accept(self): ''' a remote user approved a follow request from local ''' + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + rel = models.UserFollowRequest.objects.create( + user_subject=self.local_user, + user_object=self.remote_user + ) activity = { "@context": "https://www.w3.org/ns/activitystreams", "id": "https://example.com/users/rat/follows/123#accepts", "type": "Accept", "actor": "https://example.com/users/rat", "object": { - "id": "https://example.com/users/rat/follows/123", + "id": rel.remote_id, "type": "Follow", "actor": "https://example.com/user/mouse", "object": "https://example.com/users/rat" } } - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - models.UserFollowRequest.objects.create( - user_subject=self.local_user, - user_object=self.remote_user - ) self.assertEqual(models.UserFollowRequest.objects.count(), 1) views.inbox.activity_task(activity) @@ -350,24 +350,24 @@ class Inbox(TestCase): def test_handle_follow_reject(self): ''' turn down a follow request ''' + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + rel = models.UserFollowRequest.objects.create( + user_subject=self.local_user, + user_object=self.remote_user + ) activity = { "@context": "https://www.w3.org/ns/activitystreams", "id": "https://example.com/users/rat/follows/123#accepts", "type": "Reject", "actor": "https://example.com/users/rat", "object": { - "id": "https://example.com/users/rat/follows/123", + "id": rel.remote_id, "type": "Follow", "actor": "https://example.com/user/mouse", "object": "https://example.com/users/rat" } } - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - models.UserFollowRequest.objects.create( - user_subject=self.local_user, - user_object=self.remote_user - ) self.assertEqual(models.UserFollowRequest.objects.count(), 1) views.inbox.activity_task(activity) diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 58356e1c..6e7b036b 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -56,7 +56,7 @@ def activity_task(activity_json): try: activity = activitypub.parse(activity_json) except activitypub.ActivitySerializerError: - raise#return + return # cool that worked, now we should do the action described by the type # (create, update, delete, etc) From d81bfb6573b8bca275e9dea578615d29abfe9fdc Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 16:35:28 -0800 Subject: [PATCH 12/42] Fixes unfollow --- bookwyrm/activitypub/base_activity.py | 7 ++-- bookwyrm/activitypub/verbs.py | 8 ++++- bookwyrm/models/relationship.py | 2 +- bookwyrm/tests/views/test_inbox.py | 49 ++++++++++++++++++--------- bookwyrm/views/inbox.py | 2 +- 5 files changed, 47 insertions(+), 21 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index db6cdc94..008d3087 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -85,11 +85,14 @@ class ActivityObject: # figure out the right model -- wish I had a better way for this model = model or get_model_from_type(self.type) - if hasattr(model, 'ignore_activity') and model.ignore_activity(self): - return instance + # only reject statuses if we're potentially creating them + if allow_create and \ + hasattr(model, 'ignore_activity') and model.ignore_activity(self): + return None # check for an existing instance instance = instance or model.find_existing(self.serialize()) + if not instance and not allow_create: # so that we don't create when we want to delete or update return None diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index 300e1bf3..aa6e7eee 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -1,6 +1,7 @@ ''' undo wrapper activity ''' from dataclasses import dataclass from typing import List +from django.apps import apps from .base_activity import ActivityObject, Signature, resolve_remote_id from .book import Edition @@ -59,7 +60,12 @@ class Undo(Verb): def action(self): ''' find and remove the activity object ''' - obj = self.object.to_model(save=False, allow_create=False) + # this is so hacky but it does make it work.... + # (because you Reject a request and Undo a follow + model = None + if self.object.type == 'Follow': + model = apps.get_model('bookwyrm.UserFollows') + obj = self.object.to_model(model=model, save=False, allow_create=False) obj.delete() diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index cdeebc86..722f0fac 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -56,7 +56,7 @@ class UserRelationship(BookWyrmModel): return '%s#%s/%d' % (base_path, status, self.id) -class UserFollows(UserRelationship): +class UserFollows(ActivitypubMixin, UserRelationship): ''' Following a user ''' status = 'follows' diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index d2e28380..2e677cbf 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -42,7 +42,7 @@ class Inbox(TestCase): 'type': 'Create', 'actor': 'hi', "to": [ - "https://www.w3.org/ns/activitystreams#Public" + "https://www.w3.org/ns/activitystreams#public" ], "cc": [ "https://example.com/user/mouse/followers" @@ -296,19 +296,23 @@ class Inbox(TestCase): def test_handle_unfollow(self): ''' remove a relationship ''' + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + rel = models.UserFollows.objects.create( + user_subject=self.remote_user, user_object=self.local_user) activity = { "type": "Undo", + "id": "bleh", + "to": ["https://www.w3.org/ns/activitystreams#Public"], + "cc": ["https://example.com/user/mouse/followers"], + 'actor': self.remote_user.remote_id, "@context": "https://www.w3.org/ns/activitystreams", "object": { - "id": "https://example.com/users/rat/follows/123", + "id": rel.remote_id, "type": "Follow", "actor": "https://example.com/users/rat", "object": "https://example.com/user/mouse" } } - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - models.UserFollows.objects.create( - user_subject=self.remote_user, user_object=self.local_user) self.assertEqual(self.remote_user, self.local_user.followers.first()) views.inbox.activity_task(activity) @@ -493,13 +497,16 @@ class Inbox(TestCase): activity = { 'id': 'https://example.com/fav/1#undo', 'type': 'Undo', + "to": ["https://www.w3.org/ns/activitystreams#Public"], + "cc": ["https://example.com/user/mouse/followers"], + 'actor': self.remote_user.remote_id, 'object': { '@context': 'https://www.w3.org/ns/activitystreams', 'id': 'https://example.com/fav/1', 'actor': 'https://example.com/users/rat', 'type': 'Like', 'published': 'Mon, 25 May 2020 19:31:20 GMT', - 'object': 'https://example.com/fav/1', + 'object': self.status.remote_id, } } models.Favorite.objects.create( @@ -557,17 +564,21 @@ class Inbox(TestCase): def test_handle_unboost(self): ''' undo a boost ''' + boost = models.Boost.objects.create( + boosted_status=self.status, user=self.remote_user) activity = { 'type': 'Undo', + 'actor': 'hi', + 'id': 'bleh', + "to": ["https://www.w3.org/ns/activitystreams#public"], + "cc": ["https://example.com/user/mouse/followers"], 'object': { 'type': 'Announce', - 'id': '%s/boost' % self.status.remote_id, + 'id': boost.remote_id, 'actor': self.local_user.remote_id, - 'object': self.status.to_activity(), + 'object': self.status.remote_id, } } - models.Boost.objects.create( - boosted_status=self.status, user=self.remote_user) views.inbox.activity_task(activity) @@ -695,12 +706,18 @@ class Inbox(TestCase): self.assertEqual(block.user_subject, self.remote_user) self.assertEqual(block.user_object, self.local_user) - activity = {'type': 'Undo', 'object': { - "@context": "https://www.w3.org/ns/activitystreams", - "id": "https://example.com/9e1f41ac-9ddd-4159", - "type": "Block", - "actor": "https://example.com/users/rat", - "object": "https://example.com/user/mouse" + activity = { + 'type': 'Undo', + 'actor': 'hi', + 'id': 'bleh', + "to": ["https://www.w3.org/ns/activitystreams#public"], + "cc": ["https://example.com/user/mouse/followers"], + 'object': { + "@context": "https://www.w3.org/ns/activitystreams", + "id": "https://example.com/9e1f41ac-9ddd-4159", + "type": "Block", + "actor": "https://example.com/users/rat", + "object": "https://example.com/user/mouse" }} views.inbox.activity_task(activity) self.assertFalse(models.UserBlocks.objects.exists()) diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 6e7b036b..58356e1c 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -56,7 +56,7 @@ def activity_task(activity_json): try: activity = activitypub.parse(activity_json) except activitypub.ActivitySerializerError: - return + raise#return # cool that worked, now we should do the action described by the type # (create, update, delete, etc) From 714202986dc859322764f749d32dfd00c06fe042 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 17:47:53 -0800 Subject: [PATCH 13/42] Fixes person/author confusion and public keys --- bookwyrm/activitypub/base_activity.py | 4 +++- bookwyrm/activitypub/book.py | 2 +- bookwyrm/tests/views/test_inbox.py | 30 +++++++++++++++++++++------ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 008d3087..5c828858 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -42,6 +42,9 @@ class Signature: def naive_parse(activity_objects, activity_json): ''' this navigates circular import issues ''' + if activity_json.get('publicKeyPem'): + # ugh + activity_json['type'] = 'PublicKey' try: activity_type = activity_json['type'] serializer = activity_objects[activity_type] @@ -82,7 +85,6 @@ class ActivityObject: def to_model(self, model=None, instance=None, allow_create=True, save=True): ''' convert from an activity to a model instance ''' - # figure out the right model -- wish I had a better way for this model = model or get_model_from_type(self.type) # only reject statuses if we're potentially creating them diff --git a/bookwyrm/activitypub/book.py b/bookwyrm/activitypub/book.py index 68036559..87c40c90 100644 --- a/bookwyrm/activitypub/book.py +++ b/bookwyrm/activitypub/book.py @@ -67,4 +67,4 @@ class Author(ActivityObject): librarythingKey: str = '' goodreadsKey: str = '' wikipediaLink: str = '' - type: str = 'Person' + type: str = 'Author' diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 2e677cbf..0251d8a2 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -391,6 +391,9 @@ class Inbox(TestCase): name='hi', remote_id='https://example.com/list/22', user=self.local_user) activity = { + 'type': 'Update', + 'to': [], 'cc': [], 'actor': 'hi', + 'id': 'sdkjf', 'object': { "id": "https://example.com/list/22", "type": "BookList", @@ -617,11 +620,16 @@ class Inbox(TestCase): self.local_user.save() datafile = pathlib.Path(__file__).parent.joinpath( - 'data/ap_user.json') + '../data/ap_user.json') userdata = json.loads(datafile.read_bytes()) del userdata['icon'] self.assertIsNone(self.local_user.name) - views.inbox.activity_task({'object': userdata}) + views.inbox.activity_task({ + 'type': 'Update', + 'to': [], 'cc': [], 'actor': 'hi', + 'id': 'sdkjf', + 'object': userdata + }) user = models.User.objects.get(id=self.local_user.id) self.assertEqual(user.name, 'MOUSE?? MOUSE!!') self.assertEqual(user.username, 'mouse@example.com') @@ -631,7 +639,7 @@ class Inbox(TestCase): def test_handle_update_edition(self): ''' update an existing edition ''' datafile = pathlib.Path(__file__).parent.joinpath( - 'data/bw_edition.json') + '../data/bw_edition.json') bookdata = json.loads(datafile.read_bytes()) models.Work.objects.create( @@ -644,7 +652,12 @@ class Inbox(TestCase): with patch( 'bookwyrm.activitypub.base_activity.set_related_field.delay'): - views.inbox.activity_task({'object': bookdata}) + views.inbox.activity_task({ + 'type': 'Update', + 'to': [], 'cc': [], 'actor': 'hi', + 'id': 'sdkjf', + 'object': bookdata + }) book = models.Edition.objects.get(id=book.id) self.assertEqual(book.title, 'Piranesi') @@ -652,7 +665,7 @@ class Inbox(TestCase): def test_handle_update_work(self): ''' update an existing edition ''' datafile = pathlib.Path(__file__).parent.joinpath( - 'data/bw_work.json') + '../data/bw_work.json') bookdata = json.loads(datafile.read_bytes()) book = models.Work.objects.create( @@ -662,7 +675,12 @@ class Inbox(TestCase): self.assertEqual(book.title, 'Test Book') with patch( 'bookwyrm.activitypub.base_activity.set_related_field.delay'): - views.inbox.activity_task({'object': bookdata}) + views.inbox.activity_task({ + 'type': 'Update', + 'to': [], 'cc': [], 'actor': 'hi', + 'id': 'sdkjf', + 'object': bookdata + }) book = models.Work.objects.get(id=book.id) self.assertEqual(book.title, 'Piranesi') From a3b7063e4bca4aed6f3bf49460d5073b87d55a6f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 18:07:57 -0800 Subject: [PATCH 14/42] makes inbox csrf exempt --- bookwyrm/tests/test_signing.py | 2 +- bookwyrm/views/inbox.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 0d55893d..3ad5d233 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -76,7 +76,7 @@ class Signature(TestCase): digest = digest or make_digest(data) signature = make_signature( signer or sender, self.rat.inbox, now, digest) - with patch('bookwyrm.incoming.handle_follow.delay'): + with patch('bookwyrm.views.inbox.activity_task.delay'): with patch('bookwyrm.models.user.set_remote_server.delay'): return self.send(signature, now, send_data or data, digest) diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 58356e1c..b4ff2736 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -4,7 +4,9 @@ from urllib.parse import urldefrag from django.http import HttpResponse from django.http import HttpResponseBadRequest, HttpResponseNotFound +from django.utils.decorators import method_decorator from django.views import View +from django.views.decorators.csrf import csrf_exempt import requests from bookwyrm import activitypub, models @@ -12,6 +14,7 @@ from bookwyrm.tasks import app from bookwyrm.signatures import Signature +@method_decorator(csrf_exempt, name='dispatch') # pylint: disable=no-self-use class Inbox(View): ''' requests sent by outside servers''' @@ -56,7 +59,7 @@ def activity_task(activity_json): try: activity = activitypub.parse(activity_json) except activitypub.ActivitySerializerError: - raise#return + return # cool that worked, now we should do the action described by the type # (create, update, delete, etc) From 91908eb1b6d7fa319e14992978c0efcd045fc029 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 18:59:26 -0800 Subject: [PATCH 15/42] Smarter way of inferring serializers (which are explicitly present) --- bookwyrm/activitypub/base_activity.py | 30 +++++++++++++++------------ bookwyrm/tests/test_signing.py | 29 +++++++++++++++++--------- bookwyrm/views/inbox.py | 5 +++-- 3 files changed, 39 insertions(+), 25 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 5c828858..7ee8ca45 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -40,16 +40,17 @@ class Signature: signatureValue: str type: str = 'RsaSignature2017' -def naive_parse(activity_objects, activity_json): +def naive_parse(activity_objects, activity_json, serializer=None): ''' this navigates circular import issues ''' - if activity_json.get('publicKeyPem'): - # ugh - activity_json['type'] = 'PublicKey' - try: - activity_type = activity_json['type'] - serializer = activity_objects[activity_type] - except KeyError as e: - raise ActivitySerializerError(e) + if not serializer: + if activity_json.get('publicKeyPem'): + # ugh + activity_json['type'] = 'PublicKey' + try: + activity_type = activity_json['type'] + serializer = activity_objects[activity_type] + except KeyError as e: + raise ActivitySerializerError(e) return serializer(activity_objects=activity_objects, **activity_json) @@ -71,8 +72,9 @@ class ActivityObject: is_subclass = issubclass(field.type, ActivityObject) except TypeError: is_subclass = False - if is_subclass and activity_objects: - value = naive_parse(activity_objects, value) + if is_subclass: + value = naive_parse( + activity_objects, value, serializer=field.type) except KeyError: if field.default == MISSING and \ @@ -89,7 +91,8 @@ class ActivityObject: # only reject statuses if we're potentially creating them if allow_create and \ - hasattr(model, 'ignore_activity') and model.ignore_activity(self): + hasattr(model, 'ignore_activity') and \ + model.ignore_activity(self): return None # check for an existing instance @@ -219,11 +222,12 @@ def get_model_from_type(activity_type): model = [m for m in models if hasattr(m, 'activity_serializer') and \ hasattr(m.activity_serializer, 'type') and \ m.activity_serializer.type == activity_type] - if not len(model): + if not model: raise ActivitySerializerError( 'No model found for activity type "%s"' % activity_type) return model[0] + def resolve_remote_id(remote_id, model=None, refresh=False, save=True): ''' take a remote_id and return an instance, creating if necessary ''' if model:# a bonus check we can do if we already know the model diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 3ad5d233..f6de11e1 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -1,3 +1,4 @@ +''' getting and verifying signatures ''' import time from collections import namedtuple from urllib.parse import urlsplit @@ -12,31 +13,33 @@ import pytest from django.test import TestCase, Client from django.utils.http import http_date -from bookwyrm.models import User +from bookwyrm import models from bookwyrm.activitypub import Follow from bookwyrm.settings import DOMAIN from bookwyrm.signatures import create_key_pair, make_signature, make_digest -def get_follow_data(follower, followee): - follow_activity = Follow( +def get_follow_activity(follower, followee): + ''' generates a test activity ''' + return Follow( id='https://test.com/user/follow/id', actor=follower.remote_id, object=followee.remote_id, ).serialize() - return json.dumps(follow_activity) KeyPair = namedtuple('KeyPair', ('private_key', 'public_key')) Sender = namedtuple('Sender', ('remote_id', 'key_pair')) class Signature(TestCase): + ''' signature test ''' def setUp(self): - self.mouse = User.objects.create_user( + ''' create users and test data ''' + self.mouse = models.User.objects.create_user( 'mouse@%s' % DOMAIN, 'mouse@example.com', '', local=True, localname='mouse') - self.rat = User.objects.create_user( + self.rat = models.User.objects.create_user( 'rat@%s' % DOMAIN, 'rat@example.com', '', local=True, localname='rat') - self.cat = User.objects.create_user( + self.cat = models.User.objects.create_user( 'cat@%s' % DOMAIN, 'cat@example.com', '', local=True, localname='cat') @@ -47,6 +50,8 @@ class Signature(TestCase): KeyPair(private_key, public_key) ) + models.SiteSettings.objects.create() + def send(self, signature, now, data, digest): ''' test request ''' c = Client() @@ -63,7 +68,7 @@ class Signature(TestCase): } ) - def send_test_request( + def send_test_request(#pylint: disable=too-many-arguments self, sender, signer=None, @@ -72,7 +77,7 @@ class Signature(TestCase): date=None): ''' sends a follow request to the "rat" user ''' now = date or http_date() - data = json.dumps(get_follow_data(sender, self.rat)) + data = json.dumps(get_follow_activity(sender, self.rat)) digest = digest or make_digest(data) signature = make_signature( signer or sender, self.rat.inbox, now, digest) @@ -81,6 +86,7 @@ class Signature(TestCase): return self.send(signature, now, send_data or data, digest) def test_correct_signature(self): + ''' this one should just work ''' response = self.send_test_request(sender=self.mouse) self.assertEqual(response.status_code, 200) @@ -120,6 +126,7 @@ class Signature(TestCase): @responses.activate def test_key_needs_refresh(self): + ''' an out of date key should be updated and the new key work ''' datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') data = json.loads(datafile.read_bytes()) data['id'] = self.fake_remote.remote_id @@ -165,6 +172,7 @@ class Signature(TestCase): @responses.activate def test_nonexistent_signer(self): + ''' fail when unable to look up signer ''' responses.add( responses.GET, self.fake_remote.remote_id, @@ -180,11 +188,12 @@ class Signature(TestCase): with patch('bookwyrm.activitypub.resolve_remote_id'): response = self.send_test_request( self.mouse, - send_data=get_follow_data(self.mouse, self.cat)) + send_data=get_follow_activity(self.mouse, self.cat)) self.assertEqual(response.status_code, 401) @pytest.mark.integration def test_invalid_digest(self): + ''' signature digest must be valid ''' with patch('bookwyrm.activitypub.resolve_remote_id'): response = self.send_test_request( self.mouse, diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index b4ff2736..4da4e5b6 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -75,7 +75,8 @@ def has_valid_signature(request, activity): if key_actor != activity.get('actor'): raise ValueError("Wrong actor created signature.") - remote_user = activitypub.resolve_remote_id(models.User, key_actor) + remote_user = activitypub.resolve_remote_id( + key_actor, model=models.User) if not remote_user: return False @@ -84,7 +85,7 @@ def has_valid_signature(request, activity): except ValueError: old_key = remote_user.key_pair.public_key remote_user = activitypub.resolve_remote_id( - models.User, remote_user.remote_id, refresh=True + remote_user.remote_id, model=models.User, refresh=True ) if remote_user.key_pair.public_key == old_key: raise # Key unchanged. From e2f921b7f510f12856436bec203f3d92e11e5900 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 19:28:23 -0800 Subject: [PATCH 16/42] better checking for empty values --- bookwyrm/activitypub/base_activity.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 7ee8ca45..933745e0 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -68,6 +68,8 @@ class ActivityObject: for field in fields(self): try: value = kwargs[field.name] + if value in (None, MISSING): + raise KeyError() try: is_subclass = issubclass(field.type, ActivityObject) except TypeError: From 3f61675a0aa9a7546d9aee3aeb240931785e4d28 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 19:35:43 -0800 Subject: [PATCH 17/42] Updates usage of resolve_remote_id --- bookwyrm/connectors/bookwyrm_connector.py | 2 +- bookwyrm/models/fields.py | 6 ++++-- bookwyrm/models/status.py | 2 +- bookwyrm/tests/activitypub/test_base_activity.py | 4 ++-- bookwyrm/views/helpers.py | 2 +- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/bookwyrm/connectors/bookwyrm_connector.py b/bookwyrm/connectors/bookwyrm_connector.py index 1f877993..00e6c62f 100644 --- a/bookwyrm/connectors/bookwyrm_connector.py +++ b/bookwyrm/connectors/bookwyrm_connector.py @@ -7,7 +7,7 @@ class Connector(AbstractMinimalConnector): ''' this is basically just for search ''' def get_or_create_book(self, remote_id): - edition = activitypub.resolve_remote_id(models.Edition, remote_id) + edition = activitypub.resolve_remote_id(remote_id, model=models.Edition) work = edition.parent_work work.default_edition = work.get_default_edition() work.save() diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index f2465fb6..029958bd 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -279,7 +279,8 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField): except ValidationError: continue items.append( - activitypub.resolve_remote_id(remote_id, model=self.related_model) + activitypub.resolve_remote_id( + remote_id, model=self.related_model) ) return items @@ -316,7 +317,8 @@ class TagField(ManyToManyField): # tags can contain multiple types continue items.append( - activitypub.resolve_remote_id(link.href, model=self.related_model) + activitypub.resolve_remote_id( + link.href, model=self.related_model) ) return items diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index d2dd7d5b..fbc94e9d 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -84,7 +84,7 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): related_status=self, ) - def delete(self, *args, **kwargs): + def delete(self, *args, **kwargs):#pylint: disable=unused-argument ''' "delete" a status ''' self.deleted = True self.deleted_date = timezone.now() diff --git a/bookwyrm/tests/activitypub/test_base_activity.py b/bookwyrm/tests/activitypub/test_base_activity.py index e84d7674..1d3e85d3 100644 --- a/bookwyrm/tests/activitypub/test_base_activity.py +++ b/bookwyrm/tests/activitypub/test_base_activity.py @@ -79,7 +79,7 @@ class BaseActivity(TestCase): def test_resolve_remote_id(self): ''' look up or load remote data ''' # existing item - result = resolve_remote_id(models.User, 'http://example.com/a/b') + result = resolve_remote_id('http://example.com/a/b', model=models.User) self.assertEqual(result, self.user) # remote item @@ -91,7 +91,7 @@ class BaseActivity(TestCase): with patch('bookwyrm.models.user.set_remote_server.delay'): result = resolve_remote_id( - models.User, 'https://example.com/user/mouse') + 'https://example.com/user/mouse', model=models.User) self.assertIsInstance(result, models.User) self.assertEqual(result.remote_id, 'https://example.com/user/mouse') self.assertEqual(result.name, 'MOUSE?? MOUSE!!') diff --git a/bookwyrm/views/helpers.py b/bookwyrm/views/helpers.py index 842b8d1c..89d99501 100644 --- a/bookwyrm/views/helpers.py +++ b/bookwyrm/views/helpers.py @@ -162,7 +162,7 @@ def handle_remote_webfinger(query): if link.get('rel') == 'self': try: user = activitypub.resolve_remote_id( - models.User, link['href'] + link['href'], model=models.User ) except KeyError: return None From a9ca3a4290015d9c50f26dcaaf869d91fa82e156 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 20:17:38 -0800 Subject: [PATCH 18/42] Fixes calls to to_model, init with activitypub partially serialized --- bookwyrm/activitypub/base_activity.py | 3 ++- bookwyrm/connectors/abstract_connector.py | 6 ++--- bookwyrm/models/fields.py | 4 ++-- .../tests/activitypub/test_base_activity.py | 24 ++++++++++++------- bookwyrm/tests/activitypub/test_person.py | 2 +- bookwyrm/tests/activitypub/test_quotation.py | 2 +- 6 files changed, 24 insertions(+), 17 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 933745e0..7292a0d7 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -74,7 +74,8 @@ class ActivityObject: is_subclass = issubclass(field.type, ActivityObject) except TypeError: is_subclass = False - if is_subclass: + # parse a dict into the appropriate activity + if is_subclass and isinstance(value, dict): value = naive_parse( activity_objects, value, serializer=field.type) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index 527d2f42..61f553a4 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -127,7 +127,7 @@ class AbstractConnector(AbstractMinimalConnector): # create activitypub object work_activity = activitypub.Work(**work_data) # this will dedupe automatically - work = work_activity.to_model(models.Work) + work = work_activity.to_model(model=models.Work) for author in self.get_authors_from_data(data): work.authors.add(author) @@ -141,7 +141,7 @@ class AbstractConnector(AbstractMinimalConnector): mapped_data = dict_from_mappings(edition_data, self.book_mappings) mapped_data['work'] = work.remote_id edition_activity = activitypub.Edition(**mapped_data) - edition = edition_activity.to_model(models.Edition) + edition = edition_activity.to_model(model=models.Edition) edition.connector = self.connector edition.save() @@ -168,7 +168,7 @@ class AbstractConnector(AbstractMinimalConnector): mapped_data = dict_from_mappings(data, self.author_mappings) activity = activitypub.Author(**mapped_data) # this will dedupe - return activity.to_model(models.Author) + return activity.to_model(model=models.Author) @abstractmethod diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index 029958bd..4ea527eb 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -367,8 +367,8 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): image_slug = value # when it's an inline image (User avatar/icon, Book cover), it's a json # blob, but when it's an attached image, it's just a url - if isinstance(image_slug, dict): - url = image_slug.get('url') + if hasattr(image_slug, 'url'): + url = image_slug.url elif isinstance(image_slug, str): url = image_slug else: diff --git a/bookwyrm/tests/activitypub/test_base_activity.py b/bookwyrm/tests/activitypub/test_base_activity.py index 1d3e85d3..ce294c1c 100644 --- a/bookwyrm/tests/activitypub/test_base_activity.py +++ b/bookwyrm/tests/activitypub/test_base_activity.py @@ -100,7 +100,7 @@ class BaseActivity(TestCase): ''' catch mismatch between activity type and model type ''' instance = ActivityObject(id='a', type='b') with self.assertRaises(ActivitySerializerError): - instance.to_model(models.User) + instance.to_model(model=models.User) def test_to_model_simple_fields(self): ''' test setting simple fields ''' @@ -118,7 +118,7 @@ class BaseActivity(TestCase): endpoints={}, ) - activity.to_model(models.User, self.user) + activity.to_model(model=models.User, instance=self.user) self.assertEqual(self.user.name, 'New Name') @@ -136,9 +136,9 @@ class BaseActivity(TestCase): endpoints={}, ) - activity.publicKey['publicKeyPem'] = 'hi im secure' + activity.publicKey.publicKeyPem = 'hi im secure' - activity.to_model(models.User, self.user) + activity.to_model(model=models.User, instance=self.user) self.assertEqual(self.user.key_pair.public_key, 'hi im secure') @responses.activate @@ -152,9 +152,15 @@ class BaseActivity(TestCase): outbox='http://www.com/', followers='', summary='', - publicKey=None, + publicKey={ + 'id': 'hi', + 'owner': self.user.remote_id, + 'publicKeyPem': 'hi'}, endpoints={}, - icon={'url': 'http://www.example.com/image.jpg'} + icon={ + 'type': 'Image', + 'url': 'http://www.example.com/image.jpg' + } ) responses.add( @@ -169,7 +175,7 @@ class BaseActivity(TestCase): # this would trigger a broadcast because it's a local user with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): - activity.to_model(models.User, self.user) + activity.to_model(model=models.User, instance=self.user) self.assertIsNotNone(self.user.avatar.name) self.assertIsNotNone(self.user.avatar.file) @@ -202,7 +208,7 @@ class BaseActivity(TestCase): }, ] ) - update_data.to_model(models.Status, instance=status) + update_data.to_model(model=models.Status, instance=status) self.assertEqual(status.mention_users.first(), self.user) self.assertEqual(status.mention_books.first(), book) @@ -239,7 +245,7 @@ class BaseActivity(TestCase): # sets the celery task call to the function call with patch( 'bookwyrm.activitypub.base_activity.set_related_field.delay'): - update_data.to_model(models.Status, instance=status) + update_data.to_model(model=models.Status, instance=status) self.assertIsNone(status.attachments.first()) diff --git a/bookwyrm/tests/activitypub/test_person.py b/bookwyrm/tests/activitypub/test_person.py index c7a8221c..7548ed93 100644 --- a/bookwyrm/tests/activitypub/test_person.py +++ b/bookwyrm/tests/activitypub/test_person.py @@ -25,7 +25,7 @@ class Person(TestCase): def test_user_to_model(self): activity = activitypub.Person(**self.user_data) with patch('bookwyrm.models.user.set_remote_server.delay'): - user = activity.to_model(models.User) + user = activity.to_model(models=models.User) self.assertEqual(user.username, 'mouse@example.com') self.assertEqual(user.remote_id, 'https://example.com/user/mouse') self.assertFalse(user.local) diff --git a/bookwyrm/tests/activitypub/test_quotation.py b/bookwyrm/tests/activitypub/test_quotation.py index 60920889..1cd1f05d 100644 --- a/bookwyrm/tests/activitypub/test_quotation.py +++ b/bookwyrm/tests/activitypub/test_quotation.py @@ -46,7 +46,7 @@ class Quotation(TestCase): def test_activity_to_model(self): ''' create a model instance from an activity object ''' activity = activitypub.Quotation(**self.status_data) - quotation = activity.to_model(models.Quotation) + quotation = activity.to_model(model=models.Quotation) self.assertEqual(quotation.book, self.book) self.assertEqual(quotation.user, self.user) From 77781d57c3e6606e6841c340ede70a9121c34dd0 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 20:24:37 -0800 Subject: [PATCH 19/42] Fixes base activity tests --- bookwyrm/activitypub/base_activity.py | 5 ++- .../tests/activitypub/test_base_activity.py | 41 +------------------ 2 files changed, 6 insertions(+), 40 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 7292a0d7..a83021ad 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -107,7 +107,10 @@ class ActivityObject: instance = instance or model() for field in instance.simple_fields: - field.set_field_from_activity(instance, self) + try: + field.set_field_from_activity(instance, self) + except AttributeError as e: + raise ActivitySerializerError(e) # image fields have to be set after other fields because they can save # too early and jank up users diff --git a/bookwyrm/tests/activitypub/test_base_activity.py b/bookwyrm/tests/activitypub/test_base_activity.py index ce294c1c..d489fdaa 100644 --- a/bookwyrm/tests/activitypub/test_base_activity.py +++ b/bookwyrm/tests/activitypub/test_base_activity.py @@ -102,44 +102,6 @@ class BaseActivity(TestCase): with self.assertRaises(ActivitySerializerError): instance.to_model(model=models.User) - def test_to_model_simple_fields(self): - ''' test setting simple fields ''' - self.assertIsNone(self.user.name) - - activity = activitypub.Person( - id=self.user.remote_id, - name='New Name', - preferredUsername='mouse', - inbox='http://www.com/', - outbox='http://www.com/', - followers='', - summary='', - publicKey=None, - endpoints={}, - ) - - activity.to_model(model=models.User, instance=self.user) - - self.assertEqual(self.user.name, 'New Name') - - def test_to_model_foreign_key(self): - ''' test setting one to one/foreign key ''' - activity = activitypub.Person( - id=self.user.remote_id, - name='New Name', - preferredUsername='mouse', - inbox='http://www.com/', - outbox='http://www.com/', - followers='', - summary='', - publicKey=self.user.key_pair.to_activity(), - endpoints={}, - ) - - activity.publicKey.publicKeyPem = 'hi im secure' - - activity.to_model(model=models.User, instance=self.user) - self.assertEqual(self.user.key_pair.public_key, 'hi im secure') @responses.activate def test_to_model_image(self): @@ -176,8 +138,9 @@ class BaseActivity(TestCase): # this would trigger a broadcast because it's a local user with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): activity.to_model(model=models.User, instance=self.user) - self.assertIsNotNone(self.user.avatar.name) self.assertIsNotNone(self.user.avatar.file) + self.assertEqual(self.user.name, 'New Name') + self.assertEqual(self.user.key_pair.public_key, 'hi') def test_to_model_many_to_many(self): ''' annoying that these all need special handling ''' From 29df2e0fac58b86c7d049fbd594bf08c9cbf1902 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 16 Feb 2021 20:26:51 -0800 Subject: [PATCH 20/42] fixes typo in person test --- bookwyrm/tests/activitypub/test_person.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/activitypub/test_person.py b/bookwyrm/tests/activitypub/test_person.py index 7548ed93..06240281 100644 --- a/bookwyrm/tests/activitypub/test_person.py +++ b/bookwyrm/tests/activitypub/test_person.py @@ -25,7 +25,7 @@ class Person(TestCase): def test_user_to_model(self): activity = activitypub.Person(**self.user_data) with patch('bookwyrm.models.user.set_remote_server.delay'): - user = activity.to_model(models=models.User) + user = activity.to_model(model=models.User) self.assertEqual(user.username, 'mouse@example.com') self.assertEqual(user.remote_id, 'https://example.com/user/mouse') self.assertFalse(user.local) From 7b27f98e2045cf442dcfb9c755fbd4e36ad45ca0 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 08:34:21 -0800 Subject: [PATCH 21/42] Fixes recursive serializer --- bookwyrm/activitypub/base_activity.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index a83021ad..10cbf3a0 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -166,7 +166,7 @@ class ActivityObject: # recursively serialize for (k, v) in data.items(): try: - is_subclass = issubclass(v, ActivityObject) + is_subclass = issubclass(type(v), ActivityObject) except TypeError: is_subclass = False if is_subclass: From cbf5479253a3d15cd9e57310c9791db34faa82db Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 08:35:17 -0800 Subject: [PATCH 22/42] Test fixes --- .../connectors/test_openlibrary_connector.py | 28 +++++++++++++++---- .../tests/models/test_activitypub_mixin.py | 25 +++++++++-------- bookwyrm/tests/models/test_fields.py | 5 ++-- bookwyrm/tests/models/test_import_model.py | 4 ++- 4 files changed, 42 insertions(+), 20 deletions(-) diff --git a/bookwyrm/tests/connectors/test_openlibrary_connector.py b/bookwyrm/tests/connectors/test_openlibrary_connector.py index 8132a203..576e353b 100644 --- a/bookwyrm/tests/connectors/test_openlibrary_connector.py +++ b/bookwyrm/tests/connectors/test_openlibrary_connector.py @@ -92,11 +92,26 @@ class Openlibrary(TestCase): responses.add( responses.GET, 'https://openlibrary.org/authors/OL382982A', - json={'hi': 'there'}, + json={ + "name": "George Elliott", + "personal_name": "George Elliott", + "last_modified": { + "type": "/type/datetime", + "value": "2008-08-31 10:09:33.413686" + }, + "key": "/authors/OL453734A", + "type": { + "key": "/type/author" + }, + "id": 1259965, + "revision": 2 + }, status=200) results = self.connector.get_authors_from_data(self.work_data) - for result in results: - self.assertIsInstance(result, models.Author) + result = list(results)[0] + self.assertIsInstance(result, models.Author) + self.assertEqual(result.name, 'George Elliott') + self.assertEqual(result.openlibrary_key, 'OL453734A') def test_get_cover_url(self): @@ -201,8 +216,11 @@ class Openlibrary(TestCase): 'https://openlibrary.org/authors/OL382982A', json={'hi': 'there'}, status=200) - result = self.connector.create_edition_from_data( - work, self.edition_data) + with patch('bookwyrm.connectors.openlibrary.Connector.' \ + 'get_authors_from_data') as mock: + mock.return_value = [] + result = self.connector.create_edition_from_data( + work, self.edition_data) self.assertEqual(result.parent_work, work) self.assertEqual(result.title, 'Sabriel') self.assertEqual(result.isbn_10, '0060273224') diff --git a/bookwyrm/tests/models/test_activitypub_mixin.py b/bookwyrm/tests/models/test_activitypub_mixin.py index 1ea4ae6d..a6f069d4 100644 --- a/bookwyrm/tests/models/test_activitypub_mixin.py +++ b/bookwyrm/tests/models/test_activitypub_mixin.py @@ -30,6 +30,12 @@ class ActivitypubMixins(TestCase): outbox='https://example.com/users/rat/outbox', ) + self.object_mock = { + 'to': 'to field', 'cc': 'cc field', + 'content': 'hi', 'id': 'bip', 'type': 'Test', + 'published': '2020-12-04T17:52:22.623807+00:00', + } + # ActivitypubMixin def test_to_activity(self): @@ -292,15 +298,10 @@ class ActivitypubMixins(TestCase): def test_to_create_activity(self): ''' wrapper for ActivityPub "create" action ''' - object_activity = { - 'to': 'to field', 'cc': 'cc field', - 'content': 'hi', - 'published': '2020-12-04T17:52:22.623807+00:00', - } MockSelf = namedtuple('Self', ('remote_id', 'to_activity')) mock_self = MockSelf( 'https://example.com/status/1', - lambda *args: object_activity + lambda *args: self.object_mock ) activity = ObjectMixin.to_create_activity( mock_self, self.local_user) @@ -312,7 +313,7 @@ class ActivitypubMixins(TestCase): self.assertEqual(activity['type'], 'Create') self.assertEqual(activity['to'], 'to field') self.assertEqual(activity['cc'], 'cc field') - self.assertEqual(activity['object'], object_activity) + self.assertIsInstance(activity['object'], dict) self.assertEqual( activity['signature'].creator, '%s#main-key' % self.local_user.remote_id @@ -323,7 +324,7 @@ class ActivitypubMixins(TestCase): MockSelf = namedtuple('Self', ('remote_id', 'to_activity')) mock_self = MockSelf( 'https://example.com/status/1', - lambda *args: {} + lambda *args: self.object_mock ) activity = ObjectMixin.to_delete_activity( mock_self, self.local_user) @@ -346,7 +347,7 @@ class ActivitypubMixins(TestCase): MockSelf = namedtuple('Self', ('remote_id', 'to_activity')) mock_self = MockSelf( 'https://example.com/status/1', - lambda *args: {} + lambda *args: self.object_mock ) activity = ObjectMixin.to_update_activity( mock_self, self.local_user) @@ -361,7 +362,7 @@ class ActivitypubMixins(TestCase): self.assertEqual( activity['to'], ['https://www.w3.org/ns/activitystreams#Public']) - self.assertEqual(activity['object'], {}) + self.assertIsInstance(activity['object'], dict) # Activity mixin @@ -370,7 +371,7 @@ class ActivitypubMixins(TestCase): MockSelf = namedtuple('Self', ('remote_id', 'to_activity', 'user')) mock_self = MockSelf( 'https://example.com/status/1', - lambda *args: {}, + lambda *args: self.object_mock, self.local_user, ) activity = ActivityMixin.to_undo_activity(mock_self) @@ -380,4 +381,4 @@ class ActivitypubMixins(TestCase): ) self.assertEqual(activity['actor'], self.local_user.remote_id) self.assertEqual(activity['type'], 'Undo') - self.assertEqual(activity['object'], {}) + self.assertIsInstance(activity['object'], dict) diff --git a/bookwyrm/tests/models/test_fields.py b/bookwyrm/tests/models/test_fields.py index 38de9c98..24c0fb02 100644 --- a/bookwyrm/tests/models/test_fields.py +++ b/bookwyrm/tests/models/test_fields.py @@ -17,6 +17,7 @@ from django.db import models from django.test import TestCase from django.utils import timezone +from bookwyrm import activitypub from bookwyrm.activitypub.base_activity import ActivityObject from bookwyrm.models import fields, User, Status from bookwyrm.models.base_model import BookWyrmModel @@ -275,7 +276,7 @@ class ActivitypubFields(TestCase): 'rat', 'rat@rat.rat', 'ratword', local=True, localname='rat') with patch('bookwyrm.models.user.set_remote_server.delay'): - value = instance.field_from_activity(userdata) + value = instance.field_from_activity(activitypub.Person(**userdata)) self.assertIsInstance(value, User) self.assertNotEqual(value, unrelated_user) self.assertEqual(value.remote_id, 'https://example.com/user/mouse') @@ -300,7 +301,7 @@ class ActivitypubFields(TestCase): local=True, localname='rat') with patch('bookwyrm.models.activitypub_mixin.ObjectMixin.broadcast'): - value = instance.field_from_activity(userdata) + value = instance.field_from_activity(activitypub.Person(**userdata)) self.assertEqual(value, user) diff --git a/bookwyrm/tests/models/test_import_model.py b/bookwyrm/tests/models/test_import_model.py index 146c60a7..7ec4e700 100644 --- a/bookwyrm/tests/models/test_import_model.py +++ b/bookwyrm/tests/models/test_import_model.py @@ -171,6 +171,8 @@ class ImportJob(TestCase): 'bookwyrm.connectors.connector_manager.first_search_result' ) as search: search.return_value = result - book = self.item_1.get_book_from_isbn() + with patch('bookwyrm.connectors.openlibrary.Connector.' \ + 'get_authors_from_data'): + book = self.item_1.get_book_from_isbn() self.assertEqual(book.title, 'Sabriel') From 8bb20730fca03ad8c5565ee3f73676816870051e Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 09:33:33 -0800 Subject: [PATCH 23/42] Fixes bug in serializing dataclasses in place --- bookwyrm/activitypub/base_activity.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 10cbf3a0..8b538f0b 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -162,15 +162,14 @@ class ActivityObject: def serialize(self): ''' convert to dictionary with context attr ''' - data = self.__dict__ + data = self.__dict__.copy() # recursively serialize for (k, v) in data.items(): try: - is_subclass = issubclass(type(v), ActivityObject) + if issubclass(type(v), ActivityObject): + data[k] = v.serialize() except TypeError: - is_subclass = False - if is_subclass: - data[k] = v.serialize() + pass data = {k:v for (k, v) in data.items() if v is not None} data['@context'] = 'https://www.w3.org/ns/activitystreams' return data From b18dac5814fd74c62782e6f94d4a7da049f4e2c1 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 10:15:22 -0800 Subject: [PATCH 24/42] Don't use generic ActivityObject as serializer --- bookwyrm/activitypub/base_activity.py | 6 +++++- bookwyrm/activitypub/ordered_collection.py | 1 + 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 8b538f0b..7b9fe519 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -76,8 +76,12 @@ class ActivityObject: is_subclass = False # parse a dict into the appropriate activity if is_subclass and isinstance(value, dict): + serializer = None + if not isinstance(field.type, ActivityObject): + # this is generic, gotta figure out the type manually + serializer = field.type value = naive_parse( - activity_objects, value, serializer=field.type) + activity_objects, value, serializer=serializer) except KeyError: if field.default == MISSING and \ diff --git a/bookwyrm/activitypub/ordered_collection.py b/bookwyrm/activitypub/ordered_collection.py index cf642994..38f1c26c 100644 --- a/bookwyrm/activitypub/ordered_collection.py +++ b/bookwyrm/activitypub/ordered_collection.py @@ -17,6 +17,7 @@ class OrderedCollection(ActivityObject): @dataclass(init=False) class OrderedCollectionPrivate(OrderedCollection): + ''' an ordered collection with privacy settings ''' to: List[str] = field(default_factory=lambda: []) cc: List[str] = field(default_factory=lambda: []) From 9225043b5d1cedbb2a38f137c9f7d929783ef6e7 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 10:16:17 -0800 Subject: [PATCH 25/42] Fixes relationship model test --- .../tests/models/test_relationship_models.py | 21 ++++++------------- bookwyrm/tests/models/test_shelf_model.py | 1 + 2 files changed, 7 insertions(+), 15 deletions(-) diff --git a/bookwyrm/tests/models/test_relationship_models.py b/bookwyrm/tests/models/test_relationship_models.py index 97f267b6..0ef53450 100644 --- a/bookwyrm/tests/models/test_relationship_models.py +++ b/bookwyrm/tests/models/test_relationship_models.py @@ -23,19 +23,6 @@ class Relationship(TestCase): self.local_user.remote_id = 'http://local.com/user/mouse' self.local_user.save(broadcast=False) - def test_user_follows(self): - ''' create a follow relationship ''' - with patch('bookwyrm.models.activitypub_mixin.ActivityMixin.broadcast'): - rel = models.UserFollows.objects.create( - user_subject=self.local_user, - user_object=self.remote_user - ) - - activity = rel.to_activity() - self.assertEqual(activity['id'], rel.remote_id) - self.assertEqual(activity['actor'], self.local_user.remote_id) - self.assertEqual(activity['object'], self.remote_user.remote_id) - def test_user_follows_from_request(self): ''' convert a follow request into a follow ''' @@ -116,13 +103,15 @@ class Relationship(TestCase): self.assertEqual(user.remote_id, self.local_user.remote_id) self.assertEqual(activity['type'], 'Accept') self.assertEqual(activity['actor'], self.local_user.remote_id) - self.assertEqual( - activity['object']['id'], request.remote_id) + self.assertEqual(activity['object']['id'], 'https://www.hi.com/') + self.local_user.manually_approves_followers = True + self.local_user.save(broadcast=False) models.UserFollowRequest.broadcast = mock_broadcast request = models.UserFollowRequest.objects.create( user_subject=self.remote_user, user_object=self.local_user, + remote_id='https://www.hi.com/' ) request.accept() @@ -145,6 +134,8 @@ class Relationship(TestCase): activity['object']['id'], request.remote_id) models.UserFollowRequest.broadcast = mock_reject + self.local_user.manually_approves_followers = True + self.local_user.save(broadcast=False) request = models.UserFollowRequest.objects.create( user_subject=self.remote_user, user_object=self.local_user, diff --git a/bookwyrm/tests/models/test_shelf_model.py b/bookwyrm/tests/models/test_shelf_model.py index 11b5ad5c..c997326e 100644 --- a/bookwyrm/tests/models/test_shelf_model.py +++ b/bookwyrm/tests/models/test_shelf_model.py @@ -4,6 +4,7 @@ from django.test import TestCase from bookwyrm import models, settings +#pylint: disable=unused-argument class Shelf(TestCase): ''' some activitypub oddness ahead ''' def setUp(self): From 92e40e1cec05c3d2cc2de66334fa605176a47d86 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 10:30:02 -0800 Subject: [PATCH 26/42] Pass model instances into activities instead of json --- bookwyrm/activitypub/base_activity.py | 11 +++++------ bookwyrm/models/activitypub_mixin.py | 10 +++++----- 2 files changed, 10 insertions(+), 11 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 7b9fe519..dbd4eacc 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -74,14 +74,13 @@ class ActivityObject: is_subclass = issubclass(field.type, ActivityObject) except TypeError: is_subclass = False + # serialize a model obj + if hasattr(value, 'to_activity'): + value = value.to_activity() # parse a dict into the appropriate activity - if is_subclass and isinstance(value, dict): - serializer = None - if not isinstance(field.type, ActivityObject): - # this is generic, gotta figure out the type manually - serializer = field.type + elif is_subclass and isinstance(value, dict): value = naive_parse( - activity_objects, value, serializer=serializer) + activity_objects, value, serializer=field.type) except KeyError: if field.default == MISSING and \ diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 6ecf8d96..9863af4c 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -260,7 +260,7 @@ class ObjectMixin(ActivitypubMixin): actor=user.remote_id, to=['%s/followers' % user.remote_id], cc=['https://www.w3.org/ns/activitystreams#Public'], - object=self.to_activity(), + object=self, ).serialize() @@ -271,7 +271,7 @@ class ObjectMixin(ActivitypubMixin): id=activity_id, actor=user.remote_id, to=['https://www.w3.org/ns/activitystreams#Public'], - object=self.to_activity() + object=self ).serialize() @@ -363,7 +363,7 @@ class CollectionItemMixin(ActivitypubMixin): return activitypub.Add( id='%s#add' % self.remote_id, actor=self.user.remote_id, - object=object_field.to_activity(), + object=object_field, target=collection_field.remote_id ).serialize() @@ -374,7 +374,7 @@ class CollectionItemMixin(ActivitypubMixin): return activitypub.Remove( id='%s#remove' % self.remote_id, actor=self.user.remote_id, - object=object_field.to_activity(), + object=object_field, target=collection_field.remote_id ).serialize() @@ -403,7 +403,7 @@ class ActivityMixin(ActivitypubMixin): return activitypub.Undo( id='%s#undo' % self.remote_id, actor=user.remote_id, - object=self.to_activity() + object=self, ).serialize() From d5ca77362bf14f951101fd59d821e85ceb1a7a6b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 10:38:09 -0800 Subject: [PATCH 27/42] Fixes boost activity type in status model test --- bookwyrm/tests/models/test_status_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index 6978d593..f9b95f34 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -63,7 +63,7 @@ class Status(TestCase): self.assertEqual(models.Review().status_type, 'Review') self.assertEqual(models.Quotation().status_type, 'Quotation') self.assertEqual(models.Comment().status_type, 'Comment') - self.assertEqual(models.Boost().status_type, 'Boost') + self.assertEqual(models.Boost().status_type, 'Announce') def test_boostable(self, _): ''' can a status be boosted, based on privacy ''' From d022fef6259d8d1c6d975a439fa88d2073c5d46f Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 11:28:54 -0800 Subject: [PATCH 28/42] broadcast accepts correctly --- bookwyrm/models/relationship.py | 20 +++++++++++++------- bookwyrm/tests/views/test_follow.py | 8 +++++--- bookwyrm/views/follow.py | 16 +++++++++------- 3 files changed, 27 insertions(+), 17 deletions(-) diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index 722f0fac..76852988 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -91,9 +91,14 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship): user_subject=self.user_object, user_object=self.user_subject, ) - return None + return except (UserFollows.DoesNotExist, UserBlocks.DoesNotExist): + pass + + # this was calling itself which is not my idea of "super" ... + if not self.id: super().save(*args, **kwargs) + return if broadcast and self.user_subject.local and not self.user_object.local: self.broadcast(self.to_activity(), self.user_subject) @@ -116,16 +121,17 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship): def accept(self): ''' turn this request into the real deal''' user = self.user_object - activity = activitypub.Accept( - id=self.get_remote_id(status='accepts'), - actor=self.user_object.remote_id, - object=self.to_activity() - ).serialize() + if not self.user_subject.local: + activity = activitypub.Accept( + id=self.get_remote_id(status='accepts'), + actor=self.user_object.remote_id, + object=self.to_activity() + ).serialize() + self.broadcast(activity, user) with transaction.atomic(): UserFollows.from_request(self) self.delete() - self.broadcast(activity, user) def reject(self): diff --git a/bookwyrm/tests/views/test_follow.py b/bookwyrm/tests/views/test_follow.py index 9a157659..b913c17d 100644 --- a/bookwyrm/tests/views/test_follow.py +++ b/bookwyrm/tests/views/test_follow.py @@ -77,7 +77,6 @@ class BookViews(TestCase): self.assertEqual(rel.status, 'follow_request') - def test_handle_follow_local(self): ''' send a follow request ''' rat = models.User.objects.create_user( @@ -106,15 +105,18 @@ class BookViews(TestCase): self.remote_user.followers.add(self.local_user) self.assertEqual(self.remote_user.followers.count(), 1) # need to see if this ACTUALLY broadcasts - raise ValueError() - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay') \ + as mock: views.unfollow(request) + self.assertEqual(mock.call_count, 1) self.assertEqual(self.remote_user.followers.count(), 0) def test_handle_accept(self): ''' accept a follow request ''' + self.local_user.manually_approves_followers = True + self.local_user.save(broadcast=False) request = self.factory.post('', {'user': self.remote_user.username}) request.user = self.local_user rel = models.UserFollowRequest.objects.create( diff --git a/bookwyrm/views/follow.py b/bookwyrm/views/follow.py index e95355e6..d015dab2 100644 --- a/bookwyrm/views/follow.py +++ b/bookwyrm/views/follow.py @@ -1,5 +1,6 @@ ''' 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.views.decorators.http import require_POST @@ -17,10 +18,13 @@ def follow(request): except models.User.DoesNotExist: return HttpResponseBadRequest() - rel, _ = models.UserFollowRequest.objects.get_or_create( - user_subject=request.user, - user_object=to_follow, - ) + try: + models.UserFollowRequest.objects.create( + user_subject=request.user, + user_object=to_follow, + ) + except IntegrityError: + pass return redirect(to_follow.local_path) @@ -38,9 +42,7 @@ def unfollow(request): models.UserFollows.objects.get( user_subject=request.user, user_object=to_unfollow - ) - - to_unfollow.followers.remove(request.user) + ).delete() return redirect(to_unfollow.local_path) From 08dc5b4d8617c1dde4e18e35948a0eb52b3b86d3 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 11:45:21 -0800 Subject: [PATCH 29/42] Fixes unfollow --- bookwyrm/models/relationship.py | 14 ++++++++++++-- bookwyrm/tests/views/test_follow.py | 1 - 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index 76852988..3e6ac0fb 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -5,7 +5,7 @@ from django.db.models import Q from django.dispatch import receiver from bookwyrm import activitypub -from .activitypub_mixin import ActivitypubMixin, ActivityMixin +from .activitypub_mixin import ActivitypubMixin, ActivityMixin, generate_activity from .base_model import BookWyrmModel from . import fields @@ -56,10 +56,20 @@ class UserRelationship(BookWyrmModel): return '%s#%s/%d' % (base_path, status, self.id) -class UserFollows(ActivitypubMixin, UserRelationship): +class UserFollows(ActivityMixin, UserRelationship): ''' Following a user ''' status = 'follows' + def save(self, *args, **kwargs): + ''' never broadcast a creation (that's handled by "accept"), only a + deletion (an unfollow, as opposed to "reject" and undo pending) ''' + super().save(*args, broadcast=False, **kwargs) + + def to_activity(self): + ''' overrides default to manually set serializer ''' + return activitypub.Follow(**generate_activity(self)) + + @classmethod def from_request(cls, follow_request): ''' converts a follow request into a follow relationship ''' diff --git a/bookwyrm/tests/views/test_follow.py b/bookwyrm/tests/views/test_follow.py index b913c17d..5f61ab03 100644 --- a/bookwyrm/tests/views/test_follow.py +++ b/bookwyrm/tests/views/test_follow.py @@ -104,7 +104,6 @@ class BookViews(TestCase): request.user = self.local_user self.remote_user.followers.add(self.local_user) self.assertEqual(self.remote_user.followers.count(), 1) - # need to see if this ACTUALLY broadcasts with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay') \ as mock: views.unfollow(request) From 7b21a0a2081af5271aa3322ad6effbe93e307aa8 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 12:23:55 -0800 Subject: [PATCH 30/42] Fix things, unfix things, refix things, break things, fix things --- bookwyrm/activitypub/base_activity.py | 6 ++++-- bookwyrm/models/relationship.py | 5 +---- bookwyrm/tests/views/test_follow.py | 2 ++ bookwyrm/tests/views/test_inbox.py | 8 +++++--- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index dbd4eacc..c360711d 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -79,8 +79,10 @@ class ActivityObject: value = value.to_activity() # parse a dict into the appropriate activity elif is_subclass and isinstance(value, dict): - value = naive_parse( - activity_objects, value, serializer=field.type) + if activity_objects: + value = naive_parse(activity_objects, value) + else: + value = naive_parse(activity_objects, value, serializer=field.type) except KeyError: if field.default == MISSING and \ diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index 3e6ac0fb..764cada6 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -105,10 +105,7 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship): except (UserFollows.DoesNotExist, UserBlocks.DoesNotExist): pass - # this was calling itself which is not my idea of "super" ... - if not self.id: - super().save(*args, **kwargs) - return + super().save(*args, **kwargs) if broadcast and self.user_subject.local and not self.user_object.local: self.broadcast(self.to_activity(), self.user_subject) diff --git a/bookwyrm/tests/views/test_follow.py b/bookwyrm/tests/views/test_follow.py index 5f61ab03..62543d2d 100644 --- a/bookwyrm/tests/views/test_follow.py +++ b/bookwyrm/tests/views/test_follow.py @@ -135,6 +135,8 @@ class BookViews(TestCase): def test_handle_reject(self): ''' reject a follow request ''' + self.local_user.manually_approves_followers = True + self.local_user.save(broadcast=False) request = self.factory.post('', {'user': self.remote_user.username}) request.user = self.local_user rel = models.UserFollowRequest.objects.create( diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 0251d8a2..1e597cb1 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -236,7 +236,7 @@ class Inbox(TestCase): self.assertEqual(book_list.remote_id, 'https://example.com/list/22') - def test_handle_follow(self): + def test_handle_follow_x(self): ''' remote user wants to follow local user ''' activity = { "@context": "https://www.w3.org/ns/activitystreams", @@ -246,7 +246,9 @@ class Inbox(TestCase): "object": "https://example.com/user/mouse" } - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay') as mock: + self.assertFalse(models.UserFollowRequest.objects.exists()) + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay') \ + as mock: views.inbox.activity_task(activity) self.assertEqual(mock.call_count, 1) @@ -736,6 +738,6 @@ class Inbox(TestCase): "type": "Block", "actor": "https://example.com/users/rat", "object": "https://example.com/user/mouse" - }} + }} views.inbox.activity_task(activity) self.assertFalse(models.UserBlocks.objects.exists()) From e8e4ed773cfa83961756f795c4f234b4d35d5638 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 13:07:19 -0800 Subject: [PATCH 31/42] Fixes deletion for boosts --- bookwyrm/models/status.py | 4 ++++ bookwyrm/tests/views/test_interaction.py | 6 ++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index fbc94e9d..2fb70801 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -86,6 +86,10 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): def delete(self, *args, **kwargs):#pylint: disable=unused-argument ''' "delete" a status ''' + if hasattr(self, 'boosted_status'): + # okay but if it's a boost really delete it + super().delete(*args, **kwargs) + return self.deleted = True self.deleted_date = timezone.now() self.save() diff --git a/bookwyrm/tests/views/test_interaction.py b/bookwyrm/tests/views/test_interaction.py index da6d5f9c..c6d39f29 100644 --- a/bookwyrm/tests/views/test_interaction.py +++ b/bookwyrm/tests/views/test_interaction.py @@ -138,7 +138,7 @@ class InteractionViews(TestCase): ''' undo a boost ''' view = views.Unboost.as_view() request = self.factory.post('') - request.user = self.remote_user + request.user = self.local_user with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): status = models.Status.objects.create( user=self.local_user, content='hi') @@ -146,7 +146,9 @@ class InteractionViews(TestCase): self.assertEqual(models.Boost.objects.count(), 1) self.assertEqual(models.Notification.objects.count(), 1) - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay') \ + as mock: view(request, status.id) + self.assertEqual(mock.call_count, 1) self.assertEqual(models.Boost.objects.count(), 0) self.assertEqual(models.Notification.objects.count(), 0) From 79875271f725dfb079b13d29c2f3a85a6b061522 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 13:33:48 -0800 Subject: [PATCH 32/42] Makes next/prev page links optional --- bookwyrm/activitypub/ordered_collection.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bookwyrm/activitypub/ordered_collection.py b/bookwyrm/activitypub/ordered_collection.py index 38f1c26c..14b35f3c 100644 --- a/bookwyrm/activitypub/ordered_collection.py +++ b/bookwyrm/activitypub/ordered_collection.py @@ -39,6 +39,6 @@ class OrderedCollectionPage(ActivityObject): ''' structure of an ordered collection activity ''' partOf: str orderedItems: List - next: str - prev: str + next: str = None + prev: str = None type: str = 'OrderedCollectionPage' From 3f02b5f6f28040b8d3f0cb1bcce5a7b3330e6e56 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 13:34:36 -0800 Subject: [PATCH 33/42] Fixes view tests --- bookwyrm/models/activitypub_mixin.py | 2 +- bookwyrm/tests/views/test_list.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 9863af4c..1abf9a75 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -248,7 +248,7 @@ class ObjectMixin(ActivitypubMixin): actor=user.remote_id, to=activity_object['to'], cc=activity_object['cc'], - object=activity_object, + object=self, signature=signature, ).serialize() diff --git a/bookwyrm/tests/views/test_list.py b/bookwyrm/tests/views/test_list.py index 8737e347..e41d9806 100644 --- a/bookwyrm/tests/views/test_list.py +++ b/bookwyrm/tests/views/test_list.py @@ -9,7 +9,7 @@ from django.test.client import RequestFactory from bookwyrm import models, views from bookwyrm.activitypub import ActivitypubResponse - +#pylint: disable=unused-argument class ListViews(TestCase): ''' tag views''' def setUp(self): @@ -45,7 +45,7 @@ class ListViews(TestCase): with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): models.List.objects.create(name='Public list', user=self.local_user) models.List.objects.create( - name='Private list', privacy='private', user=self.local_user) + name='Private list', privacy='direct', user=self.local_user) request = self.factory.get('') request.user = self.local_user @@ -164,7 +164,7 @@ class ListViews(TestCase): with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): models.List.objects.create(name='Public list', user=self.local_user) models.List.objects.create( - name='Private list', privacy='private', user=self.local_user) + name='Private list', privacy='direct', user=self.local_user) request = self.factory.get('') request.user = self.local_user From e707374888f0c9a8571c6356f41bb19bb7086e96 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 17 Feb 2021 14:37:20 -0800 Subject: [PATCH 34/42] Don't broadcast from inbox tests --- bookwyrm/models/relationship.py | 18 ++++++++++-------- bookwyrm/tests/views/test_inbox.py | 9 +++------ 2 files changed, 13 insertions(+), 14 deletions(-) diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index 764cada6..14f702b4 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -5,7 +5,8 @@ from django.db.models import Q from django.dispatch import receiver from bookwyrm import activitypub -from .activitypub_mixin import ActivitypubMixin, ActivityMixin, generate_activity +from .activitypub_mixin import ActivitypubMixin, ActivityMixin +from .activitypub_mixin import generate_activity from .base_model import BookWyrmModel from . import fields @@ -143,14 +144,15 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship): def reject(self): ''' generate a Reject for this follow request ''' - user = self.user_object - activity = activitypub.Reject( - id=self.get_remote_id(status='rejects'), - actor=self.user_object.remote_id, - object=self.to_activity() - ).serialize() + if self.user_object.local: + activity = activitypub.Reject( + id=self.get_remote_id(status='rejects'), + actor=self.user_object.remote_id, + object=self.to_activity() + ).serialize() + self.broadcast(activity, self.user_object) + self.delete() - self.broadcast(activity, user) class UserBlocks(ActivityMixin, UserRelationship): diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 1e597cb1..c2658917 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -379,11 +379,8 @@ class Inbox(TestCase): views.inbox.activity_task(activity) # request should be deleted - self.assertEqual(models.UserFollowRequest.objects.count(), 0) - - # relationship should be created - follows = self.remote_user.followers - self.assertEqual(follows.count(), 0) + self.assertFalse(models.UserFollowRequest.objects.exists()) + self.assertFalse(self.remote_user.followers.exists()) def test_handle_update_list(self): @@ -580,7 +577,7 @@ class Inbox(TestCase): 'object': { 'type': 'Announce', 'id': boost.remote_id, - 'actor': self.local_user.remote_id, + 'actor': self.remote_user.remote_id, 'object': self.status.remote_id, } } From fb98ef4b384f6fb2643623782cd3ffd551b9ebb8 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Fri, 19 Feb 2021 11:16:01 -0800 Subject: [PATCH 35/42] Remove redundant activitypub dataclass --- bookwyrm/activitypub/__init__.py | 2 +- bookwyrm/activitypub/verbs.py | 8 +------- bookwyrm/models/shelf.py | 2 +- bookwyrm/models/tag.py | 2 +- 4 files changed, 4 insertions(+), 10 deletions(-) diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index 5303d1b2..f06154ab 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -15,7 +15,7 @@ from .response import ActivitypubResponse from .book import Edition, Work, Author from .verbs import Create, Delete, Undo, Update from .verbs import Follow, Accept, Reject, Block -from .verbs import Add, AddBook, AddListItem, Remove +from .verbs import Add, AddListItem, Remove from .verbs import Announce, Like # this creates a list of all the Activity types that we can serialize, diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index aa6e7eee..6f1a4d44 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -132,13 +132,7 @@ class Add(Verb): @dataclass(init=False) -class AddBook(Add): - '''Add activity that's aware of the book obj ''' - object: Edition - - -@dataclass(init=False) -class AddListItem(AddBook): +class AddListItem(Add): '''Add activity that's aware of the book obj ''' notes: str = None order: int = 0 diff --git a/bookwyrm/models/shelf.py b/bookwyrm/models/shelf.py index 921b8617..dfb8b9b3 100644 --- a/bookwyrm/models/shelf.py +++ b/bookwyrm/models/shelf.py @@ -57,7 +57,7 @@ class ShelfBook(CollectionItemMixin, BookWyrmModel): user = fields.ForeignKey( 'User', on_delete=models.PROTECT, activitypub_field='actor') - activity_serializer = activitypub.AddBook + activity_serializer = activitypub.Add object_field = 'book' collection_field = 'shelf' diff --git a/bookwyrm/models/tag.py b/bookwyrm/models/tag.py index d75f6e05..12645b9e 100644 --- a/bookwyrm/models/tag.py +++ b/bookwyrm/models/tag.py @@ -50,7 +50,7 @@ class UserTag(CollectionItemMixin, BookWyrmModel): tag = fields.ForeignKey( 'Tag', on_delete=models.PROTECT, activitypub_field='target') - activity_serializer = activitypub.AddBook + activity_serializer = activitypub.Add object_field = 'book' collection_field = 'tag' From dbe9431d5adf93276e4ab381ddf3868b338f0642 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sat, 20 Feb 2021 11:24:41 -0800 Subject: [PATCH 36/42] Fixes pure serializer --- bookwyrm/models/activitypub_mixin.py | 25 +++++++++++-------- bookwyrm/models/status.py | 24 ++++++++++-------- .../tests/models/test_activitypub_mixin.py | 23 ----------------- bookwyrm/tests/models/test_status_model.py | 21 ++++++++++++++++ 4 files changed, 49 insertions(+), 44 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 1abf9a75..12bbda96 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -191,7 +191,7 @@ class ObjectMixin(ActivitypubMixin): try: software = None - # do we have a "pure" activitypub version of this for mastodon? + # do we have a "pure" activitypub version of this for mastodon? if hasattr(self, 'pure_content'): pure_activity = self.to_create_activity(user, pure=True) self.broadcast(pure_activity, user, software='other') @@ -199,7 +199,7 @@ class ObjectMixin(ActivitypubMixin): # sends to BW only if we just did a pure version for masto activity = self.to_create_activity(user) self.broadcast(activity, user, software=software) - except KeyError: + except AttributeError: # janky as heck, this catches the mutliple inheritence chain # for boosts and ignores this auxilliary broadcast return @@ -228,27 +228,27 @@ class ObjectMixin(ActivitypubMixin): def to_create_activity(self, user, **kwargs): ''' returns the object wrapped in a Create activity ''' - activity_object = self.to_activity(**kwargs) + activity_object = self.to_activity_dataclass(**kwargs) signature = None create_id = self.remote_id + '/activity' - if 'content' in activity_object and activity_object['content']: + if hasattr(activity_object, 'content') and activity_object.content: signer = pkcs1_15.new(RSA.import_key(user.key_pair.private_key)) - content = activity_object['content'] + content = activity_object.content signed_message = signer.sign(SHA256.new(content.encode('utf8'))) signature = activitypub.Signature( creator='%s#main-key' % user.remote_id, - created=activity_object['published'], + created=activity_object.published, signatureValue=b64encode(signed_message).decode('utf8') ) return activitypub.Create( id=create_id, actor=user.remote_id, - to=activity_object['to'], - cc=activity_object['cc'], - object=self, + to=activity_object.to, + cc=activity_object.cc, + object=activity_object, signature=signature, ).serialize() @@ -312,7 +312,7 @@ class OrderedCollectionPageMixin(ObjectMixin): activity['first'] = '%s?page=1' % remote_id activity['last'] = '%s?page=%d' % (remote_id, paginated.num_pages) - return serializer(**activity).serialize() + return serializer(**activity) class OrderedCollectionMixin(OrderedCollectionPageMixin): @@ -324,9 +324,12 @@ class OrderedCollectionMixin(OrderedCollectionPageMixin): activity_serializer = activitypub.OrderedCollection + def to_activity_dataclass(self, **kwargs): + return self.to_ordered_collection(self.collection_queryset, **kwargs) + def to_activity(self, **kwargs): ''' an ordered collection of the specified model queryset ''' - return self.to_ordered_collection(self.collection_queryset, **kwargs) + return self.to_ordered_collection(self.collection_queryset, **kwargs).serialize() class CollectionItemMixin(ActivitypubMixin): diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index 2fb70801..aff028a5 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -157,7 +157,7 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): **kwargs ) - def to_activity(self, pure=False):# pylint: disable=arguments-differ + def to_activity_dataclass(self, pure=False):# pylint: disable=arguments-differ ''' return tombstone if the status is deleted ''' if self.deleted: return activitypub.Tombstone( @@ -165,25 +165,29 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): url=self.remote_id, deleted=self.deleted_date.isoformat(), published=self.deleted_date.isoformat() - ).serialize() - activity = ActivitypubMixin.to_activity(self) - activity['replies'] = self.to_replies() + ) + activity = ActivitypubMixin.to_activity_dataclass(self) + activity.replies = self.to_replies() # "pure" serialization for non-bookwyrm instances if pure and hasattr(self, 'pure_content'): - activity['content'] = self.pure_content - if 'name' in activity: - activity['name'] = self.pure_name - activity['type'] = self.pure_type - activity['attachment'] = [ + activity.content = self.pure_content + if hasattr(activity, 'name'): + activity.name = self.pure_name + activity.type = self.pure_type + activity.attachment = [ image_serializer(b.cover, b.alt_text) \ for b in self.mention_books.all()[:4] if b.cover] if hasattr(self, 'book') and self.book.cover: - activity['attachment'].append( + activity.attachment.append( image_serializer(self.book.cover, self.book.alt_text) ) return activity + def to_activity(self, pure=False):# pylint: disable=arguments-differ + ''' json serialized activitypub class ''' + return self.to_activity_dataclass(pure=pure).serialize() + class GeneratedNote(Status): ''' these are app-generated messages about user activity ''' diff --git a/bookwyrm/tests/models/test_activitypub_mixin.py b/bookwyrm/tests/models/test_activitypub_mixin.py index a6f069d4..11b944d9 100644 --- a/bookwyrm/tests/models/test_activitypub_mixin.py +++ b/bookwyrm/tests/models/test_activitypub_mixin.py @@ -296,29 +296,6 @@ class ActivitypubMixins(TestCase): id=1, user=self.local_user, deleted=True).save() - def test_to_create_activity(self): - ''' wrapper for ActivityPub "create" action ''' - MockSelf = namedtuple('Self', ('remote_id', 'to_activity')) - mock_self = MockSelf( - 'https://example.com/status/1', - lambda *args: self.object_mock - ) - activity = ObjectMixin.to_create_activity( - mock_self, self.local_user) - self.assertEqual( - activity['id'], - 'https://example.com/status/1/activity' - ) - self.assertEqual(activity['actor'], self.local_user.remote_id) - self.assertEqual(activity['type'], 'Create') - self.assertEqual(activity['to'], 'to field') - self.assertEqual(activity['cc'], 'cc field') - self.assertIsInstance(activity['object'], dict) - self.assertEqual( - activity['signature'].creator, - '%s#main-key' % self.local_user.remote_id - ) - def test_to_delete_activity(self): ''' wrapper for Delete activity ''' MockSelf = namedtuple('Self', ('remote_id', 'to_activity')) diff --git a/bookwyrm/tests/models/test_status_model.py b/bookwyrm/tests/models/test_status_model.py index f9b95f34..c6911b6d 100644 --- a/bookwyrm/tests/models/test_status_model.py +++ b/bookwyrm/tests/models/test_status_model.py @@ -284,3 +284,24 @@ class Status(TestCase): with self.assertRaises(IntegrityError): models.Notification.objects.create( user=self.user, notification_type='GLORB') + + + def test_create_broadcast(self, broadcast_mock): + ''' should send out two verions of a status on create ''' + models.Comment.objects.create( + content='hi', user=self.user, book=self.book) + self.assertEqual(broadcast_mock.call_count, 2) + pure_call = broadcast_mock.call_args_list[0] + bw_call = broadcast_mock.call_args_list[1] + + self.assertEqual(pure_call[1]['software'], 'other') + args = pure_call[0][0] + self.assertEqual(args['type'], 'Create') + self.assertEqual(args['object']['type'], 'Note') + self.assertTrue('content' in args['object']) + + + self.assertEqual(bw_call[1]['software'], 'bookwyrm') + args = bw_call[0][0] + self.assertEqual(args['type'], 'Create') + self.assertEqual(args['object']['type'], 'Comment') From cbccdea4683359c3b03865fa972a8a0eb6e70e7d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Feb 2021 11:13:20 -0800 Subject: [PATCH 37/42] fixes ordered collection serializations --- bookwyrm/models/activitypub_mixin.py | 3 ++- bookwyrm/models/status.py | 2 +- bookwyrm/models/user.py | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 12bbda96..7ea632b3 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -329,7 +329,8 @@ class OrderedCollectionMixin(OrderedCollectionPageMixin): def to_activity(self, **kwargs): ''' an ordered collection of the specified model queryset ''' - return self.to_ordered_collection(self.collection_queryset, **kwargs).serialize() + return self.to_ordered_collection( + self.collection_queryset, **kwargs).serialize() class CollectionItemMixin(ActivitypubMixin): diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index aff028a5..ba9727f5 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -155,7 +155,7 @@ class Status(OrderedCollectionPageMixin, BookWyrmModel): remote_id='%s/replies' % self.remote_id, collection_only=True, **kwargs - ) + ).serialize() def to_activity_dataclass(self, pure=False):# pylint: disable=arguments-differ ''' return tombstone if the status is deleted ''' diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 61d119a8..a64a8add 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -131,7 +131,7 @@ class User(OrderedCollectionPageMixin, AbstractUser): privacy__in=['public', 'unlisted'], ).select_subclasses().order_by('-published_date') return self.to_ordered_collection(queryset, \ - collection_only=True, remote_id=self.outbox, **kwargs) + collection_only=True, remote_id=self.outbox, **kwargs).serialize() def to_following_activity(self, **kwargs): ''' activitypub following list ''' From 6e6bcb2f4833e27d4c626d9d6d7c73c2616db5ff Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Feb 2021 15:51:02 -0800 Subject: [PATCH 38/42] gotta simplify the add activity --- bookwyrm/activitypub/__init__.py | 2 +- bookwyrm/activitypub/verbs.py | 11 ++--- bookwyrm/models/list.py | 2 +- bookwyrm/tests/views/test_inbox.py | 75 ++++++++++++++++++++++++++++++ 4 files changed, 80 insertions(+), 10 deletions(-) diff --git a/bookwyrm/activitypub/__init__.py b/bookwyrm/activitypub/__init__.py index f06154ab..fdfbb1f0 100644 --- a/bookwyrm/activitypub/__init__.py +++ b/bookwyrm/activitypub/__init__.py @@ -15,7 +15,7 @@ from .response import ActivitypubResponse from .book import Edition, Work, Author from .verbs import Create, Delete, Undo, Update from .verbs import Follow, Accept, Reject, Block -from .verbs import Add, AddListItem, Remove +from .verbs import Add, Remove from .verbs import Announce, Like # this creates a list of all the Activity types that we can serialize, diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index 6f1a4d44..1236338b 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -121,6 +121,9 @@ class Add(Verb): target: str object: Edition type: str = 'Add' + notes: str = None + order: int = 0 + approved: bool = True def action(self): ''' add obj to collection ''' @@ -131,14 +134,6 @@ class Add(Verb): self.to_model(model=model) -@dataclass(init=False) -class AddListItem(Add): - '''Add activity that's aware of the book obj ''' - notes: str = None - order: int = 0 - approved: bool = True - - @dataclass(init=False) class Remove(Verb): '''Remove activity ''' diff --git a/bookwyrm/models/list.py b/bookwyrm/models/list.py index ef48ed95..1b14c2aa 100644 --- a/bookwyrm/models/list.py +++ b/bookwyrm/models/list.py @@ -68,7 +68,7 @@ class ListItem(CollectionItemMixin, BookWyrmModel): order = fields.IntegerField(blank=True, null=True) endorsement = models.ManyToManyField('User', related_name='endorsers') - activity_serializer = activitypub.AddListItem + activity_serializer = activitypub.Add object_field = 'book' collection_field = 'book_list' diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index c2658917..45966d59 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -612,6 +612,81 @@ class Inbox(TestCase): self.assertEqual(shelf.books.first(), book) +# def test_handle_tag_book(self): +# ''' tagging a book ''' +# work = models.Work.objects.create(title='work title') +# book = models.Edition.objects.create( +# title='Test', remote_id='https://bookwyrm.social/book/37292', +# parent_work=work) +# +# activity = { +# "id": "https://bookwyrm.social/shelfbook/6189#add", +# "type": "Add", +# "actor": "https://example.com/users/rat", +# "object": { +# "type": "Edition", +# "title": "Test Title", +# "work": work.remote_id, +# "id": "https://bookwyrm.social/book/37292", +# }, +# "target": "", +# "@context": "https://www.w3.org/ns/activitystreams" +# } +# views.inbox.activity_task(activity) +# self.assertEqual(shelf.books.first(), book) + + + @responses.activate + def test_handle_add_book_to_list(self): + ''' listing a book ''' + work = models.Work.objects.create(title='work title') + book = models.Edition.objects.create( + title='Test', remote_id='https://bookwyrm.social/book/37292', + parent_work=work) + + responses.add( + responses.GET, + 'https://bookwyrm.social/user/mouse/list/to-read', + json={ + "id": "https://example.com/list/22", + "type": "BookList", + "totalItems": 1, + "first": "https://example.com/list/22?page=1", + "last": "https://example.com/list/22?page=1", + "name": "Test List", + "owner": "https://example.com/user/mouse", + "to": [ + "https://www.w3.org/ns/activitystreams#Public" + ], + "cc": [ + "https://example.com/user/mouse/followers" + ], + "summary": "summary text", + "curation": "curated", + "@context": "https://www.w3.org/ns/activitystreams" + } + ) + + activity = { + "id": "https://bookwyrm.social/listbook/6189#add", + "type": "Add", + "actor": "https://example.com/users/rat", + "object": { + "type": "Edition", + "title": "Test Title", + "work": work.remote_id, + "id": "https://bookwyrm.social/book/37292", + }, + "target": "https://bookwyrm.social/user/mouse/list/to-read", + "@context": "https://www.w3.org/ns/activitystreams" + } + views.inbox.activity_task(activity) + + booklist = models.List.objects.get() + self.assertEqual(booklist.name, 'Test List') + self.assertEqual(booklist.books.first(), book) + + def test_handle_update_user(self): ''' update an existing user ''' # we only do this with remote users From 4d0e52bf517a53d46c070a0d82b4efa89ced70a7 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Feb 2021 17:18:25 -0800 Subject: [PATCH 39/42] Test tag and list add --- bookwyrm/activitypub/base_activity.py | 3 +- bookwyrm/models/activitypub_mixin.py | 2 +- bookwyrm/models/tag.py | 19 ++++---- bookwyrm/tests/views/test_inbox.py | 67 +++++++++++++++++---------- bookwyrm/tests/views/test_tag.py | 15 ++++++ bookwyrm/views/tag.py | 7 ++- 6 files changed, 73 insertions(+), 40 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index c360711d..57f1a713 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -82,7 +82,8 @@ class ActivityObject: if activity_objects: value = naive_parse(activity_objects, value) else: - value = naive_parse(activity_objects, value, serializer=field.type) + value = naive_parse( + activity_objects, value, serializer=field.type) except KeyError: if field.default == MISSING and \ diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 7ea632b3..fe89f267 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -330,7 +330,7 @@ class OrderedCollectionMixin(OrderedCollectionPageMixin): def to_activity(self, **kwargs): ''' an ordered collection of the specified model queryset ''' return self.to_ordered_collection( - self.collection_queryset, **kwargs).serialize() + self.collection_queryset, **kwargs) class CollectionItemMixin(ActivitypubMixin): diff --git a/bookwyrm/models/tag.py b/bookwyrm/models/tag.py index 12645b9e..83359170 100644 --- a/bookwyrm/models/tag.py +++ b/bookwyrm/models/tag.py @@ -1,6 +1,7 @@ ''' models for storing different kinds of Activities ''' import urllib.parse +from django.apps import apps from django.db import models from bookwyrm import activitypub @@ -15,17 +16,15 @@ class Tag(OrderedCollectionMixin, BookWyrmModel): name = fields.CharField(max_length=100, unique=True) identifier = models.CharField(max_length=100) - @classmethod - def book_queryset(cls, identifier): - ''' county of books associated with this tag ''' - return cls.objects.filter( - identifier=identifier - ).order_by('-updated_date') - @property - def collection_queryset(self): - ''' books associated with this tag ''' - return self.book_queryset(self.identifier) + def books(self): + ''' count of books associated with this tag ''' + edition_model = apps.get_model('bookwyrm.Edition', require_ready=True) + return edition_model.objects.filter( + usertag__tag__identifier=self.identifier + ).order_by('-created_date').distinct() + + collection_queryset = books def get_remote_id(self): ''' tag should use identifier not id in remote_id ''' diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index 45966d59..ff55ad04 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -612,30 +612,6 @@ class Inbox(TestCase): self.assertEqual(shelf.books.first(), book) -# def test_handle_tag_book(self): -# ''' tagging a book ''' -# work = models.Work.objects.create(title='work title') -# book = models.Edition.objects.create( -# title='Test', remote_id='https://bookwyrm.social/book/37292', -# parent_work=work) -# -# activity = { -# "id": "https://bookwyrm.social/shelfbook/6189#add", -# "type": "Add", -# "actor": "https://example.com/users/rat", -# "object": { -# "type": "Edition", -# "title": "Test Title", -# "work": work.remote_id, -# "id": "https://bookwyrm.social/book/37292", -# }, -# "target": "", -# "@context": "https://www.w3.org/ns/activitystreams" -# } -# views.inbox.activity_task(activity) -# self.assertEqual(shelf.books.first(), book) - - @responses.activate def test_handle_add_book_to_list(self): ''' listing a book ''' @@ -687,6 +663,49 @@ class Inbox(TestCase): self.assertEqual(booklist.books.first(), book) + @responses.activate + def test_handle_tag_book(self): + ''' listing a book ''' + work = models.Work.objects.create(title='work title') + book = models.Edition.objects.create( + title='Test', remote_id='https://bookwyrm.social/book/37292', + parent_work=work) + + responses.add( + responses.GET, + 'https://www.example.com/tag/cool-tag', + json={ + "id": "https://1b1a78582461.ngrok.io/tag/tag", + "type": "OrderedCollection", + "totalItems": 0, + "first": "https://1b1a78582461.ngrok.io/tag/tag?page=1", + "last": "https://1b1a78582461.ngrok.io/tag/tag?page=1", + "name": "cool tag", + "@context": "https://www.w3.org/ns/activitystreams" + } + ) + + activity = { + "id": "https://bookwyrm.social/listbook/6189#add", + "type": "Add", + "actor": "https://example.com/users/rat", + "object": { + "type": "Edition", + "title": "Test Title", + "work": work.remote_id, + "id": "https://bookwyrm.social/book/37292", + }, + "target": "https://www.example.com/tag/cool-tag", + "@context": "https://www.w3.org/ns/activitystreams" + } + views.inbox.activity_task(activity) + + tag = models.Tag.objects.get() + self.assertFalse(models.List.objects.exists()) + self.assertEqual(tag.name, 'cool tag') + self.assertEqual(tag.books.first(), book) + + def test_handle_update_user(self): ''' update an existing user ''' # we only do this with remote users diff --git a/bookwyrm/tests/views/test_tag.py b/bookwyrm/tests/views/test_tag.py index 21a7e22e..ef809b46 100644 --- a/bookwyrm/tests/views/test_tag.py +++ b/bookwyrm/tests/views/test_tag.py @@ -59,6 +59,21 @@ class TagViews(TestCase): self.assertEqual(result.status_code, 200) + def test_tag_page_activitypub_page(self): + ''' there are so many views, this just makes sure it LOADS ''' + view = views.Tag.as_view() + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + tag = models.Tag.objects.create(name='hi there') + models.UserTag.objects.create( + tag=tag, user=self.local_user, book=self.book) + request = self.factory.get('', {'page': 1}) + with patch('bookwyrm.views.tag.is_api_request') as is_api: + is_api.return_value = True + result = view(request, tag.identifier) + self.assertIsInstance(result, ActivitypubResponse) + self.assertEqual(result.status_code, 200) + + def test_tag(self): ''' add a tag to a book ''' view = views.AddTag.as_view() diff --git a/bookwyrm/views/tag.py b/bookwyrm/views/tag.py index b50bc0ef..710f9415 100644 --- a/bookwyrm/views/tag.py +++ b/bookwyrm/views/tag.py @@ -16,12 +16,11 @@ class Tag(View): ''' tag page ''' def get(self, request, tag_id): ''' see books related to a tag ''' - tag_obj = models.Tag.objects.filter(identifier=tag_id).first() - if not tag_obj: - return HttpResponseNotFound() + tag_obj = get_object_or_404(models.Tag, identifier=tag_id) if is_api_request(request): - return ActivitypubResponse(tag_obj.to_activity(**request.GET)) + return ActivitypubResponse( + tag_obj.to_activity(**request.GET), safe=False) books = models.Edition.objects.filter( usertag__tag__identifier=tag_id From fba53c72e0d1ff2c2efc12967696386a7880b96d Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Feb 2021 17:19:47 -0800 Subject: [PATCH 40/42] default safe mode for activity serialization --- bookwyrm/activitypub/response.py | 2 +- bookwyrm/views/tag.py | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/bookwyrm/activitypub/response.py b/bookwyrm/activitypub/response.py index bbc44c4d..8f3c050b 100644 --- a/bookwyrm/activitypub/response.py +++ b/bookwyrm/activitypub/response.py @@ -9,7 +9,7 @@ class ActivitypubResponse(JsonResponse): configures some stuff beforehand. Made to be a drop-in replacement of JsonResponse. """ - def __init__(self, data, encoder=ActivityEncoder, safe=True, + def __init__(self, data, encoder=ActivityEncoder, safe=False, json_dumps_params=None, **kwargs): if 'content_type' not in kwargs: diff --git a/bookwyrm/views/tag.py b/bookwyrm/views/tag.py index 710f9415..502f5ea5 100644 --- a/bookwyrm/views/tag.py +++ b/bookwyrm/views/tag.py @@ -1,6 +1,5 @@ ''' tagging views''' 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 @@ -20,7 +19,7 @@ class Tag(View): if is_api_request(request): return ActivitypubResponse( - tag_obj.to_activity(**request.GET), safe=False) + tag_obj.to_activity(**request.GET)) books = models.Edition.objects.filter( usertag__tag__identifier=tag_id From 94e95dc39dd5af993d14f02b59383a5b500f1493 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 24 Feb 2021 10:07:03 -0800 Subject: [PATCH 41/42] Adds test for delete activity --- bookwyrm/tests/views/test_status.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/bookwyrm/tests/views/test_status.py b/bookwyrm/tests/views/test_status.py index d490f484..4594a203 100644 --- a/bookwyrm/tests/views/test_status.py +++ b/bookwyrm/tests/views/test_status.py @@ -1,4 +1,5 @@ ''' test for app action functionality ''' +import json from unittest.mock import patch from django.test import TestCase from django.test.client import RequestFactory @@ -236,7 +237,11 @@ class StatusViews(TestCase): self.assertFalse(status.deleted) request = self.factory.post('') request.user = self.local_user - with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay'): + with patch('bookwyrm.models.activitypub_mixin.broadcast_task.delay') \ + as mock: view(request, status.id) + activity = json.loads(mock.call_args_list[0][0][1]) + self.assertEqual(activity['type'], 'Delete') + self.assertEqual(activity['object']['type'], 'Tombstone') status.refresh_from_db() self.assertTrue(status.deleted) From bb7c41ee5fb4e51aa230778426ac8306ce807354 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Wed, 24 Feb 2021 13:13:29 -0800 Subject: [PATCH 42/42] Tweaks where serialize is called --- bookwyrm/models/activitypub_mixin.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 23752e02..f8b50afa 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -161,7 +161,7 @@ class ActivitypubMixin: activity = generate_activity(self) return self.activity_serializer(**activity) - def to_activity(self): + def to_activity(self, **kwargs): # pylint: disable=unused-argument ''' convert from a model to a json activity ''' return self.to_activity_dataclass().serialize() @@ -286,7 +286,7 @@ class OrderedCollectionPageMixin(ObjectMixin): def to_ordered_collection(self, queryset, \ remote_id=None, page=False, collection_only=False, **kwargs): - 'pure=pure, '' an ordered collection of whatevers ''' + ''' an ordered collection of whatevers ''' if not queryset.ordered: raise RuntimeError('queryset must be ordered') @@ -330,7 +330,7 @@ class OrderedCollectionMixin(OrderedCollectionPageMixin): def to_activity(self, **kwargs): ''' an ordered collection of the specified model queryset ''' return self.to_ordered_collection( - self.collection_queryset, **kwargs) + self.collection_queryset, **kwargs).serialize() class CollectionItemMixin(ActivitypubMixin): @@ -502,4 +502,4 @@ def to_ordered_collection_page( orderedItems=items, next=next_page, prev=prev_page - ).serialize() + )