diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TenantNotificationAudienceCatalogTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TenantNotificationAudienceCatalogTests.cs new file mode 100644 index 00000000..bef09db7 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TenantNotificationAudienceCatalogTests.cs @@ -0,0 +1,39 @@ +using AutopilotMonitor.Functions.Services; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Tests for . +/// +/// CORRECTNESS GUARD: The catalog is the single source of truth for who sees which tenant +/// notification type. An unknown type must fail closed (Admin) so that a newly added type +/// does not leak to non-admin members until it is explicitly registered here. +/// +public class TenantNotificationAudienceCatalogTests +{ + [Theory] + [InlineData("hardware_rejection", NotificationAudience.Admin)] + [InlineData("sla_breach", NotificationAudience.Member)] + [InlineData("sla_consecutive_failures", NotificationAudience.Member)] + public void Resolve_KnownTypes_ReturnsExpectedAudience(string type, NotificationAudience expected) + { + Assert.Equal(expected, TenantNotificationAudienceCatalog.Resolve(type)); + } + + [Fact] + public void Resolve_IsCaseInsensitive() + { + Assert.Equal(NotificationAudience.Admin, TenantNotificationAudienceCatalog.Resolve("HARDWARE_REJECTION")); + Assert.Equal(NotificationAudience.Member, TenantNotificationAudienceCatalog.Resolve("Sla_Breach")); + } + + [Theory] + [InlineData("brand_new_type_not_in_catalog")] + [InlineData("")] + [InlineData(null)] + [InlineData(" ")] + public void Resolve_UnknownOrEmpty_FailsClosedToAdmin(string? type) + { + Assert.Equal(NotificationAudience.Admin, TenantNotificationAudienceCatalog.Resolve(type)); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/TenantNotificationServiceTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/TenantNotificationServiceTests.cs new file mode 100644 index 00000000..4e7eff7b --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/TenantNotificationServiceTests.cs @@ -0,0 +1,82 @@ +using AutopilotMonitor.Functions.Services; +using AutopilotMonitor.Shared.DataAccess; +using AutopilotMonitor.Shared.Models; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Tests for audience filtering. +/// +/// CORRECTNESS GUARD: Member-tier callers (Operator/Viewer) must NOT see Admin-tier notifications +/// (e.g. hardware_rejection) — those expose admin-only links and information that non-admins +/// cannot act on. The catalog at drives this; this +/// test locks in that the service actually consults it. +/// +public class TenantNotificationServiceTests +{ + private const string TenantId = "test-tenant-aaaa-bbbb-cccc-dddddddddddd"; + + private static TenantNotificationService CreateService(IEnumerable notifications) + { + var repo = new Mock(); + repo.Setup(r => r.GetNotificationsAsync(TenantId, It.IsAny())) + .ReturnsAsync(notifications.ToList()); + return new TenantNotificationService(repo.Object, NullLogger.Instance); + } + + private static GlobalNotification Notif(string type, string id) => new() + { + NotificationId = id, + Type = type, + Title = $"{type} title", + Message = "msg", + CreatedAt = DateTime.UtcNow, + CreatedBy = "system", + }; + + [Fact] + public async Task GetActive_AsMember_HidesAdminAudienceTypes() + { + var svc = CreateService(new[] + { + Notif("hardware_rejection", "n1"), // Admin + Notif("sla_breach", "n2"), // Member + Notif("sla_consecutive_failures", "n3"), // Member + }); + + var result = await svc.GetActiveNotificationsAsync(TenantId, NotificationAudience.Member); + + Assert.Equal(2, result.Count); + Assert.DoesNotContain(result, n => n.Type == "hardware_rejection"); + Assert.Contains(result, n => n.Id == "n2"); + Assert.Contains(result, n => n.Id == "n3"); + } + + [Fact] + public async Task GetActive_AsAdmin_SeesAllAudienceTypes() + { + var svc = CreateService(new[] + { + Notif("hardware_rejection", "n1"), + Notif("sla_breach", "n2"), + }); + + var result = await svc.GetActiveNotificationsAsync(TenantId, NotificationAudience.Admin); + + Assert.Equal(2, result.Count); + } + + [Fact] + public async Task GetActive_UnknownType_HiddenFromMember_VisibleToAdmin() + { + var notifs = new[] { Notif("brand_new_unregistered_type", "n1") }; + + var asMember = await CreateService(notifs).GetActiveNotificationsAsync(TenantId, NotificationAudience.Member); + var asAdmin = await CreateService(notifs).GetActiveNotificationsAsync(TenantId, NotificationAudience.Admin); + + Assert.Empty(asMember); + Assert.Single(asAdmin); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableTenantNotificationRepository.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableTenantNotificationRepository.cs index 771a19a4..b1aa4ca5 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableTenantNotificationRepository.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableTenantNotificationRepository.cs @@ -98,6 +98,12 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage } } + // TODO(per-user-dismiss): Dismissal is currently tenant-shared — flipping `Dismissed` on the + // single row hides the notification for every member of the tenant. When notification volume + // grows enough that this matters, model dismissal as a separate per-user fact: + // PartitionKey = $"{tenantId}|{userObjectId}", RowKey = notificationId. + // The GET path then left-anti-joins the dismissed-by-this-user set against the tenant rows; + // the dismiss endpoints can drop to MemberRead because each user only writes their own row. public async Task DismissNotificationAsync(string tenantId, string notificationId, string dismissedBy) { if (string.IsNullOrWhiteSpace(tenantId) || string.IsNullOrWhiteSpace(notificationId)) diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Notifications/TenantNotificationsFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Notifications/TenantNotificationsFunction.cs index 277fa4c7..bef5e7b6 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Notifications/TenantNotificationsFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Notifications/TenantNotificationsFunction.cs @@ -28,14 +28,22 @@ public class TenantNotificationsFunction /// /// GET /api/notifications - /// Returns all active (non-dismissed) notifications for the caller's tenant, newest first. + /// Returns all active (non-dismissed) notifications visible to the caller, newest first. + /// Visibility tier is derived from the resolved request context: Tenant Admins and Global + /// Admins see -tier notifications too; + /// Operators and Viewers see only -tier notifications. /// [Function("GetTenantNotifications")] public async Task GetNotifications( [HttpTrigger(AuthorizationLevel.Anonymous, "get", Route = "notifications")] HttpRequestData req) { + var ctx = req.GetRequestContext(); var tenantId = TenantHelper.GetTenantId(req); - var notifications = await _notificationService.GetActiveNotificationsAsync(tenantId); + var callerAudience = (ctx.IsTenantAdmin || ctx.IsGlobalAdmin) + ? NotificationAudience.Admin + : NotificationAudience.Member; + + var notifications = await _notificationService.GetActiveNotificationsAsync(tenantId, callerAudience); var response = req.CreateResponse(HttpStatusCode.OK); await response.WriteAsJsonAsync(new { success = true, notifications }); @@ -45,6 +53,9 @@ public class TenantNotificationsFunction /// /// POST /api/notifications/{notificationId}/dismiss /// Dismisses a single notification within the caller's tenant scope. + /// Currently restricted to Tenant Admins / Global Admins because dismissal is tenant-shared + /// (see TODO in ). + /// When per-user dismiss lands, this can drop to MemberRead. /// [Function("DismissTenantNotification")] public async Task DismissNotification( diff --git a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs index 172dda6c..c28f0eea 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/EndpointAccessPolicyCatalog.cs @@ -288,8 +288,12 @@ public static class EndpointAccessPolicyCatalog 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. - // TODO: when EndpointPolicy.TenantAdminOrOperator lands, switch these from TenantAdminOrGA so Operators see the bell too. - new("GET", "notifications", EndpointPolicy.TenantAdminOrGA, TenantScoping.Jwt), + // GET is MemberRead (Admin/Operator/Viewer) — per-type visibility is filtered server-side + // via TenantNotificationAudienceCatalog, so Member-tier callers transparently see only the + // notifications they are entitled to. + // Dismiss endpoints stay TenantAdminOrGA: dismissal is currently tenant-shared (clearing for + // one user clears for all). When per-user dismiss lands, dismiss can drop to MemberRead too. + 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), diff --git a/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs b/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs index f00a797c..72a83da5 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/SlaBreachEvaluationService.cs @@ -25,7 +25,7 @@ namespace AutopilotMonitor.Functions.Services private readonly IMaintenanceRepository _maintenanceRepo; private readonly ISessionRepository _sessionRepo; private readonly WebhookNotificationService _webhookService; - private readonly GlobalNotificationService _globalNotificationService; + private readonly TenantNotificationService _tenantNotificationService; private readonly SlaNotificationThrottleService _throttle; private readonly OpsEventService _opsEventService; private readonly TelemetryClient _telemetryClient; @@ -37,7 +37,7 @@ namespace AutopilotMonitor.Functions.Services IMaintenanceRepository maintenanceRepo, ISessionRepository sessionRepo, WebhookNotificationService webhookService, - GlobalNotificationService globalNotificationService, + TenantNotificationService tenantNotificationService, SlaNotificationThrottleService throttle, OpsEventService opsEventService, TelemetryClient telemetryClient, @@ -48,7 +48,7 @@ namespace AutopilotMonitor.Functions.Services _maintenanceRepo = maintenanceRepo; _sessionRepo = sessionRepo; _webhookService = webhookService; - _globalNotificationService = globalNotificationService; + _tenantNotificationService = tenantNotificationService; _throttle = throttle; _opsEventService = opsEventService; _telemetryClient = telemetryClient; @@ -200,8 +200,9 @@ namespace AutopilotMonitor.Functions.Services await _webhookService.SendNotificationAsync(webhookUrl, (Shared.Models.Notifications.WebhookProviderType)providerType, alert); } - // In-app global notification - _ = _globalNotificationService.CreateNotificationAsync( + // In-app tenant notification — visible to all tenant members (Member audience) + await _tenantNotificationService.CreateNotificationAsync( + config.TenantId, "sla_breach", alert.Title, alert.Summary ?? "", @@ -277,8 +278,9 @@ namespace AutopilotMonitor.Functions.Services await _webhookService.SendNotificationAsync(webhookUrl, (Shared.Models.Notifications.WebhookProviderType)providerType, alert); } - // In-app global notification - _ = _globalNotificationService.CreateNotificationAsync( + // In-app tenant notification — visible to all tenant members (Member audience) + await _tenantNotificationService.CreateNotificationAsync( + tenantId, "sla_consecutive_failures", alert.Title, alert.Summary ?? "", diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TenantNotificationAudienceCatalog.cs b/src/Backend/AutopilotMonitor.Functions/Services/TenantNotificationAudienceCatalog.cs new file mode 100644 index 00000000..cd27c18f --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions/Services/TenantNotificationAudienceCatalog.cs @@ -0,0 +1,38 @@ +using System.Collections.Generic; + +namespace AutopilotMonitor.Functions.Services; + +/// +/// Visibility tier for a tenant notification. Compared with the caller's resolved tier +/// in : +/// a notification is visible iff notificationAudience <= callerAudience. +/// +public enum NotificationAudience +{ + /// Visible to every tenant member (Admin, Operator, Viewer) and Global Admins. + Member = 0, + + /// Visible only to Tenant Admins and Global Admins. + Admin = 1, +} + +/// +/// Single source of truth for which audience may see each tenant notification type. +/// Lookup is by notification Type string; unknown types default to +/// so a newly introduced type fails closed (hidden from non-admins) until it is registered here. +/// +public static class TenantNotificationAudienceCatalog +{ + private static readonly Dictionary _map = new(System.StringComparer.OrdinalIgnoreCase) + { + ["hardware_rejection"] = NotificationAudience.Admin, + ["sla_breach"] = NotificationAudience.Member, + ["sla_consecutive_failures"] = NotificationAudience.Member, + }; + + public static NotificationAudience Resolve(string? type) + { + if (string.IsNullOrWhiteSpace(type)) return NotificationAudience.Admin; + return _map.TryGetValue(type, out var audience) ? audience : NotificationAudience.Admin; + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TenantNotificationService.cs b/src/Backend/AutopilotMonitor.Functions/Services/TenantNotificationService.cs index b5f0f434..3066e4dc 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TenantNotificationService.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TenantNotificationService.cs @@ -56,21 +56,28 @@ public class TenantNotificationService } } - public async Task> GetActiveNotificationsAsync(string tenantId) + /// + /// Returns active (non-dismissed) tenant notifications visible to a caller of the given audience tier. + /// A notification is included iff TenantNotificationAudienceCatalog.Resolve(type) <= callerAudience: + /// Admin callers see Member + Admin notifications, Member callers see only Member notifications. + /// + public async Task> GetActiveNotificationsAsync(string tenantId, NotificationAudience callerAudience) { try { var notifications = await _repo.GetNotificationsAsync(tenantId); - return notifications.Select(n => new GlobalNotificationDto - { - Id = n.NotificationId, - Type = n.Type, - Title = n.Title, - Message = n.Message, - Href = n.Href, - CreatedAt = n.CreatedAt - }).ToList(); + return notifications + .Where(n => TenantNotificationAudienceCatalog.Resolve(n.Type) <= callerAudience) + .Select(n => new GlobalNotificationDto + { + Id = n.NotificationId, + Type = n.Type, + Title = n.Title, + Message = n.Message, + Href = n.Href, + CreatedAt = n.CreatedAt + }).ToList(); } catch (Exception ex) { diff --git a/src/Web/autopilot-monitor-web/components/Navbar.tsx b/src/Web/autopilot-monitor-web/components/Navbar.tsx index 3075e301..85fa1199 100644 --- a/src/Web/autopilot-monitor-web/components/Navbar.tsx +++ b/src/Web/autopilot-monitor-web/components/Navbar.tsx @@ -145,6 +145,10 @@ export default function Navbar() { const isTenantAdmin = user?.isTenantAdmin ?? false; const isOperator = user?.role === 'Operator'; const isAdminOrOperator = isTenantAdmin || isOperator; + // Tenant notification dismiss is currently tenant-shared (clearing for one user clears for + // all). Only Admins / Global Admins are permitted to dismiss; Operators and Viewers see the + // bell as read-only. See TenantNotificationContext + EndpointAccessPolicyCatalog. + const canDismissTenant = isTenantAdmin || (user?.isGlobalAdmin ?? false); // Regular users (non-Admin, non-Operator): show minimal navbar with only Progress Portal if (!isAdminOrOperator && !user?.isGlobalAdmin) { @@ -293,7 +297,7 @@ export default function Navbar() { Mark all read )} - @@ -332,11 +336,13 @@ export default function Navbar() { - + {canDismissTenant && ( + + )} ))} @@ -671,9 +677,11 @@ export default function Navbar() {

{n.title}

{n.message}

- + {canDismissTenant && ( + + )} ))} {/* Global admin notifications */} diff --git a/src/Web/autopilot-monitor-web/contexts/TenantNotificationContext.tsx b/src/Web/autopilot-monitor-web/contexts/TenantNotificationContext.tsx index f3abbd59..204bd0fb 100644 --- a/src/Web/autopilot-monitor-web/contexts/TenantNotificationContext.tsx +++ b/src/Web/autopilot-monitor-web/contexts/TenantNotificationContext.tsx @@ -2,6 +2,7 @@ import React, { createContext, useContext, useState, useCallback, useEffect, useRef } from 'react'; import { useAuth } from './AuthContext'; +import { canFetchTenantNotifications } from './tenantNotificationsGate'; import { api } from '@/lib/api'; import { authenticatedFetch } from '@/lib/authenticatedFetch'; @@ -33,10 +34,14 @@ export function TenantNotificationProvider({ children }: { children: React.React const fetchingRef = useRef(false); const intervalRef = useRef | null>(null); - const isAuthenticated = user != null; + // Only tenant members (Admin/Operator/Viewer) and Global Admins are entitled to the bell. + // The backend filters per-notification visibility by audience tier, so it is safe to fetch + // for every member role — but unauthenticated users and users without a tenant role would + // just produce 401/403 noise, so we skip them entirely. + const canFetchNotifications = canFetchTenantNotifications(user); const fetchNotifications = useCallback(async () => { - if (!isAuthenticated || fetchingRef.current) return; + if (!canFetchNotifications || fetchingRef.current) return; fetchingRef.current = true; try { @@ -47,19 +52,16 @@ export function TenantNotificationProvider({ children }: { children: React.React if (response.ok) { const data = await response.json(); setTenantNotifications(data.notifications ?? []); - } else if (response.status === 403) { - // Backend gates this on TenantAdminOrGA today; non-admins get 403 — treat as empty. - setTenantNotifications([]); } } catch { // Silently ignore — notifications are best-effort } finally { fetchingRef.current = false; } - }, [isAuthenticated, getAccessToken]); + }, [canFetchNotifications, getAccessToken]); useEffect(() => { - if (!isAuthenticated) { + if (!canFetchNotifications) { setTenantNotifications([]); return; } @@ -72,7 +74,7 @@ export function TenantNotificationProvider({ children }: { children: React.React return () => { if (intervalRef.current) clearInterval(intervalRef.current); }; - }, [isAuthenticated, fetchNotifications]); + }, [canFetchNotifications, fetchNotifications]); const dismissTenantNotification = useCallback(async (id: string) => { setTenantNotifications(prev => prev.filter(n => n.id !== id)); diff --git a/src/Web/autopilot-monitor-web/contexts/tenantNotificationsGate.ts b/src/Web/autopilot-monitor-web/contexts/tenantNotificationsGate.ts new file mode 100644 index 00000000..c2aadc6e --- /dev/null +++ b/src/Web/autopilot-monitor-web/contexts/tenantNotificationsGate.ts @@ -0,0 +1,14 @@ +/** + * Returns true iff the current user is entitled to fetch tenant notifications. + * Tenant members of any role (Admin/Operator/Viewer) and Global Admins qualify; + * unauthenticated users and authenticated users without a tenant role do not. + * + * Extracted to a plain .ts module so that vitest can import it without pulling in + * the React/JSX-laden context module. + */ +export function canFetchTenantNotifications( + user: { role: string | null; isGlobalAdmin: boolean } | null | undefined +): boolean { + if (user == null) return false; + return user.role != null || user.isGlobalAdmin === true; +} diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/tenantNotificationsGate.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/tenantNotificationsGate.test.ts new file mode 100644 index 00000000..c4c97a90 --- /dev/null +++ b/src/Web/autopilot-monitor-web/lib/__tests__/tenantNotificationsGate.test.ts @@ -0,0 +1,36 @@ +import { describe, it, expect } from "vitest"; +import { canFetchTenantNotifications } from "../../contexts/tenantNotificationsGate"; + +/** + * CORRECTNESS GUARD: The bell is visible to every tenant member (Admin/Operator/Viewer) + * and to Global Admins. Polling for users who do NOT match either category produces a + * permanent 401/403 storm — the original bug that motivated PR-N5. This test locks in + * the gating predicate so that future refactors of the role check do not regress to the + * old "fetch on user != null" condition. + */ +describe("canFetchTenantNotifications", () => { + it("returns false for unauthenticated user", () => { + expect(canFetchTenantNotifications(null)).toBe(false); + expect(canFetchTenantNotifications(undefined)).toBe(false); + }); + + it("returns false for authenticated user without tenant role and not GA", () => { + expect(canFetchTenantNotifications({ role: null, isGlobalAdmin: false })).toBe(false); + }); + + it.each([ + ["Admin", "Admin"], + ["Operator", "Operator"], + ["Viewer", "Viewer"], + ])("returns true for tenant role %s", (_label, role) => { + expect(canFetchTenantNotifications({ role, isGlobalAdmin: false })).toBe(true); + }); + + it("returns true for Global Admin without tenant role", () => { + expect(canFetchTenantNotifications({ role: null, isGlobalAdmin: true })).toBe(true); + }); + + it("returns true for Global Admin with tenant role", () => { + expect(canFetchTenantNotifications({ role: "Admin", isGlobalAdmin: true })).toBe(true); + }); +});