oidc-authn-plugin: per-issuer typ/aud/clock-skew-leeway validation#3
Merged
Conversation
Signed-off-by: Andre Smith <andre.smith+oss@acronis.com>
Signed-off-by: Fluid <fluidcells@pm.me>
Signed-off-by: Andre Smith <andre.smith+oss@acronis.com>
chore: bump the Rust version to 1.96.0
…sdk-canonical-projection refactor(type-registry): adopt SDK canonical-projection pattern
…sdk-canonical-projection refactor(resource-group): adopt SDK canonical-projection pattern
|
Warning Review limit reached
More reviews will be available in 55 minutes and 42 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, 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 include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (121)
📝 WalkthroughWalkthroughThe OIDC AuthN plugin now supports per-issuer JWT validation overrides. Configuration is extended with expected audience patterns, required JOSE typ headers, and per-issuer clock-skew leeway. The JWT validator uses a new ChangesPer-Issuer JWT Validation Overrides
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
diffora
force-pushed
the
oidc-authn-per-issuer-validation
branch
from
June 13, 2026 19:01
2cfb500 to
38a39b1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
gears/system/authn-resolver/plugins/oidc-authn-plugin/src/test_support/test_fixtures.rs (1)
83-89: ⚡ Quick winFail fast in test token signing helpers instead of returning an empty token.
Line 88 and Line 101 currently swallow JWT encoding errors via
unwrap_or_default(), which can mask the true failure source in downstream validator tests. In test fixtures, panic-on-failure is usually better for diagnosis.Suggested patch
pub fn sign_jwt_with_typ(claims: &serde_json::Value, kid: Option<&str>, typ: &str) -> String { use jsonwebtoken::{Header, encode}; let mut header = Header::new(Algorithm::RS256); header.kid = kid.map(str::to_owned); header.typ = Some(typ.to_owned()); - encode(&header, claims, &key_material().encoding_key).unwrap_or_default() + encode(&header, claims, &key_material().encoding_key) + .unwrap_or_else(|error| panic!("test JWT encoding should succeed: {error}")) } @@ pub fn sign_jwt_without_typ(claims: &serde_json::Value, kid: Option<&str>) -> String { use jsonwebtoken::{Header, encode}; let mut header = Header::new(Algorithm::RS256); header.kid = kid.map(str::to_owned); header.typ = None; - encode(&header, claims, &key_material().encoding_key).unwrap_or_default() + encode(&header, claims, &key_material().encoding_key) + .unwrap_or_else(|error| panic!("test JWT encoding should succeed: {error}")) }Also applies to: 96-102
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@gears/system/authn-resolver/plugins/oidc-authn-plugin/src/test_support/test_fixtures.rs` around lines 83 - 89, The test JWT helpers currently swallow encoding errors by calling encode(...).unwrap_or_default(), which masks failures; in the sign_jwt_with_typ function (and any other test signing helpers in this file that call encode(...).unwrap_or_default()) replace unwrap_or_default() with a panic-on-failure such as expect("failed to encode test JWT") or unwrap() so test failures surface immediately, keeping header.kid and header.typ handling unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@gears/system/authn-resolver/plugins/oidc-authn-plugin/src/test_support/test_fixtures.rs`:
- Around line 83-89: The test JWT helpers currently swallow encoding errors by
calling encode(...).unwrap_or_default(), which masks failures; in the
sign_jwt_with_typ function (and any other test signing helpers in this file that
call encode(...).unwrap_or_default()) replace unwrap_or_default() with a
panic-on-failure such as expect("failed to encode test JWT") or unwrap() so test
failures surface immediately, keeping header.kid and header.typ handling
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c6b6350-6e3a-4be2-a3ce-b7730e93b345
📒 Files selected for processing (12)
gears/system/authn-resolver/plugins/oidc-authn-plugin/benches/jwt_validation.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/benches/s2s_exchange.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/src/config.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/error.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/error_tests.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/metrics.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/metrics/definitions.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/validator.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/validator_tests.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/src/infra/token_client_tests.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/src/test_support/test_fixtures.rsgears/system/authn-resolver/plugins/oidc-authn-plugin/tests/common/mod.rs
✅ Files skipped from review due to trivial changes (2)
- gears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/metrics/definitions.rs
- gears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/metrics.rs
🚧 Files skipped from review as they are similar to previous changes (9)
- gears/system/authn-resolver/plugins/oidc-authn-plugin/benches/s2s_exchange.rs
- gears/system/authn-resolver/plugins/oidc-authn-plugin/tests/common/mod.rs
- gears/system/authn-resolver/plugins/oidc-authn-plugin/benches/jwt_validation.rs
- gears/system/authn-resolver/plugins/oidc-authn-plugin/src/infra/token_client_tests.rs
- gears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/error.rs
- gears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/error_tests.rs
- gears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/validator.rs
- gears/system/authn-resolver/plugins/oidc-authn-plugin/src/domain/validator_tests.rs
- gears/system/authn-resolver/plugins/oidc-authn-plugin/src/config.rs
diffora
force-pushed
the
oidc-authn-per-issuer-validation
branch
from
June 13, 2026 19:17
38a39b1 to
1dbea54
Compare
Add optional per-issuer expected_audience, jose_typ, and clock_skew_leeway_secs overrides on TrustedIssuerInput; enforce them in the JWT validator (typ checked case-insensitively, per-issuer audience replaces the global list when set, per-issuer leeway overrides global). Issuers without overrides keep existing behavior. Signed-off-by: Diffora <ddiffora@gmail.com>
diffora
force-pushed
the
oidc-authn-per-issuer-validation
branch
2 times, most recently
from
June 13, 2026 21:27
d294ecd to
868647c
Compare
…igration Add #[serde(flatten)] extra to JwtClaims so claim mapping can read non-standard claims (e.g. cap tokens' subject_tenant/scopes) by name instead of dropping them at deserialize. Signed-off-by: Diffora <ddiffora@gmail.com>
diffora
force-pushed
the
oidc-authn-per-issuer-validation
branch
from
June 13, 2026 21:28
868647c to
e07f8d1
Compare
diffora
pushed a commit
that referenced
this pull request
Jul 16, 2026
- Security (major): the multipart report-part callback now validates the caller-supplied part size against the token's authoritative claims.multipart.size (the exact per-part size from compute_plan) and persists that value, so a holder of the signed token can no longer forge part sizes and corrupt the completed version.size. Test: report_part_rejects_forged_size. - finalize: drop the redundant second SHA-256 — reuse the already-verified hash_value (verify_content_hash only returns Ok on an exact match). - IdempotencyRepo::insert takes the IdempotencyInsert struct instead of 6 positional args (removes the too_many_arguments allow). - policy_service: extract a shared authorize_admin_or_owner helper (dedup the ADMIN_POLICY-first / owner-check logic). - mark the test-only expires_at mutators #[doc(hidden)] with a "test-support only" note (a Cargo test-support feature was rejected: it would break/skip the default `cargo test` since external integration-test crates call them). Retention-rule 404 mislabel (CodeRabbit #3) was already fixed by the Tier 3 RetentionRuleNotFound change. Signed-off-by: Roland From <rfedorov@linkentools.com>
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
4 participants
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.
Summary
Adds per-issuer JWT validation policy to
oidc-authn-plugin. Each trusted issuer can now pin its own:typheader,audaudience(s),instead of a single global policy applied to every issuer. This lets one plugin instance trust multiple issuers with distinct, tighter rules — e.g. accept long-lived tokens from a general-purpose IdP while requiring a specific token type and audience (and a narrower clock skew) from a second, stricter issuer.
What changed
TrustedIssuerInput): new#[serde(default)]fieldsexpected_audience,jose_typ,clock_skew_leeway_secs, validated at load (non-empty/trimmed/lowercasedtyp; leeway ≤ 300s; audience patterns compiled). Backward-compatible — when a field is unset, the issuer falls back to the existing global policy.typenforced case-insensitively (a missing header fails closed when atypis pinned); leeway applied only toexp/nbf/iat; issuer policy is keyed on the verifiediss(re-checked post-signature, so one issuer's policy can never apply to another).expected_audiencenow requires theaudclaim to be present, rather than depending on the separate globalrequire_audienceflag. A token from a pinned issuer that omitsaudis rejected.Tests
Per-issuer matrix:
typaccepted (case-insensitive) / wrong-typ/ missing-typ-header rejected; per-issuer audience rejects the global audience, accepts its own, accepts an arrayaudmember, and rejects a missingaudeven with the global flag off; an issuer without overrides falls back to the global policy and rejects another issuer's audience; leeway wider-accepts / narrower-rejects at the boundary; config fail-fast (blank typ, leeway > 300, bad pattern); deserialization round-trip.cargo test -p cf-gears-oidc-authn-plugin— all green; clippy clean.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes