Support refresh tokens

[WhammyLeaf]

Summary

Support refresh tokens so that an authenticated session can be extended by more than 1 hour (or whatever the expiration on your access token is)

Changes

• Updates the handle_token method in the handlers.go file so that refresh_token grant is supported in addition to authorization_code grant

Testing

• Tests pass locally
• New tests added (if applicable)

Related Issues

#18

Summary by CodeRabbit

• New Features

  • Added support for the authorization_code grant flow with optional PKCE verification.
  • Continued support for refresh_token grant to obtain new access tokens.
  • Per-grant validation and token exchange to ensure correct responses.

• Bug Fixes

  • Explicit rejection of unsupported grant types with proper error responses.
  • Enhanced error handling within each grant flow to avoid unexpected outcomes.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Adds grant-type routing to the token endpoint: implements explicit authorization_code and refresh_token branches (including PKCE and redirect URI handling for authorization_code), and returns HTTP 400 for unsupported grant types.

Changes

Cohort / File(s)	Summary
OAuth2 Token Endpoint handlers.go	Replaces single-path token handling with a switch on grant_type. Adds authorization_code branch (validates code, resolves redirect_uri, optional PKCE via code_verifier, exchanges code for tokens) and refresh_token branch (validates refresh_token, uses TokenSource to obtain token). Adds default error for unsupported grant types; preserves token response fields and no-store semantics.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant TokenEndpoint as Token Endpoint
    participant OAuth2Cfg as OAuth2 Config
    participant TokenSrc as Token Source

    Client->>TokenEndpoint: POST /token (grant_type, params)
    alt grant_type == authorization_code
        TokenEndpoint->>TokenEndpoint: Validate `code`
        TokenEndpoint->>TokenEndpoint: Resolve `redirect_uri` (fallbacks / allowlist)
        TokenEndpoint->>TokenEndpoint: Configure PKCE if `code_verifier`
        TokenEndpoint->>OAuth2Cfg: Exchange code for tokens
        OAuth2Cfg-->>TokenEndpoint: Token response (access, id, refresh, scope)
    else grant_type == refresh_token
        TokenEndpoint->>TokenEndpoint: Validate `refresh_token`
        TokenEndpoint->>TokenSrc: Create TokenSource from refresh token
        TokenSrc-->>TokenEndpoint: New access token
    else unsupported grant_type
        TokenEndpoint-->>Client: 400 Unsupported grant type
    end
    TokenEndpoint-->>Client: 200 Token response (when successful)

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

• Support refresh tokens #18 — Implements explicit refresh_token grant handling and TokenSource usage, matching the issue's requested feature.

Poem

🐰 In tunnels of code I nimbly hop,
I switch the paths and never stop,
Codes exchanged and refreshes spun,
PKCE kept safe till work is done,
Tokens delivered — hop! — hooray, we’re done.

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Support refresh tokens' directly aligns with the main objective and primary change in the pull request, which adds refresh token support to the HandleToken method.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7437fb2 and 54b0d20.

📒 Files selected for processing (1)

• handlers.go

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Never log raw OAuth tokens; only log SHA-256 hash of token formatted as fmt.Sprintf("%x", sha256.Sum256([]byte(token)))[:16]

Files:

• handlers.go

🔇 Additional comments (2)

handlers.go (2)

538-601: LGTM - Grant type routing and refresh token support implemented correctly.

The switch-based routing cleanly separates the authorization_code and refresh_token flows. The refresh token implementation correctly uses oauth2.Config.TokenSource() to obtain a new token from the upstream provider.

Minor observation: The error responses use different HTTP status codes (500 for authorization_code failure at line 576 vs 502 for refresh_token failure at line 595). This may be intentional to distinguish local vs upstream failures, but consider aligning them for consistency if that distinction isn't needed.

---

587-592: Fix token refresh logic: zero-value Expiry prevents RefreshToken from being used.

The oauth2 library treats tokens with zero-value Expiry as non-expiring and will NOT use the RefreshToken. According to the golang.org/x/oauth2 documentation, the RefreshToken is only used when the token is detected as expired—but a zero-value Expiry means the token is never considered expired.

Wrap the TokenSource with ReuseTokenSource(nil, src) or set the token's Expiry to a non-zero value to enable refresh behavior:

Example fix

ctx := context.Background()
src := h.oauth2Config.TokenSource(ctx, &oauth2.Token{
    RefreshToken: refreshToken,
})
// Wrap to enable refresh behavior
src = oauth2.ReuseTokenSource(nil, src)
token, err = src.Token()

⛔ Skipped due to learnings

Learnt from: CR
Repo: tuannvm/oauth-mcp-proxy PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:03:53.298Z
Learning: Applies to cache.go : Token caching uses sync.RWMutex with 5-minute TTL and background cleanup via deleteExpiredToken() goroutine to avoid lock contention

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

handlers.go (1)

579-597: Refresh token implementation looks correct.

The usage of oauth2.Config.TokenSource with a token containing only the RefreshToken field is the idiomatic way to perform token refresh with the golang.org/x/oauth2 library. When Token() is called, it will detect the token needs refreshing and use the refresh token appropriately.

Minor inconsistency: Line 576 returns http.StatusInternalServerError (500) for authorization code exchange failures, while line 595 returns http.StatusBadGateway (502) for refresh token failures. Both are upstream OAuth provider errors, so consider using consistent status codes.

♻️ Optional: Use consistent error status codes

 		token, err = h.oauth2Config.Exchange(ctx, code)
 		if err != nil {
 			h.logger.Error("OAuth2: Token exchange failed: %v", err)
-			http.Error(w, "Token exchange failed", http.StatusInternalServerError)
+			http.Error(w, "Token exchange failed", http.StatusBadGateway)
 			return
 		}

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c64f389 and dcd76c3.

📒 Files selected for processing (1)

• handlers.go

🧰 Additional context used

📓 Path-based instructions (1)

**/*.go

📄 CodeRabbit inference engine (CLAUDE.md)

Never log raw OAuth tokens; only log SHA-256 hash of token formatted as fmt.Sprintf("%x", sha256.Sum256([]byte(token)))[:16]

Files:

• handlers.go

🧠 Learnings (1)

📚 Learning: 2025-12-13T17:03:53.298Z

Learnt from: CR
Repo: tuannvm/oauth-mcp-proxy PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:03:53.298Z
Learning: Applies to **/*.go : Never log raw OAuth tokens; only log SHA-256 hash of token formatted as fmt.Sprintf("%x", sha256.Sum256([]byte(token)))[:16]

Applied to files:

• handlers.go

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: Build Verification

🔇 Additional comments (4)

handlers.go (4)

527-527: LGTM!

Extracting refresh_token from form values is appropriate for this grant type.

---

535-536: LGTM!

Declaring token and err outside the switch block allows proper scoping for the response handling after the switch.

---

538-578: LGTM!

The authorization_code flow is correctly refactored into the switch case, preserving the existing PKCE handling and redirect URI logic.

---

598-601: LGTM!

Proper handling of unsupported grant types with appropriate logging and HTTP 400 response.

[WhammyLeaf]

@tuannvm There seems to be a CI issue blocking Cursor Code Review

[tuannvm]

thanks @WhammyLeaf