From 2d3853c38fc1abeb63ace5b87aa23498a48efca4 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Mon, 22 Jun 2026 19:07:10 +0200 Subject: [PATCH] feat(rbac): add read-only GlobalReader platform role (backend + MCP + web) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GlobalReader = cross-tenant READ visibility like a Global Admin, but no mutating operations and no platform settings. Provisioned exactly like a GA: a GlobalAdmins table row with Role=GlobalReader (empty/legacy Role = GlobalAdmin, back-compat). Semantics are ADDITIVE — a user who is also their own tenant's Admin keeps edit rights on that tenant; a pure GlobalReader is read-only everywhere. Backend: - GlobalAdmins.Role column + GlobalAdminService.GetGlobalRoleAsync; IsGlobalAdminAsync (incl. TableAdminRepository) delegates so a GlobalReader row never reads back as admin - RequestContext.IsGlobalReader + HasGlobalScope - Split GlobalAdminOnly into GlobalReadOrAdmin (cross-tenant reads) vs GlobalAdminOnly (writes/settings) + TenantAdminOrGlobalReader for config-read - PolicyEnforcementMiddleware: extracted testable DecideAsync seam; cross-tenant gate keys off HasGlobalScope; additive own-tenant role carried so IsTenantAdmin stays correct - Config secret redaction for readers (RedactedCopyForReader) + save-time placeholder restore (RestoreRedactedSecretsFrom) so a redacted view can't blank real secrets - raw table/log tools stay GlobalAdminOnly; SignalR global-admins read group admits HasGlobalScope - /auth/me emits isGlobalReader (+ preview-gate bypass, no auto-admin); /auth/mcp emits globalRole MCP: hasGlobalScope (GA or Reader) drives routing + tool catalog; raw secret-bearing tools (query_table/list_tables/query_backend_logs) gated strict-GA only. Web: AuthContext isGlobalReader + hasGlobalScope; reader reaches /dashboard + cross-tenant views; platform Settings/Ops/Backups/Software GA-only (nav-hidden + requireGlobalAdmin route layouts); mutating controls on mixed data pages disabled via useCanMutatePlatform; rules pages read-only on foreign tenants; additive own-tenant edit preserved. Tests: backend 2369, MCP 252, web 469 green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../buildcounter.txt | 2 +- .../AutopilotMonitor.Agent/buildcounter.txt | 2 +- .../AuthFunctionTests.cs | 81 +++- .../ConfigRedactionTests.cs | 140 +++++++ .../CrossTenantAccessTests.cs | 66 +++- .../EndpointPolicyCatalogCompletenessTests.cs | 36 +- .../PolicyEnforcementMiddlewareTests.cs | 265 +++++++++++++ .../TableStorage/TableAdminRepository.cs | 35 +- .../Config/GetAdminConfigurationFunction.cs | 16 +- .../GetAllTenantConfigurationsFunction.cs | 9 +- .../Config/GetTenantConfigurationFunction.cs | 16 +- .../UpdateTenantConfigurationFunction.cs | 13 +- .../Functions/Infrastructure/AuthFunction.cs | 24 +- .../Infrastructure/McpUserFunction.cs | 12 +- .../SignalRAddToGroupFunction.cs | 23 +- .../SignalRRemoveFromGroupFunction.cs | 19 +- .../Metrics/GetImeVersionHistoryFunction.cs | 2 +- .../Metrics/McpUsageMetricsFunction.cs | 2 +- .../Functions/Rules/GetRuleResultsFunction.cs | 6 +- .../GetSessionDecisionGraphFunction.cs | 4 +- .../GetSessionDeletePreviewFunction.cs | 2 +- .../Sessions/GetSessionEventsFunction.cs | 12 +- .../Functions/Sessions/GetSessionFunction.cs | 6 +- .../GetSessionReducerVerificationFunction.cs | 10 +- .../Sessions/GetSessionSignalsFunction.cs | 4 +- .../Sessions/QuickSearchSessionsFunction.cs | 5 +- .../GetVulnerabilityReportFunction.cs | 6 +- .../Helpers/RequestContext.cs | 20 +- .../Middleware/PolicyEnforcementMiddleware.cs | 373 +++++++++++------- .../Security/EndpointAccessPolicyCatalog.cs | 177 +++++---- .../Services/GlobalAdminService.cs | 60 ++- .../Services/McpUserService.cs | 31 +- .../src/__tests__/access-guard.test.ts | 23 +- .../src/__tests__/client-routing.test.ts | 9 + .../src/__tests__/tools-catalog-order.test.ts | 19 +- .../autopilot-monitor-mcp/src/access-guard.ts | 26 +- .../autopilot-monitor-mcp/src/client.ts | 36 +- .../autopilot-monitor-mcp/src/index.ts | 25 +- .../autopilot-monitor-mcp/src/tools.ts | 13 +- .../autopilot-monitor-mcp/src/tools/admin.ts | 15 +- .../AutopilotMonitor.Shared/Constants.cs | 20 + .../DataAccess/IAdminRepository.cs | 7 + .../Models/Config/AdminConfiguration.cs | 23 ++ .../Models/Config/TenantConfiguration.cs | 38 ++ .../app/admin/AdminConfigContext.tsx | 11 +- .../app/admin/backups/layout.tsx | 7 + .../admin/components/DeviceBlockSection.tsx | 30 +- .../components/SessionReportsSection.tsx | 13 +- .../admin/components/TenantAdminSection.tsx | 17 +- .../components/TenantManagementSection.tsx | 12 +- .../admin/components/VersionBlockSection.tsx | 25 +- .../components/VulnerabilityDataSection.tsx | 15 +- .../app/admin/customs-archive/layout.tsx | 7 + .../app/admin/layout.tsx | 5 +- .../app/admin/ops/layout.tsx | 7 + .../app/admin/settings/layout.tsx | 10 +- .../app/admin/software/layout.tsx | 7 + .../app/analyze-rules/page.tsx | 5 +- .../dashboard/hooks/useDashboardSessions.ts | 3 +- .../app/dashboard/page.tsx | 14 +- .../app/gather-rules/page.tsx | 5 +- .../app/settings/components/SaveResetBar.tsx | 11 +- .../components/GlobalSidebar.tsx | 13 +- .../components/Navbar.tsx | 57 ++- .../components/ProtectedRoute.tsx | 21 +- .../components/landing/AuthGate.tsx | 2 +- .../contexts/AuthContext.tsx | 14 + .../contexts/GlobalNotificationContext.tsx | 40 +- .../hooks/useAggregatedAdminScope.ts | 5 +- .../hooks/useCanMutatePlatform.ts | 14 + .../hooks/useGlobalAdminScope.ts | 12 +- .../lib/globalNavConfig.tsx | 18 +- 72 files changed, 1621 insertions(+), 512 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/ConfigRedactionTests.cs create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/PolicyEnforcementMiddlewareTests.cs create mode 100644 src/Web/autopilot-monitor-web/app/admin/backups/layout.tsx create mode 100644 src/Web/autopilot-monitor-web/app/admin/customs-archive/layout.tsx create mode 100644 src/Web/autopilot-monitor-web/app/admin/ops/layout.tsx create mode 100644 src/Web/autopilot-monitor-web/app/admin/software/layout.tsx create mode 100644 src/Web/autopilot-monitor-web/hooks/useCanMutatePlatform.ts diff --git a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt index 378d4b14..34684091 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent.V2/buildcounter.txt @@ -1 +1 @@ -1162 +1169 diff --git a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt index 07c2bc82..f488b513 100644 --- a/src/Agent/AutopilotMonitor.Agent/buildcounter.txt +++ b/src/Agent/AutopilotMonitor.Agent/buildcounter.txt @@ -1 +1 @@ -1459 +1466 diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionTests.cs index c6f592ca..7a7e446d 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/AuthFunctionTests.cs @@ -34,7 +34,7 @@ public class AuthFunctionTests public void HappyPath_AdminUser_Returns200WithCorrectFields() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: AdminRole(), mcpCheck: McpAllowed(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -59,7 +59,7 @@ public class AuthFunctionTests public void HappyPath_OperatorUser_ReturnsCorrectRole() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: OperatorRole(), mcpCheck: McpDenied(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -76,7 +76,7 @@ public class AuthFunctionTests public void HappyPath_ViewerUser_ReturnsCorrectRole() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: ViewerRole(), mcpCheck: McpDenied(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -91,7 +91,7 @@ public class AuthFunctionTests public void HappyPath_GlobalAdmin_ReturnsIsGlobalAdminTrue() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: true, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: true, isGlobalReader: false, isPreviewApproved: true, memberRole: AdminRole(), mcpCheck: McpAllowed(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -101,6 +101,53 @@ public class AuthFunctionTests Assert.True((bool)body.isGlobalAdmin); } + [Fact] + public void HappyPath_GlobalReader_ReturnsIsGlobalReaderTrue_NotGlobalAdmin() + { + var result = AuthFunction.BuildAuthResult( + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: true, isPreviewApproved: true, + memberRole: null, mcpCheck: McpAllowed(), + hasTenantAdmins: true, + TenantId, Upn, DisplayName, ObjectId); + + Assert.True(result.IsSuccess); + var body = ToDynamic(result.Body); + Assert.False((bool)body.isGlobalAdmin); + Assert.True((bool)body.isGlobalReader); + } + + [Fact] + public void PreviewGate_GlobalReaderBypassesPreviewGate() + { + // A read-only platform reader is staff and must bypass the private-preview gate, exactly like a GA. + var result = AuthFunction.BuildAuthResult( + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: true, isPreviewApproved: false, + memberRole: null, mcpCheck: McpAllowed(), + hasTenantAdmins: true, + TenantId, Upn, DisplayName, ObjectId); + + Assert.True(result.IsSuccess); + } + + [Fact] + public void AutoAdmin_NeverTriggered_ForGlobalReader_EvenAsFirstUser() + { + // GlobalReader is additive (it never removes tenant rights) but must not SILENTLY acquire new + // write power: the first-login auto-admin convenience is declined for a read-only-flagged + // identity. (Explicit TenantAdmins membership would still grant write — that's the additive path.) + var result = AuthFunction.BuildAuthResult( + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: true, isPreviewApproved: true, + memberRole: null, + mcpCheck: McpAllowed(), + hasTenantAdmins: false, // first user in this tenant + TenantId, Upn, DisplayName, ObjectId); + + Assert.True(result.IsSuccess); + Assert.False(result.NeedsAutoAdmin); + var body = ToDynamic(result.Body); + Assert.False((bool)body.isTenantAdmin); + } + // ------------------------------------------------------------------------- // Gate 1: Suspended Tenant // ------------------------------------------------------------------------- @@ -112,7 +159,7 @@ public class AuthFunctionTests config.Disabled = true; var result = AuthFunction.BuildAuthResult( - config, isGlobalAdmin: false, isPreviewApproved: true, + config, isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: AdminRole(), mcpCheck: McpAllowed(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -131,7 +178,7 @@ public class AuthFunctionTests config.DisabledReason = "Terms of service violation"; var result = AuthFunction.BuildAuthResult( - config, isGlobalAdmin: false, isPreviewApproved: true, + config, isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: AdminRole(), mcpCheck: McpAllowed(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -148,7 +195,7 @@ public class AuthFunctionTests config.Disabled = true; var result = AuthFunction.BuildAuthResult( - config, isGlobalAdmin: true, isPreviewApproved: true, + config, isGlobalAdmin: true, isGlobalReader: false, isPreviewApproved: true, memberRole: AdminRole(), mcpCheck: McpAllowed(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -167,7 +214,7 @@ public class AuthFunctionTests config.DisabledUntil = DateTime.UtcNow.AddHours(-1); // expired var result = AuthFunction.BuildAuthResult( - config, isGlobalAdmin: false, isPreviewApproved: true, + config, isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: AdminRole(), mcpCheck: McpAllowed(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -183,7 +230,7 @@ public class AuthFunctionTests public void PreviewGate_NotApproved_Returns403PrivatePreview() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: false, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: false, memberRole: AdminRole(), mcpCheck: McpAllowed(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -198,7 +245,7 @@ public class AuthFunctionTests public void PreviewGate_GlobalAdminBypassesPreviewGate() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: true, isPreviewApproved: false, + DefaultConfig(), isGlobalAdmin: true, isGlobalReader: false, isPreviewApproved: false, memberRole: AdminRole(), mcpCheck: McpAllowed(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -214,7 +261,7 @@ public class AuthFunctionTests public void AutoAdmin_FirstUser_NoExistingAdmins_BecomesAdmin() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: null, // no role yet mcpCheck: McpDenied(), hasTenantAdmins: false, // no existing admins @@ -232,7 +279,7 @@ public class AuthFunctionTests public void AutoAdmin_NotTriggered_WhenAdminsExist() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: null, // no role — new user but admins exist mcpCheck: McpDenied(), hasTenantAdmins: true, // admins already exist @@ -252,7 +299,7 @@ public class AuthFunctionTests // user into the TenantAdmins table. needsAutoAdmin keys off memberRole == null, not on // "not admin", so an Entra app-role Operator in a claim-only tenant stays an Operator. var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: OperatorRole(), // came from the "roles" claim mcpCheck: McpDenied(), hasTenantAdmins: false, // no explicit TenantAdmins table rows @@ -269,7 +316,7 @@ public class AuthFunctionTests public void AutoAdmin_NotTriggered_WhenUserAlreadyAdmin() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: AdminRole(), mcpCheck: McpDenied(), hasTenantAdmins: true, @@ -289,7 +336,7 @@ public class AuthFunctionTests public void McpAccess_Allowed_ReturnsTrue() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: AdminRole(), mcpCheck: McpAllowed(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -302,7 +349,7 @@ public class AuthFunctionTests public void McpAccess_Denied_ReturnsFalse() { var result = AuthFunction.BuildAuthResult( - DefaultConfig(), isGlobalAdmin: false, isPreviewApproved: true, + DefaultConfig(), isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: true, memberRole: AdminRole(), mcpCheck: McpDenied(), hasTenantAdmins: true, TenantId, Upn, DisplayName, ObjectId); @@ -322,7 +369,7 @@ public class AuthFunctionTests config.Disabled = true; var result = AuthFunction.BuildAuthResult( - config, isGlobalAdmin: false, isPreviewApproved: false, + config, isGlobalAdmin: false, isGlobalReader: false, isPreviewApproved: false, memberRole: null, mcpCheck: McpDenied(), hasTenantAdmins: false, TenantId, Upn, DisplayName, ObjectId); diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/ConfigRedactionTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/ConfigRedactionTests.cs new file mode 100644 index 00000000..9f1e68dd --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/ConfigRedactionTests.cs @@ -0,0 +1,140 @@ +using System.Reflection; +using AutopilotMonitor.Shared; +using AutopilotMonitor.Shared.Models; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Guards the GlobalReader secret-redaction contract on the config models. The redactors are +/// deny-lists (RedactedCopyForReader masks an explicit set of secret fields); these tests fail +/// if a NEW secret-looking string field is added to a model but not to its redactor — closing the +/// drift gap that would otherwise leak a fresh SAS/webhook/API-key field to a read-only reader. +/// +public class ConfigRedactionTests +{ + // A string property whose NAME matches one of these fragments is considered secret-bearing. + private static readonly string[] SecretNameFragments = + { "SasUrl", "WebhookUrl", "ApiKey", "CustomHeadersJson", "Secret", "Password", "BotToken" }; + + private static bool IsSecretName(string name) + => SecretNameFragments.Any(f => name.Contains(f, StringComparison.OrdinalIgnoreCase)); + + private static IEnumerable WritableStringProps(Type t) + => t.GetProperties(BindingFlags.Public | BindingFlags.Instance) + .Where(p => p.PropertyType == typeof(string) && p.CanRead && p.CanWrite); + + [Fact] + public void AdminConfiguration_RedactsEverySecretLookingStringField() + { + var cfg = new AdminConfiguration(); + foreach (var p in WritableStringProps(typeof(AdminConfiguration))) + p.SetValue(cfg, "POPULATED_SECRET_VALUE"); + + var redacted = cfg.RedactedCopyForReader(); + + foreach (var p in WritableStringProps(typeof(AdminConfiguration)).Where(p => IsSecretName(p.Name))) + { + Assert.Equal(Constants.RedactedSecretPlaceholder, (string?)p.GetValue(redacted)); + } + } + + [Fact] + public void TenantConfiguration_RedactsEverySecretLookingStringField() + { + var cfg = new TenantConfiguration(); + foreach (var p in WritableStringProps(typeof(TenantConfiguration))) + p.SetValue(cfg, "POPULATED_SECRET_VALUE"); + + var redacted = cfg.RedactedCopyForReader(); + + foreach (var p in WritableStringProps(typeof(TenantConfiguration)).Where(p => IsSecretName(p.Name))) + { + Assert.Equal(Constants.RedactedSecretPlaceholder, (string?)p.GetValue(redacted)); + } + } + + [Fact] + public void AdminConfiguration_RedactionLeavesNonSecretsAndOriginalIntact() + { + var cfg = new AdminConfiguration { NvdApiKey = "real-key", UpdatedBy = "admin@contoso.com" }; + + var redacted = cfg.RedactedCopyForReader(); + + Assert.Equal(Constants.RedactedSecretPlaceholder, redacted.NvdApiKey); + Assert.Equal("admin@contoso.com", redacted.UpdatedBy); // non-secret preserved + Assert.Equal("real-key", cfg.NvdApiKey); // original NOT mutated + } + + [Fact] + public void TenantConfiguration_RedactionLeavesNonSecretsAndOriginalIntact() + { + var cfg = new TenantConfiguration + { + DiagnosticsBlobSasUrl = "https://acct.blob.core.windows.net/c?sig=secret", + WebhookUrl = "https://hooks.example/abc", + DomainName = "contoso.com", + }; + + var redacted = cfg.RedactedCopyForReader(); + + Assert.Equal(Constants.RedactedSecretPlaceholder, redacted.DiagnosticsBlobSasUrl); + Assert.Equal(Constants.RedactedSecretPlaceholder, redacted.WebhookUrl); + Assert.Equal("contoso.com", redacted.DomainName); // non-secret preserved + Assert.Equal("https://acct.blob.core.windows.net/c?sig=secret", cfg.DiagnosticsBlobSasUrl); // original intact + } + + // ── Save-guard: a redacted view round-tripped on save must NOT overwrite real secrets ── + + [Fact] + public void TenantConfiguration_RestoreRedactedSecrets_RecoversRealValuesFromExisting() + { + var existing = new TenantConfiguration + { + DiagnosticsBlobSasUrl = "https://acct.blob.core.windows.net/c?sig=real", + TeamsWebhookUrl = "https://teams.example/real", + WebhookUrl = "https://hooks.example/real", + WebhookCustomHeadersJson = "{\"X-Api-Key\":\"real\"}", + DomainName = "contoso.com", + }; + + // Simulate the read-only reader view being saved back unchanged. + var incoming = existing.RedactedCopyForReader(); + Assert.Equal(Constants.RedactedSecretPlaceholder, incoming.WebhookUrl); // sanity: it really is redacted + + incoming.RestoreRedactedSecretsFrom(existing); + + Assert.Equal(existing.DiagnosticsBlobSasUrl, incoming.DiagnosticsBlobSasUrl); + Assert.Equal(existing.TeamsWebhookUrl, incoming.TeamsWebhookUrl); + Assert.Equal(existing.WebhookUrl, incoming.WebhookUrl); + Assert.Equal(existing.WebhookCustomHeadersJson, incoming.WebhookCustomHeadersJson); + } + + [Fact] + public void TenantConfiguration_RestoreRedactedSecrets_KeepsGenuineNewValues() + { + // A real edit (non-placeholder) must be preserved — restore only swaps the sentinel. + var existing = new TenantConfiguration { WebhookUrl = "https://hooks.example/old" }; + var incoming = new TenantConfiguration + { + WebhookUrl = "https://hooks.example/NEW", // genuine change + DiagnosticsBlobSasUrl = Constants.RedactedSecretPlaceholder, // untouched in UI + }; + existing.DiagnosticsBlobSasUrl = "https://acct.blob.core.windows.net/c?sig=old"; + + incoming.RestoreRedactedSecretsFrom(existing); + + Assert.Equal("https://hooks.example/NEW", incoming.WebhookUrl); // genuine edit kept + Assert.Equal("https://acct.blob.core.windows.net/c?sig=old", incoming.DiagnosticsBlobSasUrl); // placeholder restored + } + + [Fact] + public void Redaction_LeavesEmptySecretsEmpty() + { + // An unset secret stays empty (UI shows "not configured", not a misleading "***REDACTED***"). + var admin = new AdminConfiguration { NvdApiKey = "" }.RedactedCopyForReader(); + Assert.Equal("", admin.NvdApiKey); + + var tenant = new TenantConfiguration { WebhookUrl = "" }.RedactedCopyForReader(); + Assert.Equal("", tenant.WebhookUrl); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/CrossTenantAccessTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/CrossTenantAccessTests.cs index eee5cf87..8999e315 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/CrossTenantAccessTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/CrossTenantAccessTests.cs @@ -22,7 +22,7 @@ public class CrossTenantAccessTests /// compare to JWT tenant, return whether access should be blocked. /// private static (bool isBlocked, string? routeTenantId) SimulateCrossTenantCheck( - string httpMethod, string requestPath, string jwtTenantId, bool isGlobalAdmin) + string httpMethod, string requestPath, string jwtTenantId, bool hasGlobalScope) { var entry = EndpointAccessPolicyCatalog.FindPolicy(httpMethod, requestPath); if (entry == null) @@ -40,8 +40,8 @@ public class CrossTenantAccessTests if (string.IsNullOrEmpty(routeTenantId)) return (false, null); // no tenantId extracted → no check - if (isGlobalAdmin) - return (false, routeTenantId); // GA bypass + if (hasGlobalScope) + return (false, routeTenantId); // global-scope bypass (GA or GlobalReader) var isCrossTenant = !string.Equals(routeTenantId, jwtTenantId, StringComparison.OrdinalIgnoreCase); return (isCrossTenant, routeTenantId); @@ -52,7 +52,7 @@ public class CrossTenantAccessTests /// fall back to JWT tenant, compare against JWT tenant. /// private static (bool isBlocked, string targetTenantId) SimulateQueryParamCheck( - string httpMethod, string requestPath, string jwtTenantId, bool isGlobalAdmin, string? queryTenantId) + string httpMethod, string requestPath, string jwtTenantId, bool hasGlobalScope, string? queryTenantId) { var entry = EndpointAccessPolicyCatalog.FindPolicy(httpMethod, requestPath); if (entry == null) @@ -63,7 +63,7 @@ public class CrossTenantAccessTests var targetTenantId = string.IsNullOrWhiteSpace(queryTenantId) ? jwtTenantId : queryTenantId; - if (isGlobalAdmin) + if (hasGlobalScope) return (false, targetTenantId); var isCrossTenant = !string.Equals(targetTenantId, jwtTenantId, StringComparison.OrdinalIgnoreCase); @@ -86,7 +86,7 @@ public class CrossTenantAccessTests public void SameTenant_NonGA_IsAllowed(string httpMethod, string pathTemplate) { var path = string.Format(pathTemplate, TenantA); - var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, isGlobalAdmin: false); + var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, hasGlobalScope: false); Assert.False(isBlocked, $"Same-tenant access should be allowed: {httpMethod} {path}"); Assert.Equal(TenantA, routeTenantId); @@ -108,7 +108,7 @@ public class CrossTenantAccessTests public void CrossTenant_NonGA_IsBlocked(string httpMethod, string pathTemplate) { var path = string.Format(pathTemplate, TenantB); // accessing TenantB - var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, isGlobalAdmin: false); + var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, hasGlobalScope: false); Assert.True(isBlocked, $"Cross-tenant access should be blocked: {httpMethod} {path}"); Assert.Equal(TenantB, routeTenantId); @@ -124,7 +124,7 @@ public class CrossTenantAccessTests public void CrossTenant_GlobalAdmin_IsAllowed(string httpMethod, string pathTemplate) { var path = string.Format(pathTemplate, TenantB); // accessing TenantB - var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, isGlobalAdmin: true); + var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, hasGlobalScope: true); Assert.False(isBlocked, $"Global Admin should be allowed cross-tenant access: {httpMethod} {path}"); Assert.Equal(TenantB, routeTenantId); @@ -139,7 +139,7 @@ public class CrossTenantAccessTests [InlineData("GET", "/api/rules/gather")] public void JwtImplicitRoutes_NoCrossTenantCheck(string httpMethod, string path) { - var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, isGlobalAdmin: false); + var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, hasGlobalScope: false); Assert.False(isBlocked, $"JWT-implicit route should have no cross-tenant check: {httpMethod} {path}"); Assert.Null(routeTenantId); @@ -151,7 +151,7 @@ public class CrossTenantAccessTests public void SameTenant_DifferentCase_IsAllowed() { var upperTenant = TenantA.ToUpperInvariant(); - var (isBlocked, _) = SimulateCrossTenantCheck("GET", $"/api/config/{upperTenant}", TenantA, isGlobalAdmin: false); + var (isBlocked, _) = SimulateCrossTenantCheck("GET", $"/api/config/{upperTenant}", TenantA, hasGlobalScope: false); Assert.False(isBlocked, "Tenant ID comparison should be case-insensitive"); } @@ -162,7 +162,7 @@ public class CrossTenantAccessTests public void AdminSubRoutes_ExtractCorrectTenantId() { var path = $"/api/tenants/{TenantA}/admins/user@contoso.com"; - var (isBlocked, routeTenantId) = SimulateCrossTenantCheck("DELETE", path, TenantA, isGlobalAdmin: false); + var (isBlocked, routeTenantId) = SimulateCrossTenantCheck("DELETE", path, TenantA, hasGlobalScope: false); Assert.False(isBlocked); Assert.Equal(TenantA, routeTenantId); @@ -172,7 +172,7 @@ public class CrossTenantAccessTests public void AdminSubRoutes_CrossTenant_IsBlocked() { var path = $"/api/tenants/{TenantB}/admins/user@contoso.com/permissions"; - var (isBlocked, routeTenantId) = SimulateCrossTenantCheck("PATCH", path, TenantA, isGlobalAdmin: false); + var (isBlocked, routeTenantId) = SimulateCrossTenantCheck("PATCH", path, TenantA, hasGlobalScope: false); Assert.True(isBlocked); Assert.Equal(TenantB, routeTenantId); @@ -184,7 +184,6 @@ public class CrossTenantAccessTests [InlineData("PATCH", "/api/config/{0}/plan")] [InlineData("POST", "/api/preview/whitelist/{0}")] [InlineData("DELETE", "/api/preview/whitelist/{0}")] - [InlineData("GET", "/api/preview/notification-email/{0}")] [InlineData("POST", "/api/preview/send-welcome-email/{0}")] public void GlobalAdminOnlyRoutes_WithTenantId_HaveRouteParamScoping(string httpMethod, string pathTemplate) { @@ -196,6 +195,39 @@ public class CrossTenantAccessTests Assert.Equal(TenantScoping.RouteParam, entry.TenantScoping); } + // ── GlobalReadOrAdmin routes with {tenantId} keep RouteParam scoping ── + // Cross-tenant READ surfaces opened to the read-only GlobalReader tier must still declare + // RouteParam so the middleware's HasGlobalScope gate (GA ∪ Reader) governs cross-tenant access. + + [Theory] + [InlineData("GET", "/api/preview/notification-email/{0}")] + [InlineData("GET", "/api/global/tenants/{0}/deletion-manifests")] + public void GlobalReadRoutes_WithTenantId_HaveRouteParamScoping(string httpMethod, string pathTemplate) + { + var path = string.Format(pathTemplate, TenantA); + var entry = EndpointAccessPolicyCatalog.FindPolicy(httpMethod, path); + + Assert.NotNull(entry); + Assert.Equal(EndpointPolicy.GlobalReadOrAdmin, entry.Policy); + Assert.Equal(TenantScoping.RouteParam, entry.TenantScoping); + } + + // ── Cross-tenant read by a GlobalReader (global scope, no write power) is allowed ── + // The middleware gate is HasGlobalScope (GA ∪ GlobalReader); a read-only reader crosses tenants + // for reads exactly like a GA. Modelled by hasGlobalScope:true on a read route. + + [Theory] + [InlineData("GET", "/api/config/{0}")] + [InlineData("GET", "/api/preview/notification-email/{0}")] + public void CrossTenant_GlobalReader_ReadRoute_IsAllowed(string httpMethod, string pathTemplate) + { + var path = string.Format(pathTemplate, TenantB); // reader in TenantA viewing TenantB + var (isBlocked, routeTenantId) = SimulateCrossTenantCheck(httpMethod, path, TenantA, hasGlobalScope: true); + + Assert.False(isBlocked, $"GlobalReader should be allowed cross-tenant read: {httpMethod} {path}"); + Assert.Equal(TenantB, routeTenantId); + } + // ── QueryParam: no query param → defaults to JWT tenant ─────────── [Theory] @@ -208,7 +240,7 @@ public class CrossTenantAccessTests [InlineData("GET", "/api/progress/sessions/abc-123/events")] public void QueryParam_NoQueryTenant_DefaultsToJwt(string httpMethod, string path) { - var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, isGlobalAdmin: false, queryTenantId: null); + var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, hasGlobalScope: false, queryTenantId: null); Assert.False(isBlocked); Assert.Equal(TenantA, targetTenantId); @@ -224,7 +256,7 @@ public class CrossTenantAccessTests [InlineData("GET", "/api/progress/sessions/abc-123/events")] public void QueryParam_SameTenant_IsAllowed(string httpMethod, string path) { - var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, isGlobalAdmin: false, queryTenantId: TenantA); + var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, hasGlobalScope: false, queryTenantId: TenantA); Assert.False(isBlocked); Assert.Equal(TenantA, targetTenantId); @@ -244,7 +276,7 @@ public class CrossTenantAccessTests [InlineData("GET", "/api/progress/sessions/abc-123/events")] public void QueryParam_CrossTenant_NonGA_IsBlocked(string httpMethod, string path) { - var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, isGlobalAdmin: false, queryTenantId: TenantB); + var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, hasGlobalScope: false, queryTenantId: TenantB); Assert.True(isBlocked, $"Cross-tenant query param should be blocked: {httpMethod} {path}"); Assert.Equal(TenantB, targetTenantId); @@ -261,7 +293,7 @@ public class CrossTenantAccessTests [InlineData("GET", "/api/progress/sessions/abc-123/events")] public void QueryParam_CrossTenant_GlobalAdmin_IsAllowed(string httpMethod, string path) { - var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, isGlobalAdmin: true, queryTenantId: TenantB); + var (isBlocked, targetTenantId) = SimulateQueryParamCheck(httpMethod, path, TenantA, hasGlobalScope: true, queryTenantId: TenantB); Assert.False(isBlocked); Assert.Equal(TenantB, targetTenantId); diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs index 66d18345..fa7a3569 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/EndpointPolicyCatalogCompletenessTests.cs @@ -159,7 +159,9 @@ public class EndpointPolicyCatalogCompletenessTests Assert.NotNull(getConfig); Assert.NotNull(putConfig); - Assert.Equal(EndpointPolicy.TenantAdminOrGA, getConfig.Policy); + // GET config read is admin-tier read (own-tenant Admin / GA / read-only GlobalReader); + // PUT config write stays Admin/GA only. + Assert.Equal(EndpointPolicy.TenantAdminOrGlobalReader, getConfig.Policy); Assert.Equal(EndpointPolicy.TenantAdminOrGA, putConfig.Policy); var getRules = EndpointAccessPolicyCatalog.FindPolicy("GET", "/api/rules/gather"); @@ -221,46 +223,48 @@ public class EndpointPolicyCatalogCompletenessTests } /// - /// The three global/apps/* routes must be locked to GlobalAdminOnly with - /// TenantScoping.None (the optional ?tenantId= query param is intentional cross-tenant - /// scoping, enforced inside the function — not by the middleware). - /// A future accidental downgrade of the policy tier would be caught here, whereas - /// the generic completeness test only guarantees that *some* entry exists. + /// The three global/apps/* routes must be cross-tenant READS — GlobalReadOrAdmin (GA + read-only + /// GlobalReader) — with TenantScoping.None (the optional ?tenantId= query param is intentional + /// cross-tenant scoping, enforced inside the function — not by the middleware). A future accidental + /// downgrade below the global-scope tier (e.g. to MemberRead, exposing them to tenant members) + /// would be caught here, whereas the generic completeness test only guarantees *some* entry exists. /// [Theory] [InlineData("GET", "/api/global/apps/list", "global/apps/list")] [InlineData("GET", "/api/global/apps/Company%20Portal/analytics", "global/apps/{appName}/analytics")] [InlineData("GET", "/api/global/apps/Company%20Portal/sessions", "global/apps/{appName}/sessions")] - public void GlobalAppsRoutes_AreGlobalAdminOnly(string method, string path, string expectedTemplate) + public void GlobalAppsRoutes_AreGlobalReadOrAdmin(string method, string path, string expectedTemplate) { var entry = EndpointAccessPolicyCatalog.FindPolicy(method, path); Assert.NotNull(entry); Assert.Equal(expectedTemplate, entry!.RouteTemplate); - Assert.Equal(EndpointPolicy.GlobalAdminOnly, entry.Policy); + Assert.Equal(EndpointPolicy.GlobalReadOrAdmin, entry.Policy); Assert.Equal(TenantScoping.None, entry.TenantScoping); } /// - /// Inspector v1 endpoints (signals, decision-graph, reducer-verification) MUST be locked - /// to GlobalAdminOnly while the UI matures (Plan §M6 — primary use case is modelling - /// 2-stage WhiteGlove deployments). Defense-in-depth against an accidental policy - /// downgrade that would expose decision internals to tenant admins before the lift. + /// Inspector v1 endpoints (signals, decision-graph, reducer-verification) MUST stay locked to a + /// platform-scope tier while the UI matures (Plan §M6 — primary use case is modelling 2-stage + /// WhiteGlove deployments). They are now GlobalReadOrAdmin (GA + read-only GlobalReader): still + /// invisible to tenant admins/members, but visible to the read-only platform tier. Defense-in-depth + /// against an accidental downgrade to a tenant tier (MemberRead/TenantAdminOrGA) that would expose + /// decision internals before the lift. /// - /// When the v2 adminMode lift happens, signals+decision-graph move back to MemberRead - /// with TenantScoping.QueryParam. reducer-verification stays GlobalAdminOnly forever. + /// When the v2 adminMode lift happens, signals+decision-graph move to MemberRead with + /// TenantScoping.QueryParam. reducer-verification stays platform-scope (never tenant-visible). /// [Theory] [InlineData("GET", "/api/sessions/abc-123/signals", "sessions/{sessionId}/signals")] [InlineData("GET", "/api/sessions/abc-123/decision-graph", "sessions/{sessionId}/decision-graph")] [InlineData("GET", "/api/sessions/abc-123/reducer-verification", "sessions/{sessionId}/reducer-verification")] - public void InspectorRoutes_AreGlobalAdminOnly(string method, string path, string expectedTemplate) + public void InspectorRoutes_AreGlobalReadOrAdmin(string method, string path, string expectedTemplate) { var entry = EndpointAccessPolicyCatalog.FindPolicy(method, path); Assert.NotNull(entry); Assert.Equal(expectedTemplate, entry!.RouteTemplate); - Assert.Equal(EndpointPolicy.GlobalAdminOnly, entry.Policy); + Assert.Equal(EndpointPolicy.GlobalReadOrAdmin, entry.Policy); Assert.Equal(TenantScoping.None, entry.TenantScoping); } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/PolicyEnforcementMiddlewareTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/PolicyEnforcementMiddlewareTests.cs new file mode 100644 index 00000000..4d135646 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/PolicyEnforcementMiddlewareTests.cs @@ -0,0 +1,265 @@ +using System.Security.Claims; +using AutopilotMonitor.Functions.Middleware; +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared; +using AutopilotMonitor.Shared.DataAccess; +using Microsoft.Extensions.Caching.Memory; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Tests for via its transport-agnostic DecideAsync seam +/// (authorization + cross-tenant enforcement + RequestContext construction — everything Invoke does +/// minus reading the request / writing the response). Uses the REAL GlobalAdminService / +/// TenantAdminsService / TenantConfigurationService — they all resolve through +/// + an in-memory cache, so only the repository is mocked. +/// +/// Primary purpose: lock in the ADDITIVE GlobalReader semantics in the resolved RequestContext — +/// a UPN that is both a GlobalReader and its own tenant's Admin must still surface IsTenantAdmin=true +/// (otherwise admin SignalR group joins + admin-audience notifications regress), while a pure GlobalReader +/// stays read-only. Also covers the cross-tenant gate keying off HasGlobalScope. +/// +public class PolicyEnforcementMiddlewareTests +{ + private const string TenantA = "11111111-1111-1111-1111-111111111111"; + private const string TenantB = "22222222-2222-2222-2222-222222222222"; + + private sealed class Harness + { + public required PolicyEnforcementMiddleware Middleware { get; init; } + public required Mock Repo { get; init; } + + public void AsGlobalRole(string role) => + Repo.Setup(r => r.GetGlobalRoleAsync(It.IsAny())).ReturnsAsync(role); + + public void AsTenantAdmin(string tenantId, string upn) => + Repo.Setup(r => r.GetTenantMemberAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync(new TenantMember + { + Upn = upn, + TenantId = tenantId, + Role = Constants.TenantRoles.Admin, + IsEnabled = true, + }); + } + + private static Harness BuildHarness() + { + var repo = new Mock(); + // Defaults: no platform role, no tenant membership. Tests override per-case. + repo.Setup(r => r.GetGlobalRoleAsync(It.IsAny())).ReturnsAsync((string?)null); + repo.Setup(r => r.GetTenantMemberAsync(It.IsAny(), It.IsAny())) + .ReturnsAsync((TenantMember?)null); + + var cache = new MemoryCache(new MemoryCacheOptions()); + var globalAdmin = new GlobalAdminService(repo.Object, cache, NullLogger.Instance); + var tenantAdmins = new TenantAdminsService(repo.Object, cache, NullLogger.Instance); + var tenantConfig = new TenantConfigurationService( + Mock.Of(), NullLogger.Instance, cache); + + var mw = new PolicyEnforcementMiddleware( + NullLogger.Instance, globalAdmin, tenantAdmins, tenantConfig); + + return new Harness { Middleware = mw, Repo = repo }; + } + + private static ClaimsPrincipal AuthedPrincipal(string tenantId, string upn) + => new(new ClaimsIdentity(new[] { new Claim("tid", tenantId), new Claim("upn", upn) }, "TestAuth")); + + // ── ADDITIVE: GlobalReader + own-tenant Admin keeps IsTenantAdmin ────────────── + + [Fact] + public async Task MemberRead_GlobalReaderWhoIsAlsoTenantAdmin_KeepsIsTenantAdmin() + { + const string upn = "dual@contoso.com"; + var h = BuildHarness(); + h.AsGlobalRole(Constants.GlobalRoles.GlobalReader); + h.AsTenantAdmin(TenantA, upn); + + // /api/notifications is MemberRead — its audience depends on IsTenantAdmin. + var result = await h.Middleware.DecideAsync("GET", "/api/notifications", null, AuthedPrincipal(TenantA, upn)); + + Assert.True(result.Allowed); + var rc = result.Context!; + Assert.True(rc.IsGlobalReader); + Assert.False(rc.IsGlobalAdmin); + Assert.True(rc.IsTenantAdmin); // additive: own-tenant admin survives the global-role branch + Assert.True(rc.HasGlobalScope); + } + + [Fact] + public async Task AuthenticatedUserWithRole_GlobalReaderWhoIsAlsoTenantAdmin_KeepsIsTenantAdmin() + { + const string upn = "dual@contoso.com"; + var h = BuildHarness(); + h.AsGlobalRole(Constants.GlobalRoles.GlobalReader); + h.AsTenantAdmin(TenantA, upn); + + // SignalR group join is AuthenticatedUserWithRole — the admin notification group gates on IsTenantAdmin. + var result = await h.Middleware.DecideAsync("POST", "/api/realtime/groups/join", null, AuthedPrincipal(TenantA, upn)); + + Assert.True(result.Allowed); + Assert.True(result.Context!.IsGlobalReader); + Assert.True(result.Context!.IsTenantAdmin); + } + + // ── Pure roles (regression guards for the TenantRole ?? UserRole fallback) ───── + + [Fact] + public async Task MemberRead_PureGlobalReader_IsNotTenantAdmin() + { + const string upn = "reader@contoso.com"; + var h = BuildHarness(); + h.AsGlobalRole(Constants.GlobalRoles.GlobalReader); + // No tenant membership (default null). + + var result = await h.Middleware.DecideAsync("GET", "/api/notifications", null, AuthedPrincipal(TenantA, upn)); + + Assert.True(result.Allowed); + Assert.True(result.Context!.IsGlobalReader); + Assert.False(result.Context!.IsTenantAdmin); // no own-tenant role ⇒ not a tenant admin + } + + [Fact] + public async Task MemberRead_PureTenantAdmin_IsTenantAdmin_NoGlobalScope() + { + const string upn = "admin@contoso.com"; + var h = BuildHarness(); + h.AsTenantAdmin(TenantA, upn); // enabled Admin row, no global role + + var result = await h.Middleware.DecideAsync("GET", "/api/notifications", null, AuthedPrincipal(TenantA, upn)); + + Assert.True(result.Allowed); + Assert.True(result.Context!.IsTenantAdmin); // via the TenantRole ?? UserRole fallback (UserRole = "Admin") + Assert.False(result.Context!.IsGlobalReader); + Assert.False(result.Context!.IsGlobalAdmin); + } + + // ── ADDITIVE write: GlobalReader + own-tenant Admin may still write own tenant ─ + + [Fact] + public async Task WriteRoute_GlobalReaderWhoIsAlsoTenantAdmin_AllowedInOwnTenant() + { + const string upn = "dual@contoso.com"; + var h = BuildHarness(); + h.AsGlobalRole(Constants.GlobalRoles.GlobalReader); + h.AsTenantAdmin(TenantA, upn); + + // PUT config/{tenantId} is TenantAdminOrGA (write). Route tenant == JWT tenant ⇒ own-tenant write. + var result = await h.Middleware.DecideAsync("PUT", $"/api/config/{TenantA}", null, AuthedPrincipal(TenantA, upn)); + + Assert.True(result.Allowed); // additive: the tenant-admin hat still grants write in the own tenant + } + + // ── Pure GlobalReader is read-only: write routes are denied ──────────────────── + + [Fact] + public async Task WriteRoute_PureGlobalReader_IsForbidden() + { + const string upn = "reader@contoso.com"; + var h = BuildHarness(); + h.AsGlobalRole(Constants.GlobalRoles.GlobalReader); + // No tenant membership ⇒ no write path. + + var result = await h.Middleware.DecideAsync("PUT", $"/api/config/{TenantA}", null, AuthedPrincipal(TenantA, upn)); + + Assert.False(result.Allowed); + Assert.Equal(403, result.StatusCode); + } + + // ── config GET (TenantAdminOrGlobalReader): own-tenant admin view vs reader view ── + // GetTenantConfigurationFunction redacts only when IsGlobalReader && !(IsTenantAdmin && Target==Tenant). + // These lock in the RequestContext that drives that decision. + + [Fact] + public async Task ConfigGet_GlobalReaderWhoIsOwnTenantAdmin_OwnTenant_IsFullView() + { + const string upn = "dual@contoso.com"; + var h = BuildHarness(); + h.AsGlobalRole(Constants.GlobalRoles.GlobalReader); + h.AsTenantAdmin(TenantA, upn); + + var result = await h.Middleware.DecideAsync("GET", $"/api/config/{TenantA}", null, AuthedPrincipal(TenantA, upn)); + + Assert.True(result.Allowed); + var rc = result.Context!; + Assert.True(rc.IsTenantAdmin); + Assert.Equal(rc.TenantId, rc.TargetTenantId); // own tenant ⇒ ownTenantAdminView ⇒ FULL config (not redacted) + } + + [Fact] + public async Task ConfigGet_GlobalReaderCrossTenant_IsReaderView() + { + const string upn = "dual@contoso.com"; + var h = BuildHarness(); + h.AsGlobalRole(Constants.GlobalRoles.GlobalReader); + h.AsTenantAdmin(TenantA, upn); // admin of their OWN tenant (A); role resolves against JWT tenant + + var result = await h.Middleware.DecideAsync("GET", $"/api/config/{TenantB}", null, AuthedPrincipal(TenantA, upn)); + + Assert.True(result.Allowed); + var rc = result.Context!; + Assert.True(rc.IsGlobalReader); + Assert.True(rc.IsTenantAdmin); // admin of A… + Assert.NotEqual(rc.TenantId, rc.TargetTenantId); // …but viewing B ⇒ reader view ⇒ REDACTED + Assert.Equal(TenantB, rc.TargetTenantId); + } + + // ── Cross-tenant read: GlobalReader crosses tenants (HasGlobalScope gate) ─────── + + [Fact] + public async Task CrossTenantRead_GlobalReader_AllowedAndTargetsForeignTenant() + { + const string upn = "reader@contoso.com"; + var h = BuildHarness(); + h.AsGlobalRole(Constants.GlobalRoles.GlobalReader); + + // Reader in TenantA reading TenantB's config (RouteParam). HasGlobalScope ⇒ cross-tenant allowed. + var result = await h.Middleware.DecideAsync("GET", $"/api/config/{TenantB}", null, AuthedPrincipal(TenantA, upn)); + + Assert.True(result.Allowed); + Assert.Equal(TenantB, result.Context!.TargetTenantId); + } + + [Fact] + public async Task CrossTenantRead_PureTenantAdmin_IsForbidden() + { + const string upn = "admin@contoso.com"; + var h = BuildHarness(); + h.AsTenantAdmin(TenantA, upn); // admin of A only, no global scope + + // Admin of TenantA trying to read TenantB's config → cross-tenant blocked. + var result = await h.Middleware.DecideAsync("GET", $"/api/config/{TenantB}", null, AuthedPrincipal(TenantA, upn)); + + Assert.False(result.Allowed); + Assert.Equal(403, result.StatusCode); + Assert.Equal("CrossTenantAccessDenied", result.ErrorCode); + } + + // ── Unauthenticated / unregistered ───────────────────────────────────────────── + + [Fact] + public async Task MemberRead_NoPrincipal_IsUnauthorized() + { + var h = BuildHarness(); + var result = await h.Middleware.DecideAsync("GET", "/api/notifications", null, principal: null); + + Assert.False(result.Allowed); + Assert.Equal(401, result.StatusCode); + } + + [Fact] + public async Task UnregisteredRoute_IsForbidden() + { + var h = BuildHarness(); + h.AsGlobalRole(Constants.GlobalRoles.GlobalAdmin); + + var result = await h.Middleware.DecideAsync("GET", "/api/does-not-exist", null, AuthedPrincipal(TenantA, "ga@contoso.com")); + + Assert.False(result.Allowed); + Assert.Equal(403, result.StatusCode); + Assert.Equal("Forbidden", result.ErrorCode); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableAdminRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableAdminRepository.cs index 615ba836..a65a496a 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableAdminRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableAdminRepository.cs @@ -31,25 +31,47 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage // --- Global Admins --- public async Task IsGlobalAdminAsync(string upn) + { + // Delegate to the role resolver so the GlobalAdmin/GlobalReader distinction is enforced at the + // repository contract too: an enabled row with Role=GlobalReader is NOT a Global Admin. (Resolving + // this only in GlobalAdminService would leave a direct repo caller as a silent privilege-escalation + // footgun — a GlobalReader row reading back as "admin".) + return await GetGlobalRoleAsync(upn) == Constants.GlobalRoles.GlobalAdmin; + } + + public async Task GetGlobalRoleAsync(string upn) { if (string.IsNullOrWhiteSpace(upn)) - return false; + return null; try { var normalizedUpn = upn.ToLowerInvariant(); var entity = await _globalAdminsTableClient.GetEntityAsync( "GlobalAdmins", normalizedUpn); - return entity?.Value != null && entity.Value.IsEnabled; + if (entity?.Value == null || !entity.Value.IsEnabled) + return null; + + // Empty/missing Role on an existing enabled row ⇒ GlobalAdmin (back-compat). An + // unrecognized Role string is treated as no role (fail-closed) rather than silently + // granting GlobalAdmin. + var role = entity.Value.Role; + if (string.IsNullOrWhiteSpace(role)) + return Constants.GlobalRoles.GlobalAdmin; + if (role == Constants.GlobalRoles.GlobalAdmin || role == Constants.GlobalRoles.GlobalReader) + return role; + + _logger.LogWarning("Unrecognized global Role '{Role}' for {Upn} — treating as no role", role, upn); + return null; } catch (RequestFailedException ex) when (ex.Status == 404) { - return false; + return null; } catch (Exception ex) { - _logger.LogError(ex, "Error checking global admin status for {Upn}", upn); - return false; + _logger.LogError(ex, "Error resolving global role for {Upn}", upn); + return null; } } @@ -64,7 +86,8 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage Upn = entity.Upn, IsEnabled = entity.IsEnabled, AddedAt = entity.AddedDate, - AddedBy = entity.AddedBy + AddedBy = entity.AddedBy, + Role = string.IsNullOrWhiteSpace(entity.Role) ? Constants.GlobalRoles.GlobalAdmin : entity.Role }); } return admins; diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetAdminConfigurationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetAdminConfigurationFunction.cs index 4110b2ca..00abb7dc 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetAdminConfigurationFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetAdminConfigurationFunction.cs @@ -28,16 +28,20 @@ namespace AutopilotMonitor.Functions.Functions.Config { try { - // Authentication + GlobalAdminOnly authorization enforced by PolicyEnforcementMiddleware - string userIdentifier = TenantHelper.GetUserIdentifier(req); + // Authentication + GlobalReadOrAdmin authorization enforced by PolicyEnforcementMiddleware + var requestCtx = req.GetRequestContext(); + string userIdentifier = requestCtx.UserPrincipalName; - _logger.LogInformation($"GetAdminConfiguration by Global Admin user {userIdentifier}"); + _logger.LogInformation("GetAdminConfiguration by {User} (role={Role})", userIdentifier, requestCtx.UserRole); var config = await _adminConfigService.GetConfigurationAsync(); - // SECURITY NOTE: Returns full AdminConfiguration including NvdApiKey. - // Acceptable because this endpoint is GlobalAdminOnly (single user during preview). - // Consider masking NvdApiKey to a write-only sentinel before GA release. + // A read-only GlobalReader gets secrets (NvdApiKey, SAS, ops webhook URLs) redacted; a + // Global Admin gets the full config. Redaction returns a copy — the cached instance is + // never mutated. + if (requestCtx.IsGlobalReader) + config = config.RedactedCopyForReader(); + var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(config); return response; diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetAllTenantConfigurationsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetAllTenantConfigurationsFunction.cs index 4e9eb04f..e5a437ae 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetAllTenantConfigurationsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetAllTenantConfigurationsFunction.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Net; using System.Threading.Tasks; using System.Web; @@ -47,9 +48,15 @@ namespace AutopilotMonitor.Functions.Functions.Config // Web consumers (tenant selectors + admin config editor) depend on this shape. if (parsed.PageSize == null) { - _logger.LogInformation($"GetAllTenantConfigurations (full) by Global Admin {userIdentifier}"); + _logger.LogInformation($"GetAllTenantConfigurations (full) by {userIdentifier}"); var configurations = await _configService.GetAllConfigurationsAsync(); + // A read-only GlobalReader gets per-tenant secrets (SAS / webhook URLs / custom + // headers) redacted; a Global Admin gets the full configs. The paginated mode below + // is already a secret-stripped keep-list projection, so it needs no role branch. + if (req.GetRequestContext().IsGlobalReader) + configurations = configurations.Select(c => c.RedactedCopyForReader()).ToList(); + var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(configurations); return response; diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetTenantConfigurationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetTenantConfigurationFunction.cs index f4b7b45f..002962d4 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetTenantConfigurationFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/GetTenantConfigurationFunction.cs @@ -37,9 +37,19 @@ namespace AutopilotMonitor.Functions.Functions.Config var config = await _configService.GetConfigurationAsync(requestCtx.TargetTenantId); - // NOTE: Returns full TenantConfiguration including DiagnosticsBlobSasUrl. - // Intentional — tenant admins need to view/manage the SAS URL in the Settings UI. - // Access restricted to TenantAdminOrGA by PolicyEnforcementMiddleware. + // Tenant admins (own tenant) and Global Admins get the full config including + // DiagnosticsBlobSasUrl / webhook URLs / custom headers — they manage those in the + // Settings UI. Secrets are redacted ONLY for the read-only-reader view (returns a copy; + // the cached instance is never mutated). ADDITIVE: a GlobalReader who is ALSO this + // tenant's own Admin is acting as the tenant admin here, so they keep the FULL config — + // redaction applies only when they are NOT the own-tenant admin (i.e. pure reader, or a + // reader viewing a DIFFERENT tenant cross-tenant). This also prevents a "***REDACTED***" + // placeholder from ever reaching the Settings save round-trip for an own-tenant admin. + var ownTenantAdminView = requestCtx.IsTenantAdmin + && string.Equals(requestCtx.TargetTenantId, requestCtx.TenantId, StringComparison.OrdinalIgnoreCase); + if (requestCtx.IsGlobalReader && !ownTenantAdminView) + config = config.RedactedCopyForReader(); + var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(config); return response; diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs index 8f03aa78..6576584c 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Config/UpdateTenantConfigurationFunction.cs @@ -63,6 +63,16 @@ namespace AutopilotMonitor.Functions.Functions.Config return badRequest; } + // Load the stored config up-front so we can (1) restore any redacted secret placeholders + // before validation/save and (2) protect GA-only fields below. + var existingConfig = await _configService.GetConfigurationAsync(requestCtx.TargetTenantId); + + // Defense-in-depth: a read-only GlobalReader is served a redacted config (secrets replaced + // with the ***REDACTED*** sentinel). If such a view is ever round-tripped back on a save, + // never persist the placeholder — restore the real secret from the stored config. (Own-tenant + // admins are served the FULL config so this is normally a no-op for them.) + config.RestoreRedactedSecretsFrom(existingConfig); + // Validate webhook URLs (SSRF protection) var webhookUrlError = SsrfGuard.ValidateWebhookUrlFormat(config.WebhookUrl); if (webhookUrlError != null) @@ -109,8 +119,7 @@ namespace AutopilotMonitor.Functions.Functions.Config // Set the actual user identifier for audit logging config.UpdatedBy = userIdentifier; - // Protect GA-only fields from non-Global-Admin callers - var existingConfig = await _configService.GetConfigurationAsync(requestCtx.TargetTenantId); + // Protect GA-only fields from non-Global-Admin callers (existingConfig loaded above). if (!requestCtx.IsGlobalAdmin) { if (config.AllowInsecureAgentRequests != existingConfig.AllowInsecureAgentRequests || diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs index 766186ca..e8d79f95 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/AuthFunction.cs @@ -86,17 +86,19 @@ public class AuthFunction // Fail-fast: if any fetch throws, AggregateException propagates → Azure Functions returns 500. // This is intentional — all 6 queries are required for the auth decision. var tenantConfigTask = _tenantConfigService.GetConfigurationAsync(tenantId); - var isGlobalAdminTask = _globalAdminService.IsGlobalAdminAsync(upn); + var globalRoleTask = _globalAdminService.GetGlobalRoleAsync(upn); var isApprovedTask = _previewWhitelistService.IsApprovedAsync(tenantId); var membershipTask = _tenantAdminsService.GetTableMembershipAsync(tenantId, upn); var mcpCheckTask = _mcpUserService.IsAllowedAsync(upn); var existingAdminsTask = _tenantAdminsService.GetTenantAdminsAsync(tenantId); - await Task.WhenAll(tenantConfigTask, isGlobalAdminTask, isApprovedTask, + await Task.WhenAll(tenantConfigTask, globalRoleTask, isApprovedTask, membershipTask, mcpCheckTask, existingAdminsTask); var tenantConfig = tenantConfigTask.Result; - var isGlobalAdmin = isGlobalAdminTask.Result; + var globalRole = globalRoleTask.Result; + var isGlobalAdmin = globalRole == Constants.GlobalRoles.GlobalAdmin; + var isGlobalReader = globalRole == Constants.GlobalRoles.GlobalReader; var isApproved = isApprovedTask.Result; var (tableState, tableRole) = membershipTask.Result; var mcpCheck = mcpCheckTask.Result; @@ -114,7 +116,7 @@ public class AuthFunction // --- Pure decision logic (tested by AuthFunctionTests) --- var decision = BuildAuthResult( - tenantConfig, isGlobalAdmin, isApproved, + tenantConfig, isGlobalAdmin, isGlobalReader, isApproved, memberRole, mcpCheck, existingAdmins.Count > 0, tenantId, upn, displayName ?? string.Empty, objectId ?? string.Empty); @@ -339,6 +341,7 @@ public class AuthFunction internal static AuthDecisionResult BuildAuthResult( TenantConfiguration tenantConfig, bool isGlobalAdmin, + bool isGlobalReader, bool isPreviewApproved, MemberRoleInfo? memberRole, McpAccessCheckResult mcpCheck, @@ -359,8 +362,8 @@ public class AuthFunction }); } - // Gate 2: Preview gate (Global Admins bypass) - if (!isGlobalAdmin && !isPreviewApproved) + // Gate 2: Preview gate (platform roles — GlobalAdmin and read-only GlobalReader — bypass) + if (!isGlobalAdmin && !isGlobalReader && !isPreviewApproved) { return AuthDecisionResult.Blocked(HttpStatusCode.Forbidden, new { @@ -372,8 +375,14 @@ public class AuthFunction // Determine admin status: auto-admin if first user (no existing admins and no role yet). // needsAutoAdmin keys off the absence of ANY effective role (table or claim) so a // claim-derived Admin/Operator in a claim-only tenant is NOT written into the table. + // GlobalReader semantics are ADDITIVE (the role adds cross-tenant read; it never removes a + // user's independent tenant-role write rights). Auto-admin, however, is not an existing right — + // it SILENTLY grants write on first login. We decline that for a read-only-flagged identity: + // it removes nothing, and a GlobalReader who should also administer their tenant can be added + // to TenantAdmins explicitly. (A pure GlobalReader stays read-only everywhere via the write + // evaluators denying a roleless caller.) bool isTenantAdmin = memberRole?.Role == Constants.TenantRoles.Admin; - bool needsAutoAdmin = memberRole == null && !hasTenantAdmins; + bool needsAutoAdmin = memberRole == null && !hasTenantAdmins && !isGlobalReader; if (needsAutoAdmin) { isTenantAdmin = true; @@ -391,6 +400,7 @@ public class AuthFunction displayName, objectId, isGlobalAdmin, + isGlobalReader, isTenantAdmin, role, canManageBootstrapTokens, diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/McpUserFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/McpUserFunction.cs index 13f5150b..a0d05010 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/McpUserFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/McpUserFunction.cs @@ -173,14 +173,18 @@ public class McpUserFunction ["accessGrant"] = result.AccessGrant, ["reason"] = result.Reason, }; - // Only surface the Global-Admin flag when it is actually true. A normal - // tenant user gets no isGlobalAdmin field at all: the MCP access-guard - // reads `=== true` (absent → non-GA), and we avoid hinting to ordinary - // callers that a Global-Admin tier even exists. + // Only surface platform-role flags when the caller actually has one. A normal tenant user gets + // neither field: the MCP access-guard reads `globalRole` to decide cross-tenant routing (and + // keeps reading `isGlobalAdmin === true` for back-compat / write-tier hints), and we avoid + // hinting to ordinary callers that a platform tier even exists. if (result.IsGlobalAdmin) { payload["isGlobalAdmin"] = true; } + if (!string.IsNullOrEmpty(result.GlobalRole)) + { + payload["globalRole"] = result.GlobalRole; // "GlobalAdmin" | "GlobalReader" + } await response.WriteAsJsonAsync(payload); return response; } diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/SignalRAddToGroupFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/SignalRAddToGroupFunction.cs index 27be6969..13f6a2d6 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/SignalRAddToGroupFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/SignalRAddToGroupFunction.cs @@ -54,17 +54,20 @@ namespace AutopilotMonitor.Functions.Functions.Infrastructure // Group names are in format: "tenant-{tenantId}", "session-{tenantId}-{sessionId}", or "global-admins" // Users can only join groups for their own tenant (unless they are Global Admin) - // Explicit validation for the global-admins group + // Explicit validation for the global-admins group. This is a READ broadcast group + // (cross-tenant new-session/new-event + global-notification live pushes), so any platform + // scope — Global Admin OR the read-only Global Reader — may join. Mutating actions remain + // gated elsewhere (the reader only RECEIVES pushes). if (request.GroupName == "global-admins") { - if (!requestCtx.IsGlobalAdmin) + if (!requestCtx.HasGlobalScope) { - _logger.LogWarning($"User {userEmail} (tenant {userTenantId}) attempted to join global-admins group without being a Global Admin"); + _logger.LogWarning($"User {userEmail} (tenant {userTenantId}) attempted to join global-admins group without platform scope"); var forbiddenResponse = req.CreateResponse(HttpStatusCode.Forbidden); - await forbiddenResponse.WriteAsJsonAsync(new { success = false, message = "Access denied: Only Global Admins can join this group" }); + await forbiddenResponse.WriteAsJsonAsync(new { success = false, message = "Access denied: platform scope (Global Admin or Global Reader) required to join this group" }); return new AddToGroupOutput { HttpResponse = forbiddenResponse }; } - _logger.LogInformation($"Global Admin {userEmail} joining global-admins group"); + _logger.LogInformation($"Platform-scope user {userEmail} (role={requestCtx.UserRole}) joining global-admins group"); } else { @@ -77,11 +80,13 @@ namespace AutopilotMonitor.Functions.Functions.Infrastructure return new AddToGroupOutput { HttpResponse = badRequestResponse }; } - // Check if user is allowed to join this tenant's group + // Check if user is allowed to join this tenant's group. Cross-tenant joins require + // platform scope (Global Admin OR read-only Global Reader) — both have cross-tenant + // READ scope, and group membership only RECEIVES live updates. The admin/member + // notification-group checks below still gate by the real tenant role. if (requestedTenantId != userTenantId) { - // Check if user is Global Admin (they can join any tenant's group) - if (!requestCtx.IsGlobalAdmin) + if (!requestCtx.HasGlobalScope) { _logger.LogWarning($"User {userEmail} (tenant {userTenantId}) attempted to join group for tenant {requestedTenantId}"); var forbiddenResponse = req.CreateResponse(HttpStatusCode.Forbidden); @@ -90,7 +95,7 @@ namespace AutopilotMonitor.Functions.Functions.Infrastructure } else { - _logger.LogInformation($"Global Admin {userEmail} joining cross-tenant group: {request.GroupName}"); + _logger.LogInformation($"Platform-scope user {userEmail} (role={requestCtx.UserRole}) joining cross-tenant group: {request.GroupName}"); } } diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/SignalRRemoveFromGroupFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/SignalRRemoveFromGroupFunction.cs index 9979c5e4..6dbfcbd7 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/SignalRRemoveFromGroupFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Infrastructure/SignalRRemoveFromGroupFunction.cs @@ -54,17 +54,18 @@ namespace AutopilotMonitor.Functions.Functions.Infrastructure // Group names are in format: "tenant-{tenantId}", "session-{tenantId}-{sessionId}", or "global-admins" // Users can only leave groups for their own tenant (unless they are Global Admin) - // Explicit validation for the global-admins group + // Explicit validation for the global-admins group — symmetric with AddToGroup: any + // platform scope (Global Admin OR read-only Global Reader) that could join may also leave. if (request.GroupName == "global-admins") { - if (!requestCtx.IsGlobalAdmin) + if (!requestCtx.HasGlobalScope) { - _logger.LogWarning($"User {userEmail} (tenant {userTenantId}) attempted to leave global-admins group without being a Global Admin"); + _logger.LogWarning($"User {userEmail} (tenant {userTenantId}) attempted to leave global-admins group without platform scope"); var forbiddenResponse = req.CreateResponse(HttpStatusCode.Forbidden); - await forbiddenResponse.WriteAsJsonAsync(new { success = false, message = "Access denied: Only Global Admins can leave this group" }); + await forbiddenResponse.WriteAsJsonAsync(new { success = false, message = "Access denied: platform scope (Global Admin or Global Reader) required for this group" }); return new RemoveFromGroupOutput { HttpResponse = forbiddenResponse }; } - _logger.LogInformation($"Global Admin {userEmail} leaving global-admins group"); + _logger.LogInformation($"Platform-scope user {userEmail} (role={requestCtx.UserRole}) leaving global-admins group"); } else { @@ -77,11 +78,11 @@ namespace AutopilotMonitor.Functions.Functions.Infrastructure return new RemoveFromGroupOutput { HttpResponse = badRequestResponse }; } - // Check if user is allowed to leave this tenant's group + // Check if user is allowed to leave this tenant's group — platform scope (GA or + // read-only Global Reader) may leave any tenant's group, symmetric with AddToGroup. if (requestedTenantId != userTenantId) { - // Check if user is Global Admin (they can leave any tenant's group) - if (!requestCtx.IsGlobalAdmin) + if (!requestCtx.HasGlobalScope) { _logger.LogWarning($"User {userEmail} (tenant {userTenantId}) attempted to leave group for tenant {requestedTenantId}"); var forbiddenResponse = req.CreateResponse(HttpStatusCode.Forbidden); @@ -90,7 +91,7 @@ namespace AutopilotMonitor.Functions.Functions.Infrastructure } else { - _logger.LogInformation($"Global Admin {userEmail} leaving cross-tenant group: {request.GroupName}"); + _logger.LogInformation($"Platform-scope user {userEmail} (role={requestCtx.UserRole}) leaving cross-tenant group: {request.GroupName}"); } } diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetImeVersionHistoryFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetImeVersionHistoryFunction.cs index a7ed2b5b..1dd0dbe3 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetImeVersionHistoryFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetImeVersionHistoryFunction.cs @@ -35,7 +35,7 @@ namespace AutopilotMonitor.Functions.Functions.Metrics var response = req.CreateResponse(HttpStatusCode.OK); - if (requestCtx.IsGlobalAdmin) + if (requestCtx.HasGlobalScope) { await response.WriteAsJsonAsync(versions); } diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/McpUsageMetricsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/McpUsageMetricsFunction.cs index ee6ef382..bb6b5c97 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/McpUsageMetricsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Metrics/McpUsageMetricsFunction.cs @@ -99,7 +99,7 @@ namespace AutopilotMonitor.Functions.Functions.Metrics var records = await _userUsageRepo.GetUsageByUserAsync(userId, dateFrom, dateTo); var ctx = req.GetRequestContext(); - if (UsageCrossTenantGuard.IsForeignTenantAccess(records, ctx.TenantId, ctx.IsGlobalAdmin)) + if (UsageCrossTenantGuard.IsForeignTenantAccess(records, ctx.TenantId, ctx.HasGlobalScope)) { var foundTenants = records .Select(r => r.TenantId) diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GetRuleResultsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GetRuleResultsFunction.cs index 87c7ee9f..8453bebc 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GetRuleResultsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Rules/GetRuleResultsFunction.cs @@ -44,9 +44,9 @@ namespace AutopilotMonitor.Functions.Functions.Rules var requestCtx = req.GetRequestContext(); var effectiveTenantId = requestCtx.TargetTenantId; - // Global Admin cross-tenant fallback: resolve actual tenant upfront - // (needed for both read and write paths) - if (requestCtx.IsGlobalAdmin) + // Global-scope (GA or read-only GlobalReader) cross-tenant fallback: resolve actual tenant + // upfront so the read works cross-tenant for any platform-scope caller. + if (requestCtx.HasGlobalScope) { var resolvedTenantId = await _sessionRepo.FindSessionTenantIdAsync(sessionId); if (resolvedTenantId != null && !string.Equals(resolvedTenantId, effectiveTenantId, StringComparison.OrdinalIgnoreCase)) diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionDecisionGraphFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionDecisionGraphFunction.cs index 723c629a..f2fc458f 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionDecisionGraphFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionDecisionGraphFunction.cs @@ -56,9 +56,9 @@ namespace AutopilotMonitor.Functions.Functions.Sessions var transitions = await _transitionRepo.QueryBySessionAsync( requestCtx.TargetTenantId, sessionId, MaxTransitionsToLoad); - // Global Admin cross-tenant fallback (same pattern as other session-detail reads). + // Global-scope (GA or read-only GlobalReader) cross-tenant fallback (same pattern as other session-detail reads). var effectiveTenantId = requestCtx.TargetTenantId; - if (transitions.Count == 0 && requestCtx.IsGlobalAdmin) + if (transitions.Count == 0 && requestCtx.HasGlobalScope) { var resolvedTenantId = await _sessionRepo.FindSessionTenantIdAsync(sessionId); if (resolvedTenantId != null && diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionDeletePreviewFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionDeletePreviewFunction.cs index 23cc986c..835afd5a 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionDeletePreviewFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionDeletePreviewFunction.cs @@ -87,7 +87,7 @@ namespace AutopilotMonitor.Functions.Functions.Sessions // "global" sentinel; preserved so existing callers without ?tenantId still work. var explicitTenantId = query["tenantId"]; string tenantId; - if (requestCtx.IsGlobalAdmin + if (requestCtx.HasGlobalScope && !string.IsNullOrEmpty(explicitTenantId) && Guid.TryParse(explicitTenantId, out _)) { diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionEventsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionEventsFunction.cs index 7d4ecef0..dedda5ed 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionEventsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionEventsFunction.cs @@ -101,7 +101,7 @@ namespace AutopilotMonitor.Functions.Functions.Sessions // Legacy unpaginated path — full list, no nextLink. var events = await _sessionRepo.GetSessionEventsAsync(requestCtx.TargetTenantId, sessionId); - if (events.Count == 0 && requestCtx.IsGlobalAdmin) + if (events.Count == 0 && requestCtx.HasGlobalScope) { var resolvedTenantId = await _sessionRepo.FindSessionTenantIdAsync(sessionId); if (resolvedTenantId != null && !string.Equals(resolvedTenantId, requestCtx.TargetTenantId, StringComparison.OrdinalIgnoreCase)) @@ -131,15 +131,15 @@ namespace AutopilotMonitor.Functions.Functions.Sessions // Paginated path. Continuation token binds (tenantId, sessionId), so the // tenantId on every page must match the tenant the token was issued for. // - // For Global Admin cross-tenant, the nextLink we emit echoes that tenant - // back as ?tenantId=, so follow-up pages can re-bind to it. On page 1, if - // GA hasn't passed tenantId yet, we resolve it from the session lookup. + // For a global-scope caller (GA or read-only GlobalReader) cross-tenant, the nextLink we + // emit echoes that tenant back as ?tenantId=, so follow-up pages can re-bind to it. On + // page 1, if no tenantId was passed yet, we resolve it from the session lookup. // - // For non-GA, ?tenantId= in the URL is ignored — middleware-bound JWT + // For non-global callers, ?tenantId= in the URL is ignored — middleware-bound JWT // tenant is authoritative; we must never let a query param override it. var effectiveTenantId = requestCtx.TargetTenantId; - if (requestCtx.IsGlobalAdmin) + if (requestCtx.HasGlobalScope) { if (!string.IsNullOrEmpty(tenantIdQueryParam)) { diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionFunction.cs index 0c94c230..82462eb0 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionFunction.cs @@ -46,9 +46,9 @@ namespace AutopilotMonitor.Functions.Functions.Sessions var session = await _sessionRepo.GetSessionAsync(requestCtx.TargetTenantId, sessionId); - // Global Admin cross-tenant fallback: if not found in the effective tenant, - // try resolving the actual tenant via SessionsIndex - if (session == null && requestCtx.IsGlobalAdmin) + // Global-scope (GA or read-only GlobalReader) cross-tenant fallback: if not found in the + // effective tenant, try resolving the actual tenant via SessionsIndex + if (session == null && requestCtx.HasGlobalScope) { var resolvedTenantId = await _sessionRepo.FindSessionTenantIdAsync(sessionId); if (resolvedTenantId != null && !string.Equals(resolvedTenantId, requestCtx.TargetTenantId, StringComparison.OrdinalIgnoreCase)) diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionReducerVerificationFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionReducerVerificationFunction.cs index 50073439..8b5bdaf6 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionReducerVerificationFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionReducerVerificationFunction.cs @@ -76,11 +76,11 @@ namespace AutopilotMonitor.Functions.Functions.Sessions var transitions = await _transitionRepo.QueryBySessionAsync( tenantIdForLoad, sessionId, MaxTransitionsToLoad); - // Global Admin cross-tenant fallback — same pattern as the other session - // detail read endpoints. Resolve via SessionsIndex when the effective tenant - // has no data. (GlobalAdminOnly gate means we don't need to check IsGlobalAdmin - // again, but the SessionsIndex lookup is still the cleanest way to find the - // actual tenant for a loose sessionId.) + // Global-scope cross-tenant fallback — same pattern as the other session detail read + // endpoints. Resolve via SessionsIndex when the effective tenant has no data. (The + // GlobalReadOrAdmin gate already restricts this route to platform-scope callers — GA or + // read-only GlobalReader — so we don't re-check the role here; the SessionsIndex lookup + // is the cleanest way to find the actual tenant for a loose sessionId.) if (signals.Count == 0 && transitions.Count == 0) { var resolvedTenantId = await _sessionRepo.FindSessionTenantIdAsync(sessionId); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionSignalsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionSignalsFunction.cs index 68b45d88..b3167102 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionSignalsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/GetSessionSignalsFunction.cs @@ -62,8 +62,8 @@ namespace AutopilotMonitor.Functions.Functions.Sessions var signals = await _signalRepo.QueryBySessionAsync( requestCtx.TargetTenantId, sessionId, maxResults); - // Global Admin cross-tenant fallback — mirror GetSessionEventsFunction pattern. - if (signals.Count == 0 && requestCtx.IsGlobalAdmin) + // Global-scope (GA or read-only GlobalReader) cross-tenant fallback — mirror GetSessionEventsFunction pattern. + if (signals.Count == 0 && requestCtx.HasGlobalScope) { var resolvedTenantId = await _sessionRepo.FindSessionTenantIdAsync(sessionId); if (resolvedTenantId != null && diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/QuickSearchSessionsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/QuickSearchSessionsFunction.cs index 17f3eb42..5c975e22 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/QuickSearchSessionsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Sessions/QuickSearchSessionsFunction.cs @@ -52,8 +52,9 @@ public class QuickSearchSessionsFunction return await req.BadRequestAsync("Query parameter 'q' must be at least 2 characters."); } - // Global Admins search across all tenants; regular users are scoped to their own tenant - string? tenantId = requestCtx.IsGlobalAdmin ? null : requestCtx.TenantId; + // Global-scope callers (GA or read-only GlobalReader) search across all tenants; + // regular users are scoped to their own tenant. + string? tenantId = requestCtx.HasGlobalScope ? null : requestCtx.TenantId; var results = await _sessionRepo.QuickSearchSessionsAsync(tenantId, q, limit: 10); diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/GetVulnerabilityReportFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/GetVulnerabilityReportFunction.cs index e9066292..2593832f 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/GetVulnerabilityReportFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Vulnerability/GetVulnerabilityReportFunction.cs @@ -51,9 +51,9 @@ namespace AutopilotMonitor.Functions.Functions.Vulnerability var userIdentifier = requestCtx.UserPrincipalName; var effectiveTenantId = requestCtx.TargetTenantId; - // Global Admin cross-tenant fallback: resolve actual tenant upfront - // (needed for both read and write paths) - if (requestCtx.IsGlobalAdmin) + // Global-scope (GA or read-only GlobalReader) cross-tenant fallback: resolve actual tenant + // upfront so the read works cross-tenant for any platform-scope caller. + if (requestCtx.HasGlobalScope) { var resolvedTenantId = await _sessionRepo.FindSessionTenantIdAsync(sessionId); if (resolvedTenantId != null && !string.Equals(resolvedTenantId, effectiveTenantId, StringComparison.OrdinalIgnoreCase)) diff --git a/src/Backend/AutopilotMonitor.Functions/Helpers/RequestContext.cs b/src/Backend/AutopilotMonitor.Functions/Helpers/RequestContext.cs index b29767f6..655fdfc4 100644 --- a/src/Backend/AutopilotMonitor.Functions/Helpers/RequestContext.cs +++ b/src/Backend/AutopilotMonitor.Functions/Helpers/RequestContext.cs @@ -19,9 +19,25 @@ public sealed record RequestContext /// The user's UPN (from JWT upn/preferred_username claim). public string UserPrincipalName { get; init; } = string.Empty; - /// True if the user is a Global Admin of the platform. + /// True if the user is a Global Admin of the platform (full read + write). public bool IsGlobalAdmin { get; init; } + /// + /// True if the user holds the Global Reader platform role: cross-tenant read visibility equal to a + /// Global Admin but conferring no platform write/settings capability. Mutually exclusive with + /// . Semantics are ADDITIVE — this role grants read and never removes a + /// user's independent tenant-role rights, so a UPN that is both a Global Reader and a Tenant Admin of + /// its own tenant still passes the tenant-scoped write tiers there (the write evaluators resolve the + /// tenant role after the GA check). A pure Global Reader (no tenant role) is read-only everywhere. + /// + public bool IsGlobalReader { get; init; } + + /// + /// True when the caller has platform-wide (cross-tenant) read scope — Global Admin OR Global Reader. + /// Use for read/visibility decisions and cross-tenant gating; use for writes. + /// + public bool HasGlobalScope => IsGlobalAdmin || IsGlobalReader; + /// True if the user is a Tenant Admin of their own tenant. public bool IsTenantAdmin { get; init; } @@ -67,7 +83,7 @@ public static class RequestContextExtensions /// the role is not resolved and this returns false. /// public static bool IsTenantMemberOrGlobalAdmin(this RequestContext ctx) - => ctx.IsGlobalAdmin + => ctx.HasGlobalScope || ctx.UserRole == Constants.TenantRoles.Admin || ctx.UserRole == Constants.TenantRoles.Operator || ctx.UserRole == Constants.TenantRoles.Viewer; diff --git a/src/Backend/AutopilotMonitor.Functions/Middleware/PolicyEnforcementMiddleware.cs b/src/Backend/AutopilotMonitor.Functions/Middleware/PolicyEnforcementMiddleware.cs index af0a49ac..69767d8c 100644 --- a/src/Backend/AutopilotMonitor.Functions/Middleware/PolicyEnforcementMiddleware.cs +++ b/src/Backend/AutopilotMonitor.Functions/Middleware/PolicyEnforcementMiddleware.cs @@ -52,31 +52,24 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware var requestPath = httpContext.Request.Path.Value ?? string.Empty; var httpMethod = httpContext.Request.Method; + var queryTenantId = httpContext.Request.Query["tenantId"].FirstOrDefault(); - // Look up the catalog policy for this route - var catalogEntry = EndpointAccessPolicyCatalog.FindPolicy(httpMethod, requestPath); - - if (catalogEntry == null) + // Authenticated principal set by AuthenticationMiddleware (null on anonymous/device routes). + // Read from FunctionContext.Items (reliable in the isolated worker) rather than httpContext.User. + ClaimsPrincipal? principal = null; + if (context.Items.TryGetValue("ClaimsPrincipal", out var principalObj) + && principalObj is ClaimsPrincipal cp + && cp.Identity?.IsAuthenticated == true) { - // Fail-closed: unregistered route → 403 - _logger.LogError("[PolicyEnforcement] BLOCKED unregistered route: {Method} {Path} — not in catalog (fail-closed)", - httpMethod, requestPath); - httpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden; - httpContext.Response.ContentType = "application/json"; - await httpContext.Response.WriteAsJsonAsync(new - { - error = "Forbidden", - message = "Access denied." - }); - return; + principal = cp; } - // Evaluate the catalog policy for the current user. + // All authorization + RequestContext construction is the transport-agnostic DecideAsync seam. // Fail-closed on service errors: return 503 (not 500) so clients can retry. - CatalogDecisionResult decision; + PolicyResult result; try { - decision = await EvaluateCatalogPolicyAsync(context, catalogEntry); + result = await DecideAsync(httpMethod, requestPath, queryTenantId, principal); } catch (Exception ex) { @@ -92,125 +85,127 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware return; } - if (decision.IsAllowed) + if (result.Allowed) { - // Store resolved context so functions can read IsGlobalAdmin/IsTenantAdmin without re-querying services - var principal = context.GetUser(); - var jwtTenantId = principal?.GetTenantId() ?? string.Empty; - var isGlobalAdmin = decision.UserRole == "GlobalAdmin"; - var targetTenantId = jwtTenantId; // default: JWT tenant - - // Cross-tenant enforcement for routes with {tenantId} in the URL - if (catalogEntry.TenantScoping == TenantScoping.RouteParam) - { - var normalizedPath = requestPath.StartsWith("/api/", StringComparison.OrdinalIgnoreCase) - ? requestPath.Substring(5) - : requestPath; - var match = catalogEntry.RouteRegex.Match(normalizedPath); - var routeTenantId = match.Groups["tenantId"].Value; - - if (!string.IsNullOrEmpty(routeTenantId)) - { - targetTenantId = routeTenantId; - - if (!isGlobalAdmin && !string.Equals(routeTenantId, jwtTenantId, StringComparison.OrdinalIgnoreCase)) - { - _logger.LogWarning("[PolicyEnforcement] BLOCKED cross-tenant access: user={User} jwtTenant={JwtTenant} routeTenant={RouteTenant} path={Path}", - decision.UserIdentifier, jwtTenantId, routeTenantId, requestPath); - httpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden; - httpContext.Response.ContentType = "application/json"; - await httpContext.Response.WriteAsJsonAsync(new - { - error = "CrossTenantAccessDenied", - message = "Access denied. You can only access your own tenant's resources." - }); - return; - } - } - } - - // Cross-tenant enforcement for routes with optional ?tenantId= query parameter - if (catalogEntry.TenantScoping == TenantScoping.QueryParam) - { - var queryTenantId = httpContext.Request.Query["tenantId"].FirstOrDefault(); - if (!string.IsNullOrWhiteSpace(queryTenantId)) - { - targetTenantId = queryTenantId; - - if (!isGlobalAdmin && !string.Equals(queryTenantId, jwtTenantId, StringComparison.OrdinalIgnoreCase)) - { - _logger.LogWarning("[PolicyEnforcement] BLOCKED cross-tenant access (query): user={User} jwtTenant={JwtTenant} queryTenant={QueryTenant} path={Path}", - decision.UserIdentifier, jwtTenantId, queryTenantId, requestPath); - httpContext.Response.StatusCode = (int)HttpStatusCode.Forbidden; - httpContext.Response.ContentType = "application/json"; - await httpContext.Response.WriteAsJsonAsync(new - { - error = "CrossTenantAccessDenied", - message = "Access denied. You can only access your own tenant's resources." - }); - return; - } - } - } - - context.Items[RequestContext.ItemsKey] = new RequestContext - { - TenantId = jwtTenantId, - TargetTenantId = targetTenantId, - UserPrincipalName = decision.UserIdentifier, - IsGlobalAdmin = isGlobalAdmin, - IsTenantAdmin = decision.UserRole == Constants.TenantRoles.Admin, - UserRole = decision.UserRole - }; - - _logger.LogDebug("[PolicyEnforcement] ALLOW {Method} {Path} policy={Policy} user={User} role={Role}", - httpMethod, requestPath, catalogEntry.Policy, decision.UserIdentifier, decision.UserRole); + context.Items[RequestContext.ItemsKey] = result.Context!; + _logger.LogDebug("[PolicyEnforcement] ALLOW {Method} {Path} user={User} role={Role}", + httpMethod, requestPath, result.UserIdentifier, result.UserRole); await next(context); return; } - // Denied — determine 401 vs 403 - var statusCode = decision.Reason is "NoJWT" or "MissingClaims" - ? HttpStatusCode.Unauthorized - : HttpStatusCode.Forbidden; - - _logger.LogWarning("[PolicyEnforcement] DENIED {Method} {Path} policy={Policy} status={Status} user={User} role={Role} reason={Reason}", - httpMethod, requestPath, catalogEntry.Policy, (int)statusCode, - decision.UserIdentifier, decision.UserRole, decision.Reason); - - httpContext.Response.StatusCode = (int)statusCode; + _logger.LogWarning("[PolicyEnforcement] DENIED {Method} {Path} status={Status} user={User} role={Role} reason={Reason}", + httpMethod, requestPath, result.StatusCode, result.UserIdentifier, result.UserRole, result.LogReason); + httpContext.Response.StatusCode = result.StatusCode; httpContext.Response.ContentType = "application/json"; + await httpContext.Response.WriteAsJsonAsync(new { error = result.ErrorCode, message = result.ErrorMessage }); + } - if (statusCode == HttpStatusCode.Unauthorized) + /// + /// Transport-agnostic authorization decision: catalog lookup → policy evaluation → cross-tenant + /// enforcement → RequestContext construction. Returns an allow (carrying the resolved RequestContext) + /// or a deny (carrying the HTTP status + error payload). Extracted from so the + /// full decision — including the additive tenant-role resolution that drives RequestContext.IsTenantAdmin — + /// is unit-testable without faking the worker's HttpContext feature. is a thin + /// adapter that reads the request, calls this, and writes the response. + /// + internal async Task DecideAsync( + string httpMethod, string requestPath, string? queryTenantId, ClaimsPrincipal? principal) + { + var catalogEntry = EndpointAccessPolicyCatalog.FindPolicy(httpMethod, requestPath); + if (catalogEntry == null) { - await httpContext.Response.WriteAsJsonAsync(new - { - error = "AuthenticationRequired", - message = "Authentication required. Please provide a valid JWT token." - }); + // Fail-closed: unregistered route → 403 + _logger.LogError("[PolicyEnforcement] BLOCKED unregistered route: {Method} {Path} — not in catalog (fail-closed)", + httpMethod, requestPath); + return PolicyResult.Deny((int)HttpStatusCode.Forbidden, "Forbidden", "Access denied.", "anonymous", "N/A", "Unregistered"); } - else + + var decision = await EvaluateCatalogPolicyAsync(principal, catalogEntry); + + if (!decision.IsAllowed) { - await httpContext.Response.WriteAsJsonAsync(new - { - error = "InsufficientPermissions", - message = "Access denied. You do not have permission to access this resource." - }); + var status = decision.Reason is "NoJWT" or "MissingClaims" + ? HttpStatusCode.Unauthorized + : HttpStatusCode.Forbidden; + var (code, message) = status == HttpStatusCode.Unauthorized + ? ("AuthenticationRequired", "Authentication required. Please provide a valid JWT token.") + : ("InsufficientPermissions", "Access denied. You do not have permission to access this resource."); + return PolicyResult.Deny((int)status, code, message, decision.UserIdentifier, decision.UserRole, decision.Reason); } + + var jwtTenantId = principal?.GetTenantId() ?? string.Empty; + var isGlobalAdmin = decision.UserRole == Constants.GlobalRoles.GlobalAdmin; + var isGlobalReader = decision.UserRole == Constants.GlobalRoles.GlobalReader; + // Platform-wide read scope (GA or Reader) governs cross-tenant visibility; write power is GA only. + var hasGlobalScope = isGlobalAdmin || isGlobalReader; + var targetTenantId = jwtTenantId; // default: JWT tenant + + // Cross-tenant enforcement for routes with {tenantId} in the URL + if (catalogEntry.TenantScoping == TenantScoping.RouteParam) + { + var normalizedPath = requestPath.StartsWith("/api/", StringComparison.OrdinalIgnoreCase) + ? requestPath.Substring(5) + : requestPath; + var match = catalogEntry.RouteRegex.Match(normalizedPath); + var routeTenantId = match.Groups["tenantId"].Value; + + if (!string.IsNullOrEmpty(routeTenantId)) + { + targetTenantId = routeTenantId; + + if (!hasGlobalScope && !string.Equals(routeTenantId, jwtTenantId, StringComparison.OrdinalIgnoreCase)) + { + _logger.LogWarning("[PolicyEnforcement] BLOCKED cross-tenant access: user={User} jwtTenant={JwtTenant} routeTenant={RouteTenant} path={Path}", + decision.UserIdentifier, jwtTenantId, routeTenantId, requestPath); + return PolicyResult.Deny((int)HttpStatusCode.Forbidden, "CrossTenantAccessDenied", + "Access denied. You can only access your own tenant's resources.", + decision.UserIdentifier, decision.UserRole, "CrossTenantRoute"); + } + } + } + + // Cross-tenant enforcement for routes with optional ?tenantId= query parameter + if (catalogEntry.TenantScoping == TenantScoping.QueryParam) + { + if (!string.IsNullOrWhiteSpace(queryTenantId)) + { + targetTenantId = queryTenantId; + + if (!hasGlobalScope && !string.Equals(queryTenantId, jwtTenantId, StringComparison.OrdinalIgnoreCase)) + { + _logger.LogWarning("[PolicyEnforcement] BLOCKED cross-tenant access (query): user={User} jwtTenant={JwtTenant} queryTenant={QueryTenant} path={Path}", + decision.UserIdentifier, jwtTenantId, queryTenantId, requestPath); + return PolicyResult.Deny((int)HttpStatusCode.Forbidden, "CrossTenantAccessDenied", + "Access denied. You can only access your own tenant's resources.", + decision.UserIdentifier, decision.UserRole, "CrossTenantQuery"); + } + } + } + + var requestContext = new RequestContext + { + TenantId = jwtTenantId, + TargetTenantId = targetTenantId, + UserPrincipalName = decision.UserIdentifier, + IsGlobalAdmin = isGlobalAdmin, + IsGlobalReader = isGlobalReader, + // Additive: a caller's tenant-admin status is its OWN-TENANT role, independent of any + // platform role. Evaluators that admit via a platform-role branch still surface the + // resolved tenant role on decision.TenantRole; pure-tenant branches leave it null and + // carry the role in UserRole — so fall back to UserRole when TenantRole is unset. + IsTenantAdmin = (decision.TenantRole ?? decision.UserRole) == Constants.TenantRoles.Admin, + UserRole = decision.UserRole + }; + + return PolicyResult.Allow(requestContext, decision.UserIdentifier, decision.UserRole); } private async Task EvaluateCatalogPolicyAsync( - FunctionContext context, EndpointPolicyEntry entry) + ClaimsPrincipal? principal, EndpointPolicyEntry entry) { - // Get the ClaimsPrincipal set by AuthenticationMiddleware (may be null for anonymous routes) - ClaimsPrincipal? principal = null; - if (context.Items.TryGetValue("ClaimsPrincipal", out var principalObj) - && principalObj is ClaimsPrincipal cp - && cp.Identity?.IsAuthenticated == true) - { - principal = cp; - } - + // principal is the authenticated ClaimsPrincipal (set by AuthenticationMiddleware), or null on + // anonymous/device routes — resolved by the caller (Invoke / DecideAsync). var tenantId = principal?.GetTenantId(); var upn = principal?.GetUserPrincipalName(); var userIdentifier = upn ?? "anonymous"; @@ -234,12 +229,18 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware case EndpointPolicy.MemberRead: return await EvaluateMemberReadAsync(tenantId, upn, principal, userIdentifier); + case EndpointPolicy.TenantAdminOrGlobalReader: + return await EvaluateTenantAdminOrGlobalReaderAsync(tenantId, upn, principal, userIdentifier); + case EndpointPolicy.TenantAdminOrGA: return await EvaluateTenantAdminOrGAAsync(tenantId, upn, principal, userIdentifier); case EndpointPolicy.BootstrapManagerOrGA: return await EvaluateBootstrapManagerOrGAAsync(tenantId, upn, principal, userIdentifier); + case EndpointPolicy.GlobalReadOrAdmin: + return await EvaluateGlobalReadOrAdminAsync(upn, userIdentifier); + case EndpointPolicy.GlobalAdminOnly: return await EvaluateGlobalAdminOnlyAsync(upn, userIdentifier); @@ -261,9 +262,22 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware if (principal == null) return CatalogDecisionResult.Deny(userIdentifier, "N/A", "NoJWT"); - // Global Admin outranks any tenant role. - if (!string.IsNullOrEmpty(upn) && await _globalAdminService.IsGlobalAdminAsync(upn)) - return CatalogDecisionResult.Allow(userIdentifier, "GlobalAdmin", "ValidJWT+GA"); + // A platform role (GlobalAdmin or read-only GlobalReader) is surfaced as the primary role so + // per-resource gates inside the function can tell GA from Reader (e.g. cross-tenant SignalR + // group joins). ADDITIVE: we ALSO resolve the caller's own-tenant role and carry it separately, + // so a user who is both GlobalReader and TenantAdmin still has IsTenantAdmin=true in the context + // (admin notification group, admin-audience notifications). + if (!string.IsNullOrEmpty(upn)) + { + var globalRole = await _globalAdminService.GetGlobalRoleAsync(upn); + if (globalRole != null) + { + var tenantRole = !string.IsNullOrEmpty(tenantId) + ? (await ResolveEffectiveRoleAsync(tenantId, upn, principal))?.Role + : null; + return CatalogDecisionResult.Allow(userIdentifier, globalRole, "ValidJWT+GlobalScope", tenantRole); + } + } // Resolve the effective tenant role (may be null for non-members). A resolved role is // surfaced so the function can distinguish members from roleless end users; either way @@ -272,7 +286,7 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware { var role = await ResolveEffectiveRoleAsync(tenantId, upn, principal); if (role?.Role != null) - return CatalogDecisionResult.Allow(userIdentifier, role.Role, "ValidJWT+Member"); + return CatalogDecisionResult.Allow(userIdentifier, role.Role, "ValidJWT+Member", role.Role); } // Authenticated but not a member and not a GA — still allowed (the function decides which @@ -286,15 +300,50 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware if (string.IsNullOrEmpty(tenantId) || string.IsNullOrEmpty(upn)) return CatalogDecisionResult.Deny(userIdentifier, "N/A", "MissingClaims"); - if (await _globalAdminService.IsGlobalAdminAsync(upn)) - return CatalogDecisionResult.Allow(userIdentifier, "GlobalAdmin", "GABypass"); + // Any platform role (GlobalAdmin or read-only GlobalReader) satisfies a member-read. ADDITIVE: + // also resolve the own-tenant role and carry it separately so IsTenantAdmin stays correct for a + // GlobalReader who is also their tenant's admin (drives admin-audience on /api/notifications). + var globalRole = await _globalAdminService.GetGlobalRoleAsync(upn); + if (globalRole != null) + { + var tenantRole = (await ResolveEffectiveRoleAsync(tenantId, upn, principal))?.Role; + return CatalogDecisionResult.Allow(userIdentifier, globalRole, "GlobalScopeBypass", tenantRole); + } var role = await ResolveEffectiveRoleAsync(tenantId, upn, principal); if (role == null) return CatalogDecisionResult.Deny(userIdentifier, "NonMember", "NotInTenant"); // MemberRead allows Admin, Operator, AND Viewer - return CatalogDecisionResult.Allow(userIdentifier, role.Role, "TenantMember"); + return CatalogDecisionResult.Allow(userIdentifier, role.Role, "TenantMember", role.Role); + } + + /// + /// Admin-tier tenant READ: own-tenant Admin, OR any platform role (GlobalAdmin / GlobalReader). + /// Used on GET routes that expose admin-tier tenant data (e.g. config) so the read-only GlobalReader + /// gains visibility without write power, while tenant Operators/Viewers remain excluded. ADDITIVE: + /// when admitted via the platform-role branch the caller's own-tenant role is ALSO resolved and + /// carried, so a GlobalReader who is also their tenant's Admin keeps IsTenantAdmin=true (config GET + /// then serves them the FULL config for their own tenant, redacting only the read-only-reader view). + /// + private async Task EvaluateTenantAdminOrGlobalReaderAsync( + string? tenantId, string? upn, ClaimsPrincipal? principal, string userIdentifier) + { + if (string.IsNullOrEmpty(tenantId) || string.IsNullOrEmpty(upn)) + return CatalogDecisionResult.Deny(userIdentifier, "N/A", "MissingClaims"); + + var globalRole = await _globalAdminService.GetGlobalRoleAsync(upn); + if (globalRole != null) + { + var tenantRole = (await ResolveEffectiveRoleAsync(tenantId, upn, principal))?.Role; + return CatalogDecisionResult.Allow(userIdentifier, globalRole, "GlobalScopeBypass", tenantRole); + } + + var role = await ResolveEffectiveRoleAsync(tenantId, upn, principal); + if (role?.Role == Constants.TenantRoles.Admin) + return CatalogDecisionResult.Allow(userIdentifier, Constants.TenantRoles.Admin, "TenantAdmin", Constants.TenantRoles.Admin); + + return CatalogDecisionResult.Deny(userIdentifier, role?.Role ?? "NonMember", "NotAdminOrGlobalScope"); } private async Task EvaluateTenantAdminOrGAAsync( @@ -357,6 +406,24 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware return EntraAppRoleResolver.Resolve(state, tableRole, principal.GetAppRoles(), config.EntraAppRolesEnabled); } + /// + /// Platform-wide cross-tenant READ: admits GlobalAdmin and the read-only GlobalReader. The + /// resolved role string flows onto RequestContext so the function still distinguishes GA from + /// Reader (e.g. for any in-function write gate). + /// + private async Task EvaluateGlobalReadOrAdminAsync( + string? upn, string userIdentifier) + { + if (string.IsNullOrEmpty(upn)) + return CatalogDecisionResult.Deny(userIdentifier, "N/A", "MissingClaims"); + + var globalRole = await _globalAdminService.GetGlobalRoleAsync(upn); + if (globalRole != null) + return CatalogDecisionResult.Allow(userIdentifier, globalRole, "GlobalScope"); + + return CatalogDecisionResult.Deny(userIdentifier, "NonGlobal", "NotGlobalScope"); + } + private async Task EvaluateGlobalAdminOnlyAsync( string? upn, string userIdentifier) { @@ -364,11 +431,44 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware return CatalogDecisionResult.Deny(userIdentifier, "N/A", "MissingClaims"); if (await _globalAdminService.IsGlobalAdminAsync(upn)) - return CatalogDecisionResult.Allow(userIdentifier, "GlobalAdmin", "IsGA"); + return CatalogDecisionResult.Allow(userIdentifier, Constants.GlobalRoles.GlobalAdmin, "IsGA"); return CatalogDecisionResult.Deny(userIdentifier, "NonGA", "NotGlobalAdmin"); } + /// + /// Outcome of : either an allow carrying the resolved + /// , or a deny carrying the HTTP status + error payload that + /// writes. Internal so the middleware decision is unit-testable. + /// + internal sealed class PolicyResult + { + public bool Allowed { get; private init; } + public RequestContext? Context { get; private init; } + public int StatusCode { get; private init; } + public string? ErrorCode { get; private init; } + public string? ErrorMessage { get; private init; } + public string UserIdentifier { get; private init; } = "anonymous"; + public string UserRole { get; private init; } = "N/A"; + public string LogReason { get; private init; } = ""; + + public static PolicyResult Allow(RequestContext context, string user, string role) + => new() { Allowed = true, Context = context, UserIdentifier = user, UserRole = role, LogReason = "Allow" }; + + public static PolicyResult Deny(int statusCode, string errorCode, string errorMessage, + string user, string role, string logReason) + => new() + { + Allowed = false, + StatusCode = statusCode, + ErrorCode = errorCode, + ErrorMessage = errorMessage, + UserIdentifier = user, + UserRole = role, + LogReason = logReason, + }; + } + /// /// Result of evaluating a catalog policy against the current request context. /// @@ -379,8 +479,17 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware public string UserRole { get; private init; } = "N/A"; public string Reason { get; private init; } = ""; - public static CatalogDecisionResult Allow(string user, string role, string reason) - => new() { IsAllowed = true, UserIdentifier = user, UserRole = role, Reason = reason }; + /// + /// The caller's independently-resolved tenant role ("Admin"/"Operator"/"Viewer"), when known. + /// Carried SEPARATELY from so additive semantics survive: a caller admitted + /// via a platform-role branch (e.g. GlobalReader) still surfaces their own-tenant role here, so + /// RequestContext.IsTenantAdmin is correct for a user who is both GlobalReader and TenantAdmin. + /// Null when not resolved (the allow-path then falls back to ). + /// + public string? TenantRole { get; private init; } + + public static CatalogDecisionResult Allow(string user, string role, string reason, string? tenantRole = null) + => new() { IsAllowed = true, UserIdentifier = user, UserRole = role, Reason = reason, TenantRole = tenantRole }; public static CatalogDecisionResult Deny(string user, string role, string reason) => new() { IsAllowed = false, UserIdentifier = user, UserRole = role, Reason = reason }; diff --git a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs index 61bc43e0..16f25d0a 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs @@ -49,16 +49,36 @@ public enum EndpointPolicy /// AuthenticatedUserWithRole, - /// Tenant member with Admin, Operator, or Viewer role. Tenant-scoped read access. + /// + /// Tenant member with Admin, Operator, or Viewer role, OR any platform role (Global Admin / + /// Global Reader). Tenant-scoped read access. + /// MemberRead, + /// + /// Own-tenant Admin, OR a platform role (Global Admin / Global Reader). Tenant-scoped admin-tier + /// READ access (e.g. GET config/{tenantId}). Like but additionally + /// admits the read-only Global Reader — used only on GET routes so the reader gets visibility + /// without write power, while tenant Operators/Viewers still cannot read admin-tier config. + /// + TenantAdminOrGlobalReader, + /// Tenant Admin or Global Admin. Tenant-scoped write access. TenantAdminOrGA, /// Admin (always) or Operator with CanManageBootstrapTokens permission, or Global Admin. BootstrapManagerOrGA, - /// Global Admin only. Platform-wide access. + /// + /// Global Admin OR Global Reader. Platform-wide cross-tenant READ access (sessions, metrics, audit, + /// config read, reports, …). The read-only counterpart to : every + /// mutating/destructive global route stays GlobalAdminOnly. NOTE: raw table/log access + /// (global/raw/tables, global/raw/logs) is deliberately NOT on this tier — it stays GlobalAdminOnly + /// because it can dump secret-bearing tables, which would bypass the GlobalReader config redaction. + /// + GlobalReadOrAdmin, + + /// Global Admin only. Platform-wide access (all writes + global settings). GlobalAdminOnly, } @@ -133,9 +153,10 @@ public static class EndpointAccessPolicyCatalog new("POST", "realtime/negotiate", EndpointPolicy.AuthenticatedUser), // Group join/leave is AuthenticatedUserWithRole — NOT MemberRead — on purpose. The real, // per-group authorization lives inside SignalRAddToGroup/RemoveFromGroupFunction: it - // requires the requested group's tenant to match the caller's JWT tenant (Global Admins - // excepted), gates the global-admins group on IsGlobalAdmin, the admin-tier notification - // group on IsTenantAdmin, and the member-tier notification group on tenant membership. + // requires the requested group's tenant to match the caller's JWT tenant (platform-scope + // callers — GA or read-only Global Reader — excepted), gates the global-admins read-broadcast + // group on HasGlobalScope (GA ∪ GlobalReader), the admin-tier notification group on + // IsTenantAdmin (or GA), and the member-tier notification group on tenant membership (or scope). // The Progress Portal is an END-USER feature: employees tracking their own device are // authenticated (JWT tid = their tenant) but are typically NOT tenant members (no // TenantAdmins row, no Entra app-role). Gating join at MemberRead let them load the @@ -181,7 +202,7 @@ public static class EndpointAccessPolicyCatalog new("GET", "metrics/vulnerability", EndpointPolicy.MemberRead), new("GET", "metrics/software-inventory", EndpointPolicy.MemberRead), new("GET", "metrics/geographic", EndpointPolicy.MemberRead), - new("GET", "metrics/mcp-usage/user/{userId}", EndpointPolicy.TenantAdminOrGA), + new("GET", "metrics/mcp-usage/user/{userId}", EndpointPolicy.TenantAdminOrGlobalReader), new("GET", "metrics/geographic/sessions", EndpointPolicy.MemberRead), new("GET", "metrics/ime-versions", EndpointPolicy.MemberRead), new("GET", "audit/logs", EndpointPolicy.MemberRead), @@ -191,7 +212,7 @@ public static class EndpointAccessPolicyCatalog new("GET", "rules/gather", EndpointPolicy.MemberRead), new("GET", "rules/analyze", EndpointPolicy.MemberRead), new("GET", "rules/ime-log-patterns", EndpointPolicy.MemberRead), - new("GET", "config/{tenantId}", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), + new("GET", "config/{tenantId}", EndpointPolicy.TenantAdminOrGlobalReader, TenantScoping.RouteParam), new("GET", "config/{tenantId}/feature-flags", EndpointPolicy.MemberRead, TenantScoping.RouteParam), // ── TenantAdminOrGA ───────────────────────────────────────────── @@ -234,7 +255,7 @@ public static class EndpointAccessPolicyCatalog // POST (not GET) so payloads with 100+ script refs comfortably fit in the request // body without hitting URL-length limits in browsers / Azure Functions front door. new("POST", "tenants/{tenantId}/scripts/display-names", EndpointPolicy.MemberRead, TenantScoping.RouteParam), - new("GET", "tenants/{tenantId}/graph-permissions/status", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), + new("GET", "tenants/{tenantId}/graph-permissions/status", EndpointPolicy.TenantAdminOrGlobalReader, TenantScoping.RouteParam), new("POST", "tenants/{tenantId}/graph-permissions/refresh", EndpointPolicy.TenantAdminOrGA, TenantScoping.RouteParam), // ── BootstrapManagerOrGA ──────────────────────────────────────── @@ -248,83 +269,91 @@ public static class EndpointAccessPolicyCatalog // ── MCP Usage (self-service) ────────────────────────────────── new("GET", "metrics/mcp-usage/me", EndpointPolicy.AuthenticatedUser), - // ── GlobalAdminOnly ──────────────────────────────────────────── - // Inspector v1 endpoints (Plan §M6 — temporarily GlobalAdminOnly while the UI - // matures; lift to MemberRead+QueryParam scoping at the v2 adminMode lift). - // Cross-tenant resolution for GAs happens inside the functions via SessionsIndex, - // so TenantScoping.None on the catalog is fine. - new("GET", "sessions/{sessionId}/signals", EndpointPolicy.GlobalAdminOnly), - new("GET", "sessions/{sessionId}/decision-graph", EndpointPolicy.GlobalAdminOnly), - new("GET", "sessions/{sessionId}/reducer-verification", EndpointPolicy.GlobalAdminOnly), + // ── GlobalReadOrAdmin + GlobalAdminOnly ───────────────────────── + // Cross-tenant routes. Curated READS (GET) are GlobalReadOrAdmin so the read-only Global Reader + // tier sees them; every mutating/destructive route stays GlobalAdminOnly. Exception in the OTHER + // direction: the raw table/log endpoints (global/raw/tables[/{tableName}], global/raw/logs) are + // reads but stay GlobalAdminOnly — they can dump secret-bearing tables and would bypass the + // GlobalReader config-secret redaction. + // + // Inspector v1 endpoints (Plan §M6 — lift to MemberRead+QueryParam scoping at the v2 adminMode + // lift). Cross-tenant resolution for global-scope callers happens inside the functions via + // SessionsIndex, so TenantScoping.None on the catalog is fine. + new("GET", "sessions/{sessionId}/signals", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "sessions/{sessionId}/decision-graph", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "sessions/{sessionId}/reducer-verification", EndpointPolicy.GlobalReadOrAdmin), // Cascade-delete admin endpoints live under /api/global/* (not /api/admin/*) because the // latter prefix collides with the Azure Functions runtime's own admin routes, which are // mTLS-gated — browsers triggering preflight on /api/admin/* get TLS-renegotiation - // failures instead of CORS responses. Keep all cross-tenant GA-only HTTP routes under + // failures instead of CORS responses. Keep all cross-tenant global-scope HTTP routes under // /api/global/. - new("GET", "global/sessions/{sessionId}/delete/preview", EndpointPolicy.GlobalAdminOnly), + new("GET", "global/sessions/{sessionId}/delete/preview", EndpointPolicy.GlobalReadOrAdmin), new("POST", "global/sessions/{sessionId}/restore", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/sessions/{sessionId}/deletion-manifest", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/tenants/{tenantId}/deletion-manifests", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), - new("GET", "global/session-deletions", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/raw/sessions", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/raw/events", EndpointPolicy.GlobalAdminOnly), + new("GET", "global/sessions/{sessionId}/deletion-manifest", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/tenants/{tenantId}/deletion-manifests", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.RouteParam), + new("GET", "global/session-deletions", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/raw/sessions", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/raw/events", EndpointPolicy.GlobalReadOrAdmin), + // Raw table/log access stays GlobalAdminOnly: query_table can dump arbitrary tables + // (incl. TenantConfiguration secrets), which would bypass the GlobalReader config-secret + // redaction. A read-only GlobalReader uses the curated read endpoints instead. new("GET", "global/raw/tables", EndpointPolicy.GlobalAdminOnly), new("GET", "global/raw/tables/{tableName}", EndpointPolicy.GlobalAdminOnly), new("POST", "global/raw/logs", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/search/sessions", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/search/sessions-by-event", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/search/sessions-by-cve", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/summary", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/config", EndpointPolicy.GlobalAdminOnly), + new("GET", "global/search/sessions", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/search/sessions-by-event", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/search/sessions-by-cve", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/summary", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/config", EndpointPolicy.GlobalReadOrAdmin), new("PUT", "global/config", EndpointPolicy.GlobalAdminOnly), new("POST", "global/config", EndpointPolicy.GlobalAdminOnly), - new("GET", "config/all", EndpointPolicy.GlobalAdminOnly), - new("GET", "auth/global-admins", EndpointPolicy.GlobalAdminOnly), + new("GET", "config/all", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "auth/global-admins", EndpointPolicy.GlobalReadOrAdmin), new("POST", "auth/global-admins", EndpointPolicy.GlobalAdminOnly), new("DELETE", "auth/global-admins/{upn}", EndpointPolicy.GlobalAdminOnly), - new("GET", "preview/whitelist", EndpointPolicy.GlobalAdminOnly), + new("GET", "preview/whitelist", EndpointPolicy.GlobalReadOrAdmin), new("POST", "preview/whitelist/{tenantId}", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), new("DELETE", "preview/whitelist/{tenantId}", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), - new("GET", "preview/notification-email/{tenantId}", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), + new("GET", "preview/notification-email/{tenantId}", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.RouteParam), new("POST", "preview/send-welcome-email/{tenantId}", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), - new("GET", "global/sessions", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/stats/sessions", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/audit/logs", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/tenants-with-deletion-manifests", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/platform", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/app", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/fleet-health", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/apps/list", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/apps/{appName}/analytics", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/apps/{appName}/sessions", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/geographic", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/geographic/sessions", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/usage", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/sla", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/rule-stats", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/vulnerability", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/mcp-usage", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/metrics/mcp-usage/daily", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/distress-reports", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/session-reports", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/session-reports/download-url", EndpointPolicy.GlobalAdminOnly), + new("GET", "global/sessions", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/stats/sessions", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/audit/logs", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/tenants-with-deletion-manifests", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/platform", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/app", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/fleet-health", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/apps/list", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/apps/{appName}/analytics", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/apps/{appName}/sessions", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/geographic", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/geographic/sessions", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/usage", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/sla", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/rule-stats", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/vulnerability", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/mcp-usage", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/metrics/mcp-usage/daily", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/distress-reports", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/session-reports", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/session-reports/download-url", EndpointPolicy.GlobalReadOrAdmin), new("PATCH", "global/session-reports/{reportId}/note", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/rules/gather", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/rules/analyze", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/devices/blocked", EndpointPolicy.GlobalAdminOnly), - new("GET", "devices/blocked", EndpointPolicy.GlobalAdminOnly), + new("GET", "global/rules/gather", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/rules/analyze", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/devices/blocked", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "devices/blocked", EndpointPolicy.GlobalReadOrAdmin), new("POST", "devices/block", EndpointPolicy.GlobalAdminOnly), new("DELETE", "devices/block/{encodedSerialNumber}", EndpointPolicy.GlobalAdminOnly), - new("GET", "versions/blocked", EndpointPolicy.GlobalAdminOnly), + new("GET", "versions/blocked", EndpointPolicy.GlobalReadOrAdmin), new("POST", "versions/block", EndpointPolicy.GlobalAdminOnly), new("DELETE", "versions/block/{encodedPattern}", EndpointPolicy.GlobalAdminOnly), new("POST", "maintenance/trigger", EndpointPolicy.GlobalAdminOnly), // ── Critical-Table Backup (plan §PR1+§PR2) ───────────────────────────── new("POST", "global/backups/trigger", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/backups", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/backups/{backupId}", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/backups/jobs/{jobId}", EndpointPolicy.GlobalAdminOnly), + new("GET", "global/backups", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/backups/{backupId}", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/backups/jobs/{jobId}", EndpointPolicy.GlobalReadOrAdmin), // PR2: single-row preview + commit. PartitionKey + RowKey live in the body // (Azure Tables permits / + % in PK/RK; the route wouldn't carry them safely). new("POST", "global/backups/{backupId}/restore-row", EndpointPolicy.GlobalAdminOnly), @@ -335,38 +364,38 @@ public static class EndpointAccessPolicyCatalog // The {tenantId} route param is the OFFBOARDED tenant whose data was archived; // the operator (GA) does not belong to that tenant. RouteParam scoping declares // that the route's tenantId is supplied from the URL, not from the caller's JWT. - new("GET", "global/customs-archive", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/customs-archive/{tenantId}/{historyRowKey}", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), - new("GET", "global/customs-archive/{tenantId}/{historyRowKey}/{archiveRowKey}", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), + new("GET", "global/customs-archive", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/customs-archive/{tenantId}/{historyRowKey}", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.RouteParam), + new("GET", "global/customs-archive/{tenantId}/{historyRowKey}/{archiveRowKey}", EndpointPolicy.GlobalReadOrAdmin, TenantScoping.RouteParam), new("DELETE", "global/customs-archive/{tenantId}/{historyRowKey}/{archiveRowKey}", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), new("DELETE", "global/customs-archive/{tenantId}/{historyRowKey}", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), new("GET", "health/detailed", EndpointPolicy.AuthenticatedUser), new("GET", "health/mcp", EndpointPolicy.AuthenticatedUser), new("POST", "rules/reseed-from-github", EndpointPolicy.GlobalAdminOnly), - new("GET", "vulnerability/unmatched-software", EndpointPolicy.GlobalAdminOnly), - new("GET", "vulnerability/software-inventory", EndpointPolicy.GlobalAdminOnly), + new("GET", "vulnerability/unmatched-software", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "vulnerability/software-inventory", EndpointPolicy.GlobalReadOrAdmin), new("POST", "vulnerability/sync", EndpointPolicy.GlobalAdminOnly), new("POST", "vulnerability/sync-msrc", EndpointPolicy.GlobalAdminOnly), - new("GET", "vulnerability/sync-status", EndpointPolicy.GlobalAdminOnly), + new("GET", "vulnerability/sync-status", EndpointPolicy.GlobalReadOrAdmin), new("POST", "vulnerability/cpe-mapping", EndpointPolicy.GlobalAdminOnly), new("POST", "vulnerability/cpe-mapping/auto-resolve", EndpointPolicy.GlobalAdminOnly), new("DELETE", "vulnerability/cpe-mapping", EndpointPolicy.GlobalAdminOnly), - new("GET", "vulnerability/cpe-mappings", EndpointPolicy.GlobalAdminOnly), - new("GET", "vulnerability/ignored-software", EndpointPolicy.GlobalAdminOnly), + new("GET", "vulnerability/cpe-mappings", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "vulnerability/ignored-software", EndpointPolicy.GlobalReadOrAdmin), new("POST", "vulnerability/ignored-software", EndpointPolicy.GlobalAdminOnly), new("DELETE", "vulnerability/ignored-software", EndpointPolicy.GlobalAdminOnly), new("POST", "rules/ime-log-patterns/reseed", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/mcp-users", EndpointPolicy.GlobalAdminOnly), + new("GET", "global/mcp-users", EndpointPolicy.GlobalReadOrAdmin), new("POST", "global/mcp-users", EndpointPolicy.GlobalAdminOnly), new("DELETE", "global/mcp-users/{upn}", EndpointPolicy.GlobalAdminOnly), new("PATCH", "global/mcp-users/{upn}/enable", EndpointPolicy.GlobalAdminOnly), new("PATCH", "global/mcp-users/{upn}/disable", EndpointPolicy.GlobalAdminOnly), new("PATCH", "global/mcp-users/{upn}/usage-plan", EndpointPolicy.GlobalAdminOnly), new("PATCH", "config/{tenantId}/plan", EndpointPolicy.GlobalAdminOnly, TenantScoping.RouteParam), - new("GET", "global/config/plan-tiers", EndpointPolicy.GlobalAdminOnly), + new("GET", "global/config/plan-tiers", EndpointPolicy.GlobalReadOrAdmin), new("PUT", "global/config/plan-tiers", EndpointPolicy.GlobalAdminOnly), - new("GET", "feedback/all", EndpointPolicy.GlobalAdminOnly), - new("GET", "global/notifications", EndpointPolicy.GlobalAdminOnly), + new("GET", "feedback/all", EndpointPolicy.GlobalReadOrAdmin), + new("GET", "global/notifications", EndpointPolicy.GlobalReadOrAdmin), new("POST", "global/notifications/dismiss-all", EndpointPolicy.GlobalAdminOnly), new("POST", "global/notifications/{notificationId}/dismiss", EndpointPolicy.GlobalAdminOnly), // Tenant-scoped persistent notifications (bell). TenantId comes from JWT — middleware enforces. @@ -378,7 +407,7 @@ public static class EndpointAccessPolicyCatalog new("GET", "notifications", EndpointPolicy.MemberRead, TenantScoping.Jwt), new("POST", "notifications/dismiss-all", EndpointPolicy.TenantAdminOrGA, TenantScoping.Jwt), new("POST", "notifications/{notificationId}/dismiss", EndpointPolicy.TenantAdminOrGA, TenantScoping.Jwt), - new("GET", "global/ops-events", EndpointPolicy.GlobalAdminOnly), + new("GET", "global/ops-events", EndpointPolicy.GlobalReadOrAdmin), }; /// diff --git a/src/Backend/AutopilotMonitor.Functions/Services/GlobalAdminService.cs b/src/Backend/AutopilotMonitor.Functions/Services/GlobalAdminService.cs index dfa41102..2ad55b84 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/GlobalAdminService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/GlobalAdminService.cs @@ -1,3 +1,4 @@ +using AutopilotMonitor.Shared; using AutopilotMonitor.Shared.DataAccess; using Azure.Data.Tables; using Microsoft.Extensions.Caching.Memory; @@ -16,6 +17,10 @@ public class GlobalAdminService private readonly ILogger _logger; private readonly TimeSpan _cacheDuration = TimeSpan.FromMinutes(5); + // Sentinel stored in the role cache to represent "no global role" (row missing or disabled). + // Lets us distinguish a cached negative from a cache miss without nullable boxing games. + private const string NoRoleSentinel = "(none)"; + public GlobalAdminService( IAdminRepository adminRepo, IMemoryCache cache, @@ -33,34 +38,42 @@ public class GlobalAdminService /// User Principal Name (e.g., oliver@contoso.com) /// True if the user is a Global Admin public virtual async Task IsGlobalAdminAsync(string? upn) + { + // Single source of truth: GlobalAdmin == the GlobalAdmin platform role. + return await GetGlobalRoleAsync(upn) == Constants.GlobalRoles.GlobalAdmin; + } + + /// + /// Resolves the caller's platform role from the GlobalAdmins table. + /// Returns , , + /// or null when the UPN has no enabled GlobalAdmins row. Cached for 5 minutes. + /// + public virtual async Task GetGlobalRoleAsync(string? upn) { if (string.IsNullOrWhiteSpace(upn)) { - _logger.LogDebug("IsGlobalAdminAsync: UPN is null or empty"); - return false; + _logger.LogDebug("GetGlobalRoleAsync: UPN is null or empty"); + return null; } // Normalize UPN to lowercase for case-insensitive comparison upn = upn.ToLowerInvariant(); - // Check cache first - var cacheKey = $"global-admin:{upn}"; - if (_cache.TryGetValue(cacheKey, out var isAdmin)) + var cacheKey = $"global-role:{upn}"; + if (_cache.TryGetValue(cacheKey, out var cached) && cached != null) { - _logger.LogDebug("Global Admin check (from cache): {Upn} -> {IsAdmin}", upn, isAdmin); - return isAdmin; + _logger.LogDebug("Global role check (from cache): {Upn} -> {Role}", upn, cached); + return cached == NoRoleSentinel ? null : cached; } - // Query via repository - _logger.LogDebug("Querying repository for Global Admin: {Upn}", upn); - var result = await _adminRepo.IsGlobalAdminAsync(upn); + _logger.LogDebug("Querying repository for global role: {Upn}", upn); + var role = await _adminRepo.GetGlobalRoleAsync(upn); - _logger.LogDebug("Global Admin check result: {Upn} -> {Result}", upn, result); + _logger.LogDebug("Global role check result: {Upn} -> {Role}", upn, role ?? "(none)"); - // Cache the result - _cache.Set(cacheKey, result, _cacheDuration); + _cache.Set(cacheKey, role ?? NoRoleSentinel, _cacheDuration); - return result; + return role; } /// @@ -76,7 +89,7 @@ public class GlobalAdminService await _adminRepo.AddGlobalAdminAsync(upn, addedBy); // Invalidate cache - _cache.Remove($"global-admin:{upn}"); + _cache.Remove($"global-role:{upn}"); return new GlobalAdminEntity { @@ -85,7 +98,8 @@ public class GlobalAdminService Upn = upn, IsEnabled = true, AddedDate = DateTime.UtcNow, - AddedBy = addedBy + AddedBy = addedBy, + Role = Constants.GlobalRoles.GlobalAdmin }; } @@ -99,7 +113,7 @@ public class GlobalAdminService await _adminRepo.RemoveGlobalAdminAsync(upn); // Invalidate cache - _cache.Remove($"global-admin:{upn}"); + _cache.Remove($"global-role:{upn}"); } /// @@ -112,7 +126,7 @@ public class GlobalAdminService await _adminRepo.DisableGlobalAdminAsync(upn); // Invalidate cache - _cache.Remove($"global-admin:{upn}"); + _cache.Remove($"global-role:{upn}"); } /// @@ -129,7 +143,8 @@ public class GlobalAdminService Upn = e.Upn, IsEnabled = e.IsEnabled, AddedDate = e.AddedAt, - AddedBy = e.AddedBy + AddedBy = e.AddedBy, + Role = string.IsNullOrEmpty(e.Role) ? Constants.GlobalRoles.GlobalAdmin : e.Role }).ToList(); } @@ -174,4 +189,11 @@ public class GlobalAdminEntity : ITableEntity /// UPN of the admin who added this user /// public string AddedBy { get; set; } = string.Empty; + + /// + /// Platform role for this entry: (default) or + /// . Empty/missing ⇒ GlobalAdmin (back-compat with + /// rows created before the GlobalReader tier existed). + /// + public string Role { get; set; } = string.Empty; } diff --git a/src/Backend/AutopilotMonitor.Functions/Services/McpUserService.cs b/src/Backend/AutopilotMonitor.Functions/Services/McpUserService.cs index dc83afe4..ae19dc1c 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/McpUserService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/McpUserService.cs @@ -1,3 +1,4 @@ +using AutopilotMonitor.Shared; using AutopilotMonitor.Shared.DataAccess; using AutopilotMonitor.Shared.Models; using Azure.Data.Tables; @@ -46,10 +47,13 @@ public class McpUserService upn = upn.ToLowerInvariant(); - // Always resolve GA status — needed by the MCP server for cross-tenant routing - // decisions (GA → /api/global/* with tenantId-as-filter; non-GA → /api/* JWT-bound). - // Resolved unconditionally so downstream consumers don't need to re-check. - var isGlobalAdmin = await _globalAdminService.IsGlobalAdminAsync(upn); + // Always resolve the platform role — needed by the MCP server for cross-tenant routing + // decisions (global scope → /api/global/* with tenantId-as-filter; non-global → /api/* JWT-bound). + // GlobalReader has the SAME cross-tenant read scope as GA in the (read-only) MCP, so both + // platform roles route the same way; only isGlobalAdmin (write power) differs. Resolved + // unconditionally so downstream consumers don't need to re-check. + var globalRole = await _globalAdminService.GetGlobalRoleAsync(upn); + var isGlobalAdmin = globalRole == Constants.GlobalRoles.GlobalAdmin; var config = await _adminConfigService.GetConfigurationAsync(); if (!Enum.TryParse(config.McpAccessPolicy, out var policy)) @@ -61,13 +65,13 @@ public class McpUserService return McpAccessCheckResult.Denied("MCP access is disabled"); case McpAccessPolicy.AllMembers: - return McpAccessCheckResult.Allowed(upn, "AllMembers", isGlobalAdmin); + return McpAccessCheckResult.Allowed(upn, "AllMembers", isGlobalAdmin, globalRole); case McpAccessPolicy.WhitelistOnly: default: - // Global Admins always have access - if (isGlobalAdmin) - return McpAccessCheckResult.Allowed(upn, "GlobalAdmin", true); + // Any platform role (GlobalAdmin or read-only GlobalReader) always has MCP access. + if (globalRole != null) + return McpAccessCheckResult.Allowed(upn, globalRole, isGlobalAdmin, globalRole); // Check McpUsers whitelist (cached) var cacheKey = $"mcp-user:{upn}"; @@ -160,12 +164,19 @@ public class McpAccessCheckResult public string AccessGrant { get; init; } = string.Empty; public bool IsGlobalAdmin { get; init; } - public static McpAccessCheckResult Allowed(string upn, string accessGrant, bool isGlobalAdmin = false) => new() + /// + /// Platform role of the caller: "GlobalAdmin", "GlobalReader", or null (no platform role). The MCP + /// server uses this to grant cross-tenant routing to BOTH platform roles (the server is read-only). + /// + public string? GlobalRole { get; init; } + + public static McpAccessCheckResult Allowed(string upn, string accessGrant, bool isGlobalAdmin = false, string? globalRole = null) => new() { IsAllowed = true, Upn = upn, AccessGrant = accessGrant, - IsGlobalAdmin = isGlobalAdmin + IsGlobalAdmin = isGlobalAdmin, + GlobalRole = globalRole }; public static McpAccessCheckResult Denied(string reason) => new() 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 fcb516f7..b7c5ab9a 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 @@ -22,7 +22,7 @@ process.env.MCP_RATE_LIMIT_PER_MINUTE = '2'; const { accessGuard } = await import('../access-guard.js'); // Same ESM singleton the guard uses internally — so the context set by // runWithCaller inside next() is observable here. -const { isGlobalAdmin, getCurrentToken } = await import('../client.js'); +const { isGlobalAdmin, hasGlobalScope, getCurrentToken } = await import('../client.js'); /** Build an unsigned JWT-shaped token carrying the given claims. */ function makeToken(claims: Record): string { @@ -106,7 +106,7 @@ function runGuard(req: Request): Promise { // Read the per-request caller context from inside the async scope. done({ nextCalled: true, - ctx: { token: getCurrentToken(), ga: isGlobalAdmin() }, + ctx: { token: getCurrentToken(), ga: isGlobalAdmin(), scope: hasGlobalScope() }, status: cap.statusCode, body: cap.body, headers: cap.headers, @@ -225,7 +225,7 @@ describe('accessGuard — allow path', () => { expect(out.nextCalled).toBe(true); expect(out.status).toBeNull(); // no response sent on the allow path - expect(out.ctx).toEqual({ token, ga: true }); + expect(out.ctx).toEqual({ token, ga: true, scope: true }); }); it('propagates isGlobalAdmin:false through the caller context', async () => { @@ -233,6 +233,23 @@ describe('accessGuard — allow path', () => { const out = await runGuard(mockReq(`Bearer ${validToken(uniqueUpn())}`)); expect(out.nextCalled).toBe(true); expect(out.ctx?.ga).toBe(false); + expect(out.ctx?.scope).toBe(false); + }); + + it('maps globalRole=GlobalReader to global scope WITHOUT global-admin (write) status', async () => { + stubBackend({ body: { allowed: true, accessGrant: 'GlobalReader', globalRole: 'GlobalReader' } }); + const out = await runGuard(mockReq(`Bearer ${validToken(uniqueUpn())}`)); + expect(out.nextCalled).toBe(true); + expect(out.ctx?.ga).toBe(false); // not a Global Admin + expect(out.ctx?.scope).toBe(true); // but has cross-tenant read scope + }); + + it('maps globalRole=GlobalAdmin to both global scope and global-admin status', async () => { + stubBackend({ body: { allowed: true, accessGrant: 'GlobalAdmin', globalRole: 'GlobalAdmin' } }); + const out = await runGuard(mockReq(`Bearer ${validToken(uniqueUpn())}`)); + expect(out.nextCalled).toBe(true); + expect(out.ctx?.ga).toBe(true); + expect(out.ctx?.scope).toBe(true); }); }); diff --git a/src/McpServer/autopilot-monitor-mcp/src/__tests__/client-routing.test.ts b/src/McpServer/autopilot-monitor-mcp/src/__tests__/client-routing.test.ts index 399dddd8..6fd3dcbf 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/__tests__/client-routing.test.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/__tests__/client-routing.test.ts @@ -14,6 +14,7 @@ import { describe, it, expect } from 'vitest'; import { pickGlobalOrTenantPath, isGlobalAdmin, + hasGlobalScope, getCurrentToken, runWithCaller, } from '../client.js'; @@ -44,6 +45,14 @@ describe('pickGlobalOrTenantPath — GA gating', () => { }); }); + it('routes a read-only Global Reader to /api/global/* (scope, not GA write status)', () => { + runWithCaller({ token: 'reader-token', isGlobalAdmin: false, isGlobalReader: true }, () => { + expect(isGlobalAdmin()).toBe(false); // not a write-tier Global Admin + expect(hasGlobalScope()).toBe(true); // but has cross-tenant read scope + expect(pickGlobalOrTenantPath(GLOBAL, TENANT)).toBe(GLOBAL); + }); + }); + it('reverts to the no-context default after the callback returns', () => { runWithCaller({ token: 't', isGlobalAdmin: true }, () => { expect(isGlobalAdmin()).toBe(true); diff --git a/src/McpServer/autopilot-monitor-mcp/src/__tests__/tools-catalog-order.test.ts b/src/McpServer/autopilot-monitor-mcp/src/__tests__/tools-catalog-order.test.ts index d3a9ce96..c3a0fd3e 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/__tests__/tools-catalog-order.test.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/__tests__/tools-catalog-order.test.ts @@ -11,11 +11,11 @@ import { describe, it, expect } from 'vitest'; import { McpServer } from '@modelcontextprotocol/sdk/server/mcp.js'; import { registerTools } from '../tools.js'; -function registeredToolNames(ga: boolean): string[] { +function registeredToolNames(ga: boolean, strictGa: boolean = ga): string[] { const server = new McpServer({ name: 'test', version: '0.0.0' }); // knowledgeBase / eventTypeIndex are optional — pass undefined so this stays a // pure unit test with no search-provider or backend dependency. - registerTools(server, undefined, undefined, ga); + registerTools(server, undefined, undefined, ga, strictGa); const internal = server as unknown as { _registeredTools: Record }; return Object.keys(internal._registeredTools); } @@ -54,4 +54,19 @@ describe('tool catalog ordering', () => { expect(gaNames).toContain('list_tenants'); expect(tenantNames).not.toContain('list_tenants'); }); + + it('gives a Global Reader the platform read tools but NOT the secret-bearing raw tools', () => { + // A Global Reader has platform scope (ga=true) but is not a real Global Admin (strictGa=false). + const readerNames = registeredToolNames(true, false); + const gaNames = registeredToolNames(true, true); + + // Curated cross-tenant read tools are present for the reader… + expect(readerNames).toContain('list_tenants'); + expect(readerNames).toContain('get_api_usage'); + // …but the raw secret-bearing tools are GA-strict only. + for (const raw of ['query_table', 'list_tables', 'query_backend_logs']) { + expect(gaNames).toContain(raw); + expect(readerNames).not.toContain(raw); + } + }); }); diff --git a/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts b/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts index 7fda078c..ff39257b 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/access-guard.ts @@ -33,6 +33,7 @@ interface AccessCacheEntry { allowed: boolean; reason: string; isGlobalAdmin: boolean; + isGlobalReader: boolean; expiresAt: number; } @@ -48,6 +49,7 @@ interface AccessCheckResult { allowed: boolean; reason: string; isGlobalAdmin: boolean; + isGlobalReader: boolean; } export function buildCacheKey(upn: string, token: string): string { @@ -59,7 +61,12 @@ async function checkAccess(upn: string, token: string): Promise next()); + runWithCaller( + { token, isGlobalAdmin: result.isGlobalAdmin, isGlobalReader: result.isGlobalReader }, + () => next(), + ); }) .catch(() => { res.status(503).json({ error: 'Access check failed' }); diff --git a/src/McpServer/autopilot-monitor-mcp/src/client.ts b/src/McpServer/autopilot-monitor-mcp/src/client.ts index 1a94516f..c4326c28 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/client.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/client.ts @@ -33,17 +33,22 @@ export class ApiError extends Error { * Per-request caller context using AsyncLocalStorage. * * Each incoming MCP request runs inside its own async context (via - * `runWithCaller`), carrying both the Bearer token and the caller's - * resolved Global-Admin status. Tools route based on the role: - * - GA → /api/global/* (cross-tenant; tenantId is a filter param) - * - Tenant-Admin → /api/* (tenant-scoped; JWT-tid is authoritative) + * `runWithCaller`), carrying the Bearer token and the caller's resolved + * platform role. Tools route on platform SCOPE (Global Admin OR the read-only + * Global Reader — the server is read-only, so both have identical cross-tenant + * reach): + * - global scope → /api/global/* (cross-tenant; tenantId is a filter param) + * - tenant user → /api/* (tenant-scoped; JWT-tid is authoritative) * * Concurrent sessions cannot overwrite each other's context even when * async operations interleave on the event loop. */ interface CallerContext { token: string; + /** True only for a platform Global Admin (write tier — not currently used by the read-only MCP). */ isGlobalAdmin: boolean; + /** True for the read-only Global Reader platform tier. */ + isGlobalReader?: boolean; } const callerStore = new AsyncLocalStorage(); @@ -71,13 +76,26 @@ export function isGlobalAdmin(): boolean { } /** - * Picks the route prefix for the current caller. GA always uses - * `/api/global/*`; tenant-admins use the tenant-scoped variant. The same - * `tenantId` query param is meaningful in both worlds — on `/api/global/*` - * it filters; on `/api/*` it's informational (backend resolves from JWT). + * True when the current caller has platform-wide (cross-tenant) scope — a Global + * Admin OR a read-only Global Reader. This is what tool routing and tool-catalog + * gating key off, because the MCP server is entirely read-only: a Global Reader + * has the same cross-tenant reach as a Global Admin here. Defaults to false when + * no context is active (e.g. unit tests without an explicit `runWithCaller`). + */ +export function hasGlobalScope(): boolean { + const store = callerStore.getStore(); + return store?.isGlobalAdmin === true || store?.isGlobalReader === true; +} + +/** + * Picks the route prefix for the current caller. A platform-scope caller (GA or + * read-only Global Reader) uses `/api/global/*`; tenant users use the + * tenant-scoped variant. The same `tenantId` query param is meaningful in both + * worlds — on `/api/global/*` it filters; on `/api/*` it's informational + * (backend resolves from JWT). */ export function pickGlobalOrTenantPath(globalPath: string, tenantPath: string): string { - return isGlobalAdmin() ? globalPath : tenantPath; + return hasGlobalScope() ? globalPath : tenantPath; } /** diff --git a/src/McpServer/autopilot-monitor-mcp/src/index.ts b/src/McpServer/autopilot-monitor-mcp/src/index.ts index 651ba316..a16d1135 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/index.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/index.ts @@ -15,7 +15,7 @@ import { validatePrecomputedIndex } from './precomputed-index.js'; import { buildEventTypeSearchDocs } from './resource-catalog.js'; import { createOAuthRouter } from './oauth.js'; import { accessGuard } from './access-guard.js'; -import { isGlobalAdmin } from './client.js'; +import { hasGlobalScope, isGlobalAdmin } from './client.js'; import { API_BASE_URL } from './config.js'; const __dirname = dirname(fileURLToPath(import.meta.url)); @@ -133,9 +133,10 @@ console.error(`Event-type index ready: ${eventTypeIndex.name} — ${eventTypeInd // into every tool description (and re-sent on every tools/list). Keep it short: // it is always-on context, not a manual. // -// Role-aware: only a Global Admin sees the cross-tenant scope hint. A normal -// tenant user gets instructions with no mention of Global-Admin / cross-tenant -// capability at all — the surface is scoped to what they can actually do. +// Role-aware: only a platform-scope caller (Global Admin or read-only Global +// Reader) sees the cross-tenant scope hint. A normal tenant user gets +// instructions with no mention of cross-tenant capability at all — the surface +// is scoped to what they can actually do. function buildInstructions(ga: boolean): string { return [ 'Autopilot-Monitor is a READ-ONLY telemetry server for Windows Autopilot enrollment sessions.', @@ -146,7 +147,7 @@ function buildInstructions(ga: boolean): string { 'Pagination: when a response carries `nextLink`, pass that whole string back as `continuation`; stop when it is absent. Results are never silently truncated.', 'Catalogs: call get_resource(name="event_types"|"device_properties") to discover valid eventType strings and deviceProperties keys before filtering.', ga - ? 'Scope: omit tenantId for cross-tenant queries (Global Admin only); pass tenantId to scope to one tenant.' + ? 'Scope: omit tenantId for cross-tenant queries (platform scope); pass tenantId to scope to one tenant.' : 'Scope: all queries are automatically limited to your tenant.', ].join('\n'); } @@ -157,12 +158,12 @@ function buildInstructions(ga: boolean): string { * role: a non-Global-Admin never sees GA-only tools or any cross-tenant / GA * wording — reducing both confusion and attack surface. */ -function createMcpServer(ga: boolean): McpServer { +function createMcpServer(ga: boolean, strictGa: boolean): McpServer { const s = new McpServer( { name: 'Autopilot-Monitor', version: SERVER_VERSION }, { instructions: buildInstructions(ga) }, ); - registerTools(s, knowledgeBase, eventTypeIndex, ga); + registerTools(s, knowledgeBase, eventTypeIndex, ga, strictGa); registerResources(s); registerPrompts(s, ga); return s; @@ -233,10 +234,12 @@ app.all('/mcp', async (req, res) => { const transport = new StreamableHTTPServerTransport({ sessionIdGenerator: undefined, // stateless: no session tracking }); - // accessGuard ran runWithCaller({ isGlobalAdmin }) around next(), so the - // caller's resolved role is available here (and stays active through - // transport.handleRequest, where tools/list and tool calls execute). - const server = createMcpServer(isGlobalAdmin()); + // accessGuard ran runWithCaller({ platform role }) around next(), so the + // caller's resolved scope is available here (and stays active through + // transport.handleRequest, where tools/list and tool calls execute). Tool + // catalog + routing key off platform SCOPE (GA or read-only Global Reader), + // since the server is read-only and both have identical cross-tenant reach. + const server = createMcpServer(hasGlobalScope(), isGlobalAdmin()); // Guarantee cleanup once the response is done, even on client disconnect. res.on('close', () => { diff --git a/src/McpServer/autopilot-monitor-mcp/src/tools.ts b/src/McpServer/autopilot-monitor-mcp/src/tools.ts index 1857c222..9c3b5aea 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/tools.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/tools.ts @@ -6,19 +6,24 @@ import { registerAdminTools } from './tools/admin.js'; /** * Registers the tool catalog for a single request, tailored to the caller's role. - * When `ga` is false (normal tenant user), Global-Admin-only tools are not - * registered at all — they never appear in tools/list — and the remaining tools' - * descriptions carry no cross-tenant / Global-Admin wording. + * + * `ga` here means platform SCOPE (Global Admin OR read-only Global Reader) — the broad gate for + * cross-tenant read tools. `strictGa` is true ONLY for a real Global Admin and gates the few raw + * tools whose backend endpoints stay GlobalAdminOnly (list_tables / query_table / query_backend_logs) + * because they can dump secret-bearing tables that the GlobalReader config redaction would otherwise + * hide. When `ga` is false (normal tenant user) no platform tool is registered at all — they never + * appear in tools/list — and the remaining tools' descriptions carry no cross-tenant wording. */ export function registerTools( server: McpServer, knowledgeBase: SearchProvider | undefined, eventTypeIndex: SearchProvider | undefined, ga: boolean, + strictGa: boolean = ga, ): void { registerSessionTools(server, ga); registerSearchTools(server, knowledgeBase, eventTypeIndex, ga); - registerAdminTools(server, ga); + registerAdminTools(server, ga, strictGa); sortToolCatalog(server); } diff --git a/src/McpServer/autopilot-monitor-mcp/src/tools/admin.ts b/src/McpServer/autopilot-monitor-mcp/src/tools/admin.ts index c59b4e93..c6cde2a3 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/tools/admin.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/tools/admin.ts @@ -166,7 +166,7 @@ export function paginateInventory( }; } -export function registerAdminTools(server: McpServer, ga: boolean): void { +export function registerAdminTools(server: McpServer, ga: boolean, strictGa: boolean = ga): void { // Tool 11: get_api_usage — Global Admin only; not registered for normal users // (the `if (ga)` guards the whole single server.registerTool(...) statement). if (ga) server.registerTool( @@ -762,12 +762,13 @@ export function registerAdminTools(server: McpServer, ga: boolean): void { // ── Admin Diagnostic Tools ──────────────────────────────────────────── // Security note: query_table and query_backend_logs accept arbitrary OData/KQL - // expressions. This is by design — these are trusted-admin-only diagnostic tools - // gated by GlobalAdmin RBAC on the backend. No client-side filter allowlist is - // needed because the backend enforces the same permission boundary. + // expressions AND can read secret-bearing tables (e.g. TenantConfiguration). They are + // therefore gated on `strictGa` (real Global Admin only), NOT the broader platform scope — + // a read-only GlobalReader must not bypass the config-secret redaction via raw table access. + // The backend endpoints (global/raw/tables, global/raw/logs) are GlobalAdminOnly to match. // Tool 20: list_tables — Global Admin only; not registered for normal users. - if (ga) server.registerTool( + if (strictGa) server.registerTool( 'list_tables', { title: 'List Tables', @@ -786,7 +787,7 @@ export function registerAdminTools(server: McpServer, ga: boolean): void { ); // Tool 21: query_table — Global Admin only; not registered for normal users. - if (ga) server.registerTool( + if (strictGa) server.registerTool( 'query_table', { title: 'Query Table', @@ -849,7 +850,7 @@ export function registerAdminTools(server: McpServer, ga: boolean): void { ); // Tool 22: query_backend_logs — Global Admin only; not registered for normal users. - if (ga) server.registerTool( + if (strictGa) server.registerTool( 'query_backend_logs', { title: 'Query Backend Logs', diff --git a/src/Shared/AutopilotMonitor.Shared/Constants.cs b/src/Shared/AutopilotMonitor.Shared/Constants.cs index 9bedd6be..675e34f9 100644 --- a/src/Shared/AutopilotMonitor.Shared/Constants.cs +++ b/src/Shared/AutopilotMonitor.Shared/Constants.cs @@ -640,6 +640,26 @@ namespace AutopilotMonitor.Shared public const string Viewer = "Viewer"; } + /// + /// Platform-wide roles stored in the GlobalAdmins table (Role column). + /// Empty/missing Role on existing entities is treated as GlobalAdmin for backward compatibility. + /// GlobalReader = read-only platform tier: cross-tenant read of telemetry + config (config secrets + /// redacted) but performs NO mutating operation and cannot write global settings. Raw table/log + /// access stays GlobalAdminOnly (it could dump secret-bearing tables, bypassing redaction). + /// + public static class GlobalRoles + { + public const string GlobalAdmin = "GlobalAdmin"; + public const string GlobalReader = "GlobalReader"; + } + + /// + /// Placeholder substituted for secret-bearing string fields (SAS URLs, webhook URLs, API keys, + /// custom webhook headers) when a config object is served to a read-only GlobalReader. Non-empty + /// secrets become this sentinel so the UI shows "configured but hidden"; empty stays empty. + /// + public const string RedactedSecretPlaceholder = "***REDACTED***"; + // ----------------------------------------------------------------------- // Audit logging // ----------------------------------------------------------------------- diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/IAdminRepository.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/IAdminRepository.cs index 96b43b5b..d7490e8f 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/IAdminRepository.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/IAdminRepository.cs @@ -12,6 +12,11 @@ namespace AutopilotMonitor.Shared.DataAccess { // --- Global Admins --- Task IsGlobalAdminAsync(string upn); + /// + /// Resolves the platform role for a UPN: "GlobalAdmin", "GlobalReader", or null when there is no + /// enabled GlobalAdmins row. Empty/missing Role on an existing enabled row ⇒ "GlobalAdmin" (back-compat). + /// + Task GetGlobalRoleAsync(string upn); Task> GetAllGlobalAdminsAsync(); Task AddGlobalAdminAsync(string upn, string addedBy); Task RemoveGlobalAdminAsync(string upn); @@ -43,6 +48,8 @@ namespace AutopilotMonitor.Shared.DataAccess public bool IsEnabled { get; set; } = true; public DateTime AddedAt { get; set; } public string AddedBy { get; set; } = string.Empty; + /// Platform role: "GlobalAdmin" (default) or "GlobalReader". Empty ⇒ GlobalAdmin (back-compat). + public string Role { get; set; } = string.Empty; } public class McpUserEntry diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs b/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs index 2bd43958..aaf4d5f4 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Config/AdminConfiguration.cs @@ -459,6 +459,29 @@ namespace AutopilotMonitor.Shared.Models /// public string MsrcLastSyncUtc { get; set; } = default!; + /// + /// Returns a shallow copy with all secret-bearing string fields replaced by + /// (empty values are left empty). Used when + /// serving the global config to a read-only GlobalReader so SAS URLs / API keys / webhook URLs + /// (which could enable external mutations) never leave the backend. The original (cached) instance + /// is never mutated. + /// + /// SECURITY: this is a deny-list — every new secret string field MUST be added here. + /// AdminConfigurationRedactionTests guards against drift. + /// + public AdminConfiguration RedactedCopyForReader() + { + var copy = (AdminConfiguration)MemberwiseClone(); + copy.PlatformStatsBlobSasUrl = Redact(copy.PlatformStatsBlobSasUrl); + copy.NvdApiKey = Redact(copy.NvdApiKey); + copy.OpsAlertTeamsWebhookUrl = Redact(copy.OpsAlertTeamsWebhookUrl); + copy.OpsAlertSlackWebhookUrl = Redact(copy.OpsAlertSlackWebhookUrl); + return copy; + + static string Redact(string? value) + => string.IsNullOrEmpty(value) ? (value ?? string.Empty) : Constants.RedactedSecretPlaceholder; + } + /// /// Creates default configuration /// diff --git a/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs b/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs index 3bf3b5de..260ce04c 100644 --- a/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs +++ b/src/Shared/AutopilotMonitor.Shared/Models/Config/TenantConfiguration.cs @@ -614,6 +614,44 @@ namespace AutopilotMonitor.Shared.Models // ===== HELPER METHODS ===== + /// + /// Returns a shallow copy with all secret-bearing string fields replaced by + /// (empty values are left empty). Used when + /// serving a tenant config to a read-only GlobalReader so SAS URLs / webhook URLs / custom + /// webhook headers (which could enable external mutations) never leave the backend. The original + /// (cached) instance is never mutated. + /// + /// SECURITY: this is a deny-list — every new secret string field MUST be added here. + /// TenantConfigurationRedactionTests guards against drift. + /// + public TenantConfiguration RedactedCopyForReader() + { + var copy = (TenantConfiguration)MemberwiseClone(); + copy.DiagnosticsBlobSasUrl = Redact(copy.DiagnosticsBlobSasUrl); + copy.TeamsWebhookUrl = Redact(copy.TeamsWebhookUrl); + copy.WebhookUrl = Redact(copy.WebhookUrl); + copy.WebhookCustomHeadersJson = Redact(copy.WebhookCustomHeadersJson); + return copy; + + static string Redact(string? value) + => string.IsNullOrEmpty(value) ? (value ?? string.Empty) : Constants.RedactedSecretPlaceholder; + } + + /// + /// Defense-in-depth for the redacted read-only view: for each secret string field, if THIS object + /// carries the sentinel (i.e. a redacted config + /// was round-tripped back on a save), restore the real value from . + /// Mutates this instance. Must mirror the field set in . + /// + public void RestoreRedactedSecretsFrom(TenantConfiguration existing) + { + if (existing == null) return; + if (DiagnosticsBlobSasUrl == Constants.RedactedSecretPlaceholder) DiagnosticsBlobSasUrl = existing.DiagnosticsBlobSasUrl; + if (TeamsWebhookUrl == Constants.RedactedSecretPlaceholder) TeamsWebhookUrl = existing.TeamsWebhookUrl; + if (WebhookUrl == Constants.RedactedSecretPlaceholder) WebhookUrl = existing.WebhookUrl; + if (WebhookCustomHeadersJson == Constants.RedactedSecretPlaceholder) WebhookCustomHeadersJson = existing.WebhookCustomHeadersJson; + } + /// /// Returns the effective webhook URL and provider type, handling legacy TeamsWebhookUrl migration. /// New fields take priority; falls back to TeamsWebhookUrl as TeamsLegacyConnector. diff --git a/src/Web/autopilot-monitor-web/app/admin/AdminConfigContext.tsx b/src/Web/autopilot-monitor-web/app/admin/AdminConfigContext.tsx index 39274a50..7d7a1a5f 100644 --- a/src/Web/autopilot-monitor-web/app/admin/AdminConfigContext.tsx +++ b/src/Web/autopilot-monitor-web/app/admin/AdminConfigContext.tsx @@ -273,6 +273,9 @@ export function AdminConfigProvider({ children }: { children: React.ReactNode }) // Save admin config const handleSaveAdminConfig = useCallback(async () => { if (!adminConfig) return; + // Platform settings are GA-only. A read-only Global Reader can reach the admin area (view scope) + // but must never persist global config — guard here too (the backend also enforces GlobalAdminOnly). + if (!isGlobalAdmin) return; try { setSavingConfig(true); @@ -319,7 +322,7 @@ export function AdminConfigProvider({ children }: { children: React.ReactNode }) } finally { setSavingConfig(false); } - }, [adminConfig, globalRateLimit, platformStatsBlobSasUrl, collectorIdleTimeoutMinutes, desktopDetectorNoCandidateTimeoutMinutes, maxSessionWindowHours, maintenanceBlockDurationHours, opsEventRetentionDays, slaNotificationCooldownHours, allowAgentDowngrade, modernDeploymentHarmlessEventIds, enableIndexDualWrite, sessionDeletionKillSwitch, getAccessToken]); + }, [isGlobalAdmin, adminConfig, globalRateLimit, platformStatsBlobSasUrl, collectorIdleTimeoutMinutes, desktopDetectorNoCandidateTimeoutMinutes, maxSessionWindowHours, maintenanceBlockDurationHours, opsEventRetentionDays, slaNotificationCooldownHours, allowAgentDowngrade, modernDeploymentHarmlessEventIds, enableIndexDualWrite, sessionDeletionKillSwitch, getAccessToken]); // Reset admin config const handleResetAdminConfig = useCallback(() => { @@ -363,6 +366,7 @@ export function AdminConfigProvider({ children }: { children: React.ReactNode }) // Save diagnostics paths const handleSaveDiagPaths = useCallback(async (paths: DiagnosticsLogPath[]) => { if (!adminConfig) return; + if (!isGlobalAdmin) return; // platform settings are GA-only (also route-gated + backend-enforced) try { setSavingDiagPaths(true); setError(null); @@ -394,7 +398,7 @@ export function AdminConfigProvider({ children }: { children: React.ReactNode }) } finally { setSavingDiagPaths(false); } - }, [adminConfig, getAccessToken]); + }, [isGlobalAdmin, adminConfig, getAccessToken]); // Save ops alert config (rules + providers) const handleSaveOpsAlertConfig = useCallback(async ( @@ -408,6 +412,7 @@ export function AdminConfigProvider({ children }: { children: React.ReactNode }) newAutoActionDurationHours: number, ) => { if (!adminConfig) return; + if (!isGlobalAdmin) return; // platform settings are GA-only (also route-gated + backend-enforced) try { setSavingOpsAlerts(true); setError(null); @@ -459,7 +464,7 @@ export function AdminConfigProvider({ children }: { children: React.ReactNode }) } finally { setSavingOpsAlerts(false); } - }, [adminConfig, getAccessToken]); + }, [isGlobalAdmin, adminConfig, getAccessToken]); return ( {children}; +} diff --git a/src/Web/autopilot-monitor-web/app/admin/components/DeviceBlockSection.tsx b/src/Web/autopilot-monitor-web/app/admin/components/DeviceBlockSection.tsx index 412f268d..f10a7936 100644 --- a/src/Web/autopilot-monitor-web/app/admin/components/DeviceBlockSection.tsx +++ b/src/Web/autopilot-monitor-web/app/admin/components/DeviceBlockSection.tsx @@ -7,6 +7,7 @@ import { authenticatedFetch, TokenExpiredError } from "@/lib/authenticatedFetch" import { trackEvent } from "@/lib/appInsights"; import { TenantConfiguration } from "./TenantManagementSection"; import { TenantSearchSelect } from "./TenantSearchSelect"; +import { useCanMutatePlatform } from "@/hooks/useCanMutatePlatform"; interface ResolvedDevice { sessionId: string; @@ -55,6 +56,8 @@ export function DeviceBlockSection({ setError, setSuccessMessage, }: DeviceBlockSectionProps) { + // Read-only Global Readers reach this page (view scope) but must not block/kill devices. + const canMutate = useCanMutatePlatform(); const [blockSerialNumber, setBlockSerialNumber] = useState(""); const [blockTenantId, setBlockTenantId] = useState(""); const [blockDurationHours, setBlockDurationHours] = useState(12); @@ -171,6 +174,7 @@ export function DeviceBlockSection({ }; const handleBlockDevice = async () => { + if (!canMutate) return; // read-only Global Reader if (!blockSerialNumber.trim() || !blockTenantId.trim()) return; if (blockAction === "Kill" && !confirm( @@ -232,6 +236,7 @@ export function DeviceBlockSection({ }; const handleUnblockDevice = async (tenantId: string, serialNumber: string) => { + if (!canMutate) return; // read-only Global Reader try { setUnblockingDevice(serialNumber); setError(null); @@ -257,6 +262,7 @@ export function DeviceBlockSection({ }; const handleKillDevice = async (device: BlockedDevice) => { + if (!canMutate) return; // read-only Global Reader if (!confirm( `REMOTE KILL: This will permanently shut down the agent on device "${device.serialNumber}" and remove all agent files. This cannot be undone. Continue?` )) return; @@ -349,7 +355,7 @@ export function DeviceBlockSection({ )} - + {canMutate ? ( + + ) : ( + Read-only + )} diff --git a/src/Web/autopilot-monitor-web/app/admin/components/SessionReportsSection.tsx b/src/Web/autopilot-monitor-web/app/admin/components/SessionReportsSection.tsx index 57ddfaed..3416fe2f 100644 --- a/src/Web/autopilot-monitor-web/app/admin/components/SessionReportsSection.tsx +++ b/src/Web/autopilot-monitor-web/app/admin/components/SessionReportsSection.tsx @@ -6,6 +6,7 @@ import { authenticatedFetch, TokenExpiredError } from "@/lib/authenticatedFetch" import { extractContinuation } from "@/lib/paginationLink"; import { isGuid } from "@/utils/inputValidation"; import { trackEvent } from "@/lib/appInsights"; +import { useCanMutatePlatform } from "@/hooks/useCanMutatePlatform"; const PAGE_SIZE = 20; @@ -73,6 +74,9 @@ export function SessionReportsSection({ getAccessToken, setError, }: SessionReportsSectionProps) { + // Admin Note is GA-only (PATCH session-reports/{id}/note is GlobalAdminOnly). A read-only Global + // Reader may view reports + notes but not edit them. + const canMutate = useCanMutatePlatform(); const [reports, setReports] = useState([]); const [loading, setLoading] = useState(false); const [selectedReport, setSelectedReport] = useState(null); @@ -482,9 +486,10 @@ export function SessionReportsSection({