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

Unable to control cookie expiry (or cookie refresh) on a per generation basis #46

Open
1Map opened this issue Dec 24, 2023 · 5 comments
Assignees
Labels
breaking breaking (major) change bug Something isn't working

Comments

@1Map
Copy link

1Map commented Dec 24, 2023

Ok, In your example you are showing a single route /

Where you fire a /csrf-token to generate a new token whenever the route is loaded. My question is. Do I need to do this on all routes where I have multiple pages?

What I do not understand is the following:

  1. A token is generated when the page is loaded and a signed cookie is sent back by the backend with an expiry date.
  2. What happens with ajax calls within the page? Do I need to also generate a new token, like when the page was loaded (/csrf-token)? Reason I ask this is: If the person open a page a cookie is created with an expiry date of now + some time. Now, when a user fires an ajax call within the page (Lets say he have a grid with some data and pages through it). Then I want my cookie to have a new expiry date whenever the user fires a successfull ajax request.
  3. Is the cookie in sync with other open tabs in the same browser (especially the expiry date)? And how do I keep that in sync?
  4. My last question is, what will happen if the cookie expired?

Excuse if it sounds stupid, but I am new to this. I just need to understand in more detail.

@psibean
Copy link
Contributor

psibean commented Dec 24, 2023

  1. A token is generated when the page is loaded and a signed cookie is sent back by the backend with an expiry date.

So this is necessary for th double submit cookie pattern, here's how csrf-csrf works:

  1. A token is generated whenever generateToken or req.csrfToken is called - this is entirely in your control.
  2. When the token is generated, a cookie is associated with the response as you've described, this cookie contains a hashed value of the token.
  3. The value returned from the generateToken or req.csrfToken call is the non-hashed token value, you should provide this to the frontend as part of a response (such as within a server side rendered template, or some other payload)
  4. Whenever your frontend makes a protected request it should include the value from step 3 in the request header, if you plan on putting it in the body (for example, with a hidden field form submit), then you need to make sure your retrieveTokenFromRequest method gets the token value from the body for form submits, and from the header for other requests.
  1. What happens with ajax calls within the page? Do I need to also generate a new token, like when the page was loaded (/csrf-token)?

You don't need to generate a new token, by default, calling generateToken (alias req.csrfToken) will return any existing token associated with the request. So you may need to request the existing token from each page unless you store the token in your application state.

  1. Is the cookie in sync with other open tabs in the same browser (especially the expiry date)? And how do I keep that in sync?

Typically this is a side effect of the double submit pattern. so csrf-csrf has been configured by default to use a token per session, so the answer to this question is yes, it will stay in sync. However, csrf-csrf also allows you to completely customize and control this with the overwrite parameter when calling generateToken, which will force the generation of a new token, invalidating the previous one.

For example, you could have a token-per-request setup, or even a token-per-route setup.

Excuse if it sounds stupid, but I am new to this. I just need to understand in more detail

The only stupid question is the one that is left unasked..

@1Map
Copy link
Author

1Map commented Dec 24, 2023

@psibean Thanks for your fast response. When you say the cookie stays in sync between browser tabs. If I refresh a page on tab 1 it gets the cookie back from the server with a new "Expire Date", but the expiry date does not get synced through browser tabs, causing where, for example a user are busy on an individual tab calling ajax calls the whole time will cause the cookie to basically never expires as long as the user is keeping it "alive". Now, the problem is, although this cookie is alive at the first tab, the other tab might have a cookie which already "expired". I want the session to be alive between all browser tabs, and, if a user session expired then all cookies should be destroyed between all tabs (So, basically sign the user out from all tabs).

@psibean
Copy link
Contributor

psibean commented Dec 24, 2023

When you say the cookie stays in sync between browser tabs. If I refresh a page on tab 1 it gets the cookie back from the server with a new "Expire Date",

So the intention is that you should be in control of the cookie expiry, but now I can see that's not quite the case because generateToken doesn't allow the passing of any cookie options. The cookie options are only passed in on initialization, and providing an expiry time then is kind of... useless.

It's obvious now, some of the cookie options should be exposed via generateToken - such that an expiry can be passed in on a per generation basis. When to refresh and re-use the expiry should also be considered for this too.

I think this is a good opportunity to make the additional options passed into generateToken an object and expose this a bit better in a major update with breaking changes.

Will need to consider a default behaviour which will either:

  1. re-use any existing cookie settings; or preferably
  2. don't touch the cookie unless a new token is being generated, in which case, only update the value (and nothing else)

@psibean psibean changed the title Multiple routes Unable to control cookie expiry (or prevent refresh) on a per generation basis Dec 24, 2023
@psibean psibean added bug Something isn't working breaking breaking (major) change labels Dec 24, 2023
@psibean psibean changed the title Unable to control cookie expiry (or prevent refresh) on a per generation basis Unable to control cookie expiry (or cookie refresh) on a per generation basis Dec 24, 2023
@psibean psibean self-assigned this Dec 30, 2023
@psibean
Copy link
Contributor

psibean commented Dec 30, 2023

I'll look at fixing this and releasing a new major version once I have. It needs a little bit of thought.

  • I'm thinking the default behavior should be that the xsrf cookie maintains an expiry/maxAge equivalent to the session cookie. But that we should also allow that to be overridden when calling generateToken.
  • Additionally, may need to consider allowing the expiry and such to be updated when a new token is generated and / or when an existing token is re-retrieved.

These two things might just work together naturally, but will see.

Tests will also need to be written to test the cookie expiry is updated / maintained in all the permutations of new cases.

@psibean
Copy link
Contributor

psibean commented Apr 6, 2024

A fix for this has been merged into master and will be released with the next major version 4.0.0, there's additional breaking changes to come before that release. This issue will remain open until it's released.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking breaking (major) change bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants