1
0
Fork 0
mirror of https://github.com/retspen/webvirtcloud synced 2026-11-08 19:26:48 +00:00

fix: require POST for user block/unblock and OTP email, flag console cookie

- user_block, user_unblock and admin_email_otp changed state on GET, so an
  <img> tag could lock a user out or trigger OTP emails through an
  admin's session. They now require POST; the buttons are CSRF forms.
- The console token cookie is set with httponly, SameSite=Lax and, on
  HTTPS, secure. Only novncd reads it; the page gets the token directly.
This commit is contained in:
catborise 2026-10-02 22:44:00 +03:00
parent e76149c71a
commit cf1cde376b
8 changed files with 77 additions and 6 deletions

View file

@ -9,9 +9,11 @@
{% block page_heading_extra %}
{% if otp_enabled %}
<a href="{% url 'accounts:admin_email_otp' user.id %}" class="btn btn-secondary" title="{% trans "Email OTP QR code" %}">
{% bs_icon 'qr-code' %}
</a>
<form method="post" action="{% url 'accounts:admin_email_otp' user.id %}" class="d-inline">{% csrf_token %}
<button type="submit" class="btn btn-secondary" title="{% trans "Email OTP QR code" %}">
{% bs_icon 'qr-code' %}
</button>
</form>
{% endif %}
<a href="{% url 'admin:user_update' user.id %}?next={% url 'accounts:account' user.id %}" class="btn btn-primary" title="{% trans "Edit user" %}">
{% bs_icon 'pencil' %}

View file

@ -0,0 +1,32 @@
from unittest.mock import patch
from django.contrib.auth import get_user_model
from django.test import TestCase
from django.urls import reverse
class AdminEmailOtpTestCase(TestCase):
"""ROADMAP S-11: sending the OTP QR code by email must not happen on GET."""
def setUp(self):
User = get_user_model()
self.superuser = User.objects.create_superuser(
username="otp_super", email="otp_super@example.com", password="x"
)
self.user = User.objects.create_user(
username="otp_user", email="otp_user@example.com", password="x"
)
self.client.force_login(self.superuser)
self.url = reverse("accounts:admin_email_otp", args=[self.user.id])
@patch("accounts.views.send_email_with_otp")
def test_get_does_not_send_email(self, mock_send):
response = self.client.get(self.url)
self.assertEqual(response.status_code, 405)
mock_send.assert_not_called()
@patch("accounts.views.send_email_with_otp")
def test_post_sends_email(self, mock_send):
response = self.client.post(self.url)
self.assertEqual(response.status_code, 302)
mock_send.assert_called_once()

View file

@ -9,6 +9,7 @@ from django.urls import reverse
from django.http import HttpResponseRedirect
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 instances.models import Instance
from accounts.forms import EmailOTPForm, ProfileForm, UserSSHKeyForm
@ -205,6 +206,7 @@ def email_otp(request):
)
@require_POST
@superuser_only
def admin_email_otp(request, user_id):
user = get_object_or_404(get_user_model(), pk=user_id)

View file

@ -60,9 +60,13 @@
<a class="btn btn-success" title="{%trans "View Profile" %}" href="{% url 'accounts:account' user.id %}">{% bs_icon 'eye-fill' %}</a>
<a class="btn btn-primary" title="{%trans "Edit" %}" href="{% url 'admin:user_update' user.id %}">{% bs_icon 'pencil-fill' %}</a>
{% if user.is_active %}
<a class="btn btn-warning" title="{%trans "Block" %}" href="{% url 'admin:user_block' user.id %}">{% bs_icon 'stop-fill' %}</a>
<form method="post" action="{% url 'admin:user_block' user.id %}">{% csrf_token %}
<button type="submit" class="btn btn-warning rounded-0" title="{%trans "Block" %}">{% bs_icon 'stop-fill' %}</button>
</form>
{% else %}
<a class="btn btn-success" title="{%trans "Unblock" %}" href="{% url 'admin:user_unblock' user.id %}">{% bs_icon 'play-fill' %}</a>
<form method="post" action="{% url 'admin:user_unblock' user.id %}">{% csrf_token %}
<button type="submit" class="btn btn-success rounded-0" title="{%trans "Unblock" %}">{% bs_icon 'play-fill' %}</button>
</form>
{% endif %}
<a class="btn btn-danger" title="{%trans "Delete" %}" href="{% url 'admin:user_delete' user.id %}">{% bs_icon 'x-circle-fill' %}</a>
</div>

View file

@ -102,11 +102,21 @@ class AdminTestCase(TestCase):
self.assertEqual(ua.max_memory, 2048)
self.assertEqual(ua.max_disk_size, 8)
# Blocking changes state, so a GET (e.g. an <img> tag) must not do it.
User.objects.filter(id=user_id).update(is_active=True)
response = self.client.get(reverse("admin:user_block", args=[user_id]))
self.assertEqual(response.status_code, 405)
self.assertTrue(User.objects.get(id=user_id).is_active)
response = self.client.post(reverse("admin:user_block", args=[user_id]))
user = User.objects.get(id=user_id)
self.assertFalse(user.is_active)
response = self.client.get(reverse("admin:user_unblock", args=[user_id]))
self.assertEqual(response.status_code, 405)
self.assertFalse(User.objects.get(id=user_id).is_active)
response = self.client.post(reverse("admin:user_unblock", args=[user_id]))
user = User.objects.get(id=user_id)
self.assertTrue(user.is_active)

View file

@ -8,6 +8,7 @@ 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.translation import gettext_lazy as _
from django.views.decorators.http import require_POST
from logs.models import Logs
from . import forms
@ -180,6 +181,7 @@ def user_delete(request, pk):
)
@require_POST
@superuser_only
def user_block(request, pk):
user: User = get_object_or_404(User, pk=pk)
@ -188,6 +190,7 @@ def user_block(request, pk):
return redirect("admin:user_list")
@require_POST
@superuser_only
def user_unblock(request, pk):
user: User = get_object_or_404(User, pk=pk)

View file

@ -175,6 +175,21 @@ class ConsoleViewsTestCase(TestCase):
self.assertEqual(response.status_code, 403)
self.assertIn("permission", response.content.decode("utf-8").lower())
@patch("console.views.wvmInstance")
def test_console_token_cookie_flags(self, mock_wvm):
# ROADMAP S-11: the token cookie is only read by novncd, never by JS.
mock_conn = MagicMock()
mock_conn.get_console_type.return_value = "vnc"
mock_conn.get_console_websocket_port.return_value = None
mock_wvm.return_value = mock_conn
self.client.force_login(self.admin_user)
response = self.client.get(reverse("console") + f"?token={self.token}")
cookie = response.cookies["token"]
self.assertTrue(cookie["httponly"])
self.assertEqual(cookie["samesite"], "Lax")
@patch("console.views.wvmInstance")
def test_console_view_instances_perm_does_not_open_console(self, mock_wvm):
# ROADMAP S-09: global view_instances is a read-only role; the console

View file

@ -166,5 +166,8 @@ def console(request):
}
response = render(request, "console-vnc-lite.html", locals())
response.set_cookie("token", token)
# Only novncd reads this cookie; JavaScript gets the token from the page.
response.set_cookie(
"token", token, httponly=True, samesite="Lax", secure=request.is_secure()
)
return response