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

What to do in case token expires #5

Closed
mebjas opened this issue Jun 5, 2014 · 17 comments
Closed

What to do in case token expires #5

mebjas opened this issue Jun 5, 2014 · 17 comments

Comments

@mebjas
Copy link
Owner

mebjas commented Jun 5, 2014

csrf token has an expiry time. let us assume its 5 minutes. And after loading a page, user did spend 5 minutes somewhere else before making another request.

Now when user makes a request, say he submits a form, the js code fetches the token from cookie before submitting the form. So what need to be done in this case?

Possible solutions:

  1. issue a quick xhr request to request for token refresh
  2. let the request proceed without a token and let ir fail ??? :octocat:
@mebjas
Copy link
Owner Author

mebjas commented Jun 28, 2014

In normal case, if token expires js will send a blank value as key. And validation will fail.

One possible case if we keep a configurable option (default: off)

expiredTokenRefresh: true
expiredTokenRefreshUrl: "https://server.com"

the js will issue a quick xhr to this server and wait for response, but in response header, refreshed cookie will be retrieved and js can then attach tokens with requests.

imp: http://server.com should have csrfprotector disabled for GET validation in this case

@mebjas
Copy link
Owner Author

mebjas commented Jul 11, 2014

#label -> next version, currently validation will fail

@chdeliens
Copy link

Isn't it insecure to have CSRFProtector disabled for the "get me a new token" XHR call?
Why not having a (expirationTime - 30") timer making an XHR call w/ the current CSRF token to get a new token? But then we'd have to handle "multiple valid tokens", in case the person has multiple tabs open to the same server... Just a thought!

@chdeliens
Copy link

"to get a new token" --> that would be done via cookie+session, not by returning the data directly. So that when the user actually wants to submit his form, the usual process is run (fetch token from cookie, attach to request, post).

@mebjas
Copy link
Owner Author

mebjas commented Jan 27, 2015

Well thats a good idea, to issue a xhr call to refresh the token few seconds before expiry. But then we need a page that handles such requests without doing any other processing. One of the way would be to issue a xhr to same page (currently open) with some special request parameter, such that backend just issue a token and terminates the connection.
And for multiple open tabs, the cookie is a global value, so once the xhr is complete and token has been retrieved, the cookie will be same for each tab.

The only doubt I have is, token expiry time is 30 minutes. Is it a wise idea to implement this?@chdeliens.

@chdeliens
Copy link

Indeed, that's what a thought of: issuing a XHR call to the same w/ special params. But then, we have to assume that the ::init() call is made soon enough to avoid any processing on the page. A cleaner integration would be to have a special page dedicated to this purpose of regenerating a token.

Regarding the token expiry time, I'm not sure I get it right. Are you afraid that the expiration time of 30 minutes is too high (and I would agree, I think 5 minutes, or even 1 minute would be more secure -- even though we can't possibly get that value because either in session or in domain-only cookie, but we never know what new attack would be possible tomorrow) or that it should be higher so that we don't need to implement to regenerating of the token (bad idea IMO)?

@mebjas
Copy link
Owner Author

mebjas commented Jan 28, 2015

Yeah thats a concern. But for this library to work properly its needed that ::init() is called before any other calls. So that the library can validate the request before any processing occur & set on output buffering.
and considering the idea to have a dedicated page dealing with token regeneration, we'd need to pass {the url information of that script} to javascript for which we'd need the absolute url of the script in configuration, coz the library can be placed anywhere within the main project directory. Which means another configuration, which needs to be altered for the library to work!

@abiusx
Copy link
Collaborator

abiusx commented Jan 28, 2015

I think the simplest and best working solution would be to have a longer expiration time. I think something in the order of a few hours is generally good.
As for multiple valid tokens, session based tokens only allow one tab to be active at a time.

@mebjas
Copy link
Owner Author

mebjas commented Jan 28, 2015

But should we not have a solution for that corner case too? lets say tab was open for hours and cookie expires?
Also, here its not purely session based we are validating cookies against session variables, so multiple tabs are possible, unless both request are triggered together but handled at different times.

@abiusx
Copy link
Collaborator

abiusx commented Jan 28, 2015

Well then there’s no problem. Any form is considered lost after several hours of inactivity. All program developers have to employ background ajax refreshment to keep those forms alive.

On Jan 28, 2015, at 1:58 PM, minhaz notifications@github.com wrote:

But should we not have a solution for that corner case too? lets say tab was open for hours and cookie expires?
Also, here its not purely session based we are validating cookies against session variables, so multiple tabs are possible, unless both request are triggered together but handled at different times.


Reply to this email directly or view it on GitHub #5 (comment).

@mebjas
Copy link
Owner Author

mebjas commented Jan 28, 2015

Ok so we just leave it to developer?

@abiusx
Copy link
Collaborator

abiusx commented Jan 28, 2015

yes. to add a lot of code and complexity for a very rare corner case would ruin the library, not help it. You can create a cookbook for the developers to use in these cases, but adding that to the main codebase is not very wise.

On Jan 28, 2015, at 2:00 PM, minhaz notifications@github.com wrote:

Ok so we just leave it to developer?


Reply to this email directly or view it on GitHub #5 (comment).

@mebjas
Copy link
Owner Author

mebjas commented Jan 28, 2015

totally agree!

@mebjas
Copy link
Owner Author

mebjas commented Feb 6, 2015

#todo: Create a wiki page, mentioning this case and suggesting possible actions to developers!

@mithilesh12
Copy link

Is it safe to set expire time - At end of session?
By replacing the code
setcookie(self::$config['CSRFP_TOKEN'], $token, time() + self::$cookieExpiryTime);
To
setcookie(self::$config['CSRFP_TOKEN'], $token); #Expires: At end of session

@mebjas
Copy link
Owner Author

mebjas commented Sep 30, 2015

@mithilesh12 yeah

@mebjas mebjas added this to the v0.2.0 milestone Sep 19, 2016
@mebjas
Copy link
Owner Author

mebjas commented Oct 11, 2016

@mebjas mebjas closed this as completed Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants