From 509397f816dc4695fe27f9ca336de66707f6ceee Mon Sep 17 00:00:00 2001 From: djkhl <49399649+djkhl@users.noreply.github.com> Date: Wed, 24 Apr 2024 15:16:09 +0200 Subject: [PATCH] Fix AttributeError due to missing key in credentials file (#574) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add verify credentials on logprep startup --------- Co-authored-by: Jörg Zimmermann <101292599+ekneg54@users.noreply.github.com> --- CHANGELOG.md | 2 + logprep/util/configuration.py | 13 +++- logprep/util/credentials.py | 37 +++++++--- tests/unit/connector/test_http_input.py | 5 +- tests/unit/util/test_configuration.py | 19 +++++ tests/unit/util/test_credentials.py | 95 ++++++++++++++++++++----- tests/unit/util/test_getter.py | 27 ++++--- 7 files changed, 159 insertions(+), 39 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 967be2736..7425b761a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ ### Bugfix + * fixes bug where missing key in credentials file leads to AttributeError + ## 11.1.0 ### Features diff --git a/logprep/util/configuration.py b/logprep/util/configuration.py index 37f8173bb..b3a2288be 100644 --- a/logprep/util/configuration.py +++ b/logprep/util/configuration.py @@ -219,8 +219,11 @@ from logprep.factory_error import FactoryError, InvalidConfigurationError from logprep.processor.base.exceptions import InvalidRuleDefinitionError from logprep.util import getter -from logprep.util.credentials import CredentialsEnvNotFoundError -from logprep.util.defaults import DEFAULT_CONFIG_LOCATION +from logprep.util.credentials import CredentialsEnvNotFoundError, CredentialsFactory +from logprep.util.defaults import ( + DEFAULT_CONFIG_LOCATION, + ENV_NAME_LOGPREP_CREDENTIALS_FILE, +) from logprep.util.getter import GetterFactory, GetterNotFoundError from logprep.util.json_handling import list_json_files_in_directory @@ -660,6 +663,12 @@ def _verify(self): self._verify_processor_outputs(processor_config) except Exception as error: # pylint: disable=broad-except errors.append(error) + if ENV_NAME_LOGPREP_CREDENTIALS_FILE in os.environ: + try: + credentials_file_path = os.environ.get(ENV_NAME_LOGPREP_CREDENTIALS_FILE) + _ = CredentialsFactory.get_content(Path(credentials_file_path)) + except Exception as error: # pylint: disable=broad-except + errors.append(error) if errors: raise InvalidConfigurationErrors(errors) diff --git a/logprep/util/credentials.py b/logprep/util/credentials.py index 86948a7c7..099361dcd 100644 --- a/logprep/util/credentials.py +++ b/logprep/util/credentials.py @@ -113,6 +113,7 @@ from urllib3 import Retry from logprep.factory_error import InvalidConfigurationError +from logprep.util.defaults import ENV_NAME_LOGPREP_CREDENTIALS_FILE yaml = YAML(typ="safe", pure=True) @@ -148,14 +149,13 @@ def from_target(cls, target_url: str) -> "Credentials": Credentials object representing the correct authorization method """ - credentials_file_path = os.environ.get("LOGPREP_CREDENTIALS_FILE") + credentials_file_path = os.environ.get(ENV_NAME_LOGPREP_CREDENTIALS_FILE) if credentials_file_path is None: return None - raw_content: dict = cls._get_content(Path(credentials_file_path)) + credentials_file: CredentialsFileSchema = cls.get_content(Path(credentials_file_path)) domain = urlparse(target_url).netloc scheme = urlparse(target_url).scheme - getter_credentials = raw_content.get("getter") - credential_mapping = getter_credentials.get(f"{scheme}://{domain}") + credential_mapping = credentials_file.getter.get(f"{scheme}://{domain}") credentials = cls.from_dict(credential_mapping) return credentials @@ -177,17 +177,17 @@ def from_endpoint(cls, target_endpoint: str) -> "Credentials": Credentials object representing the correct authorization method """ - credentials_file_path = os.environ.get("LOGPREP_CREDENTIALS_FILE") + credentials_file_path = os.environ.get(ENV_NAME_LOGPREP_CREDENTIALS_FILE) if credentials_file_path is None: return None - raw_content: dict = cls._get_content(Path(credentials_file_path)) - endpoint_credentials = raw_content.get("input").get("endpoints") + credentials_file: CredentialsFileSchema = cls.get_content(Path(credentials_file_path)) + endpoint_credentials = credentials_file.input.get("endpoints") credential_mapping = endpoint_credentials.get(target_endpoint) credentials = cls.from_dict(credential_mapping) return credentials @staticmethod - def _get_content(file_path: Path) -> dict: + def get_content(file_path: Path) -> dict: """gets content from credentials file file can be either json or yaml @@ -210,9 +210,9 @@ def _get_content(file_path: Path) -> dict: try: file_content = file_path.read_text(encoding="utf-8") try: - return json.loads(file_content) + return CredentialsFileSchema(**json.loads(file_content)) except (json.JSONDecodeError, ValueError): - return yaml.load(file_content) + return CredentialsFileSchema(**yaml.load(file_content)) except (TypeError, YAMLError) as error: raise InvalidConfigurationError( f"Invalid credentials file: {file_path} {error.args[0]}" @@ -443,6 +443,23 @@ def _handle_bad_requests_errors(self, response): raise +@define(kw_only=True) +class CredentialsFileSchema: + """class for credentials file""" + + input: dict = field( + validator=[ + validators.instance_of(dict), + validators.deep_mapping( + key_validator=validators.in_(["endpoints"]), + value_validator=validators.instance_of(dict), + ), + ], + default={"endpoints": {}}, + ) + getter: dict = field(validator=validators.instance_of(dict), default={}) + + @define(kw_only=True) class BasicAuthCredentials(Credentials): """Basic Authentication Credentials diff --git a/tests/unit/connector/test_http_input.py b/tests/unit/connector/test_http_input.py index 27c6f7bc1..c39fbdaec 100644 --- a/tests/unit/connector/test_http_input.py +++ b/tests/unit/connector/test_http_input.py @@ -14,6 +14,7 @@ from logprep.abc.input import FatalInputError from logprep.connector.http.input import HttpConnector from logprep.factory import Factory +from logprep.util.defaults import ENV_NAME_LOGPREP_CREDENTIALS_FILE from tests.unit.connector.base import BaseInputTestCase @@ -316,7 +317,7 @@ def test_get_next_with_hmac_of_raw_message(self): @pytest.mark.parametrize("endpoint", ["/auth-json-secret", "/.*/[A-Z]{2}/json$"]) def test_endpoint_has_credentials(self, endpoint, credentials_file_path): - mock_env = {"LOGPREP_CREDENTIALS_FILE": credentials_file_path} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: credentials_file_path} with mock.patch.dict("os.environ", mock_env): new_connector = Factory.create({"test connector": self.CONFIG}, logger=self.logger) new_connector.pipeline_index = 1 @@ -326,7 +327,7 @@ def test_endpoint_has_credentials(self, endpoint, credentials_file_path): assert endpoint_config.credentials.password, endpoint def test_endpoint_has_basic_auth(self, credentials_file_path): - mock_env = {"LOGPREP_CREDENTIALS_FILE": credentials_file_path} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: credentials_file_path} with mock.patch.dict("os.environ", mock_env): new_connector = Factory.create({"test connector": self.CONFIG}, logger=self.logger) new_connector.pipeline_index = 1 diff --git a/tests/unit/util/test_configuration.py b/tests/unit/util/test_configuration.py index 6fc8feee6..56f25ba38 100644 --- a/tests/unit/util/test_configuration.py +++ b/tests/unit/util/test_configuration.py @@ -19,6 +19,7 @@ InvalidConfigurationErrors, MetricsConfig, ) +from logprep.util.defaults import ENV_NAME_LOGPREP_CREDENTIALS_FILE from logprep.util.getter import FileGetter, GetterNotFoundError from tests.testdata.metadata import ( path_to_config, @@ -1162,6 +1163,24 @@ def test_versions_are_aggregated_for_multiple_configs_with_second_unset(self, tm config = Configuration.from_sources([str(config1_path), str(config2_path)]) assert config.version == "first, unset" + def test_verify_credentials_file_raises_for_unexpected_key(self, config_path, tmp_path): + credential_file_path = tmp_path / "credentials.yml" + credential_file_path.write_text( + """--- +endpoints: + /some/auth/endpoint: + username: test_user + password: myverysecretpassword +""" + ) + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credential_file_path)} + with mock.patch.dict("os.environ", mock_env): + with pytest.raises( + InvalidConfigurationError, + match="Invalid credentials file.* unexpected keyword argument", + ): + _ = Configuration.from_sources([str(config_path)]) + class TestInvalidConfigurationErrors: @pytest.mark.parametrize( diff --git a/tests/unit/util/test_credentials.py b/tests/unit/util/test_credentials.py index a7cb44ac4..f2ab3b3d4 100644 --- a/tests/unit/util/test_credentials.py +++ b/tests/unit/util/test_credentials.py @@ -17,11 +17,13 @@ Credentials, CredentialsBadRequestError, CredentialsFactory, + CredentialsFileSchema, MTLSCredentials, OAuth2ClientFlowCredentials, OAuth2PasswordFlowCredentials, OAuth2TokenCredentials, ) +from logprep.util.defaults import ENV_NAME_LOGPREP_CREDENTIALS_FILE class TestBasicAuthCredentials: @@ -105,10 +107,10 @@ class TestOAuth2TokenCredentials: ) def test_init(self, testcase, kwargs, error, error_message): if error is None: - test = OAuth2TokenCredentials(**kwargs) + _ = OAuth2TokenCredentials(**kwargs) else: with pytest.raises(error, match=error_message): - test = OAuth2TokenCredentials(**kwargs) + _ = OAuth2TokenCredentials(**kwargs) def test_get_session_returns_session(self): test = OAuth2TokenCredentials(token="tooooooken") @@ -209,10 +211,10 @@ class TestOAuth2PasswordFlowCredentials: ) def test_init(self, testcase, error, kwargs, error_message): if error is None: - test = OAuth2PasswordFlowCredentials(**kwargs) + _ = OAuth2PasswordFlowCredentials(**kwargs) else: with pytest.raises(error, match=error_message): - test = OAuth2PasswordFlowCredentials(**kwargs) + _ = OAuth2PasswordFlowCredentials(**kwargs) @responses.activate def test_get_session_returns_session(self): @@ -486,10 +488,10 @@ class TestOAuth2ClientFlowCredentials: ) def test_init(self, testcase, kwargs, error, error_message): if error is None: - test = OAuth2ClientFlowCredentials(**kwargs) + _ = OAuth2ClientFlowCredentials(**kwargs) else: with pytest.raises(error, match=error_message): - test = OAuth2ClientFlowCredentials(**kwargs) + _ = OAuth2ClientFlowCredentials(**kwargs) @responses.activate def test_get_session_returns_session(self): @@ -799,7 +801,7 @@ class TestCredentialsFactory: InvalidConfigurationError, ), ( - "Return OAuth2PassowordFlowCredentials object with additional client_id in credentials file", + "Return OAuth2PassowordFlowCredentials object with extra client_id", """--- getter: "https://some.url": @@ -908,7 +910,7 @@ class TestCredentialsFactory: None, ), ( - "Return MTLSCredentials object if certificate key and ca cert are given with extra params", + "Return MTLSCredentials object if cert key and ca cert are given with extra params", """--- getter: "https://some.url": @@ -920,7 +922,7 @@ class TestCredentialsFactory: None, ), ( - "Return MTLSCredentials object if certificate and key are given with extra parameters", + "Return MTLSCredentials object if cert and key are given with extra parameters", """--- getter: "https://some.url": @@ -960,7 +962,7 @@ def test_getter_credentials_returns_expected_credential_object( ): credential_file_path = tmp_path / "credentials" credential_file_path.write_text(credential_file_content) - mock_env = {"LOGPREP_CREDENTIALS_FILE": str(credential_file_path)} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credential_file_path)} with mock.patch.dict("os.environ", mock_env): if error is not None: with pytest.raises(error): @@ -980,7 +982,7 @@ def test_input_credentials_returns_expected_credentials_object(self, tmp_path): password: myverysecretpassword """ ) - mock_env = {"LOGPREP_CREDENTIALS_FILE": str(credential_file_path)} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credential_file_path)} with mock.patch.dict("os.environ", mock_env): creds = CredentialsFactory.from_endpoint("/some/auth/endpoint") assert isinstance(creds, BasicAuthCredentials) @@ -1001,13 +1003,13 @@ def test_credentials_from_root_url(self, tmp_path): client_secret: test """ ) - mock_env = {"LOGPREP_CREDENTIALS_FILE": str(credential_file_path)} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credential_file_path)} with mock.patch.dict("os.environ", mock_env): creds = CredentialsFactory.from_target("http://some.url") assert isinstance(creds, OAuth2ClientFlowCredentials) def test_credentials_is_none_on_invalid_credentials_file_path(self): - mock_env = {"LOGPREP_CREDENTIALS_FILE": "this is something useless"} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: "this is something useless"} with mock.patch.dict("os.environ", mock_env): with pytest.raises(InvalidConfigurationError, match=r"wrong credentials file path"): creds = CredentialsFactory.from_target("https://some.url") @@ -1038,7 +1040,7 @@ def test_credentials_is_none_on_invalid_credentials_file_path(self): OAuth2TokenCredentials, ), ( - "Return BasicAuthCredential object when no endpoint is given and password_file is given", + "Return BasicAuthCredential object with no endpoint and password_file", "password_file", "", "hiansdnjskwuthisisaverysecretsecret", @@ -1062,7 +1064,7 @@ def test_credentials_reads_secret_file_content( """ ) secret_file_path.write_text(secret_content) - mock_env = {"LOGPREP_CREDENTIALS_FILE": str(credential_file_path)} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credential_file_path)} with mock.patch.dict("os.environ", mock_env): creds = CredentialsFactory.from_target("http://some.url/configuration") assert isinstance(creds, instance), testcase @@ -1086,7 +1088,7 @@ def test_credentials_reads_secret_file_content_from_every_given_file(self, tmp_p secret_file_path_0.write_text("thisismysecretsecretclientsecret") secret_file_path_1.write_text("thisismysecorndsecretsecretpasswordsecret") - mock_env = {"LOGPREP_CREDENTIALS_FILE": str(credential_file_path)} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credential_file_path)} with mock.patch.dict("os.environ", mock_env): creds = CredentialsFactory.from_target("http://some.url/configuration") assert isinstance(creds, Credentials) @@ -1101,13 +1103,50 @@ def test_warning_logged_when_extra_params_given(self, mock_logger): "password": "password", "extra_param": "extra", } - creds = CredentialsFactory.from_dict(credentials_file_content_with_extra_params) + _ = CredentialsFactory.from_dict(credentials_file_content_with_extra_params) mock_logger.warning.assert_called_once() assert re.search( r"OAuth password authorization for confidential clients", mock_logger.mock_calls[0][1][0], ) + def test_from_target_raises_when_getter_key_not_set(self, tmp_path): + credential_file_path = tmp_path / "credentials.yml" + credential_file_path.write_text( + """--- + "http://some.url": + endpoint: "https://endpoint.end" + username: testuser + password_file: "thisismysecretsecretclientsecret" +""" + ) + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credential_file_path)} + with mock.patch.dict("os.environ", mock_env): + with pytest.raises( + InvalidConfigurationError, + match="Invalid credentials file.* unexpected keyword argument 'http://some.url'", + ): + creds = CredentialsFactory.from_target("http://some.url/configuration") + assert isinstance(creds, InvalidConfigurationError) + + def test_from_endpoint_raises_when_input_key_not_set(self, tmp_path): + credential_file_path = tmp_path / "credentials.yml" + credential_file_path.write_text( + """--- + /some/endpoint: + username: testuser + password_file: "thisismysecretsecretclientsecret" +""" + ) + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credential_file_path)} + with mock.patch.dict("os.environ", mock_env): + with pytest.raises( + InvalidConfigurationError, + match="Invalid credentials file.* unexpected keyword argument '/some/endpoint'", + ): + creds = CredentialsFactory.from_endpoint("/some/endpoint") + assert isinstance(creds, InvalidConfigurationError) + class TestMTLSCredentials: def test_get_session_returns_session_and_cert_is_set(self): @@ -1128,3 +1167,25 @@ def test_get_session_sets_ca_cert_for_verification(self): assert "path/to/cert" in test._session.cert assert "path/to/key" in test._session.cert assert "path/to/ca/cert" in test._session.verify + + +class TestCredentialsFileSchema: + def test_credential_file_can_be_instanciated(self): + credentials_file_content = { + "input": { + "endpoints": { + "some/endpoint": { + "username": "user1", + "password": "password", + } + } + }, + "getter": { + "some/endpoint": { + "username": "user1", + "password": "password", + } + }, + } + creds = CredentialsFileSchema(**credentials_file_content) + assert isinstance(creds, CredentialsFileSchema) diff --git a/tests/unit/util/test_getter.py b/tests/unit/util/test_getter.py index 9ac642187..6badf6ac5 100644 --- a/tests/unit/util/test_getter.py +++ b/tests/unit/util/test_getter.py @@ -17,6 +17,7 @@ from ruamel.yaml import YAML from logprep.util.credentials import Credentials, CredentialsEnvNotFoundError +from logprep.util.defaults import ENV_NAME_LOGPREP_CREDENTIALS_FILE from logprep.util.getter import ( FileGetter, GetterFactory, @@ -406,7 +407,9 @@ def test_credentials_returns_credentials_if_set(self, tmp_path): } credentials_file: Path = tmp_path / "credentials.json" credentials_file.write_text(json.dumps(credentials_file_content)) - with mock.patch.dict("os.environ", {"LOGPREP_CREDENTIALS_FILE": str(credentials_file)}): + with mock.patch.dict( + "os.environ", {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credentials_file)} + ): http_getter = GetterFactory.from_string("https://does-not-matter/bar") assert isinstance(http_getter.credentials, Credentials) @@ -439,7 +442,9 @@ def test_get_raw_gets_token_before_request(self, tmp_path): } credentials_file: Path = tmp_path / "credentials.json" credentials_file.write_text(json.dumps(credentials_file_content)) - with mock.patch.dict("os.environ", {"LOGPREP_CREDENTIALS_FILE": str(credentials_file)}): + with mock.patch.dict( + "os.environ", {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credentials_file)} + ): http_getter = GetterFactory.from_string(f"https://{domain}/bar") return_content = http_getter.get_json() assert return_content == {"key": "the cooooontent"} @@ -475,7 +480,9 @@ def test_get_raw_reuses_existing_session(self, tmp_path): } credentials_file: Path = tmp_path / "credentials.json" credentials_file.write_text(json.dumps(credentials_file_content)) - with mock.patch.dict("os.environ", {"LOGPREP_CREDENTIALS_FILE": str(credentials_file)}): + with mock.patch.dict( + "os.environ", {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credentials_file)} + ): http_getter = GetterFactory.from_string(f"https://{domain}/bar") return_content = http_getter.get_json() return_content = http_getter.get_json() @@ -512,7 +519,9 @@ def test_get_raw_refreshes_token_if_expired(self, tmp_path): } credentials_file: Path = tmp_path / "credentials.json" credentials_file.write_text(json.dumps(credentials_file_content)) - with mock.patch.dict("os.environ", {"LOGPREP_CREDENTIALS_FILE": str(credentials_file)}): + with mock.patch.dict( + "os.environ", {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credentials_file)} + ): http_getter: HttpGetter = GetterFactory.from_string(f"https://{domain}/bar") return_content = http_getter.get_json() assert return_content == {"key": "the cooooontent"} @@ -548,7 +557,9 @@ def test_get_raw_raises_if_credential_file_env_set_and_unauthorizes(self): http_getter: HttpGetter = GetterFactory.from_string(f"https://{domain}/bar") with mock.patch.dict( "os.environ", - {"LOGPREP_CREDENTIALS_FILE": "quickstart/exampledata/config/credentials.yml"}, + { + ENV_NAME_LOGPREP_CREDENTIALS_FILE: "quickstart/exampledata/config/credentials.yml" + }, ): http_getter.get_json() assert error.value.response.status_code == 401 @@ -576,7 +587,7 @@ def test_get_raw_uses_mtls_and_session_cert_is_set_and_used_in_request(self, tmp } credentials_file: Path = tmp_path / "credentials.json" credentials_file.write_text(json.dumps(credentials_file_content)) - mock_env = {"LOGPREP_CREDENTIALS_FILE": str(credentials_file)} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credentials_file)} with mock.patch.dict("os.environ", mock_env): http_getter: HttpGetter = GetterFactory.from_string(f"https://{domain}/bar") http_getter.get_raw() @@ -609,7 +620,7 @@ def test_get_raw_uses_mtls_with_session_cert_and_ca_cert(self, tmp_path): } credentials_file: Path = tmp_path / "credentials.json" credentials_file.write_text(json.dumps(credentials_file_content)) - mock_env = {"LOGPREP_CREDENTIALS_FILE": str(credentials_file)} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credentials_file)} with mock.patch.dict("os.environ", mock_env): http_getter: HttpGetter = GetterFactory.from_string(f"https://{domain}/bar") http_getter.get_raw() @@ -649,7 +660,7 @@ def test_get_raw_reuses_mtls_session_and_ca_cert_is_not_updated(self, tmp_path): } credentials_file: Path = tmp_path / "credentials.json" credentials_file.write_text(json.dumps(credentials_file_content)) - mock_env = {"LOGPREP_CREDENTIALS_FILE": str(credentials_file)} + mock_env = {ENV_NAME_LOGPREP_CREDENTIALS_FILE: str(credentials_file)} with mock.patch.dict("os.environ", mock_env): http_getter: HttpGetter = GetterFactory.from_string(f"https://{domain}/bar") http_getter.get_raw()