Merge pull request #56 from jaywink/profile-validate

Validate fetched remote profile

CHANGELOG.md

@@ -1,5 +1,9 @@
 ## [unreleased]
 
+### Backwards incompatible changes
+* `federation.utils.diaspora.retrieve_and_parse_profile` will now return `None` if the `Profile` retrieved doesn't validate. This will affect also the output of `federation.fetchers.retrieve_remote_profile` which is the high level function to retrieve profiles.
+* Remove unnecessary `protocol` parameter from `federation.fetchers.retrieve_remote_profile`. We're miles away from including other protocols and ideally the caller shouldn't have to pass in the protocol anyway.
+
 ### Added
 * Added `Retraction` entity with `DiasporaRetraction` counterpart.
 

federation/fetchers.py

@@ -1,9 +1,8 @@
 # -*- coding: utf-8 -*-
 import importlib
-import warnings
 
 
-def retrieve_remote_profile(handle, protocol=None):
+def retrieve_remote_profile(handle):
     """High level retrieve profile method.
 
     Retrieve the profile from a remote location, using either the given protocol or by checking each
@@ -13,10 +12,10 @@ def retrieve_remote_profile(handle, protocol=None):
 
     Args:
         handle (str) - The profile handle in format username@domain.tld
+
+    Returns:
+        Profile or None
     """
-    if protocol:
-        warnings.warn("Currently retrieve_remote_profile doesn't use the protocol argument. Diaspora protocol"
-                      "will always be used.")
     protocol_name = "diaspora"
     utils = importlib.import_module("federation.utils.%s" % protocol_name)
     return utils.retrieve_and_parse_profile(handle)

federation/tests/utils/test_diaspora.py

@@ -1,9 +1,10 @@
 # -*- coding: utf-8 -*-
-from unittest.mock import patch
+from unittest.mock import patch, Mock
 from urllib.parse import quote
 
 from lxml import html
 
+from federation.entities.base import Profile
 from federation.hostmeta.generators import DiasporaWebFinger, DiasporaHostMeta, DiasporaHCard, generate_hcard
 from federation.utils.diaspora import retrieve_diaspora_hcard, retrieve_diaspora_webfinger, retrieve_diaspora_host_meta, \
     _get_element_text_or_none, _get_element_attr_or_none, parse_profile_from_hcard, retrieve_and_parse_profile
@@ -163,3 +164,48 @@ class TestRetrieveAndParseProfile(object):
         mock_retrieve.return_value = hcard
         retrieve_and_parse_profile("foo@bar")
         mock_parse.assert_called_with(hcard, "foo@bar")
+
+    @patch("federation.utils.diaspora.parse_profile_from_hcard")
+    @patch("federation.utils.diaspora.retrieve_diaspora_hcard")
+    def test_profile_that_doesnt_validate_returns_none(self, mock_retrieve, mock_parse):
+        hcard = generate_hcard(
+            "diaspora",
+            hostname="https://hostname",
+            fullname="fullname",
+            firstname="firstname",
+            lastname="lastname",
+            photo300="photo300",
+            photo100="photo100",
+            photo50="photo50",
+            searchable="true",
+            guid="guid",
+            public_key="public_key",
+            username="username",
+        )
+        mock_retrieve.return_value = hcard
+        mock_parse.return_value = Profile(guid="123")
+        profile = retrieve_and_parse_profile("foo@bar")
+        assert profile == None
+
+    @patch("federation.utils.diaspora.parse_profile_from_hcard")
+    @patch("federation.utils.diaspora.retrieve_diaspora_hcard")
+    def test_profile_validate_is_called(self, mock_retrieve, mock_parse):
+        hcard = generate_hcard(
+            "diaspora",
+            hostname="https://hostname",
+            fullname="fullname",
+            firstname="firstname",
+            lastname="lastname",
+            photo300="photo300",
+            photo100="photo100",
+            photo50="photo50",
+            searchable="true",
+            guid="guid",
+            public_key="public_key",
+            username="username",
+        )
+        mock_retrieve.return_value = hcard
+        mock_profile = Mock()
+        mock_parse.return_value = mock_profile
+        retrieve_and_parse_profile("foo@bar")
+        assert mock_profile.validate.called

federation/utils/diaspora.py

@@ -1,4 +1,5 @@
 # -*- coding: utf-8 -*-
+import logging
 from urllib.parse import quote
 
 from lxml import html
@@ -7,6 +8,8 @@ from xrd import XRD
 from federation.entities.base import Profile
 from federation.utils.network import fetch_document
 
+logger = logging.getLogger("social-federation")
+
 
 def retrieve_diaspora_hcard(handle):
     """
@@ -127,9 +130,16 @@ def retrieve_and_parse_profile(handle):
         handle (str) - User handle in username@domain.tld format
 
     Returns:
-        Profile
+        Profile or None
     """
     hcard = retrieve_diaspora_hcard(handle)
     if not hcard:
         return None
-    return parse_profile_from_hcard(hcard, handle)
+    profile = parse_profile_from_hcard(hcard, handle)
+    try:
+        profile.validate()
+    except ValueError as ex:
+        logger.warning("retrieve_and_parse_profile - found profile %s but it didn't validate: %s",
+                       profile, ex)
+        return None
+    return profile