Public Access
The delta pull was cheap enough to run on a timer and the client did, once a minute. That is fine for a machine and wrong for two people: an edit a colleague makes is up to a minute stale, which is long enough for both of them to make it and produce a conflict neither needed to have. Shortening the interval is the obvious answer and the wrong one — it costs a request per client per interval whether or not anything happened, and it converges on a busier server that is still late. So the server now says so. A client holds a WebSocket open at GET /api/v1/events, subprotocol dodossh.events.v1, and gets a line down it when something it can read has changed. ADR 0012 has the reasoning; three parts of it are worth repeating here, because they are what everything else rests on. **What crosses the socket is a notice, never data.** A frame names a vault and how far its change log has got. No item, no ciphertext, not even which item it was. The client's answer is the delta pull it would have run anyway, so there is still exactly one code path that applies a change to a keychain, and it is not this one. Pushing the items themselves would save a round trip and fork that path in two, with the cursor, the merge and the tombstone rules duplicated across both — ADR 0003 put every mutation through one write path for that reason, and this keeps every read on one for the same one. It also makes a dropped notice harmless, which is what lets the fan-out below be as simple as it is. **Polling stays, and is what guarantees a pass.** The minute timer is unchanged. A network that eats WebSockets, a server with Events:Enabled off, an older server, a proxy that will not upgrade, a notice dropped under backpressure — every one of those leaves a client behaving exactly as it did before this commit. Nothing is reachable only over the socket and nothing is meant to become so; VaultViewModel's AutoSyncInterval remark now says that where somebody changing it will read it. **The bearer token authorises the upgrade, unlike the relay's ticket.** Not an inconsistency with ADR 0004: the relay's socket is a byte pipe whose whole authorization decision — which host, which IPs, which port — is made before it opens and never revisited, and it is the extraction seam for a process that must hold no ACL code. This one is a view of the caller's own vault list and has to keep answering "what may this account read" for as long as it is held. A ticket would carry that answer in a token and be wrong the moment the account's access changed. The two bounds that arrangement needs are met rather than waved at: the socket is closed at the token's exp with close code 4401 and the client comes straight back with a fresh one, and the vault set is re-resolved every few minutes as well as on the changes known to affect it. Both bound *metadata*, because a notice contains nothing else and reading a vault still needs a key this server has never held. **The fan-out.** VaultEventHub is a singleton holding the sockets this node accepted; publishing walks them and asks each whether it cares, rather than keeping a vault-to-subscriber index that every re-subscription would have to move entries between under a lock publishing also takes. At a few hundred sockets per node and an event rate bounded by how often people edit keychains, the walk is not measurable and its races are obvious. Per-connection queues are bounded and drop the *oldest*: a notice means "pull vault X, which is at least at sequence N", so the newest subsumes what it displaces and the client's answer is identical either way — which is what lets the publish path be void, never block, and never fail. Announced from the endpoint rather than from SyncService, and that placement is the point: by then the push has committed and released the per-vault advisory lock. From inside it would name a sequence no reader can see yet and would hold the lock that serialises writers across a socket write. Only the highest *applied* sequence, so a batch of pure conflicts announces nothing, and a duplicate — already announced when it first landed — announces nothing either. Grants and membership publish too, and those take the *recipient* rather than the actor. This is what AdmitNewVaultsAsync has been apologising for since sharing shipped — "the recipient is handed nothing, there is no push channel" — and the README with it. A vault shared with somebody now turns up as it is shared. The comment and the README paragraph both say what is true now, and both keep saying that the pass is what *discovers* the vault, because a client with no socket has to arrive at the same place. **On the client**, VaultEventStream is really a reconnection policy wrapped round a ClientWebSocket: a dropped socket is the ordinary case here — laptops sleep, proxies time out, tokens expire, servers are redeployed — so nothing in it treats a failure as exceptional, and every path ends in "wait, then dial again". A connection that lived long enough to say hello resets the backoff, so a laptop that woke, worked, and lost its network an hour later does not inherit a minute-long wait it has already proved it need not take. A 4401 close skips the backoff entirely and asks the token provider again, which is the whole reason that close code is distinct. A server that does not advertise the events feature gets IdleVaultEventStream, which never delivers — so IVaultServer.Events is never null and every caller stays on one shape, because the correct behaviour without a socket is the behaviour with a silent one. The shell's background loop now selects between the timer and a notice, and both waits are held across iterations. That is load-bearing rather than tidy: PeriodicTimer permits one outstanding WaitForNextTickAsync and throws on a second, and an abandoned channel read stays registered and consumes the next notice written. Either defect leaves the first notice working and every one after it silently lost, which is why NoticesKeepWakingTheLoop_NotJustTheFirst pushes three and not one. Notices are coalesced over a quarter of a second, so one person's save — a host and its log entry are two items — and a colleague clearing a folder each cost one pass rather than a dozen. **The kind is a string, not an enum**, and that is a compatibility decision. UseStringEnumConverter throws on a value it does not know, so a newer server sending a kind an older client had never heard of would not add an unreadable frame — it would break that client's socket outright. A string is ignored instead. ProblemCodes is the same shape for the same reason. **Tested on both sides, through the real pipeline.** The endpoint suite opens a genuine socket against TestServer and proves a push produces a notice, that another account's push does not reach it, that a ping is answered, and that a frame this server cannot parse does not end the connection. Two of those assert on *ordering* rather than on absence within a timeout — the stranger's write goes first, so a socket that leaked would have announced it before the one the test waits for — because "nothing arrived in two seconds" is a test that passes on a slow machine for the wrong reason. And ANoticeCarriesNoCiphertext asserts on the bytes that crossed the wire rather than on the record's fields, since the latter would only prove that this type has no payload member, which is a tautology; the former is what catches a field added later without anybody thinking about disclosure. The client suite drives VaultEventStream through an injected connector, because the one thing a test cannot do to a real network is make it fail on cue — and failure is the entire subject. The shell suite proves a notice produces a pull inside ten seconds against a sixty-second timer, so the timer cannot be what caused it. **Two limits, stated rather than left to be discovered.** Fan-out is in-process, so a deployment running more than one API replica only pushes for writes its own replica handled and the rest arrive on the timer. IVaultEventPublisher is the seam a PostgreSQL LISTEN/NOTIFY backplane implements and it is deliberately not implemented: an untested backplane is worse than a documented gap, and multiple replicas degrade to the behaviour before this commit rather than breaking. And a client is notified of its own writes; it pushed, so it already pulled, and the extra pass finds nothing. Suppressing that echo correctly needs a per-device identity on the socket, and the same user's other machines must still be told. Manual checks phase 15 covers what no test here can reach, which is the network in between: a proxy that will not upgrade, one that drops an idle socket without telling either end, a laptop lid, a token expiring. Every one of those is invisible inside a test host, and every check there passes only if the change arrives quickly *and* still arrives with the socket taken away. ADR 0012 also fixes one thing about the shared terminal session this is the transport for, so it need not be renegotiated later: session data will be binary frames on this same socket, because base64 in a JSON envelope is the wrong shape for the one payload here that is continuous rather than occasional. Two questions it explicitly does not answer by implication — whether those bytes go through the API at all, and what end-to-end encryption means when the second party watches a stream rather than holding a key — are ADR 0001 questions and get their own decision. 1512 tests pass. DodoSSH.SystemTests was not run — it needs the whole compose stack — so the end-to-end path is unverified for this change beyond what the manual checks describe.
219 lines
14 KiB
C#
219 lines
14 KiB
C#
using Microsoft.AspNetCore.Authorization;
|
|
using Microsoft.AspNetCore.Http;
|
|
using Microsoft.AspNetCore.Http.Metadata;
|
|
using Microsoft.AspNetCore.Routing;
|
|
using Microsoft.Extensions.DependencyInjection;
|
|
using Shouldly;
|
|
using Xunit;
|
|
|
|
namespace DodoSSH.Api.Tests;
|
|
|
|
/// <summary>
|
|
/// Every route the server exposes, and the authorization decision behind each one.
|
|
/// </summary>
|
|
/// <remarks>
|
|
/// <para>
|
|
/// The inventory test ADR 0002 asked for. Endpoints are registered from an explicit type list in
|
|
/// <c>Setup/EndpointRegistration.cs</c> rather than found by scanning, which trades one failure mode for
|
|
/// another: nothing can appear by accident, but a type left off the list is a route that quietly does
|
|
/// not exist, with no compile error and — without this test — no failure either. Asserting the whole set
|
|
/// rather than a subset is the point. A new endpoint cannot ship until somebody writes down, here, who
|
|
/// is allowed to call it.
|
|
/// </para>
|
|
/// <para>
|
|
/// It is also the only coverage <c>/api/v1/meta</c> and <c>/.well-known/dodossh-configuration</c> have
|
|
/// in <em>this</em> assembly — <c>DodoSSH.SystemTests</c> does fetch both anonymously, but that suite
|
|
/// needs a Docker daemon and several containers, so it is not what a developer runs before pushing.
|
|
/// Both are anonymous only because they say so, and a client has nothing to authenticate with at the
|
|
/// point it reads them, so a 401 on either is unrecoverable rather than merely wrong.
|
|
/// </para>
|
|
/// <para>
|
|
/// What this does <em>not</em> cover: <see cref="Describe"/> reads only <see cref="IAuthorizeData.Policy"/>,
|
|
/// so a role, claim, scope or authentication-scheme requirement could be added or removed without
|
|
/// failing here. Nothing uses those today; the day something does, this table needs a column.
|
|
/// </para>
|
|
/// </remarks>
|
|
[Collection(ApiCollection.Name)]
|
|
public sealed class EndpointInventoryTests(ApiFixture fixture)
|
|
{
|
|
private static readonly string[] Expected =
|
|
[
|
|
// Anonymous by necessity: discovery has to work before a token exists.
|
|
"GET /api/v1/meta name=GetMeta tags= policies= anon=True",
|
|
"GET /.well-known/dodossh-configuration name=GetDodoSshConfiguration tags= policies= anon=True",
|
|
|
|
// Authenticated, not Enrolled: these three are how a caller discovers it must enroll, does so,
|
|
// and — for revocation — withdraws a lost machine even if its enrollment state is in doubt.
|
|
"GET /api/v1/me name=GetMe tags=Identity policies=Authenticated anon=False",
|
|
"POST /api/v1/me/enrollment name=Enroll tags=Identity policies=Authenticated anon=False",
|
|
"DELETE /api/v1/me/devices/{deviceId:guid} name=RevokeDevice tags=Identity policies=Authenticated anon=False",
|
|
|
|
// The one endpoint in the /me area that needs a key bundle to already exist.
|
|
"POST /api/v1/me/devices name=RegisterDevice tags=Identity policies=Authenticated,Enrolled anon=False",
|
|
|
|
// Enrolled: a caller with no identity key can neither write ciphertext anyone can read nor read
|
|
// what is there.
|
|
"POST /api/v1/vaults/{vaultId:guid}/sync/pull name=SyncPull tags=Sync policies=Enrolled anon=False",
|
|
"POST /api/v1/vaults/{vaultId:guid}/sync/push name=SyncPush tags=Sync policies=Enrolled anon=False",
|
|
|
|
// The WebSocket, gated exactly as sync is and for the same reason — it announces changes to
|
|
// vaults, and a caller who could not read one has nothing to be told about. It appears here as
|
|
// an ordinary route because that is what it is until the upgrade: the bearer token authorises
|
|
// the handshake, unlike the relay's ticket. See ADR 0012.
|
|
"GET /api/v1/events name=VaultEvents tags=Events policies=Enrolled anon=False",
|
|
|
|
// Enrolled, because the answer exists to be wrapped to and a caller with no key of their own has
|
|
// nothing to wrap and no signature to attribute it with. There is no search here — see
|
|
// DirectoryService for why an exact-match-only directory is a decision rather than a shortcut.
|
|
"GET /api/v1/directory name=LookupDirectory tags=Identity policies=Enrolled anon=False",
|
|
|
|
// The other half of the same decision: the directory says what a key is, this is how a client
|
|
// checks that claim against a chain the server cannot rewrite without every other client
|
|
// noticing. Nothing in it is secret.
|
|
"GET /api/v1/keylog name=ReadKeyLog tags=Identity policies=Enrolled anon=False",
|
|
|
|
// Authenticated, not Enrolled: reading and joining teams needs no key, and a member added before
|
|
// they have set a vault up must still be able to see the team they are now in.
|
|
"GET /api/v1/teams name=ListTeams tags=Teams policies=Authenticated anon=False",
|
|
"GET /api/v1/teams/{teamId:guid}/members name=ListTeamMembers tags=Teams policies=Authenticated anon=False",
|
|
"POST /api/v1/teams/{teamId:guid}/members name=AddTeamMember tags=Teams policies=Authenticated anon=False",
|
|
"PUT /api/v1/teams/{teamId:guid}/members/{userId:guid}/role name=ChangeTeamMemberRole tags=Teams policies=Authenticated anon=False",
|
|
"DELETE /api/v1/teams/{teamId:guid}/members/{userId:guid} name=RemoveTeamMember tags=Teams policies=Authenticated anon=False",
|
|
|
|
// Administering a team you are already in, and none of it moves key material — so Authenticated
|
|
// for the same reason the membership routes above are. Two of the three are gated harder inside
|
|
// the handler than this table can show: archiving and handing the team over check for the owner
|
|
// rather than for an admin, because an admin the owner promoted must not be able to take the
|
|
// team from them. See TeamAccess.IsOwner.
|
|
"PUT /api/v1/teams/{teamId:guid} name=UpdateTeam tags=Teams policies=Authenticated anon=False",
|
|
"DELETE /api/v1/teams/{teamId:guid} name=ArchiveTeam tags=Teams policies=Authenticated anon=False",
|
|
"POST /api/v1/teams/{teamId:guid}/owner name=TransferTeamOwnership tags=Teams policies=Authenticated anon=False",
|
|
|
|
// Authenticated, and pointedly not Enrolled. An invitation names an address that may have no
|
|
// account at all and certainly holds no key; gating these on Enrolled would be demanding a key
|
|
// of the one participant the feature exists for. Membership is not readability — somebody still
|
|
// has to wrap the vault key afterwards — so no key is involved on either side.
|
|
"GET /api/v1/teams/{teamId:guid}/invitations name=ListTeamInvitations tags=Teams policies=Authenticated anon=False",
|
|
"POST /api/v1/teams/{teamId:guid}/invitations name=CreateTeamInvitation tags=Teams policies=Authenticated anon=False",
|
|
"DELETE /api/v1/teams/{teamId:guid}/invitations/{invitationId:guid} name=RevokeTeamInvitation tags=Teams policies=Authenticated anon=False",
|
|
|
|
// Enrolled, because both end in a vault key being wrapped: creating a team means creating a vault
|
|
// in it, and neither is reachable without a key of one's own.
|
|
"POST /api/v1/teams name=CreateTeam tags=Teams policies=Enrolled anon=False",
|
|
"POST /api/v1/teams/{teamId:guid}/vaults name=CreateTeamVault tags=Teams policies=Enrolled anon=False",
|
|
|
|
// Enrolled. The listing is gated on Read rather than Share — every member can already see the
|
|
// sharing graph — and the two writes are gated on Share inside the handler, which this table
|
|
// cannot see. See VaultGrantEndpoints.
|
|
// Authenticated, alone among the vault routes: renaming touches no key material, so refusing
|
|
// somebody who has not published an identity key would be refusing them for an unrelated
|
|
// reason. Gated on Admin inside the handler, which this table cannot see.
|
|
"PUT /api/v1/vaults/{vaultId:guid} name=RenameVault tags=Vaults policies=Authenticated anon=False",
|
|
|
|
// Authenticated for the same reason as the rename it sits beside — deleting withdraws grants
|
|
// rather than wrapping anything, so it needs no key of the caller's — and gated on Admin inside
|
|
// the handler, which this table cannot see. It refuses a personal vault there too.
|
|
"DELETE /api/v1/vaults/{vaultId:guid} name=DeleteVault tags=Vaults policies=Authenticated anon=False",
|
|
|
|
"GET /api/v1/vaults/{vaultId:guid}/grants name=ListVaultGrants tags=Vaults policies=Enrolled anon=False",
|
|
"POST /api/v1/vaults/{vaultId:guid}/grants name=IssueVaultGrant tags=Vaults policies=Enrolled anon=False",
|
|
"DELETE /api/v1/vaults/{vaultId:guid}/grants/{userId:guid} name=RevokeVaultGrant tags=Vaults policies=Enrolled anon=False",
|
|
|
|
// Gated on Share inside the handler as the two writes above are, and additionally on holding the
|
|
// current key — which no policy could express, since it is a row in vault_key_grant.
|
|
"POST /api/v1/vaults/{vaultId:guid}/rekey name=RekeyVault tags=Vaults policies=Enrolled anon=False",
|
|
|
|
// Anonymous on purpose, and load-bearing: DodoSSH.SystemTests waits on /healthz/ready before any
|
|
// token exists, and an orchestrator probe that needs credentials reports the wrong thing.
|
|
// MapHealthChecks constrains no verb, hence ANY.
|
|
"ANY /healthz/live name= tags= policies= anon=True",
|
|
"ANY /healthz/ready name= tags= policies= anon=True",
|
|
"ANY /healthz/startup name= tags= policies= anon=True",
|
|
|
|
// Not ours. FastEndpoints maps this one itself, in every environment, with no way to opt out; it
|
|
// answers with the server's whole endpoint-name-to-route table. It stays registered and is
|
|
// short-circuited to 404 instead — see RouteTableIsNotReachable below. Listed so that a version
|
|
// bump which adds a second hidden route, or renames this one out from under the block, fails
|
|
// here rather than in production.
|
|
"GET _test_url_cache_ name= tags= policies= anon=False",
|
|
];
|
|
|
|
[Fact]
|
|
public void TheServerExposesExactlyTheEndpointsWeMeantTo()
|
|
{
|
|
// Forces the pipeline to be built. FastEndpoints registers its routes when the application is
|
|
// built, not when its services are, so resolving the data sources from an unstarted host finds
|
|
// the health checks and nothing else.
|
|
using var client = fixture.CreateClient();
|
|
|
|
var actual = fixture.Services.GetServices<EndpointDataSource>()
|
|
.SelectMany(source => source.Endpoints)
|
|
.OfType<RouteEndpoint>()
|
|
.Select(Describe)
|
|
.Order(StringComparer.Ordinal)
|
|
.ToArray();
|
|
|
|
actual.ShouldBe([.. Expected.Order(StringComparer.Ordinal)]);
|
|
}
|
|
|
|
/// <remarks>
|
|
/// <para>
|
|
/// Distinct from the inventory above, which only proves the route is registered. This proves it does
|
|
/// not answer.
|
|
/// </para>
|
|
/// <para>
|
|
/// Every spelling routing accepts is asserted, not just the canonical one. Routing matches literal
|
|
/// segments case-insensitively and tolerates a trailing slash, so a block that compares the request
|
|
/// path with <c>StringComparison.Ordinal</c> passes a canonical-spelling test while leaving the
|
|
/// listing fully readable at <c>/_TEST_URL_CACHE_</c>. That is not hypothetical — it is what the
|
|
/// first version of this guard did, and a single-spelling test is what let it look correct.
|
|
/// </para>
|
|
/// </remarks>
|
|
[Theory]
|
|
[InlineData("/_test_url_cache_")]
|
|
[InlineData("/_TEST_URL_CACHE_")]
|
|
[InlineData("/_Test_Url_Cache_")]
|
|
[InlineData("/_test_url_cache_/")]
|
|
public async Task RouteTableIsNotReachable(string path)
|
|
{
|
|
// Authenticated on purpose: the deny-by-default policy already stops an anonymous caller, so a
|
|
// 404 for one would prove nothing about whether the listing is exposed.
|
|
var client = fixture.CreateClientFor(subject: $"route-table-{Guid.CreateVersion7()}");
|
|
|
|
var response = await client.GetAsync(
|
|
new Uri(path, UriKind.Relative),
|
|
TestContext.Current.CancellationToken);
|
|
|
|
response.StatusCode.ShouldBe(System.Net.HttpStatusCode.NotFound);
|
|
|
|
// A 404 with the listing in the body would satisfy the status assertion on its own.
|
|
var body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken);
|
|
body.ShouldNotContain("Endpoint", Case.Insensitive);
|
|
}
|
|
|
|
private static string Describe(RouteEndpoint endpoint)
|
|
{
|
|
var methods = endpoint.Metadata.GetMetadata<HttpMethodMetadata>()?.HttpMethods;
|
|
var verbs = methods is { Count: > 0 } ? string.Join(",", methods) : "ANY";
|
|
|
|
var name = endpoint.Metadata.GetMetadata<IEndpointNameMetadata>()?.EndpointName ?? string.Empty;
|
|
var tags = string.Join(",", endpoint.Metadata.GetMetadata<ITagsMetadata>()?.Tags ?? []);
|
|
|
|
// FastEndpoints adds a synthetic "epPolicy:<full type name>" beside the named policies, and
|
|
// including it would couple this table to endpoint class names. Dropping it is not free: that
|
|
// policy is also where FastEndpoints folds Roles(), Claims() and Permissions(), so those become
|
|
// invisible here. Nothing calls them — the two named policies carry the whole authorization
|
|
// decision — and the class remarks say so, rather than this filter pretending it discards nothing.
|
|
var policies = string.Join(
|
|
",",
|
|
endpoint.Metadata.OfType<IAuthorizeData>()
|
|
.Select(data => data.Policy)
|
|
.Where(policy => !string.IsNullOrEmpty(policy))
|
|
.Where(policy => !policy!.StartsWith("epPolicy:", StringComparison.Ordinal)));
|
|
|
|
var anonymous = endpoint.Metadata.GetMetadata<IAllowAnonymous>() is not null;
|
|
|
|
return $"{verbs} {endpoint.RoutePattern.RawText} name={name} tags={tags} policies={policies} anon={anonymous}";
|
|
}
|
|
}
|