Relay: end a user's sessions into a vault the moment they lose access to it

src/DodoSSH.Api/Features/Events/VaultEventFanOut.cs

@@ -0,0 +1,24 @@
+using DodoSSH.Api.Features.Relay;
+
+namespace DodoSSH.Api.Features.Events;
+
+/// <summary>
+/// Announces a change to everything on this node that acts on one: the event sockets, and the relay.
+/// </summary>
+/// <remarks>
+/// What the write paths are given as their <see cref="IVaultEventPublisher"/>. 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.
+/// </remarks>
+internal sealed class VaultEventFanOut(VaultEventHub hub, RelayAccessWatch relay) : IVaultEventPublisher
+{
+    /// <inheritdoc />
+    public void VaultChanged(Guid vaultId, long sequence) => hub.VaultChanged(vaultId, sequence);
+
+    /// <inheritdoc />
+    public void VaultAccessChanged(Guid userId)
+    {
+        hub.VaultAccessChanged(userId);
+        _ = relay.AccessChangedAsync(userId);
+    }
+}

src/DodoSSH.Api/Features/Relay/RelayAccessWatch.cs

@@ -0,0 +1,62 @@
+using DodoSSH.Api.Authorization;
+using DodoSSH.Domain.Authorization;
+
+namespace DodoSSH.Api.Features.Relay;
+
+/// <summary>
+/// Ends a user's relay sessions to vaults they can no longer reach, when their access changes.
+/// </summary>
+/// <remarks>
+/// <para>
+/// 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.
+/// </para>
+/// <para>
+/// Told by the same announcement that tells their event socket to re-read its vaults — see
+/// <see cref="Events.IVaultEventPublisher.VaultAccessChanged"/> — 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.
+/// </para>
+/// </remarks>
+internal sealed class RelayAccessWatch(
+    RelaySessionRegistry sessions,
+    IServiceScopeFactory scopes,
+    ILogger<RelayAccessWatch> logger)
+{
+    /// <summary>Re-checks every session the user has open, in the background.</summary>
+    /// <returns>The re-check, for a caller that wants to wait for it; the announcement does not.</returns>
+    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<RelaySessionRegistry.Lease> open)
+    {
+        try
+        {
+            var scope = scopes.CreateAsyncScope();
+            await using var _ = scope.ConfigureAwait(false);
+
+            var access = scope.ServiceProvider.GetRequiredService<IVaultAccessService>();
+
+            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);
+        }
+    }
+}

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(
     }
 
     /// <summary>Dials the target, pipes until something ends it, and writes down what happened.</summary>
-    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.
     /// </para>
     /// </remarks>
-    private async Task<Ending?> MindAsync(Counters counters, DateTimeOffset started, CancellationToken cancellationToken)
+    private async Task<Ending?> 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(
+                return ending;
-                    RelayCloseReason.IdleTimeout,
-                    RelayProtocol.IdleTimeoutCloseCode,
-                    "This relay session carried nothing for the idle timeout.");
             }
 
             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, revoked, lifetime.ApplicationStopping)
-                : CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
+                : 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;
     }
 
+    /// <summary>Whichever reason to end the session has come due, or null while none has.</summary>
+    /// <remarks>
+    /// 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.
+    /// </remarks>
+    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);
 

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);
+
+    /// <remarks>
+    /// 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.
+    /// </remarks>
+    [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);
 }

src/DodoSSH.Api/Features/Relay/RelaySessionRegistry.cs

@@ -22,6 +22,7 @@ internal sealed class RelaySessionRegistry(IOptions<RelayOptions> options)
 {
     private readonly Lock gate = new();
     private readonly Dictionary<Guid, int> perUser = [];
+    private readonly HashSet<Lease> open = [];
     private int total;
 
     /// <summary>How many sessions are open on this node.</summary>
@@ -55,7 +56,16 @@ internal sealed class RelaySessionRegistry(IOptions<RelayOptions> options)
 
     /// <summary>Admits a session, or answers null when a limit is met.</summary>
     /// <returns>A lease that gives the place back when disposed.</returns>
-    internal Lease? TryAdmit(Guid userId)
+    /// <summary>The sessions a user has open now, for re-checking their access.</summary>
+    internal IReadOnlyList<Lease> 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<RelayOptions> 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<RelayOptions> options)
     }
 
     /// <summary>One admitted session's place. Idempotent to dispose.</summary>
-    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;
 
+        /// <summary>Whose session this is.</summary>
+        internal Guid UserId { get; } = userId;
+
+        /// <summary>The vault the ticket's host is filed in, which is what the session's access hangs on.</summary>
+        internal Guid VaultId { get; } = vaultId;
+
+        /// <summary>Cancelled when the session's user loses access to <see cref="VaultId"/>.</summary>
+        internal CancellationToken Revoked => revoked.Token;
+
+        /// <summary>Ends the session, because its user can no longer reach the vault it was opened for.</summary>
+        internal void Revoke()
+        {
+            try
+            {
+                revoked.Cancel();
+            }
+            catch (ObjectDisposedException)
+            {
+                // Already over.
+            }
+        }
+
         /// <inheritdoc />
         public void Dispose()
         {
             if (Interlocked.Exchange(ref released, 1) == 0)
             {
-                registry.Release(userId);
+                registry.Release(this, UserId);
+                revoked.Dispose();
             }
         }
     }

src/DodoSSH.Api/Program.cs

@@ -41,13 +41,12 @@ builder.Services.AddScoped<VaultGrantService>();
 builder.Services.AddScoped<IIdentityBindingVerifier, IdentityBindingVerifier>();
 builder.Services.AddSingleton<ICursorKeyProvider, CursorKeyProvider>();
 
-// A singleton, because the sockets it holds outlive the requests that opened them. Registered twice
+// A singleton, because the sockets it holds outlive the requests that opened them. The endpoint needs the
-// resolving to the same instance: the endpoint needs the whole hub — admit, remove, count — while the
+// whole hub — admit, remove, count — while the write paths that announce a change need only the two methods
-// write paths that announce a change need only the two methods that announce one, and should not gain
+// that announce one, and get them from a fan-out that also tells the relay: a user who loses access to a
-// a reference to connection management to get them.
+// vault loses their relay sessions into it too. See VaultEventFanOut and RelayAccessWatch.
 builder.Services.AddSingleton<VaultEventHub>();
-builder.Services.AddSingleton<IVaultEventPublisher>(
+builder.Services.AddSingleton<IVaultEventPublisher, VaultEventFanOut>();
-    provider => provider.GetRequiredService<VaultEventHub>());
 
 // The relay's services, its ticket key ring, and a shutdown long enough for its drain. See ADR 0004.
 builder.Services.AddDodoRelay();

src/DodoSSH.Api/Setup/RelaySetup.cs

@@ -24,6 +24,7 @@ internal static class RelaySetup
         // the network seam holds no state.
         services.AddSingleton<RelayTicketCodec>();
         services.AddSingleton<RelaySessionRegistry>();
+        services.AddSingleton<RelayAccessWatch>();
         services.AddSingleton<IRelayNetwork, SystemRelayNetwork>();
 
         // The application name isolates this deployment's payloads from any other ASP.NET application

src/DodoSSH.Client.Session/ServerRelay.cs

@@ -80,6 +80,9 @@ public sealed class RelayRoute : IConnectionRoute
         RelayProtocol.DurationLimitCloseCode =>
             "The server relay closed the connection: it reached the longest session the server allows.",
 
+        RelayProtocol.AccessRevokedCloseCode =>
+            "The server relay closed the connection: you no longer have access to this host's vault.",
+
         _ => null,
     };
 }

src/DodoSSH.Contracts/PublicAPI.Unshipped.txt

@@ -382,6 +382,7 @@ DodoSSH.Contracts.RelayConfiguration.RelayConfiguration(bool Enabled, System.Uri
 DodoSSH.Contracts.RelayConfiguration.WebSocketUrl.get -> System.Uri?
 DodoSSH.Contracts.RelayConfiguration.WebSocketUrl.init -> void
 DodoSSH.Contracts.RelaySessionCloseReason
+DodoSSH.Contracts.RelaySessionCloseReason.AccessRevoked = 8 -> DodoSSH.Contracts.RelaySessionCloseReason
 DodoSSH.Contracts.RelaySessionCloseReason.ClientClosed = 1 -> DodoSSH.Contracts.RelaySessionCloseReason
 DodoSSH.Contracts.RelaySessionCloseReason.DurationLimit = 4 -> DodoSSH.Contracts.RelaySessionCloseReason
 DodoSSH.Contracts.RelaySessionCloseReason.Error = 7 -> DodoSSH.Contracts.RelaySessionCloseReason
@@ -969,6 +970,7 @@ static DodoSSH.Contracts.VaultKeyWrap.operator ==(DodoSSH.Contracts.VaultKeyWrap
 static DodoSSH.Contracts.VaultSummary.operator !=(DodoSSH.Contracts.VaultSummary? left, DodoSSH.Contracts.VaultSummary? right) -> bool
 static DodoSSH.Contracts.VaultSummary.operator ==(DodoSSH.Contracts.VaultSummary? left, DodoSSH.Contracts.VaultSummary? right) -> bool
 const DodoSSH.Contracts.ProblemCodes.RelayUnavailable = "relay-unavailable" -> string!
+const DodoSSH.Contracts.RelayProtocol.AccessRevokedCloseCode = 4403 -> int
 const DodoSSH.Contracts.RelayProtocol.ConnectPath = "/api/v1/relay/connect" -> string!
 const DodoSSH.Contracts.RelayProtocol.DurationLimitCloseCode = 4409 -> int
 const DodoSSH.Contracts.RelayProtocol.Feature = "relay" -> string!

src/DodoSSH.Contracts/Relay.cs

@@ -70,6 +70,15 @@ public static class RelayProtocol
 
     /// <summary>Close code for a session that reached the maximum lifetime this server allows.</summary>
     public const int DurationLimitCloseCode = 4409;
+
+    /// <summary>
+    /// Close code for a session whose user lost access to the host's vault while it was open.
+    /// </summary>
+    /// <remarks>
+    /// Removed from the team, or the vault's key withdrawn. Reconnecting will not help: a new ticket is
+    /// refused for the same reason.
+    /// </remarks>
+    public const int AccessRevokedCloseCode = 4403;
 }
 
 
@@ -143,6 +152,9 @@ public enum RelaySessionCloseReason
 
     /// <summary>An error ended the session.</summary>
     Error = 7,
+
+    /// <summary>The user lost access to the host's vault while the session was open.</summary>
+    AccessRevoked = 8,
 }
 
 /// <summary>

src/DodoSSH.Domain/Relay.cs

@@ -120,4 +120,7 @@ public enum RelayCloseReason
 
     /// <summary>An error ended the session.</summary>
     Error = 7,
+
+    /// <summary>The user lost access to the host's vault while the session was open.</summary>
+    AccessRevoked = 8,
 }

tests/DodoSSH.Api.Tests/RelayEndpointTests.cs

@@ -2,6 +2,7 @@ using System.Net;
 using System.Net.Sockets;
 using System.Net.WebSockets;
 using System.Text;
+using DodoSSH.Api.Features.Events;
 using DodoSSH.Contracts;
 using DodoSSH.Domain;
 using DodoSSH.Infrastructure;
@@ -266,6 +267,56 @@ public sealed class RelayEndpointTests(ApiFixture fixture)
             .ShouldBe(RelayCloseReason.IdleTimeout);
     }
 
+    /// <remarks>
+    /// <para>
+    /// A session is authorised once, when its ticket is issued, and used to run on for as long as it carried
+    /// bytes — removing somebody from a team left their shells into its machines open for up to the session's
+    /// maximum lifetime. The announcement that tells their event socket to re-read its vaults now has the
+    /// relay ask again too.
+    /// </para>
+    /// <para>
+    /// Access is withdrawn here by deleting the vault rather than by a team removal, because what is under
+    /// test is the relay hearing the announcement and asking the access model again — any way of making the
+    /// answer change will do, and this one needs no team. The removal path announces through the same call;
+    /// see <c>TeamService.RemoveMemberAsync</c>.
+    /// </para>
+    /// </remarks>
+    [Fact]
+    public async Task ASessionWhoseUserLosesAccess_IsClosedWithItsOwnCode()
+    {
+        using var target = LoopbackTarget.Echo();
+        var (subject, hostId) = await SeedRoutedHostAsync(target, "10.40.8.1");
+        var ticket = await IssueAsync(subject, hostId);
+
+        using var timeout = Timeout();
+        using var socket = await ConnectAsync(ticket, timeout.Token);
+
+        await RoundTripAsync(socket, "still here", timeout.Token);
+
+        Guid userId;
+
+        await using (var scope = fixture.CreateScope())
+        {
+            var database = scope.ServiceProvider.GetRequiredService<DodoDbContext>();
+
+            userId = (await database.Users.SingleAsync(u => u.Subject == subject, timeout.Token)).Id;
+
+            var vaultId = (await database.Hosts.SingleAsync(h => h.Id == hostId, timeout.Token)).VaultId;
+            var vault = await database.Vaults.SingleAsync(v => v.Id == vaultId, timeout.Token);
+
+            vault.DeletedAtUtc = Now;
+            await database.SaveChangesAsync(timeout.Token);
+
+            scope.ServiceProvider.GetRequiredService<IVaultEventPublisher>().VaultAccessChanged(userId);
+        }
+
+        await ReadUntilCloseAsync(socket, timeout.Token);
+
+        socket.CloseStatus.ShouldBe((WebSocketCloseStatus)RelayProtocol.AccessRevokedCloseCode);
+        (await WaitForClosedAuditAsync(ticket.SessionId, timeout.Token)).CloseReason
+            .ShouldBe(RelayCloseReason.AccessRevoked);
+    }
+
     [Fact]
     public async Task AUserAtTheCap_IsRefusedATicket_AndASocketOverItIsClosed()
     {