Merge lp:~bac/launchpad/bug-452491-captcha2-boogaloo into lp:launchpad
- bug-452491-captcha2-boogaloo
- Merge into devel
Proposed by
Brad Crittenden
| Status: | Merged | ||||
|---|---|---|---|---|---|
| Merged at revision: | not available | ||||
| Proposed branch: | lp:~bac/launchpad/bug-452491-captcha2-boogaloo | ||||
| Merge into: | lp:launchpad | ||||
| Diff against target: |
451 lines 9 files modified
lib/canonical/launchpad/browser/tests/registration.py (+1/-1) lib/canonical/launchpad/pagetests/standalone/xx-new-account-redirection-url.txt (+1/-1) lib/canonical/launchpad/templates/launchpad-forgottenpassword.pt (+34/-7) lib/canonical/launchpad/templates/launchpad-login.pt (+1/-0) lib/canonical/launchpad/webapp/login.py (+48/-38) lib/lp/registry/stories/foaf/xx-createaccount.txt (+2/-2) lib/lp/registry/stories/foaf/xx-reg-with-existing-email.txt (+4/-4) lib/lp/registry/stories/foaf/xx-resetpassword.txt (+34/-24) lib/lp/testing/registration.py (+2/-2) |
||||
| To merge this branch: | bzr merge lp:~bac/launchpad/bug-452491-captcha2-boogaloo | ||||
| Related bugs: |
|
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Martin Albisetti (community) | ui | Approve | |
| Abel Deuring (community) | Approve | ||
|
Review via email:
|
|||
Commit message
Add simple captcha to the +forgottenpassword page.
Description of the change
To post a comment you must log in.
Revision history for this message
| Brad Crittenden (bac) wrote : | # |
Revision history for this message
| Abel Deuring (adeuring) wrote : | # |
Hi Brad,
a nice branch r=me. I have only a minor cosmetic suggestion, see below.
Abel
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -24,9 +24,10 @@
> return ''
>
>
> -def set_captcha_
> +def set_captcha_
> """Given a browser, set the login captcha with the correct answer."""
> if answer is None:
> answer = get_captcha_
> - browser.
> + control_name = prefix + 'captcha_
> + browser.
> answer)
I think you can merge the last two lines in one ;)
review:
Approve
Revision history for this message
| Brad Crittenden (bac) wrote : | # |
> Hi Brad,
>
> a nice branch r=me. I have only a minor cosmetic suggestion, see below.
>
> Abel
>
> > === modified file 'lib/lp/
> > --- lib/lp/
> > +++ lib/lp/
> > @@ -24,9 +24,10 @@
> > return ''
> >
> >
> > -def set_captcha_
> > +def set_captcha_
> > """Given a browser, set the login captcha with the correct answer."""
> > if answer is None:
> > answer = get_captcha_
> > - browser.
> > + control_name = prefix + 'captcha_
> > + browser.
> > answer)
>
> I think you can merge the last two lines in one ;)
Done
Revision history for this message
| Martin Albisetti (beuno) wrote : | # |
Bonus points if the forms look like everywhere else in Launchpad.
review:
Approve
(ui)
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
| 1 | === modified file 'lib/canonical/launchpad/browser/tests/registration.py' | |||
| 2 | --- lib/canonical/launchpad/browser/tests/registration.py 2009-10-08 19:52:06 +0000 | |||
| 3 | +++ lib/canonical/launchpad/browser/tests/registration.py 2009-10-16 19:59:17 +0000 | |||
| 4 | @@ -21,7 +21,7 @@ | |||
| 5 | 21 | browser = setupBrowser() | 21 | browser = setupBrowser() |
| 6 | 22 | browser.open('http://launchpad.dev/+login') | 22 | browser.open('http://launchpad.dev/+login') |
| 7 | 23 | browser.getControl(name='loginpage_email', index=1).value = email | 23 | browser.getControl(name='loginpage_email', index=1).value = email |
| 9 | 24 | set_captcha_answer(browser) | 24 | set_captcha_answer(browser, prefix='loginpage_') |
| 10 | 25 | browser.getControl('Register').click() | 25 | browser.getControl('Register').click() |
| 11 | 26 | return browser | 26 | return browser |
| 12 | 27 | 27 | ||
| 13 | 28 | 28 | ||
| 14 | === modified file 'lib/canonical/launchpad/pagetests/standalone/xx-new-account-redirection-url.txt' | |||
| 15 | --- lib/canonical/launchpad/pagetests/standalone/xx-new-account-redirection-url.txt 2009-10-08 19:52:06 +0000 | |||
| 16 | +++ lib/canonical/launchpad/pagetests/standalone/xx-new-account-redirection-url.txt 2009-10-16 19:59:17 +0000 | |||
| 17 | @@ -42,7 +42,7 @@ | |||
| 18 | 42 | >>> from lp.testing.registration import set_captcha_answer | 42 | >>> from lp.testing.registration import set_captcha_answer |
| 19 | 43 | >>> anon_browser.getControl('E-mail address:', index=1).value = ( | 43 | >>> anon_browser.getControl('E-mail address:', index=1).value = ( |
| 20 | 44 | ... 'granny@canonical.com') | 44 | ... 'granny@canonical.com') |
| 22 | 45 | >>> set_captcha_answer(anon_browser) | 45 | >>> set_captcha_answer(anon_browser, prefix='loginpage_') |
| 23 | 46 | >>> anon_browser.getControl('Register').click() | 46 | >>> anon_browser.getControl('Register').click() |
| 24 | 47 | >>> print anon_browser.contents | 47 | >>> print anon_browser.contents |
| 25 | 48 | <... | 48 | <... |
| 26 | 49 | 49 | ||
| 27 | === modified file 'lib/canonical/launchpad/templates/launchpad-forgottenpassword.pt' | |||
| 28 | --- lib/canonical/launchpad/templates/launchpad-forgottenpassword.pt 2009-07-17 17:59:07 +0000 | |||
| 29 | +++ lib/canonical/launchpad/templates/launchpad-forgottenpassword.pt 2009-10-16 19:59:17 +0000 | |||
| 30 | @@ -21,22 +21,49 @@ | |||
| 31 | 21 | 21 | ||
| 32 | 22 | <div tal:condition="not: view/success"> | 22 | <div tal:condition="not: view/success"> |
| 33 | 23 | 23 | ||
| 36 | 24 | <div class="error" tal:condition="view/errortext" | 24 | <tal:block condition="view/errortext"> |
| 37 | 25 | tal:content="structure view/errortext" /> | 25 | <h1>Your password reset request was unsuccessful</h1> |
| 38 | 26 | <p class="error message" | ||
| 39 | 27 | style="margin-top: 2em;" | ||
| 40 | 28 | tal:content="structure view/errortext" /> | ||
| 41 | 29 | </tal:block> | ||
| 42 | 26 | 30 | ||
| 43 | 27 | <p> | 31 | <p> |
| 44 | 28 | Enter your e-mail address, and we will send you instructions | 32 | Enter your e-mail address, and we will send you instructions |
| 46 | 29 | on how to reset your password. | 33 | on how to reset your password. You'll also need to answer a simple |
| 47 | 34 | question so we know that you're human. | ||
| 48 | 30 | </p> | 35 | </p> |
| 49 | 31 | 36 | ||
| 50 | 32 | <form name="forgottenpassword" method="POST"> | 37 | <form name="forgottenpassword" method="POST"> |
| 51 | 38 | <input type="hidden" | ||
| 52 | 39 | tal:attributes="name view/captcha_hash; | ||
| 53 | 40 | value view/get_captcha_hash" /> | ||
| 54 | 33 | <div class="field"> | 41 | <div class="field"> |
| 57 | 34 | <label for="email">Email address:</label> | 42 | <table> |
| 58 | 35 | <input type="text" size="50" name="email" /> | 43 | <tr> |
| 59 | 44 | <th> | ||
| 60 | 45 | <label for="email">Email address:</label> | ||
| 61 | 46 | </th> | ||
| 62 | 47 | <td> | ||
| 63 | 48 | <input type="text" size="50" name="email" /> | ||
| 64 | 49 | </td> | ||
| 65 | 50 | </tr> | ||
| 66 | 51 | <tr> | ||
| 67 | 52 | <th> | ||
| 68 | 53 | <label>Random question:</label> | ||
| 69 | 54 | </th> | ||
| 70 | 55 | <td> | ||
| 71 | 56 | <span id="problem" tal:content="view/captcha_problem" /> | ||
| 72 | 57 | <input type="text" size="5" | ||
| 73 | 58 | id="captcha_submission" value="" | ||
| 74 | 59 | tal:attributes="name view/captcha_submission" | ||
| 75 | 60 | /> | ||
| 76 | 61 | </td> | ||
| 77 | 62 | </tr> | ||
| 78 | 63 | </table> | ||
| 79 | 36 | <div class="formHelp"> | 64 | <div class="formHelp"> |
| 80 | 37 | <p> | 65 | <p> |
| 83 | 38 | This should be the email address registered for your Launchpad | 66 | Enter the email address registered for your Launchpad account. |
| 82 | 39 | account. | ||
| 84 | 40 | </p> | 67 | </p> |
| 85 | 41 | <p> | 68 | <p> |
| 86 | 42 | If you are claiming an existing account but don't know the email | 69 | If you are claiming an existing account but don't know the email |
| 87 | 43 | 70 | ||
| 88 | === modified file 'lib/canonical/launchpad/templates/launchpad-login.pt' | |||
| 89 | --- lib/canonical/launchpad/templates/launchpad-login.pt 2009-10-08 15:57:01 +0000 | |||
| 90 | +++ lib/canonical/launchpad/templates/launchpad-login.pt 2009-10-16 19:59:17 +0000 | |||
| 91 | @@ -164,6 +164,7 @@ | |||
| 92 | 164 | <tal:block condition="view/registration_error"> | 164 | <tal:block condition="view/registration_error"> |
| 93 | 165 | <h1>Your registration was unsuccessful</h1> | 165 | <h1>Your registration was unsuccessful</h1> |
| 94 | 166 | <p class="error message" | 166 | <p class="error message" |
| 95 | 167 | style="margin-top: 2em;" | ||
| 96 | 167 | tal:content="structure view/registration_error" /> | 168 | tal:content="structure view/registration_error" /> |
| 97 | 168 | </tal:block> | 169 | </tal:block> |
| 98 | 169 | <!-- 5. might want to register: --> | 170 | <!-- 5. might want to register: --> |
| 99 | 170 | 171 | ||
| 100 | === modified file 'lib/canonical/launchpad/webapp/login.py' | |||
| 101 | --- lib/canonical/launchpad/webapp/login.py 2009-10-08 15:57:01 +0000 | |||
| 102 | +++ lib/canonical/launchpad/webapp/login.py 2009-10-16 19:59:17 +0000 | |||
| 103 | @@ -153,8 +153,44 @@ | |||
| 104 | 153 | return getUtility(IPersonSet).getByName( | 153 | return getUtility(IPersonSet).getByName( |
| 105 | 154 | config.launchpad.restrict_to_team).title | 154 | config.launchpad.restrict_to_team).title |
| 106 | 155 | 155 | ||
| 109 | 156 | 156 | class CaptchaMixin: | |
| 110 | 157 | class LoginOrRegister: | 157 | """Mixin class to provide simple captcha capabilities.""" |
| 111 | 158 | def validateCaptcha(self): | ||
| 112 | 159 | """Validate the submitted captcha value matches what we expect.""" | ||
| 113 | 160 | expected = self.request.form.get(self.captcha_hash) | ||
| 114 | 161 | submitted = self.request.form.get(self.captcha_submission) | ||
| 115 | 162 | if expected is not None and submitted is not None: | ||
| 116 | 163 | return md5.new(submitted).hexdigest() == expected | ||
| 117 | 164 | return False | ||
| 118 | 165 | |||
| 119 | 166 | @cachedproperty | ||
| 120 | 167 | def captcha_answer(self): | ||
| 121 | 168 | """Get the answer for the current captcha challenge. | ||
| 122 | 169 | |||
| 123 | 170 | With each failed attempt a new challenge will be given. Our answer | ||
| 124 | 171 | space is acknowledged to be ridiculously small but is chosen in the | ||
| 125 | 172 | interest of ease-of-use. We're not trying to create an iron-clad | ||
| 126 | 173 | challenge but only a minimal obstacle to dumb bots. | ||
| 127 | 174 | """ | ||
| 128 | 175 | return random.randint(10, 20) | ||
| 129 | 176 | |||
| 130 | 177 | @property | ||
| 131 | 178 | def get_captcha_hash(self): | ||
| 132 | 179 | """Get the captcha hash. | ||
| 133 | 180 | |||
| 134 | 181 | The hash is the value we put in the form for later comparison. | ||
| 135 | 182 | """ | ||
| 136 | 183 | return md5.new(str(self.captcha_answer)).hexdigest() | ||
| 137 | 184 | |||
| 138 | 185 | @property | ||
| 139 | 186 | def captcha_problem(self): | ||
| 140 | 187 | """Create the captcha challenge.""" | ||
| 141 | 188 | op1 = random.randint(1, self.captcha_answer - 1) | ||
| 142 | 189 | op2 = self.captcha_answer - op1 | ||
| 143 | 190 | return '%d + %d =' % (op1, op2) | ||
| 144 | 191 | |||
| 145 | 192 | |||
| 146 | 193 | class LoginOrRegister(CaptchaMixin): | ||
| 147 | 158 | """Merges the former CookieLoginPage and JoinLaunchpadView classes | 194 | """Merges the former CookieLoginPage and JoinLaunchpadView classes |
| 148 | 159 | to allow the two forms to appear on a single page. | 195 | to allow the two forms to appear on a single page. |
| 149 | 160 | 196 | ||
| 150 | @@ -399,41 +435,6 @@ | |||
| 151 | 399 | L.append(html % (name, cgi.escape(value, quote=True))) | 435 | L.append(html % (name, cgi.escape(value, quote=True))) |
| 152 | 400 | return '\n'.join(L) | 436 | return '\n'.join(L) |
| 153 | 401 | 437 | ||
| 154 | 402 | def validateCaptcha(self): | ||
| 155 | 403 | """Validate the submitted captcha value matches what we expect.""" | ||
| 156 | 404 | expected = self.request.form.get(self.captcha_hash) | ||
| 157 | 405 | submitted = self.request.form.get(self.captcha_submission) | ||
| 158 | 406 | if expected is not None and submitted is not None: | ||
| 159 | 407 | return md5.new(submitted).hexdigest() == expected | ||
| 160 | 408 | return False | ||
| 161 | 409 | |||
| 162 | 410 | @cachedproperty | ||
| 163 | 411 | def captcha_answer(self): | ||
| 164 | 412 | """Get the answer for the current captcha challenge. | ||
| 165 | 413 | |||
| 166 | 414 | With each failed attempt a new challenge will be given. Our answer | ||
| 167 | 415 | space is acknowledged to be ridiculously small but is chosen in the | ||
| 168 | 416 | interest of ease-of-use. We're not trying to create an iron-clad | ||
| 169 | 417 | challenge but only a minimal obstacle to dumb bots. | ||
| 170 | 418 | """ | ||
| 171 | 419 | return random.randint(10, 20) | ||
| 172 | 420 | |||
| 173 | 421 | @property | ||
| 174 | 422 | def get_captcha_hash(self): | ||
| 175 | 423 | """Get the captcha hash. | ||
| 176 | 424 | |||
| 177 | 425 | The hash is the value we put in the form for later comparison. | ||
| 178 | 426 | """ | ||
| 179 | 427 | return md5.new(str(self.captcha_answer)).hexdigest() | ||
| 180 | 428 | |||
| 181 | 429 | @property | ||
| 182 | 430 | def captcha_problem(self): | ||
| 183 | 431 | """Create the captcha challenge.""" | ||
| 184 | 432 | op1 = random.randint(1, self.captcha_answer) | ||
| 185 | 433 | op2 = self.captcha_answer - op1 | ||
| 186 | 434 | return '%d + %d =' % (op1, op2) | ||
| 187 | 435 | |||
| 188 | 436 | |||
| 189 | 437 | def logInPrincipal(request, principal, email): | 438 | def logInPrincipal(request, principal, email): |
| 190 | 438 | """Log the principal in. Password validation must be done in callsites.""" | 439 | """Log the principal in. Password validation must be done in callsites.""" |
| 191 | 439 | session = ISession(request) | 440 | session = ISession(request) |
| 192 | @@ -542,16 +543,25 @@ | |||
| 193 | 542 | return '' | 543 | return '' |
| 194 | 543 | 544 | ||
| 195 | 544 | 545 | ||
| 197 | 545 | class ForgottenPasswordPage: | 546 | class ForgottenPasswordPage(CaptchaMixin): |
| 198 | 546 | 547 | ||
| 199 | 547 | errortext = None | 548 | errortext = None |
| 200 | 548 | submitted = False | 549 | submitted = False |
| 201 | 550 | captcha_submission = 'captcha_submission' | ||
| 202 | 551 | captcha_hash = 'captcha_hash' | ||
| 203 | 549 | 552 | ||
| 204 | 550 | def process_form(self): | 553 | def process_form(self): |
| 205 | 551 | request = self.request | 554 | request = self.request |
| 206 | 552 | if request.method != "POST": | 555 | if request.method != "POST": |
| 207 | 553 | return | 556 | return |
| 208 | 554 | 557 | ||
| 209 | 558 | # Validate the user is human, more or less. | ||
| 210 | 559 | if not self.validateCaptcha(): | ||
| 211 | 560 | self.errortext = ( | ||
| 212 | 561 | "The answer to the simple math question was incorrect " | ||
| 213 | 562 | "or missing. Please try again.") | ||
| 214 | 563 | return | ||
| 215 | 564 | |||
| 216 | 555 | email = request.form.get("email").strip() | 565 | email = request.form.get("email").strip() |
| 217 | 556 | person = getUtility(IPersonSet).getByEmail(email) | 566 | person = getUtility(IPersonSet).getByEmail(email) |
| 218 | 557 | if person is None: | 567 | if person is None: |
| 219 | 558 | 568 | ||
| 220 | === modified file 'lib/lp/registry/stories/foaf/xx-createaccount.txt' | |||
| 221 | --- lib/lp/registry/stories/foaf/xx-createaccount.txt 2009-10-08 19:52:06 +0000 | |||
| 222 | +++ lib/lp/registry/stories/foaf/xx-createaccount.txt 2009-10-16 19:59:17 +0000 | |||
| 223 | @@ -36,7 +36,7 @@ | |||
| 224 | 36 | email address from before has been retained in the form. | 36 | email address from before has been retained in the form. |
| 225 | 37 | 37 | ||
| 226 | 38 | >>> from lp.testing.registration import set_captcha_answer | 38 | >>> from lp.testing.registration import set_captcha_answer |
| 228 | 39 | >>> set_captcha_answer(browser) | 39 | >>> set_captcha_answer(browser, prefix='loginpage_') |
| 229 | 40 | >>> browser.getControl('Register').click() | 40 | >>> browser.getControl('Register').click() |
| 230 | 41 | >>> print_feedback_messages(browser.contents) | 41 | >>> print_feedback_messages(browser.contents) |
| 231 | 42 | >>> print extract_text(find_main_content(browser.contents)) | 42 | >>> print extract_text(find_main_content(browser.contents)) |
| 232 | @@ -103,7 +103,7 @@ | |||
| 233 | 103 | >>> browser.open('http://launchpad.dev/+login') | 103 | >>> browser.open('http://launchpad.dev/+login') |
| 234 | 104 | >>> browser.getControl(name='loginpage_email', index=1).value = ( | 104 | >>> browser.getControl(name='loginpage_email', index=1).value = ( |
| 235 | 105 | ... 'jperson@example.com') | 105 | ... 'jperson@example.com') |
| 237 | 106 | >>> set_captcha_answer(browser) | 106 | >>> set_captcha_answer(browser, prefix='loginpage_') |
| 238 | 107 | >>> browser.getControl('Register').click() | 107 | >>> browser.getControl('Register').click() |
| 239 | 108 | 108 | ||
| 240 | 109 | >>> from_addr, to_addrs, raw_msg = stub.test_emails.pop() | 109 | >>> from_addr, to_addrs, raw_msg = stub.test_emails.pop() |
| 241 | 110 | 110 | ||
| 242 | === modified file 'lib/lp/registry/stories/foaf/xx-reg-with-existing-email.txt' | |||
| 243 | --- lib/lp/registry/stories/foaf/xx-reg-with-existing-email.txt 2009-10-08 19:52:06 +0000 | |||
| 244 | +++ lib/lp/registry/stories/foaf/xx-reg-with-existing-email.txt 2009-10-16 19:59:17 +0000 | |||
| 245 | @@ -16,7 +16,7 @@ | |||
| 246 | 16 | >>> browser.getControl('E-mail address:', index=1).value = ( | 16 | >>> browser.getControl('E-mail address:', index=1).value = ( |
| 247 | 17 | ... 'test@canonical.com') | 17 | ... 'test@canonical.com') |
| 248 | 18 | >>> from lp.testing.registration import set_captcha_answer | 18 | >>> from lp.testing.registration import set_captcha_answer |
| 250 | 19 | >>> set_captcha_answer(browser) | 19 | >>> set_captcha_answer(browser, prefix='loginpage_') |
| 251 | 20 | >>> browser.getControl('Register').click() | 20 | >>> browser.getControl('Register').click() |
| 252 | 21 | 21 | ||
| 253 | 22 | >>> for message in get_feedback_messages(browser.contents): | 22 | >>> for message in get_feedback_messages(browser.contents): |
| 254 | @@ -33,7 +33,7 @@ | |||
| 255 | 33 | on with the registration process. | 33 | on with the registration process. |
| 256 | 34 | 34 | ||
| 257 | 35 | >>> browser.getControl('E-mail address:', index=1).value = 'mpo@iki.fi' | 35 | >>> browser.getControl('E-mail address:', index=1).value = 'mpo@iki.fi' |
| 259 | 36 | >>> set_captcha_answer(browser) | 36 | >>> set_captcha_answer(browser, prefix='loginpage_') |
| 260 | 37 | >>> browser.getControl('Register').click() | 37 | >>> browser.getControl('Register').click() |
| 261 | 38 | 38 | ||
| 262 | 39 | >>> print extract_text(find_tag_by_id(browser.contents, 'address')) | 39 | >>> print extract_text(find_tag_by_id(browser.contents, 'address')) |
| 263 | @@ -85,7 +85,7 @@ | |||
| 264 | 85 | 85 | ||
| 265 | 86 | >>> anon_browser.getControl('E-mail address:', index=1).value = ( | 86 | >>> anon_browser.getControl('E-mail address:', index=1).value = ( |
| 266 | 87 | ... 'christian.reis@ubuntulinux.com') | 87 | ... 'christian.reis@ubuntulinux.com') |
| 268 | 88 | >>> set_captcha_answer(anon_browser) | 88 | >>> set_captcha_answer(anon_browser, prefix='loginpage_') |
| 269 | 89 | >>> anon_browser.getControl('Register').click() | 89 | >>> anon_browser.getControl('Register').click() |
| 270 | 90 | 90 | ||
| 271 | 91 | >>> from_addr, to_addr, msg = stub.test_emails.pop() | 91 | >>> from_addr, to_addr, msg = stub.test_emails.pop() |
| 272 | @@ -147,7 +147,7 @@ | |||
| 273 | 147 | 147 | ||
| 274 | 148 | >>> browser.getControl('E-mail address:', index=1).value = ( | 148 | >>> browser.getControl('E-mail address:', index=1).value = ( |
| 275 | 149 | ... 'bad-user@canonical.com') | 149 | ... 'bad-user@canonical.com') |
| 277 | 150 | >>> set_captcha_answer(browser) | 150 | >>> set_captcha_answer(browser, prefix='loginpage_') |
| 278 | 151 | >>> browser.getControl('Register').click() | 151 | >>> browser.getControl('Register').click() |
| 279 | 152 | 152 | ||
| 280 | 153 | >>> print extract_text(find_main_content(browser.contents).p) | 153 | >>> print extract_text(find_main_content(browser.contents).p) |
| 281 | 154 | 154 | ||
| 282 | === modified file 'lib/lp/registry/stories/foaf/xx-resetpassword.txt' | |||
| 283 | --- lib/lp/registry/stories/foaf/xx-resetpassword.txt 2009-09-18 01:09:10 +0000 | |||
| 284 | +++ lib/lp/registry/stories/foaf/xx-resetpassword.txt 2009-10-16 19:59:17 +0000 | |||
| 285 | @@ -15,10 +15,12 @@ | |||
| 286 | 15 | >>> browser.title | 15 | >>> browser.title |
| 287 | 16 | 'Need a new Launchpad password?' | 16 | 'Need a new Launchpad password?' |
| 288 | 17 | 17 | ||
| 290 | 18 | He types the email address registered in Launchpad and submit the form. | 18 | He types the email address registered in Launchpad and submits the form. |
| 291 | 19 | 19 | ||
| 292 | 20 | >>> from lp.testing.registration import set_captcha_answer | ||
| 293 | 20 | >>> browser.getControl(name='email').value = ( | 21 | >>> browser.getControl(name='email').value = ( |
| 294 | 21 | ... 'david.allouche@canonical.com') | 22 | ... 'david.allouche@canonical.com') |
| 295 | 23 | >>> set_captcha_answer(browser) | ||
| 296 | 22 | >>> browser.getControl('Request Reset').click() | 24 | >>> browser.getControl('Request Reset').click() |
| 297 | 23 | >>> print extract_text(find_main_content(browser.contents)) | 25 | >>> print extract_text(find_main_content(browser.contents)) |
| 298 | 24 | Need a new Launchpad password? | 26 | Need a new Launchpad password? |
| 299 | @@ -68,8 +70,7 @@ | |||
| 300 | 68 | >>> browser.url | 70 | >>> browser.url |
| 301 | 69 | 'http://launchpad.dev/token/.../+resetpassword' | 71 | 'http://launchpad.dev/token/.../+resetpassword' |
| 302 | 70 | 72 | ||
| 305 | 71 | >>> for tag in get_feedback_messages(browser.contents): | 73 | >>> print_feedback_messages(browser.contents) |
| 304 | 72 | ... print extract_text(tag) | ||
| 306 | 73 | There is 1 error. | 74 | There is 1 error. |
| 307 | 74 | The password provided contains non-ASCII characters. | 75 | The password provided contains non-ASCII characters. |
| 308 | 75 | 76 | ||
| 309 | @@ -82,8 +83,7 @@ | |||
| 310 | 82 | >>> browser.url | 83 | >>> browser.url |
| 311 | 83 | 'http://launchpad.dev' | 84 | 'http://launchpad.dev' |
| 312 | 84 | 85 | ||
| 315 | 85 | >>> for tag in get_feedback_messages(browser.contents): | 86 | >>> print_feedback_messages(browser.contents) |
| 314 | 86 | ... print extract_text(tag) | ||
| 316 | 87 | Your password has been reset successfully. | 87 | Your password has been reset successfully. |
| 317 | 88 | 88 | ||
| 318 | 89 | >>> print extract_text(find_tag_by_id(browser.contents, 'logincontrol')) | 89 | >>> print extract_text(find_tag_by_id(browser.contents, 'logincontrol')) |
| 319 | @@ -104,8 +104,7 @@ | |||
| 320 | 104 | logs out and then logs in again: | 104 | logs out and then logs in again: |
| 321 | 105 | 105 | ||
| 322 | 106 | >>> browser.open('http://launchpad.dev/+logout') | 106 | >>> browser.open('http://launchpad.dev/+logout') |
| 325 | 107 | >>> for tag in get_feedback_messages(browser.contents): | 107 | >>> print_feedback_messages(browser.contents) |
| 324 | 108 | ... print extract_text(tag) | ||
| 326 | 109 | You have been logged out | 108 | You have been logged out |
| 327 | 110 | 109 | ||
| 328 | 111 | >>> browser.getLink('Log in / Register').click() | 110 | >>> browser.getLink('Log in / Register').click() |
| 329 | @@ -122,6 +121,20 @@ | |||
| 330 | 122 | David Allouche... | 121 | David Allouche... |
| 331 | 123 | 122 | ||
| 332 | 124 | 123 | ||
| 333 | 124 | == Reset is protected by a math captcha == | ||
| 334 | 125 | |||
| 335 | 126 | When requesting a password reset David is presented with a simple math | ||
| 336 | 127 | problem which must be solved correctly before proceeding. | ||
| 337 | 128 | |||
| 338 | 129 | >>> browser.open('http://launchpad.dev/+forgottenpassword') | ||
| 339 | 130 | >>> browser.getControl(name='email').value = ( | ||
| 340 | 131 | ... 'david.allouche@canonical.com') | ||
| 341 | 132 | >>> browser.getControl(name='captcha_submission').value = '-1' | ||
| 342 | 133 | >>> browser.getControl('Request Reset').click() | ||
| 343 | 134 | >>> print_feedback_messages(browser.contents) | ||
| 344 | 135 | The answer to the simple math question was incorrect or missing. Please try again. | ||
| 345 | 136 | |||
| 346 | 137 | |||
| 347 | 125 | == Using email addresses other than the preferred one == | 138 | == Using email addresses other than the preferred one == |
| 348 | 126 | 139 | ||
| 349 | 127 | Any of a person's validated email addresses can be used to reset his | 140 | Any of a person's validated email addresses can be used to reset his |
| 350 | @@ -134,8 +147,9 @@ | |||
| 351 | 134 | >>> browser.open('http://launchpad.dev/+login') | 147 | >>> browser.open('http://launchpad.dev/+login') |
| 352 | 135 | >>> browser.getLink('Forgotten your password?').click() | 148 | >>> browser.getLink('Forgotten your password?').click() |
| 353 | 136 | >>> browser.getControl(name='email').value = 'david@canonical.com' | 149 | >>> browser.getControl(name='email').value = 'david@canonical.com' |
| 354 | 150 | >>> set_captcha_answer(browser) | ||
| 355 | 137 | >>> browser.getControl('Request Reset').click() | 151 | >>> browser.getControl('Request Reset').click() |
| 357 | 138 | >>> print "\n".join(get_feedback_messages(browser.contents)) | 152 | >>> print_feedback_messages(browser.contents) |
| 358 | 139 | 153 | ||
| 359 | 140 | He follows the link sent to his email just like he did before. | 154 | He follows the link sent to his email just like he did before. |
| 360 | 141 | 155 | ||
| 361 | @@ -155,7 +169,7 @@ | |||
| 362 | 155 | >>> browser.getControl('Continue').click() | 169 | >>> browser.getControl('Continue').click() |
| 363 | 156 | >>> browser.url | 170 | >>> browser.url |
| 364 | 157 | 'http://launchpad.dev' | 171 | 'http://launchpad.dev' |
| 366 | 158 | >>> print "\n".join(get_feedback_messages(browser.contents)) | 172 | >>> print_feedback_messages(browser.contents) |
| 367 | 159 | Your password has been reset successfully. | 173 | Your password has been reset successfully. |
| 368 | 160 | 174 | ||
| 369 | 161 | 175 | ||
| 370 | @@ -165,12 +179,11 @@ | |||
| 371 | 165 | 179 | ||
| 372 | 166 | >>> browser.open('http://launchpad.dev/+forgottenpassword') | 180 | >>> browser.open('http://launchpad.dev/+forgottenpassword') |
| 373 | 167 | >>> browser.getControl(name='email').value = 'support@ubuntu.com' | 181 | >>> browser.getControl(name='email').value = 'support@ubuntu.com' |
| 374 | 182 | >>> set_captcha_answer(browser) | ||
| 375 | 168 | >>> browser.getControl('Request Reset').click() | 183 | >>> browser.getControl('Request Reset').click() |
| 381 | 169 | 184 | >>> print_feedback_messages(browser.contents) | |
| 382 | 170 | >>> for tag in find_tags_by_class(browser.contents, 'error'): | 185 | The email address support@ubuntu.com |
| 383 | 171 | ... print tag | 186 | belongs to a team, and teams cannot log in to Launchpad. |
| 379 | 172 | <div class="error">The email address <strong>support@ubuntu.com</strong> | ||
| 380 | 173 | belongs to a team, and teams cannot log in to Launchpad.</div> | ||
| 384 | 174 | 187 | ||
| 385 | 175 | 188 | ||
| 386 | 176 | == Reactivating an account == | 189 | == Reactivating an account == |
| 387 | @@ -202,8 +215,7 @@ | |||
| 388 | 202 | >>> browser.title | 215 | >>> browser.title |
| 389 | 203 | 'Log in or register with Launchpad' | 216 | 'Log in or register with Launchpad' |
| 390 | 204 | 217 | ||
| 393 | 205 | >>> for tag in get_feedback_messages(browser.contents): | 218 | >>> print_feedback_messages(browser.contents) |
| 392 | 206 | ... print extract_text(tag) | ||
| 394 | 207 | The email address belongs to a deactivated account. Use the | 219 | The email address belongs to a deactivated account. Use the |
| 395 | 208 | "Forgotten your password" link to reactivate it. | 220 | "Forgotten your password" link to reactivate it. |
| 396 | 209 | 221 | ||
| 397 | @@ -215,6 +227,7 @@ | |||
| 398 | 215 | 227 | ||
| 399 | 216 | >>> browser.getControl(name='email').value = ( | 228 | >>> browser.getControl(name='email').value = ( |
| 400 | 217 | ... 'former-user@canonical.com') | 229 | ... 'former-user@canonical.com') |
| 401 | 230 | >>> set_captcha_answer(browser) | ||
| 402 | 218 | >>> browser.getControl('Request Reset').click() | 231 | >>> browser.getControl('Request Reset').click() |
| 403 | 219 | 232 | ||
| 404 | 220 | >>> print extract_text(find_main_content(browser.contents).p) | 233 | >>> print extract_text(find_main_content(browser.contents).p) |
| 405 | @@ -248,8 +261,7 @@ | |||
| 406 | 248 | >>> browser.url | 261 | >>> browser.url |
| 407 | 249 | 'http://launchpad.dev' | 262 | 'http://launchpad.dev' |
| 408 | 250 | 263 | ||
| 411 | 251 | >>> for tag in get_feedback_messages(browser.contents): | 264 | >>> print_feedback_messages(browser.contents) |
| 410 | 252 | ... print extract_text(tag) | ||
| 412 | 253 | Welcome back to Launchpad. | 265 | Welcome back to Launchpad. |
| 413 | 254 | Your password has been reset successfully. | 266 | Your password has been reset successfully. |
| 414 | 255 | 267 | ||
| 415 | @@ -300,6 +312,7 @@ | |||
| 416 | 300 | 'Need a new Launchpad password?' | 312 | 'Need a new Launchpad password?' |
| 417 | 301 | >>> browser.getControl(name='email').value = ( | 313 | >>> browser.getControl(name='email').value = ( |
| 418 | 302 | ... 'bad-user@canonical.com') | 314 | ... 'bad-user@canonical.com') |
| 419 | 315 | >>> set_captcha_answer(browser) | ||
| 420 | 303 | >>> browser.getControl('Request Reset').click() | 316 | >>> browser.getControl('Request Reset').click() |
| 421 | 304 | 317 | ||
| 422 | 305 | >>> print extract_text(find_main_content(browser.contents).p) | 318 | >>> print extract_text(find_main_content(browser.contents).p) |
| 423 | @@ -330,9 +343,6 @@ | |||
| 424 | 330 | >>> browser.getControl(name='field.password_dupe').value = 'test' | 343 | >>> browser.getControl(name='field.password_dupe').value = 'test' |
| 425 | 331 | >>> browser.getControl('Continue').click() | 344 | >>> browser.getControl('Continue').click() |
| 426 | 332 | 345 | ||
| 433 | 333 | >>> for tag in find_tags_by_class( | 346 | >>> print_feedback_messages(browser.contents) |
| 434 | 334 | ... browser.contents, 'warning'): | 347 | Your password cannot be reset because your account is suspended. |
| 435 | 335 | ... print tag | 348 | Contact a Launchpad admin about this issue. |
| 430 | 336 | <div ...>Your password cannot be reset because your account is suspended. | ||
| 431 | 337 | Contact a <a href="mailto:feedback@launchpad.net?subject=SU...">Launchpad | ||
| 432 | 338 | admin</a> about this issue.</div> | ||
| 436 | 339 | 349 | ||
| 437 | === modified file 'lib/lp/testing/registration.py' | |||
| 438 | --- lib/lp/testing/registration.py 2009-10-08 20:22:25 +0000 | |||
| 439 | +++ lib/lp/testing/registration.py 2009-10-16 19:59:17 +0000 | |||
| 440 | @@ -24,9 +24,9 @@ | |||
| 441 | 24 | return '' | 24 | return '' |
| 442 | 25 | 25 | ||
| 443 | 26 | 26 | ||
| 445 | 27 | def set_captcha_answer(browser, answer=None): | 27 | def set_captcha_answer(browser, answer=None, prefix=''): |
| 446 | 28 | """Given a browser, set the login captcha with the correct answer.""" | 28 | """Given a browser, set the login captcha with the correct answer.""" |
| 447 | 29 | if answer is None: | 29 | if answer is None: |
| 448 | 30 | answer = get_captcha_answer(browser.contents) | 30 | answer = get_captcha_answer(browser.contents) |
| 450 | 31 | browser.getControl(name='loginpage_captcha_submission').value = ( | 31 | browser.getControl(name=prefix + 'captcha_submission').value = ( |
| 451 | 32 | answer) | 32 | answer) |
= Summary =
Bug 452491 requests a simple captcha be placed on the forgotten password page.
== Proposed fix ==
The captcha already existed on the registration page. It was quick work to refactor
the bits into a mixin class and let the forgotten password page use it. Sadly both
of those pages are done without using LaunchpadFormView, made instead with grout and
twine, so it took a little massaging to get things to work.
The test helper set_captcha_answer had to be made a little smarter to account for
form prefixes.
The reset password test needed some clean up too. There is a fair amount of drive by
stuff in here but it's easy to sort out, I hope.
I also changed the spacing on the error messages for this page and the registration
page to fix some overlap problems. Screenshots are at:
http://
== Pre-implementation notes ==
None
== Implementation details ==
As above.
== Tests ==
bin/test -vvm lp.registry -t foaf -t xx-new-
== Demo and Q/A ==
= Launchpad lint =
Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.
Linting changed files:
lib/canonical
lib/canonical
lib/lp/
lib/lp/
lib/canonical
lib/canonical
lib/lp/
lib/canonical
lib/lp/