fix(data-connections): stop listing other accounts' connections on product create

src/app/(app)/products/new/page.test.tsx

@@ -0,0 +1,118 @@
+import NewProductPage from "./page";
+import { accountsTable, membershipsTable, getPageSession } from "@/lib";
+import { listUsableDataConnections } from "@/lib/data-connections";
+import {
+  Account,
+  Membership,
+  MembershipRole,
+  MembershipState,
+  UserSession,
+} from "@/types";
+
+jest.mock("@/lib", () => ({
+  getPageSession: jest.fn(),
+  accountsTable: { fetchManyByIds: jest.fn() },
+  membershipsTable: { listByUser: jest.fn() },
+}));
+
+jest.mock("@/lib/data-connections", () => ({
+  listUsableDataConnections: jest.fn(),
+}));
+
+jest.mock("@/lib/api/authz", () => ({
+  isAuthorized: jest.fn(() => true),
+}));
+
+const mockGetPageSession = getPageSession as jest.MockedFunction<
+  typeof getPageSession
+>;
+const mockAccountsTable = accountsTable as jest.Mocked<typeof accountsTable>;
+const mockMembershipsTable = membershipsTable as jest.Mocked<
+  typeof membershipsTable
+>;
+const mockListUsable = listUsableDataConnections as jest.MockedFunction<
+  typeof listUsableDataConnections
+>;
+
+function membership(overrides: Partial<Membership>): Membership {
+  return {
+    membership_id: "m",
+    account_id: "user",
+    membership_account_id: "org",
+    role: MembershipRole.Owners,
+    state: MembershipState.Member,
+    ...overrides,
+  } as Membership;
+}
+
+/** The account ids the page decides may own the new product. */
+async function ownerIdsPassedToConnectionListing(memberships: Membership[]) {
+  mockMembershipsTable.listByUser.mockResolvedValue(memberships);
+  mockAccountsTable.fetchManyByIds.mockImplementation(async (ids: string[]) =>
+    ids.map((account_id) => ({ account_id }) as Account)
+  );
+  await NewProductPage({ params: Promise.resolve({}), searchParams: Promise.resolve({}) } as never);
+  return mockListUsable.mock.calls.at(-1)?.[1];
+}
+
+beforeEach(() => {
+  jest.clearAllMocks();
+  mockGetPageSession.mockResolvedValue({
+    identity_id: "id-1",
+    account: { account_id: "user", flags: [] },
+  } as unknown as UserSession);
+  mockListUsable.mockResolvedValue([]);
+});
+
+// The list drives which owned connections are serialized into the page, so a
+// membership that can't own a product must not widen it (see #462).
+describe("NewProductPage owner scoping", () => {
+  test("includes an account the user owns account-wide", async () => {
+    expect(
+      await ownerIdsPassedToConnectionListing([
+        membership({ membership_account_id: "org-a" }),
+      ])
+    ).toEqual(["user", "org-a"]);
+  });
+
+  test("includes an account-wide maintainer's account", async () => {
+    expect(
+      await ownerIdsPassedToConnectionListing([
+        membership({
+          membership_account_id: "org-a",
+          role: MembershipRole.Maintainers,
+        }),
+      ])
+    ).toEqual(["user", "org-a"]);
+  });
+
+  test("excludes an account where the membership is scoped to one product", async () => {
+    expect(
+      await ownerIdsPassedToConnectionListing([
+        membership({ membership_account_id: "org-b", repository_id: "prod-1" }),
+      ])
+    ).toEqual(["user"]);
+  });
+
+  test("excludes an account where the user only reads data", async () => {
+    expect(
+      await ownerIdsPassedToConnectionListing([
+        membership({
+          membership_account_id: "org-b",
+          role: MembershipRole.ReadData,
+        }),
+      ])
+    ).toEqual(["user"]);
+  });
+
+  test("excludes an invited-but-not-yet-member account", async () => {
+    expect(
+      await ownerIdsPassedToConnectionListing([
+        membership({
+          membership_account_id: "org-b",
+          state: MembershipState.Invited,
+        }),
+      ])
+    ).toEqual(["user"]);
+  });
+});

src/app/(app)/products/new/page.tsx

@@ -2,7 +2,12 @@ import { ProductCreationForm } from "@/components/features/products/ProductCreat
 import { accountsTable, getPageSession, membershipsTable } from "@/lib";
 import { isAuthorized } from "@/lib/api/authz";
 import { listUsableDataConnections } from "@/lib/data-connections";
-import { Actions, DataConnectionObjectSchema, MembershipState } from "@/types";
+import {
+  Actions,
+  DataConnectionObjectSchema,
+  MembershipRole,
+  MembershipState,
+} from "@/types";
 import { Heading, Text } from "@radix-ui/themes";
 import { FormTitle } from "@/components/core";
 
@@ -42,21 +47,37 @@ export default async function NewProductPage({
   const memberships = await membershipsTable.listByUser(
     session.account.account_id
   );
+  // Only accounts this user may actually create products under. A read-data
+  // member can't (createRepository requires owner/maintainer), and a membership
+  // scoped to one product doesn't reach the account at all — offering either
+  // would both present an owner createProduct rejects and, since this list also
+  // scopes the connections below, expose that account's storage details.
   const potentialOwnerAccounts = [
     session.account,
     ...(await accountsTable.fetchManyByIds(
       memberships
-        .filter((membership) => membership.state === MembershipState.Member)
+        .filter(
+          (membership) =>
+            membership.state === MembershipState.Member &&
+            !membership.repository_id &&
+            (membership.role === MembershipRole.Owners ||
+              membership.role === MembershipRole.Maintainers)
+        )
         .map((membership) => membership.membership_account_id)
     )),
   ];
 
-  // Strip credentials before handing connections to the client component.
-  const dataConnections = (await listUsableDataConnections(session)).map(
-    (connection) =>
-      DataConnectionObjectSchema.omit({ authentication: true }).parse(
-        connection
-      )
+  // Only connections usable by an account this user can create products under —
+  // an owned connection exposes its account's bucket names and prefixes, so the
+  // form's owner filter must not be the only one. Strip credentials before
+  // handing what remains to the client component.
+  const dataConnections = (
+    await listUsableDataConnections(
+      session,
+      potentialOwnerAccounts.map((account) => account.account_id)
+    )
+  ).map((connection) =>
+    DataConnectionObjectSchema.omit({ authentication: true }).parse(connection)
   );
 
   return (

src/lib/data-connections.test.ts

@@ -137,7 +137,9 @@ describe("listUsableDataConnections (issue #461)", () => {
   test("offers an org's own connection to an org owner", async () => {
     listing([orgConnection]);
     expect(
-      ids(await listUsableDataConnections(sessions["organization-owner-user"]))
+      ids(await listUsableDataConnections(sessions["organization-owner-user"], [
+        "organization",
+      ]))
     ).toEqual(["organization--byob"]);
   });
 
@@ -150,14 +152,35 @@ describe("listUsableDataConnections (issue #461)", () => {
 
     listing([orgConnection]);
     expect(
-      ids(await listUsableDataConnections(sessions["organization-owner-user"]))
+      ids(await listUsableDataConnections(sessions["organization-owner-user"], [
+        "organization",
+      ]))
     ).toEqual(["organization--byob"]);
   });
 
   test("offers a read-only connection", async () => {
     listing([{ ...orgConnection, read_only: true } as DataConnection]);
     expect(
-      ids(await listUsableDataConnections(sessions["organization-owner-user"]))
+      ids(await listUsableDataConnections(sessions["organization-owner-user"], [
+        "organization",
+      ]))
+    ).toEqual(["organization--byob"]);
+  });
+
+  // The owner filter has to happen here, not in ProductCreationForm: an owned
+  // connection carries its account's bucket name and prefixes, so a client-side
+  // filter would still have shipped them to every user's browser.
+  test("withholds another account's connection from the listing", async () => {
+    listing([orgConnection]);
+    expect(
+      ids(await listUsableDataConnections(sessions["regular-user"], ["regular"]))
+    ).toEqual([]);
+  });
+
+  test("still offers system-level connections to any account", async () => {
+    listing([{ ...orgConnection, owner: undefined } as DataConnection]);
+    expect(
+      ids(await listUsableDataConnections(sessions["regular-user"], ["regular"]))
     ).toEqual(["organization--byob"]);
   });
 });
@@ -215,3 +238,4 @@ describe("canUseDataConnectionFor", () => {
     ).toBe(false);
   });
 });
+

src/lib/data-connections.ts

@@ -63,20 +63,31 @@ export function canUseDataConnectionFor(
 }
 
 /**
- * List the data connections a user is permitted to use when creating a product.
+ * List the data connections a user is permitted to use when creating a product
+ * under one of `ownerAccountIds` (the accounts they may create products for).
+ *
+ * A connection is usable when it is available to one of those accounts —
+ * system-level (unowned) or owned by it — and the session is authorized both to
+ * read it (`GetDataConnection`) and to create products against it
+ * (`UseDataConnection`).
+ *
+ * The owner filter belongs here rather than at the call site: an owned
+ * connection carries its account's bucket names and prefixes, so filtering it
+ * out in the browser would still have shipped it there.
  *
- * A connection is usable when the session is authorized both to read it
- * (`GetDataConnection`) and to create products against it (`UseDataConnection`).
  * The returned objects are unsanitized (credentials intact); callers that hand
  * these to the client must strip `authentication` first.
  */
 export async function listUsableDataConnections(
-  session: UserSession | null
+  session: UserSession | null,
+  ownerAccountIds: string[]
 ): Promise<DataConnection[]> {
+  const usableBy = new Set(ownerAccountIds);
   const dataConnections = await dataConnectionsTable.listAll();
 
   return dataConnections.filter(
     (dataConnection) =>
+      (!dataConnection.owner || usableBy.has(dataConnection.owner)) &&
       isAuthorized(session, dataConnection, Actions.UseDataConnection) &&
       isAuthorized(session, dataConnection, Actions.GetDataConnection)
   );