Merge lp:~salgado/launchpad/bug-415977-2 into lp:launchpad

Proposed by Guilherme Salgado
Status: Merged
Approved by: Gary Poster
Approved revision: no longer in the source branch.
Merged at revision: not available
Proposed branch: lp:~salgado/launchpad/bug-415977-2
Merge into: lp:launchpad
Diff against target: 150 lines
3 files modified
database/schema/security.cfg (+1/-0)
lib/canonical/launchpad/webapp/login.py (+16/-7)
lib/lp/registry/stories/foaf/xx-resetpassword-of-sso-account.txt (+83/-0)
To merge this branch: bzr merge lp:~salgado/launchpad/bug-415977-2
Reviewer Review Type Date Requested Status
Gary Poster (community) Approve
Review via email: mp+14187@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) wrote :

= Summary =

As reported on bug 415977, personless accounts can't use the
+forgottenpassword form on Launchpad because it doesn't know how to deal
with these accounts.

== Proposed fix ==

In the mailing list we discussed getting rid of LP's +forgottenpassword
and change all links to point to the same page on SSO, but that's not
possible because that page doesn't exist on SSO anymore -- they use the
login form with some javascript and a regular link to request a password
reset and we're not doing that in LP.

For that reason we had to resort to the other alternative, which is to
change the +forgottenpassword page to generate AuthTokens when the email
address is associated with a personless account.

== Pre-implementation notes ==

== Implementation details ==

== Tests ==

./bin/test -vvt resetpassword-of-sso-account.txt

== 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:
  database/schema/security.cfg
  lib/canonical/launchpad/webapp/login.py
  lib/lp/registry/stories/foaf/xx-resetpassword-of-sso-account.txt

Revision history for this message
Gary Poster (gary) wrote :

Very nicely done. Reviewing this patch introduced me to a number of valuable pieces of our testing infrastructure. :-)

Thank you!

Gary

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2009-10-29 05:50:08 +0000
+++ database/schema/security.cfg 2009-10-29 19:50:24 +0000
@@ -98,6 +98,7 @@
98type=user98type=user
99public.account = SELECT, INSERT, UPDATE, DELETE99public.account = SELECT, INSERT, UPDATE, DELETE
100public.accountpassword = SELECT, INSERT, UPDATE, DELETE100public.accountpassword = SELECT, INSERT, UPDATE, DELETE
101public.authtoken = SELECT, INSERT, UPDATE
101public.emailaddress = SELECT, INSERT, UPDATE, DELETE102public.emailaddress = SELECT, INSERT, UPDATE, DELETE
102public.language = SELECT103public.language = SELECT
103public.openidrpconfig = SELECT104public.openidrpconfig = SELECT
104105
=== modified file 'lib/canonical/launchpad/webapp/login.py'
--- lib/canonical/launchpad/webapp/login.py 2009-10-16 16:13:00 +0000
+++ lib/canonical/launchpad/webapp/login.py 2009-10-29 19:50:24 +0000
@@ -24,7 +24,8 @@
24from canonical.config import config24from canonical.config import config
25from canonical.launchpad import _25from canonical.launchpad import _
26from canonical.launchpad.interfaces.account import AccountStatus26from canonical.launchpad.interfaces.account import AccountStatus
27from canonical.launchpad.interfaces.authtoken import LoginTokenType27from canonical.launchpad.interfaces.authtoken import (
28 IAuthTokenSet, LoginTokenType)
28from canonical.launchpad.interfaces.emailaddress import IEmailAddressSet29from canonical.launchpad.interfaces.emailaddress import IEmailAddressSet
29from canonical.launchpad.interfaces.logintoken import ILoginTokenSet30from canonical.launchpad.interfaces.logintoken import ILoginTokenSet
30from lp.registry.interfaces.person import (31from lp.registry.interfaces.person import (
@@ -563,22 +564,30 @@
563 return564 return
564565
565 email = request.form.get("email").strip()566 email = request.form.get("email").strip()
566 person = getUtility(IPersonSet).getByEmail(email)567 email_address = getUtility(IEmailAddressSet).getByEmail(email)
567 if person is None:568 if email_address is None:
568 self.errortext = ("Your account details have not been found. "569 self.errortext = ("Your account details have not been found. "
569 "Please check your subscription email "570 "Please check your subscription email "
570 "address and try again.")571 "address and try again.")
571 return572 return
572573
573 if person.isTeam():574 person = email_address.person
575 if person is not None and person.isTeam():
574 self.errortext = ("The email address <strong>%s</strong> "576 self.errortext = ("The email address <strong>%s</strong> "
575 "belongs to a team, and teams cannot log in to "577 "belongs to a team, and teams cannot log in to "
576 "Launchpad." % email)578 "Launchpad." % email)
577 return579 return
578580
579 logintokenset = getUtility(ILoginTokenSet)581 if person is None:
580 token = logintokenset.new(582 account = email_address.account
581 person, email, email, LoginTokenType.PASSWORDRECOVERY)583 redirection_url = urlappend(
584 self.request.getApplicationURL(), '+login')
585 token = getUtility(IAuthTokenSet).new(
586 account, email, email, LoginTokenType.PASSWORDRECOVERY,
587 redirection_url=redirection_url)
588 else:
589 token = getUtility(ILoginTokenSet).new(
590 person, email, email, LoginTokenType.PASSWORDRECOVERY)
582 token.sendPasswordResetEmail()591 token.sendPasswordResetEmail()
583 self.submitted = True592 self.submitted = True
584 return593 return
585594
=== added file 'lib/lp/registry/stories/foaf/xx-resetpassword-of-sso-account.txt'
--- lib/lp/registry/stories/foaf/xx-resetpassword-of-sso-account.txt 1970-01-01 00:00:00 +0000
+++ lib/lp/registry/stories/foaf/xx-resetpassword-of-sso-account.txt 2009-10-29 19:50:24 +0000
@@ -0,0 +1,83 @@
1= Resetting the password of SSO-only accounts =
2
3Every once in a while people come to register Launchpad accounts using email
4addresses that belong to SSO-only accounts. In these cases we explain to them
5that they should use the SSO credentials to log into Launchpad, but it's
6possible that they've forgotten their password so they'll try to use the
7'Forgotten your password?' link to reset it.
8
9Since their account is SSO-only (i.e. it has no Person record associated
10with), we can't use Launchpad's workflow for resetting the password -- it
11expects a Person entry to be associated with the given email addres. Because
12of that we'll allow the user to initiate the password-reset workflow on
13Launchpad but we'll force the use of SSO's password-reset workflow to complete
14it.
15
16 # Create a personless account to demostrate what we want.
17 >>> login(ANONYMOUS)
18 >>> account = factory.makeAccount('Test', email='test@example.com')
19 >>> logout()
20
21Trying to register will give an error, telling the user to use the SSO
22credentials to login.
23
24 >>> from lp.testing.registration import set_captcha_answer
25 >>> browser.open('http://launchpad.dev/+login')
26 >>> browser.getControl(name='loginpage_email', index=1).value = (
27 ... 'test@example.com')
28 >>> set_captcha_answer(browser, prefix='loginpage_')
29 >>> browser.getControl('Register').click()
30 >>> print_feedback_messages(browser.contents)
31 The email address test@example.com is already registered in the Launchpad
32 Login Service... Please use the same email and password to log into
33 Launchpad.
34
35But the user has forgotten their credentials, so they'll have to reset the
36password.
37
38 >>> browser.getLink('Forgotten your password?').click()
39 >>> browser.url
40 'http://launchpad.dev/+forgottenpassword'
41 >>> browser.getControl(name='email').value = 'test@example.com'
42 >>> set_captcha_answer(browser)
43 >>> browser.getControl('Request Reset').click()
44 >>> print extract_text(find_main_content(browser.contents))
45 Need a new Launchpad password?
46 We have sent you an email with instructions to reset your password.
47
48 # Get the link from the email.
49 >>> from lp.services.mail import stub
50 >>> from canonical.launchpad.ftests.logintoken import (
51 ... get_token_url_from_email)
52 >>> len(stub.test_emails)
53 1
54 >>> from_addr, to_addrs, raw_msg = stub.test_emails.pop()
55 >>> link = get_token_url_from_email(raw_msg)
56 >>> to_addrs
57 ['test@example.com']
58
59Following the link sent by email will take the user to the SSO site to
60complete the password reset.
61
62 >>> browser.open(link)
63 >>> browser.url
64 'http://openid.launchpad.dev/token/.../+resetpassword'
65 >>> browser.getControl(name='field.email').value = 'test@example.com'
66 >>> browser.getControl(name='field.password').value = 'test'
67 >>> browser.getControl(name='field.password_dupe').value = 'test'
68 >>> browser.getControl('Continue').click()
69 >>> print_feedback_messages(browser.contents)
70 Your password has been reset successfully.
71
72Since the password reset was done on SSO, the user won't be logged into
73Launchpad, so we redirect the user back to Launchpad's +login page for them to
74login.
75
76 >>> browser.url
77 'http://launchpad.dev/+login'
78 >>> browser.getControl(
79 ... 'E-mail address:', index=0).value = 'test@example.com'
80 >>> browser.getControl('Password').value = 'test'
81 >>> browser.getControl('Log In').click()
82 >>> print extract_text(find_tag_by_id(browser.contents, 'logincontrol'))
83 Test...