From fa170253fe662c3ae2d8f6579f9a064281e4e06d Mon Sep 17 00:00:00 2001 From: Ryan Barrett Date: Mon, 12 Aug 2024 13:34:40 -0700 Subject: [PATCH] /convert/: check that object (if necessary) and user are bridged for #1248, https://github.com/snarfed/bridgy-fed/security/advisories/GHSA-6q9w-mpqc-j4j4 --- convert.py | 24 ++++++++++++++++-------- tests/test_convert.py | 43 +++++++++++++++++++++++++++++++++++++++---- 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/convert.py b/convert.py index dd7a2829..3c341243 100644 --- a/convert.py +++ b/convert.py @@ -77,18 +77,26 @@ def convert(dest, _, src=None): logger.info(f'{type} activity, redirecting to Object {obj_id}') return redirect(f'/{path_prefix}{obj_id}', code=301) - headers = { + # don't serve deletes or deleted objects + if obj.deleted or type == 'delete': + error('Deleted', status=410) + + # don't serve for a given protocol if we haven't bridged it there + if dest_cls.HAS_COPIES and not obj.get_copy(dest_cls): + error(f"{id} hasn't been bridged to {dest_cls.LABEL}", status=404) + + # check that owner has this protocol enabled + if owner := as1.get_owner(obj.as1): + user = src_cls.get_or_create(owner) + if not user or not user.is_enabled(dest_cls): + error(f"{src_cls.LABEL} user {owner} isn't bridged to {dest_cls.LABEL}", status=404) + + # convert and serve + return dest_cls.convert(obj), { 'Content-Type': dest_cls.CONTENT_TYPE, 'Vary': 'Accept', } - # don't serve deletes or deleted objects - if obj.deleted or type == 'delete': - return '', 410, headers - - # convert and serve - return dest_cls.convert(obj), headers - @app.get('/render') def render_redirect(): diff --git a/tests/test_convert.py b/tests/test_convert.py index 82d84b40..0cf411e8 100644 --- a/tests/test_convert.py +++ b/tests/test_convert.py @@ -5,13 +5,13 @@ from unittest.mock import patch from granary import as2 from granary.tests.test_as1 import ACTOR, COMMENT, DELETE_OF_ID, UPDATE -from models import Object +from models import Object, Target from oauth_dropins.webutil.testutil import requests_response from oauth_dropins.webutil.util import json_loads, parse_mf2 # import first so that Fake is defined before URL routes are registered from . import testutil -from .testutil import Fake, OtherFake +from .testutil import ExplicitEnableFake, Fake, OtherFake from activitypub import ActivityPub from common import CONTENT_TYPE_HTML @@ -92,16 +92,50 @@ class ConvertTest(testutil.TestCase): self.assertEqual(404, resp.status_code) def test_fake_to_other(self): - self.store_object(id='fake:post', our_as1={'foo': 'bar'}) + self.make_user('eefake:user', cls=ExplicitEnableFake, + enabled_protocols=['other']) + self.store_object(id='eefake:post', our_as1={'actor': 'eefake:user'}, + copies=[Target(protocol='other', uri='other:post')]) + resp = self.client.get('/convert/other/eefake:post', + base_url='https://eefake.brid.gy/') + self.assertEqual(200, resp.status_code) + self.assertEqual(OtherFake.CONTENT_TYPE, resp.content_type) + self.assertEqual({ + 'id': 'other:post', + 'actor': 'other:u:eefake:user', + }, json_loads(resp.get_data())) + + def test_fake_to_other_no_object_owner(self): + self.store_object(id='fake:post', our_as1={'foo': 'bar'}, + copies=[Target(protocol='other', uri='other:post')]) resp = self.client.get('/convert/other/fake:post', base_url='https://fa.brid.gy/') self.assertEqual(200, resp.status_code) self.assertEqual(OtherFake.CONTENT_TYPE, resp.content_type) self.assertEqual({ - 'id': 'other:o:fa:fake:post', + 'id': 'other:post', 'foo': 'bar', }, json_loads(resp.get_data())) + def test_fake_to_other_no_copy(self): + """https://github.com/snarfed/bridgy-fed/issues/1248""" + self.make_user('fake:user', cls=Fake, enabled_protocols=['other']) + self.store_object(id='fake:post', our_as1={'foo': 'bar'}, copies=[]) + + resp = self.client.get(f'/convert/other/fake:post', + base_url='https://fa.brid.gy/') + self.assertEqual(404, resp.status_code) + + def test_fake_to_other_user_not_enabled_protocol(self): + """https://github.com/snarfed/bridgy-fed/issues/1248""" + self.make_user('eefake:user', cls=ExplicitEnableFake, enabled_protocols=[]) + self.store_object(id='eefake:post', our_as1={'author': 'eefake:user'}, + copies=[Target(protocol='other', uri='other:post')]) + + resp = self.client.get(f'/convert/other/eefake:post', + base_url='https://eefake.brid.gy/') + self.assertEqual(404, resp.status_code) + def test_fake_to_activitypub(self): self.make_user('fake:alice', cls=Fake) self.store_object(id='fake:post', our_as1={ @@ -311,6 +345,7 @@ A ☕ reply hcard, hcard, hcard, + requests_response(''), # user for is_enabled protocol check ] resp = self.client.get(f'/convert/ap/https://nope.com/post',