William Grant has proposed merging lp:~wgrant/launchpad/bug-918843 into
lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #918843 in Launchpad itself: "IntegrityError: new row for relation
"person" violates check constraint "teams_have_no_account" "
https://bugs.launchpad.net/launchpad/+bug/918843
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/bug-918843/+merge/89392
My refactoring of the login machinery last week broke some untested and
probably unintentional behaviour: if the OpenID provider returned an email
address associated with a team, a new person would be created and steal the
team's address. After my refactoring this case crashes instead.
Since stealing email addresses from teams is probably not something that we
want to happen accidentally, this branch rejects login attempts that try to use
a team email address. feedback@ can then sort out the SSO account or team email
address to let authentication succeed.
SCA also uses that API, but https://pastebin.canonical.com/58485/ and
https://pastebin.canonical.com/58486/ suggest that it will handle it like
AccountSuspendedError, and this is done before payment.
A proper solution that does not induce nausea is achievable, but as discussed
in bug #556680 it is somewhat non-trivial.
--
https://code.launchpad.net/~wgrant/launchpad/bug-918843/+merge/89392
Your team Launchpad code reviewers is requested to review the proposed merge of
lp:~wgrant/launchpad/bug-918843 into lp:launchpad.
=== modified file 'lib/lp/registry/interfaces/person.py'
--- lib/lp/registry/interfaces/person.py 2012-01-15 11:52:24 +0000
+++ lib/lp/registry/interfaces/person.py 2012-01-20 07:03:32 +0000
@@ -35,6 +35,7 @@
'PersonalStanding',
'PRIVATE_TEAM_PREFIX',
'TeamContactMethod',
+ 'TeamEmailAddressError',
'TeamMembershipRenewalPolicy',
'TeamSubscriptionPolicy',
'validate_person',
@@ -2542,6 +2543,10 @@
_message_prefix = "No such person"
+class TeamEmailAddressError(Exception):
+ """The person cannot be created as a team owns its email address."""
+
+
# Fix value_type.schema of IPersonViewRestricted attributes.
for name in [
'all_members_prepopulated',
=== modified file 'lib/lp/registry/model/person.py'
--- lib/lp/registry/model/person.py 2012-01-17 23:55:44 +0000
+++ lib/lp/registry/model/person.py 2012-01-20 07:03:32 +0000
@@ -180,6 +180,7 @@
PersonalStanding,
PersonCreationRationale,
PersonVisibility,
+ TeamEmailAddressError,
TeamMembershipRenewalPolicy,
TeamSubscriptionPolicy,
validate_public_person,
@@ -3094,6 +3095,13 @@
identifier = IStore(OpenIdIdentifier).find(
OpenIdIdentifier, identifier=openid_identifier).one()
+ # XXX wgrant 2012-01-20 bug=556680: This is awful, as it can
+ # lock people out of their account until they change their
+ # SSO address. But stealing addresses from other accounts is
+ # probably worse.
+ if email is not None and email.person.is_team:
+ raise TeamEmailAddressError()
+
if email is None:
if identifier is None:
# Neither the Email Address not the OpenId Identifier
=== modified file 'lib/lp/registry/tests/test_personset.py'
--- lib/lp/registry/tests/test_personset.py 2012-01-12 08:13:59 +0000
+++ lib/lp/registry/tests/test_personset.py 2012-01-20 07:03:32 +0000
@@ -33,6 +33,7 @@
IPersonSet,
PersonCreationRationale,
PersonVisibility,
+ TeamEmailAddressError,
)
from lp.registry.interfaces.personnotification import IPersonNotificationSet
from lp.registry.model.accesspolicy import AccessPolicyGrant
@@ -793,6 +794,18 @@
self.assertIs(found.account, self.identifier.account)
self.assertIn(self.identifier, list(found.account.openid_identifiers))
+ def testTeamEmailAddress(self):
+ # If the EmailAddress is linked to a team, login fails. There is
+ # no way to automatically recover -- someone must manually fix
+ # the email address of the team or the SSO account.
+ self.factory.makeTeam(email="f...@bar.com")
+
+ self.assertRaises(
+ TeamEmailAddressError,
+ self.person_set.getOrCreateByOpenIDIdentifier,
+ u'other-openid-identifier', 'f...@bar.com', 'New Name',
+ PersonCreationRationale.UNKNOWN, 'No Comment')
+
class TestCreatePersonAndEmail(TestCase):
"""Test `IPersonSet`.createPersonAndEmail()."""
=== modified file 'lib/lp/registry/tests/test_xmlrpc.py'
--- lib/lp/registry/tests/test_xmlrpc.py 2012-01-05 00:23:45 +0000
+++ lib/lp/registry/tests/test_xmlrpc.py 2012-01-20 07:03:32 +0000
@@ -86,7 +86,7 @@
removeSecurityProxy(
person.account).openid_identifiers.any().identifier)
- def test_getOrCreateSoftwareCenterCustomer_xmlrpc_error(self):
+ def test_getOrCreateSoftwareCenterCustomer_xmlrpc_suspended(self):
# A suspended account results in an appropriate xmlrpc fault.
suspended_person = self.factory.makePerson(
displayname='Joe Blogs', email='a...@b.com',
@@ -106,6 +106,22 @@
self.assertTrue(fault_raised)
+ def test_getOrCreateSoftwareCenterCustomer_xmlrpc_team(self):
+ # A team email address results in an appropriate xmlrpc fault.
+ self.factory.makeTeam(email='a...@b.com')
+
+ # assertRaises doesn't let us check the type of Fault.
+ fault_raised = False
+ try:
+ self.rpc_proxy.getOrCreateSoftwareCenterCustomer(
+ 'foo', 'a...@b.com', 'Joe Blogs')
+ except xmlrpclib.Fault, e:
+ fault_raised = True
+ self.assertEqual(400, e.faultCode)
+ self.assertIn('a...@b.com', e.faultString)
+
+ self.assertTrue(fault_raised)
+
def test_not_available_on_public_api(self):
# The person set api is not available on the public xmlrpc
# service.
=== modified file 'lib/lp/registry/xmlrpc/softwarecenteragent.py'
--- lib/lp/registry/xmlrpc/softwarecenteragent.py 2012-01-01 02:58:52 +0000
+++ lib/lp/registry/xmlrpc/softwarecenteragent.py 2012-01-20 07:03:32 +0000
@@ -17,6 +17,7 @@
ISoftwareCenterAgentAPI,
ISoftwareCenterAgentApplication,
PersonCreationRationale,
+ TeamEmailAddressError,
)
from lp.services.identity.interfaces.account import AccountSuspendedError
from lp.services.webapp import LaunchpadXMLRPCView
@@ -38,6 +39,8 @@
"when purchasing an application via Software Center.")
except AccountSuspendedError:
return faults.AccountSuspended(openid_identifier)
+ except TeamEmailAddressError:
+ return faults.TeamEmailAddress(email, openid_identifier)
return person.name
@@ -47,4 +50,3 @@
implements(ISoftwareCenterAgentApplication)
title = "Software Center Agent API"
-
=== modified file 'lib/lp/services/webapp/login.py'
--- lib/lp/services/webapp/login.py 2012-01-03 11:08:31 +0000
+++ lib/lp/services/webapp/login.py 2012-01-20 07:03:32 +0000
@@ -42,6 +42,7 @@
from lp.registry.interfaces.person import (
IPersonSet,
PersonCreationRationale,
+ TeamEmailAddressError,
)
from lp.services.config import config
from lp.services.database.readonly import is_read_only
@@ -295,6 +296,9 @@
suspended_account_template = ViewPageTemplateFile(
'templates/login-suspended-account.pt')
+ team_email_address_template = ViewPageTemplateFile(
+ 'templates/login-team-email-address.pt')
+
def _gather_params(self, request):
params = dict(request.form)
for key, value in request.query_string_params.iteritems():
@@ -381,6 +385,8 @@
should_update_last_write = db_updated
except AccountSuspendedError:
return self.suspended_account_template()
+ except TeamEmailAddressError:
+ return self.team_email_address_template()
with MasterDatabasePolicy():
self.login(person)
=== added file 'lib/lp/services/webapp/templates/login-team-email-address.pt'
--- lib/lp/services/webapp/templates/login-team-email-address.pt 1970-01-01 00:00:00 +0000
+++ lib/lp/services/webapp/templates/login-team-email-address.pt 2012-01-20 07:03:32 +0000
@@ -0,0 +1,22 @@
+<html
+ xmlns="http://www.w3.org/1999/xhtml";
+ xmlns:tal="http://xml.zope.org/namespaces/tal";
+ xmlns:metal="http://xml.zope.org/namespaces/metal";
+ xmlns:i18n="http://xml.zope.org/namespaces/i18n";
+ metal:use-macro="view/macro:page/locationless"
+ i18n:domain="launchpad">
+
+ <body>
+ <div class="top-portlet" metal:fill-slot="main">
+
+ <h1>Team email address conflict</h1>
+ <p class="error">
+ Your can't log in because your OpenID account's email address is
+ associated with a Launchpad team.
+ Contact <a href="mailto:feedb...@launchpad.net?subject=Team%20email%20address%20conflict"
+ >Launchpad Support</a> about this issue.
+ </p>
+
+ </div>
+ </body>
+</html>
=== modified file 'lib/lp/services/webapp/tests/test_login.py'
--- lib/lp/services/webapp/tests/test_login.py 2012-01-14 12:47:39 +0000
+++ lib/lp/services/webapp/tests/test_login.py 2012-01-20 07:03:32 +0000
@@ -424,6 +424,20 @@
main_content = extract_text(find_main_content(html))
self.assertIn('This account has been suspended', main_content)
+ def test_account_with_team_email_address(self):
+ # If the email address from the OpenID provider is owned by a
+ # team, there's not much we can do. See bug #556680 for
+ # discussions about a proper solution.
+ self.factory.makeTeam(email="f...@bar.com")
+ person = self.factory.makePerson()
+
+ with SRegResponse_fromSuccessResponse_stubbed():
+ view, html = self._createViewWithResponse(
+ person.account, email="f...@bar.com")
+ self.assertFalse(view.login_called)
+ main_content = extract_text(find_main_content(html))
+ self.assertIn('Team email address conflict', main_content)
+
def test_negative_openid_assertion(self):
# The OpenID provider responded with a negative assertion, so the
# login error page is shown.
=== modified file 'lib/lp/xmlrpc/configure.zcml'
--- lib/lp/xmlrpc/configure.zcml 2011-12-24 17:49:30 +0000
+++ lib/lp/xmlrpc/configure.zcml 2012-01-20 07:03:32 +0000
@@ -201,6 +201,9 @@
<class class="lp.xmlrpc.faults.AccountSuspended">
<require like_class="xmlrpclib.Fault" />
</class>
+ <class class="lp.xmlrpc.faults.TeamEmailAddress">
+ <require like_class="xmlrpclib.Fault" />
+ </class>
<class class="lp.xmlrpc.faults.InvalidSourcePackageName">
<require like_class="xmlrpclib.Fault" />
</class>
=== modified file 'lib/lp/xmlrpc/faults.py'
--- lib/lp/xmlrpc/faults.py 2011-08-11 21:24:28 +0000
+++ lib/lp/xmlrpc/faults.py 2012-01-20 07:03:32 +0000
@@ -39,6 +39,7 @@
'NotInTeam',
'NoUrlForBranch',
'RequiredParameterMissing',
+ 'TeamEmailAddress',
'UnexpectedStatusReport',
]
@@ -487,3 +488,17 @@
def __init__(self, name):
self.name = name
LaunchpadFault.__init__(self, name=name)
+
+
+class TeamEmailAddress(LaunchpadFault):
+ """Raised by `ISoftwareCenterAgentAPI` when an email address is a team."""
+
+ error_code = 400
+ msg_template = (
+ 'The email address \'%(email)s\' '
+ '(openid_identifier \'%(openid_identifier)s\') '
+ 'is linked to a Launchpad team.')
+
+ def __init__(self, email, openid_identifier):
+ LaunchpadFault.__init__(
+ self, email=email, openid_identifier=openid_identifier)
_______________________________________________
Mailing list: https://launchpad.net/~launchpad-reviewers
Post to : launchpad-reviewers@lists.launchpad.net
Unsubscribe : https://launchpad.net/~launchpad-reviewers
More help : https://help.launchpad.net/ListHelp
