From c0ccb7065c450f24ea67c2958603ddc90857c8b3 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 7 Mar 2021 09:22:35 -0800 Subject: [PATCH 1/5] Safer federation of book data changes Only broadcast to other BW instances, plus bonus error handling --- bookwyrm/models/activitypub_mixin.py | 2 +- bookwyrm/models/book.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index bebe00d0..10015bf1 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -449,7 +449,7 @@ def broadcast_task(sender_id, activity, recipients): for recipient in recipients: try: sign_and_send(sender, activity, recipient) - except (HTTPError, SSLError) as e: + except (HTTPError, SSLError, ConnectionError) as e: logger.exception(e) diff --git a/bookwyrm/models/book.py b/bookwyrm/models/book.py index 6a1a18b1..84bfbc6b 100644 --- a/bookwyrm/models/book.py +++ b/bookwyrm/models/book.py @@ -37,6 +37,10 @@ class BookDataModel(ObjectMixin, BookWyrmModel): self.remote_id = None return super().save(*args, **kwargs) + def broadcast(self, activity, sender, software='bookwyrm'): + ''' only send book data updates to other bookwyrm instances ''' + super().broadcast(activity, sender, software=software) + class Book(BookDataModel): ''' a generic book, which can mean either an edition or a work ''' From 71bbea83f97ca6cda270180aab940ab882687a7b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 7 Mar 2021 09:42:31 -0800 Subject: [PATCH 2/5] Adds discard check to favs --- bookwyrm/activitypub/base_activity.py | 2 +- bookwyrm/models/favorite.py | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 57f1a713..c732fe1d 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -102,7 +102,7 @@ class ActivityObject: if allow_create and \ hasattr(model, 'ignore_activity') and \ model.ignore_activity(self): - return None + raise ActivitySerializerError() # check for an existing instance instance = instance or model.find_existing(self.serialize()) diff --git a/bookwyrm/models/favorite.py b/bookwyrm/models/favorite.py index f9019501..66befd80 100644 --- a/bookwyrm/models/favorite.py +++ b/bookwyrm/models/favorite.py @@ -17,6 +17,11 @@ class Favorite(ActivityMixin, BookWyrmModel): activity_serializer = activitypub.Like + @classmethod + def ignore_activity(cls, activity): + ''' don't bother with incoming favs of unknown statuses ''' + return cls.objects.filter(remote_id=activity.object).exists() + def save(self, *args, **kwargs): ''' update user active time ''' self.user.last_active_date = timezone.now() From 09b77e567f8dc886ed4a505a87d79141fbddc8d2 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 7 Mar 2021 09:44:42 -0800 Subject: [PATCH 3/5] Check for invalid json before verifying signature --- bookwyrm/views/inbox.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 4da4e5b6..46385093 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -20,7 +20,7 @@ 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 + # make sure the user's inbox even exists if username: try: models.User.objects.get(localname=username) @@ -33,6 +33,11 @@ class Inbox(View): except json.decoder.JSONDecodeError: return HttpResponseBadRequest() + if not 'object' in activity_json or \ + not 'type' in activity_json or \ + not activity_json['type'] in activitypub.activity_objects: + return HttpResponseNotFound() + # verify the signature if not has_valid_signature(request, activity_json): if activity_json['type'] == 'Delete': @@ -42,12 +47,6 @@ class Inbox(View): return HttpResponse() return HttpResponse(status=401) - # 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(activity_json) return HttpResponse() @@ -63,7 +62,11 @@ def activity_task(activity_json): # cool that worked, now we should do the action described by the type # (create, update, delete, etc) - activity.action() + try: + activity.action() + except activitypub.ActivitySerializerError: + # this is raised if the activity is discarded + return def has_valid_signature(request, activity): From 47cf77145d19adb1ffc11364a8f45ec948ec8018 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 7 Mar 2021 09:45:02 -0800 Subject: [PATCH 4/5] Updates tests for inbox tweaks --- bookwyrm/models/favorite.py | 2 +- bookwyrm/tests/views/test_inbox.py | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/bookwyrm/models/favorite.py b/bookwyrm/models/favorite.py index 66befd80..de500e51 100644 --- a/bookwyrm/models/favorite.py +++ b/bookwyrm/models/favorite.py @@ -20,7 +20,7 @@ class Favorite(ActivityMixin, BookWyrmModel): @classmethod def ignore_activity(cls, activity): ''' don't bother with incoming favs of unknown statuses ''' - return cls.objects.filter(remote_id=activity.object).exists() + return not cls.objects.filter(remote_id=activity.object).exists() def save(self, *args, **kwargs): ''' update user active time ''' diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index ff55ad04..b0bd3e42 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -74,7 +74,7 @@ class Inbox(TestCase): mock_valid.return_value = False result = self.client.post( '/user/mouse/inbox', - '{"type": "Test", "object": "exists"}', + '{"type": "Announce", "object": "exists"}', content_type="application/json" ) self.assertEqual(result.status_code, 401) @@ -494,6 +494,21 @@ class Inbox(TestCase): self.assertEqual(fav.remote_id, 'https://example.com/fav/1') self.assertEqual(fav.user, self.remote_user) + def test_ignore_favorite(self): + ''' don't try to save an unknown status ''' + activity = { + '@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://unknown.status/not-found', + } + + views.inbox.activity_task(activity) + + self.assertFalse(models.Favorite.objects.exists()) + def test_handle_unfavorite(self): ''' fav a status ''' activity = { From 410e0b04bb3ec780b760bbb28bc86890585a32c7 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Sun, 7 Mar 2021 13:13:16 -0800 Subject: [PATCH 5/5] Fixes fav logic and base activity test --- bookwyrm/models/favorite.py | 3 ++- bookwyrm/tests/activitypub/test_base_activity.py | 5 ++++- bookwyrm/tests/views/test_inbox.py | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/bookwyrm/models/favorite.py b/bookwyrm/models/favorite.py index de500e51..d34cbcba 100644 --- a/bookwyrm/models/favorite.py +++ b/bookwyrm/models/favorite.py @@ -7,6 +7,7 @@ from bookwyrm import activitypub from .activitypub_mixin import ActivityMixin from .base_model import BookWyrmModel from . import fields +from .status import Status class Favorite(ActivityMixin, BookWyrmModel): ''' fav'ing a post ''' @@ -20,7 +21,7 @@ class Favorite(ActivityMixin, BookWyrmModel): @classmethod def ignore_activity(cls, activity): ''' don't bother with incoming favs of unknown statuses ''' - return not cls.objects.filter(remote_id=activity.object).exists() + return not Status.objects.filter(remote_id=activity.object).exists() def save(self, *args, **kwargs): ''' update user active time ''' diff --git a/bookwyrm/tests/activitypub/test_base_activity.py b/bookwyrm/tests/activitypub/test_base_activity.py index d489fdaa..de108eae 100644 --- a/bookwyrm/tests/activitypub/test_base_activity.py +++ b/bookwyrm/tests/activitypub/test_base_activity.py @@ -208,7 +208,10 @@ 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(model=models.Status, instance=status) + with patch('bookwyrm.models.status.Status.ignore_activity') \ + as discarder: + discarder.return_value = False + update_data.to_model(model=models.Status, instance=status) self.assertIsNone(status.attachments.first()) diff --git a/bookwyrm/tests/views/test_inbox.py b/bookwyrm/tests/views/test_inbox.py index b0bd3e42..4202979b 100644 --- a/bookwyrm/tests/views/test_inbox.py +++ b/bookwyrm/tests/views/test_inbox.py @@ -484,7 +484,7 @@ class Inbox(TestCase): 'actor': 'https://example.com/users/rat', 'type': 'Like', 'published': 'Mon, 25 May 2020 19:31:20 GMT', - 'object': 'https://example.com/status/1', + 'object': self.status.remote_id, } views.inbox.activity_task(activity)