From 418d45a5be3f4e06443aae7313eb79d88027da85 Mon Sep 17 00:00:00 2001 From: James Sutton <1892175+zeryx@users.noreply.github.com> Date: Wed, 22 Dec 2021 15:18:54 -0400 Subject: [PATCH 1/7] critical fix to bearer token breakage --- Algorithmia/client.py | 33 ++++++++++++++++++++++----------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/Algorithmia/client.py b/Algorithmia/client.py index 7247376..9692051 100644 --- a/Algorithmia/client.py +++ b/Algorithmia/client.py @@ -31,11 +31,10 @@ def __init__(self, apiKey = None, apiAddress = None, caCert = None, bearerToken= self.requestSession = requests.Session() if apiKey is None and 'ALGORITHMIA_API_KEY' in os.environ: apiKey = os.environ['ALGORITHMIA_API_KEY'] - if apiKey is None: - if bearerToken is None and 'ALGORITHMIA_BEARER_TOKEN' in os.environ: - bearerToken = os.environ['ALGORITHMIA_BEARER_TOKEN'] - self.bearerToken = bearerToken - + elif bearerToken is None and 'ALGORITHMIA_BEARER_TOKEN' in os.environ: + bearerToken = os.environ['ALGORITHMIA_BEARER_TOKEN'] + + self.bearerToken = bearerToken self.apiKey = apiKey if apiAddress is not None: self.apiAddress = apiAddress @@ -254,24 +253,30 @@ def getHelper(self, url, **query_parameters): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - else: + elif self.bearerToken is not None: headers['Authorization'] = 'Bearer '+ self.bearerToken + else: + raise Exception("No authentication provided") return self.requestSession.get(self.apiAddress + url, headers=headers, params=query_parameters) def getStreamHelper(self, url, **query_parameters): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - else: + elif self.bearerToken is not None: headers['Authorization'] = 'Bearer '+ self.bearerToken + else: + raise Exception("No authentication provided") return self.requestSession.get(self.apiAddress + url, headers=headers, params=query_parameters, stream=True) def patchHelper(self, url, params): headers = {'content-type': 'application/json'} if self.apiKey is not None: headers['Authorization'] = self.apiKey - else: + elif self.bearerToken is not None: headers['Authorization'] = 'Bearer '+ self.bearerToken + else: + raise Exception("No authentication provided") return self.requestSession.patch(self.apiAddress + url, headers=headers, data=json.dumps(params)) # Used internally to get http head result @@ -279,8 +284,10 @@ def headHelper(self, url): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - else: + elif self.bearerToken is not None: headers['Authorization'] = 'Bearer '+ self.bearerToken + else: + raise Exception("No authentication provided") return self.requestSession.head(self.apiAddress + url, headers=headers) # Used internally to http put a file @@ -288,8 +295,10 @@ def putHelper(self, url, data): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - else: + elif self.bearerToken is not None: headers['Authorization'] = 'Bearer '+ self.bearerToken + else: + raise Exception("No authentication provided") if isJson(data): headers['Content-Type'] = 'application/json' @@ -303,8 +312,10 @@ def deleteHelper(self, url): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - else: + elif self.bearerToken is not None: headers['Authorization'] = 'Bearer '+ self.bearerToken + else: + raise Exception("No authentication provided") response = self.requestSession.delete(self.apiAddress + url, headers=headers) if response.reason == "No Content": return response From 6e01553e488e807ff9ad7ac4eee421570236efdf Mon Sep 17 00:00:00 2001 From: James Sutton <1892175+zeryx@users.noreply.github.com> Date: Wed, 22 Dec 2021 15:26:53 -0400 Subject: [PATCH 2/7] corrected issue with header initialization in json helper --- Algorithmia/client.py | 38 ++++++++++++++++++++------------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/Algorithmia/client.py b/Algorithmia/client.py index 9692051..fe01da4 100644 --- a/Algorithmia/client.py +++ b/Algorithmia/client.py @@ -24,8 +24,7 @@ class Client(object): requestSession = None bearerToken = None - - def __init__(self, apiKey = None, apiAddress = None, caCert = None, bearerToken=None): + def __init__(self, apiKey=None, apiAddress=None, caCert=None, bearerToken=None): # Override apiKey with environment variable config = None self.requestSession = requests.Session() @@ -33,7 +32,7 @@ def __init__(self, apiKey = None, apiAddress = None, caCert = None, bearerToken= apiKey = os.environ['ALGORITHMIA_API_KEY'] elif bearerToken is None and 'ALGORITHMIA_BEARER_TOKEN' in os.environ: bearerToken = os.environ['ALGORITHMIA_BEARER_TOKEN'] - + self.bearerToken = bearerToken self.apiKey = apiKey if apiAddress is not None: @@ -224,8 +223,10 @@ def postJsonHelper(self, url, input_object, parse_response_as_json=True, **query headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - else: - headers['Authorization'] = "Bearer "+ self.bearerToken + elif self.bearerToken is not None: + headers['Authorization'] = 'Bearer ' + self.bearerToken + else: + raise Exception("No authentication provided") input_json = None if input_object is None: @@ -253,8 +254,8 @@ def getHelper(self, url, **query_parameters): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - elif self.bearerToken is not None: - headers['Authorization'] = 'Bearer '+ self.bearerToken + elif self.bearerToken is not None: + headers['Authorization'] = 'Bearer ' + self.bearerToken else: raise Exception("No authentication provided") return self.requestSession.get(self.apiAddress + url, headers=headers, params=query_parameters) @@ -263,8 +264,8 @@ def getStreamHelper(self, url, **query_parameters): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - elif self.bearerToken is not None: - headers['Authorization'] = 'Bearer '+ self.bearerToken + elif self.bearerToken is not None: + headers['Authorization'] = 'Bearer ' + self.bearerToken else: raise Exception("No authentication provided") return self.requestSession.get(self.apiAddress + url, headers=headers, params=query_parameters, stream=True) @@ -273,8 +274,8 @@ def patchHelper(self, url, params): headers = {'content-type': 'application/json'} if self.apiKey is not None: headers['Authorization'] = self.apiKey - elif self.bearerToken is not None: - headers['Authorization'] = 'Bearer '+ self.bearerToken + elif self.bearerToken is not None: + headers['Authorization'] = 'Bearer ' + self.bearerToken else: raise Exception("No authentication provided") return self.requestSession.patch(self.apiAddress + url, headers=headers, data=json.dumps(params)) @@ -284,8 +285,8 @@ def headHelper(self, url): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - elif self.bearerToken is not None: - headers['Authorization'] = 'Bearer '+ self.bearerToken + elif self.bearerToken is not None: + headers['Authorization'] = 'Bearer ' + self.bearerToken else: raise Exception("No authentication provided") return self.requestSession.head(self.apiAddress + url, headers=headers) @@ -295,8 +296,8 @@ def putHelper(self, url, data): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - elif self.bearerToken is not None: - headers['Authorization'] = 'Bearer '+ self.bearerToken + elif self.bearerToken is not None: + headers['Authorization'] = 'Bearer ' + self.bearerToken else: raise Exception("No authentication provided") if isJson(data): @@ -312,8 +313,8 @@ def deleteHelper(self, url): headers = {} if self.apiKey is not None: headers['Authorization'] = self.apiKey - elif self.bearerToken is not None: - headers['Authorization'] = 'Bearer '+ self.bearerToken + elif self.bearerToken is not None: + headers['Authorization'] = 'Bearer ' + self.bearerToken else: raise Exception("No authentication provided") response = self.requestSession.delete(self.apiAddress + url, headers=headers) @@ -375,11 +376,12 @@ def freeze(self, manifest_path, manifest_output_dir="."): required_files[i]['md5_checksum'] = md5_checksum lock_md5_checksum = md5_for_str(str(manifest_file)) manifest_file['lock_checksum'] = lock_md5_checksum - with open(manifest_output_dir+'/'+'model_manifest.json.freeze', 'w') as f: + with open(manifest_output_dir + '/' + 'model_manifest.json.freeze', 'w') as f: json.dump(manifest_file, f) else: print("Expected to find a model_manifest.json file, none was discovered in working directory") + def isJson(myjson): try: json_object = json.loads(myjson) From 47ec2550756fad4fe126f9f7788267f9db623440 Mon Sep 17 00:00:00 2001 From: James Sutton <1892175+zeryx@users.noreply.github.com> Date: Wed, 22 Dec 2021 15:42:45 -0400 Subject: [PATCH 3/7] removed else exception, no auth is passed by internal algo API processing --- Algorithmia/client.py | 12 ------------ 1 file changed, 12 deletions(-) diff --git a/Algorithmia/client.py b/Algorithmia/client.py index fe01da4..d9f911d 100644 --- a/Algorithmia/client.py +++ b/Algorithmia/client.py @@ -225,8 +225,6 @@ def postJsonHelper(self, url, input_object, parse_response_as_json=True, **query headers['Authorization'] = self.apiKey elif self.bearerToken is not None: headers['Authorization'] = 'Bearer ' + self.bearerToken - else: - raise Exception("No authentication provided") input_json = None if input_object is None: @@ -266,8 +264,6 @@ def getStreamHelper(self, url, **query_parameters): headers['Authorization'] = self.apiKey elif self.bearerToken is not None: headers['Authorization'] = 'Bearer ' + self.bearerToken - else: - raise Exception("No authentication provided") return self.requestSession.get(self.apiAddress + url, headers=headers, params=query_parameters, stream=True) def patchHelper(self, url, params): @@ -276,8 +272,6 @@ def patchHelper(self, url, params): headers['Authorization'] = self.apiKey elif self.bearerToken is not None: headers['Authorization'] = 'Bearer ' + self.bearerToken - else: - raise Exception("No authentication provided") return self.requestSession.patch(self.apiAddress + url, headers=headers, data=json.dumps(params)) # Used internally to get http head result @@ -287,8 +281,6 @@ def headHelper(self, url): headers['Authorization'] = self.apiKey elif self.bearerToken is not None: headers['Authorization'] = 'Bearer ' + self.bearerToken - else: - raise Exception("No authentication provided") return self.requestSession.head(self.apiAddress + url, headers=headers) # Used internally to http put a file @@ -298,8 +290,6 @@ def putHelper(self, url, data): headers['Authorization'] = self.apiKey elif self.bearerToken is not None: headers['Authorization'] = 'Bearer ' + self.bearerToken - else: - raise Exception("No authentication provided") if isJson(data): headers['Content-Type'] = 'application/json' @@ -315,8 +305,6 @@ def deleteHelper(self, url): headers['Authorization'] = self.apiKey elif self.bearerToken is not None: headers['Authorization'] = 'Bearer ' + self.bearerToken - else: - raise Exception("No authentication provided") response = self.requestSession.delete(self.apiAddress + url, headers=headers) if response.reason == "No Content": return response From 536ed0c34fa9dacde97ad829f65c1c319b9e0273 Mon Sep 17 00:00:00 2001 From: James Sutton <1892175+zeryx@users.noreply.github.com> Date: Wed, 22 Dec 2021 16:06:48 -0400 Subject: [PATCH 4/7] added a test case tracking authorization required server responses and system handling no auth provided gracefully --- Algorithmia/client.py | 2 -- Test/api/__init__.py | 3 +++ Test/client_test.py | 20 +++++++++++++++----- 3 files changed, 18 insertions(+), 7 deletions(-) diff --git a/Algorithmia/client.py b/Algorithmia/client.py index d9f911d..82078d0 100644 --- a/Algorithmia/client.py +++ b/Algorithmia/client.py @@ -254,8 +254,6 @@ def getHelper(self, url, **query_parameters): headers['Authorization'] = self.apiKey elif self.bearerToken is not None: headers['Authorization'] = 'Bearer ' + self.bearerToken - else: - raise Exception("No authentication provided") return self.requestSession.get(self.apiAddress + url, headers=headers, params=query_parameters) def getStreamHelper(self, url, **query_parameters): diff --git a/Test/api/__init__.py b/Test/api/__init__.py index 9057bcf..ead17d3 100644 --- a/Test/api/__init__.py +++ b/Test/api/__init__.py @@ -23,6 +23,9 @@ def _start_webserver(): async def process_algo_req(request: Request, username, algoname, output: Optional[str] = None): metadata = {"request_id": "req-55c0480d-6af3-4a21-990a-5c51d29f5725", "duration": 0.000306774} content_type = request.headers['Content-Type'] + auth = request.headers.get('Authorization', None) + if auth is None: + return {"error": {"message": "authorization required"}} request = await request.body() if output and output == "void": return {"async": "abcd123", "request_id": "req-55c0480d-6af3-4a21-990a-5c51d29f5725"} diff --git a/Test/client_test.py b/Test/client_test.py index 0519266..b765fc9 100644 --- a/Test/client_test.py +++ b/Test/client_test.py @@ -9,11 +9,13 @@ import unittest import Algorithmia +from Algorithmia.errors import AlgorithmException from uuid import uuid4 if sys.version_info.major >= 3: unicode = str + class ClientDummyTest(unittest.TestCase): @classmethod def setUpClass(cls): @@ -71,7 +73,6 @@ def test_get_build_logs(self): self.assertTrue(u'error' not in result) - def test_edit_org(self): org_name = "a_myOrg84" @@ -138,7 +139,7 @@ def test_algorithm_programmatic_create_process(self): algorithm_name = "algo_e2d_test" payload = "John" expected_response = "hello John" - full_path = "a_Mrtest/" + algorithm_name + full_path = "a_Mrtest/" + algorithm_name details = { "summary": "Example Summary", "label": "QA", @@ -189,6 +190,17 @@ def test_algorithm_programmatic_create_process(self): response = created_algo.info(git_hash) self.assertEqual(response.version_info.semantic_version, "0.1.0", "information is incorrect") + + def test_no_auth_client(self): + client = Algorithmia.client(api_address="http://localhost:8080") + error = None + try: + client.algo("demo/hello").pipe("world") + except Exception as e: + error = e + finally: + self.assertEqual(str(error), str(AlgorithmException(message="authorization required", stack_trace=None, error_type=None))) + else: class ClientTest(unittest.TestCase): seed(datetime.now().microsecond) @@ -201,7 +213,7 @@ class ClientTest(unittest.TestCase): def setUp(self): self.admin_api_key = unicode(os.environ.get('ALGORITHMIA_A_KEY')) self.regular_api_key = unicode(os.environ.get('ALGORITHMIA_API_KEY')) - + self.admin_username = self.admin_username + str(int(random() * 10000)) self.admin_org_name = self.admin_org_name + str(int(random() * 10000)) self.admin_client = Algorithmia.client(api_address="https://test.algorithmia.com", @@ -400,7 +412,5 @@ def test_algorithm_programmatic_create_process(self): def test_algo_freeze(self): self.regular_client.freeze("Test/resources/manifests/example_manifest.json", "Test/resources/manifests") - - if __name__ == '__main__': unittest.main() From c74987839a2b5e53e1b774957eec6b42b991c3f1 Mon Sep 17 00:00:00 2001 From: James Sutton <1892175+zeryx@users.noreply.github.com> Date: Wed, 22 Dec 2021 16:10:58 -0400 Subject: [PATCH 5/7] added a false flag to actually invalidate environment variables for client auth --- Algorithmia/client.py | 4 +++- Test/client_test.py | 12 +++++++++++- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/Algorithmia/client.py b/Algorithmia/client.py index 82078d0..e65e72b 100644 --- a/Algorithmia/client.py +++ b/Algorithmia/client.py @@ -28,7 +28,9 @@ def __init__(self, apiKey=None, apiAddress=None, caCert=None, bearerToken=None): # Override apiKey with environment variable config = None self.requestSession = requests.Session() - if apiKey is None and 'ALGORITHMIA_API_KEY' in os.environ: + if apiKey is False: + apiKey = None + elif apiKey is None and 'ALGORITHMIA_API_KEY' in os.environ: apiKey = os.environ['ALGORITHMIA_API_KEY'] elif bearerToken is None and 'ALGORITHMIA_BEARER_TOKEN' in os.environ: bearerToken = os.environ['ALGORITHMIA_BEARER_TOKEN'] diff --git a/Test/client_test.py b/Test/client_test.py index b765fc9..42338c4 100644 --- a/Test/client_test.py +++ b/Test/client_test.py @@ -192,7 +192,7 @@ def test_algorithm_programmatic_create_process(self): self.assertEqual(response.version_info.semantic_version, "0.1.0", "information is incorrect") def test_no_auth_client(self): - client = Algorithmia.client(api_address="http://localhost:8080") + client = Algorithmia.client(api_address="http://localhost:8080", api_key=False) error = None try: client.algo("demo/hello").pipe("world") @@ -412,5 +412,15 @@ def test_algorithm_programmatic_create_process(self): def test_algo_freeze(self): self.regular_client.freeze("Test/resources/manifests/example_manifest.json", "Test/resources/manifests") + def test_no_auth_client(self): + client = Algorithmia.client(api_key=False) + error = None + try: + client.algo("demo/hello").pipe("world") + except Exception as e: + error = e + finally: + self.assertEqual(str(error), str(AlgorithmException(message="authorization required", stack_trace=None, error_type=None))) + if __name__ == '__main__': unittest.main() From 3c66afc92565c172c51c81a9ff9f27f3c6207e59 Mon Sep 17 00:00:00 2001 From: James Sutton <1892175+zeryx@users.noreply.github.com> Date: Wed, 22 Dec 2021 17:16:14 -0400 Subject: [PATCH 6/7] cleaned up client, used environment variable manipulation in the test suite --- Algorithmia/client.py | 4 +--- Test/client_test.py | 13 +++++++++++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Algorithmia/client.py b/Algorithmia/client.py index e65e72b..82078d0 100644 --- a/Algorithmia/client.py +++ b/Algorithmia/client.py @@ -28,9 +28,7 @@ def __init__(self, apiKey=None, apiAddress=None, caCert=None, bearerToken=None): # Override apiKey with environment variable config = None self.requestSession = requests.Session() - if apiKey is False: - apiKey = None - elif apiKey is None and 'ALGORITHMIA_API_KEY' in os.environ: + if apiKey is None and 'ALGORITHMIA_API_KEY' in os.environ: apiKey = os.environ['ALGORITHMIA_API_KEY'] elif bearerToken is None and 'ALGORITHMIA_BEARER_TOKEN' in os.environ: bearerToken = os.environ['ALGORITHMIA_BEARER_TOKEN'] diff --git a/Test/client_test.py b/Test/client_test.py index 42338c4..6117439 100644 --- a/Test/client_test.py +++ b/Test/client_test.py @@ -192,13 +192,19 @@ def test_algorithm_programmatic_create_process(self): self.assertEqual(response.version_info.semantic_version, "0.1.0", "information is incorrect") def test_no_auth_client(self): - client = Algorithmia.client(api_address="http://localhost:8080", api_key=False) + + key = os.environ.get('ALGORITHMIA_API_KEY', "") + if key != "": + del os.environ['ALGORITHMIA_API_KEY'] + + client = Algorithmia.client(api_address="http://localhost:8080") error = None try: client.algo("demo/hello").pipe("world") except Exception as e: error = e finally: + os.environ['ALGORITHMIA_API_KEY'] = key self.assertEqual(str(error), str(AlgorithmException(message="authorization required", stack_trace=None, error_type=None))) else: @@ -413,13 +419,16 @@ def test_algo_freeze(self): self.regular_client.freeze("Test/resources/manifests/example_manifest.json", "Test/resources/manifests") def test_no_auth_client(self): - client = Algorithmia.client(api_key=False) + key = os.environ.get('ALGORITHMIA_API_KEY', None) + del os.environ['ALGORITHMIA_API_KEY'] + client = Algorithmia.client() error = None try: client.algo("demo/hello").pipe("world") except Exception as e: error = e finally: + os.environ['ALGORITHMIA_API_KEY'] = key self.assertEqual(str(error), str(AlgorithmException(message="authorization required", stack_trace=None, error_type=None))) if __name__ == '__main__': From e593dbf27b9e474a9dda244e87ce47e3faf6111b Mon Sep 17 00:00:00 2001 From: James Sutton <1892175+zeryx@users.noreply.github.com> Date: Wed, 22 Dec 2021 17:22:15 -0400 Subject: [PATCH 7/7] removing problematic server test --- Test/client_test.py | 13 ------------- 1 file changed, 13 deletions(-) diff --git a/Test/client_test.py b/Test/client_test.py index 6117439..3be87ad 100644 --- a/Test/client_test.py +++ b/Test/client_test.py @@ -418,18 +418,5 @@ def test_algorithm_programmatic_create_process(self): def test_algo_freeze(self): self.regular_client.freeze("Test/resources/manifests/example_manifest.json", "Test/resources/manifests") - def test_no_auth_client(self): - key = os.environ.get('ALGORITHMIA_API_KEY', None) - del os.environ['ALGORITHMIA_API_KEY'] - client = Algorithmia.client() - error = None - try: - client.algo("demo/hello").pipe("world") - except Exception as e: - error = e - finally: - os.environ['ALGORITHMIA_API_KEY'] = key - self.assertEqual(str(error), str(AlgorithmException(message="authorization required", stack_trace=None, error_type=None))) - if __name__ == '__main__': unittest.main()