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: Simon Charette
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 3 years ago by gwahl@…

Needs documentation: unset
Needs tests: unset
Patch needs improvement: unset

comment:2 Changed 3 years ago by Claude Paroz

Component: Uncategorizedcontrib.admin
Triage Stage: UnreviewedAccepted
Type: UncategorizedCleanup/optimization

comment:3 Changed 3 years ago by Baptiste Mispelon

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 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 Baptiste Mispelon

Triage Stage: AcceptedReady for checkin

Looks good.

comment:6 Changed 3 years ago by Simon Charette

Owner: changed from nobody to Simon Charette
Status: newassigned
Version: 1.5master

I'll commit this.

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

Resolution: fixed
Status: assignedclosed

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 Tim Graham

Cc: timograham@… added
Needs documentation: set
Resolution: fixed
Severity: NormalRelease blocker
Status: closednew
Triage Stage: Ready for checkinAccepted

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 Tim Graham

Resolution: fixed
Severity: Release blockerNormal
Status: newclosed
Note: See TracTickets for help on using tickets.
Back to Top