launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #22451
[Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
Colin Watson has proposed merging lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad.
Commit message:
Allow macaroon authentication to the librarian for BPBs' source files.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~cjwatson/launchpad/librarian-accept-macaroon/+merge/345079
This will later allow us to dispatch private builds immediately, rather than having to wait for the source to be published first.
In this case we accept the macaroon as the password part of basic authentication with an empty username, similar to the git authentication method used by code import jobs. This is a bit weird, but it avoids needing to change launchpad-buildd, and we could always switch to a more conventional "Authorization: Macaroon ..." scheme later. (Alternatively, I suppose we could put the macaroon in a query parameter instead?)
The only things I can see that we'll need to do deployment-wise are to set up *.restricted.dogfood.launchpadlibrarian.net (both DNS and listening to it on a different IP address from the public librarian) and to configure librarian.macaroon_secret_key.
--
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad.
=== modified file 'configs/testrunner-appserver/launchpad-lazr.conf'
--- configs/testrunner-appserver/launchpad-lazr.conf 2018-03-16 21:04:45 +0000
+++ configs/testrunner-appserver/launchpad-lazr.conf 2018-05-04 10:59:37 +0000
@@ -9,7 +9,7 @@
launch: False
[codeimport]
-macaroon_secret_key: dev-macaroon-secret
+macaroon_secret_key: codeimport-dev-macaroon-secret
[google_test_service]
launch: False
=== modified file 'configs/testrunner/launchpad-lazr.conf'
--- configs/testrunner/launchpad-lazr.conf 2018-03-16 21:04:45 +0000
+++ configs/testrunner/launchpad-lazr.conf 2018-05-04 10:59:37 +0000
@@ -129,6 +129,7 @@
restricted_download_url: http://localhost:58005/
restricted_upload_port: 59095
restricted_download_port: 58005
+macaroon_secret_key: librarian-dev-macaroon-secret
[librarian_server]
root: /var/tmp/fatsam.test
=== modified file 'database/schema/security.cfg'
--- database/schema/security.cfg 2018-04-10 13:07:25 +0000
+++ database/schema/security.cfg 2018-05-04 10:59:37 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
#
# Possible permissions: SELECT, INSERT, UPDATE, EXECUTE
@@ -392,8 +392,11 @@
type=user
[librarian]
+public.binarypackagebuild = SELECT
public.libraryfilealias = SELECT, INSERT, UPDATE
public.libraryfilecontent = SELECT, INSERT
+public.sourcepackagerelease = SELECT
+public.sourcepackagereleasefile = SELECT
type=user
[librarianfeedswift]
=== modified file 'lib/lp/code/model/codeimportjob.py'
--- lib/lp/code/model/codeimportjob.py 2018-03-15 20:44:04 +0000
+++ lib/lp/code/model/codeimportjob.py 2018-05-04 10:59:37 +0000
@@ -442,6 +442,8 @@
def verifyMacaroon(self, macaroon, context):
"""See `IMacaroonIssuer`."""
+ if not ICodeImportJob.providedBy(context):
+ return False
if not self.checkMacaroonIssuer(macaroon):
return False
try:
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2018-04-12 19:59:21 +0000
+++ lib/lp/services/config/schema-lazr.conf 2018-05-04 10:59:37 +0000
@@ -1194,6 +1194,9 @@
use_https = True
+# Secret key for macaroons used to grant library file download permissions.
+macaroon_secret_key: none
+
[librarian_gc]
# The database user which will be used by this process.
=== modified file 'lib/lp/services/librarianserver/db.py'
--- lib/lp/services/librarianserver/db.py 2016-01-10 22:37:40 +0000
+++ lib/lp/services/librarianserver/db.py 2018-05-04 10:59:37 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
"""Database access layer for the Librarian."""
@@ -11,10 +11,13 @@
import hashlib
import urllib
+from pymacaroons import Macaroon
from storm.expr import (
And,
SQL,
)
+from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
from lp.services.database.interfaces import IStore
from lp.services.database.sqlbase import session_store
@@ -23,6 +26,7 @@
LibraryFileContent,
TimeLimitedToken,
)
+from lp.services.macaroons.interfaces import IMacaroonIssuer
class Library:
@@ -69,16 +73,26 @@
#
# This needs to match url_path_quote.
normalised_path = urllib.quote(urllib.unquote(path), safe='/~+')
- store = session_store()
- token_found = store.find(TimeLimitedToken,
- SQL("age(created) < interval '1 day'"),
- TimeLimitedToken.token == hashlib.sha256(token).hexdigest(),
- TimeLimitedToken.path == normalised_path).is_empty()
- store.reset()
- if token_found:
+ if isinstance(token, Macaroon):
+ # We have no Zope interaction, so must remove the proxy.
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, token.identifier))
+ # Macaroons have enough other constraints that they don't
+ # need to be path-specific; it's simpler and faster to just
+ # check the alias ID.
+ token_ok = issuer.verifyMacaroon(token, aliasid)
+ else:
+ store = session_store()
+ token_ok = not store.find(TimeLimitedToken,
+ SQL("age(created) < interval '1 day'"),
+ TimeLimitedToken.token ==
+ hashlib.sha256(token).hexdigest(),
+ TimeLimitedToken.path == normalised_path).is_empty()
+ store.reset()
+ if token_ok:
+ restricted = True
+ else:
raise LookupError("Token stale/pruned/path mismatch")
- else:
- restricted = True
alias = LibraryFileAlias.selectOne(And(
LibraryFileAlias.id == aliasid,
LibraryFileAlias.contentID == LibraryFileContent.q.id,
=== modified file 'lib/lp/services/librarianserver/tests/test_web.py'
--- lib/lp/services/librarianserver/tests/test_web.py 2018-01-02 10:54:31 +0000
+++ lib/lp/services/librarianserver/tests/test_web.py 2018-05-04 10:59:37 +0000
@@ -1,26 +1,25 @@
-# Copyright 2009-2016 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
-from cStringIO import StringIO
from datetime import datetime
+from gzip import GzipFile
import hashlib
import httplib
+from io import BytesIO
import os
import unittest
-from urllib2 import (
- HTTPError,
- urlopen,
- )
from urlparse import urlparse
from lazr.uri import URI
import pytz
+import requests
from storm.expr import SQL
-import testtools
from testtools.matchers import EndsWith
import transaction
from zope.component import getUtility
+from zope.security.proxy import removeSecurityProxy
+from lp.buildmaster.enums import BuildStatus
from lp.services.config import config
from lp.services.database.interfaces import IMasterStore
from lp.services.database.sqlbase import (
@@ -39,7 +38,12 @@
TimeLimitedToken,
)
from lp.services.librarianserver.storage import LibrarianStorage
-from lp.testing.dbuser import switch_dbuser
+from lp.services.macaroons.interfaces import IMacaroonIssuer
+from lp.testing import TestCaseWithFactory
+from lp.testing.dbuser import (
+ dbuser,
+ switch_dbuser,
+ )
from lp.testing.layers import (
LaunchpadFunctionalLayer,
LaunchpadZopelessLayer,
@@ -52,10 +56,9 @@
return str(parsed.replace(path=parsed.path.replace(old, new)))
-class LibrarianWebTestCase(testtools.TestCase):
+class LibrarianWebTestCase(TestCaseWithFactory):
"""Test the librarian's web interface."""
layer = LaunchpadFunctionalLayer
- dbuser = 'librarian'
# Add stuff to a librarian via the upload port, then check that it's
# immediately visible on the web interface. (in an attempt to test ddaa's
@@ -75,9 +78,9 @@
for count in range(10):
# Upload a file. This should work without any exceptions being
# thrown.
- sampleData = 'x' + ('blah' * (count % 5))
+ sampleData = b'x' + (b'blah' * (count % 5))
fileAlias = client.addFile('sample', len(sampleData),
- StringIO(sampleData),
+ BytesIO(sampleData),
contentType='text/plain')
# Make sure we can get its URL
@@ -98,9 +101,9 @@
fileObj.close()
# And make sure the URL works too
- fileObj = urlopen(url)
- self.assertEqual(sampleData, fileObj.read())
- fileObj.close()
+ response = requests.get(url)
+ response.raise_for_status()
+ self.assertEqual(sampleData, response.content)
def test_checkGzipEncoding(self):
# Files that end in ".txt.gz" are treated special and are returned
@@ -108,29 +111,34 @@
# displaying Ubuntu build logs in the browser. The mimetype should be
# "text/plain" for these files.
client = LibrarianClient()
- contents = 'Build log...'
- build_log = StringIO(contents)
+ contents = b'Build log...'
+ build_log = BytesIO()
+ with GzipFile(mode='wb', fileobj=build_log) as f:
+ f.write(contents)
+ build_log.seek(0)
alias_id = client.addFile(name="build_log.txt.gz",
- size=len(contents),
+ size=len(build_log.getvalue()),
file=build_log,
contentType="text/plain")
self.commit()
url = client.getURLForAlias(alias_id)
- fileObj = urlopen(url)
- mimetype = fileObj.headers['content-type']
- encoding = fileObj.headers['content-encoding']
+ response = requests.get(url)
+ response.raise_for_status()
+ mimetype = response.headers['content-type']
+ encoding = response.headers['content-encoding']
self.assertTrue(mimetype == "text/plain; charset=utf-8",
"Wrong mimetype. %s != 'text/plain'." % mimetype)
self.assertTrue(encoding == "gzip",
"Wrong encoding. %s != 'gzip'." % encoding)
+ self.assertEqual(contents, response.content)
def test_checkNoEncoding(self):
# Other files should have no encoding.
client = LibrarianClient()
- contents = 'Build log...'
- build_log = StringIO(contents)
+ contents = b'Build log...'
+ build_log = BytesIO(contents)
alias_id = client.addFile(name="build_log.tgz",
size=len(contents),
file=build_log,
@@ -139,10 +147,10 @@
self.commit()
url = client.getURLForAlias(alias_id)
- fileObj = urlopen(url)
- mimetype = fileObj.headers['content-type']
- self.assertRaises(KeyError, fileObj.headers.__getitem__,
- 'content-encoding')
+ response = requests.get(url)
+ response.raise_for_status()
+ mimetype = response.headers['content-type']
+ self.assertNotIn('content-encoding', response.headers)
self.assertTrue(
mimetype == "application/x-tar",
"Wrong mimetype. %s != 'application/x-tar'." % mimetype)
@@ -157,13 +165,17 @@
# ignored
client = LibrarianClient()
filename = 'sample.txt'
- aid = client.addFile(filename, 6, StringIO('sample'), 'text/plain')
+ aid = client.addFile(filename, 6, BytesIO(b'sample'), 'text/plain')
self.commit()
url = client.getURLForAlias(aid)
- self.assertEqual(urlopen(url).read(), 'sample')
+ response = requests.get(url)
+ response.raise_for_status()
+ self.assertEqual(response.content, b'sample')
old_url = uri_path_replace(url, str(aid), '42/%d' % aid)
- self.assertEqual(urlopen(old_url).read(), 'sample')
+ response = requests.get(url)
+ response.raise_for_status()
+ self.assertEqual(response.content, b'sample')
# If the content and alias IDs are not integers, a 404 is raised
old_url = uri_path_replace(url, str(aid), 'foo/%d' % aid)
@@ -174,10 +186,12 @@
def test_404(self):
client = LibrarianClient()
filename = 'sample.txt'
- aid = client.addFile(filename, 6, StringIO('sample'), 'text/plain')
+ aid = client.addFile(filename, 6, BytesIO(b'sample'), 'text/plain')
self.commit()
url = client.getURLForAlias(aid)
- self.assertEqual(urlopen(url).read(), 'sample')
+ response = requests.get(url)
+ response.raise_for_status()
+ self.assertEqual(response.content, b'sample')
# Change the aliasid and assert we get a 404
self.assertIn(str(aid), url)
@@ -192,29 +206,30 @@
def test_duplicateuploads(self):
client = LibrarianClient()
filename = 'sample.txt'
- id1 = client.addFile(filename, 6, StringIO('sample'), 'text/plain')
- id2 = client.addFile(filename, 6, StringIO('sample'), 'text/plain')
+ id1 = client.addFile(filename, 6, BytesIO(b'sample'), 'text/plain')
+ id2 = client.addFile(filename, 6, BytesIO(b'sample'), 'text/plain')
self.assertNotEqual(id1, id2, 'Got allocated the same id!')
self.commit()
- self.assertEqual(client.getFileByAlias(id1).read(), 'sample')
- self.assertEqual(client.getFileByAlias(id2).read(), 'sample')
+ self.assertEqual(client.getFileByAlias(id1).read(), b'sample')
+ self.assertEqual(client.getFileByAlias(id2).read(), b'sample')
def test_robotsTxt(self):
url = 'http://%s:%d/robots.txt' % (
config.librarian.download_host, config.librarian.download_port)
- f = urlopen(url)
- self.assertIn('Disallow: /', f.read())
+ response = requests.get(url)
+ response.raise_for_status()
+ self.assertIn('Disallow: /', response.text)
def test_headers(self):
client = LibrarianClient()
# Upload a file so we can retrieve it.
- sample_data = 'blah'
+ sample_data = b'blah'
file_alias_id = client.addFile(
- 'sample', len(sample_data), StringIO(sample_data),
+ 'sample', len(sample_data), BytesIO(sample_data),
contentType='text/plain')
url = client.getURLForAlias(file_alias_id)
@@ -229,9 +244,10 @@
self.commit()
# Fetch the file via HTTP, recording the interesting headers
- result = urlopen(url)
- last_modified_header = result.info()['Last-Modified']
- cache_control_header = result.info()['Cache-Control']
+ response = requests.get(url)
+ response.raise_for_status()
+ last_modified_header = response.headers['Last-Modified']
+ cache_control_header = response.headers['Cache-Control']
# URLs point to the same content for ever, so we have a hardcoded
# 1 year max-age cache policy.
@@ -247,9 +263,9 @@
client = LibrarianClient()
# Upload a file so we can retrieve it.
- sample_data = 'blah'
+ sample_data = b'blah'
file_alias_id = client.addFile(
- 'sample', len(sample_data), StringIO(sample_data),
+ 'sample', len(sample_data), BytesIO(sample_data),
contentType='text/plain')
url = client.getURLForAlias(file_alias_id)
@@ -262,18 +278,19 @@
self.commit()
# Fetch the file via HTTP.
- urlopen(url)
+ response = requests.get(url)
+ response.raise_for_status()
# Delete the on-disk file.
storage = LibrarianStorage(config.librarian_server.root, None)
os.remove(storage._fileLocation(file_alias.contentID))
# The URL now 500s, since the DB says it should exist.
- exception = self.assertRaises(HTTPError, urlopen, url)
- self.assertEqual(500, exception.code)
- self.assertIn('Server', exception.info())
- self.assertNotIn('Last-Modified', exception.info())
- self.assertNotIn('Cache-Control', exception.info())
+ response = requests.get(url)
+ self.assertEqual(500, response.status_code)
+ self.assertIn('Server', response.headers)
+ self.assertNotIn('Last-Modified', response.headers)
+ self.assertNotIn('Cache-Control', response.headers)
def get_restricted_file_and_public_url(self, filename='sample'):
# Use a regular LibrarianClient to ensure we speak to the
@@ -281,10 +298,10 @@
# restricted files are served from.
client = LibrarianClient()
fileAlias = client.addFile(
- filename, 12, StringIO('a' * 12), contentType='text/plain')
+ filename, 12, BytesIO(b'a' * 12), contentType='text/plain')
# Note: We're deliberately using the wrong url here: we should be
# passing secure=True to getURLForAlias, but to use the returned URL
- # we would need a wildcard DNS facility patched into urlopen; instead
+ # we would need a wildcard DNS facility patched into requests; instead
# we use the *deliberate* choice of having the path of secure and
# insecure urls be the same, so that we can test it: the server code
# doesn't need to know about the fancy wildcard domains.
@@ -301,9 +318,9 @@
# IFF there is a .restricted. in the host, then the library file alias
# in the subdomain must match that in the path.
client = LibrarianClient()
- fileAlias = client.addFile('sample', 12, StringIO('a' * 12),
+ fileAlias = client.addFile('sample', 12, BytesIO(b'a' * 12),
contentType='text/plain')
- fileAlias2 = client.addFile('sample', 12, StringIO('b' * 12),
+ fileAlias2 = client.addFile('sample', 12, BytesIO(b'b' * 12),
contentType='text/plain')
self.commit()
url = client.getURLForAlias(fileAlias)
@@ -313,7 +330,8 @@
template_host = 'i%%d.restricted.%s' % download_host
path = get_libraryfilealias_download_path(fileAlias, 'sample')
# The basic URL must work.
- urlopen(url)
+ response = requests.get(url)
+ response.raise_for_status()
# Use the network level protocol because DNS resolution won't work
# here (no wildcard support)
connection = httplib.HTTPConnection(
@@ -356,20 +374,17 @@
fileAlias, url = self.get_restricted_file_and_public_url()
# The file should not be able to be opened - the token supplied
# is not one we issued.
- self.require404(url + '?token=haxx0r')
+ self.require404(url, params={"token": "haxx0r"})
def test_restricted_with_token(self):
fileAlias, url = self.get_restricted_file_and_public_url()
# We have the base url for a restricted file; grant access to it
# for a short time.
token = TimeLimitedToken.allocate(url)
- url = url + "?token=%s" % token
# Now we should be able to access the file.
- fileObj = urlopen(url)
- try:
- self.assertEqual("a" * 12, fileObj.read())
- finally:
- fileObj.close()
+ response = requests.get(url, params={"token": token})
+ response.raise_for_status()
+ self.assertEqual(b"a" * 12, response.content)
def test_restricted_with_token_encoding(self):
fileAlias, url = self.get_restricted_file_and_public_url('foo~%')
@@ -380,20 +395,16 @@
token = TimeLimitedToken.allocate(url)
# Now we should be able to access the file.
- fileObj = urlopen(url + "?token=%s" % token)
- try:
- self.assertEqual("a" * 12, fileObj.read())
- finally:
- fileObj.close()
+ response = requests.get(url, params={"token": token})
+ response.raise_for_status()
+ self.assertEqual(b"a" * 12, response.content)
# The token is valid even if the filename is encoded differently.
mangled_url = url.replace('~', '%7E')
self.assertNotEqual(mangled_url, url)
- fileObj = urlopen(mangled_url + "?token=%s" % token)
- try:
- self.assertEqual("a" * 12, fileObj.read())
- finally:
- fileObj.close()
+ response = requests.get(url, params={"token": token})
+ response.raise_for_status()
+ self.assertEqual(b"a" * 12, response.content)
def test_restricted_with_expired_token(self):
fileAlias, url = self.get_restricted_file_and_public_url()
@@ -407,14 +418,55 @@
TimeLimitedToken.token == hashlib.sha256(token).hexdigest())
tokens.set(
TimeLimitedToken.created == SQL("created - interval '1 week'"))
- url = url + "?token=%s" % token
# Now, as per test_restricted_no_token we should get a 404.
- self.require404(url)
+ self.require404(url, params={"token": token})
+
+ def test_restricted_with_macaroon(self):
+ fileAlias, url = self.get_restricted_file_and_public_url()
+ lfa = IMasterStore(LibraryFileAlias).get(LibraryFileAlias, fileAlias)
+ with dbuser('testadmin'):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ naked_build = removeSecurityProxy(build)
+ self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=naked_build.source_package_release,
+ library_file=lfa)
+ naked_build.updateStatus(BuildStatus.BUILDING)
+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+ self.commit()
+ response = requests.get(url, auth=("", macaroon.serialize()))
+ response.raise_for_status()
+ self.assertEqual(b"a" * 12, response.content)
+
+ def test_restricted_with_invalid_macaroon(self):
+ fileAlias, url = self.get_restricted_file_and_public_url()
+ lfa = IMasterStore(LibraryFileAlias).get(LibraryFileAlias, fileAlias)
+ with dbuser('testadmin'):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ naked_build = removeSecurityProxy(build)
+ self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=naked_build.source_package_release,
+ library_file=lfa)
+ naked_build.updateStatus(BuildStatus.BUILDING)
+ self.commit()
+ self.require404(url, auth=("", "not-a-macaroon"))
+
+ def test_restricted_with_unverifiable_macaroon(self):
+ fileAlias, url = self.get_restricted_file_and_public_url()
+ with dbuser('testadmin'):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ removeSecurityProxy(build).updateStatus(BuildStatus.BUILDING)
+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+ self.commit()
+ self.require404(url, auth=("", macaroon.serialize()))
def test_restricted_file_headers(self):
fileAlias, url = self.get_restricted_file_and_public_url()
token = TimeLimitedToken.allocate(url)
- url = url + "?token=%s" % token
# Change the date_created to a known value for testing.
file_alias = IMasterStore(LibraryFileAlias).get(
LibraryFileAlias, fileAlias)
@@ -423,9 +475,9 @@
# Commit the update.
self.commit()
# Fetch the file via HTTP, recording the interesting headers
- result = urlopen(url)
- last_modified_header = result.info()['Last-Modified']
- cache_control_header = result.info()['Cache-Control']
+ response = requests.get(url, params={"token": token})
+ last_modified_header = response.headers['Last-Modified']
+ cache_control_header = response.headers['Cache-Control']
# No caching for restricted files.
self.assertEqual(cache_control_header, 'max-age=0, private')
# And we should have a correct Last-Modified header too.
@@ -433,13 +485,10 @@
last_modified_header, 'Tue, 30 Jan 2001 13:45:59 GMT')
# Perhaps we should also set Expires to the Last-Modified.
- def require404(self, url):
+ def require404(self, url, **kwargs):
"""Assert that opening `url` raises a 404."""
- try:
- urlopen(url)
- self.fail('404 not raised')
- except HTTPError as e:
- self.assertEqual(e.code, 404)
+ response = requests.get(url, **kwargs)
+ self.assertEqual(404, response.status_code)
class LibrarianZopelessWebTestCase(LibrarianWebTestCase):
@@ -455,9 +504,9 @@
def test_getURLForAliasObject(self):
# getURLForAliasObject returns the same URL as getURLForAlias.
client = LibrarianClient()
- content = "Test content"
+ content = b"Test content"
alias_id = client.addFile(
- 'test.txt', len(content), StringIO(content),
+ 'test.txt', len(content), BytesIO(content),
contentType='text/plain')
self.commit()
@@ -481,7 +530,7 @@
switch_dbuser('testadmin')
alias = getUtility(ILibraryFileAliasSet).create(
- 'whatever', 8, StringIO('xxx\nxxx\n'), 'text/plain')
+ 'whatever', 8, BytesIO(b'xxx\nxxx\n'), 'text/plain')
alias_id = alias.id
transaction.commit()
@@ -493,8 +542,9 @@
# And it can be retrieved via the web
url = alias.http_url
- retrieved_content = urlopen(url).read()
- self.assertEqual(retrieved_content, 'xxx\nxxx\n')
+ response = requests.get(url)
+ response.raise_for_status()
+ self.assertEqual(response.content, b'xxx\nxxx\n')
# But when we flag the content as deleted
cur = cursor()
@@ -508,8 +558,5 @@
self.assertRaises(DownloadFailed, alias.open)
# And people see a 404 page
- try:
- urlopen(url)
- self.fail('404 not raised')
- except HTTPError as x:
- self.assertEqual(x.code, 404)
+ response = requests.get(url)
+ self.assertEqual(404, response.status_code)
=== modified file 'lib/lp/services/librarianserver/web.py'
--- lib/lp/services/librarianserver/web.py 2018-01-01 17:40:03 +0000
+++ lib/lp/services/librarianserver/web.py 2018-05-04 10:59:37 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2013 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -7,6 +7,7 @@
import time
from urlparse import urlparse
+from pymacaroons import Macaroon
from storm.exceptions import DisconnectionError
from twisted.internet import defer
from twisted.internet.threads import deferToThread
@@ -116,6 +117,12 @@
return fourOhFour
token = request.args.get('token', [None])[0]
+ if token is None:
+ if not request.getUser() and request.getPassword():
+ try:
+ token = Macaroon.deserialize(request.getPassword())
+ except Exception:
+ pass
path = request.path
deferred = deferToThread(
self._getFileAlias, self.aliasID, token, path)
=== modified file 'lib/lp/soyuz/configure.zcml'
--- lib/lp/soyuz/configure.zcml 2017-07-18 16:22:03 +0000
+++ lib/lp/soyuz/configure.zcml 2018-05-04 10:59:37 +0000
@@ -1,4 +1,4 @@
-<!-- Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+<!-- Copyright 2009-2018 Canonical Ltd. This software is licensed under the
GNU Affero General Public License version 3 (see the file LICENSE).
-->
@@ -480,6 +480,15 @@
<allow interface="lp.buildmaster.interfaces.buildfarmjob.ISpecificBuildFarmJobSource"/>
</securedutility>
+ <!-- BinaryPackageBuildMacaroonIssuer -->
+
+ <securedutility
+ class="lp.soyuz.model.binarypackagebuild.BinaryPackageBuildMacaroonIssuer"
+ provides="lp.services.macaroons.interfaces.IMacaroonIssuer"
+ name="binary-package-build">
+ <allow interface="lp.services.macaroons.interfaces.IMacaroonIssuerPublic"/>
+ </securedutility>
+
<!-- DistroArchSeriesBinaryPackage -->
<class
=== modified file 'lib/lp/soyuz/model/binarypackagebuild.py'
--- lib/lp/soyuz/model/binarypackagebuild.py 2017-03-29 09:28:09 +0000
+++ lib/lp/soyuz/model/binarypackagebuild.py 2018-05-04 10:59:37 +0000
@@ -1,4 +1,4 @@
-# Copyright 2009-2017 Canonical Ltd. This software is licensed under the
+# Copyright 2009-2018 Canonical Ltd. This software is licensed under the
# GNU Affero General Public License version 3 (see the file LICENSE).
__metaclass__ = type
@@ -21,6 +21,10 @@
import apt_pkg
from debian.deb822 import PkgRelation
+from pymacaroons import (
+ Macaroon,
+ Verifier,
+ )
import pytz
from sqlobject import SQLObjectNotFound
from storm.expr import (
@@ -81,6 +85,7 @@
LibraryFileAlias,
LibraryFileContent,
)
+from lp.services.macaroons.interfaces import IMacaroonIssuer
from lp.soyuz.adapters.buildarch import determine_architectures_to_build
from lp.soyuz.enums import (
ArchivePurpose,
@@ -102,7 +107,10 @@
from lp.soyuz.mail.binarypackagebuild import BinaryPackageBuildMailer
from lp.soyuz.model.binarypackagename import BinaryPackageName
from lp.soyuz.model.binarypackagerelease import BinaryPackageRelease
-from lp.soyuz.model.files import BinaryPackageFile
+from lp.soyuz.model.files import (
+ BinaryPackageFile,
+ SourcePackageReleaseFile,
+ )
from lp.soyuz.model.packageset import Packageset
from lp.soyuz.model.queue import (
PackageUpload,
@@ -1362,3 +1370,82 @@
% (build.title, build.id, build.archive.displayname,
build_queue.lastscore))
return new_builds
+
+
+@implementer(IMacaroonIssuer)
+class BinaryPackageBuildMacaroonIssuer:
+
+ @property
+ def _root_secret(self):
+ secret = config.librarian.macaroon_secret_key
+ if not secret:
+ raise RuntimeError("librarian.macaroon_secret_key not configured.")
+ return secret
+
+ def issueMacaroon(self, context):
+ """See `IMacaroonIssuer`.
+
+ For issuing, the context is an `IBinaryPackageBuild`.
+ """
+ if not removeSecurityProxy(context).archive.private:
+ raise AssertionError(
+ "Refusing to issue macaroon for public build.")
+ macaroon = Macaroon(
+ location=config.vhost.mainsite.hostname,
+ identifier="binary-package-build", key=self._root_secret)
+ macaroon.add_first_party_caveat(
+ "lp.binary-package-build %s" % removeSecurityProxy(context).id)
+ return macaroon
+
+ def checkMacaroonIssuer(self, macaroon):
+ """See `IMacaroonIssuer`."""
+ if macaroon.location != config.vhost.mainsite.hostname:
+ return False
+ try:
+ verifier = Verifier()
+ verifier.satisfy_general(
+ lambda caveat: caveat.startswith("lp.binary-package-build "))
+ return verifier.verify(macaroon, self._root_secret)
+ except Exception:
+ return False
+
+ def verifyMacaroon(self, macaroon, context):
+ """See `IMacaroonIssuer`.
+
+ For verification, the context is a `LibraryFileAlias` ID. We check
+ that the file is one of those required to build the
+ `IBinaryPackageBuild` that is the context of the macaroon, and that
+ the context build is currently building.
+ """
+ # Circular import.
+ from lp.soyuz.model.sourcepackagerelease import SourcePackageRelease
+
+ if not isinstance(context, int):
+ return False
+ if not self.checkMacaroonIssuer(macaroon):
+ return False
+
+ def verify_build(caveat):
+ prefix = "lp.binary-package-build "
+ if not caveat.startswith(prefix):
+ return False
+ try:
+ build_id = int(caveat[len(prefix):])
+ except ValueError:
+ return False
+ return not IStore(BinaryPackageBuild).find(
+ BinaryPackageBuild,
+ BinaryPackageBuild.id == build_id,
+ BinaryPackageBuild.source_package_release_id ==
+ SourcePackageRelease.id,
+ SourcePackageReleaseFile.sourcepackagereleaseID ==
+ SourcePackageRelease.id,
+ SourcePackageReleaseFile.libraryfileID == context,
+ BinaryPackageBuild.status == BuildStatus.BUILDING).is_empty()
+
+ try:
+ verifier = Verifier()
+ verifier.satisfy_general(verify_build)
+ return verifier.verify(macaroon, self._root_secret)
+ except Exception:
+ return False
=== modified file 'lib/lp/soyuz/tests/test_binarypackagebuild.py'
--- lib/lp/soyuz/tests/test_binarypackagebuild.py 2018-02-09 14:56:43 +0000
+++ lib/lp/soyuz/tests/test_binarypackagebuild.py 2018-05-04 10:59:37 +0000
@@ -10,8 +10,13 @@
timedelta,
)
+from pymacaroons import Macaroon
import pytz
from simplejson import dumps
+from testtools.matchers import (
+ MatchesListwise,
+ MatchesStructure,
+ )
from zope.component import getUtility
from zope.security.proxy import removeSecurityProxy
@@ -22,7 +27,9 @@
from lp.registry.interfaces.pocket import PackagePublishingPocket
from lp.registry.interfaces.series import SeriesStatus
from lp.registry.interfaces.sourcepackage import SourcePackageUrgency
+from lp.services.config import config
from lp.services.log.logger import DevNullLogger
+from lp.services.macaroons.interfaces import IMacaroonIssuer
from lp.services.webapp.interaction import ANONYMOUS
from lp.services.webapp.interfaces import OAuthPermission
from lp.soyuz.enums import (
@@ -897,3 +904,139 @@
with anonymous_logged_in():
self.assertScoreWriteableByTeam(
archive, getUtility(ILaunchpadCelebrities).ppa_admin)
+
+
+class TestBinaryPackageBuildMacaroonIssuer(TestCaseWithFactory):
+ """Test BinaryPackageBuild macaroon issuing and verification.
+
+ The librarian server verifies these macaroons, and has no Zope
+ interaction when doing so, so we take care to test that verification
+ works without an interaction.
+ """
+
+ layer = LaunchpadZopelessLayer
+
+ def setUp(self):
+ super(TestBinaryPackageBuildMacaroonIssuer, self).setUp()
+ self.pushConfig("librarian", macaroon_secret_key="some-secret")
+
+ def test_issueMacaroon_refuses_public_archive(self):
+ build = self.factory.makeBinaryPackageBuild()
+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+ self.assertRaises(
+ AssertionError, removeSecurityProxy(issuer).issueMacaroon, build)
+
+ def test_issueMacaroon_good(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+ self.assertEqual("launchpad.dev", macaroon.location)
+ self.assertEqual("binary-package-build", macaroon.identifier)
+ self.assertThat(macaroon.caveats, MatchesListwise([
+ MatchesStructure.byEquality(
+ caveat_id="lp.binary-package-build %s" % build.id),
+ ]))
+
+ def test_checkMacaroonIssuer_good(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+ macaroon = removeSecurityProxy(issuer).issueMacaroon(build)
+ self.assertTrue(issuer.checkMacaroonIssuer(macaroon))
+
+ def test_checkMacaroonIssuer_wrong_location(self):
+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+ macaroon = Macaroon(
+ location="another-location",
+ key=removeSecurityProxy(issuer)._root_secret)
+ self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
+
+ def test_checkMacaroonIssuer_wrong_key(self):
+ issuer = getUtility(IMacaroonIssuer, "binary-package-build")
+ macaroon = Macaroon(
+ location=config.vhost.mainsite.hostname, key="another-secret")
+ self.assertFalse(issuer.checkMacaroonIssuer(macaroon))
+
+ def test_verifyMacaroon_good(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ sprf = self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=build.source_package_release)
+ lfa_id = sprf.libraryfile.id
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "binary-package-build"))
+ macaroon = issuer.issueMacaroon(build)
+ logout()
+ self.assertTrue(issuer.verifyMacaroon(macaroon, lfa_id))
+
+ def test_verifyMacaroon_wrong_location(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ sprf = self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=build.source_package_release)
+ lfa_id = sprf.libraryfile.id
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "binary-package-build"))
+ macaroon = Macaroon(
+ location="another-location", key=issuer._root_secret)
+ logout()
+ self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))
+
+ def test_verifyMacaroon_wrong_key(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ sprf = self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=build.source_package_release)
+ lfa_id = sprf.libraryfile.id
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "binary-package-build"))
+ macaroon = Macaroon(
+ location=config.vhost.mainsite.hostname, key="another-secret")
+ logout()
+ self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))
+
+ def test_verifyMacaroon_not_building(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ sprf = self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=build.source_package_release)
+ lfa_id = sprf.libraryfile.id
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "binary-package-build"))
+ macaroon = issuer.issueMacaroon(build)
+ logout()
+ self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))
+
+ def test_verifyMacaroon_wrong_build(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ sprf = self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=build.source_package_release)
+ lfa_id = sprf.libraryfile.id
+ build.updateStatus(BuildStatus.BUILDING)
+ other_build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ other_build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "binary-package-build"))
+ macaroon = issuer.issueMacaroon(other_build)
+ logout()
+ self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))
+
+ def test_verifyMacaroon_wrong_file(self):
+ build = self.factory.makeBinaryPackageBuild(
+ archive=self.factory.makeArchive(private=True))
+ self.factory.makeSourcePackageReleaseFile(
+ sourcepackagerelease=build.source_package_release)
+ lfa = self.factory.makeLibraryFileAlias()
+ lfa_id = lfa.id
+ build.updateStatus(BuildStatus.BUILDING)
+ issuer = removeSecurityProxy(
+ getUtility(IMacaroonIssuer, "binary-package-build"))
+ macaroon = issuer.issueMacaroon(build)
+ logout()
+ self.assertFalse(issuer.verifyMacaroon(macaroon, lfa_id))
Follow ups
-
[Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: noreply, 2019-04-23
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: William Grant, 2019-04-23
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: William Grant, 2019-04-23
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: Colin Watson, 2019-04-23
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: William Grant, 2019-04-23
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: William Grant, 2019-04-23
-
[Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: Colin Watson, 2019-03-13
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: Colin Watson, 2018-11-02
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: Colin Watson, 2018-11-01
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: Colin Watson, 2018-06-05
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: William Grant, 2018-05-10
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: Colin Watson, 2018-05-09
-
Re: [Merge] lp:~cjwatson/launchpad/librarian-accept-macaroon into lp:launchpad
From: William Grant, 2018-05-09