Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#9381 closed (invalid)

Errors in django.contib.auth PasswordResetTest

Reported by: kit1980 Owned by: nobody
Component: contrib.auth Version: master
Severity: Keywords: PasswordResetTest, tests
Cc: Triage Stage: Unreviewed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

Current version of tests gets wrong path to password reset confirm page (regexp doesn't takes token and uid) from password reset confirmation email.
And actually we don't need full path from email, only uid + token part, because tests.views from django.contib.auth sets urls = 'django.contrib.auth.urls' and real project urls are not valid.

For example, if we send email "Please confirm http://127.0.0.1:8000/accounts/password/reset/1-271-fc1325747eabe02fab7e/ Thanks!", in current version with

re.search(r"https?://[^/]*(/.*reset/\S*)", email.body) 

we get confirmation path as /accounts/password/reset/ , but it should be /reset/1-271-fc1325747eabe02fab7e/

Simple patch:

--- views.py    (revision 9232)
+++ views.py    (working copy)
@@ -34,9 +34,9 @@
         return self._read_signup_email(mail.outbox[0])

     def _read_signup_email(self, email):
-        urlmatch = re.search(r"https?://[^/]*(/.*reset/\S*)", email.body)
+        urlmatch = re.search(r"https?://.*(/.+/)", email.body)
         self.assert_(urlmatch is not None, "No URL found in sent email")
-        return urlmatch.group(), urlmatch.groups()[0]
+        return urlmatch.group(), "/reset" + urlmatch.groups()[0]

     def test_confirm_valid(self):
         url, path = self._test_confirm_start()


Attachments (1)

9383.diff (756 bytes) - added by kit1980 7 years ago.

Download all attachments as: .zip

Change History (3)

Changed 7 years ago by kit1980

comment:1 follow-up: Changed 7 years ago by mtredinnick

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to invalid
  • Status changed from new to closed

I don't believe this patch is necessary. The current regular expression looks like it works as intended. \S matches all non-whitespace characters. If I pass in the string you give, it returns the token and everything, as one would expect.

>>> re.search(r"https?://[^/]*(/.*reset/\S*)", "Please confirm http://127.0.0.1:8000/accounts/password/reset/1-271-fc1325747eabe02fab7e/ Thanks!").groups()
('/accounts/password/reset/1-271-fc1325747eabe02fab7e/',)

So you have perhaps left something out of the steps you took to see the problem in the first place. Ideally, a patch against Django's tests that shows the failure would help, but, failing that, at least some steps to take to repeat the problem in a rapid fashion.

Closing for now, since I really cannot see what the problem is here. If you can still repeat the problem, please reopen with some more details.

comment:2 in reply to: ↑ 1 Changed 7 years ago by kit1980

Replying to mtredinnick:
Yes, thanks.
The real problem was in my registration/password_reset_email.html template: it had hardcoded path to /accounts/password/reset/confirm/ instead of {% url %} tag.

Note: See TracTickets for help on using tickets.
Back to Top