fix: delete the PKCE request session only after a successful verification

[shilps1583]

HandleTokenEndpointRequest deletes the stored PKCE request session
immediately after fetching it, before the verifier has been checked
against the bound challenge. A failed verification (wrong verifier,
disallowed method, PKCE not enforced but a challenge existed, etc.)
still leaves the session deleted.

That session is what binds an authorization code to its
code_challenge. Once it's gone, the same code can be replayed at the
token endpoint with no code_verifier at all: GetPKCERequestSession
returns ErrNotFound, nv == 0, and validateNoPKCE lets the exchange
through as a non-PKCE request (unless EnforcePKCE is on). A failed
PKCE attempt against a code should not make a second, verifier-less
attempt against that same code succeed -- that's a downgrade from a
PKCE-protected exchange to an unprotected one.

Move the delete to the two points where the session's job is actually
done: a confirmed challenge/verifier match, and the non-PKCE path
where no challenge was ever bound and none is required. A verifier
presented against a session with no bound challenge is also consumed,
since there is no challenge there for a downgrade to strip. Every other
error path (validate() failures, verifier format failures, a
challenge/verifier mismatch) now leaves the session in place.

TestHandleTokenEndpointRequest_SessionLifecycle covers both outcomes;
two of its five cases fail against the prior behavior.

Summary by CodeRabbit

• Bug Fixes

  • Improved PKCE session lifecycle handling during token exchanges.
  • PKCE request sessions are now deleted only after appropriate verifier/challenge confirmation, and handled consistently for non-PKCE and error cases.
  • Prevented replay/downgrade scenarios where sessions could be removed or preserved incorrectly.

• Tests

  • Added end-to-end coverage for PKCE session consumption/preservation across successful verification, mismatched verifier, missing verifier, non-PKCE, and invalid-grant scenarios.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[coderabbitai]

📝 Walkthrough

Walkthrough

Changes

PKCE session lifecycle

Layer / File(s)	Summary
Session consumption flow handler/pkce/handler.go	Token endpoint handling defers PKCE session deletion, consumes sessions for valid non-PKCE exchanges and missing-challenge verifier submissions, and centralizes deletion error handling.
Session lifecycle validation handler/pkce/handler_test.go	Tests verify session consumption and preservation across successful, failed, incomplete, and non-enforced PKCE exchanges.

Estimated code review effort: 2 (Simple) | ~10 minutes

Suggested reviewers: aeneasr

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title clearly and concisely describes the main PKCE session-deletion fix.
Description check	✅ Passed	The description explains the bug, why it matters, the fix, and test coverage; only optional checklist items are unfilled.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}