Add uselesskey-axum crate with JWKS/OIDC routers and mock JWT middleware#337
Conversation
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 22 minutes and 46 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (8)
WalkthroughAdds a new workspace crate Changes
Sequence DiagramsequenceDiagram
participant Client
participant Middleware as mock_jwt_verifier_layer
participant Validator as JWT Validator
participant JWKS as DeterministicJwksPhase
participant App as Axum Router/Handler
Client->>Middleware: HTTP request (Authorization: Bearer <token>)
Middleware->>Validator: Parse header, decode JWT
Validator->>JWKS: Resolve key by kid, verify RS256 signature
alt signature fails or kid unknown
Middleware->>Client: 401 Unauthorized
else signature ok
Validator->>Validator: Validate iss, aud, exp claims
alt claims invalid or expired
Middleware->>Client: 401 Unauthorized
else claims valid
Validator->>Middleware: Create TestAuthContext
Middleware->>App: Insert TestAuthContext into request extensions
App->>App: Handler extracts TestAuthContext
App->>Client: 200 OK + response
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f43c4eb9fe
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
crates/uselesskey-axum/src/lib.rs
Outdated
| let mut validation = Validation::new(Algorithm::RS256); | ||
| validation.set_issuer(&[expected.issuer.clone()]); | ||
| validation.set_audience(&[expected.audience.clone()]); |
There was a problem hiding this comment.
iss and aud claims during JWT verification
Validation::new(Algorithm::RS256) only requires exp by default, and set_issuer/set_audience validate those claims only when they are present. As written, a token with a valid signature and exp but no iss/aud is accepted, which lets malformed tokens pass this middleware and can hide auth regressions in tests that rely on issuer/audience enforcement.
Useful? React with 👍 / 👎.
| .get("aud") | ||
| .and_then(Value::as_str) | ||
| .unwrap_or_default() |
There was a problem hiding this comment.
aud claims when building auth context
JWT aud is allowed to be an array, and jsonwebtoken validation supports that form, but this extraction path only uses Value::as_str(). When a valid token contains aud as an array, verification succeeds but TestAuthContext.aud is populated as an empty string, which can produce incorrect downstream assertions in handlers/tests.
Useful? React with 👍 / 👎.
EffortlessSteven
force-pushed
the
codex/add-auth-test-helpers-for-rust-frameworks-8owqyk
branch
from
f43c4eb to
741c566
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@crates/uselesskey-axum/src/lib.rs`:
- Around line 317-322: The code currently synthesizes "unknown" when
token.claims.get("sub") is missing; instead either reject the token at
authentication time or make TestAuthContext.sub optional. Replace the
unwrap_or("unknown").to_owned() usage on token.claims.get("sub") with explicit
handling: return an authentication error (e.g., Err(AuthError::MissingSubject)
or similar) from the auth function when no "sub" is present, or change the
TestAuthContext.sub field to Option<String> and propagate None through the
context and callers; update any callers of TestAuthContext or the auth function
to handle the new error or optional sub accordingly.
In `@crates/uselesskey-axum/tests/integration.rs`:
- Around line 18-36: auth_state() and signer_fixture() produce different
deterministic keypairs so the negative-path tokens in
verifier_rejects_wrong_audience_and_expired_tokens() are signed with the wrong
key and fail signature verification before audience/expiry checks; fix by
creating the negative-case tokens using the same MockJwtVerifierState that
auth_state() returns (use MockJwtVerifierState::issue_token or
state.issue_token(...)) instead of signing with signer_fixture() so signature
verification passes and the test actually exercises audience and expiry
branches.
- Around line 52-53: Replace the #[tokio::test] attribute on the async
integration test function jwks_and_oidc_routes_round_trip with a plain #[test],
create a Tokio runtime via tokio::runtime::Runtime::new().unwrap(), and run the
existing async body inside runtime.block_on(async { ... }); do the same change
for the other integration test functions that currently use #[tokio::test] so
all integration tests use #[test] with an explicit runtime instead of the
attribute.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 99dc7644-1d4c-43e6-8334-e6aab963dff1
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockfuzz/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (5)
Cargo.tomlcrates/uselesskey-axum/Cargo.tomlcrates/uselesskey-axum/README.mdcrates/uselesskey-axum/src/lib.rscrates/uselesskey-axum/tests/integration.rs
| let sub = token | ||
| .claims | ||
| .get("sub") | ||
| .and_then(Value::as_str) | ||
| .unwrap_or("unknown") | ||
| .to_owned(); |
There was a problem hiding this comment.
Don’t synthesize "unknown" for a missing sub.
A token with valid signature/issuer/audience/expiry but no subject currently becomes an authenticated request with sub = "unknown". That hides malformed fixtures and can make subject-dependent handler tests pass for the wrong reason. Reject the token here, or make TestAuthContext.sub optional instead of inventing an identity.
Proposed fix
- let sub = token
- .claims
- .get("sub")
- .and_then(Value::as_str)
- .unwrap_or("unknown")
- .to_owned();
+ let Some(sub) = token.claims.get("sub").and_then(Value::as_str) else {
+ return (StatusCode::UNAUTHORIZED, "missing sub claim").into_response();
+ };
+ let sub = sub.to_owned();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let sub = token | |
| .claims | |
| .get("sub") | |
| .and_then(Value::as_str) | |
| .unwrap_or("unknown") | |
| .to_owned(); | |
| let Some(sub) = token.claims.get("sub").and_then(Value::as_str) else { | |
| return (StatusCode::UNAUTHORIZED, "missing sub claim").into_response(); | |
| }; | |
| let sub = sub.to_owned(); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/uselesskey-axum/src/lib.rs` around lines 317 - 322, The code currently
synthesizes "unknown" when token.claims.get("sub") is missing; instead either
reject the token at authentication time or make TestAuthContext.sub optional.
Replace the unwrap_or("unknown").to_owned() usage on token.claims.get("sub")
with explicit handling: return an authentication error (e.g.,
Err(AuthError::MissingSubject) or similar) from the auth function when no "sub"
is present, or change the TestAuthContext.sub field to Option<String> and
propagate None through the context and callers; update any callers of
TestAuthContext or the auth function to handle the new error or optional sub
accordingly.
| fn auth_state() -> MockJwtVerifierState { | ||
| let seed = Seed::from_env_value("uselesskey-axum-integration-v1").expect("seed"); | ||
| let phase = DeterministicJwksPhase::new( | ||
| seed, | ||
| "auth-suite", | ||
| RotationPhase::Primary, | ||
| "https://issuer.example.test", | ||
| "api://example-aud", | ||
| ); | ||
| MockJwtVerifierState::new(phase) | ||
| } | ||
|
|
||
| fn signer_fixture() -> (uselesskey_rsa::RsaKeyPair, String, String) { | ||
| let fx = | ||
| Factory::deterministic(Seed::from_env_value("uselesskey-axum-signer-v1").expect("seed")); | ||
| let key = fx.rsa("auth-suite-signer", RsaSpec::rs256()); | ||
| let issuer = "https://issuer.example.test".to_string(); | ||
| let audience = "api://example-aud".to_string(); | ||
| (key, issuer, audience) |
There was a problem hiding this comment.
Negative-path tokens are signed with the wrong key.
auth_state() and signer_fixture() derive different deterministic keypairs, so both requests in verifier_rejects_wrong_audience_and_expired_tokens() fail during signature verification before the audience/expiry checks run. That means this test can pass without ever exercising the branches it claims to cover. Build those tokens with state.issue_token(...) instead.
Proposed fix
- fn signer_fixture() -> (uselesskey_rsa::RsaKeyPair, String, String) {
- let fx =
- Factory::deterministic(Seed::from_env_value("uselesskey-axum-signer-v1").expect("seed"));
- let key = fx.rsa("auth-suite-signer", RsaSpec::rs256());
- let issuer = "https://issuer.example.test".to_string();
- let audience = "api://example-aud".to_string();
- (key, issuer, audience)
- }
-
- fn signed_token(key: &uselesskey_rsa::RsaKeyPair, claims: serde_json::Value, kid: &str) -> String {
- let mut header = Header::new(Algorithm::RS256);
- header.kid = Some(kid.to_owned());
-
- encode(
- &header,
- &claims,
- &EncodingKey::from_rsa_pem(key.private_key_pkcs8_pem().as_bytes())
- .expect("valid private key PEM"),
- )
- .expect("token encoding should succeed")
- }
-
#[tokio::test]
async fn verifier_rejects_wrong_audience_and_expired_tokens() {
let state = auth_state();
- let (key, issuer, audience) = signer_fixture();
+ let issuer = "https://issuer.example.test";
+ let audience = "api://example-aud";
let app = mock_jwt_verifier_layer(
Router::new().route("/me", get(|| async { StatusCode::OK })),
state.clone(),
);
- let wrong_aud = signed_token(
- &key,
+ let wrong_aud = state.issue_token(
json!({
"sub": "alice",
"iss": issuer,
"aud": "api://wrong-aud",
"exp": 4_102_444_800i64,
}),
- &state.expectations().kid,
+ 300,
);
@@
- let expired = signed_token(
- &key,
+ let expired = state.issue_token(
json!({
"sub": "alice",
"iss": issuer,
"aud": audience,
"exp": now.saturating_sub(5),
}),
- &state.expectations().kid,
+ 300,
);Also applies to: 143-186
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/uselesskey-axum/tests/integration.rs` around lines 18 - 36,
auth_state() and signer_fixture() produce different deterministic keypairs so
the negative-path tokens in verifier_rejects_wrong_audience_and_expired_tokens()
are signed with the wrong key and fail signature verification before
audience/expiry checks; fix by creating the negative-case tokens using the same
MockJwtVerifierState that auth_state() returns (use
MockJwtVerifierState::issue_token or state.issue_token(...)) instead of signing
with signer_fixture() so signature verification passes and the test actually
exercises audience and expiry branches.
| #[tokio::test] | ||
| async fn jwks_and_oidc_routes_round_trip() { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's examine the integration test file
cd crates/uselesskey-axum && wc -l tests/integration.rsRepository: EffortlessMetrics/uselesskey
Length of output: 96
🏁 Script executed:
# Read the relevant sections of the integration test file
head -n 210 crates/uselesskey-axum/tests/integration.rs | tail -n +40Repository: EffortlessMetrics/uselesskey
Length of output: 5096
🏁 Script executed:
# Look for other test patterns in the codebase to understand convention
find . -name "*.rs" -path "*/tests/*" -type f | head -20Repository: EffortlessMetrics/uselesskey
Length of output: 1131
🏁 Script executed:
# Check what test attributes are actually used across the project
rg "#\[(?:tokio::)?test\]" --type rs | head -30Repository: EffortlessMetrics/uselesskey
Length of output: 98
🏁 Script executed:
# Search for test attributes across the codebase
rg "#\[(?:tokio::)?test\]" --type rust | head -40Repository: EffortlessMetrics/uselesskey
Length of output: 1375
🏁 Script executed:
# Let's check a few test files to see what patterns they use
cat crates/uselesskey-core-base62/tests/integration.rs | head -50Repository: EffortlessMetrics/uselesskey
Length of output: 1476
🏁 Script executed:
# Check if there are examples of explicit runtime wrapping vs #[tokio::test]
rg "fn test" --type rust crates/ | head -20Repository: EffortlessMetrics/uselesskey
Length of output: 1701
🏁 Script executed:
# Look at the coding guidelines section more carefully - check if there's config
cat rustfmt.toml && cat clippy.tomlRepository: EffortlessMetrics/uselesskey
Length of output: 216
🏁 Script executed:
# Let's check the crates/uselesskey-axum/tests/integration.rs more carefully to see all test attributes
rg "#\[.*test.*\]" crates/uselesskey-axum/tests/integration.rs -A 1 | head -40Repository: EffortlessMetrics/uselesskey
Length of output: 386
🏁 Script executed:
# Check if any crate has async tests with #[test] + explicit runtime
rg -B 1 "async fn.*test" crates/ --type rust | head -40Repository: EffortlessMetrics/uselesskey
Length of output: 54
🏁 Script executed:
# Let's see if there are any examples of #[tokio::test] vs #[test] with runtime in other crates
find crates -name "*.rs" -path "*/tests/*" -type f -exec grep -l "tokio::test\|#\[test\].*async" {} \; | head -10Repository: EffortlessMetrics/uselesskey
Length of output: 115
🏁 Script executed:
# Check what the actual test runner configuration is
find . -name "Cargo.toml" | head -5 | xargs grep -A 10 "\[dev-dependencies\]" | head -50Repository: EffortlessMetrics/uselesskey
Length of output: 2614
🏁 Script executed:
# Let's check the entire codebase for #[tokio::test] usage
rg "#\[tokio::test\]" --type rust crates/Repository: EffortlessMetrics/uselesskey
Length of output: 572
🏁 Script executed:
# Check crates/uselesskey-axum/Cargo.toml to see tokio dependency details
cat crates/uselesskey-axum/Cargo.toml | grep -A 20 "^\[dependencies\]\|^\[dev-dependencies\]"Repository: EffortlessMetrics/uselesskey
Length of output: 526
🏁 Script executed:
# Look for examples of how async is handled with #[test] in the repo
rg "#\[test\].*async|async.*fn.*test" --type rust crates/ -B 1 -A 3 | head -50Repository: EffortlessMetrics/uselesskey
Length of output: 54
🏁 Script executed:
# Let's check what the actual patterns are in the repo - check one of the test files more deeply
cat crates/uselesskey-core-base62/tests/integration.rsRepository: EffortlessMetrics/uselesskey
Length of output: 2454
🏁 Script executed:
# Check if there's any documentation about testing conventions
find . -name "*.md" -type f | xargs grep -l "test\|#\[test\]" 2>/dev/null | head -5Repository: EffortlessMetrics/uselesskey
Length of output: 333
🏁 Script executed:
# Let's check if tokio::test is mentioned in any contributing or guidelines
rg "tokio::test" --type rust --type mdRepository: EffortlessMetrics/uselesskey
Length of output: 572
Use #[test] with explicit runtime instead of #[tokio::test] in integration tests.
The coding guidelines specify #[test] for all integration tests. Replace #[tokio::test] with #[test] and wrap async bodies with an explicit tokio runtime (e.g., tokio::runtime::Runtime::new().unwrap().block_on(async { ... })).
Also applies to: lines 97-98, 140-141, 200-201
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@crates/uselesskey-axum/tests/integration.rs` around lines 52 - 53, Replace
the #[tokio::test] attribute on the async integration test function
jwks_and_oidc_routes_round_trip with a plain #[test], create a Tokio runtime via
tokio::runtime::Runtime::new().unwrap(), and run the existing async body inside
runtime.block_on(async { ... }); do the same change for the other integration
test functions that currently use #[tokio::test] so all integration tests use
#[test] with an explicit runtime instead of the attribute.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/metadata/workspace-docs.json`:
- Around line 45-48: The adapter metadata now lists "uselesskey-axum" in
adapter_crates but adapter_feature_matrix lacks a corresponding row; update the
adapter_feature_matrix JSON to include a "uselesskey-axum" entry (with
appropriate capability booleans or explicit "N/A" values) so the generated docs
remain consistent, referencing the adapter_crates name "uselesskey-axum" and the
adapter_feature_matrix object to locate where to add the row.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 243c6b1e-dc33-44bb-83dd-8e114e290916
📒 Files selected for processing (3)
README.mddocs/metadata/workspace-docs.jsondocs/reference/support-matrix.md
EffortlessSteven
force-pushed
the
codex/add-auth-test-helpers-for-rust-frameworks-8owqyk
branch
2 times, most recently
from
d5d647e to
43194ae
Compare
EffortlessSteven
force-pushed
the
codex/add-auth-test-helpers-for-rust-frameworks-8owqyk
branch
from
43194ae to
9238e6c
Compare
1 participant
Motivation
axumbuilt on existing deterministicuselesskeyfixtures so tests can easily exercise JWKS/OIDC and bearer-token flows.Description
crates/uselesskey-axumcontainingsrc/lib.rsandREADME.mdthat implementsjwks_router(),oidc_router(),mock_jwt_verifier_layer()(attachable middleware),inject_auth_context_layer(),DeterministicJwksPhase,RotationPhase,AuthExpectations, and the typed extractorTestAuthContext.Factory::deterministic(Seed)fixtures to produce RSA signing keys and JWKS payloads and provideissue_token()helpers that mint RS256 tokens for tests.Cargo.toml, enable thejwkfeature foruselesskey-rsain the new crate, and update lockfiles to includeaxumand related deps.Testing
cargo check -p uselesskey-axumwhich completed successfully.cargo test -p uselesskey-axumand all unit tests passed (5 tests covering router responses, rotation-phase kid differences, wrong-audience rejection, expired-token rejection, and auth-context injection).axum).Codex Task