From b347993628fdb2f157cebf158d3a8d219c9e1530 Mon Sep 17 00:00:00 2001 From: thedoubl3j Date: Mon, 14 Oct 2024 15:46:21 -0400 Subject: [PATCH 1/6] Adjust what is returned in the aim URL 404 --- pyproject.toml | 2 +- src/awx_plugins/credentials/aim.py | 10 ++++++++++ tests/credential_plugins_test.py | 11 ++++++++++- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index bdbdf64e1b..06ba4d5588 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ dependencies = [ # runtime deps # https://packaging.python.org/en/latest/guide # GUIDANCE: only add things that this project imports directly # GUIDANCE: only set lower version bounds # "awx_plugins.base_interface.api", # keep `__init__.py` empty - "awx_plugins.interfaces", # standard interface declarations for AWX plugins + "awx_plugins.interfaces @git+https://github.com/ansible/awx_plugins.interfaces.git@ad4a965", # standard interface declarations for AWX plugins "PyYAML", # credentials.injectors, inventory.plugins "azure-identity", # credentials.azure_kv "azure-keyvault", # credentials.azure_kv diff --git a/src/awx_plugins/credentials/aim.py b/src/awx_plugins/credentials/aim.py index f535e02194..5ff68f5b9e 100644 --- a/src/awx_plugins/credentials/aim.py +++ b/src/awx_plugins/credentials/aim.py @@ -111,6 +111,16 @@ def aim_backend(**kwargs): verify=verify, allow_redirects=False, ) + sensitive_query_params = { + 'AppId' : "****", + 'Query' : "****", + 'QueryFormat' : object_query_format, + } + if reason: + sensitive_query_params['reason'] = "****" + sensitive_request_qs = '?' + urlencode(sensitive_query_params, safe="*", quote_via=quote) + res.url = request_url + sensitive_request_qs + raise_for_status(res) # CCP returns the property name capitalized, username is camel case # so we need to handle that case diff --git a/tests/credential_plugins_test.py b/tests/credential_plugins_test.py index 2c2f3eae67..2ef05a43d8 100644 --- a/tests/credential_plugins_test.py +++ b/tests/credential_plugins_test.py @@ -6,7 +6,7 @@ import pytest from awx_plugins.credentials import hashivault - +from awx_plugins.credentials import aim def test_imported_azure_cloud_sdk_vars() -> None: from awx_plugins.credentials import azure_kv @@ -162,3 +162,12 @@ def test_tss_import(self) -> None: ): # assert this module as opposed to older thycotic.secrets.server assert cls.__module__ == 'delinea.secrets.server' + +def test_aim_sensitive_traceback(): + aim.aim_backend( + url='https://google.com', + app_id='test', + object_query='test', + object_query_format='test', + verify=True, + ) From c1214889fdac972b22d07cf330db587f1b890612 Mon Sep 17 00:00:00 2001 From: thedoubl3j Date: Wed, 16 Oct 2024 14:13:16 -0400 Subject: [PATCH 2/6] Add aim test and assert for known failing response --- src/awx_plugins/credentials/aim.py | 16 +++++++---- tests/credential_plugins_test.py | 44 ++++++++++++++++++++++++------ 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/src/awx_plugins/credentials/aim.py b/src/awx_plugins/credentials/aim.py index 5ff68f5b9e..c7332c16b1 100644 --- a/src/awx_plugins/credentials/aim.py +++ b/src/awx_plugins/credentials/aim.py @@ -112,14 +112,18 @@ def aim_backend(**kwargs): allow_redirects=False, ) sensitive_query_params = { - 'AppId' : "****", - 'Query' : "****", - 'QueryFormat' : object_query_format, + 'AppId': '****', + 'Query': '****', + 'QueryFormat': object_query_format, } if reason: - sensitive_query_params['reason'] = "****" - sensitive_request_qs = '?' + urlencode(sensitive_query_params, safe="*", quote_via=quote) - res.url = request_url + sensitive_request_qs + sensitive_query_params['reason'] = '****' + sensitive_request_qs = urlencode( + sensitive_query_params, + safe='*', + quote_via=quote, + ) + res.url = f'{request_url}?{sensitive_request_qs}' raise_for_status(res) # CCP returns the property name capitalized, username is camel case diff --git a/tests/credential_plugins_test.py b/tests/credential_plugins_test.py index 2ef05a43d8..77786d07eb 100644 --- a/tests/credential_plugins_test.py +++ b/tests/credential_plugins_test.py @@ -5,8 +5,11 @@ import pytest -from awx_plugins.credentials import hashivault -from awx_plugins.credentials import aim +import requests +from pytest_mock import MockerFixture + +from awx_plugins.credentials import aim, hashivault + def test_imported_azure_cloud_sdk_vars() -> None: from awx_plugins.credentials import azure_kv @@ -163,11 +166,34 @@ def test_tss_import(self) -> None: # assert this module as opposed to older thycotic.secrets.server assert cls.__module__ == 'delinea.secrets.server' -def test_aim_sensitive_traceback(): - aim.aim_backend( - url='https://google.com', - app_id='test', - object_query='test', - object_query_format='test', - verify=True, + +def test_aim_sensitive_traceback_masked(mocker: MockerFixture) -> None: + """Ensure that the sensitive information is not leaked in the traceback.""" + my_response = requests.Response() + my_response.status_code = 404 + my_response.url = 'not_found' + + aim.requests.get = mocker.Mock(name='aim_request') + aim.requests.get.return_value = my_response + + expected_url_in_exc = ( + r'.*http://testurl\.com/AIMWebService/api/Accounts\?' + r'AppId=\*\*\*\*&Query=\*\*\*\*&QueryFormat=test&reason=\*\*\*\*.*' ) + expected_response_url_literal = ( + 'http://testurl.com/AIMWebService/api/Accounts?' + 'AppId=****&Query=****&QueryFormat=test&reason=****' + ) + + with pytest.raises(requests.exceptions.HTTPError, match=expected_url_in_exc) as e: + aim.aim_backend( + url='http://testurl.com', + app_id='foobar123', + object_query='foobar123', + object_query_format='test', + reason='foobar123', + verify=True, + ) + + assert e._excinfo[1].response.url == expected_response_url_literal + assert 'foobar123' not in str(e) From f6e9bb2a2803be846a5bf4ba080d34e670ecf4e0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sviatoslav=20Sydorenko=20=28=D0=A1=D0=B2=D1=8F=D1=82=D0=BE?= =?UTF-8?q?=D1=81=D0=BB=D0=B0=D0=B2=20=D0=A1=D0=B8=D0=B4=D0=BE=D1=80=D0=B5?= =?UTF-8?q?=D0=BD=D0=BA=D0=BE=29?= Date: Mon, 21 Oct 2024 16:54:36 +0200 Subject: [PATCH 3/6] Use `monkeypatch` in the aim traceback test --- pyproject.toml | 2 +- tests/credential_plugins_test.py | 7 +++++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 06ba4d5588..bdbdf64e1b 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -18,7 +18,7 @@ dependencies = [ # runtime deps # https://packaging.python.org/en/latest/guide # GUIDANCE: only add things that this project imports directly # GUIDANCE: only set lower version bounds # "awx_plugins.base_interface.api", # keep `__init__.py` empty - "awx_plugins.interfaces @git+https://github.com/ansible/awx_plugins.interfaces.git@ad4a965", # standard interface declarations for AWX plugins + "awx_plugins.interfaces", # standard interface declarations for AWX plugins "PyYAML", # credentials.injectors, inventory.plugins "azure-identity", # credentials.azure_kv "azure-keyvault", # credentials.azure_kv diff --git a/tests/credential_plugins_test.py b/tests/credential_plugins_test.py index 77786d07eb..a481a6619e 100644 --- a/tests/credential_plugins_test.py +++ b/tests/credential_plugins_test.py @@ -167,13 +167,16 @@ def test_tss_import(self) -> None: assert cls.__module__ == 'delinea.secrets.server' -def test_aim_sensitive_traceback_masked(mocker: MockerFixture) -> None: +def test_aim_sensitive_traceback_masked( + mocker: MockerFixture, + monkeypatch: pytest.MonkeyPatch, +) -> None: """Ensure that the sensitive information is not leaked in the traceback.""" my_response = requests.Response() my_response.status_code = 404 my_response.url = 'not_found' - aim.requests.get = mocker.Mock(name='aim_request') + monkeypatch.setattr(aim.requests, 'get', mocker.Mock(name='aim_request')) aim.requests.get.return_value = my_response expected_url_in_exc = ( From b2e883c5b801c3fcc7a39136d3a720fe66fc547a Mon Sep 17 00:00:00 2001 From: thedoubl3j Date: Mon, 21 Oct 2024 16:18:00 -0400 Subject: [PATCH 4/6] Adjust aim imports and reformat aim test code --- .flake8 | 2 +- tests/credential_plugins_test.py | 15 +++++++++++---- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/.flake8 b/.flake8 index 9db7fa80aa..0d131f8ba9 100644 --- a/.flake8 +++ b/.flake8 @@ -123,7 +123,7 @@ per-file-ignores = src/awx_plugins/credentials/plugins.py: B950,D100, D101, D103, D105, D107, D205, D400, LN001, WPS204, WPS229, WPS433, WPS440 src/awx_plugins/credentials/tss.py: ANN003, ANN201, D100, D103, E712, WPS433, WPS440, WPS503 src/awx_plugins/inventory/plugins.py: ANN001, ANN002, ANN003, ANN101, ANN102, ANN201, ANN202, ANN206, B950, C812, C819, D100, D101, D102, D205, D209, D400, D401, LN001, LN002, N801, WPS110, WPS111, WPS202, WPS210, WPS214, WPS301, WPS319, WPS324, WPS331, WPS336, WPS337, WPS338, WPS347, WPS421, WPS433, WPS450, WPS510, WPS529 - tests/credential_plugins_test.py: ANN101, B017, C419, D100, D102, D103, D205, D209, D400, DAR, PT011, S105, WPS111, WPS117, WPS118, WPS202, WPS352, WPS421, WPS433, WPS507 + tests/credential_plugins_test.py: ANN101, B017, C419, D100, D102, D103, D205, D209, D400, DAR, PT011, S105, WPS111, WPS117, WPS118, WPS202, WPS352, WPS421, WPS433, WPS507, WPS441 tests/importable_test.py: ANN101, DAR # Count the number of occurrences of each error/warning code and print a report: diff --git a/tests/credential_plugins_test.py b/tests/credential_plugins_test.py index a481a6619e..bb4f87a7b7 100644 --- a/tests/credential_plugins_test.py +++ b/tests/credential_plugins_test.py @@ -176,8 +176,12 @@ def test_aim_sensitive_traceback_masked( my_response.status_code = 404 my_response.url = 'not_found' - monkeypatch.setattr(aim.requests, 'get', mocker.Mock(name='aim_request')) - aim.requests.get.return_value = my_response + aim_request_mock = mocker.Mock( + autospec=True, + name='aim_request', + return_value=my_response, + ) + monkeypatch.setattr(aim.requests, 'get', aim_request_mock) expected_url_in_exc = ( r'.*http://testurl\.com/AIMWebService/api/Accounts\?' @@ -188,7 +192,10 @@ def test_aim_sensitive_traceback_masked( 'AppId=****&Query=****&QueryFormat=test&reason=****' ) - with pytest.raises(requests.exceptions.HTTPError, match=expected_url_in_exc) as e: + with pytest.raises( + requests.exceptions.HTTPError, + match=expected_url_in_exc, + ) as e: aim.aim_backend( url='http://testurl.com', app_id='foobar123', @@ -198,5 +205,5 @@ def test_aim_sensitive_traceback_masked( verify=True, ) - assert e._excinfo[1].response.url == expected_response_url_literal + assert e.value.response.url == expected_response_url_literal assert 'foobar123' not in str(e) From 6d3246cb4e657637dca11bdd68d716a1a66f35cf Mon Sep 17 00:00:00 2001 From: thedoubl3j Date: Wed, 23 Oct 2024 13:52:23 -0400 Subject: [PATCH 5/6] Adjust requests import in aim module --- src/awx_plugins/credentials/aim.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/awx_plugins/credentials/aim.py b/src/awx_plugins/credentials/aim.py index c7332c16b1..ccac67ac14 100644 --- a/src/awx_plugins/credentials/aim.py +++ b/src/awx_plugins/credentials/aim.py @@ -7,7 +7,7 @@ gettext_noop as _, ) -import requests +import requests as requests from .plugin import CertFiles, CredentialPlugin, raise_for_status From 117a1e48efaa5788c5d8fd9810032753f6270c5d Mon Sep 17 00:00:00 2001 From: thedoubl3j Date: Wed, 30 Oct 2024 17:19:41 -0400 Subject: [PATCH 6/6] Update aim test with parameterization for kwargs Update aim test URL logic --- tests/credential_plugins_test.py | 44 +++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/tests/credential_plugins_test.py b/tests/credential_plugins_test.py index bb4f87a7b7..acd8c10c12 100644 --- a/tests/credential_plugins_test.py +++ b/tests/credential_plugins_test.py @@ -167,9 +167,38 @@ def test_tss_import(self) -> None: assert cls.__module__ == 'delinea.secrets.server' +@pytest.mark.parametrize( + ( + 'reason', + 'expected_url_in_exc', + 'expected_response_url_literal', + ), + ( + pytest.param( + 'foobar123', + r'.*http://testurl\.com/AIMWebService/api/Accounts\?' + r'AppId=\*\*\*\*&Query=\*\*\*\*&QueryFormat=test&' + r'reason=\*\*\*\*.*', + 'http://testurl.com/AIMWebService/api/Accounts?' + 'AppId=****&Query=****&QueryFormat=test&reason=****', + id='with-reason', + ), + pytest.param( + '', + r'.*http://testurl\.com/AIMWebService/api/Accounts\?' + r'AppId=\*\*\*\*&Query=\*\*\*\*&QueryFormat=test.*', + 'http://testurl.com/AIMWebService/api/Accounts?' + 'AppId=****&Query=****&QueryFormat=test', + id='no-reason', + ), + ), +) def test_aim_sensitive_traceback_masked( - mocker: MockerFixture, - monkeypatch: pytest.MonkeyPatch, + mocker: MockerFixture, + monkeypatch: pytest.MonkeyPatch, + reason: str, + expected_url_in_exc: str, + expected_response_url_literal: str, ) -> None: """Ensure that the sensitive information is not leaked in the traceback.""" my_response = requests.Response() @@ -183,15 +212,6 @@ def test_aim_sensitive_traceback_masked( ) monkeypatch.setattr(aim.requests, 'get', aim_request_mock) - expected_url_in_exc = ( - r'.*http://testurl\.com/AIMWebService/api/Accounts\?' - r'AppId=\*\*\*\*&Query=\*\*\*\*&QueryFormat=test&reason=\*\*\*\*.*' - ) - expected_response_url_literal = ( - 'http://testurl.com/AIMWebService/api/Accounts?' - 'AppId=****&Query=****&QueryFormat=test&reason=****' - ) - with pytest.raises( requests.exceptions.HTTPError, match=expected_url_in_exc, @@ -201,7 +221,7 @@ def test_aim_sensitive_traceback_masked( app_id='foobar123', object_query='foobar123', object_query_format='test', - reason='foobar123', + reason=reason, verify=True, )