Opened 8 years ago
Last modified 2 years ago
#27775 assigned Bug
Signed cookies does not support custom expiry
Reported by: | Andreas Pelme | Owned by: | Dawid Bugajewski |
---|---|---|---|
Component: | contrib.sessions | Version: | 1.10 |
Severity: | Normal | Keywords: | |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | yes |
Needs tests: | yes | Patch needs improvement: | yes |
Easy pickings: | no | UI/UX: | no |
Description
Calling set_expiry() when using the signed cookies backend does not do anything. This has been known for quite some time, see:
- https://github.com/django/django/blob/5890d6ab03ebc7dac46ce7d9540b5768785caa34/django/contrib/sessions/backends/signed_cookies.py#L18-L19
- Ticket #19201
Ticket #19201 already exists that goes into details about problems with session expiration. This ticket exists to track the particular bug that custom expiration does not work in the signed cookies backend.
I propose that we should either (and I would be happy to work to get PR:s for):
- Raise an explicit exception when
set_expiry()
is called on signed cookie session backend. Currently the call is just ignored which may lead to security issues if the default configured session timeout is very high and some sensitive login/session need like to have a much lower expiration.
- Handle expiration in a signed cookies-specific way (see my PR at https://github.com/django/django/pull/7885 for an attempt at this).
I very much agree with the conclusion in #19201 that expiration is messy across backends and is in need of refactor but I think the current state is even worse where we have silent failures for potentially secret sensitive code.
I don't have the time to work on a full refactor of the expiration (given that it needs to be very backward compatible it is probably a bit of work involved).
Change History (5)
comment:1 by , 8 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 8 years ago
Patch needs improvement: | set |
---|
comment:3 by , 2 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:5 by , 2 years ago
Needs documentation: | set |
---|---|
Needs tests: | set |
Patch needs improvement: | set |
PR