Opened 3 years ago

Closed 9 months ago

Last modified 4 weeks 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: master
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 (14)

comment:1 Changed 3 years ago by Simon Charette

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 Changed 3 years ago by Zach Liu

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

comment:3 Changed 3 years ago by Zach Liu

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?

Also currently token is made of days, so after change it should be made of seconds I think.

Last edited 3 years ago by Zach Liu (previous) (diff)

comment:4 Changed 3 years ago by 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.

comment:5 Changed 3 years ago by Tim Graham

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.

comment:6 in reply to:  4 Changed 3 years ago by Zach Liu

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 Changed 2 years ago by Tim Graham

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 Changed 22 months ago by Nahuel

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 Changed 9 months ago by Hasan Ramezani

Needs documentation: unset
Owner: changed from Zach Liu to Hasan Ramezani
Last edited 9 months ago by felixxm (previous) (diff)

comment:11 Changed 9 months ago by felixxm

Patch needs improvement: set

comment:12 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

Resolution: fixed
Status: assignedclosed

In 226ebb1:

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

comment:13 Changed 9 months ago by Mariusz Felisiak <felisiak.mariusz@…>

In 45304e44:

Refs #28622 -- Clarified security implications of PASSWORD_RESET_TIMEOUT.

comment:14 Changed 4 weeks ago by Mariusz Felisiak <felisiak.mariusz@…>

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.

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