Merge lp:~thumper/launchpad/process-email-oops-on-person-adaptation into lp:launchpad

[Tim Penhey]

[r=lifeless,wgrant][bug=728132] Fix OOPS generated when adapting accounts to IPerson when there is no linked person.

I found this while looking through the process mail log file.

[William Grant (wgrant)]

Looks good, as long as you s/principle/principal/.

review: Approve (code*)

[Robert Collins (lifeless)]

@wgrant agreed.

review: Approve

=== modified file 'lib/lp/services/mail/incoming.py'
--- lib/lp/services/mail/incoming.py	2010-12-14 03:48:06 +0000
+++ lib/lp/services/mail/incoming.py	2011-03-03 02:32:47 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2010 Canonical Ltd.  This software is licensed under the
+# Copyright 2009-2011 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Functions dealing with mails coming into Launchpad."""
@@ -186,9 +186,14 @@
     # Check that sender is registered in Launchpad and the email is signed.
     if principal is None:
         setupInteraction(authutil.unauthenticatedPrincipal())
-        return
+        return None
 
-    person = IPerson(principal)
+    # 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(

=== modified file 'lib/lp/services/mail/tests/test_incoming.py'
--- lib/lp/services/mail/tests/test_incoming.py	2010-12-01 06:15:29 +0000
+++ lib/lp/services/mail/tests/test_incoming.py	2011-03-03 02:32:47 +0000
@@ -6,6 +6,7 @@
 import os
 import unittest
 
+from testtools.matchers import Is
 import transaction
 from zope.security.management import setSecurityPolicy
 
@@ -104,6 +105,19 @@
             authenticateEmail,
             msg, fail_all_timestamps)
 
+    def test_unknown_email(self):
+        # An unknown email address returns no principal.
+        unknown = '[e-mail]'
+        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 = '[e-mail]'
+        self.factory.makeAccount(email=email)
+        mail = self.factory.makeSignedMessage(email_address=email)
+        self.assertThat(authenticateEmail(mail), Is(None))
+
 
 def setUp(test):
     test._old_policy = setSecurityPolicy(LaunchpadSecurityPolicy)

=== modified file 'lib/lp/testing/factory.py'
--- lib/lp/testing/factory.py	2011-03-02 17:48:28 +0000
+++ lib/lp/testing/factory.py	2011-03-03 02:32:47 +0000
@@ -502,10 +502,12 @@
             pocket)
         return ProxyFactory(location)
 
-    def makeAccount(self, displayname, email=None, password=None,
+    def makeAccount(self, displayname=None, email=None, password=None,
                     status=AccountStatus.ACTIVE,
                     rationale=AccountCreationRationale.UNKNOWN):
         """Create and return a new Account."""
+        if displayname is None:
+            displayname = self.getUniqueString('displayname')
         account = getUtility(IAccountSet).new(
             rationale, displayname, password=password)
         removeSecurityProxy(account).status = status