Skip to content

Commit

Permalink
Object._pre_put_hook: require that protocol owns id
Browse files Browse the repository at this point in the history
  • Loading branch information
snarfed committed Jan 13, 2024
1 parent e39f67a commit 7941b63
Show file tree
Hide file tree
Showing 8 changed files with 92 additions and 82 deletions.
14 changes: 10 additions & 4 deletions models.py
Original file line number Diff line number Diff line change
Expand Up @@ -658,18 +658,24 @@ def _pre_put_hook(self):
* Set/remove the activity label
* Strip @context from as2 (we don't do LD) to save disk space
"""
assert '^^' not in self.key.id()
id = self.key.id()
assert '^^' not in id

if self.key.id().startswith('at://'):
repo, _, _ = parse_at_uri(self.key.id())
if self.source_protocol not in (None, 'ui'):
proto = PROTOCOLS[self.source_protocol]
assert proto.owns_id(id) is not False, \
f'Protocol {proto.LABEL} does not own id {id}'

if id.startswith('at://'):
repo, _, _ = parse_at_uri(id)
if not repo.startswith('did:'):
# TODO: if we hit this, that means the AppView gave us an AT URI
# with a handle repo/authority instead of DID. that's surprising!
# ...if so, and if we need to handle it, add a new
# arroba.did.canonicalize_at_uri() function, then use it here,
# or before.
raise ValueError(
f'at:// URI ids must have DID repos; got {self.key.id()}')
f'at:// URI ids must have DID repos; got {id}')

if self.as1 and self.as1.get('objectType') == 'activity':
# can't self.add because we're inside self.put, which has the lock
Expand Down
38 changes: 18 additions & 20 deletions tests/test_activitypub.py
Original file line number Diff line number Diff line change
Expand Up @@ -629,14 +629,14 @@ def test_inbox_reply_protocol_subdomain(self, mock_head, mock_get, mock_post):
})
reply = {
**REPLY_OBJECT,
'id': 'fake:my-reply',
'id': 'http://my/reply',
'inReplyTo': 'fake:post',
}

got = self.post('/ap/fake:user/inbox', json=reply,
base_url='https://fa.brid.gy/')
self.assertEqual(202, got.status_code)
self.assertEqual([('fake:my-reply#bridgy-fed-create', 'fake:post:target')],
self.assertEqual([('http://my/reply#bridgy-fed-create', 'fake:post:target')],
Fake.sent)

def test_inbox_reply_to_self_domain(self, mock_head, mock_get, mock_post):
Expand Down Expand Up @@ -670,7 +670,7 @@ def _test_inbox_create_obj(self, path, mock_head, mock_get, mock_post):
swentel = self.make_user('https://mas.to/users/swentel', cls=ActivityPub)
Follower.get_or_create(to=swentel, from_=self.user)
bar = self.make_user('fake:bar', cls=Fake, obj_id='fake:bar')
Follower.get_or_create(to=self.make_user('https://other.actor',
Follower.get_or_create(to=self.make_user('https://other/actor',
cls=ActivityPub),
from_=bar)
baz = self.make_user('fake:baz', cls=Fake, obj_id='fake:baz')
Expand Down Expand Up @@ -784,7 +784,7 @@ def test_shared_inbox_repost_of_fediverse(self, mock_head, mock_get, mock_post):
self.assert_object(REPOST['id'],
source_protocol='activitypub',
status='complete',
our_as1=as2.to_as1({**REPOST, 'actor': ACTOR}),
as2=REPOST,
users=[self.swentel_key],
feed=[self.user.key, baz.key],
delivered=['shared:target'],
Expand Down Expand Up @@ -1111,13 +1111,11 @@ def test_inbox_undo_follow(self, mock_head, mock_get, mock_post):
def test_inbox_follow_inactive(self, mock_head, mock_get, mock_post):
follower = Follower.get_or_create(
to=self.user,
from_=self.make_user(ACTOR['id'], cls=ActivityPub),
from_=self.make_user(ACTOR['id'], cls=ActivityPub, obj_as2=ACTOR),
status='inactive')

mock_head.return_value = requests_response(url='https://user.com/')
mock_get.side_effect = [
# source actor
self.as2_resp(FOLLOW_WITH_ACTOR['actor']),
test_web.ACTOR_HTML_RESP,
WEBMENTION_DISCOVERY,
]
Expand Down Expand Up @@ -1468,12 +1466,12 @@ def test_inbox_http_sig_is_not_actor_author(self, mock_head, mock_get, mock_post
with self.assertLogs() as logs:
got = self.post('/user.com/inbox', json={
**NOTE_OBJECT,
'author': 'https://alice',
'author': 'https://al/ice',
})
self.assertEqual(204, got.status_code, got.get_data(as_text=True))

self.assertIn(
"WARNING:protocol:actor https://alice isn't authed user http://my/key/id",
"WARNING:protocol:actor https://al/ice isn't authed user http://my/key/id",
logs.output)

def test_followers_collection_unknown_user(self, *_):
Expand Down Expand Up @@ -1504,32 +1502,32 @@ def store_followers(self):
from_=self.make_user('http://bar', cls=ActivityPub, obj_as2=ACTOR),
follow=follow)
Follower.get_or_create(
to=self.make_user('https://other.actor', cls=ActivityPub),
to=self.make_user('https://other/actor', cls=ActivityPub),
from_=self.user)
Follower.get_or_create(
to=self.user,
from_=self.make_user('http://baz', cls=ActivityPub, obj_as2=ACTOR),
follow=follow)
Follower.get_or_create(
to=self.user,
from_=self.make_user('http://baj', cls=Fake),
from_=self.make_user('fake:baj', cls=Fake),
status='inactive')

def test_followers_collection_fake(self, *_):
self.make_user('foo.com', cls=Fake)
self.make_user('fake:foo', cls=Fake)

resp = self.client.get('/ap/foo.com/followers',
resp = self.client.get('/ap/fake:foo/followers',
base_url='https://fa.brid.gy')
self.assertEqual(200, resp.status_code)
self.assertEqual({
'@context': 'https://www.w3.org/ns/activitystreams',
'id': 'https://fa.brid.gy/ap/foo.com/followers',
'id': 'https://fa.brid.gy/ap/fake:foo/followers',
'type': 'Collection',
'summary': "foo.com's followers",
'summary': "fake:foo's followers",
'totalItems': 0,
'first': {
'type': 'CollectionPage',
'partOf': 'https://fa.brid.gy/ap/foo.com/followers',
'partOf': 'https://fa.brid.gy/ap/fake:foo/followers',
'items': [],
},
}, resp.json)
Expand Down Expand Up @@ -1601,12 +1599,12 @@ def store_following(self):
follow=follow)
Follower.get_or_create(
to=self.user,
from_=self.make_user('https://other.actor', cls=ActivityPub))
from_=self.make_user('https://other/actor', cls=ActivityPub))
Follower.get_or_create(
to=self.make_user('http://baz', cls=ActivityPub, obj_as2=ACTOR),
from_=self.user, follow=follow)
Follower.get_or_create(
to=self.make_user('http://baj', cls=ActivityPub),
to=self.make_user('http://ba/j', cls=ActivityPub),
from_=self.user,
status='inactive')

Expand Down Expand Up @@ -2212,9 +2210,9 @@ def test_web_url(self):
self.assertEqual('http://my/url', user.web_url())

def test_handle(self):
user = self.make_user('http://foo', cls=ActivityPub)
user = self.make_user('http://foo/ey', cls=ActivityPub)
self.assertIsNone(user.handle)
self.assertEqual('http://foo', user.handle_or_id())
self.assertEqual('http://foo/ey', user.handle_or_id())

user.obj = Object(id='a', as2=ACTOR)
self.assertEqual('@[email protected]', user.handle)
Expand Down
6 changes: 3 additions & 3 deletions tests/test_convert.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,12 +157,12 @@ def test_activitypub_to_web_fetch_fails(self, mock_get):
mock_get.assert_has_calls((self.as2_req('http://foo'),))

def test_activitypub_to_web_with_author(self):
Object(id='http://foo', our_as1={**COMMENT, 'author': 'http://bar'},
Object(id='http://fo/o', our_as1={**COMMENT, 'author': 'http://ba/r'},
source_protocol='activitypub').put()
Object(id='http://bar', our_as1=ACTOR,
Object(id='http://ba/r', our_as1=ACTOR,
source_protocol='activitypub').put()

resp = self.client.get('/convert/web/http://foo',
resp = self.client.get('/convert/web/http://fo/o',
base_url='https://ap.brid.gy/')
self.assertEqual(200, resp.status_code)
self.assert_multiline_equals(AUTHOR_HTML, resp.get_data(as_text=True),
Expand Down
22 changes: 16 additions & 6 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ def test_private_pem(self):
def test_user_page_path(self):
self.assertEqual('/web/y.z', self.user.user_page_path())
self.assertEqual('/web/y.z/followers', self.user.user_page_path('followers'))
self.assertEqual('/fa/foo', self.make_user('foo', cls=Fake).user_page_path())

fake_foo = self.make_user('fake:foo', cls=Fake)
self.assertEqual('/fa/fake:handle:foo', fake_foo.user_page_path())

def test_user_link(self):
self.assert_multiline_equals("""\
Expand Down Expand Up @@ -486,8 +488,9 @@ def test_actor_link_user(self):
self.assertIn('Alice', got)

def test_actor_link_object_in_datastore(self):
Object(id='fake:alice', as2={"name": "Alice"}).put()
obj = Object(id='x', source_protocol='fake', our_as1={'actor': 'fake:alice'})
Object(id='fake:alice', as2={'name': 'Alice'}).put()
obj = Object(id='fake:bob', source_protocol='fake',
our_as1={'actor': 'fake:alice'})
self.assertIn('Alice', obj.actor_link())

def test_actor_link_no_image(self):
Expand Down Expand Up @@ -674,6 +677,13 @@ def test_put_strips_context(self):
'object': {},
}, obj.key.get().as2)

def test_put_requires_protocol_owns_id(self):
Object(id='asdf foo').put() # ok, no source protocol
Object(id='fake:foo', source_protocol='fake').put() # ok, valid id

with self.assertRaises(AssertionError):
Object(id='not a fake', source_protocol='fake').put()

def test_resolve_ids_empty(self):
obj = Object()
obj.resolve_ids()
Expand Down Expand Up @@ -839,8 +849,8 @@ class FollowerTest(TestCase):

def setUp(self):
super().setUp()
self.user = self.make_user('foo', cls=Fake)
self.other_user = self.make_user('bar', cls=Fake)
self.user = self.make_user('fake:foo', cls=Fake)
self.other_user = self.make_user('fake:bar', cls=Fake)

def test_from_to_same_type_fails(self):
with self.assertRaises(AssertionError):
Expand All @@ -861,7 +871,7 @@ def test_get_or_create(self):
self.assertEqual(1, Follower.query().count())

Follower.get_or_create(to=self.user, from_=self.other_user)
Follower.get_or_create(from_=self.user, to=self.make_user('baz', cls=Fake))
Follower.get_or_create(from_=self.user, to=self.make_user('fake:baz', cls=Fake))
self.assertEqual(3, Follower.query().count())

# check that kwargs get set on existing entity
Expand Down
12 changes: 6 additions & 6 deletions tests/test_pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@
from .test_web import ACTOR_AS2, REPOST_AS2

ACTOR_WITH_PREFERRED_USERNAME = {
**ACTOR,
'preferredUsername': 'me',
'url': 'https://plus.google.com/bob',
}


Expand Down Expand Up @@ -56,7 +56,7 @@ def test_user_fake(self):
self.assert_equals(200, got.status_code)

def test_user_page_handle(self):
user = self.make_user('http://foo', cls=ActivityPub,
user = self.make_user('http://fo/o', cls=ActivityPub,
obj_as2=ACTOR_WITH_PREFERRED_USERNAME)
self.assertEqual('@[email protected]', user.handle_as(ActivityPub))

Expand Down Expand Up @@ -167,9 +167,9 @@ def test_user_before_and_after(self):
def test_followers(self):
Follower.get_or_create(
to=self.user,
from_=self.make_user('http://unused', cls=ActivityPub, obj_as2={
from_=self.make_user('http://un/used', cls=ActivityPub, obj_as2={
**ACTOR,
'id': 'unused',
'id': 'http://un/used',
'url': 'http://stored/users/follow',
}))
Follower.get_or_create(
Expand Down Expand Up @@ -261,9 +261,9 @@ def test_followers_redirect(self):
def test_following(self):
Follower.get_or_create(
from_=self.user,
to=self.make_user('http://unused', cls=ActivityPub, obj_as2={
to=self.make_user('http://un/used', cls=ActivityPub, obj_as2={
**ACTOR,
'id': 'unused',
'id': 'http://un/used',
'url': 'http://stored/users/follow',
}))
Follower.get_or_create(
Expand Down
Loading

0 comments on commit 7941b63

Please sign in to comment.