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
=== modified file 'lib/lp/app/browser/launchpad.py'
--- lib/lp/app/browser/launchpad.py 2015-08-06 00:46:39 +0000
+++ lib/lp/app/browser/launchpad.py 2016-04-26 13:09:54 +0000
@@ -857,12 +857,15 @@
857 # team membership or Launchpad administration.857 # team membership or Launchpad administration.
858 if (person.is_team and858 if (person.is_team and
859 not check_permission('launchpad.LimitedView', person)):859 not check_permission('launchpad.LimitedView', person)):
860 raise NotFound(self.context, name)860 return None
861 # Only admins are permitted to see suspended users.861 # Only admins are permitted to see suspended users.
862 if person.account_status == AccountStatus.SUSPENDED:862 if person.account_status == AccountStatus.SUSPENDED:
863 if not check_permission('launchpad.Moderate', person):863 if not check_permission('launchpad.Moderate', person):
864 raise GoneError(864 raise GoneError(
865 'User is suspended: %s' % name)865 'User is suspended: %s' % name)
866 if person.account_status == AccountStatus.PLACEHOLDER:
867 if not check_permission('launchpad.Moderate', person):
868 return None
866 return person869 return person
867870
868 # Dapper and Edgy shipped with https://launchpad.net/bazaar hard coded871 # Dapper and Edgy shipped with https://launchpad.net/bazaar hard coded
869872
=== modified file 'lib/lp/app/browser/tests/test_launchpad.py'
--- lib/lp/app/browser/tests/test_launchpad.py 2015-01-07 00:35:41 +0000
+++ lib/lp/app/browser/tests/test_launchpad.py 2016-04-26 13:09:54 +0000
@@ -370,6 +370,23 @@
370 login_person(self.any_user)370 login_person(self.any_user)
371 self.assertRaises(GoneError, self.traverse, segment, segment)371 self.assertRaises(GoneError, self.traverse, segment, segment)
372372
373 def test_placeholder_person_visibility(self):
374 # Verify a placeholder user is only traversable by an admin.
375 name = u'placeholder-person'
376 person = getUtility(IPersonSet).createPlaceholderPerson(name, name)
377 login_person(self.admin)
378 segment = '~%s' % name
379 # Admins can see the placeholder user.
380 traversed = self.traverse(segment, segment)
381 self.assertEqual(person, traversed)
382 # Registry experts can see the placeholder user.
383 login_person(self.registry_expert)
384 traversed = self.traverse(segment, segment)
385 self.assertEqual(person, traversed)
386 # Regular users cannot see the placeholder user.
387 login_person(self.any_user)
388 self.assertRaises(NotFound, self.traverse, segment, segment)
389
373 def test_public_team(self):390 def test_public_team(self):
374 # Verify a public team is returned.391 # Verify a public team is returned.
375 name = 'public-team'392 name = 'public-team'
376393
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2016-03-18 03:30:26 +0000
+++ lib/lp/registry/interfaces/person.py 2016-04-26 13:09:54 +0000
@@ -406,6 +406,14 @@
406 and commercial archive) was made via Software Center.406 and commercial archive) was made via Software Center.
407 """)407 """)
408408
409 USERNAME_PLACEHOLDER = DBItem(17, """
410 Created by setting a username in SSO.
411
412 Somebody without a Launchpad account set their username in SSO.
413 Since SSO doesn't store usernames directly, an invisible
414 placeholder Launchpad account is required.
415 """)
416
409417
410class PersonNameField(BlacklistableContentNameField):418class PersonNameField(BlacklistableContentNameField):
411 """A `Person` team name, which is unique and performs psuedo blacklisting.419 """A `Person` team name, which is unique and performs psuedo blacklisting.
@@ -2117,6 +2125,19 @@
2117 used.2125 used.
2118 """2126 """
21192127
2128 def createPlaceholderPerson(openid_identifier, name):
2129 """Create and return an SSO username placeholder `IPerson`.
2130
2131 The returned Person will have no email address, just a username and an
2132 OpenID identifier.
2133
2134 :param openid_identifier: The SSO account's OpenID suffix.
2135 :param name: The person's name.
2136 :raises InvalidName: When the passed name isn't valid.
2137 :raises NameAlreadyTaken: When the passed name has already been
2138 used.
2139 """
2140
2120 def ensurePerson(email, displayname, rationale, comment=None,2141 def ensurePerson(email, displayname, rationale, comment=None,
2121 registrant=None):2142 registrant=None):
2122 """Make sure that there is a person in the database with the given2143 """Make sure that there is a person in the database with the given
21232144
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2016-03-01 13:02:42 +0000
+++ lib/lp/registry/model/person.py 2016-04-26 13:09:54 +0000
@@ -3397,7 +3397,13 @@
3397 raise NameAlreadyTaken(3397 raise NameAlreadyTaken(
3398 "The account matching the identifier is inactive.")3398 "The account matching the identifier is inactive.")
3399 elif person.account.status in [AccountStatus.DEACTIVATED,3399 elif person.account.status in [AccountStatus.DEACTIVATED,
3400 AccountStatus.NOACCOUNT]:3400 AccountStatus.NOACCOUNT,
3401 AccountStatus.PLACEHOLDER]:
3402 if person.account.status == AccountStatus.PLACEHOLDER:
3403 # Placeholder accounts were never visible to anyone
3404 # before, so make them appear fresh to the user.
3405 removeSecurityProxy(person).display_name = full_name
3406 removeSecurityProxy(person).datecreated = UTC_NOW
3401 removeSecurityProxy(person.account).reactivate(comment)3407 removeSecurityProxy(person.account).reactivate(comment)
3402 if email is None:3408 if email is None:
3403 email = getUtility(IEmailAddressSet).new(3409 email = getUtility(IEmailAddressSet).new(
@@ -3490,6 +3496,17 @@
3490 name, displayname, hide_email_addresses=True, rationale=rationale,3496 name, displayname, hide_email_addresses=True, rationale=rationale,
3491 comment=comment, registrant=registrant)3497 comment=comment, registrant=registrant)
34923498
3499 def createPlaceholderPerson(self, openid_identifier, name):
3500 """See `IPersonSet`."""
3501 account = getUtility(IAccountSet).new(
3502 AccountCreationRationale.USERNAME_PLACEHOLDER, name,
3503 openid_identifier=openid_identifier,
3504 status=AccountStatus.PLACEHOLDER)
3505 return self._newPerson(
3506 name, name, True,
3507 rationale=PersonCreationRationale.USERNAME_PLACEHOLDER,
3508 comment="when setting a username in SSO", account=account)
3509
3493 def _newPerson(self, name, displayname, hide_email_addresses,3510 def _newPerson(self, name, displayname, hide_email_addresses,
3494 rationale, comment=None, registrant=None, account=None):3511 rationale, comment=None, registrant=None, account=None):
3495 """Create and return a new Person with the given attributes."""3512 """Create and return a new Person with the given attributes."""
34963513
=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py 2015-10-01 10:25:19 +0000
+++ lib/lp/registry/tests/test_personset.py 2016-04-26 13:09:54 +0000
@@ -6,7 +6,10 @@
6__metaclass__ = type6__metaclass__ = type
77
88
9from testtools.matchers import LessThan9from testtools.matchers import (
10 GreaterThan,
11 LessThan,
12 )
10import transaction13import transaction
11from zope.component import getUtility14from zope.component import getUtility
12from zope.security.interfaces import Unauthorized15from zope.security.interfaces import Unauthorized
@@ -424,6 +427,25 @@
424 self.assertEqual(AccountStatus.ACTIVE, self.person.account_status)427 self.assertEqual(AccountStatus.ACTIVE, self.person.account_status)
425 self.assertEqual(addr, self.person.preferredemail.email)428 self.assertEqual(addr, self.person.preferredemail.email)
426429
430 def testPlaceholderAccount(self):
431 # Logging into a username placeholder account activates the
432 # account and adds the email address.
433 email = u'placeholder@example.com'
434 openid = u'placeholder-id'
435 name = u'placeholder'
436 person = self.person_set.createPlaceholderPerson(openid, name)
437 self.assertEqual(AccountStatus.PLACEHOLDER, person.account.status)
438 original_created = person.datecreated
439 transaction.commit()
440 found, updated = self.person_set.getOrCreateByOpenIDIdentifier(
441 openid, email, 'New Name', PersonCreationRationale.UNKNOWN,
442 'No Comment')
443 self.assertEqual(person, found)
444 self.assertEqual(AccountStatus.ACTIVE, person.account.status)
445 self.assertEqual(name, person.name)
446 self.assertEqual('New Name', person.display_name)
447 self.assertThat(person.datecreated, GreaterThan(original_created))
448
427449
428class TestCreatePersonAndEmail(TestCase):450class TestCreatePersonAndEmail(TestCase):
429 """Test `IPersonSet`.createPersonAndEmail()."""451 """Test `IPersonSet`.createPersonAndEmail()."""
430452
=== modified file 'lib/lp/services/identity/interfaces/account.py'
--- lib/lp/services/identity/interfaces/account.py 2016-01-26 15:47:37 +0000
+++ lib/lp/services/identity/interfaces/account.py 2016-04-26 13:09:54 +0000
@@ -50,6 +50,16 @@
50class AccountStatus(DBEnumeratedType):50class AccountStatus(DBEnumeratedType):
51 """The status of an account."""51 """The status of an account."""
5252
53 PLACEHOLDER = DBItem(5, """
54 Placeholder
55
56 The account was created automatically by SSO and exists solely
57 to hold a username. It is visible only to staff.
58
59 XXX wgrant 2016-03-03: This is a hack until usernames are moved
60 into SSO properly.
61 """)
62
53 NOACCOUNT = DBItem(10, """63 NOACCOUNT = DBItem(10, """
54 Unactivated64 Unactivated
5565
@@ -76,7 +86,8 @@
7686
7787
78INACTIVE_ACCOUNT_STATUSES = [88INACTIVE_ACCOUNT_STATUSES = [
79 AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED]89 AccountStatus.PLACEHOLDER, AccountStatus.DEACTIVATED,
90 AccountStatus.SUSPENDED]
8091
8192
82class AccountCreationRationale(DBEnumeratedType):93class AccountCreationRationale(DBEnumeratedType):
@@ -206,6 +217,14 @@
206 and commercial archive) was made via Software Center.217 and commercial archive) was made via Software Center.
207 """)218 """)
208219
220 USERNAME_PLACEHOLDER = DBItem(17, """
221 Created by setting a username in SSO.
222
223 Somebody without a Launchpad account set their username in SSO.
224 Since SSO doesn't store usernames directly, an invisible
225 placeholder Launchpad account is required.
226 """)
227
209228
210class AccountStatusError(LaunchpadValidationError):229class AccountStatusError(LaunchpadValidationError):
211 """The account status cannot change to the proposed status."""230 """The account status cannot change to the proposed status."""
@@ -215,6 +234,8 @@
215 """A valid status and transition."""234 """A valid status and transition."""
216235
217 transitions = {236 transitions = {
237 AccountStatus.PLACEHOLDER: [
238 AccountStatus.NOACCOUNT, AccountStatus.ACTIVE],
218 AccountStatus.NOACCOUNT: [AccountStatus.ACTIVE],239 AccountStatus.NOACCOUNT: [AccountStatus.ACTIVE],
219 AccountStatus.ACTIVE: [240 AccountStatus.ACTIVE: [
220 AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED],241 AccountStatus.DEACTIVATED, AccountStatus.SUSPENDED],
@@ -224,7 +245,7 @@
224245
225 def constraint(self, value):246 def constraint(self, value):
226 """See `IField`."""247 """See `IField`."""
227 if not IAccount.providedBy(self.context):248 if removeSecurityProxy(self.context)._SO_creating:
228 # This object is initializing.249 # This object is initializing.
229 return True250 return True
230 if self.context.status == value:251 if self.context.status == value:
231252
=== modified file 'lib/lp/services/identity/model/account.py'
--- lib/lp/services/identity/model/account.py 2015-07-08 16:05:11 +0000
+++ lib/lp/services/identity/model/account.py 2016-04-26 13:09:54 +0000
@@ -92,11 +92,13 @@
92class AccountSet:92class AccountSet:
93 """See `IAccountSet`."""93 """See `IAccountSet`."""
9494
95 def new(self, rationale, displayname, openid_identifier=None):95 def new(self, rationale, displayname, openid_identifier=None,
96 status=AccountStatus.NOACCOUNT):
96 """See `IAccountSet`."""97 """See `IAccountSet`."""
9798 assert status in (AccountStatus.NOACCOUNT, AccountStatus.PLACEHOLDER)
98 account = Account(99 account = Account(
99 displayname=displayname, creation_rationale=rationale)100 displayname=displayname, creation_rationale=rationale,
101 status=status)
100102
101 # Create an OpenIdIdentifier record if requested.103 # Create an OpenIdIdentifier record if requested.
102 if openid_identifier is not None:104 if openid_identifier is not None: