-
Notifications
You must be signed in to change notification settings - Fork 20
[V3] CookieFrom request and TokenFromRequest doesn't have the same form #88
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
Comments
For version 3, yes, this is intended. The cookie value, based on version 3, has a preference to be There's been a variety of discussion over the benefits of using Additionally there are always confusions on this topic even from OWASP maintainers themselves - where here they recommend it.. If you're running an SPA, even Okta (a reputable security source) recommend using
The implementation has changed greatly for the next version (v4, not yet released), where a hmac and a nonce are used to form the CSRF token. This way the cookie value can be used directly, the hmac can be reconstructed by using the nonce and the users session id. For v4 |
Thank you for your detailed response! I'm currently working on rebuilding an api for a SPA from laravel to nest.js. I like this solution ↓ (It means I need to update the SPA also, but thats not a big deal)
|
Do make sure you provide the Otherwise anyone could request a CSRF token and it would be valid for any other user. Note that, despite the name, |
Hello,
It took me a lot of time to debug why my implementation was failing.
I've found that in the validateRequest function, the cookie is being split into token and hash, but the header content its not. I don't know if this is intentional or should I send the header without the hash from the front.
https://github.com/Psifi-Solutions/csrf-csrf/blob/v3.x.x/src/index.ts#L160
I've fixed with this configuration:
But using
String(request.headers['x-xsrf-token']).split('|')[0];
doesn't feel right.Is this intentional or am I missing something?
The text was updated successfully, but these errors were encountered: