From 9343356ed4b71c3bd9e3c73e54cfe48f77df7e03 Mon Sep 17 00:00:00 2001 From: Muhammad Sufyan Date: Fri, 8 Dec 2023 18:11:32 +0500 Subject: [PATCH] feat(multiple-auth): improvements in multiple auth implementation (#48) This commit adds improvements in the implementation of multiple authentication schemes. The improvements include the addition of auth-validation exception type and early returning in case of OR authentication group. closes #47 --- README.md | 7 ++ .../authentication/multiple/or_auth_group.py | 12 ++- .../authentication/multiple/single_auth.py | 6 +- apimatic_core/exceptions/__init__.py | 3 +- .../exceptions/anyof_validation_exception.py | 5 + .../exceptions/auth_validation_exception.py | 10 ++ .../exceptions/oneof_validation_exception.py | 5 + apimatic_core/request_builder.py | 3 +- setup.py | 2 +- .../api_logger_tests/test_api_logger.py | 6 +- .../mocks/authentications/basic_auth.py | 2 +- .../mocks/authentications/bearer_auth.py | 2 +- .../test_request_builder.py | 91 +++++++++++-------- 13 files changed, 101 insertions(+), 53 deletions(-) create mode 100644 apimatic_core/exceptions/auth_validation_exception.py diff --git a/README.md b/README.md index 50365de..1d070f6 100644 --- a/README.md +++ b/README.md @@ -46,6 +46,13 @@ pip install apimatic-core |--------------------------------------------------------------|--------------------------------------------------------------------------------------| | [`LazyProperty`](apimatic_core/decorators/lazy_property.py) | A decorator class for lazy instantiation | +## Exceptions +| Name | Description | +|--------------------------------------------------------------------------------------|--------------------------------------------------------------------------| +| [`OneOfValidationException`](apimatic_core/exceptions/oneof_validation_exception.py) | An exception class for the failed validation of oneOf (union-type) cases | +| [`AnyOfValidationException`](apimatic_core/exceptions/anyof_validation_exception.py) | An exception class for the failed validation of anyOf (union-type) cases | +| [`AuthValidationException`](apimatic_core/exceptions/auth_validation_exception.py) | An exception class for the failed validation of authentication schemes | + ## Factories | Name | Description | |---------------------------------------------------------------------------|-----------------------------------------------------------------------------| diff --git a/apimatic_core/authentication/multiple/or_auth_group.py b/apimatic_core/authentication/multiple/or_auth_group.py index 560643d..2074d0a 100644 --- a/apimatic_core/authentication/multiple/or_auth_group.py +++ b/apimatic_core/authentication/multiple/or_auth_group.py @@ -16,10 +16,12 @@ def is_valid(self): return False for participant in self.mapped_group: - if participant.is_valid(): - self._is_valid_group = True - else: - self.error_messages.append(participant.error_message) + self._is_valid_group = participant.is_valid() + # returning as we encounter the first participant as valid + if self._is_valid_group: + return self._is_valid_group - return self.is_valid_group + self.error_messages.append(participant.error_message) + + return self._is_valid_group diff --git a/apimatic_core/authentication/multiple/single_auth.py b/apimatic_core/authentication/multiple/single_auth.py index 13557b8..877a21f 100644 --- a/apimatic_core/authentication/multiple/single_auth.py +++ b/apimatic_core/authentication/multiple/single_auth.py @@ -12,7 +12,7 @@ def __init__(self, auth_participant): self._auth_participant = auth_participant self._mapped_auth = None self._error_message = None - self._is_valid = True + self._is_valid = False def with_auth_managers(self, auth_managers): if not auth_managers.get(self._auth_participant): @@ -23,9 +23,9 @@ def with_auth_managers(self, auth_managers): return self def is_valid(self): - if not self._mapped_auth.is_valid(): + self._is_valid = self._mapped_auth.is_valid() + if not self._is_valid: self._error_message = self._mapped_auth.error_message - self._is_valid = False return self._is_valid diff --git a/apimatic_core/exceptions/__init__.py b/apimatic_core/exceptions/__init__.py index 3a56e42..ea8ef72 100644 --- a/apimatic_core/exceptions/__init__.py +++ b/apimatic_core/exceptions/__init__.py @@ -1,4 +1,5 @@ __all__ = [ 'oneof_validation_exception', - 'anyof_validation_exception' + 'anyof_validation_exception', + 'auth_validation_exception' ] \ No newline at end of file diff --git a/apimatic_core/exceptions/anyof_validation_exception.py b/apimatic_core/exceptions/anyof_validation_exception.py index 3e2583d..ff37b3b 100644 --- a/apimatic_core/exceptions/anyof_validation_exception.py +++ b/apimatic_core/exceptions/anyof_validation_exception.py @@ -1,5 +1,10 @@ +""" +This is an exception class which will be raised when union type validation fails. +""" + class AnyOfValidationException(Exception): + def __init__(self, message): self.message = message super().__init__(self.message) diff --git a/apimatic_core/exceptions/auth_validation_exception.py b/apimatic_core/exceptions/auth_validation_exception.py new file mode 100644 index 0000000..eb74a9c --- /dev/null +++ b/apimatic_core/exceptions/auth_validation_exception.py @@ -0,0 +1,10 @@ +""" +This is an exception class which will be raised when auth validation fails. +""" + + +class AuthValidationException(Exception): + + def __init__(self, message): + self.message = message + super().__init__(self.message) diff --git a/apimatic_core/exceptions/oneof_validation_exception.py b/apimatic_core/exceptions/oneof_validation_exception.py index 3d527d9..9d6bb3a 100644 --- a/apimatic_core/exceptions/oneof_validation_exception.py +++ b/apimatic_core/exceptions/oneof_validation_exception.py @@ -1,5 +1,10 @@ +""" +This is an exception class which will be raised when union type validation fails. +""" + class OneOfValidationException(Exception): + def __init__(self, message): self.message = message super().__init__(self.message) diff --git a/apimatic_core/request_builder.py b/apimatic_core/request_builder.py index 4b8cfe2..7f4bc76 100644 --- a/apimatic_core/request_builder.py +++ b/apimatic_core/request_builder.py @@ -1,3 +1,4 @@ +from apimatic_core.exceptions.auth_validation_exception import AuthValidationException from apimatic_core.http.request.http_request import HttpRequest from apimatic_core.types.array_serialization_format import SerializationFormats from apimatic_core.utilities.api_helper import ApiHelper @@ -229,4 +230,4 @@ def apply_auth(self, auth_managers, http_request): if self._auth.with_auth_managers(auth_managers).is_valid(): self._auth.apply(http_request) else: - raise PermissionError(self._auth.error_message) + raise AuthValidationException(self._auth.error_message) diff --git a/setup.py b/setup.py index 7d34a71..bb0c59c 100644 --- a/setup.py +++ b/setup.py @@ -12,7 +12,7 @@ setup( name='apimatic-core', - version='0.2.5', + version='0.2.6', description='A library that contains core logic and utilities for ' 'consuming REST APIs using Python SDKs generated by APIMatic.', long_description=long_description, diff --git a/tests/apimatic_core/api_logger_tests/test_api_logger.py b/tests/apimatic_core/api_logger_tests/test_api_logger.py index 399baee..ae5cb24 100644 --- a/tests/apimatic_core/api_logger_tests/test_api_logger.py +++ b/tests/apimatic_core/api_logger_tests/test_api_logger.py @@ -30,15 +30,15 @@ def test_end_to_end_success_case(self): 'Calling the on_before_request method of http_call_back for end-to-end-test.', 'Raw request for end-to-end-test is: {\'http_method\': \'POST\', \'query_url\': ' '\'http://localhost:3000/body/model\', \'headers\': {\'Content-Type\': ' - '\'application/json\', \'accept\': \'application/json\', \'Authorization\': ' + '\'application/json\', \'accept\': \'application/json\', \'Basic-Authorization\': ' '\'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk\'}, \'query_parameters\': None, ' '\'parameters\': \'{"Key": "Value"}\', \'files\': {}}', 'Raw response for end-to-end-test is: {\'status_code\': 200, \'reason_phrase\': None, ' '\'headers\': {\'Content-Type\': \'application/json\', \'accept\': \'application/json\', ' - '\'Authorization\': \'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk\'}, \'text\': ' + '\'Basic-Authorization\': \'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk\'}, \'text\': ' '\'{"Key": "Value"}\', \'request\': {\'http_method\': \'POST\', \'query_url\': ' '\'http://localhost:3000/body/model\', \'headers\': {\'Content-Type\': ' - '\'application/json\', \'accept\': \'application/json\', \'Authorization\': ' + '\'application/json\', \'accept\': \'application/json\', \'Basic-Authorization\': ' '\'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk\'}, \'query_parameters\': None, ' '\'parameters\': \'{"Key": "Value"}\', \'files\': {}}}', 'Calling on_after_response method of http_call_back for end-to-end-test.', diff --git a/tests/apimatic_core/mocks/authentications/basic_auth.py b/tests/apimatic_core/mocks/authentications/basic_auth.py index 8a14823..764c3ab 100644 --- a/tests/apimatic_core/mocks/authentications/basic_auth.py +++ b/tests/apimatic_core/mocks/authentications/basic_auth.py @@ -12,7 +12,7 @@ def error_message(self): def __init__(self, basic_auth_user_name, basic_auth_password): auth_params = {} if basic_auth_user_name and basic_auth_password: - auth_params = {'Authorization': "Basic {}".format( + auth_params = {'Basic-Authorization': "Basic {}".format( AuthHelper.get_base64_encoded_value(basic_auth_user_name, basic_auth_password))} super().__init__(auth_params=auth_params) self._basic_auth_user_name = basic_auth_user_name diff --git a/tests/apimatic_core/mocks/authentications/bearer_auth.py b/tests/apimatic_core/mocks/authentications/bearer_auth.py index e244abf..6a37b29 100644 --- a/tests/apimatic_core/mocks/authentications/bearer_auth.py +++ b/tests/apimatic_core/mocks/authentications/bearer_auth.py @@ -11,6 +11,6 @@ def error_message(self): def __init__(self, access_token): auth_params = {} if access_token: - auth_params = {'Authorization': "Bearer {}".format(access_token)} + auth_params = {'Bearer-Authorization': "Bearer {}".format(access_token)} super().__init__(auth_params=auth_params) self._access_token = access_token diff --git a/tests/apimatic_core/request_builder_tests/test_request_builder.py b/tests/apimatic_core/request_builder_tests/test_request_builder.py index 9ec6d4f..7f84b20 100644 --- a/tests/apimatic_core/request_builder_tests/test_request_builder.py +++ b/tests/apimatic_core/request_builder_tests/test_request_builder.py @@ -1,6 +1,8 @@ from datetime import datetime, date import pytest import sys + +from apimatic_core.exceptions.auth_validation_exception import AuthValidationException from apimatic_core_interfaces.types.http_method_enum import HttpMethodEnum from apimatic_core.authentication.multiple.and_auth_group import And from apimatic_core.authentication.multiple.or_auth_group import Or @@ -589,9 +591,9 @@ def test_multipart_request_with_file_wrapper(self, input_multipart_param_value1, expected_multipart_param_value1.close() @pytest.mark.parametrize('input_auth_scheme, expected_auth_header_key, expected_auth_header_value', [ - (Single('basic_auth'), 'Authorization', 'Basic {}'.format( + (Single('basic_auth'), 'Basic-Authorization', 'Basic {}'.format( AuthHelper.get_base64_encoded_value('test_username', 'test_password'))), - (Single('bearer_auth'), 'Authorization', 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42'), + (Single('bearer_auth'), 'Bearer-Authorization', 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42'), (Single('custom_header_auth'), 'token', 'Qaws2W233WedeRe4T56G6Vref2') ]) def test_header_authentication(self, input_auth_scheme, expected_auth_header_key, expected_auth_header_value): @@ -620,44 +622,59 @@ def test_invalid_key_authentication(self, input_invalid_auth_scheme): .build(self.global_configuration_with_auth) assert validation_error.value.args[0] == 'Auth key is invalid.' - @pytest.mark.parametrize('input_auth_scheme, expected_auth_header_key_1,' - 'expected_auth_header_value_1, expected_auth_header_key_2,' - 'expected_auth_header_value_2', [ - (Or('basic_auth', 'bearer_auth', 'custom_header_auth'), 'Authorization', - 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42', 'token', - 'Qaws2W233WedeRe4T56G6Vref2'), - (And('basic_auth', 'bearer_auth', 'custom_header_auth'), 'Authorization', - 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42', 'token', - 'Qaws2W233WedeRe4T56G6Vref2'), - (Or('basic_auth', And('bearer_auth', 'custom_header_auth')), 'Authorization', - 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42', 'token', - 'Qaws2W233WedeRe4T56G6Vref2'), - (And('basic_auth', Or('bearer_auth', 'custom_header_auth')), 'Authorization', - 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42', 'token', - 'Qaws2W233WedeRe4T56G6Vref2'), - (And('basic_auth', And('bearer_auth', 'custom_header_auth')), 'Authorization', - 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42', 'token', - 'Qaws2W233WedeRe4T56G6Vref2'), - (Or('basic_auth', Or('bearer_auth', 'custom_header_auth')), 'Authorization', - 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42', 'token', - 'Qaws2W233WedeRe4T56G6Vref2'), - (Or('basic_auth', Or(None, 'custom_header_auth')), 'Authorization', - 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', 'token', - 'Qaws2W233WedeRe4T56G6Vref2'), - (Or('basic_auth', And(None, 'custom_header_auth')), 'Authorization', - 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', 'token', - 'Qaws2W233WedeRe4T56G6Vref2'), + @pytest.mark.parametrize('input_auth_scheme, expected_request_headers', [ + (Or('basic_auth', 'custom_header_auth'), + { + 'Basic-Authorization': 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', + 'token': None + }), + (And('basic_auth', 'bearer_auth', 'custom_header_auth'), + { + 'Basic-Authorization': 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', + 'Bearer-Authorization': 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42', + 'token': 'Qaws2W233WedeRe4T56G6Vref2' + }), + (Or('basic_auth', And('bearer_auth', 'custom_header_auth')), + { + 'Basic-Authorization': 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', + 'Bearer-Authorization': None, + 'token': None + }), + (And('basic_auth', Or('bearer_auth', 'custom_header_auth')), + { + 'Basic-Authorization': 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', + 'Bearer-Authorization': 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42', + 'token': None + }), + (And('basic_auth', And('bearer_auth', 'custom_header_auth')), + { + 'Basic-Authorization': 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', + 'Bearer-Authorization': 'Bearer 0b79bab50daca910b000d4f1a2b675d604257e42', + 'token': 'Qaws2W233WedeRe4T56G6Vref2' + }), + (Or('basic_auth', Or('bearer_auth', 'custom_header_auth')), + { + 'Basic-Authorization': 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', + 'Bearer-Authorization': None, + 'token': None + }), + (Or('basic_auth', Or(None, 'custom_header_auth')), + { + 'Basic-Authorization': 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', + 'token': None + }), + (Or('basic_auth', And(None, 'custom_header_auth')), + { + 'Basic-Authorization': 'Basic dGVzdF91c2VybmFtZTp0ZXN0X3Bhc3N3b3Jk', + 'token': None + }), ]) - def test_success_case_of_multiple_authentications(self, input_auth_scheme, expected_auth_header_key_1, - expected_auth_header_value_1, - expected_auth_header_key_2, - expected_auth_header_value_2): + def test_success_case_of_multiple_authentications(self, input_auth_scheme, expected_request_headers): http_request = self.new_request_builder \ .auth(input_auth_scheme) \ .build(self.global_configuration_with_auth) - - assert http_request.headers[expected_auth_header_key_1] == expected_auth_header_value_1 \ - and http_request.headers[expected_auth_header_key_2] == expected_auth_header_value_2 + for key in expected_request_headers.keys(): + assert http_request.headers.get(key) == expected_request_headers.get(key) @pytest.mark.parametrize('input_auth_scheme, expected_error_message', [ (Or('basic_auth', 'bearer_auth', 'custom_header_auth'), '[BasicAuth: _basic_auth_user_name or ' @@ -693,7 +710,7 @@ def test_success_case_of_multiple_authentications(self, input_auth_scheme, expec (And(None, 'basic_auth'), '[BasicAuth: _basic_auth_user_name or _basic_auth_password is undefined.]') ]) def test_failed_case_of_multiple_authentications(self, input_auth_scheme, expected_error_message): - with pytest.raises(PermissionError) as errors: + with pytest.raises(AuthValidationException) as errors: self.new_request_builder \ .auth(input_auth_scheme) \ .build(self.global_configuration_with_uninitialized_auth_params)