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

Proposed by Colin Watson on 2019-10-07
Status: Needs review
Proposed branch: ~cjwatson/launchpad:login-interstitial
Merge into: launchpad:master
Diff against target: 1078 lines (+534/-110)
10 files modified
lib/lp/app/browser/configure.zcml (+12/-0)
lib/lp/app/browser/launchpad.py (+3/-1)
lib/lp/services/webapp/login.py (+208/-41)
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)
Reviewer Review Type Date Requested Status
Launchpad code reviewers 2019-10-07 Pending
Review via email: mp+373748@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.

This is essentially the same as https://code.launchpad.net/~cjwatson/launchpad/login-interstitial/+merge/346908, converted to git and rebased on master.

To post a comment you must log in.

Unmerged commits

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

Subscribers

People subscribed via source and target branches