launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #20568
[Merge] lp:~cjwatson/launchpad/snap-mail-store-auth-errors into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/snap-mail-store-auth-errors into lp:launchpad.
Commit message:
Mail the snap build requester if store uploading returns an unauthorized response.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/snap-mail-store-auth-errors/+merge/296443
Mail the snap build requester if store uploading returns an unauthorized response.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/snap-mail-store-auth-errors into lp:launchpad.
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2016-06-02 19:08:32 +0000
+++ database/schema/security.cfg 2016-06-03 14:54:17 +0000
@@ -2541,13 +2541,16 @@
[snap-build-job]
type=user
groups=script
+public.archive = SELECT
public.distribution = SELECT
public.distroarchseries = SELECT
public.distroseries = SELECT
+public.emailaddress = SELECT
public.job = SELECT, UPDATE
public.libraryfilealias = SELECT
public.libraryfilecontent = SELECT
public.person = SELECT
+public.personsettings = SELECT
public.snap = SELECT
public.snapbuild = SELECT, UPDATE
public.snapbuildjob = SELECT, UPDATE
=== modified file 'lib/lp/snappy/interfaces/snapstoreclient.py'
--- lib/lp/snappy/interfaces/snapstoreclient.py 2016-05-28 01:00:33 +0000
+++ lib/lp/snappy/interfaces/snapstoreclient.py 2016-06-03 14:54:17 +0000
@@ -12,6 +12,7 @@
'BadUploadResponse',
'ISnapStoreClient',
'NeedsRefreshResponse',
+ 'UnauthorizedUploadResponse',
]
from zope.interface import Interface
@@ -33,6 +34,10 @@
pass
+class UnauthorizedUploadResponse(Exception):
+ pass
+
+
class ISnapStoreClient(Interface):
"""Interface for the API provided by the snap store."""
=== modified file 'lib/lp/snappy/mail/snapbuild.py'
--- lib/lp/snappy/mail/snapbuild.py 2015-08-23 22:53:55 +0000
+++ lib/lp/snappy/mail/snapbuild.py 2016-06-03 14:54:17 +0000
@@ -1,4 +1,4 @@
-# Copyright 2015 Canonical Ltd. This software is licensed under the
+# Copyright 2015-2016 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -30,13 +30,27 @@
return cls(
"[Snap build #%(build_id)d] %(build_title)s",
"snapbuild-notification.txt", recipients,
- config.canonical.noreply_from_address, build)
+ config.canonical.noreply_from_address, "snap-build-status", build)
+
+ @classmethod
+ def forUnauthorizedUpload(cls, build):
+ """Create a mailer for notifying about unauthorized store uploads.
+
+ :param build: The relevant build.
+ """
+ requester = build.requester
+ recipients = {requester: RecipientReason.forBuildRequester(requester)}
+ return cls(
+ "Store authorization failed for %(snap_name)s",
+ "snapbuild-unauthorized.txt", recipients,
+ config.canonical.noreply_from_address,
+ "snap-build-upload-unauthorized", build)
def __init__(self, subject, template_name, recipients, from_address,
- build):
+ notification_type, build):
super(SnapBuildMailer, self).__init__(
subject, template_name, recipients, from_address,
- notification_type="snap-build-status")
+ notification_type=notification_type)
self.build = build
def _getHeaders(self, email, recipient):
@@ -63,7 +77,9 @@
"log_url": "",
"upload_log_url": "",
"builder_url": "",
- "build_url": canonical_url(self.build),
+ "build_url": canonical_url(build),
+ "snap_authorize_url": canonical_url(
+ build.snap, view_name="+authorize"),
})
if build.duration is not None:
duration_formatter = DurationFormatterAPI(build.duration)
=== modified file 'lib/lp/snappy/model/snapbuildjob.py'
--- lib/lp/snappy/model/snapbuildjob.py 2016-05-06 16:34:21 +0000
+++ lib/lp/snappy/model/snapbuildjob.py 2016-06-03 14:54:17 +0000
@@ -47,7 +47,11 @@
ISnapStoreUploadJob,
ISnapStoreUploadJobSource,
)
-from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
+from lp.snappy.interfaces.snapstoreclient import (
+ ISnapStoreClient,
+ UnauthorizedUploadResponse,
+ )
+from lp.snappy.mail.snapbuild import SnapBuildMailer
class SnapBuildJobType(DBEnumeratedType):
@@ -187,5 +191,8 @@
# message.
transaction.abort()
self.error_message = str(e)
+ if isinstance(e, UnauthorizedUploadResponse):
+ mailer = SnapBuildMailer.forUnauthorizedUpload(self.snapbuild)
+ mailer.sendAll()
transaction.commit()
raise
=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
--- lib/lp/snappy/model/snapstoreclient.py 2016-05-31 10:54:32 +0000
+++ lib/lp/snappy/model/snapstoreclient.py 2016-06-03 14:54:17 +0000
@@ -28,6 +28,7 @@
BadUploadResponse,
ISnapStoreClient,
NeedsRefreshResponse,
+ UnauthorizedUploadResponse,
)
@@ -163,10 +164,12 @@
snap.store_secrets["root"],
snap.store_secrets["discharge"]))
except requests.HTTPError as e:
- if (e.response.status_code == 401 and
- e.response.headers.get("WWW-Authenticate") ==
- "Macaroon needs_refresh=1"):
- raise NeedsRefreshResponse()
+ if e.response.status_code == 401:
+ if (e.response.headers.get("WWW-Authenticate") ==
+ "Macaroon needs_refresh=1"):
+ raise NeedsRefreshResponse()
+ else:
+ raise UnauthorizedUploadResponse("Authorization failed.")
raise BadUploadResponse(e.args[0])
def upload(self, snapbuild):
=== modified file 'lib/lp/snappy/tests/test_snapbuildjob.py'
--- lib/lp/snappy/tests/test_snapbuildjob.py 2016-05-06 16:34:21 +0000
+++ lib/lp/snappy/tests/test_snapbuildjob.py 2016-06-03 14:54:17 +0000
@@ -17,7 +17,10 @@
ISnapBuildJob,
ISnapStoreUploadJob,
)
-from lp.snappy.interfaces.snapstoreclient import ISnapStoreClient
+from lp.snappy.interfaces.snapstoreclient import (
+ ISnapStoreClient,
+ UnauthorizedUploadResponse,
+ )
from lp.snappy.model.snapbuildjob import (
SnapBuildJob,
SnapBuildJobType,
@@ -31,6 +34,7 @@
DatabaseFunctionalLayer,
LaunchpadZopelessLayer,
)
+from lp.testing.mail_helpers import pop_notifications
@implementer(ISnapStoreClient)
@@ -89,6 +93,7 @@
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertIsNone(job.error_message)
+ self.assertEqual([], pop_notifications())
def test_run_failed(self):
# A failed run sets the store upload status to FAILED.
@@ -103,3 +108,42 @@
self.assertEqual([((snapbuild,), {})], client.upload.calls)
self.assertContentEqual([job], snapbuild.store_upload_jobs)
self.assertEqual("An upload failure", job.error_message)
+ self.assertEqual([], pop_notifications())
+
+ def test_run_unauthorized_notifies(self):
+ # A run that gets 401 from the store sends mail.
+ requester = self.factory.makePerson(name="requester")
+ snapbuild = self.factory.makeSnapBuild(
+ requester=requester, name="test-snap", owner=requester)
+ self.assertContentEqual([], snapbuild.store_upload_jobs)
+ job = SnapStoreUploadJob.create(snapbuild)
+ client = FakeSnapStoreClient()
+ client.upload.failure = UnauthorizedUploadResponse(
+ "Authorization failed.")
+ self.useFixture(ZopeUtilityFixture(client, ISnapStoreClient))
+ with dbuser(config.ISnapStoreUploadJobSource.dbuser):
+ JobRunner([job]).runAll()
+ self.assertEqual([((snapbuild,), {})], client.upload.calls)
+ self.assertContentEqual([job], snapbuild.store_upload_jobs)
+ self.assertEqual("Authorization failed.", job.error_message)
+ [notification] = pop_notifications()
+ self.assertEqual(
+ config.canonical.noreply_from_address, notification["From"])
+ self.assertEqual(
+ "Requester <%s>" % requester.preferredemail.email,
+ notification["To"])
+ subject = notification["Subject"].replace("\n ", " ")
+ self.assertEqual("Store authorization failed for test-snap", subject)
+ self.assertEqual(
+ "Requester", notification["X-Launchpad-Message-Rationale"])
+ self.assertEqual(
+ requester.name, notification["X-Launchpad-Message-For"])
+ self.assertEqual(
+ "snap-build-upload-unauthorized",
+ notification["X-Launchpad-Notification-Type"])
+ body, footer = notification.get_payload(decode=True).split("\n-- \n")
+ self.assertIn(
+ "http://launchpad.dev/~requester/+snap/test-snap/+authorize";, body)
+ self.assertEqual(
+ "http://launchpad.dev/~requester/+snap/test-snap/+build/%d\n";
+ "You are the requester of the build.\n" % snapbuild.id, footer)
=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
--- lib/lp/snappy/tests/test_snapstoreclient.py 2016-05-31 10:54:32 +0000
+++ lib/lp/snappy/tests/test_snapstoreclient.py 2016-06-03 14:54:17 +0000
@@ -44,6 +44,7 @@
from lp.snappy.interfaces.snapstoreclient import (
BadRequestPackageUploadResponse,
ISnapStoreClient,
+ UnauthorizedUploadResponse,
)
from lp.snappy.model.snapstoreclient import MacaroonAuth
from lp.testing import (
@@ -279,6 +280,29 @@
"name": "test-snap", "updown_id": 1, "series": "rolling",
}))
+ def test_upload_unauthorized(self):
+ @urlmatch(path=r".*/snap-push/$")
+ def snap_push_handler(url, request):
+ self.snap_push_request = request
+ return {
+ "status_code": 401,
+ "headers": {"WWW-Authenticate": 'Macaroon realm="Devportal"'},
+ }
+
+ store_secrets = self._make_store_secrets()
+ snap = self.factory.makeSnap(
+ store_upload=True,
+ store_series=self.factory.makeSnappySeries(name="rolling"),
+ store_name="test-snap", store_secrets=store_secrets)
+ snapbuild = self.factory.makeSnapBuild(snap=snap)
+ lfa = self.factory.makeLibraryFileAlias(content="dummy snap content")
+ self.factory.makeSnapFile(snapbuild=snapbuild, libraryfile=lfa)
+ transaction.commit()
+ with HTTMock(self._unscanned_upload_handler, snap_push_handler,
+ self._macaroon_refresh_handler):
+ self.assertRaises(
+ UnauthorizedUploadResponse, self.client.upload, snapbuild)
+
def test_upload_needs_discharge_macaroon_refresh(self):
@urlmatch(path=r".*/snap-push/$")
def snap_push_handler(url, request):
Follow ups
