Merge pull request from GHSA-g8x5-p9qc-cf95

* feat: pass request into checkStateFunction

* feat: add state in cookies

* fix: check presence

* fix: check cookie only

* fix: change defaultGenerateStateFunction

* Update index.js

Co-authored-by: Filip Skokan <[email protected]>

* fix: unsign

* fix: assume stateCookie is always present

* renamed state

* fix: use session

---------

Co-authored-by: Filip Skokan <[email protected]>

Author: marco-ippolito

README.md

@@ -125,11 +125,10 @@ fastify.register(oauthPlugin, {
 ## Set custom state
 
 The `generateStateFunction` accepts a function to generate the `state` parameter for the OAUTH flow. This function receives the Fastify instance's `request` object as parameter.
+The `state` parameter will be also set into a `httpOnly`, `sameSite: Lax` cookie.
 When you set it, it is required to provide the function `checkStateFunction` in order to validate the states generated.
 
 ```js
-  const validStates = new Set()
-
   fastify.register(oauthPlugin, {
     name: 'facebookOAuth2',
     credentials: {
@@ -146,12 +145,12 @@ When you set it, it is required to provide the function `checkStateFunction` in
     // custom function to generate the state
     generateStateFunction: (request) => {
       const state = request.query.customCode
-      validStates.add(state)
+      request.session.state = state
       return state
     },
     // custom function to check the state is valid
-    checkStateFunction: (returnedState, callback) => {
-      if (validStates.has(returnedState)) {
+    checkStateFunction: (request, callback) => {
+      if (request.query.state === request.session.state) {
         callback()
         return
       }

index.js

@@ -1,6 +1,6 @@
 'use strict'
 
-const defaultState = require('crypto').randomBytes(10).toString('hex')
+const crypto = require('crypto')
 
 const fp = require('fastify-plugin')
 const { AuthorizationCode } = require('simple-oauth2')
@@ -10,11 +10,13 @@ const promisify = require('util').promisify
 const callbackify = require('util').callbackify
 
 function defaultGenerateStateFunction () {
-  return defaultState
+  return crypto.randomBytes(16).toString('base64url')
 }
 
-function defaultCheckStateFunction (state, callback) {
-  if (state === defaultState) {
+function defaultCheckStateFunction (request, callback) {
+  const state = request.query.state
+  const stateCookie = request.cookies['oauth2-redirect-state']
+  if (stateCookie && state === stateCookie) {
     callback()
     return
   }
@@ -60,6 +62,10 @@ function fastifyOauth2 (fastify, options, next) {
     return next(new Error('options.schema should be a object'))
   }
 
+  if (!fastify.hasReplyDecorator('cookie')) {
+    fastify.register(require('@fastify/cookie'))
+  }
+
   const name = options.name
   const credentials = options.credentials
   const callbackUri = options.callbackUri
@@ -73,9 +79,8 @@ function fastifyOauth2 (fastify, options, next) {
   const tags = options.tags || []
   const schema = options.schema || { tags }
 
-  function generateAuthorizationUri (requestObject) {
-    const state = generateStateFunction(requestObject)
-    const urlOptions = Object.assign({}, generateCallbackUriParams(callbackUriParams, requestObject, scope, state), {
+  function generateAuthorizationUri (request, state) {
+    const urlOptions = Object.assign({}, generateCallbackUriParams(callbackUriParams, request, scope, state), {
       redirect_uri: callbackUri,
       scope,
       state
@@ -86,9 +91,13 @@ function fastifyOauth2 (fastify, options, next) {
   }
 
   function startRedirectHandler (request, reply) {
-    const authorizationUri = generateAuthorizationUri(request)
+    const state = generateStateFunction(request)
+    const authorizationUri = generateAuthorizationUri(request, state)
 
-    reply.redirect(authorizationUri)
+    reply.setCookie('oauth2-redirect-state', state, {
+      httpOnly: true,
+      sameSite: 'lax'
+    }).redirect(authorizationUri)
   }
 
   const cbk = function (o, code, callback) {
@@ -102,9 +111,8 @@ function fastifyOauth2 (fastify, options, next) {
 
   function getAccessTokenFromAuthorizationCodeFlowCallbacked (request, callback) {
     const code = request.query.code
-    const state = request.query.state
 
-    checkStateFunction(state, function (err) {
+    checkStateFunction(request, function (err) {
       if (err) {
         callback(err)
         return

package.json

@@ -38,6 +38,7 @@
     "tsd": "^0.28.0"
   },
   "dependencies": {
+    "@fastify/cookie": "^8.3.0",
     "fastify-plugin": "^4.0.0",
     "simple-oauth2": "^5.0.0"
   },

test/index.test.js

@@ -3,7 +3,7 @@
 const t = require('tap')
 const nock = require('nock')
 const createFastify = require('fastify')
-
+const crypto = require('crypto')
 const fastifyOauth2 = require('..')
 
 nock.disableNetConnect()
@@ -54,7 +54,10 @@ function makeRequests (t, fastify) {
 
       fastify.inject({
         method: 'GET',
-        url: '/?code=my-code&state=' + state
+        url: '/?code=my-code&state=' + state,
+        cookies: {
+          'oauth2-redirect-state': state
+        }
       }, function (err, responseEnd) {
         t.error(err)
 
@@ -426,7 +429,6 @@ t.test('options.generateStateFunction with request', t => {
 })
 
 t.test('generateAuthorizationUri redirect with request object', t => {
-  t.plan(4)
   const fastify = createFastify()
 
   fastify.register(fastifyOauth2, {
@@ -448,7 +450,7 @@ t.test('generateAuthorizationUri redirect with request object', t => {
   })
 
   fastify.get('/gh', function (request, reply) {
-    const redirectUrl = this.theName.generateAuthorizationUri(request)
+    const redirectUrl = this.theName.generateAuthorizationUri(request, 'generated_code')
     return reply.redirect(redirectUrl)
   })
 
@@ -463,6 +465,7 @@ t.test('generateAuthorizationUri redirect with request object', t => {
     t.equal(responseStart.statusCode, 302)
     const matched = responseStart.headers.location.match(/https:\/\/github\.com\/login\/oauth\/authorize\?response_type=code&client_id=my-client-id&redirect_uri=%2Fcallback&scope=notifications&state=generated_code/)
     t.ok(matched)
+    t.end()
   })
 })
 
@@ -799,3 +802,53 @@ t.test('preset configuration generate-callback-uri-params', t => {
     t.equal(typeof fastifyOauth2[configName].authorizePath, 'string')
   }
 })
+
+t.test('options.generateStateFunction with signing key', t => {
+  t.plan(5)
+  const fastify = createFastify()
+
+  const hmacKey = 'hello'
+  const expectedState = crypto.createHmac('sha1', hmacKey).update('foo').digest('hex')
+
+  fastify.register(require('@fastify/cookie'))
+
+  fastify.register(fastifyOauth2, {
+    name: 'the-name',
+    credentials: {
+      client: {
+        id: 'my-client-id',
+        secret: 'my-secret'
+      },
+      auth: fastifyOauth2.GITHUB_CONFIGURATION
+    },
+    startRedirectPath: '/login/github',
+    callbackUri: '/callback',
+    generateStateFunction: (request) => {
+      const state = crypto.createHmac('sha1', hmacKey).update(request.headers.foo).digest('hex')
+      t.ok(request, 'the request param has been set')
+      return state
+    },
+    checkStateFunction: (request) => {
+      const generatedState = crypto.createHmac('sha1', hmacKey).update(request.headers.foo).digest('hex')
+      return generatedState === request.query.state
+    },
+    scope: ['notifications']
+  })
+
+  t.teardown(fastify.close.bind(fastify))
+
+  fastify.listen({ port: 0 }, function (err) {
+    t.error(err)
+    fastify.inject({
+      method: 'GET',
+      url: '/login/github',
+      query: { code: expectedState },
+      headers: { foo: 'foo' }
+    }, function (err, responseStart) {
+      t.error(err)
+      t.equal(responseStart.statusCode, 302)
+      const matched = responseStart.headers.location.match(/https:\/\/github\.com\/login\/oauth\/authorize\?response_type=code&client_id=my-client-id&redirect_uri=%2Fcallback&scope=notifications&state=1e864fbd840212c1ed9ce60175d373f3a48681b2/)
+      t.ok(matched)
+    })
+  })
+})