Skip to content

Commit

Permalink
Fix AttributeError due to missing key in credentials file (#574)
Browse files Browse the repository at this point in the history
* add verify credentials on logprep startup

---------

Co-authored-by: Jörg Zimmermann <101292599+ekneg54@users.noreply.github.com>
  • Loading branch information
djkhl and ekneg54 authored Apr 24, 2024
1 parent d93cd99 commit 509397f
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 39 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

### Bugfix

* fixes bug where missing key in credentials file leads to AttributeError

## 11.1.0

### Features
Expand Down
13 changes: 11 additions & 2 deletions logprep/util/configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)

Expand Down
37 changes: 27 additions & 10 deletions logprep/util/credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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

Expand All @@ -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
Expand All @@ -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]}"
Expand Down Expand Up @@ -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
Expand Down
5 changes: 3 additions & 2 deletions tests/unit/connector/test_http_input.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
19 changes: 19 additions & 0 deletions tests/unit/util/test_configuration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down
95 changes: 78 additions & 17 deletions tests/unit/util/test_credentials.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
Credentials,
CredentialsBadRequestError,
CredentialsFactory,
CredentialsFileSchema,
MTLSCredentials,
OAuth2ClientFlowCredentials,
OAuth2PasswordFlowCredentials,
OAuth2TokenCredentials,
)
from logprep.util.defaults import ENV_NAME_LOGPREP_CREDENTIALS_FILE


class TestBasicAuthCredentials:
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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":
Expand Down Expand Up @@ -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":
Expand All @@ -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":
Expand Down Expand Up @@ -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):
Expand All @@ -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)
Expand All @@ -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")
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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):
Expand All @@ -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)
Loading

0 comments on commit 509397f

Please sign in to comment.