fix upsert_statuses

- remote_id is now updated on import of statuses
- statuses cannot be imported unless source has target listed in alsoKnownAs or movedTo
- add alert boxes to import and export screens advising of the above
- update tests accordingly
This commit is contained in:
Hugh Rundle 2023-12-13 20:55:38 +11:00
parent 029b438355
commit 7fcadb1d4d
No known key found for this signature in database
GPG key ID: A7E35779918253F9
4 changed files with 103 additions and 28 deletions

View file

@ -178,21 +178,25 @@ def upsert_statuses(user, cls, data, book_remote_id):
find or create the instances in the database""" find or create the instances in the database"""
for status in data: for status in data:
if is_alias(
user, status["attributedTo"]
): # don't let l33t hax0rs steal other people's posts
# update ids and remove replies # update ids and remove replies
status["attributedTo"] = user.remote_id status["attributedTo"] = user.remote_id
status["to"] = update_followers_address(user, status["to"]) status["to"] = update_followers_address(user, status["to"])
status["cc"] = update_followers_address(user, status["cc"]) status["cc"] = update_followers_address(user, status["cc"])
status[ status[
"replies" "replies"
] = {} # this parses incorrectly but we can't set it without knowing the new id ] = (
{}
) # this parses incorrectly but we can't set it without knowing the new id
status["inReplyToBook"] = book_remote_id status["inReplyToBook"] = book_remote_id
# save new status or do update it if it already exists
parsed = activitypub.parse(status) parsed = activitypub.parse(status)
instance = parsed.to_model(model=cls, save=True, overwrite=True) if not status_already_exists(
user, parsed
): # don't duplicate posts on multiple import
print(instance.id, instance.privacy) instance = parsed.to_model(model=cls, save=True, overwrite=True)
for val in [ for val in [
"progress", "progress",
@ -202,9 +206,13 @@ def upsert_statuses(user, cls, data, book_remote_id):
"position_mode", "position_mode",
]: ]:
if status.get(val): if status.get(val):
print(val, status[val])
instance.val = status[val] instance.val = status[val]
instance.save()
instance.remote_id = instance.get_remote_id() # update the remote_id
instance.save() # save and broadcast
else:
logger.info("User does not have permission to import statuses")
def upsert_lists(user, lists, book_id): def upsert_lists(user, lists, book_id):
@ -369,7 +377,7 @@ def upsert_follows(user, values):
if not created: if not created:
# this request probably failed to connect with the remote # this request probably failed to connect with the remote
# that means we should save to trigger a re-broadcast # and should save to trigger a re-broadcast
follow_request.save() follow_request.save()
@ -419,3 +427,33 @@ def update_followers_address(user, field):
field[i] = user.followers_url field[i] = user.followers_url
return field return field
def is_alias(user, remote_id):
"""check that the user is listed as movedTo or also_known_as
in the remote user's profile"""
remote_user = activitypub.resolve_remote_id(
remote_id=remote_id, model=models.User, save=False
)
if remote_user:
if remote_user.moved_to:
return user.remote_id == remote_user.moved_to
if remote_user.also_known_as:
return user in remote_user.also_known_as.all()
return False
def status_already_exists(user, status):
"""check whether this status has already been published
by this user. We can't rely on to_model() because it
only matches on remote_id, which we have to change
*after* saving because it needs the primary key (id)"""
return models.Status.objects.filter(
user=user, content=status.content, published_date=status.published
).exists()

View file

@ -13,7 +13,11 @@
{% trans "Not a valid import file" %} {% trans "Not a valid import file" %}
</div> </div>
{% endif %} {% endif %}
<p class="notification is-warning">
{% spaceless %}
{% trans "If you wish to migrate any statuses (comments, reviews, or quotes) you must either set this account as an <strong>alias</strong> of the one you are migrating from, or <strong>move</strong> that account to this one, before you import your user data." %}
{% endspaceless %}
</p>
{% if not site.imports_enabled %} {% if not site.imports_enabled %}
<div class="box notification has-text-centered is-warning m-6 content"> <div class="box notification has-text-centered is-warning m-6 content">
<p class="mt-5"> <p class="mt-5">

View file

@ -41,6 +41,11 @@
{% endblocktrans %} {% endblocktrans %}
</div> </div>
<p class="block">{% trans "In your new BookWyrm account can choose what to import: you will not have to import everything that is exported." %}</p> <p class="block">{% trans "In your new BookWyrm account can choose what to import: you will not have to import everything that is exported." %}</p>
<p class="notification is-warning">
{% spaceless %}
{% trans "If you wish to migrate any statuses (comments, reviews, or quotes) you must either set the account you are moving to as an <strong>alias</strong> of this one, or <strong>move</strong> this account to the new account, before you import your user data." %}
{% endspaceless %}
</p>
{% if next_available %} {% if next_available %}
<p class="notification is-warning"> <p class="notification is-warning">
{% blocktrans trimmed %} {% blocktrans trimmed %}

View file

@ -312,7 +312,10 @@ class BookwyrmImport(TestCase): # pylint: disable=too-many-public-methods
self.assertEqual(models.Review.objects.filter(user=self.local_user).count(), 0) self.assertEqual(models.Review.objects.filter(user=self.local_user).count(), 0)
reviews = self.json_data["books"][0]["reviews"] reviews = self.json_data["books"][0]["reviews"]
with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): with patch(
"bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
), patch("bookwyrm.models.bookwyrm_import_job.is_alias", return_value=True):
bookwyrm_import_job.upsert_statuses( bookwyrm_import_job.upsert_statuses(
self.local_user, models.Review, reviews, self.book.remote_id self.local_user, models.Review, reviews, self.book.remote_id
) )
@ -345,7 +348,11 @@ class BookwyrmImport(TestCase): # pylint: disable=too-many-public-methods
self.assertEqual(models.Comment.objects.filter(user=self.local_user).count(), 0) self.assertEqual(models.Comment.objects.filter(user=self.local_user).count(), 0)
comments = self.json_data["books"][1]["comments"] comments = self.json_data["books"][1]["comments"]
with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"):
with patch(
"bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
), patch("bookwyrm.models.bookwyrm_import_job.is_alias", return_value=True):
bookwyrm_import_job.upsert_statuses( bookwyrm_import_job.upsert_statuses(
self.local_user, models.Comment, comments, self.book.remote_id self.local_user, models.Comment, comments, self.book.remote_id
) )
@ -371,7 +378,10 @@ class BookwyrmImport(TestCase): # pylint: disable=too-many-public-methods
models.Quotation.objects.filter(user=self.local_user).count(), 0 models.Quotation.objects.filter(user=self.local_user).count(), 0
) )
quotes = self.json_data["books"][1]["quotations"] quotes = self.json_data["books"][1]["quotations"]
with patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"): with patch(
"bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
), patch("bookwyrm.models.bookwyrm_import_job.is_alias", return_value=True):
bookwyrm_import_job.upsert_statuses( bookwyrm_import_job.upsert_statuses(
self.local_user, models.Quotation, quotes, self.book.remote_id self.local_user, models.Quotation, quotes, self.book.remote_id
) )
@ -394,6 +404,24 @@ class BookwyrmImport(TestCase): # pylint: disable=too-many-public-methods
models.Quotation.objects.filter(book=self.book).first().position_mode, "PG" models.Quotation.objects.filter(book=self.book).first().position_mode, "PG"
) )
def test_get_or_create_quote_unauthorized(self):
"""Test get_or_create_review_status with a quote but not authorized"""
self.assertEqual(
models.Quotation.objects.filter(user=self.local_user).count(), 0
)
quotes = self.json_data["books"][1]["quotations"]
with patch(
"bookwyrm.models.activitypub_mixin.broadcast_task.apply_async"
), patch("bookwyrm.models.bookwyrm_import_job.is_alias", return_value=False):
bookwyrm_import_job.upsert_statuses(
self.local_user, models.Quotation, quotes, self.book.remote_id
)
self.assertEqual(
models.Quotation.objects.filter(user=self.local_user).count(), 0
)
def test_upsert_list_existing(self): def test_upsert_list_existing(self):
"""Take a list and ListItems as JSON and create DB entries """Take a list and ListItems as JSON and create DB entries
if they don't already exist""" if they don't already exist"""