From 2fa8271af7d687558d6ea9d1d50236a4324fae5b Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Sun, 3 Mar 2019 02:51:49 +0200 Subject: [PATCH] Refactor handle_receive to require a RequestType The ActivityPub protocol handlers require access to HTTP headers, method and url from the incoming request. Thus require passing in a RequestType object, which has the same structure as a Django HttpRequest for compatibility. This is a breaking backwards compatible change requiring Diaspora payloads be wrapped in a RequestType object. Refs: #7 --- CHANGELOG.md | 9 ++--- federation/__init__.py | 14 +++++--- federation/inbound.py | 24 ++++++------- federation/protocols/activitypub/protocol.py | 30 +++++++++++----- federation/protocols/diaspora/protocol.py | 34 ++++++++++++------- .../protocols/activitypub/test_protocol.py | 19 ++++++----- .../tests/protocols/diaspora/test_protocol.py | 21 ++++++------ federation/tests/test_inbound.py | 5 +-- federation/types.py | 15 +++++++- 9 files changed, 104 insertions(+), 67 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d57229..55b3170 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,10 +18,6 @@ * Added network utility `network.fetch_host_ip` to fetch IP by hostname. -* Inbound helper utility `handle_receive` now also supports ActivityPub payloads. Protocol will be identified by looking at the payload contents. - -* Fetcher helper utility `retrieve_remote_profile` now also supports fetching ActivityPub profiles. Response will be a serialized protocol specific profile entity. - ### Changed * **Backwards incompatible.** Lowest compatible Python version is now 3.6. @@ -31,18 +27,17 @@ * Reversal of all the work previously done to use Diaspora URL format identifiers. Working with the Diaspora protocol now always requires using handles and GUID's as before the changes introduced in v0.15.0. It ended up impossible to construct a Diaspora URL in all cases in a way that apps only need to store one identifier. * The `id` and possible `target_id` are now either URL format identifiers (ActivityPub) or a handle or GUID (Diaspora, depending on entity). Additionally a new `actor_id` has been added which for ActivityPub is an URL and for Diaspora a handle. Note, Diaspora entities always have also the `guid`, `handle`, `target_guid` and `target_handle` as before v0.15.0, depending on the entity. When creating Diaspora entities, you must pass these in for sending to work. * The high level `fetchers.retrieve_remote_content` signature has changed. It now expects an `id` for fetching from AP protocol and `handle`, `guid` and `entity_type` to fetch from Diaspora. Additionally a `sender_key_fetcher` can be passed in as before to optimize public key fetching using a callable. - * The high level `fetchers.retrieve_remote_profile` signature has changed. It now expects an `id` for fetching from AP protocol and `handle` for fetching from Diaspora. Additionally a `sender_key_fetcher` can be passed in as before to optimize public key fetching using a callable. + * The high level `fetchers.retrieve_remote_profile` signature has changed. It now expects as first parameter an `id` which for ActivityPub objects is the URL ID and for Diaspora objects is the handle. Additionally a `sender_key_fetcher` can be passed in as before to optimize public key fetching using a callable. * The generator class `RFC7033Webfinger` now expects instead of an `id` the `handle` and `guid` of the profile. * NodeInfo2 parser now returns the admin user in `handle` format instead of a Diaspora format URL. * The high level inbound and outbound functions `inbound.handle_receive`, `outbound.handle_send` parameter `user` must now receive a `UserType` compatible object. This must have the attributes `id` and `private_key`. If Diaspora support is required then also `handle` and `guid` should exist. The type can be found as a class in `types.UserType`. + * The high level inbound function `inbound.handle_receive` first parameter has been changed to `request` which must be a `RequestType` compatible object. This must have the attribute `body` which corrresponds to the old `payload` parameter. For ActivityPub inbound requests the object must also contain `headers`, `method` and `url`. * The outbound function `outbound.handle_send` parameter `recipients` structure has changed. It must now for Diaspora contain either a `handle` (public delivery) or tuple of `handle, RSAPublicKey, guid` for private delivery. For AP delivery either `url ID` for public delivery or tuple of `url ID, RSAPublicKey` for private delivery. * **Backwards incompatible.** Generator `RFC3033Webfinger` and the related `rfc3033_webfinger_view` have been renamed to `RFC7033Webfinger` and `rfc7033_webfinger_view` to reflect the right RFC number. * Network helper utility `fetch_document` can now also take a dictionary of `headers`. They will be passed to the underlying `requests` method call as is. -* **Backwards incompatible.** * Fetcher helper utility `retrieve_remote_profile` parameter `handle` has been removed. Pass in the Diaspora protocol handle as the first parameter to fetch a Diaspora remote profile. - ### Removed * **Backwards incompatible.** Support for Legacy Diaspora payloads have been removed to reduce the amount of code needed to maintain while refactoring for ActivityPub. diff --git a/federation/__init__.py b/federation/__init__.py index 9ca8bbf..47c0f68 100644 --- a/federation/__init__.py +++ b/federation/__init__.py @@ -1,7 +1,11 @@ import importlib +from typing import Union, TYPE_CHECKING from federation.exceptions import NoSuitableProtocolFoundError +if TYPE_CHECKING: + from federation.types import RequestType + __version__ = "0.18.0-dev" PROTOCOLS = ( @@ -10,9 +14,10 @@ PROTOCOLS = ( ) -def identify_protocol(method: str, value: str): +def identify_protocol(method, value): + # type: (str, Union[str, RequestType]) -> str """ - Loop through protocols, import the protocol module and try to identify the id or payload. + Loop through protocols, import the protocol module and try to identify the id or request. """ for protocol_name in PROTOCOLS: protocol = importlib.import_module(f"federation.protocols.{protocol_name}.protocol") @@ -26,5 +31,6 @@ def identify_protocol_by_id(id: str): return identify_protocol('id', id) -def identify_protocol_by_payload(payload: str): - return identify_protocol('payload', payload) +def identify_protocol_by_request(request): + # type: (RequestType) -> str + return identify_protocol('request', request) diff --git a/federation/inbound.py b/federation/inbound.py index 8ed26c1..3ed8c8b 100644 --- a/federation/inbound.py +++ b/federation/inbound.py @@ -2,19 +2,19 @@ import importlib import logging from typing import Tuple, List, Callable -from federation import identify_protocol_by_payload -from federation.types import UserType +from federation import identify_protocol_by_request +from federation.types import UserType, RequestType logger = logging.getLogger("federation") def handle_receive( - payload: str, - user: UserType=None, - sender_key_fetcher: Callable[[str], str]=None, - skip_author_verification: bool=False + request: RequestType, + user: UserType = None, + sender_key_fetcher: Callable[[str], str] = None, + skip_author_verification: bool = False ) -> Tuple[str, str, List]: - """Takes a payload and passes it to the correct protocol. + """Takes a request and passes it to the correct protocol. Returns a tuple of: - sender id @@ -22,23 +22,23 @@ def handle_receive( - list of entities NOTE! The returned sender is NOT necessarily the *author* of the entity. By sender here we're - talking about the sender of the *payload*. If this object is being relayed by the sender, the author + talking about the sender of the *request*. If this object is being relayed by the sender, the author could actually be a different identity. - :arg payload: Payload blob (str) + :arg request: Request object of type RequestType - note not a HTTP request even though the structure is similar :arg user: User that will be passed to `protocol.receive` (only required on private encrypted content) MUST have a `private_key` and `id` if given. :arg sender_key_fetcher: Function that accepts sender handle and returns public key (optional) :arg skip_author_verification: Don't verify sender (test purposes, false default) :returns: Tuple of sender id, protocol name and list of entity objects """ - logger.debug("handle_receive: processing payload: %s", payload) - found_protocol = identify_protocol_by_payload(payload) + logger.debug("handle_receive: processing request: %s", request) + found_protocol = identify_protocol_by_request(request) logger.debug("handle_receive: using protocol %s", found_protocol.PROTOCOL_NAME) protocol = found_protocol.Protocol() sender, message = protocol.receive( - payload, user, sender_key_fetcher, skip_author_verification=skip_author_verification) + request, user, sender_key_fetcher, skip_author_verification=skip_author_verification) logger.debug("handle_receive: sender %s, message %s", sender, message) mappers = importlib.import_module("federation.entities.%s.mappers" % found_protocol.PROTOCOL_NAME) diff --git a/federation/protocols/activitypub/protocol.py b/federation/protocols/activitypub/protocol.py index a550c24..d292dcb 100644 --- a/federation/protocols/activitypub/protocol.py +++ b/federation/protocols/activitypub/protocol.py @@ -1,10 +1,10 @@ import json import logging import re -from typing import Union, Callable, Tuple +from typing import Callable, Tuple from federation.entities.activitypub.enums import ActorType -from federation.types import UserType +from federation.types import UserType, RequestType from federation.utils.text import decode_if_bytes logger = logging.getLogger('federation') @@ -19,12 +19,13 @@ def identify_id(id: str) -> bool: return re.match(r'^https?://', id, flags=re.IGNORECASE) is not None -def identify_payload(payload: Union[str, bytes]) -> bool: +def identify_request(request: RequestType) -> bool: """ - Try to identify whether this is an ActivityPub payload. + Try to identify whether this is an ActivityPub request. """ + # noinspection PyBroadException try: - data = json.loads(decode_if_bytes(payload)) + data = json.loads(decode_if_bytes(request.body)) if "@context" in data: return True except Exception: @@ -33,22 +34,33 @@ def identify_payload(payload: Union[str, bytes]) -> bool: class Protocol: + actor = None + get_contact_key = None + payload = None + request = None + user = None + def extract_actor(self): if self.payload.get('type') in ActorType.values(): self.actor = self.payload.get('id') else: self.actor = self.payload.get('actor') - def receive(self, payload: str, user: UserType=None, sender_key_fetcher: Callable[[str], str]=None, - skip_author_verification: bool=False) -> Tuple[str, dict]: + def receive( + self, + request: RequestType, + user: UserType = None, + sender_key_fetcher: Callable[[str], str] = None, + skip_author_verification: bool = False) -> Tuple[str, dict]: """ - Receive a payload. + Receive a request. For testing purposes, `skip_author_verification` can be passed. Authorship will not be verified. """ self.user = user self.get_contact_key = sender_key_fetcher - self.payload = json.loads(decode_if_bytes(payload)) + self.payload = json.loads(decode_if_bytes(request.body)) + self.request = request self.extract_actor() # Verify the message is from who it claims to be if not skip_author_verification: diff --git a/federation/protocols/diaspora/protocol.py b/federation/protocols/diaspora/protocol.py index 7483948..60875b4 100644 --- a/federation/protocols/diaspora/protocol.py +++ b/federation/protocols/diaspora/protocol.py @@ -11,7 +11,7 @@ from federation.entities.mixins import BaseEntity from federation.exceptions import EncryptedMessageError, NoSenderKeyFoundError from federation.protocols.diaspora.encrypted import EncryptedPayload from federation.protocols.diaspora.magic_envelope import MagicEnvelope -from federation.types import UserType +from federation.types import UserType, RequestType from federation.utils.diaspora import fetch_public_key from federation.utils.text import decode_if_bytes, encode_if_text, validate_handle @@ -29,21 +29,22 @@ def identify_id(id: str) -> bool: return validate_handle(id) -def identify_payload(payload): - """Try to identify whether this is a Diaspora payload. +# noinspection PyBroadException +def identify_request(request: RequestType): + """Try to identify whether this is a Diaspora request. Try first public message. Then private message. The check if this is a legacy payload. """ # Private encrypted JSON payload try: - data = json.loads(decode_if_bytes(payload)) + data = json.loads(decode_if_bytes(request.body)) if "encrypted_magic_envelope" in data: return True except Exception: pass # Public XML payload try: - xml = etree.fromstring(encode_if_text(payload)) + xml = etree.fromstring(encode_if_text(request.body)) if xml.tag == MAGIC_ENV_TAG: return True except Exception: @@ -56,9 +57,15 @@ class Protocol: Original legacy implementation mostly taken from Pyaspora (https://github.com/lukeross/pyaspora). """ + content = None + doc = None + get_contact_key = None + user = None + sender_handle = None + def get_json_payload_magic_envelope(self, payload): """Encrypted JSON payload""" - private_key = self._get_user_key(self.user) + private_key = self._get_user_key() return EncryptedPayload.decrypt(payload=payload, private_key=private_key) def store_magic_envelope_doc(self, payload): @@ -75,17 +82,18 @@ class Protocol: logger.debug("diaspora.protocol.store_magic_envelope_doc: json payload: %s", json_payload) self.doc = self.get_json_payload_magic_envelope(json_payload) - def receive(self, - payload: str, - user: UserType=None, - sender_key_fetcher: Callable[[str], str]=None, - skip_author_verification: bool=False) -> Tuple[str, str]: + def receive( + self, + request: RequestType, + user: UserType = None, + sender_key_fetcher: Callable[[str], str] = None, + skip_author_verification: bool = False) -> Tuple[str, str]: """Receive a payload. For testing purposes, `skip_author_verification` can be passed. Authorship will not be verified.""" self.user = user self.get_contact_key = sender_key_fetcher - self.store_magic_envelope_doc(payload) + self.store_magic_envelope_doc(request.body) # Open payload and get actual message self.content = self.get_message_content() # Get sender handle @@ -95,7 +103,7 @@ class Protocol: self.verify_signature() return self.sender_handle, self.content - def _get_user_key(self, user): + def _get_user_key(self): if not getattr(self.user, "private_key", None): raise EncryptedMessageError("Cannot decrypt private message without user key") return self.user.private_key diff --git a/federation/tests/protocols/activitypub/test_protocol.py b/federation/tests/protocols/activitypub/test_protocol.py index f7d0098..b449155 100644 --- a/federation/tests/protocols/activitypub/test_protocol.py +++ b/federation/tests/protocols/activitypub/test_protocol.py @@ -1,6 +1,7 @@ import json -from federation.protocols.activitypub.protocol import identify_payload, identify_id +from federation.protocols.activitypub.protocol import identify_request, identify_id +from federation.types import RequestType def test_identify_id(): @@ -11,12 +12,12 @@ def test_identify_id(): assert identify_id('https://foobar@example.com') is True -class TestIdentifyPayload: - def test_identifies_activitypub_payload(self): - assert identify_payload(json.dumps('{"@context": "foo"}')) - assert identify_payload(json.dumps('{"@context": "foo"}').encode('utf-8')) +class TestIdentifyRequest: + def test_identifies_activitypub_request(self): + assert identify_request(RequestType(body=json.dumps('{"@context": "foo"}'))) + assert identify_request(RequestType(body=json.dumps('{"@context": "foo"}').encode('utf-8'))) - def test_passes_gracefully_for_non_activitypub_payload(self): - assert not identify_payload('foo') - assert not identify_payload('') - assert not identify_payload(b'') + def test_passes_gracefully_for_non_activitypub_request(self): + assert not identify_request(RequestType(body='foo')) + assert not identify_request(RequestType(body='')) + assert not identify_request(RequestType(body=b'')) diff --git a/federation/tests/protocols/diaspora/test_protocol.py b/federation/tests/protocols/diaspora/test_protocol.py index 8067815..6efc0ef 100644 --- a/federation/tests/protocols/diaspora/test_protocol.py +++ b/federation/tests/protocols/diaspora/test_protocol.py @@ -6,9 +6,10 @@ import pytest from federation.entities.diaspora.entities import DiasporaPost from federation.entities.diaspora.mappers import get_outbound_entity from federation.exceptions import NoSenderKeyFoundError, SignatureVerificationError -from federation.protocols.diaspora.protocol import Protocol, identify_payload +from federation.protocols.diaspora.protocol import Protocol, identify_request from federation.tests.fixtures.keys import PUBKEY, get_dummy_private_key from federation.tests.fixtures.payloads import DIASPORA_PUBLIC_PAYLOAD, DIASPORA_ENCRYPTED_PAYLOAD +from federation.types import RequestType class MockUser: @@ -46,7 +47,7 @@ class TestDiasporaProtocol(DiasporaTestBase): protocol = self.init_protocol() user = self.get_mock_user() protocol.get_message_content = self.mock_get_message_content - sender, content = protocol.receive(DIASPORA_PUBLIC_PAYLOAD, user, mock_get_contact_key, + sender, content = protocol.receive(RequestType(body=DIASPORA_PUBLIC_PAYLOAD), user, mock_get_contact_key, skip_author_verification=True) assert sender == "foobar@example.com" assert content == "" @@ -56,7 +57,7 @@ class TestDiasporaProtocol(DiasporaTestBase): protocol = self.init_protocol() user = self.get_mock_user() with pytest.raises(NoSenderKeyFoundError): - protocol.receive(DIASPORA_PUBLIC_PAYLOAD, user, mock_not_found_get_contact_key) + protocol.receive(RequestType(body=DIASPORA_PUBLIC_PAYLOAD), user, mock_not_found_get_contact_key) assert not mock_fetch.called @patch("federation.protocols.diaspora.protocol.fetch_public_key", autospec=True, return_value=None) @@ -64,7 +65,7 @@ class TestDiasporaProtocol(DiasporaTestBase): protocol = self.init_protocol() user = self.get_mock_user() with pytest.raises(NoSenderKeyFoundError): - protocol.receive(DIASPORA_PUBLIC_PAYLOAD, user) + protocol.receive(RequestType(body=DIASPORA_PUBLIC_PAYLOAD), user) mock_fetch.assert_called_once_with("foobar@example.com") @patch("federation.protocols.diaspora.protocol.MagicEnvelope", autospec=True) @@ -72,7 +73,7 @@ class TestDiasporaProtocol(DiasporaTestBase): def test_receive_creates_and_verifies_magic_envelope_instance(self, mock_fetch, mock_env): protocol = self.init_protocol() user = self.get_mock_user() - protocol.receive(DIASPORA_PUBLIC_PAYLOAD, user) + protocol.receive(RequestType(body=DIASPORA_PUBLIC_PAYLOAD), user) mock_env.assert_called_once_with(doc=protocol.doc, public_key="key", verify=True) @patch("federation.protocols.diaspora.protocol.fetch_public_key", autospec=True) @@ -81,7 +82,7 @@ class TestDiasporaProtocol(DiasporaTestBase): protocol = self.init_protocol() user = self.get_mock_user() with pytest.raises(SignatureVerificationError): - protocol.receive(DIASPORA_PUBLIC_PAYLOAD, user) + protocol.receive(RequestType(body=DIASPORA_PUBLIC_PAYLOAD), user) def test_get_message_content(self): protocol = self.init_protocol() @@ -90,14 +91,14 @@ class TestDiasporaProtocol(DiasporaTestBase): assert body == b"bar" def test_identify_payload_with_diaspora_public_payload(self): - assert identify_payload(DIASPORA_PUBLIC_PAYLOAD) == True - assert identify_payload(bytes(DIASPORA_PUBLIC_PAYLOAD, encoding="utf-8")) == True + assert identify_request(RequestType(body=DIASPORA_PUBLIC_PAYLOAD)) is True + assert identify_request(RequestType(body=bytes(DIASPORA_PUBLIC_PAYLOAD, encoding="utf-8"))) is True def test_identify_payload_with_diaspora_encrypted_payload(self): - assert identify_payload(DIASPORA_ENCRYPTED_PAYLOAD) == True + assert identify_request(RequestType(body=DIASPORA_ENCRYPTED_PAYLOAD)) is True def test_identify_payload_with_other_payload(self): - assert identify_payload("foobar not a diaspora protocol") == False + assert identify_request(RequestType(body="foobar not a diaspora protocol")) is False @patch("federation.protocols.diaspora.protocol.MagicEnvelope") def test_build_send_does_right_calls(self, mock_me): diff --git a/federation/tests/test_inbound.py b/federation/tests/test_inbound.py index 47f61fa..0d7173f 100644 --- a/federation/tests/test_inbound.py +++ b/federation/tests/test_inbound.py @@ -6,11 +6,12 @@ from federation.exceptions import NoSuitableProtocolFoundError from federation.inbound import handle_receive from federation.protocols.diaspora.protocol import Protocol from federation.tests.fixtures.payloads import DIASPORA_PUBLIC_PAYLOAD +from federation.types import RequestType class TestHandleReceiveProtocolIdentification: def test_handle_receive_routes_to_identified_protocol(self): - payload = DIASPORA_PUBLIC_PAYLOAD + payload = RequestType(body=DIASPORA_PUBLIC_PAYLOAD) with patch.object( Protocol, 'receive', @@ -22,6 +23,6 @@ class TestHandleReceiveProtocolIdentification: assert mock_receive.called def test_handle_receive_raises_on_unidentified_protocol(self): - payload = "foobar" + payload = RequestType(body="foobar") with pytest.raises(NoSuitableProtocolFoundError): handle_receive(payload) diff --git a/federation/types.py b/federation/types.py index bd19cac..bff11e8 100644 --- a/federation/types.py +++ b/federation/types.py @@ -1,8 +1,21 @@ -from typing import Optional +from typing import Optional, Dict, Union import attr +@attr.s +class RequestType: + """ + Emulates structure of a Django HttpRequest for compatibility. + """ + body: Union[str, bytes] = attr.ib() + + # Required when dealing with incoming AP payloads + headers: Dict = attr.ib(default=None) + method: str = attr.ib(default=None) + url: str = attr.ib(default=None) + + @attr.s class UserType: id: str = attr.ib()