Merge lp:~cjwatson/launchpad/snap-fix-macaroon-auth into lp:launchpad

Proposed by Colin Watson
Status: Merged
Merged at revision: 18062
Proposed branch: lp:~cjwatson/launchpad/snap-fix-macaroon-auth
Merge into: lp:launchpad
Diff against target: 204 lines (+87/-22)
2 files modified
lib/lp/snappy/model/snapstoreclient.py (+26/-13)
lib/lp/snappy/tests/test_snapstoreclient.py (+61/-9)
To merge this branch: bzr merge lp:~cjwatson/launchpad/snap-fix-macaroon-auth
Reviewer Review Type Date Requested Status
William Grant code Approve
Review via email: mp+295563@code.launchpad.net

Commit message

Bind the SSO discharge macaroon before sending it to the store.

Description of the change

I wrote most of SnapStoreClient before we switched to the new handling of discharge macaroons which requires the client to bind them, and forgot to update it. This rectifies that omission.

To post a comment you must log in.
William Grant (wgrant) :
review: Approve (code)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/snappy/model/snapstoreclient.py'
2--- lib/lp/snappy/model/snapstoreclient.py 2016-05-12 09:27:59 +0000
3+++ lib/lp/snappy/model/snapstoreclient.py 2016-05-24 09:41:29 +0000
4@@ -11,12 +11,9 @@
5 ]
6
7 import string
8-try:
9- from urllib.parse import quote_plus
10-except ImportError:
11- from urllib import quote_plus
12
13 from lazr.restful.utils import get_current_browser_request
14+from pymacaroons import Macaroon
15 import requests
16 from requests_toolbelt import MultipartEncoder
17 from zope.interface import implementer
18@@ -59,16 +56,31 @@
19 # The union of the base64 and URL-safe base64 alphabets.
20 allowed_chars = set(string.digits + string.letters + "+/=-_")
21
22- def __init__(self, tokens):
23- self.tokens = tokens
24+ def __init__(self, root_macaroon_raw, unbound_discharge_macaroon_raw):
25+ self.root_macaroon_raw = root_macaroon_raw
26+ self.unbound_discharge_macaroon_raw = unbound_discharge_macaroon_raw
27+
28+ @classmethod
29+ def _makeAuthParam(cls, key, value):
30+ # Check framing.
31+ assert set(key).issubset(cls.allowed_chars)
32+ assert set(value).issubset(cls.allowed_chars)
33+ return '%s="%s"' % (key, value)
34+
35+ @property
36+ def discharge_macaroon_raw(self):
37+ root_macaroon = Macaroon.deserialize(self.root_macaroon_raw)
38+ unbound_discharge_macaroon = Macaroon.deserialize(
39+ self.unbound_discharge_macaroon_raw)
40+ discharge_macaroon = root_macaroon.prepare_for_request(
41+ unbound_discharge_macaroon)
42+ return discharge_macaroon.serialize()
43
44 def __call__(self, r):
45 params = []
46- for k, v in self.tokens.items():
47- # Check framing.
48- assert set(k).issubset(self.allowed_chars)
49- assert set(v).issubset(self.allowed_chars)
50- params.append('%s="%s"' % (k, v))
51+ params.append(self._makeAuthParam("root", self.root_macaroon_raw))
52+ params.append(
53+ self._makeAuthParam("discharge", self.discharge_macaroon_raw))
54 r.headers["Authorization"] = "Macaroon " + ", ".join(params)
55 return r
56
57@@ -144,10 +156,11 @@
58 # that's currently difficult in jobs.
59 try:
60 assert snap.store_secrets is not None
61- assert "discharge" in snap.store_secrets
62 urlfetch(
63 upload_url, method="POST", data=data,
64- auth=MacaroonAuth(snap.store_secrets))
65+ auth=MacaroonAuth(
66+ snap.store_secrets["root"],
67+ snap.store_secrets["discharge"]))
68 except requests.HTTPError as e:
69 raise BadUploadResponse(e.args[0])
70
71
72=== modified file 'lib/lp/snappy/tests/test_snapstoreclient.py'
73--- lib/lp/snappy/tests/test_snapstoreclient.py 2016-05-12 09:27:59 +0000
74+++ lib/lp/snappy/tests/test_snapstoreclient.py 2016-05-24 09:41:29 +0000
75@@ -8,7 +8,7 @@
76 __metaclass__ = type
77
78 from cgi import FieldStorage
79-from collections import OrderedDict
80+import hashlib
81 import io
82 import json
83
84@@ -18,14 +18,20 @@
85 urlmatch,
86 )
87 from lazr.restful.utils import get_current_browser_request
88+from pymacaroons import (
89+ Macaroon,
90+ Verifier,
91+ )
92 from requests import Request
93 from requests.utils import parse_dict_header
94 from testtools.matchers import (
95 Contains,
96 Equals,
97+ KeysEqual,
98 Matcher,
99 MatchesDict,
100 MatchesStructure,
101+ Mismatch,
102 StartsWith,
103 )
104 import transaction
105@@ -46,17 +52,49 @@
106 from lp.testing.layers import LaunchpadZopelessLayer
107
108
109+class MacaroonsVerify(Matcher):
110+ """Matches if serialised macaroons pass verification."""
111+
112+ def __init__(self, key):
113+ self.key = key
114+
115+ def match(self, macaroons):
116+ mismatch = KeysEqual("root", "discharge").match(macaroons)
117+ if mismatch is not None:
118+ return mismatch
119+ root_macaroon = Macaroon.deserialize(macaroons["root"])
120+ discharge_macaroon = Macaroon.deserialize(macaroons["discharge"])
121+ try:
122+ Verifier().verify(root_macaroon, self.key, [discharge_macaroon])
123+ except Exception as e:
124+ return Mismatch("Macaroons do not verify: %s" % e)
125+
126+
127 class TestMacaroonAuth(TestCase):
128
129 def test_good(self):
130 r = Request()
131- MacaroonAuth(OrderedDict([("root", "abc"), ("discharge", "def")]))(r)
132- self.assertEqual(
133- 'Macaroon root="abc", discharge="def"', r.headers["Authorization"])
134+ root_key = hashlib.sha256("root").hexdigest()
135+ root_macaroon = Macaroon(key=root_key)
136+ discharge_key = hashlib.sha256("discharge").hexdigest()
137+ discharge_caveat_id = '{"secret": "thing"}'
138+ root_macaroon.add_third_party_caveat(
139+ "sso.example", discharge_key, discharge_caveat_id)
140+ unbound_discharge_macaroon = Macaroon(
141+ location="sso.example", key=discharge_key,
142+ identifier=discharge_caveat_id)
143+ MacaroonAuth(
144+ root_macaroon.serialize(),
145+ unbound_discharge_macaroon.serialize())(r)
146+ auth_value = r.headers["Authorization"]
147+ self.assertThat(auth_value, StartsWith("Macaroon "))
148+ self.assertThat(
149+ parse_dict_header(auth_value[len("Macaroon "):]),
150+ MacaroonsVerify(root_key))
151
152 def test_bad_framing(self):
153 r = Request()
154- self.assertRaises(AssertionError, MacaroonAuth({"root": 'ev"il'}), r)
155+ self.assertRaises(AssertionError, MacaroonAuth('ev"il', 'wic"ked'), r)
156
157
158 class RequestMatches(Matcher):
159@@ -79,11 +117,11 @@
160 if mismatch is not None:
161 return mismatch
162 auth_value = request.headers["Authorization"]
163- auth_scheme, auth_params = self.auth
164+ auth_scheme, auth_params_matcher = self.auth
165 mismatch = StartsWith(auth_scheme + " ").match(auth_value)
166 if mismatch is not None:
167 return mismatch
168- mismatch = Equals(auth_params).match(
169+ mismatch = auth_params_matcher.match(
170 parse_dict_header(auth_value[len(auth_scheme + " "):]))
171 if mismatch is not None:
172 return mismatch
173@@ -181,7 +219,20 @@
174 self.snap_upload_request = request
175 return {"status_code": 202, "content": {"success": True}}
176
177- store_secrets = {"root": "dummy-root", "discharge": "dummy-discharge"}
178+ root_key = hashlib.sha256(self.factory.getUniqueString()).hexdigest()
179+ root_macaroon = Macaroon(key=root_key)
180+ discharge_key = hashlib.sha256(
181+ self.factory.getUniqueString()).hexdigest()
182+ discharge_caveat_id = self.factory.getUniqueString()
183+ root_macaroon.add_third_party_caveat(
184+ "sso.example", discharge_key, discharge_caveat_id)
185+ unbound_discharge_macaroon = Macaroon(
186+ location="sso.example", key=discharge_key,
187+ identifier=discharge_caveat_id)
188+ store_secrets = {
189+ "root": root_macaroon.serialize(),
190+ "discharge": unbound_discharge_macaroon.serialize(),
191+ }
192 snap = self.factory.makeSnap(
193 store_upload=True,
194 store_series=self.factory.makeSnappySeries(name="rolling"),
195@@ -203,7 +254,8 @@
196 )}))
197 self.assertThat(self.snap_upload_request, RequestMatches(
198 url=Equals("http://sca.example/dev/api/snap-upload/"),
199- method=Equals("POST"), auth=("Macaroon", store_secrets),
200+ method=Equals("POST"),
201+ auth=("Macaroon", MacaroonsVerify(root_key)),
202 form_data={
203 "name": MatchesStructure.byEquality(
204 name="name", value="test-snap"),