diff --git a/fedireads/incoming.py b/fedireads/incoming.py index 00fea094c..d20258927 100644 --- a/fedireads/incoming.py +++ b/fedireads/incoming.py @@ -10,7 +10,7 @@ from django.views.decorators.csrf import csrf_exempt from fedireads import books_manager, models, outgoing from fedireads import status as status_builder -from fedireads.remote_user import get_or_create_remote_user +from fedireads.remote_user import get_or_create_remote_user, refresh_remote_user from fedireads.tasks import app from fedireads.signatures import Signature @@ -51,9 +51,16 @@ def shared_inbox(request): if key_actor != activity.get('actor'): raise ValueError("Wrong actor created signature.") - key = get_public_key(key_actor) + remote_user = get_or_create_remote_user(key_actor) - signature.verify(key, request) + try: + signature.verify(remote_user.public_key, request) + except ValueError: + old_key = remote_user.public_key + refresh_remote_user(remote_user) + if remote_user.public_key == old_key: + raise # Key unchanged. + signature.verify(remote_user.public_key, request) except (ValueError, requests.exceptions.HTTPError): return HttpResponse(status=401) @@ -89,11 +96,6 @@ def shared_inbox(request): return HttpResponse() -def get_public_key(key_actor): - ''' try a stored key or load it from remote ''' - user = get_or_create_remote_user(key_actor) - return user.public_key - @app.task def handle_follow(activity): ''' someone wants to follow a local user ''' diff --git a/fedireads/remote_user.py b/fedireads/remote_user.py index 72a0391c3..9cf11889c 100644 --- a/fedireads/remote_user.py +++ b/fedireads/remote_user.py @@ -17,17 +17,7 @@ def get_or_create_remote_user(actor): except models.User.DoesNotExist: pass - # load the user's info from the actor url - response = requests.get( - actor, - headers={'Accept': 'application/activity+json'} - ) - if not response.ok: - response.raise_for_status() - data = response.json() - - # make sure our actor is who they say they are - assert actor == data['id'] + data = fetch_user_data(actor) actor_parts = urlparse(actor) with transaction.atomic(): @@ -43,6 +33,21 @@ def get_or_create_remote_user(actor): get_remote_reviews(user) return user +def fetch_user_data(actor): + # load the user's info from the actor url + response = requests.get( + actor, + headers={'Accept': 'application/activity+json'} + ) + if not response.ok: + response.raise_for_status() + data = response.json() + + # make sure our actor is who they say they are + if actor != data['id']: + raise ValueError("Remote actor id must match url.") + return data + def create_remote_user(data): ''' parse the activitypub actor data into a user ''' @@ -72,6 +77,24 @@ def create_remote_user(data): 'manuallyApprovesFollowers', False), ) +def refresh_remote_user(user): + data = fetch_user_data(user.remote_id) + + shared_inbox = data.get('endpoints').get('sharedInbox') if \ + data.get('endpoints') else None + + # TODO - I think dataclasses change will mean less repetition here later. + user.name = data.get('name') + user.summary = data.get('summary') + user.inbox = data['inbox'] #fail if there's no inbox + user.outbox = data['outbox'] # fail if there's no outbox + user.shared_inbox = shared_inbox + user.public_key = data.get('publicKey').get('publicKeyPem') + user.local = False + user.fedireads_user = data.get('fedireadsUser', False) + user.manually_approves_followers = data.get( + 'manuallyApprovesFollowers', False) + user.save() def get_avatar(data): ''' find the icon attachment and load the image from the remote sever ''' diff --git a/fedireads/tests/test_signing.py b/fedireads/tests/test_signing.py index 62bdc761f..0293a2d3c 100644 --- a/fedireads/tests/test_signing.py +++ b/fedireads/tests/test_signing.py @@ -98,6 +98,58 @@ class Signature(TestCase): response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 200) + @responses.activate + def test_key_needs_refresh(self): + datafile = pathlib.Path(__file__).parent.joinpath('data/ap_user.json') + data = json.loads(datafile.read_bytes()) + data['id'] = self.fake_remote.remote_id + data['publicKey']['publicKeyPem'] = self.fake_remote.public_key + del data['icon'] # Avoid having to return an avatar. + responses.add( + responses.GET, + self.fake_remote.remote_id, + json=data, + status=200) + responses.add( + responses.GET, + 'https://localhost/.well-known/nodeinfo', + status=404) + responses.add( + responses.GET, + 'https://example.com/user/mouse/outbox?page=true', + json={'orderedItems': []}, + status=200 + ) + + # Second and subsequent fetches get a different key: + new_private_key, new_public_key = create_key_pair() + new_sender = Sender( + self.fake_remote.remote_id, new_private_key, new_public_key) + data['publicKey']['publicKeyPem'] = new_public_key + responses.add( + responses.GET, + self.fake_remote.remote_id, + json=data, + status=200) + + + # Key correct: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 200) + + # Old key is cached, so still works: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 200) + + # Try with new key: + response = self.send_test_request(sender=new_sender) + self.assertEqual(response.status_code, 200) + + # Now the old key will fail: + response = self.send_test_request(sender=self.fake_remote) + self.assertEqual(response.status_code, 401) + + @responses.activate def test_nonexistent_signer(self): responses.add(