Morgbn / nuxt-csurf

Nuxt Cross-Site Request Forgery (CSRF) Prevention
https://nuxt-csurf.vercel.app
MIT License
81 stars 16 forks source link

fix: add csrf token to event context #22

Closed outofthisworld closed 11 months ago

outofthisworld commented 11 months ago
  1. Add nitro request hook.
  2. Add CSRF token to event context, making it accessible in pages.

see issue #20

Morgbn commented 11 months ago

Hello @outofthisworld, thanks for the pull request, Not sure if it's a good idea to set a cookie at each request. I need to explore this further. In the meantime, I'm not sure what the point is. Can't you create a minimal reproducible example that demonstrates the problem this PR solves ?

outofthisworld commented 11 months ago

Hello @outofthisworld, thanks for the pull request, Not sure if it's a good idea to set a cookie at each request. I need to explore this further. In the meantime, I'm not sure what the point is. Can't you create a minimal reproducible example that demonstrates the problem this PR solves ?

Hi @Morgbn,

This doesn't set a cookie on each request. It functions in the exact same way that the original code works however makes the generated csrf token available server side within a page by attaching it to event.context (recommended by nuxt/nitro). This means that any SSR requests that need to include a CSRF token can indeed access the generated token.

You can see below that cookie is only ever set if it is not already set:

 if (!secret) {
      secret = csrf.randomSecret()
      setCookie(event, cookieKey, secret, csrfConfig.cookie)
}

It addition "request" is absolutely no different from "render:html" which is also called every time the nitro server is hit.

If you would like the reasoning check issue #20

I'm not sure a minimal reproducible example is required, just hit any CSRF protected endpoint using $fetch on the server and check the page nuxt data. There will be an error.

Morgbn commented 11 months ago

"request" is hit at all request (get, post, ...) while "render:html" only when nitro render an html page... But you're right, if we want useCsrfFetch(..., { method: 'post'|'put'|'patch' }) to be called on server side we need to add the csrfToken in event.context. I will add an opt-in option for that specific case 👍

outofthisworld commented 11 months ago

"request" is hit at all request (get, post, ...) while "render:html" only when nitro render an html page... But you're right, if we want useCsrfFetch(..., { method: 'post'|'put'|'patch' }) to be called on server side we need to add the csrfToken in event.context. I will add an opt-in option for that specific case 👍

Well, I'm sure its possible to check the request method is 'GET' in the request hook. In addition, we can probably check its a page route.

Regardless apart from the slight extra computation to produce the csrf token from the secret that's stored in the cookie - it doesn't really make much difference and this would still function correctly.

Sounds good! Looking forward to seeing it and feel free to tag onto this MR. I do think it'll be a useful addition.

github-actions[bot] commented 11 months ago

:tada: This issue has been resolved in version 1.4.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

Morgbn commented 11 months ago

Thank you @outofthisworld, it was easier for me to just write a commit (with the option directly and useCsrf updated), but I appreciate your contribution !