From 21a0e94bd605a90b454445c7bd4ec5f8b4af1ec1 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Wed, 11 Mar 2026 11:11:43 +0100 Subject: [PATCH 01/39] Add revoke Vault token functionality to OIDC lookup creds --- src/awx_plugins/credentials/hashivault.py | 289 +++++++++++++--------- tests/unit/credentials/hashivault_test.py | 24 ++ 2 files changed, 199 insertions(+), 114 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 293af0e6dd..b234b1e283 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -4,6 +4,7 @@ import os import pathlib import time +from os.path import join from urllib.parse import urljoin from awx_plugins.interfaces._temporary_private_django_api import ( # noqa: WPS436 @@ -481,6 +482,48 @@ def workload_identity_auth(**kwargs): return {'role': kwargs.get('jwt_role'), 'jwt': workload_identity_token} +def revoke_token(token, **kwargs): + """ + Revoke a Vault token using the token revoke-self endpoint. + + This minimizes the lifetime of tokens obtained through JWT authentication, + improving the security posture by ensuring tokens are only valid for the + duration of the credential operation. + + Args: + token: The Vault client token to revoke + **kwargs: Additional arguments (url, cacert, namespace) + + Returns: + None. Revocation is best-effort and exceptions are suppressed to avoid + failing the overall credential operation. + """ + if not token: + return + + try: + url = join(kwargs['url'], 'v1') + cacert = kwargs.get('cacert') + + request_kwargs = {'timeout': 10} + + sess = requests.Session() + sess.mount(url, requests.adapters.HTTPAdapter(max_retries=3)) + sess.headers['X-Vault-Token'] = token + if kwargs.get('namespace'): + sess.headers['X-Vault-Namespace'] = kwargs['namespace'] + + request_url = join(url, 'auth/token/revoke-self') + + with CertFiles(cacert) as cert: + request_kwargs['verify'] = cert + sess.post(request_url, **request_kwargs) + # Best effort - don't check response status as token may already be + # expired/revoked, which is acceptable + except Exception: + pass + + def method_auth(**kwargs): # get auth method specific params request_kwargs = {'json': kwargs['auth_param'], 'timeout': 30} @@ -523,126 +566,144 @@ def method_auth(**kwargs): def kv_backend(**kwargs): - token = handle_auth(**kwargs) - url = kwargs['url'] - secret_path = kwargs['secret_path'] - secret_backend = kwargs.get('secret_backend') - secret_key = kwargs.get('secret_key') - cacert = kwargs.get('cacert') - api_version = kwargs['api_version'] - - request_kwargs = { - 'timeout': 30, - 'allow_redirects': False, - } - - sess = requests.Session() - sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) - sess.headers['Authorization'] = f'Bearer {token}' - # Compatibility header for older installs of Hashicorp Vault - sess.headers['X-Vault-Token'] = token - if kwargs.get('namespace'): - sess.headers['X-Vault-Namespace'] = kwargs['namespace'] - - if api_version == 'v2': - if kwargs.get('secret_version'): - request_kwargs['params'] = { # type: ignore[assignment] # FIXME - 'version': kwargs['secret_version'], - } - if secret_backend: - path_segments = [secret_backend, 'data', secret_path] + try: + token = handle_auth(**kwargs) + + url = kwargs['url'] + secret_path = kwargs['secret_path'] + secret_backend = kwargs.get('secret_backend') + secret_key = kwargs.get('secret_key') + cacert = kwargs.get('cacert') + api_version = kwargs['api_version'] + + request_kwargs = { + 'timeout': 30, + 'allow_redirects': False, + } + + sess = requests.Session() + sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) + sess.headers['Authorization'] = f'Bearer {token}' + # Compatibility header for older installs of Hashicorp Vault + sess.headers['X-Vault-Token'] = token + if kwargs.get('namespace'): + sess.headers['X-Vault-Namespace'] = kwargs['namespace'] + + if api_version == 'v2': + if kwargs.get('secret_version'): + request_kwargs['params'] = { # type: ignore[assignment] # FIXME + 'version': kwargs['secret_version'], + } + if secret_backend: + path_segments = [secret_backend, 'data', secret_path] + else: + try: + mount_point, *path = pathlib.Path( + secret_path.lstrip(os.sep), + ).parts + '/'.join(path) + except Exception: + mount_point, path = secret_path, [] + # https://www.vaultproject.io/api/secret/kv/kv-v2.html#read-secret-version + path_segments = [mount_point, 'data'] + path + elif secret_backend: + path_segments = [secret_backend, secret_path] else: - try: - mount_point, *path = pathlib.Path( - secret_path.lstrip(os.sep), - ).parts - '/'.join(path) - except Exception: - mount_point, path = secret_path, [] - # https://www.vaultproject.io/api/secret/kv/kv-v2.html#read-secret-version - path_segments = [mount_point, 'data'] + path - elif secret_backend: - path_segments = [secret_backend, secret_path] - else: - path_segments = [secret_path] + path_segments = [secret_path] - request_url = urljoin(url, '/'.join(['v1'] + path_segments)).rstrip('/') - with CertFiles(cacert) as cert: - request_kwargs['verify'] = cert - request_retries = 0 - while request_retries < 5: - response = sess.get(request_url, **request_kwargs) - # https://developer.hashicorp.com/vault/docs/enterprise/consistency - if response.status_code == 412: - request_retries += 1 - time.sleep(1) - else: - break - raise_for_status(response) - - json = response.json() - if api_version == 'v2': - json = json['data'] - - if secret_key: - try: - if ( - (secret_key != 'data') - and ( # noqa: S105; not a password - secret_key not in json['data'] + request_url = urljoin(url, '/'.join(['v1'] + path_segments)).rstrip( + '/', + ) + with CertFiles(cacert) as cert: + request_kwargs['verify'] = cert + request_retries = 0 + while request_retries < 5: + response = sess.get(request_url, **request_kwargs) + # https://developer.hashicorp.com/vault/docs/enterprise/consistency + if response.status_code == 412: + request_retries += 1 + time.sleep(1) + else: + break + raise_for_status(response) + + json = response.json() + if api_version == 'v2': + json = json['data'] + + if secret_key: + try: + if ( + (secret_key != 'data') + and ( # noqa: S105; not a password + secret_key not in json['data'] + ) + and ('data' in json['data']) + ): + return json['data']['data'][secret_key] + return json['data'][secret_key] + except KeyError: + raise RuntimeError( + f'{secret_key} is not present at {secret_path}', ) - and ('data' in json['data']) - ): - return json['data']['data'][secret_key] - return json['data'][secret_key] - except KeyError: - raise RuntimeError(f'{secret_key} is not present at {secret_path}') - return json['data'] + return json['data'] + finally: + # Only revoke ephemeral vault tokens + if 'workload_identity_token' in kwargs: + # Revoke token to minimize token lifetime and improve security posture + revoke_token(token, **kwargs) def ssh_backend(**kwargs): - token = handle_auth(**kwargs) - url = urljoin(kwargs['url'], 'v1') - secret_path = kwargs['secret_path'] - role = kwargs['role'] - cacert = kwargs.get('cacert') - - request_kwargs = { - 'timeout': 30, - 'allow_redirects': False, - } - - request_kwargs['json'] = { # type: ignore[assignment] # FIXME - 'public_key': kwargs['public_key'], - } - if kwargs.get('valid_principals'): - request_kwargs['json'][ # type: ignore[index] # FIXME - 'valid_principals' - ] = kwargs['valid_principals'] - - sess = requests.Session() - sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) - sess.headers['Authorization'] = f'Bearer {token}' - if kwargs.get('namespace'): - sess.headers['X-Vault-Namespace'] = kwargs['namespace'] - # Compatibility header for older installs of Hashicorp Vault - sess.headers['X-Vault-Token'] = token - # https://www.vaultproject.io/api/secret/ssh/index.html#sign-ssh-key - request_url = '/'.join([url, secret_path, 'sign', role]).rstrip('/') - - with CertFiles(cacert) as cert: - request_kwargs['verify'] = cert - request_retries = 0 - while request_retries < 5: - resp = sess.post(request_url, **request_kwargs) - # https://developer.hashicorp.com/vault/docs/enterprise/consistency - if resp.status_code == 412: - request_retries += 1 - time.sleep(1) - else: - break - raise_for_status(resp) - return resp.json()['data']['signed_key'] + try: + token = handle_auth(**kwargs) + + url = urljoin(kwargs['url'], 'v1') + secret_path = kwargs['secret_path'] + role = kwargs['role'] + cacert = kwargs.get('cacert') + + request_kwargs = { + 'timeout': 30, + 'allow_redirects': False, + } + + request_kwargs['json'] = { # type: ignore[assignment] # FIXME + 'public_key': kwargs['public_key'], + } + if kwargs.get('valid_principals'): + request_kwargs['json'][ # type: ignore[index] # FIXME + 'valid_principals' + ] = kwargs['valid_principals'] + + sess = requests.Session() + sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) + sess.headers['Authorization'] = f'Bearer {token}' + if kwargs.get('namespace'): + sess.headers['X-Vault-Namespace'] = kwargs['namespace'] + # Compatibility header for older installs of Hashicorp Vault + sess.headers['X-Vault-Token'] = token + # https://www.vaultproject.io/api/secret/ssh/index.html#sign-ssh-key + request_url = '/'.join([url, secret_path, 'sign', role]).rstrip('/') + + with CertFiles(cacert) as cert: + request_kwargs['verify'] = cert + request_retries = 0 + while request_retries < 5: + resp = sess.post(request_url, **request_kwargs) + # https://developer.hashicorp.com/vault/docs/enterprise/consistency + if resp.status_code == 412: + request_retries += 1 + time.sleep(1) + else: + break + raise_for_status(resp) + return resp.json()['data']['signed_key'] + finally: + # Only revoke ephemeral vault tokens + if 'workload_identity_token' in kwargs: + # Revoke token to minimize token lifetime and improve security posture + revoke_token(token, **kwargs) hashivault_kv_plugin = CredentialPlugin( diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 2ed48fb441..bc30fc5dfa 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -395,6 +395,30 @@ def test_non_oidc_plugins_have_no_internal_fields( assert internal_fields == [] +def test_revoke_token_simple(mocker: MockerFixture) -> None: + """Test ``revoke_token()`` hits the correct endpoint, with all network calls mocked.""" + mock_session = mocker.MagicMock() + mock_session.headers = {} + mock_post = mocker.MagicMock() + mock_session.post = mock_post + mocker.patch('requests.Session', return_value=mock_session) + mocker.patch.object( + hashivault, + 'CertFiles', + return_value=mocker.MagicMock(), + ) + + kwargs = { + 'url': 'https://vault.example.com', + } + + hashivault.revoke_token('test_token', **kwargs) # type: ignore[no-untyped-call] + + assert mock_session.headers['X-Vault-Token'] == 'test_token' + mock_post.assert_called_once() + assert 'auth/token/revoke-self' in mock_post.call_args[0][0] + + @pytest.mark.parametrize( 'plugin', ( From a10e229029b05c76c79baf220590690b56ab2c16 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 16 Mar 2026 09:30:42 +0100 Subject: [PATCH 02/39] Add linter suggestions --- src/awx_plugins/credentials/hashivault.py | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index b234b1e283..74cd7141cb 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -482,22 +482,8 @@ def workload_identity_auth(**kwargs): return {'role': kwargs.get('jwt_role'), 'jwt': workload_identity_token} -def revoke_token(token, **kwargs): - """ - Revoke a Vault token using the token revoke-self endpoint. - - This minimizes the lifetime of tokens obtained through JWT authentication, - improving the security posture by ensuring tokens are only valid for the - duration of the credential operation. - - Args: - token: The Vault client token to revoke - **kwargs: Additional arguments (url, cacert, namespace) - - Returns: - None. Revocation is best-effort and exceptions are suppressed to avoid - failing the overall credential operation. - """ +def revoke_token(token: str, **kwargs): + """Revoke a Vault token using the token revoke-self endpoint.""" if not token: return From 62aedd4d7a946a509787cab430f276a616d3b03f Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 16 Mar 2026 15:13:53 +0100 Subject: [PATCH 03/39] Add more unit tests --- src/awx_plugins/credentials/hashivault.py | 6 +- tests/unit/credentials/hashivault_test.py | 130 ++++++++++++++++++++++ 2 files changed, 134 insertions(+), 2 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 74cd7141cb..acba020e23 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -7,15 +7,17 @@ from os.path import join from urllib.parse import urljoin +from logging import getLogger + from awx_plugins.interfaces._temporary_private_django_api import ( # noqa: WPS436 gettext_noop as _, ) import requests - from . import _types from .plugin import CertFiles, CredentialPlugin, raise_for_status +logger = getLogger(__name__) # Base input fields url_field: _types.FieldDict = { @@ -507,7 +509,7 @@ def revoke_token(token: str, **kwargs): # Best effort - don't check response status as token may already be # expired/revoked, which is acceptable except Exception: - pass + logger.warning('Failed to revoke ephemeral Vault token') def method_auth(**kwargs): diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index bc30fc5dfa..9bb95fbf54 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -419,6 +419,136 @@ def test_revoke_token_simple(mocker: MockerFixture) -> None: assert 'auth/token/revoke-self' in mock_post.call_args[0][0] +def test_revoke_token_with_empty_token(mocker: MockerFixture) -> None: + """Test ``revoke_token()`` returns early when token is empty.""" + mock_session = mocker.MagicMock() + mocker.patch('requests.Session', return_value=mock_session) + + kwargs = { + 'url': 'https://vault.example.com', + } + + # Test with None + hashivault.revoke_token(None, **kwargs) # type: ignore[no-untyped-call,arg-type] + mock_session.post.assert_not_called() + + # Test with empty string + hashivault.revoke_token('', **kwargs) # type: ignore[no-untyped-call] + mock_session.post.assert_not_called() + + +def test_revoke_token_with_namespace(mocker: MockerFixture) -> None: + """Test ``revoke_token()`` includes namespace header when provided.""" + mock_session = mocker.MagicMock() + mock_session.headers = {} + mock_post = mocker.MagicMock() + mock_session.post = mock_post + mocker.patch('requests.Session', return_value=mock_session) + mocker.patch.object( + hashivault, + 'CertFiles', + return_value=mocker.MagicMock(), + ) + + kwargs = { + 'url': 'https://vault.example.com', + 'namespace': 'test-namespace', + } + + hashivault.revoke_token('test_token', **kwargs) # type: ignore[no-untyped-call] + + assert mock_session.headers['X-Vault-Token'] == 'test_token' + assert mock_session.headers['X-Vault-Namespace'] == 'test-namespace' + mock_post.assert_called_once() + + +def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: + """Test ``revoke_token()`` handles exceptions gracefully.""" + mock_session = mocker.MagicMock() + mock_session.headers = {} + mock_session.post.side_effect = Exception('Network error') + mocker.patch('requests.Session', return_value=mock_session) + mocker.patch.object( + hashivault, + 'CertFiles', + return_value=mocker.MagicMock(), + ) + mock_logger = mocker.patch.object(hashivault, 'logger') + + kwargs = { + 'url': 'https://vault.example.com', + } + + # Should not raise exception + hashivault.revoke_token('test_token', **kwargs) # type: ignore[no-untyped-call] + + mock_logger.warning.assert_called_once_with( + 'Failed to revoke ephemeral Vault token', + ) + + +def test_kv_backend_revokes_oidc_token(mocker: MockerFixture) -> None: + """Test ``kv_backend()`` revokes token in finally block for OIDC auth.""" + mock_handle_auth = mocker.patch.object( + hashivault, + 'handle_auth', + return_value='test_token', + ) + mock_revoke = mocker.patch.object(hashivault, 'revoke_token') + mocker.patch('requests.Session') + mocker.patch.object(hashivault, 'CertFiles') + mocker.patch.object(hashivault, 'raise_for_status') + + kwargs = { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/secret/path', + 'api_version': 'v1', + 'secret_key': 'password', + } + + try: + hashivault.kv_backend(**kwargs) # type: ignore[no-untyped-call] + except Exception: + pass # We don't care if it fails, we just want to ensure revoke is called + + mock_handle_auth.assert_called_once() + mock_revoke.assert_called_once_with('test_token', **kwargs) + + +def test_ssh_backend_revokes_oidc_token(mocker: MockerFixture) -> None: + """Test ``ssh_backend()`` revokes token in finally block for OIDC auth.""" + mock_handle_auth = mocker.patch.object( + hashivault, + 'handle_auth', + return_value='test_token', + ) + mock_revoke = mocker.patch.object(hashivault, 'revoke_token') + mocker.patch('requests.Session') + mocker.patch.object(hashivault, 'CertFiles') + mocker.patch.object(hashivault, 'raise_for_status') + + kwargs = { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/ssh', + 'role': 'test_ssh_role', + 'public_key': 'ssh-rsa AAAAB...', + } + + try: + hashivault.ssh_backend(**kwargs) # type: ignore[no-untyped-call] + except Exception: + pass # We don't care if it fails, we just want to ensure revoke is called + + mock_handle_auth.assert_called_once() + mock_revoke.assert_called_once_with('test_token', **kwargs) + + @pytest.mark.parametrize( 'plugin', ( From 5d36c79704f154d77a26f634e70b1f459bb00dca Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 16 Mar 2026 15:16:33 +0100 Subject: [PATCH 04/39] Refactor tests --- tests/unit/credentials/hashivault_test.py | 162 +++++++++++----------- 1 file changed, 82 insertions(+), 80 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 9bb95fbf54..e36d50b2e3 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -395,31 +395,18 @@ def test_non_oidc_plugins_have_no_internal_fields( assert internal_fields == [] -def test_revoke_token_simple(mocker: MockerFixture) -> None: - """Test ``revoke_token()`` hits the correct endpoint, with all network calls mocked.""" - mock_session = mocker.MagicMock() - mock_session.headers = {} - mock_post = mocker.MagicMock() - mock_session.post = mock_post - mocker.patch('requests.Session', return_value=mock_session) - mocker.patch.object( - hashivault, - 'CertFiles', - return_value=mocker.MagicMock(), - ) - - kwargs = { - 'url': 'https://vault.example.com', - } - - hashivault.revoke_token('test_token', **kwargs) # type: ignore[no-untyped-call] - - assert mock_session.headers['X-Vault-Token'] == 'test_token' - mock_post.assert_called_once() - assert 'auth/token/revoke-self' in mock_post.call_args[0][0] - - -def test_revoke_token_with_empty_token(mocker: MockerFixture) -> None: +@pytest.mark.parametrize( + ('token', 'should_call_post'), + ( + pytest.param(None, False, id='none-token'), + pytest.param('', False, id='empty-token'), + ), +) +def test_revoke_token_with_empty_token( + mocker: MockerFixture, + token: str | None, + should_call_post: bool, +) -> None: """Test ``revoke_token()`` returns early when token is empty.""" mock_session = mocker.MagicMock() mocker.patch('requests.Session', return_value=mock_session) @@ -428,17 +415,35 @@ def test_revoke_token_with_empty_token(mocker: MockerFixture) -> None: 'url': 'https://vault.example.com', } - # Test with None - hashivault.revoke_token(None, **kwargs) # type: ignore[no-untyped-call,arg-type] - mock_session.post.assert_not_called() + hashivault.revoke_token(token, **kwargs) # type: ignore[no-untyped-call,arg-type] - # Test with empty string - hashivault.revoke_token('', **kwargs) # type: ignore[no-untyped-call] - mock_session.post.assert_not_called() + if should_call_post: + mock_session.post.assert_called_once() + else: + mock_session.post.assert_not_called() -def test_revoke_token_with_namespace(mocker: MockerFixture) -> None: - """Test ``revoke_token()`` includes namespace header when provided.""" +@pytest.mark.parametrize( + ('extra_kwargs', 'expected_headers'), + ( + pytest.param( + {}, + {'X-Vault-Token': 'test_token'}, + id='without-namespace', + ), + pytest.param( + {'namespace': 'test-namespace'}, + {'X-Vault-Token': 'test_token', 'X-Vault-Namespace': 'test-namespace'}, + id='with-namespace', + ), + ), +) +def test_revoke_token_success( + mocker: MockerFixture, + extra_kwargs: dict[str, str], + expected_headers: dict[str, str], +) -> None: + """Test ``revoke_token()`` hits the correct endpoint and sets appropriate headers.""" mock_session = mocker.MagicMock() mock_session.headers = {} mock_post = mocker.MagicMock() @@ -452,14 +457,15 @@ def test_revoke_token_with_namespace(mocker: MockerFixture) -> None: kwargs = { 'url': 'https://vault.example.com', - 'namespace': 'test-namespace', + **extra_kwargs, } hashivault.revoke_token('test_token', **kwargs) # type: ignore[no-untyped-call] - assert mock_session.headers['X-Vault-Token'] == 'test_token' - assert mock_session.headers['X-Vault-Namespace'] == 'test-namespace' + for header, value in expected_headers.items(): + assert mock_session.headers[header] == value mock_post.assert_called_once() + assert 'auth/token/revoke-self' in mock_post.call_args[0][0] def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: @@ -487,39 +493,43 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: ) -def test_kv_backend_revokes_oidc_token(mocker: MockerFixture) -> None: - """Test ``kv_backend()`` revokes token in finally block for OIDC auth.""" - mock_handle_auth = mocker.patch.object( - hashivault, - 'handle_auth', - return_value='test_token', - ) - mock_revoke = mocker.patch.object(hashivault, 'revoke_token') - mocker.patch('requests.Session') - mocker.patch.object(hashivault, 'CertFiles') - mocker.patch.object(hashivault, 'raise_for_status') - - kwargs = { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/secret/path', - 'api_version': 'v1', - 'secret_key': 'password', - } - - try: - hashivault.kv_backend(**kwargs) # type: ignore[no-untyped-call] - except Exception: - pass # We don't care if it fails, we just want to ensure revoke is called - - mock_handle_auth.assert_called_once() - mock_revoke.assert_called_once_with('test_token', **kwargs) - - -def test_ssh_backend_revokes_oidc_token(mocker: MockerFixture) -> None: - """Test ``ssh_backend()`` revokes token in finally block for OIDC auth.""" +@pytest.mark.parametrize( + ('backend_func', 'backend_kwargs'), + ( + pytest.param( + 'kv_backend', + { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/secret/path', + 'api_version': 'v1', + 'secret_key': 'password', + }, + id='kv-backend', + ), + pytest.param( + 'ssh_backend', + { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/ssh', + 'role': 'test_ssh_role', + 'public_key': 'ssh-rsa AAAAB...', + }, + id='ssh-backend', + ), + ), +) +def test_backend_revokes_oidc_token( + mocker: MockerFixture, + backend_func: str, + backend_kwargs: dict[str, str], +) -> None: + """Test backend functions revoke token in finally block for OIDC auth.""" mock_handle_auth = mocker.patch.object( hashivault, 'handle_auth', @@ -530,23 +540,15 @@ def test_ssh_backend_revokes_oidc_token(mocker: MockerFixture) -> None: mocker.patch.object(hashivault, 'CertFiles') mocker.patch.object(hashivault, 'raise_for_status') - kwargs = { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/ssh', - 'role': 'test_ssh_role', - 'public_key': 'ssh-rsa AAAAB...', - } + backend = getattr(hashivault, backend_func) try: - hashivault.ssh_backend(**kwargs) # type: ignore[no-untyped-call] + backend(**backend_kwargs) # type: ignore[no-untyped-call] except Exception: pass # We don't care if it fails, we just want to ensure revoke is called mock_handle_auth.assert_called_once() - mock_revoke.assert_called_once_with('test_token', **kwargs) + mock_revoke.assert_called_once_with('test_token', **backend_kwargs) @pytest.mark.parametrize( From a7ed1c8291183696308037c728444752e6795d06 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Mon, 16 Mar 2026 14:18:39 +0000 Subject: [PATCH 05/39] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- src/awx_plugins/credentials/hashivault.py | 5 +++-- tests/unit/credentials/hashivault_test.py | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index acba020e23..3637d06a55 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -4,19 +4,20 @@ import os import pathlib import time +from logging import getLogger from os.path import join from urllib.parse import urljoin -from logging import getLogger - from awx_plugins.interfaces._temporary_private_django_api import ( # noqa: WPS436 gettext_noop as _, ) import requests + from . import _types from .plugin import CertFiles, CredentialPlugin, raise_for_status + logger = getLogger(__name__) # Base input fields diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index e36d50b2e3..9f1909142e 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -433,7 +433,10 @@ def test_revoke_token_with_empty_token( ), pytest.param( {'namespace': 'test-namespace'}, - {'X-Vault-Token': 'test_token', 'X-Vault-Namespace': 'test-namespace'}, + { + 'X-Vault-Token': 'test_token', + 'X-Vault-Namespace': 'test-namespace', + }, id='with-namespace', ), ), From 9b6e9402e83f772273ce9da4328cbbf1aa25ade8 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 16 Mar 2026 16:53:28 +0100 Subject: [PATCH 06/39] Address pre-commit failures --- src/awx_plugins/credentials/hashivault.py | 4 ++-- tests/unit/credentials/hashivault_test.py | 20 ++++++++++---------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 3637d06a55..62d25ae603 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -554,7 +554,7 @@ def method_auth(**kwargs): return token -def kv_backend(**kwargs): +def kv_backend(**kwargs): # noqa: PLR0915 try: token = handle_auth(**kwargs) @@ -624,7 +624,7 @@ def kv_backend(**kwargs): try: if ( (secret_key != 'data') - and ( # noqa: S105; not a password + and ( # noqa: S105 secret_key not in json['data'] ) and ('data' in json['data']) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 9f1909142e..6b5a3e75ee 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -1,5 +1,6 @@ """Tests for HashiCorp Vault credential plugins.""" +import contextlib import typing as _t import pytest @@ -396,15 +397,16 @@ def test_non_oidc_plugins_have_no_internal_fields( @pytest.mark.parametrize( - ('token', 'should_call_post'), + ('should_call_post', 'token'), ( - pytest.param(None, False, id='none-token'), - pytest.param('', False, id='empty-token'), + pytest.param(False, None, id='none-token'), + pytest.param(False, '', id='empty-token'), ), ) def test_revoke_token_with_empty_token( mocker: MockerFixture, token: str | None, + *, should_call_post: bool, ) -> None: """Test ``revoke_token()`` returns early when token is empty.""" @@ -415,7 +417,7 @@ def test_revoke_token_with_empty_token( 'url': 'https://vault.example.com', } - hashivault.revoke_token(token, **kwargs) # type: ignore[no-untyped-call,arg-type] + hashivault.revoke_token(token, **kwargs) if should_call_post: mock_session.post.assert_called_once() @@ -463,7 +465,7 @@ def test_revoke_token_success( **extra_kwargs, } - hashivault.revoke_token('test_token', **kwargs) # type: ignore[no-untyped-call] + hashivault.revoke_token('test_token', **kwargs) for header, value in expected_headers.items(): assert mock_session.headers[header] == value @@ -489,7 +491,7 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: } # Should not raise exception - hashivault.revoke_token('test_token', **kwargs) # type: ignore[no-untyped-call] + hashivault.revoke_token('test_token', **kwargs) mock_logger.warning.assert_called_once_with( 'Failed to revoke ephemeral Vault token', @@ -545,10 +547,8 @@ def test_backend_revokes_oidc_token( backend = getattr(hashivault, backend_func) - try: - backend(**backend_kwargs) # type: ignore[no-untyped-call] - except Exception: - pass # We don't care if it fails, we just want to ensure revoke is called + with contextlib.suppress(Exception): + backend(**backend_kwargs) mock_handle_auth.assert_called_once() mock_revoke.assert_called_once_with('test_token', **backend_kwargs) From 40c560637e20224462448206e69dea526f79fe54 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 16 Mar 2026 17:06:29 +0100 Subject: [PATCH 07/39] Add test case for kv_backend v2 --- tests/unit/credentials/hashivault_test.py | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 6b5a3e75ee..4032b10341 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -512,7 +512,20 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: 'api_version': 'v1', 'secret_key': 'password', }, - id='kv-backend', + id='kv-backend-v1', + ), + pytest.param( + 'kv_backend', + { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/secret/path', + 'api_version': 'v2', + 'secret_key': 'password', + }, + id='kv-backend-v2', ), pytest.param( 'ssh_backend', From 6f0241894f6d20cb9d1d2d96cccac0a38a0ef408 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Tue, 17 Mar 2026 16:13:30 +0100 Subject: [PATCH 08/39] Add more test cases --- tests/unit/credentials/hashivault_test.py | 72 +++++++++++++++++++++++ 1 file changed, 72 insertions(+) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 4032b10341..c148a2e926 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -527,6 +527,64 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: }, id='kv-backend-v2', ), + pytest.param( + 'kv_backend', + { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/secret/path', + 'api_version': 'v2', + 'secret_key': 'password', + 'namespace': 'test-namespace', + }, + id='kv-backend-v2-with-namespace', + ), + pytest.param( + 'kv_backend', + { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/secret/path', + 'api_version': 'v2', + 'secret_key': 'password', + 'secret_version': '3', + }, + id='kv-backend-v2-with-secret-version', + ), + pytest.param( + 'kv_backend', + { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/secret/path', + 'api_version': 'v2', + 'secret_key': 'password', + 'secret_backend': 'kv', + }, + id='kv-backend-v2-with-secret-backend', + ), + pytest.param( + 'kv_backend', + { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/secret/path', + 'api_version': 'v2', + 'secret_key': 'password', + 'namespace': 'test-namespace', + 'secret_backend': 'kv', + 'secret_version': '5', + }, + id='kv-backend-v2-with-all-params', + ), pytest.param( 'ssh_backend', { @@ -540,6 +598,20 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: }, id='ssh-backend', ), + pytest.param( + 'ssh_backend', + { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + 'secret_path': '/ssh', + 'role': 'test_ssh_role', + 'public_key': 'ssh-rsa AAAAB...', + 'namespace': 'test-namespace', + }, + id='ssh-backend-with-namespace', + ), ), ) def test_backend_revokes_oidc_token( From 5f0703a3bc552a02b432ec126c02c6cfadffa3dd Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 23 Mar 2026 11:01:52 +0100 Subject: [PATCH 09/39] Use urllib join instead of ospath join --- src/awx_plugins/credentials/hashivault.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 62d25ae603..7193d81236 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -5,7 +5,6 @@ import pathlib import time from logging import getLogger -from os.path import join from urllib.parse import urljoin from awx_plugins.interfaces._temporary_private_django_api import ( # noqa: WPS436 @@ -491,7 +490,7 @@ def revoke_token(token: str, **kwargs): return try: - url = join(kwargs['url'], 'v1') + url = urljoin(kwargs['url'], 'v1') cacert = kwargs.get('cacert') request_kwargs = {'timeout': 10} @@ -502,7 +501,7 @@ def revoke_token(token: str, **kwargs): if kwargs.get('namespace'): sess.headers['X-Vault-Namespace'] = kwargs['namespace'] - request_url = join(url, 'auth/token/revoke-self') + request_url = urljoin(url + '/', 'auth/token/revoke-self') with CertFiles(cacert) as cert: request_kwargs['verify'] = cert From 0aaed5f33c84c721baa0282881c8affc82d74c1a Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 23 Mar 2026 11:45:59 +0100 Subject: [PATCH 10/39] Move token definition out of try block --- src/awx_plugins/credentials/hashivault.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 7193d81236..0c6972b32f 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -554,9 +554,9 @@ def method_auth(**kwargs): def kv_backend(**kwargs): # noqa: PLR0915 - try: - token = handle_auth(**kwargs) + token = handle_auth(**kwargs) + try: url = kwargs['url'] secret_path = kwargs['secret_path'] secret_backend = kwargs.get('secret_backend') @@ -643,9 +643,9 @@ def kv_backend(**kwargs): # noqa: PLR0915 def ssh_backend(**kwargs): - try: - token = handle_auth(**kwargs) + token = handle_auth(**kwargs) + try: url = urljoin(kwargs['url'], 'v1') secret_path = kwargs['secret_path'] role = kwargs['role'] From 3da915fe19241599f768c28676a19d6bbe196c5e Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 23 Mar 2026 12:10:04 +0100 Subject: [PATCH 11/39] Simplify kv_backend to meet pre-commit linter --- src/awx_plugins/credentials/hashivault.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 0c6972b32f..a401bb14f8 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -557,11 +557,9 @@ def kv_backend(**kwargs): # noqa: PLR0915 token = handle_auth(**kwargs) try: - url = kwargs['url'] secret_path = kwargs['secret_path'] secret_backend = kwargs.get('secret_backend') secret_key = kwargs.get('secret_key') - cacert = kwargs.get('cacert') api_version = kwargs['api_version'] request_kwargs = { @@ -570,7 +568,7 @@ def kv_backend(**kwargs): # noqa: PLR0915 } sess = requests.Session() - sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) + sess.mount(kwargs['url'], requests.adapters.HTTPAdapter(max_retries=5)) sess.headers['Authorization'] = f'Bearer {token}' # Compatibility header for older installs of Hashicorp Vault sess.headers['X-Vault-Token'] = token @@ -589,7 +587,6 @@ def kv_backend(**kwargs): # noqa: PLR0915 mount_point, *path = pathlib.Path( secret_path.lstrip(os.sep), ).parts - '/'.join(path) except Exception: mount_point, path = secret_path, [] # https://www.vaultproject.io/api/secret/kv/kv-v2.html#read-secret-version @@ -599,10 +596,13 @@ def kv_backend(**kwargs): # noqa: PLR0915 else: path_segments = [secret_path] - request_url = urljoin(url, '/'.join(['v1'] + path_segments)).rstrip( + request_url = urljoin( + kwargs['url'], + '/'.join(['v1'] + path_segments), + ).rstrip( '/', ) - with CertFiles(cacert) as cert: + with CertFiles(kwargs.get('cacert')) as cert: request_kwargs['verify'] = cert request_retries = 0 while request_retries < 5: From 025669d3357dd5be4e65fd9c71beddf8284da5e4 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 23 Mar 2026 12:26:46 +0100 Subject: [PATCH 12/39] Fix token typing linter --- src/awx_plugins/credentials/hashivault.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index a401bb14f8..928164e723 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -484,7 +484,7 @@ def workload_identity_auth(**kwargs): return {'role': kwargs.get('jwt_role'), 'jwt': workload_identity_token} -def revoke_token(token: str, **kwargs): +def revoke_token(token: str | None, **kwargs): """Revoke a Vault token using the token revoke-self endpoint.""" if not token: return From c50577c30269601e1f2a9f4dcd293388779bf20c Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Mon, 23 Mar 2026 15:10:56 +0100 Subject: [PATCH 13/39] Remove unused parameter in test token empty --- tests/unit/credentials/hashivault_test.py | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index c148a2e926..2a135e5f46 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -397,17 +397,15 @@ def test_non_oidc_plugins_have_no_internal_fields( @pytest.mark.parametrize( - ('should_call_post', 'token'), + ('token'), ( - pytest.param(False, None, id='none-token'), - pytest.param(False, '', id='empty-token'), + pytest.param(None, id='none-token'), + pytest.param('', id='empty-token'), ), ) def test_revoke_token_with_empty_token( mocker: MockerFixture, token: str | None, - *, - should_call_post: bool, ) -> None: """Test ``revoke_token()`` returns early when token is empty.""" mock_session = mocker.MagicMock() @@ -419,10 +417,7 @@ def test_revoke_token_with_empty_token( hashivault.revoke_token(token, **kwargs) - if should_call_post: - mock_session.post.assert_called_once() - else: - mock_session.post.assert_not_called() + mock_session.post.assert_not_called() @pytest.mark.parametrize( From 52931cbe930c4fd79fa9119cecdf9230928f799e Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Tue, 24 Mar 2026 11:48:59 +0100 Subject: [PATCH 14/39] Add context manager to manage vault token Modify the tests so they use the new context manager approach for testing Removed logging module and function Removed try-except in most scenarios (the vault token CM still uses it) --- src/awx_plugins/credentials/hashivault.py | 64 +++---- tests/unit/credentials/hashivault_test.py | 199 +++++++++++----------- 2 files changed, 124 insertions(+), 139 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 928164e723..f634bebe04 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -1,10 +1,11 @@ # FIXME: the following violations must be addressed gradually and unignored # mypy: disable-error-code="arg-type, no-untyped-call, no-untyped-def" +import contextlib as _ctx import os import pathlib import time -from logging import getLogger +from collections.abc import Generator from urllib.parse import urljoin from awx_plugins.interfaces._temporary_private_django_api import ( # noqa: WPS436 @@ -17,8 +18,6 @@ from .plugin import CertFiles, CredentialPlugin, raise_for_status -logger = getLogger(__name__) - # Base input fields url_field: _types.FieldDict = { 'id': 'url', @@ -484,32 +483,23 @@ def workload_identity_auth(**kwargs): return {'role': kwargs.get('jwt_role'), 'jwt': workload_identity_token} -def revoke_token(token: str | None, **kwargs): - """Revoke a Vault token using the token revoke-self endpoint.""" - if not token: - return - +@_ctx.contextmanager +def vault_token(**kwargs) -> Generator[str, None, None]: + """Context manager that yields a Vault token and revokes it on exit if obtained via workload identity.""" + token = handle_auth(**kwargs) try: - url = urljoin(kwargs['url'], 'v1') - cacert = kwargs.get('cacert') - - request_kwargs = {'timeout': 10} - - sess = requests.Session() - sess.mount(url, requests.adapters.HTTPAdapter(max_retries=3)) - sess.headers['X-Vault-Token'] = token - if kwargs.get('namespace'): - sess.headers['X-Vault-Namespace'] = kwargs['namespace'] - - request_url = urljoin(url + '/', 'auth/token/revoke-self') - - with CertFiles(cacert) as cert: - request_kwargs['verify'] = cert - sess.post(request_url, **request_kwargs) - # Best effort - don't check response status as token may already be - # expired/revoked, which is acceptable - except Exception: - logger.warning('Failed to revoke ephemeral Vault token') + yield token + finally: + # Only revoke tokens obtained via workload identity authentication + if kwargs.get('workload_identity_token'): + url = urljoin(kwargs['url'], 'v1/auth/token/revoke-self') + sess = requests.Session() + sess.headers['X-Vault-Token'] = token + if kwargs.get('namespace'): + sess.headers['X-Vault-Namespace'] = kwargs['namespace'] + with CertFiles(kwargs.get('cacert')) as cert: + resp = sess.post(url, verify=cert, timeout=30) + resp.raise_for_status() def method_auth(**kwargs): @@ -554,9 +544,7 @@ def method_auth(**kwargs): def kv_backend(**kwargs): # noqa: PLR0915 - token = handle_auth(**kwargs) - - try: + with vault_token(**kwargs) as token: secret_path = kwargs['secret_path'] secret_backend = kwargs.get('secret_backend') secret_key = kwargs.get('secret_key') @@ -635,17 +623,10 @@ def kv_backend(**kwargs): # noqa: PLR0915 f'{secret_key} is not present at {secret_path}', ) return json['data'] - finally: - # Only revoke ephemeral vault tokens - if 'workload_identity_token' in kwargs: - # Revoke token to minimize token lifetime and improve security posture - revoke_token(token, **kwargs) def ssh_backend(**kwargs): - token = handle_auth(**kwargs) - - try: + with vault_token(**kwargs) as token: url = urljoin(kwargs['url'], 'v1') secret_path = kwargs['secret_path'] role = kwargs['role'] @@ -687,11 +668,6 @@ def ssh_backend(**kwargs): break raise_for_status(resp) return resp.json()['data']['signed_key'] - finally: - # Only revoke ephemeral vault tokens - if 'workload_identity_token' in kwargs: - # Revoke token to minimize token lifetime and improve security posture - revoke_token(token, **kwargs) hashivault_kv_plugin = CredentialPlugin( diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 2a135e5f46..8389713958 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -396,27 +396,27 @@ def test_non_oidc_plugins_have_no_internal_fields( assert internal_fields == [] -@pytest.mark.parametrize( - ('token'), - ( - pytest.param(None, id='none-token'), - pytest.param('', id='empty-token'), - ), -) -def test_revoke_token_with_empty_token( +def test_vault_token_no_workload_identity( mocker: MockerFixture, - token: str | None, ) -> None: - """Test ``revoke_token()`` returns early when token is empty.""" + """Test ``vault_token`` context manager doesn't revoke token without workload_identity_token.""" + mock_handle_auth = mocker.patch.object( + hashivault, + 'handle_auth', + return_value='test_token', + ) mock_session = mocker.MagicMock() mocker.patch('requests.Session', return_value=mock_session) kwargs = { 'url': 'https://vault.example.com', + 'token': 'test_token', } - hashivault.revoke_token(token, **kwargs) + with hashivault.vault_token(**kwargs) as token: + assert token == 'test_token' + mock_handle_auth.assert_called_once_with(**kwargs) mock_session.post.assert_not_called() @@ -438,98 +438,104 @@ def test_revoke_token_with_empty_token( ), ), ) -def test_revoke_token_success( +def test_vault_token_revokes_oidc_token( mocker: MockerFixture, extra_kwargs: dict[str, str], expected_headers: dict[str, str], ) -> None: - """Test ``revoke_token()`` hits the correct endpoint and sets appropriate headers.""" + """Test ``vault_token`` context manager revokes token for workload identity auth.""" + mock_handle_auth = mocker.patch.object( + hashivault, + 'handle_auth', + return_value='test_token', + ) mock_session = mocker.MagicMock() mock_session.headers = {} - mock_post = mocker.MagicMock() - mock_session.post = mock_post + mock_response = mocker.MagicMock() + mock_session.post.return_value = mock_response mocker.patch('requests.Session', return_value=mock_session) + mock_cert_files = mocker.MagicMock() + mock_cert_files.__enter__ = mocker.MagicMock(return_value='cert_path') + mock_cert_files.__exit__ = mocker.MagicMock(return_value=False) mocker.patch.object( hashivault, 'CertFiles', - return_value=mocker.MagicMock(), + return_value=mock_cert_files, ) kwargs = { 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', **extra_kwargs, } - hashivault.revoke_token('test_token', **kwargs) + with hashivault.vault_token(**kwargs) as token: + assert token == 'test_token' - for header, value in expected_headers.items(): - assert mock_session.headers[header] == value - mock_post.assert_called_once() - assert 'auth/token/revoke-self' in mock_post.call_args[0][0] + mock_handle_auth.assert_called_once_with(**kwargs) + for header, contents in expected_headers.items(): + assert mock_session.headers[header] == contents + mock_session.post.assert_called_once() + assert 'auth/token/revoke-self' in mock_session.post.call_args[0][0] + mock_response.raise_for_status.assert_called_once() -def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: - """Test ``revoke_token()`` handles exceptions gracefully.""" +def test_vault_token_revoke_failure( + mocker: MockerFixture, +) -> None: + """Test ``vault_token`` context manager raises when token revocation fails.""" + mock_handle_auth = mocker.patch.object( + hashivault, + 'handle_auth', + return_value='test_token', + ) mock_session = mocker.MagicMock() mock_session.headers = {} - mock_session.post.side_effect = Exception('Network error') + mock_response = mocker.MagicMock() + mock_response.raise_for_status.side_effect = Exception('Revocation failed') + mock_session.post.return_value = mock_response mocker.patch('requests.Session', return_value=mock_session) + mock_cert_files = mocker.MagicMock() + mock_cert_files.__enter__ = mocker.MagicMock(return_value='cert_path') + mock_cert_files.__exit__ = mocker.MagicMock(return_value=False) mocker.patch.object( hashivault, 'CertFiles', - return_value=mocker.MagicMock(), + return_value=mock_cert_files, ) - mock_logger = mocker.patch.object(hashivault, 'logger') kwargs = { 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', } - # Should not raise exception - hashivault.revoke_token('test_token', **kwargs) + with pytest.raises(Exception, match='Revocation failed'): + with hashivault.vault_token(**kwargs) as token: + assert token == 'test_token' - mock_logger.warning.assert_called_once_with( - 'Failed to revoke ephemeral Vault token', - ) + mock_handle_auth.assert_called_once_with(**kwargs) @pytest.mark.parametrize( - ('backend_func', 'backend_kwargs'), + ('backend_func', 'extra_kwargs'), ( pytest.param( 'kv_backend', - { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/secret/path', - 'api_version': 'v1', - 'secret_key': 'password', - }, + {'api_version': 'v1', 'secret_key': 'password'}, id='kv-backend-v1', ), pytest.param( 'kv_backend', - { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/secret/path', - 'api_version': 'v2', - 'secret_key': 'password', - }, + {'api_version': 'v2', 'secret_key': 'password'}, id='kv-backend-v2', ), pytest.param( 'kv_backend', { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/secret/path', 'api_version': 'v2', 'secret_key': 'password', 'namespace': 'test-namespace', @@ -539,11 +545,6 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: pytest.param( 'kv_backend', { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/secret/path', 'api_version': 'v2', 'secret_key': 'password', 'secret_version': '3', @@ -553,11 +554,6 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: pytest.param( 'kv_backend', { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/secret/path', 'api_version': 'v2', 'secret_key': 'password', 'secret_backend': 'kv', @@ -567,11 +563,6 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: pytest.param( 'kv_backend', { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/secret/path', 'api_version': 'v2', 'secret_key': 'password', 'namespace': 'test-namespace', @@ -582,29 +573,12 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: ), pytest.param( 'ssh_backend', - { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/ssh', - 'role': 'test_ssh_role', - 'public_key': 'ssh-rsa AAAAB...', - }, + {}, id='ssh-backend', ), pytest.param( 'ssh_backend', - { - 'url': 'https://vault.example.com', - 'workload_identity_token': 'jwt_token', - 'jwt_role': 'test_role', - 'default_auth_path': 'jwt', - 'secret_path': '/ssh', - 'role': 'test_ssh_role', - 'public_key': 'ssh-rsa AAAAB...', - 'namespace': 'test-namespace', - }, + {'namespace': 'test-namespace'}, id='ssh-backend-with-namespace', ), ), @@ -612,17 +586,49 @@ def test_revoke_token_handles_exceptions(mocker: MockerFixture) -> None: def test_backend_revokes_oidc_token( mocker: MockerFixture, backend_func: str, - backend_kwargs: dict[str, str], + extra_kwargs: dict[str, str], ) -> None: - """Test backend functions revoke token in finally block for OIDC auth.""" + """Test backend functions revoke token via context manager for OIDC auth.""" + # Common base kwargs for all OIDC auth scenarios + base_kwargs = { + 'url': 'https://vault.example.com', + 'workload_identity_token': 'jwt_token', + 'jwt_role': 'test_role', + 'default_auth_path': 'jwt', + } + + # Backend-specific kwargs + if backend_func == 'kv_backend': + backend_specific = {'secret_path': '/secret/path'} + else: # ssh_backend + backend_specific = { + 'secret_path': '/ssh', + 'role': 'test_ssh_role', + 'public_key': 'ssh-rsa AAAAB...', + } + + backend_kwargs = {**base_kwargs, **backend_specific, **extra_kwargs} + mock_handle_auth = mocker.patch.object( hashivault, 'handle_auth', return_value='test_token', ) - mock_revoke = mocker.patch.object(hashivault, 'revoke_token') - mocker.patch('requests.Session') - mocker.patch.object(hashivault, 'CertFiles') + mock_get_or_post_session = mocker.MagicMock() + mock_revoke_session = mocker.MagicMock() + mock_revoke_session.headers = {} + mock_revoke_response = mocker.MagicMock() + mock_revoke_session.post.return_value = mock_revoke_response + + # requests.Session is called twice: once for secret fetch, once for revoke + mocker.patch( + 'requests.Session', + side_effect=[mock_get_or_post_session, mock_revoke_session], + ) + mock_cert_files = mocker.MagicMock() + mock_cert_files.__enter__ = mocker.MagicMock(return_value='cert_path') + mock_cert_files.__exit__ = mocker.MagicMock(return_value=False) + mocker.patch.object(hashivault, 'CertFiles', return_value=mock_cert_files) mocker.patch.object(hashivault, 'raise_for_status') backend = getattr(hashivault, backend_func) @@ -631,7 +637,10 @@ def test_backend_revokes_oidc_token( backend(**backend_kwargs) mock_handle_auth.assert_called_once() - mock_revoke.assert_called_once_with('test_token', **backend_kwargs) + # Verify revocation was attempted + mock_revoke_session.post.assert_called_once() + assert 'auth/token/revoke-self' in mock_revoke_session.post.call_args[0][0] + mock_revoke_response.raise_for_status.assert_called_once() @pytest.mark.parametrize( From c7674ca3e1c979c25c7a47c0fd2cc8aee5a09e04 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Wed, 25 Mar 2026 09:09:43 +0100 Subject: [PATCH 15/39] Address linter issues and restore removed comments --- src/awx_plugins/credentials/hashivault.py | 8 ++++---- tests/unit/credentials/hashivault_test.py | 9 ++++++--- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index f634bebe04..75cca46ecb 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -484,7 +484,7 @@ def workload_identity_auth(**kwargs): @_ctx.contextmanager -def vault_token(**kwargs) -> Generator[str, None, None]: +def vault_token(**kwargs: str) -> Generator[str, None, None]: """Context manager that yields a Vault token and revokes it on exit if obtained via workload identity.""" token = handle_auth(**kwargs) try: @@ -543,7 +543,7 @@ def method_auth(**kwargs): return token -def kv_backend(**kwargs): # noqa: PLR0915 +def kv_backend(**kwargs): with vault_token(**kwargs) as token: secret_path = kwargs['secret_path'] secret_backend = kwargs.get('secret_backend') @@ -565,7 +565,7 @@ def kv_backend(**kwargs): # noqa: PLR0915 if api_version == 'v2': if kwargs.get('secret_version'): - request_kwargs['params'] = { # type: ignore[assignment] # FIXME + request_kwargs['params'] = { # type: ignore[assignment] 'version': kwargs['secret_version'], } if secret_backend: @@ -611,7 +611,7 @@ def kv_backend(**kwargs): # noqa: PLR0915 try: if ( (secret_key != 'data') - and ( # noqa: S105 + and ( # noqa: S105; not a password secret_key not in json['data'] ) and ('data' in json['data']) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 8389713958..2d84603d1d 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -475,8 +475,8 @@ def test_vault_token_revokes_oidc_token( assert token == 'test_token' mock_handle_auth.assert_called_once_with(**kwargs) - for header, contents in expected_headers.items(): - assert mock_session.headers[header] == contents + for header, header_contents in expected_headers.items(): + assert mock_session.headers[header] == header_contents mock_session.post.assert_called_once() assert 'auth/token/revoke-self' in mock_session.post.call_args[0][0] mock_response.raise_for_status.assert_called_once() @@ -513,10 +513,13 @@ def test_vault_token_revoke_failure( 'default_auth_path': 'jwt', } - with pytest.raises(Exception, match='Revocation failed'): + def _use_vault_token() -> None: with hashivault.vault_token(**kwargs) as token: assert token == 'test_token' + with pytest.raises(Exception, match='Revocation failed'): + _use_vault_token() + mock_handle_auth.assert_called_once_with(**kwargs) From f0185c32d7740fc1d484f6044193a732b569b42b Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Thu, 26 Mar 2026 17:43:02 +0100 Subject: [PATCH 16/39] Addressed PR review suggestions - Hidden imports to not clutter global namespace - Added context manager as decorator for kv and ssh backends - Removed the usage of kwargs into explicit function argument names for kv and ssh backends Co-Authored-By: Sviatoslav Sydorenko --- src/awx_plugins/credentials/hashivault.py | 334 +++++++++++++--------- tests/unit/credentials/hashivault_test.py | 12 +- 2 files changed, 206 insertions(+), 140 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 75cca46ecb..91ae48701d 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -2,10 +2,12 @@ # mypy: disable-error-code="arg-type, no-untyped-call, no-untyped-def" import contextlib as _ctx +import functools as _functools import os import pathlib import time -from collections.abc import Generator +import typing as _t +from collections import abc as _abc from urllib.parse import urljoin from awx_plugins.interfaces._temporary_private_django_api import ( # noqa: WPS436 @@ -18,6 +20,10 @@ from .plugin import CertFiles, CredentialPlugin, raise_for_status +class _EmptyKwargs(_t.TypedDict): + """Schema for zero keyword arguments.""" + + # Base input fields url_field: _types.FieldDict = { 'id': 'url', @@ -483,23 +489,68 @@ def workload_identity_auth(**kwargs): return {'role': kwargs.get('jwt_role'), 'jwt': workload_identity_token} +def revoke_self_token( + vault_token: str, + url: str, + namespace: str, + cacert: str, +) -> None: + """Revoke the passed-in Vault token.""" + url = urljoin(url, 'v1/auth/token/revoke-self') + sess = requests.Session() + sess.headers['X-Vault-Token'] = vault_token + if namespace: + sess.headers['X-Vault-Namespace'] = namespace + with CertFiles(cacert) as cert: + resp = sess.post(url, verify=cert, timeout=30) + resp.raise_for_status() + + @_ctx.contextmanager -def vault_token(**kwargs: str) -> Generator[str, None, None]: +def _vault_token(**kwargs: str) -> _abc.Iterator[str]: """Context manager that yields a Vault token and revokes it on exit if obtained via workload identity.""" + is_oidc_context = 'workload_identity_token' in kwargs token = handle_auth(**kwargs) try: yield token finally: - # Only revoke tokens obtained via workload identity authentication - if kwargs.get('workload_identity_token'): - url = urljoin(kwargs['url'], 'v1/auth/token/revoke-self') - sess = requests.Session() - sess.headers['X-Vault-Token'] = token - if kwargs.get('namespace'): - sess.headers['X-Vault-Namespace'] = kwargs['namespace'] - with CertFiles(kwargs.get('cacert')) as cert: - resp = sess.post(url, verify=cert, timeout=30) - resp.raise_for_status() + # Only revoke tokens obtained via OIDC authentication + if is_oidc_context: + revoke_self_token( + vault_token=token, + url=kwargs['url'], + namespace=kwargs['namespace'], + cacert=kwargs['cacert'], + ) + + +def _inject_auth_token_with_revocation[T, **P]( + decorated_function: _t.Callable[P, T], + /, +) -> _t.Callable[P, T]: + @_functools.wraps(decorated_function) + def _decorate_the_function_with_revocation( # noqa: WPS430 -- in-decorator + **kwargs: P.kwargs, + ) -> T: + with _vault_token(**kwargs) as token: + kwargs['token'] = token + return decorated_function(**kwargs) + + # Fools mypy + return _t.cast('_t.Callable[P, T]', _decorate_the_function_with_revocation) + + +# def _inject_auth_token_with_revocation( +# decorated_function: _t.Callable[[str, dict[str, str]], str], +# /, +# ) -> _t.Callable[[dict[str, str]], str]: +# @_functools.wraps(decorated_function) +# def _decorate_the_function_with_revokation(**kwargs: str) -> str: +# with _vault_token(**kwargs) as token: +# return decorated_function(token=token, **kwargs) + +# # Needed to satisfy mypy return type expectations +# return _t.cast(_t.Callable[[dict[str, str]], str], _decorate_the_function_with_revokation) def method_auth(**kwargs): @@ -543,131 +594,146 @@ def method_auth(**kwargs): return token -def kv_backend(**kwargs): - with vault_token(**kwargs) as token: - secret_path = kwargs['secret_path'] - secret_backend = kwargs.get('secret_backend') - secret_key = kwargs.get('secret_key') - api_version = kwargs['api_version'] - - request_kwargs = { - 'timeout': 30, - 'allow_redirects': False, - } - - sess = requests.Session() - sess.mount(kwargs['url'], requests.adapters.HTTPAdapter(max_retries=5)) - sess.headers['Authorization'] = f'Bearer {token}' - # Compatibility header for older installs of Hashicorp Vault - sess.headers['X-Vault-Token'] = token - if kwargs.get('namespace'): - sess.headers['X-Vault-Namespace'] = kwargs['namespace'] - - if api_version == 'v2': - if kwargs.get('secret_version'): - request_kwargs['params'] = { # type: ignore[assignment] - 'version': kwargs['secret_version'], - } - if secret_backend: - path_segments = [secret_backend, 'data', secret_path] - else: - try: - mount_point, *path = pathlib.Path( - secret_path.lstrip(os.sep), - ).parts - except Exception: - mount_point, path = secret_path, [] - # https://www.vaultproject.io/api/secret/kv/kv-v2.html#read-secret-version - path_segments = [mount_point, 'data'] + path - elif secret_backend: - path_segments = [secret_backend, secret_path] - else: - path_segments = [secret_path] +@_inject_auth_token_with_revocation +# NOTE: The "too many args" rules of flake8 and pylint are disabled due to such +# NOTE: many arguments being a common public plugin API at the moment. +# pylint: disable-next=too-many-arguments +def kv_backend( # noqa: WPS211 -- the same as too-many-arguments + *, + token: str, + url: str, + api_version: str, + secret_path: str, + secret_key: str | None = None, + secret_backend: str | None = None, + secret_version: str | None = None, + cacert: str | None = None, + namespace: str | None = None, + **_discarded_kwargs: _t.Unpack[_EmptyKwargs], +) -> str: + request_kwargs = { + 'timeout': 30, + 'allow_redirects': False, + } - request_url = urljoin( - kwargs['url'], - '/'.join(['v1'] + path_segments), - ).rstrip( - '/', - ) - with CertFiles(kwargs.get('cacert')) as cert: - request_kwargs['verify'] = cert - request_retries = 0 - while request_retries < 5: - response = sess.get(request_url, **request_kwargs) - # https://developer.hashicorp.com/vault/docs/enterprise/consistency - if response.status_code == 412: - request_retries += 1 - time.sleep(1) - else: - break - raise_for_status(response) - - json = response.json() - if api_version == 'v2': - json = json['data'] - - if secret_key: + sess = requests.Session() + sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) + sess.headers['Authorization'] = f'Bearer {token}' + # Compatibility header for older installs of Hashicorp Vault + sess.headers['X-Vault-Token'] = token + if namespace: + sess.headers['X-Vault-Namespace'] = namespace + + if api_version == 'v2': + if secret_version: + request_kwargs['params'] = { # type: ignore[assignment] + 'version': secret_version, + } + if secret_backend: + path_segments = [secret_backend, 'data', secret_path] + else: try: - if ( - (secret_key != 'data') - and ( # noqa: S105; not a password - secret_key not in json['data'] - ) - and ('data' in json['data']) - ): - return json['data']['data'][secret_key] - return json['data'][secret_key] - except KeyError: - raise RuntimeError( - f'{secret_key} is not present at {secret_path}', + mount_point, *path = pathlib.Path( + secret_path.lstrip(os.sep), + ).parts + except Exception: + mount_point, path = secret_path, [] + # https://www.vaultproject.io/api/secret/kv/kv-v2.html#read-secret-version + path_segments = [mount_point, 'data'] + path + elif secret_backend: + path_segments = [secret_backend, secret_path] + else: + path_segments = [secret_path] + + request_url = urljoin(url, '/'.join(['v1'] + path_segments)).rstrip('/') + with CertFiles(cacert) as cert: + request_kwargs['verify'] = cert + request_retries = 0 + while request_retries < 5: + response = sess.get(request_url, **request_kwargs) + # https://developer.hashicorp.com/vault/docs/enterprise/consistency + if response.status_code == 412: + request_retries += 1 + time.sleep(1) + else: + break + raise_for_status(response) + + json = response.json() + if api_version == 'v2': + json = json['data'] + + if secret_key: + try: + if ( + (secret_key != 'data') + and ( # noqa: S105; not a password + secret_key not in json['data'] ) - return json['data'] - - -def ssh_backend(**kwargs): - with vault_token(**kwargs) as token: - url = urljoin(kwargs['url'], 'v1') - secret_path = kwargs['secret_path'] - role = kwargs['role'] - cacert = kwargs.get('cacert') - - request_kwargs = { - 'timeout': 30, - 'allow_redirects': False, - } - - request_kwargs['json'] = { # type: ignore[assignment] # FIXME - 'public_key': kwargs['public_key'], - } - if kwargs.get('valid_principals'): - request_kwargs['json'][ # type: ignore[index] # FIXME - 'valid_principals' - ] = kwargs['valid_principals'] - - sess = requests.Session() - sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) - sess.headers['Authorization'] = f'Bearer {token}' - if kwargs.get('namespace'): - sess.headers['X-Vault-Namespace'] = kwargs['namespace'] - # Compatibility header for older installs of Hashicorp Vault - sess.headers['X-Vault-Token'] = token - # https://www.vaultproject.io/api/secret/ssh/index.html#sign-ssh-key - request_url = '/'.join([url, secret_path, 'sign', role]).rstrip('/') - - with CertFiles(cacert) as cert: - request_kwargs['verify'] = cert - request_retries = 0 - while request_retries < 5: - resp = sess.post(request_url, **request_kwargs) - # https://developer.hashicorp.com/vault/docs/enterprise/consistency - if resp.status_code == 412: - request_retries += 1 - time.sleep(1) - else: - break - raise_for_status(resp) - return resp.json()['data']['signed_key'] + and ('data' in json['data']) + ): + return str(json['data']['data'][secret_key]) + return str(json['data'][secret_key]) + except KeyError: + raise RuntimeError( + f'{secret_key} is not present at {secret_path}', + ) + return str(json['data']) + + +@_inject_auth_token_with_revocation +# NOTE: The "too many args" rules of flake8 and pylint are disabled due to such +# NOTE: many arguments being a common public plugin API at the moment. +# pylint: disable-next=too-many-arguments +def ssh_backend( # noqa: WPS211 -- the same as too-many-arguments + *, + token: str, + url: str, + secret_path: str, + role: str, + public_key: str, + cacert: str | None = None, + namespace: str | None = None, + valid_principals: str | None = None, + **_discarded_kwargs: _t.Unpack[_EmptyKwargs], +) -> str: + url = urljoin(url, 'v1') + + request_kwargs = { + 'timeout': 30, + 'allow_redirects': False, + 'json': { + 'public_key': public_key, + }, + } + if valid_principals: + request_kwargs['json'][ # type: ignore[index] # FIXME + 'valid_principals' + ] = valid_principals + + sess = requests.Session() + sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) + sess.headers['Authorization'] = f'Bearer {token}' + if namespace: + sess.headers['X-Vault-Namespace'] = namespace + # Compatibility header for older installs of Hashicorp Vault + sess.headers['X-Vault-Token'] = token + # https://www.vaultproject.io/api/secret/ssh/index.html#sign-ssh-key + request_url = '/'.join([url, secret_path, 'sign', role]).rstrip('/') + + with CertFiles(cacert) as cert: + request_kwargs['verify'] = cert + request_retries = 0 + while request_retries < 5: + resp = sess.post(request_url, **request_kwargs) + # https://developer.hashicorp.com/vault/docs/enterprise/consistency + if resp.status_code == 412: + request_retries += 1 + time.sleep(1) + else: + break + raise_for_status(resp) + return str(resp.json()['data']['signed_key']) hashivault_kv_plugin = CredentialPlugin( diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 2d84603d1d..c76bb8292c 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -399,7 +399,7 @@ def test_non_oidc_plugins_have_no_internal_fields( def test_vault_token_no_workload_identity( mocker: MockerFixture, ) -> None: - """Test ``vault_token`` context manager doesn't revoke token without workload_identity_token.""" + """Test ``_vault_token`` context manager doesn't revoke token without workload_identity_token.""" mock_handle_auth = mocker.patch.object( hashivault, 'handle_auth', @@ -413,7 +413,7 @@ def test_vault_token_no_workload_identity( 'token': 'test_token', } - with hashivault.vault_token(**kwargs) as token: + with hashivault._vault_token(**kwargs) as token: assert token == 'test_token' mock_handle_auth.assert_called_once_with(**kwargs) @@ -443,7 +443,7 @@ def test_vault_token_revokes_oidc_token( extra_kwargs: dict[str, str], expected_headers: dict[str, str], ) -> None: - """Test ``vault_token`` context manager revokes token for workload identity auth.""" + """Test ``_vault_token`` context manager revokes token for workload identity auth.""" mock_handle_auth = mocker.patch.object( hashivault, 'handle_auth', @@ -471,7 +471,7 @@ def test_vault_token_revokes_oidc_token( **extra_kwargs, } - with hashivault.vault_token(**kwargs) as token: + with hashivault._vault_token(**kwargs) as token: assert token == 'test_token' mock_handle_auth.assert_called_once_with(**kwargs) @@ -485,7 +485,7 @@ def test_vault_token_revokes_oidc_token( def test_vault_token_revoke_failure( mocker: MockerFixture, ) -> None: - """Test ``vault_token`` context manager raises when token revocation fails.""" + """Test ``_vault_token`` context manager raises when token revocation fails.""" mock_handle_auth = mocker.patch.object( hashivault, 'handle_auth', @@ -514,7 +514,7 @@ def test_vault_token_revoke_failure( } def _use_vault_token() -> None: - with hashivault.vault_token(**kwargs) as token: + with hashivault._vault_token(**kwargs) as token: assert token == 'test_token' with pytest.raises(Exception, match='Revocation failed'): From 97c080a5c5ab69713be66d7cab994d059f3bd81a Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Fri, 27 Mar 2026 08:51:37 +0100 Subject: [PATCH 17/39] Fix typevar definition in decorator function for mypy --- src/awx_plugins/credentials/hashivault.py | 35 +++++++++-------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 91ae48701d..850da6388a 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -524,33 +524,26 @@ def _vault_token(**kwargs: str) -> _abc.Iterator[str]: ) -def _inject_auth_token_with_revocation[T, **P]( - decorated_function: _t.Callable[P, T], +# Param Spec to represent decorated function parameters +_PT = _t.ParamSpec('_PT') +# TypeVar to represent decorated function return type +_RT = _t.TypeVar('_RT') + + +def _inject_auth_token_with_revocation( + decorated_function: _t.Callable[_PT, _RT], /, -) -> _t.Callable[P, T]: +) -> _t.Callable[_PT, _RT]: @_functools.wraps(decorated_function) def _decorate_the_function_with_revocation( # noqa: WPS430 -- in-decorator - **kwargs: P.kwargs, - ) -> T: + *args: _PT.args, + **kwargs: _PT.kwargs, + ) -> _RT: with _vault_token(**kwargs) as token: kwargs['token'] = token - return decorated_function(**kwargs) - - # Fools mypy - return _t.cast('_t.Callable[P, T]', _decorate_the_function_with_revocation) - - -# def _inject_auth_token_with_revocation( -# decorated_function: _t.Callable[[str, dict[str, str]], str], -# /, -# ) -> _t.Callable[[dict[str, str]], str]: -# @_functools.wraps(decorated_function) -# def _decorate_the_function_with_revokation(**kwargs: str) -> str: -# with _vault_token(**kwargs) as token: -# return decorated_function(token=token, **kwargs) + return decorated_function(*args, **kwargs) -# # Needed to satisfy mypy return type expectations -# return _t.cast(_t.Callable[[dict[str, str]], str], _decorate_the_function_with_revokation) + return _decorate_the_function_with_revocation def method_auth(**kwargs): From cb1dc4f59b7f718f85b5282c28a62f85a47c785e Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Fri, 27 Mar 2026 09:15:07 +0100 Subject: [PATCH 18/39] Made utility revoke_self_token function private --- src/awx_plugins/credentials/hashivault.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 850da6388a..e19a5ba61d 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -489,7 +489,7 @@ def workload_identity_auth(**kwargs): return {'role': kwargs.get('jwt_role'), 'jwt': workload_identity_token} -def revoke_self_token( +def _revoke_self_token( vault_token: str, url: str, namespace: str, @@ -516,7 +516,7 @@ def _vault_token(**kwargs: str) -> _abc.Iterator[str]: finally: # Only revoke tokens obtained via OIDC authentication if is_oidc_context: - revoke_self_token( + _revoke_self_token( vault_token=token, url=kwargs['url'], namespace=kwargs['namespace'], From e66bbd84099cc4ec11538ac8a30fc236f491cfc1 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Fri, 27 Mar 2026 09:33:07 +0100 Subject: [PATCH 19/39] Set default value for namespace and cacert calling revoke_self_token --- src/awx_plugins/credentials/hashivault.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index e19a5ba61d..66b9089a53 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -493,13 +493,13 @@ def _revoke_self_token( vault_token: str, url: str, namespace: str, - cacert: str, + cacert: str | None = None, ) -> None: """Revoke the passed-in Vault token.""" url = urljoin(url, 'v1/auth/token/revoke-self') sess = requests.Session() sess.headers['X-Vault-Token'] = vault_token - if namespace: + if namespace != '': sess.headers['X-Vault-Namespace'] = namespace with CertFiles(cacert) as cert: resp = sess.post(url, verify=cert, timeout=30) @@ -519,8 +519,8 @@ def _vault_token(**kwargs: str) -> _abc.Iterator[str]: _revoke_self_token( vault_token=token, url=kwargs['url'], - namespace=kwargs['namespace'], - cacert=kwargs['cacert'], + namespace=kwargs.get('namespace', ''), + cacert=kwargs.get('cacert'), ) From a1beb25ed1b7616437c8b566ff0a8982fc310da5 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Fri, 27 Mar 2026 09:35:46 +0100 Subject: [PATCH 20/39] Use autospec in the test mocks --- tests/unit/credentials/hashivault_test.py | 68 +++++++++++------------ 1 file changed, 33 insertions(+), 35 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index c76bb8292c..8b5e15c61a 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -403,10 +403,10 @@ def test_vault_token_no_workload_identity( mock_handle_auth = mocker.patch.object( hashivault, 'handle_auth', + autospec=True, return_value='test_token', ) - mock_session = mocker.MagicMock() - mocker.patch('requests.Session', return_value=mock_session) + mock_session = mocker.patch('requests.Session', autospec=True) kwargs = { 'url': 'https://vault.example.com', @@ -417,7 +417,7 @@ def test_vault_token_no_workload_identity( assert token == 'test_token' mock_handle_auth.assert_called_once_with(**kwargs) - mock_session.post.assert_not_called() + mock_session.return_value.post.assert_not_called() @pytest.mark.parametrize( @@ -447,21 +447,18 @@ def test_vault_token_revokes_oidc_token( mock_handle_auth = mocker.patch.object( hashivault, 'handle_auth', + autospec=True, return_value='test_token', ) - mock_session = mocker.MagicMock() + mock_session_class = mocker.patch('requests.Session', autospec=True) + mock_session = mock_session_class.return_value mock_session.headers = {} - mock_response = mocker.MagicMock() - mock_session.post.return_value = mock_response - mocker.patch('requests.Session', return_value=mock_session) - mock_cert_files = mocker.MagicMock() - mock_cert_files.__enter__ = mocker.MagicMock(return_value='cert_path') - mock_cert_files.__exit__ = mocker.MagicMock(return_value=False) - mocker.patch.object( + mock_cert_files = mocker.patch.object( hashivault, 'CertFiles', - return_value=mock_cert_files, - ) + autospec=True, + ).return_value + mock_cert_files.__enter__.return_value = 'cert_path' kwargs = { 'url': 'https://vault.example.com', @@ -479,7 +476,7 @@ def test_vault_token_revokes_oidc_token( assert mock_session.headers[header] == header_contents mock_session.post.assert_called_once() assert 'auth/token/revoke-self' in mock_session.post.call_args[0][0] - mock_response.raise_for_status.assert_called_once() + mock_session.post.return_value.raise_for_status.assert_called_once() def test_vault_token_revoke_failure( @@ -489,22 +486,21 @@ def test_vault_token_revoke_failure( mock_handle_auth = mocker.patch.object( hashivault, 'handle_auth', + autospec=True, return_value='test_token', ) - mock_session = mocker.MagicMock() + mock_session_class = mocker.patch('requests.Session', autospec=True) + mock_session = mock_session_class.return_value mock_session.headers = {} - mock_response = mocker.MagicMock() - mock_response.raise_for_status.side_effect = Exception('Revocation failed') - mock_session.post.return_value = mock_response - mocker.patch('requests.Session', return_value=mock_session) - mock_cert_files = mocker.MagicMock() - mock_cert_files.__enter__ = mocker.MagicMock(return_value='cert_path') - mock_cert_files.__exit__ = mocker.MagicMock(return_value=False) - mocker.patch.object( + mock_session.post.return_value.raise_for_status.side_effect = Exception( + 'Revocation failed', + ) + mock_cert_files = mocker.patch.object( hashivault, 'CertFiles', - return_value=mock_cert_files, - ) + autospec=True, + ).return_value + mock_cert_files.__enter__.return_value = 'cert_path' kwargs = { 'url': 'https://vault.example.com', @@ -615,24 +611,26 @@ def test_backend_revokes_oidc_token( mock_handle_auth = mocker.patch.object( hashivault, 'handle_auth', + autospec=True, return_value='test_token', ) - mock_get_or_post_session = mocker.MagicMock() - mock_revoke_session = mocker.MagicMock() + mock_get_or_post_session = mocker.Mock() + mock_revoke_session = mocker.Mock() mock_revoke_session.headers = {} - mock_revoke_response = mocker.MagicMock() - mock_revoke_session.post.return_value = mock_revoke_response # requests.Session is called twice: once for secret fetch, once for revoke mocker.patch( 'requests.Session', + autospec=True, side_effect=[mock_get_or_post_session, mock_revoke_session], ) - mock_cert_files = mocker.MagicMock() - mock_cert_files.__enter__ = mocker.MagicMock(return_value='cert_path') - mock_cert_files.__exit__ = mocker.MagicMock(return_value=False) - mocker.patch.object(hashivault, 'CertFiles', return_value=mock_cert_files) - mocker.patch.object(hashivault, 'raise_for_status') + mock_cert_files = mocker.patch.object( + hashivault, + 'CertFiles', + autospec=True, + ).return_value + mock_cert_files.__enter__.return_value = 'cert_path' + mocker.patch.object(hashivault, 'raise_for_status', autospec=True) backend = getattr(hashivault, backend_func) @@ -643,7 +641,7 @@ def test_backend_revokes_oidc_token( # Verify revocation was attempted mock_revoke_session.post.assert_called_once() assert 'auth/token/revoke-self' in mock_revoke_session.post.call_args[0][0] - mock_revoke_response.raise_for_status.assert_called_once() + mock_revoke_session.post.return_value.raise_for_status.assert_called_once() @pytest.mark.parametrize( From 1758c9bceddff9d5ffeec4b146f69c2285a2cf2c Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Fri, 27 Mar 2026 09:55:59 +0100 Subject: [PATCH 21/39] Remove suppress exception context --- tests/unit/credentials/hashivault_test.py | 33 ++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 8b5e15c61a..6ae31324ce 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -1,6 +1,5 @@ """Tests for HashiCorp Vault credential plugins.""" -import contextlib import typing as _t import pytest @@ -615,6 +614,34 @@ def test_backend_revokes_oidc_token( return_value='test_token', ) mock_get_or_post_session = mocker.Mock() + mock_get_or_post_session.headers = {} + + # Configure mock response for secret fetch based on backend type + mock_secret_response = mocker.Mock() + if backend_func == 'kv_backend': + api_version = extra_kwargs.get('api_version', 'v1') + # kv_backend v1 expects {'data': {secret_key: value}} + mock_secret_response.json.return_value = { + 'data': {'password': 'test_value'}, + } + if api_version == 'v2': + # kv_backend v2 expects {'data': {'data': {secret_key: value}}} + mock_secret_response.json.return_value = { + 'data': mock_secret_response.json.return_value, + } + else: # ssh_backend + # ssh_backend expects {'data': {'signed_key': value}} + mock_secret_response.json.return_value = { + 'data': {'signed_key': 'test_signed_key'}, + } + mock_secret_response.status_code = 200 + + # Configure session mock based on backend type + if backend_func == 'kv_backend': + mock_get_or_post_session.get.return_value = mock_secret_response + else: # ssh_backend + mock_get_or_post_session.post.return_value = mock_secret_response + mock_revoke_session = mocker.Mock() mock_revoke_session.headers = {} @@ -634,8 +661,8 @@ def test_backend_revokes_oidc_token( backend = getattr(hashivault, backend_func) - with contextlib.suppress(Exception): - backend(**backend_kwargs) + # Call the backend function to retrieve the secret + backend(**backend_kwargs) mock_handle_auth.assert_called_once() # Verify revocation was attempted From 58ba8ac96be9eacb886666a471f4d94beb4b7a01 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Fri, 27 Mar 2026 10:07:55 +0100 Subject: [PATCH 22/39] Use backend function reference instead of getattr lookup in tests --- tests/unit/credentials/hashivault_test.py | 69 +++++++++++++---------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 6ae31324ce..c853aab199 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -522,68 +522,89 @@ def _use_vault_token() -> None: ('backend_func', 'extra_kwargs'), ( pytest.param( - 'kv_backend', - {'api_version': 'v1', 'secret_key': 'password'}, + hashivault.kv_backend, + { + 'api_version': 'v1', + 'secret_key': 'password', + 'secret_path': '/secret/path', + }, id='kv-backend-v1', ), pytest.param( - 'kv_backend', - {'api_version': 'v2', 'secret_key': 'password'}, + hashivault.kv_backend, + { + 'api_version': 'v2', + 'secret_key': 'password', + 'secret_path': '/secret/path', + }, id='kv-backend-v2', ), pytest.param( - 'kv_backend', + hashivault.kv_backend, { 'api_version': 'v2', 'secret_key': 'password', 'namespace': 'test-namespace', + 'secret_path': '/secret/path', }, id='kv-backend-v2-with-namespace', ), pytest.param( - 'kv_backend', + hashivault.kv_backend, { 'api_version': 'v2', 'secret_key': 'password', 'secret_version': '3', + 'secret_path': '/secret/path', }, id='kv-backend-v2-with-secret-version', ), pytest.param( - 'kv_backend', + hashivault.kv_backend, { 'api_version': 'v2', 'secret_key': 'password', 'secret_backend': 'kv', + 'secret_path': '/secret/path', }, id='kv-backend-v2-with-secret-backend', ), pytest.param( - 'kv_backend', + hashivault.kv_backend, { 'api_version': 'v2', 'secret_key': 'password', 'namespace': 'test-namespace', 'secret_backend': 'kv', 'secret_version': '5', + 'secret_path': '/secret/path', }, id='kv-backend-v2-with-all-params', ), pytest.param( - 'ssh_backend', - {}, + hashivault.ssh_backend, + { + 'secret_path': '/ssh', + 'role': 'test_ssh_role', + 'public_key': 'ssh-rsa AAAAB...', + }, id='ssh-backend', ), pytest.param( - 'ssh_backend', - {'namespace': 'test-namespace'}, + hashivault.ssh_backend, + { + 'namespace': 'test-namespace', + 'secret_path': '/ssh', + 'role': 'test_ssh_role', + 'public_key': 'ssh-rsa AAAAB...', + }, id='ssh-backend-with-namespace', ), ), ) def test_backend_revokes_oidc_token( mocker: MockerFixture, - backend_func: str, + backend_func: _t.Callable[[dict[str, str]], str], extra_kwargs: dict[str, str], ) -> None: """Test backend functions revoke token via context manager for OIDC auth.""" @@ -595,17 +616,7 @@ def test_backend_revokes_oidc_token( 'default_auth_path': 'jwt', } - # Backend-specific kwargs - if backend_func == 'kv_backend': - backend_specific = {'secret_path': '/secret/path'} - else: # ssh_backend - backend_specific = { - 'secret_path': '/ssh', - 'role': 'test_ssh_role', - 'public_key': 'ssh-rsa AAAAB...', - } - - backend_kwargs = {**base_kwargs, **backend_specific, **extra_kwargs} + backend_kwargs = {**base_kwargs, **extra_kwargs} mock_handle_auth = mocker.patch.object( hashivault, @@ -618,7 +629,7 @@ def test_backend_revokes_oidc_token( # Configure mock response for secret fetch based on backend type mock_secret_response = mocker.Mock() - if backend_func == 'kv_backend': + if backend_func.__name__ == 'kv_backend': api_version = extra_kwargs.get('api_version', 'v1') # kv_backend v1 expects {'data': {secret_key: value}} mock_secret_response.json.return_value = { @@ -637,7 +648,7 @@ def test_backend_revokes_oidc_token( mock_secret_response.status_code = 200 # Configure session mock based on backend type - if backend_func == 'kv_backend': + if backend_func.__name__ == 'kv_backend': mock_get_or_post_session.get.return_value = mock_secret_response else: # ssh_backend mock_get_or_post_session.post.return_value = mock_secret_response @@ -659,10 +670,10 @@ def test_backend_revokes_oidc_token( mock_cert_files.__enter__.return_value = 'cert_path' mocker.patch.object(hashivault, 'raise_for_status', autospec=True) - backend = getattr(hashivault, backend_func) - # Call the backend function to retrieve the secret - backend(**backend_kwargs) + # Adding ignore[call-arg] since adding the explicit args to the backend + # functions makes the linter trigger the "too few arguments supplied" + backend_func(**backend_kwargs) # type: ignore[call-arg] mock_handle_auth.assert_called_once() # Verify revocation was attempted From 705a52798ad7d72fbfa5d938c0b5b7aa46e7d766 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Fri, 27 Mar 2026 10:11:57 +0100 Subject: [PATCH 23/39] Use less generic name in a test kwargs var --- tests/unit/credentials/hashivault_test.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index c853aab199..3d1bc3c446 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -407,15 +407,15 @@ def test_vault_token_no_workload_identity( ) mock_session = mocker.patch('requests.Session', autospec=True) - kwargs = { + vault_token_kwargs = { 'url': 'https://vault.example.com', 'token': 'test_token', } - with hashivault._vault_token(**kwargs) as token: + with hashivault._vault_token(**vault_token_kwargs) as token: assert token == 'test_token' - mock_handle_auth.assert_called_once_with(**kwargs) + mock_handle_auth.assert_called_once_with(**vault_token_kwargs) mock_session.return_value.post.assert_not_called() From 942d309f892dec811d45f58422675c6795f1c82e Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Fri, 27 Mar 2026 10:20:13 +0100 Subject: [PATCH 24/39] Use a more realistic exception being raised in tests --- tests/unit/credentials/hashivault_test.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 3d1bc3c446..128c9900eb 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -5,6 +5,8 @@ import pytest from pytest_mock import MockerFixture +import requests as _requests + from awx_plugins.credentials import _types, hashivault from awx_plugins.credentials.plugin import CredentialPlugin @@ -491,8 +493,8 @@ def test_vault_token_revoke_failure( mock_session_class = mocker.patch('requests.Session', autospec=True) mock_session = mock_session_class.return_value mock_session.headers = {} - mock_session.post.return_value.raise_for_status.side_effect = Exception( - 'Revocation failed', + mock_session.post.return_value.raise_for_status.side_effect = ( + _requests.HTTPError('403 Client Error: Forbidden for url: ...') ) mock_cert_files = mocker.patch.object( hashivault, @@ -512,7 +514,7 @@ def _use_vault_token() -> None: with hashivault._vault_token(**kwargs) as token: assert token == 'test_token' - with pytest.raises(Exception, match='Revocation failed'): + with pytest.raises(_requests.HTTPError, match='403 Client Error'): _use_vault_token() mock_handle_auth.assert_called_once_with(**kwargs) From c14ad50145d40869c790070ffa2db75013812347 Mon Sep 17 00:00:00 2001 From: Daniel Finca Date: Fri, 27 Mar 2026 10:25:11 +0100 Subject: [PATCH 25/39] Add back lost comment --- src/awx_plugins/credentials/hashivault.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 66b9089a53..fc8dcf9357 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -619,7 +619,7 @@ def kv_backend( # noqa: WPS211 -- the same as too-many-arguments if api_version == 'v2': if secret_version: - request_kwargs['params'] = { # type: ignore[assignment] + request_kwargs['params'] = { # type: ignore[assignment] # FIXME 'version': secret_version, } if secret_backend: From 60ad1c82cd39d06a18a6e71010bf8c30e77f47fc Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Fri, 27 Mar 2026 20:29:11 +0100 Subject: [PATCH 26/39] =?UTF-8?q?=F0=9F=93=9D=20Record=20the=20detached=20?= =?UTF-8?q?param=20spec=20justification?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/awx_plugins/credentials/hashivault.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index fc8dcf9357..a5a569e0cf 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -525,7 +525,9 @@ def _vault_token(**kwargs: str) -> _abc.Iterator[str]: # Param Spec to represent decorated function parameters -_PT = _t.ParamSpec('_PT') +_PT = _t.ParamSpec( # FIXME: Use [_RT, **_PT] in the signature in Python 3.12 + '_PT', +) # TypeVar to represent decorated function return type _RT = _t.TypeVar('_RT') From d4fd37526cae8f1470d76507d7dbdfe40c183d3d Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Fri, 27 Mar 2026 20:31:43 +0100 Subject: [PATCH 27/39] =?UTF-8?q?=E2=9C=A8=20Enforce=20`=5Frevoke=5Fself?= =?UTF-8?q?=5Ftoken`=20to=20be=20kwarg-only?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is the actual use internally and so we shouldn't allow a mix of invocation styles for consistency. --- src/awx_plugins/credentials/hashivault.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index a5a569e0cf..a2a3791edc 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -490,6 +490,7 @@ def workload_identity_auth(**kwargs): def _revoke_self_token( + *, vault_token: str, url: str, namespace: str, From 2832592447540aef1898bc3c4fdf5704b99ad389 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Fri, 27 Mar 2026 20:33:26 +0100 Subject: [PATCH 28/39] =?UTF-8?q?=F0=9F=92=85=20Pass=20token=20injection?= =?UTF-8?q?=20in=20decorator=20explicitly?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, it was passed through mutating an unpacked dict but it's more readable to pass it into the call directly. --- src/awx_plugins/credentials/hashivault.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index a2a3791edc..0049c03fe4 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -543,8 +543,7 @@ def _decorate_the_function_with_revocation( # noqa: WPS430 -- in-decorator **kwargs: _PT.kwargs, ) -> _RT: with _vault_token(**kwargs) as token: - kwargs['token'] = token - return decorated_function(*args, **kwargs) + return decorated_function(*args, token=token, **kwargs) return _decorate_the_function_with_revocation From 105f3c03f58e427acf41d849c9a1256dff6fc887 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Fri, 27 Mar 2026 20:35:14 +0100 Subject: [PATCH 29/39] =?UTF-8?q?=F0=9F=92=85=20Concat=20token=20type=20in?= =?UTF-8?q?=20decorated=20callable=20def?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The stdlib docs suggest that this is the way but this somehow does not influence how MyPy processes typing. --- src/awx_plugins/credentials/hashivault.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 0049c03fe4..40d4a549ab 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -534,7 +534,7 @@ def _vault_token(**kwargs: str) -> _abc.Iterator[str]: def _inject_auth_token_with_revocation( - decorated_function: _t.Callable[_PT, _RT], + decorated_function: _t.Callable[_t.Concatenate[str, _PT], _RT], /, ) -> _t.Callable[_PT, _RT]: @_functools.wraps(decorated_function) From 39eecf0a24dddb0dfcf81571aa3ddb98c9df723b Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Mon, 30 Mar 2026 16:53:07 +0200 Subject: [PATCH 30/39] =?UTF-8?q?=F0=9F=A7=AA=20Let=20MyPy=20know=20that?= =?UTF-8?q?=20`token`=20is=20internal?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PEP 612 defines `typing.ParamSpec` but leaves out the problem of using `typing.Concatenate` for declaring keyword-only arguments that would not be considered a part of a decorator's generic keyword arguments as out of the scope [[1]]. This means that we cannot annotate decorators that add or remove arguments when wrapping decorated functions accurately. We, however, can attempt using type narrowing to let MyPy know what's definitely not expected to be passed into the wrapper function, which this patch does through the `assert` statements. [1]: https://peps.python.org/pep-0612/#concatenating-keyword-parameters --- src/awx_plugins/credentials/hashivault.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index 40d4a549ab..e925913b11 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -542,6 +542,9 @@ def _decorate_the_function_with_revocation( # noqa: WPS430 -- in-decorator *args: _PT.args, **kwargs: _PT.kwargs, ) -> _RT: + assert not args + assert 'token' not in kwargs + with _vault_token(**kwargs) as token: return decorated_function(*args, token=token, **kwargs) From 4f6b847e8b945ea3aaadef8900a2446c6d4ba56a Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Mon, 30 Mar 2026 17:13:57 +0200 Subject: [PATCH 31/39] =?UTF-8?q?=F0=9F=A7=AA=20Patch=20`requests.Session`?= =?UTF-8?q?=20though=20actual=20module?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/unit/credentials/hashivault_test.py | 32 ++++++++++++++++------- 1 file changed, 23 insertions(+), 9 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 128c9900eb..655ea86bf7 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -5,8 +5,6 @@ import pytest from pytest_mock import MockerFixture -import requests as _requests - from awx_plugins.credentials import _types, hashivault from awx_plugins.credentials.plugin import CredentialPlugin @@ -407,7 +405,11 @@ def test_vault_token_no_workload_identity( autospec=True, return_value='test_token', ) - mock_session = mocker.patch('requests.Session', autospec=True) + mock_session = mocker.patch.object( + hashivault.requests, + 'Session', + autospec=True, + ) vault_token_kwargs = { 'url': 'https://vault.example.com', @@ -451,7 +453,11 @@ def test_vault_token_revokes_oidc_token( autospec=True, return_value='test_token', ) - mock_session_class = mocker.patch('requests.Session', autospec=True) + mock_session_class = mocker.patch.object( + hashivault.requests, + 'Session', + autospec=True, + ) mock_session = mock_session_class.return_value mock_session.headers = {} mock_cert_files = mocker.patch.object( @@ -490,11 +496,15 @@ def test_vault_token_revoke_failure( autospec=True, return_value='test_token', ) - mock_session_class = mocker.patch('requests.Session', autospec=True) + mock_session_class = mocker.patch.object( + hashivault.requests, + 'Session', + autospec=True, + ) mock_session = mock_session_class.return_value mock_session.headers = {} mock_session.post.return_value.raise_for_status.side_effect = ( - _requests.HTTPError('403 Client Error: Forbidden for url: ...') + hashivault.requests.HTTPError('403 Client Error: Forbidden for url: ...') ) mock_cert_files = mocker.patch.object( hashivault, @@ -514,7 +524,10 @@ def _use_vault_token() -> None: with hashivault._vault_token(**kwargs) as token: assert token == 'test_token' - with pytest.raises(_requests.HTTPError, match='403 Client Error'): + with pytest.raises( + hashivault.requests.HTTPError, + match='403 Client Error', + ): _use_vault_token() mock_handle_auth.assert_called_once_with(**kwargs) @@ -659,8 +672,9 @@ def test_backend_revokes_oidc_token( mock_revoke_session.headers = {} # requests.Session is called twice: once for secret fetch, once for revoke - mocker.patch( - 'requests.Session', + mocker.patch.object( + hashivault.requests, + 'Session', autospec=True, side_effect=[mock_get_or_post_session, mock_revoke_session], ) From bff3686ffc2290c6a012d8f6eeb1e5829c659ccf Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Mon, 30 Mar 2026 18:18:11 +0200 Subject: [PATCH 32/39] =?UTF-8?q?=F0=9F=A7=AA=F0=9F=92=85=20Move=20most=20?= =?UTF-8?q?in-test=20mock=20defs=20into=20fixtures?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/unit/credentials/hashivault_test.py | 133 ++++++++++------------ 1 file changed, 62 insertions(+), 71 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 655ea86bf7..bd4e0d218a 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -2,6 +2,11 @@ import typing as _t +# NOTE: The forbidden import here is only used for typing, which warrants +# NOTE: suppressing the respective pylint and ruff rules. +# pylint: disable-next=deprecated-module,preferred-module +from unittest import mock as _unittest_mock # noqa: TID251 -- forbidden import + import pytest from pytest_mock import MockerFixture @@ -395,22 +400,48 @@ def test_non_oidc_plugins_have_no_internal_fields( assert internal_fields == [] -def test_vault_token_no_workload_identity( - mocker: MockerFixture, -) -> None: - """Test ``_vault_token`` context manager doesn't revoke token without workload_identity_token.""" - mock_handle_auth = mocker.patch.object( +@pytest.fixture +def handle_auth_mock(mocker: MockerFixture) -> object: + """Make a mocked ``handle_auth`` callable returning 'test_token'.""" + return mocker.patch.object( hashivault, 'handle_auth', autospec=True, return_value='test_token', ) - mock_session = mocker.patch.object( + + +@pytest.fixture +def session_class_mock(mocker: MockerFixture) -> object: + """Make a mocked ``requests.Session`` class dummy.""" + return mocker.patch.object( hashivault.requests, 'Session', autospec=True, ) + +@pytest.fixture +def _cert_files_mock(mocker: MockerFixture) -> None: + """Replace ``CertFiles`` class with a dummy.""" + mocker.patch.object( + hashivault, + 'CertFiles', + autospec=True, + ).return_value.__enter__.return_value = 'cert_path' + + +@pytest.fixture +def _suppress_raise_for_status(mocker: MockerFixture) -> None: + """Suppress ``requests``' HTTP return code checks.""" + mocker.patch.object(hashivault, 'raise_for_status', autospec=True) + + +def test_vault_token_no_workload_identity( + handle_auth_mock: _unittest_mock.Mock, + session_class_mock: _unittest_mock.Mock, +) -> None: + """Test ``_vault_token`` context manager doesn't revoke token without workload_identity_token.""" vault_token_kwargs = { 'url': 'https://vault.example.com', 'token': 'test_token', @@ -419,8 +450,8 @@ def test_vault_token_no_workload_identity( with hashivault._vault_token(**vault_token_kwargs) as token: assert token == 'test_token' - mock_handle_auth.assert_called_once_with(**vault_token_kwargs) - mock_session.return_value.post.assert_not_called() + handle_auth_mock.assert_called_once_with(**vault_token_kwargs) + session_class_mock.return_value.post.assert_not_called() @pytest.mark.parametrize( @@ -441,31 +472,16 @@ def test_vault_token_no_workload_identity( ), ), ) +@pytest.mark.usefixtures('_cert_files_mock') def test_vault_token_revokes_oidc_token( - mocker: MockerFixture, + handle_auth_mock: _unittest_mock.Mock, + session_class_mock: _unittest_mock.Mock, extra_kwargs: dict[str, str], expected_headers: dict[str, str], ) -> None: """Test ``_vault_token`` context manager revokes token for workload identity auth.""" - mock_handle_auth = mocker.patch.object( - hashivault, - 'handle_auth', - autospec=True, - return_value='test_token', - ) - mock_session_class = mocker.patch.object( - hashivault.requests, - 'Session', - autospec=True, - ) - mock_session = mock_session_class.return_value + mock_session = session_class_mock.return_value mock_session.headers = {} - mock_cert_files = mocker.patch.object( - hashivault, - 'CertFiles', - autospec=True, - ).return_value - mock_cert_files.__enter__.return_value = 'cert_path' kwargs = { 'url': 'https://vault.example.com', @@ -478,7 +494,7 @@ def test_vault_token_revokes_oidc_token( with hashivault._vault_token(**kwargs) as token: assert token == 'test_token' - mock_handle_auth.assert_called_once_with(**kwargs) + handle_auth_mock.assert_called_once_with(**kwargs) for header, header_contents in expected_headers.items(): assert mock_session.headers[header] == header_contents mock_session.post.assert_called_once() @@ -486,32 +502,19 @@ def test_vault_token_revokes_oidc_token( mock_session.post.return_value.raise_for_status.assert_called_once() +@pytest.mark.usefixtures('_cert_files_mock') def test_vault_token_revoke_failure( - mocker: MockerFixture, + handle_auth_mock: _unittest_mock.Mock, + session_class_mock: _unittest_mock.Mock, ) -> None: """Test ``_vault_token`` context manager raises when token revocation fails.""" - mock_handle_auth = mocker.patch.object( - hashivault, - 'handle_auth', - autospec=True, - return_value='test_token', - ) - mock_session_class = mocker.patch.object( - hashivault.requests, - 'Session', - autospec=True, - ) - mock_session = mock_session_class.return_value + mock_session = session_class_mock.return_value mock_session.headers = {} mock_session.post.return_value.raise_for_status.side_effect = ( - hashivault.requests.HTTPError('403 Client Error: Forbidden for url: ...') + hashivault.requests.HTTPError( + '403 Client Error: Forbidden for url: ...', + ) ) - mock_cert_files = mocker.patch.object( - hashivault, - 'CertFiles', - autospec=True, - ).return_value - mock_cert_files.__enter__.return_value = 'cert_path' kwargs = { 'url': 'https://vault.example.com', @@ -525,12 +528,12 @@ def _use_vault_token() -> None: assert token == 'test_token' with pytest.raises( - hashivault.requests.HTTPError, - match='403 Client Error', + hashivault.requests.HTTPError, + match='403 Client Error', ): _use_vault_token() - mock_handle_auth.assert_called_once_with(**kwargs) + handle_auth_mock.assert_called_once_with(**kwargs) @pytest.mark.parametrize( @@ -617,8 +620,11 @@ def _use_vault_token() -> None: ), ), ) +@pytest.mark.usefixtures('_cert_files_mock', '_suppress_raise_for_status') def test_backend_revokes_oidc_token( + handle_auth_mock: _unittest_mock.Mock, mocker: MockerFixture, + session_class_mock: _unittest_mock.Mock, backend_func: _t.Callable[[dict[str, str]], str], extra_kwargs: dict[str, str], ) -> None: @@ -633,12 +639,6 @@ def test_backend_revokes_oidc_token( backend_kwargs = {**base_kwargs, **extra_kwargs} - mock_handle_auth = mocker.patch.object( - hashivault, - 'handle_auth', - autospec=True, - return_value='test_token', - ) mock_get_or_post_session = mocker.Mock() mock_get_or_post_session.headers = {} @@ -672,26 +672,17 @@ def test_backend_revokes_oidc_token( mock_revoke_session.headers = {} # requests.Session is called twice: once for secret fetch, once for revoke - mocker.patch.object( - hashivault.requests, - 'Session', - autospec=True, - side_effect=[mock_get_or_post_session, mock_revoke_session], - ) - mock_cert_files = mocker.patch.object( - hashivault, - 'CertFiles', - autospec=True, - ).return_value - mock_cert_files.__enter__.return_value = 'cert_path' - mocker.patch.object(hashivault, 'raise_for_status', autospec=True) + session_class_mock.side_effect = [ + mock_get_or_post_session, + mock_revoke_session, + ] # Call the backend function to retrieve the secret # Adding ignore[call-arg] since adding the explicit args to the backend # functions makes the linter trigger the "too few arguments supplied" backend_func(**backend_kwargs) # type: ignore[call-arg] - mock_handle_auth.assert_called_once() + handle_auth_mock.assert_called_once() # Verify revocation was attempted mock_revoke_session.post.assert_called_once() assert 'auth/token/revoke-self' in mock_revoke_session.post.call_args[0][0] From 400d01645707ea55abd70fe4f11874387a610aee Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Mon, 30 Mar 2026 18:21:55 +0200 Subject: [PATCH 33/39] =?UTF-8?q?=F0=9F=A7=AA=20Use=20indentity=20check=20?= =?UTF-8?q?for=20test=20function=20params?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/unit/credentials/hashivault_test.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index bd4e0d218a..ed498e9d12 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -644,7 +644,9 @@ def test_backend_revokes_oidc_token( # Configure mock response for secret fetch based on backend type mock_secret_response = mocker.Mock() - if backend_func.__name__ == 'kv_backend': + # MyPy is unhappy due to incomplete typing of the `backend_func` param but + # we don't care too much for now. + if backend_func is hashivault.kv_backend: # type: ignore[comparison-overlap] api_version = extra_kwargs.get('api_version', 'v1') # kv_backend v1 expects {'data': {secret_key: value}} mock_secret_response.json.return_value = { @@ -663,7 +665,9 @@ def test_backend_revokes_oidc_token( mock_secret_response.status_code = 200 # Configure session mock based on backend type - if backend_func.__name__ == 'kv_backend': + # MyPy is unhappy due to incomplete typing of the `backend_func` param but + # we don't care too much for now. + if backend_func is hashivault.kv_backend: # type: ignore[comparison-overlap] mock_get_or_post_session.get.return_value = mock_secret_response else: # ssh_backend mock_get_or_post_session.post.return_value = mock_secret_response From f2801f843629447e92ab867b8ed5e20ec928e1d4 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 31 Mar 2026 14:40:52 +0200 Subject: [PATCH 34/39] Unify mock @ test_backend_revokes_oidc_token This patch reduces the number of duplicate checks in the test function as means of refactoring and reducing the control flow branches. --- tests/unit/credentials/hashivault_test.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index ed498e9d12..776da6f45e 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -643,10 +643,15 @@ def test_backend_revokes_oidc_token( mock_get_or_post_session.headers = {} # Configure mock response for secret fetch based on backend type + # and use it as a get or post method on the session mock mock_secret_response = mocker.Mock() + mock_secret_response.status_code = 200 + # MyPy is unhappy due to incomplete typing of the `backend_func` param but # we don't care too much for now. if backend_func is hashivault.kv_backend: # type: ignore[comparison-overlap] + mock_get_or_post_session.get.return_value = mock_secret_response + api_version = extra_kwargs.get('api_version', 'v1') # kv_backend v1 expects {'data': {secret_key: value}} mock_secret_response.json.return_value = { @@ -658,19 +663,12 @@ def test_backend_revokes_oidc_token( 'data': mock_secret_response.json.return_value, } else: # ssh_backend + mock_get_or_post_session.post.return_value = mock_secret_response + # ssh_backend expects {'data': {'signed_key': value}} mock_secret_response.json.return_value = { 'data': {'signed_key': 'test_signed_key'}, } - mock_secret_response.status_code = 200 - - # Configure session mock based on backend type - # MyPy is unhappy due to incomplete typing of the `backend_func` param but - # we don't care too much for now. - if backend_func is hashivault.kv_backend: # type: ignore[comparison-overlap] - mock_get_or_post_session.get.return_value = mock_secret_response - else: # ssh_backend - mock_get_or_post_session.post.return_value = mock_secret_response mock_revoke_session = mocker.Mock() mock_revoke_session.headers = {} From 8350839da133b119931162860e3d00c4878ffa2a Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 31 Mar 2026 14:59:06 +0200 Subject: [PATCH 35/39] =?UTF-8?q?=F0=9F=92=85=20Replace=20token=20arg=20in?= =?UTF-8?q?jection=20w/=20`contextvars`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This allows preserving signatures of the decorated function annotations without leaking the token managed on a dedicated abstration layer and removing it from the context upon the lifetime exit. --- src/awx_plugins/credentials/hashivault.py | 33 +++++++++++++++-------- 1 file changed, 22 insertions(+), 11 deletions(-) diff --git a/src/awx_plugins/credentials/hashivault.py b/src/awx_plugins/credentials/hashivault.py index e925913b11..93ee6b7735 100644 --- a/src/awx_plugins/credentials/hashivault.py +++ b/src/awx_plugins/credentials/hashivault.py @@ -2,6 +2,7 @@ # mypy: disable-error-code="arg-type, no-untyped-call, no-untyped-def" import contextlib as _ctx +import contextvars as _ctx_vars import functools as _functools import os import pathlib @@ -20,6 +21,10 @@ from .plugin import CertFiles, CredentialPlugin, raise_for_status +_AUTH_TOKEN: _ctx_vars.ContextVar[str] = _ctx_vars.ContextVar('_AUTH_TOKEN') +"""Authentication token for use in plugin handlers.""" + + class _EmptyKwargs(_t.TypedDict): """Schema for zero keyword arguments.""" @@ -525,6 +530,16 @@ def _vault_token(**kwargs: str) -> _abc.Iterator[str]: ) +@_ctx.contextmanager +def _token_in_context(token: str, /) -> _abc.Iterator[None]: + """Set a token for the execution context lifetime.""" + var_ctx_token = _AUTH_TOKEN.set(token) + try: + yield + finally: + _AUTH_TOKEN.reset(var_ctx_token) + + # Param Spec to represent decorated function parameters _PT = _t.ParamSpec( # FIXME: Use [_RT, **_PT] in the signature in Python 3.12 '_PT', @@ -534,7 +549,7 @@ def _vault_token(**kwargs: str) -> _abc.Iterator[str]: def _inject_auth_token_with_revocation( - decorated_function: _t.Callable[_t.Concatenate[str, _PT], _RT], + decorated_function: _t.Callable[_PT, _RT], /, ) -> _t.Callable[_PT, _RT]: @_functools.wraps(decorated_function) @@ -542,11 +557,9 @@ def _decorate_the_function_with_revocation( # noqa: WPS430 -- in-decorator *args: _PT.args, **kwargs: _PT.kwargs, ) -> _RT: - assert not args - assert 'token' not in kwargs - with _vault_token(**kwargs) as token: - return decorated_function(*args, token=token, **kwargs) + with _token_in_context(token): + return decorated_function(*args, **kwargs) return _decorate_the_function_with_revocation @@ -598,7 +611,6 @@ def method_auth(**kwargs): # pylint: disable-next=too-many-arguments def kv_backend( # noqa: WPS211 -- the same as too-many-arguments *, - token: str, url: str, api_version: str, secret_path: str, @@ -616,9 +628,9 @@ def kv_backend( # noqa: WPS211 -- the same as too-many-arguments sess = requests.Session() sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) - sess.headers['Authorization'] = f'Bearer {token}' + sess.headers['Authorization'] = f'Bearer {_AUTH_TOKEN.get()}' # Compatibility header for older installs of Hashicorp Vault - sess.headers['X-Vault-Token'] = token + sess.headers['X-Vault-Token'] = _AUTH_TOKEN.get() if namespace: sess.headers['X-Vault-Namespace'] = namespace @@ -685,7 +697,6 @@ def kv_backend( # noqa: WPS211 -- the same as too-many-arguments # pylint: disable-next=too-many-arguments def ssh_backend( # noqa: WPS211 -- the same as too-many-arguments *, - token: str, url: str, secret_path: str, role: str, @@ -711,11 +722,11 @@ def ssh_backend( # noqa: WPS211 -- the same as too-many-arguments sess = requests.Session() sess.mount(url, requests.adapters.HTTPAdapter(max_retries=5)) - sess.headers['Authorization'] = f'Bearer {token}' + sess.headers['Authorization'] = f'Bearer {_AUTH_TOKEN.get()}' if namespace: sess.headers['X-Vault-Namespace'] = namespace # Compatibility header for older installs of Hashicorp Vault - sess.headers['X-Vault-Token'] = token + sess.headers['X-Vault-Token'] = _AUTH_TOKEN.get() # https://www.vaultproject.io/api/secret/ssh/index.html#sign-ssh-key request_url = '/'.join([url, secret_path, 'sign', role]).rstrip('/') From 0abbb4192b5b035cae24d42f146865a00a75a0a0 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 31 Mar 2026 15:43:45 +0200 Subject: [PATCH 36/39] =?UTF-8?q?=F0=9F=A7=AA=20Suppress=20WPS201=20in=20`?= =?UTF-8?q?hashivault`=20module?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This is needed due to mandatory imports exceeding the limit. The previously existing module structure pattern encapsulates big chunks of implementation in one module. In the future, we should look into breaking it up into smaller parts so that the suppression would not be needed anymore. --- .flake8 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.flake8 b/.flake8 index 62a48b1697..88abba3426 100644 --- a/.flake8 +++ b/.flake8 @@ -126,7 +126,7 @@ per-file-ignores = src/awx_plugins/credentials/dsv.py: ANN003, ANN201, D100, D103, P103, WPS210 # NOTE: `awx_plugins.credentials.github_app` may need restructuring to comply with WPS 202. src/awx_plugins/credentials/github_app.py: WPS202 - src/awx_plugins/credentials/hashivault.py: ANN003, ANN201, B950, C901, CCR001, D100, D103, LN001, N400, WPS202, WPS204, WPS210, WPS221, WPS223, WPS229, WPS231, WPS232, WPS331, WPS336, WPS337, WPS432, WPS454 + src/awx_plugins/credentials/hashivault.py: ANN003, ANN201, B950, C901, CCR001, D100, D103, LN001, N400, WPS201, WPS202, WPS204, WPS210, WPS221, WPS223, WPS229, WPS231, WPS232, WPS331, WPS336, WPS337, WPS432, WPS454 src/awx_plugins/credentials/injectors.py: ANN001, ANN201, ANN202, C408, D100, D103, WPS110, WPS111, WPS202, WPS210, WPS347, WPS433, WPS440 src/awx_plugins/credentials/plugin.py: ANN001, ANN002, ANN101, ANN201, ANN204, B010, D100, D101, D103, D105, D107, D205, D400, E731, WPS432, WPS433, WPS440, WPS442, WPS601 src/awx_plugins/credentials/plugins.py: B950,D100, D101, D103, D105, D107, D205, D400, LN001, WPS204, WPS229, WPS433, WPS440 From 170c75d806ca88e5ec16b5e1b8325e47e888827f Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 31 Mar 2026 17:01:37 +0200 Subject: [PATCH 37/39] =?UTF-8?q?=F0=9F=A7=AA=20Let=20pylint=20know=20priv?= =?UTF-8?q?ate=20access=20is=20ok=20in=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/unit/credentials/hashivault_test.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/unit/credentials/hashivault_test.py b/tests/unit/credentials/hashivault_test.py index 776da6f45e..918d7ef94a 100644 --- a/tests/unit/credentials/hashivault_test.py +++ b/tests/unit/credentials/hashivault_test.py @@ -1,3 +1,4 @@ +# pylint: disable=protected-access # tests access private methods legitimately """Tests for HashiCorp Vault credential plugins.""" import typing as _t From f5203c6b7e7bc25c51f12c418ef104176fc12e40 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 31 Mar 2026 17:01:58 +0200 Subject: [PATCH 38/39] =?UTF-8?q?=F0=9F=A7=AA=20Make=20MyPy=20allow=20priv?= =?UTF-8?q?ate=20attr=20access=20in=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .mypy.ini | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/.mypy.ini b/.mypy.ini index 8520d2ae1a..27122d135b 100644 --- a/.mypy.ini +++ b/.mypy.ini @@ -111,3 +111,12 @@ disallow_any_expr = false disallow_any_expr = false # fails on `@hypothesis.given()`: disallow_any_decorated = false + +# It is sometimes necessary to access "officially-private" attributes in our +# own tests for the purposes of assertion, mocking and spying on objects. And +# that's fine since it's within the same repository fully under our control, +# and so we don't need to have MyPy complain about those in this specific +# context. This is why we disable `attr-defined` in our tests [1]. +# +# [1] https://til.codeinthehole.com/posts/how-to-handle-convenience-imports-with-mypy/ +disable_error_code = attr-defined From 09eb6e5c9a10e75604d60b61594d1265ade3c988 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Tue, 31 Mar 2026 17:31:37 +0200 Subject: [PATCH 39/39] =?UTF-8?q?=F0=9F=A9=B9=F0=9F=93=9D=20Suppress=20typ?= =?UTF-8?q?es=20that=20Sphinx=20can't=20resolve?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/conf.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/conf.py b/docs/conf.py index 1cb0f78ee1..8e116a2339 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -233,6 +233,12 @@ 'py:class', 'awx_plugins.interfaces._temporary_private_credential_api.Credential', ), + ( # generic return type variable: + 'py:class', + 'awx_plugins.credentials.hashivault._RT', + ), + ('py:class', '_PT'), # generic ParamSpec type variable + ('py:class', '_contextvars.ContextVar'), # unresolved context var type ('py:class', 'EnvVarsType'), ]