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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ yarn-error.log*
/playwright/.cache/

# project specific
.local/
packages/lib/uploads
apps/web/public/js
packages/database/generated
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { toast } from "react-hot-toast";
import { useTranslation } from "react-i18next";
import { authClient } from "@/modules/auth/lib/auth-client";
import { Button } from "@/modules/ui/components/button";
import { resolveConsentRedirectUrl } from "../lib/consent-redirect";

export const OAuthConsentActions = () => {
const { t } = useTranslation();
Expand All @@ -21,21 +22,14 @@ export const OAuthConsentActions = () => {
return;
}

const redirectUri =
response.data &&
typeof response.data === "object" &&
"redirect_uri" in response.data &&
typeof response.data.redirect_uri === "string" &&
response.data.redirect_uri.length > 0
? response.data.redirect_uri
: null;

if (!redirectUri) {
// Better Auth returns { redirect: true, url } (both accept and deny) — not { redirect_uri }.
const redirectUrl = resolveConsentRedirectUrl(response.data);
if (!redirectUrl) {
toast.error(t("auth.oauth.consent_failed"));
return;
}

window.location.href = redirectUri;
window.location.href = redirectUrl;
} catch {
toast.error(t("auth.oauth.consent_failed"));
} finally {
Expand Down
68 changes: 68 additions & 0 deletions apps/web/app/(app)/account/authorize/lib/consent-redirect.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import { describe, expect, test } from "vitest";
import { resolveConsentRedirectUrl } from "./consent-redirect";

describe("resolveConsentRedirectUrl", () => {
test("reads `url` from the real accept shape ({ redirect: true, url })", () => {
expect(
resolveConsentRedirectUrl({ redirect: true, url: "https://client.example/callback?code=abc&state=xyz" })
).toBe("https://client.example/callback?code=abc&state=xyz");
});

test("reads the error `url` from the deny shape", () => {
expect(
resolveConsentRedirectUrl({
redirect: true,
url: "https://client.example/callback?error=access_denied",
})
).toBe("https://client.example/callback?error=access_denied");
});

test("allows loopback and custom-scheme redirect targets (native MCP clients)", () => {
expect(resolveConsentRedirectUrl({ url: "http://127.0.0.1:52765/callback?code=abc" })).toBe(
"http://127.0.0.1:52765/callback?code=abc"
);
expect(resolveConsentRedirectUrl({ url: "cursor://anysphere.cursor-mcp/callback?code=abc" })).toBe(
"cursor://anysphere.cursor-mcp/callback?code=abc"
);
});

test("falls back to `redirect_uri` when only that is present (documented/legacy shape)", () => {
expect(resolveConsentRedirectUrl({ redirect_uri: "https://client.example/cb?code=abc" })).toBe(
"https://client.example/cb?code=abc"
);
});

test("prefers `url` over `redirect_uri` when both are present", () => {
expect(
resolveConsentRedirectUrl({ url: "https://a.example/cb", redirect_uri: "https://b.example/cb" })
).toBe("https://a.example/cb");
});

test("returns null for missing/empty/non-object/non-string values (→ toast)", () => {
expect(resolveConsentRedirectUrl({})).toBeNull();
expect(resolveConsentRedirectUrl({ url: "" })).toBeNull();
expect(resolveConsentRedirectUrl({ url: " " })).toBeNull();
expect(resolveConsentRedirectUrl({ url: 123 })).toBeNull();
expect(resolveConsentRedirectUrl(null)).toBeNull();
expect(resolveConsentRedirectUrl(undefined)).toBeNull();
expect(resolveConsentRedirectUrl("https://client.example/cb")).toBeNull();
});

test("rejects dangerous schemes (XSS guard for the location.href sink)", () => {
expect(resolveConsentRedirectUrl({ url: "javascript:alert(1)" })).toBeNull();
expect(resolveConsentRedirectUrl({ url: " JavaScript:alert(1)" })).toBeNull();
// URL parsing normalizes control-char-obfuscated schemes, so this is caught too.
expect(resolveConsentRedirectUrl({ url: "java\tscript:alert(1)" })).toBeNull();
expect(resolveConsentRedirectUrl({ url: "data:text/html,<script>alert(1)</script>" })).toBeNull();
expect(resolveConsentRedirectUrl({ url: "file:///etc/passwd" })).toBeNull();
expect(resolveConsentRedirectUrl({ url: "blob:https://x/1" })).toBeNull();
// falls through to the fallback, which is also dangerous → null
expect(resolveConsentRedirectUrl({ url: "javascript:1", redirect_uri: "data:x" })).toBeNull();
});

test("rejects malformed / protocol-relative values (not a well-formed absolute URL)", () => {
expect(resolveConsentRedirectUrl({ url: "//evil.com/path" })).toBeNull();
expect(resolveConsentRedirectUrl({ url: "not a url" })).toBeNull();
expect(resolveConsentRedirectUrl({ url: "/relative/path" })).toBeNull();
});
});
37 changes: 37 additions & 0 deletions apps/web/app/(app)/account/authorize/lib/consent-redirect.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
/**
* Extract the post-consent redirect URL from a Better Auth `oauth2.consent` response.
*
* On accept, `@better-auth/oauth-provider` delegates to its authorize handler, which for a fetch/JSON
* request returns `{ redirect: true, url: "<callback?code=…>" }` (see `handleRedirect` in the plugin) —
* NOT `{ redirect_uri }`. The plugin's OpenAPI schema documents `redirect_uri`, which is misleading;
* the runtime field is `url`. Deny returns the same shape with an `error=access_denied` URL. We read
* `url` first, and keep `redirect_uri` as a defensive fallback for the documented/legacy shape.
*
* Security: the returned string is fed to `window.location.href`, so we reject XSS-capable schemes.
* We parse with the `URL` constructor and deny the dangerous protocols rather than allowlisting — native
* MCP clients use arbitrary custom/loopback schemes (`cursor://`, `vscode://`, `http://127.0.0.1:PORT`),
* so the safe set can't be enumerated, and the oauth-provider already validated the target `redirect_uri`
* against the registered client at `/authorize`. Parsing (vs a regex) also normalizes obfuscated schemes
* like `java\tscript:` and rejects malformed / protocol-relative values.
*/
const DANGEROUS_PROTOCOLS = new Set(["javascript:", "data:", "vbscript:", "file:", "blob:"]);

const asRedirectString = (value: unknown): string | null => {
if (typeof value !== "string") return null;
const trimmed = value.trim();
if (trimmed.length === 0) return null;
let protocol: string;
try {
protocol = new URL(trimmed).protocol;
} catch {
return null; // not a well-formed absolute URL
}
if (DANGEROUS_PROTOCOLS.has(protocol.toLowerCase())) return null;
return trimmed;
};

export const resolveConsentRedirectUrl = (data: unknown): string | null => {
if (typeof data !== "object" || data === null) return null;
const record = data as Record<string, unknown>;
return asRedirectString(record.url) ?? asRedirectString(record.redirect_uri);
};
1 change: 1 addition & 0 deletions apps/web/app/api/mcp/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,7 @@ describe("POST /api/mcp", () => {
"validate_survey",
"patch_survey",
"delete_survey",
"list_workspaces",
]);
const tools = new Map(message.result.tools.map((tool: { name: string }) => [tool.name, tool]));
expect(Object.keys((tools.get("create_survey") as any).inputSchema.properties)).toEqual(
Expand Down
122 changes: 122 additions & 0 deletions apps/web/app/api/v3/workspaces/lib/operations.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,122 @@
import { beforeEach, describe, expect, test, vi } from "vitest";
import type { TV3Authentication } from "@/app/api/v3/lib/types";
import { getOrganizationsByUserId } from "@/lib/organization/service";
import { getUserWorkspaces, getWorkspace } from "@/lib/workspace/service";
import { listV3Workspaces } from "./operations";

vi.mock("@/lib/organization/service", () => ({ getOrganizationsByUserId: vi.fn() }));
vi.mock("@/lib/workspace/service", () => ({ getUserWorkspaces: vi.fn(), getWorkspace: vi.fn() }));
vi.mock("@formbricks/logger", () => ({
logger: { withContext: vi.fn(() => ({ error: vi.fn(), warn: vi.fn() })) },
}));

const sessionAuth = {
user: { id: "user_1" },
expires: "2026-07-01T00:00:00.000Z",
} as unknown as TV3Authentication;

const params = (authentication: TV3Authentication) => ({
authentication,
requestId: "req_1",
instance: "/api/mcp",
});

// A workspace as the services return it: full entity. The operation must serialize it down to the DTO.
const ws = (id: string, name: string, organizationId: string) =>
({
id,
name,
organizationId,
createdAt: new Date(),
updatedAt: new Date(),
config: { secret: "should-not-leak" },
styling: {},
}) as any;

describe("listV3Workspaces", () => {
beforeEach(() => vi.clearAllMocks());

test("session user: aggregates + dedupes across orgs and returns the minimal DTO only", async () => {
vi.mocked(getOrganizationsByUserId).mockResolvedValue([
{ id: "org_1", name: "Org 1" },
{ id: "org_2", name: "Org 2" },
] as any);
vi.mocked(getUserWorkspaces).mockImplementation(async (_userId: string, orgId: string) =>
orgId === "org_1"
? [ws("w1", "Alpha", "org_1"), ws("w2", "Beta", "org_1")]
: [ws("w2", "Beta", "org_1"), ws("w3", "Gamma", "org_2")]
);

const res = await listV3Workspaces(params(sessionAuth));
expect(res.status).toBe(200);

const body = await res.json();
expect(body.data).toEqual([
{ id: "w1", name: "Alpha", organizationId: "org_1" },
{ id: "w2", name: "Beta", organizationId: "org_1" },
{ id: "w3", name: "Gamma", organizationId: "org_2" },
]);
expect(body.meta.totalCount).toBe(3);
// Only the DTO fields — no config/styling/entity internals leak.
expect(Object.keys(body.data[0])).toEqual(["id", "name", "organizationId"]);
});

test("returns workspaces in a deterministic order (name, then id)", async () => {
vi.mocked(getOrganizationsByUserId).mockResolvedValue([{ id: "org_1", name: "Org 1" }] as any);
vi.mocked(getUserWorkspaces).mockResolvedValue([
ws("w3", "Zeta", "org_1"),
ws("w1", "alpha", "org_1"),
ws("w2", "Beta", "org_1"),
]);

const res = await listV3Workspaces(params(sessionAuth));
const body = await res.json();

expect(body.data.map((w: { name: string }) => w.name)).toEqual(["alpha", "Beta", "Zeta"]);
});

test("session user with no orgs → empty list, no workspace lookups", async () => {
vi.mocked(getOrganizationsByUserId).mockResolvedValue([] as any);

const res = await listV3Workspaces(params(sessionAuth));
const body = await res.json();

expect(res.status).toBe(200);
expect(body.data).toEqual([]);
expect(getUserWorkspaces).not.toHaveBeenCalled();
});

test("api key: returns only the workspaces in workspacePermissions", async () => {
const keyAuth = {
apiKeyId: "key_1",
workspacePermissions: [
{ workspaceId: "w1", permission: "read" },
{ workspaceId: "w9", permission: "write" },
],
} as unknown as TV3Authentication;
vi.mocked(getWorkspace).mockImplementation(async (id: string) =>
id === "w1" ? ws("w1", "Alpha", "org_1") : id === "w9" ? ws("w9", "Zeta", "org_2") : null
);

const res = await listV3Workspaces(params(keyAuth));
const body = await res.json();

expect(body.data).toEqual([
{ id: "w1", name: "Alpha", organizationId: "org_1" },
{ id: "w9", name: "Zeta", organizationId: "org_2" },
]);
// API-key path must never fall back to the user/org aggregation.
expect(getOrganizationsByUserId).not.toHaveBeenCalled();
});

test("no authentication → 401", async () => {
const res = await listV3Workspaces(params(null));
expect(res.status).toBe(401);
});

test("an unexpected service failure is logged and returned as a 500 (never thrown)", async () => {
vi.mocked(getOrganizationsByUserId).mockRejectedValue(new Error("db exploded"));
const res = await listV3Workspaces(params(sessionAuth));
expect(res.status).toBe(500);
});
});
109 changes: 109 additions & 0 deletions apps/web/app/api/v3/workspaces/lib/operations.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
import "server-only";
import { logger } from "@formbricks/logger";
import type { TAuthenticationApiKey } from "@formbricks/types/auth";
import { problemInternalError, problemUnauthorized, successListResponse } from "@/app/api/v3/lib/response";
import type { TV3Authentication } from "@/app/api/v3/lib/types";
import { getOrganizationsByUserId } from "@/lib/organization/service";
import { getUserWorkspaces, getWorkspace } from "@/lib/workspace/service";

type TListV3WorkspacesParams = {
authentication: TV3Authentication;
requestId: string;
instance: string;
};

/** Minimal DTO — never return the raw Workspace entity (no config/env/styling internals). */
type TV3WorkspaceListItem = {
id: string;
name: string;
organizationId: string;
};

const serializeV3WorkspaceListItem = (workspace: {
id: string;
name: string;
organizationId: string;
}): TV3WorkspaceListItem => ({
id: workspace.id,
name: workspace.name,
organizationId: workspace.organizationId,
});

/**
* Session user's accessible workspaces: every workspace across the orgs they're a member of. Scoping is
* enforced by the reused services — `getOrganizationsByUserId` limits to the user's own orgs, and
* `getUserWorkspaces` returns all of an org's workspaces for owners/managers but only team-scoped ones
* for `member`-role users.
*/
async function fetchSessionWorkspaces(userId: string): Promise<TV3WorkspaceListItem[]> {
const organizations = await getOrganizationsByUserId(userId);
const workspacesPerOrg = await Promise.all(
organizations.map((organization) => getUserWorkspaces(userId, organization.id))
);
return workspacesPerOrg.flat().map(serializeV3WorkspaceListItem);
}

/** API key's accessible workspaces: exactly the ones named in its `workspacePermissions`, nothing else. */
async function fetchApiKeyWorkspaces(keyAuth: TAuthenticationApiKey): Promise<TV3WorkspaceListItem[]> {
const workspaceIds = Array.from(new Set(keyAuth.workspacePermissions.map((p) => p.workspaceId)));
const workspaces = await Promise.all(workspaceIds.map((id) => getWorkspace(id)));
return workspaces
.filter((workspace): workspace is NonNullable<typeof workspace> => workspace !== null)
.map(serializeV3WorkspaceListItem);
}

/**
* List the workspaces the authenticated principal can access (session user or API key). There is no
* `workspaceId` input, so there is no IDOR surface — the result is always derived from the caller's own
* live membership / key grants, never from client-supplied ids.
*
* Thin orchestrator: resolve the principal's workspaces → dedupe → order → cap → respond. The per-
* principal access resolution lives in the `fetch*Workspaces` helpers.
*/
export async function listV3Workspaces({
authentication,
requestId,
instance,
}: TListV3WorkspacesParams): Promise<Response> {
if (!authentication) {
return problemUnauthorized(requestId, "Not authenticated", instance);
}

const log = logger.withContext({ requestId });

try {
let items: TV3WorkspaceListItem[];

if ("user" in authentication && authentication.user?.id) {
items = await fetchSessionWorkspaces(authentication.user.id);
} else if (
"apiKeyId" in authentication &&
authentication.apiKeyId &&
Array.isArray(authentication.workspacePermissions)
) {
items = await fetchApiKeyWorkspaces(authentication);
} else {
return problemUnauthorized(requestId, "Not authenticated", instance);
}

// Dedupe by id (defensive) + a stable, deterministic order — the underlying queries have no ORDER BY,
// so without this the output would vary between calls.
const deduped = Array.from(new Map(items.map((item) => [item.id, item])).values()).sort(
(a, b) => a.name.localeCompare(b.name) || a.id.localeCompare(b.id)
);

// No pagination: a principal's accessible workspaces are bounded by their memberships (small), so we
// return them all. No arbitrary cap — a truncation here would silently hide workspaces (the tool
// accepts no cursor) without bounding DB work, which was already done above.
return successListResponse(
deduped,
{ nextCursor: null, totalCount: deduped.length },
{ requestId, cache: "private, no-store" }
);
} catch (error) {
// Log every failure with request context (not just DatabaseError) so nothing significant is lost,
// and always return a clean 500 instead of throwing a raw error past this boundary.
log.error({ error, statusCode: 500 }, "Failed to list workspaces");
return problemInternalError(requestId, "An unexpected error occurred.", instance);
}
}
Loading
Loading