From 5f3424a02e6ad24bb9675bad09a44d26d654d2ac Mon Sep 17 00:00:00 2001 From: pk-zipstack Date: Tue, 1 Sep 2026 16:43:30 +0530 Subject: [PATCH 1/6] UN-4016 [FEAT] Resolve the organisation from a platform API key via a whoami endpoint A caller holding a platform API key knows the secret but not what it is scoped to: `organization_id` is only discoverable by reading it out of a web-app URL, and every organisation-scoped endpoint takes it as a path segment. This adds `GET /api/v1/unstract/whoami/`, which returns `{organization_id, organization_name, permission, key_name}` read off the key row itself. The route carries no organisation segment, and that is what made it more than a view. `OrganizationMiddleware` matches `^/api/v1/unstract//`, so it parsed `whoami` as the organisation and rewrote the path to a 404. Its whitelist escape hatch then returned without ever setting `request.organization_id`, which `CustomAuthMiddleware` reads by bare attribute access -- a 500 rather than a skip. Both are fixed: the path is whitelisted for the organisation middleware only (it still authenticates), and the whitelist branch now sets the attribute, which protects any future organisation-less path. The endpoint also flows through the committed OpenAPI spec, which needed three gates widened: the published-prefix check now accepts the tenant mount as well as the deployment one, `SPEC_URLCONFS` gains the new urlconf, and the two spec tests that looped over every operation asserting deployment-specific facts now pin those facts to the deployment operations and check only genuinely universal ones globally. The regenerated spec is +160/-0 -- the deployment contract is byte-identical. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM --- backend/api_v2/deployment_spec_urls.py | 2 +- .../commands/generate_docstudio_spec.py | 23 ++- backend/api_v2/tests/test_docstudio_spec.py | 97 ++++++++++- backend/backend/base_urls.py | 8 + backend/backend/settings/base.py | 34 +++- backend/middleware/organization_middleware.py | 5 + backend/platform_api/openapi_schema.py | 80 +++++++++ backend/platform_api/tests/test_whoami.py | 147 ++++++++++++++++ backend/platform_api/whoami_urls.py | 15 ++ backend/platform_api/whoami_views.py | 53 ++++++ specs/docstudio-oss.json | 160 ++++++++++++++++++ 11 files changed, 599 insertions(+), 25 deletions(-) create mode 100644 backend/platform_api/openapi_schema.py create mode 100644 backend/platform_api/tests/test_whoami.py create mode 100644 backend/platform_api/whoami_urls.py create mode 100644 backend/platform_api/whoami_views.py diff --git a/backend/api_v2/deployment_spec_urls.py b/backend/api_v2/deployment_spec_urls.py index 9dff155d9d..727997ee0f 100644 --- a/backend/api_v2/deployment_spec_urls.py +++ b/backend/api_v2/deployment_spec_urls.py @@ -13,7 +13,7 @@ from backend import base_urls -SPEC_URLCONFS = ("api_v2.execution_urls",) +SPEC_URLCONFS = ("api_v2.execution_urls", "platform_api.whoami_urls") urlpatterns = [ entry diff --git a/backend/api_v2/management/commands/generate_docstudio_spec.py b/backend/api_v2/management/commands/generate_docstudio_spec.py index dc5ddabe88..a67e00b2f4 100644 --- a/backend/api_v2/management/commands/generate_docstudio_spec.py +++ b/backend/api_v2/management/commands/generate_docstudio_spec.py @@ -25,10 +25,12 @@ DEFAULT_OUT = Path(__file__).resolve().parents[4] / "specs" / "docstudio-oss.json" URLCONF = "api_v2.deployment_spec_urls" REGENERATE = "uv run python manage.py generate_docstudio_spec" -# The mount the deployment is served at publicly. `API_DEPLOYMENT_PATH_PREFIX` -# can move it per installation, and a spec carrying a private prefix would send -# every generated client to a URL only that installation answers. -PUBLISHED_PATH_PREFIX = "deployment" +# The mounts these routes are served at publicly. `API_DEPLOYMENT_PATH_PREFIX` +# and `PATH_PREFIX` can move them per installation, and a spec carrying a +# private prefix would send every generated client to a URL only that +# installation answers. Written as literals rather than read from settings, so +# an override fails the gate rather than being baked into the artifact. +PUBLISHED_PATH_PREFIXES = ("deployment", "api/v1/unstract") # Named in every failure message: the repos that regenerate from this file are # the ones a spec change actually breaks, and nothing there watches this repo. DOWNSTREAM = ( @@ -69,16 +71,13 @@ def render_spec() -> str: f"API nobody implements:\n{diagnostics}" ) - off_prefix = [ - path - for path in schema["paths"] - if not path.startswith(f"/{PUBLISHED_PATH_PREFIX}/") - ] + published = tuple(f"/{prefix}/" for prefix in PUBLISHED_PATH_PREFIXES) + off_prefix = [path for path in schema["paths"] if not path.startswith(published)] if off_prefix: raise SpecGenerationFailed( - f"Generated paths are not under /{PUBLISHED_PATH_PREFIX}/: " - f"{', '.join(sorted(off_prefix))}. Unset API_DEPLOYMENT_PATH_PREFIX " - f"and regenerate." + f"Generated paths are outside the published mounts " + f"({', '.join(published)}): {', '.join(sorted(off_prefix))}. Unset " + f"API_DEPLOYMENT_PATH_PREFIX and PATH_PREFIX and regenerate." ) # Hand-written fragments (path parameter schemas, security schemes) reach diff --git a/backend/api_v2/tests/test_docstudio_spec.py b/backend/api_v2/tests/test_docstudio_spec.py index 218975c969..ca81c7f36a 100644 --- a/backend/api_v2/tests/test_docstudio_spec.py +++ b/backend/api_v2/tests/test_docstudio_spec.py @@ -19,6 +19,7 @@ from drf_spectacular.drainage import warn from drf_spectacular.generators import SchemaGenerator from middleware.exception import drf_logging_exc_handler +from platform_api.models import ApiKeyPermission from rest_framework.exceptions import APIException, ValidationError from rest_framework.test import APIRequestFactory from workflow_manager.endpoint_v2.dto import FileExecutionResult @@ -41,6 +42,11 @@ #: up from the extraction metadata when the request asks for it. _PROMOTED_FILE_RESULT_FIELDS = {"extracted_text"} +#: The operations served by an API deployment, as opposed to the platform-key +#: operations that describe the account. They authenticate differently and can +#: fail differently, so several checks below split on this. +DEPLOYMENT_OPERATIONS = {"execute", "status"} + def _committed() -> dict: return json.loads(DEFAULT_OUT.read_text()) @@ -115,23 +121,62 @@ def test_spec_documents_the_deployment_operations() -> None: assert "deployment" in [tag["name"] for tag in spec["tags"]] -def test_operations_require_the_deployment_key() -> None: +def test_every_operation_names_the_credential_it_takes() -> None: """Without this the unset DRF authentication default is published as though it were a decision, and no generated client can authenticate. + + Which credential differs by operation -- a deployment key runs a + deployment, a platform key describes itself -- so what is pinned here is + that each operation names exactly one, and that the scheme it names is + declared and is a bearer token. """ spec = _committed() - scheme = spec["components"]["securitySchemes"]["deploymentKey"] + schemes = spec["components"]["securitySchemes"] - assert (scheme["type"], scheme["scheme"]) == ("http", "bearer") for path, method, operation in _operations(spec): - assert operation["security"] == [{"deploymentKey": []}], f"{method} {path}" + security = operation["security"] + assert len(security) == 1, f"{method} {path}" + (requirement,) = security + (name,) = requirement + assert requirement[name] == [], f"{method} {path}" + assert (schemes[name]["type"], schemes[name]["scheme"]) == ( + "http", + "bearer", + ), f"{method} {path}" + + +def test_the_deployment_operations_take_the_deployment_key() -> None: + """The credential each operation names is part of its contract, so the + pairing is pinned rather than left to the loop above. + """ + for path, method, operation in _operations(_committed()): + if operation["operationId"] in DEPLOYMENT_OPERATIONS: + assert operation["security"] == [{"deploymentKey": []}], f"{method} {path}" + else: + assert operation["security"] == [{"platformKey": []}], f"{method} {path}" def test_clients_can_branch_on_every_failure_they_will_see() -> None: + """Every operation authenticates and can fail on the server, so these three + are the branches a client needs whatever it is calling. + """ for path, method, operation in _operations(_committed()): - assert {"400", "401", "403", "404", "500"} <= set( - operation["responses"] - ), f"{method} {path}" + assert {"401", "403", "500"} <= set(operation["responses"]), f"{method} {path}" + + +def test_the_deployment_operations_document_a_rejected_request_and_a_missing_one() -> ( + None +): + """Kept off the universal check above: a request carrying no body and + naming no resource cannot be malformed or miss its target, and documenting + a status an operation cannot return hands clients a dead branch. + """ + for path, method, operation in _operations(_committed()): + declared = {"400", "404"} & set(operation["responses"]) + if operation["operationId"] in DEPLOYMENT_OPERATIONS: + assert declared == {"400", "404"}, f"{method} {path}" + else: + assert not declared, f"{method} {path}" def test_only_the_execution_endpoint_documents_the_statuses_only_it_returns() -> None: @@ -216,6 +261,44 @@ def test_the_status_read_documents_the_two_keys_it_returns() -> None: ) +def test_spec_documents_the_identity_operation() -> None: + spec = _committed() + documented = {operation["operationId"] for _, _, operation in _operations(spec)} + + assert "whoami" in documented + assert "identity" in [tag["name"] for tag in spec["tags"]] + + +def test_the_identity_read_documents_the_keys_it_returns() -> None: + """The view builds its body literally, so the spec is the only place the + set is written down. + """ + whoami = _schema("WhoAmIResponse") + fields = {"organization_id", "organization_name", "permission", "key_name"} + + assert set(whoami["properties"]) == fields + # All four are read off a key row that always has them, so a client can + # treat every one as present rather than guarding each. + assert set(whoami["required"]) == fields + + +def test_the_documented_permission_tiers_are_the_ones_the_model_defines() -> None: + """A tier added to the model but not the spec reaches clients as a value + their generated enum rejects. + """ + assert _schema("ApiKeyPermission")["enum"] == list(ApiKeyPermission.values) + + +def test_the_identity_read_asks_for_no_organisation() -> None: + """Resolving the organisation from the key is the whole point: a path + parameter here would mean the caller had to know the answer first. + """ + for path, _, operation in _operations(_committed()): + if operation["operationId"] == "whoami": + assert "{" not in path, path + assert not operation.get("parameters"), path + + @pytest.mark.parametrize( "exc", [APIException("Unauthorized"), ValidationError("at least one file is required")], diff --git a/backend/backend/base_urls.py b/backend/backend/base_urls.py index 8450add04d..b6be18d87f 100644 --- a/backend/backend/base_urls.py +++ b/backend/backend/base_urls.py @@ -9,6 +9,14 @@ # Combine the URL patterns urlpatterns = [ + # Organisation-less, and so mounted ahead of the tenant urlconf rather than + # relying on falling through it. `OrganizationMiddleware` rewrites every + # organisation-scoped path before routing, and none of those rewrites can + # produce `whoami/`, so this shadows nothing. + path( + f"{settings.TENANT_SUBFOLDER_PREFIX}/", + include("platform_api.whoami_urls"), + ), path( f"{settings.TENANT_SUBFOLDER_PREFIX}/", include((tenant_urls, "tenant"), namespace="tenant"), diff --git a/backend/backend/settings/base.py b/backend/backend/settings/base.py index 6edee88d73..0d4c979593 100644 --- a/backend/backend/settings/base.py +++ b/backend/backend/settings/base.py @@ -671,12 +671,24 @@ def filter(self, record): "type": "http", "scheme": "bearer", "description": "The API deployment's own key.", - } + }, + "platformKey": { + "type": "http", + "scheme": "bearer", + "description": ( + "An organisation-wide platform API key, minted under " + "Settings. It carries the organisation it belongs to, but " + "cannot execute an API deployment." + ), + }, } }, # Without this the enum component is named after the field that holds it, # and generated clients get a class called `TypeEnum`. - "ENUM_NAME_OVERRIDES": {"ErrorType": "api_v2.openapi_schema.ERROR_TYPES"}, + "ENUM_NAME_OVERRIDES": { + "ErrorType": "api_v2.openapi_schema.ERROR_TYPES", + "ApiKeyPermission": "platform_api.models.ApiKeyPermission.choices", + }, # Group descriptions generated clients show in their help; without this # the spec has no root `tags` array for the text to live in. "TAGS": [ @@ -686,7 +698,14 @@ def filter(self, record): "Run an API deployment against one or more documents and poll " "the result." ), - } + }, + { + "name": "identity", + "description": ( + "Resolve what a platform API key is scoped to, so a client can " + "discover its organisation rather than being told it." + ), + }, ], } @@ -709,8 +728,13 @@ def filter(self, record): # Whitelisting health check API WHITELISTED_PATHS.append("/health") -# These path will work without organization in request -ORGANIZATION_MIDDLEWARE_WHITELISTED_PATHS = [] +# These path will work without organization in request. +# `whoami` resolves the organisation from the API key itself, so it carries no +# organisation segment -- without this the middleware would read `whoami` as one. +# Note this is not WHITELISTED_PATHS: the endpoint still authenticates. +ORGANIZATION_MIDDLEWARE_WHITELISTED_PATHS = [ + rf"^/{PATH_PREFIX}/unstract/whoami/", +] # Social Auth Settings SOCIAL_AUTH_LOGIN_REDIRECT_URL = f"{WEB_APP_ORIGIN_URL}/oauth-status/?status=success" diff --git a/backend/middleware/organization_middleware.py b/backend/middleware/organization_middleware.py index 5b0ffa6052..75000014b5 100644 --- a/backend/middleware/organization_middleware.py +++ b/backend/middleware/organization_middleware.py @@ -16,6 +16,11 @@ def process_request(self, request): re.match(path, request.path) for path in settings.ORGANIZATION_MIDDLEWARE_WHITELISTED_PATHS ): + # Set before returning: downstream middleware reads this + # attribute directly, so leaving it undefined on a whitelisted + # path raises AttributeError rather than skipping the org check + # the way returning here intends. + request.organization_id = None return org_id = match.group("org_id") diff --git a/backend/platform_api/openapi_schema.py b/backend/platform_api/openapi_schema.py new file mode 100644 index 0000000000..9dd627ac3a --- /dev/null +++ b/backend/platform_api/openapi_schema.py @@ -0,0 +1,80 @@ +"""OpenAPI annotation for the ``whoami`` endpoint. + +The serializer here shapes the published spec only; it never parses a request +or builds a response. It lives outside ``serializers.py`` so that nothing at +request time imports it by accident, matching ``api_v2.openapi_schema``. + +Its docstring and help texts are published as the client-facing descriptions, +so they are written for the caller rather than the maintainer. + +The error body is imported rather than restated: it comes from the project-wide +exception handler, so it is the same shape for every endpoint in the spec. +""" + +from api_v2.openapi_schema import ErrorResponse +from drf_spectacular.utils import ( + OpenApiResponse, + extend_schema, + extend_schema_view, +) +from rest_framework import serializers + +from platform_api.models import ApiKeyPermission + + +class WhoAmIResponse(serializers.Serializer): + """The organisation a platform API key belongs to, and what it may do.""" + + organization_id = serializers.CharField( + help_text="The organisation's identifier, as it appears in web-app URLs " + "and in every organisation-scoped API path." + ) + organization_name = serializers.CharField( + help_text="The organisation's display name." + ) + permission = serializers.ChoiceField( + # Sourced from the model so a new tier cannot reach the API without + # reaching the spec. + choices=ApiKeyPermission.choices, + help_text="The key's permission tier, which decides the HTTP methods it " + "may issue.", + ) + key_name = serializers.CharField(help_text="The key's name, as it was minted.") + + +WHOAMI_DESCRIPTION = ( + "Resolve the organisation a platform API key belongs to.\n\n" + "The organisation is read from the key itself, so this route carries no " + "organisation segment and needs nothing but the key. Call it once and store " + "`organization_id`; every other endpoint takes it as a path segment.\n\n" + "Only a platform API key is accepted. An API deployment key authenticates " + "against a different table on a path that never reaches this endpoint, and " + "is rejected as unauthenticated." +) + + +# Generated clients take their method names and module paths from here, so this +# is part of the public API surface. +WHOAMI_SCHEMA = extend_schema_view( + get=extend_schema( + operation_id="whoami", + tags=["identity"], + auth=[{"platformKey": []}], + responses={ + 200: WhoAmIResponse, + 401: OpenApiResponse( + ErrorResponse, + description="No usable platform API key was supplied.", + ), + 403: OpenApiResponse( + ErrorResponse, + description="The key is not permitted to issue this request.", + ), + 500: OpenApiResponse( + ErrorResponse, + description="The organisation could not be resolved.", + ), + }, + description=WHOAMI_DESCRIPTION, + ), +) diff --git a/backend/platform_api/tests/test_whoami.py b/backend/platform_api/tests/test_whoami.py new file mode 100644 index 0000000000..a76957c8b4 --- /dev/null +++ b/backend/platform_api/tests/test_whoami.py @@ -0,0 +1,147 @@ +"""Request-level tests for the organisation-less ``whoami`` endpoint. + +Everything that makes this endpoint work happens before the view: the route has +to survive `OrganizationMiddleware`, which reads the first path segment after +`unstract/` as an organisation and would otherwise take `whoami` for one, and +the organisation has to arrive from the key row rather than from the URL. None +of that is observable from the view in isolation, so these go through the real +URLconf and a real middleware chain. +""" + +import secrets +import uuid + +import pytest +from account_v2.models import Organization, User +from django.conf import settings +from django.test import override_settings +from django.urls import Resolver404, resolve +from platform_api.models import ApiKeyPermission, PlatformApiKey +from rest_framework.test import APITestCase + +ORG_A = "org-a" +ORG_B = "org-b" + +WHOAMI_URL = f"/{settings.PATH_PREFIX}/unstract/whoami/" + +# Trimmed from the production chain, preserving its relative order. The cloud +# test settings drop CustomAuthMiddleware, so pinning the list keeps this suite +# behaving the same in both trees. +_MIDDLEWARE = [ + "middleware.request_id.CustomRequestIDMiddleware", + settings.TENANT_MIDDLEWARE, + "django.contrib.sessions.middleware.SessionMiddleware", + "django.middleware.csrf.CsrfViewMiddleware", + "django.contrib.auth.middleware.AuthenticationMiddleware", + settings.CUSTOM_AUTH_MIDDLEWARE, +] + + +@pytest.mark.critical_path("platform-key-whoami") +@override_settings(MIDDLEWARE=_MIDDLEWARE) +class WhoAmITest(APITestCase): + def setUp(self) -> None: + self.org_a = Organization.objects.create( + name=ORG_A, display_name="Org A", organization_id=ORG_A + ) + self.org_b = Organization.objects.create( + name=ORG_B, display_name="Org B", organization_id=ORG_B + ) + + @staticmethod + def _make_user() -> User: + email = f"svc-{uuid.uuid4().hex[:8]}@platform.internal" + return User.objects.create_user( + username=email, email=email, password=secrets.token_urlsafe() + ) + + def _make_key(self, organization=None, api_user=..., **kwargs) -> PlatformApiKey: + # A fresh service account per key: api_user is a OneToOneField, so + # sharing one across two keys in the same test is an IntegrityError. + return PlatformApiKey.objects.create( + name=f"key-{uuid.uuid4().hex[:8]}", + description="test key", + organization=organization or self.org_a, + api_user=self._make_user() if api_user is ... else api_user, + **kwargs, + ) + + def _get(self, token: str | None = None, url: str = WHOAMI_URL): + headers = {"HTTP_AUTHORIZATION": f"Bearer {token}"} if token else {} + return self.client.get(url, **headers) + + # --- routing ----------------------------------------------------------- + + def test_the_route_is_reachable(self) -> None: + """A 404 from the organisation regex eating `whoami` would otherwise + look identical to a rejected request in every test below. + """ + try: + resolve(WHOAMI_URL) + except Resolver404: # pragma: no cover - the failure this guards + self.fail(f"{WHOAMI_URL} resolves to nothing; the route is not mounted") + + def test_the_endpoint_is_not_whitelisted(self) -> None: + """WHITELISTED_PATHS skips authentication entirely. This endpoint has + nothing to say without a key, so landing there would make it answer + with someone else's organisation or none at all. + """ + assert not any( + WHOAMI_URL.startswith(path) for path in settings.WHITELISTED_PATHS + ), f"{WHOAMI_URL} is whitelisted — it would bypass authentication entirely" + + def test_the_organisation_middleware_still_defines_organisation_id(self) -> None: + """The whitelist branch returns early. Downstream middleware reads the + attribute directly, so leaving it unset is a 500 rather than a skip. + """ + response = self._get(str(self._make_key().key)) + self.assertEqual(response.status_code, 200) + + # --- the answer -------------------------------------------------------- + + def test_a_key_describes_itself(self) -> None: + key = self._make_key(permission=ApiKeyPermission.READ) + response = self._get(str(key.key)) + + self.assertEqual(response.status_code, 200) + self.assertEqual( + response.json(), + { + "organization_id": ORG_A, + "organization_name": "Org A", + "permission": ApiKeyPermission.READ.value, + "key_name": key.name, + }, + ) + + def test_the_organisation_comes_from_the_key_not_the_url(self) -> None: + """The point of the endpoint: two keys, one URL, two answers.""" + key_a = self._make_key(organization=self.org_a) + key_b = self._make_key(organization=self.org_b) + + self.assertEqual(self._get(str(key_a.key)).json()["organization_id"], ORG_A) + self.assertEqual(self._get(str(key_b.key)).json()["organization_id"], ORG_B) + + def test_every_tier_can_read_its_own_identity(self) -> None: + """The tier gates methods, not endpoints, and this is a GET.""" + for tier in ApiKeyPermission: + with self.subTest(tier=tier.value): + key = self._make_key(permission=tier) + response = self._get(str(key.key)) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["permission"], tier.value) + + # --- rejections -------------------------------------------------------- + + def test_a_request_with_no_key_is_rejected(self) -> None: + self.assertEqual(self._get().status_code, 401) + + def test_a_malformed_token_is_rejected(self) -> None: + self.assertEqual(self._get("not-a-uuid").status_code, 401) + + def test_an_unknown_key_is_rejected(self) -> None: + self.assertEqual(self._get(str(uuid.uuid4())).status_code, 401) + + def test_an_inactive_key_is_rejected(self) -> None: + key = self._make_key(is_active=False) + self.assertEqual(self._get(str(key.key)).status_code, 401) diff --git a/backend/platform_api/whoami_urls.py b/backend/platform_api/whoami_urls.py new file mode 100644 index 0000000000..31ada0e847 --- /dev/null +++ b/backend/platform_api/whoami_urls.py @@ -0,0 +1,15 @@ +"""The organisation-less ``whoami`` route. + +Kept apart from ``platform_api.urls`` because it is mounted directly on the +tenant prefix rather than under ``platform-api/``, and because the OpenAPI +spec's urlconf selector can only pick out a mount declared with a dotted +module path (see ``api_v2.deployment_spec_urls``). +""" + +from django.urls import path + +from platform_api.whoami_views import WhoAmIView + +urlpatterns = [ + path("whoami/", WhoAmIView.as_view(), name="platform_whoami"), +] diff --git a/backend/platform_api/whoami_views.py b/backend/platform_api/whoami_views.py new file mode 100644 index 0000000000..8452233f17 --- /dev/null +++ b/backend/platform_api/whoami_views.py @@ -0,0 +1,53 @@ +"""Describe the platform API key a request is authenticated with. + +A caller holding a key knows the secret but not what it is scoped to, and the +organisation identifier is otherwise only discoverable by reading it out of a +web-app URL. This endpoint answers "which organisation does this key belong +to?", so a client can resolve it once and store it. + +The organisation is read off the key row, never off the URL: ``PlatformApiKey`` +carries an ``organization`` FK stamped at mint time and a globally unique +``key``, so a bearer token maps to exactly one organisation by construction. +That is why this route carries no organisation segment. +""" + +from rest_framework import status, views +from rest_framework.exceptions import NotAuthenticated +from rest_framework.request import Request +from rest_framework.response import Response + +from platform_api.openapi_schema import WHOAMI_SCHEMA + + +@WHOAMI_SCHEMA +class WhoAmIView(views.APIView): + """Report the organisation and scope of the calling platform API key.""" + + # Authentication is CustomAuthMiddleware's job: it resolves the Bearer + # token to a key row, binds the service account to request.user and + # enforces the key's permission tier against the method. A permission + # class here would only re-ask a question already answered. + permission_classes: list = [] + + # authentication_classes is deliberately not set. The project configures no + # DEFAULT_AUTHENTICATION_CLASSES that resolves a user, so DRF's default + # returns None and the middleware's request.user survives into the view. + + def get(self, request: Request) -> Response: + key = getattr(request, "platform_api_key", None) + if key is None: + # A session-authenticated browser user reaches this with no key to + # describe. There is nothing to report, and reporting the session's + # organisation instead would answer a question nobody asked. + raise NotAuthenticated("This endpoint requires a platform API key.") + + organization = key.organization + return Response( + { + "organization_id": organization.organization_id, + "organization_name": organization.display_name, + "permission": key.permission, + "key_name": key.name, + }, + status=status.HTTP_200_OK, + ) diff --git a/specs/docstudio-oss.json b/specs/docstudio-oss.json index c7f8ebb4b0..66498cfa1c 100644 --- a/specs/docstudio-oss.json +++ b/specs/docstudio-oss.json @@ -17,6 +17,15 @@ ], "type": "object" }, + "ApiKeyPermission": { + "description": "* `read` - Read\n* `read_write` - Read/Write\n* `full_access` - Full Access", + "enum": [ + "read", + "read_write", + "full_access" + ], + "type": "string" + }, "ErrorDetail": { "description": "One problem found with the request.", "properties": { @@ -226,6 +235,38 @@ "status" ], "type": "object" + }, + "WhoAmIResponse": { + "description": "The organisation a platform API key belongs to, and what it may do.", + "properties": { + "key_name": { + "description": "The key's name, as it was minted.", + "type": "string" + }, + "organization_id": { + "description": "The organisation's identifier, as it appears in web-app URLs and in every organisation-scoped API path.", + "type": "string" + }, + "organization_name": { + "description": "The organisation's display name.", + "type": "string" + }, + "permission": { + "allOf": [ + { + "$ref": "#/components/schemas/ApiKeyPermission" + } + ], + "description": "The key's permission tier, which decides the HTTP methods it may issue.\n\n* `read` - Read\n* `read_write` - Read/Write\n* `full_access` - Full Access" + } + }, + "required": [ + "key_name", + "organization_id", + "organization_name", + "permission" + ], + "type": "object" } }, "securitySchemes": { @@ -233,6 +274,11 @@ "description": "The API deployment's own key.", "scheme": "bearer", "type": "http" + }, + "platformKey": { + "description": "An organisation-wide platform API key, minted under Settings. It carries the organisation it belongs to, but cannot execute an API deployment.", + "scheme": "bearer", + "type": "http" } } }, @@ -242,6 +288,116 @@ }, "openapi": "3.0.3", "paths": { + "/api/v1/unstract/whoami/": { + "get": { + "description": "Resolve the organisation a platform API key belongs to.\n\nThe organisation is read from the key itself, so this route carries no organisation segment and needs nothing but the key. Call it once and store `organization_id`; every other endpoint takes it as a path segment.\n\nOnly a platform API key is accepted. An API deployment key authenticates against a different table on a path that never reaches this endpoint, and is rejected as unauthenticated.", + "operationId": "whoami", + "responses": { + "200": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/WhoAmIResponse" + } + } + }, + "description": "" + }, + "401": { + "content": { + "application/json": { + "examples": { + "AuthenticationFailed": { + "value": { + "errors": [ + { + "attr": null, + "code": "authentication_failed", + "detail": "Incorrect authentication credentials." + } + ], + "type": "client_error" + } + }, + "NotAuthenticated": { + "value": { + "errors": [ + { + "attr": null, + "code": "not_authenticated", + "detail": "Authentication credentials were not provided." + } + ], + "type": "client_error" + } + } + }, + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + }, + "description": "No usable platform API key was supplied." + }, + "403": { + "content": { + "application/json": { + "examples": { + "PermissionDenied": { + "value": { + "errors": [ + { + "attr": null, + "code": "permission_denied", + "detail": "You do not have permission to perform this action." + } + ], + "type": "client_error" + } + } + }, + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + }, + "description": "The key is not permitted to issue this request." + }, + "500": { + "content": { + "application/json": { + "examples": { + "APIException": { + "value": { + "errors": [ + { + "attr": null, + "code": "error", + "detail": "A server error occurred." + } + ], + "type": "server_error" + } + } + }, + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + }, + "description": "The organisation could not be resolved." + } + }, + "security": [ + { + "platformKey": [] + } + ], + "tags": [ + "identity" + ] + } + }, "/deployment/api/{org_name}/{api_name}/": { "get": { "description": "Read the result of a previously started execution.\n\nThis read is one-shot: the first call that observes a completed execution acknowledges it and the stored result is discarded, so every later call for that execution answers 406. Poll while the execution is pending, and keep the payload of the call that returns it \u2014 it cannot be fetched again.\n\nA still-running execution answers 422 carrying its current `status`, so a polling loop should treat 422 as the normal reply and stop on 200. Clients that raise on any non-2xx need to allow for that.", @@ -717,6 +873,10 @@ { "description": "Run an API deployment against one or more documents and poll the result.", "name": "deployment" + }, + { + "description": "Resolve what a platform API key is scoped to, so a client can discover its organisation rather than being told it.", + "name": "identity" } ] } From b64b6f99f82dd3f97ddbcfdbfd0188e641b90e59 Mon Sep 17 00:00:00 2001 From: pk-zipstack Date: Tue, 1 Sep 2026 17:52:14 +0530 Subject: [PATCH 2/6] UN-4016 [FIX] Remediate review findings: publish the error shape the endpoint sends MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Iteration 1 of unstract:remediation against PR #2269. Fifteen of sixteen findings; F6 is handed back (see below). The two High findings were both invisible from inside the change: F1 — the spec published `ErrorResponse` for 401/403, but `whoami` is deliberately not whitelisted, so `CustomAuthMiddleware` answers every rejection itself with a bare `{"message": ...}` and DRF's exception handler is never reached. A generated client branching on `errors[0].code` would raise on the most common failure the endpoint has. Now declares a `PlatformKeyError` shape that matches the wire, with the 500 carrying no body schema because a Django HTML 500 has none. F2 — `@pytest.mark.critical_path("platform-key-whoami")` named an id in no registry, and `tests/rig/cli.py` sets `overall_exit = 1` on an unknown marker. The rig would have failed the build on every run. CI never caught it because `test` skips on draft PRs. Now registered. Also fixed: the view returned 403 where the spec said 401 (DRF coerces `NotAuthenticated` unless the first authenticator offers a WWW-Authenticate header, and SessionAuthentication offers none) — it now returns 401 explicitly, and the branch has a test, which it never had because every other rejection is answered before the view runs. The whitelist regex is anchored, so an organisation named `whoami` no longer has its whole API treated as organisation-less. The organisation-scoped alias `//whoami/`, which the mount comment wrongly claimed could not exist, is now documented and tested in both directions. The dominant defect class was not any single bug: seven of sixteen findings were confidently-worded comments asserting mechanisms the code does not implement — that DRF resolves no user, that the org FK is non-null by construction, that no rewrite can produce `whoami/`, that a test observes middleware it never invokes. Each is corrected to what the code does, or deleted. Every fix is mutation-checked: reverting it fails a named test. That check also caught one of the new tests passing vacuously — it matched an error string the OpenAPI-validity gate produces, so it went green with the gate it was written for removed. F6 is not fixed here and needs a decision: setting `organization_id = None` makes `whoami` bypass `SubscriptionMiddleware` in the enterprise tree, where every other org-less path has an explicit `SUBSCRIPTION_WHITELISTED_PATHS_LIST` entry. The fix belongs in unstract-cloud and the intent is a product call. Findings: F1 F2 F3 F4 F5 F7(partial) F8 F9 F10 F11 F12 F13 F14 F15 F16 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM --- backend/README.md | 3 +- .../commands/generate_docstudio_spec.py | 8 +- backend/api_v2/tests/test_docstudio_spec.py | 61 ++++++++++++- backend/backend/base_urls.py | 13 ++- backend/backend/settings/base.py | 4 +- backend/platform_api/openapi_schema.py | 50 +++++++---- backend/platform_api/tests/test_whoami.py | 89 +++++++++++++++++-- backend/platform_api/whoami_views.py | 34 ++++--- specs/docstudio-oss.json | 46 ++++------ tests/critical_paths.yaml | 5 ++ 10 files changed, 242 insertions(+), 71 deletions(-) diff --git a/backend/README.md b/backend/README.md index bd3ea6f0ad..9d14f331ad 100644 --- a/backend/README.md +++ b/backend/README.md @@ -175,7 +175,8 @@ psql -d unstract_db -U unstract_dev ## API Docs -The OpenAPI spec for the API deployment endpoints is committed at +The OpenAPI spec for the publicly served API -- the API deployment endpoints, +and the organisation-less `whoami` -- is committed at [`specs/docstudio-oss.json`](../specs/docstudio-oss.json) and is the contract the published clients and their generated SDKs are built from. It is not served at runtime — regenerate it in the same PR as any route, serializer or schema-annotation change: diff --git a/backend/api_v2/management/commands/generate_docstudio_spec.py b/backend/api_v2/management/commands/generate_docstudio_spec.py index a67e00b2f4..ad7b1c784b 100644 --- a/backend/api_v2/management/commands/generate_docstudio_spec.py +++ b/backend/api_v2/management/commands/generate_docstudio_spec.py @@ -7,10 +7,10 @@ uv run python manage.py generate_docstudio_spec # from backend/ uv run python manage.py generate_docstudio_spec --check # no write, drift is an error -The generated paths carry ``API_DEPLOYMENT_PATH_PREFIX``, so generation refuses -to produce a spec mounted anywhere but the public default: the committed -artifact describes the deployment as it is served publicly, not as one -installation chooses to mount it. +The generated paths carry ``API_DEPLOYMENT_PATH_PREFIX`` or ``PATH_PREFIX`` +depending on the mount, so generation refuses to produce a spec mounted anywhere +but the public defaults: the committed artifact describes the API as it is +served publicly, not as one installation chooses to mount it. """ import json diff --git a/backend/api_v2/tests/test_docstudio_spec.py b/backend/api_v2/tests/test_docstudio_spec.py index ca81c7f36a..44d3a61e94 100644 --- a/backend/api_v2/tests/test_docstudio_spec.py +++ b/backend/api_v2/tests/test_docstudio_spec.py @@ -95,6 +95,30 @@ def guessing_generator(self, request=None, public=False) -> dict: render_spec() +def test_a_path_outside_the_published_mounts_fails_generation(monkeypatch) -> None: + """The gate the diff widened, exercised on the branch it protects. + + Widening it from one prefix to two is exactly the edit that could admit + everything; the accept direction is covered incidentally by every other + test here, and only this one covers the refusal. + """ + + def off_prefix_generator(self, request=None, public=False) -> dict: + # Otherwise-valid, so the OpenAPI-validity gate below cannot be what + # raises: matching on the path alone passed even with the prefix gate + # removed, because the validity error quotes the instance back. + return { + "openapi": "3.0.3", + "info": {"title": "t", "version": "v1"}, + "paths": {"/private/api/{org_name}/": {}}, + } + + monkeypatch.setattr(SchemaGenerator, "get_schema", off_prefix_generator) + + with pytest.raises(SpecGenerationFailed, match="outside the published mounts"): + render_spec() + + def test_spec_paths_are_the_urls_the_server_serves() -> None: """Resolves the real mount rather than restating it: a spec generated for URLs the server does not serve is the failure this file exists to catch. @@ -289,14 +313,43 @@ def test_the_documented_permission_tiers_are_the_ones_the_model_defines() -> Non assert _schema("ApiKeyPermission")["enum"] == list(ApiKeyPermission.values) +def test_the_identity_reads_errors_are_the_shape_the_middleware_sends() -> None: + """`whoami` authenticates in middleware, which answers with a bare + `message` and never reaches the project exception handler -- so it must not + publish the handler's `{type, errors[]}` shape the way the deployment + operations legitimately do. + + Paired with `test_a_rejection_carries_the_body_the_spec_publishes` in + `platform_api`, which pins the same claim against the wire. + """ + spec = _committed() + for path, _, operation in _operations(spec): + if operation["operationId"] != "whoami": + continue + for code in ("401", "403"): + ref = operation["responses"][code]["content"]["application/json"]["schema"][ + "$ref" + ] + assert ref.endswith("/PlatformKeyError"), f"{code} on {path}: {ref}" + assert set(_schema("PlatformKeyError")["properties"]) == {"message"} + + def test_the_identity_read_asks_for_no_organisation() -> None: """Resolving the organisation from the key is the whole point: a path parameter here would mean the caller had to know the answer first. """ - for path, _, operation in _operations(_committed()): - if operation["operationId"] == "whoami": - assert "{" not in path, path - assert not operation.get("parameters"), path + reads = [ + (path, operation) + for path, _, operation in _operations(_committed()) + if operation["operationId"] == "whoami" + ] + + # Guarded like its sibling at `test_the_one_shot_read_...`: an unguarded + # loop passes by finding nothing the day the operation is renamed. + assert reads + for path, operation in reads: + assert "{" not in path, path + assert not operation.get("parameters"), path @pytest.mark.parametrize( diff --git a/backend/backend/base_urls.py b/backend/backend/base_urls.py index b6be18d87f..0b763e3c59 100644 --- a/backend/backend/base_urls.py +++ b/backend/backend/base_urls.py @@ -10,9 +10,16 @@ # Combine the URL patterns urlpatterns = [ # Organisation-less, and so mounted ahead of the tenant urlconf rather than - # relying on falling through it. `OrganizationMiddleware` rewrites every - # organisation-scoped path before routing, and none of those rewrites can - # produce `whoami/`, so this shadows nothing. + # relying on falling through it. Nothing is shadowed today because no tenant + # urlconf declares `whoami/`; mounting first is what makes this route win if + # one ever does. + # + # Note this is not the only URL that reaches the view. `OrganizationMiddleware` + # strips the organisation segment before routing, so + # `/api/v1/unstract//whoami/` is rewritten to exactly this path and + # resolves here too -- with `organization_id` set, so the key-belongs-to-org + # check in `CustomAuthMiddleware` additionally applies. That alias is + # stricter, not looser; `test_whoami.py` covers both forms. path( f"{settings.TENANT_SUBFOLDER_PREFIX}/", include("platform_api.whoami_urls"), diff --git a/backend/backend/settings/base.py b/backend/backend/settings/base.py index 0d4c979593..7079de7ae4 100644 --- a/backend/backend/settings/base.py +++ b/backend/backend/settings/base.py @@ -732,8 +732,10 @@ def filter(self, record): # `whoami` resolves the organisation from the API key itself, so it carries no # organisation segment -- without this the middleware would read `whoami` as one. # Note this is not WHITELISTED_PATHS: the endpoint still authenticates. +# Anchored: `re.match` is a prefix test, so without the `$` an organisation +# literally named `whoami` would have its entire API treated as organisation-less. ORGANIZATION_MIDDLEWARE_WHITELISTED_PATHS = [ - rf"^/{PATH_PREFIX}/unstract/whoami/", + rf"^/{PATH_PREFIX}/unstract/whoami/$", ] # Social Auth Settings diff --git a/backend/platform_api/openapi_schema.py b/backend/platform_api/openapi_schema.py index 9dd627ac3a..0eeea2ffe3 100644 --- a/backend/platform_api/openapi_schema.py +++ b/backend/platform_api/openapi_schema.py @@ -1,17 +1,19 @@ """OpenAPI annotation for the ``whoami`` endpoint. -The serializer here shapes the published spec only; it never parses a request -or builds a response. It lives outside ``serializers.py`` so that nothing at -request time imports it by accident, matching ``api_v2.openapi_schema``. +The serializers here shape the published spec only; they never parse a request +or build a response. They live outside ``serializers.py`` so that nothing at +request time imports one by accident, matching ``api_v2.openapi_schema``. -Its docstring and help texts are published as the client-facing descriptions, -so they are written for the caller rather than the maintainer. +Their docstrings and help texts are published as the client-facing +descriptions, so they are written for the caller rather than the maintainer. -The error body is imported rather than restated: it comes from the project-wide -exception handler, so it is the same shape for every endpoint in the spec. +This operation does **not** reuse ``api_v2.openapi_schema.ErrorResponse``. That +shape comes from the project-wide exception handler, and this route never +reaches it: ``whoami`` is deliberately absent from ``WHITELISTED_PATHS``, so +``CustomAuthMiddleware`` authenticates it and answers every rejection itself, +with a bare ``{"message": ...}`` body, before DRF is entered. """ -from api_v2.openapi_schema import ErrorResponse from drf_spectacular.utils import ( OpenApiResponse, extend_schema, @@ -42,6 +44,17 @@ class WhoAmIResponse(serializers.Serializer): key_name = serializers.CharField(help_text="The key's name, as it was minted.") +class PlatformKeyError(serializers.Serializer): + """Why a platform-key request was refused. + + Produced by the authentication middleware rather than by the project's + exception handler, so it carries a single human-readable message and none + of the per-field structure the organisation-scoped endpoints return. + """ + + message = serializers.CharField(help_text="Human-readable reason for the refusal.") + + WHOAMI_DESCRIPTION = ( "Resolve the organisation a platform API key belongs to.\n\n" "The organisation is read from the key itself, so this route carries no " @@ -49,7 +62,11 @@ class WhoAmIResponse(serializers.Serializer): "`organization_id`; every other endpoint takes it as a path segment.\n\n" "Only a platform API key is accepted. An API deployment key authenticates " "against a different table on a path that never reaches this endpoint, and " - "is rejected as unauthenticated." + "is rejected as unauthenticated.\n\n" + "The same route also answers under an organisation segment " + "(`/api/v1/unstract/{org}/whoami/`), where the key must additionally belong " + "to the organisation named. Prefer the form documented here: it is the one " + "that needs no organisation to begin with." ) @@ -63,16 +80,19 @@ class WhoAmIResponse(serializers.Serializer): responses={ 200: WhoAmIResponse, 401: OpenApiResponse( - ErrorResponse, - description="No usable platform API key was supplied.", + PlatformKeyError, + description="No usable platform API key was supplied — absent, " + "malformed, unknown, or revoked.", ), 403: OpenApiResponse( - ErrorResponse, - description="The key is not permitted to issue this request.", + PlatformKeyError, + description="The key was recognised but refused: its permission " + "tier is not one this deployment knows, or the request named an " + "organisation the key does not belong to.", ), 500: OpenApiResponse( - ErrorResponse, - description="The organisation could not be resolved.", + description="The request could not be served. The body is not " + "guaranteed to be JSON.", ), }, description=WHOAMI_DESCRIPTION, diff --git a/backend/platform_api/tests/test_whoami.py b/backend/platform_api/tests/test_whoami.py index a76957c8b4..09d9e22af8 100644 --- a/backend/platform_api/tests/test_whoami.py +++ b/backend/platform_api/tests/test_whoami.py @@ -17,7 +17,8 @@ from django.test import override_settings from django.urls import Resolver404, resolve from platform_api.models import ApiKeyPermission, PlatformApiKey -from rest_framework.test import APITestCase +from platform_api.whoami_views import WhoAmIView +from rest_framework.test import APIRequestFactory, APITestCase ORG_A = "org-a" ORG_B = "org-b" @@ -73,8 +74,11 @@ def _get(self, token: str | None = None, url: str = WHOAMI_URL): # --- routing ----------------------------------------------------------- def test_the_route_is_reachable(self) -> None: - """A 404 from the organisation regex eating `whoami` would otherwise - look identical to a rejected request in every test below. + """The mount exists. Nothing more: `resolve()` runs no middleware, so + this cannot see the organisation regex at all -- removing the whitelist + entry leaves this test green. The middleware regression is covered by + `test_the_organisation_middleware_still_defines_organisation_id`, and + the failure it produces is a 403, not a 404. """ try: resolve(WHOAMI_URL) @@ -82,14 +86,28 @@ def test_the_route_is_reachable(self) -> None: self.fail(f"{WHOAMI_URL} resolves to nothing; the route is not mounted") def test_the_endpoint_is_not_whitelisted(self) -> None: - """WHITELISTED_PATHS skips authentication entirely. This endpoint has - nothing to say without a key, so landing there would make it answer - with someone else's organisation or none at all. + """WHITELISTED_PATHS skips authentication entirely, so `platform_api_key` + would never be bound and every caller would get the view's own 401. The + endpoint would stop working rather than leak -- but it would stop + working silently, which this pins. """ assert not any( WHOAMI_URL.startswith(path) for path in settings.WHITELISTED_PATHS ), f"{WHOAMI_URL} is whitelisted — it would bypass authentication entirely" + def test_the_whitelist_does_not_swallow_paths_beneath_it(self) -> None: + """`re.match` is a prefix test. Unanchored, an organisation literally + named `whoami` would have every one of its paths treated as + organisation-less. + """ + import re + + beneath = f"/{settings.PATH_PREFIX}/unstract/whoami/workflow/" + assert not any( + re.match(pattern, beneath) + for pattern in settings.ORGANIZATION_MIDDLEWARE_WHITELISTED_PATHS + ), f"{beneath} is treated as organisation-less" + def test_the_organisation_middleware_still_defines_organisation_id(self) -> None: """The whitelist branch returns early. Downstream middleware reads the attribute directly, so leaving it unset is a 500 rather than a skip. @@ -145,3 +163,62 @@ def test_an_unknown_key_is_rejected(self) -> None: def test_an_inactive_key_is_rejected(self) -> None: key = self._make_key(is_active=False) self.assertEqual(self._get(str(key.key)).status_code, 401) + + def test_a_rejection_carries_the_body_the_spec_publishes(self) -> None: + """The status alone was asserted everywhere above, and the status alone + is what let the spec claim a body shape this route never sends. + + These rejections come from the middleware, not from DRF's exception + handler, so the body is a bare `message` -- not the `{type, errors[]}` + the organisation-scoped endpoints return. + """ + response = self._get(str(uuid.uuid4())) + + self.assertEqual(response.status_code, 401) + self.assertEqual(list(response.json()), ["message"]) + + def test_the_view_answers_401_when_no_key_reached_it(self) -> None: + """The one rejection the view itself owns, and the only one the + middleware cannot answer first: a session-authenticated caller with no + platform key. + + Called directly, because every request that goes through the middleware + is rejected before the view runs -- which is why this branch was + uncovered while returning the wrong status. `raise NotAuthenticated` + answers 403 here: DRF coerces it unless the first authenticator offers + a WWW-Authenticate header, and SessionAuthentication offers none. + """ + request = APIRequestFactory().get(WHOAMI_URL) + response = WhoAmIView.as_view()(request) + + self.assertEqual(response.status_code, 401) + self.assertEqual(list(response.data), ["message"]) + + # --- the organisation-scoped alias ------------------------------------- + + def test_the_alias_under_an_organisation_segment_also_answers(self) -> None: + """`OrganizationMiddleware` strips the segment, so `/whoami/` is + rewritten onto this same view. Untested, this behaviour would move the + next time either the mount order or the org-match branch changed. + """ + key = self._make_key(organization=self.org_a) + + response = self._get( + str(key.key), url=f"/{settings.PATH_PREFIX}/unstract/{ORG_A}/whoami/" + ) + + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json()["organization_id"], ORG_A) + + def test_the_alias_rejects_a_key_from_another_organisation(self) -> None: + """The alias is stricter than the documented route, not looser: naming + an organisation re-arms the key-belongs-to-org check that the org-less + form has nothing to check against. + """ + key = self._make_key(organization=self.org_b) + + response = self._get( + str(key.key), url=f"/{settings.PATH_PREFIX}/unstract/{ORG_A}/whoami/" + ) + + self.assertEqual(response.status_code, 403) diff --git a/backend/platform_api/whoami_views.py b/backend/platform_api/whoami_views.py index 8452233f17..16fee1f635 100644 --- a/backend/platform_api/whoami_views.py +++ b/backend/platform_api/whoami_views.py @@ -5,14 +5,17 @@ web-app URL. This endpoint answers "which organisation does this key belong to?", so a client can resolve it once and store it. -The organisation is read off the key row, never off the URL: ``PlatformApiKey`` -carries an ``organization`` FK stamped at mint time and a globally unique -``key``, so a bearer token maps to exactly one organisation by construction. -That is why this route carries no organisation segment. +The organisation is read off the key row, never off the URL: ``PlatformApiKey.key`` +is unique, so a bearer token selects at most one row, and that row names its own +organisation. That is why this route carries no organisation segment. + +``mcp_server.tools.platform.whoami`` answers the same question over MCP for the +same key. It predates this endpoint and spells the tier ``permission_tier``; +this one uses ``permission``, matching the model field. Adding a field to either +does not add it to the other. """ from rest_framework import status, views -from rest_framework.exceptions import NotAuthenticated from rest_framework.request import Request from rest_framework.response import Response @@ -29,17 +32,28 @@ class WhoAmIView(views.APIView): # class here would only re-ask a question already answered. permission_classes: list = [] - # authentication_classes is deliberately not set. The project configures no - # DEFAULT_AUTHENTICATION_CLASSES that resolves a user, so DRF's default - # returns None and the middleware's request.user survives into the view. - + # authentication_classes is deliberately left alone. The project sets no + # DEFAULT_AUTHENTICATION_CLASSES, so DRF's own default applies -- + # SessionAuthentication first -- and that is what carries the user + # CustomAuthMiddleware bound into the view. Emptying this list would not + # simplify anything and would drop that. def get(self, request: Request) -> Response: key = getattr(request, "platform_api_key", None) if key is None: # A session-authenticated browser user reaches this with no key to # describe. There is nothing to report, and reporting the session's # organisation instead would answer a question nobody asked. - raise NotAuthenticated("This endpoint requires a platform API key.") + # + # Returned rather than raised: DRF coerces NotAuthenticated to 403 + # unless the first authenticator offers a WWW-Authenticate header, + # and SessionAuthentication offers none -- so a raise here would + # answer 403 to a request whose problem is a missing credential. + # The body matches what CustomAuthMiddleware sends for the same + # class of failure, so one shape covers every rejection. + return Response( + {"message": "This endpoint requires a platform API key."}, + status=status.HTTP_401_UNAUTHORIZED, + ) organization = key.organization return Response( diff --git a/specs/docstudio-oss.json b/specs/docstudio-oss.json index 66498cfa1c..bfe3ae5b20 100644 --- a/specs/docstudio-oss.json +++ b/specs/docstudio-oss.json @@ -218,6 +218,19 @@ ], "type": "object" }, + "PlatformKeyError": { + "description": "Why a platform-key request was refused.\n\nProduced by the authentication middleware rather than by the project's\nexception handler, so it carries a single human-readable message and none\nof the per-field structure the organisation-scoped endpoints return.", + "properties": { + "message": { + "description": "Human-readable reason for the refusal.", + "type": "string" + } + }, + "required": [ + "message" + ], + "type": "object" + }, "StatusResponse": { "properties": { "message": { @@ -290,7 +303,7 @@ "paths": { "/api/v1/unstract/whoami/": { "get": { - "description": "Resolve the organisation a platform API key belongs to.\n\nThe organisation is read from the key itself, so this route carries no organisation segment and needs nothing but the key. Call it once and store `organization_id`; every other endpoint takes it as a path segment.\n\nOnly a platform API key is accepted. An API deployment key authenticates against a different table on a path that never reaches this endpoint, and is rejected as unauthenticated.", + "description": "Resolve the organisation a platform API key belongs to.\n\nThe organisation is read from the key itself, so this route carries no organisation segment and needs nothing but the key. Call it once and store `organization_id`; every other endpoint takes it as a path segment.\n\nOnly a platform API key is accepted. An API deployment key authenticates against a different table on a path that never reaches this endpoint, and is rejected as unauthenticated.\n\nThe same route also answers under an organisation segment (`/api/v1/unstract/{org}/whoami/`), where the key must additionally belong to the organisation named. Prefer the form documented here: it is the one that needs no organisation to begin with.", "operationId": "whoami", "responses": { "200": { @@ -333,11 +346,11 @@ } }, "schema": { - "$ref": "#/components/schemas/ErrorResponse" + "$ref": "#/components/schemas/PlatformKeyError" } } }, - "description": "No usable platform API key was supplied." + "description": "No usable platform API key was supplied \u2014 absent, malformed, unknown, or revoked." }, "403": { "content": { @@ -357,35 +370,14 @@ } }, "schema": { - "$ref": "#/components/schemas/ErrorResponse" + "$ref": "#/components/schemas/PlatformKeyError" } } }, - "description": "The key is not permitted to issue this request." + "description": "The key was recognised but refused: its permission tier is not one this deployment knows, or the request named an organisation the key does not belong to." }, "500": { - "content": { - "application/json": { - "examples": { - "APIException": { - "value": { - "errors": [ - { - "attr": null, - "code": "error", - "detail": "A server error occurred." - } - ], - "type": "server_error" - } - } - }, - "schema": { - "$ref": "#/components/schemas/ErrorResponse" - } - } - }, - "description": "The organisation could not be resolved." + "description": "The request could not be served. The body is not guaranteed to be JSON." } }, "security": [ diff --git a/tests/critical_paths.yaml b/tests/critical_paths.yaml index 94aa29f006..4aec281293 100644 --- a/tests/critical_paths.yaml +++ b/tests/critical_paths.yaml @@ -76,6 +76,11 @@ paths: covered_by: [integration-backend] proof: marker + - id: platform-key-whoami + description: "A platform API key resolves its own organisation over the org-less whoami endpoint; the org comes from the key row, not the URL." + covered_by: [integration-backend] + proof: marker + - id: prompt-studio-author description: "Create a Prompt Studio project and add a prompt to it." covered_by: [integration-backend] From 7eb41bc1ed48091f292fb0ec03dc2d7df0c4edc5 Mon Sep 17 00:00:00 2001 From: pk-zipstack Date: Tue, 1 Sep 2026 18:18:28 +0530 Subject: [PATCH 3/6] UN-4016 [FIX] Stop publishing an error example the endpoint contradicts Iteration 2. The adversarial verifier re-found F1: iteration 1 changed the 401/403 `$ref` to `PlatformKeyError` but left the `examples` block beside it showing the handler's `{type, errors[]}` body, so the artifact contradicted itself in a single media-type object -- schema requiring `message`, example showing `errors[0].code`. A client author reads the example, not the `$ref`, so the defect F1 was filed against survived its own fix. Cause: `drf_standardized_errors` appends an example of the handler body to every 4xx/5xx keyed on the status code alone, never consulting the declared serializer (`openapi.py:343-356`), so overriding `responses` was never going to be enough. `PlatformKeyAutoSchema` suppresses that injection for this view only; the deployment operations keep their examples, which are correct there because those really do return the handler body. Also fixed, all four raised by the same verifier against iteration 1's own work: the module docstring claimed the middleware answers *every* rejection, which the 401 iteration 1 added to the view had just made false; the new spec test lacked the `assert reads` guard that the same commit added twelve lines below it, so it passed vacuously on an operationId rename; the alias test's docstring claimed mount-order coverage a mutation disproved (swapping the mount leaves every test green); and the 403 body was declared but asserted nowhere, which is the exact "status asserted, body unasserted" gap that let the original spec lie. `test_no_published_example_contradicts_its_own_schema` now pins the property structurally for every operation. It found three pre-existing instances in the merged deployment spec -- `status` 406/500 and `execute` 500 all declare a non-error body beside a handler-shaped example. Those are recorded in `_KNOWN_EXAMPLE_DIVERGENCES` rather than fixed here: they predate this branch (verified against c49968e3e) and belong to whoever owns that spec. The check fails on any new instance. Findings: F1(re-fixed) N2 N3 N4 N5 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM --- backend/api_v2/tests/test_docstudio_spec.py | 51 +++++++++++++++++++-- backend/platform_api/openapi_schema.py | 27 +++++++++-- backend/platform_api/tests/test_whoami.py | 8 +++- backend/platform_api/whoami_views.py | 6 ++- specs/docstudio-oss.json | 40 ---------------- 5 files changed, 83 insertions(+), 49 deletions(-) diff --git a/backend/api_v2/tests/test_docstudio_spec.py b/backend/api_v2/tests/test_docstudio_spec.py index 44d3a61e94..53a8df1620 100644 --- a/backend/api_v2/tests/test_docstudio_spec.py +++ b/backend/api_v2/tests/test_docstudio_spec.py @@ -42,6 +42,17 @@ #: up from the extraction metadata when the request asks for it. _PROMOTED_FILE_RESULT_FIELDS = {"extracted_text"} +#: Responses whose published example already contradicted its schema before +#: this check existed. All three are the deployment operations declaring a +#: non-error body for a status the standardized-errors schema class also +#: injects a handler-shaped example for. Recorded rather than silently skipped: +#: the check below fails on any *new* instance, and this list is the debt. +_KNOWN_EXAMPLE_DIVERGENCES = { + ("status", "406"), + ("status", "500"), + ("execute", "500"), +} + #: The operations served by an API deployment, as opposed to the platform-key #: operations that describe the account. They authenticate differently and can #: fail differently, so several checks below split on this. @@ -323,9 +334,17 @@ def test_the_identity_reads_errors_are_the_shape_the_middleware_sends() -> None: `platform_api`, which pins the same claim against the wire. """ spec = _committed() - for path, _, operation in _operations(spec): - if operation["operationId"] != "whoami": - continue + reads = [ + (path, operation) + for path, _, operation in _operations(spec) + if operation["operationId"] == "whoami" + ] + + # Guarded like its sibling below: without this the whole check is skipped + # the day the operation id moves, which is the same vacuity this commit + # fixed twelve lines down and reintroduced here. + assert reads + for path, operation in reads: for code in ("401", "403"): ref = operation["responses"][code]["content"]["application/json"]["schema"][ "$ref" @@ -334,6 +353,32 @@ def test_the_identity_reads_errors_are_the_shape_the_middleware_sends() -> None: assert set(_schema("PlatformKeyError")["properties"]) == {"message"} +def test_no_published_example_contradicts_its_own_schema() -> None: + """The standardized-errors schema class appends an example of the exception + handler's body to every 4xx/5xx, keyed on the status code alone -- so an + operation that overrides the schema keeps examples describing the shape it + replaced, and the artifact contradicts itself in one media-type object. + + Checked structurally rather than by name: any response declaring a body + other than `ErrorResponse` must carry no handler-shaped example. + """ + for path, method, operation in _operations(_committed()): + if (operation["operationId"], "") in _KNOWN_EXAMPLE_DIVERGENCES: + continue + for code, response in operation["responses"].items(): + media = response.get("content", {}).get("application/json", {}) + ref = media.get("schema", {}).get("$ref", "") + if ref.endswith("/ErrorResponse"): + continue + for name, example in media.get("examples", {}).items(): + if (operation["operationId"], code) in _KNOWN_EXAMPLE_DIVERGENCES: + continue + assert "errors" not in example.get("value", {}), ( + f"{method} {path} {code}: example {name!r} shows the handler " + f"body, but the response declares {ref.split('/')[-1]!r}" + ) + + def test_the_identity_read_asks_for_no_organisation() -> None: """Resolving the organisation from the key is the whole point: a path parameter here would mean the caller had to know the answer first. diff --git a/backend/platform_api/openapi_schema.py b/backend/platform_api/openapi_schema.py index 0eeea2ffe3..39eddcf9b2 100644 --- a/backend/platform_api/openapi_schema.py +++ b/backend/platform_api/openapi_schema.py @@ -8,10 +8,12 @@ descriptions, so they are written for the caller rather than the maintainer. This operation does **not** reuse ``api_v2.openapi_schema.ErrorResponse``. That -shape comes from the project-wide exception handler, and this route never -reaches it: ``whoami`` is deliberately absent from ``WHITELISTED_PATHS``, so -``CustomAuthMiddleware`` authenticates it and answers every rejection itself, -with a bare ``{"message": ...}`` body, before DRF is entered. +shape comes from the project-wide exception handler, and nothing on this route +produces it. ``whoami`` is deliberately absent from ``WHITELISTED_PATHS``, so +``CustomAuthMiddleware`` answers almost every rejection itself, before DRF is +entered, with a bare ``{"message": ...}`` body. The one it does not reach --- a +caller who is authenticated but carries no platform key --- is answered by the +view in that same shape, deliberately, so one declaration covers both. """ from drf_spectacular.utils import ( @@ -19,11 +21,28 @@ extend_schema, extend_schema_view, ) +from drf_standardized_errors.openapi import AutoSchema as StandardizedErrorsAutoSchema from rest_framework import serializers from platform_api.models import ApiKeyPermission +class PlatformKeyAutoSchema(StandardizedErrorsAutoSchema): + """The project schema class, minus its error-body examples. + + ``drf_standardized_errors`` appends an example of the exception handler's + ``{type, errors[]}`` body to every 4xx/5xx response, keyed on the status + code alone and never on the declared serializer + (``drf_standardized_errors/openapi.py:343-356``). On this operation the + handler is never reached, so those examples contradict the ``$ref`` beside + them -- a reader following the example writes ``errors[0].code`` and gets a + ``KeyError`` on the wire. + """ + + def _get_error_response_examples(self) -> list: + return [] + + class WhoAmIResponse(serializers.Serializer): """The organisation a platform API key belongs to, and what it may do.""" diff --git a/backend/platform_api/tests/test_whoami.py b/backend/platform_api/tests/test_whoami.py index 09d9e22af8..0ab16b8e0f 100644 --- a/backend/platform_api/tests/test_whoami.py +++ b/backend/platform_api/tests/test_whoami.py @@ -199,7 +199,10 @@ def test_the_view_answers_401_when_no_key_reached_it(self) -> None: def test_the_alias_under_an_organisation_segment_also_answers(self) -> None: """`OrganizationMiddleware` strips the segment, so `/whoami/` is rewritten onto this same view. Untested, this behaviour would move the - next time either the mount order or the org-match branch changed. + next time the org-match branch changed. + + Not mount order: swapping this mount below the tenant urlconf leaves + every test here green, because no tenant urlconf declares `whoami/`. """ key = self._make_key(organization=self.org_a) @@ -222,3 +225,6 @@ def test_the_alias_rejects_a_key_from_another_organisation(self) -> None: ) self.assertEqual(response.status_code, 403) + # The 403 body is declared too, and "status asserted, body unasserted" + # is exactly what let the original spec publish a shape nothing sends. + self.assertEqual(list(response.json()), ["message"]) diff --git a/backend/platform_api/whoami_views.py b/backend/platform_api/whoami_views.py index 16fee1f635..c9bca7ac06 100644 --- a/backend/platform_api/whoami_views.py +++ b/backend/platform_api/whoami_views.py @@ -19,7 +19,7 @@ from rest_framework.request import Request from rest_framework.response import Response -from platform_api.openapi_schema import WHOAMI_SCHEMA +from platform_api.openapi_schema import WHOAMI_SCHEMA, PlatformKeyAutoSchema @WHOAMI_SCHEMA @@ -32,6 +32,10 @@ class WhoAmIView(views.APIView): # class here would only re-ask a question already answered. permission_classes: list = [] + # Suppresses the standardized-errors example bodies, which this route never + # sends. See PlatformKeyAutoSchema. + schema = PlatformKeyAutoSchema() + # authentication_classes is deliberately left alone. The project sets no # DEFAULT_AUTHENTICATION_CLASSES, so DRF's own default applies -- # SessionAuthentication first -- and that is what carries the user diff --git a/specs/docstudio-oss.json b/specs/docstudio-oss.json index bfe3ae5b20..63097a2483 100644 --- a/specs/docstudio-oss.json +++ b/specs/docstudio-oss.json @@ -319,32 +319,6 @@ "401": { "content": { "application/json": { - "examples": { - "AuthenticationFailed": { - "value": { - "errors": [ - { - "attr": null, - "code": "authentication_failed", - "detail": "Incorrect authentication credentials." - } - ], - "type": "client_error" - } - }, - "NotAuthenticated": { - "value": { - "errors": [ - { - "attr": null, - "code": "not_authenticated", - "detail": "Authentication credentials were not provided." - } - ], - "type": "client_error" - } - } - }, "schema": { "$ref": "#/components/schemas/PlatformKeyError" } @@ -355,20 +329,6 @@ "403": { "content": { "application/json": { - "examples": { - "PermissionDenied": { - "value": { - "errors": [ - { - "attr": null, - "code": "permission_denied", - "detail": "You do not have permission to perform this action." - } - ], - "type": "client_error" - } - } - }, "schema": { "$ref": "#/components/schemas/PlatformKeyError" } From 32ec14279285a73bf7eff2acf3c03d31bda3a28c Mon Sep 17 00:00:00 2001 From: pk-zipstack Date: Tue, 1 Sep 2026 18:36:00 +0530 Subject: [PATCH 4/6] UN-4016 [FIX] Declare the 405 this endpoint really returns, and stop claiming it cannot Iteration 3. The verifier re-found N2 with a counter-example rounds 1 and 2 both missed: `POST /api/v1/unstract/whoami/` with a `read_write` key -- the model default -- is passed by the middleware, refused by DRF, and comes back 405 in the exception handler's `{type, errors[]}`. Four comments across three files asserted that shape was unreachable here, and that claim was the stated justification for both not reusing `ErrorResponse` and suppressing every error example on the view. So the endpoint has always returned two error shapes and the spec described one. 405 is now declared with `ErrorResponse`, and `PlatformKeyAutoSchema` narrows its suppression to the statuses the middleware actually answers (401, 403) rather than blanket-stripping, so the 405 keeps the handler example that is correct for it. Verified on the wire: a `read_write` key POSTing gets 405 `{type, errors[]}`; a `read` key gets 403 `{message}` from the middleware instead. Both are now pinned, against each other, so neither path can look like the only one. The absolute clauses are deleted rather than rewritten a third time. Two rounds of rewriting them produced two more false statements; what remains says only what is enforced. Also from the same verifier: `_KNOWN_EXAMPLE_DIVERGENCES` was keyed on `(operationId, code)`, which exempted those coordinates forever -- a new divergent example injected at `status` 406 passed. Now keyed on the example name too, so the three recorded rows stay exempt and a fourth fails; the comment claiming the check catches new instances is true as written for the first time. Dropped a dead exemption branch that could never match, and a comment misattributing which commit added a guard and how far below it sits. Findings: N2(re-fixed) + 4 new from verifier round 2 Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM --- backend/api_v2/tests/test_docstudio_spec.py | 19 +++---- backend/platform_api/openapi_schema.py | 56 +++++++++++++++------ backend/platform_api/tests/test_whoami.py | 27 ++++++++++ backend/platform_api/whoami_views.py | 4 +- specs/docstudio-oss.json | 26 +++++++++- 5 files changed, 105 insertions(+), 27 deletions(-) diff --git a/backend/api_v2/tests/test_docstudio_spec.py b/backend/api_v2/tests/test_docstudio_spec.py index 53a8df1620..beb0c37bad 100644 --- a/backend/api_v2/tests/test_docstudio_spec.py +++ b/backend/api_v2/tests/test_docstudio_spec.py @@ -48,9 +48,9 @@ #: injects a handler-shaped example for. Recorded rather than silently skipped: #: the check below fails on any *new* instance, and this list is the debt. _KNOWN_EXAMPLE_DIVERGENCES = { - ("status", "406"), - ("status", "500"), - ("execute", "500"), + ("status", "406", "NotAcceptable"), + ("status", "500", "APIException"), + ("execute", "500", "APIException"), } #: The operations served by an API deployment, as opposed to the platform-key @@ -326,7 +326,7 @@ def test_the_documented_permission_tiers_are_the_ones_the_model_defines() -> Non def test_the_identity_reads_errors_are_the_shape_the_middleware_sends() -> None: """`whoami` authenticates in middleware, which answers with a bare - `message` and never reaches the project exception handler -- so it must not + `message` and does not reach the project exception handler for its credential failures -- so it must not publish the handler's `{type, errors[]}` shape the way the deployment operations legitimately do. @@ -341,8 +341,7 @@ def test_the_identity_reads_errors_are_the_shape_the_middleware_sends() -> None: ] # Guarded like its sibling below: without this the whole check is skipped - # the day the operation id moves, which is the same vacuity this commit - # fixed twelve lines down and reintroduced here. + # the day the operation id moves. assert reads for path, operation in reads: for code in ("401", "403"): @@ -363,15 +362,17 @@ def test_no_published_example_contradicts_its_own_schema() -> None: other than `ErrorResponse` must carry no handler-shaped example. """ for path, method, operation in _operations(_committed()): - if (operation["operationId"], "") in _KNOWN_EXAMPLE_DIVERGENCES: - continue for code, response in operation["responses"].items(): media = response.get("content", {}).get("application/json", {}) ref = media.get("schema", {}).get("$ref", "") if ref.endswith("/ErrorResponse"): continue for name, example in media.get("examples", {}).items(): - if (operation["operationId"], code) in _KNOWN_EXAMPLE_DIVERGENCES: + if ( + operation["operationId"], + code, + name, + ) in _KNOWN_EXAMPLE_DIVERGENCES: continue assert "errors" not in example.get("value", {}), ( f"{method} {path} {code}: example {name!r} shows the handler " diff --git a/backend/platform_api/openapi_schema.py b/backend/platform_api/openapi_schema.py index 39eddcf9b2..c8c2c75948 100644 --- a/backend/platform_api/openapi_schema.py +++ b/backend/platform_api/openapi_schema.py @@ -7,13 +7,13 @@ Their docstrings and help texts are published as the client-facing descriptions, so they are written for the caller rather than the maintainer. -This operation does **not** reuse ``api_v2.openapi_schema.ErrorResponse``. That -shape comes from the project-wide exception handler, and nothing on this route -produces it. ``whoami`` is deliberately absent from ``WHITELISTED_PATHS``, so -``CustomAuthMiddleware`` answers almost every rejection itself, before DRF is -entered, with a bare ``{"message": ...}`` body. The one it does not reach --- a -caller who is authenticated but carries no platform key --- is answered by the -view in that same shape, deliberately, so one declaration covers both. +This operation publishes two error shapes, because it really sends two. +``whoami`` is deliberately absent from ``WHITELISTED_PATHS``, so +``CustomAuthMiddleware`` authenticates it and answers the credential failures +itself, before DRF is entered, with a bare ``{"message": ...}`` --- that is +``PlatformKeyError``. Anything DRF itself raises after that point still goes +through the project exception handler and comes back as ``ErrorResponse``; a +method this view does not implement is the reachable case. """ from drf_spectacular.utils import ( @@ -22,25 +22,43 @@ extend_schema_view, ) from drf_standardized_errors.openapi import AutoSchema as StandardizedErrorsAutoSchema + +from api_v2.openapi_schema import ErrorResponse from rest_framework import serializers from platform_api.models import ApiKeyPermission +#: The statuses this operation answers from the authentication middleware, in +#: ``PlatformKeyError`` shape. Everything else it can return is DRF's own and +#: keeps the handler shape. +_MIDDLEWARE_ANSWERED_STATUSES = frozenset({"401", "403"}) + + class PlatformKeyAutoSchema(StandardizedErrorsAutoSchema): - """The project schema class, minus its error-body examples. + """The project schema class, with the injected error examples narrowed. ``drf_standardized_errors`` appends an example of the exception handler's ``{type, errors[]}`` body to every 4xx/5xx response, keyed on the status code alone and never on the declared serializer - (``drf_standardized_errors/openapi.py:343-356``). On this operation the - handler is never reached, so those examples contradict the ``$ref`` beside - them -- a reader following the example writes ``errors[0].code`` and gets a - ``KeyError`` on the wire. + (``drf_standardized_errors/openapi.py:343-356``). Where this operation + declares ``PlatformKeyError`` that example contradicts the ``$ref`` beside + it, and a reader following it writes ``errors[0].code`` and gets a + ``KeyError`` on the wire. Where the operation really does return the + handler body -- a 405, say -- the example is correct and is kept. """ - def _get_error_response_examples(self) -> list: - return [] + def _get_examples( + self, serializer, direction, media_type, status_code=None, extras=None + ): + if direction == "response" and str(status_code) in _MIDDLEWARE_ANSWERED_STATUSES: + # Skip the standardized-errors override, not the whole chain. + return super(StandardizedErrorsAutoSchema, self)._get_examples( + serializer, direction, media_type, status_code, extras + ) + return super()._get_examples( + serializer, direction, media_type, status_code, extras + ) class WhoAmIResponse(serializers.Serializer): @@ -68,7 +86,9 @@ class PlatformKeyError(serializers.Serializer): Produced by the authentication middleware rather than by the project's exception handler, so it carries a single human-readable message and none - of the per-field structure the organisation-scoped endpoints return. + of the per-field structure the organisation-scoped endpoints return. It is + the shape of this operation's credential failures specifically, not of + every failure it can return. """ message = serializers.CharField(help_text="Human-readable reason for the refusal.") @@ -109,6 +129,12 @@ class PlatformKeyError(serializers.Serializer): "tier is not one this deployment knows, or the request named an " "organisation the key does not belong to.", ), + 405: OpenApiResponse( + ErrorResponse, + description="This route serves GET only. A key whose tier " + "permits the method reaches the view and is refused here; a " + "tier that does not is refused earlier, as a 403.", + ), 500: OpenApiResponse( description="The request could not be served. The body is not " "guaranteed to be JSON.", diff --git a/backend/platform_api/tests/test_whoami.py b/backend/platform_api/tests/test_whoami.py index 0ab16b8e0f..61e809d844 100644 --- a/backend/platform_api/tests/test_whoami.py +++ b/backend/platform_api/tests/test_whoami.py @@ -164,6 +164,33 @@ def test_an_inactive_key_is_rejected(self) -> None: key = self._make_key(is_active=False) self.assertEqual(self._get(str(key.key)).status_code, 401) + def test_a_disallowed_method_is_the_handler_shape_the_spec_declares(self) -> None: + """The endpoint returns two error shapes, and this is the second one. + + A tier that permits POST gets past the middleware and is refused by + DRF, in `{type, errors[]}` -- not the `{message}` the credential + failures use. Undeclared and unasserted, that is the same spec-lies- + about-the-wire defect this suite already carries two other pins for. + """ + key = self._make_key(permission=ApiKeyPermission.READ_WRITE) + + response = self.client.post(WHOAMI_URL, HTTP_AUTHORIZATION=f"Bearer {key.key}") + + self.assertEqual(response.status_code, 405) + self.assertEqual(set(response.json()), {"type", "errors"}) + + def test_a_tier_that_forbids_the_method_is_refused_earlier(self) -> None: + """The 403 the middleware sends for the same request, so the two + rejection paths for one method are pinned against each other rather + than each looking like the only one. + """ + key = self._make_key(permission=ApiKeyPermission.READ) + + response = self.client.post(WHOAMI_URL, HTTP_AUTHORIZATION=f"Bearer {key.key}") + + self.assertEqual(response.status_code, 403) + self.assertEqual(list(response.json()), ["message"]) + def test_a_rejection_carries_the_body_the_spec_publishes(self) -> None: """The status alone was asserted everywhere above, and the status alone is what let the spec claim a body shape this route never sends. diff --git a/backend/platform_api/whoami_views.py b/backend/platform_api/whoami_views.py index c9bca7ac06..25846a567b 100644 --- a/backend/platform_api/whoami_views.py +++ b/backend/platform_api/whoami_views.py @@ -32,8 +32,8 @@ class WhoAmIView(views.APIView): # class here would only re-ask a question already answered. permission_classes: list = [] - # Suppresses the standardized-errors example bodies, which this route never - # sends. See PlatformKeyAutoSchema. + # Narrows the standardized-errors example bodies to the statuses that + # actually carry them. See PlatformKeyAutoSchema. schema = PlatformKeyAutoSchema() # authentication_classes is deliberately left alone. The project sets no diff --git a/specs/docstudio-oss.json b/specs/docstudio-oss.json index 63097a2483..c9776c8b2e 100644 --- a/specs/docstudio-oss.json +++ b/specs/docstudio-oss.json @@ -219,7 +219,7 @@ "type": "object" }, "PlatformKeyError": { - "description": "Why a platform-key request was refused.\n\nProduced by the authentication middleware rather than by the project's\nexception handler, so it carries a single human-readable message and none\nof the per-field structure the organisation-scoped endpoints return.", + "description": "Why a platform-key request was refused.\n\nProduced by the authentication middleware rather than by the project's\nexception handler, so it carries a single human-readable message and none\nof the per-field structure the organisation-scoped endpoints return. It is\nthe shape of this operation's credential failures specifically, not of\nevery failure it can return.", "properties": { "message": { "description": "Human-readable reason for the refusal.", @@ -336,6 +336,30 @@ }, "description": "The key was recognised but refused: its permission tier is not one this deployment knows, or the request named an organisation the key does not belong to." }, + "405": { + "content": { + "application/json": { + "examples": { + "MethodNotAllowed": { + "value": { + "errors": [ + { + "attr": null, + "code": "method_not_allowed", + "detail": "Method \"get\" not allowed." + } + ], + "type": "client_error" + } + } + }, + "schema": { + "$ref": "#/components/schemas/ErrorResponse" + } + } + }, + "description": "This route serves GET only. A key whose tier permits the method reaches the view and is refused here; a tier that does not is refused earlier, as a 403." + }, "500": { "description": "The request could not be served. The body is not guaranteed to be JSON." } From c670225040ca0e8fdce0b3def02d94e244788cc1 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 13:06:32 +0000 Subject: [PATCH 5/6] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- backend/platform_api/openapi_schema.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/backend/platform_api/openapi_schema.py b/backend/platform_api/openapi_schema.py index c8c2c75948..d2b2561ef8 100644 --- a/backend/platform_api/openapi_schema.py +++ b/backend/platform_api/openapi_schema.py @@ -16,19 +16,17 @@ method this view does not implement is the reachable case. """ +from api_v2.openapi_schema import ErrorResponse from drf_spectacular.utils import ( OpenApiResponse, extend_schema, extend_schema_view, ) from drf_standardized_errors.openapi import AutoSchema as StandardizedErrorsAutoSchema - -from api_v2.openapi_schema import ErrorResponse from rest_framework import serializers from platform_api.models import ApiKeyPermission - #: The statuses this operation answers from the authentication middleware, in #: ``PlatformKeyError`` shape. Everything else it can return is DRF's own and #: keeps the handler shape. From 245672ae95ae5722716cf79a3b8f1f8a00717b96 Mon Sep 17 00:00:00 2001 From: pk-zipstack Date: Tue, 1 Sep 2026 18:56:23 +0530 Subject: [PATCH 6/6] UN-4016 [FIX] Withdraw the 405 declaration; it named a status GET cannot return Iteration 4, and the last: N2 is escalated rather than attempted a fourth time. Round 3 declared 405 under the `get` operation. A verifier's 42-cell matrix (3 tiers x 7 methods x 2 routes) shows GET returns 200, 401 or 403 and never 405 -- so the declaration was unreachable at the coordinate it was written, and the 405s that do occur (POST/PUT/PATCH at read_write+, DELETE at full_access) have no operation in the spec to attach to. The kept example made it worse: it published `Method "get" not allowed.` under the GET operation, a hardcoded placeholder from drf_standardized_errors, not derived from the route. The declaration is withdrawn. The behaviour is real and is now stated in the operation description in prose, which is where a fact about methods the route does not serve can honestly live. Also corrected, all introduced by round 3 and all found by the same verifier: an I001 failure of the repo's PINNED ruff 0.3.4 hook, which round 3 introduced and the installed ruff 0.15 does not report -- verified clean at 7eb41bc1e and failing at 32ec14279, then confirmed the fix satisfies both versions; a comment asserting everything else "keeps the handler shape", contradicted by the 500 declared eight lines below it; a true clause round 3 deleted while rewriting the sentence around it, restored verbatim; and a test whose name claimed a relationship to the spec that nothing in it tested -- renamed to what it actually asserts, with the gap it does not cover named in its docstring. WHAT IS ESCALATED, and why this loop stops here: N2 has survived three fix attempts. Each attempt was a correct reading of the previous failure and each introduced a new defect: round 2 rewrote a false claim into a stronger false one, round 3 fixed the claim and declared the status in the wrong place. Three consecutive rounds introducing new defects is the convergence tripwire, and the discriminator applies -- the fixes keep needing an exception to a shared rule, which means the rule is wrong, not the wording. The rule that is wrong: this path serves GET, but answers non-GET methods with a status and a body shape that no `get` operation can describe. OpenAPI attaches responses to operations, not paths. Options, none of which a remediation loop should pick unilaterally: declare stub operations for the methods purely to carry a 405; accept prose (what this commit does); or change the view so every method the tier permits is answered in one shape. Findings: NEW-1..NEW-5 from verifier round 3 fixed; N2 ESCALATE (attempts=3) Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01K92qFRNecJ5kNkKcmJ8qkM --- backend/platform_api/openapi_schema.py | 27 +++++++++++------------ backend/platform_api/tests/test_whoami.py | 11 ++++++--- specs/docstudio-oss.json | 26 +--------------------- 3 files changed, 22 insertions(+), 42 deletions(-) diff --git a/backend/platform_api/openapi_schema.py b/backend/platform_api/openapi_schema.py index d2b2561ef8..86768e2406 100644 --- a/backend/platform_api/openapi_schema.py +++ b/backend/platform_api/openapi_schema.py @@ -9,14 +9,15 @@ This operation publishes two error shapes, because it really sends two. ``whoami`` is deliberately absent from ``WHITELISTED_PATHS``, so -``CustomAuthMiddleware`` authenticates it and answers the credential failures -itself, before DRF is entered, with a bare ``{"message": ...}`` --- that is -``PlatformKeyError``. Anything DRF itself raises after that point still goes +``CustomAuthMiddleware`` authenticates it and answers almost every credential +failure itself, before DRF is entered, with a bare ``{"message": ...}`` --- that +is ``PlatformKeyError``. The one it does not reach --- a caller who is +authenticated but carries no platform key --- is answered by the view in that +same shape, deliberately, so one declaration covers both. Anything DRF itself raises after that point still goes through the project exception handler and comes back as ``ErrorResponse``; a method this view does not implement is the reachable case. """ -from api_v2.openapi_schema import ErrorResponse from drf_spectacular.utils import ( OpenApiResponse, extend_schema, @@ -28,8 +29,7 @@ from platform_api.models import ApiKeyPermission #: The statuses this operation answers from the authentication middleware, in -#: ``PlatformKeyError`` shape. Everything else it can return is DRF's own and -#: keeps the handler shape. +#: ``PlatformKeyError`` shape. _MIDDLEWARE_ANSWERED_STATUSES = frozenset({"401", "403"}) @@ -42,8 +42,9 @@ class PlatformKeyAutoSchema(StandardizedErrorsAutoSchema): (``drf_standardized_errors/openapi.py:343-356``). Where this operation declares ``PlatformKeyError`` that example contradicts the ``$ref`` beside it, and a reader following it writes ``errors[0].code`` and gets a - ``KeyError`` on the wire. Where the operation really does return the - handler body -- a 405, say -- the example is correct and is kept. + ``KeyError`` on the wire. The narrowing is by status rather than blanket so + that a response later declared with the handler's own shape keeps the + example that is correct for it. """ def _get_examples( @@ -100,6 +101,10 @@ class PlatformKeyError(serializers.Serializer): "Only a platform API key is accepted. An API deployment key authenticates " "against a different table on a path that never reaches this endpoint, and " "is rejected as unauthenticated.\n\n" + "This route serves GET only. Another method is refused either by the " + "key's permission tier or by the route itself; neither refusal is " + "described here, because OpenAPI attaches responses to an operation and " + "there is no operation for a method the route does not serve.\n\n" "The same route also answers under an organisation segment " "(`/api/v1/unstract/{org}/whoami/`), where the key must additionally belong " "to the organisation named. Prefer the form documented here: it is the one " @@ -127,12 +132,6 @@ class PlatformKeyError(serializers.Serializer): "tier is not one this deployment knows, or the request named an " "organisation the key does not belong to.", ), - 405: OpenApiResponse( - ErrorResponse, - description="This route serves GET only. A key whose tier " - "permits the method reaches the view and is refused here; a " - "tier that does not is refused earlier, as a 403.", - ), 500: OpenApiResponse( description="The request could not be served. The body is not " "guaranteed to be JSON.", diff --git a/backend/platform_api/tests/test_whoami.py b/backend/platform_api/tests/test_whoami.py index 61e809d844..9a1c4256fa 100644 --- a/backend/platform_api/tests/test_whoami.py +++ b/backend/platform_api/tests/test_whoami.py @@ -164,13 +164,18 @@ def test_an_inactive_key_is_rejected(self) -> None: key = self._make_key(is_active=False) self.assertEqual(self._get(str(key.key)).status_code, 401) - def test_a_disallowed_method_is_the_handler_shape_the_spec_declares(self) -> None: + def test_a_disallowed_method_returns_the_handler_shape(self) -> None: """The endpoint returns two error shapes, and this is the second one. A tier that permits POST gets past the middleware and is refused by DRF, in `{type, errors[]}` -- not the `{message}` the credential - failures use. Undeclared and unasserted, that is the same spec-lies- - about-the-wire defect this suite already carries two other pins for. + failures use. + + This asserts the wire only. It says nothing about the spec, and it does + not fail if the spec stops describing this: the route serves GET, so + OpenAPI has no operation to hang a POST response on. That gap is + recorded on the PR rather than papered over with a test that reads as + coverage it does not provide. """ key = self._make_key(permission=ApiKeyPermission.READ_WRITE) diff --git a/specs/docstudio-oss.json b/specs/docstudio-oss.json index c9776c8b2e..1cd497b757 100644 --- a/specs/docstudio-oss.json +++ b/specs/docstudio-oss.json @@ -303,7 +303,7 @@ "paths": { "/api/v1/unstract/whoami/": { "get": { - "description": "Resolve the organisation a platform API key belongs to.\n\nThe organisation is read from the key itself, so this route carries no organisation segment and needs nothing but the key. Call it once and store `organization_id`; every other endpoint takes it as a path segment.\n\nOnly a platform API key is accepted. An API deployment key authenticates against a different table on a path that never reaches this endpoint, and is rejected as unauthenticated.\n\nThe same route also answers under an organisation segment (`/api/v1/unstract/{org}/whoami/`), where the key must additionally belong to the organisation named. Prefer the form documented here: it is the one that needs no organisation to begin with.", + "description": "Resolve the organisation a platform API key belongs to.\n\nThe organisation is read from the key itself, so this route carries no organisation segment and needs nothing but the key. Call it once and store `organization_id`; every other endpoint takes it as a path segment.\n\nOnly a platform API key is accepted. An API deployment key authenticates against a different table on a path that never reaches this endpoint, and is rejected as unauthenticated.\n\nThis route serves GET only. Another method is refused either by the key's permission tier or by the route itself; neither refusal is described here, because OpenAPI attaches responses to an operation and there is no operation for a method the route does not serve.\n\nThe same route also answers under an organisation segment (`/api/v1/unstract/{org}/whoami/`), where the key must additionally belong to the organisation named. Prefer the form documented here: it is the one that needs no organisation to begin with.", "operationId": "whoami", "responses": { "200": { @@ -336,30 +336,6 @@ }, "description": "The key was recognised but refused: its permission tier is not one this deployment knows, or the request named an organisation the key does not belong to." }, - "405": { - "content": { - "application/json": { - "examples": { - "MethodNotAllowed": { - "value": { - "errors": [ - { - "attr": null, - "code": "method_not_allowed", - "detail": "Method \"get\" not allowed." - } - ], - "type": "client_error" - } - } - }, - "schema": { - "$ref": "#/components/schemas/ErrorResponse" - } - } - }, - "description": "This route serves GET only. A key whose tier permits the method reaches the view and is refused here; a tier that does not is refused earlier, as a 403." - }, "500": { "description": "The request could not be served. The body is not guaranteed to be JSON." }