Skip to content

Add global CSRF token handlers for all jQuery AJAX and HTMX POST requests#3783

Merged
lunkwill42 merged 3 commits intomasterfrom
chore/jquery-and-htmx-csrf-fixes
Feb 13, 2026
Merged

Add global CSRF token handlers for all jQuery AJAX and HTMX POST requests#3783
lunkwill42 merged 3 commits intomasterfrom
chore/jquery-and-htmx-csrf-fixes

Conversation

@lunkwill42
Copy link
Copy Markdown
Member

@lunkwill42 lunkwill42 commented Feb 11, 2026 •
edited
Loading

Scope and purpose

I tested #3396 once more. While we remain pretty confident all Django/Python forms now provide CSRF tokens in a correct manner, the same cannot be said for various JS code that runs POST requests against the backend.

I kindly asked Claude to analyze the code base and find potential issues. It found a lot, and I sampled some of them and confirmed them to have issues when #3396 applied.

The fix Claude suggested (again) was to simply add a global handler for HTMX requests, and a similar handler for jQuery AJAX requests.

This PR provides both those solutions, and I hope @Simrayz can do a proper evaluation (I guess we should potentially add some tests as well)

The Claude analysis conclusion

Claude purported to find these issues with the codebase:

A. jQuery $.post() / $.ajax() calls without CSRF tokens
┌─────┬───────────────────────────────────────────────────────────┬──────┬──────────────────────────────────────┐
│  #  │                           File                            │ Line │          Function / Context          │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 1   │ python/nav/web/static/js/src/portadmin.js                 │ 476  │ restartInterfaces()                  │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 2   │ python/nav/web/static/js/src/business.js                  │ 99   │ addRemoveSubscriptionListener()      │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 3   │ python/nav/web/static/js/src/business.js                  │ 112  │ addUndoUnsubscribeListener()         │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 4   │ python/nav/web/static/js/src/webfront.js                  │ 68   │ addDroppableDashboardTargets()       │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 5   │ python/nav/web/static/js/src/sensor_details.js            │ 50   │ add-to-dashboard button              │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 6   │ python/nav/web/static/js/src/plugins/graphfetcher.js      │ 149  │ add graph widget to dashboard        │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 7   │ python/nav/web/static/js/src/plugins/sensor_controller.js │ 78   │ addDashboardListener()               │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 8   │ python/nav/web/static/js/src/plugins/navlet_pdu.js        │ 38   │ fetch PDU data via Graphite proxy    │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 9   │ python/nav/web/static/js/src/plugins/navlet_ups.js        │ 38   │ fetch UPS data via Graphite proxy    │
├─────┼───────────────────────────────────────────────────────────┼──────┼──────────────────────────────────────┤
│ 10  │ python/nav/web/static/js/src/ipdevinfo.js                 │ 83   │ fetch sensor data via Graphite proxy │
└─────┴───────────────────────────────────────────────────────────┴──────┴──────────────────────────────────────┘
Not an issue: main.js:36 POSTs to refresh_session which is @csrf_exempt.

Already protected (no changes needed): portadmin.js (save/commit), report.js, image_upload.js, info_room_rack.js, netmap/control_view.js, netmap/models.js, navlet_controller.js (title), navlets_htmx_controller.js, status2/views.js, neighbors.js —
these already include X-CSRFToken header or csrfmiddlewaretoken field.

Form-serialized (already safe): business.js:73 (subscription form), webfront.js:221,236 (add/rename dashboard), navlet_controller.js:210 (edit form submit) — these use $(form).serialize() on forms that contain {% csrf_token %}.

B. HTMX hx-post elements NOT inside a <form> (missing CSRF token)
┌─────┬────────────────────────────────────────────┬────────────┬───────────────────────────────────┐
│  #  │                    File                    │    Line    │              Element              │
├─────┼────────────────────────────────────────────┼────────────┼───────────────────────────────────┤
│ 11  │ webfront/index.html                        │ 78         │ "Set as default dashboard" button │
├─────┼────────────────────────────────────────────┼────────────┼───────────────────────────────────┤
│ 12  │ maintenance/new_task.html                  │ 52         │ Component search input            │
├─────┼────────────────────────────────────────────┼────────────┼───────────────────────────────────┤
│ 13  │ devicehistory/_component_search_input.html │ 10         │ Component search input            │
├─────┼────────────────────────────────────────────┼────────────┼───────────────────────────────────┤
│ 14  │ info/room/fragment_rack.html               │ 56, 74, 92 │ Add sensor buttons                │
├─────┼────────────────────────────────────────────┼────────────┼───────────────────────────────────┤
│ 15  │ webfront/_dashboard_subscribe_button.html  │ 4, 8       │ Subscribe/unsubscribe buttons     │
├─────┼────────────────────────────────────────────┼────────────┼───────────────────────────────────┤
│ 16  │ maintenance/_selected-components-list.html │ 42         │ Remove selected button            │
├─────┼────────────────────────────────────────────┼────────────┼───────────────────────────────────┤
│ 17  │ maintenance/_component-search-results.html │ 24         │ Add component button              │
└─────┴────────────────────────────────────────────┴────────────┴───────────────────────────────────┘
Already safe: HTMX hx-post elements that ARE inside <form> tags with {% csrf_token %} (all navlet edit forms, dashboard settings forms, seeddb forms, etc.) — HTMX automatically includes enclosing form values for POST requests.

Contributor Checklist

Every pull request should have this checklist filled out, no matter how small it is.
More information about contributing to NAV can be found in the
Hacker's guide to NAV.

  • Added a changelog fragment for towncrier
  • Added/amended tests for new/changed code
  • Added/changed documentation
  • Linted/formatted the code with ruff, easiest by using pre-commit
  • Wrote the commit message so that the first line continues the sentence "If applied, this commit will ...", starts with a capital letter, does not end with punctuation and is 50 characters or less long. See https://cbea.ms/git-commit/
  • Based this pull request on the correct upstream branch: For a patch/bugfix affecting the latest stable version, it should be based on that version's branch (<major>.<minor>.x). For a new feature or other additions, it should be based on master.
  • If applicable: Created new issues if this PR does not fix the issue completely/there is further work to be done
  • If it's not obvious from a linked issue, described how to interact with NAV in order for a reviewer to observe the effects of this change first-hand (commands, URLs, UI interactions)
  • If this results in changes in the UI: Added screenshots of the before and after
  • If this adds a new Python source code file: Added the boilerplate header to that file

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 11, 2026 •
edited
Loading

Test results

    20 files      20 suites   25m 35s ⏱️
 2 864 tests  2 864 ✅ 0 💤 0 ❌
16 356 runs  16 356 ✅ 0 💤 0 ❌

Results for commit 56d4a96.

♻️ This comment has been updated with latest results.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 11, 2026 •
edited
Loading

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 63.32%. Comparing base (ecaca47) to head (56d4a96).
⚠️ Report is 4 commits behind head on master.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #3783      +/-   ##
==========================================
- Coverage   63.33%   63.32%   -0.01%     
==========================================
  Files         619      619              
  Lines       45873    45873              
  Branches       43       43              
==========================================
- Hits        29053    29051       -2     
- Misses      16810    16812       +2     
  Partials       10       10

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@Simrayz
Copy link
Copy Markdown
Contributor

Simrayz commented Feb 12, 2026

Claude was fairly correct in the suggested changes, but instead of re-using CSRFUtils the code was duplicated. While $jquery.ajaxSetup(...) is not recommended in the docs, there are so many ajax requests that manually patching them all is prone to developer error. As such, global patching is the safest option.

I have added two fixup commits:

  • Updated csrf-utils.js to work as both a standalone script and RequireJS module (UMD pattern)
  • Refactored main.js to import and use CsrfUtils.getCsrfToken() instead of inline regex
  • Refactored base.html to load csrf-utils.js and call the global getCsrfToken() function
  • Added explanatory comments about why the global approach is used

These changes eliminate code duplication by using a single shared implementation in csrf-utils.js, making the codebase more maintainable.

@Simrayz
Copy link
Copy Markdown
Contributor

Simrayz commented Feb 12, 2026

I merged in #3396 locally, and verified that the following smoke test locations worked (and included a CSRF token header):

  • Dashboard: Set as default (HTMX)
  • Dashboard: Drag widget to another dashboard (jQuery)
  • Maintenance: Search components (HTMX)
  • Maintenance: Add component (HTMX)
  • Business: Unsubscribe from report (jQuery)

@Simrayz Simrayz mentioned this pull request Feb 12, 2026
Open
lunkwill42
lunkwill42 commented Feb 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member Author

@lunkwill42 lunkwill42 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, @Simrayz - I approve (though I cannot approve through GH, since I authored the PR 😆 )

@lunkwill42 lunkwill42 marked this pull request as ready for review February 13, 2026 08:36
@lunkwill42 lunkwill42 force-pushed the chore/jquery-and-htmx-csrf-fixes branch from ced50bb to fa6e3c2 Compare February 13, 2026 08:41
@lunkwill42
Copy link
Copy Markdown
Member Author

lunkwill42 commented Feb 13, 2026

Squashed the fixups, but now there's some unrelated GitHub Actions problems that wasn't there before 😝

lunkwill42 and others added 3 commits February 13, 2026 11:08
@lunkwill42 @Simrayz
Add global CSRF token setup for jQuery AJAX reqs
78e0679 
Co-authored-by: Simen Abelsen <simen.abelsen@sikt.no>
@lunkwill42 @Simrayz
Add global CSRF token setup HTMX reqs
d5c6826 
Co-authored-by: Simen Abelsen <simen.abelsen@sikt.no>
@lunkwill42
Add news fragment
56d4a96
@lunkwill42 lunkwill42 force-pushed the chore/jquery-and-htmx-csrf-fixes branch from fa6e3c2 to 56d4a96 Compare February 13, 2026 10:08
@lunkwill42
Copy link
Copy Markdown
Member Author

lunkwill42 commented Feb 13, 2026

rebaed on the latest master, which fixes the github issue

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud bot commented Feb 13, 2026

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@lunkwill42 lunkwill42 merged commit b45a062 into master Feb 13, 2026
19 checks passed
@lunkwill42 lunkwill42 deleted the chore/jquery-and-htmx-csrf-fixes branch February 13, 2026 10:23
@lunkwill42 lunkwill42 mentioned this pull request Feb 13, 2026
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Simrayz Simrayz Simrayz approved these changes

@johannaengland johannaengland johannaengland approved these changes

Assignees

@lunkwill42 lunkwill42

@Simrayz Simrayz

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@lunkwill42 @Simrayz @johannaengland