Skip to content

Conversation

@jrf0110
Copy link
Contributor

@jrf0110 jrf0110 commented Jan 22, 2026

Addresses feedback in #5297

  • Re-enable the RFC 8707 resource parameter in the authorization URL
  • Add warning logs to capture specific failures during RFC 8414 and OIDC metadata discovery to improve observability

- Re-enable the RFC 8707 resource parameter in the authorization URL
- Add warning logs to capture specific failures during RFC 8414 and OIDC metadata discovery to improve observability
@changeset-bot
Copy link

changeset-bot bot commented Jan 22, 2026

⚠️ No Changeset found

Latest commit: 14d7bba

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@kiloconnect
Copy link
Contributor

kiloconnect bot commented Jan 22, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

This PR makes two changes to the MCP OAuth service:

  1. Added debug logging in McpAuthorizationDiscovery.ts - console.warn statements now log errors when RFC 8414 or OIDC metadata fetching fails. This improves debuggability.

  2. Re-enabled resource parameter in McpOAuthService.ts - The RFC 8707 resource parameter is now included in OAuth authorization requests.

Other Observations (not in diff)

The comment on lines 187-188 of McpOAuthService.ts states:

"Note: We don't include the 'resource' parameter by default as some servers (like Cloudflare) don't support RFC 8707 and return internal server error"

However, the code now does include the resource parameter. Consider updating this comment to reflect the new behavior, or document why the change was made despite the Cloudflare compatibility concern.

Files Reviewed (2 files)
  • src/services/mcp/oauth/McpAuthorizationDiscovery.ts - Added debug logging
  • src/services/mcp/oauth/McpOAuthService.ts - Re-enabled resource parameter

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.

3 participants