mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 12:42:38 +00:00
Push: a Global Admin's platform device also receives the alerts of the owner's home tenant (D-334)
A receiver holds one pairing, so a Global Admin whose phone is paired in the platform scope could not get their own tenant's enrollment alerts without replacing that pairing. The push transport now resolves a tenant channel's recipients as the tenant's Active devices plus the Active platform devices whose OwnerHomeTenantId is that tenant, filtered by the platform eligibility rule and deduplicated by endpoint; no other tenant reaches a platform device, there is no opt-in and no setting. The home tenant's device list shows such a row with PushDeviceDto.Scope = "platform" (manifest and wire types regenerated); the panel marks it with a Platform badge and no actions — it is managed under the platform's device list, and the tenant routes do not know the id. Tests: fan-out incl. a foreign home tenant, a former Global Admin and a shared endpoint; the list in the home tenant, the owner's and an operator's view, and the refusing tenant routes. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
@@ -38,7 +38,10 @@ public class PushDeliveryServiceTests
|
||||
var active = h.AddDevice("d1", Constants.Push.DeviceStatus.Active);
|
||||
h.AddDevice("d2", Constants.Push.DeviceStatus.Pending);
|
||||
h.AddDevice("d3", Constants.Push.DeviceStatus.Paused);
|
||||
h.AddDevice("d4", Constants.Push.DeviceStatus.Active, scope: Constants.Push.PlatformScope);
|
||||
// Another org's Global Admin: a tenant alert never reaches a platform device unless this is the
|
||||
// owner's home tenant (D-334, pinned below).
|
||||
h.AddDevice("d4", Constants.Push.DeviceStatus.Active, scope: Constants.Push.PlatformScope)
|
||||
.OwnerHomeTenantId = "22222222-2222-2222-2222-222222222222";
|
||||
|
||||
var stats = await h.Sut.SendCoreAsync(Tenant, Alert());
|
||||
|
||||
@@ -505,6 +508,63 @@ public class PushDeliveryServiceTests
|
||||
public void Dispose() { }
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Tenant_alert_also_reaches_platform_devices_whose_owners_home_tenant_it_is()
|
||||
{
|
||||
var h = new Harness();
|
||||
var own = h.AddDevice("d1", Constants.Push.DeviceStatus.Active);
|
||||
var ga = h.AddDevice("ga", Constants.Push.DeviceStatus.Active, upn: "ga@contoso.invalid", scope: Constants.Push.PlatformScope);
|
||||
h.AddDevice("ga-paused", Constants.Push.DeviceStatus.Paused, upn: "ga@contoso.invalid", scope: Constants.Push.PlatformScope);
|
||||
// Another Global Admin whose home tenant is a different one: never reached by this tenant.
|
||||
h.AddDevice("other-ga", Constants.Push.DeviceStatus.Active, upn: "other@contoso.invalid", scope: Constants.Push.PlatformScope)
|
||||
.OwnerHomeTenantId = "22222222-2222-2222-2222-222222222222";
|
||||
|
||||
var stats = await h.Sut.SendCoreAsync(Tenant, Alert());
|
||||
|
||||
Assert.Equal(2, stats.Targets);
|
||||
Assert.Equal(2, stats.Delivered);
|
||||
Assert.Equal(
|
||||
new[] { own.Endpoint, ga.Endpoint }.OrderBy(x => x),
|
||||
h.Http.Requests.Select(r => r.RequestUri!.ToString()).OrderBy(x => x));
|
||||
// The tenant row passes the tenant rule, the platform row the platform rule.
|
||||
h.Eligibility.Verify(e => e.IsEligibleAsync(Tenant, "admin@contoso.invalid", It.IsAny<string>(), TenantId), Times.Once);
|
||||
h.Eligibility.Verify(e => e.IsEligibleAsync(NotificationScope.Platform, "ga@contoso.invalid", It.IsAny<string>(), TenantId), Times.Once);
|
||||
h.Eligibility.Verify(e => e.IsEligibleAsync(Tenant, "ga@contoso.invalid", It.IsAny<string>(), It.IsAny<string>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Former_global_admins_platform_device_gets_nothing_from_its_home_tenant_and_a_platform_alert_never_pulls_tenant_rows()
|
||||
{
|
||||
var h = new Harness();
|
||||
var tenantRow = h.AddDevice("d1", Constants.Push.DeviceStatus.Active);
|
||||
var ga = h.AddDevice("ga", Constants.Push.DeviceStatus.Active, upn: "ga@contoso.invalid", scope: Constants.Push.PlatformScope);
|
||||
h.Eligibility.Setup(e => e.IsEligibleAsync(NotificationScope.Platform, "ga@contoso.invalid", It.IsAny<string>(), It.IsAny<string>())).ReturnsAsync(false);
|
||||
|
||||
var tenantStats = await h.Sut.SendCoreAsync(Tenant, Alert());
|
||||
Assert.Equal(1, tenantStats.Targets);
|
||||
Assert.Equal(tenantRow.Endpoint, Assert.Single(h.Http.Requests).RequestUri!.ToString());
|
||||
|
||||
h.Eligibility.Setup(e => e.IsEligibleAsync(NotificationScope.Platform, "ga@contoso.invalid", It.IsAny<string>(), It.IsAny<string>())).ReturnsAsync(true);
|
||||
var platformStats = await h.Sut.SendCoreAsync(NotificationScope.Platform, Alert());
|
||||
Assert.Equal(1, platformStats.Targets);
|
||||
Assert.Equal(2, h.Http.Requests.Count);
|
||||
Assert.Equal(ga.Endpoint, h.Http.Requests[1].RequestUri!.ToString());
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Same_endpoint_in_both_scopes_is_sent_to_once()
|
||||
{
|
||||
var h = new Harness();
|
||||
var tenantRow = h.AddDevice("d1", Constants.Push.DeviceStatus.Active, upn: "ga@contoso.invalid");
|
||||
h.AddDevice("ga", Constants.Push.DeviceStatus.Active, upn: "ga@contoso.invalid", scope: Constants.Push.PlatformScope)
|
||||
.Endpoint = tenantRow.Endpoint;
|
||||
|
||||
var stats = await h.Sut.SendCoreAsync(Tenant, Alert());
|
||||
|
||||
Assert.Equal(1, stats.Targets);
|
||||
Assert.Single(h.Http.Requests);
|
||||
}
|
||||
|
||||
private static NotificationAlert Alert() => new()
|
||||
{
|
||||
EventType = "enrollment_failed",
|
||||
|
||||
@@ -388,6 +388,50 @@ public class PushPairingServiceTests
|
||||
}
|
||||
|
||||
/// <summary>Records every request (and its body, read before the sender disposes it) and answers with a configurable status (201 by default).</summary>
|
||||
[Fact]
|
||||
public async Task Home_tenant_list_shows_a_global_admins_platform_device_which_the_tenant_routes_refuse()
|
||||
{
|
||||
var h = new Harness();
|
||||
var ga = new PushCaller("ga@contoso.invalid", "aaaaaaaa-0000-0000-0000-00000000000a", TenantId, IsScopeAdmin: true, IsGlobalAdmin: true);
|
||||
h.Repo.Devices[(Constants.Push.PlatformScope, "ga-phone")] = PlatformDevice("ga-phone", ga, TenantId, h.Keys.Active.Kid);
|
||||
h.Repo.Devices[(Constants.Push.PlatformScope, "other-phone")] = PlatformDevice("other-phone",
|
||||
new PushCaller("other@contoso.invalid", "aaaaaaaa-0000-0000-0000-00000000000b", "22222222-2222-2222-2222-222222222222", IsScopeAdmin: true, IsGlobalAdmin: true),
|
||||
"22222222-2222-2222-2222-222222222222", h.Keys.Active.Kid);
|
||||
|
||||
// Scope admins see it (as somebody else's device), the owner as their own, an operator not at all.
|
||||
var adminView = Assert.Single((await h.Sut.ListAsync(Tenant, Admin)).Devices);
|
||||
Assert.Equal("ga-phone", adminView.DeviceId);
|
||||
Assert.Equal("platform", adminView.Scope);
|
||||
Assert.False(adminView.IsOwn);
|
||||
Assert.True(Assert.Single((await h.Sut.ListAsync(Tenant, ga)).Devices).IsOwn);
|
||||
Assert.Empty((await h.Sut.ListAsync(Tenant, Operator)).Devices);
|
||||
// The platform list itself is untouched by the rule.
|
||||
Assert.All((await h.Sut.ListAsync(NotificationScope.Platform, ga)).Devices, d => Assert.Equal("platform", d.Scope));
|
||||
|
||||
// Managed under the platform's device list: the tenant routes do not know the row.
|
||||
Assert.Equal(PushOpError.NotFound, await h.Sut.DeleteAsync(Tenant, "ga-phone", Admin));
|
||||
Assert.False((await h.Sut.TestAsync(Tenant, "ga-phone", Admin)).Ok);
|
||||
Assert.Equal(2, h.Repo.Devices.Count);
|
||||
}
|
||||
|
||||
private static PushDevice PlatformDevice(string id, PushCaller owner, string homeTenantId, string kid) => new()
|
||||
{
|
||||
Scope = Constants.Push.PlatformScope,
|
||||
DeviceId = id,
|
||||
OwnerUpn = owner.Upn,
|
||||
OwnerObjectId = owner.ObjectId,
|
||||
OwnerHomeTenantId = homeTenantId,
|
||||
Endpoint = $"https://web.push.apple.com/QF/{id}",
|
||||
P256dh = "p",
|
||||
Auth = "a",
|
||||
DeviceSecretHash = "hash",
|
||||
VapidKid = kid,
|
||||
Label = id,
|
||||
Platform = "ios-homescreen",
|
||||
Status = Constants.Push.DeviceStatus.Active,
|
||||
PairedUtc = DateTime.UtcNow,
|
||||
};
|
||||
|
||||
internal sealed class RecordingHandler : HttpMessageHandler
|
||||
{
|
||||
public List<HttpRequestMessage> Requests { get; } = new();
|
||||
|
||||
@@ -133,13 +133,7 @@ public class PushDeliveryService : IPushChannelSender, IPushDeviceRevoker
|
||||
}
|
||||
|
||||
var now = DateTime.UtcNow;
|
||||
var candidates = (await _repo.GetDevicesAsync(scope.Key).ConfigureAwait(false))
|
||||
.Where(d => d.Status == Constants.Push.DeviceStatus.Active && d.Kind == "webpush")
|
||||
.ToList();
|
||||
if (candidates.Count == 0)
|
||||
return stats;
|
||||
|
||||
var recipients = await FilterEligibleAsync(scope, candidates, now).ConfigureAwait(false);
|
||||
var recipients = await ResolveRecipientsAsync(scope, now).ConfigureAwait(false);
|
||||
if (recipients.Count == 0)
|
||||
return stats;
|
||||
|
||||
@@ -177,6 +171,39 @@ public class PushDeliveryService : IPushChannelSender, IPushDeviceRevoker
|
||||
return stats;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The Active devices an alert in <paramref name="scope"/> reaches, role and freshness re-checked
|
||||
/// per owner (D-327). A tenant scope additionally reaches the Active platform devices whose
|
||||
/// owner's home tenant it is (D-334): a Global Admin's receiver holds one pairing, and in their
|
||||
/// own tenant they are an admin like any other — those rows pass the platform eligibility rule,
|
||||
/// and one endpoint is sent to once whichever scope its row lives in. No other tenant ever
|
||||
/// reaches a platform device.
|
||||
/// </summary>
|
||||
internal async Task<List<PushDevice>> ResolveRecipientsAsync(NotificationScope scope, DateTime now)
|
||||
{
|
||||
var own = ActiveWebPush(await _repo.GetDevicesAsync(scope.Key).ConfigureAwait(false));
|
||||
var recipients = own.Count == 0 ? new List<PushDevice>() : await FilterEligibleAsync(scope, own, now).ConfigureAwait(false);
|
||||
if (scope.IsPlatform)
|
||||
return recipients;
|
||||
|
||||
var homeTenantRows = ActiveWebPush(await _repo.GetDevicesAsync(NotificationScope.Platform.Key).ConfigureAwait(false))
|
||||
.Where(d => string.Equals(d.OwnerHomeTenantId, scope.Key, StringComparison.OrdinalIgnoreCase))
|
||||
.ToList();
|
||||
if (homeTenantRows.Count == 0)
|
||||
return recipients;
|
||||
|
||||
var endpoints = new HashSet<string>(recipients.Select(d => d.Endpoint), StringComparer.Ordinal);
|
||||
foreach (var device in await FilterEligibleAsync(NotificationScope.Platform, homeTenantRows, now).ConfigureAwait(false))
|
||||
{
|
||||
if (endpoints.Add(device.Endpoint))
|
||||
recipients.Add(device);
|
||||
}
|
||||
return recipients;
|
||||
}
|
||||
|
||||
private static List<PushDevice> ActiveWebPush(List<PushDevice> devices)
|
||||
=> devices.Where(d => d.Status == Constants.Push.DeviceStatus.Active && d.Kind == "webpush").ToList();
|
||||
|
||||
/// <summary>One push to one device, outside the flood window: test, paired, revoked, paused, session watch.</summary>
|
||||
public async Task<PushOutcome> SendSystemAsync(PushDevice device, PushPayload payload, TimeSpan? ttl = null)
|
||||
{
|
||||
|
||||
@@ -398,6 +398,14 @@ public class PushPairingService
|
||||
public async Task<PushDeviceListResponse> ListAsync(NotificationScope scope, PushCaller caller)
|
||||
{
|
||||
var devices = await _repo.GetDevicesAsync(scope.Key).ConfigureAwait(false);
|
||||
// A Global Admin's platform device is listed in their home tenant as well: it receives this
|
||||
// tenant's alerts without a second pairing (D-334), so it belongs where the recipients show.
|
||||
// The tenant routes never act on it (DELETE/test look the id up in the tenant partition only).
|
||||
if (!scope.IsPlatform)
|
||||
{
|
||||
devices.AddRange((await _repo.GetDevicesAsync(NotificationScope.Platform.Key).ConfigureAwait(false))
|
||||
.Where(d => string.Equals(d.OwnerHomeTenantId, scope.Key, StringComparison.OrdinalIgnoreCase)));
|
||||
}
|
||||
var items = devices
|
||||
.Where(d => d.Status != Constants.Push.DeviceStatus.Pending)
|
||||
.Where(d => caller.IsScopeAdmin || IsOwn(d, caller))
|
||||
@@ -415,6 +423,7 @@ public class PushPairingService
|
||||
LastOpenedUtc = d.LastOpenedUtc,
|
||||
OwnerUpn = d.OwnerUpn,
|
||||
IsOwn = IsOwn(d, caller),
|
||||
Scope = d.Scope == Constants.Push.PlatformScope ? "platform" : "tenant",
|
||||
})
|
||||
.ToList();
|
||||
return new PushDeviceListResponse { Devices = items };
|
||||
|
||||
@@ -3331,6 +3331,8 @@ export interface PushDeviceDto {
|
||||
ownerUpn: string;
|
||||
/** True for the caller's own devices. */
|
||||
isOwn: boolean;
|
||||
/** "tenant": paired in this scope. "platform": a Global Administrator's platform device, listed in their home tenant because it receives this tenant's alerts without a second pairing (D-334); the tenant routes do not act on it — it is managed under the platform's device list. */
|
||||
scope: string;
|
||||
}
|
||||
|
||||
/** GET push/devices · GET global/push/devices — own devices (everyone) plus every device of the scope (admins). */
|
||||
|
||||
@@ -133,6 +133,12 @@ namespace AutopilotMonitor.Shared.Models
|
||||
public string OwnerUpn { get; set; } = default!;
|
||||
/// <summary>True for the caller's own devices.</summary>
|
||||
public bool IsOwn { get; set; }
|
||||
/// <summary>
|
||||
/// "tenant": paired in this scope. "platform": a Global Administrator's platform device, listed
|
||||
/// in their home tenant because it receives this tenant's alerts without a second pairing
|
||||
/// (D-334); the tenant routes do not act on it — it is managed under the platform's device list.
|
||||
/// </summary>
|
||||
public string Scope { get; set; } = "tenant";
|
||||
}
|
||||
|
||||
/// <summary>GET/PUT/DELETE sessions/{sessionId}/watch — "notify my devices when this session ends".</summary>
|
||||
|
||||
@@ -167,10 +167,21 @@ export function PushDevicesPanel({ scope }: PushDevicesPanelProps) {
|
||||
const confirming = confirmRemoveId === device.deviceId;
|
||||
const busy = removingId === device.deviceId || testingId === device.deviceId;
|
||||
const canTest = device.status !== "Pending";
|
||||
// A Global Administrator's platform device shown in their home tenant (D-334): it receives
|
||||
// this tenant's alerts without a second pairing and is managed on the platform side.
|
||||
const viaPlatform = device.scope === "platform";
|
||||
return (
|
||||
<li key={device.deviceId} className="py-2 flex flex-wrap items-center gap-x-3 gap-y-1 text-sm">
|
||||
<span className="font-medium text-gray-900 min-w-0 break-words">{device.label || "Device"}</span>
|
||||
<span className="text-gray-500">{platformLabel(device.platform)}</span>
|
||||
{viaPlatform && (
|
||||
<span
|
||||
className="inline-flex items-center px-2 py-0.5 rounded-full text-xs font-medium bg-amber-100 text-amber-800"
|
||||
title="A Global Administrator's platform device: this is the owner's home tenant, so it receives this tenant's alerts without a second pairing."
|
||||
>
|
||||
Platform
|
||||
</span>
|
||||
)}
|
||||
<span
|
||||
className={`inline-flex items-center px-2 py-0.5 rounded-full text-xs font-medium ${CHIP_CLASSES[tone]}`}
|
||||
title={describePortalDeviceStatus(device.status)}
|
||||
@@ -184,49 +195,53 @@ export function PushDevicesPanel({ scope }: PushDevicesPanelProps) {
|
||||
{/* The owner is named only for someone else's device — a scope admin's view. */}
|
||||
{!device.isOwn ? ` · ${device.ownerUpn}` : ""}
|
||||
</span>
|
||||
<span className="flex flex-wrap items-center gap-2 sm:ml-auto">
|
||||
{confirming ? (
|
||||
<>
|
||||
<span className="text-xs text-gray-600">{`Remove ${device.label || "this device"}?`}</span>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void remove(device)}
|
||||
disabled={busy}
|
||||
className="px-3 py-1 text-sm bg-red-600 text-white rounded hover:bg-red-700 disabled:opacity-50 disabled:cursor-not-allowed transition-colors"
|
||||
>
|
||||
{removingId === device.deviceId ? "Removing…" : "Remove"}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setConfirmRemoveId(null)}
|
||||
disabled={removingId === device.deviceId}
|
||||
className="px-2 py-1 text-sm text-gray-600 hover:text-gray-800 disabled:opacity-50"
|
||||
>
|
||||
Cancel
|
||||
</button>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void sendTest(device)}
|
||||
disabled={busy || !canTest}
|
||||
title={canTest ? "Sends a test alert to this device." : "Only an active device can receive a test."}
|
||||
className="px-3 py-1 text-sm border border-gray-300 rounded text-gray-700 bg-white hover:bg-gray-50 disabled:opacity-50 disabled:cursor-not-allowed transition-colors"
|
||||
>
|
||||
{testingId === device.deviceId ? "Sending…" : "Send test"}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setConfirmRemoveId(device.deviceId)}
|
||||
disabled={busy}
|
||||
className="px-3 py-1 text-sm text-gray-600 hover:text-red-600 disabled:opacity-50 transition-colors"
|
||||
>
|
||||
Remove
|
||||
</button>
|
||||
</>
|
||||
)}
|
||||
</span>
|
||||
{viaPlatform ? (
|
||||
<span className="text-xs text-gray-500 sm:ml-auto">Managed under Platform › Push devices</span>
|
||||
) : (
|
||||
<span className="flex flex-wrap items-center gap-2 sm:ml-auto">
|
||||
{confirming ? (
|
||||
<>
|
||||
<span className="text-xs text-gray-600">{`Remove ${device.label || "this device"}?`}</span>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void remove(device)}
|
||||
disabled={busy}
|
||||
className="px-3 py-1 text-sm bg-red-600 text-white rounded hover:bg-red-700 disabled:opacity-50 disabled:cursor-not-allowed transition-colors"
|
||||
>
|
||||
{removingId === device.deviceId ? "Removing…" : "Remove"}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setConfirmRemoveId(null)}
|
||||
disabled={removingId === device.deviceId}
|
||||
className="px-2 py-1 text-sm text-gray-600 hover:text-gray-800 disabled:opacity-50"
|
||||
>
|
||||
Cancel
|
||||
</button>
|
||||
</>
|
||||
) : (
|
||||
<>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => void sendTest(device)}
|
||||
disabled={busy || !canTest}
|
||||
title={canTest ? "Sends a test alert to this device." : "Only an active device can receive a test."}
|
||||
className="px-3 py-1 text-sm border border-gray-300 rounded text-gray-700 bg-white hover:bg-gray-50 disabled:opacity-50 disabled:cursor-not-allowed transition-colors"
|
||||
>
|
||||
{testingId === device.deviceId ? "Sending…" : "Send test"}
|
||||
</button>
|
||||
<button
|
||||
type="button"
|
||||
onClick={() => setConfirmRemoveId(device.deviceId)}
|
||||
disabled={busy}
|
||||
className="px-3 py-1 text-sm text-gray-600 hover:text-red-600 disabled:opacity-50 transition-colors"
|
||||
>
|
||||
Remove
|
||||
</button>
|
||||
</>
|
||||
)}
|
||||
</span>
|
||||
)}
|
||||
</li>
|
||||
);
|
||||
})}
|
||||
|
||||
@@ -15564,6 +15564,14 @@
|
||||
"name": "boolean"
|
||||
},
|
||||
"doc": "True for the caller's own devices."
|
||||
},
|
||||
{
|
||||
"name": "scope",
|
||||
"type": {
|
||||
"kind": "primitive",
|
||||
"name": "string"
|
||||
},
|
||||
"doc": "\"tenant\": paired in this scope. \"platform\": a Global Administrator's platform device, listed in their home tenant because it receives this tenant's alerts without a second pairing (D-334); the tenant routes do not act on it — it is managed under the platform's device list."
|
||||
}
|
||||
]
|
||||
},
|
||||
|
||||
@@ -3331,6 +3331,8 @@ export interface PushDeviceDto {
|
||||
ownerUpn: string;
|
||||
/** True for the caller's own devices. */
|
||||
isOwn: boolean;
|
||||
/** "tenant": paired in this scope. "platform": a Global Administrator's platform device, listed in their home tenant because it receives this tenant's alerts without a second pairing (D-334); the tenant routes do not act on it — it is managed under the platform's device list. */
|
||||
scope: string;
|
||||
}
|
||||
|
||||
/** GET push/devices · GET global/push/devices — own devices (everyone) plus every device of the scope (admins). */
|
||||
|
||||
Reference in New Issue
Block a user