Skip to content

Commit

Permalink
Protocol.receive bug fix: preserve actor id if we fetch actor profile
Browse files Browse the repository at this point in the history
we were accidentally using the profile id, which broke some ATProto => AP delivery, #1720
  • Loading branch information
snarfed committed Jan 27, 2025
1 parent 893f8d1 commit 264905a
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 11 deletions.
9 changes: 8 additions & 1 deletion protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -1044,7 +1044,14 @@ def receive(from_cls, obj, authed_as=None, internal=False, received_at=None):
logger.debug('Fetching actor so we have name, profile photo, etc')
actor_obj = from_cls.load(actor['id'], raise_=False)
if actor_obj and actor_obj.as1:
obj.our_as1 = {**obj.as1, 'actor': actor_obj.as1}
obj.our_as1 = {
**obj.as1, 'actor': {
**actor_obj.as1,
# override profile id with actor id
# https://github.com/snarfed/bridgy-fed/issues/1720
'id': actor['id'],
}
}

# fetch object if necessary
if (obj.type in ('post', 'update', 'share')
Expand Down
2 changes: 1 addition & 1 deletion tests/test_follow.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ def test_callback_user_with_custom_username(self, mock_get, mock_post):
**as2.to_as1(unwrap(FOLLOW_URL)),
'actor': {
'objectType': 'person',
'id': 'https://alice.com/',
'id': 'alice.com',
'url': 'acct:[email protected]',
},
})
Expand Down
66 changes: 59 additions & 7 deletions tests/test_integrations.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,8 +113,61 @@ def firehose(self, **op):
assert atproto_firehose.commits.empty()

@patch('requests.post')
def test_atproto_notify_reply_to_activitypub(self, mock_post):
"""ATProto poll notifications, deliver reply to ActivityPub.
def test_atproto_post_to_activitypub(self, mock_post):
"""ATProto post, from firehose to ActivityPub.
ATProto original post at://did:plc:alice/app.bsky.feed.post/123
ActivityPub follower http://inst/bob
"""
self.store_object(id='did:plc:alice', raw=DID_DOC)
alice = self.make_user(
id='did:plc:alice',
cls=ATProto,
enabled_protocols=['activitypub'],
obj_bsky=test_atproto.ACTOR_PROFILE_BSKY,
# make sure we handle our_as1 with profile id ok
obj_as1={
**test_atproto.ACTOR_AS,
'id': 'at://did:plc:alice/app.bsky.actor.profile/self',
})

bob = self.make_ap_user('http://inst/bob')
Follower.get_or_create(to=alice, from_=bob)

# need at least one repo for firehose subscriber to load DIDs and run
Repo.create(self.storage, 'did:unused', signing_key=ATPROTO_KEY)

post = {
'$type': 'app.bsky.feed.post',
'text': 'I hereby post',
}
self.firehose(repo='did:plc:alice', action='create', seq=123,
path='app.bsky.feed.post/123', record=post)

self.assert_ap_deliveries(mock_post, ['http://inst/bob/inbox'],
from_user=alice, data={
'@context': 'https://www.w3.org/ns/activitystreams',
'type': 'Create',
'id': 'https://bsky.brid.gy/convert/ap/at://did:plc:alice/app.bsky.feed.post/123#bridgy-fed-create',
'actor': 'https://bsky.brid.gy/ap/did:plc:alice',
'published': '2022-01-02T03:04:05+00:00',
'object': {
'type': 'Note',
'id': 'https://bsky.brid.gy/convert/ap/at://did:plc:alice/app.bsky.feed.post/123',
'url': 'http://localhost/r/https://bsky.app/profile/did:plc:alice/post/123',
'attributedTo': 'https://bsky.brid.gy/ap/did:plc:alice',
'content': '<p>I hereby post</p>',
'contentMap': {'en': '<p>I hereby post</p>'},
'to': ['https://www.w3.org/ns/activitystreams#Public'],
},
'to': ['https://www.w3.org/ns/activitystreams#Public'],
})



@patch('requests.post')
def test_atproto_reply_to_activitypub(self, mock_post):
"""ATProto reply, from firehose to ActivityPub.
ActivityPub original post http://inst/post by bob
ATProto reply 123 by alice.com (did:plc:alice)
Expand Down Expand Up @@ -150,9 +203,8 @@ def test_atproto_notify_reply_to_activitypub(self, mock_post):
self.firehose(repo='did:plc:alice', action='create', seq=456,
path='app.bsky.feed.post/456', record=reply)

web_test = test_web.WebTest()
web_test.user = alice
web_test.assert_ap_deliveries(mock_post, ['http://inst/bob/inbox'], data={
self.assert_ap_deliveries(mock_post, ['http://inst/bob/inbox'],
from_user=alice, data={
'@context': 'https://www.w3.org/ns/activitystreams',
'type': 'Create',
'id': 'https://bsky.brid.gy/convert/ap/at://did:plc:alice/app.bsky.feed.post/456#bridgy-fed-create',
Expand All @@ -177,8 +229,8 @@ def test_atproto_notify_reply_to_activitypub(self, mock_post):

@patch('requests.post', return_value=requests_response(''))
@patch('requests.get', return_value=test_web.WEBMENTION_REL_LINK)
def test_atproto_follow_to_web(self, mock_get, mock_post):
"""ATProto poll notifications, deliver follow to Web.
def test_atproto_follow_of_web(self, mock_get, mock_post):
"""ATProto follow to Web.
ATProto user alice.com (did:plc:alice)
ATProto follow at://did:plc:alice/app.bsky.graph.follow/123
Expand Down
6 changes: 4 additions & 2 deletions tests/testutil.py
Original file line number Diff line number Diff line change
Expand Up @@ -630,7 +630,8 @@ def assert_task(self, mock_create_task, queue, eta_seconds=None, **params):
task=expected,
)

def assert_ap_deliveries(self, mock_post, inboxes, data, ignore=()):
def assert_ap_deliveries(self, mock_post, inboxes, data, from_user=None,
ignore=()):
self.assertEqual(len(inboxes), len(mock_post.call_args_list),
mock_post.call_args_list)

Expand All @@ -639,7 +640,8 @@ def assert_ap_deliveries(self, mock_post, inboxes, data, ignore=()):
self.assertEqual(as2.CONTENT_TYPE_LD_PROFILE,
kwargs['headers']['Content-Type'])
rsa_key = kwargs['auth'].header_signer._rsa._key
self.assertEqual(self.user.private_pem(), rsa_key.exportKey())
self.assertEqual((from_user or self.user).private_pem(),
rsa_key.exportKey())
calls[args[0]] = json_loads(kwargs['data'])

for inbox in inboxes:
Expand Down

0 comments on commit 264905a

Please sign in to comment.