From 55c8d9bec688e7b05ed11c9249473a2f497633dc Mon Sep 17 00:00:00 2001 From: Ryan Barrett Date: Mon, 16 Oct 2023 06:37:52 -0700 Subject: [PATCH] authorization: log updates, deletes, creates from the wrong actor for #566. just logging for now, want to see if we're already hitting this at all. --- models.py | 13 ++++++++++++- protocol.py | 5 +++-- tests/test_protocol.py | 25 +++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 3 deletions(-) diff --git a/models.py b/models.py index 18b32d5..08e6819 100644 --- a/models.py +++ b/models.py @@ -740,13 +740,18 @@ class Object(StringIdModel): @classmethod @ndb.transactional() - def get_or_create(cls, id, **props): + def get_or_create(cls, id, actor=None, **props): """Returns an :class:`Object` with the given property values. If a matching :class:`Object` doesn't exist in the datastore, creates it first. Only populates non-False/empty property values in props into the object. Also populates the :attr:`new` and :attr:`changed` properties. + Args: + actor (str): if a matching :class:`Object` already exists, its + `author` or `actor` must contain this actor id. Implements basic + authorization for updates and deletes. + Returns: Object: """ @@ -754,6 +759,12 @@ class Object(StringIdModel): if obj: obj.new = False orig_as1 = obj.as1 + if orig_as1: + if not actor: + logger.warning(f'Cowardly refusing to overwrite {id} without checking actor') + elif actor not in (as1.get_ids(orig_as1, 'author') + + as1.get_ids(orig_as1, 'actor')): + logger.warning(f"actor {actor} isn't {id}'s author or actor {authors_actors}") else: obj = Object(id=id) obj.new = True diff --git a/protocol.py b/protocol.py index 086e433..550957c 100644 --- a/protocol.py +++ b/protocol.py @@ -530,7 +530,8 @@ class Protocol: # write Object to datastore orig = obj - obj = Object.get_or_create(id, **orig.to_dict()) + actor = as1.get_owner(orig.as1) + obj = Object.get_or_create(id, **orig.to_dict(), actor=actor) if orig.new is not None: obj.new = orig.new if orig.changed is not None: @@ -564,7 +565,7 @@ class Protocol: inner_obj = None if obj.type in ('post', 'update') and inner_obj_as1.keys() > set(['id']): Object.get_or_create(inner_obj_id, our_as1=inner_obj_as1, - source_protocol=from_cls.LABEL) + source_protocol=from_cls.LABEL, actor=actor) actor = as1.get_object(obj.as1, 'actor') actor_id = actor.get('id') diff --git a/tests/test_protocol.py b/tests/test_protocol.py index b99aaf6..841db6d 100644 --- a/tests/test_protocol.py +++ b/tests/test_protocol.py @@ -1,5 +1,6 @@ """Unit tests for protocol.py.""" import copy +import logging from unittest import skip from unittest.mock import patch @@ -544,6 +545,30 @@ class ProtocolReceiveTest(TestCase): self.assertEqual(1, len(Fake.sent)) self.assertEqual('shared:target', Fake.sent[0][1]) + def test_update_post_wrong_actor_error(self): + post_as1 = { + 'id': 'fake:post', + 'objectType': 'note', + 'author': 'fake:user', + } + 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: + Fake.receive_as1({ + 'id': 'fake:update', + 'objectType': 'activity', + 'verb': 'update', + 'actor': 'fake:other', + 'object': post_as1, + }) + logging.disable(orig_disable_level) + + self.assertIn( + "WARNING:models:actor fake:other isn't fake:post's author or actor ['fake:user']", + logs.output) + def test_update_post(self): self.make_followers()