[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/upload-key-expired-notification into lp:launchpad

2018-03-26 Thread noreply
The proposal to merge lp:~cjwatson/launchpad/upload-key-expired-notification 
into lp:launchpad has been updated.

Status: Needs review => Merged

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/upload-key-expired-notification/+merge/340530
-- 
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
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


Re: [Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/upload-key-expired-notification into lp:launchpad

2018-03-25 Thread William Grant
Review: Approve code


-- 
https://code.launchpad.net/~cjwatson/launchpad/upload-key-expired-notification/+merge/340530
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.

___
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


[Launchpad-reviewers] [Merge] lp:~cjwatson/launchpad/upload-key-expired-notification into lp:launchpad

2018-03-02 Thread Colin Watson
Colin Watson has proposed merging 
lp:~cjwatson/launchpad/upload-key-expired-notification into lp:launchpad.

Commit message:
Send email notifications when an upload is signed with an expired key.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/upload-key-expired-notification/+merge/340530

I think I'd sort of half-thought that 
https://code.launchpad.net/~cjwatson/launchpad/better-upload-error-notifications/+merge/311179
 would cover this case - I certainly intended to consider expired keys as 
sufficient for notification purposes - but of course expired keys fail 
signature verification in a different way, so that involves a bit more work.
-- 
Your team Launchpad code reviewers is requested to review the proposed merge of 
lp:~cjwatson/launchpad/upload-key-expired-notification into lp:launchpad.
=== modified file 'lib/lp/archiveuploader/dscfile.py'
--- lib/lp/archiveuploader/dscfile.py	2017-09-17 15:21:22 +
+++ lib/lp/archiveuploader/dscfile.py	2018-03-02 17:27:11 +
@@ -66,6 +66,7 @@
 from lp.registry.interfaces.sourcepackagename import ISourcePackageNameSet
 from lp.services.encoding import guess as guess_encoding
 from lp.services.gpg.interfaces import (
+GPGKeyExpired,
 GPGVerificationError,
 IGPGHandler,
 )
@@ -143,15 +144,24 @@
 
 if verify_signature:
 # We set self.signingkey regardless of whether the key is
-# deactivated, since a deactivated key is still good enough for
-# determining whom to notify, and raising UploadError is enough
-# to prevent the upload being accepted.
-self.signingkey, self.parsed_content = self._verifySignature(
-self.raw_content, self.filepath)
-if not self.signingkey.active:
-raise UploadError("File %s is signed with a deactivated key %s"
-  % (self.filepath,
- self.signingkey.fingerprint))
+# deactivated or expired, since a deactivated or expired key is
+# still good enough for determining whom to notify, and raising
+# UploadError is enough to prevent the upload being accepted.
+try:
+self.signingkey, self.parsed_content = self._verifySignature(
+self.raw_content, self.filepath)
+if not self.signingkey.active:
+raise UploadError(
+"File %s is signed with a deactivated key %s" %
+(self.filepath, self.signingkey.fingerprint))
+except GPGKeyExpired as e:
+# This may theoretically return None, but the "expired"
+# error will take precedence anyway.
+self.signingkey = getUtility(IGPGKeySet).getByFingerprint(
+e.key.fingerprint)
+raise UploadError(
+"File %s is signed with an expired key %s" %
+(self.filepath, e.key.fingerprint))
 else:
 self.logger.debug("%s can be unsigned." % self.filename)
 self.parsed_content = self.raw_content

=== modified file 'lib/lp/archiveuploader/tests/test_uploadprocessor.py'
--- lib/lp/archiveuploader/tests/test_uploadprocessor.py	2018-01-02 16:10:26 +
+++ lib/lp/archiveuploader/tests/test_uploadprocessor.py	2018-03-02 17:27:11 +
@@ -101,7 +101,10 @@
 )
 from lp.testing.dbuser import switch_dbuser
 from lp.testing.fakemethod import FakeMethod
-from lp.testing.gpgkeys import import_public_test_keys
+from lp.testing.gpgkeys import (
+import_public_key,
+import_public_test_keys,
+)
 from lp.testing.layers import LaunchpadZopelessLayer
 from lp.testing.mail_helpers import pop_notifications
 
@@ -1997,6 +2000,47 @@
 self.assertEmails(expected)
 self.assertEqual([], self.oopses)
 
+def test_expired_key_upload_sends_mail(self):
+# An upload signed with an expired key does not OOPS and sends a
+# rejection email.
+self.switchToAdmin()
+email = "expired@canonical.com"
+fingerprint = "0DD64D28E5F41138533495200E3DB4D402F53CC6"
+# This key's email address doesn't exist in sampledata, so we need
+# to create a person for it and import it.
+self.factory.makePerson(email=email)
+import_public_key(email)
+self.switchToUploader()
+
+uploadprocessor = self.setupBreezyAndGetUploadProcessor()
+upload_dir = self.queueUpload("netapplet_1.0-1-expiredkey")
+
+[result] = self.processUpload(uploadprocessor, upload_dir)
+
+self.assertEqual(UploadStatusEnum.REJECTED, result)
+base_contents = [
+"Subject: [ubuntu] netapplet_1.0-1_source.changes (Rejected)",
+"File "
+"%s/netapplet_1.0-1-expiredkey/netapplet_1.0-1_source.changes "
+"is signed