MCP Server Part 10: OAuth discovery#3767
Conversation
3e146f8 to
0a82ca0
Compare
1d9eb46 to
7121548
Compare
fd13290 to
5c45baf
Compare
7121548 to
2e87a4b
Compare
01e8744 to
1366bd4
Compare
2e87a4b to
c2801af
Compare
f4254e2 to
3ef3409
Compare
cff5ebd to
41a5689
Compare
| server. Auth enforcement is the responsibility of the hosting platform | ||
| (e.g. Plotly Cloud gateway, Dash Embedded, or a reverse proxy). | ||
| """ | ||
| well_known_path = urljoin("/.well-known/oauth-protected-resource/", mcp_path) |
There was a problem hiding this comment.
Could you double check the URL construction? It looks like the final result might not be correct. Shouldn't the pathname prefix be included before the MCP path?
There was a problem hiding this comment.
As discussed offline, I am now raising when mcp_authorization_server is used in conjunction with requests_pathname_prefix since we cannot accommodate both.
d18681e to
8a24973
Compare
8a24973 to
2bbbc02
Compare
camdecoster
left a comment
There was a problem hiding this comment.
I left some suggestions, but this should work as is.
| "`requests_pathname_prefix`. " | ||
| "Authorization must be implemented at the platform level " | ||
| "(see https://www.rfc-editor.org/rfc/rfc9728#section-3). " | ||
| "Remove the mcp_authorization_server parameter." |
There was a problem hiding this comment.
It might be worth it to guide the user a bit more on how to fix the issue with a URL template or something. At a minimum, I'd update the last sentence to be a little less of a command.
| "Remove the mcp_authorization_server parameter." | |
| "Remove the `mcp_authorization_server` or `requests_pathname_prefix` parameter to avoid this error." |
| json.dumps( | ||
| { | ||
| "resource": _url_from_path( | ||
| app, app.config.requests_pathname_prefix, mcp_path |
There was a problem hiding this comment.
The prefix will only ever be "/" here, right?
| (e.g. Plotly Cloud gateway, Dash Embedded, or a reverse proxy). | ||
| """ | ||
| if app.config.requests_pathname_prefix != "/": | ||
| raise ValueError( |
There was a problem hiding this comment.
Raising here means that the error will get caught in the try/except block in dash.py. Would it be better to raise earlier when first checking on config options?
| ) | ||
| app.routes.append(mcp_url) | ||
|
|
||
| if mcp_authorization_server: |
There was a problem hiding this comment.
Should this check happen before registering the routes? If the auth server has been configured, then the routes get registered but MCP can't be enabled. It might be better to fail earlier.
Summary
Support OAuth so MCP clients can discover how to authenticate against an authorization server when connecting to the Dash MCP endpoint. Implements the discovery half of the MCP Authorization spec.
mcp_authorization_serverconstructor arg (andDASH_MCP_AUTHORIZATION_SERVERenv var). When set, registers an RFC 9728 Protected Resource Metadata endpoint at/.well-known/oauth-protected-resource/<mcp-path>advertising the authorization server.WWW-Authenticateheaders are the responsibility of the hosting platform (middleware, reverse proxy, etc.).