- Notifications
You must be signed in to change notification settings - Fork5.5k
[FIX] notebookapp, auth:get_secure_cookie kwargs#3778
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
[FIX] notebookapp, auth:get_secure_cookie kwargs#3778
Uh oh!
There was an error while loading.Please reload this page.
Conversation
minrk commentedJul 18, 2018
|
Per Tornado's documentation:>By default, Tornado’s secure cookies expire after 30 days.>To change this, use the expires_days keyword argument to>set_secure_cookie and the max_age_days argument to get_secure_cookie.>These two values are passed separately so that you may>e.g. have a cookie that is valid for 30 days for most purposes,>but for certain sensitive actions>(such as changing billing information)>you use a smaller max_age_days when reading the cookie.With the current implementation in `auth/login.py`,this is possible to pass the `expires_days` optionbut not possible to enforce it as this is not possibleto pass `max_age_days` to `get_secure_cookie`This makes impossible to set the cookie expiration withoutusing a custom `LoginHandler`.This revision is about adding the possibility to pass optionsto Tornado's `get_secure_cookie` method,so it can be possible to set the cookies expiration,among others.
beledouxdenis commentedJul 19, 2018
Thank you for your review :) I updated the PR. I chose the That said, for my own purposes, I am indeed only interested to the |
minrk commentedJul 30, 2018
Thanks! |
beledouxdenis commentedJul 30, 2018
Thank you for having considered the PR : ) |
Per Tornado's documentation:
With the current implementation in
auth/login.py,this is possible to pass the
expires_daysoptionbut not possible to enforce it as this is not possible
to pass
max_age_daystoget_secure_cookieThis makes impossible to set the cookie expiration without
using a custom
LoginHandler.This revision is about adding the possibility to pass options
to Tornado's
get_secure_cookiemethod,so it can be possible to set the cookies expiration,
among others.
In my opinion,
get_cookie_optionsis a weird naming given the options to pass toset_secure_cookieare calledcookie_options. That said, I am not sure what is your policy regarding the retro-compatibility of the settings, ifcookie_optionscan be renamedset_cookie_optionsor not. Anyway we first have to discuss the feasibility of this change.