Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/Humans.Application/Interfaces/ITeamResourceService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,15 @@ Task<IReadOnlyDictionary<Guid, TeamResourceSummary>> GetTeamResourceSummariesAsy
/// </summary>
Task UnlinkResourceAsync(Guid resourceId, CancellationToken ct = default);

/// <summary>
/// Deactivates every Google resource owned by a team (soft-delete: IsActive = false)
/// and writes an audit log entry for each. Called by <see cref="ITeamService"/> when a
/// team is soft-deleted so the downstream sync jobs stop provisioning access to its
/// Drive folders and Groups. The actual revocation of Drive permissions and Group
/// membership happens on the next sync tick via the normal remove-user paths.
/// </summary>
Task DeactivateResourcesForTeamAsync(Guid teamId, CancellationToken ct = default);

/// <summary>
/// Checks whether a user can manage resources for a team.
/// Board members can always manage. Leads can manage if the admin setting allows it.
Expand Down
19 changes: 19 additions & 0 deletions src/Humans.Infrastructure/Jobs/SystemTeamSyncJob.cs
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,25 @@ public async Task SyncBarrioLeadsMembershipForUserAsync(Guid userId, Cancellatio
.AsNoTracking()
.AnyAsync(l => l.UserId == userId && l.LeftAt == null, cancellationToken);

// Idempotency guard: if the user should be a member and already has an active
// team_members row, do nothing. This avoids unique-index violations
// (IX_team_members_active_unique) on the Barrio Leads team when the user is
// registering another camp and already has an active membership from a
// previous registration. Checks against the database directly rather than the
// filtered Include collection to be robust against any tracker staleness.
if (isLeadAnywhere)
{
var alreadyActive = await _dbContext.TeamMembers
.AsNoTracking()
.AnyAsync(
tm => tm.TeamId == team.Id && tm.UserId == userId && tm.LeftAt == null,
cancellationToken);
if (alreadyActive)
{
return;
}
}

var eligibleUserIds = isLeadAnywhere ? [userId] : new List<Guid>();
await SyncTeamMembershipAsync(team, eligibleUserIds, cancellationToken, singleUserSync: userId);
}
Expand Down
13 changes: 7 additions & 6 deletions src/Humans.Infrastructure/Services/GoogleWorkspaceSyncService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -773,7 +773,7 @@ public async Task AddUserToTeamResourcesAsync(
: null;

var resources = await _dbContext.GoogleResources
.Where(r => r.TeamId == teamId && r.IsActive)
.Where(r => r.TeamId == teamId && r.IsActive && r.Team.IsActive)
.ToListAsync(cancellationToken);

foreach (var resource in resources)
Expand Down Expand Up @@ -803,7 +803,7 @@ public async Task AddUserToTeamResourcesAsync(
if (team?.ParentTeamId is not null)
{
var parentResources = await _dbContext.GoogleResources
.Where(r => r.TeamId == team.ParentTeamId && r.IsActive)
.Where(r => r.TeamId == team.ParentTeamId && r.IsActive && r.Team.IsActive)
.ToListAsync(cancellationToken);

foreach (var resource in parentResources)
Expand Down Expand Up @@ -926,7 +926,7 @@ public async Task<SyncPreviewResult> SyncResourcesByTypeAsync(
.Include(r => r.Team)
.ThenInclude(t => t.Members.Where(tm => tm.LeftAt == null))
.ThenInclude(tm => tm.User)
.Where(r => r.ResourceType == resourceType && r.IsActive)
.Where(r => r.ResourceType == resourceType && r.IsActive && r.Team.IsActive)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep soft-deleted team resources in reconciliation sync

SyncResourcesByTypeAsync now filters resources with r.Team.IsActive, but DeleteTeamAsync marks the team inactive before the next sync tick (TeamService.DeleteTeamAsync). That means resources for deleted teams are excluded from the reconciliation pass, so the remove paths (RemoveUserFromGroupAsync / RemoveUserFromDriveAsync reached via execute sync actions) never run for those memberships. In practice, users who already had Google Group/Drive access can retain it after a soft-delete, which defeats the intended access revocation behavior.

Useful? React with 👍 / 👎.

.ToListAsync(cancellationToken);

var now = _clock.GetCurrentInstant();
Expand Down Expand Up @@ -1843,7 +1843,7 @@ public async Task<GroupSettingsDriftResult> CheckGroupSettingsAsync(Cancellation

var groupResources = await _dbContext.GoogleResources
.Include(r => r.Team)
.Where(r => r.ResourceType == GoogleResourceType.Group && r.IsActive)
.Where(r => r.ResourceType == GoogleResourceType.Group && r.IsActive && r.Team.IsActive)
.ToListAsync(cancellationToken);

_logger.LogInformation("Checking group settings for {Count} active Google Groups", groupResources.Count);
Expand Down Expand Up @@ -2297,7 +2297,7 @@ public async Task<AllGroupsResult> GetAllDomainGroupsAsync(CancellationToken can
public async Task<int> UpdateDriveFolderPathsAsync(CancellationToken cancellationToken = default)
{
var driveResources = await _dbContext.GoogleResources
.Where(r => r.ResourceType == GoogleResourceType.DriveFolder && r.IsActive)
.Where(r => r.ResourceType == GoogleResourceType.DriveFolder && r.IsActive && r.Team.IsActive)
.ToListAsync(cancellationToken);

if (driveResources.Count == 0)
Expand Down Expand Up @@ -2410,7 +2410,8 @@ public async Task<int> EnforceInheritedAccessRestrictionsAsync(CancellationToken
var restrictedResources = await _dbContext.GoogleResources
.Where(r => r.RestrictInheritedAccess
&& r.ResourceType == GoogleResourceType.DriveFolder
&& r.IsActive)
&& r.IsActive
&& r.Team.IsActive)
.ToListAsync(cancellationToken);

if (restrictedResources.Count == 0)
Expand Down
31 changes: 31 additions & 0 deletions src/Humans.Infrastructure/Services/StubTeamResourceService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,37 @@ public async Task UnlinkResourceAsync(Guid resourceId, CancellationToken ct = de
_logger.LogInformation("[STUB] Unlinked resource {ResourceId}", resourceId);
}

/// <inheritdoc />
public async Task DeactivateResourcesForTeamAsync(Guid teamId, CancellationToken ct = default)
{
var resources = await _dbContext.GoogleResources
.Where(r => r.TeamId == teamId && r.IsActive)
.ToListAsync(ct);

if (resources.Count == 0)
{
return;
}

var auditLogService = _serviceProvider.GetRequiredService<IAuditLogService>();

foreach (var resource in resources)
{
resource.IsActive = false;
await auditLogService.LogAsync(
AuditAction.GoogleResourceDeactivated,
nameof(GoogleResource),
resource.Id,
$"Resource '{resource.Name}' deactivated because owning team was soft-deleted.",
nameof(StubTeamResourceService));
}

await _dbContext.SaveChangesAsync(ct);
_logger.LogInformation(
"[STUB] Deactivated {Count} Google resources for soft-deleted team {TeamId}",
resources.Count, teamId);
}

/// <inheritdoc />
public async Task<bool> CanManageTeamResourcesAsync(Guid teamId, Guid userId, CancellationToken ct = default)
{
Expand Down
32 changes: 32 additions & 0 deletions src/Humans.Infrastructure/Services/TeamResourceService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -505,6 +505,38 @@ public async Task UnlinkResourceAsync(Guid resourceId, CancellationToken ct = de
_logger.LogInformation("Unlinked resource {ResourceId} ({ResourceName})", resourceId, resource.Name);
}

/// <inheritdoc />
public async Task DeactivateResourcesForTeamAsync(Guid teamId, CancellationToken ct = default)
{
var resources = await _dbContext.GoogleResources
.Where(r => r.TeamId == teamId && r.IsActive)
.ToListAsync(ct);

if (resources.Count == 0)
{
return;
}

var auditLogService = _serviceProvider.GetRequiredService<IAuditLogService>();

foreach (var resource in resources)
{
resource.IsActive = false;
await auditLogService.LogAsync(
AuditAction.GoogleResourceDeactivated,
nameof(GoogleResource),
resource.Id,
$"Resource '{resource.Name}' deactivated because owning team was soft-deleted.",
nameof(TeamResourceService));
}

await _dbContext.SaveChangesAsync(ct);

_logger.LogInformation(
"Deactivated {Count} Google resources for soft-deleted team {TeamId}",
resources.Count, teamId);
}

/// <inheritdoc />
public async Task<bool> CanManageTeamResourcesAsync(Guid teamId, Guid userId, CancellationToken ct = default)
{
Expand Down
25 changes: 23 additions & 2 deletions src/Humans.Infrastructure/Services/TeamService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -640,14 +640,35 @@ public async Task DeleteTeamAsync(Guid teamId, CancellationToken cancellationTok
throw new InvalidOperationException("Cannot deactivate a team that has active sub-teams. Remove or reassign sub-teams first.");
}

var now = _clock.GetCurrentInstant();

// Close out all active team memberships so downstream sync jobs stop treating
// this team's roster as current and can revoke Drive/Group access on their
// next tick. (See #494.)
var activeMembers = await _dbContext.TeamMembers
.Where(tm => tm.TeamId == teamId && tm.LeftAt == null)
.ToListAsync(cancellationToken);
foreach (var member in activeMembers)
{
member.LeftAt = now;
}

team.IsActive = false;
team.UpdatedAt = _clock.GetCurrentInstant();
team.UpdatedAt = now;

await _dbContext.SaveChangesAsync(cancellationToken);

// Deactivate the team's Google resources via the owning service so the bulk
// sync queries stop hitting them. Actual revocation of user permissions and
// group memberships happens on the next sync tick through the normal
// remove-user paths.
await TeamResourceService.DeactivateResourcesForTeamAsync(teamId, cancellationToken);

RemoveCachedTeam(teamId);

_logger.LogInformation("Deactivated team {TeamId} ({TeamName})", teamId, team.Name);
_logger.LogInformation(
"Deactivated team {TeamId} ({TeamName}); closed {MemberCount} memberships",
teamId, team.Name, activeMembers.Count);
}

public async Task<TeamJoinRequest> RequestToJoinTeamAsync(
Expand Down
13 changes: 13 additions & 0 deletions src/Humans.Web/Controllers/CampController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using Humans.Domain.Helpers;
using Humans.Domain.ValueObjects;
using Humans.Web.Models;
using Microsoft.EntityFrameworkCore;
using Microsoft.Extensions.Localization;
using NodaTime;

Expand Down Expand Up @@ -320,6 +321,18 @@ public async Task<IActionResult> Register(CampRegisterViewModel model)
await PopulateRegistrationInfoAsync();
return View(model);
}
catch (DbUpdateException ex)
{
// Belt-and-suspenders for any future race in downstream sync side effects
// (e.g. duplicate system-team memberships). The primary fix lives in the
// owning service; this ensures the user always sees a friendly error
// instead of a 500.
_logger.LogError(ex, "Camp registration failed with DB error for user {UserId} in year {Year}", user.Id, year);
ModelState.AddModelError(string.Empty, "We couldn't register your camp right now. Please try again, or contact an admin if the problem persists.");
await PopulateRegisterSeasonYearAsync();
await PopulateRegistrationInfoAsync();
return View(model);
}
}

// ======================================================================
Expand Down
Loading
Loading