diff --git a/.devcontainer/.dev_config.yaml b/.devcontainer/.dev_config.yaml index 482ffd3..fc1aef2 100644 --- a/.devcontainer/.dev_config.yaml +++ b/.devcontainer/.dev_config.yaml @@ -2,7 +2,6 @@ service_instance_id: "1" token_hashes: # plaintext token: 43fadc91-b98f-4925-bd31-1b054b13dc55 - 7ad83b6b9183c91674eec897935bc154ba9ff9704f8be0840e77f476b5062b6e -vault_token: "dev-token" vault_url: "http://vault:8200" vault_secrets_mount_point: secret vault_path: sms diff --git a/.pyproject_generation/pyproject_custom.toml b/.pyproject_generation/pyproject_custom.toml index 92c3c2e..979b95c 100644 --- a/.pyproject_generation/pyproject_custom.toml +++ b/.pyproject_generation/pyproject_custom.toml @@ -1,6 +1,6 @@ [project] name = "sms" -version = "2.0.0" +version = "3.0.0" description = "State Management Service - Provides a REST API for basic infrastructure technology state management." dependencies = [ "typer >= 0.12", diff --git a/README.md b/README.md index 51d6c8f..434b855 100644 --- a/README.md +++ b/README.md @@ -24,13 +24,13 @@ We recommend using the provided Docker container. A pre-build version is available at [docker hub](https://hub.docker.com/repository/docker/ghga/state-management-service): ```bash -docker pull ghga/state-management-service:2.0.0 +docker pull ghga/state-management-service:3.0.0 ``` Or you can build the container yourself from the [`./Dockerfile`](./Dockerfile): ```bash # Execute in the repo's root dir: -docker build -t ghga/state-management-service:2.0.0 . +docker build -t ghga/state-management-service:3.0.0 . ``` For production-ready deployment, we recommend using Kubernetes, however, @@ -38,7 +38,7 @@ for simple use cases, you could execute the service using docker on a single server: ```bash # The entrypoint is preconfigured: -docker run -p 8080:8080 ghga/state-management-service:2.0.0 --help +docker run -p 8080:8080 ghga/state-management-service:3.0.0 --help ``` If you prefer not to use containers, you may install the service from source: @@ -135,16 +135,6 @@ The service requires the following configuration parameters: ``` -- **`vault_token`** *(string, required)*: Token for the Vault. - - - Examples: - - ```json - "dev-token" - ``` - - - **`vault_secrets_mount_point`** *(string)*: Name used to address the secret engine under a custom mount path. Default: `"secret"`. diff --git a/config_schema.json b/config_schema.json index e7c3ca3..93c3fc6 100644 --- a/config_schema.json +++ b/config_schema.json @@ -196,14 +196,6 @@ "title": "Vault Url", "type": "string" }, - "vault_token": { - "description": "Token for the Vault", - "examples": [ - "dev-token" - ], - "title": "Vault Token", - "type": "string" - }, "vault_secrets_mount_point": { "default": "secret", "description": "Name used to address the secret engine under a custom mount path.", @@ -535,7 +527,6 @@ "kafka_servers", "object_storages", "vault_url", - "vault_token", "vault_path", "token_hashes", "db_prefix", diff --git a/example_config.yaml b/example_config.yaml index f920baa..b38c24e 100644 --- a/example_config.yaml +++ b/example_config.yaml @@ -53,7 +53,6 @@ vault_path: sms vault_role_id: '**********' vault_secret_id: '**********' vault_secrets_mount_point: secret -vault_token: dev-token vault_url: http://vault:8200 vault_verify: true workers: 1 diff --git a/openapi.yaml b/openapi.yaml index b464be4..ca1f6bf 100644 --- a/openapi.yaml +++ b/openapi.yaml @@ -57,7 +57,7 @@ components: info: description: A service for basic infrastructure technology state management. title: State Management Service - version: 2.0.0 + version: 3.0.0 openapi: 3.1.0 paths: /documents/permissions: diff --git a/pyproject.toml b/pyproject.toml index fdf93a7..3e847d4 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -21,7 +21,7 @@ classifiers = [ "Intended Audience :: Developers", ] name = "sms" -version = "2.0.0" +version = "3.0.0" description = "State Management Service - Provides a REST API for basic infrastructure technology state management." dependencies = [ "typer >= 0.12", diff --git a/src/sms/adapters/inbound/fastapi_/routers/secrets.py b/src/sms/adapters/inbound/fastapi_/routers/secrets.py index 2e7fb8e..34a2462 100644 --- a/src/sms/adapters/inbound/fastapi_/routers/secrets.py +++ b/src/sms/adapters/inbound/fastapi_/routers/secrets.py @@ -48,6 +48,10 @@ async def get_secrets( """Returns a list of secrets in the specified vault""" try: return secrets_handler.get_secrets(vault_path) + except PermissionError as err: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, detail=str(err) + ) from err except Exception as exc: raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR) from exc @@ -66,5 +70,9 @@ async def delete_secrets( """Delete all secrets from the specified vault.""" try: secrets_handler.delete_secrets(vault_path) + except PermissionError as err: + raise HTTPException( + status_code=status.HTTP_403_FORBIDDEN, detail=str(err) + ) from err except Exception as exc: raise HTTPException(status_code=status.HTTP_500_INTERNAL_SERVER_ERROR) from exc diff --git a/src/sms/core/secrets_handler.py b/src/sms/core/secrets_handler.py index 5555924..ed86335 100644 --- a/src/sms/core/secrets_handler.py +++ b/src/sms/core/secrets_handler.py @@ -19,7 +19,7 @@ from hvac import Client as HvacClient from hvac.api.auth_methods import Kubernetes -from hvac.exceptions import InvalidPath +from hvac.exceptions import Forbidden, InvalidPath from pydantic import Field, SecretStr, field_validator from pydantic_settings import BaseSettings @@ -34,9 +34,6 @@ class VaultConfig(BaseSettings): vault_url: str = Field( default=..., description="URL for the Vault", examples=["http://vault:8200"] ) - vault_token: str = Field( - default=..., description="Token for the Vault", examples=["dev-token"] - ) vault_secrets_mount_point: str = Field( default="secret", examples=["secret"], @@ -82,6 +79,22 @@ class VaultConfig(BaseSettings): description="Path to service account token used by kube auth adapter.", ) + @field_validator("vault_verify") + @classmethod + def validate_vault_ca(cls, value: bool | str) -> bool | str: + """Check that the CA bundle can be read if it is specified.""" + if isinstance(value, str): + path = Path(value) + if not path.exists(): + raise ValueError(f"Vault CA bundle not found at: {path}") + try: + bundle = path.open().read() + except OSError as error: + raise ValueError("Vault CA bundle cannot be read") from error + if "-----BEGIN CERTIFICATE-----" not in bundle: + raise ValueError("Vault CA bundle does not contain a certificate") + return value + class SecretsHandler(SecretsHandlerPort): """Adapter wrapping hvac.Client""" @@ -89,6 +102,7 @@ class SecretsHandler(SecretsHandlerPort): def __init__(self, config: VaultConfig): """Initialized approle-based client and log in""" self._config = config + self.client = HvacClient(url=config.vault_url) self._auth_mount_point = config.vault_auth_mount_point self._secrets_mount_point = config.vault_secrets_mount_point self._kube_role = config.vault_kube_role @@ -123,7 +137,6 @@ def _login(self): ) else: self._kube_adapter.login(role=self._kube_role, jwt=jwt) - elif self._auth_mount_point: self.client.auth.approle.login( role_id=self._role_id, @@ -135,31 +148,6 @@ def _login(self): role_id=self._role_id, secret_id=self._secret_id ) - @field_validator("vault_verify") - @classmethod - def validate_vault_ca(cls, value: bool | str) -> bool | str: - """Check that the CA bundle can be read if it is specified.""" - if isinstance(value, str): - path = Path(value) - if not path.exists(): - raise ValueError(f"Vault CA bundle not found at: {path}") - try: - bundle = path.open().read() - except OSError as error: - raise ValueError("Vault CA bundle cannot be read") from error - if "-----BEGIN CERTIFICATE-----" not in bundle: - raise ValueError("Vault CA bundle does not contain a certificate") - return value - - @property - def client(self) -> HvacClient: - """Return an instance of a vault client""" - return HvacClient( - url=self._config.vault_url, - token=self._config.vault_token, - verify=self._config.vault_verify, - ) - def get_secrets(self, vault_path: str) -> list[str]: """Return the IDs of all secrets in the specified vault.""" self._check_auth() @@ -178,6 +166,12 @@ def get_secrets(self, vault_path: str) -> list[str]: ) log.warning(msg, vault_path) return [] + except Forbidden as err: + permission_error = PermissionError( + f"Permission not configured for vault path '{vault_path}'", + ) + log.error(permission_error) + raise permission_error from err def delete_secrets(self, vault_path: str): """Delete all secrets from the specified vault.""" @@ -188,7 +182,14 @@ def delete_secrets(self, vault_path: str): return for secret in secrets: - self.client.secrets.kv.v2.delete_metadata_and_all_versions( - path=f"{vault_path}/{secret}", - mount_point=self._config.vault_secrets_mount_point, - ) + try: + self.client.secrets.kv.v2.delete_metadata_and_all_versions( + path=f"{vault_path}/{secret}", + mount_point=self._config.vault_secrets_mount_point, + ) + except Forbidden as err: + permission_error = PermissionError( + f"Permission not configured for vault path '{vault_path}'", + ) + log.error(permission_error) + raise permission_error from err diff --git a/tests/fixtures/test_config.yaml b/tests/fixtures/test_config.yaml index e233a8c..cf294ac 100644 --- a/tests/fixtures/test_config.yaml +++ b/tests/fixtures/test_config.yaml @@ -2,7 +2,6 @@ service_instance_id: "1" token_hashes: # plaintext token: 43fadc91-b98f-4925-bd31-1b054b13dc55 - 7ad83b6b9183c91674eec897935bc154ba9ff9704f8be0840e77f476b5062b6e -vault_token: "dev-token" vault_url: "http://vault:8200" vault_path: sms vault_role_id: dummy-role diff --git a/tests/fixtures/vault.py b/tests/fixtures/vault.py index c596f15..d10c2e2 100644 --- a/tests/fixtures/vault.py +++ b/tests/fixtures/vault.py @@ -20,6 +20,7 @@ import hvac import pytest +from hvac.api.auth_methods import Kubernetes from hvac.exceptions import InvalidPath from testcontainers.core.generic import DockerContainer @@ -30,7 +31,7 @@ VAULT_URL = DEFAULT_TEST_CONFIG.vault_url DEFAULT_PORT = 8200 VAULT_PATH = DEFAULT_TEST_CONFIG.vault_path -VAULT_TOKEN = DEFAULT_TEST_CONFIG.vault_token +VAULT_TOKEN = "dev-token" VAULT_MOUNT_POINT = DEFAULT_TEST_CONFIG.vault_secrets_mount_point @@ -43,12 +44,58 @@ class VaultFixture: def __init__(self, config: VaultConfig): self.config = config self.vaults_used: set[str] = set() + self.client = hvac.Client(url=self.config.vault_url) + self._auth_mount_point = config.vault_auth_mount_point + self._secrets_mount_point = config.vault_secrets_mount_point + self._kube_role = config.vault_kube_role + + if self._kube_role: + # use kube role and service account token + self._kube_adapter = Kubernetes(self.client.adapter) + self._service_account_token_path = config.service_account_token_path + elif config.vault_role_id and config.vault_secret_id: + # use role and secret ID instead + self._role_id = config.vault_role_id.get_secret_value() + self._secret_id = config.vault_secret_id.get_secret_value() + else: + raise ValueError( + "There is no way to log in to vault:\n" + + "Neither kube role nor both role and secret ID were provided." + ) + + def _check_auth(self): + """Check if authentication timed out and re-authenticate if needed""" + if not self.client.is_authenticated(): + self._login() + + def _login(self): + """Log in using Kubernetes Auth or AppRole""" + if self._kube_role: + with self._service_account_token_path.open() as token_file: + jwt = token_file.read() + if self._auth_mount_point: + self._kube_adapter.login( + role=self._kube_role, jwt=jwt, mount_point=self._auth_mount_point + ) + else: + self._kube_adapter.login(role=self._kube_role, jwt=jwt) + + elif self._auth_mount_point: + self.client.auth.approle.login( + role_id=self._role_id, + secret_id=self._secret_id, + mount_point=self._auth_mount_point, + ) + else: + self.client.auth.approle.login( + role_id=self._role_id, secret_id=self._secret_id + ) def store_secret(self, *, key: str, vault_path: str = VAULT_PATH): """Store a secret in vault""" - client = hvac.Client(url=self.config.vault_url, token=VAULT_TOKEN) + self._check_auth() - client.secrets.kv.v2.create_or_update_secret( + self.client.secrets.kv.v2.create_or_update_secret( path=f"{vault_path}/{key}", secret={key: f"secret_for_{key}"}, mount_point=self.config.vault_secrets_mount_point, @@ -57,15 +104,15 @@ def store_secret(self, *, key: str, vault_path: str = VAULT_PATH): def delete_all_secrets(self, vault_path: str): """Remove all secrets from the vault to reset for next test""" - client = hvac.Client(url=self.config.vault_url, token=VAULT_TOKEN) + self._check_auth() with suppress(InvalidPath): - secrets = client.secrets.kv.v2.list_secrets( + secrets = self.client.secrets.kv.v2.list_secrets( path=vault_path, mount_point=self.config.vault_secrets_mount_point, ) for key in secrets["data"]["keys"]: - client.secrets.kv.v2.delete_metadata_and_all_versions( + self.client.secrets.kv.v2.delete_metadata_and_all_versions( path=f"{vault_path}/{key}", mount_point=self.config.vault_secrets_mount_point, ) @@ -107,7 +154,6 @@ def vault_container_fixture() -> Generator[VaultContainerFixture, None, None]: vault_role_id=role_id, vault_secret_id=secret_id, vault_verify=DEFAULT_TEST_CONFIG.vault_verify, - vault_token=VAULT_TOKEN, vault_path=VAULT_PATH, vault_secrets_mount_point=VAULT_MOUNT_POINT, ) @@ -144,18 +190,15 @@ def configure_vault(*, host: str, port: int): # create access policy to bind to role policy = f""" path "{VAULT_MOUNT_POINT}/data/{VAULT_PATH}/*" {{ - capabilities = ["read", "create"] + capabilities = ["read", "list", "create", "update", "delete"] }} path "{VAULT_MOUNT_POINT}/metadata/{VAULT_PATH}/*" {{ - capabilities = ["delete"] + capabilities = ["read", "list", "create", "update", "delete"] }} """ # inject policy - client.sys.create_or_update_policy( - name=VAULT_PATH, - policy=policy, - ) + client.sys.create_or_update_policy(name=VAULT_PATH, policy=policy) role_name = "test_role" # create role and bind policy @@ -170,9 +213,7 @@ def configure_vault(*, host: str, port: int): role_id = response["data"]["role_id"] # retrieve secret_id - response = client.auth.approle.generate_secret_id( - role_name=role_name, - ) + response = client.auth.approle.generate_secret_id(role_name=role_name) secret_id = response["data"]["secret_id"] # log out root token client diff --git a/tests/integration/test_secrets.py b/tests/integration/test_secrets.py index 1d4e8da..e6f6de0 100644 --- a/tests/integration/test_secrets.py +++ b/tests/integration/test_secrets.py @@ -94,8 +94,13 @@ async def test_nonexistent_vault_path(vault: VaultFixture): AsyncTestClient(app=app) as client, ): response = await client.get("/secrets/doesnotexist", headers=HEADERS) - assert response.status_code == 200 - assert response.json() == [] + assert response.status_code == 403 + assert response.json() == { + "detail": "Permission not configured for vault path 'doesnotexist'" + } response = await client.delete("/secrets/doesnotexist", headers=HEADERS) - assert response.status_code == 204 + assert response.status_code == 403 + assert response.json() == { + "detail": "Permission not configured for vault path 'doesnotexist'" + } diff --git a/tests/unit/secrets/test_secrets_handler.py b/tests/unit/secrets/test_secrets_handler.py index b690019..b117ba8 100644 --- a/tests/unit/secrets/test_secrets_handler.py +++ b/tests/unit/secrets/test_secrets_handler.py @@ -30,25 +30,25 @@ [[], ["key1"], ["key1", "key2"]], ids=["empty", "single", "multiple"], ) -def test_get_secrets(monkeypatch: pytest.MonkeyPatch, secrets: list[str]): +def test_get_secrets(secrets: list[str]): """Test get_secrets method without errors""" # patch the hvac client with a mock mock_client = Mock() mock_client.secrets.kv.v2.list_secrets.return_value = {"data": {"keys": secrets}} - monkeypatch.setattr(SecretsHandler, "client", mock_client) secrets_handler = SecretsHandler(config=DEFAULT_TEST_CONFIG) + secrets_handler.client = mock_client assert secrets_handler.get_secrets(VAULT_PATH) == secrets mock_client.secrets.kv.v2.list_secrets.assert_called_once() -def test_get_secrets_error(monkeypatch: pytest.MonkeyPatch, caplog): +def test_get_secrets_error(caplog): """Test get_secrets method with an error""" # patch the hvac client with a mock mock_client = Mock() mock_client.secrets.kv.v2.list_secrets.side_effect = InvalidPath("Invalid path") - monkeypatch.setattr(SecretsHandler, "client", mock_client) secrets_handler = SecretsHandler(config=DEFAULT_TEST_CONFIG) + secrets_handler.client = mock_client # Make sure the error is logged as a warning but an empty list is still returned caplog.clear() @@ -63,13 +63,13 @@ def test_get_secrets_error(monkeypatch: pytest.MonkeyPatch, caplog): mock_client.secrets.kv.v2.list_secrets.assert_called_once() -def test_delete_secrets_error(monkeypatch: pytest.MonkeyPatch): +def test_delete_secrets_error(): """Test delete_secrets method on empty vault.""" # patch the hvac client with a mock mock_client = Mock() mock_client.secrets.kv.v2.list_secrets.side_effect = InvalidPath("Invalid path") - monkeypatch.setattr(SecretsHandler, "client", mock_client) secrets_handler = SecretsHandler(config=DEFAULT_TEST_CONFIG) + secrets_handler.client = mock_client # Call delete_secrets() in order to trigger get_secrets secrets_handler.delete_secrets(VAULT_PATH) @@ -82,10 +82,7 @@ def test_delete_secrets_error(monkeypatch: pytest.MonkeyPatch): [[], ["key1"], ["key1", "key2"]], ids=["Empty", "OneSecret", "TwoSecrets"], ) -def test_delete_successful( - monkeypatch: pytest.MonkeyPatch, - stored_secrets: list[str], -): +def test_delete_successful(stored_secrets: list[str]): """Test delete_secrets method.""" # create a mock for the hvac client list_stored_secrets = {"data": {"keys": stored_secrets}} @@ -98,8 +95,8 @@ def test_delete_successful( mock_client.secrets.kv.v2.list_secrets.side_effect = InvalidPath("Invalid path") # apply the mock to the SecretsHandler - monkeypatch.setattr(SecretsHandler, "client", mock_client) secrets_handler = SecretsHandler(config=DEFAULT_TEST_CONFIG) + secrets_handler.client = mock_client # call delete_secrets() secrets_handler.delete_secrets(VAULT_PATH) @@ -119,7 +116,7 @@ def test_delete_successful( ) -def test_non_default_mount_point(monkeypatch: pytest.MonkeyPatch): +def test_non_default_mount_point(): """Test get_secrets and delete_secrets method with non-default mount point. Makes sure that the actual config value is passed to the hvac Client. @@ -128,15 +125,13 @@ def test_non_default_mount_point(monkeypatch: pytest.MonkeyPatch): list_stored_secrets = {"data": {"keys": ["key1"]}} mock_client.secrets.kv.v2.list_secrets.return_value = list_stored_secrets - # apply the mock to the SecretsHandler - monkeypatch.setattr(SecretsHandler, "client", mock_client) - # use alternative mount point mount_point = "test" config = DEFAULT_TEST_CONFIG.model_copy( update={"vault_secrets_mount_point": mount_point} ) secrets_handler = SecretsHandler(config=config) + secrets_handler.client = mock_client # call delete_secrets() secrets_handler.delete_secrets(f"{VAULT_PATH}")