diff --git a/external/mcp/src/plugin.spec.ts b/external/mcp/src/plugin.spec.ts new file mode 100644 index 000000000..b1470c6ba --- /dev/null +++ b/external/mcp/src/plugin.spec.ts @@ -0,0 +1,79 @@ +import { McpPlugin } from './plugin'; + +// Test subclass to access protected method +class TestMcpPlugin extends McpPlugin { + public testIsCallToolResult(value: any): boolean { + return this.isCallToolResult(value); + } +} + +describe('McpPlugin', () => { + describe('isCallToolResult', () => { + let plugin: TestMcpPlugin; + + beforeEach(() => { + plugin = new TestMcpPlugin(); + }); + + it('should return true for valid CallToolResult with text content', () => { + const result = { + content: [{ type: 'text', text: 'hello' }], + }; + expect(plugin.testIsCallToolResult(result)).toBe(true); + }); + + it('should return true for valid CallToolResult with image content', () => { + const result = { + content: [{ type: 'image', data: 'base64...', mimeType: 'image/png' }], + }; + expect(plugin.testIsCallToolResult(result)).toBe(true); + }); + + it('should return true for valid CallToolResult with resource content', () => { + const result = { + content: [{ type: 'resource', resource: {} }], + }; + expect(plugin.testIsCallToolResult(result)).toBe(true); + }); + + it('should return true for mixed content types', () => { + const result = { + content: [ + { type: 'text', text: 'hello' }, + { type: 'image', data: 'base64...' }, + ], + }; + expect(plugin.testIsCallToolResult(result)).toBe(true); + }); + + it('should return false for null', () => { + expect(plugin.testIsCallToolResult(null)).toBe(false); + }); + + it('should return false for undefined', () => { + expect(plugin.testIsCallToolResult(undefined)).toBe(false); + }); + + it('should return false for object without content', () => { + expect(plugin.testIsCallToolResult({ text: 'hello' })).toBe(false); + }); + + it('should return false for plain string', () => { + expect(plugin.testIsCallToolResult('hello')).toBe(false); + }); + + it('should return false for content with unknown type', () => { + const result = { + content: [{ type: 'unknown', data: 'foo' }], + }; + expect(plugin.testIsCallToolResult(result)).toBe(false); + }); + + it('should return false for non-array content', () => { + const result = { + content: 'not an array', + }; + expect(plugin.testIsCallToolResult(result)).toBe(false); + }); + }); +}); diff --git a/external/mcp/src/plugin.ts b/external/mcp/src/plugin.ts index e29b8e977..8721a9585 100644 --- a/external/mcp/src/plugin.ts +++ b/external/mcp/src/plugin.ts @@ -287,7 +287,7 @@ export class McpPlugin implements IPlugin { } protected isCallToolResult(value: any): value is CallToolResult { - if (!!value || !('content' in value)) return false; + if (!value || typeof value !== 'object' || !('content' in value)) return false; const { content } = value; return ( diff --git a/packages/api/src/auth/cloud-environment.ts b/packages/api/src/auth/cloud-environment.ts index e0578a21f..5a4e71431 100644 --- a/packages/api/src/auth/cloud-environment.ts +++ b/packages/api/src/auth/cloud-environment.ts @@ -18,6 +18,8 @@ export type CloudEnvironment = { readonly tokenIssuer: string; /** The Microsoft Graph token scope (e.g. "https://graph.microsoft.com/.default") */ readonly graphScope: string; + /** Allowed service URL hostnames for this cloud environment */ + readonly allowedServiceUrls: readonly string[]; }; /** Microsoft public (commercial) cloud. */ @@ -29,6 +31,7 @@ export const PUBLIC: CloudEnvironment = Object.freeze({ openIdMetadataUrl: 'https://login.botframework.com/v1/.well-known/openidconfiguration', tokenIssuer: 'https://api.botframework.com', graphScope: 'https://graph.microsoft.com/.default', + allowedServiceUrls: ['smba.trafficmanager.net', 'smba.onyx.prod.teams.trafficmanager.net', 'smba.infra.gcc.teams.microsoft.com'], }); /** US Government Community Cloud High (GCCH). */ @@ -40,6 +43,7 @@ export const US_GOV: CloudEnvironment = Object.freeze({ openIdMetadataUrl: 'https://login.botframework.azure.us/v1/.well-known/openidconfiguration', tokenIssuer: 'https://api.botframework.us', graphScope: 'https://graph.microsoft.us/.default', + allowedServiceUrls: ['smba.infra.gov.teams.microsoft.us'], }); /** US Government Department of Defense (DoD). */ @@ -51,6 +55,7 @@ export const US_GOV_DOD: CloudEnvironment = Object.freeze({ openIdMetadataUrl: 'https://login.botframework.azure.us/v1/.well-known/openidconfiguration', tokenIssuer: 'https://api.botframework.us', graphScope: 'https://dod-graph.microsoft.us/.default', + allowedServiceUrls: ['smba.infra.dod.teams.microsoft.us'], }); /** China cloud (21Vianet). */ @@ -62,6 +67,7 @@ export const CHINA: CloudEnvironment = Object.freeze({ openIdMetadataUrl: 'https://login.botframework.azure.cn/v1/.well-known/openidconfiguration', tokenIssuer: 'https://api.botframework.azure.cn', graphScope: 'https://microsoftgraph.chinacloudapi.cn/.default', + allowedServiceUrls: ['frontend.botapi.msg.infra.teams.microsoftonline.cn'], }); /** @@ -80,6 +86,7 @@ export function withOverrides( openIdMetadataUrl: overrides.openIdMetadataUrl ?? base.openIdMetadataUrl, tokenIssuer: overrides.tokenIssuer ?? base.tokenIssuer, graphScope: overrides.graphScope ?? base.graphScope, + allowedServiceUrls: overrides.allowedServiceUrls ?? base.allowedServiceUrls, }); } diff --git a/packages/apps/src/app.ts b/packages/apps/src/app.ts index 1dd1d43d2..eeb5cb96d 100644 --- a/packages/apps/src/app.ts +++ b/packages/apps/src/app.ts @@ -145,6 +145,14 @@ export type AppOptions = { */ readonly skipAuth?: boolean; + /** + * Additional allowed service URL hostnames beyond the built-in defaults. + * Use this if your bot receives activities from non-standard channels + * with service URLs outside the cloud environment's allowed hostnames. + * @example ['api.my-custom-channel.com'] + */ + readonly additionalAllowedDomains?: string[]; + /** * URL path for the Teams messaging endpoint * @default '/api/messages' @@ -373,6 +381,7 @@ export class App { onError: (err) => this.onError({ error: err }) }), { skipAuth: this.options.skipAuth, + additionalAllowedDomains: this.options.additionalAllowedDomains, logger: this.log, messagingEndpoint: this.options.messagingEndpoint ?? '/api/messages', }); diff --git a/packages/apps/src/http/http-server.spec.ts b/packages/apps/src/http/http-server.spec.ts index e1b6203ca..ba10285be 100644 --- a/packages/apps/src/http/http-server.spec.ts +++ b/packages/apps/src/http/http-server.spec.ts @@ -90,13 +90,13 @@ describe('HttpServer', () => { server.onRequest = jest.fn().mockResolvedValue({ status: 200 }); await adapter.simulateRequest('/api/messages', { - serviceUrl: 'https://test.botframework.com', + serviceUrl: 'https://smba.trafficmanager.net/amer/', }); const event = (server.onRequest as jest.Mock).mock.calls[0][0]; expect(event.token.appId).toBe(''); expect(event.token.from).toBe('azure'); - expect(event.token.serviceUrl).toBe('https://test.botframework.com'); + expect(event.token.serviceUrl).toBe('https://smba.trafficmanager.net/amer/'); }); it('should return 500 when onRequest is not set', async () => { diff --git a/packages/apps/src/http/http-server.ts b/packages/apps/src/http/http-server.ts index 909c5d90d..4dec0a911 100644 --- a/packages/apps/src/http/http-server.ts +++ b/packages/apps/src/http/http-server.ts @@ -2,13 +2,14 @@ import { CloudEnvironment, Credentials, InvokeResponse, - IToken + IToken, + PUBLIC } from '@microsoft/teams.api'; import { ConsoleLogger, ILogger } from '@microsoft/teams.common'; import { IActivityEvent, ICoreActivity } from '../events'; -import { ServiceTokenValidator } from '../middleware/auth/service-token-validator'; +import { ServiceTokenValidator, isAllowedServiceUrl } from '../middleware/auth/service-token-validator'; import { HttpMethod, IHttpServerAdapter, IHttpServerRequest, IHttpServerResponse, HttpRouteHandler } from './adapter'; @@ -18,6 +19,7 @@ type AuthResult = export type HttpServerOptions = { readonly skipAuth?: boolean; + readonly additionalAllowedDomains?: string[]; readonly logger?: ILogger; /** * URL path for the Teams messaging endpoint @@ -47,6 +49,8 @@ export class HttpServer implements IHttpServer { protected logger: ILogger; protected credentials?: Credentials; protected skipAuth: boolean; + protected additionalAllowedDomains?: string[]; + protected cloud?: CloudEnvironment; protected initialized: boolean = false; protected serviceTokenValidator?: ServiceTokenValidator; @@ -71,6 +75,7 @@ export class HttpServer implements IHttpServer { constructor(adapter: IHttpServerAdapter, options: HttpServerOptions) { this._adapter = adapter; this.skipAuth = options.skipAuth ?? false; + this.additionalAllowedDomains = options.additionalAllowedDomains; this.logger = options.logger ?? new ConsoleLogger('HttpServer'); this._messagingEndpoint = options.messagingEndpoint; } @@ -90,6 +95,11 @@ export class HttpServer implements IHttpServer { } this.credentials = deps.credentials; + this.cloud = deps.cloud; + + if (this.additionalAllowedDomains?.includes('*')) { + this.logger.warn('Service URL validation is disabled via wildcard in additionalAllowedDomains'); + } // Initialize service token validator if credentials provided and auth not skipped if (this.credentials && !this.skipAuth) { @@ -98,6 +108,7 @@ export class HttpServer implements IHttpServer { this.credentials.tenantId, undefined, // serviceUrl will be validated from activity body this.logger, + this.additionalAllowedDomains, deps.cloud ); } @@ -188,13 +199,21 @@ export class HttpServer implements IHttpServer { body: ICoreActivity ): Promise { if (this.skipAuth || !this.credentials) { + const serviceUrl = body.serviceUrl || ''; + + // Validate serviceUrl even when auth is skipped + if (serviceUrl && !isAllowedServiceUrl(serviceUrl, this.cloud ?? PUBLIC, this.additionalAllowedDomains)) { + this.logger.warn(`Rejected service URL: ${serviceUrl}`); + return { success: false, error: 'Service URL is not from an allowed domain' }; + } + return { success: true, token: { appId: '', from: 'azure', fromId: '', - serviceUrl: body.serviceUrl || '', + serviceUrl, isExpired: () => false, }, }; diff --git a/packages/apps/src/middleware/auth/jwt-validator.spec.ts b/packages/apps/src/middleware/auth/jwt-validator.spec.ts index 1e587af28..bcc219e1b 100644 --- a/packages/apps/src/middleware/auth/jwt-validator.spec.ts +++ b/packages/apps/src/middleware/auth/jwt-validator.spec.ts @@ -319,6 +319,25 @@ describe('JwtValidator', () => { expect(result).toEqual(expect.objectContaining(mockTokenPayload)); }); + it('should throw when allowedTenantIds is configured but tenantId is missing', async () => { + const validator = new JwtValidator({ + clientId: mockClientId, + // tenantId intentionally omitted + jwksUriOptions: { type: 'tenantId' }, + validateIssuer: { allowedTenantIds: [mockTenantId] } + }, mockLogger); + + const result = await validator.validateAccessToken(validToken); + + expect(result).toBeNull(); + expect(mockLogger.error).toHaveBeenCalledWith( + 'Custom validation failed:', + expect.objectContaining({ + message: 'Tenant ID is required when allowedTenantIds is configured' + }) + ); + }); + it('should reject token with missing issuer', async () => { const validator = new JwtValidator({ clientId: mockClientId, @@ -370,6 +389,51 @@ describe('JwtValidator', () => { expect(result).toEqual(expect.objectContaining(mockTokenPayload)); }); + it('should reject token with substring-matching scope', async () => { + const validator = new JwtValidator({ + clientId: mockClientId, + tenantId: mockTenantId, + jwksUriOptions: { type: 'tenantId' }, + validateScope: { requiredScope: 'User.Read' } + }, mockLogger); + + const substringToken = createTestToken({ + ...mockTokenPayload, + scp: 'User.ReadBasic.All' + }); + + const result = await validator.validateAccessToken(substringToken); + + expect(result).toBeNull(); + expect(mockLogger.error).toHaveBeenCalledWith( + 'Custom validation failed:', + expect.objectContaining({ + message: 'Token missing required scope: User.Read' + }) + ); + }); + + it('should accept exact scope among multiple scopes', async () => { + const validator = new JwtValidator({ + clientId: mockClientId, + tenantId: mockTenantId, + jwksUriOptions: { type: 'tenantId' }, + validateScope: { requiredScope: 'User.Read' } + }); + + const multiScopeToken = createTestToken({ + ...mockTokenPayload, + scp: 'Mail.Read User.Read Files.ReadWrite' + }); + + const result = await validator.validateAccessToken(multiScopeToken); + + expect(result).toEqual(expect.objectContaining({ + ...mockTokenPayload, + scp: 'Mail.Read User.Read Files.ReadWrite' + })); + }); + it('should reject token with missing scope', async () => { const validator = new JwtValidator({ clientId: mockClientId, diff --git a/packages/apps/src/middleware/auth/jwt-validator.ts b/packages/apps/src/middleware/auth/jwt-validator.ts index 1f5750cbd..c305bfaba 100644 --- a/packages/apps/src/middleware/auth/jwt-validator.ts +++ b/packages/apps/src/middleware/auth/jwt-validator.ts @@ -204,7 +204,7 @@ export class JwtValidator { } if (!this.options.tenantId) { - return; + throw new Error('Tenant ID is required when allowedTenantIds is configured'); } const isMultiTenant = ['common', 'organizations', 'consumers'].includes(this.options.tenantId); @@ -234,8 +234,8 @@ export class JwtValidator { private validateScope(scp: string | undefined, overrideValidateScope?: { requiredScope: string }): void { const validateScope = overrideValidateScope || this.options.validateScope; if (validateScope) { - const scopes = scp ?? ''; - if (!scopes.includes(validateScope.requiredScope)) { + const scopeSet = new Set((scp ?? '').split(' ')); + if (!scopeSet.has(validateScope.requiredScope)) { throw new Error(`Token missing required scope: ${validateScope.requiredScope}`); } } diff --git a/packages/apps/src/middleware/auth/service-token-validator.spec.ts b/packages/apps/src/middleware/auth/service-token-validator.spec.ts index ed4430dca..78e0fe546 100644 --- a/packages/apps/src/middleware/auth/service-token-validator.spec.ts +++ b/packages/apps/src/middleware/auth/service-token-validator.spec.ts @@ -148,11 +148,202 @@ describe('ServiceTokenValidator', () => { expect(result.appId).toBe(mockClientId); }); - it('should prefer payload.serviceurl over body.serviceUrl', async () => { + it('should reject serviceUrl from non-allowed domain', async () => { const validator = new ServiceTokenValidator(mockClientId, mockTenantId); - const payloadServiceUrl = 'https://payload.example.com'; - const bodyServiceUrl = 'https://body.example.com'; + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'https://evil.com/api' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'https://evil.com/api' }; + + await expect(validator.check(authHeader, body)).rejects.toThrow( + 'Service URL is not from an allowed domain' + ); + }); + + it('should accept serviceUrl from cloud preset', async () => { + const validator = new ServiceTokenValidator(mockClientId, mockTenantId); + + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'https://smba.trafficmanager.net/amer/' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'https://smba.trafficmanager.net/amer/' }; + + const result = await validator.check(authHeader, body); + expect(result.serviceUrl).toBe('https://smba.trafficmanager.net/amer/'); + }); + + it('should accept localhost serviceUrl', async () => { + const validator = new ServiceTokenValidator(mockClientId, mockTenantId); + + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'http://localhost:3978' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'http://localhost:3978' }; + + const result = await validator.check(authHeader, body); + expect(result.serviceUrl).toBe('http://localhost:3978'); + }); + + it('should reject botframework.com by default (non-Teams channel)', async () => { + const validator = new ServiceTokenValidator(mockClientId, mockTenantId); + + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'https://webchat.botframework.com' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'https://webchat.botframework.com' }; + + await expect(validator.check(authHeader, body)).rejects.toThrow( + 'is not from an allowed domain' + ); + }); + + it('should reject domain that contains allowed suffix as substring', async () => { + const validator = new ServiceTokenValidator(mockClientId, mockTenantId); + + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'https://botframework.com.evil.com' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'https://botframework.com.evil.com' }; + + await expect(validator.check(authHeader, body)).rejects.toThrow( + 'is not from an allowed domain' + ); + }); + + it('should accept serviceUrl with additionalAllowedDomains', async () => { + const validator = new ServiceTokenValidator( + mockClientId, mockTenantId, undefined, undefined, ['api.custom-channel.com'] + ); + + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'https://api.custom-channel.com' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'https://api.custom-channel.com' }; + + const result = await validator.check(authHeader, body); + expect(result.serviceUrl).toBe('https://api.custom-channel.com'); + }); + + it('should reject attacker-controlled trafficmanager subdomain', async () => { + const validator = new ServiceTokenValidator(mockClientId, mockTenantId); + + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'https://attacker.trafficmanager.net' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'https://attacker.trafficmanager.net' }; + + await expect(validator.check(authHeader, body)).rejects.toThrow( + 'is not from an allowed domain' + ); + }); + + it('should accept smba.onyx.prod.teams.trafficmanager.net', async () => { + const validator = new ServiceTokenValidator(mockClientId, mockTenantId); + + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'https://smba.onyx.prod.teams.trafficmanager.net' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'https://smba.onyx.prod.teams.trafficmanager.net' }; + + const result = await validator.check(authHeader, body); + expect(result.serviceUrl).toBe('https://smba.onyx.prod.teams.trafficmanager.net'); + }); + + it('should accept any domain when additionalAllowedDomains includes wildcard', async () => { + const validator = new ServiceTokenValidator( + mockClientId, mockTenantId, undefined, undefined, ['*'] + ); + + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'https://any-domain.com/api' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'https://any-domain.com/api' }; + + const result = await validator.check(authHeader, body); + expect(result.serviceUrl).toBe('https://any-domain.com/api'); + }); + + it('should accept US Government serviceUrl with US_GOV cloud', async () => { + const { US_GOV } = await import('@microsoft/teams.api'); + const validator = new ServiceTokenValidator( + mockClientId, mockTenantId, undefined, undefined, undefined, US_GOV + ); + + const mockPayload = { + appid: mockClientId, + sub: 'bot-id', + serviceurl: 'https://smba.infra.gov.teams.microsoft.us/' + }; + + mockValidateAccessToken.mockResolvedValue(mockPayload); + + const authHeader = 'Bearer test-token'; + const body = { serviceUrl: 'https://smba.infra.gov.teams.microsoft.us/' }; + + const result = await validator.check(authHeader, body); + expect(result.serviceUrl).toBe('https://smba.infra.gov.teams.microsoft.us/'); + }); + + it('should prefer body.serviceUrl over payload.serviceurl', async () => { + const validator = new ServiceTokenValidator(mockClientId, mockTenantId); + + const payloadServiceUrl = 'https://smba.trafficmanager.net/emea/'; + const bodyServiceUrl = 'https://smba.trafficmanager.net/amer/'; const mockPayload = { appid: mockClientId, @@ -188,7 +379,7 @@ describe('ServiceTokenValidator', () => { }); it('should use US_GOV cloud issuer and JWKS URI', () => { - new ServiceTokenValidator(mockClientId, mockTenantId, undefined, undefined, US_GOV); + new ServiceTokenValidator(mockClientId, mockTenantId, undefined, undefined, undefined, US_GOV); expect(JwtValidator).toHaveBeenCalledWith( expect.objectContaining({ @@ -204,7 +395,7 @@ describe('ServiceTokenValidator', () => { }); it('should use CHINA cloud issuer and JWKS URI', () => { - new ServiceTokenValidator(mockClientId, mockTenantId, undefined, undefined, CHINA); + new ServiceTokenValidator(mockClientId, mockTenantId, undefined, undefined, undefined, CHINA); expect(JwtValidator).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/packages/apps/src/middleware/auth/service-token-validator.ts b/packages/apps/src/middleware/auth/service-token-validator.ts index 4c5bf634f..f287c262e 100644 --- a/packages/apps/src/middleware/auth/service-token-validator.ts +++ b/packages/apps/src/middleware/auth/service-token-validator.ts @@ -12,6 +12,40 @@ function openIdMetadataToKeysUri(openIdMetadataUrl: string): string { return openIdMetadataUrl.replace(/\/openidconfiguration$/, '/keys'); } +/** + * Validates that a service URL hostname is allowed. + * Checks against the cloud environment's allowed service URLs, + * plus any additional domains provided by the caller. + * Localhost is always allowed for local development. + */ +export function isAllowedServiceUrl( + serviceUrl: string, + cloud: CloudEnvironment, + additionalDomains?: string[] +): boolean { + try { + const url = new URL(serviceUrl); + const hostname = url.hostname.toLowerCase(); + + if (hostname === 'localhost' || hostname === '127.0.0.1') { + return true; + } + + if (url.protocol !== 'https:') { + return false; + } + + const allowed = [...cloud.allowedServiceUrls, ...(additionalDomains ?? [])].map((d) => d.toLowerCase()); + if (allowed.includes('*')) { + return true; + } + + return allowed.some((domain) => hostname === domain); + } catch { + return false; + } +} + /** * Service token validator for Bot Framework /api/messages requests * Validates Bot Framework service tokens @@ -19,15 +53,19 @@ function openIdMetadataToKeysUri(openIdMetadataUrl: string): string { export class ServiceTokenValidator { private jwtValidator: JwtValidator; private credentials?: Credentials; + private additionalAllowedDomains?: string[]; + private cloud: CloudEnvironment; constructor( appId: string, tenantId?: string, serviceUrl?: string, logger?: ILogger, + additionalAllowedDomains?: string[], cloud?: CloudEnvironment ) { const env = cloud ?? PUBLIC; + this.cloud = env; this.jwtValidator = new JwtValidator({ clientId: appId, tenantId, @@ -41,6 +79,7 @@ export class ServiceTokenValidator { }, logger); this.credentials = { clientId: appId, tenantId }; + this.additionalAllowedDomains = additionalAllowedDomains; } async check(authHeader: string, body: any): Promise { @@ -58,12 +97,19 @@ export class ServiceTokenValidator { throw new Error('Invalid token'); } + const serviceUrl = body.serviceUrl || payload.serviceurl as string || ''; + + // Validate serviceUrl against allowed domains + if (serviceUrl && !isAllowedServiceUrl(serviceUrl, this.cloud, this.additionalAllowedDomains)) { + throw new Error('Service URL is not from an allowed domain'); + } + // Convert JWT payload to IToken return { appId: payload.appid as string || this.credentials?.clientId || '', from: 'azure', fromId: payload.sub as string || '', - serviceUrl: body.serviceUrl || payload.serviceurl as string || '', + serviceUrl, isExpired: () => false, // Already validated by JWT validator }; } diff --git a/packages/apps/src/middleware/jwt-validation-middleware.ts b/packages/apps/src/middleware/jwt-validation-middleware.ts index 421bb0588..b874b2010 100644 --- a/packages/apps/src/middleware/jwt-validation-middleware.ts +++ b/packages/apps/src/middleware/jwt-validation-middleware.ts @@ -27,6 +27,7 @@ export function withJwtValidation(params: JwtValidationParams) { credentials.tenantId, undefined, logger, + undefined, // additionalAllowedDomains cloud ); } else { diff --git a/packages/dev/src/plugin.spec.ts b/packages/dev/src/plugin.spec.ts new file mode 100644 index 000000000..e2cfff7d7 --- /dev/null +++ b/packages/dev/src/plugin.spec.ts @@ -0,0 +1,41 @@ +import { DevtoolsPlugin } from './plugin'; + +describe('DevtoolsPlugin', () => { + const originalNodeEnv = process.env.NODE_ENV; + + afterEach(() => { + if (originalNodeEnv === undefined) { + delete process.env.NODE_ENV; + } else { + process.env.NODE_ENV = originalNodeEnv; + } + }); + + describe('onInit', () => { + it('should throw in production environment', () => { + process.env.NODE_ENV = 'production'; + const plugin = new DevtoolsPlugin(); + + expect(() => plugin.onInit()).toThrow( + 'Devtools plugin cannot be used in production environments' + ); + }); + + it('should not throw in development environment', () => { + process.env.NODE_ENV = 'development'; + const plugin = new DevtoolsPlugin(); + // Mock the logger since @Logger() decorator isn't run in unit tests + (plugin as any).log = { warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; + + expect(() => plugin.onInit()).not.toThrow(); + }); + + it('should not throw when NODE_ENV is not set', () => { + delete process.env.NODE_ENV; + const plugin = new DevtoolsPlugin(); + (plugin as any).log = { warn: jest.fn(), info: jest.fn(), debug: jest.fn() }; + + expect(() => plugin.onInit()).not.toThrow(); + }); + }); +}); diff --git a/packages/dev/src/plugin.ts b/packages/dev/src/plugin.ts index de84379d2..c4e62e85c 100644 --- a/packages/dev/src/plugin.ts +++ b/packages/dev/src/plugin.ts @@ -91,6 +91,13 @@ export class DevtoolsPlugin { } onInit() { + if (process.env.NODE_ENV === 'production') { + throw new Error( + 'Devtools plugin cannot be used in production environments (NODE_ENV=production). ' + + 'Remove the devtools plugin from your app configuration.' + ); + } + this.log.warn( new String() .bold(