diff --git a/src/Backend/AutopilotMonitor.Functions/Services/McpUserService.cs b/src/Backend/AutopilotMonitor.Functions/Services/McpUserService.cs index ae19dc1c..8d127f18 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/McpUserService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/McpUserService.cs @@ -83,7 +83,10 @@ public class McpUserService return isMcpUser ? McpAccessCheckResult.Allowed(upn, "McpUser", false) - : McpAccessCheckResult.Denied("Not authorized for MCP access"); + // Surfaced to the end user by the MCP server's access guard. Keep it + // self-explanatory so a denied colleague understands they simply need + // to be whitelisted, rather than reading it as an auth failure. + : McpAccessCheckResult.Denied("User not enabled for MCP usage (account is not on the MCP whitelist)"); } } diff --git a/src/McpServer/autopilot-monitor-mcp/src/__tests__/access-guard.test.ts b/src/McpServer/autopilot-monitor-mcp/src/__tests__/access-guard.test.ts index b7c5ab9a..efd621dd 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/__tests__/access-guard.test.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/__tests__/access-guard.test.ts @@ -190,6 +190,28 @@ describe('accessGuard — 403 (authorization + fail-closed)', () => { expect((out.body as { reason: string }).reason).toBe('not on the MCP whitelist'); }); + it('gives a genuine whitelist denial an actionable message naming the user', async () => { + const upn = uniqueUpn(); + stubBackend({ body: { allowed: false, reason: 'User not enabled for MCP usage' } }); + const out = await runGuard(mockReq(`Bearer ${validToken(upn)}`)); + expect(out.status).toBe(403); + const body = out.body as { error: string; message: string }; + expect(body.error).toBe('User not enabled for MCP usage'); + // The message must name the account and point at the fix (get whitelisted). + expect(body.message).toContain(upn); + expect(body.message).toMatch(/whitelist/i); + }); + + it('does NOT label an infrastructure failure as a whitelist problem', async () => { + // A cold/unreachable backend must not tell the user they are "not enabled". + stubBackend({ reject: true }); + const out = await runGuard(mockReq(`Bearer ${validToken(uniqueUpn())}`)); + expect(out.status).toBe(403); + const body = out.body as { error: string; message?: string }; + expect(body.error).not.toBe('User not enabled for MCP usage'); + expect(body.message).toBeUndefined(); + }); + it('fail-closed: denies when the backend fetch throws (tests-2)', async () => { // scale-to-zero ⇒ a cold/unreachable backend is routine. checkAccess must // resolve allowed:false, never allowed:true. diff --git a/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts b/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts index ff39257b..ca27be7b 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts @@ -50,6 +50,12 @@ interface AccessCheckResult { reason: string; isGlobalAdmin: boolean; isGlobalReader: boolean; + // Distinguishes a genuine authorization denial (the backend reached a verdict + // and said "no" — e.g. user not on the MCP whitelist) from an infrastructure + // failure (backend unreachable / malformed response). Both fail closed, but + // only the former should be surfaced to the user as "you are not enabled — + // ask to be whitelisted"; an infra error is not the user's fault. + infraError: boolean; } export function buildCacheKey(upn: string, token: string): string { @@ -66,6 +72,9 @@ async function checkAccess(upn: string, token: string): Promise { if (!result.allowed) { - res.status(403).json({ error: 'MCP access denied', reason: result.reason }); + if (result.infraError) { + // Backend could not reach a verdict (unreachable / malformed). Fail + // closed, but do NOT tell the user they are "not whitelisted" — it is + // not their fault and would send them chasing the wrong fix. + res.status(403).json({ error: 'MCP access check unavailable', reason: result.reason }); + return; + } + // Genuine authorization denial — most commonly the user's account is not + // on the MCP whitelist. Spell that out and tell them what to do, so they + // can ask the MCP server owner to enable their account instead of being + // left guessing why authentication "failed". + res.status(403).json({ + error: 'User not enabled for MCP usage', + reason: result.reason, + message: + `Your account (${upn}) is not enabled to use this Autopilot Monitor MCP server. ` + + 'Ask the MCP server owner/administrator to whitelist your account for MCP access, ' + + 'then reconnect.', + }); return; }