diff --git a/deploy/lib/api-stack.ts b/deploy/lib/api-stack.ts index 414d818d0..bff342ec2 100644 --- a/deploy/lib/api-stack.ts +++ b/deploy/lib/api-stack.ts @@ -55,6 +55,7 @@ export class ApiStack extends cdk.Stack { database.dataConnectionsTable, database.apiKeysTable, database.membershipsTable, + database.identityBindingsTable, ]) { table.grantReadWriteData(vercel.vercelRole); } diff --git a/deploy/lib/database-construct.ts b/deploy/lib/database-construct.ts index 01ad92046..2e88866b7 100644 --- a/deploy/lib/database-construct.ts +++ b/deploy/lib/database-construct.ts @@ -26,6 +26,7 @@ export class DatabaseConstruct extends Construct { public readonly dataConnectionsTable: dynamodb.Table; public readonly apiKeysTable: dynamodb.Table; public readonly membershipsTable: dynamodb.Table; + public readonly identityBindingsTable: dynamodb.Table; constructor( scope: Construct, @@ -60,6 +61,23 @@ export class DatabaseConstruct extends Construct { removalPolicy, }); + this.identityBindingsTable = this.createTable({ + name: "identity-bindings", + stage, + // (issuer, subject) is the key, so a subject binds to one account per + // issuer and nothing more. + partitionKey: "issuer", + sortKey: "subject", + indexes: [ + { + // fetch the bindings of an account + name: "account_id", + partitionKey: "account_id", + }, + ], + removalPolicy, + }); + this.apiKeysTable = this.createTable({ name: "api-keys", stage, diff --git a/scripts/init-local.ts b/scripts/init-local.ts index d078945f2..c0b74866f 100644 --- a/scripts/init-local.ts +++ b/scripts/init-local.ts @@ -49,7 +49,8 @@ async function tablesExist() { tables.TableNames?.includes(getTableName("products")) && tables.TableNames?.includes(getTableName("api-keys")) && tables.TableNames?.includes(getTableName("data-connections")) && - tables.TableNames?.includes(getTableName("memberships")) + tables.TableNames?.includes(getTableName("memberships")) && + tables.TableNames?.includes(getTableName("identity-bindings")) ); } @@ -77,6 +78,7 @@ async function createTables() { await deleteTable(getTableName("api-keys")); await deleteTable(getTableName("data-connections")); await deleteTable(getTableName("memberships")); + await deleteTable(getTableName("identity-bindings")); // Wait for tables to be fully deleted await new Promise((resolve) => setTimeout(resolve, 2000)); @@ -143,6 +145,45 @@ async function createTables() { throw e; } + // Create identity-bindings table + try { + await client.send( + new CreateTableCommand({ + TableName: getTableName("identity-bindings"), + AttributeDefinitions: [ + { AttributeName: "issuer", AttributeType: "S" }, + { AttributeName: "subject", AttributeType: "S" }, + { AttributeName: "account_id", AttributeType: "S" }, + ], + KeySchema: [ + { AttributeName: "issuer", KeyType: "HASH" }, + { AttributeName: "subject", KeyType: "RANGE" }, + ], + GlobalSecondaryIndexes: [ + { + IndexName: "account_id", + KeySchema: [{ AttributeName: "account_id", KeyType: "HASH" }], + Projection: { + ProjectionType: "ALL", + }, + ProvisionedThroughput: { + ReadCapacityUnits: 5, + WriteCapacityUnits: 5, + }, + }, + ], + ProvisionedThroughput: { + ReadCapacityUnits: 5, + WriteCapacityUnits: 5, + }, + }) + ); + console.log(`✓ Created ${getTableName("identity-bindings")} table`); + } catch (e) { + console.error(`✗ Error creating ${getTableName("identity-bindings")} table:`, e); + throw e; + } + // Create api-keys table try { await client.send( diff --git a/src/lib/clients/database/accounts.identity.test.ts b/src/lib/clients/database/accounts.identity.test.ts new file mode 100644 index 000000000..7426c2fdc --- /dev/null +++ b/src/lib/clients/database/accounts.identity.test.ts @@ -0,0 +1,118 @@ +import { type DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb"; +import { AccountsTable } from "./accounts"; +import { IdentityBindingsTable } from "./identity-bindings"; +import { createMemoizedRead } from "./request-cache"; +import { fakeReactCache } from "./__test-helpers__/fake-react-cache"; +import { AccountType, type Account, type IdentityBinding } from "@/types"; + +jest.mock("@/lib/config", () => ({ + CONFIG: { environment: { stage: "test" }, database: {} }, +})); +jest.mock("@/lib/logging", () => ({ + LOGGER: { error: jest.fn(), warn: jest.fn(), debug: jest.fn(), info: jest.fn() }, +})); + +const account = (fields: Partial): Account => + ({ type: AccountType.INDIVIDUAL, disabled: false, ...fields }) as Account; + +const jane = account({ account_id: "jane-doe", identity_id: "ory-jane" }); +const acme = account({ account_id: "acme", type: AccountType.ORGANIZATION }); +const bot = account({ account_id: "acme-bot", type: AccountType.SERVICE }); + +/** Answers the key query and the identity_id index query from `items`; records writes. */ +function fakeAccounts(items: Account[]) { + const send = jest.fn(async (command: { input: Record }) => { + const values = command.input.ExpressionAttributeValues as Record | undefined; + if (!values) return {}; // a write + if (values[":account_id"] !== undefined) { + return { Items: items.filter((a) => a.account_id === values[":account_id"]) }; + } + return { Items: items.filter((a) => a.identity_id === values[":identity_id"]) }; + }); + return { send, client: { send } as unknown as DynamoDBDocumentClient }; +} + +/** Answers Get by key and Query on the account_id index over `bindings`. */ +function fakeBindings(bindings: IdentityBinding[]) { + const send = jest.fn(async (command: { input: Record }) => { + const key = command.input.Key as { issuer: string; subject: string } | undefined; + if (key) return { Item: bindings.find((b) => b.issuer === key.issuer && b.subject === key.subject) }; + const values = command.input.ExpressionAttributeValues as Record; + return { Items: bindings.filter((b) => b.account_id === values[":account_id"]) }; + }); + const table = new IdentityBindingsTable({ + client: { send } as unknown as DynamoDBDocumentClient, + memoizedRead: createMemoizedRead(fakeReactCache), + }); + return { table, send }; +} + +function tableFor(items: Account[], bindings: IdentityBinding[]) { + const accounts = fakeAccounts(items); + const bound = fakeBindings(bindings); + const table = new AccountsTable({ + client: accounts.client, + memoizedRead: createMemoizedRead(fakeReactCache), + bindings: bound.table, + }); + return { table, send: accounts.send, bindingsSend: bound.send }; +} + +const CI = "https://ci.example"; +const PROXY = "https://data.example"; + +const bound = (issuer: string, subject: string, account_id: string) => + ({ issuer, subject, account_id, created_at: "2024-01-01T00:00:00Z" }) as IdentityBinding; + +describe("AccountsTable identity resolution", () => { + beforeEach(() => jest.clearAllMocks()); + + it("resolves an Ory identity through the identity_id index, never the bindings table", async () => { + // Ory identities are not bindings: a person's identity_id is on the row, + // and the session, the email lookup and the proxy credentials read it there. + const { table, bindingsSend } = tableFor([jane], [bound(PROXY, "ory-jane", "acme-bot")]); + + expect(await table.fetchByOryId("ory-jane")).toEqual(jane); + expect(await table.fetchByOryId("ory-nobody")).toBeNull(); + expect(bindingsSend).not.toHaveBeenCalled(); + }); + + it("resolves any account type through a binding, while fetchByOryId stays individual-only", async () => { + const bindings = [bound(CI, "ci-subject", "acme"), bound(PROXY, "acme-bot", "acme-bot")]; + const { table } = tableFor([acme, bot], bindings); + + expect(await table.fetchByIdentity(CI, "ci-subject")).toEqual(acme); + expect(await table.fetchByIdentity(PROXY, "acme-bot")).toEqual(bot); + // A bound subject is not an Ory identity; nothing on the row carries it. + expect(await table.fetchByOryId("ci-subject")).toBeNull(); + expect(await table.fetchByIdentity(CI, "unknown")).toBeNull(); + }); + + it("removes an account and its bindings in one transaction, keyed as the tables are", async () => { + // Otherwise an orphaned pair would keep that subject from ever binding + // again — and a half-done cascade would leave an account some of whose + // sign-in paths are gone. + const bindings = [bound(PROXY, "acme-bot", "acme-bot"), bound(CI, "ci", "acme-bot")]; + const { table, send, bindingsSend } = tableFor([bot], bindings); + + await table.delete("acme-bot"); + + // The list is read fresh, not from the request cache. + expect(bindingsSend.mock.calls[0][0].constructor.name).toBe("QueryCommand"); + const write = send.mock.calls.find((c) => c[0].constructor.name === "TransactWriteCommand")!; + expect(write[0].input.TransactItems).toEqual([ + { Delete: { TableName: "sc-test-identity-bindings", Key: { issuer: PROXY, subject: "acme-bot" } } }, + { Delete: { TableName: "sc-test-identity-bindings", Key: { issuer: CI, subject: "ci" } } }, + { Delete: { TableName: "sc-test-accounts", Key: { account_id: "acme-bot" } } }, + ]); + }); + + it("tells issuers apart", async () => { + const { table } = tableFor( + [acme, bot], + [bound(CI, "shared", "acme"), bound(PROXY, "shared", "acme-bot")] + ); + expect(await table.fetchByIdentity(CI, "shared")).toEqual(acme); + expect(await table.fetchByIdentity(PROXY, "shared")).toEqual(bot); + }); +}); diff --git a/src/lib/clients/database/accounts.ts b/src/lib/clients/database/accounts.ts index dc3096587..2f00a2e51 100644 --- a/src/lib/clients/database/accounts.ts +++ b/src/lib/clients/database/accounts.ts @@ -4,7 +4,7 @@ import { ScanCommand, type ScanCommandOutput, UpdateCommand, - DeleteCommand, + TransactWriteCommand, PutCommand, BatchGetCommand, } from "@aws-sdk/lib-dynamodb"; @@ -17,6 +17,7 @@ import { } from "@/types"; import { BaseTable } from "./base"; +import { IdentityBindingsTable, identityBindingsTable } from "./identity-bindings"; import { LOGGER } from "@/lib/logging"; /** @@ -33,6 +34,17 @@ export interface AccountSuggestion { export class AccountsTable extends BaseTable { model = "accounts"; + private readonly bindings: IdentityBindingsTable; + + constructor({ + bindings, + ...base + }: { + bindings?: IdentityBindingsTable; + } & ConstructorParameters[0] = {}) { + super(base); + this.bindings = bindings ?? identityBindingsTable; + } async fetchById(account_id: string): Promise { try { @@ -107,6 +119,19 @@ export class AccountsTable extends BaseTable { return accountBatches; } + /** + * The account `subject` resolves to, as named by `issuer` — for identities + * the platform does not own. An Ory identity is `fetchByOryId`'s to resolve, + * from the row itself. + */ + async fetchByIdentity( + issuer: string, + subject: string + ): Promise { + const binding = await this.bindings.resolve(issuer, subject); + return binding ? this.fetchById(binding.account_id) : null; + } + async fetchByOryId(identity_id: string): Promise { try { LOGGER.debug(`Trying to fetch account by Ory ID`, { @@ -319,11 +344,26 @@ export class AccountsTable extends BaseTable { ) as ServiceAccount[]; } - async delete(Key: { account_id: string; type: AccountType }): Promise { + /** + * Removes the account and its bindings together. An orphaned binding would + * keep its subject from ever binding again, and a cascade that stopped + * half-way would leave an account some of whose sign-in paths are gone, so + * the deletes are one transaction. The bindings are read fresh: a memoized + * list from earlier in the request could predate one of them. + */ + async delete(account_id: string): Promise { + const bindings = await this.bindings.listByAccount(account_id, true); await this.client.send( - new DeleteCommand({ - TableName: this.table, - Key, + new TransactWriteCommand({ + TransactItems: [ + ...bindings.map((b) => ({ + Delete: { + TableName: this.bindings.table, + Key: { issuer: b.issuer, subject: b.subject }, + }, + })), + { Delete: { TableName: this.table, Key: { account_id } } }, + ], }) ); } diff --git a/src/lib/clients/database/identity-bindings.test.ts b/src/lib/clients/database/identity-bindings.test.ts new file mode 100644 index 000000000..316fdcf64 --- /dev/null +++ b/src/lib/clients/database/identity-bindings.test.ts @@ -0,0 +1,83 @@ +import { type DynamoDBDocumentClient } from "@aws-sdk/lib-dynamodb"; +import { IdentityAlreadyBoundError, IdentityBindingsTable } from "./identity-bindings"; +import { createMemoizedRead } from "./request-cache"; +import { fakeReactCache } from "./__test-helpers__/fake-react-cache"; + +jest.mock("@/lib/config", () => ({ + CONFIG: { environment: { stage: "test" }, database: {} }, +})); +jest.mock("@/lib/logging", () => ({ + LOGGER: { error: jest.fn(), warn: jest.fn(), debug: jest.fn(), info: jest.fn() }, +})); + +function tableWith(send: jest.Mock) { + return new IdentityBindingsTable({ + client: { send } as unknown as DynamoDBDocumentClient, + memoizedRead: createMemoizedRead(fakeReactCache), + }); +} + +const binding = { + issuer: "https://ci.example", + subject: "repo:o/r:ref:refs/heads/main", + account_id: "o-bot", + created_at: "2024-01-01T00:00:00Z", +}; + +describe("IdentityBindingsTable", () => { + it("writes a binding only if the pair is not already bound", async () => { + const send = jest.fn(async () => ({})); + await tableWith(send).create(binding); + + const input = send.mock.calls[0][0].input; + expect(input.TableName).toBe("sc-test-identity-bindings"); + expect(input.Item).toEqual(binding); + expect(input.ConditionExpression).toBe("attribute_not_exists(issuer)"); + }); + + it("names a conflict on the pair for what it is", async () => { + const send = jest.fn(async () => { + const error = new Error("The conditional request failed") as Error & { name: string }; + error.name = "ConditionalCheckFailedException"; + throw error; + }); + await expect(tableWith(send).create(binding)).rejects.toBeInstanceOf(IdentityAlreadyBoundError); + }); + + it("resolves by the (issuer, subject) key", async () => { + const send = jest.fn(async () => ({ Item: binding })); + const table = tableWith(send); + + expect(await table.resolve(binding.issuer, binding.subject)).toEqual(binding); + expect(send.mock.calls[0][0].input.Key).toEqual({ + issuer: binding.issuer, + subject: binding.subject, + }); + expect(await tableWith(jest.fn(async () => ({}))).resolve("x", "y")).toBeNull(); + }); + + it("refuses a malformed row before touching the table", async () => { + // An issuer read from unset config is "", and every binding written under + // it would share one partition; an empty account_id cannot be indexed. + const send = jest.fn(async () => ({})); + const table = tableWith(send); + await expect(table.create({ ...binding, issuer: "" })).rejects.toThrow(); + await expect(table.create({ ...binding, subject: "" })).rejects.toThrow(); + await expect(table.create({ ...binding, account_id: "" })).rejects.toThrow(); + await expect(table.create({ ...binding, created_at: "yesterday" })).rejects.toThrow(); + expect(send).not.toHaveBeenCalled(); + }); + + it("lists an account's bindings from the request cache unless told not to", async () => { + const send = jest.fn(async () => ({ Items: [binding] })); + const table = tableWith(send); + + expect(await table.listByAccount("o-bot")).toEqual([binding]); + expect(await table.listByAccount("o-bot")).toEqual([binding]); + expect(send).toHaveBeenCalledTimes(1); + + // A destructive caller reads what the table holds now. + expect(await table.listByAccount("o-bot", true)).toEqual([binding]); + expect(send).toHaveBeenCalledTimes(2); + }); +}); diff --git a/src/lib/clients/database/identity-bindings.ts b/src/lib/clients/database/identity-bindings.ts new file mode 100644 index 000000000..951dcab81 --- /dev/null +++ b/src/lib/clients/database/identity-bindings.ts @@ -0,0 +1,99 @@ +import { + DeleteCommand, + GetCommand, + PutCommand, + QueryCommand, +} from "@aws-sdk/lib-dynamodb"; +import { IdentityBindingSchema, type IdentityBinding } from "@/types"; +import { BaseTable } from "./base"; + +/** The (issuer, subject) pair already resolves to an account. */ +export class IdentityAlreadyBoundError extends Error { + constructor(issuer: string, subject: string) { + super(`Identity already bound: ${issuer} ${subject}`); + this.name = "IdentityAlreadyBoundError"; + } +} + +/** + * Subjects the platform cannot derive from an account — a GitHub Actions + * workflow, whatever platform IdP comes next — resolved to the account they + * belong to. An individual's Ory identity is not one of them: it stays on the + * account row as `identity_id`, where the session, the email lookup and the + * proxy credentials already read it. Nor is an API key's subject, which is the + * service account's own id and resolves by id. + */ +export class IdentityBindingsTable extends BaseTable { + model = "identity-bindings"; + + /** The binding for `subject` as named by `issuer`, or null. */ + async resolve( + issuer: string, + subject: string + ): Promise { + try { + const result = await this.cachedSend( + new GetCommand({ TableName: this.table, Key: { issuer, subject } }) + ); + return (result.Item as IdentityBinding | undefined) ?? null; + } catch (error) { + this.logError("resolve", error, { issuer, subject }); + throw error; + } + } + + /** + * An account's bindings. A destructive caller passes `bypassCache` so it + * acts on what the table holds now, not on a read memoized earlier in the + * request that may predate a binding. + */ + async listByAccount( + account_id: string, + bypassCache = false + ): Promise { + const command = new QueryCommand({ + TableName: this.table, + IndexName: "account_id", + KeyConditionExpression: "account_id = :account_id", + ExpressionAttributeValues: { ":account_id": account_id }, + }); + const result = bypassCache + ? await this.client.send(command) + : await this.cachedSend(command); + return (result.Items ?? []) as IdentityBinding[]; + } + + /** + * Binds the pair, or throws `IdentityAlreadyBoundError` if it is already + * bound — to this account or any other. The row is validated first: an + * issuer read from unset config would be `""`, and every binding written + * under it would share one partition. + */ + async create(binding: IdentityBinding): Promise { + const row = IdentityBindingSchema.parse(binding); + try { + await this.client.send( + new PutCommand({ + TableName: this.table, + Item: row, + ConditionExpression: "attribute_not_exists(issuer)", + }) + ); + } catch (error) { + if ((error as { name?: string })?.name === "ConditionalCheckFailedException") { + throw new IdentityAlreadyBoundError(row.issuer, row.subject); + } + this.logError("create", error, { account_id: row.account_id }); + throw error; + } + return row; + } + + async delete(issuer: string, subject: string): Promise { + await this.client.send( + new DeleteCommand({ TableName: this.table, Key: { issuer, subject } }) + ); + } +} + +export const identityBindingsTable = new IdentityBindingsTable({}); diff --git a/src/lib/clients/database/index.ts b/src/lib/clients/database/index.ts index 13c619e5c..7571ec44f 100644 --- a/src/lib/clients/database/index.ts +++ b/src/lib/clients/database/index.ts @@ -1,5 +1,6 @@ export * from "./accounts"; export * from "./api-keys"; export * from "./data-connections"; +export * from "./identity-bindings"; export * from "./memberships"; export * from "./products"; diff --git a/src/types/identity-binding.ts b/src/types/identity-binding.ts new file mode 100644 index 000000000..6e49a1c42 --- /dev/null +++ b/src/types/identity-binding.ts @@ -0,0 +1,20 @@ +import { extendZodWithOpenApi } from "@asteasolutions/zod-to-openapi"; +import { z } from "zod"; + +extendZodWithOpenApi(z); + +/** + * One way an account may authenticate: a subject as named by an issuer, and + * the account it resolves to. The pair is the key, so a subject binds to one + * account per issuer and nothing more. + */ +export const IdentityBindingSchema = z + .object({ + issuer: z.string().min(1).openapi({ example: "https://token.actions.githubusercontent.com" }), + subject: z.string().min(1).openapi({ example: "repo:acme/data:ref:refs/heads/main" }), + account_id: z.string().min(1).openapi({ example: "acme-nightly-sync" }), + created_at: z.string().datetime(), + }) + .openapi("IdentityBinding"); + +export type IdentityBinding = z.infer; diff --git a/src/types/index.ts b/src/types/index.ts index e372fd7d8..a4142c69b 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -1,5 +1,6 @@ export * from "./shared"; export * from "./account"; +export * from "./identity-binding"; export * from "./api-key"; export * from "./data-connection"; export * from "./membership";