Smarter way of inferring serializers (which are explicitly present)

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

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,

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.