diff --git a/docs/specs/2026-07-30-mu-directory-design.md b/docs/specs/2026-07-30-mu-directory-design.md index 80df35a..c5b8669 100644 --- a/docs/specs/2026-07-30-mu-directory-design.md +++ b/docs/specs/2026-07-30-mu-directory-design.md @@ -842,10 +842,42 @@ a self-report with extra steps. Owner-published outputs: a live player-count SVG badge and a JSON endpoint for the game's own site. -**Claiming lights up two paths that are currently unreachable**, and that is worth knowing when -testing it: nothing sets `game.is_claimed` today, so the `claimed` badge in the listing and -`ArchivePolicy`'s ceiling-grace-for-claimed-games (§7.5) have never once been exercised against real -data. +**Claiming is wired end to end, and it was not for a while after it shipped.** A verified probe now +sets `game.is_claimed`, so the `claimed` badge in the listing and `ArchivePolicy`'s +ceiling-grace-for-claimed-games (§7.5) are both reachable. What kept them dark was not the claim +logic, which was complete and tested, but the **composition** — and the two are worth telling apart, +because the second has no compiler and no unit test looking at it: + +- The site has two compositions of the same objects. `mui-crawl` builds the crawl loop by hand and + passes every collaborator explicitly; the deployed site assembles it through DI. `CrawlCycle` takes + its `ClaimService` as an *optional* parameter — a crawl with no database behind it should do + slightly less rather than refuse to run — so a composition that omits it settles no beacons and + says nothing about it. The crawl graph omitted it, and the site only had one because the accounts + module happened to register the same type for the dashboard. +- `ClaimService` was registered scoped while the crawl loop that needs it is a singleton + `BackgroundService`. With scope validation on, which is what `dotnet run` does, **the container + refused to build**: the site would not start with a connection string set. Production leaves + validation off, so there it worked — by accident, and only there. +- `IClaimStore` was registered nowhere at all. The dashboard service-locates it and reads a null as + "this site has no database", so every operator's list of claimed games was empty on a site that + had them, and the on-demand check endpoint threw on request. + +The lesson generalises past claiming: **an optional dependency and a service-located one both fail +silently when the composition is wrong**, and this codebase has one of each on the claim path. +`CompositionTests` resolves the graph `Program` builds — literally, by calling the same +`AddMuiSite` the deployable calls, because a harness that restated the registrations would be a +second copy that agrees with the first only until somebody edits one of them — under scope +validation, in both environments, and asserts the services these paths need are really there. A +wiring test, because the wiring is what was broken while every part it joined was correct. + +**The on-demand check was the same shape of gap and is fixed with it.** §8.1 offers a claimant one +requested probe per few minutes; `RequestCheckAsync` wrote `last_checked_at` and a `check_requested` +event, and due-ness comes only from `crawl_target.next_probe_at`, so nothing was ever probed and the +page said the button dialled a real server. A rate limiter on an action that does not happen is the +most convincing possible no-op. `IOnDemandProbes` brings the game's targets forward instead — +`LEAST`, so an ask can only make a probe sooner — and the crawl loop still does the dialling under +`CRAWL DELAY` and §7.2's address gate, which is what keeps a button on a page from becoming a way to +make us connect to a stranger's server. ## 9. Site surface, v1 diff --git a/src/MUI.Catalog/Claims.cs b/src/MUI.Catalog/Claims.cs index 4983861..bf1e2a8 100644 --- a/src/MUI.Catalog/Claims.cs +++ b/src/MUI.Catalog/Claims.cs @@ -207,3 +207,30 @@ public enum ClaimVerdict /// A token we issued, matching a claim that has expired or been revoked. Stale, } + +/// +/// Brings a game's next probe forward, for §8.1's on-demand check. +/// +/// +/// +/// An interface here and an implementation over crawl_target in MUI.Crawler, because +/// due-ness is the crawl registry's business and MUI.Catalog may not know a socket exists. +/// decides whether a claimant may ask; this decides nothing and +/// only moves the schedule. +/// +/// +/// It brings a probe forward; it does not dial. The crawler's own loop still does the +/// dialling, still honours CRAWL DELAY and still refuses a target outside scope — so a button +/// on a page cannot become a way to make us connect to a stranger's server on demand. What §8.1 asks +/// for is that an operator who has just edited mush.cnf is not left waiting on the scheduler, +/// and that is exactly this and no more. +/// +/// +public interface IOnDemandProbes +{ + /// + /// Asks for to be probed no later than . + /// + /// Whether any target was brought forward. + Task BringForwardAsync(Guid gameId, DateTimeOffset at, CancellationToken cancellationToken = default); +} diff --git a/src/MUI.Catalog/Persistence/ClaimService.cs b/src/MUI.Catalog/Persistence/ClaimService.cs index 51bf7e7..c4f8396 100644 --- a/src/MUI.Catalog/Persistence/ClaimService.cs +++ b/src/MUI.Catalog/Persistence/ClaimService.cs @@ -17,7 +17,11 @@ namespace MUI.Catalog.Persistence; /// hands this a string and a channel. /// /// -public sealed class ClaimService(IClaimStore claims, IGameStore games, TimeProvider time) +public sealed class ClaimService( + IClaimStore claims, + IGameStore games, + TimeProvider time, + IOnDemandProbes? probes = null) { /// /// How often a claimant may ask us to look again (spec §8.1). @@ -153,7 +157,24 @@ public bool MayRecheck(GameClaim claim) return claim.LastCheckedAt is not { } last || time.GetUtcNow() - last >= RecheckInterval; } - /// Records that a check was asked for. The dialling itself belongs to the crawler. + /// + /// Brings the game's next probe forward, and records that the claimant asked. + /// + /// + /// + /// It moves the schedule; the crawler still does the dialling. Keeping the two apart is + /// what stops a button on a page becoming a way to make us connect to a stranger's server on + /// demand: the rate limit is per claim, a claim cannot exist for a game nobody has been offered, + /// and CRAWL DELAY and the address gate both still bind when the loop gets there. + /// + /// + /// This recorded the ask and moved nothing for as long as it existed. Due-ness comes only from + /// crawl_target.next_probe_at, so last_checked_at and the check_requested + /// event went into the database and no probe ever came of it, while the page told the operator + /// the button dialled a real server. A rate limiter on an action that does not happen is the + /// most convincing possible no-op. + /// + /// public async Task RequestCheckAsync(Guid claimId, CancellationToken cancellationToken = default) { if (await claims.FindAsync(claimId, cancellationToken) is not { } claim || !MayRecheck(claim)) @@ -163,11 +184,19 @@ public async Task RequestCheckAsync(Guid claimId, CancellationToken cancel var now = time.GetUtcNow(); + // Recorded whether or not a target moved. An ask is a thing that happened, and a claim on a + // game the registry has no target for — merged away, or added by hand — is not the + // claimant's mistake to be told about in an audit log. await claims.UpdateAsync(claim with { LastCheckedAt = now }, cancellationToken); await claims.RecordEventAsync( new ClaimEvent(claim.Id, now, ClaimEventKind.CheckRequested), cancellationToken); + if (probes is not null) + { + await probes.BringForwardAsync(claim.GameId, now, cancellationToken); + } + return true; } diff --git a/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs b/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs index 657516c..2fcf1ef 100644 --- a/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs +++ b/src/MUI.Crawler/CrawlerServiceCollectionExtensions.cs @@ -87,6 +87,21 @@ public static IServiceCollection AddMuiCrawlerCore(this IServiceCollection servi services.TryAddSingleton(s => s.GetRequiredService()); services.TryAddSingleton(s => s.GetRequiredService()); + // §8's claim settling. Every probe of a game whose owner has published a token completes the + // claim, and every probe of a claimed game refreshes beacon_last_seen_at — both on the + // ordinary schedule, which is why this belongs to the crawl graph rather than to the web + // tier that mints the tokens. + // + // The consumer is the in-process CrawlCycle of the one deployable (§4.11) — this method has + // exactly one caller, MUI.Web's Program, and mui-crawl builds its own graph by hand. It was + // missing here, and CrawlCycle takes its ClaimService as an OPTIONAL parameter, so the site + // settled beacons only insofar as some other registration happened to supply one. That is + // the shape of the bug rather than an argument for a deployment nobody builds. + services.TryAddSingleton(s => new NpgsqlClaimStore(s.GetRequiredService())); + services.TryAddSingleton( + s => new NpgsqlOnDemandProbes(s.GetRequiredService())); + services.TryAddSingleton(); + // The three writers of §6.5, plus the field registry they judge staleness against. services.TryAddSingleton(FieldRegistry.Instance); services.TryAddSingleton(); diff --git a/src/MUI.Crawler/Persistence/NpgsqlOnDemandProbes.cs b/src/MUI.Crawler/Persistence/NpgsqlOnDemandProbes.cs new file mode 100644 index 0000000..6d2d5b9 --- /dev/null +++ b/src/MUI.Crawler/Persistence/NpgsqlOnDemandProbes.cs @@ -0,0 +1,49 @@ +using Dapper; + +using MUI.Catalog; + +using Npgsql; + +namespace MUI.Crawler.Persistence; + +/// +/// §8.1's on-demand check, against the crawl registry: bring this game's next probe forward. +/// +/// +/// +/// LEAST rather than an assignment, so an ask can only ever make a probe sooner. A target +/// already overdue stays where it is in the queue rather than being pushed back to now, and a +/// claimant pressing the button twice cannot walk their own game backwards. +/// +/// +/// Every endpoint of the game moves, because a game is claimed and a target is an address: an +/// operator who has just edited mush.cnf does not know or care which of their listeners we +/// happen to have a row for. +/// +/// +/// It schedules and does not dial. The crawl loop picks the target up on its next pass, where +/// CRAWL DELAY, the concurrency cap and §7.2's resolved-address gate all still apply — none +/// of which a page may bypass. +/// +/// +public sealed class NpgsqlOnDemandProbes(NpgsqlDataSource source) : IOnDemandProbes +{ + public async Task BringForwardAsync( + Guid gameId, + DateTimeOffset at, + CancellationToken cancellationToken = default) + { + await using var connection = await source.OpenConnectionAsync(cancellationToken); + + var moved = await connection.ExecuteAsync(new CommandDefinition( + """ + UPDATE crawl_target + SET next_probe_at = LEAST(next_probe_at, @at) + WHERE game_id = @gameId + """, + new { gameId, at = at.ToUniversalTime() }, + cancellationToken: cancellationToken)); + + return moved > 0; + } +} diff --git a/src/MUI.Web/Accounts/Passkeys.cs b/src/MUI.Web/Accounts/Passkeys.cs index acb7618..4b73cf9 100644 --- a/src/MUI.Web/Accounts/Passkeys.cs +++ b/src/MUI.Web/Accounts/Passkeys.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Identity; +using Microsoft.Extensions.DependencyInjection.Extensions; using MUI.Catalog; using MUI.Catalog.Persistence; @@ -52,10 +53,26 @@ public static IServiceCollection AddMuiAccounts( s.GetRequiredService(), s.GetRequiredService())); - services.AddScoped(s => new ClaimService( - new NpgsqlClaimStore(s.GetRequiredService()), - new NpgsqlGameStore(s.GetRequiredService()), - s.GetRequiredService())); + // Claiming's own two services, and both are singletons for a reason that is not tidiness. + // + // The dashboard resolves IClaimStore from the container and treats a null as "this site has + // no database" — so an unregistered store did not throw, it made every operator's list of + // claimed games empty on a site that had them. Nothing registered it, here or anywhere. + // + // ClaimService was scoped, and the crawl loop that settles beacons is a singleton + // BackgroundService: a scoped dependency is one CrawlCycle can never legally be given. With + // scope validation on — which is what `dotnet run` does — the container refused to build at + // all, so the site did not start with a connection string set. Both are stateless over a + // pooled NpgsqlDataSource, so a singleton is what they should always have been. + // + // TryAdd, because AddMuiCrawler registers the same pair for the crawl loop and this method + // registers it for the dashboard — one deployable calls both (§4.11), and neither may end up + // with a second instance or a second lifetime. + services.TryAddSingleton(s => new NpgsqlClaimStore( + s.GetRequiredService())); + services.TryAddSingleton(s => new NpgsqlGameStore( + s.GetRequiredService())); + services.TryAddSingleton(); services.Configure(options => { @@ -196,11 +213,15 @@ await signIn.MakePasskeyRequestOptionsAsync(user: null), : Results.Unauthorized(); }); - // §8.1's on-demand check. It does not dial anything itself — it records that the claimant - // asked, and the crawler's own scheduler is what brings the probe forward. Keeping the two - // apart is what stops a button on a public page becoming a way to make us connect to a - // stranger's server on demand: the rate limit is per claim, and a claim cannot exist for a - // game nobody has been offered. + // §8.1's on-demand check. It does not dial anything itself — it brings the game's crawl + // targets forward and the crawler's own loop does the dialling, under CRAWL DELAY and §7.2's + // address gate. Keeping the two apart is what stops a button on a public page becoming a way + // to make us connect to a stranger's server on demand: the rate limit is per claim, and a + // claim cannot exist for a game nobody has been offered. + // + // For as long as this existed it moved nothing at all — due-ness comes only from + // crawl_target.next_probe_at, and this wrote last_checked_at and an audit event. See + // ClaimService.RequestCheckAsync and IOnDemandProbes. app.MapPost("/g/{slug}/claim/check", async ( HttpContext context, UserManager users, diff --git a/src/MUI.Web/Components/Pages/Claim.razor b/src/MUI.Web/Components/Pages/Claim.razor index 5e0a094..d5fdef2 100644 --- a/src/MUI.Web/Components/Pages/Claim.razor +++ b/src/MUI.Web/Components/Pages/Claim.razor @@ -100,14 +100,25 @@ else
- - @if (!CanCheck) - { - - Just looked. You can ask again in a few minutes — the button dials a real - server, so it is rationed. - - } + + @* + It moves your game to the front of the crawl queue; the crawler still does the + dialling, on its own schedule and under CRAWL DELAY. Saying it "dials a real + server" was both an overstatement and — for as long as nothing moved the + schedule — a description of something that did not happen at all. + *@ + + @if (CanCheck) + { + @("Brings your game to the front of the queue. We dial on our own schedule, " + + "so this is sooner rather than now.") + } + else + { + @("Just asked. You can ask again in a few minutes — it is rationed because it " + + "makes us connect to a real server sooner than we would have.") + } + } diff --git a/src/MUI.Web/Data/PostgresData.cs b/src/MUI.Web/Data/PostgresData.cs index aed984c..d71afc7 100644 --- a/src/MUI.Web/Data/PostgresData.cs +++ b/src/MUI.Web/Data/PostgresData.cs @@ -1,5 +1,8 @@ using MUI.Catalog; using MUI.Catalog.Persistence; + +using Microsoft.Extensions.DependencyInjection.Extensions; + using Npgsql; namespace MUI.Web.Data; @@ -49,13 +52,27 @@ public static class PostgresData /// public static void AddPostgresCatalogue(this IServiceCollection services, string connectionString) { - services.AddSingleton(_ => NpgsqlDataSource.Create(connectionString)); + ArgumentNullException.ThrowIfNull(services); + + // TryAdd throughout, and the availability store registered once and exposed through its + // interfaces rather than newed per interface. Both matter because AddMuiCrawler registers + // the same objects for the crawl loop and one deployable calls both (§4.11): with AddSingleton + // this method won the IAvailabilityStore registration while the crawler's TryAdd was skipped, + // leaving the concrete type and IReachableHistory pointing at a SECOND instance. Harmless on + // one pool, and a direct contradiction of the comment in the crawler that says the store is + // registered once because two would be two connection paths answering one question. + services.TryAddSingleton(_ => NpgsqlDataSource.Create(connectionString)); + + // The registry is a lookup table with no state, so the shared instance rather than a second + // one — which is also what the crawler registers. + services.TryAddSingleton(FieldRegistry.Instance); + + services.TryAddSingleton(s => new NpgsqlAvailabilityStore(s.GetRequiredService())); + services.TryAddSingleton(s => s.GetRequiredService()); + services.TryAddSingleton(s => s.GetRequiredService()); + services.TryAddSingleton(); - services.AddSingleton(); - services.AddSingleton(s => - new NpgsqlAvailabilityStore(s.GetRequiredService())); - services.AddSingleton(); - services.AddSingleton(s => new NpgsqlGameQueries( + services.TryAddSingleton(s => new NpgsqlGameQueries( s.GetRequiredService(), s.GetRequiredService())); } diff --git a/src/MUI.Web/Program.cs b/src/MUI.Web/Program.cs index 0082294..526ad05 100644 --- a/src/MUI.Web/Program.cs +++ b/src/MUI.Web/Program.cs @@ -1,60 +1,17 @@ -using MUI.Catalog; -using MUI.Crawler; -using MUI.Web.Accounts; -using MUI.Web.Api; -using MUI.Web.Components; +using MUI.Web; using MUI.Web.Data; -using MUI.Web.Fixtures; var builder = WebApplication.CreateBuilder(args); -builder.Services.AddRazorComponents(); - -// The read API (spec §10) reads through the same IGameQueries the pages do, so the two surfaces -// cannot disagree about a fact. What it adds of its own — the dataset licence, the slug aliases and -// the attribution list — is configuration, because none of it is a measurement. -builder.Services.AddMuiApi(builder.Configuration); - -// The site reads through IGameQueries and nothing else, and it prefers a real database to a -// fixture. Point MUI_POSTGRES (or ConnectionStrings:MUIndex) at the catalogue the crawler writes and -// every page renders measurements; without one the site still starts, but it says loudly and on -// every page that what it is showing was not measured. -// -// The fixture is not a fallback so much as a confession. A directory whose whole claim is that its -// data is measured must never quietly present invented data as though it were real — so the demo -// path is opt-in by absence, announced in the log, and marked in the page itself. +// The site reads through IGameQueries and nothing else, and it prefers a real database to a fixture. +// Point MUI_POSTGRES (or ConnectionStrings:MUIndex) at the catalogue the crawler writes and every +// page renders measurements; without one the site still starts, but it says loudly and on every page +// that what it is showing was not measured. var connectionString = PostgresData.ResolveConnectionString(builder.Configuration); -if (connectionString is not null) -{ - builder.Services.AddPostgresCatalogue(connectionString); - builder.Services.AddMuiCrawler(connectionString, configure => - { - // MUI.Web already applies migrations during startup; the hosted crawler should not repeat - // them when it takes the lease. - configure.ApplyMigrations = false; - }); -} -else -{ - builder.Services.AddSingleton(); - builder.Services.AddSingleton(s => s.GetRequiredService()); - builder.Services.AddSingleton(s => s.GetRequiredService()); -} - -builder.Services.AddSingleton(new CatalogueSource(connectionString is not null)); - -// Claiming needs a database: an account, a passkey and a claim are all rows (spec §8). Against the -// demo fixture the sign-in and claim surfaces are simply absent rather than present and broken — -// half a claim flow over invented games would be a worse answer than none. -if (connectionString is not null) -{ - builder.Services.AddMuiAccounts(builder.Configuration); -} - -// Ages are relative to a clock, and a clock is a dependency like any other — the plain surface and -// the rendered page must not each reach for DateTimeOffset.UtcNow and disagree by a tick. -builder.Services.AddSingleton(TimeProvider.System); +// The graph itself lives in SiteComposition, so that CompositionTests can resolve THE SAME +// registrations rather than a copy of them that agrees until somebody edits one of the two. +builder.Services.AddMuiSite(builder.Configuration, connectionString); var app = builder.Build(); @@ -73,22 +30,7 @@ PostgresData.EnvironmentVariable, PostgresData.ConfigurationKey); } -app.UseStaticFiles(); - -// Razor Components register anti-forgery metadata on every endpoint, so the middleware has to be -// present even though this site has no POST form yet. The facet panel is a GET form deliberately — -// a filter is a bookmarkable question, not a state change — so nothing here is token-protected. -app.UseAntiforgery(); - -if (connectionString is not null) -{ - app.UseAuthentication(); - app.UseAuthorization(); - app.MapMuiAccounts(); -} - -app.MapRazorComponents(); -app.MapMuiApi(); +app.UseMuiSite(connectionString); app.Run(); diff --git a/src/MUI.Web/SiteComposition.cs b/src/MUI.Web/SiteComposition.cs new file mode 100644 index 0000000..291fb55 --- /dev/null +++ b/src/MUI.Web/SiteComposition.cs @@ -0,0 +1,122 @@ +using MUI.Catalog; +using MUI.Crawler; +using MUI.Web.Accounts; +using MUI.Web.Api; +using MUI.Web.Components; +using MUI.Web.Data; +using MUI.Web.Fixtures; + +namespace MUI.Web; + +/// +/// Everything the deployable is made of, as two calls Program makes and a test can make too. +/// +/// +/// +/// This exists so that the composition has one spelling. The graph used to live in +/// Program.cs's top-level statements, where nothing but the running site could reach it — so +/// a test of the composition had to restate it, and a restatement is a second copy that +/// agrees with the first only until somebody edits one of them. The failure that motivates the whole +/// of CompositionTests is a registration nobody noticed was wrong; a harness that mirrored +/// Program would have gone on passing through exactly that edit. +/// +/// +/// Program.cs keeps what has side effects — reading the connection string, applying +/// migrations, saying in the log which of the two worlds it is in — because those are things a +/// process does on the way up rather than parts of the graph. +/// +/// +public static class SiteComposition +{ + /// + /// Registers the whole site, with a database behind it or on the demo fixture. + /// + /// The host's service collection. + /// The host's configuration. + /// + /// A PostgreSQL connection string, or null for the demo fixture. Null is not a fallback so much + /// as a confession: a directory whose whole claim is that its data is measured must never + /// quietly present invented data as though it were real, so the demo path is opt-in by absence, + /// announced in the log, and marked on every page through . + /// + public static IServiceCollection AddMuiSite( + this IServiceCollection services, + IConfiguration configuration, + string? connectionString) + { + ArgumentNullException.ThrowIfNull(services); + ArgumentNullException.ThrowIfNull(configuration); + + services.AddRazorComponents(); + + // The read API (spec §10) reads through the same IGameQueries the pages do, so the two + // surfaces cannot disagree about a fact. What it adds of its own — the dataset licence, the + // slug aliases and the attribution list — is configuration, because none of it is a + // measurement. + services.AddMuiApi(configuration); + + // Ages are relative to a clock, and a clock is a dependency like any other — the plain + // surface and the rendered page must not each reach for DateTimeOffset.UtcNow and disagree + // by a tick. Registered before the two worlds below, because both want it. + services.AddSingleton(TimeProvider.System); + + if (connectionString is not null) + { + services.AddPostgresCatalogue(connectionString); + services.AddMuiCrawler(connectionString, configure => + { + // MUI.Web already applies migrations during startup; the hosted crawler should not + // repeat them when it takes the lease. + configure.ApplyMigrations = false; + }); + + // Claiming needs a database: an account, a passkey and a claim are all rows (spec §8). + services.AddMuiAccounts(configuration); + } + else + { + // Against the demo fixture the sign-in and claim surfaces are simply absent rather than + // present and broken — half a claim flow over invented games would be a worse answer + // than none. + services.AddSingleton(); + services.AddSingleton(s => s.GetRequiredService()); + services.AddSingleton(s => s.GetRequiredService()); + } + + services.AddSingleton(new CatalogueSource(connectionString is not null)); + + return services; + } + + /// + /// The middleware and the routes, in the order they have to be in. + /// + /// + /// The order is part of the composition and not a detail of it, so it lives here with the rest of + /// it rather than in a file only the running process reads. + /// + public static WebApplication UseMuiSite(this WebApplication app, string? connectionString) + { + ArgumentNullException.ThrowIfNull(app); + + app.UseStaticFiles(); + + // Razor Components register anti-forgery metadata on every endpoint, so the middleware has + // to be present even though this site has no POST form yet. The facet panel is a GET form + // deliberately — a filter is a bookmarkable question, not a state change — so nothing here + // is token-protected. + app.UseAntiforgery(); + + if (connectionString is not null) + { + app.UseAuthentication(); + app.UseAuthorization(); + app.MapMuiAccounts(); + } + + app.MapRazorComponents(); + app.MapMuiApi(); + + return app; + } +} diff --git a/tests/MUI.Crawler.Tests/OnDemandProbePostgresTests.cs b/tests/MUI.Crawler.Tests/OnDemandProbePostgresTests.cs new file mode 100644 index 0000000..9e25ddb --- /dev/null +++ b/tests/MUI.Crawler.Tests/OnDemandProbePostgresTests.cs @@ -0,0 +1,187 @@ +using Dapper; + +using MUI.Catalog; +using MUI.Catalog.Persistence; +using MUI.Crawler.Persistence; +using MUI.Crawler.Tests.Support; +using MUI.Discovery; + +namespace MUI.Crawler.Tests; + +/// +/// §8.1's on-demand check, against the table that decides what gets probed. +/// +/// +/// The claimant-facing half of this was written first and moved nothing: RequestCheckAsync +/// wrote last_checked_at and a check_requested event, due-ness comes only from +/// crawl_target.next_probe_at, and the page told the operator the button dialled their +/// server. A rate limiter on an action that does not happen is the most convincing possible no-op, +/// so these assert on the schedule rather than on the audit log. +/// +public class OnDemandProbePostgresTests +{ + /// An owner who has just edited mush.cnf is not left waiting on the scheduler. + [Test] + public async Task AskingForACheckBringsTheGamesTargetForward() + { + await using var db = await PostgresFixture.MigratedAsync(); + var now = DateTimeOffset.UtcNow; + + var (game, target) = await SeedAsync(db, next: now.AddHours(6)); + + var moved = await new NpgsqlOnDemandProbes(db.DataSource).BringForwardAsync(game, now); + + await Assert.That(moved).IsTrue(); + await Assert.That(await NextProbeAsync(db, target)).IsEqualTo(now).Within(TimeSpan.FromSeconds(1)); + } + + /// + /// An ask can only make a probe sooner, never later. + /// + /// + /// LEAST rather than an assignment. A target already overdue keeps its place in the queue + /// — pushing it back to "now" would let a claimant pressing the button walk their own game + /// backwards, and would let a busy game's ordinary schedule be reset by anyone who claimed it. + /// + [Test] + public async Task AnAskNeverPushesAProbeBack() + { + await using var db = await PostgresFixture.MigratedAsync(); + var now = DateTimeOffset.UtcNow; + var overdue = now.AddHours(-3); + + var (game, target) = await SeedAsync(db, next: overdue); + + await new NpgsqlOnDemandProbes(db.DataSource).BringForwardAsync(game, now); + + await Assert.That(await NextProbeAsync(db, target)) + .IsEqualTo(overdue).Within(TimeSpan.FromSeconds(1)); + } + + /// + /// Every address of the game moves, because a game is claimed and a target is an address. + /// + /// + /// A game may have several endpoints (§5.5) and an operator does not know or care which of them + /// we happen to hold a row for. Moving one would make the button work or not depending on which + /// listener the crawler had found first. + /// + [Test] + public async Task EveryTargetForTheGameMoves() + { + await using var db = await PostgresFixture.MigratedAsync(); + var now = DateTimeOffset.UtcNow; + + var (game, first) = await SeedAsync(db, next: now.AddHours(6)); + var second = await AttachAsync(db, game, "other.example.org", 4202, now.AddHours(9)); + + await new NpgsqlOnDemandProbes(db.DataSource).BringForwardAsync(game, now); + + await Assert.That(await NextProbeAsync(db, first)).IsEqualTo(now).Within(TimeSpan.FromSeconds(1)); + await Assert.That(await NextProbeAsync(db, second)).IsEqualTo(now).Within(TimeSpan.FromSeconds(1)); + } + + /// + /// A game the registry has no target for is not an error, and says so by answering false. + /// + /// + /// It happens: a game merged away under §7.3, or one added to the catalogue by a path that never + /// minted a target. The claimant's ask is still recorded — it is a thing that happened — and the + /// caller learns nothing moved rather than being handed an exception to render. + /// + [Test] + public async Task AGameWithNoTargetMovesNothingAndDoesNotThrow() + { + await using var db = await PostgresFixture.MigratedAsync(); + + var moved = await new NpgsqlOnDemandProbes(db.DataSource) + .BringForwardAsync(Guid.CreateVersion7(), DateTimeOffset.UtcNow); + + await Assert.That(moved).IsFalse(); + } + + /// + /// One game's ask does not disturb another's schedule. + /// + [Test] + public async Task AnAskTouchesOnlyTheGameItNames() + { + await using var db = await PostgresFixture.MigratedAsync(); + var now = DateTimeOffset.UtcNow; + var later = now.AddHours(6); + + var (mine, _) = await SeedAsync(db, next: later); + var (_, theirs) = await SeedAsync(db, next: later, slug: "other", host: "third.example.org"); + + await new NpgsqlOnDemandProbes(db.DataSource).BringForwardAsync(mine, now); + + await Assert.That(await NextProbeAsync(db, theirs)) + .IsEqualTo(later).Within(TimeSpan.FromSeconds(1)); + } + + private static async Task<(Guid Game, Guid Target)> SeedAsync( + TestDatabase db, + DateTimeOffset next, + string slug = "corvid", + string host = "mud.example.org") + { + var game = Guid.CreateVersion7(); + + await new NpgsqlGameStore(db.DataSource).InsertAsync(new GameRecord( + game, + slug, + slug, + Tagline: null, + LifecycleState.Active, + IsClaimed: true, + FirstSeenAt: DateTimeOffset.UtcNow.AddYears(-1), + LastReachableAt: null, + ArchivedAt: null)); + + return (game, await AttachAsync(db, game, host, 4201, next)); + } + + private static async Task AttachAsync( + TestDatabase db, + Guid game, + string host, + int port, + DateTimeOffset next) + { + var targets = new NpgsqlCrawlTargetRepository(db.DataSource); + + var id = await targets.AddAsync( + new CrawlTarget + { + Id = Guid.CreateVersion7(), + Host = host, + Port = port, + Depth = 0, + NextProbeAt = next, + FirstSeenAt = DateTimeOffset.UtcNow, + }, + CancellationToken.None); + + await targets.AttachGameAsync(id, game, CancellationToken.None); + + return id; + } + + /// + /// The stored schedule, read back. + /// + /// + /// Through , because Npgsql hands back a DateTime for a + /// timestamptz and asking Dapper for a is an invalid cast — + /// the same thing NpgsqlClaimStore's own row type exists to work around. + /// + private static async Task NextProbeAsync(TestDatabase db, Guid target) + { + await using var connection = await db.DataSource.OpenConnectionAsync(); + + var at = await connection.ExecuteScalarAsync( + "SELECT next_probe_at FROM crawl_target WHERE id = @target", new { target }); + + return new DateTimeOffset(DateTime.SpecifyKind(at, DateTimeKind.Utc)); + } +} diff --git a/tests/MUI.Web.Tests/CompositionTests.cs b/tests/MUI.Web.Tests/CompositionTests.cs new file mode 100644 index 0000000..c14f3af --- /dev/null +++ b/tests/MUI.Web.Tests/CompositionTests.cs @@ -0,0 +1,317 @@ +using Microsoft.AspNetCore.Builder; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +using MUI.Catalog; +using MUI.Catalog.Persistence; +using MUI.Crawler; +using MUI.Web; +using MUI.Web.Data; +using MUI.Web.Fixtures; + +namespace MUI.Web.Tests; + +/// +/// The graph Program builds, resolved. +/// +/// +/// +/// This exists because the site has two compositions and only one of them was ever exercised. +/// mui-crawl constructs the crawl loop by hand and passes every collaborator explicitly, so +/// its claim path works and has tests; the deployed site assembles the same objects through DI, and +/// there a missing registration is not a compile error — it is a null in an optional parameter, a +/// service locator answering null, or an exception on the first request to one endpoint. +/// +/// +/// Every assertion below was a live hole when it was written. None of them could have been caught by +/// a unit test of the thing that was broken, because none of the things were broken: the wiring +/// between them was. +/// +/// +public class CompositionTests +{ + /// + /// A connection string is a string. Nothing here opens a connection — NpgsqlDataSource is + /// lazy — so the graph can be built and validated without a database. + /// + private const string ConnectionString = "Host=127.0.0.1;Port=1;Database=mui;Username=mui"; + + /// + /// The dashboard could not list a single claim, because nothing registered the store it reads. + /// + /// + /// Account.razor asks the container for and treats a null as + /// "this site has no database", which is exactly what it looked like: the page rendered, said + /// nothing was claimed, and was wrong for every operator who had ever claimed anything. A + /// service-located dependency fails silently by construction, which is why this is asserted at + /// the composition root rather than left to the page. + /// + [Test] + public async Task TheDashboardCanReachTheClaimsItListsAndTheEndpointThatChecksThem() + { + await using var site = Site(); + var provider = site.Services; + + await Assert.That(provider.GetService()).IsNotNull(); + await Assert.That(provider.GetService()).IsNotNull(); + } + + /// + /// The crawl loop can settle a claim, which is the only thing that ever verifies one. + /// + /// + /// + /// takes its as an optional parameter — a + /// crawl with no database behind it is a crawl doing slightly less, not one that should refuse to + /// run — and the deployed site registered no ClaimService the container could put there. + /// So the parameter took its default, SettleClaimsAsync returned on its first line, and no + /// probe has ever verified a claim on the one deployment that has operators. + /// + /// + /// The symptom named in §8.5 — nothing sets game.is_claimed, so the listing badge and + /// §7.5's ceiling grace are unexercised — is this, and it survived claiming shipping because + /// mui-crawl passes the service by hand and its tests construct the cycle the same way. + /// + /// + [Test] + public async Task TheHostedCrawlerIsGivenTheServiceThatVerifiesAClaim() + { + await using var site = Site(); + var provider = site.Services; + + var cycle = provider.GetRequiredService(); + + await Assert.That(ClaimsOf(cycle)).IsNotNull(); + } + + /// + /// The claim service is not scoped, because the thing that needs it most is a singleton. + /// + /// + /// A background service outlives every request scope, so a scoped is + /// one can never be given: with scope validation on it is a startup + /// failure, and with it off it is a captive dependency holding one scope's objects for the + /// lifetime of the process. Validation is switched on here so the wrong answer cannot pass. + /// + [Test] + public async Task TheGraphSurvivesScopeValidation() + { + await using var site = Site(); + var provider = site.Services; + + await Assert.That(provider.GetRequiredService()).IsNotNull(); + + await using var scope = provider.CreateAsyncScope(); + + await Assert.That(scope.ServiceProvider.GetRequiredService()).IsNotNull(); + } + + /// Everything the owner write path needs, from a request scope. + [Test] + public async Task TheOwnerWritePathResolves() + { + await using var site = Site(); + var provider = site.Services; + await using var scope = provider.CreateAsyncScope(); + + await Assert.That(scope.ServiceProvider.GetService()).IsNotNull(); + await Assert.That(scope.ServiceProvider.GetService()).IsNotNull(); + } + + /// + /// The same graph in Production, where nothing validates it for us. + /// + /// + /// + /// Asserted separately because the two environments behaved differently, and this one is the + /// control rather than the finding: it passed on the parent commit. Production leaves + /// scope validation off, so the container resolved the scoped ClaimService from the root + /// and handed CrawlCycle a working one — a captive dependency, harmless for a stateless + /// service over a pooled data source, and the reason claims verified in production while + /// Development would not start at all. + /// + /// + /// It is kept because it is what stops the fix regressing in the other direction: Production + /// must go on resolving the cycle once the lifetime changes. + /// + /// + [Test] + public async Task TheHostedCrawlerCanBeResolvedInProductionToo() + { + await using var site = Site(Environments.Production); + + await Assert.That(site.Services.GetRequiredService()).IsNotNull(); + } + + /// + /// The store the dashboard reads, asked for in the environment that can build a container. + /// + /// + /// Separated from the Development assertion so the two findings cannot be confused for one. The + /// lifetime mistake stops Development dead and leaves Production working; this one is a plain + /// missing registration and is missing in both, which is why every operator's dashboard was + /// empty on a running production site. + /// + [Test] + public async Task TheDashboardsClaimStoreIsMissingInEveryEnvironmentNotJustTheValidatedOne() + { + await using var site = Site(Environments.Production); + + await Assert.That(site.Services.GetService()).IsNotNull(); + } + + /// + /// The crawl loop is handed a claim service in Production, where the container tolerates it. + /// + /// + /// This is what makes §8.5's closing note stale rather than true: a production site does settle + /// beacons and does set game.is_claimed. It gets there by resolving a scoped service from + /// the root, which the container permits only because Production leaves scope validation off — so + /// the behaviour is correct by accident and stops the moment anybody switches validation on. + /// + [Test] + public async Task TheProductionCrawlLoopDoesGetItsClaimService() + { + await using var site = Site(Environments.Production); + + await Assert.That(ClaimsOf(site.Services.GetRequiredService())).IsNotNull(); + } + + /// + /// The demo composition, which is a different graph and not a smaller one. + /// + /// + /// With no connection string there are no accounts, no crawler and no claims — the sign-in and + /// claim surfaces are absent rather than present and broken (§8), and half a claim flow over + /// invented games would be a worse answer than none. The assertion that matters is the last one: + /// the fixture composition must still say on every page that nothing on it was measured. + /// + [Test] + public async Task TheDemoCompositionStandsUpAndAdmitsWhatItIs() + { + await using var site = Site(connectionString: null); + var provider = site.Services; + + await Assert.That(provider.GetService()).IsTypeOf(); + await Assert.That(provider.GetService()).IsNotNull(); + + // Absent, not broken. A page asks the container whether claiming exists at all. + await Assert.That(provider.GetService()).IsNull(); + await Assert.That(provider.GetService()).IsNull(); + await Assert.That(provider.GetService()).IsNull(); + + await Assert.That(provider.GetRequiredService().IsMeasured).IsFalse(); + } + + /// + /// One availability store, however many names it answers to. + /// + /// + /// The crawler registers it once and exposes it through two interfaces, saying in a comment that + /// two instances would be two connection paths answering one question. The web tier then + /// registered its own with AddSingleton, so the crawler's TryAdd for + /// IAvailabilityStore was skipped and the concrete type and IReachableHistory + /// pointed at a second one. Harmless on a shared pool, and a comment that had stopped being true. + /// + [Test] + public async Task TheAvailabilityStoreIsOneObjectUnderEveryNameItHas() + { + await using var site = Site(); + var provider = site.Services; + + var concrete = provider.GetRequiredService(); + + await Assert.That(provider.GetRequiredService()).IsSameReferenceAs(concrete); + await Assert.That(provider.GetRequiredService()).IsSameReferenceAs(concrete); + } + + /// + /// §8.1's on-demand check can reach the schedule it claims to move. + /// + /// + /// The same shape as the bug this file exists for: ClaimService takes + /// optionally, because it is constructible without a crawl registry + /// and mui-crawl constructs it that way — so nothing but the composition can say whether + /// the deployed site has one. Without it the button records an ask, moves no probe, and tells the + /// operator it dialled their server. + /// + [Test] + public async Task TheOnDemandCheckIsGivenSomethingToBringForward() + { + await using var site = Site(); + + await Assert.That(site.Services.GetService()).IsNotNull(); + await Assert.That(ProbesOf(site.Services.GetRequiredService())).IsNotNull(); + } + + /// + /// Program's graph, built by calling Program's own registration. + /// + /// + /// + /// and not a copy of it. This restated the + /// registrations to begin with, which is a second copy that agrees with the first only until + /// somebody edits one of them — and the edit that breaks it is precisely the one these tests + /// exist to catch: a scoped service consumed by a singleton, or AddMuiAccounts moving. + /// The graph therefore moved out of Program's top-level statements so that the site and + /// this can call the same thing. + /// + /// + /// A real rather than a bare , + /// because half the framework's own registrations want IConfiguration and + /// IHostEnvironment and a hand-built collection fails validation on those before it can say + /// anything about ours. What is left after the host supplies them is our graph, and only ours. + /// + /// + /// Development by default, where + /// switches scope validation on by itself — so this is the check a developer already gets on + /// dotnet run and a production deployment does not. + /// + /// + /// Nothing is started and no migration runs: Program does both after the graph is built, + /// and this stops at the graph. is never connected to — + /// NpgsqlDataSource is lazy. + /// + /// + private static WebApplication Site(string? environment = null, string? connectionString = ConnectionString) + { + var builder = WebApplication.CreateBuilder(new WebApplicationOptions + { + EnvironmentName = environment ?? Environments.Development, + }); + + builder.Logging.ClearProviders(); + builder.Services.AddMuiSite(builder.Configuration, connectionString); + + return builder.Build(); + } + + /// + /// The cycle's claim service, read off the instance. + /// + /// + /// Reflection, because the collaborator is a private primary-constructor parameter and the + /// property under test is precisely that the container supplied one rather than letting the + /// default stand. Exposing it publicly to be asserted on would widen the type for the test's + /// convenience; a wiring test is allowed to look at the wiring. + /// + private static object? ClaimsOf(CrawlCycle cycle) => Collaborator(cycle); + + private static object? ProbesOf(ClaimService claims) => Collaborator(claims); + + /// + /// One privately-held collaborator, read off an instance. + /// + /// + /// Reflection, because both of these are private primary-constructor parameters and the property + /// under test is precisely that the container supplied one rather than letting the default stand. + /// Exposing them publicly to be asserted on would widen two types for a test's convenience; a + /// wiring test is allowed to look at the wiring. + /// + private static object? Collaborator(object instance) => + instance.GetType() + .GetFields(System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic) + .Single(field => field.FieldType == typeof(T)) + .GetValue(instance); +} diff --git a/tests/MUI.Web.Tests/PasskeyEndpointTests.cs b/tests/MUI.Web.Tests/PasskeyEndpointTests.cs new file mode 100644 index 0000000..72c02e5 --- /dev/null +++ b/tests/MUI.Web.Tests/PasskeyEndpointTests.cs @@ -0,0 +1,184 @@ +using System.Net; +using System.Text; + +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Hosting.Server; +using Microsoft.AspNetCore.Hosting.Server.Features; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; +using Microsoft.Extensions.Logging; + +using MUI.Web; + +namespace MUI.Web.Tests; + +/// +/// The passkey ceremony's four endpoints, driven as the browser drives them. +/// +/// +/// +/// §8.2 makes passkeys the only way in, so if these refuse a request nobody can sign in, +/// nobody can claim anything, and every owner surface on the site is unreachable. Nothing tested +/// them: the suites that exercise sign-in are the ones that stub it, and the composition tests stop +/// at the graph. +/// +/// +/// These drive the real routes through the real pipeline — +/// and , the same two calls the deployable makes — because +/// the property at issue is what the middleware does to a request, and no synthetic endpoint can +/// answer that for the endpoints that actually exist. +/// +/// +/// No database is behind them. The connection string points at a port nothing listens on, which is +/// enough for everything asserted here: assertion-options answers in full without storage, +/// and the two that need it are asserted on what the middleware did rather than on how the handler +/// ended. Against a real Postgres all three answer 200, 200 and 401 — measured, on a host with +/// migrations applied. +/// +/// +public class PasskeyEndpointTests +{ + /// What passkey.js posts: a JSON envelope with the credential as a string. + private const string SignInBody = + """ + {"credential":"{\"id\":\"abc\",\"rawId\":\"abc\",\"type\":\"public-key\",\"response\":{\"clientDataJSON\":\"e30\",\"authenticatorData\":\"e30\",\"signature\":\"e30\",\"userHandle\":null}}","name":null} + """; + + /// + /// The anti-forgery middleware is live on this host, which is what makes the rest meaningful. + /// + /// + /// The control, and it has to come first: "the passkey endpoints are not blocked" says nothing + /// unless something on the same host is. MapRazorComponents puts anti-forgery + /// metadata on every component route, so a POST to one without a token is refused — and that is + /// the same middleware, in the same pipeline, one route over. + /// + [Test] + public async Task AntiforgeryIsLiveAndRefusesAnUntokenedPostToAPage() + { + await using var host = await Host.StartAsync(); + + var response = await host.PostAsync("/account", content: null); + + await Assert.That(response.StatusCode).IsEqualTo(HttpStatusCode.BadRequest); + } + + /// + /// The ceremony's JSON endpoints are not anti-forgery-gated, and answer. + /// + /// + /// + /// A minimal API is given anti-forgery metadata only when it binds form data — + /// IFormCollection, IFormFile, [FromForm]. These bind JSON or a query + /// string, so they carry none and the middleware passes them through. That is the whole + /// mechanism, and it is why the control above is refused while these are not. + /// + /// + /// Both of these are reachable before any account exists, which is the point: a first-time + /// visitor's very first request to this site is one of them. assertion-options mints a + /// challenge without reading anything, so it is asserted in full; registration-options + /// reads the account's existing credentials and therefore needs the database this host does not + /// have, so it is asserted on the only thing under test here — that it was not refused as forged. + /// + /// + [Test] + public async Task TheOptionsEndpointsAnswerAJsonPostWithNoToken() + { + await using var host = await Host.StartAsync(); + + var assertion = await host.PostAsync("/account/passkey/assertion-options", content: null); + var registration = await host.PostAsync( + "/account/passkey/registration-options?name=probe", content: null); + + await Assert.That(assertion.StatusCode).IsEqualTo(HttpStatusCode.OK); + await Assert.That(await assertion.Content.ReadAsStringAsync()).Contains("challenge"); + + await Assert.That(registration.StatusCode).IsNotEqualTo(HttpStatusCode.BadRequest); + } + + /// + /// The sign-in POST reaches its handler rather than being refused as forged. + /// + /// + /// Asserted as "not 400", deliberately, because what is under test is the middleware and not the + /// outcome of the ceremony: with no database behind it the handler cannot finish, and pinning a + /// specific failure would be pinning the wrong thing. Against a real database this same request + /// answers 401 — measured, on a host with Postgres behind it and migrations applied. + /// + [Test] + public async Task TheSignInPostIsNotRefusedAsForged() + { + await using var host = await Host.StartAsync(); + + var response = await host.PostAsync("/account/passkey/sign-in", SignInBody); + + await Assert.That(response.StatusCode).IsNotEqualTo(HttpStatusCode.BadRequest); + } + + /// The site, composed exactly as the deployable composes it, on a loopback port. + private sealed class Host : IAsyncDisposable + { + /// Never connected to. NpgsqlDataSource is lazy and nothing here dials it. + private const string ConnectionString = "Host=127.0.0.1;Port=1;Database=mui;Username=mui"; + + private readonly WebApplication _app; + private readonly HttpClient _client; + + private Host(WebApplication app, HttpClient client) + { + _app = app; + _client = client; + } + + public static async Task StartAsync() + { + var builder = WebApplication.CreateBuilder(new WebApplicationOptions + { + EnvironmentName = Environments.Production, + }); + + builder.Logging.ClearProviders(); + builder.WebHost.UseUrls("http://127.0.0.1:0"); + builder.Services.AddMuiSite(builder.Configuration, ConnectionString); + + // The crawl loop is not under test and would spend the test dialling a database that is + // not there. It is built never to fault the host, so this is noise rather than risk — + // and removing exactly one descriptor leaves every other registration where it was. + if (builder.Services.FirstOrDefault(d => + d.ImplementationType?.Name == "CrawlerService") is { } crawler) + { + builder.Services.Remove(crawler); + } + + var app = builder.Build(); + app.UseMuiSite(ConnectionString); + + await app.StartAsync(); + + var address = app.Services.GetRequiredService().Features + .Get()!.Addresses.First(); + + return new Host( + app, + new HttpClient(new HttpClientHandler { AllowAutoRedirect = false }) + { + BaseAddress = new Uri(address), + }); + } + + public Task PostAsync(string path, string? content) => + _client.PostAsync( + path, + content is null + ? null + : new StringContent(content, Encoding.UTF8, "application/json")); + + public async ValueTask DisposeAsync() + { + _client.Dispose(); + await _app.StopAsync(); + await _app.DisposeAsync(); + } + } +}