feat(accounts): resolve accounts by (issuer, subject)

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);
     }

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,

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(

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>): 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<string, unknown> }) => {
+    const values = command.input.ExpressionAttributeValues as Record<string, string> | 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<string, unknown> }) => {
+    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<string, string>;
+    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);
+  });
+});

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<typeof BaseTable>[0] = {}) {
+    super(base);
+    this.bindings = bindings ?? identityBindingsTable;
+  }
 
   async fetchById(account_id: string): Promise<Account | null> {
     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<Account | null> {
+    const binding = await this.bindings.resolve(issuer, subject);
+    return binding ? this.fetchById(binding.account_id) : null;
+  }
+
   async fetchByOryId(identity_id: string): Promise<IndividualAccount | null> {
     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<void> {
+  /**
+   * 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<void> {
+    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 } } },
+        ],
       })
     );
   }

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);
+  });
+});