UID for password reset incorrectly uses base64

[gsheni]

When a password reset link is request, the UID is encoded into the url as such, by django allauth

/accounts/password/reset/key/7w-1x7-d1a11z1sa1s1a1s/

Such that the uid is 7w and the token is 1x7-d1a11z1sa1s1a1s

The UID is encoded here, as base36:
https://github.com/pennersr/django-allauth/blob/330bf899dd77046fd0510221f3c12e69eb2bc64d/allauth/account/forms.py#L524

However, when the UID decoded in django-rest-auth as base64
https://github.com/jazzband/dj-rest-auth/blob/02c9242e3d069164692ef44a0f15eaca31a41cac/dj_rest_auth/serializers.py#L214

[iMerica]

How does this manifest into an actual problem? Like, Is there an exception raised? Or is there a 404?

We have tests on password reset and they're all passing.

[mr-engin3er]

When a password reset link is request, the UID is encoded into the url as such, by django allauth

password-reset/confirm/NDc/5m9-db35d6f0efc16645b352/

Here UID is NDc
And token is 5m9-db35d6f0efc16645b352

[squio]

The way I understand this question is how to support the following use case, where a website is configured to use Django Allauth for user management and Dj Rest Auth for REST API auth.

Password reset flow:

• user initiates a password reset through the REST API (e.g. as part of a mobile app)
• user gets confirmation email in their mail client
• user clicks link to open main website (in web browser)
• link is valid for website and user completes password reset

This only works when the password reset link is formatted in a compatible way with Django Allauth.

And that would be solved by PR #77 if I'm not mistaken.

So, why exactly has this PR been closed, is it not in scope for this project or any other reason?

[gsheni]

@squio see this pull request: Tivix/django-rest-auth#643

I had originally closed the PR because it wasn't being merged in.

[squio]

Hi @iMerica what is the status of this issue and the associated PR?

As far as I can tell this does not introduce a hard dependency on Allauth, while still providing compatibility with the password reset scheme when Allauth is available.

That is exactly the use case I currently have, so it would be really helpful if this PR got integrated.
Do you have any plans and a time line if so?

[iMerica]

That PR has some failing tests.

[squio]

Hmm @gsheni are you still interested in this PR?

Otherwise I'll have a look and see if I can make fixes and maybe extra unit tests

[gsheni]

@squio you can go ahead and take over my PR.

[squio]

So I finally got around it and got it fixed in #266.

After closely inspecting the code the fix was not complicated (tests should use the right encoder corresponding to the decoder used in the Serializer class.

Also the selection of encoder / decoder is now similar to the other checks where we test if allauth is in installed apps.

[squio]

Thanks 👍

[kut]

@iMerica should this actually be a setting, and not just based on whether allauth is installed? In our use case, we have allauth installed but not used to generate the UID/token for password resets (we're using the default dj-rest-auth functionality which relies on PasswordResetForm which is in Django core). Surely having allauth installed doesn't necessarily dictate that it will be used for password reset purposes?

(One could also certainly imagine an errant and unused installation of allauth in your environment, eg in a dependency, which would alter the above decoding functionality.)

Could this instead be a setting (called something like REST_USE_ALLAUTH_UID_DECODER) that allows you to concretely specify which decoder to use?

(The result for us it that we cannot use dj-rest-auth v2.1.6 onwards.)

[KaczuH]

Unfortunately the problem persists. Uid is generated by Django related code in PasswordResetForm and decoded by allauth in PasswordResetConfirmSerializer

Updated tests in #266 assume in _generate_uid_and_token uid and token are generated by allauth. Which is not the case after conducting manual test.

[squio]

Hmm indeed, this patch didn't solve my real world issue either; now reverted to an intermediate view to catch the difference in encoding.

For anyone interested:

• decode the Django standard tokens
• if valid, encode using allauth's strategy
• redirect to allauth reset password view

https://gist.github.com/squio/28aadec528bea9302c168744fd0af3b6

[mohmyo]

@iMerica I have the same issue as @Kub-AT mentioned above, this should be a setting definitely.

[georgkrause]

@iMerica Would you accept a patch that enforces the Django encoding of the UID instead of only checking for weather or not allauth is installed?