Add CSRF protection to state-changing views

[benadida]

Helios disables Django's CsrfViewMiddleware and protects views individually with check_csrf(). That is opt-in, and a number of state-changing views never opted in.

check_csrf()

On a non-POST request it returned an HttpResponseNotAllowed, but all 19 callers invoke it as a bare statement, so the response was discarded and the view carried on as if the check had passed. Both branches now raise SuspiciousOperation (Django → 400), which also lets it serve as a "this must be a POST" filter. The token comparison reads the session with .get() so a missing token fails closed instead of raising KeyError.

Views with side effects on GET

These were reachable by following a link, so a logged-in admin could be made to trigger them. Now POST-only, with the templates submitting inline forms that carry the token:

voter_delete, delete_trustee, trustee_send_url, new_trustee_helios, one_election_copy, one_election_archive, one_election_set_featured, one_election_set_reg, voters_upload_cancel, force_queue

one_election_set_reg is worth a look — it changes openreg via ?open_p=, and nothing in the UI links to it.

Views already POST-only but unchecked

Now call check_csrf(): voters_email, voters_eligibility, voters_upload, trustee_upload_pk, password_voter_login, one_election_set_result_and_proof, ldap_login_view, password_login_view, password_forgotten_view

Also

• SESSION_COOKIE_SAMESITE set explicitly to Lax (env-overridable). Already the Django default, but it's load-bearing here and shouldn't be inherited silently.
• The markup around these actions moved from p to div in election_view.html and stats.html, and out of the h5 in list_trustees.html. A form is flow content: the parser silently closes an open p when it reaches one (which would move these actions out of the block they appear to belong to), and neither a p nor a heading may contain one. election_view.html already had the p bug for the existing delete form.
• Fixes a latent KeyError in voters_upload_cancel, which unconditionally deleted a session key that may not be set.
• New helios/test_csrf_smoke.py covers three things: the views reject a GET, a tokenless POST and a badly-signed POST; the admin templates render their actions as POST forms carrying the token; and no rendered form ends up nested in a p or an h1-h6 (parsed with html5lib, added as a dev dependency).

Existing tests that drove these endpoints were updated to send a token. Full suite passes on top of current master: 214 tests, 1 skip (the LDAP test, which needs a live LDAP server — skipped on master too).

Left alone deliberately

post_audited_ballot and trustee_upload_decryption are @election_view() with no authentication at all — anyone can POST to them unauthenticated. CSRF is meaningless without a session privilege to ride, and a token check would break the booth without adding security. That's a missing-authorization issue, worth its own change.

[Copilot]

Pull request overview

This PR hardens Helios’ opt-in CSRF protection by making check_csrf() fail closed (via exceptions) and converting previously state-changing GET endpoints into CSRF-protected POST flows, including UI/template updates to submit inline POST forms with the session CSRF token.

Changes:

• Make check_csrf() raise SuspiciousOperation for non-POST and invalid/missing CSRF tokens, and avoid KeyError on missing session token.
• Convert multiple admin/state-changing actions from GET to POST and update templates to render them as POST forms carrying csrf_token.
• Add a CSRF regression smoke test suite and update existing tests to include CSRF tokens on POSTs; explicitly set SESSION_COOKIE_SAMESITE (env-overridable).

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
settings.py	Explicitly sets SESSION_COOKIE_SAMESITE to support the app’s CSRF model.
server_ui/media/main.css	Adds styling to make inline POST-form buttons look like links.
helios/views.py	Adds check_csrf() to many state-changing views and switches parameters from GET to POST where needed.
helios/tests.py	Updates existing tests to send csrf_token on POSTs and switches force-queue test to POST.
helios/test_csrf_smoke.py	Adds regression tests to ensure state-changing endpoints reject GET/tokenless/bad-token requests and templates render protected forms.
helios/templates/voters_upload_confirm.html	Converts upload-cancel actions from links to CSRF-protected POST forms.
helios/templates/voters_manage.html	Converts voter delete from GET link to CSRF-protected POST form.
helios/templates/voters_list.html	Converts voter delete from GET link to CSRF-protected POST form.
helios/templates/stats.html	Converts force-queue from GET link to CSRF-protected POST form; uses <div> to allow forms.
helios/templates/list_trustees.html	Converts trustee actions from GET links to CSRF-protected POST forms.
helios/templates/election_view.html	Converts admin actions (archive/copy/set-featured) to CSRF-protected POST forms; avoids <p> auto-close issues.
helios/templates/election_keygenerator.html	Adds csrf_token to trustee public-key upload POST form.
helios/stats_views.py	Adds check_csrf() to force_queue.
helios_auth/tests.py	Primes session and adds csrf_token to LDAP login POST test.
helios_auth/security/init.py	Updates check_csrf() to raise SuspiciousOperation and fail closed on missing session token.
helios_auth/auth_systems/password.py	Adds check_csrf() to password login/forgotten POST handlers.
helios_auth/auth_systems/ldapauth.py	Adds check_csrf() to LDAP login POST handler.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[benadida]

Fixed in 7f94a82, though the stated mechanism isn't what happens.

A <form> start tag does not implicitly close an open heading. Per the HTML5 tree construction algorithm it closes an open <p> (in button scope) and nothing else; h1-h6 are only popped by another heading start tag. Checked against html5lib:

<h5>Name <form><button>x</button></form></h5>
  -> <h5> 'Name'
       <form>
         <button> 'x'          # stays nested, h5 not closed

<p>text <form><button>x</button></form></p>
  -> <p> 'text'
     <form>                    # p closed, form is a sibling
       <button> 'x'
     <p>                       # stray empty p

So there was no broken layout here. The real issue is the content model: a <form> is flow content and a heading only accepts phrasing content, which makes it invalid even though it parses and renders fine. That's worth fixing regardless, and it was introduced by this PR — before it, the heading held only <a> elements.

I didn't move the forms below the heading, since that would push [x] and send login onto their own line. Instead the heading and its actions are wrapped in a div.trustee-heading with the <h5> inlined, so the markup is valid and the layout is unchanged. Verified in Chromium: the <h5> and its forms share a baseline (y=363 vs 365), and the rendered line still reads Trustee #1: Alice Trustee (alice@example.com) [x] [send login].

The <p> half of your point was real, and it's why the same commit's election_view.html/stats.html changes moved those blocks to <div>. test_action_forms_are_not_nested_in_phrasing_only_elements now parses the rendered admin pages and asserts no <form> sits inside a <p> or an <h1>-<h6>.

---

Generated by Claude Code