refactor return to

mittab/apps/tab/helpers.py

@@ -11,14 +11,19 @@ def get_redirect_target(request, path=None, fallback="/"):
         1) Explicit path argument
         2) Hidden form field (POST) named 'return_to'
         3) Explicit GET parameters (?return_to= or ?next=)
-        4) HTTP referer header
-        5) Provided fallback (defaults to '/')
+        4) Session-stored return target
+        5) HTTP referer header
+        6) Provided fallback (defaults to '/')
     """
+    session = getattr(request, "session", None)
+    session_return_to = session.get("_return_to") if session is not None else None
+
     candidates = [
         path,
         request.POST.get("return_to"),
         request.GET.get("return_to"),
         request.GET.get("next"),
+        session_return_to,
         request.META.get("HTTP_REFERER"),
     ]
 

mittab/apps/tab/middleware.py

@@ -1,7 +1,9 @@
 import re
+from urllib.parse import urlsplit
 
 from django.contrib.auth.views import LoginView
 from django.http import HttpResponse, JsonResponse
+from django.utils.http import url_has_allowed_host_and_scheme
 
 from mittab.apps.tab.helpers import redirect_and_flash_info
 from mittab.apps.tab.public_rankings import get_standings_publication_setting
@@ -48,6 +50,27 @@ def __init__(self, get_response):
 
     def __call__(self, request):
         path = request.path
+        should_store_return_to = (
+            request.method == "GET"
+            and not request.path.startswith("/api/")
+            and request.headers.get("X-Requested-With") != "XMLHttpRequest"
+        )
+        if should_store_return_to:
+            session_target = request.get_full_path()
+            referer = request.META.get("HTTP_REFERER")
+            if referer and url_has_allowed_host_and_scheme(
+                referer,
+                allowed_hosts={request.get_host()},
+                require_https=request.is_secure(),
+            ):
+                parts = urlsplit(referer)
+                referer_path = parts.path or "/"
+                if parts.query:
+                    referer_path = f"{referer_path}?{parts.query}"
+                if referer_path != request.get_full_path():
+                    session_target = referer_path
+            request.session["_return_to"] = session_target
+
         whitelisted = (
             path in LOGIN_WHITELIST
             or path.startswith("/public/")

mittab/apps/tab/templatetags/tags.py

@@ -1,5 +1,3 @@
-from urllib.parse import urlencode
-
 from django import template
 from django.forms.fields import FileField
 
@@ -68,7 +66,7 @@ def return_to_value(context):
     request = context.get("request")
     if not request:
         return ""
-    return get_redirect_target(request, fallback=None) or ""
+    return get_redirect_target(request, fallback=None) or request.get_full_path()
 
 
 @register.inclusion_tag("common/_return_to_input.html", takes_context=True)
@@ -79,10 +77,7 @@ def return_to_input(context, target=None):
 
 @register.filter
 def with_return_to(url):
-    if not url:
-        return url
-    separator = "&" if "?" in url else "?"
-    return f"{url}{separator}{urlencode({'return_to': url})}"
+    return url
 
 
 @register.simple_tag

mittab/libs/tests/views/test_return_to.py

@@ -0,0 +1,77 @@
+from django.http import HttpResponse
+from django.test import RequestFactory
+
+from mittab.apps.tab.helpers import get_redirect_target
+from mittab.apps.tab.middleware import Login
+from mittab.apps.tab.templatetags.tags import return_to_value, with_return_to
+
+
+def test_get_redirect_target_prefers_session_value_over_referer():
+    request = RequestFactory().get(
+        "/pairings/status/?round=2",
+        HTTP_HOST="testserver",
+        HTTP_REFERER="/public/login/",
+    )
+    request.session = {"_return_to": "/pairings/status/?round=2"}
+
+    assert get_redirect_target(request) == "/pairings/status/?round=2"
+
+
+def test_get_redirect_target_still_honors_explicit_get_return_to():
+    request = RequestFactory().get(
+        "/pairings/status/?return_to=/public/teams/",
+        HTTP_HOST="testserver",
+    )
+
+    assert get_redirect_target(request) == "/public/teams/"
+
+
+def test_return_to_value_uses_current_request_url():
+    request = RequestFactory().get(
+        "/pairings/status/?round=4",
+        HTTP_HOST="testserver",
+    )
+    request.session = {}
+
+    assert return_to_value({"request": request}) == "/pairings/status/?round=4"
+
+
+def test_return_to_value_prefers_session_target():
+    request = RequestFactory().get(
+        "/team/12/",
+        HTTP_HOST="testserver",
+    )
+    request.session = {"_return_to": "/batch_checkin/"}
+
+    assert return_to_value({"request": request}) == "/batch_checkin/"
+
+
+def test_with_return_to_keeps_urls_clean():
+    assert with_return_to("/public/teams/") == "/public/teams/"
+    assert with_return_to("/public/teams/?foo=1") == "/public/teams/?foo=1"
+
+
+def test_login_middleware_stores_return_to_for_get_requests():
+    request = RequestFactory().get("/pairings/status/?round=1", HTTP_HOST="testserver")
+    request.user = type("User", (), {"is_anonymous": False})()
+    request.session = {}
+
+    response = Login(lambda req: HttpResponse("ok"))(request)
+
+    assert response.status_code == 200
+    assert request.session["_return_to"] == "/pairings/status/?round=1"
+
+
+def test_login_middleware_prefers_internal_referer_for_return_to():
+    request = RequestFactory().get(
+        "/team/12/",
+        HTTP_HOST="testserver",
+        HTTP_REFERER="http://testserver/batch_checkin/",
+    )
+    request.user = type("User", (), {"is_anonymous": False})()
+    request.session = {}
+
+    response = Login(lambda req: HttpResponse("ok"))(request)
+
+    assert response.status_code == 200
+    assert request.session["_return_to"] == "/batch_checkin/"