mirror of
https://github.com/retspen/webvirtcloud
synced 2026-11-08 11:16:47 +00:00
fix(admin): saving a user returns to ?next= only on this site
The user edit page redirected to its ?next= parameter after saving, whatever it held: a link to the edit page with next=https://other.site sent the administrator who saved the form to that site (CodeQL py/url-redirection). The account page links there with a next of this site. next is now followed only when url_has_allowed_host_and_scheme accepts it for this host, as user_instance_delete already does; otherwise the user list. The test fails on the previous code for an absolute URL, a scheme-relative one and a backslash variant.
This commit is contained in:
parent
1fb74d6378
commit
940545c2c4
2 changed files with 33 additions and 2 deletions
26
admin/test_user_update_next.py
Normal file
26
admin/test_user_update_next.py
Normal file
|
|
@ -0,0 +1,26 @@
|
|||
"""After saving a user, the edit page goes back to ?next= only on this site."""
|
||||
|
||||
from django.contrib.auth.models import User
|
||||
from django.test import TestCase
|
||||
from django.urls import reverse
|
||||
|
||||
LIMITS = {"max_instances": 1, "max_cpus": 1, "max_memory": 1024, "max_disk_size": 4}
|
||||
|
||||
|
||||
class UserUpdateNextTests(TestCase):
|
||||
def setUp(self):
|
||||
self.client.force_login(User.objects.create_superuser("un-admin", "un@example.com", "pw"))
|
||||
self.user = User.objects.create_user("un-user", password="pw")
|
||||
|
||||
def save(self, next_url):
|
||||
url = reverse("admin:user_update", args=[self.user.id]) + "?next=" + next_url
|
||||
return self.client.post(url, {"username": "un-user", "is_active": "on", **LIMITS})
|
||||
|
||||
def test_a_page_of_this_site(self):
|
||||
account = reverse("accounts:account", args=[self.user.id])
|
||||
self.assertRedirects(self.save(account), account, fetch_redirect_response=False)
|
||||
|
||||
def test_another_site_is_not_followed(self):
|
||||
for next_url in ("https://evil.example/", "//evil.example/", "/\\evil.example/"):
|
||||
with self.subTest(next_url=next_url):
|
||||
self.assertRedirects(self.save(next_url), reverse("admin:user_list"), fetch_redirect_response=False)
|
||||
|
|
@ -7,6 +7,7 @@ from django.contrib.auth.forms import AdminPasswordChangeForm
|
|||
from django.contrib.auth.models import Group, User
|
||||
from django.core.paginator import Paginator
|
||||
from django.shortcuts import get_object_or_404, redirect, render
|
||||
from django.utils.http import url_has_allowed_host_and_scheme
|
||||
from django.utils.translation import gettext_lazy as _
|
||||
from django.views.decorators.http import require_POST
|
||||
from logs.models import Logs
|
||||
|
|
@ -123,8 +124,12 @@ def user_update(request, pk):
|
|||
if user_form.is_valid() and attributes_form.is_valid():
|
||||
user_form.save()
|
||||
attributes_form.save()
|
||||
next = request.GET.get("next")
|
||||
return redirect(next or "admin:user_list")
|
||||
next_url = request.GET.get("next")
|
||||
if next_url and url_has_allowed_host_and_scheme(
|
||||
url=next_url, allowed_hosts={request.get_host()}, require_https=request.is_secure()
|
||||
):
|
||||
return redirect(next_url)
|
||||
return redirect("admin:user_list")
|
||||
|
||||
return render(
|
||||
request,
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue