feat(jwtinfo): add flags to refresh and write to file JWT tokens#31
Conversation
…sh it before it expires
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ 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: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThis PR adds JWT token refresh and renew capabilities to the ChangesJWT Token Refresh Feature
Sequence DiagramsequenceDiagram
participant CLI as CLI (jwtinfo)
participant TokData as JwtTokenData
participant Client as HTTP Client
participant File as File System
participant OS as OS Signals
CLI->>TokData: RequestToken() → *JwtTokenData
TokData->>Client: POST /token (initial request)
Client-->>TokData: access_token, refresh_token
TokData->>File: WriteTokenToFile() [if --token-output-file]
File-->>TokData: ✓ persisted
alt --refresh enabled
CLI->>OS: signal.Notify (SIGINT/SIGTERM)
CLI->>TokData: RefreshLoop(ctx, ...)
loop Until context cancelled
TokData->>TokData: calculateWaitDuration(renewThreshold)
TokData->>TokData: time.After(duration)
TokData->>TokData: Refresh()
TokData->>Client: POST /token (with refresh_token)
Client-->>TokData: new access_token
TokData->>File: WriteTokenToFile()
File-->>TokData: ✓ refreshed
end
OS-->>CLI: signal received
CLI->>TokData: ctx.Cancel()
TokData-->>CLI: RefreshLoop() returns
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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. Review rate limit: 0/1 reviews remaining, refill in 19 minutes and 38 seconds.Comment |
xenOs76
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal/jwtinfo/jwtinfo.go (1)
419-427:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't require
iat/expon refresh tokens.
DecodeBase64now accepts JWT-shaped refresh tokens, butPrintTokenInfostill unconditionally extracts time claims for every decoded token. Many providers issue refresh tokens withoutiat/exp, so this can makejwtinfofail after an otherwise successful decode. Skip the time table when those claims are absent, or limit it to the access token.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@internal/jwtinfo/jwtinfo.go` around lines 419 - 427, PrintTokenInfo currently always calls unmarshalTokenTimeClaims and returns an error when iat/exp are missing; change it to first check token.claims for "iat" or "exp" and only call unmarshalTokenTimeClaims and render the cTable when at least one of those keys exists (or, if you prefer, only for access tokens). In practice: inspect token.claims (the map passed to unmarshalTokenTimeClaims) for "iat"/"exp" before calling unmarshalTokenTimeClaims, and if neither key is present simply skip creating/printing the time table instead of returning an error; keep using token, tokenTimeClaims, unmarshalTokenTimeClaims and cTable names so the change is localized.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@internal/cmd/jwtinfo.go`:
- Around line 174-187: The refresh loop currently re-creates refreshClient and
refreshValuesMap and calls jwtinfo.ReadRequestValuesFile and
jwtinfo.ParseRequestJSONValues with ignored errors, which can drop parameters;
instead hoist the existing client and requestValuesMap (the variables originally
created and validated earlier) into a scope visible to the refresh loop and
reuse them there, removing the redundant refreshClient/refreshValuesMap
re-parsing; if you must re-read, propagate and handle errors from
jwtinfo.ReadRequestValuesFile and jwtinfo.ParseRequestJSONValues rather than
assigning to “_”, and ensure the refresh code uses the validated client variable
(client) and requestValuesMap rather than refreshClient/refreshValuesMap.
In `@internal/jwtinfo/jwtinfo.go`:
- Around line 613-647: In calculateWaitDuration (on JwtTokenData) validate
renewThreshold is within 0..100 before any time math: if renewThreshold is < 0
or > 100 return a descriptive error (e.g. "renewThreshold must be between 0 and
100") so invalid values cannot schedule refreshes after expiry or force
immediate tight retries; add this check at the top of the function (before
GetExpiration/GetIssuedAt and before any lifetime/wakeTime calculation).
- Around line 653-659: The current write using os.WriteFile(outFileName,
[]byte(jtd.AccessTokenRaw), 0o600) can leave the target file empty/partial for
readers; instead, write jtd.AccessTokenRaw atomically by creating a temp file in
the same directory (e.g., via os.CreateTemp(dir, "token-*")), write the bytes,
fsync if possible, close it, then os.Rename(tempPath, outFileName) and set
permissions 0600; on any error remove the temp file and report the failure to
outWriter similarly, and keep the existing timestamped success message when
rename succeeds.
In `@README.md`:
- Around line 237-246: The README examples for the jwtinfo command incorrectly
say refresh runs “in the background”; update the wording for the https-wrench
jwtinfo examples and the second occurrence (lines 267-277) to state that
--refresh runs as a foreground loop that blocks the command and keeps the token
refreshed until interrupted (or similar phrasing like “keep it refreshed until
interrupted”), and ensure the flags section (--refresh, --renew-threshold)
reflects this foreground/blocking behavior rather than implying background
operation.
---
Outside diff comments:
In `@internal/jwtinfo/jwtinfo.go`:
- Around line 419-427: PrintTokenInfo currently always calls
unmarshalTokenTimeClaims and returns an error when iat/exp are missing; change
it to first check token.claims for "iat" or "exp" and only call
unmarshalTokenTimeClaims and render the cTable when at least one of those keys
exists (or, if you prefer, only for access tokens). In practice: inspect
token.claims (the map passed to unmarshalTokenTimeClaims) for "iat"/"exp" before
calling unmarshalTokenTimeClaims, and if neither key is present simply skip
creating/printing the time table instead of returning an error; keep using
token, tokenTimeClaims, unmarshalTokenTimeClaims and cTable names so the change
is localized.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6b988d94-338e-44e2-890f-4b285649abad
📒 Files selected for processing (5)
README.mdinternal/cmd/jwtinfo.gointernal/jwtinfo/jwtinfo.gointernal/jwtinfo/jwtinfo_refresh_test.gointernal/jwtinfo/jwtinfo_test.go
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary by CodeRabbit
New Features
Documentation