diff --git a/fedireads/incoming.py b/fedireads/incoming.py index 6ca51d0e..58da9292 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 @@ -44,17 +44,11 @@ def shared_inbox(request): if not activity.get('object'): return HttpResponseBadRequest() - try: - signature = Signature.parse(request) - - key_actor = urldefrag(signature.key_id).url - if key_actor != activity.get('actor'): - raise ValueError("Wrong actor created signature.") - - key = get_public_key(key_actor) - - signature.verify(key, request) - except ValueError: + 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) handlers = { @@ -89,22 +83,28 @@ def shared_inbox(request): return HttpResponse() -def get_public_key(key_actor): - ''' try a stored key or load it from remote ''' +def has_valid_signature(request, activity): try: - user = models.User.objects.get(remote_id=key_actor) - public_key = user.public_key - except models.User.DoesNotExist: - response = requests.get( - key_actor, - headers={'Accept': 'application/activity+json'} - ) - if not response.ok: - raise ValueError('Could not load public key') - user_data = response.json() - public_key = user_data['publicKey']['publicKeyPem'] + 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 = get_or_create_remote_user(key_actor) + + 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 False + return True - return public_key @app.task def handle_follow(activity): diff --git a/fedireads/remote_user.py b/fedireads/remote_user.py index 81c8c34c..9cf11889 100644 --- a/fedireads/remote_user.py +++ b/fedireads/remote_user.py @@ -17,6 +17,23 @@ def get_or_create_remote_user(actor): except models.User.DoesNotExist: pass + data = fetch_user_data(actor) + + actor_parts = urlparse(actor) + with transaction.atomic(): + user = create_remote_user(data) + user.federated_server = get_or_create_remote_server(actor_parts.netloc) + user.save() + + avatar = get_avatar(data) + if avatar: + user.avatar.save(*avatar) + + if user.fedireads_user: + get_remote_reviews(user) + return user + +def fetch_user_data(actor): # load the user's info from the actor url response = requests.get( actor, @@ -27,20 +44,9 @@ def get_or_create_remote_user(actor): data = response.json() # make sure our actor is who they say they are - assert actor == data['id'] - - actor_parts = urlparse(actor) - with transaction.atomic(): - user = create_remote_user(data) - user.federated_server = get_or_create_remote_server(actor_parts.netloc) - user.save() - - avatar = get_avatar(data) - user.avatar.save(*avatar) - - if user.fedireads_user: - get_remote_reviews(user) - return user + if actor != data['id']: + raise ValueError("Remote actor id must match url.") + return data def create_remote_user(data): @@ -71,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 b93d263e..0293a2d3 100644 --- a/fedireads/tests/test_signing.py +++ b/fedireads/tests/test_signing.py @@ -1,6 +1,7 @@ import time from collections import namedtuple from urllib.parse import urlsplit +import pathlib import json import responses @@ -73,17 +74,82 @@ class Signature(TestCase): @responses.activate def test_remote_signer(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={'publicKey': { - 'publicKeyPem': self.fake_remote.public_key - }}, + 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 + ) 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(