diff --git a/src/DodoSSH.Api/Features/Events/VaultEventFanOut.cs b/src/DodoSSH.Api/Features/Events/VaultEventFanOut.cs new file mode 100644 index 0000000..c63aaf6 --- /dev/null +++ b/src/DodoSSH.Api/Features/Events/VaultEventFanOut.cs @@ -0,0 +1,24 @@ +using DodoSSH.Api.Features.Relay; + +namespace DodoSSH.Api.Features.Events; + +/// +/// Announces a change to everything on this node that acts on one: the event sockets, and the relay. +/// +/// +/// What the write paths are given as their . They announce a change once and +/// do not know who listens, which is the point: the relay learning about removals from teams did not have to +/// touch any of the places a removal happens. +/// +internal sealed class VaultEventFanOut(VaultEventHub hub, RelayAccessWatch relay) : IVaultEventPublisher +{ + /// + public void VaultChanged(Guid vaultId, long sequence) => hub.VaultChanged(vaultId, sequence); + + /// + public void VaultAccessChanged(Guid userId) + { + hub.VaultAccessChanged(userId); + _ = relay.AccessChangedAsync(userId); + } +} diff --git a/src/DodoSSH.Api/Features/Identity/EnrollmentService.cs b/src/DodoSSH.Api/Features/Identity/EnrollmentService.cs index 8e61083..7bf49c5 100644 --- a/src/DodoSSH.Api/Features/Identity/EnrollmentService.cs +++ b/src/DodoSSH.Api/Features/Identity/EnrollmentService.cs @@ -1,5 +1,6 @@ using System.Security.Cryptography; using System.Text.Json; +using DodoSSH.Api.Setup; using DodoSSH.Contracts; using DodoSSH.Crypto; using DodoSSH.Domain; @@ -106,9 +107,7 @@ internal sealed class EnrollmentService( byte[] fingerprint, CancellationToken cancellationToken) { - var strategy = database.Database.CreateExecutionStrategy(); - - return await strategy.ExecuteAsync(async () => + return await database.ExecuteFreshAsync([user], async () => { var transaction = await database.Database .BeginTransactionAsync(cancellationToken) diff --git a/src/DodoSSH.Api/Features/Relay/RelayAccessWatch.cs b/src/DodoSSH.Api/Features/Relay/RelayAccessWatch.cs new file mode 100644 index 0000000..72fe818 --- /dev/null +++ b/src/DodoSSH.Api/Features/Relay/RelayAccessWatch.cs @@ -0,0 +1,62 @@ +using DodoSSH.Api.Authorization; +using DodoSSH.Domain.Authorization; + +namespace DodoSSH.Api.Features.Relay; + +/// +/// Ends a user's relay sessions to vaults they can no longer reach, when their access changes. +/// +/// +/// +/// A relay session is authorised once, when its ticket is issued, and used to run on for as long as it +/// carried bytes — up to the maximum lifetime, twelve hours by default. Removing somebody from a team took +/// their keys and their vault list away at once, and left any shell they had open through the relay running +/// into the team's machines until it went idle. +/// +/// +/// Told by the same announcement that tells their event socket to re-read its vaults — see +/// — and it asks the same question the ticket +/// endpoint asked, against the database as it now stands. Off the announcing request, because that request +/// has already committed and answered, and a slow re-check must not hold it. +/// +/// +internal sealed class RelayAccessWatch( + RelaySessionRegistry sessions, + IServiceScopeFactory scopes, + ILogger logger) +{ + /// Re-checks every session the user has open, in the background. + /// The re-check, for a caller that wants to wait for it; the announcement does not. + internal Task AccessChangedAsync(Guid userId) + { + var open = sessions.SessionsOf(userId); + + return open.Count == 0 ? Task.CompletedTask : Task.Run(() => RecheckAsync(userId, open)); + } + + private async Task RecheckAsync(Guid userId, IReadOnlyList open) + { + try + { + var scope = scopes.CreateAsyncScope(); + await using var _ = scope.ConfigureAwait(false); + + var access = scope.ServiceProvider.GetRequiredService(); + + foreach (var lease in open) + { + var resolved = await access.ResolveAsync(userId, lease.VaultId, CancellationToken.None) + .ConfigureAwait(false); + + if (!resolved.Granted || !resolved.Permissions.HasFlag(PermissionFlags.Connect)) + { + lease.Revoke(); + } + } + } + catch (Exception exception) when (exception is not OutOfMemoryException) + { + RelayLog.AccessRecheckFailed(logger, userId, exception); + } + } +} diff --git a/src/DodoSSH.Api/Features/Relay/RelayConnectEndpoint.cs b/src/DodoSSH.Api/Features/Relay/RelayConnectEndpoint.cs index d440f87..a964adb 100644 --- a/src/DodoSSH.Api/Features/Relay/RelayConnectEndpoint.cs +++ b/src/DodoSSH.Api/Features/Relay/RelayConnectEndpoint.cs @@ -83,7 +83,7 @@ internal sealed class RelayConnectEndpoint( return; } - var lease = sessions.TryAdmit(ticket.UserId); + var lease = sessions.TryAdmit(ticket.UserId, ticket.VaultId); try { @@ -113,7 +113,7 @@ internal sealed class RelayConnectEndpoint( return; } - await RelayAsync(socket, ticket, ct).ConfigureAwait(false); + await RelayAsync(socket, ticket, lease.Revoked, ct).ConfigureAwait(false); } finally { @@ -239,7 +239,11 @@ internal sealed class RelayConnectEndpoint( } /// Dials the target, pipes until something ends it, and writes down what happened. - private async Task RelayAsync(WebSocket socket, RelayTicket ticket, CancellationToken requestAborted) + private async Task RelayAsync( + WebSocket socket, + RelayTicket ticket, + CancellationToken revoked, + CancellationToken requestAborted) { var started = clock.GetUtcNow(); @@ -269,7 +273,7 @@ internal sealed class RelayConnectEndpoint( { RelayLog.SessionOpened(logger, ticket.SessionId, ticket.UserId, ticket.HostId, sessions.Count); - ending = await PumpAsync(socket, target, counters, started, requestAborted).ConfigureAwait(false); + ending = await PumpAsync(socket, target, counters, started, revoked, requestAborted).ConfigureAwait(false); } RelayLog.SessionClosed( @@ -338,13 +342,14 @@ internal sealed class RelayConnectEndpoint( Stream target, Counters counters, DateTimeOffset started, + CancellationToken revoked, CancellationToken requestAborted) { using var pump = CancellationTokenSource.CreateLinkedTokenSource(requestAborted); var upstream = ClientToTargetAsync(socket, target, counters, pump.Token); var downstream = TargetToClientAsync(target, socket, counters, pump.Token); - var minding = MindAsync(counters, started, pump.Token); + var minding = MindAsync(counters, started, revoked, pump.Token); var first = await Task.WhenAny(upstream, downstream, minding).ConfigureAwait(false); var ending = await first.ConfigureAwait(false) ?? new Ending( @@ -512,7 +517,11 @@ internal sealed class RelayConnectEndpoint( /// count; SSH's own keepalives do, which is how a client that wants an idle shell kept open says so. /// /// - private async Task MindAsync(Counters counters, DateTimeOffset started, CancellationToken cancellationToken) + private async Task MindAsync( + Counters counters, + DateTimeOffset started, + CancellationToken revoked, + CancellationToken cancellationToken) { var settings = options.Value; var hardDeadline = started + settings.MaxSessionDuration; @@ -527,27 +536,11 @@ internal sealed class RelayConnectEndpoint( drainDeadline = now + settings.DrainTimeout; } - if (now >= hardDeadline) - { - return new Ending( - RelayCloseReason.DurationLimit, - RelayProtocol.DurationLimitCloseCode, - "This relay session reached its maximum lifetime."); - } - - if (drainDeadline is { } drain && now >= drain) - { - return ShuttingDown; - } - var idleDeadline = counters.LastActivity + settings.IdleTimeout; - if (now >= idleDeadline) + if (Due(now, revoked.IsCancellationRequested, hardDeadline, drainDeadline, idleDeadline) is { } ending) { - return new Ending( - RelayCloseReason.IdleTimeout, - RelayProtocol.IdleTimeoutCloseCode, - "This relay session carried nothing for the idle timeout."); + return ending; } var next = new[] { hardDeadline, idleDeadline, drainDeadline ?? DateTimeOffset.MaxValue }.Min(); @@ -555,8 +548,8 @@ internal sealed class RelayConnectEndpoint( // Woken early by the start of a shutdown, so the drain is measured from when it began rather // than from whenever the idle clock next came round. using var wake = drainDeadline is null - ? CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, lifetime.ApplicationStopping) - : CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + ? CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, revoked, lifetime.ApplicationStopping) + : CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, revoked); try { @@ -564,7 +557,7 @@ internal sealed class RelayConnectEndpoint( } catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) { - // The shutdown began. Round again to start the drain. + // The shutdown began, or access was withdrawn. Round again to act on whichever it was. } catch (OperationCanceledException) { @@ -575,6 +568,50 @@ internal sealed class RelayConnectEndpoint( return null; } + /// Whichever reason to end the session has come due, or null while none has. + /// + /// Revocation first, because it is the one reason here that is about the user rather than the session: + /// they were removed from the team, or the vault was taken from them, after this session was admitted. + /// It used to run on for as long as it carried bytes, up to the maximum lifetime. + /// + private static Ending? Due( + DateTimeOffset now, + bool revoked, + DateTimeOffset hardDeadline, + DateTimeOffset? drainDeadline, + DateTimeOffset idleDeadline) + { + if (revoked) + { + return AccessRevoked; + } + + if (now >= hardDeadline) + { + return new Ending( + RelayCloseReason.DurationLimit, + RelayProtocol.DurationLimitCloseCode, + "This relay session reached its maximum lifetime."); + } + + if (drainDeadline is { } drain && now >= drain) + { + return ShuttingDown; + } + + return now >= idleDeadline + ? new Ending( + RelayCloseReason.IdleTimeout, + RelayProtocol.IdleTimeoutCloseCode, + "This relay session carried nothing for the idle timeout.") + : null; + } + + private static Ending AccessRevoked { get; } = new( + RelayCloseReason.AccessRevoked, + RelayProtocol.AccessRevokedCloseCode, + "You no longer have access to this host."); + private static Ending ClientGone { get; } = new(RelayCloseReason.ClientClosed, (int)WebSocketCloseStatus.NormalClosure, string.Empty); diff --git a/src/DodoSSH.Api/Features/Relay/RelayLog.cs b/src/DodoSSH.Api/Features/Relay/RelayLog.cs index 3ea1b46..463faf1 100644 --- a/src/DodoSSH.Api/Features/Relay/RelayLog.cs +++ b/src/DodoSSH.Api/Features/Relay/RelayLog.cs @@ -91,4 +91,14 @@ internal static partial class RelayLog Level = LogLevel.Warning, Message = "Relay session {SessionId} ended but its audit record could not be closed.")] internal static partial void AuditCloseFailed(ILogger logger, Guid sessionId, Exception exception); + + /// + /// A warning: the sessions it was checking stay open, which is how they all behaved before the check + /// existed, and each still ends at its idle timeout or its lifetime. + /// + [LoggerMessage( + EventId = 2309, + Level = LogLevel.Warning, + Message = "Re-checking relay access for user {UserId} after an access change failed.")] + internal static partial void AccessRecheckFailed(ILogger logger, Guid userId, Exception exception); } diff --git a/src/DodoSSH.Api/Features/Relay/RelaySessionRegistry.cs b/src/DodoSSH.Api/Features/Relay/RelaySessionRegistry.cs index a2b3514..68c2a6d 100644 --- a/src/DodoSSH.Api/Features/Relay/RelaySessionRegistry.cs +++ b/src/DodoSSH.Api/Features/Relay/RelaySessionRegistry.cs @@ -22,6 +22,7 @@ internal sealed class RelaySessionRegistry(IOptions options) { private readonly Lock gate = new(); private readonly Dictionary perUser = []; + private readonly HashSet open = []; private int total; /// How many sessions are open on this node. @@ -55,7 +56,16 @@ internal sealed class RelaySessionRegistry(IOptions options) /// Admits a session, or answers null when a limit is met. /// A lease that gives the place back when disposed. - internal Lease? TryAdmit(Guid userId) + /// The sessions a user has open now, for re-checking their access. + internal IReadOnlyList SessionsOf(Guid userId) + { + lock (gate) + { + return [.. open.Where(lease => lease.UserId == userId)]; + } + } + + internal Lease? TryAdmit(Guid userId, Guid vaultId) { var settings = options.Value; @@ -71,14 +81,18 @@ internal sealed class RelaySessionRegistry(IOptions options) perUser[userId] = held + 1; total++; - return new Lease(this, userId); + var lease = new Lease(this, userId, vaultId); + open.Add(lease); + + return lease; } } - private void Release(Guid userId) + private void Release(Lease lease, Guid userId) { lock (gate) { + open.Remove(lease); total--; // Removed at zero rather than left behind, so the dictionary is the size of the users with a @@ -95,16 +109,41 @@ internal sealed class RelaySessionRegistry(IOptions options) } /// One admitted session's place. Idempotent to dispose. - internal sealed class Lease(RelaySessionRegistry registry, Guid userId) : IDisposable + internal sealed class Lease(RelaySessionRegistry registry, Guid userId, Guid vaultId) : IDisposable { + private readonly CancellationTokenSource revoked = new(); + private int released; + /// Whose session this is. + internal Guid UserId { get; } = userId; + + /// The vault the ticket's host is filed in, which is what the session's access hangs on. + internal Guid VaultId { get; } = vaultId; + + /// Cancelled when the session's user loses access to . + internal CancellationToken Revoked => revoked.Token; + + /// Ends the session, because its user can no longer reach the vault it was opened for. + internal void Revoke() + { + try + { + revoked.Cancel(); + } + catch (ObjectDisposedException) + { + // Already over. + } + } + /// public void Dispose() { if (Interlocked.Exchange(ref released, 1) == 0) { - registry.Release(userId); + registry.Release(this, UserId); + revoked.Dispose(); } } } diff --git a/src/DodoSSH.Api/Features/Sync/SyncService.cs b/src/DodoSSH.Api/Features/Sync/SyncService.cs index 7245aaf..bd59d94 100644 --- a/src/DodoSSH.Api/Features/Sync/SyncService.cs +++ b/src/DodoSSH.Api/Features/Sync/SyncService.cs @@ -100,9 +100,7 @@ internal sealed class SyncService( { ValidateBatchLimits(request); - var strategy = database.Database.CreateExecutionStrategy(); - - return await strategy.ExecuteAsync(async () => + return await database.ExecuteFreshAsync([vault], async () => { var transaction = await database.Database .BeginTransactionAsync(cancellationToken) 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 ec952fd..44e8828 100644 --- a/src/DodoSSH.Api/Features/Teams/TeamService.cs +++ b/src/DodoSSH.Api/Features/Teams/TeamService.cs @@ -1,5 +1,6 @@ using System.Globalization; using DodoSSH.Api.Features.Events; +using DodoSSH.Api.Setup; using DodoSSH.Contracts; using DodoSSH.Domain; using DodoSSH.Infrastructure; @@ -254,26 +255,27 @@ 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 strategy = database.Database.CreateExecutionStrategy(); - - var archived = await strategy.ExecuteAsync(async () => + var archived = await database.ExecuteFreshAsync([team], async () => { var transaction = await database.Database .BeginTransactionAsync(cancellationToken) .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) @@ -343,9 +345,7 @@ internal sealed class TeamService( var incoming = await RequireMembershipAsync(teamId, request.UserId, cancellationToken) .ConfigureAwait(false); - var strategy = database.Database.CreateExecutionStrategy(); - - await strategy.ExecuteAsync(async () => + await database.ExecuteFreshAsync([incoming, outgoing], async () => { var transaction = await database.Database .BeginTransactionAsync(cancellationToken) @@ -724,9 +724,7 @@ internal sealed class TeamService( } var now = clock.GetUtcNow(); - var strategy = database.Database.CreateExecutionStrategy(); - - var revoked = await strategy.ExecuteAsync(async () => + var revoked = await database.ExecuteFreshAsync([membership], async () => { var transaction = await database.Database .BeginTransactionAsync(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, diff --git a/src/DodoSSH.Api/Program.cs b/src/DodoSSH.Api/Program.cs index a1a99dd..fa5bc2e 100644 --- a/src/DodoSSH.Api/Program.cs +++ b/src/DodoSSH.Api/Program.cs @@ -41,13 +41,12 @@ builder.Services.AddScoped(); builder.Services.AddScoped(); builder.Services.AddSingleton(); -// A singleton, because the sockets it holds outlive the requests that opened them. Registered twice -// resolving to the same instance: the endpoint needs the whole hub — admit, remove, count — while the -// write paths that announce a change need only the two methods that announce one, and should not gain -// a reference to connection management to get them. +// A singleton, because the sockets it holds outlive the requests that opened them. The endpoint needs the +// whole hub — admit, remove, count — while the write paths that announce a change need only the two methods +// that announce one, and get them from a fan-out that also tells the relay: a user who loses access to a +// vault loses their relay sessions into it too. See VaultEventFanOut and RelayAccessWatch. builder.Services.AddSingleton(); -builder.Services.AddSingleton( - provider => provider.GetRequiredService()); +builder.Services.AddSingleton(); // The relay's services, its ticket key ring, and a shutdown long enough for its drain. See ADR 0004. builder.Services.AddDodoRelay(); diff --git a/src/DodoSSH.Api/Setup/FreshAttempts.cs b/src/DodoSSH.Api/Setup/FreshAttempts.cs new file mode 100644 index 0000000..30e5f99 --- /dev/null +++ b/src/DodoSSH.Api/Setup/FreshAttempts.cs @@ -0,0 +1,72 @@ +using DodoSSH.Infrastructure; +using Microsoft.EntityFrameworkCore; + +namespace DodoSSH.Api.Setup; + +/// +/// Runs a write transaction under the retrying execution strategy so that a retry starts from the database +/// rather than from what the failed attempt left in the change tracker. +/// +/// +/// +/// The strategy re-runs the delegate on the same context, and the context remembers the attempt that +/// failed. When the failure is the commit itself — a connection dropped after SaveChanges had +/// already succeeded — every entity that attempt wrote is marked unchanged, holding values the database +/// rolled back. The retry then assigns the same values again, change detection sees nothing to save, and +/// the transaction commits without them. Enrolment lost EnrolledAtUtc that way, and a team hand-over +/// its two role changes, each reporting success. +/// +/// +/// So a retry clears the tracker, and reloads each entity the delegate was handed from outside it — the ones +/// it modifies or reads but did not load itself — so they are tracked again with what the database actually +/// holds and the delegate's own assignments register as changes. The first attempt runs exactly as it did +/// before: nothing is cleared or reloaded until something has failed. +/// +/// +internal static class FreshAttempts +{ + /// Runs under the context's execution strategy, freshly each time. + /// The request's context. + /// Entities loaded before the transaction that the attempt reads or modifies. + /// The transaction, which begins and commits its own. + internal static Task ExecuteFreshAsync( + this DodoDbContext database, + IReadOnlyList carried, + Func> attempt) + { + ArgumentNullException.ThrowIfNull(database); + ArgumentNullException.ThrowIfNull(carried); + ArgumentNullException.ThrowIfNull(attempt); + + var strategy = database.Database.CreateExecutionStrategy(); + var attempts = 0; + + return strategy.ExecuteAsync(async () => + { + if (attempts++ > 0) + { + database.ChangeTracker.Clear(); + + foreach (var entity in carried) + { + await database.Entry(entity).ReloadAsync().ConfigureAwait(false); + } + } + + return await attempt().ConfigureAwait(false); + }); + } + + /// + internal static Task ExecuteFreshAsync( + this DodoDbContext database, + IReadOnlyList carried, + Func attempt) => + database.ExecuteFreshAsync( + carried, + async () => + { + await attempt().ConfigureAwait(false); + return true; + }); +} diff --git a/src/DodoSSH.Api/Setup/Persistence.cs b/src/DodoSSH.Api/Setup/Persistence.cs index f50d21d..aca0a80 100644 --- a/src/DodoSSH.Api/Setup/Persistence.cs +++ b/src/DodoSSH.Api/Setup/Persistence.cs @@ -1,5 +1,6 @@ using DodoSSH.Infrastructure; using Microsoft.EntityFrameworkCore; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.Extensions.Diagnostics.HealthChecks; using Microsoft.Extensions.Options; using Npgsql; @@ -29,14 +30,18 @@ internal static class Persistence ?? throw new InvalidOperationException( $"ConnectionStrings:{ConnectionStringName} is not configured."); - services.AddDbContext(options => options + services.AddDbContext((provider, options) => options .UseNpgsql(connectionString, npgsql => npgsql .MigrationsHistoryTable("__EFMigrationsHistory", DodoDbContext.SchemaName) - // Retry on transient faults only. Note this is safe here because writes go - // through explicitly-managed transactions rather than relying on the execution - // strategy to replay ambient ones. + // Retry on transient faults only. A write transaction replayed by the strategy runs on + // the same context, which still holds what the failed attempt tracked — every such + // transaction goes through FreshAttempts.ExecuteFreshAsync, which starts a retry from + // the database instead. .EnableRetryOnFailure(3)) - .UseSnakeCaseNamingConvention()); + .UseSnakeCaseNamingConvention() + // None in production. The seam a test uses to make a commit fail the way a dropped + // connection would, which is the only way to watch a retry happen. + .AddInterceptors(provider.GetServices())); services.AddHealthChecks() .AddCheck( diff --git a/src/DodoSSH.Api/Setup/RelaySetup.cs b/src/DodoSSH.Api/Setup/RelaySetup.cs index cee1ca0..68ccb22 100644 --- a/src/DodoSSH.Api/Setup/RelaySetup.cs +++ b/src/DodoSSH.Api/Setup/RelaySetup.cs @@ -24,6 +24,7 @@ internal static class RelaySetup // the network seam holds no state. services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); services.AddSingleton(); // The application name isolates this deployment's payloads from any other ASP.NET application diff --git a/src/DodoSSH.Client.App/App.axaml.cs b/src/DodoSSH.Client.App/App.axaml.cs index b49c046..0849859 100644 --- a/src/DodoSSH.Client.App/App.axaml.cs +++ b/src/DodoSSH.Client.App/App.axaml.cs @@ -48,6 +48,10 @@ internal sealed partial class DodoSshApp : Application { if (ApplicationLifetime is IClassicDesktopStyleApplicationLifetime desktop) { + // Recorded, not handled: see CrashLog for why the process still goes. + Avalonia.Threading.Dispatcher.UIThread.UnhandledException += + (_, e) => CrashLog.RecordFromTheUiThread(e.Exception); + ListenForOpenedLinks(); ShowSomethingWhileComposing(desktop); } diff --git a/src/DodoSSH.Client.App/Platform/CrashLog.cs b/src/DodoSSH.Client.App/Platform/CrashLog.cs new file mode 100644 index 0000000..4168e4f --- /dev/null +++ b/src/DodoSSH.Client.App/Platform/CrashLog.cs @@ -0,0 +1,85 @@ +using DodoSSH.Client.Session; + +namespace DodoSSH.Client.App.Platform; + +/// +/// Writes down why the application died, in the profile, before it does. +/// +/// +/// +/// An exception nothing caught used to close the window and leave nothing behind — no file, no event, no +/// sentence — so the only report anybody could make was "it closed". This does not keep the process alive: +/// an exception that reached the top is one no code was written to recover from, and carrying on in whatever +/// state it left would be a guess. What it adds is the record. +/// +/// +/// Bounded, and appended to rather than rewritten, so a run of crashes keeps its history without the file +/// growing for ever. Every write is best-effort: a profile that cannot be written to is a reason to lose the +/// log, never a reason to fail harder. +/// +/// +internal static class CrashLog +{ + /// Past this, the next entry starts a fresh file. + private const long MaxBytes = 1024 * 1024; + + private static readonly Lock Gate = new(); + + private static string? file; + + /// Starts recording, for the process's two last-chance hooks. + /// + /// The UI thread's own hook is added by the application once Avalonia is running — see + /// — because the dispatcher must not be touched before the platform + /// has set it up. + /// + internal static void Install(ClientPaths paths) + { + ArgumentNullException.ThrowIfNull(paths); + + file = paths.CrashLogFile; + + AppDomain.CurrentDomain.UnhandledException += (_, e) => Write("Unhandled", e.ExceptionObject as Exception); + + // A fire-and-forget task that faulted. It does not end the process, which is exactly why it needs + // writing down: the work it was doing simply never happened, and nothing else would ever say so. + TaskScheduler.UnobservedTaskException += (_, e) => + { + Write("Unobserved task", e.Exception); + e.SetObserved(); + }; + } + + /// The UI thread's hook: records the exception and lets it take the process as before. + internal static void RecordFromTheUiThread(Exception exception) => Write("UI thread", exception); + + /// Appends one entry; never throws. + internal static void Write(string where, Exception? exception) + { + if (file is not { } path) + { + return; + } + + var entry = $"{TimeProvider.System.GetUtcNow():O} {where}{Environment.NewLine}{exception}{Environment.NewLine}{Environment.NewLine}"; + + lock (Gate) + { + try + { + Directory.CreateDirectory(Path.GetDirectoryName(path)!); + + if (File.Exists(path) && new FileInfo(path).Length > MaxBytes) + { + File.Delete(path); + } + + File.AppendAllText(path, entry); + } + catch (Exception failure) when (failure is IOException or UnauthorizedAccessException) + { + // Best-effort, by design: see the type's remarks. + } + } + } +} diff --git a/src/DodoSSH.Client.App/Platform/ProbedDeviceKeyStore.cs b/src/DodoSSH.Client.App/Platform/ProbedDeviceKeyStore.cs new file mode 100644 index 0000000..e8c290a --- /dev/null +++ b/src/DodoSSH.Client.App/Platform/ProbedDeviceKeyStore.cs @@ -0,0 +1,62 @@ +using DodoSSH.Client.Session; + +namespace DodoSSH.Client.App.Platform; + +/// +/// The device key store this machine supports, chosen once and off the UI thread. +/// +/// +/// +/// Choosing a store means probing for it, and on Windows the probe is creating an RSA-2048 key inside the TPM +/// and deleting it again — see — which is a slow device doing +/// real work. It used to run synchronously on the UI thread three times over: once to choose the store at +/// startup, again when the unlock card asked whether to offer the gesture, and again after every unlock when +/// the security page asked whether to offer registering. A machine does not gain or lose a TPM while this +/// process runs, so the answer is worked out once, on the thread pool, and every question after it is a +/// lookup. +/// +/// +/// Available means the choice landed on a real store, which is what the probe that chose it established. +/// Everything else waits for the choice and hands over to it — in practice it has long since finished, +/// since nothing reaches for the device key until somebody is at the unlock card. +/// +/// +internal sealed class ProbedDeviceKeyStore : IDeviceKeyStore +{ + private readonly Task chosen; + + /// Starts choosing at once, on the thread pool. + internal ProbedDeviceKeyStore(Func choose) + { + ArgumentNullException.ThrowIfNull(choose); + chosen = Task.Run(choose); + } + + /// The store the probe settled on, for a test that needs to know which. + internal Task Chosen => chosen; + + /// + public async ValueTask IsAvailableAsync(CancellationToken cancellationToken) => + await chosen.WaitAsync(cancellationToken).ConfigureAwait(false) is not UnavailableDeviceKeyStore; + + /// + public async ValueTask SaveAsync(ReadOnlyMemory devicePrivateKey, CancellationToken cancellationToken) + { + var store = await chosen.WaitAsync(cancellationToken).ConfigureAwait(false); + await store.SaveAsync(devicePrivateKey, cancellationToken).ConfigureAwait(false); + } + + /// + public async ValueTask TryLoadAsync(CancellationToken cancellationToken) + { + var store = await chosen.WaitAsync(cancellationToken).ConfigureAwait(false); + return await store.TryLoadAsync(cancellationToken).ConfigureAwait(false); + } + + /// + public async ValueTask ForgetAsync(CancellationToken cancellationToken) + { + var store = await chosen.WaitAsync(cancellationToken).ConfigureAwait(false); + await store.ForgetAsync(cancellationToken).ConfigureAwait(false); + } +} diff --git a/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs b/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs index c88b659..d3f7fb6 100644 --- a/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs +++ b/src/DodoSSH.Client.App/Platform/WindowsDeviceKeyStore.cs @@ -41,7 +41,12 @@ namespace DodoSSH.Client.App.Platform; /// public static class DesktopDeviceKeyStores { - /// The best store this machine supports. + /// The best store this machine supports, chosen on the thread pool. + /// + /// Returns at once: the probing that decides between them is slow — a TPM key created and deleted on + /// Windows — and runs behind rather than here, which is on the UI + /// thread at startup. + /// /// Where a stored key lives. /// /// Shows the Linux PIN prompt, and is never called on the other two platforms — there the platform @@ -57,6 +62,13 @@ public static class DesktopDeviceKeyStores ArgumentNullException.ThrowIfNull(paths); ArgumentNullException.ThrowIfNull(askForPin); + return new ProbedDeviceKeyStore(() => Choose(paths, askForPin)); + } + + private static IDeviceKeyStore Choose( + ClientPaths paths, + Func> askForPin) + { if (OperatingSystem.IsWindows() && WindowsDeviceKeyStore.IsSupported()) { return new WindowsDeviceKeyStore(paths); diff --git a/src/DodoSSH.Client.App/Program.cs b/src/DodoSSH.Client.App/Program.cs index 54c4d15..2d26c22 100644 --- a/src/DodoSSH.Client.App/Program.cs +++ b/src/DodoSSH.Client.App/Program.cs @@ -75,6 +75,9 @@ internal static class Program LaunchLink = link; + // After the claim, so a second launch that only hands its link over writes nothing. + CrashLog.Install(paths); + BuildAvaloniaApp().StartWithClassicDesktopLifetime(args); } diff --git a/src/DodoSSH.Client.App/Views/CaptionPresses.cs b/src/DodoSSH.Client.App/Views/CaptionPresses.cs new file mode 100644 index 0000000..63ccb7a --- /dev/null +++ b/src/DodoSSH.Client.App/Views/CaptionPresses.cs @@ -0,0 +1,54 @@ +using Avalonia; +using Avalonia.Controls; +using Avalonia.Input; + +namespace DodoSSH.Client.App.Views; + +/// +/// Tells a drawn titlebar's double-click from a press that starts a drag. +/// +/// +/// +/// Both titlebars used to answer a double-click through DoubleTapped and start a drag on every press. +/// The two do not mix: BeginMoveDrag hands the press to the operating system's own move loop, which +/// eats the release — and a double tap is a gesture Avalonia assembles from presses and releases, +/// so the second click often never completed one and the window did not maximise. The convention it was +/// imitating is one nobody thinks about until it does not work. +/// +/// +/// So the press decides, before the drag begins: a second press inside the platform's double-click time and +/// distance is the maximise, and does not start a drag at all. The click count is honoured where the platform +/// reports one, and the time and distance are measured here for where the move loop kept it from counting. +/// +/// +internal sealed class CaptionPresses +{ + private ulong lastAt; + + private Point lastWhere; + + private bool waitingForASecond; + + /// Whether this press is the second of a double-click on . + internal bool IsSecond(PointerPressedEventArgs e, Visual bar) + { + var settings = Application.Current?.PlatformSettings; + var time = settings?.GetDoubleTapTime(e.Pointer.Type) ?? TimeSpan.FromMilliseconds(500); + var size = settings?.GetDoubleTapSize(e.Pointer.Type) ?? new Size(4, 4); + var where = e.GetPosition(bar); + + var second = e.ClickCount == 2 + || (waitingForASecond + && e.Timestamp - lastAt <= (ulong)time.TotalMilliseconds + && Math.Abs(where.X - lastWhere.X) <= size.Width + && Math.Abs(where.Y - lastWhere.Y) <= size.Height); + + // A double-click spends both presses: a third that follows quickly starts again rather than toggling + // the window straight back. + waitingForASecond = !second; + lastAt = e.Timestamp; + lastWhere = where; + + return second; + } +} diff --git a/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml b/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml index 93b1e01..751141d 100644 --- a/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml +++ b/src/DodoSSH.Client.App/Views/ConfirmDeleteCard.axaml @@ -59,7 +59,7 @@