feat(fm): accept the partner client secret by value, stored encrypted

Sergio Gimenezrequested to merge
feat/partner-typed-secret into develop

Closes #28 (closed).

The portal can now register a partner with the secret itself instead of a path to a file mounted in FM's container. FM encrypts it, keeps it in fm_db, and never hands it back.

sequenceDiagram
    participant P as Portal
    participant OEG as OEG
    participant FM as FM-A
    participant DB as fm_db
    participant KC as Partner Keycloak
    participant B as FM-B
    P->>OEG: PATCH /platform/v1/federation/partners/{id} {our_client_secret}
    OEG->>FM: PATCH /internal/partners/{id} (relayed)
    FM->>DB: partner_op_secrets = AES-256-GCM(secret)
    FM-->>P: {our_client_secret_set: true, our_client_secret_ref: null}
    Note over FM: token cache miss
    FM->>DB: read ciphertext
    FM->>KC: client_credentials, our_client_id + decrypted secret
    KC-->>FM: access token
    FM->>B: EWBI GET zone (Bearer)

What changes:

  • POST/PATCH /internal/partners accept our_client_secret (write-only, 1–4096 chars). Responses gain our_client_secret_set; the value never comes back, not even in a 400.
  • One source per partner. Sending both our_client_secret and our_client_secret_ref is a 400; setting one on an existing partner drops the other. A PATCH without the field keeps the stored secret, null removes it. The "all three outbound fields or none" 422 counts either source as the secret.
  • New partner_op_secrets table (migration 0007), written in the same transaction as the partner row and read only by the token provider.
  • FM_PARTNER_SECRET_KEY: base64url, 32 bytes, checked at startup without echoing it on failure. Unset, FM starts with a warning and answers a request carrying a secret with 501 partner-secret-storage-unavailable. Partners on our_client_secret_ref don't need it.
  • FileClientSecretTokenProvider is now ClientSecretTokenProvider: file when there's a ref, otherwise the decrypted stored secret. A replaced secret is used from the next token refresh, same as a rewritten file.
  • cryptography is a direct dependency now (it came in through pyjwt[crypto] before); uv.lock updated.

Not in this MR: the helm value for the key, and the portal form change (password field instead of the path). OEG lets the 501 through in open-exposure-gateway!39 (merged).

This goes against #21's "no secret value is accepted, stored, returned or logged". ADR-0067 (oop/architecture!12) records the decision and updates the FM docs; it follows what OEG already does for callback tokens (ADR-0063, encrypted in oeg_db under one app-wide key).

Tests:

  • unit: 333 pass, including the new registry, provider, cipher and key-validation tests
  • integration against Postgres: 15 pass; tables.py and the migrations produce no autogenerate diff
  • e2e on the two-operator docker harness, 27 checks through OEG: with a wrong typed secret Keycloak refuses the token and the zone call gets 503; with the right one FM-B answers the zone, so the decrypted secret is what FM-A used. The secret shows up in no response, error body or container log, fm_db holds 12 + len + 16 bytes of ciphertext, FM-B without a key returns 501 through OEG-B and stores nothing, and switching back to the mounted file works. OEG ran with auth in log-only mode, so the operator role was not exercised.
Edited by Sergio Gimenez

Merge request reports

src/federation_manager/adapters/database/partner_repo.py

+51 −14
Changes for src/federation_manager/adapters/database/partner_repo.py: 51 added lines, 14 removed lines.
Original line number Diff line number Diff line 
from typing import Any

from uuid import UUID



from sqlalchemy import Select, insert, select, update

from sqlalchemy import Select, delete, func, insert, select, update

from sqlalchemy.dialects.postgresql import insert as pg_insert

from sqlalchemy.exc import IntegrityError

from sqlalchemy.ext.asyncio import AsyncSession



from federation_manager.adapters.database.tables import partner_ops

from federation_manager.adapters.database.tables import partner_op_secrets, partner_ops

from federation_manager.domain.errors import PartnerRegistrationConflict

from federation_manager.domain.models import PartnerOP
@@ -13,43 +14,78 @@ from federation_manager.domain.models import PartnerOP 
_UNIQUE_FIELDS = ("mcc_mnc", "oauth2_client_id")





def _partners() -> Select[tuple[object, ...]]:

    secret_set = partner_op_secrets.c.partner_op_id.is_not(None).label("our_client_secret_set")

    return select(partner_ops, secret_set).outerjoin(

        partner_op_secrets, partner_op_secrets.c.partner_op_id == partner_ops.c.id

    )





class PostgresPartnerRepo:

    def __init__(self, session: AsyncSession) -> None:

        self._session = session



    async def find_by_oauth2_client_id(self, client_id: str) -> PartnerOP | None:

        return await self._one(

            select(partner_ops).where(partner_ops.c.oauth2_client_id == client_id)

        )

        return await self._one(_partners().where(partner_ops.c.oauth2_client_id == client_id))



    async def find_by_id(self, partner_id: UUID) -> PartnerOP | None:

        return await self._one(select(partner_ops).where(partner_ops.c.id == partner_id))

        return await self._one(_partners().where(partner_ops.c.id == partner_id))



    async def list_active(self) -> list[PartnerOP]:

        stmt = select(partner_ops).where(partner_ops.c.status == "active")

        stmt = _partners().where(partner_ops.c.status == "active")

        rows = (await self._session.execute(stmt)).all()

        return [self._to_partner(row) for row in rows]



    async def find_by_mcc_mnc(self, mcc_mnc: str) -> PartnerOP | None:

        return await self._one(select(partner_ops).where(partner_ops.c.mcc_mnc == mcc_mnc))

        return await self._one(_partners().where(partner_ops.c.mcc_mnc == mcc_mnc))



    async def list_all(self) -> list[PartnerOP]:

        stmt = select(partner_ops).order_by(partner_ops.c.created_at)

        stmt = _partners().order_by(partner_ops.c.created_at)

        rows = (await self._session.execute(stmt)).all()

        return [self._to_partner(row) for row in rows]



    async def add(self, partner: PartnerOP) -> None:

        await self._write(insert(partner_ops).values(id=partner.id, **self._values(partner)))

    async def find_encrypted_secret(self, partner_id: UUID) -> bytes | None:

        stmt = select(partner_op_secrets.c.encrypted_secret).where(

            partner_op_secrets.c.partner_op_id == partner_id

        )

        blob: bytes | None = (await self._session.execute(stmt)).scalar_one_or_none()

        return blob



    async def add(self, partner: PartnerOP, encrypted_secret: bytes | None = None) -> None:

        await self._write(

            insert(partner_ops).values(id=partner.id, **self._values(partner)),

            *self._secret_writes(partner, encrypted_secret),

        )



    async def update(self, partner: PartnerOP) -> None:

    async def update(self, partner: PartnerOP, encrypted_secret: bytes | None = None) -> None:

        await self._write(

            update(partner_ops)

            .where(partner_ops.c.id == partner.id)

            .values(**self._values(partner))

            .values(**self._values(partner)),

            *self._secret_writes(partner, encrypted_secret),

        )



    @staticmethod

    def _secret_writes(partner: PartnerOP, encrypted_secret: bytes | None) -> list[Any]:

        if not partner.our_client_secret_set:

            return [

                delete(partner_op_secrets).where(partner_op_secrets.c.partner_op_id == partner.id)

            ]

        if encrypted_secret is None:

            return []

        upsert = pg_insert(partner_op_secrets).values(

            partner_op_id=partner.id, encrypted_secret=encrypted_secret

        )

        return [

            upsert.on_conflict_do_update(

                index_elements=[partner_op_secrets.c.partner_op_id],

                set_={"encrypted_secret": encrypted_secret, "updated_at": func.now()},

            )

        ]



    async def _write(self, stmt: Any) -> None:

    async def _write(self, *stmts: Any) -> None:

        try:

            for stmt in stmts:

                await self._session.execute(stmt)

            await self._session.commit()

        except IntegrityError as error:
@@ -89,4 +125,5 @@ class PostgresPartnerRepo: 
            token_endpoint=row.token_endpoint,

            base_url=row.base_url,

            name=row.name,

            our_client_secret_set=row.our_client_secret_set,

        )

src/federation_manager/adapters/database/tables.py

+23 −0
Changes for src/federation_manager/adapters/database/tables.py: 23 added lines, 0 removed lines.
Original line number Diff line number Diff line
@@ -7,6 +7,7 @@ from sqlalchemy import ( 
    ForeignKey,

    Index,

    Integer,

    LargeBinary,

    MetaData,

    String,

    Table,
@@ -59,6 +60,28 @@ partner_ops = Table( 
    ),

)



# Kept apart from partner_ops so the ciphertext is read only when a token is requested.

partner_op_secrets = Table(

    "partner_op_secrets",

    metadata,

    Column(

        "partner_op_id",

        PGUUID(as_uuid=True),

        ForeignKey("partner_ops.id", ondelete="CASCADE"),

        primary_key=True,

    ),

    # AES-256-GCM: 12-byte nonce followed by ciphertext and tag

    Column("encrypted_secret", LargeBinary, nullable=False),

    Column("created_at", DateTime(timezone=True), nullable=False, server_default=func.now()),

    Column(

        "updated_at",

        DateTime(timezone=True),

        nullable=False,

        server_default=func.now(),

        onupdate=func.now(),

    ),

)



federation_agreements = Table(

    "federation_agreements",

    metadata,

src/federation_manager/adapters/security/client_secret_token_provider.py

+29 −3
Changes for src/federation_manager/adapters/security/client_secret_token_provider.py: 29 added lines, 3 removed lines.
Original line number Diff line number Diff line 
import asyncio

import time

from collections.abc import Callable

from collections.abc import Awaitable, Callable

from dataclasses import dataclass

from pathlib import Path

from typing import TypeAlias, cast
@@ -11,10 +11,14 @@ import httpx 
from federation_manager.domain.errors import (

    PartnerTokenConfigurationError,

    PartnerTokenRequestFailed,

    SecretUndecryptable,

)

from federation_manager.domain.models import PartnerOP

from federation_manager.domain.ports import SecretCipherPort



_CacheKey: TypeAlias = tuple[UUID, str, str, str, str]

StoredSecretLoader: TypeAlias = Callable[[UUID], Awaitable[bytes | None]]

_STORED = "stored"





@dataclass(frozen=True)
@@ -23,16 +27,20 @@ class _CachedToken: 
    expires_at: float





class FileClientSecretTokenProvider:

class ClientSecretTokenProvider:

    def __init__(

        self,

        client: httpx.AsyncClient,

        *,

        stored_secrets: StoredSecretLoader | None = None,

        cipher: SecretCipherPort | None = None,

        refresh_skew_seconds: float = 30.0,

        clock: Callable[[], float] = time.monotonic,

        allow_insecure: bool = False,

    ) -> None:

        self._client = client

        self._stored_secrets = stored_secrets

        self._cipher = cipher

        self._allow_insecure = allow_insecure

        self._refresh_skew_seconds = refresh_skew_seconds

        self._clock = clock
@@ -41,8 +49,12 @@ class FileClientSecretTokenProvider: 


    async def token_for(self, partner: PartnerOP, scope: str = "fed-mgmt") -> str:

        client_id = self._required(partner, "client id", partner.our_client_id)

        secret_ref = self._required(partner, "client secret ref", partner.our_client_secret_ref)

        secret_ref = partner.our_client_secret_ref or (

            _STORED if partner.our_client_secret_set else None

        )

        secret_ref = self._required(partner, "client secret", secret_ref)

        token_endpoint = self._token_endpoint(partner)

        # a replaced stored secret is picked up at the next refresh, as a rewritten file is

        cache_key = (partner.id, scope, client_id, secret_ref, token_endpoint)



        cached = self._cache.get(cache_key)
@@ -55,6 +67,9 @@ class FileClientSecretTokenProvider: 
            if cached is not None and self._is_usable(cached):

                return cached.access_token



            if secret_ref == _STORED:

                secret = await self._stored_secret(partner)

            else:

                secret = self._read_secret(partner, secret_ref)

            access_token, expires_in = await self._request_token(

                partner,
@@ -90,6 +105,17 @@ class FileClientSecretTokenProvider: 
            raise PartnerTokenConfigurationError(partner.id, "token endpoint")

        return value



    async def _stored_secret(self, partner: PartnerOP) -> str:

        if self._stored_secrets is None or self._cipher is None:

            raise PartnerTokenConfigurationError(partner.id, "client secret")

        blob = await self._stored_secrets(partner.id)

        if blob is None:

            raise PartnerTokenConfigurationError(partner.id, "client secret")

        try:

            return self._cipher.decrypt(blob)

        except SecretUndecryptable:

            raise PartnerTokenConfigurationError(partner.id, "client secret") from None



    @staticmethod

    def _read_secret(partner: PartnerOP, secret_ref: str) -> str:

        try:

src/federation_manager/adapters/security/secret_cipher.py

0 → 100644
+25 −0
Changes for src/federation_manager/adapters/security/secret_cipher.py: 25 added lines, 0 removed lines.
Original line number Diff line number Diff line 
import os



from cryptography.exceptions import InvalidTag

from cryptography.hazmat.primitives.ciphers.aead import AESGCM



from federation_manager.domain.errors import SecretUndecryptable



_NONCE_BYTES = 12





class AesGcmSecretCipher:

    def __init__(self, key: bytes) -> None:

        self._aead = AESGCM(key)



    def encrypt(self, plaintext: str) -> bytes:

        # a fresh nonce per value is what makes one key safe for every secret

        nonce = os.urandom(_NONCE_BYTES)

        return nonce + self._aead.encrypt(nonce, plaintext.encode(), None)



    def decrypt(self, blob: bytes) -> str:

        nonce, ciphertext = blob[:_NONCE_BYTES], blob[_NONCE_BYTES:]

        try:

            return self._aead.decrypt(nonce, ciphertext, None).decode()

        except (InvalidTag, ValueError):

            raise SecretUndecryptable from None

src/federation_manager/api/internal/partners.py

+42 −7
Changes for src/federation_manager/api/internal/partners.py: 42 added lines, 7 removed lines.
Original line number Diff line number Diff line 
from typing import Annotated, Literal, Self

from typing import Annotated, Any, Literal, Self

from urllib.parse import urlsplit

from uuid import UUID



from fastapi import APIRouter, Depends

from pydantic import AfterValidator, BaseModel, ConfigDict, Field, model_validator

from pydantic import AfterValidator, BaseModel, ConfigDict, Field, SecretStr, model_validator



from federation_manager.application.partners import PartnerRegistration, PartnerRegistryService

from federation_manager.dependencies import get_partner_registry_service
@@ -23,7 +23,7 @@ def _http_url(value: str) -> str: 




def _secret_ref(value: str) -> str:

    # a path to the mounted secret, never the secret itself (persistence model: no inline secrets)

    # catches a secret pasted into the path field: it would be stored in clear

    if not value.startswith("/"):

        raise ValueError("must be an absolute path to the mounted secret file")

    return value
@@ -34,6 +34,7 @@ MccMnc = Annotated[str, Field(pattern=r"^[0-9]{3}-?[0-9]{2,3}$")] 
ClientId = Annotated[str, Field(min_length=1, max_length=255)]

Url = Annotated[str, AfterValidator(_http_url)]

SecretRef = Annotated[str, AfterValidator(_secret_ref)]

Secret = Annotated[SecretStr, Field(min_length=1, max_length=4096)]

Status = Literal["pending", "active", "suspended", "decommissioned"]
@@ -49,12 +50,27 @@ class PartnerCreate(BaseModel): 
    our_client_id: ClientId | None = Field(

        default=None, description="Client id the partner issued to us for outbound calls"

    )

    our_client_secret_ref: SecretRef | None = None

    our_client_secret_ref: SecretRef | None = Field(

        default=None, description="Path to a mounted file holding the secret for our_client_id"

    )

    our_client_secret: Secret | None = Field(

        default=None,

        description="Secret for our_client_id, stored encrypted and never returned",

    )

    token_endpoint: Url | None = Field(

        default=None, description="Partner's OAuth2 token endpoint for our outbound calls"

    )

    status: Literal["pending", "active"] = "pending"



    @model_validator(mode="after")

    def _one_secret_source(self) -> Self:

        _require_one_secret_source(self.our_client_secret, self.our_client_secret_ref)

        return self



    def registration(self) -> PartnerRegistration:

        fields = self.model_dump(exclude={"our_client_secret"})

        return PartnerRegistration(**fields, our_client_secret=_reveal(self.our_client_secret))





class PartnerUpdate(BaseModel):

    """Partial update: fields left out are unchanged; null clears an optional field."""
@@ -66,6 +82,7 @@ class PartnerUpdate(BaseModel): 
    base_url: Url | None = None

    our_client_id: ClientId | None = None

    our_client_secret_ref: SecretRef | None = None

    our_client_secret: Secret | None = None

    token_endpoint: Url | None = None

    status: Status | None = None
@@ -74,8 +91,24 @@ class PartnerUpdate(BaseModel): 
        for field in ("name", "oauth2_client_id", "base_url", "status"):

            if field in self.model_fields_set and getattr(self, field) is None:

                raise ValueError(f"{field} cannot be null")

        _require_one_secret_source(self.our_client_secret, self.our_client_secret_ref)

        return self



    def changes(self) -> dict[str, Any]:

        changes = self.model_dump(exclude_unset=True, exclude={"our_client_secret"})

        if "our_client_secret" in self.model_fields_set:

            changes["our_client_secret"] = _reveal(self.our_client_secret)

        return changes





def _require_one_secret_source(secret: SecretStr | None, secret_ref: str | None) -> None:

    if secret is not None and secret_ref is not None:

        raise ValueError("our_client_secret and our_client_secret_ref are mutually exclusive")





def _reveal(secret: SecretStr | None) -> str | None:

    return None if secret is None else secret.get_secret_value()





class PartnerView(BaseModel):

    id: UUID
@@ -87,6 +120,9 @@ class PartnerView(BaseModel): 
    our_client_secret_ref: str | None

    token_endpoint: str | None

    status: Status

    our_client_secret_set: bool = Field(

        description="An encrypted our_client_secret is stored; the value is never returned"

    )



    @classmethod

    def of(cls, partner: PartnerOP) -> "PartnerView":
@@ -95,7 +131,7 @@ class PartnerView(BaseModel): 


@router.post("", status_code=201, response_model=PartnerView)

async def register_partner(body: PartnerCreate, service: Service) -> PartnerView:

    return PartnerView.of(await service.register(PartnerRegistration(**body.model_dump())))

    return PartnerView.of(await service.register(body.registration()))





@router.get("", response_model=list[PartnerView])
@@ -110,5 +146,4 @@ async def get_partner(partner_op_id: UUID, service: Service) -> PartnerView: 


@router.patch("/{partner_op_id}", response_model=PartnerView)

async def update_partner(partner_op_id: UUID, body: PartnerUpdate, service: Service) -> PartnerView:

    changes = body.model_dump(exclude_unset=True)

    return PartnerView.of(await service.update(partner_op_id, changes))
 
    return PartnerView.of(await service.update(partner_op_id, body.changes()))