Misleading (Wrong) documentation for checkStateFunction

[agjs]

Prerequisites

• [X] I have written a descriptive issue title
• [X] I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.19.0

Plugin version

7.0.1

Node.js version

20.5.0

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

20.04

Description

Hey folks. I've spent couple of hours last night in a live stream, and also today after work trying to figure out why I'm getting invalid state error when I try to authorize with Github. While I haven't yet figure out why, I've noticed that the documentation under the section Set custom state is wrong.

The documentation states that checkStateFunction receives a request object, and that's incorrect based on the source code:

  function getAccessTokenFromAuthorizationCodeFlowCallbacked (request, callback) {
    const code = request.query.code
    const state = request.query.state

    checkStateFunction(state, function (err) {
      if (err) {
        callback(err)
        return
      }
      cbk(fastify[name], code, callback)
    })
  }

We can see that checkStateFunction gets the primitive string value from request.query.

With that said, and please correct me if I'm wrong, the entire example under Set custom state is wrong.

I will keep digging into this, as I keep getting an error only in production that the state is invalid, while locally, it works. It makes no sense, but something here feels off. In case I find out, I'll do a pull request.

Snippets of source code that illustrate the issue, in case I'm missing something.

const defaultState = require('crypto').randomBytes(10).toString('hex')

function defaultGenerateStateFunction () {
  return defaultState
}

function defaultCheckStateFunction (state, callback) {
  if (state === defaultState) {
    callback()
    return
  }
  callback(new Error('Invalid state'))
}

 const checkStateFunction = options.checkStateFunction || defaultCheckStateFunction

  function getAccessTokenFromAuthorizationCodeFlowCallbacked (request, callback) {
    const code = request.query.code
    const state = request.query.state

    checkStateFunction(state, function (err) {
      if (err) {
        callback(err)
        return
      }
      cbk(fastify[name], code, callback)
    })
  }

Steps to Reproduce

Described in the issue.

Expected Behavior

I'd expect that the documentation states accurate information about how to use the library.

[Uzlopak]

I guess, the documentation was not changed after the code was changed. Tbh this plugin seems a little bit sloppy maintained. If you could have an overall look on it, if you have time, it would be great.

[agjs]

I guess, the documentation was not changed after the code was changed. Tbh this plugin seems a little bit sloppy maintained. If you could have an overall look on it, if you have time, it would be great.

Absolutely, happy to help. I'll spend most of my weekend working on a feature that requires this to work, so expect a PR either today or over the weekend.

[fa-sharp]

I think the referenced code has been updated since this issue was opened, and the docs and code now match up. In particular, the checkStateFunction now receives the entire request object, as seen here:

https://github.com/fastify/fastify-oauth2/blob/3d8479781d24f9c4b29613d3d779b1114bbe1c49/index.js#L225-L243

[gotenxds]

Hello, over a year later and this issue is still open, I know the last comment by @fa-sharp says it was fixed but it was not, the function still only gets the state, this should really be fixed in the docs

[climba03003]

Hello, over a year later and this issue is still open, I know the last comment by @fa-sharp says it was fixed but it was not, the function still only gets the state, this should really be fixed in the docs

Please provide which version you are using. I can see it providing request as the document stated. https://github.com/fastify/fastify-oauth2/blob/130a0c804a6e0b79c08fd4e78e36582d2a15ab99/index.js#L26-L34 https://github.com/fastify/fastify-oauth2/blob/130a0c804a6e0b79c08fd4e78e36582d2a15ab99/index.js#L210-L226