diff --git a/AGENTS.md b/AGENTS.md index 9478dd1..63e63b4 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -55,6 +55,8 @@ own default include globs rather than the root config's `**/tests/**` + `**/src/ Integration tests are `pnpm run test:integration` (config in `tests/vitest.config.ts`). Most files start their own in-process server; `api-endpoints` and `sdk-compat` need a real gateway on `localhost:8080` via `bash start.sh`, which needs Docker Postgres. +`api-endpoints` additionally needs *seeded* data: it reads the first existing ontology rather than creating one, so against an empty store its whole first test yields nothing and 20 tests cascade into "expected undefined to be defined". +Seed with any `scripts/demo/seed-*.sh` before blaming a change. With data seeded it is 36/37, the one failure being Actions apply returning 404 where the test expects 400; that failure predates this note. Suites that start their own server pass `new OntologyStore(null)` / `new ObjectStore(null)` so they stay off the shared file store; a store built with the default path loads `/tmp/openfoundry-data/`, @@ -101,6 +103,24 @@ Without `DATABASE_URL` every service uses its in-memory or `/tmp/openfoundry-dat Dev login is `admin` / `admin123` (see `DEV_USERS` in `services/svc-multipass/src/routes/auth.ts`). +## Identity and permissions + +Two different `requirePermission` exports coexist and only one is reachable. +`services/svc-gateway/src/middleware/rbac.ts` has zero importers repo-wide; the one that actually guards 118 routes is `@openfoundry/permissions`, whose header-based variant reads `x-user-id` / `x-user-roles`. +Those headers are a trusted-hop input, never a client input: the gateway proxy drops every `x-user-*` header so a caller cannot assert its own roles. Do not re-add them to the forwarded set, and do not introduce a second identity header without a hop that sets it from a verified token. +Seed and sync scripts talk to the service ports directly, not through the gateway, so that strip does not reach them - which is also why a service port must never be publicly reachable. +Role enforcement is still absent after that strip: what it removed was a role claim any caller could forge, not working enforcement, and nothing yet sets those headers from a verified token. +Until a trusted hop does, `ENFORCE_PERMISSIONS=true` makes every gateway-proxied route answer 403, because the permission hook sees neither `x-user-id` nor `request.claims`. + +`request.claims` is never populated: `authPlugin` is registered unencapsulated at `services/svc-gateway/src/server.ts:153` without `fastify-plugin`, so its `onRequest` hook applies to nothing. +Until that is fixed, no token-based permission check enforces anything. `claims.sub` is also a username, not a RID (`svc-multipass/src/routes/auth.ts:122`), and issued tokens carry no `roles` claim at all (`packages/auth-tokens/src/token-creator.ts`). + +Neither model is Foundry's, so do not converge on either as-is. +Foundry grants *roles on a resource* - `POST /v2/filesystem/resources/{resourceRid}/roles/add`, inherited by the container's contents, with principals that may be groups - while roles themselves are platform-defined and read-only over the API (`GET /v2/admin/roles/{roleId}` and `getBatch` only, no create or delete). +Against that, `@openfoundry/permissions` roles are global (an EDITOR may write every ontology in the deployment) and `rbac.ts` is a bare `(subject, resource, permission)` ACL with no role, container, group or owner. +`createPgStoreHook` in `packages/permissions/src/middleware.ts` is the one piece already shaped right: admin bypass, then resource grant, then role fallback. +Tenant isolation is a separate, *mandatory* layer in Foundry (Organizations) and is `org_rid` + RLS here - no permission check delivers it, and none should be described as doing so. + ## Front-end to service sharp edges The console must not send `Content-Type: application/json` on a request with no body: Fastify rejects it with `FST_ERR_CTP_EMPTY_JSON_BODY`, which the gateway reports as a 500. diff --git a/deploy/.env.production b/deploy/.env.production index c84324d..7b3a714 100644 --- a/deploy/.env.production +++ b/deploy/.env.production @@ -53,4 +53,7 @@ TRACING_ENABLED=false TRACING_ENDPOINT= # --- Permissions -------------------------------------------------------------- +# The gateway strips client-asserted `x-user-*` headers and nothing yet sets them +# from a verified token, so `true` makes guarded gateway-proxied routes answer 403. +# See the identity-header note in services/svc-gateway/src/proxy.ts. ENFORCE_PERMISSIONS=true diff --git a/deploy/docker-compose.yml b/deploy/docker-compose.yml index f649b19..7dacab3 100644 --- a/deploy/docker-compose.yml +++ b/deploy/docker-compose.yml @@ -30,6 +30,8 @@ x-service-env: &service-env LOG_LEVEL: ${LOG_LEVEL:-warn} LOG_FORMAT: json NODE_ENV: production + # Guarded gateway-proxied routes answer 403 while this is "true": see the + # identity-header note in services/svc-gateway/src/proxy.ts. ENFORCE_PERMISSIONS: "true" S3_ENDPOINT: http://minio:9000 S3_ACCESS_KEY: ${MINIO_ROOT_USER} diff --git a/services/svc-gateway/src/proxy.test.ts b/services/svc-gateway/src/proxy.test.ts index 39c6ca8..52a2981 100644 --- a/services/svc-gateway/src/proxy.test.ts +++ b/services/svc-gateway/src/proxy.test.ts @@ -206,6 +206,58 @@ describe("Gateway proxy routing", () => { expect(headers["x-forwarded-for"]).toBeDefined(); }); + it("does not forward client-asserted x-user-* headers to upstream", async () => { + const mockResponse = new Response("{}", { + status: 200, + headers: { "content-type": "application/json" }, + }); + vi.mocked(globalThis.fetch).mockResolvedValueOnce(mockResponse); + + const app = await createServer({ config: TEST_CONFIG }); + await app.inject({ + method: "GET", + url: "/api/v2/ontologies", + headers: { + // Downstream, requirePermission("ontology:read") would honour these. + "x-user-id": "mallory", + "x-user-roles": "ADMIN", + }, + }); + + const [, fetchOptions] = vi.mocked(globalThis.fetch).mock.calls[0]; + const headers = (fetchOptions as RequestInit).headers as Record; + expect(headers["x-user-id"]).toBeUndefined(); + expect(headers["x-user-roles"]).toBeUndefined(); + }); + + it("strips x-user-* regardless of header casing, and keeps other headers", async () => { + const mockResponse = new Response("{}", { + status: 200, + headers: { "content-type": "application/json" }, + }); + vi.mocked(globalThis.fetch).mockResolvedValueOnce(mockResponse); + + const app = await createServer({ config: TEST_CONFIG }); + await app.inject({ + method: "GET", + url: "/api/v2/ontologies", + headers: { + "X-User-Roles": "ADMIN", + "X-User-Groups": "platform-admins", + authorization: "Bearer token-123", + "x-uploaded-by": "alice", + }, + }); + + const [, fetchOptions] = vi.mocked(globalThis.fetch).mock.calls[0]; + const headers = (fetchOptions as RequestInit).headers as Record; + const names = Object.keys(headers).map((k) => k.toLowerCase()); + expect(names.some((n) => n.startsWith("x-user-"))).toBe(false); + // Everything the caller is entitled to assert still goes through. + expect(headers["authorization"]).toBe("Bearer token-123"); + expect(headers["x-uploaded-by"]).toBe("alice"); + }); + it("strips upstream hop-by-hop headers from response", async () => { const mockResponse = new Response("{}", { status: 200, diff --git a/services/svc-gateway/src/proxy.ts b/services/svc-gateway/src/proxy.ts index 2672c15..2153b22 100644 --- a/services/svc-gateway/src/proxy.ts +++ b/services/svc-gateway/src/proxy.ts @@ -15,6 +15,37 @@ const HOP_BY_HOP_HEADERS = new Set([ "upgrade", ]); +// --------------------------------------------------------------------------- +// Client-asserted identity headers +// --------------------------------------------------------------------------- + +/** + * Headers by which a caller asserts *who it is* rather than *what it wants*. + * + * Downstream services treat these as trusted: the permission middleware in + * `@openfoundry/permissions` reads `x-user-roles` and grants whatever those + * roles imply. They are therefore only ever safe when a trusted hop sets them + * from a verified token, and never safe to forward from the public edge. + * + * The gateway is that edge, so it drops every `x-user-*` header arriving from + * a client. Nothing legitimate sends them today: the console authenticates + * with `Authorization: Bearer`, and the seed and sync scripts talk to the + * service ports directly rather than through this proxy. + * + * This only removes an inbound trust. It does not grant anyone access, and it + * does not populate the headers either; `request.claims` is never set either, + * because `authPlugin` is registered unencapsulated in `server.ts`. + * + * Known consequence: role-based enforcement does NOT work after this change. + * What was removed is fake enforcement - a client could assert its own roles - + * and real enforcement waits on the gateway task that validates JWTs and sets + * these headers from validated claims. Until that trusted hop exists, running + * with `ENFORCE_PERMISSIONS=true` makes every gateway-proxied route answer 403, + * because the permission hook sees neither `x-user-id` nor `request.claims`. + * That is known and accepted, not an oversight. + */ +const CLIENT_ASSERTED_IDENTITY_PREFIX = "x-user-"; + // --------------------------------------------------------------------------- // Proxy // --------------------------------------------------------------------------- @@ -26,6 +57,7 @@ const HOP_BY_HOP_HEADERS = new Set([ * Behaviour: * - Preserves method, path, query string, headers, and body. * - Strips hop-by-hop headers from both the outgoing and incoming directions. + * - Strips client-asserted `x-user-*` identity headers from the outgoing direction. * - Injects `X-Forwarded-For` and `X-Request-Id` headers. * - Returns 502 Bad Gateway when the target is unreachable. */ @@ -40,8 +72,11 @@ export async function proxyRequest( // Build outgoing headers, stripping hop-by-hop entries. const outgoingHeaders: Record = {}; for (const [key, value] of Object.entries(request.headers)) { - if (HOP_BY_HOP_HEADERS.has(key.toLowerCase())) continue; - if (key.toLowerCase() === "host") continue; // let fetch set the correct Host + const lowerKey = key.toLowerCase(); + if (HOP_BY_HOP_HEADERS.has(lowerKey)) continue; + if (lowerKey === "host") continue; // let fetch set the correct Host + // Never forward a caller's own claim about its identity or roles. + if (lowerKey.startsWith(CLIENT_ASSERTED_IDENTITY_PREFIX)) continue; if (value !== undefined) { outgoingHeaders[key] = Array.isArray(value) ? value.join(", ") : value; }