Opened 11 years ago

Closed 11 years ago

#20532 closed Cleanup/optimization (fixed)

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

Reported by: gwahl@… Owned by: Simon Charette
Component: contrib.admin Version: dev
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:2 by Claude Paroz, 11 years ago

Component: Uncategorized → contrib.admin
Triage Stage: Unreviewed → Accepted
Type: Uncategorized → Cleanup/optimization

comment:3 by Baptiste Mispelon, 11 years ago

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 by gwahl@…, 11 years ago

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 by Baptiste Mispelon, 11 years ago

Triage Stage: Accepted → Ready for checkin

Looks good.

comment:6 by Simon Charette, 11 years ago

Owner: changed from nobody to Simon Charette
Status: new → assigned
Version: 1.5 → master

I'll commit this.

comment:7 by Simon Charette <charette.s@…>, 11 years ago

Resolution: → fixed
Status: assigned → 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 by Tim Graham, 11 years ago

Cc: timograham@… added
Needs documentation: set
Resolution: fixed
Severity: Normal → Release blocker
Status: closed → new
Triage Stage: Ready for checkin → 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 by Tim Graham <timograham@…>, 11 years ago

In 70d7e45eb0888f979775c997935bfee84c85d189:

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

Refs #20532

comment:10 by Tim Graham, 11 years ago

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