diff --git a/deploy/lib/database-construct.ts b/deploy/lib/database-construct.ts index 35e84893..b599a648 100644 --- a/deploy/lib/database-construct.ts +++ b/deploy/lib/database-construct.ts @@ -77,8 +77,9 @@ export class DatabaseConstruct extends Construct { this.serviceAccountKeysTable = this.createTable({ name: "service-account-keys", stage, - // The key's jti; the key itself is never stored. - partitionKey: "jti", + // SHA-256 of the key, which the proxy presents to look it up; the key + // itself is never stored. + partitionKey: "key_hash", indexes: [ { // fetch the keys of a service account diff --git a/scripts/init-local.ts b/scripts/init-local.ts index 2ede2725..c2fff4bb 100644 --- a/scripts/init-local.ts +++ b/scripts/init-local.ts @@ -178,10 +178,10 @@ async function createTables() { new CreateTableCommand({ TableName: getTableName("service-account-keys"), AttributeDefinitions: [ - { AttributeName: "jti", AttributeType: "S" }, + { AttributeName: "key_hash", AttributeType: "S" }, { AttributeName: "account_id", AttributeType: "S" }, ], - KeySchema: [{ AttributeName: "jti", KeyType: "HASH" }], + KeySchema: [{ AttributeName: "key_hash", KeyType: "HASH" }], GlobalSecondaryIndexes: [ { IndexName: "account_id", diff --git a/src/app/(app)/edit/account/[account_id]/service-accounts/[service_account_id]/page.tsx b/src/app/(app)/edit/account/[account_id]/service-accounts/[service_account_id]/page.tsx index 74b103dd..31200366 100644 --- a/src/app/(app)/edit/account/[account_id]/service-accounts/[service_account_id]/page.tsx +++ b/src/app/(app)/edit/account/[account_id]/service-accounts/[service_account_id]/page.tsx @@ -14,7 +14,7 @@ import { CONFIG } from "@/lib/config"; import { getPageSession } from "@/lib/api/utils"; import { managedServiceAccount } from "@/lib/accounts/service-accounts"; import { editAccountServiceAccountsUrl } from "@/lib/urls"; -import { MembershipState } from "@/types"; +import { MembershipState, publicKey } from "@/types"; export const metadata: Metadata = { title: "Service account" }; @@ -47,7 +47,8 @@ export default async function ServiceAccountPage({ params }: PageProps) { account, trusts, grants: memberships.filter((m) => m.state === MembershipState.Member), - keys, + // The hash stays on the server; the client component sees the rest. + keys: keys.map(publicKey), }} products={products.map(({ product_id, title }) => ({ product_id, title }))} proxyOrigin={CONFIG.storage.endpoint} diff --git a/src/app/(app)/edit/account/[account_id]/service-accounts/page.tsx b/src/app/(app)/edit/account/[account_id]/service-accounts/page.tsx index 6667b87f..9a70b328 100644 --- a/src/app/(app)/edit/account/[account_id]/service-accounts/page.tsx +++ b/src/app/(app)/edit/account/[account_id]/service-accounts/page.tsx @@ -14,7 +14,7 @@ import { import { getPageSession } from "@/lib/api/utils"; import { canManageAccount } from "@/lib/api/authz"; import { createServiceAccountUrl } from "@/lib/urls"; -import { MembershipState, type ServiceAccountSummary } from "@/types"; +import { MembershipState, publicKey, type ServiceAccountSummary } from "@/types"; export const metadata: Metadata = { title: "Service accounts" }; @@ -38,7 +38,7 @@ export default async function ServiceAccountsPage({ params }: PageProps) { grants: (await membershipsTable.listByUser(account.account_id)).filter( (m) => m.state === MembershipState.Member ), - keys: await serviceAccountKeysTable.listByAccount(account.account_id), + keys: (await serviceAccountKeysTable.listByAccount(account.account_id)).map(publicKey), })) ); diff --git a/src/app/api/v1/service-account-keys/[jti]/exchanges/route.test.ts b/src/app/api/v1/service-account-keys/[jti]/exchanges/route.test.ts deleted file mode 100644 index eb009a6c..00000000 --- a/src/app/api/v1/service-account-keys/[jti]/exchanges/route.test.ts +++ /dev/null @@ -1,48 +0,0 @@ -/** @jest-environment node */ -import { NextRequest } from "next/server"; -import { serviceAccountKeysTable } from "@/lib/clients/database"; -import { getApiSession } from "@/lib/api/utils"; -import { isAdmin } from "@/lib/api/authz"; - -jest.mock("@/lib/clients/database", () => ({ - serviceAccountKeysTable: { fetchByJti: jest.fn(), set: jest.fn() }, -})); -jest.mock("@/lib/api/utils", () => ({ getApiSession: jest.fn() })); -jest.mock("@/lib/api/authz", () => ({ isAdmin: jest.fn() })); - -const { POST } = require("./route"); - -const key = { jti: "j1", account_id: "acme--nightly-sync", label: "HPC", expires_at: null }; -const params = { params: { jti: "j1" } }; -const req = () => new NextRequest("http://localhost/api/v1/service-account-keys/j1/exchanges", { method: "POST" }); - -describe("POST /api/v1/service-account-keys/[jti]/exchanges", () => { - beforeEach(() => { - jest.resetAllMocks(); - (serviceAccountKeysTable.fetchByJti as jest.Mock).mockResolvedValue(key); - (getApiSession as jest.Mock).mockResolvedValue({ account: { account_id: "acme--nightly-sync" } }); - (isAdmin as jest.Mock).mockReturnValue(false); - }); - - test("answers active for a live key, as the account it belongs to, and records the use", async () => { - const res = await POST(req(), params); - expect(res.status).toBe(200); - await expect(res.json()).resolves.toMatchObject({ account_id: "acme--nightly-sync", active: true }); - expect(serviceAccountKeysTable.set).toHaveBeenCalledWith("j1", "last_used_at", expect.any(String)); - }); - - test("answers inactive for a revoked or expired key, without recording a use", async () => { - (serviceAccountKeysTable.fetchByJti as jest.Mock).mockResolvedValue({ ...key, revoked_at: "2026-01-01T00:00:00Z" }); - await expect((await POST(req(), params)).json()).resolves.toMatchObject({ active: false }); - (serviceAccountKeysTable.fetchByJti as jest.Mock).mockResolvedValue({ ...key, expires_at: "2020-01-01T00:00:00Z" }); - await expect((await POST(req(), params)).json()).resolves.toMatchObject({ active: false }); - expect(serviceAccountKeysTable.set).not.toHaveBeenCalled(); - }); - - test("is 401 for any other account, 404 for an unknown jti", async () => { - (getApiSession as jest.Mock).mockResolvedValue({ account: { account_id: "someone-else" } }); - expect((await POST(req(), params)).status).toBe(401); - (serviceAccountKeysTable.fetchByJti as jest.Mock).mockResolvedValue(null); - expect((await POST(req(), params)).status).toBe(404); - }); -}); diff --git a/src/app/api/v1/service-account-keys/[jti]/exchanges/route.ts b/src/app/api/v1/service-account-keys/[jti]/exchanges/route.ts deleted file mode 100644 index b84e74f1..00000000 --- a/src/app/api/v1/service-account-keys/[jti]/exchanges/route.ts +++ /dev/null @@ -1,54 +0,0 @@ -/** - * @openapi - * /service-account-keys/{jti}/exchanges: - * post: - * tags: [Accounts] - * summary: Record that an API key was presented, and say whether it may be exchanged - * description: | - * Called by the data proxy at `/.sts` when a token carrying this `jti` is presented, as the service account the key belongs to. Records the use and answers whether the key is still active — not revoked, not expired. The proxy caches the answer briefly, so revocation takes effect for new exchanges within that TTL. - * parameters: - * - in: path - * name: jti - * required: true - * schema: - * type: string - * responses: - * 200: - * description: The key's standing - * 401: - * description: Unauthorized - * 404: - * description: Not Found - */ -import { NextRequest, NextResponse } from "next/server"; -import { StatusCodes } from "http-status-codes"; -import { getApiSession } from "@/lib/api/utils"; -import { isAdmin } from "@/lib/api/authz"; -import { serviceAccountKeysTable } from "@/lib/clients/database"; -import { isKeyActive } from "@/types"; - -export async function POST( - request: NextRequest, - { params }: { params: Promise<{ jti: string }> } -) { - const session = await getApiSession(request); - const { jti } = await params; - const key = await serviceAccountKeysTable.fetchByJti(jti); - if (!key) { - return NextResponse.json({ error: "No such key" }, { status: StatusCodes.NOT_FOUND }); - } - // Only the account the key belongs to — the proxy, calling as it — may ask. - if (!session?.account || (session.account.account_id !== key.account_id && !isAdmin(session))) { - return NextResponse.json({ error: "Unauthorized" }, { status: StatusCodes.UNAUTHORIZED }); - } - const active = isKeyActive(key); - if (active) { - await serviceAccountKeysTable.set(jti, "last_used_at", new Date().toISOString()); - } - return NextResponse.json({ - account_id: key.account_id, - active, - expires_at: key.expires_at, - revoked_at: key.revoked_at ?? null, - }); -} diff --git a/src/app/api/v1/service-account-keys/exchanges/route.test.ts b/src/app/api/v1/service-account-keys/exchanges/route.test.ts new file mode 100644 index 00000000..38d39ebd --- /dev/null +++ b/src/app/api/v1/service-account-keys/exchanges/route.test.ts @@ -0,0 +1,74 @@ +/** @jest-environment node */ +import { NextRequest } from "next/server"; +import { accountsTable, serviceAccountKeysTable } from "@/lib/clients/database"; +import { verifyProxyAssertion } from "@/lib/api/oidc"; + +jest.mock("@/lib/clients/database", () => ({ + serviceAccountKeysTable: { fetchByHash: jest.fn(), set: jest.fn() }, + accountsTable: { fetchById: jest.fn() }, +})); +jest.mock("@/lib/api/oidc", () => ({ + verifyProxyAssertion: jest.fn(), + PROXY_SELF_SUBJECT: "urn:source:data-proxy", +})); + +const { POST } = require("./route"); + +const HASH = "a".repeat(64); +const key = { key_hash: HASH, key_id: "k1", account_id: "acme--nightly-sync", label: "HPC", expires_at: null }; +const req = (body: unknown = { key_hash: HASH }) => + new NextRequest("http://localhost/api/v1/service-account-keys/exchanges", { + method: "POST", + headers: { authorization: "Bearer proxy", "x-request-id": "r1" }, + body: JSON.stringify(body), + }); + +describe("POST /api/v1/service-account-keys/exchanges", () => { + beforeEach(() => { + jest.resetAllMocks(); + (verifyProxyAssertion as jest.Mock).mockResolvedValue({ sub: "urn:source:data-proxy" }); + (serviceAccountKeysTable.fetchByHash as jest.Mock).mockResolvedValue(key); + (serviceAccountKeysTable.set as jest.Mock).mockResolvedValue(undefined); + (accountsTable.fetchById as jest.Mock).mockResolvedValue({ account_id: "acme--nightly-sync", disabled: false }); + }); + + test("answers active with the account for a live key, and records the use", async () => { + const res = await POST(req()); + expect(res.status).toBe(200); + await expect(res.json()).resolves.toEqual({ account_id: "acme--nightly-sync", key_id: "k1", active: true }); + expect(serviceAccountKeysTable.fetchByHash).toHaveBeenCalledWith(HASH); + expect(serviceAccountKeysTable.set).toHaveBeenCalledWith(HASH, "last_used_at", expect.any(String)); + }); + + test("still answers active when recording the use fails", async () => { + (serviceAccountKeysTable.set as jest.Mock).mockRejectedValue(new Error("throttled")); + await expect((await POST(req())).json()).resolves.toMatchObject({ active: true }); + }); + + test("answers inactive, naming no account, for a revoked, expired, disabled or unknown key, without recording a use", async () => { + (serviceAccountKeysTable.fetchByHash as jest.Mock).mockResolvedValue({ ...key, revoked_at: "2026-01-01T00:00:00Z" }); + await expect((await POST(req())).json()).resolves.toEqual({ active: false }); + (serviceAccountKeysTable.fetchByHash as jest.Mock).mockResolvedValue({ ...key, expires_at: "2020-01-01T00:00:00Z" }); + await expect((await POST(req())).json()).resolves.toEqual({ active: false }); + (serviceAccountKeysTable.fetchByHash as jest.Mock).mockResolvedValue(key); + (accountsTable.fetchById as jest.Mock).mockResolvedValue({ account_id: "acme--nightly-sync", disabled: true }); + await expect((await POST(req())).json()).resolves.toEqual({ active: false }); + (serviceAccountKeysTable.fetchByHash as jest.Mock).mockResolvedValue(null); + await expect((await POST(req())).json()).resolves.toEqual({ active: false }); + expect(serviceAccountKeysTable.set).not.toHaveBeenCalled(); + }); + + test("is 401 for any subject but the proxy's own, or no assertion", async () => { + (verifyProxyAssertion as jest.Mock).mockResolvedValue({ sub: "acme--nightly-sync" }); + expect((await POST(req())).status).toBe(401); + (verifyProxyAssertion as jest.Mock).mockResolvedValue(null); + expect((await POST(req())).status).toBe(401); + expect(serviceAccountKeysTable.fetchByHash).not.toHaveBeenCalled(); + }); + + test("is 400 for a body without a hex SHA-256", async () => { + expect((await POST(req({ key_hash: "sck_notahash" }))).status).toBe(400); + expect((await POST(req({}))).status).toBe(400); + expect(serviceAccountKeysTable.fetchByHash).not.toHaveBeenCalled(); + }); +}); diff --git a/src/app/api/v1/service-account-keys/exchanges/route.ts b/src/app/api/v1/service-account-keys/exchanges/route.ts new file mode 100644 index 00000000..579e9144 --- /dev/null +++ b/src/app/api/v1/service-account-keys/exchanges/route.ts @@ -0,0 +1,82 @@ +/** + * @openapi + * /service-account-keys/exchanges: + * post: + * tags: [Accounts] + * summary: Say whether an API key may be exchanged, and which service account it belongs to + * description: | + * Called by the data proxy at `/.sts` when an API key is presented, authenticated as the proxy itself: the key is opaque, so nothing names an account before this lookup. The proxy sends the key's SHA-256; this answers whether the key is active — known, not revoked, not expired, and its service account not disabled — and, if so, which account it is. An unknown hash is answered as inactive, indistinguishable from a revoked one. Records last use. The proxy caches the answer briefly, so revocation takes effect for new exchanges within that TTL. + * requestBody: + * required: true + * content: + * application/json: + * schema: + * type: object + * required: [key_hash] + * properties: + * key_hash: + * type: string + * description: Hex SHA-256 of the key + * responses: + * 200: + * description: The key's standing + * 400: + * description: Bad Request + * 401: + * description: Unauthorized + */ +import { NextRequest, NextResponse } from "next/server"; +import { StatusCodes } from "http-status-codes"; +import { z } from "zod"; +import { PROXY_SELF_SUBJECT, verifyProxyAssertion } from "@/lib/api/oidc"; +import { accountsTable, serviceAccountKeysTable } from "@/lib/clients/database"; +import { LOGGER } from "@/lib/logging"; +import { isKeyActive } from "@/types"; + +const BodySchema = z.object({ key_hash: z.string().regex(/^[0-9a-f]{64}$/) }); + +export async function POST(request: NextRequest) { + // Only the proxy, as itself. Never a session: the sentinel subject resolves + // to no account, and a cookie must not reach this route. + const assertion = await verifyProxyAssertion( + request.headers.get("authorization"), + new URL(request.url).origin + ); + if (assertion?.sub !== PROXY_SELF_SUBJECT) { + return NextResponse.json({ error: "Unauthorized" }, { status: StatusCodes.UNAUTHORIZED }); + } + const parsed = BodySchema.safeParse(await request.json().catch(() => null)); + if (!parsed.success) { + return NextResponse.json({ error: "key_hash is required" }, { status: StatusCodes.BAD_REQUEST }); + } + + const request_id = request.headers.get("x-request-id") ?? undefined; + const key = await serviceAccountKeysTable.fetchByHash(parsed.data.key_hash); + const account = key ? await accountsTable.fetchById(key.account_id) : null; + const reason = !key + ? "unknown" + : key.revoked_at + ? "revoked" + : !isKeyActive(key) + ? "expired" + : !account || account.disabled + ? "disabled" + : null; + const meta = { request_id, key_id: key?.key_id, account_id: key?.account_id, reason }; + if (!key || reason) { + LOGGER.warn("API key not exchangeable", { operation: "serviceAccountKeyExchange", metadata: meta }); + return NextResponse.json({ active: false }); + } + + // Best-effort: a throttled write must not refuse a live key. + await serviceAccountKeysTable + .set(key.key_hash, "last_used_at", new Date().toISOString()) + .catch((error: unknown) => + LOGGER.warn("Could not record API key use", { + operation: "serviceAccountKeyExchange", + metadata: { ...meta, error: error instanceof Error ? error.message : String(error) }, + }) + ); + LOGGER.info("API key exchanged", { operation: "serviceAccountKeyExchange", metadata: meta }); + return NextResponse.json({ account_id: key.account_id, key_id: key.key_id, active: true }); +} diff --git a/src/components/features/service-accounts/ApiKeyExpiryField.stories.tsx b/src/components/features/service-accounts/ApiKeyExpiryField.stories.tsx new file mode 100644 index 00000000..a79b38f0 --- /dev/null +++ b/src/components/features/service-accounts/ApiKeyExpiryField.stories.tsx @@ -0,0 +1,21 @@ +import type { Meta, StoryObj } from "@storybook/nextjs-vite"; +import { ApiKeyExpiryField } from "./ApiKeyExpiryField"; + +/** + * How long an API key lasts, counted from now: 30 days, 90 days, a year, or + * until it is revoked. Issuing a key starts at 90 days; changing the expiry of + * a key that never expires starts at never. + */ +const meta = { + title: "Features/Service accounts/ApiKeyExpiryField", + component: ApiKeyExpiryField, + parameters: { layout: "padded" }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +export const Default: Story = { args: { id: "expiry" } }; + +/** A key that already never expires. */ +export const Never: Story = { args: { id: "expiry", never: true } }; diff --git a/src/components/features/service-accounts/ApiKeyExpiryField.test.tsx b/src/components/features/service-accounts/ApiKeyExpiryField.test.tsx new file mode 100644 index 00000000..7bd44e7f --- /dev/null +++ b/src/components/features/service-accounts/ApiKeyExpiryField.test.tsx @@ -0,0 +1,28 @@ +import { render } from "@testing-library/react"; +import { Theme } from "@radix-ui/themes"; +import { ApiKeyExpiryField } from "./ApiKeyExpiryField"; + +// jsdom has no ResizeObserver, and Radix's Select measures its trigger with one. +global.ResizeObserver ??= class { + observe() {} + unobserve() {} + disconnect() {} +} as unknown as typeof ResizeObserver; + +// The field's one job beyond looks: submit `expires_in_days` the way the key +// actions read it. "Never" can't be an empty Select value, so it must reach the +// form as an empty string, which the actions take as no expiry. +const submitted = (ui: React.ReactElement) => + (render({ui}).container.querySelector( + 'input[name="expires_in_days"]' + ) as HTMLInputElement).value; + +describe("ApiKeyExpiryField", () => { + it("submits 90 days by default", () => { + expect(submitted()).toBe("90"); + }); + + it("submits an empty value for a key that never expires", () => { + expect(submitted()).toBe(""); + }); +}); diff --git a/src/components/features/service-accounts/ApiKeyExpiryField.tsx b/src/components/features/service-accounts/ApiKeyExpiryField.tsx new file mode 100644 index 00000000..3e09a252 --- /dev/null +++ b/src/components/features/service-accounts/ApiKeyExpiryField.tsx @@ -0,0 +1,39 @@ +"use client"; + +import React, { useState } from "react"; +import { Select } from "@radix-ui/themes"; +import { Field } from "@/components/core"; + +// Select forbids an empty item value, so "never" stands for the empty +// `expires_in_days` the key actions read as no expiry. +const NEVER = "never"; + +const EXPIRIES = [ + { value: "30", label: "30 days" }, + { value: "90", label: "90 days" }, + { value: "365", label: "A year" }, + { value: NEVER, label: "Never — until revoked" }, +]; + +/** + * How long an API key lasts, counted from now: the `expires_in_days` field + * that both issuing a key and changing its expiry submit. + */ +export function ApiKeyExpiryField({ id, never = false }: { id: string; never?: boolean }) { + const [expiry, setExpiry] = useState(never ? NEVER : "90"); + return ( + + + + + + {EXPIRIES.map((option) => ( + + {option.label} + + ))} + + + + ); +} diff --git a/src/components/features/service-accounts/IssueApiKeyDialog.stories.tsx b/src/components/features/service-accounts/IssueApiKeyDialog.stories.tsx index 62f108f8..00eefcb6 100644 --- a/src/components/features/service-accounts/IssueApiKeyDialog.stories.tsx +++ b/src/components/features/service-accounts/IssueApiKeyDialog.stories.tsx @@ -3,11 +3,13 @@ import { IssueApiKeyDialog } from "./IssueApiKeyDialog"; /** * Issuing an API key: a label and an expiry, then the key — shown once, with - * the copy affordance and the warning that says so. + * the copy affordance, the warning that says so, and the variables that point + * any AWS SDK or the AWS CLI at it. * - * `issueApiKey` is mocked in `.storybook/preview.tsx` and resolves as though - * the proxy had signed a key, so **submitting the form shows the show-once - * view**. Open the dialog, give it a label, and issue. + * `issueApiKey` is mocked in `.storybook/preview.tsx` and resolves with a key + * of the real shape, `sck_` and 43 characters of nothing secret, so + * **submitting the form shows the show-once view**. Open the dialog, give it + * a label, and issue. */ const meta = { title: "Features/Service accounts/IssueApiKeyDialog", @@ -19,5 +21,5 @@ export default meta; type Story = StoryObj; export const Default: Story = { - args: { accountId: "miskatonic--nightly-sync" }, + args: { accountId: "miskatonic--nightly-sync", proxyOrigin: "https://data.source.coop" }, }; diff --git a/src/components/features/service-accounts/IssueApiKeyDialog.tsx b/src/components/features/service-accounts/IssueApiKeyDialog.tsx index bab779f8..9bb4d859 100644 --- a/src/components/features/service-accounts/IssueApiKeyDialog.tsx +++ b/src/components/features/service-accounts/IssueApiKeyDialog.tsx @@ -1,36 +1,45 @@ "use client"; -import React, { useActionState, useState } from "react"; -import { - Button, - Callout, - Code, - Dialog, - Flex, - Select, - Text, - TextField, -} from "@radix-ui/themes"; +import React, { useActionState } from "react"; +import { Box, Button, Callout, Code, Dialog, Flex, Text, TextField } from "@radix-ui/themes"; import { ExclamationTriangleIcon, PlusIcon } from "@radix-ui/react-icons"; import { CopyToClipboard } from "@/components/core/CopyToClipboard"; import { Field } from "@/components/core"; import { issueApiKey } from "@/lib/actions/service-account-keys"; import { IDLE_API_KEY_ACTION_STATE } from "@/types"; +import { ApiKeyExpiryField } from "./ApiKeyExpiryField"; -const EXPIRIES = [ - { value: "30", label: "30 days" }, - { value: "90", label: "90 days" }, - { value: "365", label: "A year" }, - { value: "", label: "Never — until revoked" }, -]; +/** + * What a stock AWS SDK or the AWS CLI needs to sign in with a key saved to a + * file: it reads the file, exchanges the key at the proxy's STS endpoint and + * refreshes on its own, so nothing else runs beside it. A key names its own + * account, so the role ARN's account segment is ignored; it is filled in to + * match the workflow snippet. + */ +const sdkEnvironment = (proxyOrigin: string, accountId: string) => + [ + `export AWS_ROLE_ARN=arn:aws:iam::${accountId}:role/FullAccess`, + "export AWS_WEB_IDENTITY_TOKEN_FILE=/path/to/the/saved/key", + `export AWS_ENDPOINT_URL_STS=${proxyOrigin}/.sts`, + `export AWS_ENDPOINT_URL_S3=${proxyOrigin}`, + "export AWS_REGION=us-west-2", + ].join("\n"); /** - * Issues an API key for a service account and shows it once. There is no - * second look: the key is not stored, only its record. + * Issues an API key for a service account and shows it once, with the + * variables an SDK needs to use it. There is no second look: the key is not + * stored, only its record. */ -export function IssueApiKeyDialog({ accountId }: { accountId: string }) { +export function IssueApiKeyDialog({ + accountId, + proxyOrigin, +}: { + accountId: string; + /** The data proxy the key signs in to; without it, no variables are shown. */ + proxyOrigin?: string; +}) { const [state, formAction, pending] = useActionState(issueApiKey, IDLE_API_KEY_ACTION_STATE); - const [expiry, setExpiry] = useState("90"); + const environment = proxyOrigin && sdkEnvironment(proxyOrigin, accountId); return ( @@ -39,7 +48,7 @@ export function IssueApiKeyDialog({ accountId }: { accountId: string }) { Issue an API key - + Issue an API key {state.issued ? ( @@ -59,10 +68,23 @@ export function IssueApiKeyDialog({ accountId }: { accountId: string }) { + {environment && ( + + + Save it to a file, then point any AWS SDK or the AWS CLI at it: + + + + 
+                    
+                      {environment}
+                    
+
+ + + )} - Put it in AWS_WEB_IDENTITY_TOKEN_FILE and point - the SDK at the data proxy's STS endpoint. Revoke it here if it - leaks; only a hash of nothing is kept — the key itself is not stored. + Revoke it here if it leaks. Only its hash is stored — the key itself is not. @@ -73,7 +95,6 @@ export function IssueApiKeyDialog({ accountId }: { accountId: string }) { ) : (
- For environments without OIDC: a server, a scheduler, an @@ -83,18 +104,7 @@ export function IssueApiKeyDialog({ accountId }: { accountId: string }) { - - - - - {EXPIRIES.map((option) => ( - - {option.label} - - ))} - - - + {state.message && ( {state.message} diff --git a/src/components/features/service-accounts/ServiceAccountDetail.stories.tsx b/src/components/features/service-accounts/ServiceAccountDetail.stories.tsx index b6ef0795..a173a6da 100644 --- a/src/components/features/service-accounts/ServiceAccountDetail.stories.tsx +++ b/src/components/features/service-accounts/ServiceAccountDetail.stories.tsx @@ -82,7 +82,7 @@ const summary: ServiceAccountSummary = { ], keys: [ { - jti: "k1", + key_id: "k1", account_id: "nightly-sync", label: "HPC cron job", created_at: "2026-03-12T00:00:00Z", @@ -91,7 +91,7 @@ const summary: ServiceAccountSummary = { last_used_at: "2026-03-20T00:00:00Z", }, { - jti: "k2", + key_id: "k2", account_id: "nightly-sync", label: "Old laptop", created_at: "2025-03-12T00:00:00Z", diff --git a/src/components/features/service-accounts/ServiceAccountDetail.tsx b/src/components/features/service-accounts/ServiceAccountDetail.tsx index 74dc8458..1447ad69 100644 --- a/src/components/features/service-accounts/ServiceAccountDetail.tsx +++ b/src/components/features/service-accounts/ServiceAccountDetail.tsx @@ -26,7 +26,7 @@ import { setServiceAccountDisabled, } from "@/lib/actions/service-accounts"; import { githubWorkflowStep } from "@/lib/services/github-workflow"; -import { revokeApiKey } from "@/lib/actions/service-account-keys"; +import { revokeApiKey, setApiKeyExpiry } from "@/lib/actions/service-account-keys"; import { GITHUB_ACTIONS_ISSUER, IDLE_API_KEY_ACTION_STATE, @@ -41,6 +41,7 @@ import { AddGithubTrustDialog } from "./AddGithubTrustDialog"; import { ProductAccessList, type ProductAccess } from "./ProductAccessList"; import { WorkflowSnippet } from "./WorkflowSnippet"; import { IssueApiKeyDialog } from "./IssueApiKeyDialog"; +import { ApiKeyExpiryField } from "./ApiKeyExpiryField"; const issuerLabel = (issuer: string) => issuer === GITHUB_ACTIONS_ISSUER ? "GitHub Actions" : issuer; @@ -83,6 +84,44 @@ function ExampleUsage({ subject, step }: { subject: string; step: string }) { ); } +/** + * A new expiry for a live key, counted from now, in a modal: longer for a + * workload that needs it, shorter during an incident, or never. + */ +function ChangeExpiry({ accountId, apiKey }: { accountId: string; apiKey: ServiceAccountKey }) { + const [state, action, saving] = useActionState(setApiKeyExpiry, IDLE_API_KEY_ACTION_STATE); + return ( + + + + + + When should {apiKey.label} expire? + + + + + + + + + + + + + + + + + ); +} + /** * One service account, and every control over it: the workflows it trusts, * each with its example usage; its API keys; the products it reaches, changed, removed or @@ -195,8 +234,8 @@ export function ServiceAccountDetail({ } + description="For environments without OIDC. Each is shown once, when it is issued. Revoke a key that leaks; to stop every key at once, disable the account below." + rightButton={} > {keys.length === 0 ? ( @@ -208,7 +247,7 @@ export function ServiceAccountDetail({ const marker = keyMarker(key); return ( {key.label} @@ -222,13 +261,17 @@ export function ServiceAccountDetail({ ].join(" · ")} actions={ !key.revoked_at && ( -
- - - -
+ + + {/* A flex box, so the button centres on the row like the one beside it. */} +
+ + + +
+ ) } /> @@ -258,8 +301,8 @@ export function ServiceAccountDetail({ {account.disabled - ? "Disabled: nothing can sign in as it. Its trusts and grants are kept." - : "Disabling stops every sign-in and keeps its trusts and grants."} + ? "Disabled: nothing can sign in as it. Enabling it lets its trusted workflows and unrevoked keys sign in again, so revoke any key that leaked first." + : "Disabling stops every sign-in, by workflow or by key, and within five minutes cuts credentials it already holds back to public data. Its trusts, keys and grants are kept."}
diff --git a/src/components/features/service-accounts/ServiceAccountList.stories.tsx b/src/components/features/service-accounts/ServiceAccountList.stories.tsx index 71b697aa..ef2e4100 100644 --- a/src/components/features/service-accounts/ServiceAccountList.stories.tsx +++ b/src/components/features/service-accounts/ServiceAccountList.stories.tsx @@ -63,7 +63,7 @@ const summary = ( ], keys: [ { - jti: `k1-${account_id}`, + key_id: `k1-${account_id}`, account_id, label: "HPC cron job", created_at: "2026-03-12T00:00:00Z", @@ -72,7 +72,7 @@ const summary = ( last_used_at: "2026-03-20T00:00:00Z", }, { - jti: `k2-${account_id}`, + key_id: `k2-${account_id}`, account_id, label: "Old laptop", created_at: "2025-03-12T00:00:00Z", diff --git a/src/lib/actions/__mocks__/service-account-keys.ts b/src/lib/actions/__mocks__/service-account-keys.ts index cd96e55c..30f3f2ad 100644 --- a/src/lib/actions/__mocks__/service-account-keys.ts +++ b/src/lib/actions/__mocks__/service-account-keys.ts @@ -4,8 +4,9 @@ import type { ApiKeyActionState } from "@/types"; /** * Storybook stand-in for the API-key server actions, redirected to by - * `sb.mock()` in `.storybook/preview.tsx`. `issueApiKey` resolves as though a - * key were signed, so the show-once view is reachable by submitting the dialog. + * `sb.mock()` in `.storybook/preview.tsx`. `issueApiKey` resolves with a key + * of the real shape, so the show-once view is reachable by submitting the + * dialog. */ const idle = (): ApiKeyActionState => ({ message: "", success: false }); @@ -14,9 +15,9 @@ export const issueApiKey: typeof Real.issueApiKey = fn( message: "", success: true, issued: { - key: "sck_eyJhbGciOiJSUzI1NiIsInR5cCI6IkpXVCIsImtpZCI6ImsxIn0.eyJpc3MiOiJodHRwczovL2RhdGEuc291cmNlLmNvb3AiLCJzdWIiOiJuaWdodGx5LXN5bmMiLCJqdGkiOiIxIiwidHlwZSI6ImFwaV9rZXkifQ.signature", + key: "sck_storyFixtureNotARealKey0123456789abcdefghij", record: { - jti: "6f1c2a3b-4d5e-4f60-8a9b-0c1d2e3f4a5b", + key_id: "6f1c2a3b-4d5e-4f60-8a9b-0c1d2e3f4a5b", account_id: "nightly-sync", label: String(formData.get("label") || "CI"), created_at: "2026-03-12T00:00:00Z", diff --git a/src/lib/actions/proxy-credentials.ts b/src/lib/actions/proxy-credentials.ts index 77fa1c90..f619080c 100644 --- a/src/lib/actions/proxy-credentials.ts +++ b/src/lib/actions/proxy-credentials.ts @@ -96,7 +96,7 @@ export async function getProxyCredentials( * If the OAuth2 client has skip_consent enabled (recommended), the consent * step is skipped and the flow completes in 4 HTTP calls instead of 6. */ -export async function getOryIdToken(identityId: string): Promise { +async function getOryIdToken(identityId: string): Promise { const { api: { backendUrl }, accessToken: adminApiKey, diff --git a/src/lib/actions/service-account-keys.test.ts b/src/lib/actions/service-account-keys.test.ts index 47d490a1..c2826a08 100644 --- a/src/lib/actions/service-account-keys.test.ts +++ b/src/lib/actions/service-account-keys.test.ts @@ -1,21 +1,19 @@ +import { createHash } from "crypto"; import { issueApiKey, revokeApiKey, setApiKeyExpiry } from "./service-account-keys"; import { serviceAccountKeysTable } from "../clients"; import { getPageSession } from "../api/utils"; import { managedServiceAccount } from "@/lib/accounts/service-accounts"; -import { mintApiKey } from "@/lib/services/proxy-keys"; -import { AccountType, type Account, type ApiKeyActionState, type UserSession } from "@/types"; +import { API_KEY_PATTERN, AccountType, type Account, type ApiKeyActionState, type UserSession } from "@/types"; jest.mock("../clients", () => ({ - serviceAccountKeysTable: { create: jest.fn(), delete: jest.fn(), fetchByJti: jest.fn(), set: jest.fn() }, + serviceAccountKeysTable: { create: jest.fn(), listByAccount: jest.fn(), set: jest.fn() }, })); jest.mock("../api/utils", () => ({ getPageSession: jest.fn() })); jest.mock("@/lib/accounts/service-accounts", () => ({ managedServiceAccount: jest.fn() })); -jest.mock("@/lib/services/proxy-keys", () => ({ mintApiKey: jest.fn() })); jest.mock("next/cache", () => ({ revalidatePath: jest.fn() })); const keys = serviceAccountKeysTable as jest.Mocked; const managed = managedServiceAccount as jest.MockedFunction; -const mint = mintApiKey as jest.MockedFunction; const IDLE: ApiKeyActionState = { message: "", success: false }; const bot = { @@ -32,24 +30,34 @@ const form = (fields: Record) => { for (const [k, v] of Object.entries(fields)) data.set(k, v); return data; }; +const sha256 = (s: string) => createHash("sha256").update(s).digest("hex"); beforeEach(() => { jest.clearAllMocks(); (getPageSession as jest.Mock).mockResolvedValue(session); managed.mockResolvedValue(bot as never); keys.create.mockImplementation(async (k) => k); - mint.mockResolvedValue("sck_eyJ.signed.key"); }); describe("issueApiKey", () => { - it("records the key, has the proxy sign it, and returns it once", async () => { + it("stores only the key's hash and returns the key once", async () => { const result = await issueApiKey(IDLE, form({ account_id: "acme--nightly-sync", label: "HPC", expires_in_days: "90" })); expect(result.success).toBe(true); - expect(result.issued?.key).toBe("sck_eyJ.signed.key"); - const record = keys.create.mock.calls[0][0]; - expect(record).toMatchObject({ account_id: "acme--nightly-sync", label: "HPC", created_by: "alice" }); - expect(record.expires_at).not.toBeNull(); - expect(mint).toHaveBeenCalledWith("ory-alice", { account_id: "acme--nightly-sync", jti: record.jti, expires_at: record.expires_at }); + const key = result.issued!.key; + expect(key).toMatch(API_KEY_PATTERN); + const stored = keys.create.mock.calls[0][0]; + expect(stored).toMatchObject({ key_hash: sha256(key), account_id: "acme--nightly-sync", label: "HPC", created_by: "alice" }); + expect(stored.expires_at).not.toBeNull(); + expect(JSON.stringify(stored)).not.toContain(key); + // The record handed back is the public one: no hash, and the same handle. + expect(result.issued!.record).not.toHaveProperty("key_hash"); + expect(result.issued!.record.key_id).toBe(stored.key_id); + }); + + it("issues a different key every time", async () => { + const a = await issueApiKey(IDLE, form({ account_id: "acme--nightly-sync", label: "a" })); + const b = await issueApiKey(IDLE, form({ account_id: "acme--nightly-sync", label: "b" })); + expect(a.issued!.key).not.toBe(b.issued!.key); }); it("issues a key with no expiry when asked", async () => { @@ -58,13 +66,6 @@ describe("issueApiKey", () => { expect(keys.create.mock.calls[0][0].expires_at).toBeNull(); }); - it("removes the record when the proxy will not sign, so no row looks like a live key", async () => { - mint.mockRejectedValue(new Error("502")); - const result = await issueApiKey(IDLE, form({ account_id: "acme--nightly-sync", label: "HPC" })); - expect(result.success).toBe(false); - expect(keys.delete).toHaveBeenCalledWith(keys.create.mock.calls[0][0].jti); - }); - it("refuses a disabled service account, even to its manager", async () => { managed.mockResolvedValue({ ...bot, disabled: true } as never); const result = await issueApiKey(IDLE, form({ account_id: "acme--nightly-sync", label: "HPC", expires_in_days: "90" })); @@ -79,27 +80,27 @@ describe("issueApiKey", () => { expect((await issueApiKey(IDLE, form({ account_id: "acme--nightly-sync", label: "" }))).success).toBe(false); expect((await issueApiKey(IDLE, form({ account_id: "acme--nightly-sync", label: "x", expires_in_days: "0" }))).success).toBe(false); expect(keys.create).not.toHaveBeenCalled(); - expect(mint).not.toHaveBeenCalled(); }); }); describe("revokeApiKey and setApiKeyExpiry", () => { - const record = { jti: "j1", account_id: "acme--nightly-sync", label: "HPC", expires_at: null } as never; + const record = { key_hash: "h1", key_id: "k1", account_id: "acme--nightly-sync", label: "HPC", expires_at: null } as never; it("revokes only a key on a service account the caller manages", async () => { - keys.fetchByJti.mockResolvedValue(record); - expect((await revokeApiKey(IDLE, form({ account_id: "acme--nightly-sync", jti: "j1" }))).success).toBe(true); - expect(keys.set).toHaveBeenCalledWith("j1", "revoked_at", expect.any(String)); + keys.listByAccount.mockResolvedValue([record]); + expect((await revokeApiKey(IDLE, form({ account_id: "acme--nightly-sync", key_id: "k1" }))).success).toBe(true); + expect(keys.set).toHaveBeenCalledWith("h1", "revoked_at", expect.any(String)); - keys.fetchByJti.mockResolvedValue({ ...(record as object), account_id: "other" } as never); - expect((await revokeApiKey(IDLE, form({ account_id: "acme--nightly-sync", jti: "j1" }))).success).toBe(false); + // A key the account does not hold is not found, whatever id is given. + keys.listByAccount.mockResolvedValue([]); + expect((await revokeApiKey(IDLE, form({ account_id: "acme--nightly-sync", key_id: "k1" }))).success).toBe(false); }); it("changes expiry after issuance, including to never", async () => { - keys.fetchByJti.mockResolvedValue(record); - await setApiKeyExpiry(IDLE, form({ account_id: "acme--nightly-sync", jti: "j1", expires_in_days: "30" })); - expect(keys.set).toHaveBeenCalledWith("j1", "expires_at", expect.any(String)); - await setApiKeyExpiry(IDLE, form({ account_id: "acme--nightly-sync", jti: "j1", expires_in_days: "" })); - expect(keys.set).toHaveBeenLastCalledWith("j1", "expires_at", null); + keys.listByAccount.mockResolvedValue([record]); + await setApiKeyExpiry(IDLE, form({ account_id: "acme--nightly-sync", key_id: "k1", expires_in_days: "30" })); + expect(keys.set).toHaveBeenCalledWith("h1", "expires_at", expect.any(String)); + await setApiKeyExpiry(IDLE, form({ account_id: "acme--nightly-sync", key_id: "k1", expires_in_days: "" })); + expect(keys.set).toHaveBeenLastCalledWith("h1", "expires_at", null); }); }); diff --git a/src/lib/actions/service-account-keys.ts b/src/lib/actions/service-account-keys.ts index 49c4a6b2..66a0920d 100644 --- a/src/lib/actions/service-account-keys.ts +++ b/src/lib/actions/service-account-keys.ts @@ -1,17 +1,18 @@ "use server"; import { revalidatePath } from "next/cache"; -import { randomUUID } from "crypto"; +import { createHash, randomBytes, randomUUID } from "crypto"; import { LOGGER } from "@/lib/logging"; import { - ServiceAccountKeySchema, + API_KEY_PREFIX, + publicKey, + ServiceAccountKeyRecordSchema, type ApiKeyActionState, - type ServiceAccountKey, + type ServiceAccountKeyRecord, } from "@/types"; import { getPageSession } from "../api/utils"; import { serviceAccountKeysTable } from "../clients"; import { managedServiceAccount } from "@/lib/accounts/service-accounts"; -import { mintApiKey } from "@/lib/services/proxy-keys"; import { editAccountServiceAccountsUrl, editServiceAccountUrl } from "@/lib/urls"; const outcome = (message: string, success: boolean): ApiKeyActionState => ({ @@ -28,10 +29,13 @@ function expiryFrom(formData: FormData): string | null | undefined { return new Date(Date.now() + days * 86_400_000).toISOString(); } +/** Hex SHA-256 of a key: what the record holds, and what the proxy presents. */ +const hashApiKey = (key: string) => createHash("sha256").update(key).digest("hex"); + /** - * Issues an API key: records it here, has the data proxy sign it, and returns - * the key once. The key's subject is the service account's own id, which the - * API resolves directly (ADR-014); nothing else is written for it. + * Issues an API key: an opaque secret (ADR-013) whose hash is the record's + * key. Nothing signs it and nothing but this response ever carries it. The + * proxy resolves it to this service account by asking the API. */ export async function issueApiKey( _prev: ApiKeyActionState, @@ -39,7 +43,7 @@ export async function issueApiKey( ): Promise { const session = await getPageSession(); const account = await managedServiceAccount(session, String(formData.get("account_id") ?? "")); - if (!account || !session?.identity_id || !session.account) { + if (!account || !session?.account) { return outcome("You do not manage that service account", false); } // Disabled means frozen: no new key until it is enabled again. @@ -47,9 +51,12 @@ export async function issueApiKey( const expires_at = expiryFrom(formData); if (expires_at === undefined) return outcome("Expiry must be between 1 and 3650 days", false); + // 32 random bytes in base64url: fixed length, no bias, all entropy. + const key = API_KEY_PREFIX + randomBytes(32).toString("base64url"); const now = new Date().toISOString(); - const parsed = ServiceAccountKeySchema.safeParse({ - jti: randomUUID(), + const parsed = ServiceAccountKeyRecordSchema.safeParse({ + key_hash: hashApiKey(key), + key_id: randomUUID(), account_id: account.account_id, label: String(formData.get("label") ?? "").trim(), created_at: now, @@ -57,42 +64,28 @@ export async function issueApiKey( expires_at, }); if (!parsed.success) return outcome("Give the key a label of up to 64 characters", false); - const record: ServiceAccountKey = parsed.data; + const record: ServiceAccountKeyRecord = parsed.data; await serviceAccountKeysTable.create(record); - let key: string; - try { - key = await mintApiKey(session.identity_id, { - account_id: account.account_id, - jti: record.jti, - expires_at: record.expires_at, - }); - } catch (error) { - // No record without a key: the row would look like a live credential. - await serviceAccountKeysTable.delete(record.jti); - LOGGER.error("API key minting failed", { - operation: "issueApiKey", - metadata: { account_id: account.account_id, jti: record.jti }, - error: error instanceof Error ? error : new Error(String(error)), - }); - return outcome("The data proxy could not sign the key. Try again.", false); - } - LOGGER.info("Issued API key", { operation: "issueApiKey", - metadata: { account_id: account.account_id, jti: record.jti, expires_at }, + metadata: { account_id: account.account_id, key_id: record.key_id, expires_at }, }); revalidatePath(editAccountServiceAccountsUrl(account.owner_account_id)); revalidatePath(editServiceAccountUrl(account.owner_account_id, account.account_id)); - return { ...outcome("", true), issued: { key, record } }; + return { ...outcome("", true), issued: { key, record: publicKey(record) } }; } +/** The key `key_id` names, if it belongs to a service account the caller manages. */ async function ownKey(formData: FormData) { const session = await getPageSession(); const account = await managedServiceAccount(session, String(formData.get("account_id") ?? "")); if (!account) return null; - const key = await serviceAccountKeysTable.fetchByJti(String(formData.get("jti") ?? "")); - return key && key.account_id === account.account_id ? { account, key } : null; + const key_id = String(formData.get("key_id") ?? ""); + const key = (await serviceAccountKeysTable.listByAccount(account.account_id)).find( + (k) => k.key_id === key_id + ); + return key ? { account, key } : null; } /** Revocation takes effect for new exchanges within the proxy's cache TTL. */ @@ -103,7 +96,7 @@ export async function revokeApiKey( const own = await ownKey(formData); if (!own) return outcome("No such key on a service account you manage", false); if (own.key.revoked_at) return outcome("Already revoked", false); - await serviceAccountKeysTable.set(own.key.jti, "revoked_at", new Date().toISOString()); + await serviceAccountKeysTable.set(own.key.key_hash, "revoked_at", new Date().toISOString()); revalidatePath(editAccountServiceAccountsUrl(own.account.owner_account_id)); revalidatePath(editServiceAccountUrl(own.account.owner_account_id, own.account.account_id)); return outcome("Key revoked", true); @@ -118,7 +111,7 @@ export async function setApiKeyExpiry( if (!own) return outcome("No such key on a service account you manage", false); const expires_at = expiryFrom(formData); if (expires_at === undefined) return outcome("Expiry must be between 1 and 3650 days", false); - await serviceAccountKeysTable.set(own.key.jti, "expires_at", expires_at); + await serviceAccountKeysTable.set(own.key.key_hash, "expires_at", expires_at); revalidatePath(editAccountServiceAccountsUrl(own.account.owner_account_id)); revalidatePath(editServiceAccountUrl(own.account.owner_account_id, own.account.account_id)); return outcome(expires_at ? "Expiry updated" : "Key no longer expires", true); diff --git a/src/lib/api/oidc.test.ts b/src/lib/api/oidc.test.ts index 7120f51f..d17c0d11 100644 --- a/src/lib/api/oidc.test.ts +++ b/src/lib/api/oidc.test.ts @@ -10,7 +10,12 @@ import { type FlattenedJWSInput, type JWSHeaderParameters, } from "jose"; -import { authenticateWithOidcToken, _setJwks } from "./oidc"; +import { + authenticateWithOidcToken, + verifyProxyAssertion, + PROXY_SELF_SUBJECT, + _setJwks, +} from "./oidc"; // Generate a test RSA key pair let privateKey: CryptoKey; @@ -316,4 +321,43 @@ describe("authenticateWithOidcToken", () => { const token = await createToken({ sub: "alice" }); expect(await authenticateWithOidcToken(`Bearer ${token}`, AUDIENCE)).toBeNull(); }); + + test("never makes a session for the proxy's own subject, even if an account had that id", async () => { + (accountsTable.fetchByOryId as jest.Mock).mockResolvedValue(null); + (accountsTable.fetchById as jest.Mock).mockResolvedValue({ + account_id: PROXY_SELF_SUBJECT, + type: "service", + owner_account_id: "acme", + disabled: false, + flags: [], + }); + + const token = await createToken({ sub: PROXY_SELF_SUBJECT }); + expect(await authenticateWithOidcToken(`Bearer ${token}`, AUDIENCE)).toBeNull(); + expect(accountsTable.fetchById).not.toHaveBeenCalled(); + }); +}); + +describe("verifyProxyAssertion", () => { + beforeEach(() => { + _setJwks(async () => importJWK(publicJwk, "RS256")); + }); + afterAll(() => _setJwks(null)); + + test("returns the claims of a valid assertion without resolving anyone", async () => { + const token = await createToken({ sub: PROXY_SELF_SUBJECT }); + const payload = await verifyProxyAssertion(`Bearer ${token}`, AUDIENCE); + expect(payload?.sub).toBe(PROXY_SELF_SUBJECT); + expect(payload?.iss).toBe(ISSUER); + expect(accountsTable.fetchById).not.toHaveBeenCalled(); + expect(accountsTable.fetchByOryId).not.toHaveBeenCalled(); + }); + + test("returns null for a wrong issuer, a wrong audience, an expired token, or no token", async () => { + expect(await verifyProxyAssertion(`Bearer ${await createToken({ sub: "x" }, { issuer: "https://evil" })}`, AUDIENCE)).toBeNull(); + expect(await verifyProxyAssertion(`Bearer ${await createToken({ sub: "x" }, { audience: "https://other" })}`, AUDIENCE)).toBeNull(); + expect(await verifyProxyAssertion(`Bearer ${await createToken({ sub: "x" }, { expiresIn: "-1m" })}`, AUDIENCE)).toBeNull(); + expect(await verifyProxyAssertion(null, AUDIENCE)).toBeNull(); + expect(await verifyProxyAssertion("Basic abc", AUDIENCE)).toBeNull(); + }); }); diff --git a/src/lib/api/oidc.ts b/src/lib/api/oidc.ts index 2d501fb7..292a77bc 100644 --- a/src/lib/api/oidc.ts +++ b/src/lib/api/oidc.ts @@ -3,6 +3,7 @@ import { createRemoteJWKSet, decodeJwt, decodeProtectedHeader, + type JWTPayload, } from "jose"; import { CONFIG } from "@/lib/config"; import { accountsTable, membershipsTable } from "@/lib/clients/database"; @@ -59,31 +60,37 @@ async function serviceAccountById(account_id: string) { } /** - * Authenticates using a signed JWT from the data proxy's OIDC provider. - * Validates the token signature, issuer, audience, and expiry, then - * resolves the subject claim to a UserSession. + * The subject the data proxy signs with when it calls as itself rather than + * on behalf of an account (ADR-013, amending ADR-005): only the API-key + * standing lookup accepts it. A URN, so no account id can ever equal it. */ -export async function authenticateWithOidcToken( +export const PROXY_SELF_SUBJECT = "urn:source:data-proxy"; + +/** + * Verifies a proxy-signed assertion — signature against the proxy's JWKS, + * issuer, audience, RS256, expiry — and returns its claims, or null. Says + * nothing about who the subject is; callers decide what a subject may do. + */ +export async function verifyProxyAssertion( authorization: string | null, audience: string, -): Promise { +): Promise { if (!authorization || !authorization.toLowerCase().startsWith("bearer ")) { // A missing or non-Bearer Authorization header is an expected, normal input // (e.g. legacy clients still sending an API key), not an anomaly — log at // debug so it doesn't flood warn-level logs on every such request. LOGGER.debug("No Bearer token for OIDC authentication, skipping", { - operation: "authenticateWithOidcToken", + operation: "verifyProxyAssertion", }); return null; } LOGGER.debug("Authenticating with OIDC token", { - operation: "authenticateWithOidcToken", + operation: "verifyProxyAssertion", metadata: { audience }, }); const token = authorization.slice(7); - let payload; try { const result = await jwtVerify(token, getJwks(), { // We assume that the data proxy is going to be hosting the JWKS @@ -95,7 +102,7 @@ export async function authenticateWithOidcToken( algorithms: ["RS256"], clockTolerance: 30, }); - payload = result.payload; + return result.payload; } catch (error) { // `Error` objects serialize to `{}`, hiding the cause. jose attaches a // machine-readable `code` (e.g. ERR_JWT_CLAIM_VALIDATION_FAILED, @@ -125,7 +132,7 @@ export async function authenticateWithOidcToken( // Leave the failure marker. } const logPayload = { - operation: "authenticateWithOidcToken", + operation: "verifyProxyAssertion", metadata: { error_name: e?.name, error_code: e?.code, @@ -145,6 +152,19 @@ export async function authenticateWithOidcToken( } return null; } +} + +/** + * Authenticates using a signed JWT from the data proxy's OIDC provider. + * Validates the token signature, issuer, audience, and expiry, then + * resolves the subject claim to a UserSession. + */ +export async function authenticateWithOidcToken( + authorization: string | null, + audience: string, +): Promise { + const payload = await verifyProxyAssertion(authorization, audience); + if (!payload) return null; const oryId = payload.sub; if (!oryId) { @@ -154,8 +174,17 @@ export async function authenticateWithOidcToken( return null; } + // The proxy calling as itself is not a session anywhere; the one route that + // takes it checks the subject directly. + if (oryId === PROXY_SELF_SUBJECT) { + LOGGER.warn("OIDC token carries the proxy's own subject; no session for it", { + operation: "authenticateWithOidcToken", + }); + return null; + } + // The token subject is whatever the data proxy authenticated: a person's Ory - // identity id, or a service account's own id — for an API key it signed + // identity id, or a service account's own id — for an API key it resolved // (ADR-013), or a workload the account trusts (ADR-014), which names the // account it wants when it exchanges its token. The two namespaces are read // together; should a subject ever name both a person and a service account, diff --git a/src/lib/clients/database/service-account-keys.ts b/src/lib/clients/database/service-account-keys.ts index a93d4cf8..5f9d3ffc 100644 --- a/src/lib/clients/database/service-account-keys.ts +++ b/src/lib/clients/database/service-account-keys.ts @@ -1,24 +1,19 @@ -import { - DeleteCommand, - GetCommand, - PutCommand, - QueryCommand, - UpdateCommand, -} from "@aws-sdk/lib-dynamodb"; -import type { ServiceAccountKey } from "@/types"; +import { GetCommand, PutCommand, QueryCommand, UpdateCommand } from "@aws-sdk/lib-dynamodb"; +import type { ServiceAccountKeyRecord } from "@/types"; import { BaseTable } from "./base"; export class ServiceAccountKeysTable extends BaseTable { model = "service-account-keys"; - async fetchByJti(jti: string): Promise { + /** The exchange path: one key get by the hash the proxy presents. */ + async fetchByHash(key_hash: string): Promise { const result = await this.cachedSend( - new GetCommand({ TableName: this.table, Key: { jti } }) + new GetCommand({ TableName: this.table, Key: { key_hash } }) ); - return (result.Item as ServiceAccountKey | undefined) ?? null; + return (result.Item as ServiceAccountKeyRecord | undefined) ?? null; } - async listByAccount(account_id: string): Promise { + async listByAccount(account_id: string): Promise { const result = await this.cachedSend( new QueryCommand({ TableName: this.table, @@ -27,15 +22,15 @@ export class ServiceAccountKeysTable extends BaseTable { ExpressionAttributeValues: { ":account_id": account_id }, }) ); - return (result.Items ?? []) as ServiceAccountKey[]; + return (result.Items ?? []) as ServiceAccountKeyRecord[]; } - async create(key: ServiceAccountKey): Promise { + async create(key: ServiceAccountKeyRecord): Promise { await this.client.send( new PutCommand({ TableName: this.table, Item: key, - ConditionExpression: "attribute_not_exists(jti)", + ConditionExpression: "attribute_not_exists(key_hash)", }) ); return key; @@ -47,25 +42,21 @@ export class ServiceAccountKeysTable extends BaseTable { * Only an existing record is written: nothing may resurrect a deleted key. */ async set( - jti: string, + key_hash: string, field: "revoked_at" | "expires_at" | "last_used_at", value: string | null ): Promise { await this.client.send( new UpdateCommand({ TableName: this.table, - Key: { jti }, + Key: { key_hash }, UpdateExpression: "SET #f = :v", ExpressionAttributeNames: { "#f": field }, ExpressionAttributeValues: { ":v": value }, - ConditionExpression: "attribute_exists(jti)", + ConditionExpression: "attribute_exists(key_hash)", }) ); } - - async delete(jti: string): Promise { - await this.client.send(new DeleteCommand({ TableName: this.table, Key: { jti } })); - } } export const serviceAccountKeysTable = new ServiceAccountKeysTable({}); diff --git a/src/lib/services/proxy-keys.ts b/src/lib/services/proxy-keys.ts deleted file mode 100644 index 4906c6f0..00000000 --- a/src/lib/services/proxy-keys.ts +++ /dev/null @@ -1,38 +0,0 @@ -import "server-only"; - -import { CONFIG } from "@/lib/config"; -import { getOryIdToken } from "@/lib/actions/proxy-credentials"; -import { API_KEY_PREFIX } from "@/types"; - -/** - * Asks the data proxy to sign an API key for `account_id` under `jti` — the - * proxy holds the signing key (ADR-013), this app holds the record. The call - * is made as `identityId`, whoever is issuing the key; the proxy checks with - * this API that they manage the account before it signs anything. - * - * SECURITY: `identityId` is trusted as-is, exactly as in getProxyCredentials. - * Pass an identity from a verified session, never from request input. - */ -export async function mintApiKey( - identityId: string, - key: { account_id: string; jti: string; expires_at: string | null } -): Promise { - const idToken = await getOryIdToken(identityId); - const resp = await fetch(`${CONFIG.storage.endpoint}/.keys`, { - method: "POST", - headers: { - authorization: `Bearer ${idToken}`, - "content-type": "application/json", - }, - body: JSON.stringify(key), - signal: AbortSignal.timeout(15_000), - }); - if (!resp.ok) { - throw new Error(`Key minting failed: ${resp.status}`); - } - const { key: minted } = (await resp.json()) as { key?: unknown }; - if (typeof minted !== "string" || !minted.startsWith(API_KEY_PREFIX)) { - throw new Error("Key minting returned no key"); - } - return minted; -} diff --git a/src/types/service-account-key.ts b/src/types/service-account-key.ts index 2622809b..28b98318 100644 --- a/src/types/service-account-key.ts +++ b/src/types/service-account-key.ts @@ -4,13 +4,13 @@ import { z } from "zod"; extendZodWithOpenApi(z); /** - * The record behind an API key. The key itself — a JWT the data proxy signs - * with this record's `jti` as its id (ADR-013) — is shown once at issue and - * never stored; this row is what revocation and expiry act on. + * What the app and the UI see of an API key: everything but the hash. The + * key itself is an opaque secret (ADR-013) shown once at issue and never + * stored; `key_id` is its public handle for listing, revoking and expiry. */ export const ServiceAccountKeySchema = z .object({ - jti: z.string().uuid(), + key_id: z.string().uuid(), account_id: z.string(), label: z.string().min(1).max(64), created_at: z.string().datetime(), @@ -25,11 +25,25 @@ export const ServiceAccountKeySchema = z export type ServiceAccountKey = z.infer; /** - * Every key starts with this. A JWT always begins `eyJ`, so a leaked key - * matches `sck_eyJ[\w-]+\.[\w-]+\.[\w-]+` — the pattern to register with - * secret scanners. The proxy strips the prefix before verifying. + * The stored row: the public fields plus `key_hash`, the table's partition + * key — hex SHA-256 of the key — which the data proxy presents to ask whether + * a key may be exchanged. It never leaves the server; pages strip it with + * `publicKey` before handing a record to a client component. + */ +export const ServiceAccountKeyRecordSchema = ServiceAccountKeySchema.extend({ + key_hash: z.string().regex(/^[0-9a-f]{64}$/), +}); + +export type ServiceAccountKeyRecord = z.infer; + +export const publicKey = ({ key_hash: _, ...key }: ServiceAccountKeyRecord): ServiceAccountKey => key; + +/** + * Every key is `sck_` + 32 random bytes in base64url: a fixed 47 characters, + * all entropy after the prefix. The pattern is what secret scanners register. */ export const API_KEY_PREFIX = "sck_"; +export const API_KEY_PATTERN = /^sck_[A-Za-z0-9_-]{43}$/; /** Whether a key may still be exchanged: not revoked, and not past its expiry. */ export const isKeyActive = (key: ServiceAccountKey, now = Date.now()): boolean =>