Merge lp:~gary/launchpad/bug650343-2 into lp:launchpad

Proposed by Gary Poster
Status: Merged
Approved by: Leonard Richardson
Approved revision: no longer in the source branch.
Merged at revision: 11705
Proposed branch: lp:~gary/launchpad/bug650343-2
Merge into: lp:launchpad
Prerequisite: lp:~gary/launchpad/bug650343
Diff against target: 666 lines (+133/-125)
10 files modified
cronscripts/process-mail.py (+2/-1)
lib/canonical/launchpad/doc/emailauthentication.txt (+7/-4)
lib/canonical/launchpad/ftests/test_system_documentation.py (+55/-77)
lib/canonical/launchpad/mail/handlers.py (+1/-1)
lib/lp/blueprints/doc/spec-mail-exploder.txt (+1/-1)
lib/lp/services/mail/incoming.py (+2/-2)
lib/lp/services/mail/tests/incomingmail.txt (+6/-6)
lib/lp/services/mail/tests/test_dkim.py (+31/-26)
lib/lp/services/mail/tests/test_incoming.py (+24/-2)
lib/lp/services/mail/tests/test_signedmessage.py (+4/-5)
To merge this branch: bzr merge lp:~gary/launchpad/bug650343-2
Reviewer Review Type Date Requested Status
Leonard Richardson (community) Approve
Review via email: mp+38224@code.launchpad.net

Commit message

Add X-Launchpad-Original-To to recipient lists; move some related files from c/l/mail to lp/services/mail.

Description of the change

In the reviews for https://code.edge.launchpad.net/~gary/launchpad/bug650343/+merge/37633 , I was asked to move the "incoming"-related files I touched over to lp/services/mail. This branch performs that move, and does the various changes that make lint then requests.

While we're speaking of lint, here's the output.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  cronscripts/process-mail.py
  lib/canonical/launchpad/doc/emailauthentication.txt
  lib/canonical/launchpad/ftests/test_system_documentation.py
  lib/canonical/launchpad/mail/handlers.py
  lib/lp/blueprints/doc/spec-mail-exploder.txt
  lib/lp/services/mail/incoming.py
  lib/lp/services/mail/tests/incomingmail.txt
  lib/lp/services/mail/tests/test_dkim.py
  lib/lp/services/mail/tests/test_incoming.py
  lib/lp/services/mail/tests/test_signedmessage.py

./cronscripts/process-mail.py
       9: '_pythonpath' imported but unused
./lib/canonical/launchpad/doc/emailauthentication.txt
     155: want exceeds 78 characters.

The ./cronscripts/process-mail.py request is because of our weird _pythonpath dance (maybe we should make these all buildout-generated scripts someday...or not). Anyway, it's not pertinent.

The request for ./lib/canonical/launchpad/doc/emailauthentication.txt appears to not be appropriate: the test explicitly turns off whitespace normalization there, so I think the long line is intended.

Tests passed on ec2 with the current branch, except for a Windmill error that appears to be spurious.

Other than that, the only thing I know to mention is that I did not move the files I touched for this move (lib/canonical/launchpad/doc/emailauthentication.txt lib/canonical/launchpad/ftests/test_system_documentation.py lib/canonical/launchpad/mail/handlers.py). Hopefully the move request does not have to cascade like that, or I may be at this for awhile. :-)

Thank you.

To post a comment you must log in.
Revision history for this message
Leonard Richardson (leonardr) wrote :

Good progress. Approved with minor changes discussed on IRC: reword "no signature is treated" and fix "message message" typo.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'cronscripts/process-mail.py'
--- cronscripts/process-mail.py 2010-10-03 15:30:06 +0000
+++ cronscripts/process-mail.py 2010-10-14 20:59:59 +0000
@@ -13,7 +13,7 @@
13from canonical.config import config13from canonical.config import config
14from lp.services.scripts.base import (14from lp.services.scripts.base import (
15 LaunchpadCronScript, LaunchpadScriptFailure)15 LaunchpadCronScript, LaunchpadScriptFailure)
16from canonical.launchpad.mail.incoming import handleMail16from lp.services.mail.incoming import handleMail
17from canonical.launchpad.interfaces import IMailBox17from canonical.launchpad.interfaces import IMailBox
1818
1919
@@ -21,6 +21,7 @@
21 usage = """%prog [options]21 usage = """%prog [options]
2222
23 """ + __doc__23 """ + __doc__
24
24 def main(self):25 def main(self):
25 try:26 try:
26 handleMail(self.txn)27 handleMail(self.txn)
2728
=== modified file 'lib/canonical/launchpad/doc/emailauthentication.txt'
--- lib/canonical/launchpad/doc/emailauthentication.txt 2010-10-04 19:50:45 +0000
+++ lib/canonical/launchpad/doc/emailauthentication.txt 2010-10-14 20:59:59 +0000
@@ -1,9 +1,10 @@
1= Authentication of Emails =1Authentication of Emails
2========================
23
3When an email arrives in Launchpad the user who sent it needs to be4When an email arrives in Launchpad the user who sent it needs to be
4authenticated. This is done by authenticateEmail:5authenticated. This is done by authenticateEmail:
56
6 >>> from canonical.launchpad.mail.incoming import authenticateEmail7 >>> from lp.services.mail.incoming import authenticateEmail
78
8The only way of authenticating the user is by looking at the OpenPGP9The only way of authenticating the user is by looking at the OpenPGP
9signature. First we have to import the OpenPGP keys we will use in the10signature. First we have to import the OpenPGP keys we will use in the
@@ -159,7 +160,8 @@
159 Sample Person160 Sample Person
160161
161162
162== IWeaklyAuthenticatedPrinipal ==163IWeaklyAuthenticatedPrincipal
164-----------------------------
163165
164It's a huge difference to signing an email with a key that is associated166It's a huge difference to signing an email with a key that is associated
165with the authenticated Person, and signing it with a key that isn't167with the authenticated Person, and signing it with a key that isn't
@@ -172,7 +174,8 @@
172174
173An unsigned email:175An unsigned email:
174176
175 >>> from canonical.launchpad.interfaces import IWeaklyAuthenticatedPrincipal177 >>> from canonical.launchpad.interfaces import (
178 ... IWeaklyAuthenticatedPrincipal)
176 >>> msg = read_test_message('unsigned_multipart.txt')179 >>> msg = read_test_message('unsigned_multipart.txt')
177 >>> principal = authenticateEmail(msg)180 >>> principal = authenticateEmail(msg)
178 >>> IWeaklyAuthenticatedPrincipal.providedBy(principal)181 >>> IWeaklyAuthenticatedPrincipal.providedBy(principal)
179182
=== modified file 'lib/canonical/launchpad/ftests/test_system_documentation.py'
--- lib/canonical/launchpad/ftests/test_system_documentation.py 2010-10-03 15:30:06 +0000
+++ lib/canonical/launchpad/ftests/test_system_documentation.py 2010-10-14 20:59:59 +0000
@@ -52,6 +52,7 @@
5252
53here = os.path.dirname(os.path.realpath(__file__))53here = os.path.dirname(os.path.realpath(__file__))
5454
55
55def lobotomize_stevea():56def lobotomize_stevea():
56 """Set SteveA's email address' status to NEW.57 """Set SteveA's email address' status to NEW.
5758
@@ -79,17 +80,20 @@
79 LaunchpadZopelessLayer.switchDbUser('poexport')80 LaunchpadZopelessLayer.switchDbUser('poexport')
80 setUp(test)81 setUp(test)
8182
83
82def poExportTearDown(test):84def poExportTearDown(test):
83 """Tear down the PO export script tests."""85 """Tear down the PO export script tests."""
84 # XXX sinzui 2007-11-14:86 # XXX sinzui 2007-11-14:
85 # This function is not needed. The test should be switched to tearDown.87 # This function is not needed. The test should be switched to tearDown.
86 tearDown(test)88 tearDown(test)
8789
90
88def uploaderSetUp(test):91def uploaderSetUp(test):
89 """setup the package uploader script tests."""92 """setup the package uploader script tests."""
90 setUp(test)93 setUp(test)
91 LaunchpadZopelessLayer.switchDbUser('uploader')94 LaunchpadZopelessLayer.switchDbUser('uploader')
9295
96
93def uploaderTearDown(test):97def uploaderTearDown(test):
94 """Tear down the package uploader script tests."""98 """Tear down the package uploader script tests."""
95 # XXX sinzui 2007-11-14:99 # XXX sinzui 2007-11-14:
@@ -115,6 +119,7 @@
115 # This function is not needed. The test should be switched to tearDown.119 # This function is not needed. The test should be switched to tearDown.
116 tearDown(test)120 tearDown(test)
117121
122
118def uploadQueueSetUp(test):123def uploadQueueSetUp(test):
119 lobotomize_stevea()124 lobotomize_stevea()
120 test_dbuser = config.uploadqueue.dbuser125 test_dbuser = config.uploadqueue.dbuser
@@ -127,6 +132,7 @@
127 """Clean up any Zope registrations."""132 """Clean up any Zope registrations."""
128 cleanUp()133 cleanUp()
129134
135
130def _createUbuntuBugTaskLinkedToQuestion():136def _createUbuntuBugTaskLinkedToQuestion():
131 """Get the id of an Ubuntu bugtask linked to a question.137 """Get the id of an Ubuntu bugtask linked to a question.
132138
@@ -154,10 +160,13 @@
154 pop_notifications()160 pop_notifications()
155 return ubuntu_bugtask.id161 return ubuntu_bugtask.id
156162
163
157def bugLinkedToQuestionSetUp(test):164def bugLinkedToQuestionSetUp(test):
158 """Setup the question and linked bug for testing."""165 """Setup the question and linked bug for testing."""
166
159 def get_bugtask_linked_to_question():167 def get_bugtask_linked_to_question():
160 return getUtility(IBugTaskSet).get(bugtask_id)168 return getUtility(IBugTaskSet).get(bugtask_id)
169
161 setUp(test)170 setUp(test)
162 bugtask_id = _createUbuntuBugTaskLinkedToQuestion()171 bugtask_id = _createUbuntuBugTaskLinkedToQuestion()
163 test.globs['get_bugtask_linked_to_question'] = (172 test.globs['get_bugtask_linked_to_question'] = (
@@ -166,6 +175,7 @@
166 # interaction in the test.175 # interaction in the test.
167 login('no-priv@canonical.com')176 login('no-priv@canonical.com')
168177
178
169def uploaderBugLinkedToQuestionSetUp(test):179def uploaderBugLinkedToQuestionSetUp(test):
170 LaunchpadZopelessLayer.switchDbUser('launchpad')180 LaunchpadZopelessLayer.switchDbUser('launchpad')
171 bugLinkedToQuestionSetUp(test)181 bugLinkedToQuestionSetUp(test)
@@ -173,6 +183,7 @@
173 uploaderSetUp(test)183 uploaderSetUp(test)
174 login(ANONYMOUS)184 login(ANONYMOUS)
175185
186
176def uploadQueueBugLinkedToQuestionSetUp(test):187def uploadQueueBugLinkedToQuestionSetUp(test):
177 LaunchpadZopelessLayer.switchDbUser('launchpad')188 LaunchpadZopelessLayer.switchDbUser('launchpad')
178 bugLinkedToQuestionSetUp(test)189 bugLinkedToQuestionSetUp(test)
@@ -185,8 +196,7 @@
185special = {196special = {
186 # No setup or teardown at all, since it is demonstrating these features.197 # No setup or teardown at all, since it is demonstrating these features.
187 'old-testing.txt': LayeredDocFileSuite(198 'old-testing.txt': LayeredDocFileSuite(
188 '../doc/old-testing.txt', layer=FunctionalLayer199 '../doc/old-testing.txt', layer=FunctionalLayer),
189 ),
190200
191 'autodecorate.txt':201 'autodecorate.txt':
192 LayeredDocFileSuite('../doc/autodecorate.txt', layer=BaseLayer),202 LayeredDocFileSuite('../doc/autodecorate.txt', layer=BaseLayer),
@@ -194,84 +204,70 @@
194204
195 # And this test want minimal environment too.205 # And this test want minimal environment too.
196 'package-relationship.txt': LayeredDocFileSuite(206 'package-relationship.txt': LayeredDocFileSuite(
197 '../doc/package-relationship.txt',207 '../doc/package-relationship.txt',
198 stdout_logging=False, layer=None208 stdout_logging=False, layer=None),
199 ),
200209
201 'webservice-configuration.txt': LayeredDocFileSuite(210 'webservice-configuration.txt': LayeredDocFileSuite(
202 '../doc/webservice-configuration.txt',211 '../doc/webservice-configuration.txt',
203 setUp=setGlobs, tearDown=layerlessTearDown, layer=None212 setUp=setGlobs, tearDown=layerlessTearDown, layer=None),
204 ),
205213
206214
207 # POExport stuff is Zopeless and connects as a different database user.215 # POExport stuff is Zopeless and connects as a different database user.
208 # poexport-distroseries-(date-)tarball.txt is excluded, since they add216 # poexport-distroseries-(date-)tarball.txt is excluded, since they add
209 # data to the database as well.217 # data to the database as well.
210 'message.txt': LayeredDocFileSuite(218 'message.txt': LayeredDocFileSuite(
211 '../doc/message.txt',219 '../doc/message.txt',
212 setUp=setUp, tearDown=tearDown, layer=LaunchpadFunctionalLayer220 setUp=setUp, tearDown=tearDown, layer=LaunchpadFunctionalLayer),
213 ),
214 'close-account.txt': LayeredDocFileSuite(221 'close-account.txt': LayeredDocFileSuite(
215 '../doc/close-account.txt', setUp=setUp, tearDown=tearDown,222 '../doc/close-account.txt', setUp=setUp, tearDown=tearDown,
216 layer=LaunchpadZopelessLayer223 layer=LaunchpadZopelessLayer),
217 ),
218 'launchpadform.txt': LayeredDocFileSuite(224 'launchpadform.txt': LayeredDocFileSuite(
219 '../doc/launchpadform.txt',225 '../doc/launchpadform.txt',
220 setUp=setUp, tearDown=tearDown,226 setUp=setUp, tearDown=tearDown,
221 layer=LaunchpadFunctionalLayer227 layer=LaunchpadFunctionalLayer),
222 ),
223 'launchpadformharness.txt': LayeredDocFileSuite(228 'launchpadformharness.txt': LayeredDocFileSuite(
224 '../doc/launchpadformharness.txt',229 '../doc/launchpadformharness.txt',
225 setUp=setUp, tearDown=tearDown,230 setUp=setUp, tearDown=tearDown,
226 layer=LaunchpadFunctionalLayer231 layer=LaunchpadFunctionalLayer),
227 ),
228 'uri.txt': LayeredDocFileSuite(232 'uri.txt': LayeredDocFileSuite(
229 '../doc/uri.txt',233 '../doc/uri.txt',
230 setUp=setUp, tearDown=tearDown,234 setUp=setUp, tearDown=tearDown,
231 layer=FunctionalLayer235 layer=FunctionalLayer),
232 ),
233 'notification-text-escape.txt': LayeredDocFileSuite(236 'notification-text-escape.txt': LayeredDocFileSuite(
234 '../doc/notification-text-escape.txt',237 '../doc/notification-text-escape.txt',
235 setUp=test_notifications.setUp,238 setUp=test_notifications.setUp,
236 tearDown=test_notifications.tearDown,239 tearDown=test_notifications.tearDown,
237 stdout_logging=False, layer=None240 stdout_logging=False, layer=None),
238 ),
239 # This test is actually run twice to prove that the AppServerLayer241 # This test is actually run twice to prove that the AppServerLayer
240 # properly isolates the database between tests.242 # properly isolates the database between tests.
241 'launchpadlib.txt': LayeredDocFileSuite(243 'launchpadlib.txt': LayeredDocFileSuite(
242 '../doc/launchpadlib.txt',244 '../doc/launchpadlib.txt',
243 layer=AppServerLayer,245 layer=AppServerLayer,
244 setUp=browser.setUp, tearDown=browser.tearDown,246 setUp=browser.setUp, tearDown=browser.tearDown,),
245 ),
246 'launchpadlib2.txt': LayeredDocFileSuite(247 'launchpadlib2.txt': LayeredDocFileSuite(
247 '../doc/launchpadlib.txt',248 '../doc/launchpadlib.txt',
248 layer=AppServerLayer,249 layer=AppServerLayer,
249 setUp=browser.setUp, tearDown=browser.tearDown,250 setUp=browser.setUp, tearDown=browser.tearDown,),
250 ),
251 # XXX gary 2008-12-08 bug=306246 bug=305858: Disabled test because of251 # XXX gary 2008-12-08 bug=306246 bug=305858: Disabled test because of
252 # multiple spurious problems with layer and test.252 # multiple spurious problems with layer and test.
253 # 'google-service-stub.txt': LayeredDocFileSuite(253 # 'google-service-stub.txt': LayeredDocFileSuite(
254 # '../doc/google-service-stub.txt',254 # '../doc/google-service-stub.txt',
255 # layer=GoogleServiceLayer,255 # layer=GoogleServiceLayer,),
256 # ),
257 'canonical_url.txt': LayeredDocFileSuite(256 'canonical_url.txt': LayeredDocFileSuite(
258 '../doc/canonical_url.txt',257 '../doc/canonical_url.txt',
259 setUp=setUp,258 setUp=setUp,
260 tearDown=tearDown,259 tearDown=tearDown,
261 layer=FunctionalLayer,260 layer=FunctionalLayer,),
262 ),
263 'google-searchservice.txt': LayeredDocFileSuite(261 'google-searchservice.txt': LayeredDocFileSuite(
264 '../doc/google-searchservice.txt',262 '../doc/google-searchservice.txt',
265 setUp=setUp, tearDown=tearDown,263 setUp=setUp, tearDown=tearDown,
266 layer=GoogleLaunchpadFunctionalLayer,264 layer=GoogleLaunchpadFunctionalLayer,),
267 ),
268 }265 }
269266
270267
271class ProcessMailLayer(LaunchpadZopelessLayer):268class ProcessMailLayer(LaunchpadZopelessLayer):
272 """Layer containing the tests running inside process-mail.py."""269 """Layer containing the tests running inside process-mail.py."""
273270
274
275 @classmethod271 @classmethod
276 def testSetUp(cls):272 def testSetUp(cls):
277 """Fixture replicating the process-mail.py environment.273 """Fixture replicating the process-mail.py environment.
@@ -287,7 +283,7 @@
287 """Tear down the test fixture."""283 """Tear down the test fixture."""
288 setSecurityPolicy(cls._old_policy)284 setSecurityPolicy(cls._old_policy)
289285
290 doctests_without_logging = [286 doctests = [
291 # XXX gary 2008-12-06 bug=305856: Spurious test failure287 # XXX gary 2008-12-06 bug=305856: Spurious test failure
292 # discovered on buildbot, build 40. Note that, to completely288 # discovered on buildbot, build 40. Note that, to completely
293 # disable the test from running, the filename has been changed289 # disable the test from running, the filename has been changed
@@ -299,21 +295,16 @@
299 '../doc/emailauthentication.txt',295 '../doc/emailauthentication.txt',
300 ]296 ]
301297
302 doctests_with_logging = [
303 '../doc/incomingmail.txt',
304 ]
305
306 @classmethod298 @classmethod
307 def addTestsToSpecial(cls):299 def addTestsToSpecial(cls):
308 """Adds all the tests related to process-mail.py to special"""300 """Adds all the tests related to process-mail.py to special"""
309 for filepath in cls.doctests_without_logging:301 for filepath in cls.doctests:
310 filename = os.path.basename(filepath)302 filename = os.path.basename(filepath)
311 special[filename] = cls.createLayeredDocFileSuite(filepath)303 special[filename] = LayeredDocFileSuite(
312304 filepath,
313 for filepath in cls.doctests_with_logging:305 setUp=setUp, tearDown=tearDown,
314 filename = os.path.basename(filepath)306 layer=cls,
315 special[filename] = cls.createLayeredDocFileSuite(307 stdout_logging=False)
316 filepath, stdout_logging=True)
317308
318 # Adds a copy of some bug doctests that will be run with309 # Adds a copy of some bug doctests that will be run with
319 # the processmail user.310 # the processmail user.
@@ -337,16 +328,6 @@
337 layer=cls,328 layer=cls,
338 stdout_logging=False)329 stdout_logging=False)
339330
340 @classmethod
341 def createLayeredDocFileSuite(cls, filename, stdout_logging=False):
342 """Helper to create a doctest using this layer."""
343 return LayeredDocFileSuite(
344 filename,
345 setUp=setUp, tearDown=tearDown,
346 layer=cls,
347 stdout_logging=stdout_logging,
348 stdout_logging_level=logging.WARNING)
349
350331
351ProcessMailLayer.addTestsToSpecial()332ProcessMailLayer.addTestsToSpecial()
352333
@@ -360,15 +341,13 @@
360 suite.addTest(special_suite)341 suite.addTest(special_suite)
361342
362 testsdir = os.path.abspath(343 testsdir = os.path.abspath(
363 os.path.normpath(os.path.join(here, '..', 'doc'))344 os.path.normpath(os.path.join(here, '..', 'doc')))
364 )
365345
366 # Add tests using default setup/teardown346 # Add tests using default setup/teardown
367 filenames = [filename347 filenames = [filename
368 for filename in os.listdir(testsdir)348 for filename in os.listdir(testsdir)
369 if filename.lower().endswith('.txt')349 if filename.lower().endswith('.txt')
370 and filename not in special350 and filename not in special]
371 ]
372 # Sort the list to give a predictable order. We do this because when351 # Sort the list to give a predictable order. We do this because when
373 # tests interfere with each other, the varying orderings that os.listdir352 # tests interfere with each other, the varying orderings that os.listdir
374 # gives on different people's systems make reproducing and debugging353 # gives on different people's systems make reproducing and debugging
@@ -383,8 +362,7 @@
383 layer=LaunchpadFunctionalLayer,362 layer=LaunchpadFunctionalLayer,
384 # 'icky way of running doctests with __future__ imports363 # 'icky way of running doctests with __future__ imports
385 globs={'with_statement': with_statement},364 globs={'with_statement': with_statement},
386 stdout_logging_level=logging.WARNING365 stdout_logging_level=logging.WARNING)
387 )
388 suite.addTest(one_test)366 suite.addTest(one_test)
389367
390 return suite368 return suite
391369
=== modified file 'lib/canonical/launchpad/mail/handlers.py'
--- lib/canonical/launchpad/mail/handlers.py 2010-10-03 15:30:06 +0000
+++ lib/canonical/launchpad/mail/handlers.py 2010-10-14 20:59:59 +0000
@@ -61,7 +61,7 @@
6161
62def extract_signature_timestamp(signed_msg):62def extract_signature_timestamp(signed_msg):
63 # break import cycle63 # break import cycle
64 from canonical.launchpad.mail.incoming import (64 from lp.services.mail.incoming import (
65 canonicalise_line_endings)65 canonicalise_line_endings)
66 signature = getUtility(IGPGHandler).getVerifiedSignature(66 signature = getUtility(IGPGHandler).getVerifiedSignature(
67 canonicalise_line_endings(signed_msg.signedContent),67 canonicalise_line_endings(signed_msg.signedContent),
6868
=== modified file 'lib/lp/blueprints/doc/spec-mail-exploder.txt'
--- lib/lp/blueprints/doc/spec-mail-exploder.txt 2010-10-14 20:59:48 +0000
+++ lib/lp/blueprints/doc/spec-mail-exploder.txt 2010-10-14 20:59:59 +0000
@@ -269,7 +269,7 @@
269 >>> moin_change['Sender'] = 'webmaster@ubuntu.com'269 >>> moin_change['Sender'] = 'webmaster@ubuntu.com'
270270
271 >>> from lp.services.mail.sendmail import sendmail271 >>> from lp.services.mail.sendmail import sendmail
272 >>> from canonical.launchpad.mail.incoming import handleMail272 >>> from lp.services.mail.incoming import handleMail
273 >>> sendmail(moin_change, bulk=False)273 >>> sendmail(moin_change, bulk=False)
274 '...'274 '...'
275275
276276
=== renamed file 'lib/canonical/launchpad/mail/incoming.py' => 'lib/lp/services/mail/incoming.py'
--- lib/canonical/launchpad/mail/incoming.py 2010-10-14 20:59:48 +0000
+++ lib/lp/services/mail/incoming.py 2010-10-14 20:59:59 +0000
@@ -325,7 +325,7 @@
325 mail = signed_message_from_string(raw_mail)325 mail = signed_message_from_string(raw_mail)
326 except email.Errors.MessageError, error:326 except email.Errors.MessageError, error:
327 mailbox.delete(mail_id)327 mailbox.delete(mail_id)
328 log = logging.getLogger('canonical.launchpad.mail')328 log = logging.getLogger('lp.services.mail')
329 log.warn(329 log.warn(
330 "Couldn't convert email to email.Message: %s" % (330 "Couldn't convert email to email.Message: %s" % (
331 file_alias_url, ),331 file_alias_url, ),
@@ -369,7 +369,7 @@
369 if 'X-Launchpad-Original-To' in mail:369 if 'X-Launchpad-Original-To' in mail:
370 addresses = [mail['X-Launchpad-Original-To']]370 addresses = [mail['X-Launchpad-Original-To']]
371 else:371 else:
372 log = logging.getLogger('canonical.launchpad.mail')372 log = logging.getLogger('lp.services.mail')
373 log.warn(373 log.warn(
374 "No X-Launchpad-Original-To header was present "374 "No X-Launchpad-Original-To header was present "
375 "in email: %s" %375 "in email: %s" %
376376
=== renamed file 'lib/canonical/launchpad/doc/incomingmail.txt' => 'lib/lp/services/mail/tests/incomingmail.txt'
--- lib/canonical/launchpad/doc/incomingmail.txt 2010-10-14 20:59:48 +0000
+++ lib/lp/services/mail/tests/incomingmail.txt 2010-10-14 20:59:59 +0000
@@ -4,7 +4,7 @@
4When an email is sent to Launchpad we need to handle it somehow. This4When an email is sent to Launchpad we need to handle it somehow. This
5is done by handleEmails:5is done by handleEmails:
66
7 >>> from canonical.launchpad.mail.incoming import handleMail7 >>> from lp.services.mail.incoming import handleMail
88
9Basically what it does is to open the Launchpad mail box, and for each9Basically what it does is to open the Launchpad mail box, and for each
10message it:10message it:
@@ -88,10 +88,10 @@
88header is missing.)88header is missing.)
8989
90 >>> handleMail(zopeless_transaction)90 >>> handleMail(zopeless_transaction)
91 WARNING:canonical.launchpad.mail:No X-Launchpad-Original-To header was present ...91 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
92 WARNING:canonical.launchpad.mail:No X-Launchpad-Original-To header was present ...92 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
93 WARNING:canonical.launchpad.mail:No X-Launchpad-Original-To header was present ...93 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
94 WARNING:canonical.launchpad.mail:No X-Launchpad-Original-To header was present ...94 WARNING:lp.services.mail:No X-Launchpad-Original-To header was present ...
9595
96Now we can see that each handler handled the emails sent to its domain:96Now we can see that each handler handled the emails sent to its domain:
9797
@@ -389,7 +389,7 @@
389 >>> len(stub.test_emails)389 >>> len(stub.test_emails)
390 2390 2
391391
392 >>> handleMail(transaction)392 >>> handleMail(zopeless_transaction)
393 ERROR:...:Upload to Librarian failed...393 ERROR:...:Upload to Librarian failed...
394 ...394 ...
395 UploadFailed: ...Connection refused...395 UploadFailed: ...Connection refused...
396396
=== modified file 'lib/lp/services/mail/tests/test_dkim.py'
--- lib/lp/services/mail/tests/test_dkim.py 2010-08-20 20:31:18 +0000
+++ lib/lp/services/mail/tests/test_dkim.py 2010-10-14 20:59:59 +0000
@@ -11,14 +11,11 @@
1111
12import dkim12import dkim
13import dns.resolver13import dns.resolver
14from zope.component import getUtility
1514
16from canonical.launchpad.interfaces.mail import IWeaklyAuthenticatedPrincipal15from canonical.launchpad.interfaces.mail import IWeaklyAuthenticatedPrincipal
17from canonical.launchpad.mail import (16from canonical.launchpad.mail import signed_message_from_string
18 incoming,17from lp.services.mail import incoming
19 signed_message_from_string,18from lp.services.mail.incoming import authenticateEmail
20 )
21from canonical.launchpad.mail.incoming import authenticateEmail
22from canonical.testing.layers import DatabaseFunctionalLayer19from canonical.testing.layers import DatabaseFunctionalLayer
23from lp.testing import TestCaseWithFactory20from lp.testing import TestCaseWithFactory
2421
@@ -77,33 +74,36 @@
7774
78 def fake_signing(self, plain_message, canonicalize=None):75 def fake_signing(self, plain_message, canonicalize=None):
79 if canonicalize is None:76 if canonicalize is None:
80 canonicalize = (dkim.Relaxed, dkim.Relaxed) 77 canonicalize = (dkim.Relaxed, dkim.Relaxed)
81 dkim_line = dkim.sign(plain_message,78 dkim_line = dkim.sign(plain_message,
82 selector='example', 79 selector='example',
83 domain='canonical.com',80 domain='canonical.com',
84 privkey=sample_privkey,81 privkey=sample_privkey,
85 debuglog=self._log_output,82 debuglog=self._log_output,
86 canonicalize=canonicalize83 canonicalize=canonicalize)
87 )
88 assert dkim_line[-1] == '\n'84 assert dkim_line[-1] == '\n'
89 return dkim_line + plain_message85 return dkim_line + plain_message
9086
91 def monkeypatch_dns(self):87 def monkeypatch_dns(self):
92 self._dns_responses = {}88 self._dns_responses = {}
89
93 def my_lookup(name):90 def my_lookup(name):
94 try:91 try:
95 return self._dns_responses[name]92 return self._dns_responses[name]
96 except KeyError:93 except KeyError:
97 raise dns.resolver.NXDOMAIN()94 raise dns.resolver.NXDOMAIN()
95
98 orig_dnstxt = dkim.dnstxt96 orig_dnstxt = dkim.dnstxt
99 dkim.dnstxt = my_lookup97 dkim.dnstxt = my_lookup
98
100 def restore():99 def restore():
101 dkim.dnstxt = orig_dnstxt100 dkim.dnstxt = orig_dnstxt
101
102 self.addCleanup(restore)102 self.addCleanup(restore)
103103
104 def get_dkim_log(self):104 def get_dkim_log(self):
105 return self._log_output.getvalue()105 return self._log_output.getvalue()
106 106
107 def assertStronglyAuthenticated(self, principal, signed_message):107 def assertStronglyAuthenticated(self, principal, signed_message):
108 if IWeaklyAuthenticatedPrincipal.providedBy(principal):108 if IWeaklyAuthenticatedPrincipal.providedBy(principal):
109 self.fail('expected strong authentication; got weak:\n'109 self.fail('expected strong authentication; got weak:\n'
@@ -157,8 +157,10 @@
157 self._dns_responses['example._domainkey.canonical.com.'] = \157 self._dns_responses['example._domainkey.canonical.com.'] = \
158 sample_dns158 sample_dns
159 saved_domains = incoming._trusted_dkim_domains[:]159 saved_domains = incoming._trusted_dkim_domains[:]
160
160 def restore():161 def restore():
161 incoming._trusted_dkim_domains = saved_domains162 incoming._trusted_dkim_domains = saved_domains
163
162 self.addCleanup(restore)164 self.addCleanup(restore)
163 incoming._trusted_dkim_domains = []165 incoming._trusted_dkim_domains = []
164 principal = authenticateEmail(166 principal = authenticateEmail(
@@ -185,14 +187,15 @@
185 'steve.alexander@ubuntulinux.com')187 'steve.alexander@ubuntulinux.com')
186188
187 def test_dkim_changed_from_address(self):189 def test_dkim_changed_from_address(self):
188 # if the address part of the message has changed, it's detected. we190 # If the address part of the message has changed, it's detected.
189 # still treat this as weakly authenticated by the purported From-header191 # We still treat this as weakly authenticated by the purported
190 # sender, though perhaps in future we would prefer to reject these192 # From-header sender, though perhaps in future we would prefer
191 # messages.193 # to reject these messages.
192 signed_message = self.fake_signing(plain_content)194 signed_message = self.fake_signing(plain_content)
193 self._dns_responses['example._domainkey.canonical.com.'] = \195 self._dns_responses['example._domainkey.canonical.com.'] = \
194 sample_dns196 sample_dns
195 fiddled_message = signed_message.replace('From: Foo Bar <foo.bar@canonical.com>',197 fiddled_message = signed_message.replace(
198 'From: Foo Bar <foo.bar@canonical.com>',
196 'From: Carlos <carlos@canonical.com>')199 'From: Carlos <carlos@canonical.com>')
197 principal = authenticateEmail(200 principal = authenticateEmail(
198 signed_message_from_string(fiddled_message))201 signed_message_from_string(fiddled_message))
@@ -202,22 +205,23 @@
202 'carlos@canonical.com')205 'carlos@canonical.com')
203206
204 def test_dkim_changed_from_realname(self):207 def test_dkim_changed_from_realname(self):
205 # if the real name part of the message has changed, it's detected208 # If the real name part of the message has changed, it's detected.
206 signed_message = self.fake_signing(plain_content)209 signed_message = self.fake_signing(plain_content)
207 self._dns_responses['example._domainkey.canonical.com.'] = \210 self._dns_responses['example._domainkey.canonical.com.'] = \
208 sample_dns211 sample_dns
209 fiddled_message = signed_message.replace('From: Foo Bar <foo.bar@canonical.com>',212 fiddled_message = signed_message.replace(
213 'From: Foo Bar <foo.bar@canonical.com>',
210 'From: Evil Foo <foo.bar@canonical.com>')214 'From: Evil Foo <foo.bar@canonical.com>')
211 principal = authenticateEmail(215 principal = authenticateEmail(
212 signed_message_from_string(fiddled_message))216 signed_message_from_string(fiddled_message))
213 # we don't care about the real name for determining the principal217 # We don't care about the real name for determining the principal.
214 self.assertWeaklyAuthenticated(principal, fiddled_message)218 self.assertWeaklyAuthenticated(principal, fiddled_message)
215 self.assertEqual(principal.person.preferredemail.email,219 self.assertEqual(principal.person.preferredemail.email,
216 'foo.bar@canonical.com')220 'foo.bar@canonical.com')
217221
218 def test_dkim_nxdomain(self):222 def test_dkim_nxdomain(self):
219 # if there's no DNS entry for the pubkey223 # If there's no DNS entry for the pubkey it should be handled
220 # it should be handled decently224 # decently.
221 signed_message = self.fake_signing(plain_content)225 signed_message = self.fake_signing(plain_content)
222 principal = authenticateEmail(226 principal = authenticateEmail(
223 signed_message_from_string(signed_message))227 signed_message_from_string(signed_message))
@@ -226,18 +230,19 @@
226 'foo.bar@canonical.com')230 'foo.bar@canonical.com')
227231
228 def test_dkim_message_unsigned(self):232 def test_dkim_message_unsigned(self):
229 # degenerate case: no signature treated as weakly authenticated233 # This is a degenerate case: a message with no signature is
234 # treated as weakly authenticated.
235 # The library doesn't log anything if there's no header at all.
230 principal = authenticateEmail(236 principal = authenticateEmail(
231 signed_message_from_string(plain_content))237 signed_message_from_string(plain_content))
232 self.assertWeaklyAuthenticated(principal, plain_content)238 self.assertWeaklyAuthenticated(principal, plain_content)
233 self.assertEqual(principal.person.preferredemail.email,239 self.assertEqual(principal.person.preferredemail.email,
234 'foo.bar@canonical.com')240 'foo.bar@canonical.com')
235 # the library doesn't log anything if there's no header at all
236241
237 def test_dkim_body_mismatch(self):242 def test_dkim_body_mismatch(self):
238 # The message message has a syntactically valid DKIM signature that243 # The message has a syntactically valid DKIM signature that
239 # doesn't actually correspond to what was signed. We log something about244 # doesn't actually correspond to what was signed. We log
240 # this but we don't want to drop the message.245 # something about this but we don't want to drop the message.
241 signed_message = self.fake_signing(plain_content)246 signed_message = self.fake_signing(plain_content)
242 signed_message += 'blah blah'247 signed_message += 'blah blah'
243 self._dns_responses['example._domainkey.canonical.com.'] = \248 self._dns_responses['example._domainkey.canonical.com.'] = \
244249
=== renamed file 'lib/canonical/launchpad/mail/tests/test_incoming.py' => 'lib/lp/services/mail/tests/test_incoming.py'
--- lib/canonical/launchpad/mail/tests/test_incoming.py 2010-10-04 19:50:45 +0000
+++ lib/lp/services/mail/tests/test_incoming.py 2010-10-14 20:59:59 +0000
@@ -2,13 +2,19 @@
2# GNU Affero General Public License version 3 (see the file LICENSE).2# GNU Affero General Public License version 3 (see the file LICENSE).
33
4from doctest import DocTestSuite4from doctest import DocTestSuite
5import logging
5import os6import os
6import unittest7import unittest
78
8import transaction9import transaction
910
11from zope.security.management import setSecurityPolicy
12
13from canonical.config import config
10from canonical.launchpad.mail.ftests.helpers import testmails_path14from canonical.launchpad.mail.ftests.helpers import testmails_path
11from canonical.launchpad.mail.incoming import (15from canonical.launchpad.testing.systemdocs import LayeredDocFileSuite
16from canonical.launchpad.webapp.authorization import LaunchpadSecurityPolicy
17from lp.services.mail.incoming import (
12 handleMail,18 handleMail,
13 MailErrorUtility,19 MailErrorUtility,
14 )20 )
@@ -77,10 +83,26 @@
77 self.assertEqual(old_oops.id, current_oops.id)83 self.assertEqual(old_oops.id, current_oops.id)
7884
7985
86def setUp(test):
87 test._old_policy = setSecurityPolicy(LaunchpadSecurityPolicy)
88 LaunchpadZopelessLayer.switchDbUser(config.processmail.dbuser)
89
90
91def tearDown(test):
92 setSecurityPolicy(test._old_policy)
93
94
80def test_suite():95def test_suite():
81 suite = unittest.TestSuite()96 suite = unittest.TestSuite()
82 suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))97 suite.addTest(unittest.TestLoader().loadTestsFromName(__name__))
83 suite.addTest(DocTestSuite('canonical.launchpad.mail.incoming'))98 suite.addTest(DocTestSuite('lp.services.mail.incoming'))
99 suite.addTest(
100 LayeredDocFileSuite(
101 'incomingmail.txt',
102 setUp=setUp,
103 tearDown=tearDown,
104 layer=LaunchpadZopelessLayer,
105 stdout_logging_level=logging.WARNING))
84 return suite106 return suite
85107
86108
87109
=== modified file 'lib/lp/services/mail/tests/test_signedmessage.py'
--- lib/lp/services/mail/tests/test_signedmessage.py 2010-08-20 20:31:18 +0000
+++ lib/lp/services/mail/tests/test_signedmessage.py 2010-10-14 20:59:59 +0000
@@ -25,18 +25,18 @@
25from canonical.launchpad.interfaces.gpghandler import IGPGHandler25from canonical.launchpad.interfaces.gpghandler import IGPGHandler
26from canonical.launchpad.interfaces.mail import IWeaklyAuthenticatedPrincipal26from canonical.launchpad.interfaces.mail import IWeaklyAuthenticatedPrincipal
27from canonical.launchpad.mail import signed_message_from_string27from canonical.launchpad.mail import signed_message_from_string
28from canonical.launchpad.mail.incoming import (28from canonical.testing.layers import DatabaseFunctionalLayer
29from lp.registry.interfaces.person import IPersonSet
30from lp.services.mail.incoming import (
29 authenticateEmail,31 authenticateEmail,
30 canonicalise_line_endings,32 canonicalise_line_endings,
31 )33 )
32from canonical.testing.layers import DatabaseFunctionalLayer
33from lp.registry.interfaces.person import IPersonSet
34from lp.testing import TestCaseWithFactory34from lp.testing import TestCaseWithFactory
35from lp.testing.factory import GPGSigningContext35from lp.testing.factory import GPGSigningContext
3636
3737
38class TestSignedMessage(TestCaseWithFactory):38class TestSignedMessage(TestCaseWithFactory):
39 """Test SignedMessage class correctly extracts and verifies the GPG signatures."""39 "Test SignedMessage class correctly extracts and verifies GPG signatures."
4040
41 layer = DatabaseFunctionalLayer41 layer = DatabaseFunctionalLayer
4242
@@ -154,4 +154,3 @@
154154
155def test_suite():155def test_suite():
156 return unittest.TestLoader().loadTestsFromName(__name__)156 return unittest.TestLoader().loadTestsFromName(__name__)
157