diff --git a/src/DodoSSH.Api/Features/Teams/TeamLock.cs b/src/DodoSSH.Api/Features/Teams/TeamLock.cs new file mode 100644 index 0000000..c4ee3ec --- /dev/null +++ b/src/DodoSSH.Api/Features/Teams/TeamLock.cs @@ -0,0 +1,27 @@ +using DodoSSH.Infrastructure; +using Microsoft.EntityFrameworkCore; + +namespace DodoSSH.Api.Features.Teams; + +/// +/// Serialises the writes that decide what a team owns, for the life of the transaction. +/// +/// +/// +/// Archiving a team refuses while it owns a vault, and creating a vault refuses on an archived team — but each +/// used to check outside any transaction the other could see, so the two could pass each other: the archive +/// counted no vaults, a create landed a moment later, and the team was archived with a live vault in it that +/// nobody could reach. Both now take this lock inside their transaction and check again under it. +/// +/// +/// The same advisory lock the sync push takes per vault, keyed apart so a team and a vault never share one. +/// Requires Multiplexing=false on the connection, as that one does. +/// +/// +internal static class TeamLock +{ + internal static Task AcquireAsync(DodoDbContext database, Guid teamId, CancellationToken cancellationToken) => + database.Database.ExecuteSqlAsync( + $"SELECT pg_advisory_xact_lock(hashtextextended({"team:" + teamId.ToString()}, 0))", + cancellationToken); +} diff --git a/src/DodoSSH.Api/Features/Teams/TeamService.cs b/src/DodoSSH.Api/Features/Teams/TeamService.cs index 0417432..44e8828 100644 --- a/src/DodoSSH.Api/Features/Teams/TeamService.cs +++ b/src/DodoSSH.Api/Features/Teams/TeamService.cs @@ -255,16 +255,6 @@ internal sealed class TeamService( throw new InvalidOperationException("Only the owner of a team can archive it."); } - var vaultCount = await CountVaultsAsync(team.Id, cancellationToken).ConfigureAwait(false); - - if (vaultCount > 0) - { - throw new TeamNotEmptyException( - string.Create( - CultureInfo.InvariantCulture, - $"This team still owns {vaultCount} vault(s), and archiving it would take them away from everybody holding a key — including you. Delete those vaults first, which asks about each one and withdraws every key to it.")); - } - var now = clock.GetUtcNow(); var archived = await database.ExecuteFreshAsync([team], async () => { @@ -273,6 +263,19 @@ internal sealed class TeamService( .ConfigureAwait(false); await using var _ = transaction.ConfigureAwait(false); + // Counted under the lock a vault create takes, so one cannot land between this and the archive. + await TeamLock.AcquireAsync(database, team.Id, cancellationToken).ConfigureAwait(false); + + var vaultCount = await CountVaultsAsync(team.Id, cancellationToken).ConfigureAwait(false); + + if (vaultCount > 0) + { + throw new TeamNotEmptyException( + string.Create( + CultureInfo.InvariantCulture, + $"This team still owns {vaultCount} vault(s), and archiving it would take them away from everybody holding a key — including you. Delete those vaults first, which asks about each one and withdraws every key to it.")); + } + var memberships = await database.TeamMemberships .Where(m => m.TeamId == team.Id && m.DeletedAtUtc == null) .ToListAsync(cancellationToken) diff --git a/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs b/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs index 3506c01..4587243 100644 --- a/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs +++ b/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs @@ -1,5 +1,6 @@ using System.Security.Cryptography; using DodoSSH.Api.Features.Events; +using DodoSSH.Api.Setup; using DodoSSH.Contracts; using DodoSSH.Domain; using DodoSSH.Infrastructure; @@ -76,19 +77,7 @@ internal sealed class VaultGrantService( "That vault id is already in use. Generate a new UUIDv7 and retry."); } - AddVaultWithSelfGrant(user, team, request, name, key, clock.GetUtcNow()); - - try - { - await database.SaveChangesAsync(cancellationToken).ConfigureAwait(false); - } - catch (DbUpdateException exception) when (IsUniqueViolation(exception)) - { - // The pre-check covers the ordinary case; this is the race between two creates choosing - // the same id, which must not surface as a 500 about a constraint. - throw new TeamInvalidException( - "That vault id is already in use. Generate a new UUIDv7 and retry."); - } + await WriteVaultAsync(user, team, request, name, key, cancellationToken).ConfigureAwait(false); TeamLog.TeamVaultCreated(logger, request.VaultId, team.Id, user.Id); @@ -103,6 +92,55 @@ internal sealed class VaultGrantService( RekeyRequired: false); } + /// Writes the vault and its first grant, under the team's lock, in one transaction. + private Task WriteVaultAsync( + UserAccount user, + Team team, + CreateTeamVaultRequest request, + string name, + UserKey key, + CancellationToken cancellationToken) => + database.ExecuteFreshAsync([team], async () => + { + var transaction = await database.Database + .BeginTransactionAsync(cancellationToken) + .ConfigureAwait(false); + await using var _ = transaction.ConfigureAwait(false); + + // Under the lock an archive takes, and the team re-read under it: see TeamLock. + await TeamLock.AcquireAsync(database, team.Id, cancellationToken).ConfigureAwait(false); + await RequireTeamStillLiveAsync(team.Id, cancellationToken).ConfigureAwait(false); + + AddVaultWithSelfGrant(user, team, request, name, key, clock.GetUtcNow()); + + try + { + await database.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + } + catch (DbUpdateException exception) when (IsUniqueViolation(exception)) + { + // The pre-check covers the ordinary case; this is the race between two creates choosing + // the same id, which must not surface as a 500 about a constraint. + throw new TeamInvalidException( + "That vault id is already in use. Generate a new UUIDv7 and retry."); + } + + await transaction.CommitAsync(cancellationToken).ConfigureAwait(false); + }); + + /// Refuses a vault for a team archived since the request resolved it. + private async Task RequireTeamStillLiveAsync(Guid teamId, CancellationToken cancellationToken) + { + var live = await database.Teams + .AnyAsync(t => t.Id == teamId && t.DeletedAtUtc == null, cancellationToken) + .ConfigureAwait(false); + + if (!live) + { + throw new TeamInvalidException("This team has been archived, so it can no longer own a new vault."); + } + } + /// Adds the vault row and the creator's own grant, in one unit of work. private void AddVaultWithSelfGrant( UserAccount user,