diff --git a/src/dfe_engine/auth/scim_mapping.py b/src/dfe_engine/auth/scim_mapping.py index f02474bc..73e03f11 100644 --- a/src/dfe_engine/auth/scim_mapping.py +++ b/src/dfe_engine/auth/scim_mapping.py @@ -29,7 +29,6 @@ SCIM face so downstream tooling can tell IdP-owned records apart. """ -import secrets from datetime import datetime from typing import TypedDict @@ -39,6 +38,7 @@ from dfe_engine.auth.accounts import Account from dfe_engine.auth.groups import Group +from dfe_engine.secrets import mint_value # Marker written to ``source_provider`` for anything the SCIM face provisions. SCIM_SOURCE_PROVIDER = "scim" @@ -126,7 +126,7 @@ def generate_provisioning_password() -> str: IdP-provisioned users authenticate via the IdP, not a local password, but the store requires a bcrypt-hashable secret. This mints one they never use. """ - return secrets.token_urlsafe(32) + return mint_value() # -- Group <-> SCIM Group ------------------------------------- diff --git a/src/dfe_engine/cli/auto/app.py b/src/dfe_engine/cli/auto/app.py index eb44bc5f..cc3e31d0 100644 --- a/src/dfe_engine/cli/auto/app.py +++ b/src/dfe_engine/cli/auto/app.py @@ -16,7 +16,6 @@ import os import re -import secrets from typing import Any from scalo.cli.openapi import ScaloHttpTransport, SpecCli, Store, Transport @@ -34,8 +33,9 @@ def load_live_spec() -> dict[str, Any]: jwt_secret, which a schema Job does not carry. A throwaway one satisfies that check. """ from dfe_engine.api.app import create_app + from dfe_engine.secrets import KEY_LENGTH, mint_value - os.environ.setdefault("DFE_ENGINE_API_JWT_SECRET", secrets.token_urlsafe(32)) + os.environ.setdefault("DFE_ENGINE_API_JWT_SECRET", mint_value(KEY_LENGTH)) return create_app().openapi() diff --git a/src/dfe_engine/governance/ch/reconciler.py b/src/dfe_engine/governance/ch/reconciler.py index 37275003..8d2cac32 100644 --- a/src/dfe_engine/governance/ch/reconciler.py +++ b/src/dfe_engine/governance/ch/reconciler.py @@ -15,7 +15,6 @@ """ import hashlib -import secrets as _stdlib_secrets from collections.abc import Collection, Mapping from typing import Any @@ -26,6 +25,7 @@ from dfe_engine.clickhouse.errors import is_connection_error from dfe_engine.orgs.tenant_scope import org_tenant_ids from dfe_engine.schema.require import missing_objects +from dfe_engine.secrets import mint_value from .models import ( DB, @@ -304,7 +304,7 @@ def _hash_for(self, path: str) -> str: user's password, so regenerating every run would desync CH from the store. The mint is create-only, so two replicas reconciling together keep one password. """ - minted = _stdlib_secrets.token_urlsafe(24)[:32] + minted = mint_value() if self._secrets is None: return hashlib.sha256(minted.encode()).hexdigest() plaintext = self._secrets.get_or_none(path) diff --git a/src/dfe_engine/secrets.py b/src/dfe_engine/secrets.py index 439f1a94..b93eddfd 100644 --- a/src/dfe_engine/secrets.py +++ b/src/dfe_engine/secrets.py @@ -18,6 +18,8 @@ The engine never imports a backend SDK - scalo owns that (see docs/deployment/backing-services.md). """ +import secrets as _stdlib_secrets +import string import time from collections.abc import Callable, Sequence from contextlib import suppress @@ -41,6 +43,23 @@ # AWS finishes a forced delete in the background, so a write to that name waits up to 30s for it. FORCED_DELETE_BACKOFF: tuple[float, ...] = (1.0, 2.0, 4.0, 8.0, 15.0) +# Letters and digits only, because shells, .env files and URL-form DSNs rewrite `$`, `-` and `_` +_MINT_ALPHABET = string.ascii_letters + string.digits +PASSWORD_LENGTH = 32 +KEY_LENGTH = 64 + + +def mint_value(length: int = PASSWORD_LENGTH) -> str: + """A random string of letters and digits. + + Args: + length: PASSWORD_LENGTH for a password, KEY_LENGTH for a signing key. + + Returns: + The minted value. + """ + return "".join(_stdlib_secrets.choice(_MINT_ALPHABET) for _ in range(length)) + class SecretNameHeldError(ProviderError): """A deleted secret that has not finished deleting holds a name the engine reads or writes.""" diff --git a/tests/unit/test_secrets_interface.py b/tests/unit/test_secrets_interface.py index 873e900b..4813f2e5 100644 --- a/tests/unit/test_secrets_interface.py +++ b/tests/unit/test_secrets_interface.py @@ -23,9 +23,12 @@ from dfe_engine.secrets import ( FORCED_DELETE_BACKOFF, + KEY_LENGTH, + PASSWORD_LENGTH, DfeSecrets, SecretNameHeldError, build_secrets, + mint_value, ) from dfe_engine.settings import SecretsSettings @@ -197,3 +200,14 @@ def test_tags_reach_aws_only(provider, tags): _scripted_store(manager, provider=provider).create("ch/groups/soc", "pw") assert manager.created_tags == [tags] + + +@pytest.mark.parametrize( + ("length", "expected"), [((), 32), ((KEY_LENGTH,), 64), ((PASSWORD_LENGTH,), 32)] +) +def test_a_minted_value_is_letters_and_digits_at_its_length(length, expected): + """A `$`, `-` or `_` in a minted value would be rewritten by a shell, a .env file or a DSN.""" + values = {mint_value(*length) for _ in range(50)} + + assert len(values) == 50 + assert all(len(value) == expected and value.isascii() and value.isalnum() for value in values)