Skip to content
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.

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

Adding CSRF form protection via Hapi Crumb plugin #911

Merged
merged 24 commits into from
Jul 31, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
60004d1
Adding CSRF form protection via Hapi Crumb plugin
rvsiyad Apr 15, 2024
bca9e14
Adding the Hapi Crumb Dependency
rvsiyad Apr 15, 2024
32591ff
Creating the Crumb plugin
rvsiyad Apr 15, 2024
e95ce45
Registering Crumb plugin in the server
rvsiyad Apr 15, 2024
b0d50ad
Adding Crumb CSRF to each form in completed views
rvsiyad Apr 15, 2024
603e13a
Adding Crumb CSRF to each form on uncompleted views
rvsiyad Apr 15, 2024
f2f8fb1
we did stuff
rvsiyad Apr 22, 2024
a85d6d5
Update following rebase
Cruikshanks Jul 29, 2024
db5685d
Solve problem of how to test crumb enabled endpoints
Cruikshanks Jul 29, 2024
dabbefa
Remove debugging from test
Cruikshanks Jul 29, 2024
0ff1f9f
Update return requirements tests to use helper
Cruikshanks Jul 29, 2024
72a916c
Update bill licence views with crumb
Cruikshanks Jul 30, 2024
a46601a
Update bill runs setup with crumb
Cruikshanks Jul 30, 2024
8023246
Update bill runs with crumb
Cruikshanks Jul 30, 2024
9e98f88
Update bills with crumb
Cruikshanks Jul 30, 2024
833f5e2
Update data for crumb
Cruikshanks Jul 30, 2024
c425df3
Add a space after the hidden crumb input value
Cruikshanks Jul 30, 2024
55a4868
Update config for API only POST endpoints
Cruikshanks Jul 30, 2024
33943bf
Add extra documentation to the plugin
Cruikshanks Jul 30, 2024
fa89099
Merge branch 'main' into hansel-and-grettel-csrf
Cruikshanks Jul 31, 2024
432c774
Merge branch 'main' into hansel-and-grettel-csrf
Cruikshanks Jul 31, 2024
6a03b86
Typo in plugin comments
Cruikshanks Jul 31, 2024
4288155
Merge branch 'main' into hansel-and-grettel-csrf
Cruikshanks Jul 31, 2024
5b73393
Merge branch 'main' into hansel-and-grettel-csrf
Cruikshanks Jul 31, 2024
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
50 changes: 50 additions & 0 deletions app/plugins/crumb.plugin.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
'use strict'

/**
* Plugin to add CSRF token to all our forms
* @module CrumbPlugin
*/

const Crumb = require('@hapi/crumb')

/**
* {@link https://hapi.dev/module/crumb/api/?v=9.0.1 | Crumb} is a Hapi plugin. Crumb is used to diminish CSRF attacks
* using a random unique token that is validated on the server side.
*
* Every view in the service that has a form that uses `method="POST"`, needs to have a hidden input field.
*
* ```html
* <input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>
* ```
*
* When the page is requested the Crumb plugin will generate a token in the `onPreResponse` event that it will save to a
* cookie called 'wrlsCrumb'. It also makes it available in the view context hence our views can reference
* `{{wrlsCrumb}}`.
*
* When the user submits the form the Crumb plugin jumps in again, this time on the `onPostAuth` event. It compares both
* values and if they match it 'authorises' the request.
*
* If the the payload is missing `{{wrlsCrumb}}`, or the value doesn't match that saved in the cookie (which is secure
* so unreadable to the client) than it rejects the request. In our service the user will see a 404 as that is the
* default for an 'unauthorised' request.
*
* We have Crumb enabled by default for all endpoints to avoid us forgetting to protect a html form. However, this means
* it performs the check for _all_ POST requests. Our service exposes API-only POST endpoints which do not expect a
* payload. For this, the route config must include the following to disable Crumb.
*
* ```javascript
* options: {
* plugins: {
* crumb: false
* }
* }
* ```
*/
const CrumbPlugin = {
plugin: Crumb,
options: {
key: 'wrlsCrumb'
}
}

module.exports = CrumbPlugin
3 changes: 3 additions & 0 deletions app/routes/bill-runs.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ const routes = [
access: {
scope: ['billing']
}
},
plugins: {
crumb: false
}
}
},
Expand Down
3 changes: 3 additions & 0 deletions app/routes/billing-accounts.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ const routes = [
access: {
scope: ['manage_billing_accounts']
}
},
plugins: {
crumb: false
}
}
}
Expand Down
15 changes: 12 additions & 3 deletions app/routes/data.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ const routes = [
excludeFromProd: true,
plainOutput: true
},
auth: false
auth: false,
plugins: {
crumb: false
}
}
},
{
Expand All @@ -48,7 +51,10 @@ const routes = [
excludeFromProd: true,
plainOutput: true
},
auth: false
auth: false,
plugins: {
crumb: false
}
}
},
{
Expand All @@ -60,7 +66,10 @@ const routes = [
excludeFromProd: true,
plainOutput: true
},
auth: false
auth: false,
plugins: {
crumb: false
}
}
}
]
Expand Down
5 changes: 4 additions & 1 deletion app/routes/import.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ const routes = [
app: {
plainOutput: true
},
auth: false
auth: false,
plugins: {
crumb: false
}
}
}
]
Expand Down
20 changes: 16 additions & 4 deletions app/routes/jobs.routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,10 @@ const routes = [
app: {
plainOutput: true
},
auth: false
auth: false,
plugins: {
crumb: false
}
}
},
{
Expand All @@ -22,7 +25,10 @@ const routes = [
app: {
plainOutput: true
},
auth: false
auth: false,
plugins: {
crumb: false
}
}
},
{
Expand All @@ -33,7 +39,10 @@ const routes = [
app: {
plainOutput: true
},
auth: false
auth: false,
plugins: {
crumb: false
}
}
},
{
Expand All @@ -44,7 +53,10 @@ const routes = [
app: {
plainOutput: true
},
auth: false
auth: false,
plugins: {
crumb: false
}
}
}
]
Expand Down
2 changes: 2 additions & 0 deletions app/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const Hapi = require('@hapi/hapi')
const AirbrakePlugin = require('./plugins/airbrake.plugin.js')
const AuthPlugin = require('./plugins/auth.plugin.js')
const ChargingModuleTokenCachePlugin = require('./plugins/charging-module-token-cache.plugin.js')
const CrumbPlugin = require('./plugins/crumb.plugin.js')
const ErrorPagesPlugin = require('./plugins/error-pages.plugin.js')
const GlobalHapiServerMethodsPlugin = require('./plugins/global-hapi-server-methods.plugin.js')
const GlobalNotifierPlugin = require('./plugins/global-notifier.plugin.js')
Expand All @@ -31,6 +32,7 @@ const registerPlugins = async (server) => {
await server.register(AirbrakePlugin)
await server.register(GlobalNotifierPlugin)
await server.register(ChargingModuleTokenCachePlugin)
await server.register(CrumbPlugin)
await server.register(ErrorPagesPlugin)
await server.register(RequestNotifierPlugin)
await server.register(PayloadCleanerPlugin)
Expand Down
1 change: 1 addition & 0 deletions app/views/bill-licences/remove.njk
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@
}}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>
{{ govukButton({ text: "Remove this licence", preventDoubleClick: true }) }}
</form>
</div>
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/amend-adjustment-factor.njk
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,8 @@
{% endif %}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukInput({
label: {
text: "Aggregate factor",
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/amend-authorised-volume.njk
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
}}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukInput({
label: {
text: "Authorised volume",
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/amend-billable-returns.njk
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,8 @@

<div class="govuk-body">
<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukRadios({
name: 'quantity-options',
errorMessage: error.radioFormElement,
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/cancel.njk
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
}}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukButton({ text: "Cancel bill run" }) }}
</form>
</div>
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/remove-licence.njk
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@
}}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukButton({ text: "Remove this licence", preventDoubleClick: true }) }}
</form>
</div>
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/review-licence.njk
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@
{% endif %}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

<div class="govuk-button-group">
{{ govukButton({
text: statusButtonText,
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/review.njk
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@
<h2 class="govuk-heading-m govuk-!-margin-bottom-3">Filter by</h2>

<form method="post" action="review">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{# Filter by licence holder or licence number #}
{{ govukInput({
label: {
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/send.njk
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,8 @@
}}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukButton({ text: "Send bill run" }) }}
</form>
</div>
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/setup/region.njk
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

<div class="govuk-body">
<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{% set regionItems = [] %}
{% for region in regions %}
{% set regionItem = { text: region.displayName, value: region.id, checked: region.id == selectedRegion } %}
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/setup/season.njk
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

<div class="govuk-body">
<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukRadios({
attributes: {
'data-test': 'bill-run-season'
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/setup/type.njk
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

<div class="govuk-body">
<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukRadios({
attributes: {
'data-test': 'bill-run-type'
Expand Down
2 changes: 2 additions & 0 deletions app/views/bill-runs/setup/year.njk
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@

<div class="govuk-body">
<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukRadios({
attributes: {
'data-test': 'bill-run-year'
Expand Down
2 changes: 2 additions & 0 deletions app/views/bills/remove.njk
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@
}}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukButton({ text: "Remove this bill", preventDoubleClick: true }) }}
</form>
</div>
Expand Down
2 changes: 2 additions & 0 deletions app/views/data/deduplicate.njk
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@

<div class="govuk-body">
<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukInput({
classes: 'govuk-!-width-one-third',
errorMessage: error,
Expand Down
2 changes: 2 additions & 0 deletions app/views/return-requirements/abstraction-period.njk
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@
</div>

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{# From #}
{{ govukDateInput({
id: "abstraction-period-start",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
{% endset %}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

<div class="govuk-body">
{{ govukCheckboxes({
name: "additionalSubmissionOptions",
Expand Down
2 changes: 2 additions & 0 deletions app/views/return-requirements/agreements-exceptions.njk
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,8 @@
</div>

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukCheckboxes({
name: "agreementsExceptions",
hint: {
Expand Down
1 change: 1 addition & 0 deletions app/views/return-requirements/cancel.njk
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
<p class="govuk-!-margin-bottom-7"></p>

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>
<input type="hidden" id="licence-id" name="licenceId" value="{{ licenceId }}" />

{{ govukButton({ text: "Confirm cancel", preventDoubleClick: true }) }}
Expand Down
4 changes: 4 additions & 0 deletions app/views/return-requirements/check.njk
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@

{# Add another requirement button #}
<form method="post" action="/system/return-requirements/{{ sessionId }}/add">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukButton({
text: "Add another requirement",
classes: "govuk-button--secondary",
Expand Down Expand Up @@ -319,6 +321,8 @@
{% endif %}

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

<div class="govuk-body">
{% if returnsRequired %}
{{ govukSummaryList({
Expand Down
2 changes: 2 additions & 0 deletions app/views/return-requirements/existing.njk
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@

<div class="govuk-body">
<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukRadios({
name: "existing",
errorMessage: error,
Expand Down
2 changes: 2 additions & 0 deletions app/views/return-requirements/frequency-collected.njk
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
</div>

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukRadios({
name: "frequencyCollected",
errorMessage: error,
Expand Down
2 changes: 2 additions & 0 deletions app/views/return-requirements/frequency-reported.njk
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@
</div>

<form method="post">
<input type="hidden" name="wrlsCrumb" value="{{wrlsCrumb}}"/>

{{ govukRadios({
name: "frequencyReported",
errorMessage: error,
Expand Down
Loading
Loading