Repository navigation
refactor(auth): route the auth endpoints through IAccessModule - #1041
Conversation
SaleAllocationPolicyTests pins when ConfirmSaleHandler reads the actor's role and the allocation policy, but nothing pinned when it reads a plain Worker's flock assignments. This test parks a restricted worker's confirm on the account lock while another transaction removes the worker's only assignment, then requires the confirm to allocate as an unrestricted worker. It passes on main and guards the move of that read behind IAccessLookup. Refs #857
…r reads behind IAccessLookup IAccessModule is Access's surface for HTTP adapters. The users and me endpoints call it for every route; each member forwards to the existing handler or identity call, so step-up checks, locks, audit rows and errors are unchanged. It takes IIdentityProvider from DI, so a registered decorator stays in the path. IAccessLookup is the peer read port: the effective role and the flock assignments. AccessLookup depends only on the scoped AppDbContext. ConfirmSaleHandler, FlockScopeGuard and FlockScopeResolutionMiddleware read through it. The effective-role query is one routine, AccessLookup.ReadEffectiveRoleAsync, which IdentityProvider.GetEffectiveRoleAsync also runs, with both statements and Roles.ResolveEffective unchanged. SQL is unchanged except confirm's assignment read, which drops ORDER BY "Id" (the rows are used as a set), and the middleware and guard reads, whose parameter is now @userid instead of @user_UserId. Four test lines change, each passing the same unreachable context through the real lookup: FlockScopeGuardTests (1), FlockScopeMiddlewareTests (2) and TenantResolutionMiddlewareTests (1). The ledger gains the GetMe and ListUsers adapter rows and two Access -> Farm symbols; the tenant-bypass rows follow the moved query. Refs #857
…ssLookup No suite put two workers with different assignments in one farm, or gave one worker both a flock row and a farm-wide row. A lookup that returned the whole farm's rows, or dropped farm-wide rows, passed every test. The first would let a restricted worker inherit a colleague's farm-wide access. These tests seed exactly those shapes, at the port and through the flock-scope middleware. Refs #857
…role read's account scope Review hardening for PR #1037; both gaps predate the PR and the port now owns them. - ListFlockAssignmentsAsync relies on the tenant query filter. Under an unresolved tenant it read no rows, and no rows means account-wide access. It now throws before the query (the SQL is unchanged). Every caller resolves the tenant first: HTTP with the actor, both seeders before acting, confirm before its account lock. AppDbContext exposes its TenantContext, as it already exposes FlockScope. - Nothing pinned the account predicate in the shared effective-role query. A new test gives one email a different role in two farms and requires each farm to see only its own user's role. Refs #857
…posing it on AppDbContext AppDbContext.Tenant made the DbContext carry request state for one caller. AccessLookup now takes the same scoped TenantContext through its own constructor, next to AppDbContext. The fail-closed guard and the SQL are unchanged; the getter is gone, so AppDbContext matches main again. The four test lines that construct AccessLookup by hand pass the tenant. Refs #857
… failures Two things the auth endpoints decide themselves, outside IIdentityProvider, were unpinned. The refresh cookie's lifetime comes from Jwt:RefreshTokenDays on login, refresh and password change, and the unknown-farm and suspended-farm login branches each log one LoginFailed with the same shape as an unknown user. Both tests pass on the current endpoints and guard their move behind IAccessModule. Refs #857
Login, refresh, logout, change-password and step-up now take IAccessModule instead of IIdentityProvider, IOptions<JwtOptions>, FirstRunStatusService, AuthSecurityEventLogger, ChangeOwnPasswordHandler and IStepUpGrantService. Each new member forwards to the same service, and the endpoints keep their order of checks, cookies and responses. Only the refresh-token lifetime leaves JwtOptions. Logout and StepUp gain adapter rows reaching Access. Refs #857
|
Review of record (Codex Astra, xhigh) at head PR #1041 security review. Reviewed No CONFIRMED or PLAUSIBLE P0–P3 findings. I found no product or security defect in this change. I compared every changed endpoint call with the base and followed it through the module to its original implementation. Locations below refer to the reviewed head.
The cookie contract is preserved at all four successful writes, including legacy upgrade.
Unknown and suspended farms still emit one identity-free Route metadata and limiter registration are unchanged. Login, change-password and step-up still share #143/#544's IP-keyed login policy and The #338 ordering remains intact. Issuance verifies the password, snapshots the credential epoch/security stamp, resets failed-access state, checks that the credential has not changed, then signs the grant with its logout epoch. Protected handlers still validate proof before target reads. Validation checks the grant and stored security stamp, then consumes the shared claim before reading the durable logout epoch. There is no new outer transaction or retry around this work. Logout still advances the bearer's step-up epoch before cookie revocation. The provider attributes the cookie to its durable owner, checks the expected account before either revocation axis, advances that owner's step-up epoch, then revokes the exact token lineage. A stale rotated cookie can still identify its owner. A cookie-revocation failure cannot undo the already-recorded bearer logout. All five auth paths retain their idempotency exemptions. No live token or consumed grant becomes a cached response. DI review found no captive dependency or new credential cache. Independent parity evidence is trace-comparison.txt: all 36 files are byte-identical, covering 35 scenarios plus the limiter check. That includes 96 SQL statements, cookie writes in 13 scenarios, JWTs in nine scenarios and ten I also measured one 60-round interleaved timing run per revision and discarded five warm-up rounds. Each cell below uses 55 samples; times are milliseconds.
These observations support the same branch costs; they are not a constant-time guarantee or a performance benchmark on an isolated host. Raw samples and measurement limitations are retained. I rebuilt and reproduced three listed mutants and one additional mutant, restoring each afterward. These failures describe deliberately broken review copies, not defects in the PR.
The disclosed filter mistake is credible and resolved in the supplied evidence: Final verification on the restored, unmodified head passed 193/193 targeted integration tests across 22 classes and 8/8 Application architecture checks across four classes. Every class ran alone through The ledger is accurate. Code quality: the thermo-nuclear review found no structural regression. The module gains direct forwarding members and no branching logic, cache, retry, transaction or generic dispatch. Source sizes are 647 lines for Ponytail-review: Lean already. Ship. Nits: none. Limits: I did not run the full integration suite or claim an exhaustive concurrent-schedule proof from the normalized trace. The trace masks random values and rounds cookie expiry to days; the new committed test adds a two-minute expiry tolerance. Separate existing race, retry, suspension and shared-store tests supply the schedule checks. #579's accepted check-then-mint window remains unchanged. The invalid stored-epoch issue identified during the earlier design review has already been addressed by #1031 on the supplied base and is not a finding against C. All reviewer mutations were restored, the scratch test was removed, and Git status was clean at the reviewed head. The assigned detached checkout was then removed with the requested command. No commit, push or GitHub comment was made; no other checkout was edited. Cleanup record. Verdict: APPROVE PR #1041 at |
…7c-auth-endpoints
|
Independent security and code-quality review (Claude Opus 5.5: security-review, thermo-nuclear and ponytail) at head PR #1041 security review (Opus): head
|
| Question | Answer | Evidence |
|---|---|---|
| Admits a revoked, disabled, suspended or foreign-farm credential? | No | RefreshAsync/RevokeRefreshTokenAsync forward expectedAccountId into the same IIdentityProvider parameter (AccessModule.cs:104-109). The reordering is safe: Guid? and CancellationToken cannot be swapped without a compile error. Making expectedAccountId required is a small improvement, because the one legacy-only revoke now writes null explicitly (AuthEndpoints.cs:620). CredentialEpochMiddleware and its fresh read are not in the diff. Reproduced C4 red. |
| Mints or refreshes when it should refuse? | No | Login order is unchanged: size check, resolve farm, unknown, suspended, credential check, then the first-run notice for the default farm only. Each endpoint makes one module call where it made one service call before. |
| Skips step-up or reuses a grant? | No | IssueStepUpGrantAsync(accountId, userId, password) maps to IStepUpGrantService.IssueAsync(Guid accountId, Guid userId, string currentPassword) (IStepUpGrantService.cs:19-20) in the same order. Grant consumption lives in the handlers, which C does not touch. Reproduced C7 (ids swapped) red. |
| Logs out the wrong lineage? | No | Logout's three revoke calls pass the same scope as before. RecordLogoutAsync(currentUser.UserId) is unchanged. |
| Changes cookie name, path, flags or lifetime? | No | AuthCookies.* is not in the diff. The lifetime is jwt.Value.RefreshTokenDays before and after, read through the same IOptions<JwtOptions>. |
| Leaks farm or user existence through a new event, error or timing? | No | RecordLoginFailure() calls the same scoped AuthSecurityEventLogger.LoginFailed() (no user id, no email). Titles are unchanged. Timing: the minimal-API binder constructs AccessModule and its graph before the handler runs, so every branch pays the same cost and no new differential appears. The PR's timing table agrees, but I did not re-measure it. Reproduced C2n red. |
| Changes the rate limiter? | No | MapAuthEndpoints (AuthEndpoints.cs:20-62) is not in the diff: .RequireRateLimiting(...) is the same on login, refresh, change-password and step-up. Logout still has no limiter, as before. |
| Captures scoped auth state in a longer-lived service? | No | IAccessModule is AddScoped (CluckworkIdentityServiceCollectionExtensions.cs:128). Every new dependency is scoped (AuthSecurityEventLogger, FirstRunStatusService, IStepUpGrantService, ChangeOwnPasswordHandler), except IOptions<>, which is a singleton snapshot. Nothing under src/ resolves IAccessModule outside an endpoint parameter. AccessModule holds no fields beyond its constructor parameters. |
| Can the extra construction throw or change an outcome on anonymous routes? | No | Login, refresh and logout now also build StepUpGrantService, PersistentStepUpGrantRegistry, FirstRunStatusService and 11 user handlers. All of them have bodiless primary constructors. The Redis multiplexer is a singleton with AbortOnConnectFail=false (SharedStateRegistration.cs:48-50), and the login and refresh limiters already resolve it. |
| Decorators still in the path? | Yes | The module takes IIdentityProvider from DI (AccessModule.cs:35), not the concrete type. |
| Architecture guards | Green | dotnet test tests/Cluckwork.Application.Tests --filter FullyQualifiedName~Architecture: 359/359 (architecture.log). This covers the ledger, adapter reach, seam surface and coupling matrix. |
Mutation runs (each reverted; one integration class at a time)
The script is mutate.py: an exact single-match replacement, a build, one filtered run, then a revert asserted by git diff --quiet. The baseline at head is 28/28 green for AuthSessionContractTests, PerFarmRefreshCookieTests and AuthCookie* (baseline.log). The checkout is clean after every run.
| # | Mutant | Class run | Result | Failing assertion |
|---|---|---|---|---|
| C2n (PR) | RecordLoginFailure emits nothing, still reads the field |
AuthSessionContractTests |
RED | Expected: 3 Actual: 1 |
| C4 (PR) | module refresh passes null for expectedAccountId |
RefreshAccountBindingTests |
RED | MismatchIsRefusedAndRotatesNothing: Expected: Unauthorized Actual: OK |
| C7 (PR) | module swaps account and user ids on step-up | StepUpAuthTests |
RED | 400 (Bad Request) on four or more tests |
| O1 (mine) | login writes the cookie with a literal 30 instead of access.RefreshTokenLifetimeDays |
AuthSessionContractTests |
GREEN, survives | 2/2 pass |
| O2 (mine) | legacy-cookie upgrade writes a one-day cookie | PerFarmRefreshCookieTests, RefreshAccountBindingTests, AuthSessionContractTests, AuthCookie* |
GREEN, survives | 32/32 pass, including LegacyCookie_RefreshSucceeds_SetsPerFarmCookie_AndDeletesLegacy, which runs the mutated line |
The three PR mutants I reproduced fail with the assertions the PR quotes.
Findings
F1: P3, CONFIRMED (test gap): the lifetime test cannot tell configuration from the default
- Where:
tests/Cluckwork.Api.IntegrationTests/AuthSessionContractTests.cs:69; the guarded line issrc/Cluckwork.Api/Endpoints/Auth/AuthEndpoints.cs:293(also 427 and 472). - Defect: the test reads
daysfrom the factory'sIOptions<JwtOptions>, and nothing in the test host setsJwt:RefreshTokenDays. Sodaysis 30, which equals bothJwtOptions.RefreshTokenDays's default (JwtOptions.cs:12) andappsettings.json:29. The test's name and header say the cookie followsJwt:RefreshTokenDays. In fact it proves only that the cookie lasts 30 days. - Scenario: a later edit replaces
access.RefreshTokenLifetimeDayswith a literal or withnew JwtOptions().RefreshTokenDays. An operator who setsJwt__RefreshTokenDays=7then gets 7-day server rows (IdentityProvider.cs:1595) under 30-day cookies, and every suite stays green. This is a usability mismatch, not a credential leak: the server row still expires. - Evidence: mutant O1 survives (
O1.log). The PR's C1 (days + 1) is red only because it moves the value away from 30. - Fix (in slice): give this class a non-default lifetime, for example
factory.WithWebHostBuilder(b => b.UseSetting("Jwt:RefreshTokenDays", "11")), and assert against 11.
F2: P3, CONFIRMED (test gap; pre-existing): no committed test pins the legacy-upgrade cookie lifetime
- Where:
src/Cluckwork.Api/Endpoints/Auth/AuthEndpoints.cs:379→LegacyUpgradeResult(:435-441). - Defect: this is the fourth refresh-cookie write. The PR's review notes say the lifetime equals
Jwt:RefreshTokenDays"on all four writes: login, refresh, the legacy upgrade and change-password". Only the uncommitted scratch trace shows the fourth.AuthSessionContractTestscovers three, and no other test readsExpireson that path. - Scenario: the upgrade path writes a short-lived per-farm cookie. Users who are still on the legacy cookie are silently signed out after the shorter lifetime. No suite notices.
- Evidence: mutant O2 (lifetime
1) survives 32 tests. That includesLegacyCookie_RefreshSucceeds_SetsPerFarmCookie_AndDeletesLegacy, which runs the mutated line (O2.log). - Fix (in slice): add an
AssertLifetimestep to the legacy upgrade, or correct the PR text to "three committed, four in the trace". The legacy path is temporary (see the#537comment at:366), so editing the text is acceptable.
No other finding survived. In particular, I looked for and rejected these:
- argument transposition in the five refresh and revoke sites (types prevent it);
- a captive dependency (everything is scoped);
- a changed limiter partition (mapping not in the diff);
- a new timing differential (construction happens before branching);
- a lost decorator (the interface comes from DI).
security-review skill output
The skill ran in the coordinator's parent worktree (HEAD at 05d7e7a7), so its injected git context was empty. The brief forbids subagents, so I applied the skill's three phases and its false-positive filter by hand to C's diff:
- Phase 1 (context). Auth uses asymmetric JWTs, rotating refresh tokens bound to a farm (
expectedAccountId), the credential-epoch middleware, a step-up registry backed byIClaimOnceStore, and Redis or in-process limiters. All of these sit outside the diff. - Phase 2 (comparison). The new code adds no input path, query, deserialization, crypto, logging of secrets or PII, or authorization decision. It replaces six injected services with one scoped facade that forwards each call unchanged.
- Phase 3 (assessment).
- Authentication bypass: none, because every token decision is still made inside
IdentityProviderandStepUpGrantService. - Authorization: unchanged, because
.RequireAuthorization()and theIsResolvedchecks are not in the diff. - Session management: the account binding and the cookie are unchanged.
- Data exposure:
RecordLoginFailurelogs the existing user-free shape. - The test file is excluded under the skill's hard exclusion 11.
- Authentication bypass: none, because every token decision is still made inside
- Result: no vulnerabilities at confidence ≥ 8. F1 and F2 are test-strength issues, which the skill excludes, so I report them only in the adversarial section above.
pstack:thermo-nuclear-code-quality-review (report only; all P3, slice-scoped)
File sizes: AuthEndpoints.cs goes from 650 to 647 lines, IAccessModule.cs from 59 to 87, and AccessModule.cs from 86 to 122. No file crosses 1k lines. No new conditionals; the endpoint diff is a parameter swap.
- Q1: P3.
IAccessModuleis a wide, shallow facade (IAccessModule.cs:21-86,AccessModule.cs:22-40). It now has 24 members across four concerns: user administration, own profile, assignments and sessions. Most are one-line forwards, andAccessModuletakes 18 constructor parameters. Every user-admin route therefore builds the session services, and every login builds the 11 user-admin handlers. Removing the six services from the endpoints moves complexity behind the interface rather than deleting it. The concepts a reader holds inLoginare the same five calls with new names. Code-judo path (after PR E, not here): split a session surface out, for exampleIAccessSessions, the same way the design already separatesIAccessOperationsfor process-role reasons, so each surface constructs only what its routes use. Then deepen it: oneSignInAsync(farmCode, email, password)returning a closed outcome (UnknownFarm | Suspended | NoOwnerProvisioned | InvalidCredentials | Success(pair)) would move the branch order into Access and removeResolveFarmCodeAsync,IsFirstRunProvisionedAsyncandRecordLoginFailurefrom the contract. Doing that in C would move security-sensitive ordering, which v2 and the parity trace deliberately freeze, so forwarding is the right shape for this step. - Q2: P3.
RecordLoginFailure()exposes Access's observability policy as a command (IAccessModule.cs:70,AccessModule.cs:102). The adapter decides when aLoginFailedhappens. Any holder ofIAccessModulecan emit the event without checking a credential. That is not a vulnerability, but it is a contract that cannot hold its own invariant. Q1'sSignInAsyncremoves it. - Q3: P3.
RefreshTokenLifetimeDaysputs a configuration value on a behavioral contract (IAccessModule.cs:86,AccessModule.cs:121), and the endpoint threads it through four cookie writes plusLegacyUpgradeResult'slifetimeDaysparameter.IdentityProvideralready computes the row'sExpiresAt = now.AddDays(RefreshTokenDays)(IdentityProvider.cs:1595). IfTokenPaircarriedRefreshTokenExpiry, the cookie would take that instant. That would remove the property, the module'sIOptions<JwtOptions>dependency and the parameter threading, and make the cookie'sExpiresmatch the stored row exactly. It would also fix F1 by construction. This is outside C's mandate: the design says "No PR writes token-minting code" andTokenPairbelongs to the mint path. Record it for PR E or later.
The approval bar is met for a mechanical move. There is no structural regression relative to the base, only a deferred deepening.
ponytail:ponytail-review (report only; all P3, slice-scoped)
AuthSessionContractTests.cs:L22-25: reuse: third private copy ofScalarOf(alsoAuthRateLimitLoggingTests.cs:56,ClientErrorReportTests.cs:57). Oneinternal staticinInfrastructure/replaces all three. Moving the other two is outside this slice, so at most stop adding copies.AuthSessionContractTests.cs:L27-28: shrink:LoginFailures()has one caller (:55). Inline it intovar events = ….AuthSessionContractTests.cs:L30-31: reuse: a problem-title reader already exists twice (RefreshAccountBindingTests.cs:257,CredentialEpochFloorTests.cs:252). Same remedy asScalarOf.src/diff: nothing to cut. Each forward exists to take one of the six dependencies off the endpoint, which is the PR's purpose; Q3 covers the only member that could disappear.
net: -6 lines possible.
Is there any product or security defect?
No. I found no product defect and no security defect. F1 and F2 are confirmed test gaps (P3). Q1–Q3 are P3 design notes for after PR E.
Verdict: merge. F1 is a cheap in-slice hardening worth adding first if another round happens anyway.
…dpoints # Conflicts: # src/Cluckwork.Application/Features/Users/IAccessModule.cs # src/Cluckwork.Infrastructure/Identity/AccessModule.cs # tests/Cluckwork.Application.Tests/Architecture/Data/coupling-matrix.md
…he legacy upgrade Review of #1041 (Opus F1, F2). The lifetime test read Jwt:RefreshTokenDays from a host that never set it, so it proved only "30 days", which a cookie written with the default also satisfied. Both lifetime tests now run on a host with Jwt:RefreshTokenDays=7. The legacy-cookie upgrade, the fourth refresh-cookie write, had no lifetime assertion; it has one now. Refs #857
|
Pushed
The mutation table and the test counts in the body are updated. |
Based on
main. This PR was stacked on #1037 (PR B), which merged as75e2b1be.mainis merged in as62832479. That merge commit's tree is identical to C's previous head plusmain, becausemainand B's branch had identical trees.git diff origin/main...HEAD --statlists only C's six files.Nothing changes for users. Sign-in, refresh, logout, password change and step-up keep every status, body, cookie, token claim, security event, lockout count and SQL statement, which the trace below shows across 35 scenarios.
For maintainers, the auth endpoints now reach Access through one surface,
IAccessModule, instead of six Identity services andIOptions<JwtOptions>. Only the refresh-cookie lifetime leaves the JWT options; the signing keys stay in Identity.This is PR C of the Access contract (#857, design v2). It moves the authentication endpoints, so it needs a security review.
Refs #857
What changed
Commit 1,
f1cfd7a0: tests first, green before the move.AuthSessionContractTestspins two things the endpoints decide themselves rather than throughIIdentityProvider, which no suite pinned:ExpiresfollowsJwt:RefreshTokenDayson login, refresh and change-password;LoginFailedevent, with the same property keys as the unknown-user failure and noUserIdorEmail.Commit 2,
649eae0b: the move.IAccessModulegains ten session members:ResolveFarmCodeAsync,LoginAsync,IsFirstRunProvisionedAsync,RecordLoginFailure,RefreshAsync,RevokeRefreshTokenAsync,RecordLogoutAsync,ChangeOwnPasswordAsync,IssueStepUpGrantAsyncandRefreshTokenLifetimeDays.AccessModuleforwards each one to the service the endpoint called before:IIdentityProvider(from DI, so decorators stay in the path),IStepUpGrantService,FirstRunStatusService,AuthSecurityEventLogger,ChangeOwnPasswordHandlerandJwtOptions.RefreshTokenDays. Nothing is cached; every member is a single call.AuthEndpoints:Login,Refresh,Logout,ChangePasswordandStepUptakeIAccessModulein place of those services. The order of checks, every cookie write and every response is unchanged.IIdentityProvider.RefreshAsyncandRevokeRefreshTokenAsynctakeexpectedAccountIdlast and optional. The module takes it before the cancellation token and requires it, so every call site names its account explicitly. Each of the five call sites (two refreshes, three revokes) passes the same account it passed before, and the one legacy-only logout revoke that relied on the default now passesnullexplicitly.AuthEndpoints.LogoutandAuthEndpoints.StepUpgain adapter rows reaching Access. The matrix cell Platform → Access becomesA (47), regenerated.Commit 3,
c17e8436: review hardening (Opus F1, F2), test-only. The lifetime test readJwt:RefreshTokenDaysfrom a host that never set it, so it proved only "30 days", which a cookie written with the default also satisfies. Both lifetime tests now run on a host withJwt:RefreshTokenDays=7. The fourth refresh-cookie write, the legacy-cookie upgrade, had no lifetime assertion, and a new test adds one. No production code changed.Auth parity trace
I adapted #854's harness into a scratch test (not committed). For each scenario it records:
Set-Cookieline: name, decoded value length, expiry relative to now, path and flags;exprelative tonbf, the time and id claims masked, andsecurity_stampas a length;SecurityEventlog event with its property keys;AccessFailedCount, locked or not, andCredentialEpoch.The 35 scenarios:
A separate fact exhausts the login rate-limit partition. Three logins return 200 and the fourth 429; step-up and change-password then return 429 from the same partition, and refresh returns 200.
Two runs on the base (
87a2eddf, B's head withmainmerged) were identical. The normalised head trace is byte-identical to the base across all 35 scenarios and the limiter partition. That covers 96 SQL statements, the cookie writes in 13 scenarios, the tokens issued in 9, and 10LoginFailedevents. The base trace also matches the one taken before #1036 merged.Mutation table
Each mutant was applied by an exact single-match replacement, built, and run against the required set, then reverted. That set is 24 classes and 170 tests: v2's list, plus every other test class declared in the same files (
BootstrapRetryBoundaryTests,PerFarmRefreshCookieTests,RefreshGraceDisabledTestsandAuthCookieSecureTests).RefreshTokenLifetimeDaysreturns the configured days + 1AuthSessionContractTests.RefreshCookie_ExpiresAfterTheConfiguredLifetime_OnLoginRefreshAndPasswordChange(new)login: expected the refresh cookie to expire about 2026-11-02T16:37…, got 2026-11-03T16:37…RecordLoginFailuredoes nothingCS9113: Parameter 'securityEvents' is unreadsecurityEventsAuthSessionContractTests.UnknownFarmAndSuspendedFarm_EachLogOneLoginFailed_WithTheUnknownUserShape(new)Assert.Equal() Failure: Values differExpected: 3Actual: 1IsFirstRunProvisionedAsyncalways trueCS9113: Parameter 'firstRun' is unreadfirstRunFirstRunLoginNoticeTests.ReportsNoOwner_UntilTheDefaultAccountHasOne_ThenTheOrdinaryDenialReturnsExpected: "Auth.NoOwnerProvisioned"Actual: "Identity.InvalidCredentials"expectedAccountIdRefreshAccountBindingTests.MismatchIsRefusedAndRotatesNothing,PerFarmRefreshCookieTests.LegacyCookie_WithDifferentFarmSelector_IsRefusedAndRotatesNothingExpected: UnauthorizedActual: OKexpectedAccountIdPerFarmRefreshCookieTests×3, the cross-farm logout casesresponse must not set or clear 'cluckwork_rt'RecordLogoutAsyncdoes nothingStepUpAuthTests×3, the logout-revokes-grant casesExpected: ForbiddenActual: CreatedStepUpAuthTests,SecurityEventLoggingTestsand others400 (Bad Request)CredentialEpochRaceTestsbarrier tests hung on a login that never succeeds, and I stopped themResponse status code does not indicate success: 401 (Unauthorized)nullPerFarmRefreshCookieTests×2response must not set or clear 'cluckwork_rt'nullfor the selected farmRefreshAccountBindingTests.MismatchIsRefusedAndRotatesNothingExpected: UnauthorizedActual: OK30(Opus O1)AuthSessionContractTests.RefreshCookie_ExpiresAfterTheConfiguredLifetime_OnLoginRefreshAndPasswordChange(commit 3; survived before it)login: expected the refresh cookie to expire about 2026-10-10T20:06…, got 2026-11-02T20:06…AuthSessionContractTests.LegacyCookieUpgrade_WritesThePerFarmCookieForTheConfiguredLifetime(new in commit 3; survived 32 tests before it)legacy upgrade: expected the refresh cookie to expire about 2026-10-10T20:06…, got 2026-10-04T20:06…AuthSessionContractTestsC2 and C3 show that
TreatWarningsAsErrorsturns a dropped forwarding dependency into a build error. C2n and C3n keep the field read, so a test, not the compiler, has to catch them. C1 and C2n are caught only by the newAuthSessionContractTests. After commit 3, C1 (days + 1) was re-run against the 7-day host and is red in both lifetime tests.A correction from the first run. My first run filtered by v2's class names and missed the three extra classes. C5 survived that run. With the full set it is red three times, so it was my filter's gap, not a test gap. The counts above are from the full run.
Timing
This is evidence, not a gate. The scratch harness timed six login branches in 60 interleaved rounds. I ran it three times on the base and three on the head, alternating, and dropped the first five rounds of each run as warm-up. That leaves 165 samples per branch per side, in milliseconds:
What it shows. The shape is the same on both sides. The four branches that reach the credential check cost one password hash each and sit within about 10% of one another. The two farm branches hash nothing and return in about 2 ms, as they did before.
What it does not show. The head's lower medians are not a speed-up. Other agents' test runs pushed the host's load average between 9 and 23 during these runs, and the p90s overlap.
Test counts
Measured on this machine.
87a2eddf(B withmainmerged)649eae0bCluckwork.Application.Tests(full, including every ledger, adapter-reach, seam, peer-contract and coupling-matrix guard)mainmerge and commit 3 (headc17e8436), with #1039's query-shape check on: every class that calls an auth, users or me route or namesIAccessModule, plus the required set andQueryShapeWarningTests(65 classes, found by walking the test files)Cluckwork.Application.Testsatc17e8436The required classes pass unedited. No existing test file changed. The full integration suite was not run locally; CI runs it once this PR targets
main. While it is stacked on B, CI may not run at all.Test edits
None. No existing test file changed (
git diff --numstat 87a2eddf...HEAD -- tests/lists only the newAuthSessionContractTests.csand the two ledger data files).Deslop record
/deslopand a by-hand/no-commentspass (subagents are off for this slice):srcdiff only forwards calls. It adds no logic, no caching and notry/catch; each module member is one call to the service the endpoint used before.IAccessModulethat repeated the interface header, and rewrapped the header.AccessModulelives in Infrastructure;IsFirstRunProvisionedAsyncasks about;RecordLoginFailureis for failures decided before any credential check;Change map
Review notes: properties to try to refute
LoginFailed, with the unknown-user shape and noUserIdorEmail.expectedAccountIdas before. One farm's logout never revokes or clears another farm's session held in the shared legacy cookie.AccessModuletakesIIdentityProviderfrom DI, so a registered decorator stays in the path.MustChangePasswordGateTestsreplaces it withChangePasswordThrowingIdentityProviderand still passes.CredentialEpochMiddleware, its fresh read per request, and the middleware order are untouched.Jwt:RefreshTokenDayson all four writes: login, refresh, the legacy upgrade and change-password. Since commit 3, a test pins each of the four against a non-default lifetime.IAccessModulenow constructs the session services as well, so the users and me routes build a few more scoped objects per request. None of them touches the database on construction. The timing section measures login.Not in this PR:
IIdentityProviderandIStepUpGrantServiceas Platform ports (PR E, sub-step 6).