-
-
Notifications
You must be signed in to change notification settings - Fork 4.4k
WebUI: add session cookie expiration date based on session timeout #23350
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
base: master
Are you sure you want to change the base?
Conversation
What about this comment? #20993 (comment) |
My pull request allows me to stay logged in to the qBittorrent WebUI without the browser restoring my session. |
cookie.setHttpOnly(true); | ||
cookie.setSecure(m_isSecureCookieEnabled && isOriginTrustworthy()); // [rfc6265] 4.1.2.5. The Secure Attribute | ||
cookie.setPath(u"/"_s); | ||
cookie.setExpirationDate(QDateTime::currentDateTime().addSecs(m_sessionTimeout)); |
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.
This is wrong. In your PR, the cookie is always going to expire when the time is up.
However, we want the cookie to not expire as long as the user is still using the webapi/webui.
I would suggest something like this:
cookie.setExpirationDate(QDateTime::currentDateTime().addSecs(m_sessionTimeout)); | |
cookie.setExpirationDate(QDateTime::fromSecsSinceEpoch(std::numeric_limits<qint64>::max())); |
AFAIK, Chrome and Firefox clamps max time to about 1 year. But Safari will only clamp to 7 days.
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.
We have WebUI Session Timeout in seconds in m_sessionTimeout. Why do you want to use a different value?
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.
m_sessionTimeout
is a value for session timeout (as long as user keep using it, it won't time out). It is not a time limit for expiring a cookie. You don't need to insist using it for cookie expiration date.
You should look around and see how m_sessionTimeout
is actually used.
Maybe it would be better to replace the cookie with a new session expiration date each time the user accesses the WebUI? |
Yes, that is the basic idea. Chrome also suggests it: https://developer.chrome.com/blog/cookie-max-age-expires#extending_cookie_expiration |
Something like this? |
Close. I'm thinking something like #23392. |
Closes #20993
WebUI: add session cookie expiration date based on session timeout