Skip to content

Commit e7f4ccf

Browse files
committed
Fix three MCP tooling bugs: bearer token fallback, x-ms-agentid header, and legacy path per-audience tokens
1 parent 61b4f0d commit e7f4ccf

7 files changed

Lines changed: 194 additions & 8 deletions

File tree

packages/agents-a365-tooling-extensions-claude/src/McpToolRegistrationService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ export class McpToolRegistrationService {
6666
for (const server of servers) {
6767
// server.headers contains the per-audience Authorization token set by listToolServers.
6868
// Merge with non-auth headers (channelId, user-agent); server.headers auth takes precedence.
69-
const baseHeaders = Utility.GetToolRequestHeaders(undefined, turnContext, options);
69+
const baseHeaders = Utility.GetToolRequestHeaders(authToken, turnContext, options);
7070
const headers = { ...baseHeaders, ...server.headers };
7171

7272
// Add each server to the config object

packages/agents-a365-tooling-extensions-langchain/src/McpToolRegistrationService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ export class McpToolRegistrationService {
8080

8181
for (const server of servers) {
8282
// Merge base headers (channel, user-agent) with per-audience Authorization from server.headers
83-
const baseHeaders: Record<string, string> = Utility.GetToolRequestHeaders(undefined, turnContext, options);
83+
const baseHeaders: Record<string, string> = Utility.GetToolRequestHeaders(authToken, turnContext, options);
8484
const headers: Record<string, string> = { ...baseHeaders, ...server.headers };
8585

8686
// Create Connection instance for LangChain agents

packages/agents-a365-tooling-extensions-openai/src/McpToolRegistrationService.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ export class McpToolRegistrationService {
6868

6969
for (const server of servers) {
7070
// Merge base headers (channel, user-agent) with per-audience Authorization from server.headers
71-
const baseHeaders: Record<string, string> = Utility.GetToolRequestHeaders(undefined, turnContext, options);
71+
const baseHeaders: Record<string, string> = Utility.GetToolRequestHeaders(authToken, turnContext, options);
7272
const headers: Record<string, string> = { ...baseHeaders, ...server.headers };
7373

7474
// Create MCPServerStreamableHttp instance for OpenAI agents

packages/agents-a365-tooling/src/McpToolServerConfigurationService.ts

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,20 @@ export class McpToolServerConfigurationService {
8989
const authToken = authTokenOrAuthorization;
9090
const toolOptions = optionsOrAuthHandlerName as ToolOptions | undefined;
9191

92-
return await (this.isDevScenario()
92+
const servers = await (this.isDevScenario()
9393
? this.getMCPServerConfigsFromManifest()
9494
: this.getMCPServerConfigsFromToolingGateway(agenticAppId, authToken, undefined, toolOptions));
95+
96+
// Apply per-audience tokens on the legacy path too, using the same structural path as the
97+
// new overload so V2 servers are never silently missing an Authorization header.
98+
// Dev: reads from BEARER_TOKEN_<NAME> / BEARER_TOKEN env vars, supports V1 and V2.
99+
// Prod: uses the shared authToken for V1 servers; throws for V2 servers (OBO requires
100+
// Authorization and authHandlerName — use the TurnContext-based overload instead).
101+
const acquire = this.isDevScenario()
102+
? this.createDevTokenAcquirer()
103+
: this.createLegacyProdTokenAcquirer(authToken);
104+
105+
return await this.attachPerAudienceTokens(servers, acquire);
95106
} else {
96107
// NEW PATH: listToolServers(turnContext, authorization, authHandlerName, authToken?, options?)
97108
const turnContext = agenticAppIdOrTurnContext;
@@ -183,6 +194,27 @@ export class McpToolServerConfigurationService {
183194
};
184195
}
185196

197+
/**
198+
* Returns a TokenAcquirer for the deprecated legacy (agenticAppId, authToken) overload in prod.
199+
* V1 servers (ATG shared scope) receive the caller-supplied authToken directly.
200+
* V2 servers (per-audience scope) throw immediately — OBO exchange requires Authorization and
201+
* authHandlerName which the legacy signature does not provide; callers must migrate to the
202+
* TurnContext-based overload.
203+
*/
204+
private createLegacyProdTokenAcquirer(authToken: string): TokenAcquirer {
205+
const sharedScope = this.configProvider.getConfiguration().mcpPlatformAuthenticationScope;
206+
return (server, scope) => {
207+
if (scope !== sharedScope) {
208+
throw new Error(
209+
`MCP server '${server.mcpServerName}' requires a per-audience token (scope: '${scope}'). ` +
210+
`Per-audience token exchange is not supported by the deprecated listToolServers(agenticAppId, authToken) overload. ` +
211+
`Migrate to listToolServers(turnContext, authorization, authHandlerName) instead.`
212+
);
213+
}
214+
return Promise.resolve(authToken);
215+
};
216+
}
217+
186218
/**
187219
* Returns a TokenAcquirer that performs OBO token exchange via AgenticAuthenticationService.
188220
* Throws if the exchange returns null so callers receive an explicit error rather than a

packages/agents-a365-tooling/src/configuration/ToolingConfiguration.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,6 @@ export class ToolingConfiguration extends RuntimeConfiguration {
9797
*/
9898
getBearerTokenForServer(mcpServerName: string): string | undefined {
9999
const key = mcpServerName.toUpperCase();
100-
return process.env[`BEARER_TOKEN_${key}`];
100+
return process.env[`BEARER_TOKEN_${key}`] ?? process.env['BEARER_TOKEN'];
101101
}
102102
}

tests/tooling/McpToolServerConfigurationService.test.ts

Lines changed: 122 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1405,9 +1405,10 @@ describe('McpToolServerConfigurationService', () => {
14051405
expect(servers[0].headers?.Authorization).toBe(`Bearer ${mockToken}`);
14061406
});
14071407

1408-
it('should pass audience and scope through from gateway into MCPServerConfig (legacy path)', async () => {
1409-
// Uses legacy path so attachPerAudienceTokens is not called — pure field passthrough check
1408+
it('should pass audience and scope through from gateway into MCPServerConfig (TurnContext path)', async () => {
1409+
// Verifies gateway response fields are preserved end-to-end using the preferred TurnContext path.
14101410
const v2Audience = 'eeeeffff-0000-1111-2222-333344445555';
1411+
const mockToken = createMockJwt('v2-fields');
14111412
// eslint-disable-next-line @typescript-eslint/no-require-imports
14121413
const axios = require('axios');
14131414
jest.spyOn(axios, 'get').mockResolvedValue({
@@ -1418,11 +1419,129 @@ describe('McpToolServerConfigurationService', () => {
14181419
scope: 'Tools.ListInvoke.All'
14191420
}]
14201421
});
1422+
getAgenticUserTokenSpy.mockResolvedValue(mockToken);
14211423

1422-
const servers = await service.listToolServers('agent-id', 'mock-auth-token');
1424+
const servers = await service.listToolServers(mockContext, mockAuthorization, 'graph', mockToken);
14231425

14241426
expect(servers[0].audience).toBe(v2Audience);
14251427
expect(servers[0].scope).toBe('Tools.ListInvoke.All');
14261428
});
14271429
});
1430+
1431+
describe('listToolServers legacy path — per-audience token attachment', () => {
1432+
const createMockJwt = () => {
1433+
const header = Buffer.from(JSON.stringify({ alg: 'HS256', typ: 'JWT' })).toString('base64url');
1434+
const payload = Buffer.from(JSON.stringify({ exp: Math.floor(Date.now() / 1000) + 3600 })).toString('base64url');
1435+
return `${header}.${payload}.mock-sig`;
1436+
};
1437+
1438+
afterEach(() => {
1439+
delete process.env.BEARER_TOKEN;
1440+
delete process.env.BEARER_TOKEN_MAILSERVER;
1441+
delete process.env.BEARER_TOKEN_V2SERVER;
1442+
});
1443+
1444+
describe('dev mode (manifest)', () => {
1445+
beforeEach(() => { process.env.NODE_ENV = 'Development'; });
1446+
1447+
it('should attach BEARER_TOKEN for a V1 server when BEARER_TOKEN is set', async () => {
1448+
process.env.BEARER_TOKEN = 'shared-dev-token';
1449+
const manifestContent = {
1450+
mcpServers: [{ mcpServerName: 'mailServer', url: 'http://localhost:3000' }]
1451+
};
1452+
jest.spyOn(fs, 'existsSync').mockReturnValue(true);
1453+
jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent));
1454+
1455+
const servers = await service.listToolServers('agent-id', 'mock-auth-token');
1456+
1457+
expect(servers[0].headers?.Authorization).toBe('Bearer shared-dev-token');
1458+
});
1459+
1460+
it('should attach BEARER_TOKEN_<NAME> for a V2 server when per-server env var is set', async () => {
1461+
process.env.BEARER_TOKEN_V2SERVER = 'v2-dev-token';
1462+
const v2Audience = 'aaaabbbb-1234-5678-abcd-111122223333';
1463+
const manifestContent = {
1464+
mcpServers: [{ mcpServerName: 'v2Server', url: 'http://localhost:3001', audience: v2Audience }]
1465+
};
1466+
jest.spyOn(fs, 'existsSync').mockReturnValue(true);
1467+
jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent));
1468+
1469+
const servers = await service.listToolServers('agent-id', 'mock-auth-token');
1470+
1471+
expect(servers[0].headers?.Authorization).toBe('Bearer v2-dev-token');
1472+
});
1473+
1474+
it('should leave headers undefined when no env var tokens are set', async () => {
1475+
const manifestContent = {
1476+
mcpServers: [{ mcpServerName: 'mailServer', url: 'http://localhost:3000' }]
1477+
};
1478+
jest.spyOn(fs, 'existsSync').mockReturnValue(true);
1479+
jest.spyOn(fs, 'readFileSync').mockReturnValue(JSON.stringify(manifestContent));
1480+
1481+
const servers = await service.listToolServers('agent-id', 'mock-auth-token');
1482+
1483+
expect(servers[0].headers?.Authorization).toBeUndefined();
1484+
});
1485+
});
1486+
1487+
describe('prod mode (gateway)', () => {
1488+
beforeEach(() => {
1489+
process.env.NODE_ENV = 'production';
1490+
jest.spyOn(Utility, 'ValidateAuthToken').mockImplementation(() => {});
1491+
});
1492+
1493+
it('should attach the shared authToken for a V1 server (no audience)', async () => {
1494+
const mockToken = createMockJwt();
1495+
// eslint-disable-next-line @typescript-eslint/no-require-imports
1496+
const axios = require('axios');
1497+
jest.spyOn(axios, 'get').mockResolvedValue({
1498+
data: [{ mcpServerName: 'mailServer', url: 'http://prod.example.com' }]
1499+
});
1500+
1501+
const servers = await service.listToolServers('agent-id', mockToken);
1502+
1503+
expect(servers[0].headers?.Authorization).toBe(`Bearer ${mockToken}`);
1504+
});
1505+
1506+
it('should throw for a V2 server (non-ATG audience) with a message directing to the TurnContext overload', async () => {
1507+
const v2Audience = 'aaaabbbb-1234-5678-abcd-111122223333';
1508+
// eslint-disable-next-line @typescript-eslint/no-require-imports
1509+
const axios = require('axios');
1510+
jest.spyOn(axios, 'get').mockResolvedValue({
1511+
data: [{ mcpServerName: 'v2Server', url: 'http://v2.example.com', audience: v2Audience }]
1512+
});
1513+
1514+
await expect(
1515+
service.listToolServers('agent-id', 'mock-auth-token')
1516+
).rejects.toThrow("MCP server 'v2Server' requires a per-audience token");
1517+
});
1518+
1519+
it('should throw with a message that names the migration overload', async () => {
1520+
const v2Audience = 'ccccdddd-5678-9012-efab-444455556666';
1521+
// eslint-disable-next-line @typescript-eslint/no-require-imports
1522+
const axios = require('axios');
1523+
jest.spyOn(axios, 'get').mockResolvedValue({
1524+
data: [{ mcpServerName: 'v2Tools', url: 'http://v2tools.example.com', audience: v2Audience }]
1525+
});
1526+
1527+
await expect(
1528+
service.listToolServers('agent-id', 'mock-auth-token')
1529+
).rejects.toThrow('listToolServers(turnContext, authorization, authHandlerName)');
1530+
});
1531+
1532+
it('should NOT throw for a V1 server whose audience explicitly equals the shared ATG AppId', async () => {
1533+
const atgAppId = 'ea9ffc3e-8a23-4a7d-836d-234d7c7565c1';
1534+
const mockToken = createMockJwt();
1535+
// eslint-disable-next-line @typescript-eslint/no-require-imports
1536+
const axios = require('axios');
1537+
jest.spyOn(axios, 'get').mockResolvedValue({
1538+
data: [{ mcpServerName: 'legacyServer', url: 'http://legacy.example.com', audience: atgAppId }]
1539+
});
1540+
1541+
const servers = await service.listToolServers('agent-id', mockToken);
1542+
1543+
expect(servers[0].headers?.Authorization).toBe(`Bearer ${mockToken}`);
1544+
});
1545+
});
1546+
});
14281547
});

tests/tooling/configuration/ToolingConfiguration.test.ts

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -225,6 +225,41 @@ describe('ToolingConfiguration', () => {
225225
});
226226
});
227227

228+
describe('getBearerTokenForServer', () => {
229+
it('should return per-server token when BEARER_TOKEN_<NAME> is set', () => {
230+
process.env.BEARER_TOKEN_MYSERVER = 'per-server-token';
231+
const config = new ToolingConfiguration({});
232+
expect(config.getBearerTokenForServer('myserver')).toBe('per-server-token');
233+
});
234+
235+
it('should fall back to BEARER_TOKEN when per-server var is not set', () => {
236+
delete process.env.BEARER_TOKEN_MYSERVER;
237+
process.env.BEARER_TOKEN = 'shared-token';
238+
const config = new ToolingConfiguration({});
239+
expect(config.getBearerTokenForServer('myserver')).toBe('shared-token');
240+
});
241+
242+
it('should return undefined when neither per-server nor shared token is set', () => {
243+
delete process.env.BEARER_TOKEN_MYSERVER;
244+
delete process.env.BEARER_TOKEN;
245+
const config = new ToolingConfiguration({});
246+
expect(config.getBearerTokenForServer('myserver')).toBeUndefined();
247+
});
248+
249+
it('should prefer per-server token over shared BEARER_TOKEN when both are set', () => {
250+
process.env.BEARER_TOKEN_MYSERVER = 'per-server-token';
251+
process.env.BEARER_TOKEN = 'shared-token';
252+
const config = new ToolingConfiguration({});
253+
expect(config.getBearerTokenForServer('myserver')).toBe('per-server-token');
254+
});
255+
256+
it('should uppercase the server name when looking up the env var', () => {
257+
process.env.BEARER_TOKEN_MY_SERVER = 'upper-token';
258+
const config = new ToolingConfiguration({});
259+
expect(config.getBearerTokenForServer('my_server')).toBe('upper-token');
260+
});
261+
});
262+
228263
describe('combined overrides', () => {
229264
it('should allow overriding both runtime and tooling settings', () => {
230265
const config = new ToolingConfiguration({

0 commit comments

Comments
 (0)