Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Deploying docs with
|
| Latest commit: |
ac21f26
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://374678cf.docs-7wm.pages.dev |
| Branch Preview URL: | https://auth-refresh-tokens-v2.docs-7wm.pages.dev |
WalkthroughAdds documentation for JWT refresh token support: token pairs (access + refresh), a new refresh endpoint and rotation flow, updated FAQ and integration guides, and a new blog post with implementation examples, security guidance, troubleshooting, and migration instructions. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthAPI as Auth API
participant TokenStore as Token Store
participant Resource as Resource API
Client->>AuthAPI: POST /auth/token (credentials)
AuthAPI->>TokenStore: store refresh_token (rotate/new)
TokenStore-->>AuthAPI: ack
AuthAPI-->>Client: 200 {access_token, refresh_token, expires_at}
Client->>Resource: Request with access_token
Resource-->>Client: 200 OK
Note over Client: When access_token near expiry
Client->>AuthAPI: POST /auth/token/refresh (refresh_token)
AuthAPI->>TokenStore: validate & rotate refresh_token (revoke old)
TokenStore-->>AuthAPI: validation result / store new token
AuthAPI-->>Client: 200 {new access_token, new refresh_token, expires_at}
Client->>Resource: Request with new access_token
Resource-->>Client: 200 OK
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📝 Coding Plan
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 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/blog/posts/2026-03-14_jwt-refresh-tokens.md`:
- Line 212: Unlabeled fenced code blocks in the post are causing markdownlint
MD040 failures; update the fences that wrap the snippet containing print(f"✓
Token refreshed, expires: {new_tokens['expires_at']}") and the other unlabeled
fenced block later in the same document to include an explicit language tag
(e.g., change ``` to ```python or ```text as appropriate) so the code blocks are
labeled for the linter.
- Around line 375-380: The example SMSServiceDaemon.run calls an undefined
method process_message_queue, making the example non-runnable; add or reference
a concrete method on SMSServiceDaemon (e.g., implement process_message_queue or
rename the call to an existing method like process_queue/process_messages) and
ensure the sample class includes that method (with a brief loop/body) so run()
calls a defined member; update the class definition for SMSServiceDaemon to
include the matching method name used in run().
In `@docs/faq/authentication.md`:
- Around line 166-173: The snippet compares a timezone-aware
self.token_expires_at to a naive datetime.now(), causing TypeError; update the
logic in the method that returns the access token (the block referencing
self.access_token, self.token_expires_at, refresh_access_token(), and
get_token(scopes)) to use a timezone-aware "now" (e.g.,
datetime.now(timezone.utc) or normalize both datetimes to UTC) when computing
datetime.now() + timedelta(minutes=5) so the comparison is between datetimes
with consistent tzinfo.
In `@docs/integration/authentication.md`:
- Around line 259-263: Update the table row for
`/3rdparty/v1/auth/token/refresh` to clarify that the `tokens:refresh` scope is
system-managed and must not be requested by clients; change the Description or
Required Scope cell to something like “Refresh token (Bearer) — system-managed
`tokens:refresh` claim” so readers understand the scope is injected by the
system rather than requested directly; ensure the change targets the table entry
for `/3rdparty/v1/auth/token/refresh` and mentions `tokens:refresh` explicitly.
- Around line 596-600: The datetime comparison uses naive datetime.now() against
the timezone-aware self.token_expires_at, causing a TypeError; update the check
in the block that references self.access_token, self.token_expires_at and
refresh_access_token to use a timezone-aware "now" (e.g.,
datetime.now(timezone.utc) or equivalent) so the timedelta(minutes=5) comparison
is between two aware datetimes.
🪄 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: CHILL
Plan: Pro
Run ID: e3151527-99d5-43aa-936d-b0431cfb9704
⛔ Files ignored due to path filters (1)
docs/assets/blog/jwt-refresh-tokens.pngis excluded by!**/*.png
📒 Files selected for processing (3)
docs/blog/posts/2026-03-14_jwt-refresh-tokens.mddocs/faq/authentication.mddocs/integration/authentication.md
capcom6
force-pushed
the
auth/refresh-tokens-v2
branch
from
739ccf4 to
9d81654
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/faq/authentication.md (1)
385-390:⚠️ Potential issue | 🟠 MajorUse timezone-aware
now()in this refresh snippet to avoidTypeError.This snippet compares
datetime.now()against a UTC-aware expiry in other examples. Usedatetime.now(timezone.utc)consistently.In Python, what happens when comparing timezone-aware datetimes with naive datetimes, and what is the recommended fix?Suggested doc fix
-# Refresh token when access token expires -if datetime.now() + timedelta(minutes=5) >= token_expires_at: +# Refresh token when access token expires +if datetime.now(timezone.utc) + timedelta(minutes=5) >= token_expires_at: new_token = refresh_access_token()🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/faq/authentication.md` around lines 385 - 390, The refresh logic uses datetime.now() (naive) while token_expires_at is UTC-aware, causing a TypeError when compared; update the snippet to use a timezone-aware current time (e.g., datetime.now(timezone.utc) or datetime.utcnow().replace(tzinfo=timezone.utc)) when evaluating if datetime.now() + timedelta(minutes=5) >= token_expires_at so the comparison between token_expires_at and the now value is between two aware datetimes; keep refresh_access_token() and token_expires_at usage unchanged.
🤖 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/blog/posts/2026-03-14_jwt-refresh-tokens.md`:
- Around line 61-63: The example response JSON uses JWT-shaped literal strings
for "access_token" and "refresh_token" which trigger secret scanners; replace
those values with explicit non-secret placeholders (e.g. "<ACCESS_TOKEN>",
"<REFRESH_TOKEN>") while keeping "expires_at" as-is, and apply the same
substitution to the other examples in the document that use the same
"access_token"/"refresh_token" keys so no JWT-like literals remain.
- Around line 152-170: The JavaScript snippets mix CommonJS require with
top-level await (e.g., const axios = require('axios') and await
axios.post(...)), which is invalid; fix by either converting to ESM (use import
axios from 'axios' and ensure module is ESM) or by wrapping the asynchronous
calls in an async function/IIFE (locate the axios variable and the await
axios.post(...) / destructuring of response.data such as access_token,
refresh_token, expires_at) and invoke that async wrapper so the await runs
inside an async context; apply the same change to both occurrences referenced in
the diff.
In `@docs/integration/authentication.md`:
- Around line 102-104: The fenced code block containing the endpoint "POST
/3rdparty/v1/auth/token/refresh" needs a language tag to satisfy MD040; update
that fenced block to use the text language tag (i.e., change the opening ``` to
```text) so markdownlint passes consistently.
---
Duplicate comments:
In `@docs/faq/authentication.md`:
- Around line 385-390: The refresh logic uses datetime.now() (naive) while
token_expires_at is UTC-aware, causing a TypeError when compared; update the
snippet to use a timezone-aware current time (e.g., datetime.now(timezone.utc)
or datetime.utcnow().replace(tzinfo=timezone.utc)) when evaluating if
datetime.now() + timedelta(minutes=5) >= token_expires_at so the comparison
between token_expires_at and the now value is between two aware datetimes; keep
refresh_access_token() and token_expires_at usage unchanged.
🪄 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: CHILL
Plan: Pro
Run ID: b083210f-9794-4ed5-9547-03d2ab5d87ce
⛔ Files ignored due to path filters (1)
docs/assets/blog/jwt-refresh-tokens.pngis excluded by!**/*.png
📒 Files selected for processing (3)
docs/blog/posts/2026-03-14_jwt-refresh-tokens.mddocs/faq/authentication.mddocs/integration/authentication.md
capcom6
force-pushed
the
auth/refresh-tokens-v2
branch
from
28012eb to
ac21f26
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
docs/integration/authentication.md (1)
102-104:⚠️ Potential issue | 🟡 MinorAdd a language tag to this fenced endpoint block.
This fence is still unlabeled and will keep triggering MD040 in markdownlint.
Suggested fix
-``` +```text POST /3rdparty/v1/auth/token/refresh</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@docs/integration/authentication.mdaround lines 102 - 104, The fenced
endpoint block containing "POST /3rdparty/v1/auth/token/refresh" is unlabeled
and causing MD040; update that fence to include a language tag (e.g., change the
opening backticks totext) so it becomestext followed by the endpoint and
closing ``` to satisfy markdownlint. Ensure you only modify the fence markers
around the existing endpoint block (the block with "POST
/3rdparty/v1/auth/token/refresh").</details> </blockquote></details> <details> <summary>docs/blog/posts/2026-03-14_jwt-refresh-tokens.md (2)</summary><blockquote> `219-241`: _⚠️ Potential issue_ | _🟠 Major_ **Avoid top-level `await` in this CommonJS snippet.** This block uses `require(...)` (CommonJS) and then `await` at top level, which is invalid in that context. Wrap usage in an async IIFE (or switch snippet to ESM). <details> <summary>Suggested fix</summary> ```diff async function refreshTokenPair(refreshToken) { const response = await axios.post( @@ }; } // Usage - const newTokens = await refreshTokenPair(oldRefreshToken); - console.log(`✓ Token refreshed, expires: ${newTokens.expires_at}`); + (async () => { + const newTokens = await refreshTokenPair(oldRefreshToken); + console.log(`✓ Token refreshed, expires: ${newTokens.expires_at}`); + })().catch(console.error); ``` </details> ```web Is top-level `await` valid in Node.js CommonJS snippets that use `require()`? What is the recommended docs pattern? ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/blog/posts/2026-03-14_jwt-refresh-tokens.md` around lines 219 - 241, The snippet mixes CommonJS (require) with a top-level await when calling refreshTokenPair(oldRefreshToken); wrap the usage in an async IIFE (e.g., immediately invoke an async function that calls refreshTokenPair and logs the result) or convert the file to ESM (use import and keep top-level await) so the await is valid; update the usage portion that references refreshTokenPair and oldRefreshToken accordingly and ensure errors are awaited/handled inside the async wrapper. ``` </details> --- `61-63`: _⚠️ Potential issue_ | _🟡 Minor_ **Replace JWT-shaped sample strings with explicit placeholders.** These literals look like real tokens and are commonly flagged by secret scanners. <details> <summary>Suggested fix</summary> ```diff - "access_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...", - "refresh_token": "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9...", + "access_token": "<ACCESS_TOKEN>", + "refresh_token": "<REFRESH_TOKEN>", @@ - -H "Authorization: Bearer eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9..." + -H "Authorization: Bearer <REFRESH_TOKEN>" @@ -refresh_token = "eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9..." # Never do this! +refresh_token = "<REFRESH_TOKEN>" # Never do this! ``` </details> Also applies to: 245-248, 411-413 <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/blog/posts/2026-03-14_jwt-refresh-tokens.md` around lines 61 - 63, Replace JWT-shaped sample strings in the JSON snippets (fields "access_token" and "refresh_token") with explicit non-secret placeholders (e.g., "<ACCESS_TOKEN_PLACEHOLDER>" and "<REFRESH_TOKEN_PLACEHOLDER>") and keep "expires_at" as-is or use a clear timestamp placeholder; update the occurrences in this file that match those patterns (including the other instances noted around lines 245-248 and 411-413) so the examples cannot be mistaken for real tokens and will not trigger secret scanners. ``` </details> </blockquote></details> <details> <summary>docs/faq/authentication.md (1)</summary><blockquote> `385-390`: _⚠️ Potential issue_ | _🟠 Major_ **Use timezone-aware `now()` in this refresh snippet.** This block reverted to naive `datetime.now()`, which is inconsistent with the UTC-aware examples above and can cause aware/naive comparison errors. <details> <summary>Suggested fix</summary> ```diff - if datetime.now() + timedelta(minutes=5) >= token_expires_at: + if datetime.now(timezone.utc) + timedelta(minutes=5) >= token_expires_at: new_token = refresh_access_token() ``` </details> ```web In Python, what happens when comparing a naive datetime (`datetime.now()`) with a timezone-aware datetime, and what is the recommended fix? ``` <details> <summary>🤖 Prompt for AI Agents</summary> ``` Verify each finding against the current code and only fix it if needed. In `@docs/faq/authentication.md` around lines 385 - 390, The snippet uses naive datetime.now() which will raise a TypeError when compared to a timezone-aware datetime (token_expires_at); change the call to produce a timezone-aware "now" (e.g., use datetime.now(timezone.utc) or otherwise ensure now() has the same tzinfo as token_expires_at) so the comparison in the refresh logic (the if checking token_expires_at) is between compatible aware datetimes; no other logic needs to change—keep the existing timedelta and refresh_access_token() usage. ``` </details> </blockquote></details> </blockquote></details> <details> <summary>🤖 Prompt for all review comments with AI agents</summary>Verify each finding against the current code and only fix it if needed.
Inline comments:
In@docs/integration/authentication.md:
- Around line 64-65: Replace the JWT-shaped example values under the JSON keys
"refresh_token" and "expires_at" with explicit non-secret placeholders (e.g.,
"REDACTED_REFRESH_TOKEN" or "<REFRESH_TOKEN_PLACEHOLDER>" and an ISO timestamp
example like "2025-11-22T08:45:00Z" if needed) to avoid secret-scanner false
positives; update both occurrences where "refresh_token" currently contains an
eyJ... string (and the corresponding block mentioned) so the docs show a clear
placeholder rather than a token-like literal.
Duplicate comments:
In@docs/blog/posts/2026-03-14_jwt-refresh-tokens.md:
- Around line 219-241: The snippet mixes CommonJS (require) with a top-level
await when calling refreshTokenPair(oldRefreshToken); wrap the usage in an async
IIFE (e.g., immediately invoke an async function that calls refreshTokenPair and
logs the result) or convert the file to ESM (use import and keep top-level
await) so the await is valid; update the usage portion that references
refreshTokenPair and oldRefreshToken accordingly and ensure errors are
awaited/handled inside the async wrapper.- Around line 61-63: Replace JWT-shaped sample strings in the JSON snippets
(fields "access_token" and "refresh_token") with explicit non-secret
placeholders (e.g., "<ACCESS_TOKEN_PLACEHOLDER>" and
"<REFRESH_TOKEN_PLACEHOLDER>") and keep "expires_at" as-is or use a clear
timestamp placeholder; update the occurrences in this file that match those
patterns (including the other instances noted around lines 245-248 and 411-413)
so the examples cannot be mistaken for real tokens and will not trigger secret
scanners.In
@docs/faq/authentication.md:
- Around line 385-390: The snippet uses naive datetime.now() which will raise a
TypeError when compared to a timezone-aware datetime (token_expires_at); change
the call to produce a timezone-aware "now" (e.g., use datetime.now(timezone.utc)
or otherwise ensure now() has the same tzinfo as token_expires_at) so the
comparison in the refresh logic (the if checking token_expires_at) is between
compatible aware datetimes; no other logic needs to change—keep the existing
timedelta and refresh_access_token() usage.In
@docs/integration/authentication.md:
- Around line 102-104: The fenced endpoint block containing "POST
/3rdparty/v1/auth/token/refresh" is unlabeled and causing MD040; update that
fence to include a language tag (e.g., change the opening backticks totext) so it becomestext followed by the endpoint and closing ``` to satisfy
markdownlint. Ensure you only modify the fence markers around the existing
endpoint block (the block with "POST /3rdparty/v1/auth/token/refresh").</details> <details> <summary>🪄 Autofix (Beta)</summary> Fix all unresolved CodeRabbit comments on this PR: - [ ] <!-- {"checkboxId": "4b0d0e0a-96d7-4f10-b296-3a18ea78f0b9"} --> Push a commit to this branch (recommended) - [ ] <!-- {"checkboxId": "ff5b1114-7d8c-49e6-8ac1-43f82af23a33"} --> Create a new PR with the fixes </details> --- <details> <summary>ℹ️ Review info</summary> <details> <summary>⚙️ Run configuration</summary> **Configuration used**: Organization UI **Review profile**: CHILL **Plan**: Pro **Run ID**: `179b2f1b-3d4b-4288-bc77-6f70fb91b9c8` </details> <details> <summary>📥 Commits</summary> Reviewing files that changed from the base of the PR and between 9d816542af674337dc2604790459f0e1f10604ce and ac21f263340998436980ae58d1f872ce29a3a521. </details> <details> <summary>⛔ Files ignored due to path filters (1)</summary> * `docs/assets/blog/jwt-refresh-tokens.png` is excluded by `!**/*.png` </details> <details> <summary>📒 Files selected for processing (3)</summary> * `docs/blog/posts/2026-03-14_jwt-refresh-tokens.md` * `docs/faq/authentication.md` * `docs/integration/authentication.md` </details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
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
1 participant
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 by CodeRabbit