diff --git a/README.md b/README.md index 5aaa367..bac43c1 100644 --- a/README.md +++ b/README.md @@ -307,10 +307,12 @@ usually staying in it. Six limits, stated rather than discovered: -- **A vault cannot be deleted.** Nothing in this product removes one, and the server refuses to archive the - membership list behind a vault that still exists — a shared vault is readable *because* of membership, so - archiving it would take the vault away from everybody holding a key, including you, quietly and all at - once. The screen says so where you would otherwise go looking for the button. +- **Deleting a vault does not reach a machine that has already synced it.** An admin or the owner can + delete a shared vault: it leaves everybody's list at once, every key to it is withdrawn, and the + membership list behind it is archived when it existed to carry that vault alone. What it cannot do is + take back the copy a colleague pulled yesterday — the same limit revocation has, for the same reason — + and the question you are asked before it happens says so. Your personal vault is refused: everything + filed nowhere else lives in it, and there is no way to make another. - **Removing a member is not retroactive.** It revokes their grants, rotates every vault behind that membership list your machine can open, and hands each new key to the members who are left — so nothing written from that point on is readable to them. Everything they already pulled is still on their machine. @@ -622,8 +624,8 @@ keychain plus a terminal — and the spike that gates all of it. membership to permissions, so a viewer may pull and may not push; the desktop client reads and syncs every vault it holds a key for, and a real VAULTS screen replaces the placeholder. The screen is vault-shaped rather than team-shaped: naming a vault makes the membership list that carries it, so the - team is behind the vault rather than a thing anybody has to create first. A vault can be renamed and - handed to another member; a member row carries when that account was last here; and an address with no + team is behind the vault rather than a thing anybody has to create first. A vault can be renamed, deleted + and handed to another member; a member row carries when that account was last here; and an address with no account on this deployment can be invited, joining the moment somebody signs in with it. See [Sharing a vault](#sharing-a-vault) for the one distinction the whole design rests on, and the limits worth knowing before you rely on it; the reasoning is in @@ -639,6 +641,11 @@ keychain plus a terminal — and the spike that gates all of it. the rotation is re-sealed as it is pushed, so nothing reaches the server under a superseded key at all. See [ADR 0010](docs/adr/0010-vault-key-rotation.md). + **A vault shared with you arrives on the next synchronisation pass**, within the minute, with no sign-in + and nothing to press. There is no push channel, so each pass asks the server which vaults this account can + reach before syncing the ones it already knows — which is also how a vault that has been deleted, or one + whose grant was withdrawn, stops being listed. + **Ownership transfer is here, and it is one write rather than two.** The member you name becomes owner and you become an admin, in a single transaction — because ownership is sole, so promoting first leaves the team owned twice and demoting first leaves it owned by nobody, and there is nobody left with the diff --git a/docs/adr/0009-team-access-model.md b/docs/adr/0009-team-access-model.md index 3e4b44a..3c1a827 100644 --- a/docs/adr/0009-team-access-model.md +++ b/docs/adr/0009-team-access-model.md @@ -145,6 +145,16 @@ Three consequences of the change belong here: still appear is a create whose second call failed; cancelling that form archives it, which is a deliberate exception to this client's rule against tidying up on the user's behalf, made because nothing else can reach it. +- **A vault can be deleted**, which is the operation this ADR's first version said the product did not + have: `DELETE /api/v1/vaults/{id}` requires `PermissionFlags.Admin`, the same line the rename draws and + for a stronger reason — it takes the vault from everybody in it at once. It soft-deletes the row, + withdraws every grant to it, and archives the owning team when that team was made to carry this vault + alone, so deleting the last vault of a one-vault team does not leave a membership list no screen has a + row for. The personal vault is refused: it is created by enrollment, everything filed nowhere else lives + in it, and no call would make another. The items are kept — ciphertext behind a vault nothing resolves — + because deleting them buys no confidentiality and destroys what an operator would need to undo a + mistake. **It reaches no machine that has already synced the vault**, which is the same limit revocation + has and for the same reason; see ADR 0001. The client says so before asking for the confirmation. - **A vault can be renamed**, which it could not before: `PUT /api/v1/vaults/{id}` requires `PermissionFlags.Admin` — the line `UpdateTeamEndpoint` already draws, because a name is what everybody in the vault sees it called rather than part of its contents. It renames the owning team with it when diff --git a/src/DodoSSH.Api/Features/Teams/TeamLog.cs b/src/DodoSSH.Api/Features/Teams/TeamLog.cs index 076dee0..d577a73 100644 --- a/src/DodoSSH.Api/Features/Teams/TeamLog.cs +++ b/src/DodoSSH.Api/Features/Teams/TeamLog.cs @@ -60,6 +60,19 @@ internal static partial class TeamLog Message = "Renamed vault {VaultId} of team {TeamId}.")] internal static partial void VaultRenamed(ILogger logger, Guid vaultId, Guid? teamId); + /// + /// A warning rather than information, and the count is why: this is the one operation that takes a + /// vault away from everybody at once, and how many keys were withdrawn with it is the number an + /// operator asked about it will want. No name, for the reason the rename gives. + /// + [LoggerMessage( + EventId = 2116, + Level = LogLevel.Warning, + Message = "Deleted vault {VaultId} of team {TeamId}, by {ActorId}; withdrew {GrantCount} key " + + "grant(s). Its items are kept and unreadable; copies already synced elsewhere are unaffected.")] + internal static partial void VaultDeleted( + ILogger logger, Guid vaultId, Guid teamId, Guid actorId, int grantCount); + [LoggerMessage( EventId = 2106, Level = LogLevel.Information, diff --git a/src/DodoSSH.Api/Features/Teams/TeamService.cs b/src/DodoSSH.Api/Features/Teams/TeamService.cs index 78fe647..737d786 100644 --- a/src/DodoSSH.Api/Features/Teams/TeamService.cs +++ b/src/DodoSSH.Api/Features/Teams/TeamService.cs @@ -218,10 +218,15 @@ internal sealed class TeamService( /// The vault check is the whole of this operation's safety and it refuses rather than /// cascades. Archiving a team hides it from every member's list at once, and a team vault /// resolves through membership — so archiving one that still owned vaults would take those vaults - /// away from people who hold keys to them, silently, including the caller. Nothing in this product - /// deletes a vault, so there is no sequence of calls that turns this refusal into a success today. - /// That is stated plainly rather than worked around, for the reason the SFTP layer refuses a - /// recursive delete: a refusal is visible and a quiet removal is not. + /// away from people who hold keys to them, silently, including the caller. The way out is to delete + /// those vaults first — VaultGrantService.DeleteVaultAsync, which asks its own question and + /// withdraws every key — and then archive what is left. A refusal that names a route is worth more + /// than a cascade, for the reason the SFTP layer refuses a recursive delete: a refusal is visible and + /// a quiet removal is not. + /// + /// + /// Deleting the last vault of a team made to carry it archives that team on the way past, so + /// the ordinary case never reaches this refusal at all. See DeleteVaultEndpoint. /// /// /// Memberships are archived with the team, in one transaction, because a live membership pointing @@ -246,7 +251,7 @@ internal sealed class TeamService( 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. There is no way to delete a vault in this product yet, so a team with vaults cannot be archived.")); + $"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(); diff --git a/src/DodoSSH.Api/Features/Teams/VaultGrantEndpoints.cs b/src/DodoSSH.Api/Features/Teams/VaultGrantEndpoints.cs index b62730c..44e32f3 100644 --- a/src/DodoSSH.Api/Features/Teams/VaultGrantEndpoints.cs +++ b/src/DodoSSH.Api/Features/Teams/VaultGrantEndpoints.cs @@ -122,6 +122,96 @@ internal sealed class RenameVaultEndpoint( } } +/// Deletes a vault. +/// +/// +/// Admin, like the rename above and for a stronger version of the same reason: this takes the vault away +/// from everybody in it at once, so it belongs to whoever administers the membership list rather than to +/// anybody who may write to its contents. +/// +/// +/// Authenticated rather than Enrolled, again like the rename. Deleting a vault touches no key material — +/// it withdraws grants rather than wrapping anything — so requiring a published identity key would refuse +/// an admin for a reason that has nothing to do with what they are asking. +/// +/// +/// The team behind it is archived in a second call, and the two are deliberately not one +/// transaction. Archiving is TeamService's, it refuses while a team owns vaults, and it can +/// only tell that this one no longer does once the deletion is committed. A crash between the two leaves +/// an empty team — invisible on every screen, archivable afterwards by the endpoint that exists for it, +/// and harmless — which is a better failure than a vault that could not be deleted because tidying up its +/// team failed. +/// +/// +internal sealed class DeleteVaultEndpoint( + ICurrentUserContext currentUser, + IVaultAccessService vaultAccess, + VaultGrantService grants, + TeamService teams) + : EndpointWithoutRequest> +{ + /// + public override void Configure() + { + Delete("/api/v1/vaults/{vaultId:guid}"); + + Policies(Auth.AuthenticatedPolicy); + + Description(b => b + .WithName("DeleteVault") + .WithSummary("Deletes a vault and withdraws every key to it.") + .WithTags("Vaults")); + } + + /// + public override async Task> ExecuteAsync( + CancellationToken ct) + { + var user = await currentUser.GetOrProvisionAsync(ct).ConfigureAwait(false); + var access = await vaultAccess + .ResolveAsync(user.Id, Route("vaultId"), ct) + .ConfigureAwait(false); + + if (!access.Granted || !access.Permissions.HasFlag(PermissionFlags.Read)) + { + // Indistinguishable from a vault that never existed, and from one somebody else deleted a + // moment ago — which is what makes a repeat of this call safe to send. + return TypedResults.NotFound(); + } + + if (!access.Permissions.HasFlag(PermissionFlags.Admin)) + { + return Problems.Coded( + StatusCodes.Status403Forbidden, + ProblemCodes.Forbidden, + "Only an admin or the owner of the team that owns this vault can delete it."); + } + + try + { + var orphaned = await grants.DeleteVaultAsync(user, access.Vault!, ct).ConfigureAwait(false); + + if (orphaned is not null) + { + await teams.ArchiveAsync(user, orphaned, ct).ConfigureAwait(false); + } + + return TypedResults.NoContent(); + } + catch (VaultGrantInvalidException exception) + { + return Problems.Coded( + StatusCodes.Status400BadRequest, ProblemCodes.InvalidVaultGrant, exception.Message); + } + catch (TeamNotEmptyException) + { + // The team gained a vault between the delete and the archive. The deletion stands and is what + // was asked for; the team staying is not a failure the caller can or should act on. + return TypedResults.NoContent(); + } + } +} + /// Wraps this vault's key to another member. /// /// The one call in this API whose body the server can neither produce nor check. It stores a sealed diff --git a/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs b/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs index 3f2b67e..277d8b1 100644 --- a/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs +++ b/src/DodoSSH.Api/Features/Teams/VaultGrantService.cs @@ -216,6 +216,86 @@ internal sealed class VaultGrantService( RekeyRequired: vault.RekeyRequired); } + /// + /// Deletes a vault, and the team behind it where that team exists to carry this vault alone. + /// + /// + /// The team that has been left owning nothing by this, or null where there is none. The caller + /// archives it; see for why that is a second step. + /// + /// + /// + /// Soft, like every other removal here. The row is stamped rather than dropped, so an operator + /// can see that a vault existed and what became of it — and so the foreign keys from its items and its + /// grants stay valid. VaultAccessService filters on the stamp at both ends, so from the moment + /// this commits the vault is absent from every member's /me and every call naming it answers 404. + /// + /// + /// The items are deliberately left alone. They are ciphertext behind a vault nothing will now + /// resolve, so deleting them buys no confidentiality — and it would destroy the one thing an operator + /// restoring a vault deleted by mistake would need. What this is not is a promise about other people's + /// machines: a member who synced yesterday still holds their copy, exactly as ADR 0001 says about + /// revocation, and the deletion message says so. + /// + /// + /// Every grant is withdrawn in the same write. Nothing reads them once the vault is gone, but a + /// live grant on a deleted vault is a row that says somebody holds a key to something that no longer + /// exists — and the grant list is the thing an operator reads to answer "who could open this". + /// + /// + internal async Task DeleteVaultAsync( + UserAccount actor, + Vault vault, + CancellationToken cancellationToken) + { + ArgumentNullException.ThrowIfNull(vault); + + // A personal vault is where everything filed nowhere else lives, and it is created by enrollment + // rather than by anybody choosing to make it. Deleting one would leave an enrolled account with a + // key, no vault, and no way to make the vault it is supposed to have. + if (vault.OwnerKind != VaultOwnerKind.Team || vault.TeamId is not { } teamId) + { + throw new VaultGrantInvalidException( + "A personal vault cannot be deleted. It is where everything filed nowhere else lives, and " + + "the account has no way to make another. Move what you want to keep into a shared vault " + + "and delete that instead."); + } + + var now = clock.GetUtcNow(); + + var grants = await database.VaultKeyGrants + .Where(g => g.VaultId == vault.Id && g.RevokedAtUtc == null) + .ToListAsync(cancellationToken) + .ConfigureAwait(false); + + foreach (var grant in grants) + { + grant.State = GrantState.Revoked; + grant.RevokedAtUtc = now; + } + + vault.DeletedAtUtc = now; + vault.UpdatedAtUtc = now; + + await database.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + + TeamLog.VaultDeleted(logger, vault.Id, teamId, actor.Id, grants.Count); + + // Read after the save, so "owns nothing else" is asked of a database that already knows this one + // is gone. The team is returned rather than archived here for the reason the rename gives about + // renaming it: a team carrying several vaults has a name and a membership list somebody chose, and + // one made to carry this vault alone is a thing its creator was never shown. + var alone = !await database.Vaults + .AnyAsync(other => other.TeamId == teamId && other.DeletedAtUtc == null, cancellationToken) + .ConfigureAwait(false); + + return alone + ? await database.Teams + .SingleOrDefaultAsync(t => t.Id == teamId && t.DeletedAtUtc == null, cancellationToken) + .ConfigureAwait(false) + : null; + } + /// Lists who can open a vault. /// /// One row per holder, not one per grant. A rotated vault holds several grants per member — one per diff --git a/src/DodoSSH.Api/Setup/EndpointRegistration.cs b/src/DodoSSH.Api/Setup/EndpointRegistration.cs index 7ddbd54..bb9d545 100644 --- a/src/DodoSSH.Api/Setup/EndpointRegistration.cs +++ b/src/DodoSSH.Api/Setup/EndpointRegistration.cs @@ -57,6 +57,7 @@ internal static class EndpointRegistration typeof(RevokeTeamInvitationEndpoint), typeof(CreateTeamVaultEndpoint), typeof(RenameVaultEndpoint), + typeof(DeleteVaultEndpoint), typeof(ListVaultGrantsEndpoint), typeof(IssueVaultGrantEndpoint), typeof(RevokeVaultGrantEndpoint), diff --git a/src/DodoSSH.Client.Android/Views/FilesScreen.axaml b/src/DodoSSH.Client.Android/Views/FilesScreen.axaml index 514a71d..bf43a61 100644 --- a/src/DodoSSH.Client.Android/Views/FilesScreen.axaml +++ b/src/DodoSSH.Client.Android/Views/FilesScreen.axaml @@ -85,7 +85,7 @@ - @@ -98,6 +98,16 @@ + + +