Merge branch 'main' into the vaults screen, and let it rotate keys too

Main built vault key rotation while this branch was reshaping the screen that
would drive it, so the two met in the same three files. Every other conflict was
textual and resolved by taking both; these are the ones where a decision had to
be made.

**The view model.** Main taught TeamsViewModel three things and this branch had
renamed and rewritten it into VaultsViewModel. All three are ported rather than
dropped, because each is a behaviour rather than wording: adding somebody now
wraps the vault to them on the spot instead of leaving SHARE KEY to be pressed,
removing somebody rotates the vault and hands the new key to whoever is left, and
a share reports how many generations were wrapped. The session calls they reach —
ShareTeamVaultsAsync and RekeyTeamVaultsAsync — are scoped to a membership list
rather than to one vault, and they are called that way here rather than narrowed:
adding somebody is a change to the list, so every vault the list carries is one
they can now fetch. This screen makes lists that carry one vault, so the sentences
name one; where a list carries several, naming them all is the honest report, and
the members section already says the list is shared.

AddMemberAsync ran two lines over the length limit once the sharing was in it, so
the calls behind it moved to AddOrInviteAsync and the three-way refusal to
WhyNobodyCanBeAdded — the command reads as its guards now, which is what it was
before the sharing arrived.

**The tests.** Main's four new cases are ported to the vault-first API, including
the one that matters most: the tampered key log is corrupted *before* the add,
because the add is now a route to a wrap and a test that corrupted it afterwards
would be asserting about the manual route only. SelectingAVault_ListsWhoHoldsAKey
now expects two holders rather than one — main's fake records the creator's own
self-grant, and a key-holder list that omitted it would show the one person who
can certainly open a new vault as somebody who cannot.

**The README.** The limits list is six rather than four or five: main's rotation
entries and this branch's "a vault cannot be deleted" describe different things
and both are true. "The rekey is flagged, never performed" is gone, since it is
now performed, and M3 reads *Done* rather than *Done, except rekey*.

One thing worth writing down that neither side had. An invitation claimed at
sign-in still leaves the key owed, where an add does not: at the moment an
invitation is issued there is no account and no published key to wrap to, and the
claim happens on the invitee's machine, which holds nothing. Manual check 12.1
says so, because a reader who knows adding shares would otherwise read that step
as stale.

1561 tests pass.

tests/DodoSSH.Client.App.Tests/FakeVaultServer.Teams.cs

@@ -26,7 +26,14 @@ internal sealed partial class FakeVaultServer : ITeamApi, IDirectoryApi, IVaultG
     private readonly List<TeamSummary> teams = [];
     private readonly Dictionary<Guid, List<TeamMemberSummary>> members = [];
     private readonly Dictionary<Guid, VaultSummary> teamVaults = [];
-    private readonly Dictionary<(Guid VaultId, Guid UserId), IssueVaultGrantRequest> grants = [];
+    /// <remarks>
+    /// Keyed by generation as well as by recipient, because the real table is: a rotation leaves a
+    /// member holding one grant per generation, and a fake that kept one per person would quietly model
+    /// sharing the history as overwriting it — which is the bug this half of the feature exists to
+    /// avoid.
+    /// </remarks>
+    private readonly Dictionary<(Guid VaultId, Guid UserId, uint KeyGeneration), IssueVaultGrantRequest>
+        grants = [];
     private readonly List<KeyLogRecord> keyLog = [];
     private readonly List<DirectoryEntry> directory = [];
     private readonly Dictionary<Guid, List<TeamInvitationSummary>> invitations = [];
@@ -51,8 +58,29 @@ internal sealed partial class FakeVaultServer : ITeamApi, IDirectoryApi, IVaultG
     /// <inheritdoc />
     public IVaultGrantApi Grants => this;
 
-    /// <summary>Grants this fake has been asked to record, for a test to assert on.</summary>
-    internal IReadOnlyDictionary<(Guid VaultId, Guid UserId), IssueVaultGrantRequest> IssuedGrants => grants;
+    /// <summary>
+    /// Grants this fake has been asked to record, newest generation per recipient.
+    /// </summary>
+    /// <remarks>
+    /// Flattened to one entry per recipient because that is the question most tests are asking — can
+    /// this person open the vault as it stands. <see cref="GenerationsGranted"/> is for the ones asking
+    /// whether they were also given its history.
+    /// </remarks>
+    internal IReadOnlyDictionary<(Guid VaultId, Guid UserId), IssueVaultGrantRequest> IssuedGrants =>
+        grants
+            .GroupBy(entry => (entry.Key.VaultId, entry.Key.UserId))
+            .ToDictionary(
+                group => group.Key,
+                group => group.OrderByDescending(entry => entry.Key.KeyGeneration).First().Value);
+
+    /// <summary>Which generations of one vault's key a recipient has been wrapped, oldest first.</summary>
+    internal IReadOnlyList<uint> GenerationsGranted(Guid vaultId, Guid userId) =>
+        [
+            .. grants.Keys
+                .Where(key => key.VaultId == vaultId && key.UserId == userId)
+                .Select(key => key.KeyGeneration)
+                .Order(),
+        ];
 
     /// <summary>
     /// When true, the log served omits its last entry's link, so its chain no longer verifies.
@@ -451,11 +479,17 @@ internal sealed partial class FakeVaultServer : ITeamApi, IDirectoryApi, IVaultG
         // Every grant they held from this team goes with them, as the real service revokes them in the
         // same transaction. A fake that removed the membership and left the grants would let a test
         // "prove" a revocation that had not happened.
-        foreach (var vaultId in teamVaults.Values
-            .Where(vault => vault.TeamId == teamId)
-            .Select(vault => vault.VaultId))
+        var theirs = grants.Keys
+            .Where(key => key.UserId == userId
+                && teamVaults.TryGetValue(key.VaultId, out var vault)
+                && vault.TeamId == teamId)
+            .ToList();
+
+        // Every generation, not only the newest. A revocation that left the history behind would let
+        // them go on reading everything written before the rotation that follows.
+        foreach (var key in theirs)
         {
-            grants.Remove((vaultId, userId));
+            grants.Remove(key);
         }
 
         Recount(teamId);
@@ -491,6 +525,17 @@ internal sealed partial class FakeVaultServer : ITeamApi, IDirectoryApi, IVaultG
 
         teamVaults[vault.VaultId] = vault;
 
+        // The creator's own grant, as the real create records it in the same transaction. Without it a
+        // rotation here would report no earlier wraps and the vault's first generation would vanish.
+        grants[(vault.VaultId, UserId, 1)] = new IssueVaultGrantRequest(
+            UserId,
+            RecipientKeyFingerprint: new byte[32],
+            KeyGeneration: 1,
+            request.WrappedVaultKey,
+            KeyLogHead: new byte[32],
+            request.GrantSignature,
+            request.GrantedAt);
+
         Recount(teamId);
 
         return Task.FromResult(vault);
@@ -581,16 +626,20 @@ internal sealed partial class FakeVaultServer : ITeamApi, IDirectoryApi, IVaultG
         CancellationToken cancellationToken) =>
         Task.FromResult(new VaultGrantsResponse(
             vaultId,
-            KeyGeneration: 1,
+            KeyGeneration: Generation(vaultId),
             RekeyRequired: false,
             Grants:
             [
-                .. grants.Where(entry => entry.Key.VaultId == vaultId).Select(entry =>
-                    new VaultGrantSummary(
-                        entry.Key.UserId,
-                        directory.Find(candidate => candidate.UserId == entry.Key.UserId)?.Email,
+                // One row per holder rather than per grant, as the real listing shows a member once
+                // and lets the generation say whether their key is current.
+                .. grants
+                    .Where(entry => entry.Key.VaultId == vaultId)
+                    .GroupBy(entry => entry.Key.UserId)
+                    .Select(group => new VaultGrantSummary(
+                        group.Key,
+                        directory.Find(candidate => candidate.UserId == group.Key)?.Email,
                         null,
-                        KeyGeneration: 1,
+                        KeyGeneration: group.Max(entry => entry.Key.KeyGeneration),
                         VaultGrantState.Active,
                         UserId,
                         DateTimeOffset.UnixEpoch,
@@ -603,17 +652,88 @@ internal sealed partial class FakeVaultServer : ITeamApi, IDirectoryApi, IVaultG
         IssueVaultGrantRequest request,
         CancellationToken cancellationToken)
     {
-        grants[(vaultId, request.RecipientUserId)] = request;
+        grants[(vaultId, request.RecipientUserId, request.KeyGeneration)] = request;
 
         return Task.CompletedTask;
     }
 
+    /// <inheritdoc />
+    /// <remarks>
+    /// Models the one part of a rotation that is the server's: the generation advances, the caller's own
+    /// grant for it is recorded, and everything older is left standing so the vault's stored items go on
+    /// opening. What comes back is what the real endpoint returns — the vault at its new generation,
+    /// with the caller's earlier wraps attached.
+    /// </remarks>
+    public Task<VaultSummary> RekeyVaultAsync(
+        Guid vaultId,
+        RekeyVaultRequest request,
+        CancellationToken cancellationToken)
+    {
+        if (!teamVaults.TryGetValue(vaultId, out var vault))
+        {
+            throw new DodoSshApiException(
+                System.Net.HttpStatusCode.NotFound, code: null, "No such vault.");
+        }
+
+        if (request.KeyGeneration != vault.KeyGeneration + 1)
+        {
+            throw new DodoSshApiException(
+                System.Net.HttpStatusCode.BadRequest,
+                ProblemCodes.InvalidVaultGrant,
+                $"This vault is at key generation {vault.KeyGeneration}.");
+        }
+
+        grants[(vaultId, UserId, request.KeyGeneration)] = new IssueVaultGrantRequest(
+            UserId,
+            RecipientKeyFingerprint: new byte[32],
+            request.KeyGeneration,
+            request.WrappedVaultKey,
+            KeyLogHead: new byte[32],
+            request.GrantSignature,
+            request.GrantedAt);
+
+        var prior = grants
+            .Where(entry => entry.Key.VaultId == vaultId
+                && entry.Key.UserId == UserId
+                && entry.Key.KeyGeneration < request.KeyGeneration)
+            .OrderBy(entry => entry.Key.KeyGeneration)
+            .Select(entry => new VaultKeyWrap(entry.Key.KeyGeneration, entry.Value.WrappedVaultKey))
+            .ToList();
+
+        var rotated = vault with
+        {
+            KeyGeneration = request.KeyGeneration,
+            WrappedVaultKey = request.WrappedVaultKey,
+            RekeyRequired = false,
+            PriorKeyWraps = prior,
+        };
+
+        teamVaults[vaultId] = rotated;
+
+        return Task.FromResult(rotated);
+    }
+
     /// <inheritdoc />
     public Task<bool> RevokeVaultGrantAsync(
         Guid vaultId,
         Guid userId,
-        CancellationToken cancellationToken) =>
-        Task.FromResult(grants.Remove((vaultId, userId)));
+        CancellationToken cancellationToken)
+    {
+        var theirs = grants.Keys
+            .Where(key => key.VaultId == vaultId && key.UserId == userId)
+            .ToList();
+
+        foreach (var key in theirs)
+        {
+            grants.Remove(key);
+        }
+
+        return Task.FromResult(theirs.Count > 0);
+    }
+
+    /// <summary>The generation a vault currently stands at.</summary>
+    private uint Generation(Guid vaultId) =>
+        teamVaults.TryGetValue(vaultId, out var vault) ? vault.KeyGeneration : 1;
 
     /// <summary>Publishes the enrolling account's own key, in the directory and the key log.</summary>
     private void RegisterSelf(KeyStatement statement, byte[] statementSignature)

tests/DodoSSH.Client.App.Tests/TransferQueueingTests.cs

@@ -217,6 +217,70 @@ public sealed class TransferQueueingTests : IDisposable
         Queued().ShouldHaveSingleItem();
     }
 
+    /// <remarks>
+    /// The phone's way in, and it has to obey the same rules as every other: a document chosen in the system
+    /// picker is copied into the cache and the copy is queued, which is an upload with one extra property —
+    /// that this application made the file and will delete it again. Everything about *what may be queued*
+    /// is the same, and this says so rather than leaving a second path free to drift.
+    /// </remarks>
+    [Fact]
+    public void StagedUploads_QueueUnderTheSameRulesAsAnyOther()
+    {
+        Connected();
+
+        var folder = Path.Combine(directory, "a-folder");
+        Directory.CreateDirectory(folder);
+
+        transfers.QueueStagedUploads([File("picked.txt"), folder]);
+
+        Queued().ShouldHaveSingleItem();
+        transfers.Status.ShouldContain("1 file");
+        transfers.Status.ShouldContain("1 folder was skipped");
+    }
+
+    /// <remarks>
+    /// <para>
+    /// The phone's way out. The delivery itself — copying the finished file into the document the save
+    /// picker made — needs a transfer that actually runs and a picker to have made something, so it is
+    /// checked by hand in <c>docs/manual-checks.md</c> phase 14. What is worth pinning here is the pair of
+    /// refusals in front of it, because both would otherwise be discovered as an empty file sitting in
+    /// somebody's Downloads: the picker creates the destination the moment it is dismissed, so anything
+    /// this method turns away after that point has already cost a visible artefact.
+    /// </para>
+    /// </remarks>
+    [Fact]
+    public void ADeliveredDownload_QueuesTheFileAndRefusesADirectory()
+    {
+        var delivered = 0;
+
+        Connected();
+
+        transfers.QueueDeliveredDownload(
+            RemoteFile("one.log"),
+            Path.Combine(directory, "staged", "one.log"),
+            _ =>
+            {
+                delivered++;
+                return Task.CompletedTask;
+            });
+
+        Queued().ShouldHaveSingleItem();
+        transfers.Status.ShouldContain("one.log");
+
+        // A directory has nothing to fetch, and the message is the same one every other path on this screen
+        // gives for the same mistake.
+        transfers.QueueDeliveredDownload(
+            RemoteDirectory("logs"),
+            Path.Combine(directory, "staged", "logs"),
+            _ => Task.CompletedTask);
+
+        Queued().Count.ShouldBe(1);
+        transfers.Status.ShouldContain("Only files");
+
+        // Nothing is delivered by queueing. The callback runs when the bytes are there and not before.
+        delivered.ShouldBe(0);
+    }
+
     /// <summary>The queue's rows, once the posts that create them have been let run.</summary>
     /// <remarks>
     /// <c>TransfersViewModel</c> adds a row from the transfer queue's own <c>Changed</c> event, which it

tests/DodoSSH.Client.App.Tests/VaultSharingTests.cs

@@ -104,12 +104,42 @@ public sealed class VaultSharingTests : IAsyncLifetime
     }
 
     /// <remarks>
-    /// The whole point of a shared vault, in one test. Note what the status line says after the add and
-    /// before the share: adding somebody grants them nothing readable, and the interface has to say so
-    /// rather than let a user believe the credential is already with their colleague.
+    /// The whole point of a shared vault, in one test. Adding somebody wraps the vault to them, so the
+    /// status line names what they were given rather than what is still owed — and the grant is on the
+    /// server before the add has finished reporting.
     /// </remarks>
     [Fact]
-    public async Task CreatingAVaultAndSharingIt_WrapsTheKeyToTheOtherMember()
+    public async Task AddingSomebody_WrapsTheVaultToThemStraightAway()
+    {
+        await UnlockedAsync();
+
+        var vaults = shell.Vaults;
+        var colleague = server.AddAccount("bob@example.com", "Bob Example");
+
+        await CreateVaultAsync(vaults, "Platform secrets");
+
+        var vaultId = vaults.SelectedVault!.VaultId;
+
+        vaults.InviteEmail = "bob@example.com";
+        await vaults.AddMemberCommand.ExecuteAsync(null);
+
+        vaults.Members.Count.ShouldBe(2, vaults.Status);
+
+        server.IssuedGrants.ShouldContainKey(
+            (vaultId, colleague),
+            "adding somebody to a vault is what shares it with them");
+
+        vaults.Status.ShouldContain("Platform secrets");
+    }
+
+    /// <remarks>
+    /// The manual path still works and is still worth having: a vault whose key this machine did not
+    /// hold when somebody was added is shared by pressing the button once it does. Re-wrapping to
+    /// somebody who already holds the key is the same call, and the server replaces the row rather than
+    /// adding a second one.
+    /// </remarks>
+    [Fact]
+    public async Task SharingAVaultByHand_WrapsTheKeyAndSaysWhatItCannotPromise()
     {
         await UnlockedAsync();
 
@@ -121,9 +151,6 @@ public sealed class VaultSharingTests : IAsyncLifetime
         vaults.InviteEmail = "bob@example.com";
         await vaults.AddMemberCommand.ExecuteAsync(null);
 
-        vaults.Members.Count.ShouldBe(2, vaults.Status);
-        vaults.Status.ShouldContain("cannot read anything yet");
-
         vaults.SelectedMember = vaults.Members.Single(member => member.UserId == colleague);
 
         await vaults.ShareVaultCommand.ExecuteAsync(null);
@@ -137,6 +164,91 @@ public sealed class VaultSharingTests : IAsyncLifetime
         vaults.Status.ShouldContain("fingerprint", Case.Insensitive);
     }
 
+    /// <remarks>
+    /// <para>
+    /// The other half of the same idea. Removing somebody withdraws their grants — which only blocks
+    /// future reads — so the vault is rotated in the same breath and the new key goes to the people who
+    /// are left. From that moment nothing written is readable to the person who went.
+    /// </para>
+    /// <para>
+    /// The remaining member is given the earlier generation as well as the new one, which is what keeps
+    /// the vault's existing items readable to them: a rotation re-keys the vault, not its contents.
+    /// </para>
+    /// </remarks>
+    [Fact]
+    public async Task RemovingSomebody_RotatesTheVaultAndHandsTheNewKeyToWhoIsLeft()
+    {
+        await UnlockedAsync();
+
+        var vaults = shell.Vaults;
+        var leaving = server.AddAccount("bob@example.com", "Bob Example");
+        var staying = server.AddAccount("carol@example.com", "Carol Example");
+
+        await CreateVaultAsync(vaults, "Platform secrets");
+
+        var vaultId = vaults.SelectedVault!.VaultId;
+
+        foreach (var address in (string[])["bob@example.com", "carol@example.com"])
+        {
+            vaults.InviteEmail = address;
+            await vaults.AddMemberCommand.ExecuteAsync(null);
+        }
+
+        vaults.Members.Count.ShouldBe(3, vaults.Status);
+
+        vaults.SelectedMember = vaults.Members.Single(member => member.UserId == leaving);
+
+        await vaults.RemoveMemberCommand.ExecuteAsync(null);
+
+        vaults.Status.ShouldContain("Rotated", customMessage: vaults.Status);
+        vaults.Status.ShouldContain("Platform secrets");
+
+        // The last act of a rotation is moving what is already stored onto the new key. Proven by the
+        // bytes in DodoSSH.Client.Sync.Tests; what this asserts is that the shell asks for it at all,
+        // and says which of the two guarantees the user has ended up with.
+        vaults.Status.ShouldContain("re-sealed under the new key", customMessage: vaults.Status);
+
+        // Gone entirely, at every generation. A revocation that left the history behind would leave them
+        // able to read everything written before they went, from a copy of the ciphertext.
+        server.GenerationsGranted(vaultId, leaving).ShouldBeEmpty();
+
+        // And the member who stayed holds both: the new key for what comes next, the old one for what
+        // is already stored under it.
+        server.GenerationsGranted(vaultId, staying).ShouldBe([1u, 2u]);
+    }
+
+    /// <remarks>
+    /// Somebody added after a rotation is given every generation the sharing machine holds, not only the
+    /// newest. A vault shared as one key would open to a list of items that will not decrypt, which
+    /// reads as corruption rather than as the missing grant it is.
+    /// </remarks>
+    [Fact]
+    public async Task AddingSomebodyToARotatedVault_HandsThemItsHistoryAsWell()
+    {
+        await UnlockedAsync();
+
+        var vaults = shell.Vaults;
+        var first = server.AddAccount("bob@example.com", "Bob Example");
+        var second = server.AddAccount("carol@example.com", "Carol Example");
+
+        await CreateVaultAsync(vaults, "Platform secrets");
+
+        var vaultId = vaults.SelectedVault!.VaultId;
+
+        vaults.InviteEmail = "bob@example.com";
+        await vaults.AddMemberCommand.ExecuteAsync(null);
+
+        // Removing them is what rotates the vault, so the next person to be added arrives at a vault
+        // with a history rather than one that has only ever had a single key.
+        vaults.SelectedMember = vaults.Members.Single(member => member.UserId == first);
+        await vaults.RemoveMemberCommand.ExecuteAsync(null);
+
+        vaults.InviteEmail = "carol@example.com";
+        await vaults.AddMemberCommand.ExecuteAsync(null);
+
+        server.GenerationsGranted(vaultId, second).ShouldBe([1u, 2u], vaults.Status);
+    }
+
     /// <remarks>
     /// <para>
     /// The test this whole design exists for. A server that wants to read a shared vault only has to
@@ -159,16 +271,24 @@ public sealed class VaultSharingTests : IAsyncLifetime
 
         await CreateVaultAsync(vaults, "Platform secrets");
 
+        // Before the add, because the add now shares. Both routes to a wrap have to refuse, and a test
+        // that corrupted the log afterwards would be asserting about the second one only.
+        server.CorruptKeyLog = true;
+
         vaults.InviteEmail = "mallory@example.com";
         await vaults.AddMemberCommand.ExecuteAsync(null);
 
-        vaults.SelectedMember = vaults.Members.Single(member => member.UserId == colleague);
+        var vaultId = vaults.SelectedVault!.VaultId;
 
-        server.CorruptKeyLog = true;
+        server.IssuedGrants.ShouldNotContainKey((vaultId, colleague));
+        vaults.Status.ShouldContain("Could not share");
+        vaults.Status.ShouldContain("key log");
+
+        vaults.SelectedMember = vaults.Members.Single(member => member.UserId == colleague);
 
         await vaults.ShareVaultCommand.ExecuteAsync(null);
 
-        server.IssuedGrants.ShouldBeEmpty();
+        server.IssuedGrants.ShouldNotContainKey((vaultId, colleague));
         vaults.Status.ShouldContain("Did not share");
         vaults.Status.ShouldContain("key log");
     }
@@ -409,8 +529,8 @@ public sealed class VaultSharingTests : IAsyncLifetime
     }
 
     /// <remarks>
-    /// The screen's answer to "who can actually open this". Asserted after a share rather than before,
-    /// because an empty list proves nothing about whether the call was made.
+    /// The screen's answer to "who can actually open this". Asserted after somebody has been added
+    /// rather than before, because an empty list proves nothing about whether the call was made.
     /// </remarks>
     [Fact]
     public async Task SelectingAVault_ListsWhoHoldsAKeyToIt()
@@ -425,13 +545,13 @@ public sealed class VaultSharingTests : IAsyncLifetime
         vaults.InviteEmail = "bob@example.com";
         await vaults.AddMemberCommand.ExecuteAsync(null);
 
-        vaults.SelectedMember = vaults.Members.Single(member => member.UserId == colleague);
+        // Two, and the creator is the other: their own self-grant is what makes a vault they just made
+        // readable at all, so a list that left it out would show the one person who can certainly open
+        // this vault as somebody who cannot.
+        vaults.Grants.Count.ShouldBe(2, vaults.Status);
 
-        await vaults.ShareVaultCommand.ExecuteAsync(null);
+        var holder = vaults.Grants.Single(row => row.UserId == colleague);
 
-        var holder = vaults.Grants.ShouldHaveSingleItem();
-
-        holder.UserId.ShouldBe(colleague);
         holder.IsLive.ShouldBeTrue(vaults.Status);
         holder.State.ShouldBe("holds a key");
     }