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 <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
2026-09-07 00:15:21 +02:00
parent e7df69efe4
commit 7ef097529c
3 changed files with 75 additions and 4 deletions
@@ -77,7 +77,7 @@ public class RevokeEnforcementTests
}
private static (TenantGroupManagementFunction Fn, Mock<IAdminRepository> Repo,
Mock<IMaintenanceRepository> Audit, FakeSignalRNotificationService SignalR) BuildGroupFn()
Mock<IMaintenanceRepository> Audit, FakeSignalRNotificationService SignalR) BuildGroupFn(ProConferralService? proConferral = null)
{
var repo = new Mock<IAdminRepository>();
// 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<TenantGroupManagementFunction>.Instance, BuildService(repo), Resolver(), audit.Object, signalR, BuildSlots(repo),
TestProConferral.Inert());
proConferral ?? TestProConferral.Inert());
return (fn, repo, audit, signalR);
}
/// <summary>A recording conferral double (CallBase off, touches no storage): the group flows only need to prove WHAT they hand it.</summary>
private static Mock<ProConferralService> RecordingProConferral() =>
new(Mock.Of<IAdminRepository>(),
new TenantConfigurationService(Mock.Of<IConfigRepository>(), NullLogger<TenantConfigurationService>.Instance, new MemoryCache(new MemoryCacheOptions())),
ManagedTenantProIndex.None,
NullLogger<ProConferralService>.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<Dictionary<string, string>?>()), 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<string> { 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<string>(), It.IsAny<string>(), It.IsAny<string>()), 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<string> { 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<string>(), It.IsAny<string>(), It.IsAny<string>()), Times.Never);
}
}
@@ -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).
/// </summary>
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,