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

Fixed transaction state not being set to expire in 30 minutes #835

Merged
merged 2 commits into from
Oct 6, 2018
Merged

Fixed transaction state not being set to expire in 30 minutes #835

merged 2 commits into from
Oct 6, 2018

Conversation

mrsvt
Copy link
Contributor

@mrsvt mrsvt commented Oct 5, 2018

Passed expires option does not override the default value.
I'd like to ask is it okay to store nonce for 30 mins? IMO, a very short time around 5 mins is enough.

Passed `expires` option does not override the default value.
I'd like to ask is it okay to store nonce for 30 mins? IMO, a very short time around 5 mins is enough.
Copy link
Contributor

@luisrudge luisrudge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sayuti-daniel wow. I'm not sure how I missed this one haha. Thanks for this PR! Can you write a test to make sure we never have this issue again?

@mrsvt
Copy link
Contributor Author

mrsvt commented Oct 6, 2018

This has been introduced since 9.5.0 😄 Btw, I'm curious to know why the cookie state is not removed instead of using expiration time. Do you mind explaining this? Thanks! 🍻

@luisrudge luisrudge merged commit 339c173 into auth0:master Oct 6, 2018
@luisrudge luisrudge changed the title fix(TransactionManager): Set state to expire in 30 mins Fixed transaction state not being set to expire in 30 minutes Oct 6, 2018
@luisrudge
Copy link
Contributor

Thanks so much for the PR 🎉

We remove the transaction state once the authentication is successful. But we can't remove if the authentication fails for any reason, because we don't get a state parameter back in that case (the state param is used to "find" the transaction state in the cookie).

@mrsvt mrsvt deleted the fix/transaction-manager/expires-option branch October 9, 2018 06:25
@mrsvt
Copy link
Contributor Author

mrsvt commented Oct 16, 2018

@luisrudge Can we have patch release for this?

@luisrudge
Copy link
Contributor

@sayuti-daniel we'll have a new release in 2 days :shipit:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants