Opened 3 years ago

Closed 3 years ago

#20532 closed Cleanup/optimization (fixed)

Password reset email template should reverse view by name, not by path

Reported by: gwahl@… Owned by: charettes
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: bmispelon@…, timograham@… Triage Stage: Accepted
Has patch: yes Needs documentation: yes
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no


password_reset_email.html reverses a url for django.contrib.auth.views.password_reset_confirm. This fails when you have substituted your own password_reset_confirm view, despite naming it correctly in your urlconf.

This is potentially backwards-incompatible if someone is using their own urlconf for the password reset views, but neglected to name them. However, these views are documented as being named:

Change History (10)

comment:1 Changed 3 years ago by gwahl@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 3 years ago by claudep

  • Component changed from Uncategorized to contrib.admin
  • Triage Stage changed from Unreviewed to Accepted
  • Type changed from Uncategorized to Cleanup/optimization

comment:3 Changed 3 years ago by bmispelon

  • Cc bmispelon@… added
  • Patch needs improvement set

FYI, this is an issue that I came upon while working on #17209.

If I remember well, there are several places in django that still reverse auth views this way (which breaks when all you have is class-based views).

I'm marking this as patch needs improvement because I think we should fix all these reverse issues at once.

I'll push the latest version of my branch for #17209 later today and maybe we can backport the fix from there.


comment:4 Changed 3 years ago by gwahl@…

  • Patch needs improvement unset

It would be nice to have this fixed in 1.6, so I think it can be fixed independently of #17209. I updated the patch to fix all occurences.

comment:5 Changed 3 years ago by bmispelon

  • Triage Stage changed from Accepted to Ready for checkin

Looks good.

comment:6 Changed 3 years ago by charettes

  • Owner changed from nobody to charettes
  • Status changed from new to assigned
  • Version changed from 1.5 to master

I'll commit this.

comment:7 Changed 3 years ago by Simon Charette <charette.s@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In 4f4e9243e4cf585e32a882804084853108ef94c0:

Fixed #20532 -- Reverse auth views by name, not by path.

Auth views should be reversed by name, not their locations in
django.contrib.auth.views. This allows substituting your own
implementations of the auth views.

comment:8 Changed 3 years ago by timo

  • Cc timograham@… added
  • Needs documentation set
  • Resolution fixed deleted
  • Severity changed from Normal to Release blocker
  • Status changed from closed to new
  • Triage Stage changed from Ready for checkin to Accepted

Our example for adding a password reset feature in the admin uses an unnamed pattern for a couple of these views. At a minimum this example needs to be updated. I think this change also deserves a mention in the release notes.

comment:9 Changed 3 years ago by Tim Graham <timograham@…>

In 70d7e45eb0888f979775c997935bfee84c85d189:

Added release notes for auth views being reversed by name, not by path.

Refs #20532

comment:10 Changed 3 years ago by timo

  • Resolution set to fixed
  • Severity changed from Release blocker to Normal
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.
Back to Top