Opened 7 years ago

Closed 5 years ago

Last modified 4 years ago

#28622 closed Cleanup/optimization (fixed)

Allow password reset token to expire in under a day

Reported by: Nijamudeen Owned by: Hasan Ramezani
Component: contrib.auth Version: dev
Severity: Normal Keywords:
Cc: Zach Liu Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

Currently, the settings only has PASSWORD_RESET_TIMEOUT_DAYS for expiring token, which means it is impossible to set the timeout to hours/minutes.

In many applications a day is far too long and doesn't meet security requirements

It should be possible to configure it using a timedelta for arbitrary expires

Change History (15)

comment:1 by Simon Charette, 7 years ago

Triage Stage: UnreviewedAccepted

Not sure if we should allow non-integer values to be passed to PASSWORD_RESET_TIMEOUT_DAYS or deprecate the setting for a new one which is seconds based (PASSWORD_RESET_TIMEOUT) but this requests make a lot of sense.

comment:2 by Zach Liu, 7 years ago

Cc: Zach Liu added
Needs documentation: set
Owner: changed from nobody to Zach Liu
Status: newassigned

comment:3 by Zach Liu, 7 years ago

I think using PASSWORD_RESET_TIMEOUT which takes seconds makes better sense. To support backward compatibility, I think we should keep PASSWORD_RESET_TIMEOUT_DAYS and its default value of 3. Only use PASSWORD_RESET_TIMEOUT when provided. Does it sound like a good idea?

Version 0, edited 7 years ago by Zach Liu (next)

comment:4 by Simon Charette, 7 years ago

Supporting PASSWORD_RESET_TIMEOUT_DAYS during the deprecation period makes sense but it should eventually be supersed by PASSWORD_RESET_TIMEOUT. I suggest you take a look at how the MIDDLEWARE_CLASSES to MIDDLEWARE transition was handled to figure out how this should be done.

comment:5 by Tim Graham, 7 years ago

Before coding, please get a consensus on how to proceed on the DevelopersMailingList. I like readability benefits of the timedelta proposal (compared to interpreting a number of seconds in a settings file, even if that could be somewhat mitigated by writing PASSWORD_RESET_TIMEOUT = 60 * 60 * 24 * 3) but I'm unsure on the best approach.

in reply to:  4 comment:6 by Zach Liu, 7 years ago

Replying to Simon Charette:

Supporting PASSWORD_RESET_TIMEOUT_DAYS during the deprecation period makes sense but it should eventually be supersed by PASSWORD_RESET_TIMEOUT. I suggest you take a look at how the MIDDLEWARE_CLASSES to MIDDLEWARE transition was handled to figure out how this should be done.

Hi Simon, you mentioned MIDDLEWARE_CLASSES to MIDDLEWARE transition, can you point me to where I can find the reference? Thanks.

comment:8 by Tim Graham, 7 years ago

Component: Core (Other)contrib.auth
Has patch: set
Type: New featureCleanup/optimization

Please uncheck "Needs documentation" if it's added to the patch.

django-developers discussion

comment:9 by Nahuel, 6 years ago

Hello,

About the token generation, is there any reason that is only attached to resseting password feature ?
Shouldn't be usefull in many other cases?

I mean, this can be helpful in many other cases that need temporary auto-login through the URL.

comment:10 by Hasan Ramezani, 5 years ago

Needs documentation: unset
Owner: changed from Zach Liu to Hasan Ramezani
Last edited 5 years ago by Mariusz Felisiak (previous) (diff)

comment:11 by Mariusz Felisiak, 5 years ago

Patch needs improvement: set

comment:12 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

Resolution: fixed
Status: assignedclosed

In 226ebb1:

Fixed #28622 -- Allowed specifying password reset link expiration in seconds and deprecated PASSWORD_RESET_TIMEOUT_DAYS.

comment:13 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In 45304e44:

Refs #28622 -- Clarified security implications of PASSWORD_RESET_TIMEOUT.

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 5 years ago

In d2b9a9fd:

Refs #28622 -- Corrected PASSWORD_RESET_TIMEOUT/PASSWORD_RESET_TIMEOUT_DAYS docs.

Removed outdated note about an extra day in PASSWORD_RESET_TIMEOUT
docs and incorrect "minimum" phrase.

comment:15 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

In 12ac4916:

Refs #28622 -- Removed settings.PASSWORD_RESET_TIMEOUT_DAYS per deprecation timeline.

Note: See TracTickets for help on using tickets.
Back to Top