diff --git a/accounts/templates/account.html b/accounts/templates/account.html index 3dc77687..ff902104 100644 --- a/accounts/templates/account.html +++ b/accounts/templates/account.html @@ -9,9 +9,11 @@ {% block page_heading_extra %} {% if otp_enabled %} - - {% bs_icon 'qr-code' %} - +
{% csrf_token %} + +
{% endif %} {% bs_icon 'pencil' %} diff --git a/accounts/test_admin_email_otp.py b/accounts/test_admin_email_otp.py new file mode 100644 index 00000000..1ade4005 --- /dev/null +++ b/accounts/test_admin_email_otp.py @@ -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() diff --git a/accounts/views.py b/accounts/views.py index 387c8718..2e6bc3af 100644 --- a/accounts/views.py +++ b/accounts/views.py @@ -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) diff --git a/admin/templates/admin/user_list.html b/admin/templates/admin/user_list.html index f0e0b276..ef5cbc8d 100644 --- a/admin/templates/admin/user_list.html +++ b/admin/templates/admin/user_list.html @@ -60,9 +60,13 @@ {% bs_icon 'eye-fill' %} {% bs_icon 'pencil-fill' %} {% if user.is_active %} - {% bs_icon 'stop-fill' %} +
{% csrf_token %} + +
{% else %} - {% bs_icon 'play-fill' %} +
{% csrf_token %} + +
{% endif %} {% bs_icon 'x-circle-fill' %} diff --git a/admin/tests.py b/admin/tests.py index 488fd2b0..46c26f12 100644 --- a/admin/tests.py +++ b/admin/tests.py @@ -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 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) diff --git a/admin/views.py b/admin/views.py index 8b71baf6..03d7433f 100644 --- a/admin/views.py +++ b/admin/views.py @@ -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) diff --git a/console/tests.py b/console/tests.py index 7ed14829..e91c1c3d 100644 --- a/console/tests.py +++ b/console/tests.py @@ -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 diff --git a/console/views.py b/console/views.py index 9e2a2751..da233e73 100644 --- a/console/views.py +++ b/console/views.py @@ -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