Repository navigation
fix(oauth2): the authorization_code grant authenticates the client (0.5.0-rc69) - #323
Merged
Merged
Conversation
) (0.5.0-rc69) Backport of 04a4f48 from main. Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YEhSGGFctL3d4hGevi7tC
|
Sorry @ciresnave-bot, you've used your own review budget of 250,000 diff characters for the last 7 days. You can request another review in 2 days and 13 hours by commenting |
Reviewer's GuideBackports the authorization-code client-authentication fix to 0.5.x by authenticating registered clients before code access, enforcing confidential secrets and public-client PKCE correctly, preserving codes on failed validation or client mismatch, and adding focused regression tests for the security-sensitive flows. Sequence diagram for authorization-code client authenticationsequenceDiagram
participant Client
participant TokenEndpoint
participant ClientStore
participant CodeStore
Client->>TokenEndpoint: handle_authorization_code_grant()
TokenEndpoint->>ClientStore: authenticate_token_client()
ClientStore-->>TokenEndpoint: ClientKind or invalid_client
alt invalid_client
TokenEndpoint-->>Client: invalid_client
else authenticated client
TokenEndpoint->>CodeStore: Read authorization code
alt client_id mismatch
TokenEndpoint->>CodeStore: Release consumed marker
TokenEndpoint-->>Client: invalid_grant
else matching client_id
TokenEndpoint->>CodeStore: Delete authorization code
TokenEndpoint-->>Client: Token response
end
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Backport of main's 04a4f48 (#140) to release/0.5.x, which never had it. On this line
POST /oauth/tokenaccepted a code exchange for a confidential client as long as the request carried someclient_secret(onlyreq.client_secret.is_none()was checked), so anyone holding an authorization code and theclient_idcould redeem it. The client is now authenticated before the code is read; a failed authentication neither reveals nor consumes the code; a code issued to another client is refused and left intact; public clients are decided by their registration and must use PKCE.Re-applied by hand (the 0.5.x file lacks the helpers main already had:
load_client_record,refusal,client_secret_ofare added withClientKindandauthenticate_token_client). Compares the stored plain secret in constant time; the #90 hashing backport follows separately.Red first, in Docker (rust:latest,
--features api-server, jobs=2): with only the ported test file added, 5 of its 7 tests fail on the current 0.5.x code; with the fix all 7 pass. Regression, same feature set plus axum-integration:auth_flow_tests12,oauth2_integration_test11,oauth_advanced_tests19 pass.cargo fmt --all -- --checkclean. Not ported: the three tests of the atomic code/refresh claims (a separate change not on this line). Not run: the rest of the suite.Version 0.5.0-rc69 (PM-allocated); base is release/0.5.x, so the PM merges by hand. GHSA and RustSec follow once the fix is live.
🤖 Generated with Claude Code
https://claude.ai/code/session_013YEhSGGFctL3d4hGevi7tC
Summary by Sourcery
Secure authorization-code exchanges by authenticating the registered client before redeeming authorization codes.
Bug Fixes:
Build:
Documentation:
Tests: