From b739bd4cee7691fa29b985618737effd83a0cd68 Mon Sep 17 00:00:00 2001 From: Ryan Barrett Date: Tue, 7 Feb 2023 18:25:24 -0800 Subject: [PATCH] return 404 on requests for unknown users instead of automatically creating them --- activitypub.py | 27 ++++++++---- common.py | 17 +++----- models.py | 2 +- tests/test_activitypub.py | 92 ++++++++++++++++++++++----------------- webmention.py | 7 ++- 5 files changed, 82 insertions(+), 63 deletions(-) diff --git a/activitypub.py b/activitypub.py index cad65d1..8ab33d2 100644 --- a/activitypub.py +++ b/activitypub.py @@ -14,7 +14,7 @@ from oauth_dropins.webutil.util import json_dumps, json_loads from app import app, cache import common -from common import CACHE_TIME, redirect_unwrap, redirect_wrap +from common import CACHE_TIME, redirect_unwrap, redirect_wrap, TLD_BLOCKLIST from models import Follower, Object, Target, User logger = logging.getLogger(__name__) @@ -40,7 +40,15 @@ SUPPORTED_TYPES = ( @flask_util.cached(cache, CACHE_TIME, http_5xx=True) def actor(domain): """Fetches a domain's h-card and converts to AS2 actor.""" - _, _, actor, _ = common.actor(domain) + tld = domain.split('.')[-1] + if tld in TLD_BLOCKLIST: + error('', status=404) + + user = User.get_by_id(domain) + if not user: + return f'User {domain} not found', 404 + + _, _, actor = common.actor(user) return (actor, { 'Content-Type': as2.CONTENT_TYPE, 'Access-Control-Allow-Origin': '*', @@ -54,6 +62,12 @@ def inbox(domain=None): body = request.get_data(as_text=True) logger.info(f'Got: {body}') + user = None + if domain: + user = User.get_by_id(domain) + if not user: + return f'User {domain} not found', 404 + # parse and validate AS2 activity try: activity = request.json @@ -108,8 +122,6 @@ def inbox(domain=None): ndb.put_multi(followers) return 'OK' - user = User.get_or_create(domain) if domain else None - # fetch actor if necessary so we have name, profile photo, etc if actor and isinstance(actor, str): actor = activity['actor'] = common.get_as2(actor, user=user).json() @@ -189,10 +201,7 @@ def accept_follow(follow, follow_unwrapped, user): follow_unwrapped.setdefault('url', f'{follower_url}#followed-{followee_url}') # store Follower - followee_domain = util.domain_from_link(followee_id, minimize=False) - # follow use_instead, if any - followee_domain = User.get_or_create(followee_domain).key.id() - follower = Follower.get_or_create(dest=followee_domain, src=follower_id, + follower = Follower.get_or_create(dest=user.key.id(), src=follower_id, last_follow=json_dumps(follow)) follower.status = 'active' follower.put() @@ -201,7 +210,7 @@ def accept_follow(follow, follow_unwrapped, user): accept = { '@context': 'https://www.w3.org/ns/activitystreams', 'id': util.tag_uri(common.PRIMARY_DOMAIN, - f'accept/{followee_domain}/{follow.get("id")}'), + f'accept/{user.key.id()}/{follow.get("id")}'), 'type': 'Accept', 'actor': followee, 'object': { diff --git a/common.py b/common.py index 53c78d2..8b73dea 100644 --- a/common.py +++ b/common.py @@ -593,7 +593,7 @@ def redirect_unwrap(val): return val -def actor(domain, user=None): +def actor(user): """Fetches a home page, converts its representative h-card to AS2 actor. Creates a User for the given domain if one doesn't already exist. @@ -601,15 +601,13 @@ def actor(domain, user=None): TODO: unify with webfinger.Actor Args: - domain: str - user: :class:`User`, optional + user: :class:`User` - Returns: (dict mf2 item, dict AS1 actor, dict AS2 actor, User) + Returns: (dict mf2 item, dict AS1 actor, dict AS2 actor) """ - tld = domain.split('.')[-1] - if tld in TLD_BLOCKLIST: - error('', status=404) + assert user + domain = user.key.id() url = f'https://{domain}/' try: mf2 = util.fetch_mf2(url, gateway=True) @@ -621,9 +619,6 @@ def actor(domain, user=None): if not hcard: error(f"Couldn't find a representative h-card (http://microformats.org/wiki/representative-hcard-parsing) on {mf2['url']}") - if not user: - user = User.get_or_create(domain) - actor_as1 = microformats2.json_to_object(hcard, rel_urls=mf2.get('rel-urls')) actor_as2 = postprocess_as2(as2.from_as1(actor_as1), user=user) actor_as2.update({ @@ -644,7 +639,7 @@ def actor(domain, user=None): }) logger.info(f'Generated AS2 actor: {json_dumps(actor_as2, indent=2)}') - return hcard, actor_as1, actor_as2, user + return hcard, actor_as1, actor_as2 def fetch_followers(domain, collection): diff --git a/models.py b/models.py index 848914a..d380f6c 100644 --- a/models.py +++ b/models.py @@ -219,7 +219,7 @@ class User(StringIdModel): # check home page try: - _, _, actor_as2, _ = common.actor(self.key.id(), user=self) + _, _, actor_as2 = common.actor(self) self.actor_as2 = json_dumps(actor_as2) self.has_hcard = True except (BadRequest, NotFound): diff --git a/tests/test_activitypub.py b/tests/test_activitypub.py index 3254ee2..36ac7e9 100644 --- a/tests/test_activitypub.py +++ b/tests/test_activitypub.py @@ -106,10 +106,10 @@ FOLLOW = { 'id': 'https://mastodon.social/6d1a', 'type': 'Follow', 'actor': ACTOR['id'], - 'object': 'https://www.realize.be/', + 'object': 'https://foo.com/', } FOLLOW_WRAPPED = copy.deepcopy(FOLLOW) -FOLLOW_WRAPPED['object'] = 'http://localhost/www.realize.be' +FOLLOW_WRAPPED['object'] = 'http://localhost/foo.com' FOLLOW_WITH_ACTOR = copy.deepcopy(FOLLOW) FOLLOW_WITH_ACTOR['actor'] = ACTOR FOLLOW_WRAPPED_WITH_ACTOR = copy.deepcopy(FOLLOW_WRAPPED) @@ -120,12 +120,12 @@ FOLLOW_WITH_OBJECT['object'] = ACTOR ACCEPT = { '@context': 'https://www.w3.org/ns/activitystreams', 'type': 'Accept', - 'id': 'tag:fed.brid.gy:accept/www.realize.be/https://mastodon.social/6d1a', - 'actor': 'http://localhost/www.realize.be', + 'id': 'tag:fed.brid.gy:accept/foo.com/https://mastodon.social/6d1a', + 'actor': 'http://localhost/foo.com', 'object': { 'type': 'Follow', 'actor': 'https://mastodon.social/users/swentel', - 'object': 'http://localhost/www.realize.be', + 'object': 'http://localhost/foo.com', } } @@ -171,6 +171,10 @@ UPDATE_NOTE = { @patch('requests.head') class ActivityPubTest(testutil.TestCase): + def setUp(self): + super().setUp() + User.get_or_create('foo.com') + def test_actor(self, _, mock_get, __): mock_get.return_value = requests_response(""" @@ -283,9 +287,17 @@ class ActivityPubTest(testutil.TestCase): mock_get.side_effect = [ ValueError('Invalid IPv6 URL'), ] - got = self.client.get('/snarfed.org]') + got = self.client.get('/foo.com') self.assertEqual(400, got.status_code) + def test_actor_no_user(self, *mocks): + got = self.client.get('/nope.com') + self.assertEqual(404, got.status_code) + + def test_individual_inbox_no_user(self, *mocks): + got = self.client.post('/nope.com/inbox', json=REPLY) + self.assertEqual(404, got.status_code) + def test_inbox_reply_object(self, *mocks): self._test_inbox_reply(REPLY_OBJECT, {'as2': REPLY_OBJECT, @@ -357,7 +369,7 @@ class ActivityPubTest(testutil.TestCase): mock_get.assert_not_called() mock_post.assert_not_called() - def test_personal_inbox_create_obj(self, *mocks): + def test_individual_inbox_create_obj(self, *mocks): self._test_inbox_create_obj('/foo.com/inbox', *mocks) def test_shared_inbox_create_obj(self, *mocks): @@ -498,15 +510,15 @@ class ActivityPubTest(testutil.TestCase): mock_head, mock_get, mock_post) follow = copy.deepcopy(FOLLOW_WITH_ACTOR) - follow['url'] = 'https://mastodon.social/users/swentel#followed-https://www.realize.be/' + follow['url'] = 'https://mastodon.social/users/swentel#followed-https://foo.com/' self.assert_object('https://mastodon.social/6d1a', - domains=['www.realize.be'], + domains=['foo.com'], source_protocol='activitypub', status='complete', as2=follow, as1=as2.to_as1(follow), - delivered=['https://www.realize.be/'], + delivered=['https://foo.com/'], type='follow', labels=['notification', 'activity'], object_ids=[FOLLOW['object']]) @@ -542,22 +554,22 @@ class ActivityPubTest(testutil.TestCase): follow.update({ 'actor': FOLLOW_WITH_ACTOR['actor'], 'object': unwrapped_user, - 'url': 'https://mastodon.social/users/swentel#followed-https://www.realize.be/', + 'url': 'https://mastodon.social/users/swentel#followed-https://foo.com/', }) self.assert_object('https://mastodon.social/6d1a', - domains=['www.realize.be'], + domains=['foo.com'], source_protocol='activitypub', status='complete', as2=follow, as1=as2.to_as1(follow), - delivered=['https://www.realize.be/'], + delivered=['https://foo.com/'], type='follow', labels=['notification', 'activity'], object_ids=[FOLLOW['object']]) def _test_inbox_follow_accept(self, follow_as2, accept_as2, mock_head, mock_get, mock_post): - mock_head.return_value = requests_response(url='https://www.realize.be/') + mock_head.return_value = requests_response(url='https://foo.com/') mock_get.side_effect = [ # source actor self.as2_resp(FOLLOW_WITH_ACTOR['actor']), @@ -582,21 +594,21 @@ class ActivityPubTest(testutil.TestCase): # check webmention args, kwargs = mock_post.call_args_list[1] - self.assertEqual(('https://www.realize.be/webmention',), args) + self.assertEqual(('https://foo.com/webmention',), args) self.assertEqual({ 'source': 'http://localhost/render?id=https%3A%2F%2Fmastodon.social%2F6d1a', - 'target': 'https://www.realize.be/', + 'target': 'https://foo.com/', }, kwargs['data']) # check that we stored a Follower object - follower = Follower.get_by_id(f'www.realize.be {FOLLOW["actor"]}') + follower = Follower.get_by_id(f'foo.com {FOLLOW["actor"]}') self.assertEqual('active', follower.status) def test_inbox_follow_use_instead_strip_www(self, mock_head, mock_get, mock_post): - root = User.get_or_create('realize.be') - User.get_or_create('www.realize.be', use_instead=root.key).put() + root = User.get_or_create('foo.com') + User.get_or_create('www.foo.com', use_instead=root.key).put() - mock_head.return_value = requests_response(url='https://www.realize.be/') + mock_head.return_value = requests_response(url='https://www.foo.com/') mock_get.side_effect = [ # source actor self.as2_resp(ACTOR), @@ -606,32 +618,32 @@ class ActivityPubTest(testutil.TestCase): mock_post.return_value = requests_response() follow = copy.deepcopy(FOLLOW_WRAPPED) - follow['object'] = 'http://localhost/realize.be' + follow['object'] = 'http://localhost/foo.com' got = self.client.post('/foo.com/inbox', json=follow) self.assertEqual(200, got.status_code) # check that the Follower doesn't have www - follower = Follower.get_by_id(f'realize.be {ACTOR["id"]}') + follower = Follower.get_by_id(f'foo.com {ACTOR["id"]}') self.assertEqual('active', follower.status) follow['actor'] = ACTOR self.assertEqual(follow, json_loads(follower.last_follow)) def test_inbox_undo_follow(self, mock_head, mock_get, mock_post): - mock_head.return_value = requests_response(url='https://www.realize.be/') + mock_head.return_value = requests_response(url='https://foo.com/') - Follower.get_or_create('www.realize.be', ACTOR['id']) + Follower.get_or_create('foo.com', ACTOR['id']) got = self.client.post('/foo.com/inbox', json=UNDO_FOLLOW_WRAPPED) self.assertEqual(200, got.status_code) - follower = Follower.get_by_id(f'www.realize.be {FOLLOW["actor"]}') + follower = Follower.get_by_id(f'foo.com {FOLLOW["actor"]}') self.assertEqual('inactive', follower.status) def test_inbox_follow_inactive(self, mock_head, mock_get, mock_post): - Follower.get_or_create('www.realize.be', ACTOR['id'], status='inactive') + Follower.get_or_create('foo.com', ACTOR['id'], status='inactive') - mock_head.return_value = requests_response(url='https://www.realize.be/') + mock_head.return_value = requests_response(url='https://foo.com/') mock_get.side_effect = [ # source actor self.as2_resp(FOLLOW_WITH_ACTOR['actor']), @@ -645,25 +657,25 @@ class ActivityPubTest(testutil.TestCase): self.assertEqual(200, got.status_code) # check that the Follower is now active - follower = Follower.get_by_id(f'www.realize.be {FOLLOW["actor"]}') + follower = Follower.get_by_id(f'foo.com {FOLLOW["actor"]}') self.assertEqual('active', follower.status) def test_inbox_undo_follow_doesnt_exist(self, mock_head, mock_get, mock_post): - mock_head.return_value = requests_response(url='https://realize.be/') + mock_head.return_value = requests_response(url='https://foo.com/') got = self.client.post('/foo.com/inbox', json=UNDO_FOLLOW_WRAPPED) self.assertEqual(200, got.status_code) def test_inbox_undo_follow_inactive(self, mock_head, mock_get, mock_post): - mock_head.return_value = requests_response(url='https://realize.be/') - Follower.get_or_create('realize.be', ACTOR['id'], status='inactive') + mock_head.return_value = requests_response(url='https://foo.com/') + Follower.get_or_create('foo.com', ACTOR['id'], status='inactive') got = self.client.post('/foo.com/inbox', json=UNDO_FOLLOW_WRAPPED) self.assertEqual(200, got.status_code) def test_inbox_undo_follow_composite_object(self, mock_head, mock_get, mock_post): - mock_head.return_value = requests_response(url='https://realize.be/') - Follower.get_or_create('realize.be', ACTOR['id'], status='inactive') + mock_head.return_value = requests_response(url='https://foo.com/') + Follower.get_or_create('foo.com', ACTOR['id'], status='inactive') undo_follow = copy.deepcopy(UNDO_FOLLOW_WRAPPED) undo_follow['object']['object'] = {'id': undo_follow['object']['object']} @@ -698,13 +710,13 @@ class ActivityPubTest(testutil.TestCase): def test_individual_inbox_delete_actor_noop(self, mock_head, mock_get, mock_post): """Deletes sent to individual users' inboxes do nothing.""" - follower = Follower.get_or_create('realize.be', DELETE['actor']) + follower = Follower.get_or_create('foo.com', DELETE['actor']) followee = Follower.get_or_create(DELETE['actor'], 'snarfed.org') # other unrelated follower - other = Follower.get_or_create('realize.be', 'https://mas.to/users/other') + other = Follower.get_or_create('foo.com', 'https://mas.to/users/other') self.assertEqual(3, Follower.query().count()) - got = self.client.post('/realize.be/inbox', json=DELETE) + got = self.client.post('/foo.com/inbox', json=DELETE) self.assertEqual(200, got.status_code) self.assertEqual('active', follower.key.get().status) self.assertEqual('active', followee.key.get().status) @@ -712,10 +724,10 @@ class ActivityPubTest(testutil.TestCase): def test_shared_inbox_delete_actor(self, mock_head, mock_get, mock_post): """Deletes sent to the shared inbox actually deactivate followers.""" - follower = Follower.get_or_create('realize.be', DELETE['actor']) + follower = Follower.get_or_create('foo.com', DELETE['actor']) followee = Follower.get_or_create(DELETE['actor'], 'snarfed.org') # other unrelated follower - other = Follower.get_or_create('realize.be', 'https://mas.to/users/other') + other = Follower.get_or_create('foo.com', 'https://mas.to/users/other') self.assertEqual(3, Follower.query().count()) got = self.client.post('/inbox', json=DELETE) @@ -768,7 +780,7 @@ class ActivityPubTest(testutil.TestCase): object_ids=[LIKE['object']]) def test_followers_collection_unknown_user(self, *args): - resp = self.client.get('/foo.com/followers') + resp = self.client.get('/nope.com/followers') self.assertEqual(404, resp.status_code) def test_followers_collection_empty(self, *args): @@ -836,7 +848,7 @@ class ActivityPubTest(testutil.TestCase): }, resp.json) def test_following_collection_unknown_user(self, *args): - resp = self.client.get('/foo.com/following') + resp = self.client.get('/nope.com/following') self.assertEqual(404, resp.status_code) def test_following_collection_empty(self, *args): diff --git a/webmention.py b/webmention.py index 94d4ee3..6f8ae0d 100644 --- a/webmention.py +++ b/webmention.py @@ -55,9 +55,12 @@ class Webmention(View): # if source is home page, send an actor Update to followers' instances if source.strip('/') == f'https://{self.source_domain}': + self.user = User.get_by_id(self.source_domain) + if not self.user: + return f'User {self.source_domain} not found', 404 + self.source_url = source - self.source_mf2, actor_as1, actor_as2, self.user = \ - common.actor(self.source_domain) + self.source_mf2, actor_as1, actor_as2 = common.actor(self.user) id = common.host_url(f'{source}#update-{util.now().isoformat()}') self.source_as1 = { 'objectType': 'activity',