feat: backwards compatible oauth for remote sessions#3005
Conversation
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 66df5fb934
ℹ️ 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".
qstearns
force-pushed
the
quinn/compat-oauth
branch
from
66df5fb to
3a3cfcb
Compare
qstearns
force-pushed
the
quinn/compat-oauth
branch
from
3821012 to
555ab9b
Compare
🚀 Preview Environment (PR #3005)Preview URL: https://pr-3005.dev.getgram.ai
Gram Preview Bot |
github-merge-queue
Bot
removed this pull request from the merge queue due to a conflict with the base branch
…istered clients (#2998) I was hoping we would not need this, but there are enough manually set up clients that have registered callback URLs in external systems that it's worth supporting the old callback URL. This PR does two things: 1. It makes the new auth challenge treat the old callback as a jumpthrough enpdoint by passing it a special flag (telling it to redirect to the new callback) 2. It automatically registers clients cloned from oauth_proxy_servers to inherit this behavior. Once this change is live, we should be able to migrate all remaining existing oauth proxy clients over to user sessions (those that are receiving traffic at least) Stacked on #2997. - **Authorize** (`challenge.go`): when `client.LegacyCallbackUrl` is true, set `redirect_uri = <serverURL>/oauth/callback` and wrap `state` in a JSON envelope `{"remote_sessions":"true","state_id":"<id>"}` so the upstream still matches its strict-registered redirect_uri. The stored `RemoteLoginState.RedirectURI` matches, so the later token-endpoint `redirect_uri` form param matches too. - **Forward** (`oauth.handleAuthorizationCallback`): after parsing state JSON, if `remote_sessions=true`, 302 to `<serverURL>/mcp/remote_login_callback?state=<state_id>&code=…&error=…&error_description=…`. The new handler finishes the exchange unchanged. - **Clone** (`CloneClientFromOAuthProxyProvider`): every cloned proxy → remote_session_client now persists `legacy_callback_url=true`, since the upstream registration already points at `/oauth/callback`. Deprecation plan lives in the schema comment from #2997 — when `oauth_proxy_servers` is fully removed, `/oauth/callback` collapses to just the forwarding shim, and once traffic to it drains, the column can be dropped (expand-contract). - [ ] `go build ./...` (server) — done locally - [ ] Unit + e2e: a legacy-flagged client end-to-end (authorize → upstream → `/oauth/callback` → `/mcp/remote_login_callback` → connect) - [ ] Non-flagged clients still take the direct `/mcp/remote_login_callback` path - [ ] Cloning an oauth_proxy_provider produces a client with `legacy_callback_url=true` --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…onClient exhaustruct lint requires every field of CreateRemoteSessionClientParams to be set explicitly. The non-clone CreateRemoteSessionClient handler was relying on the zero value; spell out LegacyCallbackUrl: false so freshly registered clients keep the default while the clone path retains true. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
qstearns
force-pushed
the
quinn/compat-oauth
branch
from
555ab9b to
7b60e9e
Compare
4 participants
accidentally merged #2998 into the migration instead of main