Launchpad itself

Merge lp:~wgrant/launchpad/invisible-person into lp:launchpad

  1. invisible-person
  2. Merge into devel
Proposed by William Grant
Status: Merged
Merged at revision: 18008
Proposed branch: lp:~wgrant/launchpad/invisible-person
Merge into: lp:launchpad
Diff against target: 251 lines (+111/-8)
7 files modified
lib/lp/app/browser/launchpad.py (+4/-1)
lib/lp/app/browser/tests/test_launchpad.py (+17/-0)
lib/lp/registry/interfaces/person.py (+21/-0)
lib/lp/registry/model/person.py (+18/-1)
lib/lp/registry/tests/test_personset.py (+23/-1)
lib/lp/services/identity/interfaces/account.py (+23/-2)
lib/lp/services/identity/model/account.py (+5/-3)
To merge this branch: bzr merge lp:~wgrant/launchpad/invisible-person
Reviewer Review Type Date Requested Status
Colin Watson (community) Approve
Review via email: mp+292940@code.launchpad.net

Commit message

Add AccountStatus.PLACEHOLDER to give usernames to SSO-only accounts.

Description of the change

Add AccountStatus.PLACEHOLDER to give usernames to SSO-only accounts. These accounts are invisible to mortals.

Username mastery should eventually move to SSO, but it is more expedient to create invisible placeholder Person records in LP for now.

To post a comment you must log in.
Revision history for this message
Colin Watson (cjwatson) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/app/browser/launchpad.py'
2--- lib/lp/app/browser/launchpad.py 2015-08-06 00:46:39 +0000
3+++ lib/lp/app/browser/launchpad.py 2016-04-26 13:09:54 +0000
4@@ -857,12 +857,15 @@
5 # team membership or Launchpad administration.
6 if (person.is_team and
7 not check_permission('launchpad.LimitedView', person)):
8- raise NotFound(self.context, name)
9+ return None
10 # Only admins are permitted to see suspended users.
11 if person.account_status == AccountStatus.SUSPENDED:
12 if not check_permission('launchpad.Moderate', person):
13 raise GoneError(
14 'User is suspended: %s' % name)
15+ if person.account_status == AccountStatus.PLACEHOLDER:
16+ if not check_permission('launchpad.Moderate', person):
17+ return None
18 return person
19
20 # Dapper and Edgy shipped with https://launchpad.net/bazaar hard coded
21
22=== modified file 'lib/lp/app/browser/tests/test_launchpad.py'
23--- lib/lp/app/browser/tests/test_launchpad.py 2015-01-07 00:35:41 +0000
24+++ lib/lp/app/browser/tests/test_launchpad.py 2016-04-26 13:09:54 +0000
25@@ -370,6 +370,23 @@
26 login_person(self.any_user)
27 self.assertRaises(GoneError, self.traverse, segment, segment)
28
29+ def test_placeholder_person_visibility(self):
30+ # Verify a placeholder user is only traversable by an admin.
31+ name = u'placeholder-person'
32+ person = getUtility(IPersonSet).createPlaceholderPerson(name, name)
33+ login_person(self.admin)
34+ segment = '~%s' % name
35+ # Admins can see the placeholder user.
36+ traversed = self.traverse(segment, segment)
37+ self.assertEqual(person, traversed)
38+ # Registry experts can see the placeholder user.
39+ login_person(self.registry_expert)
40+ traversed = self.traverse(segment, segment)
41+ self.assertEqual(person, traversed)
42+ # Regular users cannot see the placeholder user.
43+ login_person(self.any_user)
44+ self.assertRaises(NotFound, self.traverse, segment, segment)
45+
46 def test_public_team(self):
47 # Verify a public team is returned.
48 name = 'public-team'
49
50=== modified file 'lib/lp/registry/interfaces/person.py'
51--- lib/lp/registry/interfaces/person.py 2016-03-18 03:30:26 +0000
52+++ lib/lp/registry/interfaces/person.py 2016-04-26 13:09:54 +0000
53@@ -406,6 +406,14 @@
54 and commercial archive) was made via Software Center.
55 """)
56
57+ USERNAME_PLACEHOLDER = DBItem(17, """
58+ Created by setting a username in SSO.
59+
60+ Somebody without a Launchpad account set their username in SSO.
61+ Since SSO doesn't store usernames directly, an invisible
62+ placeholder Launchpad account is required.
63+ """)
64+
65
66 class PersonNameField(BlacklistableContentNameField):
67 """A `Person` team name, which is unique and performs psuedo blacklisting.
68@@ -2117,6 +2125,19 @@
69 used.
70 """
71
72+ def createPlaceholderPerson(openid_identifier, name):
73+ """Create and return an SSO username placeholder `IPerson`.
74+
75+ The returned Person will have no email address, just a username and an
76+ OpenID identifier.
77+
78+ :param openid_identifier: The SSO account's OpenID suffix.
79+ :param name: The person's name.
80+ :raises InvalidName: When the passed name isn't valid.
81+ :raises NameAlreadyTaken: When the passed name has already been
82+ used.
83+ """
84+
85 def ensurePerson(email, displayname, rationale, comment=None,
86 registrant=None):
87 """Make sure that there is a person in the database with the given
88
89=== modified file 'lib/lp/registry/model/person.py'
90--- lib/lp/registry/model/person.py 2016-03-01 13:02:42 +0000
91+++ lib/lp/registry/model/person.py 2016-04-26 13:09:54 +0000
92@@ -3397,7 +3397,13 @@
93 raise NameAlreadyTaken(
94 "The account matching the identifier is inactive.")
95 elif person.account.status in [AccountStatus.DEACTIVATED,
96- AccountStatus.NOACCOUNT]:
97+ AccountStatus.NOACCOUNT,
98+ AccountStatus.PLACEHOLDER]:
99+ if person.account.status == AccountStatus.PLACEHOLDER:
100+ # Placeholder accounts were never visible to anyone
101+ # before, so make them appear fresh to the user.
102+ removeSecurityProxy(person).display_name = full_name
103+ removeSecurityProxy(person).datecreated = UTC_NOW
104 removeSecurityProxy(person.account).reactivate(comment)
105 if email is None:
106 email = getUtility(IEmailAddressSet).new(
107@@ -3490,6 +3496,17 @@
108 name, displayname, hide_email_addresses=True, rationale=rationale,
109 comment=comment, registrant=registrant)
110
111+ def createPlaceholderPerson(self, openid_identifier, name):
112+ """See `IPersonSet`."""
113+ account = getUtility(IAccountSet).new(
114+ AccountCreationRationale.USERNAME_PLACEHOLDER, name,
115+ openid_identifier=openid_identifier,
116+ status=AccountStatus.PLACEHOLDER)
117+ return self._newPerson(
118+ name, name, True,
119+ rationale=PersonCreationRationale.USERNAME_PLACEHOLDER,
120+ comment="when setting a username in SSO", account=account)
121+
122 def _newPerson(self, name, displayname, hide_email_addresses,
123 rationale, comment=None, registrant=None, account=None):
124 """Create and return a new Person with the given attributes."""
125
126=== modified file 'lib/lp/registry/tests/test_personset.py'
127--- lib/lp/registry/tests/test_personset.py 2015-10-01 10:25:19 +0000
128+++ lib/lp/registry/tests/test_personset.py 2016-04-26 13:09:54 +0000
129@@ -6,7 +6,10 @@
130 __metaclass__ = type
131
132
133-from testtools.matchers import LessThan
134+from testtools.matchers import (
135+ GreaterThan,
136+ LessThan,
137+ )
138 import transaction
139 from zope.component import getUtility
140 from zope.security.interfaces import Unauthorized
141@@ -424,6 +427,25 @@
142 self.assertEqual(AccountStatus.ACTIVE, self.person.account_status)
143 self.assertEqual(addr, self.person.preferredemail.email)
144
145+ def testPlaceholderAccount(self):
146+ # Logging into a username placeholder account activates the
147+ # account and adds the email address.
148+ email = u'placeholder@example.com'
149+ openid = u'placeholder-id'
150+ name = u'placeholder'
151+ person = self.person_set.createPlaceholderPerson(openid, name)
152+ self.assertEqual(AccountStatus.PLACEHOLDER, person.account.status)
153+ original_created = person.datecreated
154+ transaction.commit()
155+ found, updated = self.person_set.getOrCreateByOpenIDIdentifier(
156+ openid, email, 'New Name', PersonCreationRationale.UNKNOWN,
157+ 'No Comment')
158+ self.assertEqual(person, found)
159+ self.assertEqual(AccountStatus.ACTIVE, person.account.status)
160+ self.assertEqual(name, person.name)
161+ self.assertEqual('New Name', person.display_name)
162+ self.assertThat(person.datecreated, GreaterThan(original_created))
163+
164
165 class TestCreatePersonAndEmail(TestCase):
166 """Test `IPersonSet`.createPersonAndEmail()."""
167
168=== modified file 'lib/lp/services/identity/interfaces/account.py'
169--- lib/lp/services/identity/interfaces/account.py 2016-01-26 15:47:37 +0000
170+++ lib/lp/services/identity/interfaces/account.py 2016-04-26 13:09:54 +0000
171@@ -50,6 +50,16 @@
172 class AccountStatus(DBEnumeratedType):
173 """The status of an account."""
174
175+ PLACEHOLDER = DBItem(5, """
176+ Placeholder
177+
178+ The account was created automatically by SSO and exists solely
179+ to hold a username. It is visible only to staff.
180+
181+ XXX wgrant 2016-03-03: This is a hack until usernames are moved
182+ into SSO properly.
183+ """)
184+
185 NOACCOUNT = DBItem(10, """
186 Unactivated
187
188@@ -76,7 +86,8 @@
189
190
191 INACTIVE_ACCOUNT_STATUSES = [
192- AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED]
193+ AccountStatus.PLACEHOLDER, AccountStatus.DEACTIVATED,
194+ AccountStatus.SUSPENDED]
195
196
197 class AccountCreationRationale(DBEnumeratedType):
198@@ -206,6 +217,14 @@
199 and commercial archive) was made via Software Center.
200 """)
201
202+ USERNAME_PLACEHOLDER = DBItem(17, """
203+ Created by setting a username in SSO.
204+
205+ Somebody without a Launchpad account set their username in SSO.
206+ Since SSO doesn't store usernames directly, an invisible
207+ placeholder Launchpad account is required.
208+ """)
209+
210
211 class AccountStatusError(LaunchpadValidationError):
212 """The account status cannot change to the proposed status."""
213@@ -215,6 +234,8 @@
214 """A valid status and transition."""
215
216 transitions = {
217+ AccountStatus.PLACEHOLDER: [
218+ AccountStatus.NOACCOUNT, AccountStatus.ACTIVE],
219 AccountStatus.NOACCOUNT: [AccountStatus.ACTIVE],
220 AccountStatus.ACTIVE: [
221 AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED],
222@@ -224,7 +245,7 @@
223
224 def constraint(self, value):
225 """See `IField`."""
226- if not IAccount.providedBy(self.context):
227+ if removeSecurityProxy(self.context)._SO_creating:
228 # This object is initializing.
229 return True
230 if self.context.status == value:
231
232=== modified file 'lib/lp/services/identity/model/account.py'
233--- lib/lp/services/identity/model/account.py 2015-07-08 16:05:11 +0000
234+++ lib/lp/services/identity/model/account.py 2016-04-26 13:09:54 +0000
235@@ -92,11 +92,13 @@
236 class AccountSet:
237 """See `IAccountSet`."""
238
239- def new(self, rationale, displayname, openid_identifier=None):
240+ def new(self, rationale, displayname, openid_identifier=None,
241+ status=AccountStatus.NOACCOUNT):
242 """See `IAccountSet`."""
243-
244+ assert status in (AccountStatus.NOACCOUNT, AccountStatus.PLACEHOLDER)
245 account = Account(
246- displayname=displayname, creation_rationale=rationale)
247+ displayname=displayname, creation_rationale=rationale,
248+ status=status)
249
250 # Create an OpenIdIdentifier record if requested.
251 if openid_identifier is not None: