Skip to content

Commit 8733815

Browse files
author
Morten Madsen Lyngstad
committed
Implement SafeRedirectMixin for secure redirection after form submissions and update templates to use next_url_input tag
1 parent fe92360 commit 8733815

File tree

8 files changed

+96
-63
lines changed

8 files changed

+96
-63
lines changed

src/staff/mixins.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
import django_tables2 as tables
55
from django.db import models
66
from django.db.models.query import QuerySet
7+
from django.utils.http import url_has_allowed_host_and_scheme
78
from django.utils.safestring import mark_safe
9+
from django.views.generic import View
810

911
from genlab_bestilling.models import (
1012
AnalysisOrder,
@@ -156,3 +158,43 @@ def order_priority(
156158
sorted_by_priority = queryset.order_by(f"{prefix}priority_order")
157159

158160
return (sorted_by_priority, True)
161+
162+
163+
class SafeRedirectMixin(View):
164+
"""Mixin to provide safe redirection after a successful form submission.
165+
This mixin checks for a 'next' parameter in the request and validates it
166+
to ensure it is a safe URL before redirecting. If no valid 'next' URL is found,
167+
it falls back to a method that must be implemented in the view to define
168+
a default redirect URL.
169+
"""
170+
171+
next_param = "next"
172+
173+
def get_fallback_url(self) -> str:
174+
msg = "You must override get_fallback_url()"
175+
raise NotImplementedError(msg)
176+
177+
def has_next_url(self) -> bool:
178+
next_url = self.request.POST.get(self.next_param) or self.request.GET.get(
179+
self.next_param
180+
)
181+
return bool(
182+
next_url
183+
and url_has_allowed_host_and_scheme(
184+
next_url,
185+
allowed_hosts={self.request.get_host()},
186+
require_https=self.request.is_secure(),
187+
)
188+
)
189+
190+
def get_next_url(self) -> str:
191+
next_url = self.request.POST.get(self.next_param) or self.request.GET.get(
192+
self.next_param
193+
)
194+
if next_url and url_has_allowed_host_and_scheme(
195+
next_url,
196+
allowed_hosts={self.request.get_host()},
197+
require_https=self.request.is_secure(),
198+
):
199+
return next_url
200+
return self.get_fallback_url()
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
<input type="hidden" name="next" value="{{ next_url }}">

src/staff/templates/staff/components/priority_column.html

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,12 @@
1+
{% load next_input %}
2+
13
{% if record.is_urgent %}
24
<i class="fa-solid fa-exclamation fa-lg text-red-500" title="Urgent"></i>
35
{% else %}
46
<form method="post" action="{% url 'staff:order-priority' pk=record.pk %}">
57
{% csrf_token %}
68

7-
<input type="hidden" name="next" value="{{ request.get_full_path }}" />
9+
{% next_url_input %}
810

911
{% comment %}Outlined flag icon does not work using the <i></i> tag, so also using the SVG for the filled flag icon for consistency. If it can be fixed in the future it should.{% endcomment %}
1012

src/staff/templates/staff/sample_filter.html

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{% extends "staff/base_filter.html" %}
22
{% load crispy_forms_tags static %}
33
{% load render_table from django_tables2 %}
4+
{% load next_input %}
45

56
{% block page-title %}
67
<div class="flex justify-between items-center w-full">
@@ -33,6 +34,7 @@
3334
<form method="post" action="{% url 'staff:generate-genlab-ids' pk=order.pk %}">
3435
{% csrf_token %}
3536
<input type="hidden" name="sort" value="{{ request.GET.sort|default:'' }}">
37+
{% next_url_input %}
3638
<div class="flex gap-5 mb-5">
3739
<button class="btn custom_order_button_green" type="submit" name="generate_genlab_ids">
3840
<i class="fa-solid fa-id-badge"></i> Generate genlab IDs
@@ -68,7 +70,7 @@
6870
<form id="prioritise-form" method="post" style="display:none;">
6971
{% csrf_token %}
7072
<input type="hidden" name="sample_id" id="prioritise-sample-id">
71-
<input type="hidden" name="next" id="prioritise-next-url" value="{{ request.get_full_path }}">
73+
{% next_url_input %}
7274
</form>
7375

7476
<script>

src/staff/templates/staff/sample_lab.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{% extends "staff/base.html" %}
22
{% load render_table from django_tables2 %}
33
{% load crispy_forms_tags static %}
4+
{% load next_input %}
45

56
{% block content %}
67
<h3 class="text-4xl mb-5">{% block page-title %}{% if order %}{{ order }} - Samples{% else %}Samples{% endif %}{% endblock page-title %}</h3>
@@ -21,7 +22,7 @@ <h3 class="text-4xl mb-5">{% block page-title %}{% if order %}{{ order }} - Samp
2122

2223
<form method="post" action="{% url 'staff:order-extraction-samples-lab' order.pk %}">
2324
{% csrf_token %}
24-
<input type="hidden" name="next" value="{{ request.get_full_path }}">
25+
{% next_url_input %}
2526
{% for status in statuses %}
2627
<button class="btn custom_order_button_green" type="submit" name="status" value="{{ status }}">
2728
{{ status|capfirst }}

src/staff/templates/staff/samplemarkeranalysis_filter.html

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
{% extends "staff/base_filter.html" %}
22
{% load crispy_forms_tags static %}
33
{% load render_table from django_tables2 %}
4+
{% load next_input %}
45

56
{% block page-title %}
67
{% if order %}{{ order }} - Samples{% else %}Samples{% endif %}
@@ -23,7 +24,7 @@
2324

2425
<form method="post" action="{% url 'staff:order-analysis-samples' order.pk %}">
2526
{% csrf_token %}
26-
<input type="hidden" name="next" value="{{ request.get_full_path }}">
27+
{% next_url_input %}
2728
{% for status in statuses %}
2829
<button class="btn custom_order_button_green" type="submit" name="status" value="{{ status }}">
2930
{% if status == "pcr" %}
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
from django import template
2+
3+
register = template.Library()
4+
5+
6+
@register.inclusion_tag("staff/components/next_url_input.html", takes_context=True)
7+
def next_url_input(context: template.Context) -> dict:
8+
request = context["request"]
9+
return {"next_url": request.get_full_path()}

src/staff/views.py

Lines changed: 34 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,9 @@
99
from django.http import HttpRequest, HttpResponse, HttpResponseRedirect, JsonResponse
1010
from django.shortcuts import get_object_or_404, redirect
1111
from django.urls import reverse, reverse_lazy
12-
from django.utils.http import url_has_allowed_host_and_scheme
1312
from django.utils.timezone import now
1413
from django.utils.translation import gettext as _
15-
from django.views.generic import CreateView, DetailView, TemplateView, View
14+
from django.views.generic import CreateView, DetailView, TemplateView
1615
from django.views.generic.detail import SingleObjectMixin
1716
from django_filters.views import FilterView
1817
from django_tables2.views import SingleTableMixin
@@ -33,6 +32,7 @@
3332
from nina.models import Project
3433
from shared.sentry import report_errors
3534
from shared.views import ActionView
35+
from staff.mixins import SafeRedirectMixin
3636

3737
from .filters import (
3838
AnalysisOrderFilter,
@@ -309,7 +309,9 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]:
309309
return context
310310

311311

312-
class OrderExtractionSamplesListView(StaffMixin, SingleTableMixin, FilterView):
312+
class OrderExtractionSamplesListView(
313+
StaffMixin, SingleTableMixin, SafeRedirectMixin, FilterView
314+
):
313315
table_pagination = False
314316

315317
model = Sample
@@ -318,7 +320,6 @@ class OrderExtractionSamplesListView(StaffMixin, SingleTableMixin, FilterView):
318320

319321
class Params:
320322
sample_id = "sample_id"
321-
next = "next"
322323

323324
def get_queryset(self) -> QuerySet[Sample]:
324325
queryset = (
@@ -354,26 +355,29 @@ def get_context_data(self, **kwargs) -> dict[str, Any]:
354355
)
355356
return context
356357

358+
def get_fallback_url(self) -> str:
359+
return reverse(
360+
"staff:order-extraction-samples", kwargs={"pk": self.kwargs["pk"]}
361+
)
362+
357363
def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
358364
sample_id = request.POST.get(self.Params.sample_id)
359-
360365
if sample_id:
361366
sample = get_object_or_404(Sample, pk=sample_id)
362367
sample.is_prioritised = not sample.is_prioritised
363368
sample.save()
364369

365-
next_url = request.POST.get(self.Params.next)
366-
if next_url and url_has_allowed_host_and_scheme(
367-
next_url, allowed_hosts=request.get_host()
368-
):
369-
return redirect(next_url)
370+
if self.has_next_url():
371+
return HttpResponseRedirect(self.get_next_url())
370372

371373
return self.get(
372374
request, *args, **kwargs
373375
) # Re-render the view with updated data
374376

375377

376-
class OrderAnalysisSamplesListView(StaffMixin, SingleTableMixin, FilterView):
378+
class OrderAnalysisSamplesListView(
379+
StaffMixin, SingleTableMixin, SafeRedirectMixin, FilterView
380+
):
377381
PCR = "pcr"
378382
ANALYSED = "analysed"
379383
OUTPUT = "output"
@@ -386,7 +390,6 @@ class OrderAnalysisSamplesListView(StaffMixin, SingleTableMixin, FilterView):
386390

387391
class Params:
388392
status = "status"
389-
next = "next"
390393

391394
def get_order(self) -> AnalysisOrder:
392395
return get_object_or_404(AnalysisOrder, pk=self.kwargs["pk"])
@@ -412,15 +415,7 @@ def get_context_data(self, **kwargs) -> dict[str, Any]:
412415
)
413416
return context
414417

415-
# Get full url to keep sorting after page reload
416-
def get_success_url(self) -> str:
417-
next_url = self.request.POST.get(self.Params.next)
418-
if next_url and url_has_allowed_host_and_scheme(
419-
next_url,
420-
allowed_hosts={self.request.get_host()},
421-
require_https=self.request.is_secure(),
422-
):
423-
return next_url
418+
def get_fallback_url(self) -> str:
424419
return reverse(
425420
"staff:order-analysis-samples", kwargs={"pk": self.get_order().pk}
426421
)
@@ -431,7 +426,7 @@ def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
431426

432427
if not selected_ids:
433428
messages.error(request, "No samples selected.")
434-
return HttpResponseRedirect(self.get_success_url())
429+
return HttpResponseRedirect(self.get_next_url())
435430

436431
order = self.get_order()
437432

@@ -444,7 +439,7 @@ def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
444439
self.check_all_output(SampleMarkerAnalysis.objects.filter(order=order))
445440
else:
446441
self.get_order().to_processing()
447-
return HttpResponseRedirect(self.get_success_url())
442+
return HttpResponseRedirect(self.get_next_url())
448443

449444
def statuses_with_lower_or_equal_priority(self, status_name: str) -> list[str]:
450445
index = self.VALID_STATUSES.index(status_name)
@@ -545,7 +540,7 @@ class SampleDetailView(StaffMixin, DetailView):
545540
model = Sample
546541

547542

548-
class SampleLabView(StaffMixin, SingleTableMixin, FilterView):
543+
class SampleLabView(StaffMixin, SingleTableMixin, SafeRedirectMixin, FilterView):
549544
MARKED = "marked"
550545
PLUCKED = "plucked"
551546
ISOLATED = "isolated"
@@ -559,7 +554,6 @@ class SampleLabView(StaffMixin, SingleTableMixin, FilterView):
559554
class Params:
560555
status = "status"
561556
isolation_method = "isolation_method"
562-
next = "next"
563557

564558
def get_order(self) -> ExtractionOrder:
565559
if not hasattr(self, "_order"):
@@ -593,15 +587,7 @@ def get_context_data(self, **kwargs: Any) -> dict[str, Any]:
593587
)
594588
return context
595589

596-
# Get full url to keep sorting after page reload
597-
def get_success_url(self) -> str:
598-
next_url = self.request.POST.get(self.Params.next)
599-
if next_url and url_has_allowed_host_and_scheme(
600-
next_url,
601-
allowed_hosts={self.request.get_host()},
602-
require_https=self.request.is_secure(),
603-
):
604-
return next_url
590+
def get_fallback_url(self) -> str:
605591
return reverse(
606592
"staff:order-extraction-samples-lab", kwargs={"pk": self.get_order().pk}
607593
)
@@ -613,7 +599,7 @@ def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
613599

614600
if not selected_ids:
615601
messages.error(request, "No samples selected.")
616-
return HttpResponseRedirect(self.get_success_url())
602+
return HttpResponseRedirect(self.get_next_url())
617603

618604
order = self.get_order()
619605

@@ -628,7 +614,7 @@ def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
628614
self.check_all_isolated(Sample.objects.filter(order=order))
629615
if isolation_method:
630616
self.update_isolation_methods(samples, isolation_method, request)
631-
return HttpResponseRedirect(self.get_success_url())
617+
return HttpResponseRedirect(self.get_next_url())
632618

633619
def statuses_with_lower_or_equal_priority(self, status_name: str) -> list[str]:
634620
index = self.VALID_STATUSES.index(status_name)
@@ -865,12 +851,17 @@ def form_invalid(self, form: Form) -> HttpResponse:
865851
return HttpResponseRedirect(self.get_success_url())
866852

867853

868-
class GenerateGenlabIDsView(SingleObjectMixin, StaffMixin, View):
854+
class GenerateGenlabIDsView(SingleObjectMixin, StaffMixin, SafeRedirectMixin):
869855
model = ExtractionOrder
870856

871857
def get_object(self) -> ExtractionOrder:
872858
return ExtractionOrder.objects.get(pk=self.kwargs["pk"])
873859

860+
def get_fallback_url(self) -> str:
861+
return reverse_lazy(
862+
"staff:order-extraction-samples", kwargs={"pk": self.object.pk}
863+
)
864+
874865
def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
875866
self.object = self.get_object()
876867

@@ -879,13 +870,13 @@ def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
879870

880871
if not selected_ids:
881872
messages.error(request, "No samples were selected.")
882-
return HttpResponseRedirect(self.get_return_url())
873+
return HttpResponseRedirect(self.get_next_url())
883874

884875
if not self.object.confirmed_at:
885876
messages.error(
886877
request, "Order needs to be confirmed before generating genlab IDs"
887878
)
888-
return HttpResponseRedirect(self.get_return_url())
879+
return HttpResponseRedirect(self.get_next_url())
889880

890881
try:
891882
self.object.order_selected_checked(selected_samples=selected_ids)
@@ -902,12 +893,7 @@ def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
902893
f"Error: {str(e)}",
903894
)
904895

905-
return HttpResponseRedirect(self.get_return_url())
906-
907-
def get_return_url(self) -> str:
908-
return reverse_lazy(
909-
"staff:order-extraction-samples", kwargs={"pk": self.object.pk}
910-
)
896+
return HttpResponseRedirect(self.get_next_url())
911897

912898

913899
class ExtractionPlateCreateView(StaffMixin, CreateView):
@@ -1006,24 +992,13 @@ def form_invalid(self, form: Form) -> HttpResponse:
1006992
return HttpResponseRedirect(self.get_success_url())
1007993

1008994

1009-
class OrderPrioritizedAdminView(StaffMixin, ActionView):
1010-
class Params:
1011-
next = "next"
1012-
1013-
def get_success_url(self) -> str:
1014-
next_url = self.request.POST.get(self.Params.next)
1015-
if next_url and url_has_allowed_host_and_scheme(
1016-
next_url,
1017-
allowed_hosts={self.request.get_host()},
1018-
require_https=self.request.is_secure(),
1019-
):
1020-
return next_url
1021-
995+
class OrderPrioritizedAdminView(StaffMixin, SafeRedirectMixin, ActionView):
996+
def get_fallback_url(self) -> str:
1022997
return reverse("staff:dashboard")
1023998

1024999
def post(self, request: HttpRequest, *args, **kwargs) -> HttpResponse:
10251000
pk = kwargs.get("pk")
10261001
order = Order.objects.get(pk=pk)
10271002
order.toggle_prioritized()
10281003

1029-
return redirect(self.get_success_url())
1004+
return redirect(self.get_next_url())

0 commit comments

Comments
 (0)