From ad0fff703054af0256ea0cf66555073f6a2b1b6b Mon Sep 17 00:00:00 2001 From: Mouse Reeve Date: Tue, 17 Aug 2021 10:08:07 -0700 Subject: [PATCH] Prevent overwriting data on import form outside data source --- bookwyrm/activitypub/base_activity.py | 13 +++++++++--- bookwyrm/connectors/abstract_connector.py | 6 +++--- bookwyrm/models/fields.py | 26 ++++++++++++++++++----- 3 files changed, 34 insertions(+), 11 deletions(-) diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index ddd45426f..c937f6725 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -107,7 +107,9 @@ class ActivityObject: setattr(self, field.name, value) # pylint: disable=too-many-locals,too-many-branches - def to_model(self, model=None, instance=None, allow_create=True, save=True): + def to_model( + self, model=None, instance=None, allow_create=True, save=True, overwrite=True + ): """convert from an activity to a model instance""" model = model or get_model_from_type(self.type) @@ -129,9 +131,12 @@ class ActivityObject: # keep track of what we've changed update_fields = [] + # sets field on the model using the activity value for field in instance.simple_fields: try: - changed = field.set_field_from_activity(instance, self) + changed = field.set_field_from_activity( + instance, self, overwrite=overwrite + ) if changed: update_fields.append(field.name) except AttributeError as e: @@ -140,7 +145,9 @@ class ActivityObject: # image fields have to be set after other fields because they can save # too early and jank up users for field in instance.image_fields: - changed = field.set_field_from_activity(instance, self, save=save) + changed = field.set_field_from_activity( + instance, self, save=save, overwrite=overwrite + ) if changed: update_fields.append(field.name) diff --git a/bookwyrm/connectors/abstract_connector.py b/bookwyrm/connectors/abstract_connector.py index fb102ea4b..ffacffdf0 100644 --- a/bookwyrm/connectors/abstract_connector.py +++ b/bookwyrm/connectors/abstract_connector.py @@ -139,7 +139,7 @@ class AbstractConnector(AbstractMinimalConnector): **dict_from_mappings(work_data, self.book_mappings) ) # this will dedupe automatically - work = work_activity.to_model(model=models.Work) + work = work_activity.to_model(model=models.Work, overwrite=False) for author in self.get_authors_from_data(work_data): work.authors.add(author) @@ -156,7 +156,7 @@ class AbstractConnector(AbstractMinimalConnector): mapped_data = dict_from_mappings(edition_data, self.book_mappings) mapped_data["work"] = work.remote_id edition_activity = activitypub.Edition(**mapped_data) - edition = edition_activity.to_model(model=models.Edition) + edition = edition_activity.to_model(model=models.Edition, overwrite=False) edition.connector = self.connector edition.save() @@ -182,7 +182,7 @@ class AbstractConnector(AbstractMinimalConnector): return None # this will dedupe - return activity.to_model(model=models.Author) + return activity.to_model(model=models.Author, overwrite=False) @abstractmethod def is_work_data(self, data): diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index b58f81747..6ed5aa5e6 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -66,7 +66,7 @@ class ActivitypubFieldMixin: self.activitypub_field = activitypub_field super().__init__(*args, **kwargs) - def set_field_from_activity(self, instance, data): + def set_field_from_activity(self, instance, data, overwrite=True): """helper function for assinging a value to the field. Returns if changed""" try: value = getattr(data, self.get_activitypub_field()) @@ -79,8 +79,15 @@ class ActivitypubFieldMixin: if formatted is None or formatted is MISSING or formatted == {}: return False + current_value = ( + getattr(instance, self.name) if hasattr(instance, self.name) else None + ) + # if we're not in overwrite mode, only continue updating the field if its unset + if current_value and not overwrite: + return False + # the field is unchanged - if hasattr(instance, self.name) and getattr(instance, self.name) == formatted: + if current_value == formatted: return False setattr(instance, self.name, formatted) @@ -210,7 +217,10 @@ class PrivacyField(ActivitypubFieldMixin, models.CharField): ) # pylint: disable=invalid-name - def set_field_from_activity(self, instance, data): + def set_field_from_activity(self, instance, data, overwrite=True): + if not overwrite: + return False + original = getattr(instance, self.name) to = data.to cc = data.cc @@ -273,8 +283,11 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField): self.link_only = link_only super().__init__(*args, **kwargs) - def set_field_from_activity(self, instance, data): + def set_field_from_activity(self, instance, data, overwrite=True): """helper function for assinging a value to the field""" + if not overwrite and getattr(instance, self.name).exists(): + return False + value = getattr(data, self.get_activitypub_field()) formatted = self.field_from_activity(value) if formatted is None or formatted is MISSING: @@ -377,13 +390,16 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): super().__init__(*args, **kwargs) # pylint: disable=arguments-differ - def set_field_from_activity(self, instance, data, save=True): + def set_field_from_activity(self, instance, data, save=True, overwrite=True): """helper function for assinging a value to the field""" value = getattr(data, self.get_activitypub_field()) formatted = self.field_from_activity(value) if formatted is None or formatted is MISSING: return False + if not overwrite and hasattr(instance, self.name): + return False + getattr(instance, self.name).save(*formatted, save=save) return True