Re: [Django] #14881: [nonrel] Do not assume ``User.id`` to be an integer in django.contrib.auth's pasword reset feature (was: Do not assume ``User.id`` to be an integer in django.contrib.auth's pasword reset feature)

Django

unread,

Dec 3, 2011, 5:04:38 PM12/3/11

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+-----------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: SVN
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Someday/Maybe
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+-----------------------------------------
Changes (by jonash):

* keywords: => nonrel
* ui_ux: => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:10>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,

Dec 14, 2011, 4:58:05 AM12/14/11

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+-----------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: SVN
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Someday/Maybe
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+-----------------------------------------

Changes (by wkornewald):

* cc: wkornewald@… (removed)

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:11>

Django

unread,

Jan 8, 2012, 12:11:29 PM1/8/12

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+-----------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: SVN
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Someday/Maybe
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+-----------------------------------------

Comment (by anonymous):

Replying to [comment:6 jezdez]:
> I don't see a reason to support non-integer IDs in 1.3 since none of the
core ORM backends support that kind of thing.

People want to use Django with non-rel backends not in core today. That's
what this patch allows.

The patch seems straightforward, the only big issue I can see is that it
breaks already generated password resets. Is that an OK backwards
incompatibility tradeoff to get the possibility to do auth with non-rel
backends?

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:12>

Django

unread,

Dec 16, 2012, 8:53:47 AM12/16/12

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+------------------------------------

Reporter: jonash | Owner: nobody
Type: New feature | Status: new

Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by aaugustin):

* stage: Someday/Maybe => Accepted

Comment:

With the introduction of custom user models in 1.5, it's now possible to
have a User model with a non-integer primary key.

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:13>

Django

unread,

Feb 15, 2013, 9:22:16 PM2/15/13

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Comment (by Russell Keith-Magee <russell@…>):

In [changeset:"91c26eadc9b4efa5399ec0f6c84b56a3f8eb84f4"]:
{{{
#!CommitTicketReference repository=""
revision="91c26eadc9b4efa5399ec0f6c84b56a3f8eb84f4"
Refs #14881 -- Document that User models need to have an integer primary
key.

Thanks to Kaloian Minkov for the reminder about this undocumented
requirement.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:14>

Django

unread,

Feb 15, 2013, 9:22:38 PM2/15/13

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Comment (by Russell Keith-Magee <russell@…>):

In [changeset:"461d6e22957770449cd99367358c5e419bc3a86d"]:
{{{
#!CommitTicketReference repository=""
revision="461d6e22957770449cd99367358c5e419bc3a86d"
[1.5.x] Refs #14881 -- Document that User models need to have an integer
primary key.

Thanks to Kaloian Minkov for the reminder about this undocumented
requirement.

(cherry picked from commit 91c26eadc9b4efa5399ec0f6c84b56a3f8eb84f4)
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:15>

Django

unread,

Jun 24, 2013, 12:00:48 PM6/24/13

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by timo):

* cc: timograham@… (added)

Comment:

I've worked to get the current patch to apply cleanly and the tests
passing on Python 2 & 3.

We need a decision whether or not we are comfortable introducing this as a
backwards incompatible change or if we need to do something to allow
password reset URLs generated in prior versions of Django to continue to
work.

If this approach is accepted, I'll also update the documentation and look
into adding an additional test.

https://github.com/timgraham/django/commit/87b2613ec25e356aed4e9d82e620d386e7f7ae33

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:16>

Django

unread,

Jun 24, 2013, 1:57:00 PM6/24/13

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Comment (by claudep):

I think it wouldn't be too hard to support both forms, at least for 2-3
releases, as far as both regexes cannot overlap (to be confirmed). So
unless faced with a major obstacle, I'd vote for keeping the old form too
(only for the decoding part, of course).

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:17>

Django

unread,

Jun 24, 2013, 7:23:41 PM6/24/13

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Comment (by lukeplant):

In the past, for similar things where we need backwards compatibility with
short-lived data, rather than code or long-lived data, we've normally gone
for a deprecation process where we have one release which has transitional
support i.e. supports the old data, then we remove it.

https://docs.djangoproject.com/en/dev/releases/1.4/#compatibility-with-
old-signed-data

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:18>

Django

unread,

Jun 25, 2013, 9:06:30 AM6/25/13

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
------------------------------+------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:
Keywords: nonrel | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
------------------------------+------------------------------------

Changes (by timo):

* needs_better_patch: 1 => 0
* needs_tests: 1 => 0
* needs_docs: 1 => 0

Comment:

Thanks for the feedback. I've added documentation and a backwards
compatible shim: https://github.com/django/django/pull/1303

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:19>

Django

unread,

Jun 26, 2013, 5:28:31 AM6/26/13

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature

-------------------------------------+-------------------------------------

Reporter: jonash | Owner: nobody
Type: New feature | Status: new
Component: contrib.auth | Version: master
Severity: Normal | Resolution:

* stage: Accepted => Ready for checkin

Comment:

I've not run the tests, but looking at the patch, it looks good. Thanks!

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:20>

Django

unread,

Jun 26, 2013, 1:14:35 PM6/26/13

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
-------------------------------------+-------------------------------------
Reporter: jonash | Owner: nobody

Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed

Changes (by Tim Graham <timograham@…>):

* status: new => closed
* resolution: => fixed

Comment:

In [changeset:"1184d077893ff1bc947e45b00a4d565f3df81776"]:
{{{
#!CommitTicketReference repository=""
revision="1184d077893ff1bc947e45b00a4d565f3df81776"
Fixed #14881 -- Modified password reset to work with a non-integer
UserModel.pk.

uid is now base64 encoded in password reset URLs/views. A backwards
compatible
password_reset_confirm view/URL will allow password reset links generated
before
this change to continue to work. This view will be removed in Django 1.7.

Thanks jonash for the initial patch and claudep for the review.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:21>

Django

unread,

Jan 1, 2015, 11:40:10 AM1/1/15

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
-------------------------------------+-------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: nonrel | Triage Stage: Ready for

| checkin

Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"a7aaabfaf1fa4c20065ab1133d49f40d4de6b409"]:
{{{
#!CommitTicketReference repository=""
revision="a7aaabfaf1fa4c20065ab1133d49f40d4de6b409"
Removed doc note about PasswordResetForm requiring an integer PK.

This limitation was lifted in refs #14881.
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:22>

Django

unread,

Jan 1, 2015, 11:42:53 AM1/1/15

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
-------------------------------------+-------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: nonrel | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"e17d98ff0292caf02b62a3e84e10e3aaff3f4015"]:
{{{
#!CommitTicketReference repository=""
revision="e17d98ff0292caf02b62a3e84e10e3aaff3f4015"
[1.6.x] Removed doc note about PasswordResetForm requiring an integer PK.

This limitation was lifted in refs #14881.

Backport of a7aaabfaf1fa4c20065ab1133d49f40d4de6b409 from master
}}}

--
Ticket URL: <https://code.djangoproject.com/ticket/14881#comment:23>

Django

unread,

Jan 1, 2015, 11:42:58 AM1/1/15

to django-...@googlegroups.com

#14881: [nonrel] Do not assume ``User.id`` to be an integer in
django.contrib.auth's pasword reset feature
-------------------------------------+-------------------------------------
Reporter: jonash | Owner: nobody
Type: New feature | Status: closed
Component: contrib.auth | Version: master
Severity: Normal | Resolution: fixed
Keywords: nonrel | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by Tim Graham <timograham@…>):

In [changeset:"8e68b590abd22dc46e8aa3c963c32713755f6172"]:
{{{
#!CommitTicketReference repository=""
revision="8e68b590abd22dc46e8aa3c963c32713755f6172"
[1.7.x] Removed doc note about PasswordResetForm requiring an integer PK.