Repository navigation
feat!: phase 6c auth (AUTH-1..AUTH-38) - #77
Merged
Merged
Conversation
…H-12, AUTH-13, AUTH-23, AUTH-25)
Contributor
Author
Review recordVerdict: approve. Two low-severity findings were verified, neither blocks merge, and both are fixed in 18471ea. The review covered 12 commits on
Verified findings
Discarded candidates
|
This was referenced Oct 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Phase 6c, authentication, built to the design and the plan. User page:
docs/sdk-documentation/auth.md.AUTH-1..AUTH-7):AuthScheme,AuthRequirement,AuthDescriptor,NoAuth, a stateless resolver over three tiers (per call, operation, client) withRequestOptions.Auth/OperationAuth, andAuthResolutionExceptionnaming what was required and what was available.AUTH-8..AUTH-10,AUTH-14,AUTH-26):AccessToken(nullable expiry, value equality, redactedToString), and validatedApiKeyCredential,BasicCredentialandTokenRequestContext. The permanentCredentialRedactionTests(Security) covers every credential type.AUTH-12,AUTH-13,AUTH-23,AUTH-25): a total, linearAuthenticationChallenge.Parse(46 vectors plus fuzz),BasicChallengeHandlerandCompositeChallengeHandler.401lifecycle inAuthorizationPolicy(AUTH-27..AUTH-33,AUTH-38):OnChallengeAsync/OnChallengehooks, one replay when the replacement is same-origin and replayable, the typedHttpsRequiredExceptionguard (which also covers a reactive scheme's outbound pass), origin comparison through 6b'sHttpOrigin, and a real sync path with no sync-over-async bridge.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 160dexpace.auth.token_refresh_failedon failure),TokenProviderException, eviction on a rejected token, and a bound of 1024 keys.BearerTokenAuthPolicyre-acquires once on aBearerchallenge.AUTH-15..AUTH-22,AUTH-24):DigestChallengeHandlerover SHA-256, SHA-256-sess, MD5 and MD5-sess (MD5 is declined when the platform lacks it), a bounded nonce counter, the cnonce fromRandomNumberGenerator,username*, the RFC vectors, and a SystemNet wire test.ChallengeAuthPolicyandMultiSchemeAuthPolicy, 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-11andREDIR-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-structureis OK; the housekeeping probe found no drift.Breaking changes
The full list is in
CHANGELOG.md[Unreleased]. In short:RequestOptionsequality now includesAuth/OperationAuth.AccessToken.ExpiresOnisDateTimeOffset?. Equality is by value (token, expiry, refresh), and a blank token is rejected.ApiKeyCredentialandBasicCredentialvalidate at construction. Basic rejects an empty username or password and a:in the username.TokenRequestContext.Scopesis now a copy.AuthorizationPolicy's protected surface is reshaped: a descriptor-taking constructor,WithheldHeaderNames, abstractGetCredential{Async}(requirement, request)that may returnnull, and the new challenge hooks.HttpsRequiredException, which still derives fromSdkException.401that carriesWWW-Authenticate.AccessTokenCacherefreshes 30 s early, in the background, logs a failed refresh, rejects a default or already-expired provider token, and is bounded.BearerTokenAuthPolicyreplays once on aBearerchallenge, and its sync path no longer blocks on the async one.Open rulings (taken as designed; the lead has not ruled)
RequestOptions, and every auth policy resolves.HttpsRequiredException) and covers a reactive scheme's outbound pass.BasicCredentialrejects:in the username, which is stricter than both siblings.AccessTokenequality includesRefreshOn.BackgroundWorkwithSuppressFlow. Its cancellation behaviour is the open part.-sesschallenge without qop is declined (new design §11 item).username*, and an unechoable challenge value is declined.MultiSchemeAuthPolicyis added. It rejects an unservable client descriptor at construction (a pinned reading).Closes #74. Closes #7. Closes #2.