-
Notifications
You must be signed in to change notification settings - Fork 468
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
AO3-5171 Make session last 2 weeks #3059
Conversation
…e user_credentials cookie
…d all this extra code
Looks reasonable, in a perfect world the 2 weeks and 3 months would be configurables. |
…ng users to log out if they are using a public or shared computer
config/config.yml
Outdated
@@ -11,6 +11,11 @@ | |||
SESSION_KEY: '_otwarchive_session' | |||
SESSION_SECRET: '898f6d0363863ec79d782238cd1c5767636d712cc0d138238bcd5bfc9d2672fb852380050e52c03a0401175d909c09dba48512a119d46b126a84c2dd05716eb5' | |||
|
|||
# In weeks | |||
DEFAULT_SESSION_LENGTH: 2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doing 2.weeks and 3.months here makes things explode because they're treated as a string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think it is right to have the numbers in different units, we could use 13 weeks ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I thought about that, but I figured as long as the config file was clear about the units, it would be less error-prone than needing to do the math on how many weeks are in a month. (Three months would be 12 weeks, wouldn't it?)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My other thought was we could include the unit in the name, but that makes it somewhat unwieldy (e.g. DEFAULT_SESSION_LENGTH_IN_WEEKS
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am happy with unwieldy names that reduce confusion.
Issue
https://otwarchive.atlassian.net/browse/AO3-5171
Purpose
Extends the lifespan of our session cookies to two weeks to mitigate the whole "trying to submit a form, getting logged out" issue
Testing
Will be added to JIRA