From c2333bfe4532102af551837fa4652bba06086643 Mon Sep 17 00:00:00 2001 From: Sergio Schvezov Date: Fri, 26 Mar 2021 16:45:42 -0300 Subject: [PATCH 1/4] httpbakery/client: catch non json discharge responses Signed-off-by: Sergio Schvezov --- macaroonbakery/httpbakery/_client.py | 8 +++++ macaroonbakery/tests/test_client.py | 45 ++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/macaroonbakery/httpbakery/_client.py b/macaroonbakery/httpbakery/_client.py index 326c33d..24ec24d 100644 --- a/macaroonbakery/httpbakery/_client.py +++ b/macaroonbakery/httpbakery/_client.py @@ -3,6 +3,7 @@ import base64 import json import logging +from json.decoder import JSONDecodeError import macaroonbakery.bakery as bakery import macaroonbakery.checkers as checkers @@ -132,6 +133,13 @@ def acquire_discharge(self, cav, payload): if resp.status_code == 200: return bakery.Macaroon.from_dict(resp.json().get('Macaroon')) cause = Error.from_dict(resp.json()) + # A 5xx error might not return json. + try: + cause = Error.from_dict(resp.json()) + except JSONDecodeError: + raise DischargeError( + 'unexpected response: [{}] {!r}'.format(resp.status_code, resp.content) + ) if cause.code != ERR_INTERACTION_REQUIRED: raise DischargeError(cause.message) if cause.info is None: diff --git a/macaroonbakery/tests/test_client.py b/macaroonbakery/tests/test_client.py index 6437a54..7c7e9bf 100644 --- a/macaroonbakery/tests/test_client.py +++ b/macaroonbakery/tests/test_client.py @@ -3,6 +3,7 @@ import base64 import datetime import json +from json.decoder import JSONDecodeError import threading import macaroonbakery.bakery as bakery @@ -11,6 +12,7 @@ import pymacaroons import requests import macaroonbakery._utils as utils +from macaroonbakery.httpbakery._error import DischargeError from fixtures import ( EnvironmentVariable, @@ -516,6 +518,49 @@ def kind(self): finally: httpd.shutdown() + def test_discharge_jsondecodeerror(self): + class _DischargerLocator(bakery.ThirdPartyLocator): + def __init__(self): + self.key = bakery.generate_key() + + def third_party_info(self, loc): + if loc == 'http://1.2.3.4': + return bakery.ThirdPartyInfo( + public_key=self.key.public_key, + version=bakery.LATEST_VERSION, + ) + d = _DischargerLocator() + b = new_bakery('loc', d, None) + + @urlmatch(path='.*/discharge') + def discharge(url, request): + return { + 'status_code': 503, + 'content': 'bad system', + } + + def handler(*args): + GetHandler(b, 'http://1.2.3.4', None, None, None, AGES, *args) + + try: + httpd = HTTPServer(('', 0), handler) + thread = threading.Thread(target=httpd.serve_forever) + thread.start() + + client = httpbakery.Client() + + with HTTMock(discharge): + with self.assertRaises(DischargeError) as discharge_error: + requests.get( + 'http://' + httpd.server_address[0] + ':' + str( + httpd.server_address[1]), + cookies=client.cookies, + auth=client.auth()) + self.assertEquals(str(discharge_error.exception), "unexpected response: [503] b'bad system'") + + finally: + httpd.shutdown() + def test_extract_macaroons_from_request(self): def encode_macaroon(m): macaroons = '[' + utils.macaroon_to_json_string(m) + ']' From bafbbf1ef6ec3a703870092785aa0c4a4de16297 Mon Sep 17 00:00:00 2001 From: Sergio Schvezov Date: Mon, 29 Mar 2021 09:56:37 -0300 Subject: [PATCH 2/4] httpbakery/client: remove left over json decode operation Signed-off-by: Sergio Schvezov --- macaroonbakery/httpbakery/_client.py | 1 - 1 file changed, 1 deletion(-) diff --git a/macaroonbakery/httpbakery/_client.py b/macaroonbakery/httpbakery/_client.py index 24ec24d..fa8dc10 100644 --- a/macaroonbakery/httpbakery/_client.py +++ b/macaroonbakery/httpbakery/_client.py @@ -132,7 +132,6 @@ def acquire_discharge(self, cav, payload): # TODO Fabrice what is the other http response possible ?? if resp.status_code == 200: return bakery.Macaroon.from_dict(resp.json().get('Macaroon')) - cause = Error.from_dict(resp.json()) # A 5xx error might not return json. try: cause = Error.from_dict(resp.json()) From d139153210134e96486a0989dd9a08ffc25b1b09 Mon Sep 17 00:00:00 2001 From: Sergio Schvezov Date: Mon, 29 Mar 2021 10:02:25 -0300 Subject: [PATCH 3/4] httpbakery/client: catch ValueError instead of JSONDecodeError ValueError is the documented exception. Also fix the error string comparison test. Signed-off-by: Sergio Schvezov --- macaroonbakery/httpbakery/_client.py | 3 +-- macaroonbakery/tests/test_client.py | 5 +++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/macaroonbakery/httpbakery/_client.py b/macaroonbakery/httpbakery/_client.py index fa8dc10..230531d 100644 --- a/macaroonbakery/httpbakery/_client.py +++ b/macaroonbakery/httpbakery/_client.py @@ -3,7 +3,6 @@ import base64 import json import logging -from json.decoder import JSONDecodeError import macaroonbakery.bakery as bakery import macaroonbakery.checkers as checkers @@ -135,7 +134,7 @@ def acquire_discharge(self, cav, payload): # A 5xx error might not return json. try: cause = Error.from_dict(resp.json()) - except JSONDecodeError: + except ValueError: raise DischargeError( 'unexpected response: [{}] {!r}'.format(resp.status_code, resp.content) ) diff --git a/macaroonbakery/tests/test_client.py b/macaroonbakery/tests/test_client.py index 7c7e9bf..37f4d72 100644 --- a/macaroonbakery/tests/test_client.py +++ b/macaroonbakery/tests/test_client.py @@ -3,7 +3,6 @@ import base64 import datetime import json -from json.decoder import JSONDecodeError import threading import macaroonbakery.bakery as bakery @@ -556,7 +555,9 @@ def handler(*args): httpd.server_address[1]), cookies=client.cookies, auth=client.auth()) - self.assertEquals(str(discharge_error.exception), "unexpected response: [503] b'bad system'") + self.assertEquals(str(discharge_error.exception), + 'third party refused dischargex: unexpected response: ' + "[503] b'bad system'") finally: httpd.shutdown() From 388d39ea750ef0afce69d50c124aeb90736d7178 Mon Sep 17 00:00:00 2001 From: Sergio Schvezov Date: Wed, 31 Mar 2021 14:49:45 -0300 Subject: [PATCH 4/4] macaroonbakery/tests: account for python2 Exposing the error as-is requires handling of byte notation between Python 2 and 3 and strings when asserting. Signed-off-by: Sergio Schvezov --- macaroonbakery/tests/test_client.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/macaroonbakery/tests/test_client.py b/macaroonbakery/tests/test_client.py index 37f4d72..b754317 100644 --- a/macaroonbakery/tests/test_client.py +++ b/macaroonbakery/tests/test_client.py @@ -3,6 +3,7 @@ import base64 import datetime import json +import platform import threading import macaroonbakery.bakery as bakery @@ -555,9 +556,14 @@ def handler(*args): httpd.server_address[1]), cookies=client.cookies, auth=client.auth()) - self.assertEquals(str(discharge_error.exception), - 'third party refused dischargex: unexpected response: ' - "[503] b'bad system'") + if platform.python_version_tuple()[0] == '2': + self.assertEquals(str(discharge_error.exception), + 'third party refused dischargex: unexpected response: ' + "[503] 'bad system'") + else: + self.assertEquals(str(discharge_error.exception), + 'third party refused dischargex: unexpected response: ' + "[503] b'bad system'") finally: httpd.shutdown()