Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixing Vault AppRole authentication with CONN_URI #18064

Merged
merged 10 commits into from
Sep 10, 2021
26 changes: 23 additions & 3 deletions airflow/providers/hashicorp/hooks/vault.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

"""Hook for HashiCorp Vault"""
import json
import warnings
from typing import Optional, Tuple

import hvac
Expand Down Expand Up @@ -63,7 +64,7 @@ class VaultHook(BaseHook):

Login/Password are used as credentials:

* approle: password -> secret_id
* approle: login -> role_id, password -> secret_id
* github: password -> token
* token: password -> token
* aws_iam: login -> key_id, password -> secret_id
Expand All @@ -83,7 +84,7 @@ class VaultHook(BaseHook):
:param kv_engine_version: Select the version of the engine to run (``1`` or ``2``). Defaults to
version defined in connection or ``2`` if not defined in connection.
:type kv_engine_version: int
:param role_id: Role ID for Authentication (for ``approle``, ``aws_iam`` auth_types)
:param role_id: Role ID for ``aws_iam`` Authentication.
:type role_id: str
:param kubernetes_role: Role for Authentication (for ``kubernetes`` auth_type)
:type kubernetes_role: str
Expand Down Expand Up @@ -148,7 +149,26 @@ def __init__(
except ValueError:
raise VaultError(f"The version is not an int: {conn_version}. ")

if auth_type in ["approle", "aws_iam"]:
if auth_type == "approle":
if role_id:
warnings.warn(
"""The usage of role_id for AppRole authentication has been deprecated.
Please use connection login.""",
DeprecationWarning,
stacklevel=2,
)
elif self.connection.extra_dejson.get('role_id'):
role_id = self.connection.extra_dejson.get('role_id')
warnings.warn(
"""The usage of role_id in connection extra for AppRole authentication has been
deprecated. Please use connection login.""",
DeprecationWarning,
stacklevel=2,
)
elif self.connection.login:
role_id = self.connection.login

if auth_type == "aws_iam":
if not role_id:
role_id = self.connection.extra_dejson.get('role_id')

Expand Down
43 changes: 34 additions & 9 deletions tests/providers/hashicorp/hooks/test_vault.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,15 +176,14 @@ def test_protocol(self, protocol, expected_url, mock_hvac, mock_get_connection):
kwargs = {
"vault_conn_id": "vault_conn_id",
"auth_type": "approle",
"role_id": "role",
"kv_engine_version": 2,
}

test_hook = VaultHook(**kwargs)
mock_get_connection.assert_called_with("vault_conn_id")
test_client = test_hook.get_conn()
mock_hvac.Client.assert_called_with(url=expected_url)
test_client.auth.approle.login.assert_called_with(role_id="role", secret_id="pass")
test_client.auth.approle.login.assert_called_with(role_id="user", secret_id="pass")
test_client.is_authenticated.assert_called_with()
assert 2 == test_hook.vault_client.kv_engine_version

Expand All @@ -202,15 +201,14 @@ def test_approle_init_params(self, mock_hvac, mock_get_connection):
kwargs = {
"vault_conn_id": "vault_conn_id",
"auth_type": "approle",
"role_id": "role",
"kv_engine_version": 2,
}

test_hook = VaultHook(**kwargs)
mock_get_connection.assert_called_with("vault_conn_id")
test_client = test_hook.get_conn()
mock_hvac.Client.assert_called_with(url='http://localhost:8180')
test_client.auth.approle.login.assert_called_with(role_id="role", secret_id="pass")
test_client.auth.approle.login.assert_called_with(role_id="user", secret_id="pass")
test_client.is_authenticated.assert_called_with()
assert 2 == test_hook.vault_client.kv_engine_version

Expand All @@ -222,10 +220,7 @@ def test_approle_dejson(self, mock_hvac, mock_get_connection):
mock_connection = self.get_mock_connection()
mock_get_connection.return_value = mock_connection

connection_dict = {
"auth_type": "approle",
'role_id': "role",
}
connection_dict = {"auth_type": "approle"}

mock_connection.extra_dejson.get.side_effect = connection_dict.get
kwargs = {
Expand All @@ -236,7 +231,20 @@ def test_approle_dejson(self, mock_hvac, mock_get_connection):
mock_get_connection.assert_called_with("vault_conn_id")
test_client = test_hook.get_conn()
mock_hvac.Client.assert_called_with(url='http://localhost:8180')
test_client.auth.approle.login.assert_called_with(role_id="role", secret_id="pass")
test_client.auth.approle.login.assert_called_with(role_id="user", secret_id="pass")
test_client.is_authenticated.assert_called_with()
assert 2 == test_hook.vault_client.kv_engine_version

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
@mock.patch.dict(
'os.environ',
AIRFLOW_CONN_VAULT_CONN_ID='https://role:[email protected]?auth_type=approle',
)
def test_approle_uri(self, mock_hvac):
test_hook = VaultHook(vault_conn_id='vault_conn_id')
test_client = test_hook.get_conn()
mock_hvac.Client.assert_called_with(url='https://vault.example.com')
test_client.auth.approle.login.assert_called_with(role_id="role", secret_id="secret")
test_client.is_authenticated.assert_called_with()
assert 2 == test_hook.vault_client.kv_engine_version

Expand Down Expand Up @@ -290,6 +298,23 @@ def test_aws_iam_dejson(self, mock_hvac, mock_get_connection):
role="role",
)

@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
@mock.patch.dict(
'os.environ',
AIRFLOW_CONN_VAULT_CONN_ID='https://login:[email protected]?auth_type=aws_iam&role_id=role',
)
def test_aws_uri(self, mock_hvac):
test_hook = VaultHook(vault_conn_id='vault_conn_id')
test_client = test_hook.get_conn()
mock_hvac.Client.assert_called_with(url='https://vault.example.com')
test_client.auth_aws_iam.assert_called_with(
access_key='login',
secret_key='pass',
role="role",
)
test_client.is_authenticated.assert_called_with()
assert 2 == test_hook.vault_client.kv_engine_version

@mock.patch("airflow.providers.hashicorp.hooks.vault.VaultHook.get_connection")
@mock.patch("airflow.providers.hashicorp._internal_client.vault_client.hvac")
def test_azure_init_params(self, mock_hvac, mock_get_connection):
Expand Down