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

Reuse session csrf token #28

Merged
merged 11 commits into from
Aug 16, 2023
Merged

Reuse session csrf token #28

merged 11 commits into from
Aug 16, 2023

Conversation

davidgonmar
Copy link
Collaborator

Reuse token from existing session cookie.
This enables multitab support by not overwriting existing csrf cookies.
Functionality can be toggled with the param 'overwrite', which defaults to true (reusability of tokens is disabled by default).

This allows multiple tabs to be open at the same time, without
compromising security.

Now, the cookie includes both a hash and the token, separated by "|".
This allows the token generator to extract the token from an existing
csrf session cookie and reuse it.
@psibean psibean self-requested a review August 15, 2023 01:49
@psibean psibean added enhancement New feature or request feature labels Aug 15, 2023
README.md Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@psibean
Copy link
Contributor

psibean commented Aug 16, 2023

So I've just had some additional thoughts:

  • Now that the plain token is also included in the cookie value, we should set the signed cookie option to true - similar to the httpOnly flag, this should be forced and passed as true directly in the inner call, so we should omit the signed option from the prop type.
  • We can then delete the signed/unsigned test case, only the default one of those matters as signed is always true.
  • Documentation will need to explicitly state that a different secret must be provided to cookie parser - we should update the existing examples to reflect this usage.

@psibean
Copy link
Contributor

psibean commented Aug 16, 2023

I've pulled down your branch and made some changes on the branch: https://github.com/Psifi-Solutions/csrf-csrf/tree/reuse-session-csrf-token

You may be able to cherry-pick the commit: b24f941

My plan is for this to be a major update given the changes, thus I am also setting the default of overwrite to false - this way it mimics the csrf-sync behaviour.

Additionally, in another PR after this one, I'll make another breaking change which will finally switch the order of response and request params for generateToken 😂

@psibean psibean marked this pull request as ready for review August 16, 2023 23:28
@psibean psibean merged commit 2f1f8cd into Psifi-Solutions:main Aug 16, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants