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

Allow configuration of how long the user has to complete IdP login, rather than hard coded 5 minutes #318

Closed
richjyoung opened this issue Sep 29, 2020 · 2 comments · Fixed by #431

Comments

@richjyoung
Copy link

richjyoung commented Sep 29, 2020

sessstore.Options.MaxAge = 300 // give the user five minutes to log in at the IdP

I am trying to diagnose issues with a 400 Bad Request error, with a log message like this (actual session ID removed):

/auth Invalid session state: stored %!s(<nil>), returned ******

I can only produce a 400 Bad Request error manually if the VouchSession cookie is not sent to the /auth callback.

At present I don't think Vouch is to blame, but my KeyCloak instance allows a configurable duration for the user to complete login, and I feel for better compatibility Vouch should probably do the same rather than a fixed 5 minutes.

Could this be added to the vouch.session section of the config? Perhaps vouch.session.maxAge?

@richjyoung
Copy link
Author

I see this was introduced in #270.

I've been seeing Bad Request errors since upgrading from 0.11.3 to 0.17.3. It's not clear why users are taking longer than 5 minutes to login, perhaps because they have 2FA to deal with as well, but at the moment I need to be able to set this to be much longer to eliminate it as a possibility. I suspect this means the number of times the user is allowed to login within a given period would need to be configurable as well based on the original issue that prompted this change.

@bnfinet
Copy link
Member

bnfinet commented Oct 2, 2020

thanks @richjyoung. That makes sense.

PR welcome!

It shouldn't be too hard to add. Please set .defaults.yml to 300 and add a clear description to config/config.yml_example for vouch.session.maxAge. And please test with both an env var as well as the config entry.

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

Successfully merging a pull request may close this issue.

2 participants