diff --git a/packages/twenty-server/src/engine/api/mcp/guards/__tests__/mcp-auth.guard.spec.ts b/packages/twenty-server/src/engine/api/mcp/guards/__tests__/mcp-auth.guard.spec.ts index 24d54cea7d..e5a87153b5 100644 --- a/packages/twenty-server/src/engine/api/mcp/guards/__tests__/mcp-auth.guard.spec.ts +++ b/packages/twenty-server/src/engine/api/mcp/guards/__tests__/mcp-auth.guard.spec.ts @@ -1,53 +1,52 @@ import { type ExecutionContext, UnauthorizedException } from '@nestjs/common'; import { McpAuthGuard } from 'src/engine/api/mcp/guards/mcp-auth.guard'; -import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { JwtAuthGuard } from 'src/engine/guards/jwt-auth.guard'; describe('McpAuthGuard', () => { let guard: McpAuthGuard; let jwtAuthGuard: jest.Mocked; - let twentyConfigService: jest.Mocked; const mockSetHeader = jest.fn(); - const mockContext = { - switchToHttp: () => ({ - getResponse: () => ({ setHeader: mockSetHeader }), - getRequest: () => ({}), - }), - } as unknown as ExecutionContext; + const buildContext = (host = 'crm.example.com'): ExecutionContext => + ({ + switchToHttp: () => ({ + getResponse: () => ({ setHeader: mockSetHeader }), + getRequest: () => ({ + protocol: 'https', + get: (name: string) => (name === 'host' ? host : undefined), + }), + }), + }) as unknown as ExecutionContext; beforeEach(() => { jwtAuthGuard = { canActivate: jest.fn(), } as unknown as jest.Mocked; - twentyConfigService = { - get: jest.fn().mockReturnValue('https://crm.example.com'), - } as unknown as jest.Mocked; - guard = new McpAuthGuard(jwtAuthGuard, twentyConfigService); + guard = new McpAuthGuard(jwtAuthGuard); mockSetHeader.mockClear(); }); it('should return true when JwtAuthGuard passes', async () => { jwtAuthGuard.canActivate.mockResolvedValue(true); - const result = await guard.canActivate(mockContext); + const result = await guard.canActivate(buildContext()); expect(result).toBe(true); expect(mockSetHeader).not.toHaveBeenCalled(); }); - it('should set WWW-Authenticate header and throw when auth fails', async () => { + it('should set WWW-Authenticate using the request host and throw when auth fails', async () => { jwtAuthGuard.canActivate.mockResolvedValue(false); - await expect(guard.canActivate(mockContext)).rejects.toThrow( - UnauthorizedException, - ); + await expect( + guard.canActivate(buildContext('acme.twenty.com')), + ).rejects.toThrow(UnauthorizedException); expect(mockSetHeader).toHaveBeenCalledWith( 'WWW-Authenticate', - 'Bearer resource_metadata="https://crm.example.com/.well-known/oauth-protected-resource"', + 'Bearer resource_metadata="https://acme.twenty.com/.well-known/oauth-protected-resource"', ); }); }); diff --git a/packages/twenty-server/src/engine/api/mcp/guards/mcp-auth.guard.ts b/packages/twenty-server/src/engine/api/mcp/guards/mcp-auth.guard.ts index 3682a67802..5db8de024e 100644 --- a/packages/twenty-server/src/engine/api/mcp/guards/mcp-auth.guard.ts +++ b/packages/twenty-server/src/engine/api/mcp/guards/mcp-auth.guard.ts @@ -5,26 +5,25 @@ import { UnauthorizedException, } from '@nestjs/common'; -import { type Response } from 'express'; +import { type Request, type Response } from 'express'; -import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { JwtAuthGuard } from 'src/engine/guards/jwt-auth.guard'; // RFC 9728: When the MCP endpoint returns 401, include a WWW-Authenticate -// header pointing to the Protected Resource Metadata URL. +// header pointing to the Protected Resource Metadata URL on the same host +// the client connected to — otherwise discovery fails for any host other +// than SERVER_URL (app.twenty.com, workspace subdomains, custom domains). @Injectable() export class McpAuthGuard implements CanActivate { - constructor( - private readonly jwtAuthGuard: JwtAuthGuard, - private readonly twentyConfigService: TwentyConfigService, - ) {} + constructor(private readonly jwtAuthGuard: JwtAuthGuard) {} async canActivate(context: ExecutionContext): Promise { const isAuthenticated = await this.jwtAuthGuard.canActivate(context); if (!isAuthenticated) { - const serverUrl = this.twentyConfigService.get('SERVER_URL'); - const resourceMetadataUrl = `${serverUrl}/.well-known/oauth-protected-resource`; + const request = context.switchToHttp().getRequest(); + const baseUrl = `${request.protocol}://${request.get('host')}`; + const resourceMetadataUrl = `${baseUrl}/.well-known/oauth-protected-resource`; // Set the header on the response before throwing, because exception // filters may not preserve custom headers from the exception payload. diff --git a/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/oauth-discovery.controller.ts b/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/oauth-discovery.controller.ts index 91d59faeca..8d760954b8 100644 --- a/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/oauth-discovery.controller.ts +++ b/packages/twenty-server/src/engine/core-modules/application/application-oauth/controllers/oauth-discovery.controller.ts @@ -1,11 +1,14 @@ -import { Controller, Get, UseGuards } from '@nestjs/common'; +import { Controller, Get, Req, UseGuards } from '@nestjs/common'; + +import { type Request } from 'express'; import { ALL_OAUTH_SCOPES } from 'src/engine/core-modules/application/application-oauth/constants/oauth-scopes'; import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service'; +import { DomainServerConfigService } from 'src/engine/core-modules/domain/domain-server-config/services/domain-server-config.service'; import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service'; import { NoPermissionGuard } from 'src/engine/guards/no-permission.guard'; import { PublicEndpointGuard } from 'src/engine/guards/public-endpoint.guard'; -import { DomainServerConfigService } from 'src/engine/core-modules/domain/domain-server-config/services/domain-server-config.service'; +import { cleanServerUrl } from 'src/utils/clean-server-url'; import { TWENTY_CLI_APPLICATION_REGISTRATION } from 'src/engine/workspace-manager/twenty-standard-application/constants/twenty-cli-application-registration.constant'; @Controller('.well-known') @@ -18,10 +21,15 @@ export class OAuthDiscoveryController { @Get('oauth-authorization-server') @UseGuards(PublicEndpointGuard, NoPermissionGuard) - async getAuthorizationServerMetadata() { - const serverUrl = this.twentyConfigService.get('SERVER_URL'); - - const frontUrl = this.domainServerConfigService.getBaseUrl().toString(); + async getAuthorizationServerMetadata(@Req() request: Request) { + const issuer = this.getRequestBaseUrl(request); + // /authorize is served by the frontend; SERVER_URL (API-only) has no such + // route, so we route the client to the default frontend base URL in that + // case. All other hosts (app.twenty.com, workspace subdomains, custom + // domains) serve both frontend and API. + const authorizeBase = this.isApiHost(request) + ? cleanServerUrl(this.domainServerConfigService.getBaseUrl().toString()) + : issuer; const cliRegistration = await this.applicationRegistrationService.findOneByUniversalIdentifier( @@ -29,12 +37,12 @@ export class OAuthDiscoveryController { ); return { - issuer: serverUrl, - authorization_endpoint: `${frontUrl.replace(/\/$/, '')}/authorize`, - token_endpoint: `${serverUrl}/oauth/token`, - registration_endpoint: `${serverUrl}/oauth/register`, - revocation_endpoint: `${serverUrl}/oauth/revoke`, - introspection_endpoint: `${serverUrl}/oauth/introspect`, + issuer, + authorization_endpoint: `${authorizeBase}/authorize`, + token_endpoint: `${issuer}/oauth/token`, + registration_endpoint: `${issuer}/oauth/register`, + revocation_endpoint: `${issuer}/oauth/revoke`, + introspection_endpoint: `${issuer}/oauth/introspect`, scopes_supported: ALL_OAUTH_SCOPES, response_types_supported: ['code'], grant_types_supported: [ @@ -52,17 +60,30 @@ export class OAuthDiscoveryController { }; } - // RFC 9728: OAuth 2.0 Protected Resource Metadata + // RFC 9728: `resource` is echoed back as the host the client connected to + // so that MCP clients can validate the resource indicator they were trying + // to reach. Without this, pasting any URL other than SERVER_URL/mcp breaks + // discovery. @Get('oauth-protected-resource') @UseGuards(PublicEndpointGuard, NoPermissionGuard) - getProtectedResourceMetadata() { - const serverUrl = this.twentyConfigService.get('SERVER_URL'); + getProtectedResourceMetadata(@Req() request: Request) { + const base = this.getRequestBaseUrl(request); return { - resource: `${serverUrl}/mcp`, - authorization_servers: [serverUrl], + resource: `${base}/mcp`, + authorization_servers: [base], scopes_supported: ALL_OAUTH_SCOPES, bearer_methods_supported: ['header'], }; } + + private getRequestBaseUrl(request: Request): string { + return `${request.protocol}://${request.get('host')}`; + } + + private isApiHost(request: Request): boolean { + const serverUrl = this.twentyConfigService.get('SERVER_URL'); + + return request.get('host') === new URL(serverUrl).host; + } }