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

Bug with sessionPlugin in NestJS with Fastify #135

Open
2 tasks done
DevDJpl opened this issue May 4, 2023 · 7 comments
Open
2 tasks done

Bug with sessionPlugin in NestJS with Fastify #135

DevDJpl opened this issue May 4, 2023 · 7 comments
Labels
good first issue Good for newcomers

Comments

@DevDJpl
Copy link

DevDJpl commented May 4, 2023

Prerequisites

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

Fastify version

4.17.0

Plugin version

6.3.0

Node.js version

20.0.0

Operating system

Windows

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

10

Description

The documentation says:

// if you want to sign cookies:
fastify.register(require('@fastify/cookie'), { secret }) // See following section to ensure security
fastify.register(require('@fastify/csrf-protection'), { cookieOpts: { signed: true } })

And when in NestJS 9.4.0 it declares csrf-protection in accordance with the documentation, code below in section "Steps to Reproduce".

Error:

error TS2322: Type '"@fastify/cookie"' is not assignable to type '"@fastify/secure-session"'.
sessionPlugin: '@fastify/cookie', 
error TS2345: Argument of type '{ cookieKey: string; cookieOpts: { httpOnly: true; sameSite: "strict"; path: string; secure: true; signed: false; }; }' is not assignable to parameter of type 'FastifyRegisterOptions<FastifyCsrfProtectionOptions>'.
  Type '{ cookieKey: string; cookieOpts: { httpOnly: true; sameSite: "strict"; path: string; secure: true; signed: false; }; }' is not assignable to type 'RegisterOptions & FastifyCsrfProtectionOptionsBase & 
FastifyCsrfProtectionOptionsFastifySecureSession'.
    Property 'sessionPlugin' is missing in type '{ cookieKey: string; cookieOpts: { httpOnly: true; sameSite: "strict"; path: string; secure: true; signed: false; }; }' but required in type 'FastifyCsrfProtectionOptionsFastifySecureSession'.
await app.register(fastifyCsrf, {
cookieKey: 'csrf-token',
},
});
  node_modules/@fastify/csrf-protection/types/index.d.ts:49:5
sessionPlugin: '@fastify/secure-session';
    'sessionPlugin' is declared here.

Even adding session Plugin and setting the value to '@fastify/cookie' gives an error and giving the value undefined shows that 1 of 3 types must be selected, e.g. @fastify/cookie or @fastify/session. So if it wasn't for the help on Stack, I would still think that I'm doing something wrong and here it turns out that it's a bug in your version, so I was forced to use version 6.2.0 and I would prefer the latest one.

If you need the code of my application, I will add it to the repo so that you can check for yourself that this bug exists :)

Steps to Reproduce

  // XCSRF - Protection
  app.register(fastifyCookie, { secret: 'ddddd' });
  app.register(fastifyCsrf, {
    sessionPlugin: '@fastify/cookie',
    cookieKey: 'csrf-token',
    cookieOpts: {
      httpOnly: true,
      sameSite: 'strict',
      path: '/',
      secure: true,
      signed: false,
    },
  });

Expected Behavior

I expected it to work according to the documentation and as it should after initialization

@jmcdo29
Copy link

jmcdo29 commented May 4, 2023

To be clear, this isn't related to NestJS directly. Only to the Typescript types. A minimum reproduction repository without NestJS can be found in my comment here

@climba03003 climba03003 added the good first issue Good for newcomers label May 4, 2023
@climba03003
Copy link
Member

Types is correct. Only document need to be updated.

@mcollina
Copy link
Member

mcollina commented May 4, 2023

@climba03003 what's the fix? The example looks good to me.

@climba03003
Copy link
Member

climba03003 commented May 4, 2023

what's the fix?

csrf-protection/index.js

Lines 46 to 48 in 0b04635

if (sessionPlugin === '@fastify/cookie' && csrfOpts.userInfo) {
assert(csrfOpts.hmacKey, 'csrfOpts.hmacKey is required')
}

csrfOpts.hmacKey is required for @fastify/cookie and the types reflect that you are missing csrfOpts property.
So, it goes to the branch @fastify/secure-session or @fastify/session

Example should be updated for @fastify/cookie to provide csrfOpts.hmacKey

Edit: Missing the csrfOpts.userInfo assertion. The types need to be extended to four situation.

@jmcdo29
Copy link

jmcdo29 commented May 4, 2023

Sure enough, it's kind of hidden in this line as well:

csrfOpts: Omit<CSRFOptions, 'hmacKey'> & Required<Pick<CSRFOptions, 'hmacKey'>>;

I can confirm that adding csrfOpts.hmacKey does remove the type error.

@mcollina
Copy link
Member

mcollina commented May 6, 2023

Would you like to send a Pull Request to address this issue? Remember to add unit tests.

@ruhankhandakar
Copy link

any update on this ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

5 participants