From 6b74f56381cdf1c58c9cb0fc468896a535dc3071 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Feb 2021 10:01:19 -0800 Subject: [PATCH 1/6] Safer set remote server --- bookwyrm/models/user.py | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index a65317fa..5e77b9e1 100644 --- a/bookwyrm/models/user.py +++ b/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 @@ -333,19 +333,24 @@ def get_or_create_remote_server(domain): except FederatedServer.DoesNotExist: pass - data = get_data('https://%s/.well-known/nodeinfo' % domain) - try: - nodeinfo_url = data.get('links')[0].get('href') - except (TypeError, KeyError): - return None + data = get_data('https://%s/.well-known/nodeinfo' % domain) + try: + nodeinfo_url = data.get('links')[0].get('href') + except (TypeError, KeyError): + return None + + data = get_data(nodeinfo_url) + application_type = data.get('software', {}).get('name') + application_type = data.get('software', {}).get('version') + except ConnectorException: + application_type = application_version = None - data = get_data(nodeinfo_url) server = FederatedServer.objects.create( server_name=domain, - application_type=data['software']['name'], - application_version=data['software']['version'], + application_type=application_type, + application_version=application_version, ) return server From ef9acaf87810a999ab01577764ba5400f0c6cd51 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Mon, 22 Feb 2021 11:38:11 -0800 Subject: [PATCH 2/6] Adds tests for setting remote server --- bookwyrm/models/user.py | 4 +- bookwyrm/tests/models/test_user_model.py | 84 +++++++++++++++++++++++- 2 files changed, 85 insertions(+), 3 deletions(-) diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index 5e77b9e1..a1f927b2 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -338,11 +338,11 @@ def get_or_create_remote_server(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_type = data.get('software', {}).get('version') + application_version = data.get('software', {}).get('version') except ConnectorException: application_type = application_version = None diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index a10c89b8..bd184b65 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -1,11 +1,13 @@ ''' 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( @@ -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) From c6a61abf79a2e51dcb4bed9c28245188326655ee Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Feb 2021 11:58:01 -0800 Subject: [PATCH 3/6] Don't try to fetch reviews for remote user in test --- bookwyrm/tests/models/test_user_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index bd184b65..34f4c54d 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -30,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') From be9198fc4ff7def40d7cd4d5040e8170814a3a43 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Feb 2021 13:09:39 -0800 Subject: [PATCH 4/6] Another place where get reviews is called in tests --- bookwyrm/tests/models/test_user_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index 34f4c54d..84ff3e9b 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -12,7 +12,7 @@ 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 ''' From 3de8a20d39db8ab2036b0352af435090ff90c712 Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Feb 2021 14:36:24 -0800 Subject: [PATCH 5/6] Fixes boolean for is bookwyrm user in test --- bookwyrm/tests/models/test_user_model.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/tests/models/test_user_model.py b/bookwyrm/tests/models/test_user_model.py index 84ff3e9b..ed3ad41a 100644 --- a/bookwyrm/tests/models/test_user_model.py +++ b/bookwyrm/tests/models/test_user_model.py @@ -64,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') From baed291889eb3672c6c503b7b023e28a72ca26ce Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 23 Feb 2021 14:45:39 -0800 Subject: [PATCH 6/6] Don't broadcast after saving remote server --- bookwyrm/models/user.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bookwyrm/models/user.py b/bookwyrm/models/user.py index a1f927b2..094a13d5 100644 --- a/bookwyrm/models/user.py +++ b/bookwyrm/models/user.py @@ -319,7 +319,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)