Skip to content

feat!: phase 6c auth (AUTH-1..AUTH-38) - #77

Merged
Wahbeh-Mohammad merged 13 commits into
mainfrom
74-phase-6c-auth
Oct 8, 2026
Merged

Wahbeh-Mohammad merged 13 commits into
mainfrom
74-phase-6c-auth

Conversation

@Wahbeh-Mohammad

Copy link
Copy Markdown
Contributor

Summary

Phase 6c, authentication, built to the design and the plan. User page: docs/sdk-documentation/auth.md.

  • Descriptor and resolver (AUTH-1..AUTH-7): AuthScheme, AuthRequirement, AuthDescriptor, NoAuth, a stateless resolver over three tiers (per call, operation, client) with RequestOptions.Auth / OperationAuth, and AuthResolutionException naming what was required and what was available.
  • Credentials (AUTH-8..AUTH-10, AUTH-14, AUTH-26): AccessToken (nullable expiry, value equality, redacted ToString), and validated ApiKeyCredential, BasicCredential and TokenRequestContext. The permanent CredentialRedactionTests (Security) covers every credential type.
  • Challenges (AUTH-12, AUTH-13, AUTH-23, AUTH-25): a total, linear AuthenticationChallenge.Parse (46 vectors plus fuzz), BasicChallengeHandler and CompositeChallengeHandler.
  • The 401 lifecycle in AuthorizationPolicy (AUTH-27..AUTH-33, AUTH-38): OnChallengeAsync / OnChallenge hooks, one replay when the replacement is same-origin and replayable, the typed HttpsRequiredException guard (which also covers a reactive scheme's outbound pass), origin comparison through 6b's HttpOrigin, and a real sync path with no sync-over-async bridge.
  • Token cache (AUTH-11, AUTH-34..AUTH-37): a 30 s refresh margin, a lock-free hot path, per-key coalescing, a background refresh while the token is still valid (event 160 dexpace.auth.token_refresh_failed on failure), TokenProviderException, eviction on a rejected token, and a bound of 1024 keys. BearerTokenAuthPolicy re-acquires once on a Bearer challenge.
  • Digest (AUTH-15..AUTH-22, AUTH-24): DigestChallengeHandler over SHA-256, SHA-256-sess, MD5 and MD5-sess (MD5 is declined when the platform lacks it), a bounded nonce counter, the cnonce from RandomNumberGenerator, username*, the RFC vectors, and a SystemNet wire test.
  • ChallengeAuthPolicy and MultiSchemeAuthPolicy, sharing the per-scheme stampers; a NativeAOT smoke check covers the auth surface.

Checklist census

38 rows, AUTH-1..AUTH-38 (36 MUST, 2 SHOULD). All 38 are ✅, with none 🚫, ⏳ or N/A. 6c also did the work for rows owned elsewhere: XCUT-12, XCUT-14, XCUT-16, XCUT-18, XCUT-19(d), XCUT-21, BODY-4, PIPE-5, PIPE-28, CTX-19, OBS-39, REDIR-7, REDIR-11 and REDIR-24. Each owner's checklist has a dated correction. See the checklist.

Local gate, run in the worktree on top of main (29a7754, with 6a and 6b merged): locked restore; Release build with 0 warnings; dotnet format --verify-no-changes; 4564 tests (4563 passed, 1 skipped as designed: the inert SSE-37); coverage 97.43% aggregate, which passes the 80% floor, and the gate self-test passes; dependency audit passes for all 3 libraries; reproducible pack is byte-identical; NativeAOT publish and run print "all checks passed"; the tools solution passes 438 of 438; scripts/knowledge verify-structure is OK; the housekeeping probe found no drift.

Breaking changes

The full list is in CHANGELOG.md [Unreleased]. In short:

  • RequestOptions equality now includes Auth / OperationAuth.
  • AccessToken.ExpiresOn is DateTimeOffset?. Equality is by value (token, expiry, refresh), and a blank token is rejected.
  • ApiKeyCredential and BasicCredential validate at construction. Basic rejects an empty username or password and a : in the username. TokenRequestContext.Scopes is now a copy.
  • AuthorizationPolicy's protected surface is reshaped: a descriptor-taking constructor, WithheldHeaderNames, abstract GetCredential{Async}(requirement, request) that may return null, and the new challenge hooks.
  • The HTTPS refusal is HttpsRequiredException, which still derives from SdkException.
  • Every auth policy honours the per-call and operation descriptors and answers a 401 that carries WWW-Authenticate.
  • AccessTokenCache refreshes 30 s early, in the background, logs a failed refresh, rejects a default or already-expired provider token, and is bounded.
  • BearerTokenAuthPolicy replays once on a Bearer challenge, and its sync path no longer blocks on the async one.

Open rulings (taken as designed; the lead has not ruled)

  • P6c-6 / P6c-7: the tiers ride on RequestOptions, and every auth policy resolves.
  • P6c-8: the guard is typed (HttpsRequiredException) and covers a reactive scheme's outbound pass.
  • P6c-17: BasicCredential rejects : in the username, which is stricter than both siblings.
  • P6c-21: AccessToken equality includes RefreshOn.
  • P6c-25: the background refresh runs under BackgroundWork with SuppressFlow. Its cancellation behaviour is the open part.
  • P6c-29: Digest prefers SHA-256 first.
  • P6c-31: a -sess challenge without qop is declined (new design §11 item).
  • P6c-33: a non-ASCII username goes out as username*, and an unechoable challenge value is declined.
  • P6c-36: MultiSchemeAuthPolicy is added. It rejects an unservable client descriptor at construction (a pinned reading).

Closes #74. Closes #7. Closes #2.

@Wahbeh-Mohammad

Copy link
Copy Markdown
Contributor Author

Review record

Verdict: approve. Two low-severity findings were verified, neither blocks merge, and both are fixed in 18471ea.

The review covered 12 commits on 74-phase-6c-auth, rebased onto main with 6a and 6b merged (merge base 29a7754).

  • Rows and rulings. All 38 AUTH rows are present and tested. Every ruling is built as designed, including the open ones: P6c-6/7, 8, 17, 21, 25, 29, 31, 33 and 36.
  • Public API. The PublicAPI.Unshipped.txt diff matches the design's removed and added blocks, with the dated R15 correction (one constructor, RefreshMargin as init).
  • RS0030. The base's bridge pragma is retired. The only new pragma is the reserved BackgroundWork site, and no BannedSymbols.txt entry was removed.
  • Security tests. AuthHttpsGuardTests is only strengthened: it asserts the exact type and the scheme. The new permanent CredentialRedactionTests covers every credential type. 6b's RedirectCredentialLeakTests passes unchanged.
  • Gates run by the reviewer. Release build with 0 errors, format clean, 4564 of 4564 tests passed, NativeAOT run prints "all checks passed", and the housekeeping probe found no drift.

Verified findings

ID Severity Location Issue Resolution
F1 low src/Dexpace.Sdk.Core/Auth/AuthRequirement.cs (Scopes init accessor and its XML doc; design §A / AUTH-2) The doc said ArgumentNullException is thrown when "the value, or an element of it, is null". A null element actually throws ArgumentException ("A scope must not be null."), and AuthDescriptorTests asserts that. Fixed in 18471ea. The doc now lists ArgumentNullException for a null value and ArgumentException for a null element. Behaviour is unchanged.
F2 low src/Dexpace.Sdk.Core/Internal/AuthOrigin.cs:16-20 (AuthOrigin.Of) After Same moved to 6b's IDN-folding HttpOrigin (bf5509e), the old string-form Of was dead production code: only AuthOriginTests called it. It was still documented as the canonical origin, so the class held two origin definitions that could disagree. Fixed in 18471ea. AuthOrigin.Of and its one test assertion are deleted, so Same (over HttpOrigin) is the only origin comparison.

Discarded candidates

  • A background-refresh failure adopted by an expired-zone foreground waiter. This is the design's P6c-24 coalescing. The request that triggered the refresh already returned its valid token, so AUTH-37 holds.
  • A race that overwrites RefreshTask in StartBackgroundRefresh. It affects only the internal PendingRefresh test hook. Every refresh catches its own failures.
  • The public AuthResolutionException constructor does not sort Available. The resolver, which is the only SDK producer, sorts before constructing, and the design fixes the order only for the resolver's message.
  • BearerStamper in MultiSchemeAuthPolicy falls back to empty scopes. The design's fallback is "the policy's scopes", and this policy has none. A requirement's own scopes are still used.
  • MultiSchemeAuthPolicy rejects an unservable client descriptor at construction. This is documented as a pinned reading under the open P6c-36 and contradicts no row.
  • ChallengeParser edge cases (a token68 with a trailing =, name= at the end of input, BWS around =, a malformed tail). Traced by hand: each matches AUTH-12 / AUTH-13. The vectors and the fuzz tests pass.
  • The Digest computation (HA1 for -sess, the legacy no-qop form, nc, the Latin-1 decline, username*, digest-uri). It matches AUTH-17/18/21/22 and P6c-31..35. The RFC vectors and the wire test pass.
  • Blocking on the sync path. AccessTokenCache.Get uses SemaphoreSlim.Wait(ct), and GetToken completes synchronously. No RS0030 pragma remains in the policies. TokenCredential's documented bridge is outside 6c's scope (plan R2).
  • Loosened security tests. None were loosened: the assertions are only strengthened, and no Security test was deleted.
  • Stale checklist citations. The only missing names are the five as-built tests the design says are replaced (recorded under "Existing assertions changed"), plus clock_gettime, which is prose.

@Wahbeh-Mohammad
Wahbeh-Mohammad merged commit 8dc8ee4 into main Oct 8, 2026
5 checks passed
@Wahbeh-Mohammad
Wahbeh-Mohammad deleted the 74-phase-6c-auth branch October 8, 2026 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant