refactor(data-connections): single definition of the product/connection rule

src/lib/actions/products.test.ts

@@ -184,6 +184,10 @@ describe("createProduct", () => {
     const result = await createProduct(undefined, buildFormData());
 
     expect(result.success).toBe(false);
+    expect(result.message).toBe(
+      "You are not permitted to use the selected data connection"
+    );
+    expect(result.fieldErrors).toEqual({});
     expect(productsTable.create).not.toHaveBeenCalled();
   });
 
@@ -210,7 +214,10 @@ describe("createProduct", () => {
     );
 
     expect(result.success).toBe(false);
-    expect(result.fieldErrors.data_connection_id).toBeDefined();
+    expect(result.message).toBe("Invalid data connection for this account");
+    expect(result.fieldErrors.data_connection_id).toEqual([
+      "Selected data connection is not available for this account",
+    ]);
     expect(productsTable.create).not.toHaveBeenCalled();
   });
 

src/lib/actions/products.ts

@@ -12,6 +12,7 @@ import {
 import { getPageSession, LOGGER } from "@/lib";
 import { FormState } from "@/components/core/DynamicForm";
 import { isAuthorized, isAdmin } from "../api/authz";
+import { denyDataConnectionFor } from "@/lib/data-connections";
 import { revalidatePath } from "next/cache";
 import { productUrl, editProductDetailsUrl, accountUrl } from "@/lib/urls";
 import { getProxyCredentials } from "@/lib/actions/proxy-credentials";
@@ -171,33 +172,33 @@ export async function createProduct(
     };
   }
 
-  // Enforce that the user may create products against this connection
-  // (connections gated behind an account flag).
-  if (!isAuthorized(session, dataConnection, Actions.UseDataConnection)) {
-    return {
-      fieldErrors: {},
-      data: formData,
-      message: "You are not permitted to use the selected data connection",
-      success: false,
-    };
-  }
-
-  // Enforce that the connection is available for the product's account. An
-  // owned connection may only be used by the account that owns it.
-  if (
-    dataConnection.owner &&
-    dataConnection.owner !== validatedFields.data.account_id
+  // Enforce that this connection may back a product under this account, and
+  // report each denial reason as its own field error.
+  switch (
+    denyDataConnectionFor(
+      session,
+      dataConnection,
+      validatedFields.data.account_id
+    )
   ) {
-    return {
-      fieldErrors: {
-        data_connection_id: [
-          "Selected data connection is not available for this account",
-        ],
-      },
-      data: formData,
-      message: "Invalid data connection for this account",
-      success: false,
-    };
+    case "not-usable":
+      return {
+        fieldErrors: {},
+        data: formData,
+        message: "You are not permitted to use the selected data connection",
+        success: false,
+      };
+    case "wrong-account":
+      return {
+        fieldErrors: {
+          data_connection_id: [
+            "Selected data connection is not available for this account",
+          ],
+        },
+        data: formData,
+        message: "Invalid data connection for this account",
+        success: false,
+      };
   }
 
   // Enforce the connection's allowed visibilities. Even though the form only

src/lib/data-connections.test.ts

@@ -1,6 +1,7 @@
 import {
   canManageDataConnection,
   canUseDataConnectionFor,
+  denyDataConnectionFor,
   listUsableDataConnections,
 } from "./data-connections";
 import { isAdmin, canManageAccountDataConnections } from "@/lib/api/authz";
@@ -162,6 +163,64 @@ describe("listUsableDataConnections (issue #461)", () => {
   });
 });
 
+// The single definition of which connections may back a product owned by a
+// given account, and why not when they may not. `createProduct` switches on the
+// reason to pick a field error. Uses the real `isAuthorized`.
+describe("denyDataConnectionFor", () => {
+  const user = sessions["regular-user"];
+
+  test("allows a system-level (unowned) connection", () => {
+    expect(
+      denyDataConnectionFor(user, connection({ owner: undefined }), "acme")
+    ).toBeNull();
+  });
+
+  test("allows a connection the account owns", () => {
+    expect(
+      denyDataConnectionFor(user, connection({ owner: "acme" }), "acme")
+    ).toBeNull();
+  });
+
+  test("reports wrong-account for a connection another account owns", () => {
+    expect(
+      denyDataConnectionFor(user, connection({ owner: "rival" }), "acme")
+    ).toBe("wrong-account");
+  });
+
+  test("allows a read-only connection", () => {
+    expect(
+      denyDataConnectionFor(
+        user,
+        connection({ owner: "acme", read_only: true }),
+        "acme"
+      )
+    ).toBeNull();
+  });
+
+  test("reports not-usable for a flag-gated connection the caller lacks the flag for", () => {
+    expect(
+      denyDataConnectionFor(
+        user,
+        connection({ required_flag: AccountFlags.CREATE_DATA_CONNECTIONS }),
+        "acme"
+      )
+    ).toBe("not-usable");
+  });
+
+  test("reports not-usable ahead of wrong-account when both apply", () => {
+    expect(
+      denyDataConnectionFor(
+        user,
+        connection({
+          owner: "rival",
+          required_flag: AccountFlags.CREATE_DATA_CONNECTIONS,
+        }),
+        "acme"
+      )
+    ).toBe("not-usable");
+  });
+});
+
 // Which connections may back a product owned by a given account: system-level
 // (unowned) plus the account's own. See issue #461. Uses the real `isAuthorized`.
 describe("canUseDataConnectionFor", () => {

src/lib/data-connections.ts

@@ -39,27 +39,44 @@ export async function canManageDataConnection(
   return canManageAccountDataConnections(session, ownerAccount);
 }
 
+export type DataConnectionDenial = "not-usable" | "wrong-account";
+
 /**
- * Whether `connection` may back a product owned by `accountId`: the connection
- * itself must permit the caller (`UseDataConnection` covers flag-gated
- * connections) and must be available to that account — either
- * system-level (unowned) or owned by it.
+ * Why `connection` may not back a product owned by `accountId`, or null if it
+ * may. Two rules: the connection itself must permit the caller
+ * (`not-usable` — `UseDataConnection` covers flag-gated connections), and it
+ * must be available to that account — either system-level (unowned) or owned
+ * by it (`wrong-account`).
+ *
+ * The reason is returned rather than a bare boolean so `createProduct` can
+ * surface each rule as its own form field error; callers that only need a
+ * yes/no use `canUseDataConnectionFor`.
  *
  * This is only the connection half of associating the two; the caller side is
  * each call site's own gate — `canManageAccount` on the owning account for the
- * mirror actions, `Actions.CreateRepository` for `createProduct` (which applies
- * the same two connection rules inline, to report them as distinct field
- * errors).
+ * mirror actions, `Actions.CreateRepository` for `createProduct`.
  */
+export function denyDataConnectionFor(
+  session: UserSession | null,
+  connection: DataConnection,
+  accountId: string
+): DataConnectionDenial | null {
+  if (!isAuthorized(session, connection, Actions.UseDataConnection)) {
+    return "not-usable";
+  }
+  if (connection.owner && connection.owner !== accountId) {
+    return "wrong-account";
+  }
+  return null;
+}
+
+/** Boolean form of {@link denyDataConnectionFor}. */
 export function canUseDataConnectionFor(
   session: UserSession | null,
   connection: DataConnection,
   accountId: string
 ): boolean {
-  return (
-    isAuthorized(session, connection, Actions.UseDataConnection) &&
-    (!connection.owner || connection.owner === accountId)
-  );
+  return denyDataConnectionFor(session, connection, accountId) === null;
 }
 
 /**