diff --git a/src/DodoSSH.Api/Features/Teams/TeamEndpoints.cs b/src/DodoSSH.Api/Features/Teams/TeamEndpoints.cs index 465a943..512f60a 100644 --- a/src/DodoSSH.Api/Features/Teams/TeamEndpoints.cs +++ b/src/DodoSSH.Api/Features/Teams/TeamEndpoints.cs @@ -356,6 +356,13 @@ internal sealed class AddTeamMemberEndpoint(ICurrentUserContext currentUser, Tea return Problems.Coded( StatusCodes.Status400BadRequest, ProblemCodes.InvalidTeam, exception.Message); } + catch (NoSuchAccountException exception) + { + // Its own code so the caller can offer an invitation rather than report a failure. 404 + // rather than 400: the request was well formed and named something that is not here. + return Problems.Coded( + StatusCodes.Status404NotFound, ProblemCodes.NoSuchAccount, exception.Message); + } } } diff --git a/src/DodoSSH.Api/Features/Teams/TeamExceptions.cs b/src/DodoSSH.Api/Features/Teams/TeamExceptions.cs index 47b0bfb..36651b9 100644 --- a/src/DodoSSH.Api/Features/Teams/TeamExceptions.cs +++ b/src/DodoSSH.Api/Features/Teams/TeamExceptions.cs @@ -34,6 +34,14 @@ internal sealed class LastTeamOwnerException(string message) : Exception(message /// internal sealed class TeamNotEmptyException(string message) : Exception(message); +/// The address given to an add has no account on this server. +/// +/// Separate from because the caller can act on it without being +/// told to: there is nobody to add, so the address is invited instead. Folded into the general code it +/// would be indistinguishable from a rejected role, and a client would have to guess which it was. +/// +internal sealed class NoSuchAccountException(string message) : Exception(message); + /// An invitation was rejected. /// /// Separate from because its commonest cause has a different diff --git a/src/DodoSSH.Api/Features/Teams/TeamService.cs b/src/DodoSSH.Api/Features/Teams/TeamService.cs index c7ecfe4..78fe647 100644 --- a/src/DodoSSH.Api/Features/Teams/TeamService.cs +++ b/src/DodoSSH.Api/Features/Teams/TeamService.cs @@ -484,6 +484,12 @@ internal sealed class TeamService( /// restore their revoked key grants: those were wrapped to a generation the vault has since been /// flagged to leave behind, and a member holding Share has to wrap the key afresh. /// + /// + /// Enrollment is not required of the account being added, and asking for it would be asking the + /// wrong question. A membership is authorization and grants nothing readable — that is the whole + /// of ADR 0009 — so somebody can be added on Monday and publish a key on Tuesday, which is what + /// TeamMemberSummary.IsEnrolled is for. + /// /// internal async Task AddMemberAsync( UserAccount actor, @@ -500,18 +506,7 @@ internal sealed class TeamService( + "by adding somebody."); } - var target = await database.Users - .SingleOrDefaultAsync( - u => u.Id == request.UserId && u.DeletedAtUtc == null, - cancellationToken) - .ConfigureAwait(false) - - // Safe to be specific: the caller supplied this id from a directory lookup they just - // made, so it confirms nothing they did not already know. - ?? throw new TeamInvalidException( - "No such account on this server. A member has to sign in here once before they can " - + "be added — that is what creates the account and publishes the key a vault would " - + "be shared with."); + var target = await ResolveTargetAsync(request, cancellationToken).ConfigureAwait(false); var now = clock.GetUtcNow(); @@ -551,6 +546,66 @@ internal sealed class TeamService( return await DescribeAsync(target, membership, cancellationToken).ConfigureAwait(false); } + /// + /// Finds the account an add is aimed at, by id when the caller has one and by address otherwise. + /// + /// + /// + /// The two are not interchangeable and the order matters. An id came from a directory lookup the + /// caller has already made, so it names an account whose key they have seen; an address is what is + /// left when the directory could not answer, which it cannot for anybody who has signed in but not + /// yet published a key. + /// + /// + /// Only the active, undeleted account matters here, and the address is matched the way the + /// directory matches it — the email column is citext, so the comparison is case-insensitive in the + /// database and the partial unique index on it means at most one row can answer. + /// + /// + /// Both misses are specific, and neither is a new oracle. An id confirms nothing the caller did not + /// already know from the lookup that produced it. An address is answered only for an admin or owner + /// of the team the add names — checked by the endpoint before this runs — and is the same fact the + /// member list would show them a moment later. It carries its own code so the caller can invite the + /// address instead of reporting a failure at somebody who simply is not here yet. + /// + /// + private async Task ResolveTargetAsync( + AddTeamMemberRequest request, + CancellationToken cancellationToken) + { + if (request.UserId != Guid.Empty) + { + return await database.Users + .SingleOrDefaultAsync( + u => u.Id == request.UserId && u.DeletedAtUtc == null, + cancellationToken) + .ConfigureAwait(false) + + ?? throw new NoSuchAccountException( + "No such account on this server. A member has to sign in here once before they " + + "can be added — that is what creates the account."); + } + + var email = (request.Email ?? string.Empty).Trim(); + + if (email.Length == 0) + { + throw new TeamInvalidException( + "Say who to add: either a user id from the directory, or the email address they sign " + + "in with."); + } + + return await database.Users + .SingleOrDefaultAsync( + u => u.Email == email && u.DeletedAtUtc == null && u.Status == UserStatus.Active, + cancellationToken) + .ConfigureAwait(false) + + ?? throw new NoSuchAccountException( + "No account here uses that address yet. Invite it instead — they join when they " + + "first sign in."); + } + /// /// Enrollment is looked up rather than inferred, because it is the one field on a member row that /// is about them and not about the membership: somebody can be added on Monday and set their diff --git a/src/DodoSSH.Client.Shell/ViewModels/TeamsViewModel.cs b/src/DodoSSH.Client.Shell/ViewModels/TeamsViewModel.cs index d9088e5..3f87d54 100644 --- a/src/DodoSSH.Client.Shell/ViewModels/TeamsViewModel.cs +++ b/src/DodoSSH.Client.Shell/ViewModels/TeamsViewModel.cs @@ -470,16 +470,31 @@ internal sealed partial class TeamsViewModel( /// Adds a member, by looking their address up in the directory first. /// /// - /// Two calls rather than one, and the order is the point: the directory is what turns an address into - /// an account and a public key, and the key that gets verified before any sharing is the one that - /// lookup returned. Letting the server resolve an address to an account inside the add would put an - /// unwitnessed step between the two. + /// + /// The directory is asked first, and the order is the point: it is what turns an address into an + /// account and a public key, and the key that gets verified before any sharing is the one + /// that lookup returned. Resolving the address server-side when the directory could answer would + /// put an unwitnessed step between the two. + /// + /// + /// A directory miss is not an absent account, and treating it as one was a bug worth naming. + /// The directory returns only accounts that have published a key, so everybody between their first + /// sign-in and their enrollment is missing from it. Falling straight through to an invitation told + /// somebody who was standing right there that they had no account here, left the members list + /// unchanged, and made them wait for a sweep that runs at most hourly. So the miss is retried as an + /// add by address, and only a server that says there is no such account reaches the invitation. + /// /// [RelayCommand] private async Task AddMemberAsync(CancellationToken cancellationToken) { if (connection() is not { } server || SelectedTeam is not { } team) { + // Never silent. This command's failures used to be visible only as a flicker of the busy + // flag, which reads as a button that does nothing at all. + Status = connection() is null + ? "Offline. Adding a member changes who the server will serve, so it needs a connection." + : "Select a team on the left first — a member is added to one team, not to all of them."; return; } @@ -496,30 +511,59 @@ internal sealed partial class TeamsViewModel( var found = await server.Directory.LookupByEmailAsync(email, cancellationToken) .ConfigureAwait(true); - if (found.Count == 0) + var request = found.Count > 0 + ? new AddTeamMemberRequest(found[0].UserId, NewMemberRole) + : new AddTeamMemberRequest(Guid.Empty, NewMemberRole, email); + + TeamMemberSummary member; + + try { + member = await server.Teams + .AddTeamMemberAsync(team.TeamId, request, cancellationToken) + .ConfigureAwait(true); + } + catch (DodoSshApiException exception) + when (string.Equals( + exception.Code, ProblemCodes.NoSuchAccount, StringComparison.Ordinal)) + { + // The address really is unknown here, which only the server can say. This is the one + // route to an invitation, and it is now a fact rather than an inference from silence. await InviteAsync(server, team, email, cancellationToken).ConfigureAwait(true); return; } - var member = await server.Teams - .AddTeamMemberAsync( - team.TeamId, - new AddTeamMemberRequest(found[0].UserId, NewMemberRole), - cancellationToken) - .ConfigureAwait(true); - InviteEmail = string.Empty; await ReloadAsync(cancellationToken).ConfigureAwait(true); - // Said out loud, every time. The single most common misunderstanding this design invites is - // that adding somebody gave them the vault. - Status = $"Added {member.Email ?? member.DisplayName ?? "the account"} as a member. They " - + "cannot read anything yet — select a vault below and share its key."; + Status = Describe(member); }).ConfigureAwait(true); } + /// + /// What just happened to the account that was added, and what is still owed them. + /// + /// + /// Both branches say out loud that nothing readable was granted, because the single most common + /// misunderstanding this design invites is that adding somebody gave them the vault. The unenrolled + /// branch says more, and has to: their row will sit in the list saying it holds no key, and without + /// this somebody would read that as the addition having half-failed rather than as a colleague who + /// has not finished setting their machine up. It is also the one case where SHARE KEY cannot be the + /// next step, so pointing at it would be pointing at a button that will refuse. + /// + private static string Describe(TeamMemberSummary member) + { + var who = member.Email ?? member.DisplayName ?? "the account"; + + return member.IsEnrolled + ? $"Added {who} as a member. They cannot read anything yet — select a vault below and " + + "share its key." + : $"Added {who} as a member. They have no key yet, so their row says so and no vault can " + + "be shared with them until they finish signing in on their own machine. The " + + "membership is real in the meantime."; + } + /// /// Invites an address the directory does not know. /// @@ -531,6 +575,11 @@ internal sealed partial class TeamsViewModel( /// is reported afterwards, because the difference decides what they have to do next. /// /// + /// It is reached only after the server has said there is no such account. The directory's silence + /// is not enough and never was: it omits everybody who has not published a key, so inviting on the + /// strength of it told people with accounts that they had none. + /// + /// /// The message has to carry the whole mechanism. Nothing is sent — this server has no outbound /// mail — so somebody who reads "invited" and waits has been misled by an interface that knew /// better. diff --git a/src/DodoSSH.Contracts/ProblemCodes.cs b/src/DodoSSH.Contracts/ProblemCodes.cs index 8a17abc..2fdd968 100644 --- a/src/DodoSSH.Contracts/ProblemCodes.cs +++ b/src/DodoSSH.Contracts/ProblemCodes.cs @@ -119,6 +119,25 @@ public static class ProblemCodes /// public const string TeamNotEmpty = "team-not-empty"; + /// + /// An address given to POST /api/v1/teams/{teamId}/members has no account on this server. + /// + /// + /// + /// Its own code rather than folded into because it is the one add failure + /// with a remedy the client can take unprompted: there is nobody to add, so invite the address + /// instead. A client that could not tell this apart from a rejected role would have to either + /// invite on every failure or never. + /// + /// + /// It answers whether an address has an account here, which CreateTeamInvitationRequest + /// deliberately does not. The exposure is bounded by the same authorization the add already needs — + /// only an admin or owner of the team reaches it — and it is what the caller learns anyway the + /// moment the account appears in the member list. + /// + /// + public const string NoSuchAccount = "no-such-account"; + /// /// An invitation was rejected: a malformed address, an unknown or ownership role, an expiry the /// server will not issue, or an address that already has an account here. diff --git a/src/DodoSSH.Contracts/PublicAPI.Unshipped.txt b/src/DodoSSH.Contracts/PublicAPI.Unshipped.txt index 795b674..35ad9f7 100644 --- a/src/DodoSSH.Contracts/PublicAPI.Unshipped.txt +++ b/src/DodoSSH.Contracts/PublicAPI.Unshipped.txt @@ -13,6 +13,7 @@ const DodoSSH.Contracts.ProblemCodes.InvalidTeamInvitation = "invalid-team-invit const DodoSSH.Contracts.ProblemCodes.InvalidVaultGrant = "invalid-vault-grant" -> string! const DodoSSH.Contracts.ProblemCodes.LastTeamOwner = "last-team-owner" -> string! const DodoSSH.Contracts.ProblemCodes.MalformedRequest = "malformed-request" -> string! +const DodoSSH.Contracts.ProblemCodes.NoSuchAccount = "no-such-account" -> string! const DodoSSH.Contracts.ProblemCodes.PushBatchTooLarge = "push-batch-too-large" -> string! const DodoSSH.Contracts.ProblemCodes.RelayLimitReached = "relay-limit-reached" -> string! const DodoSSH.Contracts.ProblemCodes.RelayTargetRejected = "relay-target-rejected" -> string! @@ -23,8 +24,10 @@ const DodoSSH.Contracts.ProblemCodes.TypeBaseUri = "https://dodossh.dev/problems const DodoSSH.Contracts.ProblemCodes.VaultConflict = "vault-conflict" -> string! DodoSSH.Contracts.AddTeamMemberRequest DodoSSH.Contracts.AddTeamMemberRequest.$() -> DodoSSH.Contracts.AddTeamMemberRequest! -DodoSSH.Contracts.AddTeamMemberRequest.AddTeamMemberRequest(System.Guid UserId, DodoSSH.Contracts.TeamMemberRole Role) -> void -DodoSSH.Contracts.AddTeamMemberRequest.Deconstruct(out System.Guid UserId, out DodoSSH.Contracts.TeamMemberRole Role) -> void +DodoSSH.Contracts.AddTeamMemberRequest.AddTeamMemberRequest(System.Guid UserId, DodoSSH.Contracts.TeamMemberRole Role, string? Email = null) -> void +DodoSSH.Contracts.AddTeamMemberRequest.Deconstruct(out System.Guid UserId, out DodoSSH.Contracts.TeamMemberRole Role, out string? Email) -> void +DodoSSH.Contracts.AddTeamMemberRequest.Email.get -> string? +DodoSSH.Contracts.AddTeamMemberRequest.Email.init -> void DodoSSH.Contracts.AddTeamMemberRequest.Equals(DodoSSH.Contracts.AddTeamMemberRequest? other) -> bool DodoSSH.Contracts.AddTeamMemberRequest.Role.get -> DodoSSH.Contracts.TeamMemberRole DodoSSH.Contracts.AddTeamMemberRequest.Role.init -> void diff --git a/src/DodoSSH.Contracts/Teams.cs b/src/DodoSSH.Contracts/Teams.cs index 85dae4b..70d0717 100644 --- a/src/DodoSSH.Contracts/Teams.cs +++ b/src/DodoSSH.Contracts/Teams.cs @@ -210,14 +210,43 @@ public sealed record TeamMemberSummary( /// Adds a member to a team. /// -/// By user id rather than by email, and the id comes from a directory lookup the caller has already -/// made. That ordering is not incidental: whoever adds a member is usually about to wrap a vault key -/// to their public key, and the key they must verify is the one the directory returned. Adding by -/// email here would put an account resolution the client never saw between those two steps. +/// +/// By user id when the caller has one, and the id comes from a directory lookup they have already +/// made. That ordering is not incidental: whoever adds a member is usually about to wrap a vault +/// key to their public key, and the key they must verify is the one the directory returned. Resolving +/// an address server-side when an id was available would put an account resolution the client never +/// saw between those two steps. +/// +/// +/// exists because the directory cannot answer for everybody. It returns +/// only accounts that have published a key — an entry exists to be wrapped to, and one carrying no key +/// is a check callers forget exactly once — so an account between its first sign-in and its enrollment +/// is invisible there. It is still an account, and it can still be a member: membership is server-side +/// authorization and grants nothing readable, which is why +/// exists to say that a member has no key yet. Without this field such a person could not be added at +/// all, and a caller reading the directory's silence as "no account here" would invite an address that +/// already has one. +/// +/// +/// No key is verified on this path, and none needs to be: nothing is wrapped by adding somebody. The +/// key that matters is fetched and checked at share time, from the directory, by the machine holding +/// the vault key. +/// /// -/// The account to add, as returned by the directory. +/// +/// The account to add, as returned by the directory. defers to +/// . +/// /// Role to grant. -public sealed record AddTeamMemberRequest(Guid UserId, TeamMemberRole Role); +/// +/// The address to resolve, used only when is . Matched +/// case-insensitively, exactly as the directory matches. An address with no account here is refused +/// with so the caller can offer an invitation instead. +/// +public sealed record AddTeamMemberRequest( + Guid UserId, + TeamMemberRole Role, + string? Email = null); /// Changes a member's role. /// The new role. diff --git a/tests/DodoSSH.Api.Tests/TeamEndpointTests.cs b/tests/DodoSSH.Api.Tests/TeamEndpointTests.cs index d8e5b06..115a4cb 100644 --- a/tests/DodoSSH.Api.Tests/TeamEndpointTests.cs +++ b/tests/DodoSSH.Api.Tests/TeamEndpointTests.cs @@ -450,6 +450,69 @@ public sealed class TeamEndpointTests(ApiFixture fixture) vault.TeamId.ShouldBe(team.TeamId); } + /// + /// + /// An account exists from its owner's first authenticated request and publishes no key until they + /// enroll, and the directory omits it for that whole window — deliberately, because an entry exists + /// to be wrapped to. So the lookup is asserted empty first: that is not a missing account, and a + /// caller that read it as one would invite an address that already has one. + /// + /// + /// Adding by address is what covers the gap, and the member it produces says IsEnrolled + /// false. Membership is authorization and grants nothing readable, so there is nothing inconsistent + /// about a member with no key — it is the state everybody passes through. + /// + /// + [Fact] + public async Task AnAccountThatHasNotEnrolled_CanBeAddedByAddressThoughTheDirectoryOmitsIt() + { + var owner = await EnrolledClientAsync("unenrolled-owner", "uowner@example.com"); + + var address = NewAddress(); + var (_, userId) = await SignInAsync(address); + + var team = await CreateTeamAsync(owner, "Newcomers"); + + var found = await ReadAsync>( + owner, $"/api/v1/directory?email={Uri.EscapeDataString(address)}"); + + found.ShouldBeEmpty("they have published no key, so there is nothing to wrap to"); + + var member = await PostAsync( + owner, + MembersUrl(team.TeamId), + new AddTeamMemberRequest(Guid.Empty, TeamMemberRole.Member, address)); + + member.UserId.ShouldBe(userId); + member.IsEnrolled.ShouldBeFalse(); + member.Role.ShouldBe(TeamMemberRole.Member); + + var listed = await ReadAsync>( + owner, MembersUrl(team.TeamId)); + + listed.ShouldContain(row => row.UserId == userId && !row.IsEnrolled); + } + + /// + /// The other side of it. An address with no account is refused under its own code rather than the + /// general one, because the caller can act on it unprompted — there is nobody to add, so invite + /// them — and a code shared with a rejected role would leave them guessing which had happened. + /// + [Fact] + public async Task AddingAnAddressWithNoAccount_IsRefusedWithItsOwnCode() + { + var owner = await EnrolledClientAsync("no-account-owner", "naowner@example.com"); + + var team = await CreateTeamAsync(owner, "Nobody"); + + var response = await owner.PostContractAsync( + MembersUrl(team.TeamId), + new AddTeamMemberRequest(Guid.Empty, TeamMemberRole.Member, NewAddress())); + + await ShouldBeProblemAsync( + response, HttpStatusCode.NotFound, ProblemCodes.NoSuchAccount); + } + /// /// A viewer may read the vault and may not write to it. The failure this guards is the quiet one: a /// role that resolved to the wrong flags would let somebody who was added to look at a vault change diff --git a/tests/DodoSSH.Client.App.Tests/FakeVaultServer.Teams.cs b/tests/DodoSSH.Client.App.Tests/FakeVaultServer.Teams.cs index 3be6758..6134346 100644 --- a/tests/DodoSSH.Client.App.Tests/FakeVaultServer.Teams.cs +++ b/tests/DodoSSH.Client.App.Tests/FakeVaultServer.Teams.cs @@ -31,6 +31,17 @@ internal sealed partial class FakeVaultServer : ITeamApi, IDirectoryApi, IVaultG private readonly List directory = []; private readonly Dictionary> invitations = []; + /// + /// Every account on this fake server, enrolled or not. + /// + /// + /// Kept apart from because the real server keeps them apart, and the gap + /// between the two is where a real bug lived: the directory omits anybody who has not published a + /// key, so a fake that had only one list could not tell an account that does not exist from one + /// that exists and has not enrolled — which is exactly the distinction the add path turns on. + /// + private readonly List<(Guid UserId, string Email, string DisplayName)> accounts = []; + /// public ITeamApi Teams => this; @@ -75,6 +86,27 @@ internal sealed partial class FakeVaultServer : ITeamApi, IDirectoryApi, IVaultG KeyGeneration: 1, KeyLogSequence: sequence)); + accounts.Add((userId, email, displayName)); + + return userId; + } + + /// + /// Registers an account that has signed in here but has not enrolled a key. + /// + /// + /// Normal rather than exotic: an account exists from its owner's first authenticated request and + /// stays keyless until they choose a passphrase on their own machine. It is absent from the + /// directory throughout, because a directory entry exists to be wrapped to and this one has nothing + /// to wrap. It can still be made a member — membership grants nothing readable. + /// + /// Their user id. + internal Guid AddUnenrolledAccount(string email, string displayName) + { + var userId = Guid.CreateVersion7(); + + accounts.Add((userId, email, displayName)); + return userId; } @@ -241,27 +273,40 @@ internal sealed partial class FakeVaultServer : ITeamApi, IDirectoryApi, IVaultG return members.TryGetValue(teamId, out var list) ? [.. list] : []; } - /// + /// Adds a member, resolved by id when the caller has one and by address otherwise. + /// + /// Resolved against rather than , which is the whole + /// point of the two being separate here: an account with no published key is missing from the + /// directory and is still perfectly addable. IsEnrolled is reported from whether the + /// directory has them rather than hardcoded, so a member row can say it holds no key. + /// public Task AddTeamMemberAsync( Guid teamId, AddTeamMemberRequest request, CancellationToken cancellationToken) { - var entry = directory.Find(candidate => candidate.UserId == request.UserId) - ?? throw new DodoSshApiException( - System.Net.HttpStatusCode.BadRequest, - ProblemCodes.InvalidTeam, + var account = request.UserId != Guid.Empty + ? accounts.Find(candidate => candidate.UserId == request.UserId) + : accounts.Find(candidate => string.Equals( + candidate.Email, request.Email, StringComparison.OrdinalIgnoreCase)); + + if (account.UserId == Guid.Empty) + { + throw new DodoSshApiException( + System.Net.HttpStatusCode.NotFound, + ProblemCodes.NoSuchAccount, "No such account on this server."); + } // LastActiveAt is left null: this account has been added, not seen. The owner's row carries a // real one, so both branches of the interface's "last active / never" split are exercised. var member = new TeamMemberSummary( - entry.UserId, - entry.Email, - entry.DisplayName, + account.UserId, + account.Email, + account.DisplayName, request.Role, TeamMemberStatus.Active, - IsEnrolled: true, + IsEnrolled: directory.Exists(entry => entry.UserId == account.UserId), DateTimeOffset.UnixEpoch, LastActiveAt: null); diff --git a/tests/DodoSSH.Client.App.Tests/TeamSharingTests.cs b/tests/DodoSSH.Client.App.Tests/TeamSharingTests.cs index bf68b83..9f8e1ca 100644 --- a/tests/DodoSSH.Client.App.Tests/TeamSharingTests.cs +++ b/tests/DodoSSH.Client.App.Tests/TeamSharingTests.cs @@ -502,6 +502,69 @@ public sealed class TeamSharingTests : IAsyncLifetime teams.Status.ShouldContain("cannot send mail"); } + /// + /// + /// The regression this whole path was rewritten for. An account exists from its owner's first + /// authenticated request and publishes no key until they choose a passphrase on their own machine, + /// and the directory omits it for that entire window — an entry exists to be wrapped to, and this + /// one has nothing to wrap. Reading that silence as "there is no such account" meant ADD MEMBER + /// quietly issued an invitation instead: the members list did not change, the screen said they had + /// no account here, and they only actually joined on the next hourly sweep. + /// + /// + /// So the assertion is that they are a member, not an invitation, and that the row says + /// what is true of them — no key, so nothing can be shared with them yet. + /// + /// + [Fact] + public async Task AddingAnAccountThatHasNotEnrolled_MakesThemAMemberWithNoKey() + { + await UnlockedAsync(); + + var teams = shell.Teams; + var colleague = server.AddUnenrolledAccount("carol@example.com", "Carol Example"); + + await CreateTeamAsync(teams, "Platform", "platform"); + + teams.InviteEmail = "carol@example.com"; + await teams.AddMemberCommand.ExecuteAsync(null); + + teams.Invitations.ShouldBeEmpty("they have an account here, so there is nothing to invite"); + + teams.Members.Count.ShouldBe(2, teams.Status); + + var member = teams.Members.Single(row => row.UserId == colleague); + + member.Email.ShouldBe("carol@example.com"); + + // The label the user asked to see, and the reason SHARE KEY is not the next step. + member.KeyState.ShouldContain("no key yet"); + + teams.Status.ShouldContain("Added"); + teams.Status.ShouldContain("no key yet"); + } + + /// + /// The other half of the pair above: an address with no account at all still falls through to an + /// invitation. It is the server that decides which, so this proves the fall-through survived being + /// moved behind it rather than being replaced by an error. + /// + [Fact] + public async Task AddingAnAddressWithNoAccount_StillInvitesRatherThanFailing() + { + await UnlockedAsync(); + + var teams = shell.Teams; + + await CreateTeamAsync(teams, "Platform", "platform"); + + teams.InviteEmail = "stranger@example.com"; + await teams.AddMemberCommand.ExecuteAsync(null); + + teams.Members.ShouldHaveSingleItem("nobody has joined — they have only been invited"); + teams.Invitations.ShouldHaveSingleItem().Email.ShouldBe("stranger@example.com"); + } + /// /// A withdrawn invitation stays on the list saying it was withdrawn, rather than vanishing. One that /// disappeared would read as never having been sent, which is the same thing the screen looks like