feat(auth): emit a PKCE S256 code challenge on the OAuth2 authorization URI - #820
Open
AmaadMartin wants to merge 3 commits into
Open
feat(auth): emit a PKCE S256 code challenge on the OAuth2 authorization URI#820AmaadMartin wants to merge 3 commits into
AmaadMartin wants to merge 3 commits into
Conversation
added 3 commits
August 25, 2026 12:00
adk-js already sent code_verifier at token exchange, but the authorization request carried no code_challenge, so the provider had nothing to bind the verifier to. generateAuthUri now derives the S256 challenge and forwards audience and nonce, matching adk-python's AuthHandler.generate_auth_uri. The SHA-256 must be synchronous because generateAuthUri is reached from the synchronous public requestCredential API, so it uses node:crypto rather than the async crypto.subtle.digest. The browser shim throws instead of degrading.
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.
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
N/A
2. Or, if no issue exists, describe the change:
Problem:
adk-jsimplements only half of PKCE (RFC 7636). The token exchange alreadysends
code_verifier(core/src/auth/oauth2/oauth2_utils.ts), butAuthHandler.generateAuthUri()puts nocode_challengeon the authorizationURI. A provider therefore has nothing to bind the verifier to, and the
code_verifiersent at exchange time is a value the provider cannot check.The credential's
audienceandnonceare dropped from the URI as well.adk-python'sAuthHandler.generate_auth_uriemits all three.Solution:
generateAuthUri()now derives the S256 challenge from the credential's codeverifier, generates a verifier with
node:crypto's CSPRNG when the callersupplies none, and returns that verifier on the exchanged credential so the
later token exchange can send it. It rejects any code challenge method other
than
S256, and forwardsaudienceandnoncewhen set. The two PKCE helperslive in
auth/oauth2/oauth2_utils.ts, next to thecode_verifierhalf of theflow.
Behaviour is unchanged unless the credential sets
codeChallengeMethod: acredential that requests no method produces the same authorization URI as
before. Only the challenge and the method reach the URI; the verifier itself is
never placed on it and is never logged.
Notes for the reviewer:
generateAuthUri()is reached fromthe synchronous public
CallbackContext.requestCredential(), socrypto.subtle.digestis unusable. It usesnode:crypto, which the webbundle aliases to
core/src/utils/crypto_shim.ts. The two new shim stubsthrow, following the file's existing
randomUUIDcontract, so PKCE isunavailable in the bundled web build. That is a documented limitation rather
than a regression — PKCE works in no build today — and the alternative is an
async public API. The shim throws rather than emit an authorization request
with a missing or forged challenge.
npm run build:bundlesucceeds and thebundle contains no
node:cryptoreference.(
code_challenge,code_challenge_method,S256,audience,nonce). Theerror message uses the JS field name
codeChallengeMethodand throwsError, like every other guard in the file.behaviour, which Python relies on.
unreserved characters, inside the 43–128 range RFC 7636 requires, with no
modulo bias.
any, no new dependency.Testing Plan
Unit Tests:
17 new cases in
core/test/auth/auth_handler_test.ts, 7 incore/test/auth/oauth2/oauth2_utils_test.ts, and a no-mocks round trip incore/test/auth/pkce_round_trip_test.ts. The four existinggenerateAuthUritests are unmodified; no existing test was weakened or removed.
New source lines are at 100% line and branch coverage.
Proof the tests can fail. Each mutation was applied to the source and
reverted:
searchParams.set('code_challenge', ...)expected null to be 'HocX9xaevoNbL5le-QirVOjee…'expected '6w2bQx644JZ1dFssOF_3J3Wu0Uor…' to match /^[A-Za-z0-9_-]{43}$/nonceforwardingexpected null to be 'n-0S6'expected [Function] to throw an erroraudienceforwardingexpected null to be 'https://api.example.com'digest('base64')instead ofdigest('base64url')expected '…OD9+DWVAoUkuWkK0CITuX…' to match /^[A-Za-z0-9_-]{43}$/code_challenge(round trip only)expected null to be 'HgM2b6EgtYh0tVg2vOZhBrKbi…'expected an auth URI and a code verifierManual End-to-End (E2E) Tests:
A live provider is not needed.
core/test/auth/pkce_round_trip_test.tsruns thereal flow with no mocks: it builds the authorization URI through
AuthHandler.generateAuthRequest(), feeds the returned verifier intocreateOAuth2TokenRequestBody(), and performs the check the provider performs —that the URI's
code_challengeis the SHA-256 of the body'scode_verifier.To verify by hand: build an OAuth2 credential with
codeChallengeMethod: 'S256',call
generateAuthUri(), and confirm against any RFC 7636 checker thatbase64url(sha256(codeVerifier))equals the URI'scode_challenge.Checklist
Additional context
PKCE stays opt-in: it activates only for credentials that set
codeChallengeMethod: 'S256'. Callers that want the protection have to requestit; nothing in this change turns it on for an existing credential.