DodoTech-Public/DodoSSH
Public Access
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 2 Wiki Activity

Move the API onto FastEndpoints, without moving the wire

Browse Source 
Eight endpoints today, around sixty planned. The minimal-API shape — a static
class per area holding static local functions, route and policy and name
asserted in one fluent chain with the handler somewhere below it — has not hurt
yet, and would. A handler's dependencies are parameters rather than injected, a
group's RequireAuthorization sits far from the handler it governs, and there is
no type to hang an endpoint's own documentation on. FastEndpoints is one class
per endpoint, its route and authorization in Configure(), its handler a method
on the same type.

Nothing about the wire moves, and the evidence is that the 94 existing HTTP
tests pass with zero edits to any of them. Same routes, verbs, route
constraints, status codes, operation ids, and the same RFC 9457 bodies with the
same code values. Every place the idiomatic FastEndpoints answer would have
changed one of those, it was refused:

Endpoints are registered from an explicit List<Type>, not found by scanning.
ADR 0002 rejected reflection discovery by name, and the reason it gave is
sharper here than in general — under WebApplicationFactory the scan reaches the
test assembly, so an endpoint written in a test would be registered into the
host under test. The cost is a line per endpoint that can be forgotten, which is
what the endpoint-inventory test is for. That test is the one ADR 0002 promised
and never got.

Handlers still return Results<Ok<T>, NotFound, ProblemHttpResult> from
ExecuteAsync. The union executes as an ordinary IResult, which is what keeps
problem bodies going through the host's serialiser and IProblemDetailsService,
and what keeps the compile-time record of which statuses an endpoint can
produce. No Send.* call appears anywhere; the moment one does, a response has
left the host's serialiser.

Validation stays in the feature services. A Validator<T> short-circuits before
the handler and answers with FastEndpoints' own envelope, which carries no code
— and the code is the only part of an error the client branches on. Twenty-odd
tests assert a specific code on a 400. It is banned in BannedSymbols.txt rather
than merely avoided, because the framework's documentation leads straight to it
and it looks like an improvement.

Three defects arrived with the framework and were caught in review. All three
were green at the time, which is the part worth remembering. FastEndpoints maps
GET /_test_url_cache_ unconditionally, in every environment, with no policy and
no way to opt out; it answers with the whole endpoint-name-to-route table. It is
short-circuited to 404 — by asking routing which endpoint it selected, after the
first attempt compared the request path with Ordinal and was therefore bypassable
at /_TEST_URL_CACHE_, certified by a test that only ever tried one spelling. The
default request binder writes query-string values over the deserialised body,
which would have let ?identityProviderToken=... put an ID token in a URL and from
there into every proxy log on the path; every endpoint now binds from the body
alone. And a route value read with Route<T>() is invisible to ApiExplorer, so the
generated document named {vaultId} in a path template with nothing declaring it —
invalid OpenAPI, and unusable by the client generators the document exists for.

Two changes to the surface, both deliberate. A body that cannot be deserialised
now answers with a problem document carrying malformed-request, rather than an
empty 400: FastEndpoints' default announces application/problem+json while
sending something else, and names the failing .NET type on the wire, in a
codebase that sets IncludeErrorDetails = false to prevent exactly that. And the
route table above returns 404 where it would otherwise have answered any
authenticated caller.

Each of the three fixes has a regression test that was checked by reverting the
fix and watching it fail — four failures for the route table and the binder, four
for the document. That check is the whole reason to trust them, since all three
defects passed a full green suite on the way in.

950 tests green across 16 projects, 14 of them new and no existing test edited.
Zero warnings, format clean, locked restore clean. FluentValidation, JobQueues
and Messaging are in the graph now and none is used.

Not verified: the generated document's response schemas, which differ from
before — FastEndpoints contributes its own Produces metadata. Nothing consumes
the document yet, and MapOpenApi runs only in Development behind the fallback
policy. It needs pinning if ADR 0002's build-time artifacts/openapi/v1.json is
ever built.
This commit is contained in:
jaap-jan
2026-07-31 08:39:06 +02:00
parent d162271a45
commit 9bc28f1c0f
21 changed files with 1287 additions and 201 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/DodoSSH.Api.Tests/RequestBindingTests.cs
+173
View File
@@ -0,0 +1,173 @@
using System.Net;
using System.Text;
using DodoSSH.Contracts;
using DodoSSH.Domain;
using DodoSSH.Infrastructure;
using Microsoft.Extensions.DependencyInjection;
using Shouldly;
using Xunit;
namespace DodoSSH.Api.Tests;
/// <summary>
/// What the server answers when a request body cannot be read at all.
/// </summary>
/// <remarks>
/// <para>
/// This is the one error the endpoint framework produces rather than a handler, so it is the one place
/// the problem-document contract could drift without any other test noticing. The strict
/// unmapped-member rule in particular has never had a test: it is configured in
/// <see cref="DodoSshJsonContext"/>, applied by <c>AddDodoJson</c>, and until now nothing asserted the
/// server actually enforced it.
/// </para>
/// <para>
/// Enrollment is the subject only because it is a POST with a body that an authenticated caller can
/// reach without being enrolled. Nothing here is about enrollment.
/// </para>
/// </remarks>
[Collection(ApiCollection.Name)]
public sealed class RequestBindingTests(ApiFixture fixture)
{
private const string EnrollUrl = "/api/v1/me/enrollment";
[Fact]
public async Task AnUnknownJsonMemberIsRejected()
{
// A misspelled or renamed client property must be a 400, not a silently missing value that
// shows up later as data loss.
var problem = await PostRawAsync("""{"statement":null,"thisMemberDoesNotExist":1}""");
problem.Code.ShouldBe(ProblemCodes.MalformedRequest);
}
[Fact]
public async Task MalformedJsonIsRejected()
{
var problem = await PostRawAsync("""{"statement":""" + " ");
problem.Code.ShouldBe(ProblemCodes.MalformedRequest);
}
[Fact]
public async Task ANumberSentAsAStringIsRejected()
{
// JsonNumberHandling.Strict. The web defaults would read "1" as 1, which is how two
// implementations end up disagreeing about what a number is.
//
// keyGeneration is reached through statement on purpose: it is a member of KeyStatement, not of
// EnrollmentRequest, so posting it at the top level is rejected as an unmapped member and never
// reaches a number-parsing decision at all. This test asserted nothing until that was spotted.
var problem = await PostRawAsync("""{"statement":{"keyGeneration":"1"}}""");
problem.Code.ShouldBe(ProblemCodes.MalformedRequest);
}
[Fact]
public async Task TheQueryStringCannotSupplyRequestFields()
{
// FastEndpoints' default binder writes query-string values over the deserialised body; minimal
// APIs read the body alone. Left on, that would let ?identityProviderToken=... put an ID token
// in a URL, and from there into every proxy log and browser history on the path.
//
// Asserted on the cursor because it is the one field where the two behaviours give visibly
// different answers: a body cursor of null means "start from the beginning" and succeeds, while
// the garbage in the query string fails its integrity tag. A test on enrollment could not tell
// the difference — every malformed enrollment is rejected either way.
var (subject, vaultId) = await SeedUserWithVaultAsync();
var client = fixture.CreateClientFor(subject);
var response = await client.PostContractAsync(
$"/api/v1/vaults/{vaultId}/sync/pull?cursor=bm90LWEtcmVhbC1jdXJzb3I",
new SyncPullRequest(Cursor: null, Limit: null, EntityTypes: null));
response.StatusCode.ShouldBe(HttpStatusCode.OK);
}
[Fact]
public async Task TheRejectionIsARfc9457ProblemDocument()
{
// The framework's own binding-failure body is neither a problem document nor free of .NET type
// names, despite being served as application/problem+json. Asserted in full here because this
// response is assembled by hand rather than by TypedResults.Problem, so nothing else keeps the
// two shapes in step.
var client = fixture.CreateClientFor(subject: $"binding-{Guid.CreateVersion7()}");
using var content = new StringContent(
"""{"nope":1}""",
Encoding.UTF8,
"application/json");
var response = await client.PostAsync(
new Uri(EnrollUrl, UriKind.Relative),
content,
TestContext.Current.CancellationToken);
response.StatusCode.ShouldBe(HttpStatusCode.BadRequest);
response.Content.Headers.ContentType?.MediaType.ShouldBe("application/problem+json");
var body = await response.Content.ReadAsStringAsync(TestContext.Current.CancellationToken);
body.ShouldNotContain("DodoSSH.Contracts", Case.Sensitive);
var problem = await response.Content.ReadProblemAsync();
problem.ShouldNotBeNull();
problem.Type.ShouldBe(ProblemCodes.TypeBaseUri + ProblemCodes.MalformedRequest);
problem.Detail.ShouldNotBeNullOrWhiteSpace();
}
/// <remarks>
/// Seeded directly rather than driven through the enrollment endpoint, for the reason
/// <see cref="Seed"/> gives: a failure here should mean the binder is wrong, not that enrollment is.
/// </remarks>
private async Task<(string Subject, Guid VaultId)> SeedUserWithVaultAsync()
{
var subject = $"binding-{Guid.CreateVersion7():N}";
var now = TimeProvider.System.GetUtcNow();
await using var scope = fixture.CreateScope();
var database = scope.ServiceProvider.GetRequiredService<DodoDbContext>();
var user = new UserAccount
{
Id = Guid.CreateVersion7(),
Issuer = fixture.IdentityProvider.Authority,
Subject = subject,
Status = UserStatus.Active,
CreatedAtUtc = now,
UpdatedAtUtc = now,
};
var vault = new Vault
{
Id = Guid.CreateVersion7(),
Name = "Personal",
OwnerKind = VaultOwnerKind.Personal,
OwnerUserId = user.Id,
KeyGeneration = 1,
CreatedAtUtc = now,
UpdatedAtUtc = now,
};
database.Users.Add(user);
database.UserKeys.Add(Seed.CurrentKey(user.Id, now));
database.Vaults.Add(vault);
await database.SaveChangesAsync(TestContext.Current.CancellationToken);
return (subject, vault.Id);
}
private async Task<JsonProblem> PostRawAsync(string json, string query = "")
{
var client = fixture.CreateClientFor(subject: $"binding-{Guid.CreateVersion7()}");
using var content = new StringContent(json, Encoding.UTF8, "application/json");
var response = await client.PostAsync(
new Uri(EnrollUrl + query, UriKind.Relative),
content,
TestContext.Current.CancellationToken);
response.StatusCode.ShouldBe(HttpStatusCode.BadRequest);
var problem = await response.Content.ReadProblemAsync();
problem.ShouldNotBeNull();
return problem;
}
}