Merge pull request #632 from mouse-reeve/remote-server-set

Safer set remote server

bookwyrm/models/user.py

@@ -9,7 +9,7 @@ from django.db import models
 from django.utils import timezone
 
 from bookwyrm import activitypub
-from bookwyrm.connectors import get_data
+from bookwyrm.connectors import get_data, ConnectorException
 from bookwyrm.models.shelf import Shelf
 from bookwyrm.models.status import Status, Review
 from bookwyrm.settings import DOMAIN
@@ -329,7 +329,7 @@ def set_remote_server(user_id):
     actor_parts = urlparse(user.remote_id)
     user.federated_server = \
         get_or_create_remote_server(actor_parts.netloc)
-    user.save()
+    user.save(broadcast=False)
     if user.bookwyrm_user:
         get_remote_reviews.delay(user.outbox)
 
@@ -343,19 +343,24 @@ def get_or_create_remote_server(domain):
     except FederatedServer.DoesNotExist:
         pass
 
+    try:
         data = get_data('https://%s/.well-known/nodeinfo' % domain)
-
         try:
             nodeinfo_url = data.get('links')[0].get('href')
         except (TypeError, KeyError):
-        return None
+            raise ConnectorException()
 
         data = get_data(nodeinfo_url)
+        application_type = data.get('software', {}).get('name')
+        application_version = data.get('software', {}).get('version')
+    except ConnectorException:
+        application_type = application_version = None
+
 
     server = FederatedServer.objects.create(
         server_name=domain,
-        application_type=data['software']['name'],
+        application_type=application_type,
-        application_version=data['software']['version'],
+        application_version=application_version,
     )
     return server
 

bookwyrm/tests/models/test_user_model.py

@@ -1,16 +1,18 @@
 ''' testing models '''
 from unittest.mock import patch
 from django.test import TestCase
+import responses
 
 from bookwyrm import models
 from bookwyrm.settings import DOMAIN
 
-
+# pylint: disable=missing-class-docstring
+# pylint: disable=missing-function-docstring
 class User(TestCase):
     def setUp(self):
         self.user = models.User.objects.create_user(
             'mouse@%s' % DOMAIN, 'mouse@mouse.mouse', 'mouseword',
-            local=True, localname='mouse', name='hi')
+            local=True, localname='mouse', name='hi', bookwyrm_user=False)
 
     def test_computed_fields(self):
         ''' username instead of id here '''
@@ -28,7 +30,7 @@ class User(TestCase):
         with patch('bookwyrm.models.user.set_remote_server.delay'):
             user = models.User.objects.create_user(
                 'rat', 'rat@rat.rat', 'ratword', local=False,
-                remote_id='https://example.com/dfjkg')
+                remote_id='https://example.com/dfjkg', bookwyrm_user=False)
         self.assertEqual(user.username, 'rat@example.com')
 
 
@@ -62,7 +64,7 @@ class User(TestCase):
         self.assertEqual(activity['name'], self.user.name)
         self.assertEqual(activity['inbox'], self.user.inbox)
         self.assertEqual(activity['outbox'], self.user.outbox)
-        self.assertEqual(activity['bookwyrmUser'], True)
+        self.assertEqual(activity['bookwyrmUser'], False)
         self.assertEqual(activity['discoverable'], True)
         self.assertEqual(activity['type'], 'Person')
 
@@ -71,3 +73,83 @@ class User(TestCase):
         self.assertEqual(activity['type'], 'OrderedCollection')
         self.assertEqual(activity['id'], self.user.outbox)
         self.assertEqual(activity['totalItems'], 0)
+
+
+    def test_set_remote_server(self):
+        server = models.FederatedServer.objects.create(
+            server_name=DOMAIN,
+            application_type='test type',
+            application_version=3
+        )
+
+        models.user.set_remote_server(self.user.id)
+        self.user.refresh_from_db()
+
+        self.assertEqual(self.user.federated_server, server)
+
+    @responses.activate
+    def test_get_or_create_remote_server(self):
+        responses.add(
+            responses.GET,
+            'https://%s/.well-known/nodeinfo' % DOMAIN,
+            json={'links': [{'href': 'http://www.example.com'}, {}]}
+        )
+        responses.add(
+            responses.GET,
+            'http://www.example.com',
+            json={'software': {'name': 'hi', 'version': '2'}},
+        )
+
+        server = models.user.get_or_create_remote_server(DOMAIN)
+        self.assertEqual(server.server_name, DOMAIN)
+        self.assertEqual(server.application_type, 'hi')
+        self.assertEqual(server.application_version, '2')
+
+    @responses.activate
+    def test_get_or_create_remote_server_no_wellknown(self):
+        responses.add(
+            responses.GET,
+            'https://%s/.well-known/nodeinfo' % DOMAIN,
+            status=404
+        )
+
+        server = models.user.get_or_create_remote_server(DOMAIN)
+        self.assertEqual(server.server_name, DOMAIN)
+        self.assertIsNone(server.application_type)
+        self.assertIsNone(server.application_version)
+
+    @responses.activate
+    def test_get_or_create_remote_server_no_links(self):
+        responses.add(
+            responses.GET,
+            'https://%s/.well-known/nodeinfo' % DOMAIN,
+            json={'links': [{'href': 'http://www.example.com'}, {}]}
+        )
+        responses.add(
+            responses.GET,
+            'http://www.example.com',
+            status=404
+        )
+
+        server = models.user.get_or_create_remote_server(DOMAIN)
+        self.assertEqual(server.server_name, DOMAIN)
+        self.assertIsNone(server.application_type)
+        self.assertIsNone(server.application_version)
+
+    @responses.activate
+    def test_get_or_create_remote_server_unknown_format(self):
+        responses.add(
+            responses.GET,
+            'https://%s/.well-known/nodeinfo' % DOMAIN,
+            json={'links': [{'href': 'http://www.example.com'}, {}]}
+        )
+        responses.add(
+            responses.GET,
+            'http://www.example.com',
+            json={'fish': 'salmon'}
+        )
+
+        server = models.user.get_or_create_remote_server(DOMAIN)
+        self.assertEqual(server.server_name, DOMAIN)
+        self.assertIsNone(server.application_type)
+        self.assertIsNone(server.application_version)