diff --git a/media/commitfest/js/commitfest.js b/media/commitfest/js/commitfest.js index 24c7b66b..a353aef2 100644 --- a/media/commitfest/js/commitfest.js +++ b/media/commitfest/js/commitfest.js @@ -260,7 +260,9 @@ function flagCommitted(committer) { ); return; } - document.location.href = `close/committed/?c=${c}`; + const form = $("#commitForm"); + form.attr("action", `close/committed/?c=${encodeURIComponent(c)}`); + form.submit(); }); return false; } @@ -317,7 +319,7 @@ function search_and_store_user() { } /* Create this user locally */ - $.get("/ajax/importUser/", { + $.post("/ajax/importUser/", { u: $("#searchUserList").val(), }) .success((data) => { diff --git a/pgcommitfest/commitfest/ajax.py b/pgcommitfest/commitfest/ajax.py index 1592e800..9e0d92e4 100644 --- a/pgcommitfest/commitfest/ajax.py +++ b/pgcommitfest/commitfest/ajax.py @@ -306,8 +306,8 @@ def importUser(request): if not request.user.is_staff: raise Http404() - if request.GET.get("u", ""): - u = user_search(userid=request.GET["u"]) + if request.POST.get("u", ""): + u = user_search(userid=request.POST["u"]) if len(u) != 1: return "Internal error, duplicate user found" diff --git a/pgcommitfest/commitfest/templates/patch.html b/pgcommitfest/commitfest/templates/patch.html index 2de090b0..25e36c28 100644 --- a/pgcommitfest/commitfest/templates/patch.html +++ b/pgcommitfest/commitfest/templates/patch.html @@ -125,14 +125,20 @@ Reviewers {{patch.reviewers_string}} - {%if is_reviewer%}Remove from reviewers{%else%}Become reviewer{%endif%} - {%if patch.can_remove_all_reviewers and patch.reviewers.all%}Remove all reviewers{%endif%} + {%if not user.is_authenticated%} + Log in to become reviewer + {%else%} +
{%csrf_token%} + {%if is_reviewer%}{%else%}{%endif%} + {%if patch.can_remove_all_reviewers and patch.reviewers.all%}{%endif%} +
+ {%endif%} Committer {%if patch.committer%}{{patch.committer.fullname}}{%endif%} - {%if is_committer%}{{is_this_committer|yesno:"Unclaim as committer,Claim as committer"}}{%endif%} + {%if is_committer%}
{%csrf_token%}
{%endif%} @@ -225,7 +231,7 @@

Annotations

{%if user.is_authenticated%} - {{is_subscribed|yesno:"Unsubscribe from patch update emails,Subscribe to patch update emails"}} +
{%csrf_token%}
{%endif%} @@ -260,6 +266,7 @@ diff --git a/pgcommitfest/commitfest/templates/patch_commands.inc b/pgcommitfest/commitfest/templates/patch_commands.inc index 2811a20b..a19bbf80 100644 --- a/pgcommitfest/commitfest/templates/patch_commands.inc +++ b/pgcommitfest/commitfest/templates/patch_commands.inc @@ -1,53 +1,62 @@ -
- Edit - +
{%csrf_token%} + {%if not user.is_authenticated%} + {%comment%} + Most of these commands are POSTs, which can't be replayed after logging in, + so send anonymous users to log in first instead of letting them submit. + {%endcomment%} + Log in to edit + + {%else%} + Edit + -
- - -
- - {%if request.user.is_staff%}
- +
+ + {%if request.user.is_staff%} + + {%endif%} {%endif%} -
+ diff --git a/pgcommitfest/commitfest/tests/test_remove_reviewer.py b/pgcommitfest/commitfest/tests/test_remove_reviewer.py index a4d34e89..8535cb05 100644 --- a/pgcommitfest/commitfest/tests/test_remove_reviewer.py +++ b/pgcommitfest/commitfest/tests/test_remove_reviewer.py @@ -32,7 +32,7 @@ def test_remove_all_reviewers(client, open_cf, alice, bob, charlie): ) client.force_login(alice) - response = client.get(f"/patch/{patch.id}/reviewer/remove_all/") + response = client.post(f"/patch/{patch.id}/reviewer/remove_all/") assert response.status_code == 302 assert list(patch.reviewers.all()) == [] assert ( @@ -54,6 +54,6 @@ def test_remove_all_reviewers_forbidden_without_tag(client, open_cf, alice, bob) ) client.force_login(alice) - response = client.get(f"/patch/{patch.id}/reviewer/remove_all/") + response = client.post(f"/patch/{patch.id}/reviewer/remove_all/") assert response.status_code == 403 assert bob in patch.reviewers.all() diff --git a/pgcommitfest/commitfest/views.py b/pgcommitfest/commitfest/views.py index a484d49b..d36ac503 100644 --- a/pgcommitfest/commitfest/views.py +++ b/pgcommitfest/commitfest/views.py @@ -15,6 +15,7 @@ from django.shortcuts import get_object_or_404, render from django.utils import timezone from django.views.decorators.csrf import csrf_exempt +from django.views.decorators.http import require_POST import collections import functools @@ -1113,6 +1114,7 @@ def newpatch(request, cfid): ) +@require_POST @login_required @transaction.atomic def status(request, patchid, status): @@ -1141,6 +1143,7 @@ def status(request, patchid, status): return HttpResponseRedirect("/patch/%s/" % (poc.patch.id)) +@require_POST @login_required @transaction.atomic def close(request, patchid, status): @@ -1235,6 +1238,7 @@ def int_param_or_none(request, param): return None +@require_POST @login_required @transaction.atomic def move(request, patchid): @@ -1260,6 +1264,7 @@ def move(request, patchid): return HttpResponseRedirect(f"/patch/{patchid}/") +@require_POST @login_required @transaction.atomic def reviewer(request, patchid, status): @@ -1286,6 +1291,7 @@ def reviewer(request, patchid, status): return HttpResponseRedirect("../../") +@require_POST @login_required @transaction.atomic def remove_all_reviewers(request, patchid): @@ -1311,6 +1317,7 @@ def remove_all_reviewers(request, patchid): return HttpResponseRedirect(f"/patch/{patchid}/") +@require_POST @login_required @transaction.atomic def committer(request, patchid, status): @@ -1344,6 +1351,7 @@ def committer(request, patchid, status): return HttpResponseRedirect("../../") +@require_POST @login_required @transaction.atomic def subscribe(request, patchid, sub):