diff --git a/apps/web/app/api/mcp/route.test.ts b/apps/web/app/api/mcp/route.test.ts index 74c4be6b4f82..7cc1c7de3df7 100644 --- a/apps/web/app/api/mcp/route.test.ts +++ b/apps/web/app/api/mcp/route.test.ts @@ -43,6 +43,7 @@ vi.mock("@formbricks/database", () => ({ })); vi.mock("@/modules/auth/lib/oauth-urls", () => ({ + MCP_RESOURCE_SCOPES: ["surveys:read", "surveys:write"], getAuthIssuerUrl: () => "http://localhost/api/auth", getMcpOrigin: () => "http://localhost", getMcpProtectedResourceMetadataUrl: () => "http://localhost/.well-known/oauth-protected-resource/api/mcp", @@ -160,7 +161,7 @@ describe("POST /api/mcp", () => { expect(response.status).toBe(401); expect(response.headers.get("Content-Type")).toBe("application/problem+json"); expect(response.headers.get("WWW-Authenticate")).toBe( - 'Bearer resource_metadata="http://localhost/.well-known/oauth-protected-resource/api/mcp" scope="surveys:read"' + 'Bearer resource_metadata="http://localhost/.well-known/oauth-protected-resource/api/mcp" scope="surveys:read surveys:write"' ); expect(applyIPRateLimit).toHaveBeenCalled(); }); @@ -410,7 +411,7 @@ describe("POST /api/mcp", () => { expect(authenticateApiKeyFromHeaders).not.toHaveBeenCalled(); expect(applyIPRateLimit).toHaveBeenCalled(); expect(response.headers.get("WWW-Authenticate")).toBe( - 'Bearer resource_metadata="http://localhost/.well-known/oauth-protected-resource/api/mcp" scope="surveys:read"' + 'Bearer resource_metadata="http://localhost/.well-known/oauth-protected-resource/api/mcp" scope="surveys:read surveys:write"' ); }); diff --git a/apps/web/modules/auth/lib/mcp-oauth-dcr.test.ts b/apps/web/modules/auth/lib/mcp-oauth-dcr.test.ts index 0a743fdff7d0..710c04d5e2e8 100644 --- a/apps/web/modules/auth/lib/mcp-oauth-dcr.test.ts +++ b/apps/web/modules/auth/lib/mcp-oauth-dcr.test.ts @@ -114,6 +114,33 @@ describe("MCP OAuth Dynamic Client Registration → authorize (real-client shape expect(registration.body.scope?.split(" ")).toEqual(expect.arrayContaining(advertisedScopes)); }); + test("default registration (no scope requested) grants read + write", async () => { + const auth = createAuthInstance(); + + // A client that registers without an explicit scope must receive write by default — otherwise the + // consent screen only offers "Read surveys" and every write tool 403s (the ENG-1055 QA regression: + // clients that key off the challenge/defaults rather than the PRM never requested write). + const response = await auth.handler( + new Request(`${BASE_URL}/api/auth/oauth2/register`, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify({ + client_name: "MCP DCR default-scope client", + redirect_uris: [REDIRECT_URI], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }), + }) + ); + const body = (await response.json()) as { scope?: string }; + + expect(response.status).toBe(200); + expect(body.scope?.split(" ")).toEqual( + expect.arrayContaining(["surveys:read", "surveys:write", "offline_access"]) + ); + }); + test("authorize accepts the PRM-advertised scopes for a DCR client (no invalid_scope)", async () => { const auth = createAuthInstance(); const advertisedScopes = await fetchAdvertisedScopes(); diff --git a/apps/web/modules/auth/lib/mcp-oauth-provider-options.ts b/apps/web/modules/auth/lib/mcp-oauth-provider-options.ts index 70b0c28e13f0..8b22f229c7da 100644 --- a/apps/web/modules/auth/lib/mcp-oauth-provider-options.ts +++ b/apps/web/modules/auth/lib/mcp-oauth-provider-options.ts @@ -20,8 +20,18 @@ export const getMcpOauthProviderOptions = (): TOauthProviderOptions => ({ validAudiences: [getMcpResourceUrl()], allowDynamicClientRegistration: true, allowUnauthenticatedClientRegistration: true, - clientRegistrationDefaultScopes: ["openid", "profile", "email", "offline_access", "surveys:read"], - clientRegistrationAllowedScopes: ["surveys:write"], + // Register MCP clients with read + write by default so the consent screen offers write and the + // write tools are reachable (clients derive their DCR/authorize scopes from what we advertise, and + // the plugin validates authorize against the client's registered scopes). Granting the write scope + // is safe: actual write access is still enforced downstream by the user's workspace permissions. + clientRegistrationDefaultScopes: [ + "openid", + "profile", + "email", + "offline_access", + "surveys:read", + "surveys:write", + ], accessTokenExpiresIn: 15 * 60, refreshTokenExpiresIn: 30 * 24 * 60 * 60, scopeExpirations: { diff --git a/apps/web/modules/mcp/auth.test.ts b/apps/web/modules/mcp/auth.test.ts index 8c29135e1024..7fbef0a52ca4 100644 --- a/apps/web/modules/mcp/auth.test.ts +++ b/apps/web/modules/mcp/auth.test.ts @@ -105,6 +105,9 @@ describe("authenticateMcpRequest", () => { expect(result.ok).toBe(false); if (!result.ok) { expect(result.response.status).toBe(401); + // The challenge must advertise read + write so clients request write at consent and can reach + // the write tools (advertising only read is why write was unreachable — ENG-1055 QA). + expect(result.response.headers.get("WWW-Authenticate")).toContain('scope="surveys:read surveys:write"'); expect(await result.response.json()).toMatchObject({ code: "not_authenticated", detail: "API key or OAuth access token required", diff --git a/apps/web/modules/mcp/auth.ts b/apps/web/modules/mcp/auth.ts index c3491823730b..ac5bc39d11ff 100644 --- a/apps/web/modules/mcp/auth.ts +++ b/apps/web/modules/mcp/auth.ts @@ -19,6 +19,7 @@ import { parseApiKeyV2 } from "@/lib/crypto"; import { authenticateApiKeyFromHeaders, getBearerTokenFromHeaders } from "@/modules/api/lib/api-key-auth"; import { auth } from "@/modules/auth/lib/auth"; import { + MCP_RESOURCE_SCOPES, getAuthIssuerUrl, getMcpOrigin, getMcpProtectedResourceMetadataUrl, @@ -36,7 +37,13 @@ const QUERY_CREDENTIAL_PARAMS = new Set([ "authorization", ]); +// Minimum scope required to authenticate against the MCP server at all. const DEFAULT_OAUTH_SCOPE = "surveys:read"; +// Scopes advertised in the 401 WWW-Authenticate challenge. Clients build their DCR + authorize +// requests from this, so it must list every resource scope (read + write) or clients only ever +// request read and can never reach the write tools. Actual write access is still gated downstream +// by the user's workspace permissions in the v3 layer. +const MCP_CHALLENGE_SCOPE = MCP_RESOURCE_SCOPES.join(" "); const oauthResourceClient = oauthProviderResourceClient(auth); export type TMcpAuthInfo = AuthInfo & { @@ -198,7 +205,7 @@ export function withMcpResponseHeaders(response: Response, requestId: string): R }); } -function withOAuthChallenge(response: Response, scope = DEFAULT_OAUTH_SCOPE): Response { +function withOAuthChallenge(response: Response, scope = MCP_CHALLENGE_SCOPE): Response { const headers = new Headers(response.headers); headers.set( "WWW-Authenticate", diff --git a/apps/web/modules/mcp/tools/surveys.test.ts b/apps/web/modules/mcp/tools/surveys.test.ts index 8f015729aa17..08caa1bfd84e 100644 --- a/apps/web/modules/mcp/tools/surveys.test.ts +++ b/apps/web/modules/mcp/tools/surveys.test.ts @@ -503,8 +503,11 @@ describe("registerSurveyTools", () => { }); }); - test("validate_survey rejects write validations for read-only OAuth scopes", async () => { + test("validate_survey allows patch validations for read-only OAuth scopes", async () => { const { tools } = createToolServer(); + vi.mocked(validateV3SurveyFromRawInput).mockResolvedValue( + successResponse({ valid: true, operation: "patch", invalid_params: [] }, { requestId: "req_tool" }) + ); const result = await tools.get("validate_survey")!.handler( { @@ -517,12 +520,10 @@ describe("registerSurveyTools", () => { { authInfo: readOnlyOAuthAuthInfo } ); - expect(validateV3SurveyFromRawInput).not.toHaveBeenCalled(); - expect(result.isError).toBe(true); - expect(result.structuredContent.error).toMatchObject({ - status: 403, - code: "forbidden", - detail: "OAuth token does not include the required MCP scope", + expect(validateV3SurveyFromRawInput).toHaveBeenCalled(); + expect(result.isError).toBeFalsy(); + expect(result.structuredContent).toEqual({ + data: { valid: true, operation: "patch", invalid_params: [] }, requestId: "req_tool", }); }); diff --git a/apps/web/modules/mcp/tools/surveys.ts b/apps/web/modules/mcp/tools/surveys.ts index f680a65bb4f6..8c2d24f1d91e 100644 --- a/apps/web/modules/mcp/tools/surveys.ts +++ b/apps/web/modules/mcp/tools/surveys.ts @@ -215,10 +215,10 @@ export function registerSurveyTools(server: McpServer): void { }, async (input: TMcpValidateSurveyInput, extra) => { const requestId = getMcpRequestId(extra.authInfo); - const requiredScopes = - input.operation === "patch" || input.operation === "create" ? ["surveys:write"] : ["surveys:read"]; - - const scopeError = await guardMcpScopes(extra.authInfo, requiredScopes, requestId); + // validate_survey never persists changes (readOnlyHint) — a dry-run validation of a create or + // patch payload only needs read access. The actual write permission is enforced by the v3 layer + // when create_survey / patch_survey run. + const scopeError = await guardMcpScopes(extra.authInfo, ["surveys:read"], requestId); if (scopeError) { return scopeError; }