Merge lp:~cjwatson/launchpad/login-interstitial into lp:launchpad

Proposed by Colin Watson on 2018-05-26
Status: Needs review
Proposed branch: lp:~cjwatson/launchpad/login-interstitial
Merge into: lp:launchpad
Diff against target: 1093 lines (+537/-113)
10 files modified
lib/lp/app/browser/configure.zcml (+13/-1)
lib/lp/app/browser/launchpad.py (+4/-2)
lib/lp/services/webapp/login.py (+209/-42)
lib/lp/services/webapp/templates/login-new-account.pt (+36/-0)
lib/lp/services/webapp/templates/login-reactivate-account.pt (+43/-0)
lib/lp/services/webapp/tests/test_login.py (+153/-25)
lib/lp/testopenid/browser/server.py (+37/-25)
lib/lp/testopenid/interfaces/server.py (+7/-2)
lib/lp/testopenid/stories/logging-in.txt (+1/-0)
utilities/make-lp-user (+34/-16)
To merge this branch: bzr merge lp:~cjwatson/launchpad/login-interstitial
Reviewer Review Type Date Requested Status
Launchpad code reviewers 2018-05-26 Pending
Review via email: mp+346908@code.launchpad.net

Commit message

Add interstitial pages when creating or reactivating an account.

Description of the change

These provide an opportunity to present the user with the terms of service and privacy policy and require that they explicitly accept them, as well as making it harder to reactivate an account by accident.

To support testing this locally, I extended make-lp-user to be able to create placeholder accounts, and adjusted testopenid so that it can authenticate as an inactive account by explicitly supplying the username.

To post a comment you must log in.

Unmerged revisions

18668. By Colin Watson on 2018-05-25

Add interstitial pages when creating or reactivating an account.

These provide an opportunity to present the user with the terms of service
and privacy policy and require that they explicitly accept them, as well as
making it harder to reactivate an account by accident.

To support testing this locally, I extended make-lp-user to be able to
create placeholder accounts, and adjusted testopenid so that it can
authenticate as an inactive account by explicitly supplying the username.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/browser/configure.zcml'
2--- lib/lp/app/browser/configure.zcml 2017-09-01 12:57:34 +0000
3+++ lib/lp/app/browser/configure.zcml 2018-05-26 07:32:51 +0000
4@@ -1,4 +1,4 @@
5-<!-- Copyright 2009-2015 Canonical Ltd. This software is licensed under the
6+<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the
7 GNU Affero General Public License version 3 (see the file LICENSE).
8 -->
9
10@@ -250,6 +250,18 @@
11 permission="zope.Public"
12 name="+openid-callback"
13 />
14+ <browser:page
15+ for="lp.services.webapp.interfaces.ILaunchpadApplication"
16+ class="lp.services.webapp.login.NewAccountView"
17+ permission="zope.Public"
18+ name="+new-account"
19+ />
20+ <browser:page
21+ for="lp.services.webapp.interfaces.ILaunchpadApplication"
22+ class="lp.services.webapp.login.ReactivateAccountView"
23+ permission="zope.Public"
24+ name="+reactivate-account"
25+ />
26
27 <browser:page
28 for="*"
29
30=== modified file 'lib/lp/app/browser/launchpad.py'
31--- lib/lp/app/browser/launchpad.py 2016-06-22 21:04:30 +0000
32+++ lib/lp/app/browser/launchpad.py 2018-05-26 07:32:51 +0000
33@@ -1,4 +1,4 @@
34-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
35+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
36 # GNU Affero General Public License version 3 (see the file LICENSE).
37
38 """Browser code for the launchpad application."""
39@@ -620,7 +620,9 @@
40 @property
41 def login_shown(self):
42 return (self.user is None and
43- '+login' not in self.request['PATH_INFO'])
44+ '+login' not in self.request['PATH_INFO'] and
45+ '+new-account' not in self.request['PATH_INFO'] and
46+ '+reactivate-account' not in self.request['PATH_INFO'])
47
48 @property
49 def logged_in(self):
50
51=== modified file 'lib/lp/services/webapp/login.py'
52--- lib/lp/services/webapp/login.py 2017-01-14 15:16:36 +0000
53+++ lib/lp/services/webapp/login.py 2018-05-26 07:32:51 +0000
54@@ -1,4 +1,4 @@
55-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
56+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
57 # GNU Affero General Public License version 3 (see the file LICENSE).
58 """Stuff to do with logging in and logging out."""
59
60@@ -44,16 +44,27 @@
61 )
62
63 from lp import _
64+from lp.app.browser.launchpadform import (
65+ action,
66+ LaunchpadFormView,
67+ )
68 from lp.registry.interfaces.person import (
69+ IPerson,
70 IPersonSet,
71 PersonCreationRationale,
72 TeamEmailAddressError,
73 )
74 from lp.services.config import config
75+from lp.services.database.interfaces import IStore
76 from lp.services.database.policy import MasterDatabasePolicy
77-from lp.services.identity.interfaces.account import AccountSuspendedError
78+from lp.services.identity.interfaces.account import (
79+ AccountStatus,
80+ AccountSuspendedError,
81+ )
82+from lp.services.identity.interfaces.emailaddress import IEmailAddressSet
83 from lp.services.openid.extensions import macaroon
84 from lp.services.openid.interfaces.openidconsumer import IOpenIDConsumerStore
85+from lp.services.openid.model.openididentifier import OpenIdIdentifier
86 from lp.services.propertycache import cachedproperty
87 from lp.services.timeline.requesttimeline import get_request_timeline
88 from lp.services.webapp import canonical_url
89@@ -275,12 +286,8 @@
90 [encode_utf8(value) for value in value_list])
91
92
93-class OpenIDCallbackView(OpenIDLogin):
94- """The OpenID callback page for logging into Launchpad.
95-
96- This is the page the OpenID provider will send the user's browser to,
97- after the user has authenticated on the provider.
98- """
99+class FinishLoginMixin:
100+ """A mixin for views that can finish the login process."""
101
102 suspended_account_template = ViewPageTemplateFile(
103 'templates/login-suspended-account.pt')
104@@ -288,9 +295,6 @@
105 team_email_address_template = ViewPageTemplateFile(
106 'templates/login-team-email-address.pt')
107
108- discharge_macaroon_template = ViewPageTemplateFile(
109- 'templates/login-discharge-macaroon.pt')
110-
111 def _gather_params(self, request):
112 params = dict(request.form)
113 for key, value in request.query_string_params.iteritems():
114@@ -301,6 +305,31 @@
115
116 return params
117
118+ def login(self, person, when=None):
119+ loginsource = getUtility(IPlacelessLoginSource)
120+ # We don't have a logged in principal, so we must remove the security
121+ # proxy of the account's preferred email.
122+ email = removeSecurityProxy(person.preferredemail).email
123+ logInPrincipal(
124+ self.request, loginsource.getPrincipalByLogin(email), email, when)
125+
126+ def _redirect(self):
127+ target = self.params.get('starting_url')
128+ if target is None:
129+ target = self.request.getApplicationURL()
130+ self.request.response.redirect(target, temporary_if_possible=True)
131+
132+
133+class OpenIDCallbackView(FinishLoginMixin, OpenIDLogin):
134+ """The OpenID callback page for logging into Launchpad.
135+
136+ This is the page the OpenID provider will send the user's browser to,
137+ after the user has authenticated on the provider.
138+ """
139+
140+ discharge_macaroon_template = ViewPageTemplateFile(
141+ 'templates/login-discharge-macaroon.pt')
142+
143 def _get_requested_url(self, request):
144 requested_url = request.getURL()
145 query_string = request.get('QUERY_STRING')
146@@ -321,13 +350,28 @@
147 timeline_action.finish()
148 self.discharge_macaroon_raw = None
149
150- def login(self, person, when=None):
151- loginsource = getUtility(IPlacelessLoginSource)
152- # We don't have a logged in principal, so we must remove the security
153- # proxy of the account's preferred email.
154- email = removeSecurityProxy(person.preferredemail).email
155- logInPrincipal(
156- self.request, loginsource.getPrincipalByLogin(email), email, when)
157+ def loginInactive(self, when=None):
158+ """Log an inactive person in.
159+
160+ This isn't a normal login, which we can't do while the person is
161+ inactive. Instead, we store a few details about the OpenID response
162+ in a separate part of the session database, which lets us render an
163+ appropriate interstitial page and then activate the account properly
164+ after the form on that page is submitted.
165+ """
166+ # Force a fresh session, per bug #828638.
167+ client_id_manager = getUtility(IClientIdManager)
168+ new_client_id = client_id_manager.generateUniqueId()
169+ client_id_manager.setRequestId(self.request, new_client_id)
170+ session = ISession(self.request)
171+ authdata = session['launchpad.pendinguser']
172+ authdata['identifier'] = self._getOpenIDIdentifier()
173+ email_address, full_name = self._getEmailAddressAndFullName()
174+ authdata['email'] = email_address
175+ authdata['fullname'] = full_name
176+ if when is None:
177+ when = datetime.utcnow()
178+ authdata['logintime'] = when
179
180 @cachedproperty
181 def sreg_response(self):
182@@ -338,6 +382,10 @@
183 return macaroon.MacaroonResponse.fromSuccessResponse(
184 self.openid_response)
185
186+ def _getOpenIDIdentifier(self):
187+ identifier = self.openid_response.identity_url.split('/')[-1]
188+ return identifier.decode('ascii')
189+
190 def _getEmailAddressAndFullName(self):
191 # Here we assume the OP sent us the user's email address and
192 # full name in the response. Note we can only do that because
193@@ -356,6 +404,49 @@
194 "No email address or full name found in sreg response.")
195 return email_address, full_name
196
197+ def _maybeRedirectToInterstitial(self, openid_identifier, email_address):
198+ """Redirect to an interstitial page in some cases.
199+
200+ If there is no existing account for this OpenID identifier or email
201+ address, or if the existing account is in certain inactive states,
202+ then instead of logging in straight away we redirect to an
203+ interstitial page to confirm what the user wants to do.
204+ """
205+ redirect_view_names = {
206+ AccountStatus.DEACTIVATED: '+reactivate-account',
207+ AccountStatus.NOACCOUNT: '+new-account',
208+ AccountStatus.PLACEHOLDER: '+new-account',
209+ }
210+ identifier = IStore(OpenIdIdentifier).find(
211+ OpenIdIdentifier, identifier=openid_identifier).one()
212+ if identifier is not None:
213+ person = IPerson(identifier.account, None)
214+ else:
215+ email = getUtility(IEmailAddressSet).getByEmail(email_address)
216+ person = email.person if email is not None else None
217+
218+ if (person is None or
219+ (not person.is_team and
220+ (not person.account or
221+ person.account.status in redirect_view_names))):
222+ self.loginInactive()
223+ trust_root = allvhosts.configs['mainsite'].rooturl
224+ url = urlappend(
225+ trust_root,
226+ redirect_view_names[
227+ person.account.status if person
228+ else AccountStatus.NOACCOUNT])
229+ params = {}
230+ target = self.params.get('starting_url')
231+ if target is not None:
232+ params['starting_url'] = target
233+ if params:
234+ url += "?%s" % urllib.urlencode(params)
235+ self.request.response.redirect(url, temporary_if_possible=True)
236+ return True
237+ else:
238+ return False
239+
240 def processPositiveAssertion(self):
241 """Process an OpenID response containing a positive assertion.
242
243@@ -369,25 +460,9 @@
244 DB writes, to ensure subsequent requests use the master DB and see
245 the changes we just did.
246 """
247- identifier = self.openid_response.identity_url.split('/')[-1]
248- identifier = identifier.decode('ascii')
249+ identifier = self._getOpenIDIdentifier()
250+ email_address, full_name = self._getEmailAddressAndFullName()
251 should_update_last_write = False
252- # Force the use of the master database to make sure a lagged slave
253- # doesn't fool us into creating a Person/Account when one already
254- # exists.
255- person_set = getUtility(IPersonSet)
256- email_address, full_name = self._getEmailAddressAndFullName()
257- try:
258- person, db_updated = person_set.getOrCreateByOpenIDIdentifier(
259- identifier, email_address, full_name,
260- comment='when logging in to Launchpad.',
261- creation_rationale=(
262- PersonCreationRationale.OWNER_CREATED_LAUNCHPAD))
263- should_update_last_write = db_updated
264- except AccountSuspendedError:
265- return self.suspended_account_template()
266- except TeamEmailAddressError:
267- return self.team_email_address_template()
268
269 if self.params.get('discharge_macaroon_field'):
270 if self.macaroon_response.discharge_macaroon_raw is None:
271@@ -396,7 +471,30 @@
272 self.discharge_macaroon_raw = (
273 self.macaroon_response.discharge_macaroon_raw)
274
275+ # Force the use of the master database to make sure a lagged slave
276+ # doesn't fool us into creating a Person/Account when one already
277+ # exists.
278 with MasterDatabasePolicy():
279+ if self._maybeRedirectToInterstitial(identifier, email_address):
280+ return None
281+
282+ # XXX cjwatson 2018-05-25: We should never create a Person at
283+ # this point; any situation that would result in that should
284+ # result in a redirection to an interstitial page. Guaranteeing
285+ # that will require a bit more refactoring, though.
286+ person_set = getUtility(IPersonSet)
287+ try:
288+ person, db_updated = person_set.getOrCreateByOpenIDIdentifier(
289+ identifier, email_address, full_name,
290+ comment='when logging in to Launchpad.',
291+ creation_rationale=(
292+ PersonCreationRationale.OWNER_CREATED_LAUNCHPAD))
293+ should_update_last_write = db_updated
294+ except AccountSuspendedError:
295+ return self.suspended_account_template()
296+ except TeamEmailAddressError:
297+ return self.team_email_address_template()
298+
299 self.login(person)
300
301 if self.params.get('discharge_macaroon_field'):
302@@ -443,12 +541,6 @@
303 transaction.commit()
304 return retval
305
306- def _redirect(self):
307- target = self.params.get('starting_url')
308- if target is None:
309- target = self.request.getApplicationURL()
310- self.request.response.redirect(target, temporary_if_possible=True)
311-
312
313 class OpenIDLoginErrorView(LaunchpadView):
314
315@@ -471,6 +563,81 @@
316 self.login_error = "Unknown error: %s" % openid_response
317
318
319+class FinishLoginInterstitialView(FinishLoginMixin, LaunchpadFormView):
320+
321+ class schema(Interface):
322+ pass
323+
324+ def initialize(self):
325+ self.params = self._gather_params(self.request)
326+ super(FinishLoginInterstitialView, self).initialize()
327+
328+ def _accept(self):
329+ session = ISession(self.request)
330+ authdata = session['launchpad.pendinguser']
331+ try:
332+ identifier = authdata['identifier']
333+ email_address = authdata['email']
334+ full_name = authdata['fullname']
335+ except KeyError:
336+ return OpenIDLoginErrorView(
337+ self.context, self.request,
338+ login_error=(
339+ "Your session expired. Please try logging in again."))
340+ should_update_last_write = False
341+
342+ # Force the use of the master database to make sure a lagged slave
343+ # doesn't fool us into creating a Person/Account when one already
344+ # exists.
345+ with MasterDatabasePolicy():
346+ person_set = getUtility(IPersonSet)
347+ try:
348+ person, db_updated = person_set.getOrCreateByOpenIDIdentifier(
349+ identifier, email_address, full_name,
350+ comment=(
351+ 'when logging in to Launchpad, after accepting '
352+ 'terms.'),
353+ creation_rationale=(
354+ PersonCreationRationale.OWNER_CREATED_LAUNCHPAD))
355+ should_update_last_write = db_updated
356+ except AccountSuspendedError:
357+ return self.suspended_account_template()
358+ except TeamEmailAddressError:
359+ return self.team_email_address_template()
360+
361+ self.login(person)
362+
363+ if should_update_last_write:
364+ # This is a GET request but we changed the database, so update
365+ # session_data['last_write'] to make sure further requests use
366+ # the master DB and thus see the changes we've just made.
367+ session_data = ISession(self.request)['lp.dbpolicy']
368+ session_data['last_write'] = datetime.utcnow()
369+ self._redirect()
370+ # No need to return anything as we redirect above.
371+ return None
372+
373+
374+class NewAccountView(FinishLoginInterstitialView):
375+
376+ page_title = label = 'Welcome to Launchpad!'
377+ template = ViewPageTemplateFile('templates/login-new-account.pt')
378+
379+ @action('Accept terms and create account', name='accept')
380+ def accept(self, action, data):
381+ return self._accept()
382+
383+
384+class ReactivateAccountView(FinishLoginInterstitialView):
385+
386+ page_title = label = 'Welcome back to Launchpad!'
387+ template = ViewPageTemplateFile('templates/login-reactivate-account.pt')
388+
389+ @action('Accept terms and reactivate account', name='accept')
390+ def accept(self, action, data):
391+ return self._accept()
392+
393+
394 class AlreadyLoggedInView(LaunchpadView):
395
396 page_title = 'Already logged in'
397
398=== added file 'lib/lp/services/webapp/templates/login-new-account.pt'
399--- lib/lp/services/webapp/templates/login-new-account.pt 1970-01-01 00:00:00 +0000
400+++ lib/lp/services/webapp/templates/login-new-account.pt 2018-05-26 07:32:51 +0000
401@@ -0,0 +1,36 @@
402+<html
403+ xmlns="http://www.w3.org/1999/xhtml"
404+ xmlns:tal="http://xml.zope.org/namespaces/tal"
405+ xmlns:metal="http://xml.zope.org/namespaces/metal"
406+ xmlns:i18n="http://xml.zope.org/namespaces/i18n"
407+ metal:use-macro="view/macro:page/main_only"
408+ i18n:domain="launchpad">
409+
410+ <body>
411+ <div class="top-portlet" metal:fill-slot="main">
412+ <div metal:use-macro="context/@@launchpad_form/form">
413+ <div metal:fill-slot="extra_info">
414+ <p>
415+ Please read and accept the
416+ <a href="/legal">Launchpad terms of service</a> and the
417+ <a href="https://www.ubuntu.com/legal/dataprivacy">data privacy
418+ policy</a> before continuing. If you accept these terms,
419+ Launchpad will create an account for you.
420+ </p>
421+
422+ <p>
423+ You may also
424+ <a tal:attributes="href view/params/starting_url|string:/">return
425+ to Launchpad without creating an account</a>.
426+ </p>
427+
428+ <input
429+ type="hidden"
430+ name="starting_url"
431+ tal:condition="view/params/starting_url|nothing"
432+ tal:attributes="value view/params/starting_url" />
433+ </div>
434+ </div>
435+ </div>
436+ </body>
437+</html>
438
439=== added file 'lib/lp/services/webapp/templates/login-reactivate-account.pt'
440--- lib/lp/services/webapp/templates/login-reactivate-account.pt 1970-01-01 00:00:00 +0000
441+++ lib/lp/services/webapp/templates/login-reactivate-account.pt 2018-05-26 07:32:51 +0000
442@@ -0,0 +1,43 @@
443+<html
444+ xmlns="http://www.w3.org/1999/xhtml"
445+ xmlns:tal="http://xml.zope.org/namespaces/tal"
446+ xmlns:metal="http://xml.zope.org/namespaces/metal"
447+ xmlns:i18n="http://xml.zope.org/namespaces/i18n"
448+ metal:use-macro="view/macro:page/main_only"
449+ i18n:domain="launchpad">
450+
451+ <body>
452+ <div class="top-portlet" metal:fill-slot="main">
453+ <div metal:use-macro="context/@@launchpad_form/form">
454+ <div metal:fill-slot="extra_info">
455+ <p>
456+ Please read and accept the
457+ <a href="/legal">Launchpad terms of service</a> and the
458+ <a href="https://www.ubuntu.com/legal/dataprivacy">data privacy
459+ policy</a> before continuing. If you accept these terms,
460+ Launchpad will reactivate your account.
461+ </p>
462+
463+ <p>
464+ Reactivating your account will not restore any information that
465+ was deleted when you deactivated it. You may start receiving
466+ email notifications again related to information that was not
467+ deleted, such as changes to any bugs you reported.
468+ </p>
469+
470+ <p>
471+ You may also
472+ <a tal:attributes="href view/params/starting_url|string:/">return
473+ to Launchpad without reactivating your account</a>.
474+ </p>
475+
476+ <input
477+ type="hidden"
478+ name="starting_url"
479+ tal:condition="view/params/starting_url|nothing"
480+ tal:attributes="value view/params/starting_url" />
481+ </div>
482+ </div>
483+ </div>
484+ </body>
485+</html>
486
487=== modified file 'lib/lp/services/webapp/tests/test_login.py'
488--- lib/lp/services/webapp/tests/test_login.py 2018-01-02 16:10:26 +0000
489+++ lib/lp/services/webapp/tests/test_login.py 2018-05-26 07:32:51 +0000
490@@ -1,4 +1,4 @@
491-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
492+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
493 # GNU Affero General Public License version 3 (see the file LICENSE).
494 """Test harness for running the new-login.txt tests."""
495
496@@ -64,8 +64,10 @@
497 from lp.services.timeline.requesttimeline import get_request_timeline
498 from lp.services.webapp.interfaces import ILaunchpadApplication
499 from lp.services.webapp.login import (
500+ NewAccountView,
501 OpenIDCallbackView,
502 OpenIDLogin,
503+ ReactivateAccountView,
504 )
505 from lp.services.webapp.servers import LaunchpadTestRequest
506 from lp.testing import (
507@@ -106,11 +108,11 @@
508 self.discharge_macaroon_raw = discharge_macaroon_raw
509
510
511-class StubbedOpenIDCallbackView(OpenIDCallbackView):
512+class StubLoginMixin:
513 login_called = False
514
515 def login(self, account):
516- super(StubbedOpenIDCallbackView, self).login(account)
517+ super(StubLoginMixin, self).login(account)
518 self.login_called = True
519 current_policy = getUtility(IStoreSelector).get_current()
520 if not isinstance(current_policy, MasterDatabasePolicy):
521@@ -118,6 +120,18 @@
522 "Not using the master store: %s" % current_policy)
523
524
525+class StubbedOpenIDCallbackView(StubLoginMixin, OpenIDCallbackView):
526+ pass
527+
528+
529+class StubbedNewAccountView(StubLoginMixin, NewAccountView):
530+ pass
531+
532+
533+class StubbedReactivateAccountView(StubLoginMixin, ReactivateAccountView):
534+ pass
535+
536+
537 class FakeConsumer:
538 """An OpenID consumer that stashes away arguments for test inspection."""
539
540@@ -212,24 +226,27 @@
541 openid_response, view_class=view_class)
542
543 def _createAndRenderView(self, response,
544- view_class=StubbedOpenIDCallbackView, form=None):
545+ view_class=StubbedOpenIDCallbackView, form=None,
546+ method='GET', **kwargs):
547 if form is None:
548 form = {'starting_url': 'http://launchpad.dev/after-login'}
549- request = LaunchpadTestRequest(form=form, environ={'PATH_INFO': '/'})
550+ request = LaunchpadTestRequest(
551+ form=form, environ={'PATH_INFO': '/'}, method=method, **kwargs)
552 # The layer we use sets up an interaction (by calling login()), but we
553 # want to use our own request in the interaction, so we logout() and
554 # setup a newInteraction() using our request.
555 logout()
556 newInteraction(request)
557 view = view_class(object(), request)
558- view.initialize()
559- view.openid_response = response
560 # Monkey-patch getByOpenIDIdentifier() to make sure the view uses the
561 # master DB. This mimics the problem we're trying to avoid, where
562 # getByOpenIDIdentifier() doesn't find a newly created account because
563 # it looks in the slave database.
564 with IAccountSet_getByOpenIDIdentifier_monkey_patched():
565- html = view.render()
566+ view.initialize()
567+ if response is not None:
568+ view.openid_response = response
569+ html = view.render() if method == 'GET' else None
570 return view, html
571
572 def test_full_fledged_account(self):
573@@ -328,9 +345,7 @@
574
575 def test_unseen_identity(self):
576 # When we get a positive assertion about an identity URL we've never
577- # seen, we automatically register an account with that identity
578- # because someone who registered on login.lp.net or login.u.c should
579- # be able to login here without any further steps.
580+ # seen, we redirect to a confirmation page.
581 identifier = u'4w7kmzU'
582 account_set = getUtility(IAccountSet)
583 self.assertRaises(
584@@ -340,15 +355,47 @@
585 email='non-existent@example.com', full_name='Foo User')
586 with SRegResponse_fromSuccessResponse_stubbed():
587 view, html = self._createAndRenderView(openid_response)
588+ self.assertFalse(view.login_called)
589+ response = view.request.response
590+ self.assertEqual(httplib.TEMPORARY_REDIRECT, response.getStatus())
591+ self.assertEqual(
592+ 'http://launchpad.dev/+new-account?' + urllib.urlencode(
593+ {'starting_url': view.request.form['starting_url']}),
594+ response.getHeader('Location'))
595+ self.assertRaises(
596+ LookupError, account_set.getByOpenIDIdentifier, identifier)
597+ self.assertThat(
598+ ISession(view.request)['launchpad.pendinguser'],
599+ ContainsDict({
600+ 'identifier': Equals(identifier),
601+ 'email': Equals('non-existent@example.com'),
602+ 'fullname': Equals('Foo User'),
603+ }))
604+
605+ # If the user accepts, we automatically register an account with
606+ # that identity, since its existence on SSO is good enough for us.
607+ cookie = response.getCookie('launchpad_tests')['value']
608+ view, _ = self._createAndRenderView(
609+ None, view_class=StubbedNewAccountView,
610+ form={
611+ 'starting_url': view.request.form['starting_url'],
612+ 'field.actions.accept': 'Accept terms and create account',
613+ },
614+ method='POST', HTTP_COOKIE='launchpad_tests=%s' % cookie)
615 self.assertTrue(view.login_called)
616+ response = view.request.response
617+ self.assertEqual(httplib.SEE_OTHER, response.getStatus())
618+ self.assertEqual(
619+ view.request.form['starting_url'], response.getHeader('Location'))
620 account = account_set.getByOpenIDIdentifier(identifier)
621- self.assertIsNot(None, account)
622+ self.assertIsNotNone(account)
623 self.assertEqual(AccountStatus.ACTIVE, account.status)
624 person = IPerson(account, None)
625- self.assertIsNot(None, person)
626+ self.assertIsNotNone(person)
627 self.assertEqual('Foo User', person.displayname)
628- self.assertEqual('non-existent@example.com',
629- removeSecurityProxy(person.preferredemail).email)
630+ self.assertEqual(
631+ 'non-existent@example.com',
632+ removeSecurityProxy(person.preferredemail).email)
633
634 # We also update the last_write flag in the session, to make sure
635 # further requests use the master DB and thus see the newly created
636@@ -363,8 +410,7 @@
637 email = 'test@example.com'
638 person = self.factory.makePerson(
639 displayname='Test account', email=email,
640- account_status=AccountStatus.DEACTIVATED,
641- email_address_status=EmailAddressStatus.NEW)
642+ account_status=AccountStatus.NOACCOUNT)
643 account = person.account
644 account_set = getUtility(IAccountSet)
645 self.assertRaises(
646@@ -374,7 +420,38 @@
647 email=email, full_name='Foo User')
648 with SRegResponse_fromSuccessResponse_stubbed():
649 view, html = self._createAndRenderView(openid_response)
650+ self.assertFalse(view.login_called)
651+ response = view.request.response
652+ self.assertEqual(httplib.TEMPORARY_REDIRECT, response.getStatus())
653+ self.assertEqual(
654+ 'http://launchpad.dev/+new-account?' + urllib.urlencode(
655+ {'starting_url': view.request.form['starting_url']}),
656+ response.getHeader('Location'))
657+ self.assertRaises(
658+ LookupError, account_set.getByOpenIDIdentifier, identifier)
659+ self.assertEqual(AccountStatus.NOACCOUNT, account.status)
660+ self.assertThat(
661+ ISession(view.request)['launchpad.pendinguser'],
662+ ContainsDict({
663+ 'identifier': Equals(identifier),
664+ 'email': Equals('test@example.com'),
665+ 'fullname': Equals('Foo User'),
666+ }))
667+
668+ # Accept the terms and proceed.
669+ cookie = response.getCookie('launchpad_tests')['value']
670+ view, _ = self._createAndRenderView(
671+ None, view_class=StubbedNewAccountView,
672+ form={
673+ 'starting_url': view.request.form['starting_url'],
674+ 'field.actions.accept': 'Accept terms and create account',
675+ },
676+ method='POST', HTTP_COOKIE='launchpad_tests=%s' % cookie)
677 self.assertTrue(view.login_called)
678+ response = view.request.response
679+ self.assertEqual(httplib.SEE_OTHER, response.getStatus())
680+ self.assertEqual(
681+ view.request.form['starting_url'], response.getHeader('Location'))
682
683 # The existing accounts had a new openid_identifier added, the
684 # account was reactivated and its preferred email was set, but
685@@ -386,7 +463,7 @@
686 self.assertEqual(
687 email, removeSecurityProxy(person.preferredemail).email)
688 person = IPerson(account, None)
689- self.assertIsNot(None, person)
690+ self.assertIsNotNone(person)
691 self.assertEqual('Test account', person.displayname)
692
693 # We also update the last_write flag in the session, to make sure
694@@ -396,7 +473,7 @@
695
696 def test_deactivated_account(self):
697 # The user has the account's password and is trying to login, so we'll
698- # just re-activate their account.
699+ # redirect them to a confirmation page.
700 email = 'foo@example.com'
701 person = self.factory.makePerson(
702 displayname='Test account', email=email,
703@@ -409,11 +486,37 @@
704 status=SUCCESS, email=email, full_name=person.displayname)
705 with SRegResponse_fromSuccessResponse_stubbed():
706 view, html = self._createAndRenderView(openid_response)
707+ self.assertFalse(view.login_called)
708+ response = view.request.response
709+ self.assertEqual(httplib.TEMPORARY_REDIRECT, response.getStatus())
710+ self.assertEqual(
711+ 'http://launchpad.dev/+reactivate-account?' + urllib.urlencode(
712+ {'starting_url': view.request.form['starting_url']}),
713+ response.getHeader('Location'))
714+ self.assertEqual(AccountStatus.DEACTIVATED, person.account.status)
715+ self.assertIsNone(person.preferredemail)
716+ self.assertThat(
717+ ISession(view.request)['launchpad.pendinguser'],
718+ ContainsDict({
719+ 'identifier': Equals(openid_identifier),
720+ 'email': Equals(email),
721+ 'fullname': Equals('Test account'),
722+ }))
723+
724+ # If the user confirms the reactivation, we do it.
725+ cookie = response.getCookie('launchpad_tests')['value']
726+ view, _ = self._createAndRenderView(
727+ None, view_class=StubbedReactivateAccountView,
728+ form={
729+ 'starting_url': view.request.form['starting_url'],
730+ 'field.actions.accept': 'Accept terms and reactivate account',
731+ },
732+ method='POST', HTTP_COOKIE='launchpad_tests=%s' % cookie)
733 self.assertTrue(view.login_called)
734 response = view.request.response
735- self.assertEqual(httplib.TEMPORARY_REDIRECT, response.getStatus())
736- self.assertEqual(view.request.form['starting_url'],
737- response.getHeader('Location'))
738+ self.assertEqual(httplib.SEE_OTHER, response.getStatus())
739+ self.assertEqual(
740+ view.request.form['starting_url'], response.getHeader('Location'))
741 self.assertEqual(AccountStatus.ACTIVE, person.account.status)
742 self.assertEqual(email, person.preferredemail.email)
743 # We also update the last_write flag in the session, to make sure
744@@ -423,12 +526,11 @@
745
746 def test_never_used_account(self):
747 # The account was created by one of our scripts but was never
748- # activated, so we just activate it.
749+ # activated, so we redirect to a confirmation page.
750 email = 'foo@example.com'
751 person = self.factory.makePerson(
752 displayname='Test account', email=email,
753- account_status=AccountStatus.DEACTIVATED,
754- email_address_status=EmailAddressStatus.NEW)
755+ account_status=AccountStatus.NOACCOUNT)
756 openid_identifier = IStore(OpenIdIdentifier).find(
757 OpenIdIdentifier.identifier,
758 OpenIdIdentifier.account_id == person.account.id).order_by(
759@@ -439,6 +541,32 @@
760 status=SUCCESS, email=email, full_name=person.displayname)
761 with SRegResponse_fromSuccessResponse_stubbed():
762 view, html = self._createAndRenderView(openid_response)
763+ self.assertFalse(view.login_called)
764+ response = view.request.response
765+ self.assertEqual(httplib.TEMPORARY_REDIRECT, response.getStatus())
766+ self.assertEqual(
767+ 'http://launchpad.dev/+new-account?' + urllib.urlencode(
768+ {'starting_url': view.request.form['starting_url']}),
769+ response.getHeader('Location'))
770+ self.assertEqual(AccountStatus.NOACCOUNT, person.account.status)
771+ self.assertIsNone(person.preferredemail)
772+ self.assertThat(
773+ ISession(view.request)['launchpad.pendinguser'],
774+ ContainsDict({
775+ 'identifier': Equals(openid_identifier),
776+ 'email': Equals(email),
777+ 'fullname': Equals('Test account'),
778+ }))
779+
780+ # If the user confirms the activation, we do it.
781+ cookie = response.getCookie('launchpad_tests')['value']
782+ view, _ = self._createAndRenderView(
783+ None, view_class=StubbedNewAccountView,
784+ form={
785+ 'starting_url': view.request.form['starting_url'],
786+ 'field.actions.accept': 'Accept terms and create account',
787+ },
788+ method='POST', HTTP_COOKIE='launchpad_tests=%s' % cookie)
789 self.assertTrue(view.login_called)
790 self.assertEqual(AccountStatus.ACTIVE, person.account.status)
791 self.assertEqual(email, person.preferredemail.email)
792
793=== modified file 'lib/lp/testopenid/browser/server.py'
794--- lib/lp/testopenid/browser/server.py 2015-07-08 16:05:11 +0000
795+++ lib/lp/testopenid/browser/server.py 2018-05-26 07:32:51 +0000
796@@ -1,4 +1,4 @@
797-# Copyright 2010-2011 Canonical Ltd. This software is licensed under the
798+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
799 # GNU Affero General Public License version 3 (see the file LICENSE).
800
801 """Test OpenID server."""
802@@ -30,7 +30,10 @@
803 from zope.authentication.interfaces import IUnauthenticatedPrincipal
804 from zope.component import getUtility
805 from zope.interface import implementer
806-from zope.security.proxy import isinstance as zisinstance
807+from zope.security.proxy import (
808+ isinstance as zisinstance,
809+ removeSecurityProxy,
810+ )
811 from zope.session.interfaces import ISession
812
813 from lp import _
814@@ -39,11 +42,11 @@
815 LaunchpadFormView,
816 )
817 from lp.app.errors import UnexpectedFormData
818-from lp.registry.interfaces.person import IPerson
819-from lp.services.identity.interfaces.account import (
820- AccountStatus,
821- IAccountSet,
822+from lp.registry.interfaces.person import (
823+ IPerson,
824+ IPersonSet,
825 )
826+from lp.services.identity.interfaces.account import IAccountSet
827 from lp.services.openid.browser.openiddiscovery import (
828 XRDSContentNegotiationMixin,
829 )
830@@ -52,13 +55,9 @@
831 get_property_cache,
832 )
833 from lp.services.webapp import LaunchpadView
834-from lp.services.webapp.interfaces import (
835- ICanonicalUrlData,
836- IPlacelessLoginSource,
837- )
838+from lp.services.webapp.interfaces import ICanonicalUrlData
839 from lp.services.webapp.login import (
840 allowUnauthenticatedSession,
841- logInPrincipal,
842 logoutPerson,
843 )
844 from lp.services.webapp.publisher import (
845@@ -105,7 +104,7 @@
846 account = getUtility(IAccountSet).getByOpenIDIdentifier(name)
847 except LookupError:
848 account = None
849- if account is None or account.status != AccountStatus.ACTIVE:
850+ if account is None:
851 return None
852 return ITestOpenIDPersistentIdentity(account)
853
854@@ -206,7 +205,7 @@
855 response.setHeader(header, value)
856 return webresponse.body
857
858- def createPositiveResponse(self):
859+ def createPositiveResponse(self, email):
860 """Create a positive assertion OpenIDResponse.
861
862 This method should be called to create the response to
863@@ -233,7 +232,7 @@
864 person = IPerson(self.account)
865 sreg_fields = dict(
866 nickname=person.name,
867- email=person.preferredemail.email,
868+ email=email,
869 fullname=self.account.displayname)
870 sreg_request = SRegRequest.fromOpenIDRequest(self.openid_request)
871 sreg_response = SRegResponse.extractResponse(
872@@ -306,21 +305,34 @@
873
874 def validate(self, data):
875 """Check that the email address is valid for login."""
876- loginsource = getUtility(IPlacelessLoginSource)
877- principal = loginsource.getPrincipalByLogin(data['email'])
878- if principal is None:
879- self.addError(
880- _("Unknown email address."))
881+ if data.get('username'):
882+ person = getUtility(IPersonSet).getByName(data['username'])
883+ if person is None:
884+ self.setFieldError('username', _("Unknown username."))
885+ elif person.preferredemail is not None:
886+ email = removeSecurityProxy(person.preferredemail).email
887+ if email != data['email']:
888+ self.setFieldError(
889+ 'email',
890+ _("Email address for user '%s' is '%s', not '%s'.") %
891+ (data['username'], email, data['email']))
892+ elif getUtility(IPersonSet).getByEmail(data['email']) is None:
893+ self.setFieldError('email', _("Unknown email address."))
894
895 @action('Continue', name='continue')
896 def continue_action(self, action, data):
897 email = data['email']
898- principal = getUtility(IPlacelessLoginSource).getPrincipalByLogin(
899- email)
900- logInPrincipal(self.request, principal, email)
901- # Update the attribute holding the cached user.
902- self._account = principal.account
903- return self.renderOpenIDResponse(self.createPositiveResponse())
904+ username = data.get('username')
905+ if username is not None:
906+ person = getUtility(IPersonSet).getByName(username)
907+ else:
908+ person = getUtility(IPersonSet).getByEmail(email)
909+ # Update the attribute holding the cached user. This fakes a login;
910+ # we don't do a true login here, because we can get away without it
911+ # and it allows testing the case of logging in as a user whose
912+ # account status is not ACTIVE.
913+ self._account = person.account
914+ return self.renderOpenIDResponse(self.createPositiveResponse(email))
915
916
917 class PersistentIdentityView(
918
919=== modified file 'lib/lp/testopenid/interfaces/server.py'
920--- lib/lp/testopenid/interfaces/server.py 2015-07-21 09:04:01 +0000
921+++ lib/lp/testopenid/interfaces/server.py 2018-05-26 07:32:51 +0000
922@@ -1,4 +1,4 @@
923-# Copyright 2010 Canonical Ltd. This software is licensed under the
924+# Copyright 2010-2018 Canonical Ltd. This software is licensed under the
925 # GNU Affero General Public License version 3 (see the file LICENSE).
926
927 __metaclass__ = type
928@@ -23,7 +23,12 @@
929
930
931 class ITestOpenIDLoginForm(Interface):
932- email = TextLine(title=u'What is your email address?', required=True)
933+ email = TextLine(title=u'Your email address', required=True)
934+ username = TextLine(
935+ title=u'Your username', required=False,
936+ description=(
937+ u'This is only required if you are logging into a placeholder '
938+ u'account for the first time.'))
939
940
941 class ITestOpenIDPersistentIdentity(IOpenIDPersistentIdentity):
942
943=== modified file 'lib/lp/testopenid/stories/logging-in.txt'
944--- lib/lp/testopenid/stories/logging-in.txt 2012-01-14 12:47:20 +0000
945+++ lib/lp/testopenid/stories/logging-in.txt 2018-05-26 07:32:51 +0000
946@@ -42,6 +42,7 @@
947 >>> for tag in find_tags_by_class(browser.contents, 'error'):
948 ... print extract_text(tag)
949 There is 1 error.
950+ Your email address:
951 Unknown email address.
952
953 If the email address matches an account, the user is logged in and
954
955=== modified file 'utilities/make-lp-user'
956--- utilities/make-lp-user 2015-05-07 09:29:30 +0000
957+++ utilities/make-lp-user 2018-05-26 07:32:51 +0000
958@@ -1,6 +1,6 @@
959 #!/usr/bin/python -S
960 #
961-# Copyright 2009-2010 Canonical Ltd. This software is licensed under the
962+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
963 # GNU Affero General Public License version 3 (see the file LICENSE).
964
965 """Create a user for testing the local Launchpad.
966@@ -29,6 +29,8 @@
967 production environments.
968 """
969
970+from __future__ import absolute_import, print_function
971+
972 import _pythonpath
973
974 from optparse import OptionParser
975@@ -49,6 +51,7 @@
976 GPGKeyAlgorithm,
977 IGPGHandler,
978 )
979+from lp.services.identity.interfaces.account import AccountStatus
980 from lp.services.scripts import execute_zcml_for_scripts
981 from lp.services.timeout import set_default_timeout_function
982 from lp.testing.factory import LaunchpadObjectFactory
983@@ -58,14 +61,23 @@
984
985 set_default_timeout_function(lambda: 100)
986
987+
988 def make_person(username, email):
989 """Create and return a person with the given username.
990
991 The email address for the user will be <username>@example.com.
992 """
993 person = factory.makePerson(name=username, email=email)
994- print "username: %s" % (username,)
995- print "email: %s" % (email,)
996+ print("username: %s" % (username,))
997+ print("email: %s" % (email,))
998+ return person
999+
1000+
1001+def make_placeholder_person(username):
1002+ """Create and return a placeholder person with the given username."""
1003+ person = factory.makePerson(
1004+ name=username, account_status=AccountStatus.PLACEHOLDER)
1005+ print("username: %s" % (username,))
1006 return person
1007
1008
1009@@ -82,15 +94,15 @@
1010 for team_name in team_names:
1011 team = person_set.getByName(team_name)
1012 if team is None:
1013- print "ERROR: %s not found." % (team_name,)
1014+ print("ERROR: %s not found." % (team_name,))
1015 continue
1016 if not team.is_team:
1017- print "ERROR: %s is not a team." % (team_name,)
1018+ print("ERROR: %s is not a team." % (team_name,))
1019 continue
1020 team.addMember(
1021 person, person, status=TeamMembershipStatus.APPROVED)
1022 teams_joined.append(team_name)
1023- print "teams: %s" % ' '.join(teams_joined)
1024+ print("teams: %s" % ' '.join(teams_joined))
1025
1026
1027 def add_ssh_public_keys(person):
1028@@ -111,10 +123,10 @@
1029 except (OSError, IOError):
1030 continue
1031 key_set.new(person, public_key)
1032- print 'Registered SSH key: %s' % (filename,)
1033+ print('Registered SSH key: %s' % (filename,))
1034 break
1035 else:
1036- print 'No SSH key files found in %s' % ssh_dir
1037+ print('No SSH key files found in %s' % ssh_dir)
1038
1039
1040 def parse_fingerprints(gpg_output):
1041@@ -143,7 +155,7 @@
1042 command_line, env=env, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
1043 stdout, stderr = pipe.communicate()
1044 if stderr != '':
1045- print stderr
1046+ print(stderr)
1047 if pipe.returncode != 0:
1048 raise Exception('GPG error during "%s"' % ' '.join(command_line))
1049
1050@@ -179,7 +191,7 @@
1051
1052 fingerprints = parse_fingerprints(output)
1053 if len(fingerprints) == 0:
1054- print "No GPG key fingerprints found!"
1055+ print("No GPG key fingerprints found!")
1056 for fingerprint in fingerprints:
1057 add_gpg_key(person, fingerprint)
1058
1059@@ -194,10 +206,13 @@
1060 parser.add_option(
1061 '-e', '--email', action='store', dest='email', default=None,
1062 help="Email address; set to use real GPG key for this address.")
1063+ parser.add_option(
1064+ '--placeholder', action='store_true', default=False,
1065+ help="Create a placeholder account rather than a full user account.")
1066
1067 options, args = parser.parse_args(arguments)
1068 if len(args) == 0:
1069- print __doc__
1070+ print(__doc__)
1071 sys.exit(2)
1072
1073 options.username = args[0]
1074@@ -217,12 +232,15 @@
1075 execute_zcml_for_scripts()
1076 transaction.begin()
1077
1078- person = make_person(options.username, email)
1079- add_person_to_teams(person, options.teams)
1080- add_ssh_public_keys(person)
1081+ if options.placeholder:
1082+ make_placeholder_person(options.username)
1083+ else:
1084+ person = make_person(options.username, email)
1085+ add_person_to_teams(person, options.teams)
1086+ add_ssh_public_keys(person)
1087
1088- if options.email is not None:
1089- attach_gpg_keys(options.email, person)
1090+ if options.email is not None:
1091+ attach_gpg_keys(options.email, person)
1092
1093 transaction.commit()
1094