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/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..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 @@ -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..beb0c37bad 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,22 @@ #: 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", "NotAcceptable"), + ("status", "500", "APIException"), + ("execute", "500", "APIException"), +} + +#: 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()) @@ -89,6 +106,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. @@ -115,23 +156,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 +296,108 @@ 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_reads_errors_are_the_shape_the_middleware_sends() -> None: + """`whoami` authenticates in middleware, which answers with a bare + `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. + + Paired with `test_a_rejection_carries_the_body_the_spec_publishes` in + `platform_api`, which pins the same claim against the wire. + """ + spec = _committed() + 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. + assert reads + for path, operation in reads: + 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_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()): + 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, + name, + ) 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. + """ + 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( "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..0b763e3c59 100644 --- a/backend/backend/base_urls.py +++ b/backend/backend/base_urls.py @@ -9,6 +9,21 @@ # Combine the URL patterns urlpatterns = [ + # Organisation-less, and so mounted ahead of the tenant urlconf rather than + # 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"), + ), 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..7079de7ae4 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,15 @@ 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. +# 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/$", +] # 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..86768e2406 --- /dev/null +++ b/backend/platform_api/openapi_schema.py @@ -0,0 +1,142 @@ +"""OpenAPI annotation for the ``whoami`` endpoint. + +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``. + +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 publishes two error shapes, because it really sends two. +``whoami`` is deliberately absent from ``WHITELISTED_PATHS``, so +``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 drf_spectacular.utils import ( + OpenApiResponse, + 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 + +#: The statuses this operation answers from the authentication middleware, in +#: ``PlatformKeyError`` shape. +_MIDDLEWARE_ANSWERED_STATUSES = frozenset({"401", "403"}) + + +class PlatformKeyAutoSchema(StandardizedErrorsAutoSchema): + """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``). 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. 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( + 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): + """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.") + + +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. 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.") + + +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.\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 " + "that needs no organisation to begin with." +) + + +# 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( + PlatformKeyError, + description="No usable platform API key was supplied — absent, " + "malformed, unknown, or revoked.", + ), + 403: OpenApiResponse( + 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( + 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 new file mode 100644 index 0000000000..9a1c4256fa --- /dev/null +++ b/backend/platform_api/tests/test_whoami.py @@ -0,0 +1,262 @@ +"""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 platform_api.whoami_views import WhoAmIView +from rest_framework.test import APIRequestFactory, 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: + """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) + 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, 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. + """ + 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) + + 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. + + 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) + + 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. + + 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 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) + + 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) + # 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_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..25846a567b --- /dev/null +++ b/backend/platform_api/whoami_views.py @@ -0,0 +1,71 @@ +"""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.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.request import Request +from rest_framework.response import Response + +from platform_api.openapi_schema import WHOAMI_SCHEMA, PlatformKeyAutoSchema + + +@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 = [] + + # 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 + # 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. + # + # 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( + { + "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..1cd497b757 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": { @@ -209,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. 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.", + "type": "string" + } + }, + "required": [ + "message" + ], + "type": "object" + }, "StatusResponse": { "properties": { "message": { @@ -226,6 +248,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 +287,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 +301,55 @@ }, "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.\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": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/WhoAmIResponse" + } + } + }, + "description": "" + }, + "401": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/PlatformKeyError" + } + } + }, + "description": "No usable platform API key was supplied \u2014 absent, malformed, unknown, or revoked." + }, + "403": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/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": { + "description": "The request could not be served. The body is not guaranteed to be JSON." + } + }, + "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 +825,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" } ] } 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]