Support allowlist mode and fixed redirect mode simultaneously

[WhammyLeaf]

Summary

Right now one of two redirect URI modes is supported:

• allow list mode, where the client-provided redirect URI must be in the list registered on the given mcp server
• Fixed redirect mode, where only one redirect URI can be registered on the mcp server and the client-provided redirect URI must be a localhost that will be proxied back to

This PR redesigns the interface of the library so that both modes can be supported at the same time.

It does so by introducing a new FixedRedirectURI variable, which defines a single redirect URI for fixed redirect mode. Correspondingly, the existing RedirectURIs variable now only defines the valid redirect URIs for allow list mode.

If RedirectURIs and FixedRedirectURI are both set, then RedirectURIs takes precedence. If the client-provided redirect URI is not in RedirectURIs then fixed redirect mode will be tried next.

Changes

• Most core logic changes are in handlers.go, specifically in the HandleAuthorize, HandleCallback and HandleToken methods.
• The new config variable is added in config.go
• The handleRegister function in metadata.go is also updated to properly handle the new fixed redirect mode

Testing

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

Related Issues

#20

Summary by CodeRabbit

• New Features

  • Add support for a fixed server-side redirect URI as an alternative to client-supplied allowlists in OAuth proxy mode.
  • Made the fixed-redirect option configurable via environment and programmatic settings so deployers can set a fixed callback URI.
  • OAuth flows now handle either allowlist or fixed-redirect strategies transparently.

• Bug Fixes / Security

  • Stronger validation, clearer logging, and explicit rejection paths for redirect URI handling to reduce misconfiguration and improve security.

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

[coderabbitai]

Walkthrough

A new FixedRedirectURI option is added and threaded through configuration, environment loading, and OAuth handlers. Redirect decision logic now accepts either a client redirect allowlist (RedirectURIs) or a fixed proxy redirect URI (FixedRedirectURI) across authorize, callback, token, and registration flows.

Changes

Cohort / File(s)	Summary
Configuration Setup config.go	Added FixedRedirectURI string to Config; added WithFixedRedirectURI(uri string) *ConfigBuilder; FromEnv() reads OAUTH_FIXED_REDIRECT_URI; proxy-mode validation now requires either RedirectURIs or FixedRedirectURI.
OAuth Handler Logic handlers.go	Added FixedRedirectURI to OAuth2Config; reworked HandleAuthorize, HandleCallback, and HandleToken to support both allowlist (RedirectURIs) and fixed-proxy (FixedRedirectURI) flows, including storing client redirect URIs, state signing/verification, security checks, and explicit rejection paths.
Client Registration / Metadata metadata.go	HandleRegister falls back to FixedRedirectURI when client provides no redirect_uris; adjusted single-value RedirectURIs handling and log messages to indicate the fallback source.

Sequence Diagram(s)

sequenceDiagram
    participant Client as OAuth Client
    participant Proxy as Proxy Server
    participant Provider as OAuth Provider

    rect rgb(220, 235, 255)
    note over Client,Proxy: Allowlist Mode (RedirectURIs)
    Client->>Proxy: GET /authorize?redirect_uri=client://cb
    Proxy->>Proxy: Validate redirect_uri against RedirectURIs allowlist
    Proxy->>Provider: Redirect to Provider with client's redirect_uri
    Provider->>Client: Callback to client's redirect_uri
    end

    rect rgb(220, 255, 220)
    note over Client,Proxy: Fixed Redirect Mode (FixedRedirectURI)
    Client->>Proxy: GET /authorize?redirect_uri=client://cb
    Proxy->>Proxy: Store client's redirect_uri, sign state
    Proxy->>Provider: Redirect to Provider with FixedRedirectURI
    Provider->>Proxy: Callback to FixedRedirectURI
    Proxy->>Proxy: Validate state, resolve stored client redirect_uri
    Proxy->>Client: Proxy callback to stored client redirect_uri
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

• Support allowlist mode and fixed redirect mode simultaneously #20: Implements FixedRedirectURI and combined allowlist + fixed-redirect handling, matching this PR's objectives.

Possibly related PRs

• Support refresh tokens #17: Alters HandleToken and redirect/PKCE handling in a way that likely overlaps with the token/redirect resolution changes here.

Poem

🐰 I hopped through config and found a new key,
FixedRedirectURI now guards the proxy tree,
State signed and stored, callbacks reroute right,
Allowlist or fixed — both paths take flight,
A little rabbit winks and nibbles with glee. 🥕

🚥 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 clearly summarizes the main objective: supporting both allowlist and fixed redirect modes simultaneously, which is the central change across all modified files.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ 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 86229d2 and 94d5f7a.

📒 Files selected for processing (3)

• config.go
• handlers.go
• metadata.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:

• config.go
• handlers.go
• metadata.go

🧠 Learnings (3)

📓 Common learnings

Learnt from: CR
Repo: tuannvm/oauth-mcp-proxy PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-13T17:03:53.298Z
Learning: Redirect URI validation must use explicit allowlist - all URIs must be in whitelist before authorization

📚 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

📚 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: Redirect URI validation must use explicit allowlist - all URIs must be in whitelist before authorization

Applied to files:

• handlers.go

🔇 Additional comments (13)

config.go (4)

14-17: LGTM - Clear field documentation.

The updated comments clearly distinguish between RedirectURIs (allowlist mode) and FixedRedirectURI (proxy callbacks), making the dual-mode behavior explicit to consumers.

---

93-95: LGTM - Validation correctly supports dual-mode configuration.

The updated validation logic properly requires at least one redirect mechanism (RedirectURIs or FixedRedirectURI) for proxy mode, aligning with the PR's goal of supporting both modes.

---

201-205: LGTM - Builder method follows existing pattern.

The WithFixedRedirectURI method correctly follows the fluent builder pattern established by other methods in ConfigBuilder.

---

329-329: LGTM - Environment variable wiring is correct.

The new OAUTH_FIXED_REDIRECT_URI environment variable is properly integrated into the builder chain.

metadata.go (1)

197-207: LGTM - Fallback logic is well-structured.

The registration handler correctly prioritizes:

1. Client-provided redirect URIs
2. FixedRedirectURI fallback
3. Single RedirectURIs value for backward compatibility

The log messages clearly indicate the source of the redirect URI, which aids debugging.

handlers.go (8)

43-44: LGTM - Field addition is consistent with Config.

The FixedRedirectURI field and its documentation mirror the corresponding field in Config.

---

188-205: LGTM - OAuth2Config initialization is complete.

All fields are properly mapped from Config to OAuth2Config, including the new FixedRedirectURI.

---

306-314: Clarify the security implications of allowlist-to-fixed fallthrough.

When RedirectURIs is configured but the client's URI is not in the allowlist, the code falls through to try fixed redirect mode. This means a client could bypass the allowlist by providing a localhost URI (which fixed mode accepts).

This behavior may be intentional (supporting both production clients via allowlist AND development tools via fixed mode), but it's worth documenting explicitly or adding a configuration option to disable fallthrough for stricter security.

Is the fallthrough from allowlist mode to fixed redirect mode intentional? If so, consider adding a comment explaining this design decision. If stricter security is needed, consider adding a config flag to disable fallthrough.

---

356-363: LGTM - Strong security constraints for fixed redirect mode.

The localhost-only restriction for fixed redirect mode is an appropriate security measure that prevents open redirect attacks while still supporting local development tools like mcp-remote. Based on learnings, this aligns with the requirement for explicit redirect URI validation.

---

365-381: LGTM - State signing provides integrity protection.

The HMAC-signed state containing the original redirect URI and client state is a solid approach to prevent tampering. This allows secure proxying of callbacks to localhost clients.

---

454-487: LGTM - Defense-in-depth callback validation.

Re-validating that the redirect URI is localhost after state verification (Line 470-474) is excellent defense-in-depth against potential key compromise scenarios.

---

551-554: LGTM - Token exchange redirect handling is consistent.

The token handler correctly uses FixedRedirectURI when the client URI is not in the allowlist, maintaining consistency with the authorize flow.

---

805-820: LGTM - isValidRedirectURI correctly handles empty allowlist.

Returning false for an empty RedirectURIs config is the correct behavior, allowing the calling code to fall through to fixed redirect mode if configured.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

---

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.}

[tuannvm]

@WhammyLeaf could you help resolve the conflict?

[WhammyLeaf]

@WhammyLeaf could you help resolve the conflict?

Of course 😄

[coderabbitai]

Actionable comments posted: 1

🤖 Fix all issues with AI agents

In `@handlers.go`:
- Around line 549-553: Inside the "authorization_code" switch branch you have an
unreachable check if grantType != "authorization_code" that calls h.logger.Error
and http.Error; remove that entire if block (the check and its body) so there is
no dead code referencing grantType, h.logger.Error or http.Error in this branch
and ensure any needed error handling remains only in the outer switch/default
branch.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 97c75a7 and 4a33e7f.

📒 Files selected for processing (3)

• config.go
• handlers.go
• metadata.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:

• metadata.go
• handlers.go
• config.go

🧠 Learnings (2)

📚 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:

• metadata.go

📚 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: Redirect URI validation must use explicit allowlist - all URIs must be in whitelist before authorization

Applied to files:

• handlers.go

🔇 Additional comments (11)

metadata.go (1)

197-206: LGTM! Redirect URI fallback logic is well-structured.

The cascading fallback from client-provided URIs → FixedRedirectURI → single RedirectURIs is clear and follows the PR's design intent. The trim operations ensure whitespace doesn't cause issues.

config.go (4)

14-17: LGTM! Config struct updated correctly.

The new FixedRedirectURI field is well-documented and follows the existing field naming convention.

---

93-95: LGTM! Validation logic correctly requires at least one redirect URI source.

The OR condition ensures proxy mode has a valid redirect URI configuration from either source.

---

201-206: LGTM! Builder method follows established pattern.

The new WithFixedRedirectURI method is consistent with other builder methods in style and functionality.

---

329-329: LGTM! Environment variable integration is correct.

The OAUTH_FIXED_REDIRECT_URI environment variable follows the existing naming convention.

handlers.go (6)

43-44: LGTM! OAuth2Config struct updated correctly.

The FixedRedirectURI field addition with documentation aligns with the config structure changes.

---

188-205: LGTM! Config mapping includes the new field.

The FixedRedirectURI field is correctly propagated from the source config.

---

304-392: Well-designed redirect URI strategy with comprehensive security controls.

The cascading approach (allowlist → fixed redirect → reject) is correctly implemented:

• Allowlist mode validates against configured URIs
• Fixed redirect mode restricts to localhost-only, preventing open redirect attacks
• HMAC-signed state protects against tampering
• Scheme validation and HTTPS enforcement add defense in depth

---

454-487: LGTM! Callback handling correctly triggers proxy mode based on FixedRedirectURI.

The defense-in-depth re-validation of localhost URIs even after state verification is a good security practice.

---

556-560: LGTM! Token exchange redirect logic aligns with authorization flow.

The fallback to FixedRedirectURI when the client URI isn't in the allowlist maintains consistency with the HandleAuthorize behavior.

---

612-634: LGTM! Token response construction is correct.

Tokens are properly included in the response for the client but are not logged. This complies with the coding guideline to avoid logging raw OAuth tokens.

_{✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.}

[WhammyLeaf]

@WhammyLeaf could you help resolve the conflict?

@tuannvm Done