Support OAuth client metadata URLs for MCP login#22531
Conversation
|
All contributors have signed the CLA ✍️ ✅ |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 687878e622
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (&Method::Get, "/.well-known/oauth-authorization-server") => { | ||
| let body = serde_json::json!({ | ||
| "authorization_endpoint": format!("{base_url_for_thread}/authorize"), | ||
| "token_endpoint": format!("{base_url_for_thread}/token"), | ||
| "registration_endpoint": format!("{base_url_for_thread}/register"), | ||
| "response_types_supported": ["code"], | ||
| "client_id_metadata_document_supported": supports_metadata_url, | ||
| }); |
There was a problem hiding this comment.
Fix the OAuth mock so the new tests pass
With this discovery mock in place, the two new OAuth tests fail before reaching their assertions: cargo test --manifest-path codex-rs/Cargo.toml -p codex-rmcp-client perform_oauth_login --no-fail-fast reports both authorization_uses_client_metadata_url_as_client_id_when_supplied and authorization_omits_client_metadata_url_by_default panicking at start_test_oauth_flow with No authorization support detected. Please adjust the mock discovery setup so OAuthState::start_authorization_with_metadata_url can discover valid authorization support; otherwise the targeted rmcp-client test suite is red.
Useful? React with 👍 / 👎.
b845246 to
103c3ec
Compare
103c3ec to
b424eb1
Compare
b424eb1 to
12d375d
Compare
|
Closing this pull request because it has had no updates for more than 14 days. If you plan to continue working on it, feel free to reopen or open a new PR. |
45455f5 to
709c3d0
Compare
709c3d0 to
8966c1e
Compare
|
Thanks for working on this, @stevenlee-oai! Excited to see CIMD support moving forward here. Do you have a rough sense of whether this is expected to land in an upcoming Codex release? It would help me plan around it for the MCP servers I’m building. No worries if there isn’t an ETA yet. |
Codex Thread 019e2258-dbd7-7270-9ed6-9492d07a01b1
Summary
clientMetadataUrlBaseto app-server MCP OAuth login params and regenerate schema/TypeScript fixturesNone, preserving dynamic registration unless a client explicitly supplies a CIMD metadata URL base{base}/{callback_id}/client.json?redirect_uri=...&token_endpoint_auth_method=noneso web serves the public-client CIMD document for local Codex token exchangenonetoken endpoint auth methods, since local Codex cannot complete aprivate_key_jwttoken exchangeRollout / compatibility
This is stacked on #20237 so the DCR path already uses callback-scoped loopback redirects.
clientMetadataUrlBaseis optional/nullable and omitted by existing callers, so this can land before any desktop/app caller sends it. Older app-server builds ignore unknown client fields and continue using DCR. Servers that omit or malformtoken_endpoint_auth_methods_supportedremain compatible for now; servers that explicitly excludenonefail before Codex sends the CIMD authorization request.Test plan
cargo run --manifest-path codex-rs/Cargo.toml -p codex-app-server-protocol --bin write_schema_fixturescargo fmt --all --manifest-path codex-rs/Cargo.tomlcargo test --manifest-path codex-rs/Cargo.toml -p codex-rmcp-client perform_oauth_logincargo test --manifest-path codex-rs/Cargo.toml -p codex-app-server-protocolcargo check --manifest-path codex-rs/Cargo.toml -p codex-app-server -p codex-cli -p codex-core./tools/argument-comment-lint/run-prebuilt-linter.py -p codex-rmcp-clientgit diff --check