Skip to content

fix: build MCP WWW-Authenticate metadata URL from host_name with port (#196)#201

Merged
buildswithpaul merged 3 commits into
developfrom
bug/196-oauth-metadata-port
Jun 8, 2026
Merged

fix: build MCP WWW-Authenticate metadata URL from host_name with port (#196)#201
buildswithpaul merged 3 commits into
developfrom
bug/196-oauth-metadata-port

Conversation

@buildswithpaul

Copy link
Copy Markdown
Owner

Summary

Fixes #196 — the MCP WWW-Authenticate 401 header advertised an OAuth resource_metadata URL that dropped a configured non-standard port, breaking the OAuth handshake behind port-restricted networks.

Root cause

fac_endpoint.py built the resource_metadata URL from Frappe's frappe.oauth.get_server_url():

def get_server_url():
    request_url = urlparse(frappe.request.url)
    request_url = f"{request_url.scheme}://{request_url.netloc}"
    return frappe.get_value("Social Login Key", "frappe", "base_url") or request_url

This reconstructs host/port from the (internal, proxied) request URL or the Social Login Key base_url and never consults the configured host_name. For a deployment whose public URL is https://mysite.example.net:8000 (ISP blocks 80/443; non-standard port forwards to internal nginx), the advertised URL came back as …example.net/.well-known/oauth-protected-resource — no port. The client then tried the implied :443, which is blocked, and failed with "couldn't reach the MCP server".

The discovery endpoints (/.well-known/oauth-*, openid-configuration) already build URLs from _get_public_base_url(), which honors host_name verbatim (scheme + port) — that was the earlier fix for the http:// issue (#156). Only the WWW-Authenticate header was still on get_server_url(), which is why the reporter saw the discovery URLs render :8000 correctly while the header dropped it.

Changes

  • Expose get_public_base_url() in oauth_discovery.py as the shared accessor for the canonical public base URL.
  • fac_endpoint.py builds all six resource_metadata URLs (bearer-not-found, token-validation-error, API-auth-error, API-other-error, no-auth fallback, and the HEAD connectivity probe) from get_public_base_url() instead of get_server_url(). Port is now preserved.

No behavior change when host_name is unset or has no scheme — the existing get_server_url() fallback inside _get_public_base_url() is unchanged.

Tests

test_oauth_metadata_port.py:

  • get_public_base_url() returns a host_name with a non-standard port verbatim, without consulting get_server_url(); and still falls back when host_name has no scheme.
  • The MCP 401 WWW-Authenticate resource_metadata URL keeps the port on both the unauthenticated and invalid-bearer-token paths.

The header tests fail against the old get_server_url() code and pass with the fix. Also verified live: with host_name = https://mysite.example.net:8000, get_public_base_url()https://mysite.example.net:8000. Existing test_oauth_cors (8) still passes.

Checklist

  • Conventional commit format
  • Targets develop
  • Passes linters (pre-commit: ruff + Frappe semgrep)
  • Tests pass
  • No DocType changes (no migration needed)

…#196)

The MCP endpoint's 401 WWW-Authenticate header advertises the OAuth
resource_metadata URL using frappe.oauth.get_server_url(). That helper
reconstructs the host/port from the request URL or the Social Login Key
base_url and does not honor a configured host_name's non-standard port, so
the advertised URL dropped the port (e.g. :8000). Behind a port-restricted
network where the public URL is only reachable on a non-standard port, the
client then tries the implied :443 and the OAuth handshake fails with
"couldn't reach the MCP server".

The discovery endpoints already build URLs from _get_public_base_url(), which
uses host_name verbatim (scheme + port) — only the WWW-Authenticate header was
still on get_server_url().

- Expose get_public_base_url() in oauth_discovery as the shared accessor for
  the canonical public base URL.
- fac_endpoint now builds all six resource_metadata URLs (bearer-not-found,
  token-validation-error, API-auth-error, API-other-error, no-auth, and the
  HEAD probe) from get_public_base_url() instead of get_server_url(), so the
  configured port is preserved.

Add test_oauth_metadata_port: get_public_base_url returns a host_name with a
non-standard port verbatim (and does not consult get_server_url), and the MCP
401 WWW-Authenticate resource_metadata URL keeps the port on both the
unauthenticated and invalid-bearer paths. The header tests fail against the
old get_server_url code and pass with the fix.

Fixes #196
@buildswithpaul buildswithpaul added the bug Something isn't working label Jun 8, 2026
@buildswithpaul buildswithpaul merged commit 4128993 into develop Jun 8, 2026
9 checks passed
@buildswithpaul buildswithpaul deleted the bug/196-oauth-metadata-port branch June 8, 2026 10:41
@hameedacpa

Copy link
Copy Markdown

@buildswithpaul
When this script will be updated in the main branch ?

@buildswithpaul

Copy link
Copy Markdown
Owner Author

@hameedacpa

This iwll be part the release 2.4.4 which will happen in a couple of days as I'm testing other implementations too.

@hameedacpa

Copy link
Copy Markdown

@hameedacpa

This iwll be part the release 2.4.4 which will happen in a couple of days as I'm testing other implementations too.

Thank you Paul, You are doing great and important job in moving and connecting ERPNext to the AI Era

@buildswithpaul

Copy link
Copy Markdown
Owner Author

@hameedacpa Thank you for the kind words. This PR has been merged to the main branch and the latest version of FAC is 2.5.0

@hameedacpa

Copy link
Copy Markdown

Thank you Paul @paulpromantia
Actually it is not working and it is giving Claude Error as following:
"Couldn't reach the MCP server. You can check the server URL and verify the server is running. If this persists, share this reference with support: "ofid_c1f8d872aca3649d""
You can contact me on WhatsApp to give you access to my ERPNext to test (+971507948028)
thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants