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

[Jonas H.]

This is a request to cherry-pick a recently submitted patch to Django-nonrel that makes it possible to use django.contrib.auth with databases that don't use integer-based auto IDs.

The URL token has to be changed anyway when Django-nonrel gets merged into Django trunk in 1.4 (or whenever), so it might be more convenient to change this behaviour earlier.

The patch does not do any backwards compatibility handling because I think it's not worth the effort although it should be rather easy (simply a compatibility URL associated with a view that un-base36s the id and then calls the wrapped view with the base64d ID).

[Waldemar Kornewald]

The patch already contains changes to the auth unit tests. Isn't this enough? If not, please explain what is missing and please re-enable the "needs tests" checkbox.

[Łukasz Rekucki]

I was thinking, about something that tests that this works with a model with non-integer pk, but maybe I misunderstood the purpose of the ticket.

I also fail to see how changing to base64 is related to this issue. I don't know what was the original cause to use base36, but I think it must had been something more then desire to use '-' in URL.

[Waldemar Kornewald]

The base36 functions only work with integers. Since we have to deal with string here we just switched to base64.

The purpose of this ticket is indeed to allow for a string-based AutoField (note, I don't mean CharField(primary_key=True)), but this is practically impossible to test on a SQL database because AutoField is always an integer. At least, I don't know how to change the id field to a CharField at runtime on SQL (not with a reasonable amount of effort).

BTW, Jonas, you indeed still have to update the documentation. The current docs state that the reset views get a "uidb36" parameter which is base36-encoded.

[Łukasz Rekucki]

Replying to wkornewald:

The base36 functions only work with integers. Since we have to deal with string here we just switched to base64.

Right, I kept reading base32. Sorry for the noise.

[Jannis Leidel]

So there are multiple things pretty unclear to me here.

First, the assumption of merging the 3rd party fork "Django-nonrel" into trunk is wrong. There has never been an endorsement nor an announcement of this. Please refrain from giving this as a reason for a feature request.

Second and more important, 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. In other words, this has to wait till we have a proper story ourselves.

Marking accordingly.

[Chris Beaven]

I'm going to shunt this to 'someday/maybe'.

[Jonas H.]

new patch against current SVN

[anonymous]

Replying to 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?

[Aymeric Augustin]

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.

[Russell Keith-Magee <russell@…>]

In 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.

[Russell Keith-Magee <russell@…>]

In 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)

[Tim Graham]

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

[Claude Paroz]

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).

[Luke Plant]

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

[Tim Graham]

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

[Claude Paroz]

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

[Tim Graham <timograham@…>]

In 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.

[Tim Graham <timograham@…>]

In a7aaabfaf1fa4c20065ab1133d49f40d4de6b409:

Removed doc note about PasswordResetForm requiring an integer PK.

This limitation was lifted in refs #14881.

[Tim Graham <timograham@…>]

In 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

[Tim Graham <timograham@…>]

In 8e68b590abd22dc46e8aa3c963c32713755f6172:

[1.7.x] Removed doc note about PasswordResetForm requiring an integer PK.

This limitation was lifted in refs #14881.

Backport of a7aaabfaf1fa4c20065ab1133d49f40d4de6b409 from master