diff --git a/app/api/routes_apikeys.py b/app/api/routes_apikeys.py index 38cdb60..ae239b5 100644 --- a/app/api/routes_apikeys.py +++ b/app/api/routes_apikeys.py @@ -10,7 +10,7 @@ from app.db.session import get_db from app.rbac import get_current_user, require_org_permission_or_platform_admin from app.schemas.apikeys import ApiKeyCreate, ApiKeyCreated, ApiKeyExchangeIn, ApiKeyExchangeOut, ApiKeyOut, ApiKeyRename -from app.services import apikey_service, org_service +from app.services import apikey_service, billing_client, org_service router = APIRouter(prefix="/orgs/{org_id}/api-keys", tags=["api-keys"]) exchange_router = APIRouter(prefix="/auth/api-keys", tags=["api-keys"]) @@ -234,6 +234,17 @@ def create_my_api_key( active = _own_keys(db, membership).filter(ApiKey.status == "active").count() if active >= MAX_ACTIVE_KEYS_PER_USER: raise HTTPException(409, f"At most {MAX_ACTIVE_KEYS_PER_USER} active keys; revoke one first") + # M17 (design audit gap #9): auto-enrollment (billing-service's own + # get_subscription_summary) means a brand-new organization's first + # call here still succeeds -- this only actually blocks an + # organization whose subscription has gone to "suspended" (mid + # payment-failure grace period) or "cancelled". Fails open on a + # billing-service outage (see billing_client's own docstring). + if not billing_client.organization_has_active_plan(membership.organization_id): + raise HTTPException( + 402, "Your organization has no active billing plan. " + "Resolve any payment issue, or reactivate a plan, before creating new API keys.", + ) public_scopes = body.scopes or DEFAULT_SELF_SERVICE_SCOPES unknown_scopes = set(public_scopes) - set(PUBLIC_SCOPE_TO_PERMISSION) if unknown_scopes: diff --git a/app/core/config.py b/app/core/config.py index fbcc921..486c66a 100644 --- a/app/core/config.py +++ b/app/core/config.py @@ -347,6 +347,15 @@ class Settings: # why, and the concurrent-login race-safety discussion there. SESSION_MAX_CONCURRENT = int(os.getenv("SESSION_MAX_CONCURRENT", 5)) + # M17 (design audit gap #9's remaining bullet: self-service API-key + # creation gated on an active billing plan). omnibioai-billing's own + # GET /organizations/{id}/subscription already accepts any validly- + # signed platform JWT whose org_id claim matches -- same shared + # signing key both services already trust -- so this needs no new + # shared secret, just the URL to call. See + # app/services/billing_client.py. + BILLING_SERVICE_URL = os.getenv("BILLING_SERVICE_URL", "http://billing-service:8005") + @property def DATABASE_URL(self): return ( diff --git a/app/services/billing_client.py b/app/services/billing_client.py new file mode 100644 index 0000000..bc103f4 --- /dev/null +++ b/app/services/billing_client.py @@ -0,0 +1,71 @@ +"""M17 (design audit gap #9's remaining bullet): self-service API-key +creation should require the organization to have an active billing +plan. The gate itself lives in routes_apikeys.py; this module is the +one cross-service call it needs -- omnibioai-billing owns subscription +state, omnibioai-auth doesn't duplicate it. + +Calls omnibioai-billing's existing GET /organizations/{id}/subscription +(no new billing-service endpoint, no new shared secret): that route +already accepts any validly-signed platform JWT whose org_id claim +matches the organization being queried (see omnibioai-billing's +app/core/iam.py::get_authorized_organization_id) -- the same signing +key this service already uses for every real session token. A +synthetic, minimal, single-use token is minted here (via the same +create_access_token every login already calls) purely to satisfy that +check; it is used for this one outbound call and never stored, logged, +or returned to any caller. + +Auto-enrollment means this call itself creates a Free-plan subscription +for an organization that has never had one at all (see +omnibioai-billing's auto_enrollment_service.py, invoked inside +get_subscription_summary) -- so a brand-new organization's very first +self-service key creation still succeeds; the gate only ever actually +blocks an organization whose subscription has gone to "suspended" +(mid payment-failure grace period) or "cancelled". +""" +import httpx + +from app.core.config import settings +from app.core.jwt import create_access_token + +_ACTIVE_STATUSES = {"active", "trial"} + + +def organization_has_active_plan(organization_id: int) -> bool: + """Fails OPEN (True) on any billing-service error (timeout, + connection refused, non-2xx, malformed response) -- an outage in a + service this one doesn't own must never block self-service key + creation. This is an availability-vs-business-gate tradeoff + deliberately decided the same direction every other cross-service + check in this project's wider system has gone (quota/rate-limit + checks all fail open too); it is not a billing-critical path the + way an actual metered usage event is. + """ + token = create_access_token({ + "sub": "system:billing-plan-check", + "email": "", + "roles": [], + "permissions": [], + "org_id": organization_id, + "org_role": [], + "auth_method": "service", + }) + try: + resp = httpx.get( + f"{settings.BILLING_SERVICE_URL}/organizations/{organization_id}/subscription", + headers={"Authorization": f"Bearer {token}"}, + timeout=3, + ) + except httpx.HTTPError: + return True + if resp.status_code == 404: + # NoActiveSubscriptionError -- auto-enrollment should make this + # unreachable in practice (see module docstring), but if it ever + # happens, "no subscription at all" is not an active plan. + return False + if resp.status_code != 200: + return True + try: + return resp.json().get("status") in _ACTIVE_STATUSES + except ValueError: + return True diff --git a/tests/test_billing_client.py b/tests/test_billing_client.py new file mode 100644 index 0000000..b2cfa53 --- /dev/null +++ b/tests/test_billing_client.py @@ -0,0 +1,84 @@ +"""app/services/billing_client.py: the one cross-service call behind +design audit gap #9's "self-service key creation gated on an active +billing plan." Mocks httpx.get directly -- no real omnibioai-billing +call is ever made in this suite. + +Developer: Manish Kumar +""" +from unittest.mock import MagicMock, patch + +from app.services import billing_client + + +def _response(status_code, json_body=None, raises_on_json=False): + resp = MagicMock() + resp.status_code = status_code + if raises_on_json: + resp.json.side_effect = ValueError("not json") + else: + resp.json.return_value = json_body or {} + return resp + + +def test_active_status_is_true(): + with patch.object(billing_client.httpx, "get", return_value=_response(200, {"status": "active"})): + assert billing_client.organization_has_active_plan(42) is True + + +def test_trial_status_is_true(): + with patch.object(billing_client.httpx, "get", return_value=_response(200, {"status": "trial"})): + assert billing_client.organization_has_active_plan(42) is True + + +def test_suspended_status_is_false(): + with patch.object(billing_client.httpx, "get", return_value=_response(200, {"status": "suspended"})): + assert billing_client.organization_has_active_plan(42) is False + + +def test_cancelled_status_is_false(): + with patch.object(billing_client.httpx, "get", return_value=_response(200, {"status": "cancelled"})): + assert billing_client.organization_has_active_plan(42) is False + + +def test_404_no_subscription_at_all_is_false(): + """NoActiveSubscriptionError -- auto-enrollment should make this + unreachable in practice, but a genuine "nothing at all" state is + correctly not an active plan.""" + with patch.object(billing_client.httpx, "get", return_value=_response(404)): + assert billing_client.organization_has_active_plan(42) is False + + +def test_network_error_fails_open(): + import httpx + + with patch.object(billing_client.httpx, "get", side_effect=httpx.ConnectError("refused")): + assert billing_client.organization_has_active_plan(42) is True + + +def test_5xx_from_billing_service_fails_open(): + with patch.object(billing_client.httpx, "get", return_value=_response(500)): + assert billing_client.organization_has_active_plan(42) is True + + +def test_malformed_json_response_fails_open(): + with patch.object(billing_client.httpx, "get", return_value=_response(200, raises_on_json=True)): + assert billing_client.organization_has_active_plan(42) is True + + +def test_sends_a_token_scoped_to_the_requested_organization(): + from app.core.jwt import decode_token + + captured = {} + + def fake_get(url, headers=None, timeout=None): + captured["url"] = url + captured["token"] = headers["Authorization"].removeprefix("Bearer ") + return _response(200, {"status": "active"}) + + with patch.object(billing_client.httpx, "get", side_effect=fake_get): + billing_client.organization_has_active_plan(99) + + assert captured["url"].endswith("/organizations/99/subscription") + claims = decode_token(captured["token"]) + assert claims["org_id"] == 99 + assert claims["type"] == "access" diff --git a/tests/test_me_api_keys.py b/tests/test_me_api_keys.py index 0930a9e..590ecf0 100644 --- a/tests/test_me_api_keys.py +++ b/tests/test_me_api_keys.py @@ -10,6 +10,7 @@ """ import uuid from datetime import datetime, timedelta +from unittest.mock import patch import pytest from sqlalchemy import create_engine @@ -57,6 +58,22 @@ def scientist(client): return {"org_id": org["id"], "headers": _hdr(_login(client, email)), "email": email} +# ── M17: self-service creation gated on an active billing plan (gap #9) ──── + + +def test_create_is_blocked_when_the_organization_has_no_active_plan(client, scientist): + with patch.object(routes_apikeys.billing_client, "organization_has_active_plan", return_value=False): + resp = client.post("/me/api-keys", json={"name": "notebook"}, headers=scientist["headers"]) + assert resp.status_code == 402 + + +def test_create_succeeds_when_the_organization_has_an_active_plan(client, scientist): + with patch.object(routes_apikeys.billing_client, "organization_has_active_plan", return_value=True) as check: + resp = client.post("/me/api-keys", json={"name": "notebook"}, headers=scientist["headers"]) + assert resp.status_code == 201 + check.assert_called_once_with(scientist["org_id"]) + + def test_create_defaults_to_literature_read_and_lists_own_keys(client, scientist): resp = client.post("/me/api-keys", json={"name": "notebook"}, headers=scientist["headers"]) assert resp.status_code == 201