Repository navigation
fix: OAuth2 client secrets stored as Argon2id hashes, plain ones upgraded on first auth (0.6.0-rc158) - #318
Merged
Merged
Conversation
|
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 16 hours by commenting |
Reviewer's GuideOAuth2 client registration now persists only Argon2id client-secret hashes, while all supported client-authentication paths verify hashes and transparently migrate legacy plaintext records on successful authentication without changing TTL or exposing secrets. New integration tests cover registration, rejection, migration, and TTL behavior, alongside the 0.6.0-rc158 metadata and documentation updates. Sequence diagram for OAuth2 client-secret registrationsequenceDiagram
participant Client
participant Register as POST /oauth/register
participant Password as utils::password
participant Storage as AuthStorage
Client->>Register: register_client()
Register->>Password: hash_password_argon2id()
Password-->>Register: client_secret_hash
Register->>Storage: store_kv(oauth2_client:{id})
Register-->>Client: client_id and client_secret once
Sequence diagram for legacy OAuth2 client-secret migrationsequenceDiagram
participant Client
participant Endpoint as OAuth2 authentication path
participant Storage as AuthStorage
participant Password as utils::password
Client->>Endpoint: present client secret
Endpoint->>Storage: load oauth2_client:{id}
Storage-->>Endpoint: legacy client_secret record
Endpoint->>Endpoint: constant_time_string_compare()
alt secret matches
Endpoint->>Password: hash_password_argon2id()
Password-->>Endpoint: client_secret_hash
Endpoint->>Storage: get_kv_ttl()
Endpoint->>Storage: store_kv(hash and remove plaintext)
Endpoint-->>Client: authentication succeeds
else secret is wrong
Endpoint-->>Client: authentication fails
end
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
ciresnave-bot
force-pushed
the
fix/client-secret-hash
branch
from
October 26, 2026 20:25
59503f0 to
67f2275
Compare
This was referenced Oct 26, 2026
…upgraded on first authentication (#90) (0.6.0-rc158) Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YEhSGGFctL3d4hGevi7tC
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013YEhSGGFctL3d4hGevi7tC
ciresnave-bot
force-pushed
the
fix/client-secret-hash
branch
from
October 26, 2026 21:43
11710d9 to
c69c182
Compare
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.
Part of #90 (step A of the design the PM agreed).
POST /oauth/registerstored the client secret in plain text underoauth2_client:{id}, and the token, refresh, introspection and authorize paths compared against it. Now the record holds onlyclient_secret_hash(Argon2id viautils::password, run on the blocking pool), the secret is still returned once, and a record that still holds a plainclient_secretkeeps authenticating and is rewritten with the hash on its first successful authentication (hash and plaintext removal are one write; the record keeps its time to live; a wrong secret changes nothing; a failed upgrade is logged without the secret and does not fail the authentication). New modulesrc/api/client_secret.rs.Red first, in Docker (rust:latest,
--features api-server,axum-integration, jobs=2): with only the new tests added, both fail on the old code (registration stores plaintext; a plain record is not upgraded). With the change both pass. Regression, same run, no name filter:client_bound_token_tests7,introspection_status_tests4,oauth2_code_grant_client_auth_tests10,oauth2_integration_test13,oauth2_refresh_client_binding_tests10,oauth2_scope_and_refresh_tests8,oauth_advanced_tests19, plus the 2 new: all pass (73 tests) (these seed plain-secret records, so they cover the legacy path).cargo fmt --all -- --checkclean. Not run: the rest of the suite, clippy.Behaviour kept as is on purpose: a record with no secret still accepts an empty secret at
/oauth/introspect; that is its own fix and advisory (rc159). The other two registries in #90 are not touched here (steps B and C). Argon2id verification is deliberately costly (46 MiB of memory per verification, parameters fromutils::password); I did not measure its time here. Introspection verifies the client secret on every call, so a deployment with heavy introspection traffic will pay that cost. Version 0.6.0-rc158 (PM-allocated); backport to 0.5.x (rc69) follows.🤖 Generated with Claude Code
https://claude.ai/code/session_013YEhSGGFctL3d4hGevi7tC
Summary by Sourcery
Protect OAuth2 client secrets at rest with Argon2id hashing and transparently migrate legacy plaintext secrets on successful authentication.
New Features:
Bug Fixes:
Enhancements:
Build:
Documentation:
Tests: