oauth2: harden configuration, JWKS refresh, and token lifetimes#12106
oauth2: harden configuration, JWKS refresh, and token lifetimes#12106edsiper wants to merge 11 commits into
Conversation
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
Signed-off-by: Eduardo Silva <eduardo@chronosphere.io>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
📝 WalkthroughWalkthroughOAuth2 capability flags and plugin checks were added, token expiration handling was adjusted, JWKS refreshes now preserve valid cached keys on failure, and integration/internal tests cover these behaviors. ChangesOAuth2 validation and refresh behavior
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant HTTPInput
participant JWKSFetcher
participant JWKSCache
HTTPInput->>JWKSFetcher: request JWKS during token validation
JWKSFetcher->>JWKSCache: build temporary cache
JWKSFetcher->>JWKSCache: replace active cache after success
JWKSFetcher-->>HTTPInput: return refresh failure
HTTPInput->>JWKSCache: validate with existing cached keys
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a3f5814add
ℹ️ 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".
| refresh_skew = ctx->refresh_skew; | ||
| if (ctx->expires_in <= (uint64_t) refresh_skew) { | ||
| refresh_skew = ctx->expires_in / 10; | ||
| } |
There was a problem hiding this comment.
Apply refresh skew to token_expired callers
This skew adjustment only affects the flb_oauth2_get_access_token() path, but several existing plugins still decide whether to refresh with flb_oauth2_token_expired() and then reuse ctx->o->access_token directly (for example BigQuery, Chronicle, Stackdriver, and Azure outputs). Since this commit now stores the full expires_in instead of the old 10% shortened lifetime, those callers lose their only early-refresh margin and can send a token right up to its exact expiry under normal output traffic; please apply the same skew logic in flb_oauth2_token_expired() or migrate those callers to the skew-aware path.
Useful? React with 👍 / 👎.
Summary
expires_invalue, accept short-lived tokens, and use a 300-second default when the field is omittedRoot cause
OAuth2 properties were collected by shared input/output configuration code without checking whether the selected plugin consumed them. JWKS refresh cleared the existing cache before validating the replacement response. Token parsing also reduced
expires_inby 10%, rejected lifetimes shorter than the configured refresh skew, and treated an omittedexpires_inas an error.User impact
Misconfigured OAuth2 settings now fail at startup instead of being silently ignored. Temporary or malformed JWKS refresh responses no longer discard usable cached keys. Standards-compliant token responses with short or omitted lifetimes are accepted without immediate refresh loops.
Validation
cmake --build build -j8 --target fluent-bit-bincmake --build build -j8 --target flb-it-oauth2 && ./build/bin/flb-it-oauth2— 12 passedcmake --build build -j8 --target flb-it-oauth2_jwt && ./build/bin/flb-it-oauth2_jwt— 9 passedSummary by CodeRabbit