From 5caf46a24b92e4a10f7fa24ffff6c6bcbc13c1da Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 17:42:42 -0400 Subject: [PATCH 01/14] feat(api): add OAuth provider for keyless hosted MCP auth MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Enable better-auth's mcp plugin (wraps oidcProvider) so the Gram-hosted MCP server authenticates users via OAuth ("Sign in with Google") instead of a pasted API key. - add OauthApplication/OauthAccessToken/OauthConsent tables (+ migration) - register Gram as an env-driven trusted OAuth client (inert when env unset) - HybridAuthGuard: validate MCP OAuth tokens via getMcpSession and bind org explicitly from active memberships (device-agent pattern) — single org binds, multiple orgs fail closed to avoid wrong-tenant access - add 6 tests for the MCP OAuth path Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/.env.example | 8 + apps/api/src/auth/auth.server.ts | 47 +++++ apps/api/src/auth/hybrid-auth.guard.spec.ts | 187 ++++++++++++++++++ apps/api/src/auth/hybrid-auth.guard.ts | 84 ++++++++ .../migration.sql | 85 ++++++++ packages/db/prisma/schema/auth.prisma | 66 +++++++ 6 files changed, 477 insertions(+) create mode 100644 apps/api/src/auth/hybrid-auth.guard.spec.ts create mode 100644 packages/db/prisma/migrations/20260528213310_add_oauth_provider_tables/migration.sql diff --git a/apps/api/.env.example b/apps/api/.env.example index 9e8714d2b7..d39ca5dc1d 100644 --- a/apps/api/.env.example +++ b/apps/api/.env.example @@ -18,6 +18,14 @@ AUTH_MICROSOFT_CLIENT_ID= AUTH_MICROSOFT_CLIENT_SECRET= AUTH_MICROSOFT_TENANT_ID= # 'common' (default), 'organizations', or your tenant GUID +# Hosted MCP (Speakeasy Gram) OAuth — better-auth acts as the OAuth provider. +# Leave unset where hosted MCP isn't configured; the trusted client is then inert. +GRAM_OAUTH_CLIENT_ID= # OAuth client id registered for the Gram MCP server +GRAM_OAUTH_CLIENT_SECRET= # OAuth client secret for the Gram MCP server +GRAM_OAUTH_REDIRECT_URI= # Gram callback, e.g. https:///oauth//callback +MCP_OAUTH_LOGIN_PAGE= # App sign-in page (defaults to ${NEXT_PUBLIC_APP_URL}/auth) +MCP_RESOURCE_URL= # Optional: the hosted MCP resource identifier (Gram server URL) + DATABASE_URL= NOVU_API_KEY= diff --git a/apps/api/src/auth/auth.server.ts b/apps/api/src/auth/auth.server.ts index 3f7a8f4611..46e9de293b 100644 --- a/apps/api/src/auth/auth.server.ts +++ b/apps/api/src/auth/auth.server.ts @@ -10,6 +10,7 @@ import { bearer, emailOTP, magicLink, + mcp, multiSession, organization, } from 'better-auth/plugins'; @@ -188,6 +189,38 @@ if ( const cookieDomain = getCookieDomain(); +// ── Hosted MCP (Speakeasy Gram) OAuth ──────────────────────────────────────── +// The MCP server is hosted on Gram. Gram obtains an OAuth access token from this +// API (better-auth as the authorization server) so users authenticate with +// "Sign in with Google" instead of pasting an API key. +// +// Gram's OAuth Proxy registers as a single static client and handles Dynamic +// Client Registration toward MCP clients on our behalf — so we keep public DCR +// off for now and register Gram as a trusted client. Configured via env so the +// secret isn't committed and the plugin is inert in envs where hosted MCP isn't +// set up yet. +const gramMcpClient = + process.env.GRAM_OAUTH_CLIENT_ID && + process.env.GRAM_OAUTH_CLIENT_SECRET && + process.env.GRAM_OAUTH_REDIRECT_URI + ? { + clientId: process.env.GRAM_OAUTH_CLIENT_ID, + clientSecret: process.env.GRAM_OAUTH_CLIENT_SECRET, + name: 'Comp AI MCP (Gram)', + type: 'web' as const, + disabled: false, + redirectUrls: [process.env.GRAM_OAUTH_REDIRECT_URI], + metadata: null, + skipConsent: false, + } + : null; + +// Where better-auth sends the user to authenticate during the OAuth flow. +// Must point at the app's sign-in page. Override per environment via env. +const mcpLoginPage = + process.env.MCP_OAUTH_LOGIN_PAGE || + `${process.env.NEXT_PUBLIC_APP_URL ?? 'https://app.trycomp.ai'}/auth`; + // ============================================================================= // Security Validation // ============================================================================= @@ -488,6 +521,20 @@ export const auth = betterAuth({ admin({ defaultRole: 'user', }), + // OAuth 2.0 / OIDC provider for hosted MCP (Gram). Wraps oidcProvider and + // exposes /api/auth/oauth2/* + /api/auth/.well-known/* and the + // auth.api.getMcpSession() helper used by HybridAuthGuard. + mcp({ + loginPage: mcpLoginPage, + ...(process.env.MCP_RESOURCE_URL + ? { resource: process.env.MCP_RESOURCE_URL } + : {}), + oidcConfig: { + loginPage: mcpLoginPage, + allowDynamicClientRegistration: false, + ...(gramMcpClient ? { trustedClients: [gramMcpClient] } : {}), + }, + }), ], socialProviders, user: { diff --git a/apps/api/src/auth/hybrid-auth.guard.spec.ts b/apps/api/src/auth/hybrid-auth.guard.spec.ts new file mode 100644 index 0000000000..a4d6c50f5f --- /dev/null +++ b/apps/api/src/auth/hybrid-auth.guard.spec.ts @@ -0,0 +1,187 @@ +import { Test, TestingModule } from '@nestjs/testing'; +import { ExecutionContext, UnauthorizedException } from '@nestjs/common'; +import { Reflector } from '@nestjs/core'; +import { HybridAuthGuard } from './hybrid-auth.guard'; +import { ApiKeyService } from './api-key.service'; + +// Mock auth.server — only the two session resolvers the guard uses. +const mockGetSession = jest.fn(); +const mockGetMcpSession = jest.fn(); +jest.mock('./auth.server', () => ({ + auth: { + api: { + getSession: (...args: unknown[]) => mockGetSession(...args), + getMcpSession: (...args: unknown[]) => mockGetMcpSession(...args), + }, + }, +})); + +// Mock @db — the guard resolves the user, then enumerates active memberships +// (device-agent style) to bind the organization for the MCP OAuth path. +const mockUserFindUnique = jest.fn(); +const mockMemberFindMany = jest.fn(); +jest.mock('@db', () => ({ + db: { + user: { findUnique: (...args: unknown[]) => mockUserFindUnique(...args) }, + member: { findMany: (...args: unknown[]) => mockMemberFindMany(...args) }, + }, +})); + +// Avoid ESM issues from the api-key.service import chain (it imports @trycompai/auth). +jest.mock('@trycompai/auth', () => ({})); + +describe('HybridAuthGuard — MCP OAuth path', () => { + let guard: HybridAuthGuard; + let reflector: Reflector; + + // A real object so the guard's mutations (userId, userRoles, …) are observable. + const createContext = ( + headers: Record, + ): { context: ExecutionContext; request: Record } => { + const request: Record = { headers }; + const context = { + switchToHttp: () => ({ getRequest: () => request }), + getHandler: () => jest.fn(), + getClass: () => jest.fn(), + } as unknown as ExecutionContext; + return { context, request }; + }; + + beforeEach(async () => { + jest.clearAllMocks(); + const module: TestingModule = await Test.createTestingModule({ + providers: [ + HybridAuthGuard, + { + provide: ApiKeyService, + useValue: { extractApiKey: jest.fn(), validateApiKey: jest.fn() }, + }, + Reflector, + ], + }).compile(); + + guard = module.get(HybridAuthGuard); + reflector = module.get(Reflector); + // Not public, and don't skip the org check. + jest.spyOn(reflector, 'getAllAndOverride').mockReturnValue(false); + // No cookie/regular session → forces the MCP OAuth fallback. + mockGetSession.mockResolvedValue(null); + }); + + it('authenticates a single-org user (admin) and binds org + roles', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_1', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_1', + email: 'admin@acme.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_1', role: 'owner,admin', department: 'it', organizationId: 'org_1' }, + ]); + + const { context, request } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(request.userId).toBe('usr_1'); + expect(request.organizationId).toBe('org_1'); + expect(request.userRoles).toEqual(['owner', 'admin']); + expect(request.authType).toBe('session'); + expect(request.isApiKey).toBe(false); + expect(request.memberId).toBe('mem_1'); + }); + + it('authenticates a read-only member and surfaces their role for RBAC', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_2', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_2', + email: 'auditor@acme.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_2', role: 'auditor', department: 'none', organizationId: 'org_1' }, + ]); + + const { context, request } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(request.userRoles).toEqual(['auditor']); + expect(request.organizationId).toBe('org_1'); + }); + + it('rejects when the bearer token is not a valid MCP OAuth token', async () => { + mockGetMcpSession.mockResolvedValue(null); + + const { context } = createContext({ + authorization: 'Bearer not_a_token', + }); + + await expect(guard.canActivate(context)).rejects.toThrow( + UnauthorizedException, + ); + expect(mockUserFindUnique).not.toHaveBeenCalled(); + }); + + it('rejects a valid token whose user has no organization', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_3', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_3', + email: 'orphan@acme.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([]); + + const { context } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).rejects.toThrow( + 'No active organization', + ); + }); + + it('fails closed for a multi-org user (no silent tenant selection)', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_5', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_5', + email: 'consultant@acme.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_a', role: 'admin', department: 'none', organizationId: 'org_a' }, + { id: 'mem_b', role: 'owner', department: 'none', organizationId: 'org_b' }, + ]); + + const { context, request } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).rejects.toThrow( + 'multiple organizations', + ); + // No tenant must have been bound. + expect(request.organizationId).toBe(''); + }); + + it('marks platform admins from the user role', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_4', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_4', + email: 'staff@trycomp.ai', + role: 'admin', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_4', role: 'owner', department: 'none', organizationId: 'org_1' }, + ]); + + const { context, request } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(request.isPlatformAdmin).toBe(true); + }); +}); diff --git a/apps/api/src/auth/hybrid-auth.guard.ts b/apps/api/src/auth/hybrid-auth.guard.ts index c97f2b43bf..09b87c1680 100644 --- a/apps/api/src/auth/hybrid-auth.guard.ts +++ b/apps/api/src/auth/hybrid-auth.guard.ts @@ -169,6 +169,11 @@ export class HybridAuthGuard implements CanActivate { const session = await auth.api.getSession({ headers }); if (!session) { + // Fallback: the hosted MCP server (Gram) sends an OAuth access token as a + // Bearer token, which getSession does not resolve. Try the MCP OAuth path. + if (await this.tryMcpOAuthAuth(request, headers, skipOrgCheck)) { + return true; + } throw new UnauthorizedException('Invalid or expired session'); } @@ -248,4 +253,83 @@ export class HybridAuthGuard implements CanActivate { throw new UnauthorizedException('Invalid or expired session'); } } + + /** + * Resolve a hosted-MCP OAuth access token (issued by better-auth's mcp/oidc + * provider and forwarded by the Gram-hosted MCP server). Populates the request + * context and returns true on success; returns false when the bearer token is + * not a valid MCP OAuth token (so the caller throws the generic 401). Throws + * when no organization can be resolved. + * + * The token carries the user identity only. The organization is resolved + * explicitly from the user's active memberships — the same approach as the + * device-agent (enumerate memberships, then bind to one), not a "most recent" + * guess. One org is used directly; multiple orgs fail closed because MCP org + * selection isn't supported yet (avoids silently acting on the wrong tenant). + * Roles come from the resolved member so the existing PermissionGuard enforces + * RBAC unchanged. + */ + private async tryMcpOAuthAuth( + request: AuthenticatedRequest, + headers: Headers, + skipOrgCheck: boolean, + ): Promise { + const token = await auth.api.getMcpSession({ headers }).catch(() => null); + if (!token?.userId) { + return false; + } + + const userId = token.userId; + const user = await db.user.findUnique({ + where: { id: userId }, + select: { id: true, email: true, role: true }, + }); + if (!user) { + return false; + } + + request.userId = user.id; + request.userEmail = user.email; + request.userRoles = null; + request.organizationId = ''; + request.authType = 'session'; + request.isApiKey = false; + request.isServiceToken = false; + request.isPlatformAdmin = user.role === 'admin'; + + if (skipOrgCheck) { + this.logger.log(`MCP OAuth token authenticated for user ${user.id}`); + return true; + } + + // Bind the organization explicitly by enumerating active memberships, + // mirroring the device-agent's getMyOrganizations + explicit selection. + const memberships = await db.member.findMany({ + where: { userId, deactivated: false }, + select: { id: true, role: true, department: true, organizationId: true }, + }); + + if (memberships.length === 0) { + throw new UnauthorizedException( + 'No active organization for this MCP token.', + ); + } + if (memberships.length > 1) { + throw new UnauthorizedException( + 'This account belongs to multiple organizations. Selecting an ' + + 'organization for MCP access is not supported yet.', + ); + } + + const member = memberships[0]; + request.organizationId = member.organizationId; + request.memberId = member.id; + request.memberDepartment = member.department; + request.userRoles = member.role ? member.role.split(',') : null; + + this.logger.log( + `MCP OAuth token authenticated for user ${user.id} (org ${member.organizationId})`, + ); + return true; + } } diff --git a/packages/db/prisma/migrations/20260528213310_add_oauth_provider_tables/migration.sql b/packages/db/prisma/migrations/20260528213310_add_oauth_provider_tables/migration.sql new file mode 100644 index 0000000000..f9e6b253e2 --- /dev/null +++ b/packages/db/prisma/migrations/20260528213310_add_oauth_provider_tables/migration.sql @@ -0,0 +1,85 @@ +-- CreateTable +CREATE TABLE "oauth_application" ( + "id" TEXT NOT NULL DEFAULT generate_prefixed_cuid('oap'::text), + "name" TEXT NOT NULL, + "icon" TEXT, + "metadata" TEXT, + "clientId" TEXT NOT NULL, + "clientSecret" TEXT, + "redirectUrls" TEXT NOT NULL, + "type" TEXT NOT NULL, + "disabled" BOOLEAN DEFAULT false, + "userId" TEXT, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updatedAt" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "oauth_application_pkey" PRIMARY KEY ("id") +); + +-- CreateTable +CREATE TABLE "oauth_access_token" ( + "id" TEXT NOT NULL DEFAULT generate_prefixed_cuid('oat'::text), + "accessToken" TEXT NOT NULL, + "refreshToken" TEXT NOT NULL, + "accessTokenExpiresAt" TIMESTAMP(3) NOT NULL, + "refreshTokenExpiresAt" TIMESTAMP(3) NOT NULL, + "clientId" TEXT NOT NULL, + "userId" TEXT, + "scopes" TEXT NOT NULL, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updatedAt" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "oauth_access_token_pkey" PRIMARY KEY ("id") +); + +-- CreateTable +CREATE TABLE "oauth_consent" ( + "id" TEXT NOT NULL DEFAULT generate_prefixed_cuid('oac'::text), + "clientId" TEXT NOT NULL, + "userId" TEXT NOT NULL, + "scopes" TEXT NOT NULL, + "consentGiven" BOOLEAN NOT NULL, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updatedAt" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "oauth_consent_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE UNIQUE INDEX "oauth_application_clientId_key" ON "oauth_application"("clientId"); + +-- CreateIndex +CREATE INDEX "oauth_application_userId_idx" ON "oauth_application"("userId"); + +-- CreateIndex +CREATE UNIQUE INDEX "oauth_access_token_accessToken_key" ON "oauth_access_token"("accessToken"); + +-- CreateIndex +CREATE UNIQUE INDEX "oauth_access_token_refreshToken_key" ON "oauth_access_token"("refreshToken"); + +-- CreateIndex +CREATE INDEX "oauth_access_token_clientId_idx" ON "oauth_access_token"("clientId"); + +-- CreateIndex +CREATE INDEX "oauth_access_token_userId_idx" ON "oauth_access_token"("userId"); + +-- CreateIndex +CREATE INDEX "oauth_consent_clientId_idx" ON "oauth_consent"("clientId"); + +-- CreateIndex +CREATE INDEX "oauth_consent_userId_idx" ON "oauth_consent"("userId"); + +-- AddForeignKey +ALTER TABLE "oauth_application" ADD CONSTRAINT "oauth_application_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "oauth_access_token" ADD CONSTRAINT "oauth_access_token_clientId_fkey" FOREIGN KEY ("clientId") REFERENCES "oauth_application"("clientId") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "oauth_access_token" ADD CONSTRAINT "oauth_access_token_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "oauth_consent" ADD CONSTRAINT "oauth_consent_clientId_fkey" FOREIGN KEY ("clientId") REFERENCES "oauth_application"("clientId") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "oauth_consent" ADD CONSTRAINT "oauth_consent_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; diff --git a/packages/db/prisma/schema/auth.prisma b/packages/db/prisma/schema/auth.prisma index 45b82fba9f..2f16e5b550 100644 --- a/packages/db/prisma/schema/auth.prisma +++ b/packages/db/prisma/schema/auth.prisma @@ -32,6 +32,10 @@ model User { offboardingChecklistCompletions OffboardingChecklistCompletion[] @relation("OffboardingChecklistCompletedBy") offboardingAccessRevocations OffboardingAccessRevocation[] @relation("AccessRevocationRevokedBy") + oauthApplications OauthApplication[] + oauthAccessTokens OauthAccessToken[] + oauthConsents OauthConsent[] + @@unique([email]) } @@ -95,6 +99,68 @@ model Verification { updatedAt DateTime @updatedAt } +// ── OAuth 2.0 / OIDC Provider — required by better-auth's mcp/oidcProvider plugin. +// The MCP server is hosted on Speakeasy Gram; Gram obtains an OAuth access token +// from this API (better-auth as the authorization server) so end users sign in +// with Google instead of pasting an API key. +// Field names MUST match better-auth's expected schema (clientId, redirectUrls, …). +model OauthApplication { + id String @id @default(dbgenerated("generate_prefixed_cuid('oap'::text)")) + name String + icon String? + metadata String? + clientId String @unique + clientSecret String? + redirectUrls String + type String + disabled Boolean? @default(false) + userId String? + user User? @relation(fields: [userId], references: [id], onDelete: Cascade) + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + + accessTokens OauthAccessToken[] + consents OauthConsent[] + + @@index([userId]) + @@map("oauth_application") +} + +model OauthAccessToken { + id String @id @default(dbgenerated("generate_prefixed_cuid('oat'::text)")) + accessToken String @unique + refreshToken String @unique + accessTokenExpiresAt DateTime + refreshTokenExpiresAt DateTime + clientId String + application OauthApplication @relation(fields: [clientId], references: [clientId], onDelete: Cascade) + userId String? + user User? @relation(fields: [userId], references: [id], onDelete: Cascade) + scopes String + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + + @@index([clientId]) + @@index([userId]) + @@map("oauth_access_token") +} + +model OauthConsent { + id String @id @default(dbgenerated("generate_prefixed_cuid('oac'::text)")) + clientId String + application OauthApplication @relation(fields: [clientId], references: [clientId], onDelete: Cascade) + userId String + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + scopes String + consentGiven Boolean + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + + @@index([clientId]) + @@index([userId]) + @@map("oauth_consent") +} + // JWT Plugin - Required by Better Auth JWT plugin // https://www.better-auth.com/docs/plugins/jwt model Jwks { From 0a3e2af6038b61158ee7bced9af486957d3e3e7c Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 17:58:32 -0400 Subject: [PATCH 02/14] docs: require endpoint summary+description for MCP dynamic-toolset discovery Add Rule 11 to the api-endpoint-contract skill + cursor rule: every endpoint needs a meaningful @ApiOperation summary/description (already enforced by openapi-docs.spec.ts), now correctness-critical because Gram dynamic toolsets discover tools via semantic search over descriptions. Co-Authored-By: Claude Opus 4.8 (1M context) --- .claude/skills/api-endpoint-contract/SKILL.md | 25 +++++++++++++++++-- .cursor/rules/api-endpoint-contract.mdc | 8 +++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/.claude/skills/api-endpoint-contract/SKILL.md b/.claude/skills/api-endpoint-contract/SKILL.md index 32d292d85c..811051d57b 100644 --- a/.claude/skills/api-endpoint-contract/SKILL.md +++ b/.claude/skills/api-endpoint-contract/SKILL.md @@ -13,7 +13,7 @@ Every customer-facing endpoint in `apps/api/src/` ends up in three places: If any one of these three is wrong, the endpoint either silently breaks for agents (Claude Desktop, Cursor, Codex, etc.) or fails validation at runtime. **Follow this contract on every body-accepting endpoint.** -## The 10 rules +## The 11 rules ### 1. DTOs MUST be classes — never interfaces, never inline types @@ -156,10 +156,30 @@ SSE streams (`@ApiProduces('text/event-stream')`) and binary file responses (`@R disabled: true ``` +### 11. Every endpoint MUST have a meaningful summary + description — it powers MCP discovery + +`@ApiOperation({ summary, description })` is **not optional**. `openapi-docs.spec.ts` (via `collectPublicOpenApiIssues` in `apps/api/src/openapi/public-docs-quality.ts`) **fails CI** if any non-excluded operation has: +- an empty `summary` → `missingSummaries` +- a missing `description` or SEO metadata → `missingMetadata` +- SEO metadata outside 80–160 chars, or a title > 60 chars → `invalidSeo` + +This matters more now that the hosted MCP (Gram) uses **dynamic toolsets**: with 300+ tools the agent never sees them all — it runs a semantic `search` over tool **names + descriptions** and only loads matches. A tool with a weak or missing description is effectively **undiscoverable**. The description is the tool's only chance of being found. + +```ts +@ApiOperation({ + summary: 'List compliance policies', // concise tool title + description: + "Returns the organization's compliance policies (SOC 2, ISO 27001, …) " + + 'with status and owner. Use to review or audit policy coverage.', // what it does + when to use it +}) +``` + +Write the description for the agent deciding *whether to call this tool*: state what it does and when to use it. (Keep it ≤ 240 chars — see Rule 4.) + ## Workflow checklist when adding a body endpoint 1. Define a `class` DTO. Two decorator stacks on every field. Add `@ApiBody({ type: DtoClass })` on the endpoint. -2. Keep `@ApiOperation.description` ≤ 240 chars. +2. Give the endpoint a meaningful `@ApiOperation({ summary, description })` — both required, CI-enforced by `openapi-docs.spec.ts`, and they power MCP dynamic-toolset discovery (Rule 11). Keep the description ≤ 240 chars (Rule 4). 3. If the auto-derived MCP tool name is ugly, set `@ApiExtension('x-speakeasy-mcp', { name: '...' })`. 4. If the endpoint requires session auth, decide: remove `SessionOnlyGuard`, or disable it for MCP via the overlay. 5. For long-running work, return a run handle and document the poll target. @@ -186,5 +206,6 @@ Every bug below was a real customer-visible MCP failure caught during the May 20 | Agent uploads stuck for 15+ min on base64 encoding | Tool accepted `fileData` as the only file input | Rule 8 | | Agent calls SSE auto-answer and hangs | Tool was generated from `@ApiProduces('text/event-stream')` | Rule 10 | | Agent tries to start OAuth and gets 403 | Endpoint was behind `SessionOnlyGuard` but generated as MCP tool | Rule 6 | +| Agent can't find a tool that exists (dynamic toolsets) | Endpoint had a missing/weak description → invisible to semantic search | Rule 11 | Follow the 10 rules and you avoid every one of these. diff --git a/.cursor/rules/api-endpoint-contract.mdc b/.cursor/rules/api-endpoint-contract.mdc index 2832e236af..d8d2187232 100644 --- a/.cursor/rules/api-endpoint-contract.mdc +++ b/.cursor/rules/api-endpoint-contract.mdc @@ -7,7 +7,7 @@ alwaysApply: false Every customer-facing endpoint in `apps/api/src/` flows into three systems: the OpenAPI spec (`packages/docs/openapi.json`), the MCP server (`@trycompai/mcp-server` on npm), and the runtime `ValidationPipe`. If any one is wrong, the endpoint silently breaks for agents or fails validation at runtime. -## The 10 rules +## The 11 rules ### 1. DTOs MUST be classes — never interfaces, never inline types @@ -99,6 +99,12 @@ SSE streams (`@ApiProduces('text/event-stream')`) and binary file responses cann disabled: true ``` +### 11. Every endpoint MUST have a meaningful summary + description — it powers MCP discovery + +`@ApiOperation({ summary, description })` is **not optional**. `openapi-docs.spec.ts` (via `collectPublicOpenApiIssues`) **fails CI** on empty `summary` (`missingSummaries`), missing `description`/metadata (`missingMetadata`), or SEO metadata outside 80–160 chars / title > 60 (`invalidSeo`). + +The hosted MCP (Gram) uses **dynamic toolsets**: with 300+ tools the agent semantic-`search`es over names + descriptions and only loads matches. A weak/missing description = the tool is **undiscoverable**. Write it for the agent deciding whether to call the tool: what it does + when to use it (≤ 240 chars, Rule 4). + ## Checklist when adding a body endpoint 1. `class` DTO, two decorator stacks per field, `@ApiBody({ type: DtoClass })` on the endpoint. From 38ac017170d27d317be0a3c2862176822d9b779e Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 18:01:30 -0400 Subject: [PATCH 03/14] fix(api): add OpenAPI summaries + descriptions to offboarding endpoints The offboarding-checklist endpoints were missing @ApiOperation summaries/descriptions, failing the OpenAPI quality contract (openapi-docs.spec.ts: missingSummaries + invalidSeo). Added a meaningful summary + ~120-155 char description to all 15 endpoints (the SEO check requires the generated description to be >= 80 chars) and regenerated packages/docs/openapi.json. Descriptions also power the hosted MCP's dynamic-toolset semantic search, so these tools are now discoverable by agents. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../offboarding-checklist.controller.ts | 78 +++++++++- packages/docs/openapi.json | 146 ++++++++++++++---- 2 files changed, 189 insertions(+), 35 deletions(-) diff --git a/apps/api/src/offboarding-checklist/offboarding-checklist.controller.ts b/apps/api/src/offboarding-checklist/offboarding-checklist.controller.ts index 32d042499c..054e251fe9 100644 --- a/apps/api/src/offboarding-checklist/offboarding-checklist.controller.ts +++ b/apps/api/src/offboarding-checklist/offboarding-checklist.controller.ts @@ -44,7 +44,11 @@ export class OffboardingChecklistController { @Get('pending') @RequirePermission('member', 'read') - @ApiOperation({ summary: 'Get members with pending offboarding checklists' }) + @ApiOperation({ + summary: 'Get members with pending offboarding checklists', + description: + 'Lists members whose offboarding checklist is still incomplete, with their outstanding items, so you can track and finish departing-employee offboarding.', + }) async getPendingOffboardings( @OrganizationId() organizationId: string, ) { @@ -55,12 +59,22 @@ export class OffboardingChecklistController { @Get('template') @RequirePermission('member', 'read') + @ApiOperation({ + summary: 'Get the offboarding checklist template', + description: + "Returns the organization's offboarding checklist template: the ordered set of items every departing member must complete during their offboarding.", + }) async getTemplate(@OrganizationId() organizationId: string) { return this.offboardingChecklistService.getTemplate(organizationId); } @Post('template') @RequirePermission('member', 'update') + @ApiOperation({ + summary: 'Add an offboarding checklist template item', + description: + "Creates a new item in the organization's offboarding checklist template so it appears on every member's offboarding checklist from now on.", + }) async createTemplateItem( @OrganizationId() organizationId: string, @Body() dto: CreateTemplateItemDto, @@ -73,6 +87,11 @@ export class OffboardingChecklistController { @Patch('template/:id') @RequirePermission('member', 'update') + @ApiOperation({ + summary: 'Update an offboarding checklist template item', + description: + "Updates an existing offboarding checklist template item by id, changing its label, description, or settings on the organization's offboarding template.", + }) async updateTemplateItem( @OrganizationId() organizationId: string, @Param('id') id: string, @@ -87,6 +106,11 @@ export class OffboardingChecklistController { @Delete('template/:id') @RequirePermission('member', 'update') + @ApiOperation({ + summary: 'Delete an offboarding checklist template item', + description: + "Removes an item from the organization's offboarding checklist template by id so it no longer appears on members' offboarding checklists.", + }) async deleteTemplateItem( @OrganizationId() organizationId: string, @Param('id') id: string, @@ -99,6 +123,11 @@ export class OffboardingChecklistController { @Get('member/:memberId') @RequirePermission('member', 'read') + @ApiOperation({ + summary: "Get a member's offboarding checklist", + description: + 'Returns the offboarding checklist for a specific member, including each item and whether it has been completed, to track that person\'s offboarding progress.', + }) async getMemberChecklist( @OrganizationId() organizationId: string, @Param('memberId') memberId: string, @@ -111,7 +140,11 @@ export class OffboardingChecklistController { @Get('export-all') @RequirePermission('member', 'read') - @ApiOperation({ summary: 'Export all offboarding evidence as a zip file' }) + @ApiOperation({ + summary: 'Export all offboarding evidence as a zip file', + description: + 'Exports a zip archive containing the offboarding checklist evidence for every member in the organization, for audits, handovers, or record-keeping.', + }) async exportAllEvidence( @OrganizationId() organizationId: string, @Res() res: Response, @@ -134,7 +167,11 @@ export class OffboardingChecklistController { @Get('member/:memberId/export') @RequirePermission('member', 'read') - @ApiOperation({ summary: 'Export offboarding evidence as a zip file' }) + @ApiOperation({ + summary: 'Export offboarding evidence as a zip file', + description: + 'Exports a zip archive of the offboarding checklist evidence collected for a single member, for audit, handover, or record-keeping purposes.', + }) @ApiParam({ name: 'memberId', description: 'Member ID' }) async exportEvidence( @Param('memberId') memberId: string, @@ -166,6 +203,11 @@ export class OffboardingChecklistController { @Post('member/:memberId/item/:templateItemId/complete') @RequirePermission('member', 'update') + @ApiOperation({ + summary: 'Complete an offboarding checklist item', + description: + "Marks a specific offboarding checklist item complete for a member, recording who completed it and when, as part of finishing that member's offboarding.", + }) async completeItem( @OrganizationId() organizationId: string, @AuthContext() authContext: AuthContextType, @@ -184,6 +226,11 @@ export class OffboardingChecklistController { @Delete('member/:memberId/item/:templateItemId/complete') @RequirePermission('member', 'update') + @ApiOperation({ + summary: 'Reopen an offboarding checklist item', + description: + 'Reverts a previously completed offboarding checklist item back to incomplete for a member, in case the step was marked done by mistake.', + }) async uncompleteItem( @OrganizationId() organizationId: string, @Param('memberId') memberId: string, @@ -198,6 +245,11 @@ export class OffboardingChecklistController { @Post('member/:memberId/item/:templateItemId/evidence') @RequirePermission('member', 'update') + @ApiOperation({ + summary: 'Upload evidence for an offboarding checklist item', + description: + "Attaches a supporting evidence file to a member's completed offboarding checklist item, documenting that the offboarding step was actually carried out.", + }) async uploadEvidence( @OrganizationId() organizationId: string, @AuthContext() authContext: AuthContextType, @@ -218,6 +270,8 @@ export class OffboardingChecklistController { @RequirePermission('member', 'read') @ApiOperation({ summary: 'Get vendor access revocation status for a member', + description: + 'Lists the vendors a departing member had access to and whether each has been revoked, so you can confirm all vendor access is removed during offboarding.', }) @ApiParam({ name: 'memberId', description: 'Member ID' }) async getAccessRevocations( @@ -232,7 +286,11 @@ export class OffboardingChecklistController { @Post('member/:memberId/access-revocations/confirm-all') @RequirePermission('member', 'update') - @ApiOperation({ summary: 'Confirm all vendor access as revoked' }) + @ApiOperation({ + summary: 'Confirm all vendor access as revoked', + description: + "Marks every vendor access record for a departing member as revoked in one step, recording who confirmed it, to complete access removal during offboarding.", + }) @ApiParam({ name: 'memberId', description: 'Member ID' }) async revokeAllVendorAccess( @OrganizationId() organizationId: string, @@ -248,7 +306,11 @@ export class OffboardingChecklistController { @Post('member/:memberId/access-revocations/:vendorId') @RequirePermission('member', 'update') - @ApiOperation({ summary: 'Mark vendor access as revoked' }) + @ApiOperation({ + summary: 'Mark vendor access as revoked', + description: + "Marks a single vendor's access for a departing member as revoked, optionally attaching evidence and notes, as part of offboarding access removal.", + }) @ApiParam({ name: 'memberId', description: 'Member ID' }) @ApiParam({ name: 'vendorId', description: 'Vendor ID' }) async revokeVendorAccess( @@ -278,7 +340,11 @@ export class OffboardingChecklistController { @Delete('member/:memberId/access-revocations/:vendorId') @RequirePermission('member', 'update') - @ApiOperation({ summary: 'Undo vendor access revocation' }) + @ApiOperation({ + summary: 'Undo vendor access revocation', + description: + "Reverses a vendor access revocation for a member, marking that vendor's access as not revoked again, in case it was confirmed by mistake during offboarding.", + }) @ApiParam({ name: 'memberId', description: 'Member ID' }) @ApiParam({ name: 'vendorId', description: 'Vendor ID' }) async undoVendorRevocation( diff --git a/packages/docs/openapi.json b/packages/docs/openapi.json index 6f3d2f65a2..39f275a872 100644 --- a/packages/docs/openapi.json +++ b/packages/docs/openapi.json @@ -22819,6 +22819,14 @@ "type": "string" } }, + { + "name": "frameworkInstanceId", + "required": true, + "in": "query", + "schema": { + "type": "string" + } + }, { "name": "formType", "required": true, @@ -22841,14 +22849,6 @@ ], "type": "string" } - }, - { - "name": "frameworkInstanceId", - "required": true, - "in": "query", - "schema": { - "type": "string" - } } ], "responses": { @@ -23288,6 +23288,7 @@ }, "/v1/offboarding-checklist/pending": { "get": { + "description": "Lists members whose offboarding checklist is still incomplete, with their outstanding items, so you can track and finish departing-employee offboarding.", "operationId": "OffboardingChecklistController_getPendingOffboardings_v1", "parameters": [], "responses": { @@ -23304,14 +23305,13 @@ "tags": [ "Offboarding Checklist" ], - "description": "Get members with pending offboarding checklists in Comp AI.", "x-mint": { "metadata": { "title": "Get members with pending offboarding | Comp AI API", "sidebarTitle": "Get members with pending offboarding checklists", - "description": "Get members with pending offboarding checklists in Comp AI.", + "description": "Lists members whose offboarding checklist is still incomplete, with their outstanding items, so you can track and finish departing-employee offboarding.", "og:title": "Get members with pending offboarding | Comp AI API", - "og:description": "Get members with pending offboarding checklists in Comp AI." + "og:description": "Lists members whose offboarding checklist is still incomplete, with their outstanding items, so you can track and finish departing-employee offboarding." } }, "x-speakeasy-mcp": { @@ -23321,6 +23321,7 @@ }, "/v1/offboarding-checklist/template": { "get": { + "description": "Returns the organization's offboarding checklist template: the ordered set of items every departing member must complete during their offboarding.", "operationId": "OffboardingChecklistController_getTemplate_v1", "parameters": [], "responses": { @@ -23333,14 +23334,25 @@ "apikey": [] } ], + "summary": "Get the offboarding checklist template", "tags": [ "Offboarding Checklist" ], + "x-mint": { + "metadata": { + "title": "Get the offboarding checklist template | Comp AI API", + "sidebarTitle": "Get the offboarding checklist template", + "description": "Returns the organization's offboarding checklist template: the ordered set of items every departing member must complete during their offboarding.", + "og:title": "Get the offboarding checklist template | Comp AI API", + "og:description": "Returns the organization's offboarding checklist template: the ordered set of items every departing member must complete during their offboarding." + } + }, "x-speakeasy-mcp": { "name": "get-template" } }, "post": { + "description": "Creates a new item in the organization's offboarding checklist template so it appears on every member's offboarding checklist from now on.", "operationId": "OffboardingChecklistController_createTemplateItem_v1", "parameters": [], "requestBody": { @@ -23363,9 +23375,19 @@ "apikey": [] } ], + "summary": "Add an offboarding checklist template item", "tags": [ "Offboarding Checklist" ], + "x-mint": { + "metadata": { + "title": "Add an offboarding checklist template item | Comp AI API", + "sidebarTitle": "Add an offboarding checklist template item", + "description": "Creates a new item in the organization's offboarding checklist template so it appears on every member's offboarding checklist from now on.", + "og:title": "Add an offboarding checklist template item | Comp AI API", + "og:description": "Creates a new item in the organization's offboarding checklist template so it appears on every member's offboarding checklist from now on." + } + }, "x-speakeasy-mcp": { "name": "create-template-item" } @@ -23373,6 +23395,7 @@ }, "/v1/offboarding-checklist/template/{id}": { "patch": { + "description": "Updates an existing offboarding checklist template item by id, changing its label, description, or settings on the organization's offboarding template.", "operationId": "OffboardingChecklistController_updateTemplateItem_v1", "parameters": [ { @@ -23404,14 +23427,25 @@ "apikey": [] } ], + "summary": "Update an offboarding checklist template item", "tags": [ "Offboarding Checklist" ], + "x-mint": { + "metadata": { + "title": "Update an offboarding checklist template item | Comp AI API", + "sidebarTitle": "Update an offboarding checklist template item", + "description": "Updates an existing offboarding checklist template item by id, changing its label, description, or settings on the organization's offboarding template.", + "og:title": "Update an offboarding checklist template item | Comp AI API", + "og:description": "Updates an existing offboarding checklist template item by id, changing its label, description, or settings on the organization's offboarding template." + } + }, "x-speakeasy-mcp": { "name": "update-template-item" } }, "delete": { + "description": "Removes an item from the organization's offboarding checklist template by id so it no longer appears on members' offboarding checklists.", "operationId": "OffboardingChecklistController_deleteTemplateItem_v1", "parameters": [ { @@ -23433,9 +23467,19 @@ "apikey": [] } ], + "summary": "Delete an offboarding checklist template item", "tags": [ "Offboarding Checklist" ], + "x-mint": { + "metadata": { + "title": "Delete an offboarding checklist template item | Comp AI API", + "sidebarTitle": "Delete an offboarding checklist template item", + "description": "Removes an item from the organization's offboarding checklist template by id so it no longer appears on members' offboarding checklists.", + "og:title": "Delete an offboarding checklist template item | Comp AI API", + "og:description": "Removes an item from the organization's offboarding checklist template by id so it no longer appears on members' offboarding checklists." + } + }, "x-speakeasy-mcp": { "name": "delete-template-item" } @@ -23443,6 +23487,7 @@ }, "/v1/offboarding-checklist/member/{memberId}": { "get": { + "description": "Returns the offboarding checklist for a specific member, including each item and whether it has been completed, to track that person's offboarding progress.", "operationId": "OffboardingChecklistController_getMemberChecklist_v1", "parameters": [ { @@ -23464,9 +23509,19 @@ "apikey": [] } ], + "summary": "Get a member's offboarding checklist", "tags": [ "Offboarding Checklist" ], + "x-mint": { + "metadata": { + "title": "Get a member's offboarding checklist | Comp AI API", + "sidebarTitle": "Get a member's offboarding checklist", + "description": "Returns the offboarding checklist for a specific member, including each item and whether it has been completed, to track that person's offboarding progress.", + "og:title": "Get a member's offboarding checklist | Comp AI API", + "og:description": "Returns the offboarding checklist for a specific member, including each item and whether it has been completed, to track that person's offboarding progress." + } + }, "x-speakeasy-mcp": { "name": "get-member-checklist" } @@ -23474,6 +23529,7 @@ }, "/v1/offboarding-checklist/export-all": { "get": { + "description": "Exports a zip archive containing the offboarding checklist evidence for every member in the organization, for audits, handovers, or record-keeping.", "operationId": "OffboardingChecklistController_exportAllEvidence_v1", "parameters": [], "responses": { @@ -23490,14 +23546,13 @@ "tags": [ "Offboarding Checklist" ], - "description": "Export all offboarding evidence as a zip file in Comp AI.", "x-mint": { "metadata": { "title": "Export all offboarding evidence as a zip file | Comp AI API", "sidebarTitle": "Export all offboarding evidence as a zip file", - "description": "Export all offboarding evidence as a zip file in Comp AI.", + "description": "Exports a zip archive containing the offboarding checklist evidence for every member in the organization, for audits, handovers, or record-keeping.", "og:title": "Export all offboarding evidence as a zip file | Comp AI API", - "og:description": "Export all offboarding evidence as a zip file in Comp AI." + "og:description": "Exports a zip archive containing the offboarding checklist evidence for every member in the organization, for audits, handovers, or record-keeping." } }, "x-speakeasy-mcp": { @@ -23507,6 +23562,7 @@ }, "/v1/offboarding-checklist/member/{memberId}/export": { "get": { + "description": "Exports a zip archive of the offboarding checklist evidence collected for a single member, for audit, handover, or record-keeping purposes.", "operationId": "OffboardingChecklistController_exportEvidence_v1", "parameters": [ { @@ -23533,14 +23589,13 @@ "tags": [ "Offboarding Checklist" ], - "description": "Export offboarding evidence as a zip file in Comp AI.", "x-mint": { "metadata": { "title": "Export offboarding evidence as a zip file | Comp AI API", "sidebarTitle": "Export offboarding evidence as a zip file", - "description": "Export offboarding evidence as a zip file in Comp AI.", + "description": "Exports a zip archive of the offboarding checklist evidence collected for a single member, for audit, handover, or record-keeping purposes.", "og:title": "Export offboarding evidence as a zip file | Comp AI API", - "og:description": "Export offboarding evidence as a zip file in Comp AI." + "og:description": "Exports a zip archive of the offboarding checklist evidence collected for a single member, for audit, handover, or record-keeping purposes." } }, "x-speakeasy-mcp": { @@ -23550,6 +23605,7 @@ }, "/v1/offboarding-checklist/member/{memberId}/item/{templateItemId}/complete": { "post": { + "description": "Marks a specific offboarding checklist item complete for a member, recording who completed it and when, as part of finishing that member's offboarding.", "operationId": "OffboardingChecklistController_completeItem_v1", "parameters": [ { @@ -23589,14 +23645,25 @@ "apikey": [] } ], + "summary": "Complete an offboarding checklist item", "tags": [ "Offboarding Checklist" ], + "x-mint": { + "metadata": { + "title": "Complete an offboarding checklist item | Comp AI API", + "sidebarTitle": "Complete an offboarding checklist item", + "description": "Marks a specific offboarding checklist item complete for a member, recording who completed it and when, as part of finishing that member's offboarding.", + "og:title": "Complete an offboarding checklist item | Comp AI API", + "og:description": "Marks a specific offboarding checklist item complete for a member, recording who completed it and when, as part of finishing that member's offboarding." + } + }, "x-speakeasy-mcp": { "name": "complete-item" } }, "delete": { + "description": "Reverts a previously completed offboarding checklist item back to incomplete for a member, in case the step was marked done by mistake.", "operationId": "OffboardingChecklistController_uncompleteItem_v1", "parameters": [ { @@ -23626,9 +23693,19 @@ "apikey": [] } ], + "summary": "Reopen an offboarding checklist item", "tags": [ "Offboarding Checklist" ], + "x-mint": { + "metadata": { + "title": "Reopen an offboarding checklist item | Comp AI API", + "sidebarTitle": "Reopen an offboarding checklist item", + "description": "Reverts a previously completed offboarding checklist item back to incomplete for a member, in case the step was marked done by mistake.", + "og:title": "Reopen an offboarding checklist item | Comp AI API", + "og:description": "Reverts a previously completed offboarding checklist item back to incomplete for a member, in case the step was marked done by mistake." + } + }, "x-speakeasy-mcp": { "name": "uncomplete-item" } @@ -23636,6 +23713,7 @@ }, "/v1/offboarding-checklist/member/{memberId}/item/{templateItemId}/evidence": { "post": { + "description": "Attaches a supporting evidence file to a member's completed offboarding checklist item, documenting that the offboarding step was actually carried out.", "operationId": "OffboardingChecklistController_uploadEvidence_v1", "parameters": [ { @@ -23675,9 +23753,19 @@ "apikey": [] } ], + "summary": "Upload evidence for an offboarding checklist item", "tags": [ "Offboarding Checklist" ], + "x-mint": { + "metadata": { + "title": "Upload evidence for an offboarding checklist | Comp AI API", + "sidebarTitle": "Upload evidence for an offboarding checklist item", + "description": "Attaches a supporting evidence file to a member's completed offboarding checklist item, documenting that the offboarding step was actually carried out.", + "og:title": "Upload evidence for an offboarding checklist | Comp AI API", + "og:description": "Attaches a supporting evidence file to a member's completed offboarding checklist item, documenting that the offboarding step was actually carried out." + } + }, "x-speakeasy-mcp": { "name": "upload-evidence" } @@ -23685,6 +23773,7 @@ }, "/v1/offboarding-checklist/member/{memberId}/access-revocations": { "get": { + "description": "Lists the vendors a departing member had access to and whether each has been revoked, so you can confirm all vendor access is removed during offboarding.", "operationId": "OffboardingChecklistController_getAccessRevocations_v1", "parameters": [ { @@ -23711,14 +23800,13 @@ "tags": [ "Offboarding Checklist" ], - "description": "Get vendor access revocation status for a member in Comp AI.", "x-mint": { "metadata": { "title": "Get vendor access revocation status for a | Comp AI API", "sidebarTitle": "Get vendor access revocation status for a member", - "description": "Get vendor access revocation status for a member in Comp AI.", + "description": "Lists the vendors a departing member had access to and whether each has been revoked, so you can confirm all vendor access is removed during offboarding.", "og:title": "Get vendor access revocation status for a | Comp AI API", - "og:description": "Get vendor access revocation status for a member in Comp AI." + "og:description": "Lists the vendors a departing member had access to and whether each has been revoked, so you can confirm all vendor access is removed during offboarding." } }, "x-speakeasy-mcp": { @@ -23728,6 +23816,7 @@ }, "/v1/offboarding-checklist/member/{memberId}/access-revocations/confirm-all": { "post": { + "description": "Marks every vendor access record for a departing member as revoked in one step, recording who confirmed it, to complete access removal during offboarding.", "operationId": "OffboardingChecklistController_revokeAllVendorAccess_v1", "parameters": [ { @@ -23754,14 +23843,13 @@ "tags": [ "Offboarding Checklist" ], - "description": "Confirm all vendor access as revoked in Comp AI.", "x-mint": { "metadata": { "title": "Confirm all vendor access as revoked | Comp AI API", "sidebarTitle": "Confirm all vendor access as revoked", - "description": "Confirm all vendor access as revoked in Comp AI.", + "description": "Marks every vendor access record for a departing member as revoked in one step, recording who confirmed it, to complete access removal during offboarding.", "og:title": "Confirm all vendor access as revoked | Comp AI API", - "og:description": "Confirm all vendor access as revoked in Comp AI." + "og:description": "Marks every vendor access record for a departing member as revoked in one step, recording who confirmed it, to complete access removal during offboarding." } }, "x-speakeasy-mcp": { @@ -23771,6 +23859,7 @@ }, "/v1/offboarding-checklist/member/{memberId}/access-revocations/{vendorId}": { "post": { + "description": "Marks a single vendor's access for a departing member as revoked, optionally attaching evidence and notes, as part of offboarding access removal.", "operationId": "OffboardingChecklistController_revokeVendorAccess_v1", "parameters": [ { @@ -23806,14 +23895,13 @@ "tags": [ "Offboarding Checklist" ], - "description": "Mark vendor access as revoked in Comp AI.", "x-mint": { "metadata": { "title": "Mark vendor access as revoked | Comp AI API", "sidebarTitle": "Mark vendor access as revoked", - "description": "Mark vendor access as revoked in Comp AI.", + "description": "Marks a single vendor's access for a departing member as revoked, optionally attaching evidence and notes, as part of offboarding access removal.", "og:title": "Mark vendor access as revoked | Comp AI API", - "og:description": "Mark vendor access as revoked in Comp AI." + "og:description": "Marks a single vendor's access for a departing member as revoked, optionally attaching evidence and notes, as part of offboarding access removal." } }, "x-speakeasy-mcp": { @@ -23821,6 +23909,7 @@ } }, "delete": { + "description": "Reverses a vendor access revocation for a member, marking that vendor's access as not revoked again, in case it was confirmed by mistake during offboarding.", "operationId": "OffboardingChecklistController_undoVendorRevocation_v1", "parameters": [ { @@ -23856,14 +23945,13 @@ "tags": [ "Offboarding Checklist" ], - "description": "Undo vendor access revocation in Comp AI.", "x-mint": { "metadata": { "title": "Undo vendor access revocation | Comp AI API", "sidebarTitle": "Undo vendor access revocation", - "description": "Undo vendor access revocation in Comp AI.", + "description": "Reverses a vendor access revocation for a member, marking that vendor's access as not revoked again, in case it was confirmed by mistake during offboarding.", "og:title": "Undo vendor access revocation | Comp AI API", - "og:description": "Undo vendor access revocation in Comp AI." + "og:description": "Reverses a vendor access revocation for a member, marking that vendor's access as not revoked again, in case it was confirmed by mistake during offboarding." } }, "x-speakeasy-mcp": { From aa7fde027d21d1c187ee43b24e38c1f803bbf648 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 18:06:32 -0400 Subject: [PATCH 04/14] fix(api): skip OAuth consent for first-party Gram MCP client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Gram is Comp AI's own hosted MCP client, so the user's login is the authorization — no consent screen needed. Avoids building a consent page UI for v1. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/auth/auth.server.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/api/src/auth/auth.server.ts b/apps/api/src/auth/auth.server.ts index 46e9de293b..c0b506aff3 100644 --- a/apps/api/src/auth/auth.server.ts +++ b/apps/api/src/auth/auth.server.ts @@ -211,7 +211,10 @@ const gramMcpClient = disabled: false, redirectUrls: [process.env.GRAM_OAUTH_REDIRECT_URI], metadata: null, - skipConsent: false, + // First-party client: Gram is Comp AI's own hosted MCP, so the user's + // login (Sign in with Google) IS the authorization — no separate consent + // screen is needed. This also avoids having to build a consent page UI. + skipConsent: true, } : null; From 81d11a5d51f55b95045990107045366c37aeaa0b Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 18:27:56 -0400 Subject: [PATCH 05/14] feat(api): multi-org support for hosted MCP Multi-org users can now use the MCP instead of being blocked. A per-user McpOrgBinding (set at connect time) records which org their MCP/OAuth token acts on; HybridAuthGuard uses it: single-org binds automatically, multi-org uses the saved choice (if still a member), otherwise returns a helpful 'choose your org' error instead of a dead 401. Adds GET/PUT /v1/mcp/organization (web-app management endpoints, deny-listed from the public spec + MCP tools so the agent can't switch tenants). Tests: guard binding cases + service. Migration is additive (1 table). Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/app.module.ts | 2 + apps/api/src/auth/hybrid-auth.guard.spec.ts | 55 ++++++++++- apps/api/src/auth/hybrid-auth.guard.ts | 33 +++++-- .../src/mcp/dto/set-mcp-organization.dto.ts | 13 +++ apps/api/src/mcp/mcp.controller.ts | 46 +++++++++ apps/api/src/mcp/mcp.module.ts | 12 +++ apps/api/src/mcp/mcp.service.spec.ts | 94 +++++++++++++++++++ apps/api/src/mcp/mcp.service.ts | 56 +++++++++++ apps/api/src/openapi/public-docs-quality.ts | 1 + .../migration.sql | 22 +++++ packages/db/prisma/schema/auth.prisma | 18 ++++ packages/db/prisma/schema/organization.prisma | 1 + 12 files changed, 342 insertions(+), 11 deletions(-) create mode 100644 apps/api/src/mcp/dto/set-mcp-organization.dto.ts create mode 100644 apps/api/src/mcp/mcp.controller.ts create mode 100644 apps/api/src/mcp/mcp.module.ts create mode 100644 apps/api/src/mcp/mcp.service.spec.ts create mode 100644 apps/api/src/mcp/mcp.service.ts create mode 100644 packages/db/prisma/migrations/20260528222357_add_mcp_org_binding/migration.sql diff --git a/apps/api/src/app.module.ts b/apps/api/src/app.module.ts index 613d2484f3..ccb8410374 100644 --- a/apps/api/src/app.module.ts +++ b/apps/api/src/app.module.ts @@ -47,6 +47,7 @@ import { FrameworkVersionsModule } from './framework-editor-versions/framework-v import { AuditModule } from './audit/audit.module'; import { ControlsModule } from './controls/controls.module'; import { RolesModule } from './roles/roles.module'; +import { McpModule } from './mcp/mcp.module'; import { EmailModule } from './email/email.module'; import { SecretsModule } from './secrets/secrets.module'; import { SecurityPenetrationTestsModule } from './security-penetration-tests/security-penetration-tests.module'; @@ -126,6 +127,7 @@ import { OffboardingChecklistModule } from './offboarding-checklist/offboarding- AdminFeatureFlagsModule, TimelinesModule, OffboardingChecklistModule, + McpModule, ], controllers: [AppController], providers: [ diff --git a/apps/api/src/auth/hybrid-auth.guard.spec.ts b/apps/api/src/auth/hybrid-auth.guard.spec.ts index a4d6c50f5f..22b0d3cb85 100644 --- a/apps/api/src/auth/hybrid-auth.guard.spec.ts +++ b/apps/api/src/auth/hybrid-auth.guard.spec.ts @@ -20,10 +20,14 @@ jest.mock('./auth.server', () => ({ // (device-agent style) to bind the organization for the MCP OAuth path. const mockUserFindUnique = jest.fn(); const mockMemberFindMany = jest.fn(); +const mockMcpBindingFindUnique = jest.fn(); jest.mock('@db', () => ({ db: { user: { findUnique: (...args: unknown[]) => mockUserFindUnique(...args) }, member: { findMany: (...args: unknown[]) => mockMemberFindMany(...args) }, + mcpOrgBinding: { + findUnique: (...args: unknown[]) => mockMcpBindingFindUnique(...args), + }, }, })); @@ -66,6 +70,8 @@ describe('HybridAuthGuard — MCP OAuth path', () => { jest.spyOn(reflector, 'getAllAndOverride').mockReturnValue(false); // No cookie/regular session → forces the MCP OAuth fallback. mockGetSession.mockResolvedValue(null); + // No org binding by default; individual tests override. + mockMcpBindingFindUnique.mockResolvedValue(null); }); it('authenticates a single-org user (admin) and binds org + roles', async () => { @@ -143,7 +149,7 @@ describe('HybridAuthGuard — MCP OAuth path', () => { ); }); - it('fails closed for a multi-org user (no silent tenant selection)', async () => { + it('multi-org with no saved choice → asks them to pick (no silent tenant)', async () => { mockGetMcpSession.mockResolvedValue({ userId: 'usr_5', scopes: 'openid' }); mockUserFindUnique.mockResolvedValue({ id: 'usr_5', @@ -154,6 +160,7 @@ describe('HybridAuthGuard — MCP OAuth path', () => { { id: 'mem_a', role: 'admin', department: 'none', organizationId: 'org_a' }, { id: 'mem_b', role: 'owner', department: 'none', organizationId: 'org_b' }, ]); + mockMcpBindingFindUnique.mockResolvedValue(null); const { context, request } = createContext({ authorization: 'Bearer mcp_access_token', @@ -166,6 +173,52 @@ describe('HybridAuthGuard — MCP OAuth path', () => { expect(request.organizationId).toBe(''); }); + it('multi-org with a saved choice → binds the chosen org', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_6', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_6', + email: 'consultant@acme.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_a', role: 'admin', department: 'none', organizationId: 'org_a' }, + { id: 'mem_b', role: 'owner', department: 'it', organizationId: 'org_b' }, + ]); + mockMcpBindingFindUnique.mockResolvedValue({ organizationId: 'org_b' }); + + const { context, request } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(request.organizationId).toBe('org_b'); + expect(request.memberId).toBe('mem_b'); + expect(request.userRoles).toEqual(['owner']); + }); + + it('multi-org with a stale choice (no longer a member) → asks them to pick', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_7', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_7', + email: 'consultant@acme.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_a', role: 'admin', department: 'none', organizationId: 'org_a' }, + { id: 'mem_b', role: 'owner', department: 'none', organizationId: 'org_b' }, + ]); + // Bound to an org they were removed from. + mockMcpBindingFindUnique.mockResolvedValue({ organizationId: 'org_gone' }); + + const { context } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).rejects.toThrow( + 'multiple organizations', + ); + }); + it('marks platform admins from the user role', async () => { mockGetMcpSession.mockResolvedValue({ userId: 'usr_4', scopes: 'openid' }); mockUserFindUnique.mockResolvedValue({ diff --git a/apps/api/src/auth/hybrid-auth.guard.ts b/apps/api/src/auth/hybrid-auth.guard.ts index 09b87c1680..f5abe2fa41 100644 --- a/apps/api/src/auth/hybrid-auth.guard.ts +++ b/apps/api/src/auth/hybrid-auth.guard.ts @@ -262,10 +262,10 @@ export class HybridAuthGuard implements CanActivate { * when no organization can be resolved. * * The token carries the user identity only. The organization is resolved - * explicitly from the user's active memberships — the same approach as the - * device-agent (enumerate memberships, then bind to one), not a "most recent" - * guess. One org is used directly; multiple orgs fail closed because MCP org - * selection isn't supported yet (avoids silently acting on the wrong tenant). + * explicitly from the user's active memberships (device-agent style), never a + * "most recent" guess. One org → used directly. Multiple orgs → the org the + * user chose for MCP (McpOrgBinding, set at connect time) is used if they're + * still a member; otherwise we ask them to choose rather than guess a tenant. * Roles come from the resolved member so the existing PermissionGuard enforces * RBAC unchanged. */ @@ -314,14 +314,27 @@ export class HybridAuthGuard implements CanActivate { 'No active organization for this MCP token.', ); } + + let member = memberships[0]; if (memberships.length > 1) { - throw new UnauthorizedException( - 'This account belongs to multiple organizations. Selecting an ' + - 'organization for MCP access is not supported yet.', - ); + // Multi-org: use the org the user chose for MCP (set at connect time), + // as long as they're still a member of it. No saved/valid choice → ask + // them to pick rather than guessing a tenant. + const binding = await db.mcpOrgBinding.findUnique({ + where: { userId }, + select: { organizationId: true }, + }); + const chosen = binding + ? memberships.find((m) => m.organizationId === binding.organizationId) + : undefined; + if (!chosen) { + throw new UnauthorizedException( + 'This account belongs to multiple organizations. Choose your ' + + 'organization for AI/MCP access in Comp AI settings, then reconnect.', + ); + } + member = chosen; } - - const member = memberships[0]; request.organizationId = member.organizationId; request.memberId = member.id; request.memberDepartment = member.department; diff --git a/apps/api/src/mcp/dto/set-mcp-organization.dto.ts b/apps/api/src/mcp/dto/set-mcp-organization.dto.ts new file mode 100644 index 0000000000..81e1130bff --- /dev/null +++ b/apps/api/src/mcp/dto/set-mcp-organization.dto.ts @@ -0,0 +1,13 @@ +import { ApiProperty } from '@nestjs/swagger'; +import { IsNotEmpty, IsString } from 'class-validator'; + +export class SetMcpOrganizationDto { + @ApiProperty({ + description: + 'The organization the MCP/AI connection should act on. Must be one you are a member of.', + example: 'org_abc123', + }) + @IsString() + @IsNotEmpty() + organizationId!: string; +} diff --git a/apps/api/src/mcp/mcp.controller.ts b/apps/api/src/mcp/mcp.controller.ts new file mode 100644 index 0000000000..1e3f62e212 --- /dev/null +++ b/apps/api/src/mcp/mcp.controller.ts @@ -0,0 +1,46 @@ +import { Body, Controller, Get, Put, UseGuards } from '@nestjs/common'; +import { ApiBody, ApiOperation, ApiSecurity, ApiTags } from '@nestjs/swagger'; +import { UserId } from '../auth/auth-context.decorator'; +import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; +import { SkipOrgCheck } from '../auth/skip-org-check.decorator'; +import { SetMcpOrganizationDto } from './dto/set-mcp-organization.dto'; +import { McpService } from './mcp.service'; + +/** + * MCP account-management endpoints (web app only — excluded from the public + * OpenAPI spec / MCP tools via the deny-list in public-docs-quality.ts). + * @SkipOrgCheck because choosing among your organizations isn't scoped to one. + */ +@ApiTags('MCP') +@Controller({ path: 'mcp', version: '1' }) +@UseGuards(HybridAuthGuard) +@ApiSecurity('apikey') +export class McpController { + constructor(private readonly mcpService: McpService) {} + + @Get('organization') + @SkipOrgCheck() + @ApiOperation({ + summary: 'Get your MCP organization selection', + description: + 'Returns the organizations you belong to and which one your AI/MCP connection currently acts on.', + }) + async getOrganization(@UserId() userId: string) { + return this.mcpService.getOrganizationSelection(userId); + } + + @Put('organization') + @SkipOrgCheck() + @ApiOperation({ + summary: 'Set your MCP organization', + description: + 'Sets which organization your AI/MCP connection acts on when you belong to more than one. Validated against your memberships.', + }) + @ApiBody({ type: SetMcpOrganizationDto }) + async setOrganization( + @UserId() userId: string, + @Body() dto: SetMcpOrganizationDto, + ) { + return this.mcpService.setOrganization(userId, dto.organizationId); + } +} diff --git a/apps/api/src/mcp/mcp.module.ts b/apps/api/src/mcp/mcp.module.ts new file mode 100644 index 0000000000..4bc05118f9 --- /dev/null +++ b/apps/api/src/mcp/mcp.module.ts @@ -0,0 +1,12 @@ +import { Module } from '@nestjs/common'; +import { AuthModule } from '../auth/auth.module'; +import { McpController } from './mcp.controller'; +import { McpService } from './mcp.service'; + +@Module({ + imports: [AuthModule], + controllers: [McpController], + providers: [McpService], + exports: [McpService], +}) +export class McpModule {} diff --git a/apps/api/src/mcp/mcp.service.spec.ts b/apps/api/src/mcp/mcp.service.spec.ts new file mode 100644 index 0000000000..af47634362 --- /dev/null +++ b/apps/api/src/mcp/mcp.service.spec.ts @@ -0,0 +1,94 @@ +import { ForbiddenException } from '@nestjs/common'; + +const mockMemberFindMany = jest.fn(); +const mockMemberFindFirst = jest.fn(); +const mockBindingFindUnique = jest.fn(); +const mockBindingUpsert = jest.fn(); +jest.mock('@db', () => ({ + db: { + member: { + findMany: (...a: unknown[]) => mockMemberFindMany(...a), + findFirst: (...a: unknown[]) => mockMemberFindFirst(...a), + }, + mcpOrgBinding: { + findUnique: (...a: unknown[]) => mockBindingFindUnique(...a), + upsert: (...a: unknown[]) => mockBindingUpsert(...a), + }, + }, +})); + +import { McpService } from './mcp.service'; + +describe('McpService', () => { + let service: McpService; + + beforeEach(() => { + jest.clearAllMocks(); + service = new McpService(); + }); + + describe('getOrganizationSelection', () => { + it('returns the user orgs and the current valid selection', async () => { + mockMemberFindMany.mockResolvedValue([ + { organization: { id: 'org_a', name: 'Acme' } }, + { organization: { id: 'org_b', name: 'Beta' } }, + ]); + mockBindingFindUnique.mockResolvedValue({ organizationId: 'org_b' }); + + const result = await service.getOrganizationSelection('usr_1'); + + expect(result.organizations).toEqual([ + { id: 'org_a', name: 'Acme' }, + { id: 'org_b', name: 'Beta' }, + ]); + expect(result.selectedOrganizationId).toBe('org_b'); + }); + + it('drops a stale selection the user is no longer a member of', async () => { + mockMemberFindMany.mockResolvedValue([ + { organization: { id: 'org_a', name: 'Acme' } }, + ]); + mockBindingFindUnique.mockResolvedValue({ organizationId: 'org_gone' }); + + const result = await service.getOrganizationSelection('usr_1'); + + expect(result.selectedOrganizationId).toBeNull(); + }); + + it('returns null selection when none is set', async () => { + mockMemberFindMany.mockResolvedValue([ + { organization: { id: 'org_a', name: 'Acme' } }, + ]); + mockBindingFindUnique.mockResolvedValue(null); + + const result = await service.getOrganizationSelection('usr_1'); + + expect(result.selectedOrganizationId).toBeNull(); + }); + }); + + describe('setOrganization', () => { + it('upserts the binding when the user is a member', async () => { + mockMemberFindFirst.mockResolvedValue({ id: 'mem_1' }); + mockBindingUpsert.mockResolvedValue({}); + + const result = await service.setOrganization('usr_1', 'org_a'); + + expect(result).toEqual({ organizationId: 'org_a' }); + expect(mockBindingUpsert).toHaveBeenCalledWith({ + where: { userId: 'usr_1' }, + create: { userId: 'usr_1', organizationId: 'org_a' }, + update: { organizationId: 'org_a' }, + }); + }); + + it('rejects when the user is not a member of the org', async () => { + mockMemberFindFirst.mockResolvedValue(null); + + await expect(service.setOrganization('usr_1', 'org_x')).rejects.toThrow( + ForbiddenException, + ); + expect(mockBindingUpsert).not.toHaveBeenCalled(); + }); + }); +}); diff --git a/apps/api/src/mcp/mcp.service.ts b/apps/api/src/mcp/mcp.service.ts new file mode 100644 index 0000000000..ba1e47ee85 --- /dev/null +++ b/apps/api/src/mcp/mcp.service.ts @@ -0,0 +1,56 @@ +import { ForbiddenException, Injectable } from '@nestjs/common'; +import { db } from '@db'; + +@Injectable() +export class McpService { + /** + * The organizations the user can choose from for MCP access, plus their + * current selection (null when unset or no longer valid). + */ + async getOrganizationSelection(userId: string) { + const memberships = await db.member.findMany({ + where: { userId, deactivated: false }, + select: { organization: { select: { id: true, name: true } } }, + }); + const organizations = memberships.map((m) => ({ + id: m.organization.id, + name: m.organization.name, + })); + + const binding = await db.mcpOrgBinding.findUnique({ + where: { userId }, + select: { organizationId: true }, + }); + // Drop a stale selection if the user is no longer a member of that org. + const selectedOrganizationId = + binding && organizations.some((o) => o.id === binding.organizationId) + ? binding.organizationId + : null; + + return { organizations, selectedOrganizationId }; + } + + /** + * Set which organization the user's MCP/OAuth token acts on. Validates that + * the user is an active member of the chosen org before saving. + */ + async setOrganization(userId: string, organizationId: string) { + const member = await db.member.findFirst({ + where: { userId, organizationId, deactivated: false }, + select: { id: true }, + }); + if (!member) { + throw new ForbiddenException( + 'You are not a member of the selected organization.', + ); + } + + await db.mcpOrgBinding.upsert({ + where: { userId }, + create: { userId, organizationId }, + update: { organizationId }, + }); + + return { organizationId }; + } +} diff --git a/apps/api/src/openapi/public-docs-quality.ts b/apps/api/src/openapi/public-docs-quality.ts index bf1ee5f50e..e133d52e3e 100644 --- a/apps/api/src/openapi/public-docs-quality.ts +++ b/apps/api/src/openapi/public-docs-quality.ts @@ -4,6 +4,7 @@ export const PUBLIC_DOCS_EXCLUDED_PREFIXES = [ '/v1/auth', '/v1/admin', '/v1/internal', + '/v1/mcp', '/v1/framework-editor', '/v1/browserbase', '/v1/assistant-chat', diff --git a/packages/db/prisma/migrations/20260528222357_add_mcp_org_binding/migration.sql b/packages/db/prisma/migrations/20260528222357_add_mcp_org_binding/migration.sql new file mode 100644 index 0000000000..a79ba3eb24 --- /dev/null +++ b/packages/db/prisma/migrations/20260528222357_add_mcp_org_binding/migration.sql @@ -0,0 +1,22 @@ +-- CreateTable +CREATE TABLE "mcp_org_binding" ( + "id" TEXT NOT NULL DEFAULT generate_prefixed_cuid('mob'::text), + "userId" TEXT NOT NULL, + "organizationId" TEXT NOT NULL, + "createdAt" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "updatedAt" TIMESTAMP(3) NOT NULL, + + CONSTRAINT "mcp_org_binding_pkey" PRIMARY KEY ("id") +); + +-- CreateIndex +CREATE UNIQUE INDEX "mcp_org_binding_userId_key" ON "mcp_org_binding"("userId"); + +-- CreateIndex +CREATE INDEX "mcp_org_binding_organizationId_idx" ON "mcp_org_binding"("organizationId"); + +-- AddForeignKey +ALTER TABLE "mcp_org_binding" ADD CONSTRAINT "mcp_org_binding_userId_fkey" FOREIGN KEY ("userId") REFERENCES "User"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- AddForeignKey +ALTER TABLE "mcp_org_binding" ADD CONSTRAINT "mcp_org_binding_organizationId_fkey" FOREIGN KEY ("organizationId") REFERENCES "Organization"("id") ON DELETE CASCADE ON UPDATE CASCADE; diff --git a/packages/db/prisma/schema/auth.prisma b/packages/db/prisma/schema/auth.prisma index 2f16e5b550..6c9c12383c 100644 --- a/packages/db/prisma/schema/auth.prisma +++ b/packages/db/prisma/schema/auth.prisma @@ -35,6 +35,7 @@ model User { oauthApplications OauthApplication[] oauthAccessTokens OauthAccessToken[] oauthConsents OauthConsent[] + mcpOrgBinding McpOrgBinding? @@unique([email]) } @@ -161,6 +162,23 @@ model OauthConsent { @@map("oauth_consent") } +// Per-user organization selection for the hosted MCP. A user who belongs to +// multiple orgs picks which one their MCP/OAuth token acts on (chosen at connect +// time); HybridAuthGuard reads this for MCP OAuth requests. Single-org users +// don't need a row — their one org is used automatically. +model McpOrgBinding { + id String @id @default(dbgenerated("generate_prefixed_cuid('mob'::text)")) + userId String @unique + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + organizationId String + organization Organization @relation(fields: [organizationId], references: [id], onDelete: Cascade) + createdAt DateTime @default(now()) + updatedAt DateTime @updatedAt + + @@index([organizationId]) + @@map("mcp_org_binding") +} + // JWT Plugin - Required by Better Auth JWT plugin // https://www.better-auth.com/docs/plugins/jwt model Jwks { diff --git a/packages/db/prisma/schema/organization.prisma b/packages/db/prisma/schema/organization.prisma index 080176465b..5fbb485609 100644 --- a/packages/db/prisma/schema/organization.prisma +++ b/packages/db/prisma/schema/organization.prisma @@ -26,6 +26,7 @@ model Organization { employeeSyncProvider String? apiKeys ApiKey[] + mcpOrgBindings McpOrgBinding[] auditLog AuditLog[] controls Control[] frameworkInstances FrameworkInstance[] From fbef686690afff245330a560e2b0a10583d7bba0 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 18:28:22 -0400 Subject: [PATCH 06/14] docs(api): correct hosted-MCP endpoint paths to /mcp/* in comment The mcp plugin registers /api/auth/mcp/{authorize,token,register}, not /oauth2/* (those aren't registered). Gram's OAuth proxy targets /mcp/*. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/auth/auth.server.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/apps/api/src/auth/auth.server.ts b/apps/api/src/auth/auth.server.ts index c0b506aff3..83f6d1bd68 100644 --- a/apps/api/src/auth/auth.server.ts +++ b/apps/api/src/auth/auth.server.ts @@ -525,8 +525,9 @@ export const auth = betterAuth({ defaultRole: 'user', }), // OAuth 2.0 / OIDC provider for hosted MCP (Gram). Wraps oidcProvider and - // exposes /api/auth/oauth2/* + /api/auth/.well-known/* and the - // auth.api.getMcpSession() helper used by HybridAuthGuard. + // exposes /api/auth/mcp/* (authorize, token, register) + the two + // /api/auth/.well-known/* discovery docs, plus the auth.api.getMcpSession() + // helper used by HybridAuthGuard. (Gram points its OAuth proxy at /mcp/*.) mcp({ loginPage: mcpLoginPage, ...(process.env.MCP_RESOURCE_URL From 8d4baea0425d2a33eb5533fceccecfc367cc7348 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 18:39:48 -0400 Subject: [PATCH 07/14] feat(app): add MCP org picker to user settings Adds an AI / MCP organization section to User Settings so multi-org users can choose which org their MCP connection acts on, and switch it anytime (applies on the next request, no reconnect). Renders only for users in more than one org; calls GET/PUT /v1/mcp/organization via a useSWR hook with optimistic update + rollback. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/McpOrganizationSelector.tsx | 86 +++++++++++++++++++ .../settings/user/hooks/useMcpOrganization.ts | 65 ++++++++++++++ .../app/(app)/[orgId]/settings/user/page.tsx | 64 ++++++++------ 3 files changed, 188 insertions(+), 27 deletions(-) create mode 100644 apps/app/src/app/(app)/[orgId]/settings/user/components/McpOrganizationSelector.tsx create mode 100644 apps/app/src/app/(app)/[orgId]/settings/user/hooks/useMcpOrganization.ts diff --git a/apps/app/src/app/(app)/[orgId]/settings/user/components/McpOrganizationSelector.tsx b/apps/app/src/app/(app)/[orgId]/settings/user/components/McpOrganizationSelector.tsx new file mode 100644 index 0000000000..e38ebdaf3e --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/settings/user/components/McpOrganizationSelector.tsx @@ -0,0 +1,86 @@ +'use client'; + +import { + Button, + Section, + Select, + SelectContent, + SelectItem, + SelectTrigger, + SelectValue, +} from '@trycompai/design-system'; +import { useState } from 'react'; +import { toast } from 'sonner'; +import { + type McpOrganizationData, + useMcpOrganization, +} from '../hooks/useMcpOrganization'; + +interface Props { + initialData: McpOrganizationData; +} + +export function McpOrganizationSelector({ initialData }: Props) { + const { data, saveOrganization } = useMcpOrganization({ initialData }); + const organizations = data?.organizations ?? []; + const savedOrgId = data?.selectedOrganizationId ?? null; + + const [selectedOrgId, setSelectedOrgId] = useState(savedOrgId); + const [saving, setSaving] = useState(false); + + // Only relevant for users who belong to more than one organization — + // single-org users always act on their one org automatically. + if (organizations.length <= 1) { + return null; + } + + const handleSave = async () => { + if (!selectedOrgId) { + toast.error('Select an organization first.'); + return; + } + setSaving(true); + try { + await saveOrganization(selectedOrgId); + toast.success('AI / MCP organization updated.'); + } catch { + toast.error('Failed to update organization. Please try again.'); + } finally { + setSaving(false); + } + }; + + return ( +
+ {saving ? 'Saving…' : 'Save'} + + } + > +
+ +
+
+ ); +} diff --git a/apps/app/src/app/(app)/[orgId]/settings/user/hooks/useMcpOrganization.ts b/apps/app/src/app/(app)/[orgId]/settings/user/hooks/useMcpOrganization.ts new file mode 100644 index 0000000000..2a356a1d7d --- /dev/null +++ b/apps/app/src/app/(app)/[orgId]/settings/user/hooks/useMcpOrganization.ts @@ -0,0 +1,65 @@ +'use client'; + +import { apiClient } from '@/lib/api-client'; +import useSWR from 'swr'; + +export interface McpOrganizationData { + organizations: Array<{ id: string; name: string }>; + selectedOrganizationId: string | null; +} + +export const mcpOrganizationKey = () => ['/v1/mcp/organization'] as const; + +interface UseMcpOrganizationOptions { + initialData?: McpOrganizationData; +} + +export function useMcpOrganization(options?: UseMcpOrganizationOptions) { + const { initialData } = options ?? {}; + + const { data, error, isLoading, mutate } = useSWR( + mcpOrganizationKey(), + async () => { + const response = await apiClient.get( + '/v1/mcp/organization', + ); + if (response.error) throw new Error(response.error); + return response.data ?? null; + }, + { + fallbackData: initialData, + revalidateOnMount: !initialData, + revalidateOnFocus: false, + }, + ); + + const saveOrganization = async (organizationId: string) => { + const previous = data ?? initialData ?? null; + // Optimistic update (guard against undefined per SWR safety). + if (previous) { + await mutate({ ...previous, selectedOrganizationId: organizationId }, false); + } + + try { + const response = await apiClient.put('/v1/mcp/organization', { + organizationId, + }); + if (response.error) throw new Error(response.error); + await mutate(); + } catch (err) { + // Roll back to the last known good value. + if (previous) { + await mutate(previous, false); + } + throw err; + } + }; + + return { + data: data ?? initialData ?? null, + isLoading: isLoading && !data, + error, + mutate, + saveOrganization, + }; +} diff --git a/apps/app/src/app/(app)/[orgId]/settings/user/page.tsx b/apps/app/src/app/(app)/[orgId]/settings/user/page.tsx index e8dc3ec5f9..dd8cf4431f 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/user/page.tsx +++ b/apps/app/src/app/(app)/[orgId]/settings/user/page.tsx @@ -1,6 +1,8 @@ import { serverApi } from '@/lib/api-server'; import type { Metadata } from 'next'; import { EmailNotificationPreferences } from './components/EmailNotificationPreferences'; +import { McpOrganizationSelector } from './components/McpOrganizationSelector'; +import type { McpOrganizationData } from './hooks/useMcpOrganization'; export default async function UserSettings({ params, @@ -9,38 +11,46 @@ export default async function UserSettings({ }) { const { orgId } = await params; - const res = await serverApi.get<{ - email: string; - preferences: { - policyNotifications: boolean; - taskReminders: boolean; - weeklyTaskDigest: boolean; - unassignedItemsNotifications: boolean; - taskMentions: boolean; - taskAssignments: boolean; - }; - isAdminOrOwner: boolean; - roleNotifications: { - policyNotifications: boolean; - taskReminders: boolean; - taskAssignments: boolean; - taskMentions: boolean; - weeklyTaskDigest: boolean; - findingNotifications: boolean; - } | null; - }>('/v1/people/me/email-preferences'); + const [emailRes, mcpRes] = await Promise.all([ + serverApi.get<{ + email: string; + preferences: { + policyNotifications: boolean; + taskReminders: boolean; + weeklyTaskDigest: boolean; + unassignedItemsNotifications: boolean; + taskMentions: boolean; + taskAssignments: boolean; + }; + isAdminOrOwner: boolean; + roleNotifications: { + policyNotifications: boolean; + taskReminders: boolean; + taskAssignments: boolean; + taskMentions: boolean; + weeklyTaskDigest: boolean; + findingNotifications: boolean; + } | null; + }>('/v1/people/me/email-preferences'), + serverApi.get('/v1/mcp/organization'), + ]); - if (!res.data?.email) { + if (!emailRes.data?.email) { return null; } return ( - +
+ + {mcpRes.data ? ( + + ) : null} +
); } From 28bbc112214d95ee32510b3292aa261cf6697dc6 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 19:16:00 -0400 Subject: [PATCH 08/14] fix(api): return 403 (not 401) when an MCP user must choose an org A 401 makes MCP clients re-run sign-in in a loop; the token is valid and the user just needs to pick an org, so 403 is correct and surfaces the message to the agent. Also re-throw HttpExceptions from the session-auth catch so the 403 propagates instead of collapsing to 401. Updates the message to 'try again' (no reconnect needed). Adds a proactive 'pick an organization' warning callout in the User Settings AI/MCP section when nothing is selected yet. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/auth/hybrid-auth.guard.spec.ts | 21 +++++---- apps/api/src/auth/hybrid-auth.guard.ts | 18 +++++--- .../components/McpOrganizationSelector.tsx | 46 ++++++++++++------- 3 files changed, 52 insertions(+), 33 deletions(-) diff --git a/apps/api/src/auth/hybrid-auth.guard.spec.ts b/apps/api/src/auth/hybrid-auth.guard.spec.ts index 22b0d3cb85..bf3b3715b5 100644 --- a/apps/api/src/auth/hybrid-auth.guard.spec.ts +++ b/apps/api/src/auth/hybrid-auth.guard.spec.ts @@ -1,5 +1,9 @@ import { Test, TestingModule } from '@nestjs/testing'; -import { ExecutionContext, UnauthorizedException } from '@nestjs/common'; +import { + ExecutionContext, + ForbiddenException, + UnauthorizedException, +} from '@nestjs/common'; import { Reflector } from '@nestjs/core'; import { HybridAuthGuard } from './hybrid-auth.guard'; import { ApiKeyService } from './api-key.service'; @@ -144,9 +148,8 @@ describe('HybridAuthGuard — MCP OAuth path', () => { authorization: 'Bearer mcp_access_token', }); - await expect(guard.canActivate(context)).rejects.toThrow( - 'No active organization', - ); + // 403 (authenticated, but no org) — not a 401 that would trigger re-auth. + await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); }); it('multi-org with no saved choice → asks them to pick (no silent tenant)', async () => { @@ -166,9 +169,8 @@ describe('HybridAuthGuard — MCP OAuth path', () => { authorization: 'Bearer mcp_access_token', }); - await expect(guard.canActivate(context)).rejects.toThrow( - 'multiple organizations', - ); + // 403 (token is valid — user just needs to pick an org), not a 401. + await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); // No tenant must have been bound. expect(request.organizationId).toBe(''); }); @@ -214,9 +216,8 @@ describe('HybridAuthGuard — MCP OAuth path', () => { authorization: 'Bearer mcp_access_token', }); - await expect(guard.canActivate(context)).rejects.toThrow( - 'multiple organizations', - ); + // 403 (token is valid — user just needs to pick an org), not a 401. + await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); }); it('marks platform admins from the user role', async () => { diff --git a/apps/api/src/auth/hybrid-auth.guard.ts b/apps/api/src/auth/hybrid-auth.guard.ts index f5abe2fa41..676611ce6b 100644 --- a/apps/api/src/auth/hybrid-auth.guard.ts +++ b/apps/api/src/auth/hybrid-auth.guard.ts @@ -1,6 +1,8 @@ import { CanActivate, ExecutionContext, + ForbiddenException, + HttpException, Injectable, Logger, UnauthorizedException, @@ -245,7 +247,9 @@ export class HybridAuthGuard implements CanActivate { return true; } catch (error) { - if (error instanceof UnauthorizedException) { + // Re-throw deliberate auth/permission errors as-is (e.g. the 403 from the + // MCP org-resolution path). Only unexpected failures collapse to a 401. + if (error instanceof HttpException) { throw error; } @@ -310,9 +314,9 @@ export class HybridAuthGuard implements CanActivate { }); if (memberships.length === 0) { - throw new UnauthorizedException( - 'No active organization for this MCP token.', - ); + // Authenticated, but the user has no organization — not an auth failure, + // so 403 (not 401) keeps the MCP client from looping on re-authentication. + throw new ForbiddenException('No active organization for this MCP token.'); } let member = memberships[0]; @@ -328,9 +332,11 @@ export class HybridAuthGuard implements CanActivate { ? memberships.find((m) => m.organizationId === binding.organizationId) : undefined; if (!chosen) { - throw new UnauthorizedException( + // 403 (not 401): the token is valid — the user just needs to pick an + // org. A 401 would make the MCP client re-run sign-in in a loop. + throw new ForbiddenException( 'This account belongs to multiple organizations. Choose your ' + - 'organization for AI/MCP access in Comp AI settings, then reconnect.', + 'organization for AI/MCP access in Comp AI settings, then try again.', ); } member = chosen; diff --git a/apps/app/src/app/(app)/[orgId]/settings/user/components/McpOrganizationSelector.tsx b/apps/app/src/app/(app)/[orgId]/settings/user/components/McpOrganizationSelector.tsx index e38ebdaf3e..f0c05c646d 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/user/components/McpOrganizationSelector.tsx +++ b/apps/app/src/app/(app)/[orgId]/settings/user/components/McpOrganizationSelector.tsx @@ -1,6 +1,8 @@ 'use client'; import { + Alert, + AlertDescription, Button, Section, Select, @@ -63,23 +65,33 @@ export function McpOrganizationSelector({ initialData }: Props) { } > -
- +
+ {!savedOrgId ? ( + + + Pick an organization to start using your AI assistant. Until you + choose one, AI / MCP requests can't act on your data. + + + ) : null} +
+ +
); From a7ccc9a6e4996647cecb912213d21bd110ac92b8 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 19:22:10 -0400 Subject: [PATCH 09/14] fix(api): block org-less users from the MCP entirely MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An MCP token is only usable by a member of at least one organization. The membership check now runs before the skipOrgCheck shortcut, so a user with zero memberships (a stranger who completed Google sign-in, or someone removed from all orgs) is rejected with 403 on EVERY MCP tool — including org-agnostic ones. They can never reach any org's data, and a user can only ever act on orgs they belong to. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/auth/hybrid-auth.guard.spec.ts | 22 +++++++++++++++++ apps/api/src/auth/hybrid-auth.guard.ts | 27 +++++++++++++-------- 2 files changed, 39 insertions(+), 10 deletions(-) diff --git a/apps/api/src/auth/hybrid-auth.guard.spec.ts b/apps/api/src/auth/hybrid-auth.guard.spec.ts index bf3b3715b5..2e6acda4b8 100644 --- a/apps/api/src/auth/hybrid-auth.guard.spec.ts +++ b/apps/api/src/auth/hybrid-auth.guard.spec.ts @@ -7,6 +7,7 @@ import { import { Reflector } from '@nestjs/core'; import { HybridAuthGuard } from './hybrid-auth.guard'; import { ApiKeyService } from './api-key.service'; +import { SKIP_ORG_CHECK_KEY } from './skip-org-check.decorator'; // Mock auth.server — only the two session resolvers the guard uses. const mockGetSession = jest.fn(); @@ -152,6 +153,27 @@ describe('HybridAuthGuard — MCP OAuth path', () => { await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); }); + it('blocks an org-less user even on org-agnostic (skipOrgCheck) endpoints', async () => { + // skipOrgCheck = true for this request, but the user belongs to no org — + // a "foreign" user must not be able to use the MCP at all. + jest + .spyOn(reflector, 'getAllAndOverride') + .mockImplementation((key: unknown) => key === SKIP_ORG_CHECK_KEY); + mockGetMcpSession.mockResolvedValue({ userId: 'usr_x', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_x', + email: 'stranger@example.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([]); // member of nothing + + const { context } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); + }); + it('multi-org with no saved choice → asks them to pick (no silent tenant)', async () => { mockGetMcpSession.mockResolvedValue({ userId: 'usr_5', scopes: 'openid' }); mockUserFindUnique.mockResolvedValue({ diff --git a/apps/api/src/auth/hybrid-auth.guard.ts b/apps/api/src/auth/hybrid-auth.guard.ts index 676611ce6b..a8832b6329 100644 --- a/apps/api/src/auth/hybrid-auth.guard.ts +++ b/apps/api/src/auth/hybrid-auth.guard.ts @@ -301,22 +301,29 @@ export class HybridAuthGuard implements CanActivate { request.isServiceToken = false; request.isPlatformAdmin = user.role === 'admin'; - if (skipOrgCheck) { - this.logger.log(`MCP OAuth token authenticated for user ${user.id}`); - return true; - } - - // Bind the organization explicitly by enumerating active memberships, - // mirroring the device-agent's getMyOrganizations + explicit selection. + // An MCP token is only usable by a member of at least one organization. + // Enumerate active memberships up front (device-agent style) so a user with + // none — e.g. someone who completed Google sign-in but was never invited to + // any org, or who was removed from all of them — is blocked from EVERY MCP + // tool, including the org-agnostic (skipOrgCheck) ones. const memberships = await db.member.findMany({ where: { userId, deactivated: false }, select: { id: true, role: true, department: true, organizationId: true }, }); if (memberships.length === 0) { - // Authenticated, but the user has no organization — not an auth failure, - // so 403 (not 401) keeps the MCP client from looping on re-authentication. - throw new ForbiddenException('No active organization for this MCP token.'); + // Authenticated, but a member of nothing — not an auth failure, so 403 + // (not 401) keeps the MCP client from looping on re-authentication. + throw new ForbiddenException( + 'This account is not a member of any organization, so it cannot use the MCP.', + ); + } + + // The user has an org. Endpoints that don't need a *specific* one + // (onboarding lookups) can proceed now. + if (skipOrgCheck) { + this.logger.log(`MCP OAuth token authenticated for user ${user.id}`); + return true; } let member = memberships[0]; From 653fc3be1ecc3243fcecf330cb71450a5ed5039b Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 19:30:43 -0400 Subject: [PATCH 10/14] feat(api): gate MCP on app-access role, not just org membership MCP now follows the same access rule as the web app: a user can only use it if their role grants app access (app:read) in the operative org. Owner/admin/auditor and custom roles with the App Access toggle qualify; Portal-only roles (employee/contractor) are rejected with 403. Combined with the existing 'must be a member of an org' check, this means: only customers, and only app-access roles, can use the MCP. Adds a reusable hasAppAccess(orgId, roleString) helper (built-in + custom roles, comma-separated union). The settings picker now only offers app-access orgs, and setOrganization validates app access before saving. 24 unit tests. Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/auth/app-access.spec.ts | 62 +++++++++++++++ apps/api/src/auth/app-access.ts | 51 +++++++++++++ apps/api/src/auth/hybrid-auth.guard.spec.ts | 83 ++++++++++++++++++++- apps/api/src/auth/hybrid-auth.guard.ts | 28 ++++--- apps/api/src/mcp/mcp.service.spec.ts | 51 ++++++++++--- apps/api/src/mcp/mcp.service.ts | 28 +++++-- 6 files changed, 273 insertions(+), 30 deletions(-) create mode 100644 apps/api/src/auth/app-access.spec.ts create mode 100644 apps/api/src/auth/app-access.ts diff --git a/apps/api/src/auth/app-access.spec.ts b/apps/api/src/auth/app-access.spec.ts new file mode 100644 index 0000000000..2c9741aa6c --- /dev/null +++ b/apps/api/src/auth/app-access.spec.ts @@ -0,0 +1,62 @@ +const mockOrgRoleFindMany = jest.fn(); +jest.mock('@db', () => ({ + db: { + organizationRole: { + findMany: (...a: unknown[]) => mockOrgRoleFindMany(...a), + }, + }, +})); + +jest.mock('@trycompai/auth', () => ({ + BUILT_IN_ROLE_PERMISSIONS: { + owner: { app: ['read'] }, + admin: { app: ['read'] }, + auditor: { app: ['read'] }, + employee: { policy: ['read'], portal: ['read', 'update'] }, + contractor: { policy: ['read'], portal: ['read', 'update'] }, + }, +})); + +import { hasAppAccess } from './app-access'; + +describe('hasAppAccess', () => { + beforeEach(() => { + jest.clearAllMocks(); + mockOrgRoleFindMany.mockResolvedValue([]); + }); + + it('grants for built-in app roles without a DB lookup', async () => { + expect(await hasAppAccess('org_1', 'owner')).toBe(true); + expect(await hasAppAccess('org_1', 'admin')).toBe(true); + expect(await hasAppAccess('org_1', 'auditor')).toBe(true); + expect(mockOrgRoleFindMany).not.toHaveBeenCalled(); + }); + + it('denies for Portal-only built-in roles', async () => { + expect(await hasAppAccess('org_1', 'employee')).toBe(false); + expect(await hasAppAccess('org_1', 'contractor')).toBe(false); + }); + + it('treats comma-separated roles as a union (any granting role wins)', async () => { + expect(await hasAppAccess('org_1', 'employee,admin')).toBe(true); + }); + + it('grants for a custom role with app:read', async () => { + mockOrgRoleFindMany.mockResolvedValue([ + { permissions: JSON.stringify({ app: ['read'], control: ['read'] }) }, + ]); + expect(await hasAppAccess('org_1', 'Compliance Lead')).toBe(true); + }); + + it('denies for a custom role without app:read', async () => { + mockOrgRoleFindMany.mockResolvedValue([ + { permissions: JSON.stringify({ policy: ['read'], portal: ['read'] }) }, + ]); + expect(await hasAppAccess('org_1', 'Portal Role')).toBe(false); + }); + + it('denies for empty or null roles', async () => { + expect(await hasAppAccess('org_1', null)).toBe(false); + expect(await hasAppAccess('org_1', '')).toBe(false); + }); +}); diff --git a/apps/api/src/auth/app-access.ts b/apps/api/src/auth/app-access.ts new file mode 100644 index 0000000000..49830dc954 --- /dev/null +++ b/apps/api/src/auth/app-access.ts @@ -0,0 +1,51 @@ +import { BUILT_IN_ROLE_PERMISSIONS } from '@trycompai/auth'; +import { db } from '@db'; + +/** + * Whether a member's role(s) grant **app access** (`app:read`) in the given org. + * + * This is the same gate the web app uses to decide who can use the product + * (owner/admin/auditor + custom roles with the "App Access" toggle) versus + * Portal-only roles (employee/contractor). Built-in roles resolve from the + * static `BUILT_IN_ROLE_PERMISSIONS` map; custom roles from the org's + * `organization_role` rows. `member.role` is comma-separated and treated as a + * union — ANY granting role is sufficient. + */ +export async function hasAppAccess( + organizationId: string, + roleString: string | null, +): Promise { + if (!roleString) return false; + + const roles = roleString + .split(',') + .map((r) => r.trim()) + .filter(Boolean); + if (roles.length === 0) return false; + + const customRoleNames: string[] = []; + for (const role of roles) { + const builtIn = BUILT_IN_ROLE_PERMISSIONS[role]; + if (builtIn) { + if (builtIn['app']?.includes('read')) return true; + } else { + customRoleNames.push(role); + } + } + + if (customRoleNames.length === 0) return false; + + const customRoles = await db.organizationRole.findMany({ + where: { organizationId, name: { in: customRoleNames } }, + select: { permissions: true }, + }); + for (const customRole of customRoles) { + const perms = + typeof customRole.permissions === 'string' + ? (JSON.parse(customRole.permissions) as Record) + : (customRole.permissions as Record); + if (perms?.['app']?.includes('read')) return true; + } + + return false; +} diff --git a/apps/api/src/auth/hybrid-auth.guard.spec.ts b/apps/api/src/auth/hybrid-auth.guard.spec.ts index 2e6acda4b8..e526ca6e5f 100644 --- a/apps/api/src/auth/hybrid-auth.guard.spec.ts +++ b/apps/api/src/auth/hybrid-auth.guard.spec.ts @@ -26,6 +26,7 @@ jest.mock('./auth.server', () => ({ const mockUserFindUnique = jest.fn(); const mockMemberFindMany = jest.fn(); const mockMcpBindingFindUnique = jest.fn(); +const mockOrgRoleFindMany = jest.fn(); jest.mock('@db', () => ({ db: { user: { findUnique: (...args: unknown[]) => mockUserFindUnique(...args) }, @@ -33,11 +34,23 @@ jest.mock('@db', () => ({ mcpOrgBinding: { findUnique: (...args: unknown[]) => mockMcpBindingFindUnique(...args), }, + organizationRole: { + findMany: (...args: unknown[]) => mockOrgRoleFindMany(...args), + }, }, })); -// Avoid ESM issues from the api-key.service import chain (it imports @trycompai/auth). -jest.mock('@trycompai/auth', () => ({})); +// Mock @trycompai/auth — the app-access gate reads BUILT_IN_ROLE_PERMISSIONS to +// decide which roles grant app access. owner/admin/auditor do; employee does not. +jest.mock('@trycompai/auth', () => ({ + BUILT_IN_ROLE_PERMISSIONS: { + owner: { app: ['read'] }, + admin: { app: ['read'] }, + auditor: { app: ['read'] }, + employee: { policy: ['read'], portal: ['read', 'update'] }, + contractor: { policy: ['read'], portal: ['read', 'update'] }, + }, +})); describe('HybridAuthGuard — MCP OAuth path', () => { let guard: HybridAuthGuard; @@ -77,6 +90,8 @@ describe('HybridAuthGuard — MCP OAuth path', () => { mockGetSession.mockResolvedValue(null); // No org binding by default; individual tests override. mockMcpBindingFindUnique.mockResolvedValue(null); + // No custom roles by default (built-in roles resolve without a DB call). + mockOrgRoleFindMany.mockResolvedValue([]); }); it('authenticates a single-org user (admin) and binds org + roles', async () => { @@ -260,4 +275,68 @@ describe('HybridAuthGuard — MCP OAuth path', () => { await expect(guard.canActivate(context)).resolves.toBe(true); expect(request.isPlatformAdmin).toBe(true); }); + + it('blocks a Portal-only role (employee) — no app access, no MCP', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_e', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_e', + email: 'employee@acme.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_e', role: 'employee', department: 'none', organizationId: 'org_1' }, + ]); + + const { context, request } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); + expect(request.organizationId).toBe(''); + }); + + it('allows a custom role that grants app access', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_c', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_c', + email: 'custom@acme.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_c', role: 'Compliance Lead', department: 'none', organizationId: 'org_1' }, + ]); + // Custom role resolved from organization_role with app access granted. + mockOrgRoleFindMany.mockResolvedValue([ + { permissions: JSON.stringify({ app: ['read'], control: ['read'] }) }, + ]); + + const { context, request } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(request.organizationId).toBe('org_1'); + expect(request.userRoles).toEqual(['Compliance Lead']); + }); + + it('blocks a custom role that lacks app access', async () => { + mockGetMcpSession.mockResolvedValue({ userId: 'usr_d', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_d', + email: 'limited@acme.com', + role: 'user', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_d', role: 'Read Only Portal', department: 'none', organizationId: 'org_1' }, + ]); + mockOrgRoleFindMany.mockResolvedValue([ + { permissions: JSON.stringify({ policy: ['read'], portal: ['read'] }) }, + ]); + + const { context } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).rejects.toThrow(ForbiddenException); + }); }); diff --git a/apps/api/src/auth/hybrid-auth.guard.ts b/apps/api/src/auth/hybrid-auth.guard.ts index a8832b6329..4c73a7a3df 100644 --- a/apps/api/src/auth/hybrid-auth.guard.ts +++ b/apps/api/src/auth/hybrid-auth.guard.ts @@ -10,6 +10,7 @@ import { import { Reflector } from '@nestjs/core'; import { db } from '@db'; import { ApiKeyService } from './api-key.service'; +import { hasAppAccess } from './app-access'; import { auth } from './auth.server'; import { IS_PUBLIC_KEY } from './public.decorator'; import { SKIP_ORG_CHECK_KEY } from './skip-org-check.decorator'; @@ -173,7 +174,7 @@ export class HybridAuthGuard implements CanActivate { if (!session) { // Fallback: the hosted MCP server (Gram) sends an OAuth access token as a // Bearer token, which getSession does not resolve. Try the MCP OAuth path. - if (await this.tryMcpOAuthAuth(request, headers, skipOrgCheck)) { + if (await this.tryMcpOAuthAuth(request, headers)) { return true; } throw new UnauthorizedException('Invalid or expired session'); @@ -272,11 +273,16 @@ export class HybridAuthGuard implements CanActivate { * still a member; otherwise we ask them to choose rather than guess a tenant. * Roles come from the resolved member so the existing PermissionGuard enforces * RBAC unchanged. + * + * Two hard gates (both 403, never 401): the user must (1) be a member of an + * organization at all — strangers who merely completed sign-in are rejected — + * and (2) hold a role with app access (`app:read`) in the operative org, the + * same rule the web app uses. Portal-only roles (employee/contractor) cannot + * use the MCP. */ private async tryMcpOAuthAuth( request: AuthenticatedRequest, headers: Headers, - skipOrgCheck: boolean, ): Promise { const token = await auth.api.getMcpSession({ headers }).catch(() => null); if (!token?.userId) { @@ -319,13 +325,6 @@ export class HybridAuthGuard implements CanActivate { ); } - // The user has an org. Endpoints that don't need a *specific* one - // (onboarding lookups) can proceed now. - if (skipOrgCheck) { - this.logger.log(`MCP OAuth token authenticated for user ${user.id}`); - return true; - } - let member = memberships[0]; if (memberships.length > 1) { // Multi-org: use the org the user chose for MCP (set at connect time), @@ -348,6 +347,17 @@ export class HybridAuthGuard implements CanActivate { } member = chosen; } + + // App-access gate: MCP follows the same rule as the web app — only roles + // that grant app access (`app:read`) may use it. Portal-only roles + // (employee/contractor, or custom roles without app access) are rejected. + if (!(await hasAppAccess(member.organizationId, member.role))) { + throw new ForbiddenException( + "Your role doesn't have access to the app, so it can't use the MCP. " + + 'Ask an organization admin for access.', + ); + } + request.organizationId = member.organizationId; request.memberId = member.id; request.memberDepartment = member.department; diff --git a/apps/api/src/mcp/mcp.service.spec.ts b/apps/api/src/mcp/mcp.service.spec.ts index af47634362..ea00801885 100644 --- a/apps/api/src/mcp/mcp.service.spec.ts +++ b/apps/api/src/mcp/mcp.service.spec.ts @@ -4,6 +4,7 @@ const mockMemberFindMany = jest.fn(); const mockMemberFindFirst = jest.fn(); const mockBindingFindUnique = jest.fn(); const mockBindingUpsert = jest.fn(); +const mockOrgRoleFindMany = jest.fn(); jest.mock('@db', () => ({ db: { member: { @@ -14,6 +15,19 @@ jest.mock('@db', () => ({ findUnique: (...a: unknown[]) => mockBindingFindUnique(...a), upsert: (...a: unknown[]) => mockBindingUpsert(...a), }, + organizationRole: { + findMany: (...a: unknown[]) => mockOrgRoleFindMany(...a), + }, + }, +})); + +jest.mock('@trycompai/auth', () => ({ + BUILT_IN_ROLE_PERMISSIONS: { + owner: { app: ['read'] }, + admin: { app: ['read'] }, + auditor: { app: ['read'] }, + employee: { policy: ['read'], portal: ['read', 'update'] }, + contractor: { policy: ['read'], portal: ['read', 'update'] }, }, })); @@ -24,29 +38,32 @@ describe('McpService', () => { beforeEach(() => { jest.clearAllMocks(); + mockOrgRoleFindMany.mockResolvedValue([]); service = new McpService(); }); describe('getOrganizationSelection', () => { - it('returns the user orgs and the current valid selection', async () => { + it('returns only app-access orgs and the current selection', async () => { mockMemberFindMany.mockResolvedValue([ - { organization: { id: 'org_a', name: 'Acme' } }, - { organization: { id: 'org_b', name: 'Beta' } }, + { role: 'owner', organization: { id: 'org_a', name: 'Acme' } }, + // Portal-only — must be excluded. + { role: 'employee', organization: { id: 'org_b', name: 'Beta' } }, + { role: 'admin', organization: { id: 'org_c', name: 'Gamma' } }, ]); - mockBindingFindUnique.mockResolvedValue({ organizationId: 'org_b' }); + mockBindingFindUnique.mockResolvedValue({ organizationId: 'org_c' }); const result = await service.getOrganizationSelection('usr_1'); expect(result.organizations).toEqual([ { id: 'org_a', name: 'Acme' }, - { id: 'org_b', name: 'Beta' }, + { id: 'org_c', name: 'Gamma' }, ]); - expect(result.selectedOrganizationId).toBe('org_b'); + expect(result.selectedOrganizationId).toBe('org_c'); }); - it('drops a stale selection the user is no longer a member of', async () => { + it('drops a selection the user can no longer use', async () => { mockMemberFindMany.mockResolvedValue([ - { organization: { id: 'org_a', name: 'Acme' } }, + { role: 'owner', organization: { id: 'org_a', name: 'Acme' } }, ]); mockBindingFindUnique.mockResolvedValue({ organizationId: 'org_gone' }); @@ -55,21 +72,22 @@ describe('McpService', () => { expect(result.selectedOrganizationId).toBeNull(); }); - it('returns null selection when none is set', async () => { + it('excludes Portal-only orgs entirely', async () => { mockMemberFindMany.mockResolvedValue([ - { organization: { id: 'org_a', name: 'Acme' } }, + { role: 'employee', organization: { id: 'org_b', name: 'Beta' } }, ]); mockBindingFindUnique.mockResolvedValue(null); const result = await service.getOrganizationSelection('usr_1'); + expect(result.organizations).toEqual([]); expect(result.selectedOrganizationId).toBeNull(); }); }); describe('setOrganization', () => { - it('upserts the binding when the user is a member', async () => { - mockMemberFindFirst.mockResolvedValue({ id: 'mem_1' }); + it('saves when the user is a member with app access', async () => { + mockMemberFindFirst.mockResolvedValue({ role: 'admin' }); mockBindingUpsert.mockResolvedValue({}); const result = await service.setOrganization('usr_1', 'org_a'); @@ -90,5 +108,14 @@ describe('McpService', () => { ); expect(mockBindingUpsert).not.toHaveBeenCalled(); }); + + it('rejects a member whose role lacks app access', async () => { + mockMemberFindFirst.mockResolvedValue({ role: 'employee' }); + + await expect(service.setOrganization('usr_1', 'org_b')).rejects.toThrow( + ForbiddenException, + ); + expect(mockBindingUpsert).not.toHaveBeenCalled(); + }); }); }); diff --git a/apps/api/src/mcp/mcp.service.ts b/apps/api/src/mcp/mcp.service.ts index ba1e47ee85..618279f08c 100644 --- a/apps/api/src/mcp/mcp.service.ts +++ b/apps/api/src/mcp/mcp.service.ts @@ -1,21 +1,30 @@ import { ForbiddenException, Injectable } from '@nestjs/common'; import { db } from '@db'; +import { hasAppAccess } from '../auth/app-access'; @Injectable() export class McpService { /** * The organizations the user can choose from for MCP access, plus their - * current selection (null when unset or no longer valid). + * current selection (null when unset or no longer valid). Only orgs where the + * user's role grants app access are offered — picking one without it wouldn't + * work (the MCP guard would reject it). */ async getOrganizationSelection(userId: string) { const memberships = await db.member.findMany({ where: { userId, deactivated: false }, - select: { organization: { select: { id: true, name: true } } }, + select: { role: true, organization: { select: { id: true, name: true } } }, }); - const organizations = memberships.map((m) => ({ - id: m.organization.id, - name: m.organization.name, - })); + + const organizations: Array<{ id: string; name: string }> = []; + for (const membership of memberships) { + if (await hasAppAccess(membership.organization.id, membership.role)) { + organizations.push({ + id: membership.organization.id, + name: membership.organization.name, + }); + } + } const binding = await db.mcpOrgBinding.findUnique({ where: { userId }, @@ -37,13 +46,18 @@ export class McpService { async setOrganization(userId: string, organizationId: string) { const member = await db.member.findFirst({ where: { userId, organizationId, deactivated: false }, - select: { id: true }, + select: { role: true }, }); if (!member) { throw new ForbiddenException( 'You are not a member of the selected organization.', ); } + if (!(await hasAppAccess(organizationId, member.role))) { + throw new ForbiddenException( + "Your role in that organization doesn't have app access, so it can't be used for the MCP.", + ); + } await db.mcpOrgBinding.upsert({ where: { userId }, From bd6ef4cb00695b8511da9c185adaada7210f74ef Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 19:41:05 -0400 Subject: [PATCH 11/14] docs: auto-attach MCP endpoint-contract rule + sync descriptions rule MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cursor rule now auto-attaches on apps/api controllers + DTOs via globs (not just description-triggered), so it applies reliably when editing endpoints. AGENTS.md (Codex/agents) gains the rule that every endpoint needs a meaningful @ApiOperation summary/description — CI-enforced and required for dynamic-toolset discovery — keeping it in sync with the Claude skill + Cursor rule. Co-Authored-By: Claude Opus 4.8 (1M context) --- .cursor/rules/api-endpoint-contract.mdc | 1 + AGENTS.md | 1 + 2 files changed, 2 insertions(+) diff --git a/.cursor/rules/api-endpoint-contract.mdc b/.cursor/rules/api-endpoint-contract.mdc index d8d2187232..9c3d3ef6a6 100644 --- a/.cursor/rules/api-endpoint-contract.mdc +++ b/.cursor/rules/api-endpoint-contract.mdc @@ -1,5 +1,6 @@ --- description: Use when writing/editing NestJS API endpoints, DTOs, or @Body() params under apps/api/src/. Ensures every endpoint is correct for OpenAPI, the MCP server (@trycompai/mcp-server), and the ValidationPipe. +globs: apps/api/src/**/*.controller.ts,apps/api/src/**/*.dto.ts alwaysApply: false --- diff --git a/AGENTS.md b/AGENTS.md index cd95151c0d..be4d3e28bb 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -82,6 +82,7 @@ Every customer-facing endpoint in `apps/api/src/` flows into three systems: the 8. **File uploads from agents use presigned URLs** — accept an `s3Key` field (read via `UploadsService.readUploadAsBase64`); never accept inline base64 from the MCP tool. 9. **Sensitive paths (e.g. `/credentials`)** are deny-listed from public docs in `apps/api/src/openapi/public-docs-quality.ts` — that's intentional, don't fight it. 10. **SSE / binary responses** can't be consumed by MCP — disable the tool in `apps/mcp-server/.speakeasy/mcp-uploads-overlay.yaml` while keeping the HTTP endpoint for the web UI. +11. **Every endpoint needs a meaningful `@ApiOperation({ summary, description })`** — required and **CI-enforced** (`openapi-docs.spec.ts` fails the build if a public op is missing one). The hosted MCP uses **dynamic toolsets**: the agent finds a tool by semantic-searching names + descriptions, so a missing/weak description makes the tool effectively undiscoverable. Write the description for the agent deciding whether to call the tool — what it does + when to use it. After adding an endpoint: `bun run --filter '@trycompai/api' dev` regenerates `packages/docs/openapi.json` on boot — **commit it with your PR**. The daily Speakeasy CI reads from that file; if it's stale, your endpoint never reaches MCP customers. From 7b61ed56b94835ec7fcd47b08802ad1cd4567200 Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 19:48:46 -0400 Subject: [PATCH 12/14] fix(api): add PermissionGuard + app:read to MCP controller MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses cubic P1: the MCP controller had HybridAuthGuard only, so the PUT bypassed API-key scope enforcement and the mutation audit hook (AuditLogInterceptor only logs when @RequirePermission is present). Now matches the rest of the API (e.g. people controller): class-level HybridAuthGuard + PermissionGuard, with @RequirePermission('app','read') on both endpoints — gating on app access (consistent with the MCP access rule) and logging the PUT mutation. Dropped @SkipOrgCheck (these are web-only, deny-listed from MCP; an active org is present for web sessions, which PermissionGuard + the audit log need). Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/mcp/mcp.controller.ts | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/apps/api/src/mcp/mcp.controller.ts b/apps/api/src/mcp/mcp.controller.ts index 1e3f62e212..bf91207984 100644 --- a/apps/api/src/mcp/mcp.controller.ts +++ b/apps/api/src/mcp/mcp.controller.ts @@ -2,24 +2,30 @@ import { Body, Controller, Get, Put, UseGuards } from '@nestjs/common'; import { ApiBody, ApiOperation, ApiSecurity, ApiTags } from '@nestjs/swagger'; import { UserId } from '../auth/auth-context.decorator'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; -import { SkipOrgCheck } from '../auth/skip-org-check.decorator'; +import { PermissionGuard } from '../auth/permission.guard'; +import { RequirePermission } from '../auth/require-permission.decorator'; import { SetMcpOrganizationDto } from './dto/set-mcp-organization.dto'; import { McpService } from './mcp.service'; /** * MCP account-management endpoints (web app only — excluded from the public * OpenAPI spec / MCP tools via the deny-list in public-docs-quality.ts). - * @SkipOrgCheck because choosing among your organizations isn't scoped to one. + * + * Gated on app access (`app:read`) like the rest of the product: only roles that + * can use the app may manage its MCP settings. Going through PermissionGuard + + * @RequirePermission also enforces API-key scopes and records the mutation in + * the audit log (the AuditLogInterceptor only logs when @RequirePermission is + * present). */ @ApiTags('MCP') @Controller({ path: 'mcp', version: '1' }) -@UseGuards(HybridAuthGuard) +@UseGuards(HybridAuthGuard, PermissionGuard) @ApiSecurity('apikey') export class McpController { constructor(private readonly mcpService: McpService) {} @Get('organization') - @SkipOrgCheck() + @RequirePermission('app', 'read') @ApiOperation({ summary: 'Get your MCP organization selection', description: @@ -30,7 +36,7 @@ export class McpController { } @Put('organization') - @SkipOrgCheck() + @RequirePermission('app', 'read') @ApiOperation({ summary: 'Set your MCP organization', description: From 03dc24a2b2cc6fb86714c8856587bc3f458328bc Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 20:08:52 -0400 Subject: [PATCH 13/14] fix(api): address cubic review findings on the MCP auth path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1: PermissionGuard rejected MCP OAuth calls — it authorizes via session-based auth.api.hasPermission, but OAuth tokens aren't sessions, so every permission-gated MCP call would 403. Added an isMcpOAuth marker + a guard branch that authorizes from the roles already resolved by HybridAuthGuard (via resolveRolePermissions). This was the critical one — the whole OAuth flow was broken without it. P1: MCP controller used @UserId() with no session-only guard, so a scoped API key would pass the guards then crash at @UserId() with a 500. Added SessionOnlyGuard (clean 403 for API keys/service tokens). P2: app-access.ts hardened — guarded JSON.parse (malformed custom-role perms no longer throw) and own-property role lookup (a custom role named e.g. 'constructor' is no longer misclassified as built-in). P2: mcp.service resolves app-access concurrently (Promise.all) instead of a serial N+1 loop. Adds tests for the MCP OAuth authorization path + the robustness cases (46 auth/mcp tests green). Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/auth/app-access.spec.ts | 14 +++ apps/api/src/auth/app-access.ts | 116 +++++++++++++++------ apps/api/src/auth/hybrid-auth.guard.ts | 1 + apps/api/src/auth/permission.guard.spec.ts | 58 +++++++++++ apps/api/src/auth/permission.guard.ts | 22 ++++ apps/api/src/auth/types.ts | 1 + apps/api/src/mcp/mcp.controller.ts | 14 +-- apps/api/src/mcp/mcp.service.ts | 16 +-- 8 files changed, 195 insertions(+), 47 deletions(-) diff --git a/apps/api/src/auth/app-access.spec.ts b/apps/api/src/auth/app-access.spec.ts index 2c9741aa6c..85524669e8 100644 --- a/apps/api/src/auth/app-access.spec.ts +++ b/apps/api/src/auth/app-access.spec.ts @@ -59,4 +59,18 @@ describe('hasAppAccess', () => { expect(await hasAppAccess('org_1', null)).toBe(false); expect(await hasAppAccess('org_1', '')).toBe(false); }); + + it('treats a role named like an Object prototype key (constructor) as custom', async () => { + // Must NOT be shadowed by Object.prototype.constructor — it's a custom role. + mockOrgRoleFindMany.mockResolvedValue([ + { permissions: JSON.stringify({ app: ['read'] }) }, + ]); + expect(await hasAppAccess('org_1', 'constructor')).toBe(true); + expect(mockOrgRoleFindMany).toHaveBeenCalled(); + }); + + it('does not throw on malformed custom-role permissions', async () => { + mockOrgRoleFindMany.mockResolvedValue([{ permissions: '{not valid json' }]); + await expect(hasAppAccess('org_1', 'Broken Role')).resolves.toBe(false); + }); }); diff --git a/apps/api/src/auth/app-access.ts b/apps/api/src/auth/app-access.ts index 49830dc954..b8bce4303d 100644 --- a/apps/api/src/auth/app-access.ts +++ b/apps/api/src/auth/app-access.ts @@ -1,51 +1,99 @@ import { BUILT_IN_ROLE_PERMISSIONS } from '@trycompai/auth'; import { db } from '@db'; +/** Safely parse a custom role's stored permissions; malformed JSON → `{}` (never throws). */ +function parsePermissions(raw: unknown): Record { + if (raw && typeof raw === 'object') { + return raw as Record; + } + if (typeof raw === 'string') { + try { + const parsed = JSON.parse(raw); + return parsed && typeof parsed === 'object' + ? (parsed as Record) + : {}; + } catch { + return {}; + } + } + return {}; +} + +function mergeInto( + target: Record>, + perms: Record, +): void { + for (const [resource, actions] of Object.entries(perms)) { + if (!Array.isArray(actions)) continue; + (target[resource] ??= new Set()); + for (const action of actions) target[resource].add(action); + } +} + /** - * Whether a member's role(s) grant **app access** (`app:read`) in the given org. - * - * This is the same gate the web app uses to decide who can use the product - * (owner/admin/auditor + custom roles with the "App Access" toggle) versus - * Portal-only roles (employee/contractor). Built-in roles resolve from the - * static `BUILT_IN_ROLE_PERMISSIONS` map; custom roles from the org's - * `organization_role` rows. `member.role` is comma-separated and treated as a - * union — ANY granting role is sufficient. + * Merge the effective permissions (`{ resource: actions[] }`) for a set of role + * names in an org. Built-in roles resolve from `BUILT_IN_ROLE_PERMISSIONS` + * (own-property lookup only, so a custom role named e.g. `constructor` is not + * mistaken for a built-in); custom roles resolve from `organization_role` rows + * (malformed JSON is ignored, not thrown). Comma-separated roles are a union. + */ +export async function resolveRolePermissions( + organizationId: string, + roles: string[], +): Promise> { + const merged: Record> = {}; + const customRoleNames: string[] = []; + + for (const role of roles) { + if (Object.prototype.hasOwnProperty.call(BUILT_IN_ROLE_PERMISSIONS, role)) { + mergeInto(merged, BUILT_IN_ROLE_PERMISSIONS[role]); + } else if (role) { + customRoleNames.push(role); + } + } + + if (customRoleNames.length > 0) { + const customRoles = await db.organizationRole.findMany({ + where: { organizationId, name: { in: customRoleNames } }, + select: { permissions: true }, + }); + for (const customRole of customRoles) { + mergeInto(merged, parsePermissions(customRole.permissions)); + } + } + + const result: Record = {}; + for (const [resource, actions] of Object.entries(merged)) { + result[resource] = [...actions]; + } + return result; +} + +/** Whether resolved permissions grant `resource:action`. */ +export function permissionsGrant( + permissions: Record, + resource: string, + action: string, +): boolean { + return permissions[resource]?.includes(action) ?? false; +} + +/** + * Whether a member's role(s) grant **app access** (`app:read`) in the given org + * — the same gate the web app uses (owner/admin/auditor + custom roles with the + * "App Access" toggle), excluding Portal-only roles (employee/contractor). */ export async function hasAppAccess( organizationId: string, roleString: string | null, ): Promise { if (!roleString) return false; - const roles = roleString .split(',') .map((r) => r.trim()) .filter(Boolean); if (roles.length === 0) return false; - const customRoleNames: string[] = []; - for (const role of roles) { - const builtIn = BUILT_IN_ROLE_PERMISSIONS[role]; - if (builtIn) { - if (builtIn['app']?.includes('read')) return true; - } else { - customRoleNames.push(role); - } - } - - if (customRoleNames.length === 0) return false; - - const customRoles = await db.organizationRole.findMany({ - where: { organizationId, name: { in: customRoleNames } }, - select: { permissions: true }, - }); - for (const customRole of customRoles) { - const perms = - typeof customRole.permissions === 'string' - ? (JSON.parse(customRole.permissions) as Record) - : (customRole.permissions as Record); - if (perms?.['app']?.includes('read')) return true; - } - - return false; + const perms = await resolveRolePermissions(organizationId, roles); + return permissionsGrant(perms, 'app', 'read'); } diff --git a/apps/api/src/auth/hybrid-auth.guard.ts b/apps/api/src/auth/hybrid-auth.guard.ts index 4c73a7a3df..6fc0883088 100644 --- a/apps/api/src/auth/hybrid-auth.guard.ts +++ b/apps/api/src/auth/hybrid-auth.guard.ts @@ -305,6 +305,7 @@ export class HybridAuthGuard implements CanActivate { request.authType = 'session'; request.isApiKey = false; request.isServiceToken = false; + request.isMcpOAuth = true; request.isPlatformAdmin = user.role === 'admin'; // An MCP token is only usable by a member of at least one organization. diff --git a/apps/api/src/auth/permission.guard.spec.ts b/apps/api/src/auth/permission.guard.spec.ts index 46ac2ec522..7287392fae 100644 --- a/apps/api/src/auth/permission.guard.spec.ts +++ b/apps/api/src/auth/permission.guard.spec.ts @@ -19,6 +19,20 @@ jest.mock('@trycompai/auth', () => ({ PRIVILEGED_ROLES: ['owner', 'admin', 'auditor'], })); +// Mock ./app-access (used to authorize MCP OAuth requests). Mocked here so the +// spec doesn't pull in @db via the real module; permissionsGrant uses the real +// (trivial) logic so only resolveRolePermissions needs stubbing. +const mockResolveRolePermissions = jest.fn(); +jest.mock('./app-access', () => ({ + resolveRolePermissions: (...args: unknown[]) => + mockResolveRolePermissions(...args), + permissionsGrant: ( + perms: Record, + resource: string, + action: string, + ) => perms?.[resource]?.includes(action) ?? false, +})); + describe('PermissionGuard', () => { let guard: PermissionGuard; let reflector: Reflector; @@ -26,6 +40,8 @@ describe('PermissionGuard', () => { const createMockExecutionContext = ( request: Partial<{ isApiKey: boolean; + isMcpOAuth: boolean; + isPlatformAdmin: boolean; apiKeyScopes: string[] | undefined; userRoles: string[] | null; headers: Record; @@ -60,6 +76,48 @@ describe('PermissionGuard', () => { guard = module.get(PermissionGuard); reflector = module.get(Reflector); mockHasPermission.mockReset(); + mockResolveRolePermissions.mockReset(); + }); + + describe('MCP OAuth authorization', () => { + it('authorizes via resolved roles, not session hasPermission', async () => { + jest + .spyOn(reflector, 'getAllAndOverride') + .mockReturnValue([{ resource: 'control', actions: ['read'] }]); + mockResolveRolePermissions.mockResolvedValue({ + control: ['read', 'create'], + }); + + const context = createMockExecutionContext({ + isMcpOAuth: true, + userRoles: ['admin'], + organizationId: 'org_1', + }); + + await expect(guard.canActivate(context)).resolves.toBe(true); + // Session-based hasPermission must NOT be used for MCP OAuth tokens. + expect(mockHasPermission).not.toHaveBeenCalled(); + expect(mockResolveRolePermissions).toHaveBeenCalledWith('org_1', [ + 'admin', + ]); + }); + + it('denies MCP OAuth when resolved roles lack the permission', async () => { + jest + .spyOn(reflector, 'getAllAndOverride') + .mockReturnValue([{ resource: 'control', actions: ['delete'] }]); + mockResolveRolePermissions.mockResolvedValue({ control: ['read'] }); + + const context = createMockExecutionContext({ + isMcpOAuth: true, + userRoles: ['auditor'], + organizationId: 'org_1', + }); + + await expect(guard.canActivate(context)).rejects.toThrow( + ForbiddenException, + ); + }); }); describe('canActivate', () => { diff --git a/apps/api/src/auth/permission.guard.ts b/apps/api/src/auth/permission.guard.ts index 033922d499..e8d0434e2b 100644 --- a/apps/api/src/auth/permission.guard.ts +++ b/apps/api/src/auth/permission.guard.ts @@ -7,6 +7,7 @@ import { } from '@nestjs/common'; import { Reflector } from '@nestjs/core'; import { RESTRICTED_ROLES, PRIVILEGED_ROLES } from '@trycompai/auth'; +import { permissionsGrant, resolveRolePermissions } from './app-access'; import { auth } from './auth.server'; import { resolveServiceByName } from './service-token.config'; import { AuthenticatedRequest } from './types'; @@ -129,6 +130,27 @@ export class PermissionGuard implements CanActivate { : perm.actions; } + // MCP OAuth tokens are not better-auth sessions, so `hasPermission` (which + // resolves a session + active org) can't authorize them. Check the required + // permissions against the roles HybridAuthGuard already resolved for the + // bound org. (Mirrors better-auth's union-of-roles semantics.) + if (request.isMcpOAuth) { + const perms = await resolveRolePermissions( + request.organizationId, + request.userRoles ?? [], + ); + const granted = Object.entries(permissionBody).every(([resource, actions]) => + actions.every((action) => permissionsGrant(perms, resource, action)), + ); + if (!granted) { + this.logger.warn( + `[PermissionGuard] MCP OAuth access denied for ${request.method} ${request.url}. Required: ${JSON.stringify(permissionBody)}`, + ); + throw new ForbiddenException('Access denied'); + } + return true; + } + try { const hasPermission = await this.checkPermission(request, permissionBody); diff --git a/apps/api/src/auth/types.ts b/apps/api/src/auth/types.ts index 2e89f19102..a7a416ad01 100644 --- a/apps/api/src/auth/types.ts +++ b/apps/api/src/auth/types.ts @@ -20,6 +20,7 @@ export interface AuthenticatedRequest extends Request { impersonatedBy?: string; // User ID of the admin who initiated impersonation (only set during impersonation sessions) sessionId?: string; // Session ID (only set for session auth) sessionDeviceAgent?: boolean; // Whether the session is a device-agent session (only set for session auth) + isMcpOAuth?: boolean; // True when authenticated via a hosted-MCP OAuth token (no real session). PermissionGuard checks RBAC from userRoles instead of better-auth's session-based hasPermission. } export interface AuthContext { diff --git a/apps/api/src/mcp/mcp.controller.ts b/apps/api/src/mcp/mcp.controller.ts index bf91207984..1da8feb87d 100644 --- a/apps/api/src/mcp/mcp.controller.ts +++ b/apps/api/src/mcp/mcp.controller.ts @@ -4,6 +4,7 @@ import { UserId } from '../auth/auth-context.decorator'; import { HybridAuthGuard } from '../auth/hybrid-auth.guard'; import { PermissionGuard } from '../auth/permission.guard'; import { RequirePermission } from '../auth/require-permission.decorator'; +import { SessionOnlyGuard } from '../auth/session-only.guard'; import { SetMcpOrganizationDto } from './dto/set-mcp-organization.dto'; import { McpService } from './mcp.service'; @@ -11,15 +12,16 @@ import { McpService } from './mcp.service'; * MCP account-management endpoints (web app only — excluded from the public * OpenAPI spec / MCP tools via the deny-list in public-docs-quality.ts). * - * Gated on app access (`app:read`) like the rest of the product: only roles that - * can use the app may manage its MCP settings. Going through PermissionGuard + - * @RequirePermission also enforces API-key scopes and records the mutation in - * the audit log (the AuditLogInterceptor only logs when @RequirePermission is - * present). + * Session-only (these are user self-management actions — `SessionOnlyGuard` + * rejects API keys / service tokens with a clean 403 instead of `@UserId()` + * throwing a 500), and gated on app access (`app:read`) like the rest of the + * product. Going through PermissionGuard + @RequirePermission also records the + * PUT mutation in the audit log (the AuditLogInterceptor only logs when + * @RequirePermission is present). */ @ApiTags('MCP') @Controller({ path: 'mcp', version: '1' }) -@UseGuards(HybridAuthGuard, PermissionGuard) +@UseGuards(HybridAuthGuard, SessionOnlyGuard, PermissionGuard) @ApiSecurity('apikey') export class McpController { constructor(private readonly mcpService: McpService) {} diff --git a/apps/api/src/mcp/mcp.service.ts b/apps/api/src/mcp/mcp.service.ts index 618279f08c..9f6aa2a3d2 100644 --- a/apps/api/src/mcp/mcp.service.ts +++ b/apps/api/src/mcp/mcp.service.ts @@ -16,15 +16,17 @@ export class McpService { select: { role: true, organization: { select: { id: true, name: true } } }, }); - const organizations: Array<{ id: string; name: string }> = []; - for (const membership of memberships) { - if (await hasAppAccess(membership.organization.id, membership.role)) { - organizations.push({ + // Resolve app-access for every membership concurrently (avoid serial N+1). + const checks = await Promise.all( + memberships.map(async (membership) => ({ + org: { id: membership.organization.id, name: membership.organization.name, - }); - } - } + }, + allowed: await hasAppAccess(membership.organization.id, membership.role), + })), + ); + const organizations = checks.filter((c) => c.allowed).map((c) => c.org); const binding = await db.mcpOrgBinding.findUnique({ where: { userId }, From 61c8016fad7fe0bf8e4cac4a6c0077e6168698ca Mon Sep 17 00:00:00 2001 From: Tofik Hasanov Date: Thu, 28 May 2026 20:17:02 -0400 Subject: [PATCH 14/14] fix: platform-admin MCP bypass + revalidate on save error P2: platform admins were blocked by the MCP app-access gate in HybridAuthGuard before PermissionGuard's isPlatformAdmin bypass could apply, incorrectly denying privileged users whose member role lacks app access. Now the gate is skipped for platform admins, consistent with the normal session path. P2: useMcpOrganization rolled back to a snapshot on save error, which can restore a stale selection. Now revalidates from the server instead (matches the team's SWR norm). Co-Authored-By: Claude Opus 4.8 (1M context) --- apps/api/src/auth/hybrid-auth.guard.spec.ts | 22 +++++++++++++++++++ apps/api/src/auth/hybrid-auth.guard.ts | 7 +++++- .../settings/user/hooks/useMcpOrganization.ts | 7 +++--- 3 files changed, 31 insertions(+), 5 deletions(-) diff --git a/apps/api/src/auth/hybrid-auth.guard.spec.ts b/apps/api/src/auth/hybrid-auth.guard.spec.ts index e526ca6e5f..ec4b7ce528 100644 --- a/apps/api/src/auth/hybrid-auth.guard.spec.ts +++ b/apps/api/src/auth/hybrid-auth.guard.spec.ts @@ -276,6 +276,28 @@ describe('HybridAuthGuard — MCP OAuth path', () => { expect(request.isPlatformAdmin).toBe(true); }); + it('lets a platform admin through even with a non-app-access member role', async () => { + // Platform admin (user.role='admin') who is only an employee in the org — + // should bypass the app-access gate, consistent with PermissionGuard. + mockGetMcpSession.mockResolvedValue({ userId: 'usr_pa', scopes: 'openid' }); + mockUserFindUnique.mockResolvedValue({ + id: 'usr_pa', + email: 'staff@trycomp.ai', + role: 'admin', + }); + mockMemberFindMany.mockResolvedValue([ + { id: 'mem_pa', role: 'employee', department: 'none', organizationId: 'org_1' }, + ]); + + const { context, request } = createContext({ + authorization: 'Bearer mcp_access_token', + }); + + await expect(guard.canActivate(context)).resolves.toBe(true); + expect(request.organizationId).toBe('org_1'); + expect(request.isPlatformAdmin).toBe(true); + }); + it('blocks a Portal-only role (employee) — no app access, no MCP', async () => { mockGetMcpSession.mockResolvedValue({ userId: 'usr_e', scopes: 'openid' }); mockUserFindUnique.mockResolvedValue({ diff --git a/apps/api/src/auth/hybrid-auth.guard.ts b/apps/api/src/auth/hybrid-auth.guard.ts index 6fc0883088..0937b91d3b 100644 --- a/apps/api/src/auth/hybrid-auth.guard.ts +++ b/apps/api/src/auth/hybrid-auth.guard.ts @@ -352,7 +352,12 @@ export class HybridAuthGuard implements CanActivate { // App-access gate: MCP follows the same rule as the web app — only roles // that grant app access (`app:read`) may use it. Portal-only roles // (employee/contractor, or custom roles without app access) are rejected. - if (!(await hasAppAccess(member.organizationId, member.role))) { + // Platform admins bypass this, consistent with PermissionGuard's own + // isPlatformAdmin bypass on the normal session path. + if ( + !request.isPlatformAdmin && + !(await hasAppAccess(member.organizationId, member.role)) + ) { throw new ForbiddenException( "Your role doesn't have access to the app, so it can't use the MCP. " + 'Ask an organization admin for access.', diff --git a/apps/app/src/app/(app)/[orgId]/settings/user/hooks/useMcpOrganization.ts b/apps/app/src/app/(app)/[orgId]/settings/user/hooks/useMcpOrganization.ts index 2a356a1d7d..7842224fb0 100644 --- a/apps/app/src/app/(app)/[orgId]/settings/user/hooks/useMcpOrganization.ts +++ b/apps/app/src/app/(app)/[orgId]/settings/user/hooks/useMcpOrganization.ts @@ -47,10 +47,9 @@ export function useMcpOrganization(options?: UseMcpOrganizationOptions) { if (response.error) throw new Error(response.error); await mutate(); } catch (err) { - // Roll back to the last known good value. - if (previous) { - await mutate(previous, false); - } + // Revalidate from the server rather than restoring a possibly-stale + // snapshot (another tab/request may have changed the selection). + await mutate(); throw err; } };