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

fix: skip safe methods #167

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
19 changes: 10 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -138,15 +138,16 @@ Apart from these safeguards, it is extremely important to [use HTTPS for your we

### Module Options

| Options | Description |
| ----------- | ----------- |
| `cookieKey` | The name of the cookie where the CSRF secret will be stored, default `_csrf`. |
| `cookieOpts` | The cookie serialization options. See [@fastify/cookie](https://github.com/fastify/fastify-cookie). |
| `sessionKey` | The key where to store the CSRF secret in the session. |
| `getToken` | A sync function to get the CSRF secret from the request. |
| `getUserInfo` | A sync function to get the a string of user-specific information to prevent cookie tossing. |
| `sessionPlugin` | The session plugin that you are using (if applicable). |
| `csrfOpts` | The csrf options. See [@fastify/csrf](https://github.com/fastify/csrf). |
| Options | Description |
|--------------------|-----------------------------------------------------------------------------------------------------------------|
| `cookieKey` | The name of the cookie where the CSRF secret will be stored, default `_csrf`. |
| `cookieOpts` | The cookie serialization options. See [@fastify/cookie](https://github.com/fastify/fastify-cookie). |
| `sessionKey` | The key where to store the CSRF secret in the session. |
| `getToken` | A sync function to get the CSRF secret from the request. |
| `getUserInfo` | A sync function to get the a string of user-specific information to prevent cookie tossing. |
| `sessionPlugin` | The session plugin that you are using (if applicable). |
| `csrfOpts` | The csrf options. See [@fastify/csrf](https://github.com/fastify/csrf). |
| `protectedMethods` | An array to define all methods protected by CSRF, default (or empty array) `['POST', 'PUT', 'PATCH', 'DELETE']` |

### `reply.generateCsrf([opts])`

Expand Down
32 changes: 21 additions & 11 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ const defaultOptions = {
sessionKey: '_csrf',
getToken: getTokenDefault,
getUserInfo: getUserInfoDefault,
sessionPlugin: '@fastify/cookie'
sessionPlugin: '@fastify/cookie',
protectedMethods: ['POST', 'PUT', 'PATCH', 'DELETE']
}

async function fastifyCsrfProtection (fastify, opts) {
Expand All @@ -24,7 +25,8 @@ async function fastifyCsrfProtection (fastify, opts) {
sessionKey,
getToken,
getUserInfo,
sessionPlugin
sessionPlugin,
protectedMethods
} = Object.assign({}, defaultOptions, opts)

const csrfOpts = opts && opts.csrfOpts ? opts.csrfOpts : {}
Expand All @@ -36,8 +38,14 @@ async function fastifyCsrfProtection (fastify, opts) {
assert(typeof cookieOpts === 'object', 'cookieOpts should be a object')
assert(
['@fastify/cookie', '@fastify/session', '@fastify/secure-session'].includes(sessionPlugin),
"sessionPlugin should be one of the following: '@fastify/cookie', '@fastify/session', '@fastify/secure-session'"
'sessionPlugin should be one of the following: \'@fastify/cookie\', \'@fastify/session\', \'@fastify/secure-session\''
)
assert(Array.isArray(protectedMethods), 'protectedMethods should be an array')

if (!protectedMethods.length) {
// for security reasons, default methods are restored
protectedMethods.push(...defaultOptions.protectedMethods)
}

if (opts.getUserInfo) {
csrfOpts.userInfo = true
Expand Down Expand Up @@ -111,14 +119,16 @@ async function fastifyCsrfProtection (fastify, opts) {
}

function csrfProtection (req, reply, next) {
const secret = getSecret(req, reply)
if (!secret) {
req.log.warn('Missing csrf secret')
return reply.send(new MissingCSRFSecretError())
}
if (!tokens.verify(secret, getToken(req), getUserInfo(req))) {
req.log.warn('Invalid csrf token')
return reply.send(new InvalidCSRFTokenError())
if (protectedMethods.indexOf(req.method) > -1) {
const secret = getSecret(req, reply)
if (!secret) {
req.log.warn('Missing csrf secret')
return reply.send(new MissingCSRFSecretError())
}
if (!tokens.verify(secret, getToken(req), getUserInfo(req))) {
req.log.warn('Invalid csrf token')
return reply.send(new InvalidCSRFTokenError())
}
}
next()
}
Expand Down
62 changes: 61 additions & 1 deletion test/basic.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ test('Cookies', t => {
fastify.decorate('testType', 'fastify-cookie')
return fastify
}

runTest(t, load, { property: '_csrf', place: 'body' }, 'preValidation')
runTest(t, load, { property: 'csrf-token', place: 'headers' })
runTest(t, load, { property: 'xsrf-token', place: 'headers' })
Expand Down Expand Up @@ -56,6 +57,7 @@ test('Cookies signed', t => {
fastify.decorate('testType', 'fastify-cookie')
return fastify
}

runTest(t, load, { property: '_csrf', place: 'body' }, 'preValidation')
runTest(t, load, { property: 'csrf-token', place: 'headers' })
runTest(t, load, { property: 'xsrf-token', place: 'headers' })
Expand All @@ -77,6 +79,7 @@ test('Fastify Session', t => {
fastify.decorate('testType', 'fastify-session')
return fastify
}

runTest(t, load, { property: '_csrf', place: 'body' }, 'preValidation')
runTest(t, load, { property: 'csrf-token', place: 'headers' }, 'preValidation')
runTest(t, load, { property: 'xsrf-token', place: 'headers' }, 'preValidation')
Expand All @@ -93,6 +96,7 @@ test('Fastify Secure Session', t => {
fastify.decorate('testType', 'fastify-secure-session')
return fastify
}

runTest(t, load, { property: '_csrf', place: 'body' }, 'preValidation')
runTest(t, load, { property: 'csrf-token', place: 'headers' })
runTest(t, load, { property: 'xsrf-token', place: 'headers' })
Expand Down Expand Up @@ -149,7 +153,17 @@ test('Validation', t => {
fastify.register(fastifyCookie)
fastify.register(fastifyCsrf, { sessionPlugin: 42 })
fastify.ready(err => {
t.equal(err.message, "sessionPlugin should be one of the following: '@fastify/cookie', '@fastify/session', '@fastify/secure-session'")
t.equal(err.message, 'sessionPlugin should be one of the following: \'@fastify/cookie\', \'@fastify/session\', \'@fastify/secure-session\'')
})
})

t.test('protectedMethods', t => {
t.plan(1)
const fastify = Fastify()
fastify.register(fastifyCookie)
fastify.register(fastifyCsrf, { protectedMethods: 42 })
fastify.ready(err => {
t.equal(err.message, 'protectedMethods should be an array')
})
})

Expand All @@ -174,6 +188,52 @@ test('csrf options', async () => {
sinon.assert.calledWith(csrf, csrfOpts)
})

test('csrfProtection on fastify instance', t => {
async function load (protectedMethods = []) {
const fastify = Fastify()
await fastify.register(fastifyCookie)
await fastify.register(fastifyCsrf, { protectedMethods })
fastify.decorate('testType', 'fastify-cookie')
fastify.addHook('onRequest', fastify.csrfProtection)
fastify.get('/', async (req, reply) => {
return {}
})
fastify.post('/', async (req, reply) => {
return {}
})
return fastify
}

t.test('GET method should failed', async t => {
const fastify = await load(['GET'])
const response = await fastify.inject({
method: 'GET',
path: '/'
})
t.match(response.json(), { message: 'Missing csrf secret' })
})

t.test('GET method should pass', async t => {
const fastify = await load()
const response = await fastify.inject({
method: 'GET',
path: '/'
})
t.equal(response.statusCode, 200)
})

t.test('POST method should failed', async t => {
const fastify = await load()
const response = await fastify.inject({
method: 'POST',
path: '/'
})
t.match(response.json(), { message: 'Missing csrf secret' })
})

t.end()
})

function runTest (t, load, tkn, hook = 'onRequest') {
t.test(`Token in ${tkn.place}`, async t => {
const fastify = await load()
Expand Down
1 change: 1 addition & 0 deletions types/index.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ declare namespace fastifyCsrfProtection {
sessionKey?: string;
getUserInfo?: (req: FastifyRequest) => string;
getToken?: GetTokenFn;
protectedMethods?: string[];
}

interface FastifyCsrfProtectionOptionsFastifyCookie {
Expand Down