From 8c222b4835dc7c48355353d1dfb981816e0d1edc Mon Sep 17 00:00:00 2001 From: man4ish Date: Sun, 4 Oct 2026 02:16:33 -0500 Subject: [PATCH] feat: internal reveal endpoint for BYOK provider keys POST /internal/organizations/{org_id}/provider-keys/{provider}/reveal decrypts a stored BYOK key for one outbound provider API call -- shared-secret gated (PROVIDER_KEY_REVEAL_SECRET), the same service-to-service shape POST /auth/api-keys/exchange already established. Deliberately no org-membership check: the question this answers is "does this organization have a key for this provider", not "may this caller manage it" (manage_org still gates setting/clearing the key). Called by omnibioai-api-gateway immediately before it forwards a BYOK-routed /v1/literature/answers call to omnibioai-rag; never persisted or logged by that caller. Co-Authored-By: Claude Sonnet 5 --- app/api/routes_organization_config.py | 49 ++++++++++- app/core/config.py | 10 +++ app/main.py | 2 + app/schemas/organization_config.py | 9 ++ app/services/organization_config_service.py | 49 +++++++---- tests/test_organization_provider_keys.py | 96 +++++++++++++++++++++ 6 files changed, 198 insertions(+), 17 deletions(-) diff --git a/app/api/routes_organization_config.py b/app/api/routes_organization_config.py index d35c78b..abfdf68 100644 --- a/app/api/routes_organization_config.py +++ b/app/api/routes_organization_config.py @@ -1,10 +1,13 @@ -from fastapi import APIRouter, Depends, HTTPException +import hmac + +from fastapi import APIRouter, Depends, Header, HTTPException from sqlalchemy.orm import Session +from app.core.config import settings from app.db.models import OrganizationMembership, User from app.db.session import get_db from app.rbac import require_org_permission_or_platform_admin -from app.schemas.organization_config import ProviderKeyIn, ProviderKeyOut +from app.schemas.organization_config import ProviderKeyIn, ProviderKeyOut, ProviderKeyRevealOut from app.services import organization_config_service # BYOK provider-key storage (design audit gap #4). Gated on manage_org -- @@ -15,6 +18,19 @@ # be issued with (see routes_apikeys.py's PUBLIC_SCOPE_TO_PERMISSION). router = APIRouter(prefix="/orgs/{org_id}/provider-keys", tags=["provider-keys"]) +# M16: service-to-service only, shared-secret gated -- never reachable +# by a user or API-key token, the same shape routes_apikeys.py's own +# exchange_router already established for POST /auth/api-keys/exchange. +# Deliberately no org-membership/manage_org dependency: the question +# this endpoint answers is "does this organization have a key +# configured for this provider", not "may this particular caller manage +# it" -- any authenticated member of the org can trigger a +# /v1/literature/answers call that uses the org's own already-configured +# key, the same way any member can spend the org's own quota today. +# manage_org still gates *setting*/*clearing* the key above; using it is +# a different, already-answered question by the time a call gets here. +reveal_router = APIRouter(prefix="/internal/organizations/{org_id}/provider-keys", tags=["provider-keys"]) + MANAGE_ORG = "manage_org" @@ -80,3 +96,32 @@ def delete_provider_key( if config is None: raise HTTPException(404, f"No {provider} key is configured for this organization.") return _to_out(db, config) + + +@reveal_router.post("/{provider}/reveal", response_model=ProviderKeyRevealOut) +def reveal_provider_key( + org_id: int, + provider: str, + db: Session = Depends(get_db), + x_provider_key_reveal_secret: str = Header(default=""), +): + """Gateway-only: decrypt this organization's stored key for `provider` + for one outbound provider API call. Every failure is framed the + same way exchange_api_key's own docstring explains: never leak + *which* precondition failed (wrong secret vs. no key configured) + beyond what the status code itself already implies. + """ + expected = settings.PROVIDER_KEY_REVEAL_SECRET + if not expected: + raise HTTPException(503, "Provider key reveal is not configured") + if not hmac.compare_digest(x_provider_key_reveal_secret.encode(), expected.encode()): + raise HTTPException(403, "Forbidden") + try: + api_key = organization_config_service.reveal_provider_key(db, org_id, provider) + except ValueError as e: + raise HTTPException(400, str(e)) + except RuntimeError as e: + raise HTTPException(500, str(e)) + if api_key is None: + raise HTTPException(404, f"No {provider} key is configured for this organization.") + return ProviderKeyRevealOut(provider=provider, api_key=api_key) diff --git a/app/core/config.py b/app/core/config.py index ce52441..fbcc921 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -59,6 +59,16 @@ class Settings: API_KEY_EXCHANGE_SECRET = os.getenv("API_KEY_EXCHANGE_SECRET", "") API_KEY_TOKEN_EXPIRE_MINUTES = int(os.getenv("API_KEY_TOKEN_EXPIRE_MINUTES", "5")) + # M16 (BYOK provider-key reveal): the one path where a stored + # organization provider key is ever decrypted and leaves this + # service -- POST /internal/organizations/{id}/provider-keys/ + # {provider}/reveal, called only by omnibioai-api-gateway immediately + # before it forwards a /v1/literature/answers call that requested + # that provider, and never persisted or logged by the caller. Same + # fail-closed shape as API_KEY_EXCHANGE_SECRET above: empty disables + # the endpoint entirely (503), not a silent no-auth fallback. + PROVIDER_KEY_REVEAL_SECRET = os.getenv("PROVIDER_KEY_REVEAL_SECRET", "") + ACCESS_TOKEN_EXPIRE_MINUTES = int(os.getenv("ACCESS_TOKEN_EXPIRE_MINUTES", 15)) REFRESH_TOKEN_EXPIRE_DAYS = int(os.getenv("REFRESH_TOKEN_EXPIRE_DAYS", 7)) diff --git a/app/main.py b/app/main.py index a578c98..539b62b 100644 --- a/app/main.py +++ b/app/main.py @@ -14,6 +14,7 @@ from app.api.routes_apikeys import exchange_router as apikeys_exchange_router from app.api.routes_apikeys import me_router as apikeys_me_router from app.api.routes_organization_config import router as provider_keys_router +from app.api.routes_organization_config import reveal_router as provider_key_reveal_router from app.api.routes_oauth_clients import router as oauth_clients_router from app.api.routes_oauth_token import router as oauth_token_router from app.api.routes_org_sso import router as org_sso_router @@ -106,6 +107,7 @@ app.include_router(apikeys_exchange_router) app.include_router(apikeys_me_router) app.include_router(provider_keys_router) +app.include_router(provider_key_reveal_router) app.include_router(oauth_clients_router) app.include_router(oauth_token_router) app.include_router(org_sso_router) diff --git a/app/schemas/organization_config.py b/app/schemas/organization_config.py index 832e0bc..89b8466 100644 --- a/app/schemas/organization_config.py +++ b/app/schemas/organization_config.py @@ -13,3 +13,12 @@ class ProviderKeyOut(BaseModel): has_key: bool updated_at: str | None updated_by_email: str | None + + +class ProviderKeyRevealOut(BaseModel): + """M16: the one response shape in this module that DOES carry the + real key -- POST /internal/.../reveal is service-to-service only + (shared-secret gated, see routes_organization_config.py), never + reachable by a user or API-key token.""" + provider: str + api_key: str diff --git a/app/services/organization_config_service.py b/app/services/organization_config_service.py index 7d41d8a..d611046 100644 --- a/app/services/organization_config_service.py +++ b/app/services/organization_config_service.py @@ -5,21 +5,14 @@ existed since the multi-tenant schema migration with nobody reading or writing it -- this module is its first real consumer. -Scoped deliberately narrowly, matching how far "storage" goes on its -own: this only stores and retrieves the encrypted key. Actually routing -a /v1/literature/answers call through the stored provider (Claude/ -OpenAI client calls, token accounting, llm.tokens.* usage events) is -the rest of gap #4 and is not built here -- storing a key an org can -never yet be routed through is a real, honestly-scoped gap, not a -hidden one (the same "mechanism before the business/product decision -that uses it" shape this project's M3/M6/M7 all already established, -here applied to an engineering dependency rather than a pricing one). - -Also not yet exposed through the public gateway's own /v1/provider-keys/ -{provider} path the design doc names -- only through this org-admin -endpoint (PUT/GET/DELETE /orgs/{org_id}/provider-keys), the same one -Studio's own LLM settings page would call. Routing this through the -gateway for direct external-developer use is follow-up work. +M15 exposed the storage endpoint through the public gateway's own +/v1/provider-keys/{provider} path. M16 adds the one remaining piece +this module needs for actual routing: reveal_provider_key(), which +decrypts a stored key for a single internal, service-to-service call -- +see app/api/routes_organization_config.py's reveal endpoint and its own +docstring for the access-control story. The Claude/OpenAI client calls, +token accounting, and llm.tokens.* usage events themselves live in +omnibioai-api-gateway/omnibioai-rag, not here. """ from datetime import datetime @@ -81,6 +74,32 @@ def set_provider_key( return config +def reveal_provider_key(db: Session, organization_id: int, provider: str) -> str | None: + """Decrypts and returns the organization's stored key for `provider`, + or None if nothing is configured for that provider (including the + case where some *other* provider is configured instead -- same "only + one slot" semantics clear_provider_key already established). Raises + ValueError for an unsupported provider name, and (via crypto.decrypt) + RuntimeError if CONFIG_ENCRYPTION_KEY isn't configured. + + Callers must never persist, log, or echo the return value anywhere + beyond the single outbound provider API call it exists for -- see + this module's own docstring and routes_organization_config.py's + reveal endpoint for the access-control story around who may call + this at all. Deliberately not audit-logged: a reveal happens on + every routed /v1/literature/answers call using BYOK, the same + per-use (not per-lifecycle-change) frequency apikey_service. + exchange_api_key already leaves unaudited for the identical reason. + """ + if provider not in SUPPORTED_PROVIDERS: + raise ValueError(f"Unsupported provider {provider!r}; supported: {sorted(SUPPORTED_PROVIDERS)}") + + config = get_organization_config(db, organization_id) + if config is None or config.llm_provider != provider or not config.llm_api_key_encrypted: + return None + return crypto.decrypt(config.llm_api_key_encrypted) + + def clear_provider_key( db: Session, organization_id: int, provider: str, updated_by_user_id: int, ) -> OrganizationConfig | None: diff --git a/tests/test_organization_provider_keys.py b/tests/test_organization_provider_keys.py index 0f284a8..22ec43a 100644 --- a/tests/test_organization_provider_keys.py +++ b/tests/test_organization_provider_keys.py @@ -226,3 +226,99 @@ def test_non_member_cannot_read_or_set_another_orgs_key(client, org, configured_ # Confirm the attempted write had no effect. listed = client.get(f"/orgs/{org['id']}/provider-keys", headers=org["owner_headers"]) assert listed.json()["has_key"] is False + + +# ── POST /internal/organizations/{org_id}/provider-keys/{provider}/reveal ── +# M16: service-to-service only, shared-secret gated -- never reachable by a +# user or API-key token. Same shape as POST /auth/api-keys/exchange's own +# tests (tests/test_apikey_exchange.py). + + +REVEAL_SECRET = "test-reveal-secret" + + +@pytest.fixture +def reveal_secret(monkeypatch): + import app.core.config as config + + monkeypatch.setattr(config.settings, "PROVIDER_KEY_REVEAL_SECRET", REVEAL_SECRET) + return REVEAL_SECRET + + +def _reveal(client, org_id, provider, secret=REVEAL_SECRET): + return client.post( + f"/internal/organizations/{org_id}/provider-keys/{provider}/reveal", + headers={"X-Provider-Key-Reveal-Secret": secret}, + ) + + +def test_reveal_disabled_without_configured_secret(client, org, configured_crypto): + client.put(f"/orgs/{org['id']}/provider-keys/claude", json={"api_key": "sk-claude"}, headers=org["owner_headers"]) + assert _reveal(client, org["id"], "claude", secret="").status_code == 503 + + +def test_reveal_rejects_wrong_or_missing_secret(client, org, configured_crypto, reveal_secret): + client.put(f"/orgs/{org['id']}/provider-keys/claude", json={"api_key": "sk-claude"}, headers=org["owner_headers"]) + + assert _reveal(client, org["id"], "claude", secret="nope").status_code == 403 + resp = client.post(f"/internal/organizations/{org['id']}/provider-keys/claude/reveal") + assert resp.status_code == 403 + + +def test_reveal_returns_the_real_decrypted_key(client, org, configured_crypto, reveal_secret): + client.put( + f"/orgs/{org['id']}/provider-keys/claude", json={"api_key": "sk-real-secret-12345"}, + headers=org["owner_headers"], + ) + resp = _reveal(client, org["id"], "claude") + assert resp.status_code == 200 + assert resp.json() == {"provider": "claude", "api_key": "sk-real-secret-12345"} + + +def test_reveal_does_not_require_any_org_membership(client, org, configured_crypto, reveal_secret): + """No user session is involved at all -- a bare shared-secret call + succeeds regardless of who (if anyone) is logged in.""" + client.put(f"/orgs/{org['id']}/provider-keys/claude", json={"api_key": "sk-claude"}, headers=org["owner_headers"]) + resp = client.post( + f"/internal/organizations/{org['id']}/provider-keys/claude/reveal", + headers={"X-Provider-Key-Reveal-Secret": REVEAL_SECRET}, + ) + assert resp.status_code == 200 + + +def test_reveal_404s_when_nothing_is_configured(client, org, reveal_secret): + resp = _reveal(client, org["id"], "claude") + assert resp.status_code == 404 + + +def test_reveal_404s_for_a_provider_that_is_not_the_configured_one(client, org, configured_crypto, reveal_secret): + client.put(f"/orgs/{org['id']}/provider-keys/claude", json={"api_key": "sk-claude"}, headers=org["owner_headers"]) + resp = _reveal(client, org["id"], "openai") + assert resp.status_code == 404 + + +def test_reveal_rejects_an_unsupported_provider_name(client, org, reveal_secret): + resp = _reveal(client, org["id"], "not-a-real-provider") + assert resp.status_code == 400 + + +def test_reveal_fails_loudly_without_an_encryption_key(client, org, reveal_secret): + """A key was stored while encryption was configured (configured_crypto + fixture, scoped to that one call), but reveal is called afterward + with no encryption key available -- decrypt() must raise 500, not + return garbage or the ciphertext itself.""" + import app.core.crypto as crypto + from cryptography.fernet import Fernet + + key = Fernet.generate_key() + original_fernet = crypto._fernet + crypto._fernet = Fernet(key) + try: + client.put( + f"/orgs/{org['id']}/provider-keys/claude", json={"api_key": "sk-claude"}, headers=org["owner_headers"], + ) + finally: + crypto._fernet = original_fernet + + resp = _reveal(client, org["id"], "claude") + assert resp.status_code == 500