Merge branch 'main' into tour

- we need to do this because of conflicting migrations

bookwyrm/connectors/connector_manager.py

@@ -53,7 +53,7 @@ async def get_results(session, url, min_confidence, query, connector):
     except asyncio.TimeoutError:
         logger.info("Connection timed out for url: %s", url)
     except aiohttp.ClientError as err:
-        logger.exception(err)
+        logger.info(err)
 
 
 async def async_connector_search(query, items, min_confidence):

bookwyrm/forms/edit_user.py

@@ -1,5 +1,8 @@
 """ using django model forms """
 from django import forms
+from django.contrib.auth.password_validation import validate_password
+from django.core.exceptions import ValidationError
+from django.utils.translation import gettext_lazy as _
 
 from bookwyrm import models
 from bookwyrm.models.fields import ClearableFileInputWithWarning
@@ -66,3 +69,33 @@ class DeleteUserForm(CustomForm):
     class Meta:
         model = models.User
         fields = ["password"]
+
+
+class ChangePasswordForm(CustomForm):
+    current_password = forms.CharField(widget=forms.PasswordInput)
+    confirm_password = forms.CharField(widget=forms.PasswordInput)
+
+    class Meta:
+        model = models.User
+        fields = ["password"]
+        widgets = {
+            "password": forms.PasswordInput(),
+        }
+
+    def clean(self):
+        """Make sure passwords match and are valid"""
+        current_password = self.data.get("current_password")
+        if not self.instance.check_password(current_password):
+            self.add_error("current_password", _("Incorrect password"))
+
+        cleaned_data = super().clean()
+        new_password = cleaned_data.get("password")
+        confirm_password = self.data.get("confirm_password")
+
+        if new_password != confirm_password:
+            self.add_error("confirm_password", _("Password does not match"))
+
+        try:
+            validate_password(new_password)
+        except ValidationError as err:
+            self.add_error("password", err)

bookwyrm/forms/landing.py

@@ -1,5 +1,7 @@
 """ Forms for the landing pages """
-from django.forms import PasswordInput
+from django import forms
+from django.contrib.auth.password_validation import validate_password
+from django.core.exceptions import ValidationError
 from django.utils.translation import gettext_lazy as _
 
 from bookwyrm import models
@@ -13,7 +15,7 @@ class LoginForm(CustomForm):
         fields = ["localname", "password"]
         help_texts = {f: None for f in fields}
         widgets = {
-            "password": PasswordInput(),
+            "password": forms.PasswordInput(),
         }
 
 
@@ -22,12 +24,16 @@ class RegisterForm(CustomForm):
         model = models.User
         fields = ["localname", "email", "password"]
         help_texts = {f: None for f in fields}
-        widgets = {"password": PasswordInput()}
+        widgets = {"password": forms.PasswordInput()}
 
     def clean(self):
         """Check if the username is taken"""
         cleaned_data = super().clean()
         localname = cleaned_data.get("localname").strip()
+        try:
+            validate_password(cleaned_data.get("password"))
+        except ValidationError as err:
+            self.add_error("password", err)
         if models.User.objects.filter(localname=localname).first():
             self.add_error("localname", _("User with this username already exists"))
 
@@ -43,3 +49,28 @@ class InviteRequestForm(CustomForm):
     class Meta:
         model = models.InviteRequest
         fields = ["email", "answer"]
+
+
+class PasswordResetForm(CustomForm):
+    confirm_password = forms.CharField(widget=forms.PasswordInput)
+
+    class Meta:
+        model = models.User
+        fields = ["password"]
+        widgets = {
+            "password": forms.PasswordInput(),
+        }
+
+    def clean(self):
+        """Make sure the passwords match and are valid"""
+        cleaned_data = super().clean()
+        new_password = cleaned_data.get("password")
+        confirm_password = self.data.get("confirm_password")
+
+        if new_password != confirm_password:
+            self.add_error("confirm_password", _("Password does not match"))
+
+        try:
+            validate_password(new_password)
+        except ValidationError as err:
+            self.add_error("password", err)

bookwyrm/migrations/0154_alter_user_preferred_language.py

@@ -0,0 +1,40 @@
+# Generated by Django 3.2.14 on 2022-07-15 19:24
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("bookwyrm", "0153_merge_20220706_2141"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="user",
+            name="preferred_language",
+            field=models.CharField(
+                blank=True,
+                choices=[
+                    ("en-us", "English"),
+                    ("ca-es", "Català (Catalan)"),
+                    ("de-de", "Deutsch (German)"),
+                    ("es-es", "Español (Spanish)"),
+                    ("gl-es", "Galego (Galician)"),
+                    ("it-it", "Italiano (Italian)"),
+                    ("fi-fi", "Suomi (Finnish)"),
+                    ("fr-fr", "Français (French)"),
+                    ("lt-lt", "Lietuvių (Lithuanian)"),
+                    ("no-no", "Norsk (Norwegian)"),
+                    ("pt-br", "Português do Brasil (Brazilian Portuguese)"),
+                    ("pt-pt", "Português Europeu (European Portuguese)"),
+                    ("ro-ro", "Română (Romanian)"),
+                    ("sv-se", "Svenska (Swedish)"),
+                    ("zh-hans", "简体中文 (Simplified Chinese)"),
+                    ("zh-hant", "繁體中文 (Traditional Chinese)"),
+                ],
+                max_length=255,
+                null=True,
+            ),
+        ),
+    ]

bookwyrm/models/notification.py

@@ -71,7 +71,9 @@ class Notification(BookWyrmModel):
         """Create a notification"""
         if related_user and (not user.local or user == related_user):
             return
-        notification, _ = cls.objects.get_or_create(user=user, **kwargs)
+        notification = cls.objects.filter(user=user, **kwargs).first()
+        if not notification:
+            notification = cls.objects.create(user=user, **kwargs)
         if related_user:
             notification.related_users.add(related_user)
         notification.read = False
@@ -298,8 +300,10 @@ def notify_user_on_follow(sender, instance, created, *args, **kwargs):
         notification.read = False
         notification.save()
     else:
+        # Only group unread follows
         Notification.notify(
             instance.user_object,
             instance.user_subject,
             notification_type=Notification.FOLLOW,
+            read=False,
         )

bookwyrm/settings.py

@@ -11,7 +11,7 @@ from django.utils.translation import gettext_lazy as _
 env = Env()
 env.read_env()
 DOMAIN = env("DOMAIN")
-VERSION = "0.4.2"
+VERSION = "0.4.4"
 
 RELEASE_API = env(
     "RELEASE_API",
@@ -280,6 +280,7 @@ AUTH_PASSWORD_VALIDATORS = [
 LANGUAGE_CODE = env("LANGUAGE_CODE", "en-us")
 LANGUAGES = [
     ("en-us", _("English")),
+    ("ca-es", _("Català (Catalan)")),
     ("de-de", _("Deutsch (German)")),
     ("es-es", _("Español (Spanish)")),
     ("gl-es", _("Galego (Galician)")),

bookwyrm/static/css/bookwyrm/components/_book_list.scss

@@ -6,11 +6,11 @@ ol.ordered-list {
     counter-reset: list-counter;
 }
 
-ol.ordered-list li {
+ol.ordered-list > li {
     counter-increment: list-counter;
 }
 
-ol.ordered-list li::before {
+ol.ordered-list > li::before {
     content: counter(list-counter);
     position: absolute;
     left: -20px;

bookwyrm/templates/confirm_email/resend_modal.html

@@ -19,16 +19,8 @@
             name="email"
             class="input"
             id="email"
-            aria-described-by="id_email_errors"
             required
         >
-        {% if error %}
-        <div id="id_email_errors">
-            <p class="help is-danger">
-                {% trans "No user matching this email address found." %}
-            </p>
-        </div>
-        {% endif %}
     </div>
 </div>
 {% endblock %}

bookwyrm/templates/landing/password_reset.html

@@ -26,7 +26,16 @@
                         {% trans "Password:" %}
                     </label>
                     <div class="control">
-                        <input type="password" name="password" maxlength="128" class="input" required="" id="id_new_password" aria-describedby="form_errors">
+                        <input
+                            type="password"
+                            name="password"
+                            maxlength="128"
+                            class="input"
+                            required=""
+                            id="id_new_password"
+                            aria-describedby="desc_password"
+                        >
+                        {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_password" %}
                     </div>
                 </div>
                 <div class="field">
@@ -34,7 +43,8 @@
                         {% trans "Confirm password:" %}
                     </label>
                     <div class="control">
-                        <input type="password" name="confirm-password" maxlength="128" class="input" required="" id="id_confirm_password" aria-describedby="form_errors">
+                        {{ form.confirm_password }}
+                        {% include 'snippets/form_errors.html' with errors_list=form.confirm_password.errors id="desc_confirm_password" %}
                     </div>
                 </div>
                 <div class="field is-grouped">

bookwyrm/templates/notifications/items/boost.html

@@ -118,7 +118,7 @@
 <div class="notification py-2 {% if notification.id in unread %}is-primary is-light{% else %}has-background-body has-text-muted{% endif %}">
     <div class="columns">
         <div class="column is-clipped">
-            {% include 'snippets/status_preview.html' with status=related_status %}
+            {% include 'notifications/items/status_preview.html' with status=related_status %}
         </div>
         <div class="column is-narrow has-text-muted">
             {{ related_status.published_date|timesince }}

bookwyrm/templates/notifications/items/fav.html

@@ -119,7 +119,7 @@
 <div class="notification py-2 {% if notification.id in unread %}is-primary is-light{% else %}has-background-body has-text-muted{% endif %}">
     <div class="columns">
         <div class="column is-clipped">
-            {% include 'snippets/status_preview.html' with status=related_status %}
+            {% include 'notifications/items/status_preview.html' with status=related_status %}
         </div>
         <div class="column is-narrow has-text-muted">
             {{ related_status.published_date|timesince }}

bookwyrm/templates/notifications/items/layout.html

@@ -2,7 +2,7 @@
 {% load humanize %}
 {% related_status notification as related_status %}
 
-{% with related_users=notification.related_users.all.distinct %}
+{% get_related_users notification as related_users %}
 {% with related_user_count=notification.related_users.count %}
 <div class="notification {% if notification.id in unread %}has-background-primary{% endif %}">
     <div class="columns is-mobile {% if notification.id in unread %}has-text-white{% else %}has-text-more-muted{% endif %}">
@@ -16,7 +16,7 @@
             {% if related_user_count > 1 %}
             <div class="block">
                 <ul class="is-flex">
-                    {% for user in related_users|slice:10 %}
+                    {% for user in related_users %}
                     <li class="mr-2">
                         <a href="{{ user.local_path }}">
                             {% include 'snippets/avatar.html' with user=user %}
@@ -28,7 +28,7 @@
             {% endif %}
             <div class="block content">
                 {% if related_user_count == 1 %}
-                    {% with user=related_users.first %}
+                    {% with user=related_users.0 %}
                     {% spaceless %}
                         <a href="{{ user.local_path }}" class="mr-2">
                             {% include 'snippets/avatar.html' with user=user %}
@@ -37,8 +37,8 @@
                     {% endwith %}
                 {% endif %}
 
-                {% with related_user=related_users.first.display_name %}
-                {% with related_user_link=related_users.first.local_path %}
+                {% with related_user=related_users.0.display_name %}
+                {% with related_user_link=related_users.0.local_path %}
                 {% with second_user=related_users.1.display_name %}
                 {% with second_user_link=related_users.1.local_path %}
                 {% with other_user_count=related_user_count|add:"-1" %}
@@ -61,4 +61,3 @@
     </div>
 </div>
 {% endwith %}
-{% endwith %}

bookwyrm/templates/notifications/items/mention.html

@@ -51,7 +51,7 @@
 <div class="notification py-2 {% if notification.id in unread %}is-primary is-light{% else %}has-background-body has-text-default{% endif %}">
     <div class="columns">
         <div class="column is-clipped">
-            {% include 'snippets/status_preview.html' with status=related_status %}
+            {% include 'notifications/items/status_preview.html' with status=related_status %}
         </div>
         <div class="column is-narrow has-text-default">
             {{ related_status.published_date|timesince }}

bookwyrm/templates/notifications/items/reply.html

@@ -54,7 +54,7 @@
 <div class="notification py-2 {% if notification.id in unread %}is-primary is-light{% else %}has-background-body has-text-default{% endif %}">
     <div class="columns">
         <div class="column is-clipped">
-            {% include 'snippets/status_preview.html' with status=related_status %}
+            {% include 'notifications/items/status_preview.html' with status=related_status %}
         </div>
         <div class="column is-narrow has-text-default">
             {{ related_status.published_date|timesince }}

bookwyrm/templates/notifications/items/status_preview.html

@@ -1,4 +1,17 @@
-{% if status.content %}
+{% load i18n %}
+{% if status.content_warning %}
+
+{% trans "Content warning" as text %}
+<span>
+    <span class="icon icon-warning is-size-5" title="{{ text }}">
+        <span class="is-sr-only">{{ text }}</span>
+    </span>
+
+    <a href="{{ status.local_path }}">
+        {{ status.content_warning }}
+    </a>
+</span>
+{% elif status.content %}
 <a href="{{ status.local_path }}">
     {{ status.content | safe | truncatewords_html:10 }}{% if status.mention_books %} <em>{{ status.mention_books.first.title }}</em>{% endif %}
 </a>

bookwyrm/templates/preferences/change_password.html

@@ -8,15 +8,31 @@
 {% endblock %}
 
 {% block panel %}
+{% if success %}
+<div class="notification is-success is-light">
+    <span class="icon icon-check" aria-hidden="true"></span>
+    <span>
+        {% trans "Successfully changed password" %}
+    </span>
+</div>
+{% endif %}
 <form name="edit-profile" action="{% url 'prefs-password' %}" method="post" enctype="multipart/form-data">
     {% csrf_token %}
+    <div class="field">
+        <label class="label" for="id_password">{% trans "Current password:" %}</label>
+        {{ form.current_password }}
+        {% include 'snippets/form_errors.html' with errors_list=form.current_password.errors id="desc_current_password" %}
+    </div>
+    <hr aria-hidden="true" />
     <div class="field">
         <label class="label" for="id_password">{% trans "New password:" %}</label>
-        <input type="password" name="password" maxlength="128" class="input" required="" id="id_password">
+        {{ form.password }}
+        {% include 'snippets/form_errors.html' with errors_list=form.password.errors id="desc_current_password" %}
     </div>
     <div class="field">
         <label class="label" for="id_confirm_password">{% trans "Confirm password:" %}</label>
-        <input type="password" name="confirm-password" maxlength="128" class="input" required="" id="id_confirm_password">
+        {{ form.confirm_password }}
+        {% include 'snippets/form_errors.html' with errors_list=form.confirm_password.errors id="desc_confirm_password" %}
     </div>
     <button class="button is-primary" type="submit">{% trans "Change Password" %}</button>
 </form>

bookwyrm/templates/preferences/export.html

@@ -13,10 +13,13 @@
         {% trans "Your export will include all the books on your shelves, books you have reviewed, and books with reading activity." %}
     </p>
     <p>
-        <a href="{% url 'prefs-export-file' %}" class="button">
-            <span class="icon icon-download" aria-hidden="true"></span>
-            <span>Download file</span>
-        </a>
+        <form name="export" method="POST" href="{% url 'prefs-export' %}">
+            {% csrf_token %}
+            <button type="submit" class="button">
+                <span class="icon icon-download" aria-hidden="true"></span>
+                <span>{% trans "Download file" %}</span>
+            </button>
+        </form>
     </p>
 </div>
 {% endblock %}

bookwyrm/templates/snippets/follow_button.html

@@ -26,7 +26,7 @@
         <form action="{% url 'unfollow' %}" method="POST" class="interaction follow_{{ user.id }} {% if not relationship.is_following and not relationship.is_follow_pending %}is-hidden{%endif %}" data-id="follow_{{ user.id }}">
             {% csrf_token %}
             <input type="hidden" name="user" value="{{ user.username }}">
-            {% if user.manually_approves_followers and not relationship.is_following %}
+            {% if relationship.is_follow_pending %}
             <button class="button is-small is-danger is-light" type="submit">
                 {% trans "Undo follow request" %}
             </button>

bookwyrm/templates/user_menu.html

@@ -68,9 +68,15 @@
             <li class="navbar-divider" role="presentation" aria-hidden="true">&nbsp;</li>
 
             <li role="menuitem">
-                <a href="{% url 'logout' %}" class="navbar-item">
-                    {% trans 'Log out' %}
-                </a>
+                <form
+                    name="logout"
+                    method="POST"
+                    action="{% url 'logout' %}"
+                    class="navbar-item"
+                >
+                    {% csrf_token %}
+                    <button type="submit">{% trans 'Log out' %}</button>
+                </form>
             </li>
         </ul>
     </div>

bookwyrm/templatetags/interaction.py

@@ -42,11 +42,11 @@ def get_relationship(context, user_object):
     """caches the relationship between the logged in user and another user"""
     user = context["request"].user
     return get_or_set(
-        f"cached-relationship-{user.id}-{user_object.id}",
+        f"relationship-{user.id}-{user_object.id}",
         get_relationship_name,
         user,
         user_object,
-        timeout=259200,
+        timeout=60 * 60,
     )
 
 

bookwyrm/templatetags/notification_page_tags.py

@@ -12,3 +12,9 @@ def related_status(notification):
     if not notification.related_status:
         return None
     return load_subclass(notification.related_status)
+
+
+@register.simple_tag(takes_context=False)
+def get_related_users(notification):
+    """Who actually was it who liked your post"""
+    return list(reversed(list(notification.related_users.distinct())))[:10]

bookwyrm/tests/models/test_notification.py

@@ -76,6 +76,17 @@ class Notification(TestCase):
         notification.refresh_from_db()
         self.assertEqual(notification.related_users.count(), 2)
 
+    def test_notify_grouping_with_dupes(self):
+        """If there are multiple options to group with, don't cause an error"""
+        models.Notification.objects.create(
+            user=self.local_user, notification_type="FAVORITE"
+        )
+        models.Notification.objects.create(
+            user=self.local_user, notification_type="FAVORITE"
+        )
+        models.Notification.notify(self.local_user, None, notification_type="FAVORITE")
+        self.assertEqual(models.Notification.objects.count(), 2)
+
     def test_notify_remote(self):
         """Don't create notifications for remote users"""
         models.Notification.notify(

bookwyrm/tests/views/landing/test_password.py

@@ -104,7 +104,9 @@ class PasswordViews(TestCase):
         """reset from code"""
         view = views.PasswordReset.as_view()
         code = models.PasswordReset.objects.create(user=self.local_user)
-        request = self.factory.post("", {"password": "hi", "confirm-password": "hi"})
+        request = self.factory.post(
+            "", {"password": "longwordsecure", "confirm_password": "longwordsecure"}
+        )
         with patch("bookwyrm.views.landing.password.login"):
             resp = view(request, code.code)
         self.assertEqual(resp.status_code, 302)
@@ -114,7 +116,9 @@ class PasswordViews(TestCase):
         """reset from code"""
         view = views.PasswordReset.as_view()
         models.PasswordReset.objects.create(user=self.local_user)
-        request = self.factory.post("", {"password": "hi", "confirm-password": "hi"})
+        request = self.factory.post(
+            "", {"password": "longwordsecure", "confirm_password": "longwordsecure"}
+        )
         resp = view(request, "jhgdkfjgdf")
         validate_html(resp.render())
         self.assertTrue(models.PasswordReset.objects.exists())
@@ -123,7 +127,18 @@ class PasswordViews(TestCase):
         """reset from code"""
         view = views.PasswordReset.as_view()
         code = models.PasswordReset.objects.create(user=self.local_user)
-        request = self.factory.post("", {"password": "hi", "confirm-password": "hihi"})
+        request = self.factory.post(
+            "", {"password": "longwordsecure", "confirm_password": "hihi"}
+        )
+        resp = view(request, code.code)
+        validate_html(resp.render())
+        self.assertTrue(models.PasswordReset.objects.exists())
+
+    def test_password_reset_invalid(self):
+        """reset from code"""
+        view = views.PasswordReset.as_view()
+        code = models.PasswordReset.objects.create(user=self.local_user)
+        request = self.factory.post("", {"password": "a", "confirm_password": "a"})
         resp = view(request, code.code)
         validate_html(resp.render())
         self.assertTrue(models.PasswordReset.objects.exists())

bookwyrm/tests/views/landing/test_register.py

@@ -122,6 +122,17 @@ class RegisterViews(TestCase):
         self.assertEqual(models.User.objects.count(), 1)
         validate_html(response.render())
 
+    def test_register_invalid_password(self, *_):
+        """gotta have an email"""
+        view = views.Register.as_view()
+        self.assertEqual(models.User.objects.count(), 1)
+        request = self.factory.post(
+            "register/", {"localname": "nutria", "password": "password", "email": "aa"}
+        )
+        response = view(request)
+        self.assertEqual(models.User.objects.count(), 1)
+        validate_html(response.render())
+
     def test_register_error_and_invite(self, *_):
         """redirect to the invite page"""
         view = views.Register.as_view()

bookwyrm/tests/views/preferences/test_change_password.py

@@ -42,17 +42,71 @@ class ChangePasswordViews(TestCase):
         """change password"""
         view = views.ChangePassword.as_view()
         password_hash = self.local_user.password
-        request = self.factory.post("", {"password": "hi", "confirm-password": "hi"})
+        request = self.factory.post(
+            "",
+            {
+                "current_password": "password",
+                "password": "longwordsecure",
+                "confirm_password": "longwordsecure",
+            },
+        )
         request.user = self.local_user
         with patch("bookwyrm.views.preferences.change_password.login"):
-            view(request)
+            result = view(request)
+        validate_html(result.render())
+        self.local_user.refresh_from_db()
         self.assertNotEqual(self.local_user.password, password_hash)
 
+    def test_password_change_wrong_current(self):
+        """change password"""
+        view = views.ChangePassword.as_view()
+        password_hash = self.local_user.password
+        request = self.factory.post(
+            "",
+            {
+                "current_password": "not my password",
+                "password": "longwordsecure",
+                "confirm_password": "hihi",
+            },
+        )
+        request.user = self.local_user
+        result = view(request)
+        validate_html(result.render())
+        self.local_user.refresh_from_db()
+        self.assertEqual(self.local_user.password, password_hash)
+
     def test_password_change_mismatch(self):
         """change password"""
         view = views.ChangePassword.as_view()
         password_hash = self.local_user.password
-        request = self.factory.post("", {"password": "hi", "confirm-password": "hihi"})
+        request = self.factory.post(
+            "",
+            {
+                "current_password": "password",
+                "password": "longwordsecure",
+                "confirm_password": "hihi",
+            },
+        )
         request.user = self.local_user
-        view(request)
+        result = view(request)
+        validate_html(result.render())
+        self.local_user.refresh_from_db()
+        self.assertEqual(self.local_user.password, password_hash)
+
+    def test_password_change_invalid(self):
+        """change password"""
+        view = views.ChangePassword.as_view()
+        password_hash = self.local_user.password
+        request = self.factory.post(
+            "",
+            {
+                "current_password": "password",
+                "password": "hi",
+                "confirm_password": "hi",
+            },
+        )
+        request.user = self.local_user
+        result = view(request)
+        validate_html(result.render())
+        self.local_user.refresh_from_db()
         self.assertEqual(self.local_user.password, password_hash)

bookwyrm/tests/views/preferences/test_export.py

@@ -54,9 +54,9 @@ class ExportViews(TestCase):
             user=self.local_user,
             book=self.book,
         )
-        request = self.factory.get("")
+        request = self.factory.post("")
         request.user = self.local_user
-        export = views.export_user_book_data(request)
+        export = views.Export.as_view()(request)
         self.assertIsInstance(export, StreamingHttpResponse)
         self.assertEqual(export.status_code, 200)
         result = list(export.streaming_content)

bookwyrm/tests/views/shelf/test_shelf_actions.py

@@ -32,6 +32,14 @@ class ShelfActionViews(TestCase):
                 localname="mouse",
                 remote_id="https://example.com/users/mouse",
             )
+            self.another_user = models.User.objects.create_user(
+                "rat@local.com",
+                "rat@rat.com",
+                "ratword",
+                local=True,
+                localname="rat",
+                remote_id="https://example.com/users/rat",
+            )
         self.work = models.Work.objects.create(title="Test Work")
         self.book = models.Edition.objects.create(
             title="Example Edition",
@@ -66,7 +74,7 @@ class ShelfActionViews(TestCase):
 
     def test_shelve_to_read(self, *_):
         """special behavior for the to-read shelf"""
-        shelf = models.Shelf.objects.get(identifier="to-read")
+        shelf = models.Shelf.objects.get(user=self.local_user, identifier="to-read")
         request = self.factory.post(
             "", {"book": self.book.id, "shelf": shelf.identifier}
         )
@@ -79,7 +87,7 @@ class ShelfActionViews(TestCase):
 
     def test_shelve_reading(self, *_):
         """special behavior for the reading shelf"""
-        shelf = models.Shelf.objects.get(identifier="reading")
+        shelf = models.Shelf.objects.get(user=self.local_user, identifier="reading")
         request = self.factory.post(
             "", {"book": self.book.id, "shelf": shelf.identifier}
         )
@@ -92,7 +100,7 @@ class ShelfActionViews(TestCase):
 
     def test_shelve_read(self, *_):
         """special behavior for the read shelf"""
-        shelf = models.Shelf.objects.get(identifier="read")
+        shelf = models.Shelf.objects.get(user=self.local_user, identifier="read")
         request = self.factory.post(
             "", {"book": self.book.id, "shelf": shelf.identifier}
         )
@@ -105,11 +113,13 @@ class ShelfActionViews(TestCase):
 
     def test_shelve_read_with_change_shelf(self, *_):
         """special behavior for the read shelf"""
-        previous_shelf = models.Shelf.objects.get(identifier="reading")
+        previous_shelf = models.Shelf.objects.get(
+            user=self.local_user, identifier="reading"
+        )
         models.ShelfBook.objects.create(
             shelf=previous_shelf, user=self.local_user, book=self.book
         )
-        shelf = models.Shelf.objects.get(identifier="read")
+        shelf = models.Shelf.objects.get(user=self.local_user, identifier="read")
 
         request = self.factory.post(
             "",
@@ -160,11 +170,24 @@ class ShelfActionViews(TestCase):
 
         views.create_shelf(request)
 
-        shelf = models.Shelf.objects.get(name="new shelf name")
+        shelf = models.Shelf.objects.get(user=self.local_user, name="new shelf name")
         self.assertEqual(shelf.privacy, "unlisted")
         self.assertEqual(shelf.description, "desc")
         self.assertEqual(shelf.user, self.local_user)
 
+    def test_create_shelf_wrong_user(self, *_):
+        """a brand new custom shelf"""
+        form = forms.ShelfForm()
+        form.data["user"] = self.another_user.id
+        form.data["name"] = "new shelf name"
+        form.data["description"] = "desc"
+        form.data["privacy"] = "unlisted"
+        request = self.factory.post("", form.data)
+        request.user = self.local_user
+
+        with self.assertRaises(PermissionDenied):
+            views.create_shelf(request)
+
     def test_delete_shelf(self, *_):
         """delete a brand new custom shelf"""
         request = self.factory.post("")
@@ -177,18 +200,8 @@ class ShelfActionViews(TestCase):
 
     def test_delete_shelf_unauthorized(self, *_):
         """delete a brand new custom shelf"""
-        with patch("bookwyrm.suggested_users.rerank_suggestions_task.delay"), patch(
-            "bookwyrm.activitystreams.populate_stream_task.delay"
-        ), patch("bookwyrm.lists_stream.populate_lists_task.delay"):
-            rat = models.User.objects.create_user(
-                "rat@local.com",
-                "rat@mouse.mouse",
-                "password",
-                local=True,
-                localname="rat",
-            )
         request = self.factory.post("")
-        request.user = rat
+        request.user = self.another_user
 
         with self.assertRaises(PermissionDenied):
             views.delete_shelf(request, self.shelf.id)

bookwyrm/tests/views/test_status.py

@@ -10,12 +10,13 @@ from bookwyrm.settings import DOMAIN
 from bookwyrm.tests.validate_html import validate_html
 
 
-# pylint: disable=invalid-name
 @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.activitystreams.remove_status_task.delay")
 @patch("bookwyrm.models.activitypub_mixin.broadcast_task.apply_async")
+# pylint: disable=invalid-name
+# pylint: disable=too-many-public-methods
 class StatusViews(TestCase):
     """viewing and creating statuses"""
 
@@ -75,6 +76,22 @@ class StatusViews(TestCase):
         self.assertEqual(status.book, self.book)
         self.assertIsNone(status.edited_date)
 
+    def test_create_status_wrong_user(self, *_):
+        """You can't compose statuses for someone else"""
+        view = views.CreateStatus.as_view()
+        form = forms.CommentForm(
+            {
+                "content": "hi",
+                "user": self.remote_user.id,
+                "book": self.book.id,
+                "privacy": "public",
+            }
+        )
+        request = self.factory.post("", form.data)
+        request.user = self.local_user
+        with self.assertRaises(PermissionDenied):
+            view(request, "comment")
+
     def test_create_status_reply(self, *_):
         """create a status in reply to an existing status"""
         view = views.CreateStatus.as_view()

bookwyrm/urls.py

@@ -482,11 +482,6 @@ urlpatterns = [
         name="prefs-password",
     ),
     re_path(r"^preferences/export/?$", views.Export.as_view(), name="prefs-export"),
-    re_path(
-        r"^preferences/export/file/?$",
-        views.export_user_book_data,
-        name="prefs-export-file",
-    ),
     re_path(r"^preferences/delete/?$", views.DeleteUser.as_view(), name="prefs-delete"),
     re_path(r"^preferences/block/?$", views.Block.as_view(), name="prefs-block"),
     re_path(r"^block/(?P<user_id>\d+)/?$", views.Block.as_view()),

bookwyrm/views/__init__.py

@@ -28,7 +28,7 @@ from .admin.user_admin import UserAdmin, UserAdminList
 # user preferences
 from .preferences.change_password import ChangePassword
 from .preferences.edit_user import EditUser
-from .preferences.export import Export, export_user_book_data
+from .preferences.export import Export
 from .preferences.delete_user import DeleteUser
 from .preferences.block import Block, unblock
 

bookwyrm/views/follow.py

@@ -1,7 +1,9 @@
 """ views for actions you can take in the application """
 import urllib.parse
 import re
+
 from django.contrib.auth.decorators import login_required
+from django.http import HttpResponse
 from django.shortcuts import get_object_or_404, redirect
 from django.template.response import TemplateResponse
 from django.views.decorators.http import require_POST
@@ -13,6 +15,7 @@ from .helpers import (
     handle_remote_webfinger,
     subscribe_remote_webfinger,
     WebFingerError,
+    is_api_request,
 )
 
 
@@ -34,6 +37,8 @@ def follow(request):
         # that means we should save to trigger a re-broadcast
         follow_request.save()
 
+    if is_api_request(request):
+        return HttpResponse()
     return redirect(to_follow.local_path)
 
 
@@ -58,8 +63,10 @@ def unfollow(request):
     except models.UserFollowRequest.DoesNotExist:
         clear_cache(request.user, to_unfollow)
 
+    if is_api_request(request):
+        return HttpResponse()
     # this is handled with ajax so it shouldn't really matter
-    return redirect(request.headers.get("Referer", "/"))
+    return redirect("/")
 
 
 @login_required

bookwyrm/views/goal.py

@@ -70,7 +70,7 @@ class Goal(View):
                 privacy=goal.privacy,
             )
 
-        return redirect(request.headers.get("Referer", "/"))
+        return redirect("user-goal", request.user.localname, year)
 
 
 @require_POST
@@ -79,4 +79,4 @@ def hide_goal(request):
     """don't keep bugging people to set a goal"""
     request.user.show_goal = False
     request.user.save(broadcast=False, update_fields=["show_goal"])
-    return redirect(request.headers.get("Referer", "/"))
+    return redirect("/")

bookwyrm/views/interaction.py

@@ -28,7 +28,7 @@ class Favorite(View):
 
         if is_api_request(request):
             return HttpResponse()
-        return redirect(request.headers.get("Referer", "/"))
+        return redirect("/")
 
 
 @method_decorator(login_required, name="dispatch")
@@ -48,7 +48,7 @@ class Unfavorite(View):
         favorite.delete()
         if is_api_request(request):
             return HttpResponse()
-        return redirect(request.headers.get("Referer", "/"))
+        return redirect("/")
 
 
 @method_decorator(login_required, name="dispatch")
@@ -67,7 +67,7 @@ class Boost(View):
             boosted_status=status, user=request.user
         ).exists():
             # you already boosted that.
-            return redirect(request.headers.get("Referer", "/"))
+            return redirect("/")
 
         models.Boost.objects.create(
             boosted_status=status,
@@ -76,7 +76,7 @@ class Boost(View):
         )
         if is_api_request(request):
             return HttpResponse()
-        return redirect(request.headers.get("Referer", "/"))
+        return redirect("/")
 
 
 @method_decorator(login_required, name="dispatch")
@@ -94,4 +94,4 @@ class Unboost(View):
         boost.delete()
         if is_api_request(request):
             return HttpResponse()
-        return redirect(request.headers.get("Referer", "/"))
+        return redirect("/")

bookwyrm/views/landing/login.py

@@ -58,7 +58,7 @@ class Login(View):
             user.update_active_date()
             if request.POST.get("first_login"):
                 return set_language(user, redirect("get-started-profile"))
-            return set_language(user, redirect(request.GET.get("next", "/")))
+            return set_language(user, redirect("/"))
 
         # maybe the user is pending email confirmation
         if models.User.objects.filter(
@@ -77,7 +77,7 @@ class Login(View):
 class Logout(View):
     """log out"""
 
-    def get(self, request):
+    def post(self, request):
         """done with this place! outa here!"""
         logout(request)
         return redirect("/")

bookwyrm/views/landing/password.py

@@ -5,7 +5,7 @@ from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.views import View
 
-from bookwyrm import models
+from bookwyrm import forms, models
 from bookwyrm.emailing import password_reset_email
 
 
@@ -57,7 +57,8 @@ class PasswordReset(View):
         except models.PasswordReset.DoesNotExist:
             raise PermissionDenied()
 
-        return TemplateResponse(request, "landing/password_reset.html", {"code": code})
+        data = {"code": code, "form": forms.PasswordResetForm()}
+        return TemplateResponse(request, "landing/password_reset.html", data)
 
     def post(self, request, code):
         """allow a user to change their password through an emailed token"""
@@ -68,14 +69,12 @@ class PasswordReset(View):
             return TemplateResponse(request, "landing/password_reset.html", data)
 
         user = reset_code.user
-
-        new_password = request.POST.get("password")
-        confirm_password = request.POST.get("confirm-password")
-
-        if new_password != confirm_password:
-            data = {"errors": ["Passwords do not match"]}
+        form = forms.PasswordResetForm(request.POST, instance=user)
+        if not form.is_valid():
+            data = {"code": code, "form": form}
             return TemplateResponse(request, "landing/password_reset.html", data)
 
+        new_password = form.cleaned_data["password"]
         user.set_password(new_password)
         user.save(broadcast=False, update_fields=["password"])
         login(request, user)

bookwyrm/views/landing/register.py

@@ -134,19 +134,19 @@ class ConfirmEmail(View):
 class ResendConfirmEmail(View):
     """you probably didn't get the email because celery is slow but you can try this"""
 
-    def get(self, request, error=False):
+    def get(self, request):
         """resend link landing page"""
-        return TemplateResponse(request, "confirm_email/resend.html", {"error": error})
+        return TemplateResponse(request, "confirm_email/resend.html")
 
     def post(self, request):
         """resend confirmation link"""
         email = request.POST.get("email")
         try:
             user = models.User.objects.get(email=email)
+            emailing.email_confirmation_email(user)
         except models.User.DoesNotExist:
-            return self.get(request, error=True)
+            pass
 
-        emailing.email_confirmation_email(user)
         return TemplateResponse(
             request, "confirm_email/confirm_email.html", {"valid": True}
         )

bookwyrm/views/preferences/change_password.py

@@ -1,10 +1,12 @@
 """ class views for password management """
 from django.contrib.auth import login
 from django.contrib.auth.decorators import login_required
-from django.shortcuts import redirect
 from django.template.response import TemplateResponse
 from django.utils.decorators import method_decorator
 from django.views import View
+from django.views.decorators.debug import sensitive_variables, sensitive_post_parameters
+
+from bookwyrm import forms
 
 
 # pylint: disable= no-self-use
@@ -14,18 +16,24 @@ class ChangePassword(View):
 
     def get(self, request):
         """change password page"""
-        data = {"user": request.user}
+        data = {"form": forms.ChangePasswordForm()}
         return TemplateResponse(request, "preferences/change_password.html", data)
 
+    @method_decorator(sensitive_variables("new_password"))
+    @method_decorator(sensitive_post_parameters("current_password"))
+    @method_decorator(sensitive_post_parameters("password"))
+    @method_decorator(sensitive_post_parameters("confirm_password"))
     def post(self, request):
         """allow a user to change their password"""
-        new_password = request.POST.get("password")
-        confirm_password = request.POST.get("confirm-password")
-
-        if new_password != confirm_password:
-            return redirect("prefs-password")
+        form = forms.ChangePasswordForm(request.POST, instance=request.user)
+        if not form.is_valid():
+            data = {"form": form}
+            return TemplateResponse(request, "preferences/change_password.html", data)
 
+        new_password = form.cleaned_data["password"]
         request.user.set_password(new_password)
         request.user.save(broadcast=False, update_fields=["password"])
+
         login(request, request.user)
-        return redirect("user-feed", request.user.localname)
+        data = {"success": True, "form": forms.ChangePasswordForm()}
+        return TemplateResponse(request, "preferences/change_password.html", data)

bookwyrm/views/preferences/export.py

@@ -7,7 +7,6 @@ from django.http import StreamingHttpResponse
 from django.template.response import TemplateResponse
 from django.views import View
 from django.utils.decorators import method_decorator
-from django.views.decorators.http import require_GET
 
 from bookwyrm import models
 
@@ -20,35 +19,34 @@ class Export(View):
         """Request csv file"""
         return TemplateResponse(request, "preferences/export.html")
 
-
-@login_required
-@require_GET
-def export_user_book_data(request):
-    """Streaming the csv file of a user's book data"""
-    data = (
-        models.Edition.viewer_aware_objects(request.user)
-        .filter(
-            Q(shelves__user=request.user)
-            | Q(readthrough__user=request.user)
-            | Q(review__user=request.user)
-            | Q(comment__user=request.user)
-            | Q(quotation__user=request.user)
+    def post(self, request):
+        """Streaming the csv file of a user's book data"""
+        data = (
+            models.Edition.viewer_aware_objects(request.user)
+            .filter(
+                Q(shelves__user=request.user)
+                | Q(readthrough__user=request.user)
+                | Q(review__user=request.user)
+                | Q(comment__user=request.user)
+                | Q(quotation__user=request.user)
+            )
+            .distinct()
         )
-        .distinct()
-    )
 
-    generator = csv_row_generator(data, request.user)
+        generator = csv_row_generator(data, request.user)
 
-    pseudo_buffer = Echo()
-    writer = csv.writer(pseudo_buffer)
-    # for testing, if you want to see the results in the browser:
-    # from django.http import JsonResponse
-    # return JsonResponse(list(generator), safe=False)
-    return StreamingHttpResponse(
-        (writer.writerow(row) for row in generator),
-        content_type="text/csv",
-        headers={"Content-Disposition": 'attachment; filename="bookwyrm-export.csv"'},
-    )
+        pseudo_buffer = Echo()
+        writer = csv.writer(pseudo_buffer)
+        # for testing, if you want to see the results in the browser:
+        # from django.http import JsonResponse
+        # return JsonResponse(list(generator), safe=False)
+        return StreamingHttpResponse(
+            (writer.writerow(row) for row in generator),
+            content_type="text/csv",
+            headers={
+                "Content-Disposition": 'attachment; filename="bookwyrm-export.csv"'
+            },
+        )
 
 
 def csv_row_generator(books, user):

bookwyrm/views/reading.py

@@ -79,13 +79,11 @@ class ReadingStatus(View):
         current_status_shelfbook = shelves[0] if shelves else None
 
         # checking the referer prevents redirecting back to the modal page
-        referer = request.headers.get("Referer", "/")
-        referer = "/" if "reading-status" in referer else referer
         if current_status_shelfbook is not None:
             if current_status_shelfbook.shelf.identifier != desired_shelf.identifier:
                 current_status_shelfbook.delete()
             else:  # It already was on the shelf
-                return redirect(referer)
+                return redirect("/")
 
         models.ShelfBook.objects.create(
             book=book, shelf=desired_shelf, user=request.user
@@ -123,7 +121,7 @@ class ReadingStatus(View):
         if is_api_request(request):
             return HttpResponse()
 
-        return redirect(referer)
+        return redirect("/")
 
 
 @method_decorator(login_required, name="dispatch")
@@ -205,7 +203,7 @@ def delete_readthrough(request):
     readthrough.raise_not_deletable(request.user)
 
     readthrough.delete()
-    return redirect(request.headers.get("Referer", "/"))
+    return redirect("/")
 
 
 @login_required
@@ -216,4 +214,4 @@ def delete_progressupdate(request):
     update.raise_not_deletable(request.user)
 
     update.delete()
-    return redirect(request.headers.get("Referer", "/"))
+    return redirect("/")

bookwyrm/views/shelf/shelf_actions.py

@@ -13,9 +13,11 @@ def create_shelf(request):
     """user generated shelves"""
     form = forms.ShelfForm(request.POST)
     if not form.is_valid():
-        return redirect(request.headers.get("Referer", "/"))
+        return redirect("user-shelves", request.user.localname)
 
-    shelf = form.save()
+    shelf = form.save(commit=False)
+    shelf.raise_not_editable(request.user)
+    shelf.save()
     return redirect(shelf.local_path)
 
 
@@ -70,7 +72,7 @@ def shelve(request):
             ):
                 current_read_status_shelfbook.delete()
             else:  # It is already on the shelf
-                return redirect(request.headers.get("Referer", "/"))
+                return redirect("/")
 
         # create the new shelf-book entry
         models.ShelfBook.objects.create(
@@ -86,7 +88,7 @@ def shelve(request):
         # Might be good to alert, or reject the action?
         except IntegrityError:
             pass
-    return redirect(request.headers.get("Referer", "/"))
+    return redirect("/")
 
 
 @login_required
@@ -100,4 +102,4 @@ def unshelve(request, book_id=False):
     )
     shelf_book.raise_not_deletable(request.user)
     shelf_book.delete()
-    return redirect(request.headers.get("Referer", "/"))
+    return redirect("/")

bookwyrm/views/status.py

@@ -82,9 +82,10 @@ class CreateStatus(View):
             if is_api_request(request):
                 logger.exception(form.errors)
                 return HttpResponseBadRequest()
-            return redirect(request.headers.get("Referer", "/"))
+            return redirect("/")
 
         status = form.save(commit=False)
+        status.raise_not_editable(request.user)
         # save the plain, unformatted version of the status for future editing
         status.raw_content = status.content
         if hasattr(status, "quote"):
@@ -146,7 +147,7 @@ class DeleteStatus(View):
 
         # perform deletion
         status.delete()
-        return redirect(request.headers.get("Referer", "/"))
+        return redirect("/")
 
 
 @login_required
@@ -195,7 +196,7 @@ def edit_readthrough(request):
 
     if is_api_request(request):
         return HttpResponse()
-    return redirect(request.headers.get("Referer", "/"))
+    return redirect("/")
 
 
 def find_mentions(content):

bookwyrm/views/user.py

@@ -164,7 +164,7 @@ def hide_suggestions(request):
     """not everyone wants user suggestions"""
     request.user.show_suggested_users = False
     request.user.save(broadcast=False, update_fields=["show_suggested_users"])
-    return redirect(request.headers.get("Referer", "/"))
+    return redirect("/")
 
 
 # pylint: disable=unused-argument