launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06077
[Merge] lp:~wgrant/launchpad/no-emailaddress-account into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/no-emailaddress-account into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #629172 in Launchpad itself: "Drop EmailAddress.account"
https://bugs.launchpad.net/launchpad/+bug/629172
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/no-emailaddress-account/+merge/88334
EmailAddress.account is redundant with the combination of EmailAddress.person and Person.account. This branch drops it, and removes support for personless accounts. No personless accounts have been created since the SSO DB divorce in April 2010, and a garbo job removed them all a week or so ago.
This obviates the need for a lot of code. Authentication can just use person, and things that create people don't have to worry about personless accounts.
--
https://code.launchpad.net/~wgrant/launchpad/no-emailaddress-account/+merge/88334
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/no-emailaddress-account into lp:launchpad.
=== modified file 'lib/lp/archiveuploader/tests/nascentuploadfile.txt'
--- lib/lp/archiveuploader/tests/nascentuploadfile.txt 2011-12-30 06:14:56 +0000
+++ lib/lp/archiveuploader/tests/nascentuploadfile.txt 2012-01-12 10:41:14 +0000
@@ -270,37 +270,6 @@
>>> addr['person'].creation_comment
u'when the some-source_6.6.6 package was uploaded to hoary/RELEASE'
-If the email address is registered but not associated with a person it will be
-associated with a new Person. This involves updating the email address,
-something for which the uploader must have explicit permissions (bug 589073).
-
- >>> sig_file.policy.create_people
- True
-
- >>> from lp.services.database.sqlbase import commit
- >>> from lp.services.identity.interfaces.account import IAccountSet
- >>> from lp.registry.interfaces.person import (
- ... PersonCreationRationale, IPersonSet)
- >>> (acct, email) = getUtility(IAccountSet).createAccountAndEmail(
- ... "foo@xxxxxxxxxxxxx", PersonCreationRationale.UNKNOWN,
- ... "fo", "secr1t")
- >>> person = getUtility(IPersonSet).createPersonWithoutEmail("fo",
- ... rationale=PersonCreationRationale.UNKNOWN)
- >>> person.account = acct
-
- Commit the changes so the emailaddress will have to be updated later
- rather than inserted.
-
- >>> commit()
-
- >>> addr = sig_file.parseAddress("Foo <foo@xxxxxxxxxxxxx>")
- >>> print addr['person'].creation_rationale.name
- UNKNOWN
- >>> commit()
-
- >>> print addr['email']
- foo@xxxxxxxxxxxxx
-
If the use an un-initialized policy to create a launchpad person the
creation_rationale will still be possible, however missing important
information, the upload target:
=== modified file 'lib/lp/bugs/browser/tests/test_bugtask.py'
--- lib/lp/bugs/browser/tests/test_bugtask.py 2011-12-30 06:14:56 +0000
+++ lib/lp/bugs/browser/tests/test_bugtask.py 2012-01-12 10:41:14 +0000
@@ -183,7 +183,7 @@
self.invalidate_caches(bug.default_bugtask)
self.getUserBrowser(url, owner)
# At least 20 of these should be removed.
- self.assertThat(recorder, HasQueryCount(LessThan(106)))
+ self.assertThat(recorder, HasQueryCount(LessThan(107)))
count_with_no_branches = recorder.count
for sp in sourcepackages:
self.makeLinkedBranchMergeProposal(sp, bug, owner)
=== modified file 'lib/lp/bugs/doc/externalbugtracker-comment-imports.txt'
--- lib/lp/bugs/doc/externalbugtracker-comment-imports.txt 2011-12-29 05:29:36 +0000
+++ lib/lp/bugs/doc/externalbugtracker-comment-imports.txt 2012-01-12 10:41:14 +0000
@@ -179,23 +179,6 @@
>>> bug.messages[-1].owner.name
u'no-priv'
-This also works if the address is associated with an Account, but not a
-Person. This should only happen when the user has logged into ShipIt but
-not Launchpad. A new Person is created.
-
- >>> account = factory.makeAccount(email="account-only@xxxxxxxxxxx")
- >>> external_bugtracker.poster_tuple = (
- ... 'Account Only', 'account-only@xxxxxxxxxxx')
- >>> external_bugtracker.remote_comments['account-only-comment'] = (
- ... "Account-only comment.")
- >>> bugwatch_updater.importBugComments()
- INFO:...:Imported 1 comments for remote bug 123456 on ...
-
- >>> bug.messages[-1].owner.name
- u'account-only'
- >>> bug.messages[-1].owner.account == account
- True
-
It's also possible for Launchpad to create Persons from remote
bugtracker users when the remote bugtracker doesn't specify an email
address. In those cases, the ExternalBugTracker's getPosterForComment()
=== modified file 'lib/lp/bugs/scripts/bugimport.py'
--- lib/lp/bugs/scripts/bugimport.py 2011-12-30 06:14:56 +0000
+++ lib/lp/bugs/scripts/bugimport.py 2012-01-12 10:41:14 +0000
@@ -168,8 +168,9 @@
person = None
if person is None:
- address = getUtility(IEmailAddressSet).getByEmail(email)
- if address is None:
+ person = getUtility(IPersonSet).getByEmail(email)
+
+ if person is None:
self.logger.debug('creating person for %s' % email)
# Has the short name been taken?
if name is not None and (
@@ -184,26 +185,6 @@
rationale=PersonCreationRationale.BUGIMPORT,
comment=('when importing bugs for %s' %
self.product.displayname)))
- elif address.personID is None:
- # The user has an Account and and EmailAddress linked
- # to that account.
- assert address.accountID is not None, (
- "Email address not linked to an Account: %s " % email)
- self.logger.debug(
- 'creating person from account for %s' % email)
- if name is not None and (
- person_set.getByName(name) is not None):
- # The short name is already taken, so we'll pass
- # None to createPerson(), which will take care of
- # creating a unique one.
- name = None
- person = address.account.createPerson(
- rationale=PersonCreationRationale.BUGIMPORT,
- name=name, comment=('when importing bugs for %s' %
- self.product.displayname))
- else:
- # EmailAddress and Person are in different stores.
- person = person_set.get(address.personID)
self.person_id_cache[email] = person.id
=== modified file 'lib/lp/bugs/scripts/tests/test_bugimport.py'
--- lib/lp/bugs/scripts/tests/test_bugimport.py 2012-01-05 00:15:32 +0000
+++ lib/lp/bugs/scripts/tests/test_bugimport.py 2012-01-12 10:41:14 +0000
@@ -297,34 +297,6 @@
self.assertNotEqual(person.preferredemail, None)
self.assertEqual(person.preferredemail.email, 'foo@xxxxxxxxxxxxx')
- def test_person_from_account(self):
- # If an Account record exists for a user's email address, but
- # no Person record is linked to it, the bug importer creates a
- # Person and links the three piece of information together.
- account = self.factory.makeAccount("Sam")
- personnode = ET.fromstring(
- '<person xmlns="https://launchpad.net/xmlns/2006/bugs"; />')
- personnode.set('name', generate_nick(account.preferredemail.email))
- personnode.set('email', account.preferredemail.email)
- personnode.text = account.displayname
-
- product = getUtility(IProductSet).getByName('netapplet')
- importer = bugimport.BugImporter(
- product, 'bugs.xml', 'bug-map.pickle', verify_users=True)
- person = importer.getPerson(personnode)
-
- # The person returned is associated with the account.
- self.failUnlessEqual(account.id, person.accountID)
- # The creation comment and rationale are set correctly.
- self.failUnlessEqual(
- 'when importing bugs for %s' % product.displayname,
- person.creation_comment)
- self.failUnlessEqual(
- PersonCreationRationale.BUGIMPORT,
- person.creation_rationale)
- # The person's email addresses are hidden by default.
- self.failUnless(person.hide_email_addresses)
-
class GetMilestoneTestCase(unittest.TestCase):
"""Tests for the BugImporter.getMilestone() method."""
=== modified file 'lib/lp/registry/doc/person-account.txt'
--- lib/lp/registry/doc/person-account.txt 2011-12-18 13:45:20 +0000
+++ lib/lp/registry/doc/person-account.txt 2012-01-12 10:41:14 +0000
@@ -31,8 +31,7 @@
the profile. Sample Person cannot claim it.
>>> login('test@xxxxxxxxxxxxx')
- >>> matsubara.account.activate(
- ... comment="test", password='ok', preferred_email=emailaddress)
+ >>> matsubara.account.reactivate(comment="test", password='ok')
Traceback (most recent call last):
...
Unauthorized: ...'launchpad.Special')
@@ -42,8 +41,8 @@
>>> from zope.security.proxy import removeSecurityProxy
>>> login('matsubara@xxxxxxxxxxxx')
- >>> matsubara.account.activate(
- ... comment="test", password='ok', preferred_email=emailaddress)
+ >>> matsubara.account.reactivate(comment="test", password='ok')
+ >>> matsubara.setPreferredEmail(emailaddress)
>>> import transaction
>>> transaction.commit()
>>> matsubara.is_valid_person
@@ -52,7 +51,7 @@
<DBItem AccountStatus.ACTIVE, ...>
>>> matsubara.account.status_comment
u'test'
- >>> removeSecurityProxy(matsubara.account.preferredemail).email
+ >>> removeSecurityProxy(matsubara.preferredemail).email
u'matsubara@xxxxxxxxxxxx'
@@ -212,15 +211,7 @@
Reactivating user accounts
--------------------------
-Accounts can be reactivated. A comment and a non-None preferred email address
-are required to reactivate() an account, though.
-
- >>> foobar.account.reactivate(
- ... 'This will raise an error.', password='', preferred_email=None)
- Traceback (most recent call last):
- ...
- AssertionError: Account ... cannot be activated without a preferred
- email address.
+Accounts can be reactivated.
>>> foobar.reactivate(
... 'User reactivated the account using reset password.',
=== modified file 'lib/lp/registry/doc/person.txt'
--- lib/lp/registry/doc/person.txt 2012-01-04 23:49:46 +0000
+++ lib/lp/registry/doc/person.txt 2012-01-12 10:41:14 +0000
@@ -132,9 +132,6 @@
>>> p.account_status
<DBItem AccountStatus.NOACCOUNT...
- >>> email.accountID == p.accountID
- True
-
>>> p.setPreferredEmail(email)
>>> email.status
<DBItem EmailAddressStatus.PREFERRED...
@@ -145,10 +142,7 @@
>>> from lp.services.identity.model.account import Account
>>> from lp.services.database.lpstorm import IMasterStore
>>> account = IMasterStore(Account).get(Account, p.accountID)
- >>> account.activate(
- ... "Activated by doc test.",
- ... password=p.password,
- ... preferred_email=email)
+ >>> account.reactivate("Activated by doc test.", password=p.password)
>>> p.account_status
<DBItem AccountStatus.ACTIVE...
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-01-06 19:56:39 +0000
+++ lib/lp/registry/model/person.py 2012-01-12 10:41:14 +0000
@@ -2540,7 +2540,8 @@
def reactivate(self, comment, password, preferred_email):
"""See `IPersonSpecialRestricted`."""
account = IMasterObject(self.account)
- account.reactivate(comment, password, preferred_email)
+ account.reactivate(comment, password)
+ self.setPreferredEmail(preferred_email)
if '-deactivatedaccount' in self.name:
# The name was changed by deactivateAccount(). Restore the
# name, but we must ensure it does not conflict with a current
@@ -3108,97 +3109,81 @@
# possible replication lag issues but this might actually be
# unnecessary.
with MasterDatabasePolicy():
- store = IMasterStore(EmailAddress)
- join = store.using(
- EmailAddress,
- LeftJoin(Account, EmailAddress.accountID == Account.id))
- email, account = (
- join.find(
- (EmailAddress, Account),
- EmailAddress.email.lower() ==
- ensure_unicode(email_address).lower()).one()
+ email, person = (
+ getUtility(IPersonSet).getByEmails([email_address]).one()
or (None, None))
- identifier = store.find(
+ identifier = IStore(OpenIdIdentifier).find(
OpenIdIdentifier, identifier=openid_identifier).one()
- if email is None and identifier is None:
- # Neither the Email Address not the OpenId Identifier
- # exist in the database. Create the email address,
- # account, and associated info. OpenIdIdentifier is
- # created later.
- account_set = getUtility(IAccountSet)
- account, email = account_set.createAccountAndEmail(
- email_address, creation_rationale, full_name,
- password=None)
- db_updated = True
-
- elif email is None:
- # The Email Address does not exist in the database,
- # but the OpenId Identifier does. Create the Email
- # Address and link it to the account.
- assert account is None, 'Retrieved an account but not email?'
- account = identifier.account
- emailaddress_set = getUtility(IEmailAddressSet)
- email = emailaddress_set.new(
- email_address, account=account)
- db_updated = True
-
- elif account is None:
- # Email address exists, but there is no Account linked
- # to it. Create the Account and link it to the
- # EmailAddress.
+ if email is None:
+ if identifier is None:
+ # Neither the Email Address not the OpenId Identifier
+ # exist in the database. Create the email address,
+ # account, and associated info. OpenIdIdentifier is
+ # created later.
+ person_set = getUtility(IPersonSet)
+ person, email = person_set.createPersonAndEmail(
+ email_address, creation_rationale, comment=comment,
+ displayname=full_name)
+ db_updated = True
+ else:
+ # The Email Address does not exist in the database,
+ # but the OpenId Identifier does. Create the Email
+ # Address and link it to the person.
+ person = IPerson(identifier.account, None)
+ assert person is not None, (
+ 'Received a personless account.')
+ emailaddress_set = getUtility(IEmailAddressSet)
+ email = emailaddress_set.new(email_address, person=person)
+ db_updated = True
+ elif email.person.account is None:
+ # Email address and person exist, but there is no
+ # account. Create and link it.
account_set = getUtility(IAccountSet)
account = account_set.new(
AccountCreationRationale.OWNER_CREATED_LAUNCHPAD,
full_name)
- email.account = account
+ removeSecurityProxy(email.person).account = account
db_updated = True
+ person = email.person
+ assert person.account is not None
+
if identifier is None:
# This is the first time we have seen that
# OpenIdIdentifier. Link it.
identifier = OpenIdIdentifier()
- identifier.account = account
+ identifier.account = person.account
identifier.identifier = openid_identifier
- store.add(identifier)
+ IStore(OpenIdIdentifier).add(identifier)
db_updated = True
-
- elif identifier.account != account:
+ elif identifier.account != person.account:
# The ISD OpenId server may have linked this OpenId
# identifier to a new email address, or the user may
# have transfered their email address to a different
# Launchpad Account. If that happened, repair the
# link - we trust the ISD OpenId server.
- identifier.account = account
+ identifier.account = person.account
db_updated = True
# We now have an account, email address, and openid identifier.
- if account.status == AccountStatus.SUSPENDED:
+ if person.account.status == AccountStatus.SUSPENDED:
raise AccountSuspendedError(
"The account matching the identifier is suspended.")
- elif account.status in [AccountStatus.DEACTIVATED,
- AccountStatus.NOACCOUNT]:
- password = '' # Needed just to please reactivate() below.
- removeSecurityProxy(account).reactivate(
- comment, password, removeSecurityProxy(email))
+ elif person.account.status in [AccountStatus.DEACTIVATED,
+ AccountStatus.NOACCOUNT]:
+ password = ''
+ removeSecurityProxy(person.account).reactivate(
+ comment, password)
+ removeSecurityProxy(person).setPreferredEmail(email)
db_updated = True
else:
# Account is active, so nothing to do.
pass
- if IPerson(account, None) is None:
- removeSecurityProxy(account).createPerson(
- creation_rationale, comment=comment)
- db_updated = True
-
- person = IPerson(account)
- if email.personID != person.id:
- removeSecurityProxy(email).person = person
- db_updated = True
-
- return person, db_updated
+ return email.person, db_updated
def newTeam(self, teamowner, name, displayname, teamdescription=None,
subscriptionpolicy=TeamSubscriptionPolicy.MODERATED,
@@ -3251,12 +3236,8 @@
person = self._newPerson(
name, displayname, hide_email_addresses, rationale=rationale,
comment=comment, registrant=registrant, account=account)
-
- email = getUtility(IEmailAddressSet).new(
- email, person, account=account)
-
- assert email.accountID is not None, (
- 'Failed to link EmailAddress to Account')
+ email = getUtility(IEmailAddressSet).new(email, person)
+
return person, email
def createPersonWithoutEmail(
@@ -3299,55 +3280,14 @@
def ensurePerson(self, email, displayname, rationale, comment=None,
registrant=None):
"""See `IPersonSet`."""
- # Start by checking if there is an `EmailAddress` for the given
- # text address. There are many cases where an email address can be
- # created without an associated `Person`. For instance, we created
- # an account linked to the address through an external system such
- # SSO or ShipIt.
- email_address = getUtility(IEmailAddressSet).getByEmail(email)
+ person = getUtility(IPersonSet).getByEmail(email)
- # There is no `EmailAddress` for this text address, so we need to
- # create both the `Person` and `EmailAddress` here and we are done.
- if email_address is None:
+ if person is None:
person, email_address = self.createPersonAndEmail(
email, rationale, comment=comment, displayname=displayname,
registrant=registrant, hide_email_addresses=True)
- return person
-
- # There is an `EmailAddress` for this text address, but no
- # associated `Person`.
- if email_address.person is None:
- assert email_address.accountID is not None, (
- '%s is not associated to a person or account'
- % email_address.email)
- account = IMasterStore(Account).get(
- Account, email_address.accountID)
- account_person = self.getByAccount(account)
- if account_person is None:
- # There is no associated `Person` to the email `Account`.
- # This is probably because the account was created externally
- # to Launchpad. Create just the `Person`, associate it with
- # the `EmailAddress` and return it.
- name = generate_nick(email)
- account_person = self._newPerson(
- name, displayname, hide_email_addresses=True,
- rationale=rationale, comment=comment,
- registrant=registrant, account=email_address.account)
- # There is (now) a `Person` linked to the `Account`, link the
- # `EmailAddress` to this `Person` and return it.
- master_email = IMasterStore(EmailAddress).get(
- EmailAddress, email_address.id)
- master_email.personID = account_person.id
- # Populate the previously empty 'preferredemail' cached
- # property, so the Person record is up-to-date.
- if master_email.status == EmailAddressStatus.PREFERRED:
- cache = get_property_cache(account_person)
- cache.preferredemail = master_email
- return account_person
-
- # Easy, return the `Person` associated with the existing
- # `EmailAddress`.
- return IMasterStore(Person).get(Person, email_address.personID)
+
+ return person
def getByName(self, name, ignore_merged=True):
"""See `IPersonSet`."""
=== modified file 'lib/lp/registry/tests/test_mailinglistapi.py'
--- lib/lp/registry/tests/test_mailinglistapi.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/tests/test_mailinglistapi.py 2012-01-12 10:41:14 +0000
@@ -73,10 +73,6 @@
def test_isRegisteredInLaunchpad_email_no_email_address(self):
self.assertFalse(self.api.isRegisteredInLaunchpad('me@xxxxxxxxx'))
- def test_isRegisteredInLaunchpad_email_without_person(self):
- self.factory.makeAccount('Me', email='me@xxxxxxxxx')
- self.assertFalse(self.api.isRegisteredInLaunchpad('me@xxxxxxxxx'))
-
def test_isRegisteredInLaunchpad_archive_address_is_false(self):
# The Mailman archive address can never be owned by an Lp user
# because such a user would have acces to all lists.
=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py 2012-01-06 15:14:48 +0000
+++ lib/lp/registry/tests/test_personset.py 2012-01-12 10:41:14 +0000
@@ -229,7 +229,6 @@
email = from_person.preferredemail
email.status = EmailAddressStatus.NEW
email.person = to_person
- email.account = to_person.account
transaction.commit()
def _do_merge(self, from_person, to_person, reviewer=None):
@@ -725,12 +724,10 @@
# The old email address is still there and correctly linked.
self.assertIs(self.email, found.preferredemail)
- self.assertIs(self.email.account, self.account)
self.assertIs(self.email.person, self.person)
# The new email address is there too and correctly linked.
new_email = self.store.find(EmailAddress, email=new_email).one()
- self.assertIs(new_email.account, self.account)
self.assertIs(new_email.person, self.person)
self.assertEqual(EmailAddressStatus.NEW, new_email.status)
@@ -751,32 +748,9 @@
# It is correctly linked to an account, emailaddress and
# identifier.
self.assertIs(found, found.preferredemail.person)
- self.assertIs(found.account, found.preferredemail.account)
self.assertEqual(
new_identifier, found.account.openid_identifiers.any().identifier)
- def testNoPerson(self):
- # If the account is not linked to a Person, create one. ShipIt
- # users fall into this category the first time they log into
- # Launchpad.
- self.email.person = None
- self.person.account = None
-
- found, updated = self.person_set.getOrCreateByOpenIDIdentifier(
- self.identifier.identifier, self.email.email, 'New Name',
- PersonCreationRationale.UNKNOWN, 'No Comment')
- found = removeSecurityProxy(found)
-
- # We have a new Person
- self.assertIs(True, updated)
- self.assertIsNot(self.person, found)
-
- # It is correctly linked to an account, emailaddress and
- # identifier.
- self.assertIs(found, found.preferredemail.person)
- self.assertIs(found.account, found.preferredemail.account)
- self.assertIn(self.identifier, list(found.account.openid_identifiers))
-
def testNoAccount(self):
# EmailAddress is linked to a Person, but there is no Account.
# Convert this stub into something valid.
@@ -795,7 +769,6 @@
self.assertEqual(
new_identifier, found.account.openid_identifiers.any().identifier)
self.assertIs(self.email.person, found)
- self.assertIs(self.email.account, found.account)
self.assertEqual(EmailAddressStatus.PREFERRED, self.email.status)
def testMovedEmailAddress(self):
@@ -916,41 +889,6 @@
self.email_address, self.displayname, self.rationale)
self.assertTrue(ensured_person.hide_email_addresses)
- def test_ensurePerson_for_existing_account(self):
- # IPerson.ensurePerson creates missing Person for existing
- # Accounts.
- test_account = self.factory.makeAccount(
- self.displayname, email=self.email_address)
- self.assertIs(None, test_account.preferredemail.person)
-
- ensured_person = self.person_set.ensurePerson(
- self.email_address, self.displayname, self.rationale)
- self.assertEquals(test_account.id, ensured_person.account.id)
- self.assertEquals(
- test_account.preferredemail, ensured_person.preferredemail)
- self.assertEquals(ensured_person, test_account.preferredemail.person)
- self.assertTrue(ensured_person.hide_email_addresses)
-
- def test_ensurePerson_for_existing_account_with_person(self):
- # IPerson.ensurePerson return existing Person for existing
- # Accounts and additionally bounds the account email to the
- # Person in question.
-
- # Create a testing `Account` and a testing `Person` directly,
- # linked.
- testing_account = self.factory.makeAccount(
- self.displayname, email=self.email_address)
- testing_person = removeSecurityProxy(
- testing_account).createPerson(self.rationale)
- self.assertEqual(
- testing_person, testing_account.preferredemail.person)
-
- # Since there's an existing Person for the given email address,
- # IPersonSet.ensurePerson() will just return it.
- ensured_person = self.person_set.ensurePerson(
- self.email_address, self.displayname, self.rationale)
- self.assertEqual(testing_person, ensured_person)
-
class TestPersonSetGetOrCreateByOpenIDIdentifier(TestCaseWithFactory):
@@ -977,25 +915,6 @@
self.assertEqual(person, result)
self.assertFalse(db_updated)
- def test_existing_account_no_person(self):
- # A person is created with the correct rationale.
- account = self.factory.makeAccount('purchaser')
- openid_ident = removeSecurityProxy(
- account).openid_identifiers.any().identifier
-
- person, db_updated = self.callGetOrCreate(openid_ident)
-
- self.assertEqual(account, person.account)
- # The person is created with the correct rationale and creation
- # comment.
- self.assertEqual(
- "when purchasing an application via Software Center.",
- person.creation_comment)
- self.assertEqual(
- PersonCreationRationale.SOFTWARE_CENTER_PURCHASE,
- person.creation_rationale)
- self.assertTrue(db_updated)
-
def test_existing_deactivated_account(self):
# An existing deactivated account will be reactivated.
person = self.factory.makePerson(
=== modified file 'lib/lp/registry/xmlrpc/mailinglist.py'
--- lib/lp/registry/xmlrpc/mailinglist.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/xmlrpc/mailinglist.py 2012-01-12 10:41:14 +0000
@@ -225,7 +225,6 @@
return False
email_address = getUtility(IEmailAddressSet).getByEmail(address)
return (email_address is not None and
- email_address.personID is not None and
not email_address.person.is_team and
email_address.status in (EmailAddressStatus.VALIDATED,
EmailAddressStatus.PREFERRED))
=== modified file 'lib/lp/scripts/garbo.py'
--- lib/lp/scripts/garbo.py 2012-01-06 18:59:36 +0000
+++ lib/lp/scripts/garbo.py 2012-01-12 10:41:14 +0000
@@ -701,7 +701,7 @@
AND Person.id IN (%s)
""" % people_ids)
self.store.execute("""
- UPDATE EmailAddress SET person=NULL
+ DELETE FROM EmailAddress
WHERE person IN (%s)
""" % people_ids)
# This cascade deletes any PersonSettings records.
=== modified file 'lib/lp/services/identity/doc/account.txt'
--- lib/lp/services/identity/doc/account.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/services/identity/doc/account.txt 2012-01-12 10:41:14 +0000
@@ -13,7 +13,7 @@
implements the IAccountSet interface.
>>> from zope.interface.verify import verifyObject
- >>> from lp.registry.interfaces.person import IPerson
+ >>> from lp.registry.interfaces.person import IPersonSet
>>> from lp.services.identity.interfaces.account import (
... IAccount, IAccountSet)
@@ -21,37 +21,8 @@
>>> verifyObject(IAccountSet, account_set)
True
-
-Looking up accounts by email address
-------------------------------------
-
-Accounts are generally looked up by email address.
-
- >>> login('no-priv@xxxxxxxxxxxxx')
- >>> account = account_set.getByEmail('no-priv@xxxxxxxxxxxxx')
- >>> IAccount.providedBy(account)
- True
-
-If the account is not found, a LookupError is raised.
-
- >>> account_set.getByEmail('invalid@whatever')
- Traceback (most recent call last):
- ...
- LookupError:...
-
-Only admins or the person attached to an account can see or edit Account
-details. This is obviously wrong, as the account should have access
-rather than the (optional) attached person. In particular, it means
-Accounts without Person records cannot be managed by the Account owner.
-Fixing this involves more surgery to Launchpad's security systems.
-
- >>> stub_account = account_set.getByEmail('stuart.bishop@xxxxxxxxxxxxx')
- >>> stub_account.date_created
- Traceback (most recent call last):
- ...
- Unauthorized...
-
- >>> del stub_account
+ >>> account = getUtility(IPersonSet).getByEmail(
+ ... 'no-priv@xxxxxxxxxxxxx').account
Looking up accounts by their database ID
@@ -102,49 +73,16 @@
True
>>> login('no-priv@xxxxxxxxxxxxx')
-An account has a displayname, and a preferred email address.
+An account has a displayname.
>>> print account.displayname
No Privileges Person
- >>> print account.preferredemail.email
- no-priv@xxxxxxxxxxxxx
Account objects have a useful string representation.
>>> account
<Account 'No Privileges Person' (Active account)>
-The account can have additional validated and guessed email
-addresses. This will be empty if the user has only a single validated
-email address.
-
- >>> [email.email for email in account.validated_emails]
- []
- >>> [email.email for email in account.guessed_emails]
- []
-
-If we add a new guessed email address, it will be included in the
-guessed list.
-
- >>> from lp.services.identity.interfaces.emailaddress import (
- ... EmailAddressStatus,
- ... IEmailAddressSet,
- ... )
- >>> email = getUtility(IEmailAddressSet).new(
- ... "guessed-email@xxxxxxxxxxx", account=account,
- ... status=EmailAddressStatus.NEW)
- >>> [email.email for email in account.guessed_emails]
- [u'guessed-email@xxxxxxxxxxx']
-
-If we add a validated email address, it will show up in the validated
-list.
-
- >>> email = getUtility(IEmailAddressSet).new(
- ... "validated-email@xxxxxxxxxxx", account=account,
- ... status=EmailAddressStatus.VALIDATED)
- >>> [email.email for email in account.validated_emails]
- [u'validated-email@xxxxxxxxxxx']
-
It also has an encrypted password.
>>> print account.password
@@ -226,8 +164,6 @@
Passwordless
>>> print passwordless_account.password
None
- >>> print passwordless_account.preferredemail
- None
The new() method accepts the optional parameters of password and
password_is_encrypted. If password_is_encrypted is False, the default,
@@ -249,95 +185,3 @@
>>> Store.of(clear_account).flush()
>>> print clear_account.password
clear_password
-
-
-Valid Accounts
---------------
-
-Like person objects, an account is considered valid if it is in the
-active state and has a preferred email address. So a newly created
-account with no email address is not valid.
-
- >>> account = account_set.new(
- ... AccountCreationRationale.USER_CREATED,
- ... "Valid Account Test")
- >>> account.status = AccountStatus.ACTIVE
- >>> account.is_valid
- False
-
-Let's add a new email address to the account.
-
- >>> email = getUtility(IEmailAddressSet).new(
- ... "valid-account-test@xxxxxxxxxxx", account=account)
- >>> account.is_valid
- False
-
-The account is still not valid because it has no preferred email.
-Setting the email to preferred fixes this.
-
- >>> from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
- >>> email.status = EmailAddressStatus.PREFERRED
- >>> account.is_valid
- True
-
-If the account is deactivated, it won't be considered valid any more:
-
- >>> account.status = AccountStatus.DEACTIVATED
- >>> account.is_valid
- False
-
-
-Creating an IPerson for an Account
-----------------------------------
-
-Newly created accounts without an associated Person can be 'promoted' to full
-Launchpad accounts with an attached Person.
-
- # We need to change database policy here again, as the SSO Server cannot
- # modify tables in the lpmain replication set.
- >>> from lp.services.webapp.dbpolicy import MasterDatabasePolicy
- >>> from lp.services.webapp.interfaces import IStoreSelector
- >>> getUtility(IStoreSelector).push(MasterDatabasePolicy())
-
- >>> from lp.registry.interfaces.person import PersonCreationRationale
- >>> fresh_account, email = account_set.createAccountAndEmail(
- ... 'foo@xxxxxxxxxxx',
- ... AccountCreationRationale.OWNER_CREATED_UBUNTU_SHOP,
- ... 'Display name', 'password')
- >>> IPerson(fresh_account)
- Traceback (most recent call last):
- ...
- TypeError: ('Could not adapt', ...
-
- >>> person = fresh_account.createPerson(
- ... PersonCreationRationale.OWNER_CREATED_LAUNCHPAD)
- >>> transaction.commit()
- >>> person.account == fresh_account
- True
- >>> IPerson(fresh_account) == person
- True
- >>> person.preferredemail == fresh_account.preferredemail
- True
- >>> person.creation_rationale
- <DBItem PersonCreationRationale.OWNER_CREATED_LAUNCHPAD...
-
-However, if the account has an associated person or has no preferred email
-address, a new Person cannot be created.
-
- >>> person = fresh_account.createPerson(
- ... PersonCreationRationale.OWNER_CREATED_LAUNCHPAD)
- Traceback (most recent call last):
- ...
- AssertionError: Can't create a Person for an account which already has
- one.
-
- >>> print clear_account.preferredemail
- None
- >>> person = clear_account.createPerson(
- ... PersonCreationRationale.OWNER_CREATED_LAUNCHPAD)
- Traceback (most recent call last):
- ...
- AssertionError: Can't create a Person for an account which has no email.
-
- >>> db_policy = getUtility(IStoreSelector).pop()
-
=== modified file 'lib/lp/services/identity/interfaces/account.py'
--- lib/lp/services/identity/interfaces/account.py 2011-12-24 16:54:44 +0000
+++ lib/lp/services/identity/interfaces/account.py 2012-01-12 10:41:14 +0000
@@ -23,21 +23,15 @@
DBEnumeratedType,
DBItem,
)
-from lazr.restful.fields import (
- CollectionField,
- Reference,
- )
from zope.interface import (
Attribute,
Interface,
)
from zope.schema import (
- Bool,
Choice,
Datetime,
Int,
Text,
- TextLine,
)
from lp import _
@@ -223,58 +217,6 @@
title=_("The status of this account"), required=True,
readonly=False, vocabulary=AccountStatus)
- is_valid = Bool(
- title=_("True if this account is active and has a valid email."),
- required=True, readonly=True)
-
- # We should use schema=IEmailAddress here, but we can't because that would
- # cause circular dependencies.
- preferredemail = Reference(
- title=_("Preferred email address"),
- description=_("The preferred email address for this person. "
- "The one we'll use to communicate with them."),
- readonly=True, required=False, schema=Interface)
-
- validated_emails = CollectionField(
- title=_("Confirmed e-mails of this account."),
- description=_(
- "Confirmed e-mails are the ones in the VALIDATED state. The "
- "user has confirmed that they are active and that they control "
- "them."),
- readonly=True, required=False,
- value_type=Reference(schema=Interface))
-
- guessed_emails = CollectionField(
- title=_("Guessed e-mails of this account."),
- description=_(
- "Guessed e-mails are the ones in the NEW state. We believe "
- "that the user owns the address, but they have not confirmed "
- "the fact."),
- readonly=True, required=False,
- value_type=Reference(schema=Interface))
-
- def setPreferredEmail(email):
- """Set the given email address as this account's preferred one.
-
- If ``email`` is None, the preferred email address is unset, which
- will make the account invalid.
- """
-
- def validateAndEnsurePreferredEmail(email):
- """Ensure this account has a preferred email.
-
- If this account doesn't have a preferred email, <email> will be set as
- this account's preferred one. Otherwise it'll be set as VALIDATED and
- this account will keep their old preferred email.
-
- This method is meant to be the only one to change the status of an
- email address, but as we all know the real world is far from ideal
- and we have to deal with this in one more place, which is the case
- when people explicitly want to change their preferred email address.
- On that case, though, all we have to do is use
- account.setPreferredEmail().
- """
-
class IAccountPrivate(Interface):
"""Private information on an `IAccount`."""
@@ -290,16 +232,6 @@
password = PasswordField(
title=_("Password."), readonly=False, required=True)
- def createPerson(rationale, name=None, comment=None):
- """Create and return a new `IPerson` associated with this account.
-
- :param rationale: A member of `AccountCreationRationale`.
- :param name: Specify a name for the `IPerson` instead of
- using an automatically generated one.
- :param comment: Populate `IPerson.creation_comment`. See
- `IPerson`.
- """
-
class IAccountSpecialRestricted(Interface):
"""Attributes of `IAccount` protected with launchpad.Special."""
@@ -312,12 +244,7 @@
title=_("Why are you deactivating your account?"),
required=False, readonly=False)
- # XXX sinzui 2008-07-14 bug=248518:
- # This method would assert the password is not None, but
- # setPreferredEmail() passes the Person's current password, which may
- # be None. Once that callsite is fixed, we will be able to check that the
- # password is not None here and get rid of the reactivate() method below.
- def activate(comment, password, preferred_email):
+ def reactivate(comment, password):
"""Activate this account.
Set the account status to ACTIVE, the account's password to the given
@@ -325,15 +252,6 @@
:param comment: An explanation of why the account status changed.
:param password: The user's password.
- :param preferred_email: The `EmailAddress` to set as the account's
- preferred email address. It cannot be None.
- """
-
- def reactivate(comment, password, preferred_email):
- """Reactivate this account.
-
- Just like `IAccountSpecialRestricted`.activate() above, but here the
- password can't be None or the empty string.
"""
@@ -364,24 +282,6 @@
:raises LookupError: If the account is not found.
"""
- def createAccountAndEmail(email, rationale, displayname, password,
- password_is_encrypted=False):
- """Create and return both a new `IAccount` and `IEmailAddress`.
-
- The account will be in the ACTIVE state, with the email address set as
- its preferred email address.
- """
-
- def getByEmail(email):
- """Return the `IAccount` linked to the given email address.
-
- :param email: A string, not an `IEmailAddress` provider.
-
- :return: An `IAccount`.
-
- :raises LookupError: If the account is not found.
- """
-
def getByOpenIDIdentifier(openid_identity):
"""Return the `IAccount` with the given OpenID identifier.
@@ -390,4 +290,3 @@
:return: An `IAccount`
:raises LookupError: If the account is not found.
"""
-
=== modified file 'lib/lp/services/identity/interfaces/emailaddress.py'
--- lib/lp/services/identity/interfaces/emailaddress.py 2012-01-05 00:15:32 +0000
+++ lib/lp/services/identity/interfaces/emailaddress.py 2012-01-12 10:41:14 +0000
@@ -32,7 +32,6 @@
from lp import _
from lp.registry.interfaces.role import IHasOwner
-from lp.services.identity.interfaces.account import IAccount
class InvalidEmailAddress(Exception):
@@ -99,8 +98,6 @@
status = Choice(
title=_('Email Address Status'), required=True, readonly=False,
vocabulary=EmailAddressStatus)
- account = Object(title=_('Account'), schema=IAccount, required=False)
- accountID = Int(title=_('AccountID'), required=False, readonly=True)
person = exported(
Reference(title=_('Person'), required=False, readonly=False,
schema=Interface))
@@ -131,12 +128,10 @@
class IEmailAddressSet(Interface):
"""The set of EmailAddresses."""
- def new(email, person=None, status=EmailAddressStatus.NEW, account=None):
+ def new(email, person=None, status=EmailAddressStatus.NEW):
"""Create a new EmailAddress with the given email.
- The newly created EmailAddress will point to the person
- and/or account. If account is omitted and the person has a linked
- account, that account will be used.
+ The newly created EmailAddress will point to the person.
The given status must be an item of EmailAddressStatus.
=== modified file 'lib/lp/services/identity/model/account.py'
--- lib/lp/services/identity/model/account.py 2011-12-30 06:14:56 +0000
+++ lib/lp/services/identity/model/account.py 2012-01-12 10:41:14 +0000
@@ -15,16 +15,13 @@
StringCol,
)
from storm.locals import ReferenceSet
-from storm.store import Store
from zope.component import getUtility
from zope.interface import implements
-from zope.security.proxy import removeSecurityProxy
from lp.services.database.constants import UTC_NOW
from lp.services.database.datetimecol import UtcDateTimeCol
from lp.services.database.enumcol import EnumCol
from lp.services.database.lpstorm import (
- IMasterObject,
IMasterStore,
IStore,
)
@@ -35,12 +32,6 @@
IAccount,
IAccountSet,
)
-from lp.services.identity.interfaces.emailaddress import (
- EmailAddressStatus,
- IEmailAddress,
- IEmailAddressSet,
- )
-from lp.services.identity.model.emailaddress import EmailAddress
from lp.services.openid.model.openididentifier import OpenIdIdentifier
from lp.services.webapp.interfaces import IPasswordEncryptor
@@ -70,100 +61,11 @@
return "<%s '%s' (%s)>" % (
self.__class__.__name__, displayname, self.status)
- def _getEmails(self, status):
- """Get related `EmailAddress` objects with the given status."""
- result = IStore(EmailAddress).find(
- EmailAddress, accountID=self.id, status=status)
- result.order_by(EmailAddress.email.lower())
- return result
-
- @property
- def preferredemail(self):
- """See `IAccount`."""
- return self._getEmails(EmailAddressStatus.PREFERRED).one()
-
- @property
- def validated_emails(self):
- """See `IAccount`."""
- return self._getEmails(EmailAddressStatus.VALIDATED)
-
- @property
- def guessed_emails(self):
- """See `IAccount`."""
- return self._getEmails(EmailAddressStatus.NEW)
-
- def setPreferredEmail(self, email):
- """See `IAccount`."""
- if email is None:
- # Mark preferred email address as validated, if it exists.
- # XXX 2009-03-30 jamesh bug=349482: we should be able to
- # use ResultSet.set() here :(
- for address in self._getEmails(EmailAddressStatus.PREFERRED):
- address.status = EmailAddressStatus.VALIDATED
- return
-
- if not IEmailAddress.providedBy(email):
- raise TypeError("Any person's email address must provide the "
- "IEmailAddress Interface. %r doesn't." % email)
-
- email = IMasterObject(removeSecurityProxy(email))
- assert email.accountID == self.id
-
- # If we have the preferred email address here, we're done.
- if email.status == EmailAddressStatus.PREFERRED:
- return
-
- existing_preferred_email = self.preferredemail
- if existing_preferred_email is not None:
- assert Store.of(email) is Store.of(existing_preferred_email), (
- "Store of %r is not the same as store of %r" %
- (email, existing_preferred_email))
- existing_preferred_email.status = EmailAddressStatus.VALIDATED
- # Make sure the old preferred email gets flushed before
- # setting the new preferred email.
- Store.of(email).add_flush_order(existing_preferred_email, email)
-
- email.status = EmailAddressStatus.PREFERRED
-
- def validateAndEnsurePreferredEmail(self, email):
- """See `IAccount`."""
- if not IEmailAddress.providedBy(email):
- raise TypeError(
- "Any person's email address must provide the IEmailAddress "
- "interface. %s doesn't." % email)
-
- assert email.accountID == self.id, 'Wrong account! %r, %r' % (
- email.accountID, self.id)
-
- # This email is already validated and is this person's preferred
- # email, so we have nothing to do.
- if email.status == EmailAddressStatus.PREFERRED:
- return
-
- email = IMasterObject(email)
-
- if self.preferredemail is None:
- # This branch will be executed only in the first time a person
- # uses Launchpad. Either when creating a new account or when
- # resetting the password of an automatically created one.
- self.setPreferredEmail(email)
- else:
- email.status = EmailAddressStatus.VALIDATED
-
- def activate(self, comment, password, preferred_email):
+ def reactivate(self, comment, password):
"""See `IAccountSpecialRestricted`."""
- if preferred_email is None:
- raise AssertionError(
- "Account %s cannot be activated without a "
- "preferred email address." % self.id)
self.status = AccountStatus.ACTIVE
self.status_comment = comment
self.password = password
- self.validateAndEnsurePreferredEmail(preferred_email)
-
- def reactivate(self, comment, password, preferred_email):
- """See `IAccountSpecialRestricted`."""
- self.activate(comment, password, preferred_email)
# The password is actually stored in a separate table for security
# reasons, so use a property to hide this implementation detail.
@@ -201,39 +103,6 @@
password = property(_get_password, _set_password)
- @property
- def is_valid(self):
- """See `IAccount`."""
- if self.status != AccountStatus.ACTIVE:
- return False
- return self.preferredemail is not None
-
- def createPerson(self, rationale, name=None, comment=None):
- """See `IAccount`."""
- # Need a local import because of circular dependencies.
- from lp.registry.model.person import (
- generate_nick, Person, PersonSet)
- assert self.preferredemail is not None, (
- "Can't create a Person for an account which has no email.")
- person = IMasterStore(Person).find(Person, accountID=self.id).one()
- assert person is None, (
- "Can't create a Person for an account which already has one.")
- if name is None:
- name = generate_nick(self.preferredemail.email)
- person = PersonSet()._newPerson(
- name, self.displayname, hide_email_addresses=True,
- rationale=rationale, account=self, comment=comment)
-
- # Update all associated email addresses to point at the new person.
- result = IMasterStore(EmailAddress).find(
- EmailAddress, accountID=self.id)
- # XXX 2009-03-30 jamesh bug=349482: we should be able to
- # use ResultSet.set() here :(
- for email in result:
- email.personID = person.id
-
- return person
-
class AccountSet:
"""See `IAccountSet`."""
@@ -269,39 +138,6 @@
raise LookupError(id)
return account
- def createAccountAndEmail(self, email, rationale, displayname, password,
- password_is_encrypted=False,
- openid_identifier=None):
- """See `IAccountSet`."""
- # Convert the PersonCreationRationale to an AccountCreationRationale.
- account_rationale = getattr(AccountCreationRationale, rationale.name)
- account = self.new(
- account_rationale, displayname, password=password,
- password_is_encrypted=password_is_encrypted,
- openid_identifier=openid_identifier)
- account.status = AccountStatus.ACTIVE
- email = getUtility(IEmailAddressSet).new(
- email, status=EmailAddressStatus.PREFERRED, account=account)
- return account, email
-
- def getByEmail(self, email):
- """See `IAccountSet`."""
- store = IStore(Account)
- try:
- email = email.decode('US-ASCII')
- except (UnicodeDecodeError, UnicodeEncodeError):
- # Non-ascii email addresses are not legal, so assume there are no
- # matching addresses in Launchpad.
- raise LookupError(repr(email))
- account = store.find(
- Account,
- EmailAddress.account == Account.id,
- EmailAddress.email.lower()
- == email.strip().lower()).one()
- if account is None:
- raise LookupError(email)
- return account
-
def getByOpenIDIdentifier(self, openid_identifier):
"""See `IAccountSet`."""
store = IStore(Account)
=== modified file 'lib/lp/services/identity/model/emailaddress.py'
--- lib/lp/services/identity/model/emailaddress.py 2012-01-05 00:15:32 +0000
+++ lib/lp/services/identity/model/emailaddress.py 2012-01-12 10:41:14 +0000
@@ -56,9 +56,6 @@
dbName='email', notNull=True, unique=True, alternateID=True)
status = EnumCol(dbName='status', schema=EmailAddressStatus, notNull=True)
person = ForeignKey(dbName='person', foreignKey='Person', notNull=False)
- account = ForeignKey(
- dbName='account', foreignKey='Account', notNull=False,
- default=None)
def __repr__(self):
return '<EmailAddress at 0x%x <%s> [%s]>' % (
@@ -116,8 +113,7 @@
return EmailAddress.selectOne(
"lower(email) = %s" % quote(email.strip().lower()))
- def new(self, email, person=None, status=EmailAddressStatus.NEW,
- account=None):
+ def new(self, email, person=None, status=EmailAddressStatus.NEW):
"""See IEmailAddressSet."""
email = email.strip()
@@ -129,26 +125,11 @@
raise EmailAddressAlreadyTaken(
"The email address '%s' is already registered." % email)
assert status in EmailAddressStatus.items
- if person is None:
- personID = None
- else:
- if account is None:
- account = person.account
- personID = person.id
- accountID = account and account.id
- assert person.accountID == accountID, (
- "Email address '%s' must be linked to same account as "
- "person '%s'. Expected %r (%s), got %r (%s)" % (
- email, person.name, person.account, person.accountID,
- account, accountID))
- # We use personID instead of just person, as in some cases the
- # Person record will not yet be replicated from the main
- # Store to the auth master Store.
+ assert person
return EmailAddress(
email=email,
status=status,
- personID=personID,
- account=account)
+ person=person)
class UndeletableEmailAddress(Exception):
=== modified file 'lib/lp/services/identity/tests/test_account.py'
--- lib/lp/services/identity/tests/test_account.py 2012-01-12 08:08:03 +0000
+++ lib/lp/services/identity/tests/test_account.py 2012-01-12 10:41:14 +0000
@@ -6,22 +6,7 @@
__metaclass__ = type
__all__ = []
-from testtools.testcase import ExpectedException
-import transaction
-from zope.component import getUtility
-
-from lp.registry.interfaces.person import (
- IPerson,
- PersonCreationRationale,
- )
-from lp.services.identity.interfaces.account import (
- AccountCreationRationale,
- IAccountSet,
- )
-from lp.services.identity.interfaces.emailaddress import EmailAddressStatus
-from lp.testing import (
- TestCaseWithFactory,
- )
+from lp.testing import TestCaseWithFactory
from lp.testing.layers import DatabaseFunctionalLayer
@@ -41,180 +26,3 @@
distro = self.factory.makeAccount(u'\u0170-account')
ignore, displayname, status_1, status_2 = repr(distro).rsplit(' ', 3)
self.assertEqual("'\\u0170-account'", displayname)
-
-
-class CreatePersonTests(TestCaseWithFactory):
- """Tests for `IAccount.createPerson`."""
-
- layer = DatabaseFunctionalLayer
-
- def setUp(self):
- super(CreatePersonTests, self).setUp(user='admin@xxxxxxxxxxxxx')
-
- def test_createPerson(self):
- account = self.factory.makeAccount("Test Account")
- # Account has no person.
- self.assertEqual(IPerson(account, None), None)
- self.assertEqual(account.preferredemail.person, None)
-
- person = account.createPerson(PersonCreationRationale.UNKNOWN)
- transaction.commit()
- self.assertNotEqual(person, None)
- self.assertEqual(person.account, account)
- self.assertEqual(IPerson(account), person)
- self.assertEqual(account.preferredemail.person, person)
-
- # Trying to create a person for an account with a person fails.
- self.assertRaises(AssertionError, account.createPerson,
- PersonCreationRationale.UNKNOWN)
-
- def test_createPerson_requires_email(self):
- # It isn't possible to create a person for an account with no
- # preferred email address.
- account = getUtility(IAccountSet).new(
- AccountCreationRationale.UNKNOWN, "Test Account")
- self.assertEqual(account.preferredemail, None)
- self.assertRaises(AssertionError, account.createPerson,
- PersonCreationRationale.UNKNOWN)
-
- def test_createPerson_sets_EmailAddress_person(self):
- # All email addresses for the account are associated with the
- # new person
- account = self.factory.makeAccount("Test Account")
- valid_email = self.factory.makeEmail(
- "validated@xxxxxxxxxxx", None, account,
- EmailAddressStatus.VALIDATED)
- new_email = self.factory.makeEmail(
- "new@xxxxxxxxxxx", None, account,
- EmailAddressStatus.NEW)
- old_email = self.factory.makeEmail(
- "old@xxxxxxxxxxx", None, account,
- EmailAddressStatus.OLD)
-
- person = account.createPerson(PersonCreationRationale.UNKNOWN)
- transaction.commit()
- self.assertEqual(valid_email.person, person)
- self.assertEqual(new_email.person, person)
- self.assertEqual(old_email.person, person)
-
- def test_createPerson_uses_name(self):
- # A optional user name can be provided. Normally the name is
- # generated from the email address.
- account = self.factory.makeAccount("Test Account")
- person = account.createPerson(
- PersonCreationRationale.UNKNOWN, name="sam.bell")
- self.failUnlessEqual("sam.bell", person.name)
-
- def test_createPerson_uses_comment(self):
- # An optional creation comment can be provided.
- account = self.factory.makeAccount("Test Account")
- person = account.createPerson(
- PersonCreationRationale.UNKNOWN,
- comment="when importing He-3 from the Moon")
- self.failUnlessEqual(
- "when importing He-3 from the Moon",
- person.creation_comment)
-
- def test_getByEmail_non_ascii_bytes(self):
- """Lookups for non-ascii addresses should raise LookupError.
-
- This tests the case where input is a bytestring.
- """
- with ExpectedException(LookupError, r"'SaraS\\xe1nchez@xxxxxxxxxxx'"):
- getUtility(IAccountSet).getByEmail('SaraS\xe1nchez@xxxxxxxxxxx')
-
- def test_getByEmail_non_ascii_unicode(self):
- """Lookups for non-ascii addresses should raise LookupError.
-
- This tests the case where input is a unicode string.
- """
- with ExpectedException(LookupError, r"u'SaraS\\xe1nchez@.*.net'"):
- getUtility(IAccountSet).getByEmail(u'SaraS\xe1nchez@xxxxxxxxxxx')
-
-
-class EmailManagementTests(TestCaseWithFactory):
- """Test email account management interfaces for `IAccount`."""
-
- layer = DatabaseFunctionalLayer
-
- def setUp(self):
- super(EmailManagementTests, self).setUp(user='admin@xxxxxxxxxxxxx')
-
- def test_setPreferredEmail(self):
- # Setting a new preferred email marks the old one as VALIDATED.
- account = self.factory.makeAccount("Test Account")
- first_email = account.preferredemail
- second_email = self.factory.makeEmail(
- "second-email@xxxxxxxxxxx", None, account,
- EmailAddressStatus.VALIDATED)
- transaction.commit()
- account.setPreferredEmail(second_email)
- transaction.commit()
- self.assertEqual(account.preferredemail, second_email)
- self.assertEqual(second_email.status, EmailAddressStatus.PREFERRED)
- self.assertEqual(first_email.status, EmailAddressStatus.VALIDATED)
-
- def test_setPreferredEmail_None(self):
- # Setting the preferred email to None sets the old preferred
- # email to VALIDATED.
- account = self.factory.makeAccount("Test Account")
- email = account.preferredemail
- transaction.commit()
- account.setPreferredEmail(None)
- transaction.commit()
- self.assertEqual(account.preferredemail, None)
- self.assertEqual(email.status, EmailAddressStatus.VALIDATED)
-
- def test_validateAndEnsurePreferredEmail(self):
- # validateAndEnsurePreferredEmail() sets the email status to
- # VALIDATED if there is no existing preferred email.
- account = self.factory.makeAccount("Test Account")
- self.assertNotEqual(account.preferredemail, None)
- new_email = self.factory.makeEmail(
- "new-email@xxxxxxxxxxx", None, account,
- EmailAddressStatus.NEW)
- transaction.commit()
- account.validateAndEnsurePreferredEmail(new_email)
- transaction.commit()
- self.assertEqual(new_email.status, EmailAddressStatus.VALIDATED)
-
- def test_validateAndEsnurePreferredEmail_no_preferred(self):
- # validateAndEnsurePreferredEmail() sets the new email as
- # preferred if there was no preferred email.
- account = self.factory.makeAccount("Test Account")
- account.setPreferredEmail(None)
- new_email = self.factory.makeEmail(
- "new-email@xxxxxxxxxxx", None, account,
- EmailAddressStatus.NEW)
- transaction.commit()
- account.validateAndEnsurePreferredEmail(new_email)
- transaction.commit()
- self.assertEqual(new_email.status, EmailAddressStatus.PREFERRED)
-
- def test_validated_emails(self):
- account = self.factory.makeAccount("Test Account")
- self.factory.makeEmail(
- "new-email@xxxxxxxxxxx", None, account,
- EmailAddressStatus.NEW)
- validated_email = self.factory.makeEmail(
- "validated-email@xxxxxxxxxxx", None, account,
- EmailAddressStatus.VALIDATED)
- self.factory.makeEmail(
- "old@xxxxxxxxxxx", None, account,
- EmailAddressStatus.OLD)
- transaction.commit()
- self.assertContentEqual(account.validated_emails, [validated_email])
-
- def test_guessed_emails(self):
- account = self.factory.makeAccount("Test Account")
- new_email = self.factory.makeEmail(
- "new-email@xxxxxxxxxxx", None, account,
- EmailAddressStatus.NEW)
- self.factory.makeEmail(
- "validated-email@xxxxxxxxxxx", None, account,
- EmailAddressStatus.VALIDATED)
- self.factory.makeEmail(
- "old@xxxxxxxxxxx", None, account,
- EmailAddressStatus.OLD)
- transaction.commit()
- self.assertContentEqual(account.guessed_emails, [new_email])
=== modified file 'lib/lp/services/mail/incoming.py'
--- lib/lp/services/mail/incoming.py 2011-12-30 02:24:09 +0000
+++ lib/lp/services/mail/incoming.py 2012-01-12 10:41:14 +0000
@@ -225,13 +225,7 @@
setupInteraction(authutil.unauthenticatedPrincipal())
return None
- # People with accounts but no related person will have a principal, but
- # the person adaptation will fail.
person = IPerson(principal, None)
- if person is None:
- setupInteraction(authutil.unauthenticatedPrincipal())
- return None
-
if person.account_status != AccountStatus.ACTIVE:
raise InactiveAccount(
"Mail from a user with an inactive account.")
=== modified file 'lib/lp/services/mail/tests/test_incoming.py'
--- lib/lp/services/mail/tests/test_incoming.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/mail/tests/test_incoming.py 2012-01-12 10:41:14 +0000
@@ -104,13 +104,6 @@
mail = self.factory.makeSignedMessage(email_address=unknown)
self.assertThat(authenticateEmail(mail), Is(None))
- def test_accounts_without_person(self):
- # An account without a person should be the same as an unknown email.
- email = 'non-person@xxxxxxxxxxx'
- self.factory.makeAccount(email=email)
- mail = self.factory.makeSignedMessage(email_address=email)
- self.assertThat(authenticateEmail(mail), Is(None))
-
class TestExtractAddresses(TestCaseWithFactory):
=== modified file 'lib/lp/services/verification/browser/logintoken.py'
--- lib/lp/services/verification/browser/logintoken.py 2012-01-05 00:15:32 +0000
+++ lib/lp/services/verification/browser/logintoken.py 2012-01-12 10:41:14 +0000
@@ -441,13 +441,11 @@
validated = (
EmailAddressStatus.VALIDATED, EmailAddressStatus.PREFERRED)
requester = self.context.requester
- account = requester.account
emailset = getUtility(IEmailAddressSet)
email = emailset.getByEmail(self.context.email)
if email is not None:
- if email.personID is not None and (
- requester is None or email.personID != requester.id):
+ if requester is None or email.personID != requester.id:
dupe = email.person
dname = cgi.escape(dupe.name)
# Yes, hardcoding an autogenerated field name is an evil
@@ -463,12 +461,6 @@
'case you should be able to <a href="${url}">merge them'
'</a> into a single one.',
mapping=dict(url=url))))
- elif account is not None and email.accountID != account.id:
- # Email address is owned by a personless account. We
- # can't offer to perform a merge here.
- self.addError(
- 'This email address is already registered for another '
- 'account')
elif email.status in validated:
self.addError(_(
"This email address is already registered and validated "
@@ -523,7 +515,7 @@
def markEmailAsValid(self, email):
"""Mark the given email address as valid."""
- self.context.requester.account.validateAndEnsurePreferredEmail(email)
+ self.context.requester.validateAndEnsurePreferredEmail(email)
class ValidateTeamEmailView(ValidateEmailView):
@@ -595,7 +587,6 @@
# that this new email does not have the PREFERRED status.
email.status = EmailAddressStatus.NEW
email.personID = requester.id
- email.accountID = requester.accountID
requester.validateAndEnsurePreferredEmail(email)
# Need to flush all changes we made, so subsequent queries we make
=== modified file 'lib/lp/services/webapp/authentication.py'
--- lib/lp/services/webapp/authentication.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/authentication.py 2012-01-12 10:41:14 +0000
@@ -68,7 +68,7 @@
if login is not None:
login_src = getUtility(IPlacelessLoginSource)
principal = login_src.getPrincipalByLogin(login)
- if principal is not None and principal.account.is_valid:
+ if principal is not None and principal.person.is_valid_person:
password = credentials.getPassword()
if principal.validate(password):
# We send a LoggedInEvent here, when the
@@ -107,7 +107,7 @@
# available in login source. This happens when account has
# become invalid for some reason, such as being merged.
return None
- elif principal.account.is_valid:
+ elif principal.person.is_valid_person:
login = authdata['login']
assert login, 'login is %s!' % repr(login)
notify(CookieAuthPrincipalIdentifiedEvent(
@@ -276,13 +276,11 @@
validate the password against so it may then email a validation
request to the user and inform them it has done so.
"""
- try:
- account = getUtility(IAccountSet).getByEmail(login)
- except LookupError:
+ person = getUtility(IPersonSet).getByEmail(login)
+ if person is None or person.account is None:
return None
- else:
- return self._principalForAccount(
- account, access_level, scope, want_password)
+ return self._principalForAccount(
+ person.account, access_level, scope, want_password)
def _principalForAccount(self, account, access_level, scope,
want_password=True):
=== modified file 'lib/lp/services/webapp/login.py'
--- lib/lp/services/webapp/login.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/login.py 2012-01-12 10:41:14 +0000
@@ -323,11 +323,11 @@
finally:
timeline_action.finish()
- def login(self, account):
+ def login(self, person):
loginsource = getUtility(IPlacelessLoginSource)
# We don't have a logged in principal, so we must remove the security
# proxy of the account's preferred email.
- email = removeSecurityProxy(account.preferredemail).email
+ email = removeSecurityProxy(person.preferredemail).email
logInPrincipal(
self.request, loginsource.getPrincipalByLogin(email), email)
@@ -383,7 +383,7 @@
return self.suspended_account_template()
with MasterDatabasePolicy():
- self.login(person.account)
+ self.login(person)
if should_update_last_write:
# This is a GET request but we changed the database, so update
=== modified file 'lib/lp/services/webapp/publication.py'
--- lib/lp/services/webapp/publication.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/publication.py 2012-01-12 10:41:14 +0000
@@ -341,9 +341,10 @@
# automated tests.
if request.get('PATH_INFO') not in [u'/+opstats', u'/+haproxy']:
principal = auth_utility.authenticate(request)
- if principal is None or principal.person is None:
- # This is either an unauthenticated user or a user who
- # authenticated on our OpenID server using a personless account.
+ if principal is not None:
+ assert principal.person is not None
+ else:
+ # This is an unauthenticated user.
principal = auth_utility.unauthenticatedPrincipal()
assert principal is not None, "Missing unauthenticated principal."
return principal
=== modified file 'lib/lp/services/webapp/tests/test_authentication.py'
--- lib/lp/services/webapp/tests/test_authentication.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/tests/test_authentication.py 2012-01-12 10:41:14 +0000
@@ -9,17 +9,7 @@
import unittest
from contrib.oauth import OAuthRequest
-from zope.app.security.principalregistry import UnauthenticatedPrincipal
-
-from lp.services.config import config
-from lp.services.webapp.authentication import LaunchpadPrincipal
-from lp.services.webapp.login import logInPrincipal
-from lp.services.webapp.publication import LaunchpadBrowserPublication
-from lp.services.webapp.servers import LaunchpadTestRequest
-from lp.testing import (
- login,
- TestCaseWithFactory,
- )
+from lp.testing import TestCaseWithFactory
from lp.testing.layers import (
DatabaseFunctionalLayer,
LaunchpadFunctionalLayer,
@@ -31,32 +21,6 @@
)
-class TestAuthenticationOfPersonlessAccounts(TestCaseWithFactory):
- layer = DatabaseFunctionalLayer
-
- def setUp(self):
- TestCaseWithFactory.setUp(self)
- self.email = 'baz@xxxxxxxxxxx'
- self.request = LaunchpadTestRequest()
- self.account = self.factory.makeAccount(
- 'Personless account', email=self.email)
- self.principal = LaunchpadPrincipal(
- self.account.id, self.account.displayname,
- self.account.displayname, self.account)
- login(self.email)
-
- def test_navigate_anonymously_on_launchpad_dot_net(self):
- # A user with the credentials of a personless account will browse
- # launchpad.net anonymously.
- logInPrincipal(self.request, self.principal, self.email)
- self.request.response.setCookie(
- config.launchpad_session.cookie, 'xxx')
-
- publication = LaunchpadBrowserPublication(None)
- principal = publication.getPrincipal(self.request)
- self.failUnless(isinstance(principal, UnauthenticatedPrincipal))
-
-
class TestOAuthParsing(TestCaseWithFactory):
layer = DatabaseFunctionalLayer
=== modified file 'lib/lp/services/webapp/tests/test_authutility.py'
--- lib/lp/services/webapp/tests/test_authutility.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/tests/test_authutility.py 2012-01-12 10:41:14 +0000
@@ -31,16 +31,16 @@
class DummyPerson(object):
implements(IPerson)
- is_valid = True
+ is_valid_person = True
class DummyAccount(object):
implements(IAccount)
- is_valid = True
person = DummyPerson()
Bruce = LaunchpadPrincipal(42, 'bruce', 'Bruce', DummyAccount(), 'bruce!')
+Bruce.person = Bruce.account.person
class DummyPlacelessLoginSource(object):
=== modified file 'lib/lp/services/webapp/tests/test_login.py'
--- lib/lp/services/webapp/tests/test_login.py 2012-01-04 11:57:57 +0000
+++ lib/lp/services/webapp/tests/test_login.py 2012-01-12 10:41:14 +0000
@@ -292,25 +292,6 @@
self.assertEquals(
view.fake_consumer.requested_url, 'http://example.com?foo=bar')
- def test_personless_account(self):
- # When there is no Person record associated with the account, we
- # create one.
- account = self.factory.makeAccount('Test account')
- self.assertIs(None, IPerson(account, None))
- with SRegResponse_fromSuccessResponse_stubbed():
- view, html = self._createViewWithResponse(account)
- self.assertIsNot(None, IPerson(account, None))
- self.assertTrue(view.login_called)
- response = view.request.response
- self.assertEquals(httplib.TEMPORARY_REDIRECT, response.getStatus())
- self.assertEquals(view.request.form['starting_url'],
- response.getHeader('Location'))
-
- # We also update the last_write flag in the session, to make sure
- # further requests use the master DB and thus see the newly created
- # stuff.
- self.assertLastWriteIsSet(view.request)
-
def test_unseen_identity(self):
# When we get a positive assertion about an identity URL we've never
# seen, we automatically register an account with that identity
@@ -329,11 +310,11 @@
account = account_set.getByOpenIDIdentifier(identifier)
self.assertIsNot(None, account)
self.assertEquals(AccountStatus.ACTIVE, account.status)
+ person = IPerson(account, None)
+ self.assertIsNot(None, person)
+ self.assertEquals('Foo User', person.displayname)
self.assertEquals('non-existent@xxxxxxxxxxx',
- removeSecurityProxy(account.preferredemail).email)
- person = IPerson(account, None)
- self.assertIsNot(None, person)
- self.assertEquals('Foo User', person.displayname)
+ removeSecurityProxy(person.preferredemail).email)
# We also update the last_write flag in the session, to make sure
# further requests use the master DB and thus see the newly created
=== modified file 'lib/lp/services/webapp/tests/test_login_account.py'
--- lib/lp/services/webapp/tests/test_login_account.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/webapp/tests/test_login_account.py 2012-01-12 10:41:14 +0000
@@ -17,7 +17,6 @@
from lp.services.webapp.authentication import LaunchpadPrincipal
from lp.services.webapp.interfaces import (
CookieAuthLoggedInEvent,
- ILaunchpadPrincipal,
IPlacelessAuthUtility,
)
from lp.services.webapp.login import (
@@ -27,7 +26,6 @@
)
from lp.services.webapp.servers import LaunchpadTestRequest
from lp.testing import (
- ANONYMOUS,
login,
TestCaseWithFactory,
)
@@ -172,34 +170,3 @@
principal = getUtility(IPlacelessAuthUtility).authenticate(
self.request)
self.failUnless(principal is None)
-
-
-class TestLoggingInWithPersonlessAccount(TestCaseWithFactory):
- layer = DatabaseFunctionalLayer
-
- def setUp(self):
- TestCaseWithFactory.setUp(self)
- self.request = LaunchpadTestRequest()
- login(ANONYMOUS)
- account_set = getUtility(IAccountSet)
- account, email = account_set.createAccountAndEmail(
- 'foo@xxxxxxxxxxx', AccountCreationRationale.UNKNOWN,
- 'Display name', 'password')
- self.principal = LaunchpadPrincipal(
- account.id, account.displayname, account.displayname, account)
- login('foo@xxxxxxxxxxx')
-
- def test_logInPrincipal(self):
- # logInPrincipal() will log the given principal in and not worry about
- # its lack of an associated Person.
- logInPrincipal(self.request, self.principal, 'foo@xxxxxxxxxxx')
-
- # Ensure we are using cookie auth.
- self.assertIsNotNone(
- self.request.response.getCookie(config.launchpad_session.cookie)
- )
-
- placeless_auth_utility = getUtility(IPlacelessAuthUtility)
- principal = placeless_auth_utility.authenticate(self.request)
- self.failUnless(ILaunchpadPrincipal.providedBy(principal))
- self.failUnless(principal.person is None)
=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py 2012-01-04 23:49:46 +0000
+++ lib/lp/testing/factory.py 2012-01-12 10:41:14 +0000
@@ -561,7 +561,7 @@
pocket)
return ProxyFactory(location)
- def makeAccount(self, displayname=None, email=None, password=None,
+ def makeAccount(self, displayname=None, password=None,
status=AccountStatus.ACTIVE,
rationale=AccountCreationRationale.UNKNOWN):
"""Create and return a new Account."""
@@ -570,13 +570,6 @@
account = getUtility(IAccountSet).new(
rationale, displayname, password=password)
removeSecurityProxy(account).status = status
- if email is None:
- email = self.getUniqueEmailAddress()
- email_status = EmailAddressStatus.PREFERRED
- if status != AccountStatus.ACTIVE:
- email_status = EmailAddressStatus.NEW
- email = self.makeEmail(
- email, person=None, account=account, email_status=email_status)
self.makeOpenIdIdentifier(account)
return account
@@ -731,10 +724,8 @@
# setPreferredEmail no longer activates the account
# automatically.
account = IMasterStore(Account).get(Account, person.accountID)
- account.activate(
- "Activated by factory.makePersonByName",
- password='foo',
- preferred_email=email)
+ account.reactivate(
+ "Activated by factory.makePersonByName", password='foo')
person.setPreferredEmail(email)
if not use_default_autosubscribe_policy:
@@ -745,20 +736,16 @@
MailingListAutoSubscribePolicy.NEVER)
account = IMasterStore(Account).get(Account, person.accountID)
getUtility(IEmailAddressSet).new(
- alternative_address, person, EmailAddressStatus.VALIDATED,
- account)
+ alternative_address, person, EmailAddressStatus.VALIDATED)
return person
- def makeEmail(self, address, person, account=None, email_status=None):
+ def makeEmail(self, address, person, email_status=None):
"""Create a new email address for a person.
:param address: The email address to create.
:type address: string
:param person: The person to assign the email address to.
:type person: `IPerson`
- :param account: The account to assign the email address to. Will use
- the given person's account if None is provided.
- :type person: `IAccount`
:param email_status: The default status of the email address,
if given. If not given, `EmailAddressStatus.VALIDATED`
will be used.
@@ -769,7 +756,7 @@
if email_status is None:
email_status = EmailAddressStatus.VALIDATED
return getUtility(IEmailAddressSet).new(
- address, person, email_status, account)
+ address, person, email_status)
def makeTeam(self, owner=None, displayname=None, email=None, name=None,
description=None, icon=None, logo=None,
=== modified file 'lib/lp/testing/tests/test_login.py'
--- lib/lp/testing/tests/test_login.py 2012-01-01 02:58:52 +0000
+++ lib/lp/testing/tests/test_login.py 2012-01-12 10:41:14 +0000
@@ -104,13 +104,6 @@
e = self.assertRaises(ValueError, login_person, team)
self.assertEqual(str(e), "Got team, expected person: %r" % (team,))
- def test_login_account(self):
- # Calling login_person with an account logs you in with that account.
- person = self.factory.makePerson()
- account = person.account
- login_person(account)
- self.assertLoggedIn(person)
-
def test_login_with_email(self):
# login() logs a person in by email.
email = 'test-email@xxxxxxxxxxx'
=== modified file 'lib/lp/testopenid/browser/server.py'
--- lib/lp/testopenid/browser/server.py 2012-01-01 02:58:52 +0000
+++ lib/lp/testopenid/browser/server.py 2012-01-12 10:41:14 +0000
@@ -230,9 +230,10 @@
else:
response = self.openid_request.answer(True)
+ person = IPerson(self.account)
sreg_fields = dict(
- nickname=IPerson(self.account).name,
- email=self.account.preferredemail.email,
+ nickname=person.name,
+ email=person.preferredemail.email,
fullname=self.account.displayname)
sreg_request = SRegRequest.fromOpenIDRequest(self.openid_request)
sreg_response = SRegResponse.extractResponse(
=== modified file 'lib/lp/translations/utilities/tests/test_file_importer.py'
--- lib/lp/translations/utilities/tests/test_file_importer.py 2011-12-30 01:48:17 +0000
+++ lib/lp/translations/utilities/tests/test_file_importer.py 2012-01-12 10:41:14 +0000
@@ -353,21 +353,6 @@
po_importer.potemplate.displayname),
'Did not create the correct comment for %s' % test_email)
- def test_getPersonByEmail_personless_account(self):
- # An Account without a Person attached is a difficult case for
- # _getPersonByEmail: it has to create the Person but re-use an
- # existing Account and email address.
- (pot_importer, po_importer) = self._createImporterForExportedEntries()
- test_email = 'freecdsplease@xxxxxxxxxxx'
- account = self.factory.makeAccount('Send me Ubuntu', test_email)
-
- person = po_importer._getPersonByEmail(test_email)
-
- self.assertEqual(account, person.account)
-
- # The same person will come up for the same address next time.
- self.assertEqual(person, po_importer._getPersonByEmail(test_email))
-
def test_getPersonByEmail_bad_address(self):
# _getPersonByEmail returns None for malformed addresses.
(pot_importer, po_importer) = self._createImporterForExportedEntries()
