Merge pull request #2544 from joachimesque/redirect-after-shelving-book

Redirect back to the original page after shelving book

bookwyrm/templates/snippets/reading_modals/start_reading_modal.html

@@ -10,9 +10,9 @@ Start "<em>{{ book_title }}</em>"
 
 {% block modal-form-open %}
 <form name="start-reading-{{ uuid }}" action="{% url 'reading-status' 'start' book.id %}" method="post" {% if not refresh %}class="submit-status"{% endif %}>
+{% csrf_token %}
 <input type="hidden" name="reading_status" value="reading">
 <input type="hidden" name="shelf" value="{{ move_from }}">
-{% csrf_token %}
 {% endblock %}
 
 {% block reading-dates %}

bookwyrm/templates/snippets/reading_modals/want_to_read_modal.html

@@ -10,9 +10,9 @@ Want to Read "<em>{{ book_title }}</em>"
 
 {% block modal-form-open %}
 <form name="want-to-read-{{ uuid }}" action="{% url 'reading-status' 'want' book.id %}" method="post" {% if not refresh %}class="submit-status"{% endif %}>
+{% csrf_token %}
 <input type="hidden" name="reading_status" value="to-read">
 <input type="hidden" name="shelf" value="{{ move_from }}"> 
-{% csrf_token %}
 {% endblock %}
 
 {% block form %}

bookwyrm/templates/snippets/shelf_selector.html

@@ -22,7 +22,6 @@
         <input type="hidden" name="book" value="{{ book.id }}">
         <input type="hidden" name="change-shelf-from" value="{{ current.identifier }}">
         <input type="hidden" name="shelf" value="{{ shelf.identifier }}">
-
         <button class="button is-fullwidth is-small shelf-option is-radiusless has-background-body" type="submit" {% if shelf.identifier == current.identifier %}disabled{% endif %}>
             <span>
                 {% include "snippets/translated_shelf_name.html" with shelf=shelf %}

bookwyrm/tests/test_utils.py

@@ -3,6 +3,7 @@ import re
 from django.test import TestCase
 
 from bookwyrm.utils import regex
+from bookwyrm.utils.validate import validate_url_domain
 
 
 class TestUtils(TestCase):
@@ -11,3 +12,20 @@ class TestUtils(TestCase):
     def test_regex(self):
         """Regexes used throughout the app"""
         self.assertTrue(re.match(regex.DOMAIN, "xn--69aa8bzb.xn--y9a3aq"))
+
+    def test_valid_url_domain(self):
+        """Check with a valid URL"""
+        self.assertEqual(
+            validate_url_domain("https://your.domain.here/legit-book-url/"),
+            "https://your.domain.here/legit-book-url/",
+        )
+
+    def test_invalid_url_domain(self):
+        """Check with an invalid URL"""
+        self.assertEqual(
+            validate_url_domain("https://up-to-no-good.tld/bad-actor.exe"), "/"
+        )
+
+    def test_default_url_domain(self):
+        """Check with a default URL"""
+        self.assertEqual(validate_url_domain("/"), "/")

bookwyrm/utils/validate.py

@@ -0,0 +1,19 @@
+"""Validations"""
+from bookwyrm.settings import DOMAIN, USE_HTTPS
+
+
+def validate_url_domain(url, default="/"):
+    """Basic check that the URL starts with the instance domain name"""
+    if not url:
+        return default
+
+    if url in ("/", default):
+        return url
+
+    protocol = "https://" if USE_HTTPS else "http://"
+    origin = f"{protocol}{DOMAIN}"
+
+    if url.startswith(origin):
+        return url
+
+    return default

bookwyrm/views/reading.py

@@ -12,6 +12,7 @@ from django.views.decorators.http import require_POST
 
 from bookwyrm import forms, models
 from bookwyrm.views.shelf.shelf_actions import unshelve
+from bookwyrm.utils.validate import validate_url_domain
 from .status import CreateStatus
 from .helpers import get_edition, handle_reading_status, is_api_request
 from .helpers import load_date_in_user_tz_as_utc
@@ -42,6 +43,8 @@ class ReadingStatus(View):
     @transaction.atomic
     def post(self, request, status, book_id):
         """Change the state of a book by shelving it and adding reading dates"""
+        next_step = request.META.get("HTTP_REFERER")
+        next_step = validate_url_domain(next_step, "/")
         identifier = {
             "want": models.Shelf.TO_READ,
             "start": models.Shelf.READING,
@@ -83,7 +86,7 @@ class ReadingStatus(View):
             if current_status_shelfbook.shelf.identifier != desired_shelf.identifier:
                 current_status_shelfbook.delete()
             else:  # It already was on the shelf
-                return redirect("/")
+                return redirect(next_step)
 
         models.ShelfBook.objects.create(
             book=book, shelf=desired_shelf, user=request.user
@@ -121,7 +124,7 @@ class ReadingStatus(View):
         if is_api_request(request):
             return HttpResponse()
 
-        return redirect("/")
+        return redirect(next_step)
 
 
 @method_decorator(login_required, name="dispatch")

bookwyrm/views/shelf/shelf_actions.py

@@ -3,6 +3,7 @@ from django.db import IntegrityError, transaction
 from django.contrib.auth.decorators import login_required
 from django.shortcuts import get_object_or_404, redirect
 from django.views.decorators.http import require_POST
+from bookwyrm.utils.validate import validate_url_domain
 
 from bookwyrm import forms, models
 
@@ -35,6 +36,8 @@ def delete_shelf(request, shelf_id):
 @transaction.atomic
 def shelve(request):
     """put a book on a user's shelf"""
+    next_step = request.META.get("HTTP_REFERER")
+    next_step = validate_url_domain(next_step, "/")
     book = get_object_or_404(models.Edition, id=request.POST.get("book"))
     desired_shelf = get_object_or_404(
         request.user.shelf_set, identifier=request.POST.get("shelf")
@@ -64,13 +67,14 @@ def shelve(request):
             .first()
         )
         if current_read_status_shelfbook is not None:
+            # If it is not already on the shelf
             if (
                 current_read_status_shelfbook.shelf.identifier
                 != desired_shelf.identifier
             ):
                 current_read_status_shelfbook.delete()
-            else:  # It is already on the shelf
-                return redirect("/")
+            else:
+                return redirect(next_step)
 
         # create the new shelf-book entry
         models.ShelfBook.objects.create(
@@ -86,13 +90,16 @@ def shelve(request):
         # Might be good to alert, or reject the action?
         except IntegrityError:
             pass
-    return redirect("/")
+
+    return redirect(next_step)
 
 
 @login_required
 @require_POST
 def unshelve(request, book_id=False):
     """remove a book from a user's shelf"""
+    next_step = request.META.get("HTTP_REFERER")
+    next_step = validate_url_domain(next_step, "/")
     identity = book_id if book_id else request.POST.get("book")
     book = get_object_or_404(models.Edition, id=identity)
     shelf_book = get_object_or_404(
@@ -100,4 +107,4 @@ def unshelve(request, book_id=False):
     )
     shelf_book.raise_not_deletable(request.user)
     shelf_book.delete()
-    return redirect("/")
+    return redirect(next_step)

bookwyrm/views/status.py

@@ -18,6 +18,7 @@ from django.views.decorators.http import require_POST
 from markdown import markdown
 from bookwyrm import forms, models
 from bookwyrm.utils import regex, sanitizer
+from bookwyrm.utils.validate import validate_url_domain
 from .helpers import handle_remote_webfinger, is_api_request
 from .helpers import load_date_in_user_tz_as_utc
 
@@ -58,6 +59,8 @@ class CreateStatus(View):
     # pylint: disable=too-many-branches
     def post(self, request, status_type, existing_status_id=None):
         """create status of whatever type"""
+        next_step = request.META.get("HTTP_REFERER")
+        next_step = validate_url_domain(next_step, "/")
         created = not existing_status_id
         existing_status = None
         if existing_status_id:
@@ -80,7 +83,7 @@ class CreateStatus(View):
             if is_api_request(request):
                 logger.exception(form.errors)
                 return HttpResponseBadRequest()
-            return redirect("/")
+            return redirect(next_step)
 
         status = form.save(request, commit=False)
         status.ready = False
@@ -134,7 +137,7 @@ class CreateStatus(View):
 
         if is_api_request(request):
             return HttpResponse()
-        return redirect("/")
+        return redirect(next_step)
 
 
 @method_decorator(login_required, name="dispatch")
@@ -167,6 +170,8 @@ def update_progress(request, book_id):  # pylint: disable=unused-argument
 def edit_readthrough(request):
     """can't use the form because the dates are too finnicky"""
     # TODO: remove this, it duplicates the code in the ReadThrough view
+    next_step = request.META.get("HTTP_REFERER")
+    next_step = validate_url_domain(next_step, "/")
     readthrough = get_object_or_404(models.ReadThrough, id=request.POST.get("id"))
 
     readthrough.start_date = load_date_in_user_tz_as_utc(
@@ -198,7 +203,7 @@ def edit_readthrough(request):
 
     if is_api_request(request):
         return HttpResponse()
-    return redirect("/")
+    return redirect(next_step)
 
 
 def find_mentions(user, content):