From e2fb99d5d87246a3dfa2d4cb293bad9dc4162ee4 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Sun, 6 Sep 2026 17:12:19 +0200 Subject: [PATCH] mcp access-guard: surface the suspension text from the policy envelope; AdminConfiguration.McpAccessPolicy comment matches the enforced AllMembers semantics; internal gitlink (MCP-Nebenbefunde, B-4s2) Co-Authored-By: Claude Fable 5.1 --- internal | 2 +- .../src/__tests__/access-guard.test.ts | 16 ++++++++++++++++ .../autopilot-monitor-mcp/src/access-guard.ts | 11 +++++++++-- .../Models/Config/AdminConfiguration.cs | 10 +++++++--- 4 files changed, 33 insertions(+), 6 deletions(-) diff --git a/internal b/internal index 15a9095e..b6c8e277 160000 --- a/internal +++ b/internal @@ -1 +1 @@ -Subproject commit 15a9095e9cc9cee7aa238aefe2d4a950c4b88ee3 +Subproject commit b6c8e277eb65ea14f97e42f7d4393c48c70b44fa 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 dfd9fbcb..158c6808 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 @@ -225,6 +225,22 @@ describe('accessGuard — 403 (authorization + fail-closed)', () => { expect(body.message).not.toMatch(/whitelist/i); }); + it('surfaces the suspension text when the policy middleware answers with the error envelope', async () => { + // A suspended home tenant is stopped by the suspension gate BEFORE the auth/mcp handler runs, so + // the body is the generic ApiErrorResponse (error + code), not the handler's verdict shape. + const upn = uniqueUpn(); + stubBackend({ + status: 403, + body: { error: 'Your tenant has been suspended. Please contact support for more information.', code: 'TenantSuspended', correlationId: 'c-1' }, + }); + const out = await runGuard(mockReq(`Bearer ${validToken(upn)}`)); + expect(out.status).toBe(403); + const body = out.body as { error: string; reason: string; message: string }; + expect(body.reason).toBe('Your tenant has been suspended. Please contact support for more information.'); + expect(body.message).toContain('has been suspended'); + expect(body.reason).not.toBe('denied'); + }); + 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 }); diff --git a/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts b/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts index 181e58a1..5afcf34b 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts @@ -12,7 +12,7 @@ import crypto from 'node:crypto'; import { APPLICATION_KEY_PREFIX, extractTokenClaims, isApplicationKey, isTokenExpired, principalKeyOf } from './auth.js'; import { runWithCaller, wantsPrettyJson } from './client.js'; import { API_BASE_URL, getPublicBaseUrl, parsePositiveInt } from './config.js'; -import type { CheckMcpAccessResponse } from './generated/wire-types.generated.js'; +import type { ApiErrorResponse, CheckMcpAccessResponse } from './generated/wire-types.generated.js'; const BASE_URL = API_BASE_URL; @@ -190,7 +190,14 @@ async function checkAccess(upn: string, token: string, clientIp: string): Promis // Wire type is generated from the backend DTO (CheckMcpAccessResponse). The read-only // GlobalReader gets the same cross-tenant routing as GA because this server is read-only. - const data = JSON.parse(text) as CheckMcpAccessResponse; + // A suspended home tenant never reaches the handler: the policy middleware's suspension gate + // answers first with the generic error envelope (ApiErrorResponse: `error` + `code`), whose + // message is the operator's suspension text. Fold it into a front-door deny so the user reads + // that text instead of a bare 'denied'. + const parsed = JSON.parse(text) as CheckMcpAccessResponse | ApiErrorResponse; + const data: CheckMcpAccessResponse = 'allowed' in parsed + ? parsed + : { allowed: false, upn, accessGrant: '', reason: parsed.error }; // Normalize defensively: accept only a non-empty array of strings, lowercased. Anything else // (missing, wrong type, empty) collapses to undefined → the caller is treated as non-delegated. const delegatedTenantIds = Array.isArray(data.delegatedTenantIds) diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs b/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs index 27d0b46a..34abcf14 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs @@ -213,9 +213,13 @@ namespace AutopilotMonitor.Shared.Models // ===== MCP ACCESS CONTROL ===== /// - /// Controls who can access the remote MCP server. - /// "Disabled" = MCP off, "WhitelistOnly" = GlobalAdmins + McpUsers table (default), - /// "AllMembers" = any authenticated user. + /// Controls who can access the remote MCP server (evaluated by McpUserService.IsAllowedAsync). + /// "Disabled" = MCP off for everyone, platform roles included. + /// "WhitelistOnly" = platform roles, delegated (MSP) admins and enabled McpUsers rows (default). + /// "AllMembers" = the same grants plus every account holding an effective member role + /// (Admin / Operator / Viewer) in the tenant its token was issued for — never "any authenticated + /// token": an employee without a role, or a tenant that never onboarded, is denied. + /// An explicitly disabled McpUsers row denies under every policy. /// public string McpAccessPolicy { get; set; } = nameof(Models.McpAccessPolicy.WhitelistOnly);