opensource/coder
3
0
Fork 0
mirror of https://github.com/coder/coder.git synced 2026-09-24 15:04:27 +08:00
Code Issues Packages Projects Releases Wiki Activity

feat!: org-scope MCP server configs with RBAC (#27942)

Browse Source 
Moves `mcp_server_configs` from deployment scope to organization scope
so each organization fully controls the MCP servers its members can use
with Coder Agents.

## Summary

- Migration: adds `organization_id` (NOT NULL, FK) and keeps existing
rows as the default organization's originals with credentials intact.
Other organizations start with no MCP servers and configure their own;
nothing is copied across organizations. Chats outside the default
organization keep any now-cross-organization `mcp_server_ids` entries;
the runtime already ignores IDs that do not resolve in the chat's
organization, so no data rewrite is needed. Slug uniqueness becomes
`(organization_id, slug)`.
- RBAC: new org-scoped `ResourceMCPServerConfig` with regosql converter
and `GetAuthorizedMCPServerConfigs`; org admins get in-org CRUD, org
members get read (replaced by ACL evaluation in the follow-up ACL PR in
this stack).
- API: all config routes nest under the organization, matching
templates: `POST|GET
/api/experimental/organizations/{organization}/mcp-servers` and
`GET|PATCH|DELETE .../mcp-servers/{mcpserverconfig}` (plus
`oauth2/connect`), resolved by a read-only param middleware that
conceals read-denied and cross-organization access as 404. Two routes
stay on the frozen `/api/experimental/mcp/servers/{mcpServer}` block:
the OAuth2 callback (the redirect URI baked into existing AS-side client
registrations) and `oauth2/disconnect`, which must remain reachable by
users removed from the organization so they can still revoke their token
grant.
- Chat runtime: selection validation and generation resolve configs
strictly by IDs, enabled state, and the chat's organization in SQL;
requested duplicates are normalized; invalid or cross-org IDs are
rejected with the precise ID list. IDs already persisted on a chat are
exempt from message-time rejection so disabling a selected server never
blocks sends; generation skips servers that are no longer usable.
- Frontend: API layer and admin settings pages target the new endpoints.
The admin page manages the default organization's servers; the org
picker is tracked separately (CODAGT-714).

- Security hardening from review: OAuth user grants are additionally
bound to `oauth2_revocation_url` (changing it invalidates grants, and a
racing OAuth callback gets 409 instead of recreating a grant).
Stack-wide SSRF protection for MCP config-directed traffic was split
into its own PR at the top of this stack (#28242) to keep this diff
reviewable; this PR keeps main's existing discovery IP-range guard.
OAuth2 auto-discovery now completes before the config row is inserted: a
failed discovery persists nothing, and there is no provisional row that
concurrent updates could race against.

Two follow-up PRs in this stack were split out to keep this diff
reviewable: #28064 completes the swagger annotations for the moved
routes (main already ships these experimental MCP handlers unannotated),
and #28065 carries hardening fixes and regression pins on top of the
cutover.

- Force On enforcement (landed on main mid-review) is org-scoped: the
forced set is read per chat organization
(`GetForcedMCPServerConfigsByOrganization`), so another organization's
`force_on` server never attaches to a chat.

## Breaking changes (experimental API)

The MCP server config endpoints move from the deployment-scoped
`/api/experimental/mcp/servers` block to organization-nested paths:
`POST|GET /api/experimental/organizations/{organization}/mcp-servers`
and `GET|PATCH|DELETE
/api/experimental/organizations/{organization}/mcp-servers/{mcpserverconfig}`
(plus `oauth2/connect`). The old paths are removed, so API consumers
must supply an organization. Two routes intentionally stay on the frozen
`/api/experimental/mcp/servers/{mcpServer}` block: the OAuth2 callback
(its redirect URI is baked into existing AS-side client registrations)
and `oauth2/disconnect` (must remain reachable by users removed from the
organization). These endpoints are under `/api/experimental`, so no
deprecation window is provided.

## Rolling upgrades

During a rolling deploy, an old replica creating an MCP config can fail
the new `NOT NULL organization_id` constraint until it is upgraded
(reads are unaffected: old binaries' generated queries select their own
column lists). This matches the repo's existing precedent for additive
NOT NULL migrations (000562) and affects only the admin config-create
path in the upgrade window.

Upgrades are expected to run in scheduled maintenance downtime with the
database locked during migration, so the migration ships no
rolling-upgrade compatibility machinery. The down migration deletes
organization-created configs (their chat references are cleaned by the
000510 delete trigger) and restores deployment-wide slug uniqueness.

Part of the MCP org-separation stack (CODAGT-711 -> CODAGT-717 audit ->
CODAGT-712 ACLs -> CODAGT-806 token RBAC).

Closes https://linear.app/codercom/issue/CODAGT-711

UAT: validated end to end on a two-org dogfood deployment, including a
real pre-migration to post-migration upgrade, cross-org isolation
(404s), same-slug-two-orgs, chat selection gating, and a live MCP tool
call through the org-scoped generation path. The migration was later
revised to keep existing rows in the default organization only (no
per-organization copies); that revision is covered by the migration test
suite.

> Mux (AI agent) authored this PR on Mike's behalf.

<!-- mux-attribution: model=claude-fable-5 thinking=high -->

---------

Co-authored-by: Mathias Fredriksson <mafredri@gmail.com>
This commit is contained in:
Michael Suchacz
2026-08-19 18:04:13 +00:00
committed by GitHub
co-authored by Mathias Fredriksson
parent 7ff1278ab3
commit 443e3b9b80
72 changed files with 3649 additions and 988 deletions
Download Patch File Download Diff File Expand all files Collapse all files
coderd/database/dbmock/dbmock.go
Generated
+83 -39
View File
@@ -1035,6 +1035,20 @@ func (mr *MockStoreMockRecorder) DeleteMCPServerUserToken(ctx, arg any) *gomock.
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteMCPServerUserToken", reflect.TypeOf((*MockStore)(nil).DeleteMCPServerUserToken), ctx, arg)
}
// DeleteMCPServerUserTokensByConfigID mocks base method.
func (m *MockStore) DeleteMCPServerUserTokensByConfigID(ctx context.Context, mcpServerConfigID uuid.UUID) error {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "DeleteMCPServerUserTokensByConfigID", ctx, mcpServerConfigID)
ret0, _ := ret[0].(error)
return ret0
}
// DeleteMCPServerUserTokensByConfigID indicates an expected call of DeleteMCPServerUserTokensByConfigID.
func (mr *MockStoreMockRecorder) DeleteMCPServerUserTokensByConfigID(ctx, mcpServerConfigID any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteMCPServerUserTokensByConfigID", reflect.TypeOf((*MockStore)(nil).DeleteMCPServerUserTokensByConfigID), ctx, mcpServerConfigID)
}
// DeleteOAuth2ProviderAppByClientID mocks base method.
func (m *MockStore) DeleteOAuth2ProviderAppByClientID(ctx context.Context, id uuid.UUID) error {
m.ctrl.T.Helper()
@@ -2505,6 +2519,21 @@ func (mr *MockStoreMockRecorder) GetAuthorizedConnectionLogsOffset(ctx, arg, pre
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAuthorizedConnectionLogsOffset", reflect.TypeOf((*MockStore)(nil).GetAuthorizedConnectionLogsOffset), ctx, arg, prepared)
}
// GetAuthorizedMCPServerConfigs mocks base method.
func (m *MockStore) GetAuthorizedMCPServerConfigs(ctx context.Context, organizationID uuid.UUID, prepared rbac.PreparedAuthorized) ([]database.MCPServerConfig, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetAuthorizedMCPServerConfigs", ctx, organizationID, prepared)
ret0, _ := ret[0].([]database.MCPServerConfig)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetAuthorizedMCPServerConfigs indicates an expected call of GetAuthorizedMCPServerConfigs.
func (mr *MockStoreMockRecorder) GetAuthorizedMCPServerConfigs(ctx, organizationID, prepared any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAuthorizedMCPServerConfigs", reflect.TypeOf((*MockStore)(nil).GetAuthorizedMCPServerConfigs), ctx, organizationID, prepared)
}
// GetAuthorizedTemplates mocks base method.
func (m *MockStore) GetAuthorizedTemplates(ctx context.Context, arg database.GetTemplatesWithFilterParams, prepared rbac.PreparedAuthorized) ([]database.Template, error) {
m.ctrl.T.Helper()
@@ -3735,19 +3764,34 @@ func (mr *MockStoreMockRecorder) GetEnabledChatModelConfigs(ctx any) *gomock.Cal
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEnabledChatModelConfigs", reflect.TypeOf((*MockStore)(nil).GetEnabledChatModelConfigs), ctx)
}
// GetEnabledMCPServerConfigs mocks base method.
func (m *MockStore) GetEnabledMCPServerConfigs(ctx context.Context) ([]database.MCPServerConfig, error) {
// GetEnabledMCPServerConfigsByOrganization mocks base method.
func (m *MockStore) GetEnabledMCPServerConfigsByOrganization(ctx context.Context, organizationID uuid.UUID) ([]database.MCPServerConfig, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetEnabledMCPServerConfigs", ctx)
ret := m.ctrl.Call(m, "GetEnabledMCPServerConfigsByOrganization", ctx, organizationID)
ret0, _ := ret[0].([]database.MCPServerConfig)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetEnabledMCPServerConfigs indicates an expected call of GetEnabledMCPServerConfigs.
func (mr *MockStoreMockRecorder) GetEnabledMCPServerConfigs(ctx any) *gomock.Call {
// GetEnabledMCPServerConfigsByOrganization indicates an expected call of GetEnabledMCPServerConfigsByOrganization.
func (mr *MockStoreMockRecorder) GetEnabledMCPServerConfigsByOrganization(ctx, organizationID any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEnabledMCPServerConfigs", reflect.TypeOf((*MockStore)(nil).GetEnabledMCPServerConfigs), ctx)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEnabledMCPServerConfigsByOrganization", reflect.TypeOf((*MockStore)(nil).GetEnabledMCPServerConfigsByOrganization), ctx, organizationID)
}
// GetEnabledMCPServerConfigsByOrganizationAndIDs mocks base method.
func (m *MockStore) GetEnabledMCPServerConfigsByOrganizationAndIDs(ctx context.Context, arg database.GetEnabledMCPServerConfigsByOrganizationAndIDsParams) ([]database.MCPServerConfig, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetEnabledMCPServerConfigsByOrganizationAndIDs", ctx, arg)
ret0, _ := ret[0].([]database.MCPServerConfig)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetEnabledMCPServerConfigsByOrganizationAndIDs indicates an expected call of GetEnabledMCPServerConfigsByOrganizationAndIDs.
func (mr *MockStoreMockRecorder) GetEnabledMCPServerConfigsByOrganizationAndIDs(ctx, arg any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetEnabledMCPServerConfigsByOrganizationAndIDs", reflect.TypeOf((*MockStore)(nil).GetEnabledMCPServerConfigsByOrganizationAndIDs), ctx, arg)
}
// GetExternalAgentTokensByTemplateID mocks base method.
@@ -3870,19 +3914,19 @@ func (mr *MockStoreMockRecorder) GetFilteredInboxNotificationsByUserID(ctx, arg
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetFilteredInboxNotificationsByUserID", reflect.TypeOf((*MockStore)(nil).GetFilteredInboxNotificationsByUserID), ctx, arg)
}
// GetForcedMCPServerConfigs mocks base method.
func (m *MockStore) GetForcedMCPServerConfigs(ctx context.Context) ([]database.MCPServerConfig, error) {
// GetForcedMCPServerConfigsByOrganization mocks base method.
func (m *MockStore) GetForcedMCPServerConfigsByOrganization(ctx context.Context, organizationID uuid.UUID) ([]database.MCPServerConfig, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetForcedMCPServerConfigs", ctx)
ret := m.ctrl.Call(m, "GetForcedMCPServerConfigsByOrganization", ctx, organizationID)
ret0, _ := ret[0].([]database.MCPServerConfig)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetForcedMCPServerConfigs indicates an expected call of GetForcedMCPServerConfigs.
func (mr *MockStoreMockRecorder) GetForcedMCPServerConfigs(ctx any) *gomock.Call {
// GetForcedMCPServerConfigsByOrganization indicates an expected call of GetForcedMCPServerConfigsByOrganization.
func (mr *MockStoreMockRecorder) GetForcedMCPServerConfigsByOrganization(ctx, organizationID any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetForcedMCPServerConfigs", reflect.TypeOf((*MockStore)(nil).GetForcedMCPServerConfigs), ctx)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetForcedMCPServerConfigsByOrganization", reflect.TypeOf((*MockStore)(nil).GetForcedMCPServerConfigsByOrganization), ctx, organizationID)
}
// GetGitSSHKey mocks base method.
@@ -4320,49 +4364,49 @@ func (mr *MockStoreMockRecorder) GetMCPServerConfigByID(ctx, id any) *gomock.Cal
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMCPServerConfigByID", reflect.TypeOf((*MockStore)(nil).GetMCPServerConfigByID), ctx, id)
}
// GetMCPServerConfigBySlug mocks base method.
func (m *MockStore) GetMCPServerConfigBySlug(ctx context.Context, slug string) (database.MCPServerConfig, error) {
// GetMCPServerConfigByIDForUpdate mocks base method.
func (m *MockStore) GetMCPServerConfigByIDForUpdate(ctx context.Context, id uuid.UUID) (database.MCPServerConfig, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetMCPServerConfigBySlug", ctx, slug)
ret := m.ctrl.Call(m, "GetMCPServerConfigByIDForUpdate", ctx, id)
ret0, _ := ret[0].(database.MCPServerConfig)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetMCPServerConfigBySlug indicates an expected call of GetMCPServerConfigBySlug.
func (mr *MockStoreMockRecorder) GetMCPServerConfigBySlug(ctx, slug any) *gomock.Call {
// GetMCPServerConfigByIDForUpdate indicates an expected call of GetMCPServerConfigByIDForUpdate.
func (mr *MockStoreMockRecorder) GetMCPServerConfigByIDForUpdate(ctx, id any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMCPServerConfigBySlug", reflect.TypeOf((*MockStore)(nil).GetMCPServerConfigBySlug), ctx, slug)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMCPServerConfigByIDForUpdate", reflect.TypeOf((*MockStore)(nil).GetMCPServerConfigByIDForUpdate), ctx, id)
}
// GetMCPServerConfigs mocks base method.
func (m *MockStore) GetMCPServerConfigs(ctx context.Context) ([]database.MCPServerConfig, error) {
// GetMCPServerConfigByOrganizationAndSlug mocks base method.
func (m *MockStore) GetMCPServerConfigByOrganizationAndSlug(ctx context.Context, arg database.GetMCPServerConfigByOrganizationAndSlugParams) (database.MCPServerConfig, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetMCPServerConfigs", ctx)
ret := m.ctrl.Call(m, "GetMCPServerConfigByOrganizationAndSlug", ctx, arg)
ret0, _ := ret[0].(database.MCPServerConfig)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetMCPServerConfigByOrganizationAndSlug indicates an expected call of GetMCPServerConfigByOrganizationAndSlug.
func (mr *MockStoreMockRecorder) GetMCPServerConfigByOrganizationAndSlug(ctx, arg any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMCPServerConfigByOrganizationAndSlug", reflect.TypeOf((*MockStore)(nil).GetMCPServerConfigByOrganizationAndSlug), ctx, arg)
}
// GetMCPServerConfigsByOrganization mocks base method.
func (m *MockStore) GetMCPServerConfigsByOrganization(ctx context.Context, organizationID uuid.UUID) ([]database.MCPServerConfig, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetMCPServerConfigsByOrganization", ctx, organizationID)
ret0, _ := ret[0].([]database.MCPServerConfig)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetMCPServerConfigs indicates an expected call of GetMCPServerConfigs.
func (mr *MockStoreMockRecorder) GetMCPServerConfigs(ctx any) *gomock.Call {
// GetMCPServerConfigsByOrganization indicates an expected call of GetMCPServerConfigsByOrganization.
func (mr *MockStoreMockRecorder) GetMCPServerConfigsByOrganization(ctx, organizationID any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMCPServerConfigs", reflect.TypeOf((*MockStore)(nil).GetMCPServerConfigs), ctx)
}
// GetMCPServerConfigsByIDs mocks base method.
func (m *MockStore) GetMCPServerConfigsByIDs(ctx context.Context, ids []uuid.UUID) ([]database.MCPServerConfig, error) {
m.ctrl.T.Helper()
ret := m.ctrl.Call(m, "GetMCPServerConfigsByIDs", ctx, ids)
ret0, _ := ret[0].([]database.MCPServerConfig)
ret1, _ := ret[1].(error)
return ret0, ret1
}
// GetMCPServerConfigsByIDs indicates an expected call of GetMCPServerConfigsByIDs.
func (mr *MockStoreMockRecorder) GetMCPServerConfigsByIDs(ctx, ids any) *gomock.Call {
mr.mock.ctrl.T.Helper()
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMCPServerConfigsByIDs", reflect.TypeOf((*MockStore)(nil).GetMCPServerConfigsByIDs), ctx, ids)
return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMCPServerConfigsByOrganization", reflect.TypeOf((*MockStore)(nil).GetMCPServerConfigsByOrganization), ctx, organizationID)
}
// GetMCPServerUserToken mocks base method.