Skip to content

Only allow modifying patches through POST requests #127

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
JelteF merged 4 commits into from
Oct 25, 2026
Merged

Only allow modifying patches through POST requests #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5f59752
Only allow modifying patches through POST requests
FennemAI Oct 25, 2026
f1212bc
Merge remote-tracking branch 'origin' into post-for-modifications
JelteF Oct 25, 2026
c8ce287
Use require_POST and send logged out users to login first
FennemAI Oct 25, 2026
96c4a3a
Remove test_post_only.py
FennemAI Oct 25, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions media/commitfest/js/commitfest.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -317,7 +319,7 @@ function search_and_store_user() {
}

/* Create this user locally */
$.get("/ajax/importUser/", {
$.post("/ajax/importUser/", {
u: $("#searchUserList").val(),
})
.success((data) => {
Expand Down
4 changes: 2 additions & 2 deletions pgcommitfest/commitfest/ajax.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
15 changes: 11 additions & 4 deletions pgcommitfest/commitfest/templates/patch.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,14 +125,20 @@
<th>Reviewers</th>
<td>
{{patch.reviewers_string}}
{%if is_reviewer%}<a href="reviewer/remove/" class="btn btn-secondary float-end">Remove from reviewers</a>{%else%}<a href="reviewer/become/" class="btn btn-secondary float-end">Become reviewer</a>{%endif%}
{%if patch.can_remove_all_reviewers and patch.reviewers.all%}<a href="reviewer/remove_all/" class="btn btn-secondary float-end me-1">Remove all reviewers</a>{%endif%}
{%if not user.is_authenticated%}
<a class="btn btn-secondary float-end" href="/account/login/?next=/patch/{{patch.id}}/">Log in to become reviewer</a>
{%else%}
<form method="post">{%csrf_token%}
{%if is_reviewer%}<button formaction="reviewer/remove/" class="btn btn-secondary float-end">Remove from reviewers</button>{%else%}<button formaction="reviewer/become/" class="btn btn-secondary float-end">Become reviewer</button>{%endif%}
{%if patch.can_remove_all_reviewers and patch.reviewers.all%}<button formaction="reviewer/remove_all/" class="btn btn-secondary float-end me-1">Remove all reviewers</button>{%endif%}
</form>
{%endif%}
</td>
</tr>
<tr>
<th>Committer</th>
<td>{%if patch.committer%}{{patch.committer.fullname}}{%endif%}
{%if is_committer%}<a href="committer/{{is_this_committer|yesno:"remove,become"}}/" class="btn btn-secondary float-end">{{is_this_committer|yesno:"Unclaim as committer,Claim as committer"}}</a>{%endif%}
{%if is_committer%}<form method="post" action="committer/{{is_this_committer|yesno:"remove,become"}}/">{%csrf_token%}<button class="btn btn-secondary float-end">{{is_this_committer|yesno:"Unclaim as committer,Claim as committer"}}</button></form>{%endif%}
</td>
</tr>
<tr>
Expand Down Expand Up @@ -225,7 +231,7 @@ <h4>Annotations</h4>
</table>
</div>
{%if user.is_authenticated%}
<a href="{{is_subscribed|yesno:"unsubscribe,subscribe"}}/" class="btn btn-secondary">{{is_subscribed|yesno:"Unsubscribe from patch update emails,Subscribe to patch update emails"}}</a>
<form method="post" action="{{is_subscribed|yesno:"unsubscribe,subscribe"}}/">{%csrf_token%}<button class="btn btn-secondary">{{is_subscribed|yesno:"Unsubscribe from patch update emails,Subscribe to patch update emails"}}</button></form>
{%endif%}
</td>
</tr>
Expand Down Expand Up @@ -260,6 +266,7 @@ <h4 class="modal-title">Flag as committed</h4>
<div class="modal-footer">
<a href="#" class="btn btn-secondary" data-bs-dismiss="modal">Close</a>
<a href="#" class="btn btn-primary" id="doCommitButton">Flag as committed</a>
<form id="commitForm" method="post">{%csrf_token%}</form>
</div>
</div>
</div>
Expand Down
101 changes: 55 additions & 46 deletions pgcommitfest/commitfest/templates/patch_commands.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,53 +1,62 @@
<div style="margin-bottom:5px;">
<a class="btn btn-secondary" href="edit/">Edit</a>
<button type="button" class="btn btn-secondary" data-bs-toggle="modal" data-bs-target="#reviewModal">Review</button>
<form method="post" style="margin-bottom:5px;">{%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%}
<a class="btn btn-secondary" href="/account/login/?next=/patch/{{patch.id}}/">Log in to edit</a>
<button type="button" class="btn btn-secondary" data-bs-toggle="modal" data-bs-target="#reviewModal">Review</button>
{%else%}
<a class="btn btn-secondary" href="edit/">Edit</a>
<button type="button" class="btn btn-secondary" data-bs-toggle="modal" data-bs-target="#reviewModal">Review</button>

<div class="btn-group">
<button class="btn btn-secondary dropdown-toggle" type="button" data-bs-toggle="dropdown" aria-expanded="false">Change Status</button>
<ul class="dropdown-menu">
<li><h6 class="dropdown-header">Open statuses</h6></li>
<li><a class="dropdown-item" href="status/review/">Needs review</a></li>
<li><a class="dropdown-item" href="status/author/">Waiting on Author</a></li>
<li><a class="dropdown-item" href="status/committer/">Ready for Committer</a></li>
<li><hr class="dropdown-divider"></li>
<li><h6 class="dropdown-header">Closed statuses</h6></li>
<li><a class="dropdown-item" href="close/reject/" onclick="return verify_reject()">Rejected</a></li>
<li><a class="dropdown-item" href="close/withdrawn/" onclick="return verify_withdrawn()">Withdrawn</a></li>
<li><a class="dropdown-item" href="close/feedback/" onclick="return verify_returned()">Returned with feedback</a></li>
<li><a class="dropdown-item" href="close/committed/" onclick="return flagCommitted({%if patch.committer%}'{{patch.committer}}'{%elif is_committer%}'{{user.username}}'{%else%}null{%endif%})">Committed</a></li>
<li><hr class="dropdown-divider"></li>
<li><h6 class="dropdown-header">Move to</h6></li>
{%if not cf.is_open_regular %}
<li>
<a class="dropdown-item"
href="move/?from_cf_id={{cf.id}}&to_cf_id={{cfs.open.id}}">
Next CF: {{cfs.open.name}}</a>
</li>
{%endif%}
{%if not cf.is_open_draft %}
<li>
<a class="dropdown-item"
href="move/?from_cf_id={{cf.id}}&to_cf_id={{cfs.draft.id}}">
{%if cf.draft %}
Next Drafts:
{%else%}
Draft:
{%endif%}
{{cfs.draft.name}}</a>
</li>
{%endif%}
</ul>
</div>

{%if request.user.is_staff%}
<div class="btn-group">
<button class="btn btn-secondary dropdown-toggle" type="button" data-bs-toggle="dropdown" aria-expanded="false">Send private mail</button>
<button class="btn btn-secondary dropdown-toggle" type="button" data-bs-toggle="dropdown" aria-expanded="false">Change Status</button>
<ul class="dropdown-menu">
<li><a class="dropdown-item" href="send_email/?authors={{patch.id}}">Send mail to authors</a></li>
<li><a class="dropdown-item" href="send_email/?reviewers={{patch.id}}">Send mail to reviewers</a></li>
<li><a class="dropdown-item" href="send_email/?authors={{patch.id}}&reviewers={{patch.id}}">Send mail to authors and reviewers</a></li>
<li><h6 class="dropdown-header">Open statuses</h6></li>
<li><button class="dropdown-item" formaction="status/review/">Needs review</button></li>
<li><button class="dropdown-item" formaction="status/author/">Waiting on Author</button></li>
<li><button class="dropdown-item" formaction="status/committer/">Ready for Committer</button></li>
<li><hr class="dropdown-divider"></li>
<li><h6 class="dropdown-header">Closed statuses</h6></li>
<li><button class="dropdown-item" formaction="close/reject/" onclick="return verify_reject()">Rejected</button></li>
<li><button class="dropdown-item" formaction="close/withdrawn/" onclick="return verify_withdrawn()">Withdrawn</button></li>
<li><button class="dropdown-item" formaction="close/feedback/" onclick="return verify_returned()">Returned with feedback</button></li>
<li><button class="dropdown-item" type="button" onclick="return flagCommitted({%if patch.committer%}'{{patch.committer}}'{%elif is_committer%}'{{user.username}}'{%else%}null{%endif%})">Committed</button></li>
<li><hr class="dropdown-divider"></li>
<li><h6 class="dropdown-header">Move to</h6></li>
{%if not cf.is_open_regular %}
<li>
<button class="dropdown-item"
formaction="move/?from_cf_id={{cf.id}}&to_cf_id={{cfs.open.id}}">
Next CF: {{cfs.open.name}}</button>
</li>
{%endif%}
{%if not cf.is_open_draft %}
<li>
<button class="dropdown-item"
formaction="move/?from_cf_id={{cf.id}}&to_cf_id={{cfs.draft.id}}">
{%if cf.draft %}
Next Drafts:
{%else%}
Draft:
{%endif%}
{{cfs.draft.name}}</button>
</li>
{%endif%}
</ul>
</div>

{%if request.user.is_staff%}
<div class="btn-group">
<button class="btn btn-secondary dropdown-toggle" type="button" data-bs-toggle="dropdown" aria-expanded="false">Send private mail</button>
<ul class="dropdown-menu">
<li><a class="dropdown-item" href="send_email/?authors={{patch.id}}">Send mail to authors</a></li>
<li><a class="dropdown-item" href="send_email/?reviewers={{patch.id}}">Send mail to reviewers</a></li>
<li><a class="dropdown-item" href="send_email/?authors={{patch.id}}&reviewers={{patch.id}}">Send mail to authors and reviewers</a></li>
</ul>
</div>
{%endif%}
{%endif%}

</div>
</form>
4 changes: 2 additions & 2 deletions pgcommitfest/commitfest/tests/test_remove_reviewer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand All @@ -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()
8 changes: 8 additions & 0 deletions pgcommitfest/commitfest/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1113,6 +1114,7 @@ def newpatch(request, cfid):
)


@require_POST
@login_required
@transaction.atomic
def status(request, patchid, status):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -1235,6 +1238,7 @@ def int_param_or_none(request, param):
return None


@require_POST
@login_required
@transaction.atomic
def move(request, patchid):
Expand All @@ -1260,6 +1264,7 @@ def move(request, patchid):
return HttpResponseRedirect(f"/patch/{patchid}/")


@require_POST
@login_required
@transaction.atomic
def reviewer(request, patchid, status):
Expand All @@ -1286,6 +1291,7 @@ def reviewer(request, patchid, status):
return HttpResponseRedirect("../../")


@require_POST
@login_required
@transaction.atomic
def remove_all_reviewers(request, patchid):
Expand All @@ -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):
Expand Down Expand Up @@ -1344,6 +1351,7 @@ def committer(request, patchid, status):
return HttpResponseRedirect("../../")


@require_POST
@login_required
@transaction.atomic
def subscribe(request, patchid, sub):
Expand Down
Loading