Security hardening: token validation and service URL improvements

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);
+    });
+  });
+});

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 (

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,
   });
 }
 

packages/apps/src/app.ts

@@ -145,6 +145,14 @@ export type AppOptions<TPlugin extends IPlugin> = {
    */
   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<TPlugin extends IPlugin = IPlugin> {
         onError: (err) => this.onError({ error: err })
       }), {
         skipAuth: this.options.skipAuth,
+        additionalAllowedDomains: this.options.additionalAllowedDomains,
         logger: this.log,
         messagingEndpoint: this.options.messagingEndpoint ?? '/api/messages',
       });

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 () => {

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<AuthResult> {
     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,
         },
       };

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,

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}`);
       }
     }