fix(oauth): return failed callbacks to setup

Co-Authored-By: Paperclip <noreply@paperclip.ing>

server/src/__tests__/generic-mcp-connection.test.ts

@@ -920,6 +920,41 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
     await expect(db.select().from(toolOauthStates).where(eq(toolOauthStates.state, state))).resolves.toHaveLength(0);
   });
 
+  it("returns browser denials to setup without reflecting provider-authored details", async () => {
+    installMcpOAuthFixture({ auth: "oauth" });
+    const company = await createCompany(db);
+    const service = toolAccessService(db);
+    const app = createRouteApp(db);
+    const connected = await service.connectGalleryApp(company.id, {
+      link: MCP_URL,
+      name: "Fixture browser denial",
+    });
+    const start = await service.startOAuth(company.id, connected.connectionId, {
+      redirectUri: REDIRECT_URI,
+      actor: { actorType: "user", actorId: "board-user" },
+    });
+    const state = new URL(start.authorizationUrl).searchParams.get("state")!;
+
+    const res = await request(app)
+      .get("/api/tools/oauth/callback")
+      .set("Accept", "text/html")
+      .query({
+        state,
+        error: "access_denied",
+        error_description: HOSTILE_ERROR_DESCRIPTION,
+        error_uri: HOSTILE_ERROR_BODY.error_uri,
+      });
+
+    expect(res.status).toBe(303);
+    const location = new URL(res.headers.location, PUBLIC_BASE_URL);
+    expect(location.pathname).toBe(`/${company.issuePrefix}/apps/${connected.connectionId}/setup`);
+    expect(location.searchParams.get("oauth")).toBe("denied");
+    expect(location.searchParams.get("code")).toBe("oauth_authorization_denied");
+    expect(res.headers.location).not.toContain(PROVIDER_CANARY);
+    expect(res.headers.location).not.toContain("error_description");
+    expect(res.headers.location).not.toContain("error_uri");
+  });
+
   it("validates the callback state before acting on a provider-reported error", async () => {
     installMcpOAuthFixture({ auth: "oauth" });
     const company = await createCompany(db);

server/src/routes/tool-access.ts

@@ -120,6 +120,16 @@ export function toolAccessRoutes(
     }
     return new URL("/api/tools/oauth/callback", configured).toString();
   }
+
+  async function oauthSetupPath(companyId: string, connectionId: string) {
+    const [company] = await db
+      .select({ issuePrefix: companies.issuePrefix })
+      .from(companies)
+      .where(eq(companies.id, companyId))
+      .limit(1);
+    if (!company) throw new Error("OAuth callback connection belongs to a missing company");
+    return `/${company.issuePrefix}/apps/${connectionId}/setup`;
+  }
   const access = accessService(db);
 
   async function assertBoardToolPermission(req: Request, companyId: string, permissionKey: PermissionKey) {
@@ -370,14 +380,34 @@ export function toolAccessRoutes(
       throw badRequest("Invalid or expired OAuth state");
     }
     assertToolAppMutationAccess(req, pendingState.companyId);
-    const result = await svc.completeOAuthCallback({
-      state,
-      code,
-      error,
-      iss,
-      redirectUri: oauthRedirectUri(),
-      actor: getActorInfo(req),
-    });
+    const acceptsHtml = req.get("accept")?.includes("text/html") === true;
+    let result: Awaited<ReturnType<typeof svc.completeOAuthCallback>>;
+    try {
+      result = await svc.completeOAuthCallback({
+        state,
+        code,
+        error,
+        iss,
+        redirectUri: oauthRedirectUri(),
+        actor: getActorInfo(req),
+      });
+    } catch (callbackError) {
+      if (!acceptsHtml) throw callbackError;
+      const details = callbackError instanceof HttpError
+        && callbackError.details
+        && typeof callbackError.details === "object"
+        && !Array.isArray(callbackError.details)
+        ? callbackError.details as Record<string, unknown>
+        : null;
+      const callbackErrorCode = typeof details?.code === "string" ? details.code : null;
+      const params = new URLSearchParams({
+        oauth: callbackErrorCode === "oauth_authorization_denied" ? "denied" : "failed",
+      });
+      if (callbackErrorCode) params.set("code", callbackErrorCode);
+      const setupPath = await oauthSetupPath(pendingState.companyId, pendingState.connectionId);
+      res.redirect(303, `${setupPath}?${params.toString()}`);
+      return;
+    }
     await logActivity(db, {
       companyId: result.connection.companyId,
       actorType: "user",
@@ -390,14 +420,9 @@ export function toolAccessRoutes(
         catalogEntryCount: result.catalog.length,
       },
     });
-    if (req.get("accept")?.includes("text/html")) {
-      const [company] = await db
-        .select({ issuePrefix: companies.issuePrefix })
-        .from(companies)
-        .where(eq(companies.id, result.connection.companyId))
-        .limit(1);
-      if (!company) throw new Error("OAuth callback connection belongs to a missing company");
-      res.redirect(303, `/${company.issuePrefix}/apps/${result.connection.id}/setup?oauth=connected`);
+    if (acceptsHtml) {
+      const setupPath = await oauthSetupPath(result.connection.companyId, result.connection.id);
+      res.redirect(303, `${setupPath}?oauth=connected`);
       return;
     }
     res.json(result);

server/src/services/tool-access.ts

@@ -7045,7 +7045,10 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {}
 
   async function peekOAuthState(state: string) {
     const [row] = await db
-      .select({ companyId: toolOauthStates.companyId })
+      .select({
+        companyId: toolOauthStates.companyId,
+        connectionId: toolOauthStates.connectionId,
+      })
       .from(toolOauthStates)
       .where(eq(toolOauthStates.state, state))
       .limit(1);