fix(backend,web): role-gated tenant notifications + audience visibility

Tenant-bell endpoint silently 403'd every 60s for Operator/Viewer because the
GET was gated TenantAdminOrGA but the navbar shows the bell to every member.
Three real bugs were tangled in this:

- SLA breach + consecutive-failures notifications were emitted into the global
  channel even though they are per-tenant data. Only Global Admins ever saw
  them; the actual tenant got no in-app indication.
- Frontend polled /api/notifications for any authenticated user regardless of
  role and silently swallowed the 403 — masking the underlying mis-gate.
- Hardware-rejection notifications would have leaked admin-only context (link
  to whitelist editor) once the policy relaxed.

Fixes:

- New TenantNotificationAudienceCatalog (Type to Member/Admin map) drives
  per-type visibility server-side. Unknown types fail closed to Admin so a
  newly added type does not leak to non-admin members until registered.
- GET /api/notifications relaxed from TenantAdminOrGA to MemberRead. Dismiss
  endpoints stay TenantAdminOrGA (tenant-shared dismiss; per-user dismiss
  deferred with TODO + memory note).
- SLA breach + consecutive-failures emitters moved to TenantNotificationService.
  GlobalNotificationService injection removed from SlaBreachEvaluationService
  entirely (we do not use it anymore, so it is gone).
- Frontend polls only when user has a tenant role or is Global Admin; dismiss
  buttons hidden for Operator/Viewer.

Tests: 6 new xUnit, 7 new vitest. Backend 873/873 green, web 234/234 green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
2026-05-05 12:35:43 +02:00
parent 5ef6c8b1d2
commit 9b0a2dca95
12 changed files with 287 additions and 38 deletions
@@ -0,0 +1,39 @@
using AutopilotMonitor.Functions.Services;
namespace AutopilotMonitor.Functions.Tests;
/// <summary>
/// Tests for <see cref="TenantNotificationAudienceCatalog"/>.
///
/// 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.
/// </summary>
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));
}
}
@@ -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;
/// <summary>
/// Tests for <see cref="TenantNotificationService.GetActiveNotificationsAsync"/> audience filtering.
///
/// CORRECTNESS GUARD: Member-tier callers (Operator/Viewer) must NOT see Admin-tier notifications
/// (e.g. <c>hardware_rejection</c>) — those expose admin-only links and information that non-admins
/// cannot act on. The catalog at <see cref="TenantNotificationAudienceCatalog"/> drives this; this
/// test locks in that the service actually consults it.
/// </summary>
public class TenantNotificationServiceTests
{
private const string TenantId = "test-tenant-aaaa-bbbb-cccc-dddddddddddd";
private static TenantNotificationService CreateService(IEnumerable<GlobalNotification> notifications)
{
var repo = new Mock<ITenantNotificationRepository>();
repo.Setup(r => r.GetNotificationsAsync(TenantId, It.IsAny<int>()))
.ReturnsAsync(notifications.ToList());
return new TenantNotificationService(repo.Object, NullLogger<TenantNotificationService>.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);
}
}
@@ -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<bool> DismissNotificationAsync(string tenantId, string notificationId, string dismissedBy)
{
if (string.IsNullOrWhiteSpace(tenantId) || string.IsNullOrWhiteSpace(notificationId))
@@ -28,14 +28,22 @@ public class TenantNotificationsFunction
/// <summary>
/// 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 <see cref="NotificationAudience.Admin"/>-tier notifications too;
/// Operators and Viewers see only <see cref="NotificationAudience.Member"/>-tier notifications.
/// </summary>
[Function("GetTenantNotifications")]
public async Task<HttpResponseData> 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
/// <summary>
/// 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 <see cref="DataAccess.TableStorage.TableTenantNotificationRepository.DismissNotificationAsync"/>).
/// When per-user dismiss lands, this can drop to MemberRead.
/// </summary>
[Function("DismissTenantNotification")]
public async Task<HttpResponseData> DismissNotification(
@@ -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),
@@ -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 ?? "",
@@ -0,0 +1,38 @@
using System.Collections.Generic;
namespace AutopilotMonitor.Functions.Services;
/// <summary>
/// Visibility tier for a tenant notification. Compared with the caller's resolved tier
/// in <see cref="TenantNotificationService.GetActiveNotificationsAsync"/>:
/// a notification is visible iff <c>notificationAudience &lt;= callerAudience</c>.
/// </summary>
public enum NotificationAudience
{
/// <summary>Visible to every tenant member (Admin, Operator, Viewer) and Global Admins.</summary>
Member = 0,
/// <summary>Visible only to Tenant Admins and Global Admins.</summary>
Admin = 1,
}
/// <summary>
/// Single source of truth for which audience may see each tenant notification type.
/// Lookup is by notification <c>Type</c> string; unknown types default to <see cref="NotificationAudience.Admin"/>
/// so a newly introduced type fails closed (hidden from non-admins) until it is registered here.
/// </summary>
public static class TenantNotificationAudienceCatalog
{
private static readonly Dictionary<string, NotificationAudience> _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;
}
}
@@ -56,21 +56,28 @@ public class TenantNotificationService
}
}
public async Task<List<GlobalNotificationDto>> GetActiveNotificationsAsync(string tenantId)
/// <summary>
/// Returns active (non-dismissed) tenant notifications visible to a caller of the given audience tier.
/// A notification is included iff <c>TenantNotificationAudienceCatalog.Resolve(type) &lt;= callerAudience</c>:
/// Admin callers see Member + Admin notifications, Member callers see only Member notifications.
/// </summary>
public async Task<List<GlobalNotificationDto>> 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)
{
@@ -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
</button>
)}
<button onClick={() => { clearAll(); if (showGlobal) dismissAllGlobal(); if (tenantNotifications.length > 0) dismissAllTenant(); }} className="text-xs text-gray-500 hover:text-gray-700">
<button onClick={() => { clearAll(); if (showGlobal) dismissAllGlobal(); if (canDismissTenant && tenantNotifications.length > 0) dismissAllTenant(); }} className="text-xs text-gray-500 hover:text-gray-700">
Clear all
</button>
</div>
@@ -332,11 +336,13 @@ export default function Navbar() {
</div>
</div>
</div>
<button onClick={(e) => { e.stopPropagation(); dismissTenantNotification(tn.id); }} className="ml-2 text-gray-300 hover:text-gray-500" title="Dismiss">
<svg className="w-3.5 h-3.5" fill="none" strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" viewBox="0 0 24 24" stroke="currentColor">
<path d="M6 18L18 6M6 6l12 12"></path>
</svg>
</button>
{canDismissTenant && (
<button onClick={(e) => { e.stopPropagation(); dismissTenantNotification(tn.id); }} className="ml-2 text-gray-300 hover:text-gray-500" title="Dismiss">
<svg className="w-3.5 h-3.5" fill="none" strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" viewBox="0 0 24 24" stroke="currentColor">
<path d="M6 18L18 6M6 6l12 12"></path>
</svg>
</button>
)}
</div>
</div>
))}
@@ -671,9 +677,11 @@ export default function Navbar() {
<p className="text-xs font-medium text-gray-800 dark:text-gray-200 truncate">{n.title}</p>
<p className="text-[11px] text-gray-500 dark:text-gray-400 line-clamp-2">{n.message}</p>
</div>
<button onClick={() => dismissTenantNotification(n.id)} className="shrink-0 text-gray-300 hover:text-gray-500">
<svg className="w-3.5 h-3.5" fill="none" strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" viewBox="0 0 24 24" stroke="currentColor"><path d="M6 18L18 6M6 6l12 12"></path></svg>
</button>
{canDismissTenant && (
<button onClick={() => dismissTenantNotification(n.id)} className="shrink-0 text-gray-300 hover:text-gray-500">
<svg className="w-3.5 h-3.5" fill="none" strokeLinecap="round" strokeLinejoin="round" strokeWidth="2" viewBox="0 0 24 24" stroke="currentColor"><path d="M6 18L18 6M6 6l12 12"></path></svg>
</button>
)}
</div>
))}
{/* Global admin notifications */}
@@ -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<ReturnType<typeof setInterval> | 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));
@@ -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;
}
@@ -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);
});
});