kopia lustrzana https://gitlab.com/jaywink/federation
Validate entities that are created through message_to_objects
To safeguard invalid entities being passed on to library users, validate entities that are created in the message_to_objects calls. Any failures are logged as errors. This means for Diaspora Profile messages we must fetch the remote GUID from the hcard. Diaspora Profile XML message does not contain the guid but our data structure enforces GUID as a required attribute. This ensures library users will get a full profile back always. Implement in the Diaspora entities a hook fill_extra_attributes that is called in message_to_objects.merge-requests/130/head
Jason Robinson
rodzic
b135d9b8a4
commit
0f2c97adf8
10
CHANGELOG.md
10
CHANGELOG.md
|
|
@ -1,5 +1,15 @@
|
|||
## [unreleased]
|
||||
|
||||
### Backwards incompatible changes
|
||||
* Made `guid` mandatory for `Profile` entity. Library users should always be able to get a full validated object as we consider `guid` a core attribute of a profile.
|
||||
* Always validate entities created through `federation.entities.diaspora.mappers.message_to_objects`. This is the code that transforms federation messages for the Diaspora protocol to actual entity objects. Previously no validation was done and callers of `federation.inbound.handle_receive` received entities that were not always valid, for example they were missing a `guid`. Now validation is done in the conversion stage and errors are pushed to the `social-federation` logger in the event of invalid messages.
|
||||
* Note Diaspora Profile XML messages do not provide a GUID. This is handled internally by fetching the guid from the remote hCard so that a valid `Profile` entity can be created.
|
||||
|
||||
### Added
|
||||
* Raise a warning if unknown parameters are passed to entities.
|
||||
* Ensure entity required attributes are validated for `None` or empty string values. Required attributes must not only exist but also have a value.
|
||||
* Add validation to entities with the attribute `public`. Only `bool` values are accepted.
|
||||
|
||||
### Changed
|
||||
* Function `federation.utils.diaspora.parse_profile_from_hcard` now requires a second argument, `handle`. Since in the future Diaspora hCard is not guaranteed to have username and domain, we now pass handle to the parser directly.
|
||||
|
||||
|
|
|
|||
|
|
@ -3,6 +3,7 @@ from lxml import etree
|
|||
|
||||
from federation.entities.base import Comment, Post, Reaction, Relationship, Profile
|
||||
from federation.entities.diaspora.utils import format_dt, struct_to_xml, get_base_attributes
|
||||
from federation.utils.diaspora import retrieve_and_parse_profile
|
||||
|
||||
|
||||
class DiasporaEntityMixin(object):
|
||||
|
|
@ -10,6 +11,20 @@ class DiasporaEntityMixin(object):
|
|||
def from_base(cls, entity):
|
||||
return cls(**get_base_attributes(entity))
|
||||
|
||||
@staticmethod
|
||||
def fill_extra_attributes(attributes):
|
||||
"""Implement in subclasses to fill extra attributes when an XML is transformed to an object.
|
||||
|
||||
This is called just before initializing the entity.
|
||||
|
||||
Args:
|
||||
attributes (dict) - Already transformed attributes that will be passed to entity create.
|
||||
|
||||
Returns:
|
||||
Must return the attributes dictionary, possibly with changed or additional values.
|
||||
"""
|
||||
return attributes
|
||||
|
||||
|
||||
class DiasporaComment(DiasporaEntityMixin, Comment):
|
||||
"""Diaspora comment."""
|
||||
|
|
@ -97,3 +112,12 @@ class DiasporaProfile(DiasporaEntityMixin, Profile):
|
|||
{"tag_string": " ".join(["#%s" % tag for tag in self.tag_list])},
|
||||
])
|
||||
return element
|
||||
|
||||
@staticmethod
|
||||
def fill_extra_attributes(attributes):
|
||||
"""Diaspora Profile XML message contains no GUID. We need the guid. Fetch it."""
|
||||
if not attributes.get("handle"):
|
||||
raise ValueError("Can't fill GUID for profile creation since there is no handle! Attrs: %s" % attributes)
|
||||
profile = retrieve_and_parse_profile(attributes.get("handle"))
|
||||
attributes["guid"] = profile.guid
|
||||
return attributes
|
||||
|
|
|
|||
|
|
@ -1,11 +1,15 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
import logging
|
||||
from datetime import datetime
|
||||
|
||||
from lxml import etree
|
||||
|
||||
from federation.entities.base import Image, Relationship, Post, Reaction, Comment, Profile
|
||||
from federation.entities.diaspora.entities import DiasporaPost, DiasporaComment, DiasporaLike, DiasporaRequest, \
|
||||
DiasporaProfile
|
||||
from federation.entities.diaspora.entities import (
|
||||
DiasporaPost, DiasporaComment, DiasporaLike, DiasporaRequest, DiasporaProfile)
|
||||
|
||||
|
||||
logger = logging.getLogger("social-federation")
|
||||
|
||||
MAPPINGS = {
|
||||
"status_message": DiasporaPost,
|
||||
|
|
@ -43,8 +47,18 @@ def message_to_objects(message):
|
|||
if cls:
|
||||
attrs = xml_children_as_dict(element)
|
||||
transformed = transform_attributes(attrs)
|
||||
if hasattr(cls, "fill_extra_attributes"):
|
||||
transformed = cls.fill_extra_attributes(transformed)
|
||||
entity = cls(**transformed)
|
||||
entities.append(entity)
|
||||
try:
|
||||
entity.validate()
|
||||
entities.append(entity)
|
||||
except ValueError as ex:
|
||||
logger.error("Failed to validate entity %s: %s", entity, ex, extra={
|
||||
"attrs": attrs,
|
||||
"transformed": transformed,
|
||||
})
|
||||
continue
|
||||
if cls == DiasporaRequest:
|
||||
# We support sharing/following separately, so also generate base Relationship for the following part
|
||||
transformed.update({"relationship": "following"})
|
||||
|
|
|
|||
|
|
@ -1,6 +1,10 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
from lxml import etree
|
||||
|
||||
from federation.entities.base import Profile
|
||||
from federation.entities.diaspora.entities import DiasporaComment, DiasporaPost, DiasporaLike, DiasporaRequest, \
|
||||
DiasporaProfile
|
||||
|
||||
|
|
@ -61,3 +65,17 @@ class TestEntitiesConvertToXML(object):
|
|||
b"<gender></gender><bio>foobar</bio><location></location><searchable>true</searchable>" \
|
||||
b"<nsfw>false</nsfw><tag_string>#socialfederation #federation</tag_string></profile>"
|
||||
assert etree.tostring(result) == converted
|
||||
|
||||
|
||||
class TestDiasporaProfileFillExtraAttributes(object):
|
||||
def test_raises_if_no_handle(self):
|
||||
attrs = {"foo": "bar"}
|
||||
with pytest.raises(ValueError):
|
||||
DiasporaProfile.fill_extra_attributes(attrs)
|
||||
|
||||
@patch("federation.entities.diaspora.entities.retrieve_and_parse_profile")
|
||||
def test_calls_retrieve_and_parse_profile(self, mock_retrieve):
|
||||
mock_retrieve.return_value = Profile(guid="guidguidguidguid")
|
||||
attrs = {"handle": "foo"}
|
||||
attrs = DiasporaProfile.fill_extra_attributes(attrs)
|
||||
assert attrs == {"handle": "foo", "guid": "guidguidguidguid"}
|
||||
|
|
|
|||
|
|
@ -1,5 +1,6 @@
|
|||
# -*- coding: utf-8 -*-
|
||||
from datetime import datetime
|
||||
from unittest.mock import patch
|
||||
|
||||
import pytest
|
||||
|
||||
|
|
@ -8,7 +9,12 @@ from federation.entities.diaspora.entities import DiasporaPost, DiasporaComment,
|
|||
DiasporaProfile
|
||||
from federation.entities.diaspora.mappers import message_to_objects, get_outbound_entity
|
||||
from federation.tests.fixtures.payloads import DIASPORA_POST_SIMPLE, DIASPORA_POST_COMMENT, DIASPORA_POST_LIKE, \
|
||||
DIASPORA_REQUEST, DIASPORA_PROFILE
|
||||
DIASPORA_REQUEST, DIASPORA_PROFILE, DIASPORA_POST_INVALID
|
||||
|
||||
|
||||
def mock_fill(attributes):
|
||||
attributes["guid"] = "guidguidguidguidguid"
|
||||
return attributes
|
||||
|
||||
|
||||
class TestDiasporaEntityMappersReceive(object):
|
||||
|
|
@ -19,7 +25,7 @@ class TestDiasporaEntityMappersReceive(object):
|
|||
assert isinstance(post, DiasporaPost)
|
||||
assert isinstance(post, Post)
|
||||
assert post.raw_content == "((status message))"
|
||||
assert post.guid == "((guid))"
|
||||
assert post.guid == "((guidguidguidguidguidguidguid))"
|
||||
assert post.handle == "alice@alice.diaspora.example.org"
|
||||
assert post.public == False
|
||||
assert post.created_at == datetime(2011, 7, 20, 1, 36, 7)
|
||||
|
|
@ -31,8 +37,8 @@ class TestDiasporaEntityMappersReceive(object):
|
|||
comment = entities[0]
|
||||
assert isinstance(comment, DiasporaComment)
|
||||
assert isinstance(comment, Comment)
|
||||
assert comment.target_guid == "((parent_guid))"
|
||||
assert comment.guid == "((guid))"
|
||||
assert comment.target_guid == "((parent_guidparent_guidparent_guidparent_guid))"
|
||||
assert comment.guid == "((guidguidguidguidguidguid))"
|
||||
assert comment.handle == "alice@alice.diaspora.example.org"
|
||||
assert comment.participation == "comment"
|
||||
assert comment.raw_content == "((text))"
|
||||
|
|
@ -43,8 +49,8 @@ class TestDiasporaEntityMappersReceive(object):
|
|||
like = entities[0]
|
||||
assert isinstance(like, DiasporaLike)
|
||||
assert isinstance(like, Reaction)
|
||||
assert like.target_guid == "((parent_guid))"
|
||||
assert like.guid == "((guid))"
|
||||
assert like.target_guid == "((parent_guidparent_guidparent_guidparent_guid))"
|
||||
assert like.guid == "((guidguidguidguidguidguid))"
|
||||
assert like.handle == "alice@alice.diaspora.example.org"
|
||||
assert like.participation == "reaction"
|
||||
assert like.reaction == "like"
|
||||
|
|
@ -65,6 +71,7 @@ class TestDiasporaEntityMappersReceive(object):
|
|||
assert sharing.relationship == "sharing"
|
||||
assert following.relationship == "following"
|
||||
|
||||
@patch("federation.entities.diaspora.entities.DiasporaProfile.fill_extra_attributes", new=mock_fill)
|
||||
def test_message_to_objects_profile(self):
|
||||
entities = message_to_objects(DIASPORA_PROFILE)
|
||||
assert len(entities) == 1
|
||||
|
|
@ -83,6 +90,12 @@ class TestDiasporaEntityMappersReceive(object):
|
|||
assert profile.nsfw == False
|
||||
assert profile.tag_list == ["socialfederation", "federation"]
|
||||
|
||||
@patch("federation.entities.diaspora.mappers.logger.error")
|
||||
def test_invalid_entity_logs_an_error(self, mock_logger):
|
||||
entities = message_to_objects(DIASPORA_POST_INVALID)
|
||||
assert len(entities) == 0
|
||||
assert mock_logger.called
|
||||
|
||||
|
||||
class TestGetOutboundEntity(object):
|
||||
def test_already_fine_entities_are_returned_as_is(self):
|
||||
|
|
|
|||
|
|
@ -32,7 +32,20 @@ DIASPORA_POST_SIMPLE = """<XML>
|
|||
<post>
|
||||
<status_message>
|
||||
<raw_message>((status message))</raw_message>
|
||||
<guid>((guid))</guid>
|
||||
<guid>((guidguidguidguidguidguidguid))</guid>
|
||||
<diaspora_handle>alice@alice.diaspora.example.org</diaspora_handle>
|
||||
<public>false</public>
|
||||
<created_at>2011-07-20 01:36:07 UTC</created_at>
|
||||
<provider_display_name>Socialhome</provider_display_name>
|
||||
</status_message>
|
||||
</post>
|
||||
</XML>
|
||||
"""
|
||||
|
||||
DIASPORA_POST_INVALID = """<XML>
|
||||
<post>
|
||||
<status_message>
|
||||
<raw_message>((status message))</raw_message>
|
||||
<diaspora_handle>alice@alice.diaspora.example.org</diaspora_handle>
|
||||
<public>false</public>
|
||||
<created_at>2011-07-20 01:36:07 UTC</created_at>
|
||||
|
|
@ -45,8 +58,8 @@ DIASPORA_POST_SIMPLE = """<XML>
|
|||
DIASPORA_POST_COMMENT = """<XML>
|
||||
<post>
|
||||
<comment>
|
||||
<guid>((guid))</guid>
|
||||
<parent_guid>((parent_guid))</parent_guid>
|
||||
<guid>((guidguidguidguidguidguid))</guid>
|
||||
<parent_guid>((parent_guidparent_guidparent_guidparent_guid))</parent_guid>
|
||||
<author_signature>((base64-encoded data))</author_signature>
|
||||
<text>((text))</text>
|
||||
<diaspora_handle>alice@alice.diaspora.example.org</diaspora_handle>
|
||||
|
|
@ -59,8 +72,8 @@ DIASPORA_POST_LIKE = """<XML>
|
|||
<post>
|
||||
<like>
|
||||
<target_type>Post</target_type>
|
||||
<guid>((guid))</guid>
|
||||
<parent_guid>((parent_guid))</parent_guid>
|
||||
<guid>((guidguidguidguidguidguid))</guid>
|
||||
<parent_guid>((parent_guidparent_guidparent_guidparent_guid))</parent_guid>
|
||||
<author_signature>((base64-encoded data))</author_signature>
|
||||
<positive>true</positive>
|
||||
<diaspora_handle>alice@alice.diaspora.example.org</diaspora_handle>
|
||||
|
|
|
|||
Ładowanie…
Reference in New Issue