Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
136 changes: 136 additions & 0 deletions lib/composio/__tests__/checkConnectorAuthority.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
import { describe, it, expect, vi, beforeEach } from "vitest";
import { checkConnectorAuthority } from "../checkConnectorAuthority";

import { getAccountOrganizations } from "@/lib/supabase/account_organization_ids/getAccountOrganizations";
import { selectArtistOrganizationIds } from "@/lib/supabase/artist_organization_ids/selectArtistOrganizationIds";
import { selectAccountOrganizationIds } from "@/lib/supabase/account_organization_ids/selectAccountOrganizationIds";
import { selectAccountWorkspaceId } from "@/lib/supabase/account_workspace_ids/selectAccountWorkspaceId";
import { validateOrganizationAccess } from "@/lib/organizations/validateOrganizationAccess";
import { selectAccountArtistId } from "@/lib/supabase/account_artist_ids/selectAccountArtistId";

vi.mock("@/lib/const", () => ({
RECOUP_ORG_ID: "recoup-admin-org-id",
}));

vi.mock("@/lib/supabase/account_organization_ids/getAccountOrganizations", () => ({
getAccountOrganizations: vi.fn(),
}));

vi.mock("@/lib/supabase/artist_organization_ids/selectArtistOrganizationIds", () => ({
selectArtistOrganizationIds: vi.fn(),
}));

vi.mock("@/lib/supabase/account_organization_ids/selectAccountOrganizationIds", () => ({
selectAccountOrganizationIds: vi.fn(),
}));

vi.mock("@/lib/supabase/account_workspace_ids/selectAccountWorkspaceId", () => ({
selectAccountWorkspaceId: vi.fn(),
}));

vi.mock("@/lib/organizations/validateOrganizationAccess", () => ({
validateOrganizationAccess: vi.fn(),
}));

vi.mock("@/lib/supabase/account_artist_ids/selectAccountArtistId", () => ({
selectAccountArtistId: vi.fn(),
}));

describe("checkConnectorAuthority", () => {
beforeEach(() => {
vi.clearAllMocks();
vi.mocked(getAccountOrganizations).mockResolvedValue([]);
vi.mocked(selectArtistOrganizationIds).mockResolvedValue([]);
vi.mocked(selectAccountOrganizationIds).mockResolvedValue([]);
vi.mocked(selectAccountWorkspaceId).mockResolvedValue(null);
vi.mocked(validateOrganizationAccess).mockResolvedValue(false);
});

it("grants self access without any database calls", async () => {
const result = await checkConnectorAuthority("account-123", "account-123");

expect(result).toBe(true);
expect(getAccountOrganizations).not.toHaveBeenCalled();
expect(selectArtistOrganizationIds).not.toHaveBeenCalled();
});

it("denies a roster-only manager (bare account_artist_ids row, no org tie)", async () => {
// Even if a roster row exists, connector authority must never read it.
vi.mocked(selectAccountArtistId).mockResolvedValue({
id: "aai-1",
artist_id: "artist-456",
pinned: false,
});

const result = await checkConnectorAuthority("account-123", "artist-456");

expect(result).toBe(false);
expect(selectAccountArtistId).not.toHaveBeenCalled();
});

it("grants access when caller and artist share an organization", async () => {
vi.mocked(selectArtistOrganizationIds).mockResolvedValue([{ organization_id: "org-1" }]);
vi.mocked(selectAccountOrganizationIds).mockResolvedValue([{ organization_id: "org-1" }]);

const result = await checkConnectorAuthority("account-123", "artist-456");

expect(selectArtistOrganizationIds).toHaveBeenCalledWith("artist-456");
expect(selectAccountOrganizationIds).toHaveBeenCalledWith("account-123", ["org-1"]);
expect(result).toBe(true);
});

it("grants RECOUP_ORG staff access to any target", async () => {
vi.mocked(getAccountOrganizations).mockResolvedValue([
{
id: "aoi-1",
account_id: "admin-account",
organization_id: "recoup-admin-org-id",
updated_at: null,
organization: null,
},
]);

const result = await checkConnectorAuthority("admin-account", "artist-456");

expect(result).toBe(true);
expect(selectArtistOrganizationIds).not.toHaveBeenCalled();
});

it("grants access to a workspace the caller owns", async () => {
vi.mocked(selectAccountWorkspaceId).mockResolvedValue({ workspace_id: "workspace-789" });

const result = await checkConnectorAuthority("account-123", "workspace-789");

expect(selectAccountWorkspaceId).toHaveBeenCalledWith("account-123", "workspace-789");
expect(result).toBe(true);
});

it("grants access to an organization the caller belongs to", async () => {
vi.mocked(validateOrganizationAccess).mockResolvedValue(true);

const result = await checkConnectorAuthority("account-123", "org-target");

expect(validateOrganizationAccess).toHaveBeenCalledWith({
accountId: "account-123",
organizationId: "org-target",
});
expect(result).toBe(true);
});

it("fails closed when artist org lookup errors", async () => {
vi.mocked(selectArtistOrganizationIds).mockResolvedValue(null);

const result = await checkConnectorAuthority("account-123", "artist-456");

expect(result).toBe(false);
});

it("fails closed when account org lookup errors", async () => {
vi.mocked(selectArtistOrganizationIds).mockResolvedValue([{ organization_id: "org-1" }]);
vi.mocked(selectAccountOrganizationIds).mockResolvedValue(null);

const result = await checkConnectorAuthority("account-123", "artist-456");

expect(result).toBe(false);
});
});
59 changes: 59 additions & 0 deletions lib/composio/checkConnectorAuthority.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,59 @@
import { RECOUP_ORG_ID } from "@/lib/const";
import { getAccountOrganizations } from "@/lib/supabase/account_organization_ids/getAccountOrganizations";
import { selectArtistOrganizationIds } from "@/lib/supabase/artist_organization_ids/selectArtistOrganizationIds";
import { selectAccountOrganizationIds } from "@/lib/supabase/account_organization_ids/selectAccountOrganizationIds";
import { selectAccountWorkspaceId } from "@/lib/supabase/account_workspace_ids/selectAccountWorkspaceId";
import { validateOrganizationAccess } from "@/lib/organizations/validateOrganizationAccess";

/**
* Check whether a caller has act-as authority over a target account for
* connector operations (authorize, execute, disconnect, tool exposure).
*
* Interim P0b cut (chat#1860): unlike `checkAccountArtistAccess`, this
* NEVER reads the bare `account_artist_ids` roster edge — a roster row
* alone must not grant use of another account's OAuth connectors.
*
* Authority is granted when:
* 1. Target is the caller itself, OR
* 2. Caller is a RECOUP_ORG member (admin bypass), OR
* 3. Caller shares an organization with the target artist, OR
* 4. Target is a workspace the caller owns, OR
* 5. Target is an organization the caller belongs to
*
* Fails closed: any database error results in denied authority.
*
* @param callerAccountId - The authenticated caller's account ID
* @param targetAccountId - The account whose connectors are being used
* @returns true if the caller has connector authority over the target
*/
export async function checkConnectorAuthority(
callerAccountId: string,
targetAccountId: string,
): Promise<boolean> {
// 1. Self-access
if (targetAccountId === callerAccountId) return true;

// 2. Admin bypass: RECOUP_ORG staff
const callerOrgs = await getAccountOrganizations({ accountId: callerAccountId });
if (callerOrgs.some(m => m.organization_id === RECOUP_ORG_ID)) return true;

// 3. Org co-membership with a target artist
const targetOrgs = await selectArtistOrganizationIds(targetAccountId);
if (targetOrgs?.length) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Connector authority can still be granted after an artist-org or account-org lookup error because null is treated like “no rows” and the function falls through to later grants. Returning false explicitly on those null results keeps the fail-closed contract for connector operations.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At lib/composio/checkConnectorAuthority.ts, line 42:

<comment>Connector authority can still be granted after an artist-org or account-org lookup error because `null` is treated like “no rows” and the function falls through to later grants. Returning `false` explicitly on those `null` results keeps the fail-closed contract for connector operations.</comment>

<file context>
@@ -0,0 +1,59 @@
+
+  // 3. Org co-membership with a target artist
+  const targetOrgs = await selectArtistOrganizationIds(targetAccountId);
+  if (targetOrgs?.length) {
+    const orgIds = targetOrgs.map(o => o.organization_id).filter((id): id is string => Boolean(id));
+    if (orgIds.length) {
</file context>

const orgIds = targetOrgs.map(o => o.organization_id).filter((id): id is string => Boolean(id));
if (orgIds.length) {
const sharedOrgs = await selectAccountOrganizationIds(callerAccountId, orgIds);
if (sharedOrgs?.length) return true;
}
}

// 4. Workspace owned by the caller
const isWorkspace = await selectAccountWorkspaceId(callerAccountId, targetAccountId);
if (isWorkspace) return true;

// 5. Organization the caller belongs to
return validateOrganizationAccess({
accountId: callerAccountId,
organizationId: targetAccountId,
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ import { NextRequest, NextResponse } from "next/server";
import { validateAuthorizeConnectorRequest } from "../validateAuthorizeConnectorRequest";

import { validateAuthContext } from "@/lib/auth/validateAuthContext";
import { checkAccountAccess } from "@/lib/auth/checkAccountAccess";
import { checkConnectorAuthority } from "../../checkConnectorAuthority";

vi.mock("@/lib/auth/validateAuthContext", () => ({
validateAuthContext: vi.fn(),
}));

vi.mock("@/lib/auth/checkAccountAccess", () => ({
checkAccountAccess: vi.fn(),
vi.mock("../../checkConnectorAuthority", () => ({
checkConnectorAuthority: vi.fn(),
}));

vi.mock("@/lib/networking/getCorsHeaders", () => ({
Expand Down Expand Up @@ -68,15 +68,15 @@ describe("validateAuthorizeConnectorRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: true, entityType: "artist" });
vi.mocked(checkConnectorAuthority).mockResolvedValue(true);

const request = new NextRequest("http://localhost/api/connectors/authorize", {
method: "POST",
body: JSON.stringify({ connector: "tiktok", account_id: mockTargetAccountId }),
});
const result = await validateAuthorizeConnectorRequest(request);

expect(checkAccountAccess).toHaveBeenCalledWith(mockAccountId, mockTargetAccountId);
expect(checkConnectorAuthority).toHaveBeenCalledWith(mockAccountId, mockTargetAccountId);
expect(result).not.toBeInstanceOf(NextResponse);
expect(result).toEqual({
accountId: mockTargetAccountId,
Expand All @@ -94,7 +94,7 @@ describe("validateAuthorizeConnectorRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: true, entityType: "artist" });
vi.mocked(checkConnectorAuthority).mockResolvedValue(true);

const request = new NextRequest("http://localhost/api/connectors/authorize", {
method: "POST",
Expand All @@ -120,7 +120,7 @@ describe("validateAuthorizeConnectorRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: true, entityType: "workspace" });
vi.mocked(checkConnectorAuthority).mockResolvedValue(true);

const request = new NextRequest("http://localhost/api/connectors/authorize", {
method: "POST",
Expand All @@ -145,10 +145,7 @@ describe("validateAuthorizeConnectorRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({
hasAccess: true,
entityType: "organization",
});
vi.mocked(checkConnectorAuthority).mockResolvedValue(true);

const request = new NextRequest("http://localhost/api/connectors/authorize", {
method: "POST",
Expand All @@ -173,7 +170,7 @@ describe("validateAuthorizeConnectorRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: false });
vi.mocked(checkConnectorAuthority).mockResolvedValue(false);

const request = new NextRequest("http://localhost/api/connectors/authorize", {
method: "POST",
Expand All @@ -197,7 +194,7 @@ describe("validateAuthorizeConnectorRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: true, entityType: "artist" });
vi.mocked(checkConnectorAuthority).mockResolvedValue(true);

const request = new NextRequest("http://localhost/api/connectors/authorize", {
method: "POST",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@ import { NextRequest, NextResponse } from "next/server";
import { validateDisconnectConnectorRequest } from "../validateDisconnectConnectorRequest";

import { validateAuthContext } from "@/lib/auth/validateAuthContext";
import { checkAccountAccess } from "@/lib/auth/checkAccountAccess";
import { checkConnectorAuthority } from "../../checkConnectorAuthority";
import { verifyConnectorOwnership } from "../verifyConnectorOwnership";

vi.mock("@/lib/auth/validateAuthContext", () => ({
validateAuthContext: vi.fn(),
}));

vi.mock("@/lib/auth/checkAccountAccess", () => ({
checkAccountAccess: vi.fn(),
vi.mock("../../checkConnectorAuthority", () => ({
checkConnectorAuthority: vi.fn(),
}));

vi.mock("../verifyConnectorOwnership", () => ({
Expand Down Expand Up @@ -91,15 +91,15 @@ describe("validateDisconnectConnectorRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: true, entityType: "artist" });
vi.mocked(checkConnectorAuthority).mockResolvedValue(true);

const request = new NextRequest("http://localhost/api/connectors", {
method: "DELETE",
body: JSON.stringify({ connected_account_id: "ca_123", account_id: mockTargetAccountId }),
});
const result = await validateDisconnectConnectorRequest(request);

expect(checkAccountAccess).toHaveBeenCalledWith("account-123", mockTargetAccountId);
expect(checkConnectorAuthority).toHaveBeenCalledWith("account-123", mockTargetAccountId);
expect(verifyConnectorOwnership).not.toHaveBeenCalled();
expect(result).not.toBeInstanceOf(NextResponse);
expect(result).toEqual({
Expand All @@ -115,15 +115,15 @@ describe("validateDisconnectConnectorRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: true, entityType: "workspace" });
vi.mocked(checkConnectorAuthority).mockResolvedValue(true);

const request = new NextRequest("http://localhost/api/connectors", {
method: "DELETE",
body: JSON.stringify({ connected_account_id: "ca_123", account_id: mockTargetAccountId }),
});
const result = await validateDisconnectConnectorRequest(request);

expect(checkAccountAccess).toHaveBeenCalledWith("account-123", mockTargetAccountId);
expect(checkConnectorAuthority).toHaveBeenCalledWith("account-123", mockTargetAccountId);
expect(result).not.toBeInstanceOf(NextResponse);
});

Expand All @@ -134,7 +134,7 @@ describe("validateDisconnectConnectorRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: false });
vi.mocked(checkConnectorAuthority).mockResolvedValue(false);

const request = new NextRequest("http://localhost/api/connectors", {
method: "DELETE",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ import { NextRequest, NextResponse } from "next/server";
import { validateExecuteConnectorActionRequest } from "../validateExecuteConnectorActionRequest";

import { validateAuthContext } from "@/lib/auth/validateAuthContext";
import { checkAccountAccess } from "@/lib/auth/checkAccountAccess";
import { checkConnectorAuthority } from "../../checkConnectorAuthority";

vi.mock("@/lib/auth/validateAuthContext", () => ({
validateAuthContext: vi.fn(),
}));

vi.mock("@/lib/auth/checkAccountAccess", () => ({
checkAccountAccess: vi.fn(),
vi.mock("../../checkConnectorAuthority", () => ({
checkConnectorAuthority: vi.fn(),
}));

vi.mock("@/lib/networking/getCorsHeaders", () => ({
Expand Down Expand Up @@ -68,13 +68,13 @@ describe("validateExecuteConnectorActionRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: true, entityType: "artist" });
vi.mocked(checkConnectorAuthority).mockResolvedValue(true);

const result = await validateExecuteConnectorActionRequest(
buildRequest({ ...validBody, account_id: target }),
);

expect(checkAccountAccess).toHaveBeenCalledWith("account-123", target);
expect(checkConnectorAuthority).toHaveBeenCalledWith("account-123", target);
expect(result).toEqual({
accountId: target,
actionSlug: "GMAIL_FETCH_EMAILS",
Expand All @@ -89,7 +89,7 @@ describe("validateExecuteConnectorActionRequest", () => {
orgId: null,
authToken: "test-token",
});
vi.mocked(checkAccountAccess).mockResolvedValue({ hasAccess: false });
vi.mocked(checkConnectorAuthority).mockResolvedValue(false);

const result = await validateExecuteConnectorActionRequest(
buildRequest({ ...validBody, account_id: target }),
Expand Down
Loading
Loading