fastify / fastify-jwt

JWT utils for Fastify
MIT License
512 stars 99 forks source link

"authenticate" example creates a new verifier instance and a new cache instance for each request #334

Open NikitaIT opened 7 months ago

NikitaIT commented 7 months ago

Prerequisites

Fastify version

4.26.1

Plugin version

8.0.0

Node.js version

20

Operating system

macOS

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

m1

Description

An example from the documentation looks like this:

fastify.decorate("authenticate", async function(request, reply) {
    try {
      await request.jwtVerify()
    } catch (err) {
      reply.send(err)
    }
  })

Steps to Reproduce

But this example creates a new verifier instance and a new cache instance for each request:

  1. request.jwtVerify(next=undefined)
  2. function requestVerify (options, next=undefined)
  3. options = {}
  4. if (next === undefined) {
  5. request[jwtVerifyName](options, function (err, val) {
  6. function requestVerify (options={...}, next=function)
  7. function verify (secretOrPublicKey, callback)
  8. if (useLocalVerifier) { and useLocalVerifier is true
  9. const localVerifier = createVerifier(verifierOptions)
  10. cache: createCache(cacheSize), src/verifier.js#L518

So, this config should call key: fn once, but with request.jwtVerify() it's no-cache policy.

{
      secret: {
        public: async (_1, _2, callback) => {
          console.log("on request: false");
          return false;
         }
      },
      // namespace: "-",
      verify: {
        algorithms: ["ES512"],
        cache: true,
        maxAge: "2d",
        cacheTTL: 100000000000,
        key: ((x: unknown, cb: (err: unknown, currentKey: unknown) => void) => {
          console.log("fetching public key", "should be cached and called once");
          return Promise.resolve("external kms key");
        }) as unknown as string,
      },
    }

Expected Behavior

So, correct usage is:

server.decorate("authenticate", async (request, reply) => {
      try {
        await new Promise((resolve, reject) => {
              request.jwtVerify((err, payload) => {
                if (err) {
                  reject(err);
                  return;
                }
                resolve(payload);
              });
        });
      } catch (err) {
        await reply.send(err);
      }
    });

Maybe this is how it should work, but I spent a lot of time trying to figure out why the cache doesn't work.

I used a test with namespaces from your library. And when using asynchronous key this test fails.

If key is async then

  1. verifier(token) => Promise<result>
  2. const user = formatUser ? formatUser(result) : result // Promise<result>
  3. request[decoratorName] = user // Promise<result>

And key should be async because it shouldn't be called under TTL.

For example, I use AWS KMS, and the library should not do kms.fetchPublicKey() for every incoming request. I can of course use my own cache for public keys, but this is supported out of the box in fast-jwt.

NikitaIT commented 7 months ago

And if key is async and signature is invalid then

{"statusCode":500,"code":"FAST_JWT_INVALID_SIGNATURE","error":"Internal Server Error","message":"The token signature is invalid."}

instead of 401.

Same with key as secret.public

public: async (_1, _2, callback) => callback(null, (_3, cb) => Promise.resolve("my public key").then(pk => cb(null,pk)))