feat: add MCP server instance OAuth URL route#13
Merged
Conversation
07285af to
fc79834
Compare
…stream sync errcheck + revive unused-parameter findings in generic_oauth_validation_test.go, oidcjwt/smokeclient/main.go, and oidcjwt/testutil/testutil.go. These predate this branch (present on main after the upstream sync); golangci-lint v2.11.4 flags them and blocks every PR. Verified: golangci-lint run -> 0 issues.
This was referenced Jun 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds a per-user, instance-scoped OAuth-URL route to the MCP server-instances API:
It resolves the instance, enforces that the caller owns it (
Spec.UserID == req.User.GetUID()), resolves the underlying server + config, and returnsCheckForMCPAuth(...)as:{ "oauthURL": "<authorize-url>" } // user must complete OAuth { "oauthURL": "" } // authorized, or server uses no OAuthChanges (3 files, +32/−6):
pkg/api/handlers/serverinstances.go— newServerInstancesHandler.GetOAuthURL;mcpOAuthChecker MCPOAuthCheckerthreaded into the handler +NewServerInstancesHandler.pkg/api/router/router.go— wireoauthCheckerinto the constructor and register the route alongside the other instance routes.pkg/api/authz/resources.go— authz entry for the new path.Why
The existing
MCPHandler.GetOAuthURLis server-scoped (…/servers/{mcp_server_id}/oauth-url). Studio's per-user MCP setup needs to know whether a specific user's instance still requires authorization, keyed byinstance.Name(the canonical per-instancemcpID). This route is the instance-scoped analogue, modeled directly on the server-scoped handler.Studio consumes it for the user-setup lifecycle (
startOAuthand the per-row OAuth-status probe-and-cache): an emptyoauthURLflips the stored per-useroauth_connectedflag to true; a non-empty URL is opened for the user to authorize.Downstream dependency
The Studio branch
feat/add-user-mcp-server(accelerate-data/vd-studio) depends on this route. This PR must merge and a new Obot image must be published before the Studio user OAuth-completion flow works end to end — until then Studio validates that path only at the contract/unit level.Safety gate (re-verified in this checkout)
CreateServerInstancebinds instances to a catalogv1.MCPServerviamcpServerID;ListServersFromAllSourcesreturns catalog server projections withCatalogEntryID/Configured/MissingOAuthCredentials; ACR matching is resource-scoped. NoSystemMCPServer → MCPServerbridge exists in the instance-creation path. This route adds no new family coupling.Verification
go build ./...— OKgo test ./pkg/api/handlers ./pkg/api/router— 239 passedNote / follow-up
No Go unit test is added for
GetOAuthURLin this PR (the existing suite passes but does not exercise the new handler directly). Happy to add a handler test for the ownership check + the empty-vs-URL response shaping if preferred before merge.