Skip to content

Commit

Permalink
authz: normalize user ids before comparing, in both receive and Objec…
Browse files Browse the repository at this point in the history
…t.get_or_create

for #566
  • Loading branch information
snarfed committed May 29, 2024
1 parent dd464ec commit bfbae4b
Show file tree
Hide file tree
Showing 6 changed files with 94 additions and 104 deletions.
13 changes: 9 additions & 4 deletions models.py
Original file line number Diff line number Diff line change
Expand Up @@ -939,12 +939,17 @@ def get_or_create(cls, id, authed_as=None, **props):
obj.new = False
orig_as1 = obj.as1
if orig_as1:
owners = (as1.get_ids(orig_as1, 'author')
+ as1.get_ids(orig_as1, 'actor'))
if not authed_as:
logger.warning(f'Auth: would cowardly refuse to overwrite {id} without checking actor')
elif authed_as not in owners + [id]:
logger.warning(f"Auth: {authed_as} isn't {id}'s author or actor: {owners}")
authed_as = 'https://un.known'
proto = PROTOCOLS.get(obj.source_protocol)
assert proto, obj.source_protocol
owners = [ids.normalize_user_id(id=id, proto=proto)
for id in (as1.get_ids(orig_as1, 'author')
+ as1.get_ids(orig_as1, 'actor')
+ [id])]
if ids.normalize_user_id(id=authed_as, proto=proto) not in owners:
logger.warning(f"Auth: {authed_as} isn't {id} 's author or actor: {owners}")
else:
obj = Object(id=id)
obj.new = True
Expand Down
33 changes: 14 additions & 19 deletions protocol.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
PROTOCOL_DOMAINS,
subdomain_wrap,
)
from ids import translate_object_id, translate_user_id
from ids import normalize_user_id, translate_object_id, translate_user_id
from models import Follower, get_originals, Object, PROTOCOLS, Target, User

SUPPORTED_TYPES = (
Expand Down Expand Up @@ -760,24 +760,17 @@ def receive(from_cls, obj, authed_as=None, internal=False):

if authed_as:
assert isinstance(authed_as, str)
authed_domain = util.domain_from_link(authed_as)

if re.match(DOMAIN_RE, authed_as):
# use domain_from_link on both to remove www. and other similar
# subdomains
if not util.domain_or_parent_in(util.domain_from_link(actor),
[util.domain_from_link(authed_as)]):
logger.warning(f"Auth: actor {actor} isn't web domain authed user {authed_as}")

elif util.domain_or_parent_in(authed_domain, NO_AUTH_DOMAINS):
logger.info(f"Auth_: we don't know how to authorize {authed_domain} activities yet")
return "Ignoring, sorry, we don't know how to authorize {authed_domain} activities yet. https://github.com/snarfed/bridgy-fed/issues/566", 204
authed_domain = util.domain_from_link(authed_as)
if util.domain_or_parent_in(authed_domain, NO_AUTH_DOMAINS):
error(f"Ignoring, sorry, we don't know how to authorize {authed_domain} activities yet. https://github.com/snarfed/bridgy-fed/issues/566", status=204)

elif actor != authed_as:
authed_as = normalize_user_id(id=authed_as, proto=from_cls)
actor = normalize_user_id(id=actor, proto=from_cls)
if actor != authed_as:
if ld_sig := obj.as1.get('signature'):
creator = ld_sig.get('creator', '')
logger.info(f'Auth_: ignoring activity with LD Signature from {creator}')
return "Ignoring, sorry, we don't verify LD Signatures yet. https://github.com/snarfed/bridgy-fed/issues/566", 204
error(f"Ignoring LD Signature from {creator} , sorry, we can't verify those yet. https://github.com/snarfed/bridgy-fed/issues/566", status=204)
logger.warning(f"Auth: actor {actor} isn't authed user {authed_as}")
else:
logger.warning(f"Auth: missing authed_as!")
Expand All @@ -801,14 +794,14 @@ def receive(from_cls, obj, authed_as=None, internal=False):

# write Object to datastore
orig = obj
obj = Object.get_or_create(id, **orig.to_dict(), authed_as=actor)
obj = Object.get_or_create(id, authed_as=actor, **orig.to_dict())
if orig.new is not None:
obj.new = orig.new
if orig.changed is not None:
obj.changed = orig.changed

# if this is a post, ie not an activity, wrap it in a create or update
obj = from_cls.handle_bare_object(obj)
obj = from_cls.handle_bare_object(obj, authed_as=authed_as)
obj.add('users', from_user.key)

if obj.type not in SUPPORTED_TYPES:
Expand Down Expand Up @@ -1137,13 +1130,14 @@ def maybe_delete_copy(copy_cls, user):
user=user.key.urlsafe())

@classmethod
def handle_bare_object(cls, obj):
def handle_bare_object(cls, obj, authed_as=None):
"""If obj is a bare object, wraps it in a create or update activity.
Checks if we've seen it before.
Args:
obj (models.Object)
authed_as (str): authenticated actor id who sent this activity
Returns:
models.Object: ``obj`` if it's an activity, otherwise a new object
Expand Down Expand Up @@ -1198,7 +1192,8 @@ def handle_bare_object(cls, obj):
}
logger.info(f'Wrapping in post: {json_dumps(create_as1, indent=2)}')
return Object.get_or_create(create_id, our_as1=create_as1,
source_protocol=obj.source_protocol)
source_protocol=obj.source_protocol,
authed_as=authed_as)

error(f'{obj.key.id()} is unchanged, nothing to do', status=204)

Expand Down
18 changes: 10 additions & 8 deletions tests/test_models.py
Original file line number Diff line number Diff line change
Expand Up @@ -532,14 +532,16 @@ def check(obj1, obj2):
self.assertEqual(3, Object.query().count())

# if no data property is set, don't clear existing data properties
obj7 = Object.get_or_create('biff', as2={'a': 'b'}, mf2={'c': 'd'})
obj7 = Object.get_or_create('biff', as2={'a': 'b'}, mf2={'c': 'd'},
source_protocol='ui')
Object.get_or_create('biff', users=[ndb.Key(Web, 'me')])
self.assert_object('biff', as2={'a': 'b'}, mf2={'c': 'd'},
users=[ndb.Key(Web, 'me')])
users=[ndb.Key(Web, 'me')],
source_protocol='ui')

def test_get_or_create_authed_as_check(self):
Object(id='foo', our_as1={'author': 'alice'}).put()
Object.get_or_create('foo', authed_as='alice', our_as1={
Object(id='foo', our_as1={'author': 'alice'}, source_protocol='ui').put()
Object.get_or_create('foo', authed_as='alice', source_protocol='ui', our_as1={
'author': 'alice',
'bar': 'baz',
})
Expand All @@ -552,15 +554,15 @@ def test_get_or_create_authed_as_check(self):
with self.assertLogs() as logs:
Object.get_or_create('foo', authed_as='eve', our_as1={'bar': 'biff'})

self.assertIn("WARNING:models:Auth: eve isn't foo's author or actor: ['alice']",
logs.output)
self.assertIn("Auth: eve isn't foo 's author or actor: ['alice', 'foo']",
' '.join(logs.output))

# actor is object id (eg user profile)
with self.assertLogs() as logs:
Object.get_or_create('foo', authed_as='foo', our_as1={})

self.assertNotIn("WARNING:models:Auth: foo isn't foo's author or actor: []",
logs.output)
self.assertNotIn("Auth: foo isn't foo's author or actor: []",
' '.join(logs.output))

def test_activity_changed(self):
obj = Object()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_pages.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,7 +293,7 @@ def test_followers_fake(self):
self.assert_equals(200, got.status_code)

def test_followers_activitypub(self):
obj = Object(id='https://inst/user', as2={
obj = Object(id='https://inst/user', source_protocol='activitypub', as2={
'id': 'https://inst/user',
'preferredUsername': 'user',
})
Expand Down
Loading

0 comments on commit bfbae4b

Please sign in to comment.