diff --git a/.env.example b/.env.example index 4c1c2eefe..2be08224b 100644 --- a/.env.example +++ b/.env.example @@ -120,3 +120,14 @@ OTEL_SERVICE_NAME= # for your instance: # https://docs.djangoproject.com/en/3.2/ref/settings/#secure-proxy-ssl-header HTTP_X_FORWARDED_PROTO=false + +# TOTP settings +# TWO_FACTOR_LOGIN_VALIDITY_WINDOW sets the number of codes either side +# which will be accepted. +TWO_FACTOR_LOGIN_VALIDITY_WINDOW=2 +TWO_FACTOR_LOGIN_MAX_SECONDS=60 + +# Additional hosts to allow in the Content-Security-Policy, "self" (should be DOMAIN) +# and AWS_S3_CUSTOM_DOMAIN (if used) are added by default. +# Value should be a comma-separated list of host names. +CSP_ADDITIONAL_HOSTS= diff --git a/bookwyrm/activitypub/base_activity.py b/bookwyrm/activitypub/base_activity.py index 02e5395fc..6751f9c8e 100644 --- a/bookwyrm/activitypub/base_activity.py +++ b/bookwyrm/activitypub/base_activity.py @@ -100,9 +100,27 @@ class ActivityObject: # pylint: disable=too-many-locals,too-many-branches,too-many-arguments def to_model( - self, model=None, instance=None, allow_create=True, save=True, overwrite=True + self, + model=None, + instance=None, + allow_create=True, + save=True, + overwrite=True, + allow_external_connections=True, ): - """convert from an activity to a model instance""" + """convert from an activity to a model instance. Args: + model: the django model that this object is being converted to + (will guess if not known) + instance: an existing database entry that is going to be updated by + this activity + allow_create: whether a new object should be created if there is no + existing object is provided or found matching the remote_id + save: store in the database if true, return an unsaved model obj if false + overwrite: replace fields in the database with this activity if true, + only update blank fields if false + allow_external_connections: look up missing data if true, + throw an exception if false and an external connection is needed + """ model = model or get_model_from_type(self.type) # only reject statuses if we're potentially creating them @@ -127,7 +145,10 @@ class ActivityObject: for field in instance.simple_fields: try: changed = field.set_field_from_activity( - instance, self, overwrite=overwrite + instance, + self, + overwrite=overwrite, + allow_external_connections=allow_external_connections, ) if changed: update_fields.append(field.name) @@ -138,7 +159,11 @@ class ActivityObject: # too early and jank up users for field in instance.image_fields: changed = field.set_field_from_activity( - instance, self, save=save, overwrite=overwrite + instance, + self, + save=save, + overwrite=overwrite, + allow_external_connections=allow_external_connections, ) if changed: update_fields.append(field.name) @@ -162,7 +187,11 @@ class ActivityObject: # add many to many fields, which have to be set post-save for field in instance.many_to_many_fields: # mention books/users, for example - field.set_field_from_activity(instance, self) + field.set_field_from_activity( + instance, + self, + allow_external_connections=allow_external_connections, + ) # reversed relationships in the models for ( @@ -266,10 +295,22 @@ def get_model_from_type(activity_type): return model[0] +# pylint: disable=too-many-arguments def resolve_remote_id( - remote_id, model=None, refresh=False, save=True, get_activity=False + remote_id, + model=None, + refresh=False, + save=True, + get_activity=False, + allow_external_connections=True, ): - """take a remote_id and return an instance, creating if necessary""" + """take a remote_id and return an instance, creating if necessary. Args: + remote_id: the unique url for looking up the object in the db or by http + model: a string or object representing the model that corresponds to the object + save: whether to return an unsaved database entry or a saved one + get_activity: whether to return the activitypub object or the model object + allow_external_connections: whether to make http connections + """ if model: # a bonus check we can do if we already know the model if isinstance(model, str): model = apps.get_model(f"bookwyrm.{model}", require_ready=True) @@ -277,6 +318,13 @@ def resolve_remote_id( if result and not refresh: return result if not get_activity else result.to_activity_dataclass() + # The above block will return the object if it already exists in the database. + # If it doesn't, an external connection would be needed, so check if that's cool + if not allow_external_connections: + raise ActivitySerializerError( + "Unable to serialize object without making external HTTP requests" + ) + # load the data and create the object try: data = get_data(remote_id) diff --git a/bookwyrm/activitypub/verbs.py b/bookwyrm/activitypub/verbs.py index b8c0ae779..4b7514b5a 100644 --- a/bookwyrm/activitypub/verbs.py +++ b/bookwyrm/activitypub/verbs.py @@ -14,12 +14,12 @@ class Verb(ActivityObject): actor: str object: ActivityObject - def action(self): + def action(self, allow_external_connections=True): """usually we just want to update and save""" # self.object may return None if the object is invalid in an expected way # ie, Question type if self.object: - self.object.to_model() + self.object.to_model(allow_external_connections=allow_external_connections) # pylint: disable=invalid-name @@ -42,7 +42,7 @@ class Delete(Verb): cc: List[str] = field(default_factory=lambda: []) type: str = "Delete" - def action(self): + def action(self, allow_external_connections=True): """find and delete the activity object""" if not self.object: return @@ -52,7 +52,11 @@ class Delete(Verb): model = apps.get_model("bookwyrm.User") obj = model.find_existing_by_remote_id(self.object) else: - obj = self.object.to_model(save=False, allow_create=False) + obj = self.object.to_model( + save=False, + allow_create=False, + allow_external_connections=allow_external_connections, + ) if obj: obj.delete() @@ -67,11 +71,13 @@ class Update(Verb): to: List[str] type: str = "Update" - def action(self): + def action(self, allow_external_connections=True): """update a model instance from the dataclass""" if not self.object: return - self.object.to_model(allow_create=False) + self.object.to_model( + allow_create=False, allow_external_connections=allow_external_connections + ) @dataclass(init=False) @@ -80,7 +86,7 @@ class Undo(Verb): type: str = "Undo" - def action(self): + def action(self, allow_external_connections=True): """find and remove the activity object""" if isinstance(self.object, str): # it may be that something should be done with these, but idk what @@ -92,13 +98,28 @@ class Undo(Verb): model = None if self.object.type == "Follow": model = apps.get_model("bookwyrm.UserFollows") - obj = self.object.to_model(model=model, save=False, allow_create=False) + obj = self.object.to_model( + model=model, + save=False, + allow_create=False, + allow_external_connections=allow_external_connections, + ) if not obj: # this could be a follow request not a follow proper model = apps.get_model("bookwyrm.UserFollowRequest") - obj = self.object.to_model(model=model, save=False, allow_create=False) + obj = self.object.to_model( + model=model, + save=False, + allow_create=False, + allow_external_connections=allow_external_connections, + ) else: - obj = self.object.to_model(model=model, save=False, allow_create=False) + obj = self.object.to_model( + model=model, + save=False, + allow_create=False, + allow_external_connections=allow_external_connections, + ) if not obj: # if we don't have the object, we can't undo it. happens a lot with boosts return @@ -112,9 +133,9 @@ class Follow(Verb): object: str type: str = "Follow" - def action(self): + def action(self, allow_external_connections=True): """relationship save""" - self.to_model() + self.to_model(allow_external_connections=allow_external_connections) @dataclass(init=False) @@ -124,9 +145,9 @@ class Block(Verb): object: str type: str = "Block" - def action(self): + def action(self, allow_external_connections=True): """relationship save""" - self.to_model() + self.to_model(allow_external_connections=allow_external_connections) @dataclass(init=False) @@ -136,7 +157,7 @@ class Accept(Verb): object: Follow type: str = "Accept" - def action(self): + def action(self, allow_external_connections=True): """accept a request""" obj = self.object.to_model(save=False, allow_create=True) obj.accept() @@ -149,7 +170,7 @@ class Reject(Verb): object: Follow type: str = "Reject" - def action(self): + def action(self, allow_external_connections=True): """reject a follow request""" obj = self.object.to_model(save=False, allow_create=False) obj.reject() @@ -163,7 +184,7 @@ class Add(Verb): object: CollectionItem type: str = "Add" - def action(self): + def action(self, allow_external_connections=True): """figure out the target to assign the item to a collection""" target = resolve_remote_id(self.target) item = self.object.to_model(save=False) @@ -177,7 +198,7 @@ class Remove(Add): type: str = "Remove" - def action(self): + def action(self, allow_external_connections=True): """find and remove the activity object""" obj = self.object.to_model(save=False, allow_create=False) if obj: @@ -191,9 +212,9 @@ class Like(Verb): object: str type: str = "Like" - def action(self): + def action(self, allow_external_connections=True): """like""" - self.to_model() + self.to_model(allow_external_connections=allow_external_connections) # pylint: disable=invalid-name @@ -207,6 +228,6 @@ class Announce(Verb): object: str type: str = "Announce" - def action(self): + def action(self, allow_external_connections=True): """boost""" - self.to_model() + self.to_model(allow_external_connections=allow_external_connections) diff --git a/bookwyrm/forms/landing.py b/bookwyrm/forms/landing.py index bd9884bc3..1da4fc4f1 100644 --- a/bookwyrm/forms/landing.py +++ b/bookwyrm/forms/landing.py @@ -8,6 +8,7 @@ import pyotp from bookwyrm import models from bookwyrm.settings import DOMAIN +from bookwyrm.settings import TWO_FACTOR_LOGIN_VALIDITY_WINDOW from .custom_form import CustomForm @@ -108,7 +109,7 @@ class Confirm2FAForm(CustomForm): otp = self.data.get("otp") totp = pyotp.TOTP(self.instance.otp_secret) - if not totp.verify(otp): + if not totp.verify(otp, valid_window=TWO_FACTOR_LOGIN_VALIDITY_WINDOW): if self.instance.hotp_secret: # maybe it's a backup code? diff --git a/bookwyrm/forms/status.py b/bookwyrm/forms/status.py index 0800166bf..b562595ee 100644 --- a/bookwyrm/forms/status.py +++ b/bookwyrm/forms/status.py @@ -53,6 +53,7 @@ class QuotationForm(CustomForm): "sensitive", "privacy", "position", + "endposition", "position_mode", ] diff --git a/bookwyrm/migrations/0174_auto_20230130_1240.py b/bookwyrm/migrations/0174_auto_20230130_1240.py new file mode 100644 index 000000000..7337b6b46 --- /dev/null +++ b/bookwyrm/migrations/0174_auto_20230130_1240.py @@ -0,0 +1,35 @@ +# Generated by Django 3.2.16 on 2023-01-30 12:40 + +import django.core.validators +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ("auth", "0012_alter_user_first_name_max_length"), + ("bookwyrm", "0173_default_user_auth_group_setting"), + ] + + operations = [ + migrations.AddField( + model_name="quotation", + name="endposition", + field=models.IntegerField( + blank=True, + null=True, + validators=[django.core.validators.MinValueValidator(0)], + ), + ), + migrations.AlterField( + model_name="sitesettings", + name="default_user_auth_group", + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.PROTECT, + to="auth.group", + ), + ), + ] diff --git a/bookwyrm/migrations/0174_auto_20230222_1742.py b/bookwyrm/migrations/0174_auto_20230222_1742.py new file mode 100644 index 000000000..f30d61a46 --- /dev/null +++ b/bookwyrm/migrations/0174_auto_20230222_1742.py @@ -0,0 +1,46 @@ +# Generated by Django 3.2.18 on 2023-02-22 17:42 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("bookwyrm", "0174_auto_20230130_1240"), + ] + + operations = [ + migrations.AddField( + model_name="notification", + name="related_link_domains", + field=models.ManyToManyField(to="bookwyrm.LinkDomain"), + ), + migrations.AlterField( + model_name="notification", + name="notification_type", + field=models.CharField( + choices=[ + ("FAVORITE", "Favorite"), + ("REPLY", "Reply"), + ("MENTION", "Mention"), + ("TAG", "Tag"), + ("FOLLOW", "Follow"), + ("FOLLOW_REQUEST", "Follow Request"), + ("BOOST", "Boost"), + ("IMPORT", "Import"), + ("ADD", "Add"), + ("REPORT", "Report"), + ("LINK_DOMAIN", "Link Domain"), + ("INVITE", "Invite"), + ("ACCEPT", "Accept"), + ("JOIN", "Join"), + ("LEAVE", "Leave"), + ("REMOVE", "Remove"), + ("GROUP_PRIVACY", "Group Privacy"), + ("GROUP_NAME", "Group Name"), + ("GROUP_DESCRIPTION", "Group Description"), + ], + max_length=255, + ), + ), + ] diff --git a/bookwyrm/models/activitypub_mixin.py b/bookwyrm/models/activitypub_mixin.py index 9361854ba..ec1c34a40 100644 --- a/bookwyrm/models/activitypub_mixin.py +++ b/bookwyrm/models/activitypub_mixin.py @@ -21,7 +21,7 @@ from django.utils.http import http_date from bookwyrm import activitypub from bookwyrm.settings import USER_AGENT, PAGE_LENGTH from bookwyrm.signatures import make_signature, make_digest -from bookwyrm.tasks import app, MEDIUM +from bookwyrm.tasks import app, MEDIUM, BROADCAST from bookwyrm.models.fields import ImageField, ManyToManyField logger = logging.getLogger(__name__) @@ -126,7 +126,7 @@ class ActivitypubMixin: # there OUGHT to be only one match return match.first() - def broadcast(self, activity, sender, software=None, queue=MEDIUM): + def broadcast(self, activity, sender, software=None, queue=BROADCAST): """send out an activity""" broadcast_task.apply_async( args=( @@ -198,7 +198,7 @@ class ActivitypubMixin: class ObjectMixin(ActivitypubMixin): """add this mixin for object models that are AP serializable""" - def save(self, *args, created=None, software=None, priority=MEDIUM, **kwargs): + def save(self, *args, created=None, software=None, priority=BROADCAST, **kwargs): """broadcast created/updated/deleted objects as appropriate""" broadcast = kwargs.get("broadcast", True) # this bonus kwarg would cause an error in the base save method @@ -506,7 +506,7 @@ def unfurl_related_field(related_field, sort_field=None): return related_field.remote_id -@app.task(queue=MEDIUM) +@app.task(queue=BROADCAST) def broadcast_task(sender_id: int, activity: str, recipients: List[str]): """the celery task for broadcast""" user_model = apps.get_model("bookwyrm.User", require_ready=True) diff --git a/bookwyrm/models/fields.py b/bookwyrm/models/fields.py index d11f5fb1d..a970e4124 100644 --- a/bookwyrm/models/fields.py +++ b/bookwyrm/models/fields.py @@ -67,7 +67,9 @@ class ActivitypubFieldMixin: self.activitypub_field = activitypub_field super().__init__(*args, **kwargs) - def set_field_from_activity(self, instance, data, overwrite=True): + def set_field_from_activity( + self, instance, data, overwrite=True, allow_external_connections=True + ): """helper function for assinging a value to the field. Returns if changed""" try: value = getattr(data, self.get_activitypub_field()) @@ -76,7 +78,9 @@ class ActivitypubFieldMixin: if self.get_activitypub_field() != "attributedTo": raise value = getattr(data, "actor") - formatted = self.field_from_activity(value) + formatted = self.field_from_activity( + value, allow_external_connections=allow_external_connections + ) if formatted is None or formatted is MISSING or formatted == {}: return False @@ -116,7 +120,8 @@ class ActivitypubFieldMixin: return {self.activitypub_wrapper: value} return value - def field_from_activity(self, value): + # pylint: disable=unused-argument + def field_from_activity(self, value, allow_external_connections=True): """formatter to convert activitypub into a model value""" if value and hasattr(self, "activitypub_wrapper"): value = value.get(self.activitypub_wrapper) @@ -138,7 +143,7 @@ class ActivitypubRelatedFieldMixin(ActivitypubFieldMixin): self.load_remote = load_remote super().__init__(*args, **kwargs) - def field_from_activity(self, value): + def field_from_activity(self, value, allow_external_connections=True): if not value: return None @@ -159,7 +164,11 @@ class ActivitypubRelatedFieldMixin(ActivitypubFieldMixin): if not self.load_remote: # only look in the local database return related_model.find_existing_by_remote_id(value) - return activitypub.resolve_remote_id(value, model=related_model) + return activitypub.resolve_remote_id( + value, + model=related_model, + allow_external_connections=allow_external_connections, + ) class RemoteIdField(ActivitypubFieldMixin, models.CharField): @@ -219,7 +228,9 @@ class PrivacyField(ActivitypubFieldMixin, models.CharField): super().__init__(*args, max_length=255, choices=PrivacyLevels, default="public") # pylint: disable=invalid-name - def set_field_from_activity(self, instance, data, overwrite=True): + def set_field_from_activity( + self, instance, data, overwrite=True, allow_external_connections=True + ): if not overwrite: return False @@ -234,7 +245,11 @@ class PrivacyField(ActivitypubFieldMixin, models.CharField): break if not user_field: raise ValidationError("No user field found for privacy", data) - user = activitypub.resolve_remote_id(getattr(data, user_field), model="User") + user = activitypub.resolve_remote_id( + getattr(data, user_field), + model="User", + allow_external_connections=allow_external_connections, + ) if to == [self.public]: setattr(instance, self.name, "public") @@ -295,13 +310,17 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField): self.link_only = link_only super().__init__(*args, **kwargs) - def set_field_from_activity(self, instance, data, overwrite=True): + def set_field_from_activity( + self, instance, data, overwrite=True, allow_external_connections=True + ): """helper function for assigning 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) + formatted = self.field_from_activity( + value, allow_external_connections=allow_external_connections + ) if formatted is None or formatted is MISSING: return False getattr(instance, self.name).set(formatted) @@ -313,7 +332,7 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField): return f"{value.instance.remote_id}/{self.name}" return [i.remote_id for i in value.all()] - def field_from_activity(self, value): + def field_from_activity(self, value, allow_external_connections=True): if value is None or value is MISSING: return None if not isinstance(value, list): @@ -326,7 +345,11 @@ class ManyToManyField(ActivitypubFieldMixin, models.ManyToManyField): except ValidationError: continue items.append( - activitypub.resolve_remote_id(remote_id, model=self.related_model) + activitypub.resolve_remote_id( + remote_id, + model=self.related_model, + allow_external_connections=allow_external_connections, + ) ) return items @@ -353,7 +376,7 @@ class TagField(ManyToManyField): ) return tags - def field_from_activity(self, value): + def field_from_activity(self, value, allow_external_connections=True): if not isinstance(value, list): return None items = [] @@ -366,7 +389,11 @@ class TagField(ManyToManyField): # tags can contain multiple types continue items.append( - activitypub.resolve_remote_id(link.href, model=self.related_model) + activitypub.resolve_remote_id( + link.href, + model=self.related_model, + allow_external_connections=allow_external_connections, + ) ) return items @@ -390,11 +417,15 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): self.alt_field = alt_field super().__init__(*args, **kwargs) - # pylint: disable=arguments-differ,arguments-renamed - def set_field_from_activity(self, instance, data, save=True, overwrite=True): + # pylint: disable=arguments-differ,arguments-renamed,too-many-arguments + def set_field_from_activity( + self, instance, data, save=True, overwrite=True, allow_external_connections=True + ): """helper function for assinging a value to the field""" value = getattr(data, self.get_activitypub_field()) - formatted = self.field_from_activity(value) + formatted = self.field_from_activity( + value, allow_external_connections=allow_external_connections + ) if formatted is None or formatted is MISSING: return False @@ -426,7 +457,7 @@ class ImageField(ActivitypubFieldMixin, models.ImageField): return activitypub.Document(url=url, name=alt) - def field_from_activity(self, value): + def field_from_activity(self, value, allow_external_connections=True): image_slug = value # when it's an inline image (User avatar/icon, Book cover), it's a json # blob, but when it's an attached image, it's just a url @@ -481,7 +512,7 @@ class DateTimeField(ActivitypubFieldMixin, models.DateTimeField): return None return value.isoformat() - def field_from_activity(self, value): + def field_from_activity(self, value, allow_external_connections=True): try: date_value = dateutil.parser.parse(value) try: @@ -495,7 +526,7 @@ class DateTimeField(ActivitypubFieldMixin, models.DateTimeField): class HtmlField(ActivitypubFieldMixin, models.TextField): """a text field for storing html""" - def field_from_activity(self, value): + def field_from_activity(self, value, allow_external_connections=True): if not value or value == MISSING: return None return clean(value) diff --git a/bookwyrm/models/notification.py b/bookwyrm/models/notification.py index fa2ce54e2..29f7b0c2d 100644 --- a/bookwyrm/models/notification.py +++ b/bookwyrm/models/notification.py @@ -2,8 +2,8 @@ from django.db import models, transaction from django.dispatch import receiver from .base_model import BookWyrmModel -from . import Boost, Favorite, GroupMemberInvitation, ImportJob, ListItem, Report -from . import Status, User, UserFollowRequest +from . import Boost, Favorite, GroupMemberInvitation, ImportJob, LinkDomain +from . import ListItem, Report, Status, User, UserFollowRequest class Notification(BookWyrmModel): @@ -28,6 +28,7 @@ class Notification(BookWyrmModel): # Admin REPORT = "REPORT" + LINK_DOMAIN = "LINK_DOMAIN" # Groups INVITE = "INVITE" @@ -43,7 +44,7 @@ class Notification(BookWyrmModel): NotificationType = models.TextChoices( # there has got be a better way to do this "NotificationType", - f"{FAVORITE} {REPLY} {MENTION} {TAG} {FOLLOW} {FOLLOW_REQUEST} {BOOST} {IMPORT} {ADD} {REPORT} {INVITE} {ACCEPT} {JOIN} {LEAVE} {REMOVE} {GROUP_PRIVACY} {GROUP_NAME} {GROUP_DESCRIPTION}", + f"{FAVORITE} {REPLY} {MENTION} {TAG} {FOLLOW} {FOLLOW_REQUEST} {BOOST} {IMPORT} {ADD} {REPORT} {LINK_DOMAIN} {INVITE} {ACCEPT} {JOIN} {LEAVE} {REMOVE} {GROUP_PRIVACY} {GROUP_NAME} {GROUP_DESCRIPTION}", ) user = models.ForeignKey("User", on_delete=models.CASCADE) @@ -64,6 +65,7 @@ class Notification(BookWyrmModel): "ListItem", symmetrical=False, related_name="notifications" ) related_reports = models.ManyToManyField("Report", symmetrical=False) + related_link_domains = models.ManyToManyField("LinkDomain", symmetrical=False) @classmethod @transaction.atomic @@ -241,6 +243,26 @@ def notify_admins_on_report(sender, instance, created, *args, **kwargs): notification.related_reports.add(instance) +@receiver(models.signals.post_save, sender=LinkDomain) +@transaction.atomic +# pylint: disable=unused-argument +def notify_admins_on_link_domain(sender, instance, created, *args, **kwargs): + """a new link domain needs to be verified""" + if not created: + # otherwise you'll get a notification when you approve a domain + return + + # moderators and superusers should be notified + admins = User.admins() + for admin in admins: + notification, _ = Notification.objects.get_or_create( + user=admin, + notification_type=Notification.LINK_DOMAIN, + read=False, + ) + notification.related_link_domains.add(instance) + + @receiver(models.signals.post_save, sender=GroupMemberInvitation) # pylint: disable=unused-argument def notify_user_on_group_invite(sender, instance, *args, **kwargs): diff --git a/bookwyrm/models/relationship.py b/bookwyrm/models/relationship.py index c8a508117..422967855 100644 --- a/bookwyrm/models/relationship.py +++ b/bookwyrm/models/relationship.py @@ -171,7 +171,11 @@ class UserFollowRequest(ActivitypubMixin, UserRelationship): return with transaction.atomic(): - UserFollows.from_request(self) + try: + UserFollows.from_request(self) + except IntegrityError: + # this just means we already saved this relationship + pass if self.id: self.delete() diff --git a/bookwyrm/models/status.py b/bookwyrm/models/status.py index bd99deee5..e51c7b2a1 100644 --- a/bookwyrm/models/status.py +++ b/bookwyrm/models/status.py @@ -329,6 +329,9 @@ class Quotation(BookStatus): position = models.IntegerField( validators=[MinValueValidator(0)], null=True, blank=True ) + endposition = models.IntegerField( + validators=[MinValueValidator(0)], null=True, blank=True + ) position_mode = models.CharField( max_length=3, choices=ProgressMode.choices, diff --git a/bookwyrm/settings.py b/bookwyrm/settings.py index a86586eeb..bf0467ebc 100644 --- a/bookwyrm/settings.py +++ b/bookwyrm/settings.py @@ -330,6 +330,7 @@ IMAGEKIT_DEFAULT_CACHEFILE_STRATEGY = "bookwyrm.thumbnail_generation.Strategy" # https://docs.djangoproject.com/en/3.2/howto/static-files/ PROJECT_DIR = os.path.dirname(os.path.abspath(__file__)) +CSP_ADDITIONAL_HOSTS = env.list("CSP_ADDITIONAL_HOSTS", []) # Storage @@ -361,15 +362,15 @@ if USE_S3: MEDIA_FULL_URL = MEDIA_URL STATIC_FULL_URL = STATIC_URL DEFAULT_FILE_STORAGE = "bookwyrm.storage_backends.ImagesStorage" - CSP_DEFAULT_SRC = ("'self'", AWS_S3_CUSTOM_DOMAIN) - CSP_SCRIPT_SRC = ("'self'", AWS_S3_CUSTOM_DOMAIN) + CSP_DEFAULT_SRC = ["'self'", AWS_S3_CUSTOM_DOMAIN] + CSP_ADDITIONAL_HOSTS + CSP_SCRIPT_SRC = ["'self'", AWS_S3_CUSTOM_DOMAIN] + CSP_ADDITIONAL_HOSTS else: STATIC_URL = "/static/" MEDIA_URL = "/images/" MEDIA_FULL_URL = f"{PROTOCOL}://{DOMAIN}{MEDIA_URL}" STATIC_FULL_URL = f"{PROTOCOL}://{DOMAIN}{STATIC_URL}" - CSP_DEFAULT_SRC = "'self'" - CSP_SCRIPT_SRC = "'self'" + CSP_DEFAULT_SRC = ["'self'"] + CSP_ADDITIONAL_HOSTS + CSP_SCRIPT_SRC = ["'self'"] + CSP_ADDITIONAL_HOSTS CSP_INCLUDE_NONCE_IN = ["script-src"] @@ -377,7 +378,8 @@ OTEL_EXPORTER_OTLP_ENDPOINT = env("OTEL_EXPORTER_OTLP_ENDPOINT", None) OTEL_EXPORTER_OTLP_HEADERS = env("OTEL_EXPORTER_OTLP_HEADERS", None) OTEL_SERVICE_NAME = env("OTEL_SERVICE_NAME", None) -TWO_FACTOR_LOGIN_MAX_SECONDS = 60 +TWO_FACTOR_LOGIN_MAX_SECONDS = env.int("TWO_FACTOR_LOGIN_MAX_SECONDS", 60) +TWO_FACTOR_LOGIN_VALIDITY_WINDOW = env.int("TWO_FACTOR_LOGIN_VALIDITY_WINDOW", 2) HTTP_X_FORWARDED_PROTO = env.bool("SECURE_PROXY_SSL_HEADER", False) if HTTP_X_FORWARDED_PROTO: diff --git a/bookwyrm/tasks.py b/bookwyrm/tasks.py index ec018e179..91977afda 100644 --- a/bookwyrm/tasks.py +++ b/bookwyrm/tasks.py @@ -16,3 +16,5 @@ MEDIUM = "medium_priority" HIGH = "high_priority" # import items get their own queue because they're such a pain in the ass IMPORTS = "imports" +# I keep making more queues?? this one broadcasting out +BROADCAST = "broadcast" diff --git a/bookwyrm/templates/discover/large-book.html b/bookwyrm/templates/discover/large-book.html index f016a2f65..939544987 100644 --- a/bookwyrm/templates/discover/large-book.html +++ b/bookwyrm/templates/discover/large-book.html @@ -46,7 +46,7 @@
- {% include "snippets/status/content_status.html" with hide_book=True trim_length=70 hide_more=True %} + {% include "snippets/status/content_status.html" with hide_book=True trim_length=70 hide_more=True expand=False %}
{% trans "View status" %} diff --git a/bookwyrm/templates/feed/layout.html b/bookwyrm/templates/feed/layout.html index 16a868c2a..b70ed99ea 100644 --- a/bookwyrm/templates/feed/layout.html +++ b/bookwyrm/templates/feed/layout.html @@ -23,7 +23,7 @@ {% block panel %}{% endblock %} {% if activities %} - {% include 'snippets/pagination.html' with page=activities path=path anchor="#feed" %} + {% include 'snippets/pagination.html' with page=activities path=path anchor="#feed" mode="chronological" %} {% endif %} diff --git a/bookwyrm/templates/notifications/item.html b/bookwyrm/templates/notifications/item.html index e8e2dcb26..b53abe3d1 100644 --- a/bookwyrm/templates/notifications/item.html +++ b/bookwyrm/templates/notifications/item.html @@ -17,6 +17,8 @@ {% include 'notifications/items/add.html' %} {% elif notification.notification_type == 'REPORT' %} {% include 'notifications/items/report.html' %} +{% elif notification.notification_type == 'LINK_DOMAIN' %} + {% include 'notifications/items/link_domain.html' %} {% elif notification.notification_type == 'INVITE' %} {% include 'notifications/items/invite.html' %} {% elif notification.notification_type == 'ACCEPT' %} diff --git a/bookwyrm/templates/notifications/items/link_domain.html b/bookwyrm/templates/notifications/items/link_domain.html new file mode 100644 index 000000000..aaed830ed --- /dev/null +++ b/bookwyrm/templates/notifications/items/link_domain.html @@ -0,0 +1,20 @@ +{% extends 'notifications/items/layout.html' %} +{% load humanize %} +{% load i18n %} + +{% block primary_link %}{% spaceless %} +{% url 'settings-link-domain' %} +{% endspaceless %}{% endblock %} + +{% block icon %} + +{% endblock %} + +{% block description %} + {% url 'settings-link-domain' as path %} + {% blocktrans trimmed count counter=notification.related_link_domains.count with display_count=notification.related_link_domains.count|intcomma %} + A new link domain needs review + {% plural %} + {{ display_count }} new link domains need moderation + {% endblocktrans %} +{% endblock %} diff --git a/bookwyrm/templates/settings/celery.html b/bookwyrm/templates/settings/celery.html index 6a3ee6574..65315da01 100644 --- a/bookwyrm/templates/settings/celery.html +++ b/bookwyrm/templates/settings/celery.html @@ -20,31 +20,37 @@ {% if queues %}

{% trans "Queues" %}

-
-
+
+

{% trans "Low priority" %}

{{ queues.low_priority|intcomma }}

-
+

{% trans "Medium priority" %}

{{ queues.medium_priority|intcomma }}

-
+

{% trans "High priority" %}

{{ queues.high_priority|intcomma }}

-
+

{% trans "Imports" %}

{{ queues.imports|intcomma }}

+
+
+

{% trans "Broadcasts" %}

+

{{ queues.broadcast|intcomma }}

+
+
{% else %} diff --git a/bookwyrm/templates/snippets/create_status/quotation.html b/bookwyrm/templates/snippets/create_status/quotation.html index a9ddb17f4..bd1d817ad 100644 --- a/bookwyrm/templates/snippets/create_status/quotation.html +++ b/bookwyrm/templates/snippets/create_status/quotation.html @@ -65,6 +65,22 @@ uuid: a unique identifier used to make html "id" attributes unique and clarify j {% if not draft %}data-cache-draft="id_position_{{ book.id }}_{{ type }}"{% endif %} > +
+ {% trans "to" %} +
+
+ +
{% endblock %} diff --git a/bookwyrm/templates/snippets/pagination.html b/bookwyrm/templates/snippets/pagination.html index db2396ae7..5ab724c06 100644 --- a/bookwyrm/templates/snippets/pagination.html +++ b/bookwyrm/templates/snippets/pagination.html @@ -9,7 +9,11 @@ {% endif %}> + {% if mode == "chronological" %} + {% trans "Newer" %} + {% else %} {% trans "Previous" %} + {% endif %} diff --git a/bookwyrm/templates/snippets/status/content_status.html b/bookwyrm/templates/snippets/status/content_status.html index b8c719ec0..138a442af 100644 --- a/bookwyrm/templates/snippets/status/content_status.html +++ b/bookwyrm/templates/snippets/status/content_status.html @@ -99,9 +99,9 @@ — {% include 'snippets/book_titleby.html' with book=status.book %} {% if status.position %} {% if status.position_mode == 'PG' %} - {% blocktrans with page=status.position|intcomma %}(Page {{ page }}){% endblocktrans %} + {% blocktrans with page=status.position|intcomma %}(Page {{ page }}{% endblocktrans%}{% if status.endposition and status.endposition != status.position %} - {% blocktrans with endpage=status.endposition|intcomma %}{{ endpage }}{% endblocktrans %}{% endif%}) {% else %} - {% blocktrans with percent=status.position %}({{ percent }}%){% endblocktrans %} + {% blocktrans with percent=status.position %}({{ percent }}%{% endblocktrans %}{% if status.endposition and status.endposition != status.position %}{% blocktrans with endpercent=status.endposition|intcomma %} - {{ endpercent }}%{% endblocktrans %}{% endif %}) {% endif %} {% endif %}

diff --git a/bookwyrm/templates/user/reviews_comments.html b/bookwyrm/templates/user/reviews_comments.html index f5c5c9265..d85f8f512 100644 --- a/bookwyrm/templates/user/reviews_comments.html +++ b/bookwyrm/templates/user/reviews_comments.html @@ -25,6 +25,6 @@ {% endif %} - {% include 'snippets/pagination.html' with page=activities path=path %} + {% include 'snippets/pagination.html' with page=activities path=path mode="chronological" %} {% endblock %} diff --git a/bookwyrm/templates/user/user.html b/bookwyrm/templates/user/user.html index 42b0ffbb5..0d015760c 100755 --- a/bookwyrm/templates/user/user.html +++ b/bookwyrm/templates/user/user.html @@ -87,7 +87,7 @@ {% trans "Back" %} - + {% endblock %} diff --git a/bookwyrm/tests/test_signing.py b/bookwyrm/tests/test_signing.py index 8a7f65249..cde193f08 100644 --- a/bookwyrm/tests/test_signing.py +++ b/bookwyrm/tests/test_signing.py @@ -95,7 +95,8 @@ class Signature(TestCase): def test_correct_signature(self): """this one should just work""" - response = self.send_test_request(sender=self.mouse) + with patch("bookwyrm.models.relationship.UserFollowRequest.accept"): + response = self.send_test_request(sender=self.mouse) self.assertEqual(response.status_code, 200) def test_wrong_signature(self): @@ -124,8 +125,12 @@ class Signature(TestCase): ) with patch("bookwyrm.models.user.get_remote_reviews.delay"): - response = self.send_test_request(sender=self.fake_remote) + with patch( + "bookwyrm.models.relationship.UserFollowRequest.accept" + ) as accept_mock: + response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 200) + self.assertTrue(accept_mock.called) @responses.activate def test_key_needs_refresh(self): @@ -148,16 +153,28 @@ class Signature(TestCase): with patch("bookwyrm.models.user.get_remote_reviews.delay"): # Key correct: - response = self.send_test_request(sender=self.fake_remote) + with patch( + "bookwyrm.models.relationship.UserFollowRequest.accept" + ) as accept_mock: + response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 200) + self.assertTrue(accept_mock.called) # Old key is cached, so still works: - response = self.send_test_request(sender=self.fake_remote) + with patch( + "bookwyrm.models.relationship.UserFollowRequest.accept" + ) as accept_mock: + response = self.send_test_request(sender=self.fake_remote) self.assertEqual(response.status_code, 200) + self.assertTrue(accept_mock.called) # Try with new key: - response = self.send_test_request(sender=new_sender) + with patch( + "bookwyrm.models.relationship.UserFollowRequest.accept" + ) as accept_mock: + response = self.send_test_request(sender=new_sender) self.assertEqual(response.status_code, 200) + self.assertTrue(accept_mock.called) # Now the old key will fail: response = self.send_test_request(sender=self.fake_remote) diff --git a/bookwyrm/tests/views/books/test_book.py b/bookwyrm/tests/views/books/test_book.py index e536ef736..a829c4a4b 100644 --- a/bookwyrm/tests/views/books/test_book.py +++ b/bookwyrm/tests/views/books/test_book.py @@ -257,6 +257,33 @@ class BookViews(TestCase): self.assertEqual(mock.call_args[0][0], "https://openlibrary.org/book/123") self.assertEqual(result.status_code, 302) + @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") + @patch("bookwyrm.activitystreams.add_status_task.delay") + def test_quotation_endposition(self, *_): + """make sure the endposition is served as well""" + view = views.Book.as_view() + + _ = models.Quotation.objects.create( + user=self.local_user, + book=self.book, + content="hi", + quote="wow", + position=12, + endposition=13, + ) + + request = self.factory.get("") + request.user = self.local_user + + with patch("bookwyrm.views.books.books.is_api_request") as is_api: + is_api.return_value = False + result = view(request, self.book.id, user_statuses="quotation") + self.assertIsInstance(result, TemplateResponse) + validate_html(result.render()) + print(result.render()) + self.assertEqual(result.status_code, 200) + self.assertEqual(result.context_data["statuses"].object_list[0].endposition, 13) + def _setup_cover_url(): """creates cover url mock""" diff --git a/bookwyrm/tests/views/test_discover.py b/bookwyrm/tests/views/test_discover.py index b0989da8f..ffe8c51c9 100644 --- a/bookwyrm/tests/views/test_discover.py +++ b/bookwyrm/tests/views/test_discover.py @@ -11,6 +11,7 @@ from bookwyrm.tests.validate_html import validate_html class DiscoverViews(TestCase): """pages you land on without really trying""" + # pylint: disable=invalid-name def setUp(self): """we need basic test data and mocks""" self.factory = RequestFactory() @@ -43,7 +44,7 @@ class DiscoverViews(TestCase): @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async") @patch("bookwyrm.activitystreams.add_status_task.delay") - def test_discover_page(self, *_): + def test_discover_page_with_posts(self, *_): """there are so many views, this just makes sure it LOADS""" view = views.Discover.as_view() request = self.factory.get("") @@ -53,17 +54,34 @@ class DiscoverViews(TestCase): title="hi", parent_work=models.Work.objects.create(title="work") ) + models.ReviewRating.objects.create( + book=book, + user=self.local_user, + rating=4, + ) + models.Review.objects.create( + book=book, + user=self.local_user, + content="hello", + rating=4, + ) models.Comment.objects.create( book=book, user=self.local_user, content="hello", ) + models.Quotation.objects.create( + book=book, + user=self.local_user, + quote="beep", + content="hello", + ) models.Status.objects.create(user=self.local_user, content="beep") with patch( "bookwyrm.activitystreams.ActivityStream.get_activity_stream" ) as mock: - mock.return_value = models.Status.objects.all() + mock.return_value = models.Status.objects.select_subclasses().all() result = view(request) self.assertEqual(mock.call_count, 1) self.assertEqual(result.status_code, 200) diff --git a/bookwyrm/views/admin/celery_status.py b/bookwyrm/views/admin/celery_status.py index 8c6b9f5a3..e0b1fe18c 100644 --- a/bookwyrm/views/admin/celery_status.py +++ b/bookwyrm/views/admin/celery_status.py @@ -8,7 +8,7 @@ from django.views.decorators.http import require_GET import redis from celerywyrm import settings -from bookwyrm.tasks import app as celery +from bookwyrm.tasks import app as celery, LOW, MEDIUM, HIGH, IMPORTS, BROADCAST r = redis.from_url(settings.REDIS_BROKER_URL) @@ -35,10 +35,11 @@ class CeleryStatus(View): try: queues = { - "low_priority": r.llen("low_priority"), - "medium_priority": r.llen("medium_priority"), - "high_priority": r.llen("high_priority"), - "imports": r.llen("imports"), + LOW: r.llen(LOW), + MEDIUM: r.llen(MEDIUM), + HIGH: r.llen(HIGH), + IMPORTS: r.llen(IMPORTS), + BROADCAST: r.llen(BROADCAST), } # pylint: disable=broad-except except Exception as err: diff --git a/bookwyrm/views/inbox.py b/bookwyrm/views/inbox.py index 747b5eccd..9b2b238f8 100644 --- a/bookwyrm/views/inbox.py +++ b/bookwyrm/views/inbox.py @@ -64,7 +64,7 @@ class Inbox(View): high = ["Follow", "Accept", "Reject", "Block", "Unblock", "Undo"] priority = HIGH if activity_json["type"] in high else MEDIUM - activity_task.apply_async(args=(activity_json,), queue=priority) + sometimes_async_activity_task(activity_json, queue=priority) return HttpResponse() @@ -102,6 +102,19 @@ def raise_is_blocked_activity(activity_json): raise PermissionDenied() +def sometimes_async_activity_task(activity_json, queue=MEDIUM): + """Sometimes we can effectively respond to a request without queuing a new task, + and whever that is possible, we should do it.""" + activity = activitypub.parse(activity_json) + + # try resolving this activity without making any http requests + try: + activity.action(allow_external_connections=False) + except activitypub.ActivitySerializerError: + # if that doesn't work, run it asynchronously + activity_task.apply_async(args=(activity_json,), queue=queue) + + @app.task(queue=MEDIUM) def activity_task(activity_json): """do something with this json we think is legit""" diff --git a/contrib/systemd/bookwyrm-worker.service b/contrib/systemd/bookwyrm-worker.service index d79fdabf6..e4f1e98b5 100644 --- a/contrib/systemd/bookwyrm-worker.service +++ b/contrib/systemd/bookwyrm-worker.service @@ -6,7 +6,7 @@ After=network.target postgresql.service redis.service User=bookwyrm Group=bookwyrm WorkingDirectory=/opt/bookwyrm/ -ExecStart=/opt/bookwyrm/venv/bin/celery -A celerywyrm worker -l info -Q high_priority,medium_priority,low_priority,import +ExecStart=/opt/bookwyrm/venv/bin/celery -A celerywyrm worker -l info -Q high_priority,medium_priority,low_priority,import,broadcast StandardOutput=journal StandardError=inherit diff --git a/docker-compose.yml b/docker-compose.yml index 4ab7a1c34..4389c08aa 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -62,7 +62,7 @@ services: build: . networks: - main - command: celery -A celerywyrm worker -l info -Q high_priority,medium_priority,low_priority,imports + command: celery -A celerywyrm worker -l info -Q high_priority,medium_priority,low_priority,imports,broadcast volumes: - .:/app - static_volume:/app/static