Merge branch 'claude/api-fastendpoints-migration-020431'

tests/DodoSSH.Api.Tests/EndpointInventoryTests.cs

@@ -0,0 +1,151 @@
+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",
+
+        // 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}";
+    }
+}

tests/DodoSSH.Api.Tests/OpenApiDocumentTests.cs

@@ -0,0 +1,103 @@
+using System.Text.Json;
+using Microsoft.AspNetCore.OpenApi;
+using Microsoft.Extensions.DependencyInjection;
+using Microsoft.OpenApi;
+using Shouldly;
+using Xunit;
+
+namespace DodoSSH.Api.Tests;
+
+/// <summary>
+/// The generated OpenAPI document, which ADR 0002 promises to third parties and a future CLI.
+/// </summary>
+/// <remarks>
+/// Generated through <see cref="IOpenApiDocumentProvider"/> rather than fetched from
+/// <c>/openapi/v1.json</c>: that route is mapped only in Development and sits behind the deny-by-default
+/// policy, so it is not reachable from this fixture. The document is the same one either way.
+/// </remarks>
+[Collection(ApiCollection.Name)]
+public sealed class OpenApiDocumentTests(ApiFixture fixture)
+{
+    [Theory]
+    [InlineData("/api/v1/vaults/{vaultId}/sync/pull", "post", "vaultId")]
+    [InlineData("/api/v1/vaults/{vaultId}/sync/push", "post", "vaultId")]
+    [InlineData("/api/v1/me/devices/{deviceId}", "delete", "deviceId")]
+    public async Task RouteParametersAreDeclared(string path, string verb, string parameterName)
+    {
+        // These endpoints read their route value with Route<T>("name") rather than binding it onto the
+        // request DTO, so nothing in the endpoint signature mentions it and the generator emits the path
+        // template with no matching parameter — which is invalid OpenAPI and unusable by any client
+        // generator. RouteParameterTransformer puts them back; without it this document silently rots.
+        var document = await GenerateAsync();
+
+        var operation = document
+            .GetProperty("paths").GetProperty(path).GetProperty(verb);
+
+        var parameters = operation.GetProperty("parameters").EnumerateArray()
+            .Where(p => string.Equals(p.GetProperty("in").GetString(), "path", StringComparison.Ordinal))
+            .ToArray();
+
+        var declared = parameters.SingleOrDefault(
+            p => string.Equals(p.GetProperty("name").GetString(), parameterName, StringComparison.Ordinal));
+
+        declared.ValueKind.ShouldNotBe(JsonValueKind.Undefined, $"{verb} {path} declares no {parameterName}");
+        declared.GetProperty("required").GetBoolean().ShouldBeTrue();
+
+        // Recovered from the :guid route constraint, not guessed from the name.
+        declared.GetProperty("schema").GetProperty("format").GetString().ShouldBe("uuid");
+    }
+
+    [Fact]
+    public async Task EveryPathTemplateExpressionHasAParameter()
+    {
+        // The general form of the rule above: no operation may name a template expression it does not
+        // declare. Asserted over the whole document so a new endpoint cannot reintroduce the defect.
+        var document = await GenerateAsync();
+
+        var offenders = new List<string>();
+
+        foreach (var path in document.GetProperty("paths").EnumerateObject())
+        {
+            var expected = path.Name.Split('/')
+                .Where(segment => segment.StartsWith('{') && segment.EndsWith('}'))
+                .Select(segment => segment[1..^1])
+                .ToArray();
+
+            if (expected.Length == 0)
+            {
+                continue;
+            }
+
+            foreach (var operation in path.Value.EnumerateObject())
+            {
+                var declared = operation.Value.TryGetProperty("parameters", out var parameters)
+                    ? parameters.EnumerateArray()
+                        .Where(p => string.Equals(p.GetProperty("in").GetString(), "path", StringComparison.Ordinal))
+                        .Select(p => p.GetProperty("name").GetString())
+                        .ToArray()
+                    : [];
+
+                offenders.AddRange(
+                    expected.Except(declared, StringComparer.Ordinal)
+                        .Select(missing => $"{operation.Name.ToUpperInvariant()} {path.Name} -> {missing}"));
+            }
+        }
+
+        offenders.ShouldBeEmpty();
+    }
+
+    private async Task<JsonElement> GenerateAsync()
+    {
+        await using var scope = fixture.CreateScope();
+        // Keyed on the document name: AddOpenApi registers one provider per document.
+        var provider = scope.ServiceProvider.GetRequiredKeyedService<IOpenApiDocumentProvider>("v1");
+
+        var document = await provider.GetOpenApiDocumentAsync(TestContext.Current.CancellationToken);
+
+        var json = await document.SerializeAsJsonAsync(
+            OpenApiSpecVersion.OpenApi3_1,
+            TestContext.Current.CancellationToken);
+
+        return JsonDocument.Parse(json).RootElement;
+    }
+}

tests/DodoSSH.Api.Tests/RequestBindingTests.cs

@@ -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;
+    }
+}

tests/DodoSSH.Api.Tests/packages.lock.json

@@ -169,6 +169,49 @@
         "resolved": "2.2.1",
         "contentHash": "21XZo/yuXK1k0EUhdLnjgRD4n0HQYmPFchV6uaORcRc65rasZ1vdm2dmJXPBKZiIBztRRYRmmg/B76W721VWkA=="
       },
+      "FastEndpoints.Attributes": {
+        "type": "Transitive",
+        "resolved": "8.2.0",
+        "contentHash": "ni128Yjqk5cAYTvkHqWvhCoFIDqUNstnNd7SljUKr3m8UdLDcZCUKy0lKe9W8R8KLTEdzwk0nufeZDb3MxAaDg==",
+        "dependencies": {
+          "Microsoft.Extensions.Primitives": "10.0.9"
+        }
+      },
+      "FastEndpoints.Core": {
+        "type": "Transitive",
+        "resolved": "8.2.0",
+        "contentHash": "jYC2hFYyH0Yfiv6ykR4SgctGu0Y1cx7gpiScRmdubepzGvPRysbNVNeysAKFg18ZP0PGXUharea3kcEAnJn8Iw=="
+      },
+      "FastEndpoints.JobQueues": {
+        "type": "Transitive",
+        "resolved": "8.2.0",
+        "contentHash": "2ZhXE0Ghq+/TqsMP/3uy+XiT4FZRZ6+IFvnDfl+roSlhkgAO30g0SYob0eX3pEr93+KtT1trDSG2H+5rZHMTmA==",
+        "dependencies": {
+          "FastEndpoints.Messaging": "8.2.0",
+          "Microsoft.Extensions.DependencyInjection.Abstractions": "10.0.9",
+          "Microsoft.Extensions.Hosting.Abstractions": "10.0.9"
+        }
+      },
+      "FastEndpoints.Messaging": {
+        "type": "Transitive",
+        "resolved": "8.2.0",
+        "contentHash": "5gyFV0GxY88WxxJZX4A+wY0a+wTpoloyyG+egJTiwZh9JrnDXgdMSKMzBf7ePi8vJUoGrVhcrJS4B+ThmUItqA==",
+        "dependencies": {
+          "FastEndpoints.Core": "8.2.0",
+          "FastEndpoints.Messaging.Core": "8.2.0",
+          "Microsoft.Extensions.DependencyInjection.Abstractions": "10.0.9"
+        }
+      },
+      "FastEndpoints.Messaging.Core": {
+        "type": "Transitive",
+        "resolved": "8.2.0",
+        "contentHash": "ubGKGIzdSos62ECTNkkPcHubBem7Nbi7T1+f+h6uHhZblwW56SOxeSzQMA3C7/2qIs94bVnlT0bFphcEewH4BQ=="
+      },
+      "FluentValidation": {
+        "type": "Transitive",
+        "resolved": "12.1.1",
+        "contentHash": "EPpkIe1yh1a0OXyC100oOA8WMbZvqUu5plwhvYcb7oSELfyUZzfxV48BLhvs3kKo4NwG7MGLNgy1RJiYtT8Dpw=="
+      },
       "GraphQL": {
         "type": "Transitive",
         "resolved": "8.5.0",
@@ -1668,6 +1711,7 @@
           "DodoSSH.Crypto": "[1.0.0, )",
           "DodoSSH.Domain": "[1.0.0, )",
           "DodoSSH.Infrastructure": "[1.0.0, )",
+          "FastEndpoints": "[8.2.0, )",
           "Microsoft.AspNetCore.Authentication.JwtBearer": "[10.0.10, )",
           "Microsoft.AspNetCore.OpenApi": "[10.0.10, )"
         }
@@ -1709,6 +1753,18 @@
           "Microsoft.Extensions.DependencyInjection.Abstractions": "10.0.1"
         }
       },
+      "FastEndpoints": {
+        "type": "CentralTransitive",
+        "requested": "[8.2.0, )",
+        "resolved": "8.2.0",
+        "contentHash": "NfsC7v8YDmZtBjWYs88+ef1/vnL+qGcw8FigGyNzJD8IVAG9ZSmtIKyLJu95BZjfAMwcGcjo+3qXsyC9L7SLlA==",
+        "dependencies": {
+          "FastEndpoints.Attributes": "8.2.0",
+          "FastEndpoints.JobQueues": "8.2.0",
+          "FastEndpoints.Messaging": "8.2.0",
+          "FluentValidation": "12.1.1"
+        }
+      },
       "libsodium": {
         "type": "CentralTransitive",
         "requested": "[1.0.22, )",