#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

Description

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: https://docs.djangoproject.com/en/1.5/topics/auth/default/#django.contrib.auth.views.password_reset_confirm

Change History (10)

comment:1 Changed 22 months ago by gwahl@…

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

comment:2 Changed 22 months 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 22 months 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.

Thanks.

comment:4 Changed 22 months 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 22 months ago by bmispelon

  • Triage Stage changed from Accepted to Ready for checkin

Looks good.

comment:6 Changed 22 months 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 22 months 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 22 months 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 22 months 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 22 months 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