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.