Skip to content

fix(auth): keep refresh token when refresh response omits refresh_token#93

Merged
amondnet merged 2 commits into
mainfrom
amondnet/token
Jul 6, 2026
Merged

fix(auth): keep refresh token when refresh response omits refresh_token#93
amondnet merged 2 commits into
mainfrom
amondnet/token

Conversation

@amondnet

@amondnet amondnet commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

Asana's OAuth refresh_token grant response omits refresh_token in the response body. The existing code was overwriting/dropping the previously stored refresh token whenever a refresh response omitted a new one. Once the access token subsequently expired too, there was no valid token pair left to refresh with, silently logging the user out.

This fixes the user-reported issue: 로그인 후 일정 시간 후 로그인이 풀리는 문제 (login silently expires/logs out after some time, ~1 hour, when using OAuth token login).

Fix: keep the existing refresh token when the refresh response doesn't include a new one.

Related issue

No specific tracked issue number — this addresses the user-reported bug: "token 사용해서 로그인시 일정시간후 로그인 풀림" (로그인 후 일정 시간 후 로그인이 풀리는 문제).

Checklist

  • PR title follows Conventional Commits
  • Tests added or updated, and the suite passes (bun run test)
  • Lint/format pass (bun run lint)
  • Documentation updated if behavior changed
  • No breaking change, or a BREAKING CHANGE: note is included

Summary by cubic

Retains the stored OAuth refresh token when Asana’s refresh response omits refresh_token, preventing silent logout after the next access token expiry (~1 hour). Adds a regression test and restores global.fetch in tests to avoid pollution.

  • Bug Fixes
    • Keep existing refreshToken when refresh responses omit refresh_token.
    • Make refresh_token optional in OAuthTokenResponse with documentation.
    • Add regression test and restore global.fetch in finally to prevent test pollution.

Written for commit 2648bd9. Summary will update on new commits.

Asana's refresh_token grant response does not rotate the refresh token,
so persisting tokenResponse.refresh_token verbatim dropped the stored
refresh token after the first renewal. Every later expiry then found no
refresh token, leaving the CLI silently logged out about an hour after
OAuth login.

Includes a failing-first regression test.
@vercel

vercel Bot commented Jul 6, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
asana Ready Ready Preview, Comment Jul 6, 2026 1:23pm

Request Review

@amondnet amondnet marked this pull request as ready for review July 6, 2026 13:14
@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@amondnet, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 36 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: cf99eceb-4e81-4bbe-a5c2-ea2f44305fb0

📥 Commits

Reviewing files that changed from the base of the PR and between f4e36a4 and 2648bd9.

📒 Files selected for processing (3)
  • src/lib/asana-client.ts
  • src/lib/oauth.ts
  • test/lib/asana-client.test.ts
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch amondnet/token

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 6, 2026

Copy link
Copy Markdown

Greptile Summary

Fixes a silent logout bug caused by Asana's OAuth refresh endpoint omitting refresh_token from its response, which caused the stored refresh token to be overwritten with undefined and lost after the first token renewal.

  • asana-client.ts: Uses the nullish-coalescing operator (??) to fall back to the previously stored config.refreshToken when the refresh response does not include a new one, preventing the token from being dropped.
  • oauth.ts: Marks refresh_token as optional in OAuthTokenResponse with a JSDoc comment explaining Asana's non-rotating refresh token behaviour.
  • asana-client.test.ts: Adds a regression test that verifies the stored refresh token is preserved when the refresh response omits the field.

Confidence Score: 5/5

Safe to merge. The change is a minimal, well-targeted one-liner with a matching regression test, touching only the token-persistence path after a successful refresh.

The fix correctly uses ?? (nullish coalescing) rather than ||, which means it only falls back to the stored token when the response field is null or undefined — precisely the Asana behaviour described. If Asana ever starts rotating refresh tokens and returns a new one, the new value will still be used. The type change to refresh_token?: string is consistent with the fix. The regression test directly exercises the broken scenario and asserts both the updated access token and the preserved refresh token, giving good confidence the fix is correct.

No files require special attention.

Important Files Changed

Filename Overview
src/lib/asana-client.ts One-line fix: nullish-coalescing fallback preserves the stored refresh token when the Asana refresh response omits the field.
src/lib/oauth.ts Makes refresh_token optional in OAuthTokenResponse; accurately reflects the Asana API contract and documents the reason.
test/lib/asana-client.test.ts Adds a targeted regression test covering the omitted-refresh_token scenario; correctly verifies both the new access token and the preserved refresh token.

Sequence Diagram

%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
    participant App
    participant Config as config.json
    participant Asana as Asana OAuth API

    Note over App,Config: Access token expires (~1 h)
    App->>Config: "loadConfig() → { accessToken, refreshToken: "stored-rt" }"
    App->>Asana: "POST /oauth_token (grant_type=refresh_token)"
    Asana-->>App: "{ access_token: "new-at", expires_in: 3600 } (refresh_token omitted)"

    Note over App: Before fix: tokenResponse.refresh_token = undefined → token lost
    Note over App: After fix: undefined ?? "stored-rt" = "stored-rt"

    App->>Config: "saveConfig({ accessToken: "new-at", refreshToken: "stored-rt" })"
    Note over App,Config: Next renewal still works ✓
Loading
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
    participant App
    participant Config as config.json
    participant Asana as Asana OAuth API

    Note over App,Config: Access token expires (~1 h)
    App->>Config: "loadConfig() → { accessToken, refreshToken: "stored-rt" }"
    App->>Asana: "POST /oauth_token (grant_type=refresh_token)"
    Asana-->>App: "{ access_token: "new-at", expires_in: 3600 } (refresh_token omitted)"

    Note over App: Before fix: tokenResponse.refresh_token = undefined → token lost
    Note over App: After fix: undefined ?? "stored-rt" = "stored-rt"

    App->>Config: "saveConfig({ accessToken: "new-at", refreshToken: "stored-rt" })"
    Note over App,Config: Next renewal still works ✓
Loading

Reviews (2): Last reviewed commit: "chore(test): restore global.fetch in fin..." | Re-trigger Greptile

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found across 3 files

Architecture diagram
sequenceDiagram
    participant Client as Client (Extension/CLI)
    participant Auth as refreshTokenIfNeeded
    participant Config as Config Store (file)
    participant Asana as Asana OAuth API

    Note over Client,Asana: OAuth token refresh flow (fix applied)

    Client->>Auth: Call refreshTokenIfNeeded()
    Auth->>Config: Read config (accessToken, refreshToken, expiresAt)
    Config-->>Auth: config object

    Auth->>Auth: Check if token expired (expiresAt < now)
    alt Token expired or near expiry
        Auth->>Asana: POST /oauth/token (grant_type=refresh_token)
        Asana-->>Auth: OAuthTokenResponse
        Note over Auth: Contains access_token, expires_in,<br/>refresh_token (optional per Asana)

        alt refresh_token present in response
            Auth->>Config: Save new accessToken and refreshToken
        else refresh_token omitted (Asana default behavior)
            Auth->>Config: Save new accessToken,<br/>keep existing refreshToken (??)
        end
        Config-->>Auth: Config saved
        Auth-->>Client: true (success)
    else Token still valid
        Auth-->>Client: false (no refresh needed)
    end
Loading

Re-trigger cubic

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates the Asana client to retain the stored refresh token when the token refresh response omits the refresh_token field, preventing auto-refresh from breaking after the first renewal. It also updates the OAuthTokenResponse interface to mark refresh_token as optional and adds a corresponding unit test. The review feedback suggests wrapping the test's fetch mocking and assertions in a try...finally block to ensure global.fetch is always restored, preventing potential test pollution if an assertion fails.

Comment thread test/lib/asana-client.test.ts Outdated
@amondnet

amondnet commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

/gemini review

@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

본 풀 리퀘스트는 Asana 토큰 갱신 응답에서 refresh_token 필드가 생략될 때 기존에 저장된 리프레시 토큰이 undefined로 덮어써지는 문제를 방지하도록 수정합니다. 이를 위해 OAuthTokenResponse 인터페이스의 refresh_token을 선택적 필드로 변경하고 관련 단위 테스트를 추가했습니다. 검토할 리뷰 댓글이 없으며, 추가적인 피드백은 없습니다.

@amondnet amondnet merged commit b90ede3 into main Jul 6, 2026
16 checks passed
@amondnet amondnet deleted the amondnet/token branch July 6, 2026 14:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant