mirror of
https://github.com/bookwyrm-social/bookwyrm.git
synced 2024-11-22 01:21:07 +00:00
refactor Move for more redundancy
As outlined in #3354, a use `Move` fails if the user is moving from a BookWyrm server to another BookWrym server. This is because: 1. the original code did not announce changes to alsoKnownAs; 2. the original code always checked the locally saved profile rather than refetching the remote data; This commit fixes both these problems by forcing `MoveUser` to always perform a "refresh" of the local data from the remote, and by saving the user with broadcast=True when updating alsoKnownAs ids.
This commit is contained in:
Hugh Rundle
parent
193aeff4d2
commit
6684d60526
6 changed files with 222 additions and 4 deletions
|
|
@ -10,7 +10,7 @@ from .notification import Notification, NotificationType
|
|||
|
||||
|
||||
class Move(ActivityMixin, BookWyrmModel):
|
||||
"""migrating an activitypub user account"""
|
||||
"""migrating an activitypub object"""
|
||||
|
||||
user = fields.ForeignKey(
|
||||
"User", on_delete=models.PROTECT, activitypub_field="actor"
|
||||
|
|
|
|||
40
bookwyrm/tests/data/ap_user_move.json
Normal file
40
bookwyrm/tests/data/ap_user_move.json
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
{
|
||||
"@context": [
|
||||
"https://www.w3.org/ns/activitystreams",
|
||||
"https://w3id.org/security/v1",
|
||||
{
|
||||
"manuallyApprovesFollowers": "as:manuallyApprovesFollowers",
|
||||
"schema": "http://schema.org#",
|
||||
"PropertyValue": "schema:PropertyValue",
|
||||
"value": "schema:value"
|
||||
}
|
||||
],
|
||||
"id": "https://example.com/user/mouse",
|
||||
"type": "Person",
|
||||
"preferredUsername": "mouse",
|
||||
"name": "MOUSE?? MOUSE!!",
|
||||
"inbox": "https://example.com/user/mouse/inbox",
|
||||
"outbox": "https://example.com/user/mouse/outbox",
|
||||
"followers": "https://example.com/user/mouse/followers",
|
||||
"following": "https://example.com/user/mouse/following",
|
||||
"summary": "",
|
||||
"publicKey": {
|
||||
"id": "https://example.com/user/mouse/#main-key",
|
||||
"owner": "https://example.com/user/mouse",
|
||||
"publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQC6QisDrjOQvkRo/MqNmSYPwqtt\nCxg/8rCW+9jKbFUKvqjTeKVotEE85122v/DCvobCCdfQuYIFdVMk+dB1xJ0iPGPg\nyU79QHY22NdV9mFKA2qtXVVxb5cxpA4PlwOHM6PM/k8B+H09OUrop2aPUAYwy+vg\n+MXyz8bAXrIS1kq6fQIDAQAB\n-----END PUBLIC KEY-----"
|
||||
},
|
||||
"endpoints": {
|
||||
"sharedInbox": "https://example.com/inbox"
|
||||
},
|
||||
"bookwyrmUser": true,
|
||||
"manuallyApprovesFollowers": false,
|
||||
"discoverable": false,
|
||||
"alsoKnownAs": ["https://your.domain.here/user/rat"],
|
||||
"devices": "https://friend.camp/users/tripofmice/collections/devices",
|
||||
"tag": [],
|
||||
"icon": {
|
||||
"type": "Image",
|
||||
"mediaType": "image/png",
|
||||
"url": "https://example.com/images/avatars/AL-2-crop-50.png"
|
||||
}
|
||||
}
|
||||
58
bookwyrm/tests/models/test_move.py
Normal file
58
bookwyrm/tests/models/test_move.py
Normal file
|
|
@ -0,0 +1,58 @@
|
|||
""" testing move models """
|
||||
from unittest.mock import patch
|
||||
from django.core.exceptions import PermissionDenied
|
||||
from django.test import TestCase
|
||||
|
||||
from bookwyrm import models
|
||||
|
||||
|
||||
class MoveUser(TestCase):
|
||||
"""move your account to another identity"""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(self): # pylint: disable=bad-classmethod-argument
|
||||
"""we need some users for this"""
|
||||
with patch("bookwyrm.models.user.set_remote_server.delay"):
|
||||
self.target_user = models.User.objects.create_user(
|
||||
"rat",
|
||||
"rat@rat.com",
|
||||
"ratword",
|
||||
local=False,
|
||||
remote_id="https://example.com/users/rat",
|
||||
inbox="https://example.com/users/rat/inbox",
|
||||
outbox="https://example.com/users/rat/outbox",
|
||||
)
|
||||
|
||||
with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
|
||||
"bookwyrm.activitystreams.populate_stream_task.delay"
|
||||
), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
|
||||
self.origin_user = models.User.objects.create_user(
|
||||
"mouse", "mouse@mouse.com", "mouseword", local=True, localname="mouse"
|
||||
)
|
||||
self.origin_user.remote_id = "http://local.com/user/mouse"
|
||||
self.origin_user.save(broadcast=False, update_fields=["remote_id"])
|
||||
|
||||
def test_user_move_unauthorized(self, *_):
|
||||
"""attempt a user move without alsoKnownAs set"""
|
||||
|
||||
with self.assertRaises(PermissionDenied):
|
||||
models.MoveUser.objects.create(
|
||||
user=self.origin_user,
|
||||
object=self.origin_user.remote_id,
|
||||
target=self.target_user,
|
||||
)
|
||||
|
||||
@patch("bookwyrm.suggested_users.remove_user_task.delay")
|
||||
@patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async")
|
||||
def test_user_move(self, *_):
|
||||
"""move user"""
|
||||
|
||||
self.target_user.also_known_as.add(self.origin_user.id)
|
||||
self.target_user.save(broadcast=False)
|
||||
|
||||
models.MoveUser.objects.create(
|
||||
user=self.origin_user,
|
||||
object=self.origin_user.remote_id,
|
||||
target=self.target_user,
|
||||
)
|
||||
self.assertEqual(self.origin_user.moved_to, self.target_user.remote_id)
|
||||
114
bookwyrm/tests/views/preferences/test_move.py
Normal file
114
bookwyrm/tests/views/preferences/test_move.py
Normal file
|
|
@ -0,0 +1,114 @@
|
|||
""" test move functionality """
|
||||
import json
|
||||
from unittest.mock import patch
|
||||
import pathlib
|
||||
from django.contrib.sessions.middleware import SessionMiddleware
|
||||
from django.test import TestCase
|
||||
from django.test.client import RequestFactory
|
||||
import responses
|
||||
|
||||
from bookwyrm import forms, models, views
|
||||
|
||||
|
||||
@patch("bookwyrm.activitystreams.add_status_task.delay")
|
||||
@patch("bookwyrm.suggested_users.rerank_suggestions_task.delay")
|
||||
@patch("bookwyrm.activitystreams.populate_stream_task.delay")
|
||||
@patch("bookwyrm.suggested_users.rerank_user_task.delay")
|
||||
class ViewsHelpers(TestCase): # pylint: disable=too-many-public-methods
|
||||
"""viewing and creating statuses"""
|
||||
|
||||
@classmethod
|
||||
def setUpTestData(self): # pylint: disable=bad-classmethod-argument
|
||||
"""we need basic test data and mocks"""
|
||||
|
||||
with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
|
||||
"bookwyrm.activitystreams.populate_stream_task.delay"
|
||||
), patch("bookwyrm.lists_stream.populate_lists_task.delay"), patch(
|
||||
"bookwyrm.suggested_users.rerank_user_task.delay"
|
||||
):
|
||||
|
||||
self.local_user = models.User.objects.create_user(
|
||||
"rat",
|
||||
"rat@rat.com",
|
||||
"ratword",
|
||||
local=True,
|
||||
discoverable=True,
|
||||
localname="rat",
|
||||
remote_id="https://your.domain.here/user/rat",
|
||||
)
|
||||
|
||||
with patch("bookwyrm.models.user.set_remote_server.delay"), patch(
|
||||
"bookwyrm.suggested_users.rerank_user_task.delay"
|
||||
):
|
||||
self.remote_user = models.User.objects.create_user(
|
||||
"mouse@example.com",
|
||||
"mouse@mouse.com",
|
||||
"mouseword",
|
||||
local=False,
|
||||
remote_id="https://example.com/user/mouse",
|
||||
)
|
||||
|
||||
def setUp(self):
|
||||
"""individual test setup"""
|
||||
self.factory = RequestFactory()
|
||||
datafile = pathlib.Path(__file__).parent.joinpath(
|
||||
"../../data/ap_user_move.json"
|
||||
)
|
||||
self.userdata = json.loads(datafile.read_bytes())
|
||||
del self.userdata["icon"]
|
||||
|
||||
@responses.activate
|
||||
@patch("bookwyrm.models.user.set_remote_server.delay")
|
||||
@patch("bookwyrm.suggested_users.remove_user_task.delay")
|
||||
@patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async")
|
||||
def test_move_user_view(self, *_):
|
||||
"""move user"""
|
||||
|
||||
self.assertEqual(self.remote_user.remote_id, "https://example.com/user/mouse")
|
||||
self.assertIsNone(self.local_user.moved_to)
|
||||
self.assertIsNone(self.remote_user.moved_to)
|
||||
self.assertIsNone(self.local_user.also_known_as.first())
|
||||
self.assertIsNone(self.remote_user.also_known_as.first())
|
||||
|
||||
username = "mouse@example.com"
|
||||
wellknown = {
|
||||
"subject": "acct:mouse@example.com",
|
||||
"links": [
|
||||
{
|
||||
"rel": "self",
|
||||
"type": "application/activity+json",
|
||||
"href": "https://example.com/user/mouse",
|
||||
}
|
||||
],
|
||||
}
|
||||
responses.add(
|
||||
responses.GET,
|
||||
f"https://example.com/.well-known/webfinger?resource=acct:{username}",
|
||||
json=wellknown,
|
||||
status=200,
|
||||
)
|
||||
responses.add(
|
||||
responses.GET,
|
||||
"https://example.com/user/mouse",
|
||||
json=self.userdata,
|
||||
status=200,
|
||||
)
|
||||
|
||||
view = views.MoveUser.as_view()
|
||||
form = forms.MoveUserForm()
|
||||
form.data["target"] = "mouse@example.com"
|
||||
form.data["password"] = "ratword"
|
||||
|
||||
request = self.factory.post("", form.data)
|
||||
request.user = self.local_user
|
||||
middleware = SessionMiddleware()
|
||||
middleware.process_request(request)
|
||||
request.session.save()
|
||||
|
||||
with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
|
||||
view(request)
|
||||
self.local_user.refresh_from_db()
|
||||
|
||||
self.assertEqual(self.local_user.also_known_as.first(), self.remote_user)
|
||||
self.assertEqual(self.remote_user.also_known_as.first(), self.local_user)
|
||||
self.assertEqual(self.local_user.moved_to, "https://example.com/user/mouse")
|
||||
|
|
@ -60,7 +60,7 @@ def is_bookwyrm_request(request):
|
|||
return True
|
||||
|
||||
|
||||
def handle_remote_webfinger(query, unknown_only=False):
|
||||
def handle_remote_webfinger(query, unknown_only=False, refresh=False):
|
||||
"""webfingerin' other servers"""
|
||||
user = None
|
||||
|
||||
|
|
@ -75,6 +75,11 @@ def handle_remote_webfinger(query, unknown_only=False):
|
|||
return None
|
||||
|
||||
try:
|
||||
|
||||
if refresh:
|
||||
# Always fetch the remote info - don't even bother checking the DB
|
||||
raise models.User.DoesNotExist("remote_only is set to True")
|
||||
|
||||
user = models.User.objects.get(username__iexact=query)
|
||||
|
||||
if unknown_only:
|
||||
|
|
@ -92,7 +97,7 @@ def handle_remote_webfinger(query, unknown_only=False):
|
|||
if link.get("rel") == "self":
|
||||
try:
|
||||
user = activitypub.resolve_remote_id(
|
||||
link["href"], model=models.User
|
||||
link["href"], model=models.User, refresh=refresh
|
||||
)
|
||||
except (KeyError, activitypub.ActivitySerializerError):
|
||||
return None
|
||||
|
|
|
|||
|
|
@ -32,7 +32,7 @@ class MoveUser(View):
|
|||
|
||||
if form.is_valid() and user.check_password(form.cleaned_data["password"]):
|
||||
username = form.cleaned_data["target"]
|
||||
target = handle_remote_webfinger(username)
|
||||
target = handle_remote_webfinger(username, refresh=True)
|
||||
|
||||
try:
|
||||
models.MoveUser.objects.create(
|
||||
|
|
@ -81,6 +81,7 @@ class AliasUser(View):
|
|||
return TemplateResponse(request, "preferences/alias_user.html", data)
|
||||
|
||||
user.also_known_as.add(remote_user.id)
|
||||
user.save(broadcast=True) # broadcast the alias
|
||||
|
||||
return redirect("prefs-alias")
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue