From 7ef097529cdf838e96ef7992b069bf0f0aaadcd4 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Mon, 7 Sep 2026 00:15:21 +0200 Subject: [PATCH] backend: deleting an owned tenant group stamps the retention grace anchor on every member (conferred Pro ended without ProDowngradedUtc) The portal hides Delete for owned (msp-{home}) groups, but DELETE global/tenant-groups/{groupId} accepts any id: a Global Admin deleting one ended Pro (MSP) for every member with no grace anchor, so a member that raised its retention under Pro was clamped to the Community cap at the next sweep. DeleteGroupCoreAsync now snapshots the owner and calls ProConferralService.RecordLossAsync(member, owner, "group-deleted") per member, owner excluded, independent of assignees. Two tests in RevokeEnforcementTests; internal docs list the fourth writer. Co-Authored-By: Claude Fable 5.1 --- internal | 2 +- .../RevokeEnforcementTests.cs | 60 ++++++++++++++++++- .../Admin/TenantGroupManagementFunction.cs | 17 +++++- 3 files changed, 75 insertions(+), 4 deletions(-) diff --git a/internal b/internal index 2b113df6..fdacc10c 160000 --- a/internal +++ b/internal @@ -1 +1 @@ -Subproject commit 2b113df6a146d119f5ad84109c526cd4d9412b37 +Subproject commit fdacc10cbd0539986ca1999358c292046a8e30a5 diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/RevokeEnforcementTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/RevokeEnforcementTests.cs index c47676a9..990b0208 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/RevokeEnforcementTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/RevokeEnforcementTests.cs @@ -77,7 +77,7 @@ public class RevokeEnforcementTests } private static (TenantGroupManagementFunction Fn, Mock Repo, - Mock Audit, FakeSignalRNotificationService SignalR) BuildGroupFn() + Mock Audit, FakeSignalRNotificationService SignalR) BuildGroupFn(ProConferralService? proConferral = null) { var repo = new Mock(); // Loose-mock defaults return a null Task — every repo read the flows touch needs a concrete setup. @@ -90,10 +90,17 @@ public class RevokeEnforcementTests var signalR = new FakeSignalRNotificationService(); var fn = new TenantGroupManagementFunction( NullLogger.Instance, BuildService(repo), Resolver(), audit.Object, signalR, BuildSlots(repo), - TestProConferral.Inert()); + proConferral ?? TestProConferral.Inert()); return (fn, repo, audit, signalR); } + /// A recording conferral double (CallBase off, touches no storage): the group flows only need to prove WHAT they hand it. + private static Mock RecordingProConferral() => + new(Mock.Of(), + new TenantConfigurationService(Mock.Of(), NullLogger.Instance, new MemoryCache(new MemoryCacheOptions())), + ManagedTenantProIndex.None, + NullLogger.Instance) { CallBase = false }; + // --- ChargeHomeTenantQuota flip: audited under every tenant, no-op on an unchanged value --- [Fact] @@ -362,4 +369,53 @@ public class RevokeEnforcementTests TenantA, "DELETE", "DelegatedGroupAccess", "*", "ga@vendor.example", It.IsAny?>()), Times.Once); } + + // --- Group delete ends conferred Pro: every member of an OWNED group gets its grace anchor --- + + private const string OwnerTenant = "33333333-3333-3333-3333-333333333333"; + + [Fact] + public async Task DeleteGroupCore_OwnedGroup_RecordsLossForEveryMember_EvenWithoutAssignees() + { + var proConferral = RecordingProConferral(); + var (fn, repo, audit, _) = BuildGroupFn(proConferral.Object); + var ownedGroupId = Constants.TenantGroupIds.ForHomeTenant(OwnerTenant); + repo.Setup(r => r.GetTenantGroupAsync(ownedGroupId)).ReturnsAsync(new TenantGroup + { + GroupId = ownedGroupId, + Name = "Customers of owner", + OwnerTenantId = OwnerTenant, + TenantIds = new List { TenantA, TenantB, OwnerTenant }, + AssigneeCount = 0, + }); + repo.Setup(r => r.DeleteTenantGroupAsync(ownedGroupId)).ReturnsAsync(true); + + await fn.DeleteGroupCoreAsync(ownedGroupId, "ga@vendor.example"); + + // The anchor does not depend on assignees (no assignee ⇒ no access audit, but Pro still ends); + // the owner itself is never a managed tenant of its own group. + proConferral.Verify(p => p.RecordLossAsync(TenantA, OwnerTenant, "group-deleted"), Times.Once); + proConferral.Verify(p => p.RecordLossAsync(TenantB, OwnerTenant, "group-deleted"), Times.Once); + proConferral.Verify(p => p.RecordLossAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(2)); + VerifyNoAudit(audit); + } + + [Fact] + public async Task DeleteGroupCore_OperatorGroup_RecordsNoLoss() + { + var proConferral = RecordingProConferral(); + var (fn, repo, _, _) = BuildGroupFn(proConferral.Object); + repo.Setup(r => r.GetTenantGroupAsync(GroupId)).ReturnsAsync(new TenantGroup + { + GroupId = GroupId, + Name = "MSP Customers", + TenantIds = new List { TenantA }, + AssigneeCount = 0, + }); + repo.Setup(r => r.DeleteTenantGroupAsync(GroupId)).ReturnsAsync(true); + + await fn.DeleteGroupCoreAsync(GroupId, "ga@vendor.example"); + + proConferral.Verify(p => p.RecordLossAsync(It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + } } diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Admin/TenantGroupManagementFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Admin/TenantGroupManagementFunction.cs index f14ce1ce..08d556aa 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Admin/TenantGroupManagementFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Admin/TenantGroupManagementFunction.cs @@ -180,17 +180,32 @@ public class TenantGroupManagementFunction /// BEFORE the delete removes them, so the bulk access removal is audited under each affected /// tenant and EVERY (former) assignee's live streams are cut — group authorization is join-time /// only, so without the kick they keep receiving the managed tenants' telemetry until their - /// connection drops. + /// connection drops. Deleting an OWNED group also ends conferred Pro for every member at once, + /// so each member's retention grace anchor is stamped (the projection leaves no trace by itself). /// internal async Task DeleteGroupCoreAsync(string groupId, string? currentUpn) { var group = await _delegatedAdminService.GetGroupAsync(groupId); var assignees = await _delegatedAdminService.GetGroupAssigneesAsync(groupId); + var owner = string.IsNullOrWhiteSpace(group?.OwnerTenantId) ? null : group!.OwnerTenantId; await _delegatedAdminService.DeleteGroupAsync(groupId); foreach (var assignee in assignees) await _signalRService.DisconnectUserAsync(assignee.Upn); + // The portal hides Delete for owned groups, but the route accepts any group id — without the + // anchor a member that raised its retention under Pro (MSP) would be clamped to the Community + // cap at the next sweep instead of getting the grace period. + if (owner != null) + { + foreach (var member in group!.TenantIds) + { + if (string.Equals(member, owner, StringComparison.OrdinalIgnoreCase)) + continue; + await _proConferral.RecordLossAsync(member, owner, "group-deleted"); + } + } + if (group != null && group.AssigneeCount > 0) { await AuditPerTenantAsync(group.TenantIds, "DELETE", "*", currentUpn,