=== modified file 'lib/lp/registry/model/person.py' --- lib/lp/registry/model/person.py 2016-04-26 11:06:17 +0000 +++ lib/lp/registry/model/person.py 2016-05-19 02:03:28 +0000 @@ -289,6 +289,7 @@ OAuthAccessToken, OAuthRequestToken, ) +from lp.services.openid.adapters.openid import CurrentOpenIDEndPoint from lp.services.openid.model.openididentifier import OpenIdIdentifier from lp.services.propertycache import ( cachedproperty, @@ -3280,13 +3281,7 @@ # + is reserved, so is not allowed to be reencoded in transit, so # should never appear as its percent-encoded equivalent. identifier_suffix = None - roots = [config.launchpad.openid_provider_root] - if config.launchpad.openid_alternate_provider_roots: - roots.extend( - [root.strip() for root in - config.launchpad.openid_alternate_provider_roots.split(',') - if root.strip()]) - for root in roots: + for root in CurrentOpenIDEndPoint.getAllRootURLs(): base = '%s+id/' % root if identifier.startswith(base): identifier_suffix = identifier.replace(base, '', 1) === modified file 'lib/lp/services/openid/adapters/openid.py' --- lib/lp/services/openid/adapters/openid.py 2016-05-18 00:33:18 +0000 +++ lib/lp/services/openid/adapters/openid.py 2016-05-19 02:03:28 +0000 @@ -24,11 +24,21 @@ class CurrentOpenIDEndPoint: """A utility for working with multiple OpenID End Points.""" - @classmethod - def getServiceURL(cls): + @staticmethod + def getServiceURL(): """The OpenID server URL (/+openid) for the current request.""" return config.launchpad.openid_provider_root + '+openid' + @staticmethod + def getAllRootURLs(): + """All configured OpenID provider root URLs.""" + yield config.launchpad.openid_provider_root + alternate_roots = config.launchpad.openid_alternate_provider_roots + if alternate_roots: + for root in [r.strip() for r in alternate_roots.split(',')]: + if root: + yield root + @adapter(IAccount) @implementer(IOpenIDPersistentIdentity) === modified file 'lib/lp/services/webapp/login.py' --- lib/lp/services/webapp/login.py 2016-05-11 10:45:12 +0000 +++ lib/lp/services/webapp/login.py 2016-05-19 02:03:28 +0000 @@ -1,4 +1,4 @@ -# Copyright 2009-2011 Canonical Ltd. This software is licensed under the +# Copyright 2009-2016 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). """Stuff to do with logging in and logging out.""" @@ -9,10 +9,6 @@ timedelta, ) import urllib -from urlparse import ( - parse_qsl, - urlunsplit, - ) from openid.consumer.consumer import ( CANCEL, @@ -62,10 +58,7 @@ from lp.services.openid.interfaces.openidconsumer import IOpenIDConsumerStore from lp.services.propertycache import cachedproperty from lp.services.timeline.requesttimeline import get_request_timeline -from lp.services.webapp import ( - canonical_url, - urlsplit, - ) +from lp.services.webapp import canonical_url from lp.services.webapp.error import SystemErrorView from lp.services.webapp.interfaces import ( CookieAuthLoggedInEvent, @@ -226,11 +219,11 @@ # '+login' bit). To do that we encode that URL as a query arg in the # return_to URL passed to the OpenID Provider starting_data = [('starting_url', self.starting_url.encode('utf-8'))] - discharge_macaroon_field = self.request.form.get( - 'discharge_macaroon_field', None) - if discharge_macaroon_field is not None: - starting_data.append( - ('discharge_macaroon_field', discharge_macaroon_field)) + for passthrough_name in ( + 'discharge_macaroon_action', 'discharge_macaroon_field'): + passthrough_field = self.request.form.get(passthrough_name, None) + if passthrough_field is not None: + starting_data.append((passthrough_name, passthrough_field)) starting_url = urllib.urlencode(starting_data) trust_root = allvhosts.configs['mainsite'].rooturl return_to = urlappend(trust_root, '+openid-callback') @@ -264,7 +257,8 @@ """ for name, value in self.request.form.items(): if name in ('loggingout', 'reauth', - 'macaroon_caveat_id', 'discharge_macaroon_field'): + 'macaroon_caveat_id', 'discharge_macaroon_action', + 'discharge_macaroon_field'): continue if name.startswith('openid.'): continue @@ -299,6 +293,9 @@ team_email_address_template = ViewPageTemplateFile( 'templates/login-team-email-address.pt') + discharge_macaroon_template = ViewPageTemplateFile( + 'templates/login-discharge-macaroon.pt') + def _gather_params(self, request): params = dict(request.form) for key, value in request.query_string_params.iteritems(): @@ -407,6 +404,9 @@ with MasterDatabasePolicy(): self.login(person) + if self.params.get('discharge_macaroon_field'): + return self.discharge_macaroon_template() + if should_update_last_write: # This is a GET request but we changed the database, so update # session_data['last_write'] to make sure further requests use @@ -448,27 +448,10 @@ transaction.commit() return retval - @staticmethod - def _appendParam(url, key, value): - """Append a parameter to a URL's query string.""" - parts = urlsplit(url) - query = parse_qsl(parts.query) - query.append((key, value)) - return urlunsplit( - (parts.scheme, parts.netloc, parts.path, urllib.urlencode(query), - parts.fragment)) - def _redirect(self): target = self.params.get('starting_url') if target is None: target = self.request.getApplicationURL() - discharge_macaroon_field = self.params.get('discharge_macaroon_field') - if (discharge_macaroon_field is not None and - self.discharge_macaroon_raw is not None): - # XXX cjwatson 2016-04-18: Do we need to POST this instead due - # to size? - target = self._appendParam( - target, discharge_macaroon_field, self.discharge_macaroon_raw) self.request.response.redirect(target, temporary_if_possible=True) === added file 'lib/lp/services/webapp/templates/login-discharge-macaroon.pt' --- lib/lp/services/webapp/templates/login-discharge-macaroon.pt 1970-01-01 00:00:00 +0000 +++ lib/lp/services/webapp/templates/login-discharge-macaroon.pt 2016-05-19 02:03:28 +0000 @@ -0,0 +1,36 @@ + + + +
+

OpenID transaction in progress

+ +
+ + +
+
+ + + + + === modified file 'lib/lp/services/webapp/tests/test_login.py' --- lib/lp/services/webapp/tests/test_login.py 2016-05-11 10:45:12 +0000 +++ lib/lp/services/webapp/tests/test_login.py 2016-05-19 02:03:28 +0000 @@ -1,4 +1,4 @@ -# Copyright 2009-2012 Canonical Ltd. This software is licensed under the +# Copyright 2009-2016 Canonical Ltd. This software is licensed under the # GNU Affero General Public License version 3 (see the file LICENSE). """Test harness for running the new-login.txt tests.""" @@ -33,7 +33,12 @@ sreg, ) from openid.yadis.discover import DiscoveryFailure -from testtools.matchers import Contains +from testtools.matchers import ( + Contains, + ContainsDict, + Equals, + MatchesListwise, + ) from zope.component import getUtility from zope.security.management import newInteraction from zope.security.proxy import removeSecurityProxy @@ -481,8 +486,8 @@ main_content) def test_discharge_macaroon(self): - # If a discharge macaroon was requested and received, it is added to - # the starting URL as a query string parameter. + # If a discharge macaroon was requested and received, the view + # returns a form that submits it to the starting URL. test_email = 'test-example@example.com' person = self.factory.makePerson(email=test_email) identifier = ITestOpenIDPersistentIdentity( @@ -492,6 +497,7 @@ full_name='Foo User', discharge_macaroon_raw='dummy discharge') form = { 'starting_url': 'http://launchpad.dev/after-login', + 'discharge_macaroon_action': 'field.actions.complete', 'discharge_macaroon_field': 'field.discharge_macaroon', } with SRegResponse_fromSuccessResponse_stubbed(): @@ -500,12 +506,20 @@ openid_response, form=form) self.assertTrue(view.login_called) self.assertEqual('dummy discharge', view.discharge_macaroon_raw) - response = view.request.response - self.assertEqual(httplib.TEMPORARY_REDIRECT, response.getStatus()) - self.assertEqual( - form['starting_url'] + - '?field.discharge_macaroon=dummy+discharge', - response.getHeader('Location')) + discharge_form = find_tag_by_id(html, 'discharge-form') + self.assertEqual(form['starting_url'], discharge_form['action']) + self.assertThat( + [dict(tag.attrs) for tag in discharge_form.findAll('input')], + MatchesListwise([ + ContainsDict({ + 'name': Equals('field.actions.complete'), + 'value': Equals('1'), + }), + ContainsDict({ + 'name': Equals('field.discharge_macaroon'), + 'value': Equals('dummy discharge'), + }), + ])) def test_discharge_macaroon_missing(self): # If a discharge macaroon was requested but not received, the login @@ -519,6 +533,7 @@ full_name='Foo User') form = { 'starting_url': 'http://launchpad.dev/after-login', + 'discharge_macaroon_action': 'field.actions.complete', 'discharge_macaroon_field': 'field.discharge_macaroon', } with SRegResponse_fromSuccessResponse_stubbed(): @@ -852,8 +867,8 @@ # extension. caveat_id = 'ask SSO' form = { - 'field.callback': '1', 'macaroon_caveat_id': caveat_id, + 'discharge_macaroon_action': 'field.actions.complete', 'discharge_macaroon_field': 'field.discharge_macaroon', } request = LaunchpadTestRequest(form=form, method='POST') @@ -871,11 +886,11 @@ return_to_args = dict(urlparse.parse_qsl( urlparse.urlsplit(view.openid_request.return_to).query)) self.assertEqual( + 'field.actions.complete', + return_to_args['discharge_macaroon_action']) + self.assertEqual( 'field.discharge_macaroon', return_to_args['discharge_macaroon_field']) - starting_url_args = dict(urlparse.parse_qsl( - urlparse.urlsplit(return_to_args['starting_url']).query)) - self.assertEqual('1', starting_url_args['field.callback']) def test_logs_to_timeline(self): # Beginning an OpenID association makes an HTTP request to the === modified file 'lib/lp/snappy/browser/configure.zcml' --- lib/lp/snappy/browser/configure.zcml 2016-05-06 11:54:18 +0000 +++ lib/lp/snappy/browser/configure.zcml 2016-05-19 02:03:28 +0000 @@ -68,6 +68,12 @@ template="../templates/snap-edit.pt" /> + 1: + raise SnapAuthorizationException( + "Macaroon has multiple SSO caveats") + return sso_caveats[0] + + @classmethod + def requestAuthorization(cls, snap, request): + """Begin the process of authorizing uploads of a snap package.""" + if snap.store_series is None: + request.response.addInfoNotification( + _(u'Cannot authorize uploads of a snap package with no ' + u'store series.')) + request.response.redirect(canonical_url(snap)) + return + if snap.store_name is None: + request.response.addInfoNotification( + _(u'Cannot authorize uploads of a snap package with no ' + u'store name.')) + request.response.redirect(canonical_url(snap)) + return + snap_store_client = getUtility(ISnapStoreClient) + root_macaroon_raw = snap_store_client.requestPackageUploadPermission( + snap.store_series, snap.store_name) + sso_caveat = cls.extractSSOCaveat( + Macaroon.deserialize(root_macaroon_raw)) + snap.store_secrets = {'root': root_macaroon_raw} + base_url = canonical_url(snap, view_name='+authorize') + login_url = urlappend(base_url, '+login') + login_url += '?%s' % urlencode([ + ('macaroon_caveat_id', sso_caveat.caveat_id), + ('discharge_macaroon_action', 'field.actions.complete'), + ('discharge_macaroon_field', 'field.discharge_macaroon'), + ]) + return login_url + + @action('Begin authorization', name='begin') + def begin_action(self, action, data): + login_url = self.requestAuthorization(self.context, self.request) + if login_url is not None: + self.request.response.redirect(login_url) + + @action('Complete authorization', name='complete') + def complete_action(self, action, data): + if not data.get('discharge_macaroon'): + self.addError(structured( + _(u'Uploads of %(snap)s to the store were not authorized.'), + snap=self.context.name)) + return + # We have to set a whole new dict here to avoid problems with + # security proxies. + new_store_secrets = dict(self.context.store_secrets) + new_store_secrets['discharge'] = data['discharge_macaroon'] + self.context.store_secrets = new_store_secrets + self.request.response.addInfoNotification(structured( + _(u'Uploads of %(snap)s to the store are now authorized.'), + snap=self.context.name)) + self.request.response.redirect(canonical_url(self.context)) + + @property + def adapters(self): + """See `LaunchpadFormView`.""" + return {self.schema: self.context} + + class SnapDeleteView(BaseSnapEditView): """View for deleting snap packages.""" === modified file 'lib/lp/snappy/browser/tests/test_snap.py' --- lib/lp/snappy/browser/tests/test_snap.py 2016-05-06 09:45:45 +0000 +++ lib/lp/snappy/browser/tests/test_snap.py 2016-05-19 02:03:28 +0000 @@ -9,17 +9,26 @@ datetime, timedelta, ) +import json import re from textwrap import dedent +from urllib2 import HTTPError +from urlparse import urlsplit from fixtures import FakeLogger +from httmock import ( + all_requests, + HTTMock, + ) from mechanize import LinkNotFoundError +from pymacaroons import Macaroon import pytz import soupmatchers from testtools.matchers import ( MatchesSetwise, MatchesStructure, ) +import transaction from zope.component import getUtility from zope.publisher.interfaces import NotFound from zope.security.interfaces import Unauthorized @@ -31,12 +40,14 @@ from lp.registry.enums import PersonVisibility from lp.registry.interfaces.pocket import PackagePublishingPocket from lp.registry.interfaces.series import SeriesStatus +from lp.services.config import config from lp.services.database.constants import UTC_NOW from lp.services.features.testing import FeatureFixture from lp.services.webapp import canonical_url from lp.services.webapp.servers import LaunchpadTestRequest from lp.snappy.browser.snap import ( SnapAdminView, + SnapAuthorizeView, SnapEditView, SnapView, ) @@ -48,6 +59,7 @@ SnapPrivateFeatureDisabled, ) from lp.testing import ( + admin_logged_in, BrowserTestCase, feature_flags, login, @@ -70,6 +82,7 @@ find_main_content, find_tags_by_class, find_tag_by_id, + get_feedback_messages, ) from lp.testing.publication import test_traverse from lp.testing.views import ( @@ -559,6 +572,146 @@ self.assertSnapProcessors(snap, ["386", "armhf"]) +class TestSnapAuthorizeView(BrowserTestCase): + + layer = LaunchpadFunctionalLayer + + def setUp(self): + super(TestSnapAuthorizeView, self).setUp() + self.useFixture(FeatureFixture(SNAP_TESTING_FLAGS)) + self.person = self.factory.makePerson( + name="test-person", displayname="Test Person") + self.distroseries = self.factory.makeUbuntuDistroSeries() + with admin_logged_in(): + self.snappyseries = self.factory.makeSnappySeries( + usable_distro_series=[self.distroseries]) + self.snap = self.factory.makeSnap( + registrant=self.person, owner=self.person, + distroseries=self.distroseries, store_upload=True, + store_series=self.snappyseries, + store_name=self.factory.getUniqueUnicode()) + + def assertRequestsAuthorization(self, snap, func, *args, **kwargs): + owner = snap.owner + root_macaroon = Macaroon() + root_macaroon.add_third_party_caveat( + urlsplit(config.launchpad.openid_provider_root).netloc, '', + 'dummy') + root_macaroon_raw = root_macaroon.serialize() + + @all_requests + def handler(url, request): + self.request = request + return { + "status_code": 200, + "content": {"macaroon": root_macaroon_raw}, + } + + self.pushConfig("snappy", store_url="http://sca.example/") + with HTTMock(handler): + ret = func(*args, **kwargs) + self.assertThat(self.request, MatchesStructure.byEquality( + url="http://sca.example/dev/api/acl/", method="POST")) + with person_logged_in(owner): + expected_body = { + "packages": [{ + "name": snap.store_name, + "series": snap.store_series.name, + }], + "permissions": ["package_upload"], + } + self.assertEqual(expected_body, json.loads(self.request.body)) + self.assertEqual({"root": root_macaroon_raw}, snap.store_secrets) + return ret + + def test_requestAuthorization(self): + def request_authorization(): + with person_logged_in(self.snap.owner): + return SnapAuthorizeView.requestAuthorization( + self.snap, LaunchpadTestRequest()) + + login_url = self.assertRequestsAuthorization( + self.snap, request_authorization) + self.assertEqual( + canonical_url(self.snap) + + "/+authorize/+login?macaroon_caveat_id=dummy&" + "discharge_macaroon_action=field.actions.complete&" + "discharge_macaroon_field=field.discharge_macaroon", + login_url) + + def test_unauthorized(self): + # A user without edit access cannot authorize snap package uploads. + self.useFixture(FakeLogger()) + other_person = self.factory.makePerson() + self.assertRaises( + Unauthorized, self.getUserBrowser, + canonical_url(self.snap) + "/+authorize", user=other_person) + + def test_begin_authorization(self): + # With no special form actions, we return a form inviting the user + # to begin authorization. This allows (re-)authorizing uploads of + # an existing snap package without having to edit it. + snap_url = canonical_url(self.snap) + + def begin_authorization(): + browser = self.getNonRedirectingBrowser( + url=snap_url + "/+authorize", user=self.snap.owner) + return self.assertRaises( + HTTPError, browser.getControl("Begin authorization").click) + + redirection = self.assertRequestsAuthorization( + self.snap, begin_authorization) + self.assertEqual(303, redirection.code) + self.assertEqual( + snap_url + "/+authorize/+login?macaroon_caveat_id=dummy&" + "discharge_macaroon_action=field.actions.complete&" + "discharge_macaroon_field=field.discharge_macaroon", + redirection.hdrs["Location"]) + + def test_complete_authorization_missing_discharge_macaroon(self): + # If the form does not include a discharge macaroon, the "complete" + # action fails. + with person_logged_in(self.snap.owner): + self.snap.store_secrets = {"root": "root"} + transaction.commit() + form = {"field.actions.complete": "1"} + view = create_initialized_view( + self.snap, "+authorize", form=form, method="POST", + principal=self.snap.owner) + html = view() + self.assertEqual( + "Uploads of %s to the store were not authorized." % + self.snap.name, + get_feedback_messages(html)[1]) + self.assertNotIn("discharge", self.snap.store_secrets) + + def test_complete_authorization(self): + # If the form includes a discharge macaroon, the "complete" action + # succeeds and records the new secrets. + with person_logged_in(self.snap.owner): + self.snap.store_secrets = {"root": "root"} + transaction.commit() + form = { + "field.actions.complete": "1", + "field.discharge_macaroon": "discharge", + } + view = create_initialized_view( + self.snap, "+authorize", form=form, method="POST", + principal=self.snap.owner) + self.assertEqual("", view()) + self.assertEqual(302, view.request.response.getStatus()) + self.assertEqual( + canonical_url(self.snap), + view.request.response.getHeader("Location")) + self.assertEqual( + "Uploads of %s to the store are now authorized." % + self.snap.name, + view.request.response.notifications[0].message) + self.assertEqual( + {"root": "root", "discharge": "discharge"}, + self.snap.store_secrets) + + class TestSnapDeleteView(BrowserTestCase): layer = LaunchpadFunctionalLayer === added file 'lib/lp/snappy/templates/snap-authorize.pt' --- lib/lp/snappy/templates/snap-authorize.pt 1970-01-01 00:00:00 +0000 +++ lib/lp/snappy/templates/snap-authorize.pt 2016-05-19 02:03:28 +0000 @@ -0,0 +1,24 @@ + + + +
+
+

+ The login service will prompt you to authorize this request. +

+ +
+ + or Cancel +
+
+
+ + + === modified file 'setup.py' --- setup.py 2016-05-03 16:38:52 +0000 +++ setup.py 2016-05-19 02:03:28 +0000 @@ -78,6 +78,7 @@ 'pgbouncer', 'psycopg2', 'pyasn1', + 'pymacaroons', 'pystache', 'python-memcached', 'python-openid', === modified file 'versions.cfg' --- versions.cfg 2016-05-12 10:53:22 +0000 +++ versions.cfg 2016-05-19 02:03:28 +0000 @@ -67,6 +67,7 @@ lazr.sshserver = 0.1.3 lazr.testing = 0.1.1 lazr.uri = 1.0.3 +libnacl = 1.3.6 lpjsmin = 0.5 manuel = 1.7.2 Markdown = 2.3.1 @@ -94,6 +95,7 @@ Pygments = 1.6 pygpgme = 0.2 pyinotify = 0.9.4 +pymacaroons = 0.9.2 pymongo = 2.1.1 pyOpenSSL = 0.13 pystache = 0.5.3