kopia lustrzana https://github.com/snarfed/bridgy-fed
authorization: log when authed user doesn't match activity's author/actor
for #566. just logging for now, want to see if we're already hitting this at all.pull/687/head
rodzic
7466a000a5
commit
c83c77a73e
|
@ -328,13 +328,17 @@ class ActivityPub(User, Protocol):
|
||||||
def verify_signature(cls, activity):
|
def verify_signature(cls, activity):
|
||||||
"""Verifies the current request's HTTP Signature.
|
"""Verifies the current request's HTTP Signature.
|
||||||
|
|
||||||
Raises :class:`werkzeug.exceptions.HTTPError` if the
|
Raises :class:`werkzeug.exceptions.HTTPError` if the signature is
|
||||||
signature is missing or invalid, otherwise does nothing and returns None.
|
missing or invalid, otherwise does nothing and returns the id of the
|
||||||
|
actor whose key signed the request.
|
||||||
|
|
||||||
Logs details of the result.
|
Logs details of the result.
|
||||||
|
|
||||||
Args:
|
Args:
|
||||||
activity (dict): AS2 activity
|
activity (dict): AS2 activity
|
||||||
|
|
||||||
|
Returns:
|
||||||
|
str: signing AP actor id
|
||||||
"""
|
"""
|
||||||
headers = dict(request.headers) # copy so we can modify below
|
headers = dict(request.headers) # copy so we can modify below
|
||||||
sig = headers.get('Signature')
|
sig = headers.get('Signature')
|
||||||
|
@ -408,6 +412,8 @@ class ActivityPub(User, Protocol):
|
||||||
else:
|
else:
|
||||||
error('HTTP Signature verification failed', status=401)
|
error('HTTP Signature verification failed', status=401)
|
||||||
|
|
||||||
|
return keyId
|
||||||
|
|
||||||
|
|
||||||
def signed_get(url, **kwargs):
|
def signed_get(url, **kwargs):
|
||||||
return signed_request(util.requests_get, url, **kwargs)
|
return signed_request(util.requests_get, url, **kwargs)
|
||||||
|
@ -838,7 +844,7 @@ def inbox(protocol=None, id=None):
|
||||||
actor_obj = ActivityPub.load(actor_id)
|
actor_obj = ActivityPub.load(actor_id)
|
||||||
ActivityPub.get_or_create(actor_id, direct=True, obj=actor_obj)
|
ActivityPub.get_or_create(actor_id, direct=True, obj=actor_obj)
|
||||||
|
|
||||||
ActivityPub.verify_signature(activity)
|
authed_as = ActivityPub.verify_signature(activity)
|
||||||
|
|
||||||
# check that this activity is public. only do this for creates, not likes,
|
# check that this activity is public. only do this for creates, not likes,
|
||||||
# follows, or other activity types, since Mastodon doesn't currently mark
|
# follows, or other activity types, since Mastodon doesn't currently mark
|
||||||
|
@ -861,7 +867,7 @@ def inbox(protocol=None, id=None):
|
||||||
activity.setdefault('url', f'{follower_url}#followed-{followee_url}')
|
activity.setdefault('url', f'{follower_url}#followed-{followee_url}')
|
||||||
|
|
||||||
obj = Object(id=activity.get('id'), as2=redirect_unwrap(activity))
|
obj = Object(id=activity.get('id'), as2=redirect_unwrap(activity))
|
||||||
return ActivityPub.receive(obj)
|
return ActivityPub.receive(obj, authed_as=authed_as)
|
||||||
|
|
||||||
|
|
||||||
# protocol in subdomain
|
# protocol in subdomain
|
||||||
|
|
|
@ -479,7 +479,7 @@ class Protocol:
|
||||||
DOMAIN_BLOCKLIST + DOMAINS)
|
DOMAIN_BLOCKLIST + DOMAINS)
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def receive(from_cls, obj):
|
def receive(from_cls, obj, authed_as=None):
|
||||||
"""Handles an incoming activity.
|
"""Handles an incoming activity.
|
||||||
|
|
||||||
If ``obj``'s key is unset, ``obj.as1``'s id field is used. If both are
|
If ``obj``'s key is unset, ``obj.as1``'s id field is used. If both are
|
||||||
|
@ -528,9 +528,13 @@ class Protocol:
|
||||||
logger.info(msg)
|
logger.info(msg)
|
||||||
return msg, 204
|
return msg, 204
|
||||||
|
|
||||||
|
# authorization check
|
||||||
|
actor = as1.get_owner(obj.as1)
|
||||||
|
if authed_as and actor != authed_as:
|
||||||
|
logger.warning(f"actor {actor} isn't authed user {authed_as}")
|
||||||
|
|
||||||
# write Object to datastore
|
# write Object to datastore
|
||||||
orig = obj
|
orig = obj
|
||||||
actor = as1.get_owner(orig.as1)
|
|
||||||
obj = Object.get_or_create(id, **orig.to_dict(), actor=actor)
|
obj = Object.get_or_create(id, **orig.to_dict(), actor=actor)
|
||||||
if orig.new is not None:
|
if orig.new is not None:
|
||||||
obj.new = orig.new
|
obj.new = orig.new
|
||||||
|
|
|
@ -1342,6 +1342,22 @@ class ActivityPubTest(TestCase):
|
||||||
self.assertEqual(204, got.status_code)
|
self.assertEqual(204, got.status_code)
|
||||||
self.assertEqual(0, Follower.query().count())
|
self.assertEqual(0, Follower.query().count())
|
||||||
|
|
||||||
|
def test_inbox_http_sig_is_not_actor_author(self, mock_head, mock_get, mock_post):
|
||||||
|
mock_get.side_effect = [
|
||||||
|
self.as2_resp(ACTOR), # author
|
||||||
|
]
|
||||||
|
|
||||||
|
with self.assertLogs() as logs:
|
||||||
|
got = self.post('/user.com/inbox', json={
|
||||||
|
**NOTE_OBJECT,
|
||||||
|
'author': 'https://alice',
|
||||||
|
})
|
||||||
|
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",
|
||||||
|
logs.output)
|
||||||
|
|
||||||
def test_followers_collection_unknown_user(self, *_):
|
def test_followers_collection_unknown_user(self, *_):
|
||||||
resp = self.client.get('/nope.com/followers')
|
resp = self.client.get('/nope.com/followers')
|
||||||
self.assertEqual(404, resp.status_code)
|
self.assertEqual(404, resp.status_code)
|
||||||
|
|
|
@ -553,8 +553,6 @@ class ProtocolReceiveTest(TestCase):
|
||||||
}
|
}
|
||||||
self.store_object(id='fake:post', our_as1=post_as1)
|
self.store_object(id='fake:post', our_as1=post_as1)
|
||||||
|
|
||||||
orig_disable_level = logging.root.manager.disable
|
|
||||||
logging.disable(logging.NOTSET)
|
|
||||||
with self.assertRaises(NoContent), self.assertLogs() as logs:
|
with self.assertRaises(NoContent), self.assertLogs() as logs:
|
||||||
Fake.receive_as1({
|
Fake.receive_as1({
|
||||||
'id': 'fake:update',
|
'id': 'fake:update',
|
||||||
|
@ -563,7 +561,6 @@ class ProtocolReceiveTest(TestCase):
|
||||||
'actor': 'fake:other',
|
'actor': 'fake:other',
|
||||||
'object': post_as1,
|
'object': post_as1,
|
||||||
})
|
})
|
||||||
logging.disable(orig_disable_level)
|
|
||||||
|
|
||||||
self.assertIn(
|
self.assertIn(
|
||||||
"WARNING:models:actor fake:other isn't fake:post's author or actor ['fake:user']",
|
"WARNING:models:actor fake:other isn't fake:post's author or actor ['fake:user']",
|
||||||
|
@ -1415,3 +1412,22 @@ class ProtocolReceiveTest(TestCase):
|
||||||
})
|
})
|
||||||
|
|
||||||
self.assertEqual([], Fake.sent)
|
self.assertEqual([], Fake.sent)
|
||||||
|
|
||||||
|
def test_like_not_authed_as_actor(self):
|
||||||
|
Fake.fetchable['fake:post'] = {
|
||||||
|
'objectType': 'note',
|
||||||
|
'author': 'fake:bob',
|
||||||
|
}
|
||||||
|
|
||||||
|
with self.assertLogs() as logs:
|
||||||
|
Fake.receive_as1({
|
||||||
|
'id': 'fake:like',
|
||||||
|
'objectType': 'activity',
|
||||||
|
'verb': 'like',
|
||||||
|
'actor': 'fake:user',
|
||||||
|
'object': 'fake:post',
|
||||||
|
}, authed_as='fake:other')
|
||||||
|
|
||||||
|
self.assertIn(
|
||||||
|
"WARNING:protocol:actor fake:user isn't authed user fake:other",
|
||||||
|
logs.output)
|
||||||
|
|
|
@ -1637,6 +1637,27 @@ class WebTest(TestCase):
|
||||||
labels=['user', 'activity'],
|
labels=['user', 'activity'],
|
||||||
)
|
)
|
||||||
|
|
||||||
|
def test_like_actor_is_not_source_domain(self, mock_get, mock_post):
|
||||||
|
like_html = LIKE_HTML.replace(
|
||||||
|
'class="p-author h-card" href="https://user.com/"',
|
||||||
|
'class="p-author h-card" href="https://eve.com/"')
|
||||||
|
mock_get.side_effect = [
|
||||||
|
requests_response(like_html, url='https://user.com/like'),
|
||||||
|
TOOT_AS2,
|
||||||
|
ACTOR,
|
||||||
|
]
|
||||||
|
|
||||||
|
with self.assertLogs() as logs:
|
||||||
|
got = self.client.post('/queue/webmention', data={
|
||||||
|
'source': 'https://user.com/like',
|
||||||
|
'target': 'https://fed.brid.gy/',
|
||||||
|
})
|
||||||
|
self.assertEqual(200, got.status_code)
|
||||||
|
|
||||||
|
self.assertIn(
|
||||||
|
"WARNING:models:actor https://user.com/ isn't https://user.com/like's author or actor ['https://eve.com/']",
|
||||||
|
logs.output)
|
||||||
|
|
||||||
def _test_verify(self, redirects, hcard, actor, redirects_error=None):
|
def _test_verify(self, redirects, hcard, actor, redirects_error=None):
|
||||||
g.user.has_redirects = False
|
g.user.has_redirects = False
|
||||||
g.user.put()
|
g.user.put()
|
||||||
|
|
|
@ -132,14 +132,14 @@ class Fake(User, protocol.Protocol):
|
||||||
return 'shared:target' if shared else f'{obj.key.id()}:target'
|
return 'shared:target' if shared else f'{obj.key.id()}:target'
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def receive(cls, obj):
|
def receive(cls, obj, **kwargs):
|
||||||
assert isinstance(obj, Object)
|
assert isinstance(obj, Object)
|
||||||
return super().receive(obj=obj)
|
return super().receive(obj=obj, **kwargs)
|
||||||
|
|
||||||
@classmethod
|
@classmethod
|
||||||
def receive_as1(cls, our_as1):
|
def receive_as1(cls, our_as1, **kwargs):
|
||||||
assert isinstance(our_as1, dict)
|
assert isinstance(our_as1, dict)
|
||||||
return super().receive(Object(id=our_as1['id'], our_as1=our_as1))
|
return super().receive(Object(id=our_as1['id'], our_as1=our_as1), **kwargs)
|
||||||
|
|
||||||
|
|
||||||
class OtherFake(Fake):
|
class OtherFake(Fake):
|
||||||
|
@ -495,7 +495,8 @@ class TestCase(unittest.TestCase, testutil.Asserts):
|
||||||
with super().assertLogs() as logs:
|
with super().assertLogs() as logs:
|
||||||
yield logs
|
yield logs
|
||||||
finally:
|
finally:
|
||||||
logging.disable(orig_disable_level)
|
|
||||||
# emit logs that were captured
|
# emit logs that were captured
|
||||||
for record in logs.records:
|
for record in logs.records:
|
||||||
logging.getLogger().handle(record)
|
if record.levelno >= orig_disable_level:
|
||||||
|
logging.root.handle(record)
|
||||||
|
logging.disable(orig_disable_level)
|
||||||
|
|
Ładowanie…
Reference in New Issue