Clean up next URL handling

apps/cfp_review/base.py

@@ -26,6 +26,7 @@
 from sqlalchemy.orm import joinedload, undefer
 from sqlalchemy_continuum.utils import version_class
 
+from apps.common import get_next_url
 from main import db, external_url, get_or_404
 from .estimation import get_cfp_estimate
 from .majority_judgement import calculate_max_normalised_score
@@ -1288,7 +1289,7 @@ def proposals_summary():
 def confidentiality_warning():
     if request.method == "POST" and request.form.get("agree"):
         session["cfp_confidentiality"] = True
-        return redirect(request.args.get("next", url_for(".proposals")))
+        return redirect(get_next_url(default=url_for(".proposals")))
 
     return render_template("cfp_review/confidentiality_warning.html")
 

apps/common/__init__.py

@@ -6,14 +6,18 @@
 from os import path
 from pathlib import Path
 from textwrap import wrap
+from typing import overload
+from urllib.parse import urljoin, urlparse, urlunparse
 
 import pendulum
 from decorator import decorator
 from flask import (
     abort,
     render_template,
     render_template_string,
+    request,
     session,
+    url_for,
 )
 from flask import (
     current_app as app,
@@ -376,3 +380,30 @@ def render_markdown(source, template="about/template.html", **view_variables):
 
     view_variables.update(content=content, title=metadata["title"])
     return render_template(page_template(metadata, template), **view_variables)
+
+
+def make_safe_url(target: str) -> str | None:
+    ref_url = urlparse(request.host_url)
+    test_url = urlparse(urljoin(request.host_url, target))
+    if test_url.scheme in ("http", "https") and ref_url.netloc == test_url.netloc:
+        return urlunparse(test_url)
+    return None
+
+
+@overload
+def get_next_url(default: str) -> str: ...
+
+
+@overload
+def get_next_url(default: None) -> str | None: ...
+
+
+def get_next_url(default=None):
+    next_url = request.args.get("next")
+    if next_url:
+        if safe_url := make_safe_url(next_url):
+            return safe_url
+        app.logger.error(f"Dropping unsafe next URL {repr(next_url)}")
+    if default is None:
+        default = url_for(".account")
+    return default

apps/users/__init__.py

@@ -1,5 +1,4 @@
 import time
-from urllib.parse import urljoin, urlparse
 
 from flask import (
     Blueprint,
@@ -22,6 +21,7 @@
 from wtforms import BooleanField, StringField, SubmitField
 from wtforms.validators import DataRequired, ValidationError
 
+from apps.common import get_next_url
 from main import db, get_or_404
 from models.basket import Basket
 from models.cfp import CFPMessage, Proposal
@@ -59,23 +59,6 @@ def users_variables():
     }
 
 
-def is_safe_url(target):
-    ref_url = urlparse(request.host_url)
-    test_url = urlparse(urljoin(request.host_url, target))
-    return test_url.scheme in ("http", "https") and ref_url.netloc == test_url.netloc
-
-
-def get_next_url(default=None):
-    next_url = request.args.get("next")
-    if next_url:
-        if is_safe_url(next_url):
-            return next_url
-        app.logger.error(f"Dropping unsafe next URL {repr(next_url)}")
-    if default is None:
-        default = url_for(".account")
-    return default
-
-
 class LoginForm(Form):
     email = EmailField("Email")
 

apps/volunteer/schedule.py

@@ -15,9 +15,8 @@
 from models.volunteer.venue import VolunteerVenue
 from models.volunteer.volunteer import Volunteer
 
-from ..common import feature_flag
+from ..common import feature_flag, get_next_url
 from ..schedule import event_tz
-from ..users import get_next_url
 from . import v_admin_required, v_user_required, volunteer
 
 

apps/volunteer/sign_up.py

@@ -14,7 +14,7 @@
 from wtforms import BooleanField, StringField, SubmitField
 from wtforms.validators import DataRequired, Email, ValidationError
 
-from apps.users import get_next_url
+from apps.common import get_next_url
 from main import db
 from models.user import User
 from models.volunteer import Volunteer as VolunteerUser