diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index a801739c41..e033f75b2e 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -1603,6 +1603,102 @@ const docTemplate = `{ } } }, + "/api/experimental/organizations/{organization}/mcp-servers/{mcpserverconfig}/acl": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "MCP" + ], + "summary": "Get MCP server config ACL", + "operationId": "get-mcp-server-config-acl", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "format": "uuid", + "description": "MCP server config ID", + "name": "mcpserverconfig", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.MCPServerConfigACL" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "patch": { + "consumes": [ + "application/json" + ], + "tags": [ + "MCP" + ], + "summary": "Update MCP server config ACL", + "operationId": "update-mcp-server-config-acl", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "format": "uuid", + "description": "MCP server config ID", + "name": "mcpserverconfig", + "in": "path", + "required": true + }, + { + "description": "Update MCP server config ACL request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.UpdateMCPServerConfigACLRequest" + } + } + ], + "responses": { + "204": { + "description": "No Content" + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, "/api/experimental/organizations/{organization}/mcp-servers/{mcpserverconfig}/oauth2/connect": { "get": { "tags": [ @@ -16879,6 +16975,7 @@ const docTemplate = `{ "mcp_server_config:create", "mcp_server_config:delete", "mcp_server_config:read", + "mcp_server_config:share", "mcp_server_config:update", "notification_message:*", "notification_message:create", @@ -17124,6 +17221,7 @@ const docTemplate = `{ "APIKeyScopeMcpServerConfigCreate", "APIKeyScopeMcpServerConfigDelete", "APIKeyScopeMcpServerConfigRead", + "APIKeyScopeMcpServerConfigShare", "APIKeyScopeMcpServerConfigUpdate", "APIKeyScopeNotificationMessageAll", "APIKeyScopeNotificationMessageCreate", @@ -22265,6 +22363,122 @@ const docTemplate = `{ } } }, + "codersdk.MCPServerConfigACL": { + "type": "object", + "properties": { + "groups": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.MCPServerConfigGroup" + } + }, + "users": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.MCPServerConfigUser" + } + } + } + }, + "codersdk.MCPServerConfigGroup": { + "type": "object", + "properties": { + "avatar_url": { + "type": "string", + "format": "uri" + }, + "display_name": { + "type": "string" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "members": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.ReducedUser" + } + }, + "name": { + "type": "string" + }, + "organization_display_name": { + "type": "string" + }, + "organization_id": { + "type": "string", + "format": "uuid" + }, + "organization_name": { + "type": "string" + }, + "quota_allowance": { + "type": "integer" + }, + "role": { + "enum": [ + "read" + ], + "allOf": [ + { + "$ref": "#/definitions/codersdk.MCPServerConfigRole" + } + ] + }, + "source": { + "$ref": "#/definitions/codersdk.GroupSource" + }, + "total_member_count": { + "description": "How many members are in this group. Shows the total count,\neven if the user is not authorized to read group member details.\nMay be greater than ` + "`" + `len(Group.Members)` + "`" + `.", + "type": "integer" + } + } + }, + "codersdk.MCPServerConfigRole": { + "type": "string", + "enum": [ + "read", + "" + ], + "x-enum-varnames": [ + "MCPServerConfigRoleRead", + "MCPServerConfigRoleDeleted" + ] + }, + "codersdk.MCPServerConfigUser": { + "type": "object", + "required": [ + "id", + "username" + ], + "properties": { + "avatar_url": { + "type": "string", + "format": "uri" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + }, + "role": { + "enum": [ + "read" + ], + "allOf": [ + { + "$ref": "#/definitions/codersdk.MCPServerConfigRole" + } + ] + }, + "username": { + "type": "string" + } + } + }, "codersdk.MCPServerOAuth2DisconnectResponse": { "type": "object", "properties": { @@ -26948,6 +27162,23 @@ const docTemplate = `{ } } }, + "codersdk.UpdateMCPServerConfigACLRequest": { + "type": "object", + "properties": { + "group_roles": { + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/codersdk.MCPServerConfigRole" + } + }, + "user_roles": { + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/codersdk.MCPServerConfigRole" + } + } + } + }, "codersdk.UpdateMCPServerConfigRequest": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 5962b014af..aae3f6f6e1 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1432,6 +1432,94 @@ } } }, + "/api/experimental/organizations/{organization}/mcp-servers/{mcpserverconfig}/acl": { + "get": { + "produces": ["application/json"], + "tags": ["MCP"], + "summary": "Get MCP server config ACL", + "operationId": "get-mcp-server-config-acl", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "format": "uuid", + "description": "MCP server config ID", + "name": "mcpserverconfig", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.MCPServerConfigACL" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "patch": { + "consumes": ["application/json"], + "tags": ["MCP"], + "summary": "Update MCP server config ACL", + "operationId": "update-mcp-server-config-acl", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "format": "uuid", + "description": "MCP server config ID", + "name": "mcpserverconfig", + "in": "path", + "required": true + }, + { + "description": "Update MCP server config ACL request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.UpdateMCPServerConfigACLRequest" + } + } + ], + "responses": { + "204": { + "description": "No Content" + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, "/api/experimental/organizations/{organization}/mcp-servers/{mcpserverconfig}/oauth2/connect": { "get": { "tags": ["MCP"], @@ -15112,6 +15200,7 @@ "mcp_server_config:create", "mcp_server_config:delete", "mcp_server_config:read", + "mcp_server_config:share", "mcp_server_config:update", "notification_message:*", "notification_message:create", @@ -15357,6 +15446,7 @@ "APIKeyScopeMcpServerConfigCreate", "APIKeyScopeMcpServerConfigDelete", "APIKeyScopeMcpServerConfigRead", + "APIKeyScopeMcpServerConfigShare", "APIKeyScopeMcpServerConfigUpdate", "APIKeyScopeNotificationMessageAll", "APIKeyScopeNotificationMessageCreate", @@ -20283,6 +20373,112 @@ } } }, + "codersdk.MCPServerConfigACL": { + "type": "object", + "properties": { + "groups": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.MCPServerConfigGroup" + } + }, + "users": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.MCPServerConfigUser" + } + } + } + }, + "codersdk.MCPServerConfigGroup": { + "type": "object", + "properties": { + "avatar_url": { + "type": "string", + "format": "uri" + }, + "display_name": { + "type": "string" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "members": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.ReducedUser" + } + }, + "name": { + "type": "string" + }, + "organization_display_name": { + "type": "string" + }, + "organization_id": { + "type": "string", + "format": "uuid" + }, + "organization_name": { + "type": "string" + }, + "quota_allowance": { + "type": "integer" + }, + "role": { + "enum": ["read"], + "allOf": [ + { + "$ref": "#/definitions/codersdk.MCPServerConfigRole" + } + ] + }, + "source": { + "$ref": "#/definitions/codersdk.GroupSource" + }, + "total_member_count": { + "description": "How many members are in this group. Shows the total count,\neven if the user is not authorized to read group member details.\nMay be greater than `len(Group.Members)`.", + "type": "integer" + } + } + }, + "codersdk.MCPServerConfigRole": { + "type": "string", + "enum": ["read", ""], + "x-enum-varnames": [ + "MCPServerConfigRoleRead", + "MCPServerConfigRoleDeleted" + ] + }, + "codersdk.MCPServerConfigUser": { + "type": "object", + "required": ["id", "username"], + "properties": { + "avatar_url": { + "type": "string", + "format": "uri" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + }, + "role": { + "enum": ["read"], + "allOf": [ + { + "$ref": "#/definitions/codersdk.MCPServerConfigRole" + } + ] + }, + "username": { + "type": "string" + } + } + }, "codersdk.MCPServerOAuth2DisconnectResponse": { "type": "object", "properties": { @@ -24781,6 +24977,23 @@ } } }, + "codersdk.UpdateMCPServerConfigACLRequest": { + "type": "object", + "properties": { + "group_roles": { + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/codersdk.MCPServerConfigRole" + } + }, + "user_roles": { + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/codersdk.MCPServerConfigRole" + } + } + } + }, "codersdk.UpdateMCPServerConfigRequest": { "type": "object", "properties": { diff --git a/coderd/coderd.go b/coderd/coderd.go index e423045b8f..3c65afd06f 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1386,11 +1386,18 @@ func New(options *Options) *API { r.Get("/", api.listMCPServerConfigs) r.Post("/", api.createMCPServerConfig) r.Route("/{mcpserverconfig}", func(r chi.Router) { - r.Use(httpmw.ExtractMCPServerConfigParam(options.Database)) - r.Get("/", api.getMCPServerConfig) - r.Patch("/", api.updateMCPServerConfig) - r.Delete("/", api.deleteMCPServerConfig) - r.Get("/oauth2/connect", api.mcpServerOAuth2Connect) + r.With(httpmw.ExtractMCPServerConfigParam(options.Database, api.HTTPAuth.Authorize, + policy.ActionRead, policy.ActionUpdate, policy.ActionDelete)).Get("/", api.getMCPServerConfig) + r.With(httpmw.ExtractMCPServerConfigParam(options.Database, api.HTTPAuth.Authorize, + policy.ActionUpdate)).Patch("/", api.updateMCPServerConfig) + r.With(httpmw.ExtractMCPServerConfigParam(options.Database, api.HTTPAuth.Authorize, + policy.ActionDelete)).Delete("/", api.deleteMCPServerConfig) + r.With(httpmw.ExtractMCPServerConfigParam(options.Database, api.HTTPAuth.Authorize, + policy.ActionShare)).Get("/acl", api.mcpServerConfigACL) + r.With(httpmw.ExtractMCPServerConfigParam(options.Database, api.HTTPAuth.Authorize, + policy.ActionShare)).Patch("/acl", api.patchMCPServerConfigACL) + r.With(httpmw.ExtractMCPServerConfigParam(options.Database, api.HTTPAuth.Authorize, + policy.ActionRead)).Get("/oauth2/connect", api.mcpServerOAuth2Connect) }) }) }) diff --git a/coderd/database/check_constraint.go b/coderd/database/check_constraint.go index 2f791242f5..6bdb82ca4e 100644 --- a/coderd/database/check_constraint.go +++ b/coderd/database/check_constraint.go @@ -43,7 +43,9 @@ const ( CheckGroupsChatSpendLimitMicrosCheck CheckConstraint = "groups_chat_spend_limit_micros_check" // groups CheckMcpServerConfigsAuthTypeCheck CheckConstraint = "mcp_server_configs_auth_type_check" // mcp_server_configs CheckMcpServerConfigsAvailabilityCheck CheckConstraint = "mcp_server_configs_availability_check" // mcp_server_configs + CheckMcpServerConfigsGroupAclIsObject CheckConstraint = "mcp_server_configs_group_acl_is_object" // mcp_server_configs CheckMcpServerConfigsTransportCheck CheckConstraint = "mcp_server_configs_transport_check" // mcp_server_configs + CheckMcpServerConfigsUserAclIsObject CheckConstraint = "mcp_server_configs_user_acl_is_object" // mcp_server_configs CheckOauth2ProviderAppCodesScopeNotEmpty CheckConstraint = "oauth2_provider_app_codes_scope_not_empty" // oauth2_provider_app_codes CheckOauth2ProviderAppTokensScopeNotEmpty CheckConstraint = "oauth2_provider_app_tokens_scope_not_empty" // oauth2_provider_app_tokens CheckOauth2ProviderAppsClientTypeCheck CheckConstraint = "oauth2_provider_apps_client_type_check" // oauth2_provider_apps diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9a17d6588e..31906ab90a 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -7677,6 +7677,13 @@ func (q *querier) UpdateMCPServerConfig(ctx context.Context, arg database.Update return q.db.UpdateMCPServerConfig(ctx, arg) } +func (q *querier) UpdateMCPServerConfigACLByID(ctx context.Context, arg database.UpdateMCPServerConfigACLByIDParams) error { + fetch := func(ctx context.Context, arg database.UpdateMCPServerConfigACLByIDParams) (database.MCPServerConfig, error) { + return q.db.GetMCPServerConfigByID(ctx, arg.ID) + } + return fetchAndExec(q.log, q.auth, policy.ActionShare, fetch, q.db.UpdateMCPServerConfigACLByID)(ctx, arg) +} + func (q *querier) UpdateMCPServerUserTokenFromRefresh(ctx context.Context, arg database.UpdateMCPServerUserTokenFromRefreshParams) (database.MCPServerUserToken, error) { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceDeploymentConfig); err != nil { return database.MCPServerUserToken{}, err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index f3ebd3b07a..415e9264e4 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1735,6 +1735,13 @@ func (s *MethodTestSuite) TestChats() { dbm.EXPECT().InsertMCPServerConfig(gomock.Any(), arg).Return(config, nil).AnyTimes() check.Args(arg).Asserts(rbac.ResourceMCPServerConfig.InOrg(arg.OrganizationID), policy.ActionCreate).Returns(config) })) + s.Run("UpdateMCPServerConfigACLByID", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + config := testutil.Fake(s.T(), faker, database.MCPServerConfig{}) + arg := database.UpdateMCPServerConfigACLByIDParams{ID: config.ID, UpdatedBy: uuid.New()} + dbm.EXPECT().GetMCPServerConfigByID(gomock.Any(), config.ID).Return(config, nil).AnyTimes() + dbm.EXPECT().UpdateMCPServerConfigACLByID(gomock.Any(), arg).Return(nil).AnyTimes() + check.Args(arg).Asserts(config, policy.ActionShare) + })) s.Run("UpdateChatMCPServerIDs", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { chat := testutil.Fake(s.T(), faker, database.Chat{}) arg := database.UpdateChatMCPServerIDsParams{ diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index 2912e098f1..841ae296dc 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -347,6 +347,17 @@ func MCPServerConfig(t testing.TB, db database.Store, seed database.MCPServerCon updatedBy = createdBy } + groupACL := seed.GroupACL + if groupACL == nil { + groupACL = database.ChatACL{ + organizationID.String(): {Permissions: []policy.Action{policy.ActionRead}}, + } + } + userACL := seed.UserACL + if userACL == nil { + userACL = database.ChatACL{} + } + cfg, err := db.InsertMCPServerConfig(genCtx, database.InsertMCPServerConfigParams{ ID: takeFirst(seed.ID, uuid.New()), OrganizationID: organizationID, @@ -376,6 +387,8 @@ func MCPServerConfig(t testing.TB, db database.Store, seed database.MCPServerCon ModelIntent: seed.ModelIntent, AllowInPlanMode: seed.AllowInPlanMode, ForwardCoderHeaders: seed.ForwardCoderHeaders, + GroupACL: groupACL, + UserACL: userACL, CreatedBy: createdBy, UpdatedBy: updatedBy, }) diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 2c1614ed4c..d621d792f3 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -5449,6 +5449,14 @@ func (m queryMetricsStore) UpdateMCPServerConfig(ctx context.Context, arg databa return r0, r1 } +func (m queryMetricsStore) UpdateMCPServerConfigACLByID(ctx context.Context, arg database.UpdateMCPServerConfigACLByIDParams) error { + start := time.Now() + r0 := m.s.UpdateMCPServerConfigACLByID(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateMCPServerConfigACLByID").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpdateMCPServerConfigACLByID").Inc() + return r0 +} + func (m queryMetricsStore) UpdateMCPServerUserTokenFromRefresh(ctx context.Context, arg database.UpdateMCPServerUserTokenFromRefreshParams) (database.MCPServerUserToken, error) { start := time.Now() r0, r1 := m.s.UpdateMCPServerUserTokenFromRefresh(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 729e58cf05..34051146fb 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -10310,6 +10310,20 @@ func (mr *MockStoreMockRecorder) UpdateMCPServerConfig(ctx, arg any) *gomock.Cal return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateMCPServerConfig", reflect.TypeOf((*MockStore)(nil).UpdateMCPServerConfig), ctx, arg) } +// UpdateMCPServerConfigACLByID mocks base method. +func (m *MockStore) UpdateMCPServerConfigACLByID(ctx context.Context, arg database.UpdateMCPServerConfigACLByIDParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateMCPServerConfigACLByID", ctx, arg) + ret0, _ := ret[0].(error) + return ret0 +} + +// UpdateMCPServerConfigACLByID indicates an expected call of UpdateMCPServerConfigACLByID. +func (mr *MockStoreMockRecorder) UpdateMCPServerConfigACLByID(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateMCPServerConfigACLByID", reflect.TypeOf((*MockStore)(nil).UpdateMCPServerConfigACLByID), ctx, arg) +} + // UpdateMCPServerUserTokenFromRefresh mocks base method. func (m *MockStore) UpdateMCPServerUserTokenFromRefresh(ctx context.Context, arg database.UpdateMCPServerUserTokenFromRefreshParams) (database.MCPServerUserToken, error) { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 0512d79845..e32e751311 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -278,7 +278,8 @@ CREATE TYPE api_key_scope AS ENUM ( 'mcp_server_config:create', 'mcp_server_config:read', 'mcp_server_config:update', - 'mcp_server_config:delete' + 'mcp_server_config:delete', + 'mcp_server_config:share' ); CREATE TYPE app_sharing_level AS ENUM ( @@ -2521,9 +2522,13 @@ CREATE TABLE mcp_server_configs ( forward_coder_headers boolean DEFAULT false NOT NULL, oauth2_revocation_url text DEFAULT ''::text NOT NULL, organization_id uuid NOT NULL, + group_acl jsonb DEFAULT '{}'::jsonb NOT NULL, + user_acl jsonb DEFAULT '{}'::jsonb NOT NULL, CONSTRAINT mcp_server_configs_auth_type_check CHECK ((auth_type = ANY (ARRAY['none'::text, 'oauth2'::text, 'api_key'::text, 'custom_headers'::text, 'user_oidc'::text]))), CONSTRAINT mcp_server_configs_availability_check CHECK ((availability = ANY (ARRAY['force_on'::text, 'default_on'::text, 'default_off'::text]))), - CONSTRAINT mcp_server_configs_transport_check CHECK ((transport = ANY (ARRAY['streamable_http'::text, 'sse'::text]))) + CONSTRAINT mcp_server_configs_group_acl_is_object CHECK ((jsonb_typeof(group_acl) = 'object'::text)), + CONSTRAINT mcp_server_configs_transport_check CHECK ((transport = ANY (ARRAY['streamable_http'::text, 'sse'::text]))), + CONSTRAINT mcp_server_configs_user_acl_is_object CHECK ((jsonb_typeof(user_acl) = 'object'::text)) ); CREATE TABLE mcp_server_user_tokens ( diff --git a/coderd/database/migrations/000576_mcp_server_config_acl.down.sql b/coderd/database/migrations/000576_mcp_server_config_acl.down.sql new file mode 100644 index 0000000000..04ddd0e967 --- /dev/null +++ b/coderd/database/migrations/000576_mcp_server_config_acl.down.sql @@ -0,0 +1,6 @@ +ALTER TABLE mcp_server_configs + DROP COLUMN group_acl, + DROP COLUMN user_acl; + +-- Enum values cannot be removed safely from api_key_scope, so the +-- share scope added by the up migration stays. diff --git a/coderd/database/migrations/000576_mcp_server_config_acl.up.sql b/coderd/database/migrations/000576_mcp_server_config_acl.up.sql new file mode 100644 index 0000000000..2ba41ac180 --- /dev/null +++ b/coderd/database/migrations/000576_mcp_server_config_acl.up.sql @@ -0,0 +1,13 @@ +ALTER TYPE api_key_scope ADD VALUE IF NOT EXISTS 'mcp_server_config:share'; + +ALTER TABLE mcp_server_configs + ADD COLUMN group_acl JSONB NOT NULL DEFAULT '{}'::JSONB, + ADD COLUMN user_acl JSONB NOT NULL DEFAULT '{}'::JSONB, + ADD CONSTRAINT mcp_server_configs_group_acl_is_object CHECK (jsonb_typeof(group_acl) = 'object'), + ADD CONSTRAINT mcp_server_configs_user_acl_is_object CHECK (jsonb_typeof(user_acl) = 'object'); + +UPDATE mcp_server_configs +SET group_acl = jsonb_build_object( + organization_id::text, + jsonb_build_object('permissions', jsonb_build_array('read')) +); diff --git a/coderd/database/migrations/migrate_test.go b/coderd/database/migrations/migrate_test.go index c9d7515473..ad543a6d61 100644 --- a/coderd/database/migrations/migrate_test.go +++ b/coderd/database/migrations/migrate_test.go @@ -3239,3 +3239,93 @@ func TestMigration000574MCPServerConfigsOrganizationID(t *testing.T) { require.NoError(t, err) require.Zero(t, danglingIDs) } + +func TestMigration000576MCPServerConfigACL(t *testing.T) { + t.Parallel() + + const priorMigrationVersion = 575 + + sqlDB := testSQLDB(t) + next, err := migrations.Stepper(sqlDB) + require.NoError(t, err) + for { + version, more, err := next() + require.NoError(t, err) + if !more { + t.Fatalf("migration %d not found", priorMigrationVersion) + } + if version == priorMigrationVersion { + break + } + } + + ctx := testutil.Context(t, testutil.WaitSuperLong) + now := time.Now().UTC().Truncate(time.Microsecond) + + var defaultOrgID uuid.UUID + err = sqlDB.QueryRowContext(ctx, `SELECT id FROM organizations WHERE is_default = true`).Scan(&defaultOrgID) + require.NoError(t, err) + + orgID := uuid.New() + _, err = sqlDB.ExecContext(ctx, ` + INSERT INTO organizations ( + id, name, display_name, description, icon, created_at, updated_at, + is_default, deleted, default_org_member_roles + ) VALUES ($1, 'migration-570-org', 'Migration 570 Org', '', '', $2, $2, false, false, '{}') + `, orgID, now) + require.NoError(t, err) + + configIDs := []uuid.UUID{uuid.New(), uuid.New()} + orgIDs := []uuid.UUID{defaultOrgID, orgID} + for i, configID := range configIDs { + _, err = sqlDB.ExecContext(ctx, ` + INSERT INTO mcp_server_configs ( + id, organization_id, display_name, slug, description, url, auth_type, + availability, enabled, created_at, updated_at + ) VALUES ($1, $2, $3, $4, 'unchanged', $5, 'none', 'default_on', true, $6, $6) + `, configID, orgIDs[i], fmt.Sprintf("Migration 570 Config %d", i), fmt.Sprintf("migration-570-config-%d", i), fmt.Sprintf("https://mcp.example.com/%d", i), now) + require.NoError(t, err) + } + + version, _, err := next() + require.NoError(t, err) + require.EqualValues(t, 576, version) + + for i, configID := range configIDs { + var displayName, description string + var enabled bool + var groupACL, userACL string + err = sqlDB.QueryRowContext(ctx, ` + SELECT display_name, description, enabled, group_acl::text, user_acl::text + FROM mcp_server_configs WHERE id = $1 + `, configID).Scan(&displayName, &description, &enabled, &groupACL, &userACL) + require.NoError(t, err) + require.Equal(t, fmt.Sprintf("Migration 570 Config %d", i), displayName) + require.Equal(t, "unchanged", description) + require.True(t, enabled) + require.JSONEq(t, fmt.Sprintf(`{%q:{"permissions":["read"]}}`, orgIDs[i].String()), groupACL) + require.JSONEq(t, `{}`, userACL) + } + + for _, column := range []string{"group_acl", "user_acl"} { + var defaultValue string + err = sqlDB.QueryRowContext(ctx, ` + SELECT column_default FROM information_schema.columns + WHERE table_name = 'mcp_server_configs' AND column_name = $1 + `, column).Scan(&defaultValue) + require.NoError(t, err) + require.Equal(t, "'{}'::jsonb", defaultValue) + } + + downSQL, err := os.ReadFile("000576_mcp_server_config_acl.down.sql") + require.NoError(t, err) + _, err = sqlDB.ExecContext(ctx, string(downSQL)) + require.NoError(t, err) + var aclColumns int + err = sqlDB.QueryRowContext(ctx, ` + SELECT COUNT(*) FROM information_schema.columns + WHERE table_name = 'mcp_server_configs' AND column_name IN ('group_acl', 'user_acl') + `).Scan(&aclColumns) + require.NoError(t, err) + require.Zero(t, aclColumns) +} diff --git a/coderd/database/migrations/testdata/fixtures/000576_mcp_server_config_acl.up.sql b/coderd/database/migrations/testdata/fixtures/000576_mcp_server_config_acl.up.sql new file mode 100644 index 0000000000..014c638766 --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000576_mcp_server_config_acl.up.sql @@ -0,0 +1,17 @@ +INSERT INTO mcp_server_configs ( + id, + organization_id, + display_name, + slug, + url, + auth_type, + group_acl +) VALUES ( + 'f5690000-0000-4000-8000-000000000001', + 'f5610000-0000-4000-8000-000000000001', + 'Fixture MCP ACL Server', + 'fixture-mcp-acl-server', + 'https://mcp.example.com/acl', + 'none', + '{"f5610000-0000-4000-8000-000000000001":{"permissions":["read"]}}'::JSONB +); diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index dfd13f5c3e..58c18a3365 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -230,7 +230,9 @@ func (c Chat) RBACObject() rbac.Object { func (m MCPServerConfig) RBACObject() rbac.Object { return rbac.ResourceMCPServerConfig. WithID(m.ID). - InOrg(m.OrganizationID) + InOrg(m.OrganizationID). + WithGroupACL(m.GroupACL.RBACACL()). + WithACLUserList(m.UserACL.RBACACL()) } func (c Chat) IsSubChat() bool { diff --git a/coderd/database/modelqueries.go b/coderd/database/modelqueries.go index dbdaa647ea..c9df9f980f 100644 --- a/coderd/database/modelqueries.go +++ b/coderd/database/modelqueries.go @@ -1207,7 +1207,7 @@ type mcpServerConfigQuerier interface { func (q *sqlQuerier) GetAuthorizedMCPServerConfigs(ctx context.Context, organizationID uuid.UUID, prepared rbac.PreparedAuthorized) ([]MCPServerConfig, error) { authorizedFilter, err := prepared.CompileToSQL(ctx, regosql.ConvertConfig{ - VariableConverter: regosql.MCPServerConfigNoACLConverter(), + VariableConverter: regosql.MCPServerConfigConverter(), }) if err != nil { return nil, xerrors.Errorf("compile authorized filter: %w", err) @@ -1261,6 +1261,8 @@ func (q *sqlQuerier) GetAuthorizedMCPServerConfigs(ctx context.Context, organiza &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ); err != nil { return nil, err } diff --git a/coderd/database/models.go b/coderd/database/models.go index 03b7b2584d..0e914ec66f 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -470,6 +470,7 @@ const ( ApiKeyScopeMcpServerConfigRead APIKeyScope = "mcp_server_config:read" ApiKeyScopeMcpServerConfigUpdate APIKeyScope = "mcp_server_config:update" ApiKeyScopeMcpServerConfigDelete APIKeyScope = "mcp_server_config:delete" + ApiKeyScopeMcpServerConfigShare APIKeyScope = "mcp_server_config:share" ) func (e *APIKeyScope) Scan(src interface{}) error { @@ -749,7 +750,8 @@ func (e APIKeyScope) Valid() bool { ApiKeyScopeMcpServerConfigCreate, ApiKeyScopeMcpServerConfigRead, ApiKeyScopeMcpServerConfigUpdate, - ApiKeyScopeMcpServerConfigDelete: + ApiKeyScopeMcpServerConfigDelete, + ApiKeyScopeMcpServerConfigShare: return true } return false @@ -998,6 +1000,7 @@ func AllAPIKeyScopeValues() []APIKeyScope { ApiKeyScopeMcpServerConfigRead, ApiKeyScopeMcpServerConfigUpdate, ApiKeyScopeMcpServerConfigDelete, + ApiKeyScopeMcpServerConfigShare, } } @@ -5474,6 +5477,8 @@ type MCPServerConfig struct { ForwardCoderHeaders bool `db:"forward_coder_headers" json:"forward_coder_headers"` OAuth2RevocationURL string `db:"oauth2_revocation_url" json:"oauth2_revocation_url"` OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + GroupACL ChatACL `db:"group_acl" json:"group_acl"` + UserACL ChatACL `db:"user_acl" json:"user_acl"` } type MCPServerUserToken struct { diff --git a/coderd/database/querier.go b/coderd/database/querier.go index fbb499a0f5..7733a397e8 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -1505,6 +1505,7 @@ type sqlcQuerier interface { UpdateInactiveUsersToDormant(ctx context.Context, arg UpdateInactiveUsersToDormantParams) ([]UpdateInactiveUsersToDormantRow, error) UpdateInboxNotificationReadStatus(ctx context.Context, arg UpdateInboxNotificationReadStatusParams) error UpdateMCPServerConfig(ctx context.Context, arg UpdateMCPServerConfigParams) (MCPServerConfig, error) + UpdateMCPServerConfigACLByID(ctx context.Context, arg UpdateMCPServerConfigACLByIDParams) error // Refresh persistence must not recreate a token deleted by disconnect. // The optimistic lock also prevents stale refreshes from replacing newer tokens. UpdateMCPServerUserTokenFromRefresh(ctx context.Context, arg UpdateMCPServerUserTokenFromRefreshParams) (MCPServerUserToken, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 4021d4b2e3..dcf1305c10 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -17147,7 +17147,7 @@ func (q *sqlQuerier) DeleteMCPServerUserTokensByConfigID(ctx context.Context, mc const getEnabledMCPServerConfigsByOrganization = `-- name: GetEnabledMCPServerConfigsByOrganization :many SELECT - id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id + id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl FROM mcp_server_configs WHERE @@ -17199,6 +17199,8 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigsByOrganization(ctx context.Contex &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ); err != nil { return nil, err } @@ -17215,7 +17217,7 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigsByOrganization(ctx context.Contex const getEnabledMCPServerConfigsByOrganizationAndIDs = `-- name: GetEnabledMCPServerConfigsByOrganizationAndIDs :many SELECT - id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id + id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl FROM mcp_server_configs WHERE @@ -17273,6 +17275,8 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigsByOrganizationAndIDs(ctx context. &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ); err != nil { return nil, err } @@ -17289,7 +17293,7 @@ func (q *sqlQuerier) GetEnabledMCPServerConfigsByOrganizationAndIDs(ctx context. const getForcedMCPServerConfigsByOrganization = `-- name: GetForcedMCPServerConfigsByOrganization :many SELECT - id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id + id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl FROM mcp_server_configs WHERE @@ -17342,6 +17346,8 @@ func (q *sqlQuerier) GetForcedMCPServerConfigsByOrganization(ctx context.Context &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ); err != nil { return nil, err } @@ -17358,7 +17364,7 @@ func (q *sqlQuerier) GetForcedMCPServerConfigsByOrganization(ctx context.Context const getMCPServerConfigByID = `-- name: GetMCPServerConfigByID :one SELECT - id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id + id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl FROM mcp_server_configs WHERE @@ -17401,13 +17407,15 @@ func (q *sqlQuerier) GetMCPServerConfigByID(ctx context.Context, id uuid.UUID) ( &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ) return i, err } const getMCPServerConfigByIDForUpdate = `-- name: GetMCPServerConfigByIDForUpdate :one SELECT - id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id + id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl FROM mcp_server_configs WHERE @@ -17451,13 +17459,15 @@ func (q *sqlQuerier) GetMCPServerConfigByIDForUpdate(ctx context.Context, id uui &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ) return i, err } const getMCPServerConfigByOrganizationAndSlug = `-- name: GetMCPServerConfigByOrganizationAndSlug :one SELECT - id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id + id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl FROM mcp_server_configs WHERE @@ -17506,13 +17516,15 @@ func (q *sqlQuerier) GetMCPServerConfigByOrganizationAndSlug(ctx context.Context &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ) return i, err } const getMCPServerConfigsByOrganization = `-- name: GetMCPServerConfigsByOrganization :many SELECT - id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id + id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl FROM mcp_server_configs WHERE @@ -17565,6 +17577,8 @@ func (q *sqlQuerier) GetMCPServerConfigsByOrganization(ctx context.Context, orga &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ); err != nil { return nil, err } @@ -17689,6 +17703,8 @@ INSERT INTO mcp_server_configs ( model_intent, allow_in_plan_mode, forward_coder_headers, + group_acl, + user_acl, created_by, updated_by ) VALUES ( @@ -17720,11 +17736,13 @@ INSERT INTO mcp_server_configs ( $26::boolean, $27::boolean, $28::boolean, - $29::uuid, - $30::uuid + $29, + $30, + $31::uuid, + $32::uuid ) RETURNING - id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id + id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl ` type InsertMCPServerConfigParams struct { @@ -17756,6 +17774,8 @@ type InsertMCPServerConfigParams struct { ModelIntent bool `db:"model_intent" json:"model_intent"` AllowInPlanMode bool `db:"allow_in_plan_mode" json:"allow_in_plan_mode"` ForwardCoderHeaders bool `db:"forward_coder_headers" json:"forward_coder_headers"` + GroupACL ChatACL `db:"group_acl" json:"group_acl"` + UserACL ChatACL `db:"user_acl" json:"user_acl"` CreatedBy uuid.UUID `db:"created_by" json:"created_by"` UpdatedBy uuid.UUID `db:"updated_by" json:"updated_by"` } @@ -17790,6 +17810,8 @@ func (q *sqlQuerier) InsertMCPServerConfig(ctx context.Context, arg InsertMCPSer arg.ModelIntent, arg.AllowInPlanMode, arg.ForwardCoderHeaders, + arg.GroupACL, + arg.UserACL, arg.CreatedBy, arg.UpdatedBy, ) @@ -17827,6 +17849,8 @@ func (q *sqlQuerier) InsertMCPServerConfig(ctx context.Context, arg InsertMCPSer &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ) return i, err } @@ -17914,7 +17938,7 @@ SET WHERE id = $28::uuid RETURNING - id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id + id, display_name, slug, description, icon_url, transport, url, auth_type, oauth2_client_id, oauth2_client_secret, oauth2_client_secret_key_id, oauth2_auth_url, oauth2_token_url, oauth2_scopes, api_key_header, api_key_value, api_key_value_key_id, custom_headers, custom_headers_key_id, tool_allow_list, tool_deny_list, availability, enabled, created_by, updated_by, created_at, updated_at, model_intent, allow_in_plan_mode, forward_coder_headers, oauth2_revocation_url, organization_id, group_acl, user_acl ` type UpdateMCPServerConfigParams struct { @@ -18013,10 +18037,40 @@ func (q *sqlQuerier) UpdateMCPServerConfig(ctx context.Context, arg UpdateMCPSer &i.ForwardCoderHeaders, &i.OAuth2RevocationURL, &i.OrganizationID, + &i.GroupACL, + &i.UserACL, ) return i, err } +const updateMCPServerConfigACLByID = `-- name: UpdateMCPServerConfigACLByID :exec +UPDATE mcp_server_configs +SET + group_acl = $1, + user_acl = $2, + updated_by = $3::uuid, + updated_at = NOW() +WHERE + id = $4::uuid +` + +type UpdateMCPServerConfigACLByIDParams struct { + GroupACL ChatACL `db:"group_acl" json:"group_acl"` + UserACL ChatACL `db:"user_acl" json:"user_acl"` + UpdatedBy uuid.UUID `db:"updated_by" json:"updated_by"` + ID uuid.UUID `db:"id" json:"id"` +} + +func (q *sqlQuerier) UpdateMCPServerConfigACLByID(ctx context.Context, arg UpdateMCPServerConfigACLByIDParams) error { + _, err := q.db.ExecContext(ctx, updateMCPServerConfigACLByID, + arg.GroupACL, + arg.UserACL, + arg.UpdatedBy, + arg.ID, + ) + return err +} + const updateMCPServerUserTokenFromRefresh = `-- name: UpdateMCPServerUserTokenFromRefresh :one UPDATE mcp_server_user_tokens SET diff --git a/coderd/database/queries/mcpserverconfigs.sql b/coderd/database/queries/mcpserverconfigs.sql index 2d648809a9..efa944fe34 100644 --- a/coderd/database/queries/mcpserverconfigs.sql +++ b/coderd/database/queries/mcpserverconfigs.sql @@ -101,6 +101,8 @@ INSERT INTO mcp_server_configs ( model_intent, allow_in_plan_mode, forward_coder_headers, + group_acl, + user_acl, created_by, updated_by ) VALUES ( @@ -132,6 +134,8 @@ INSERT INTO mcp_server_configs ( @model_intent::boolean, @allow_in_plan_mode::boolean, @forward_coder_headers::boolean, + @group_acl, + @user_acl, @created_by::uuid, @updated_by::uuid ) @@ -175,6 +179,16 @@ WHERE RETURNING *; +-- name: UpdateMCPServerConfigACLByID :exec +UPDATE mcp_server_configs +SET + group_acl = @group_acl, + user_acl = @user_acl, + updated_by = @updated_by::uuid, + updated_at = NOW() +WHERE + id = @id::uuid; + -- name: DeleteMCPServerConfigByID :exec DELETE FROM mcp_server_configs diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index f7d820780c..22976e4812 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -83,6 +83,12 @@ sql: - column: "chats.group_acl" go_type: type: "ChatACL" + - column: "mcp_server_configs.user_acl" + go_type: + type: "ChatACL" + - column: "mcp_server_configs.group_acl" + go_type: + type: "ChatACL" - column: "chats_expanded.user_acl" go_type: type: "ChatACL" diff --git a/coderd/httpmw/mcpserverconfigparam.go b/coderd/httpmw/mcpserverconfigparam.go index 0ea4a1eeed..9098074472 100644 --- a/coderd/httpmw/mcpserverconfigparam.go +++ b/coderd/httpmw/mcpserverconfigparam.go @@ -5,7 +5,10 @@ import ( "net/http" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/codersdk" ) @@ -22,9 +25,13 @@ func MCPServerConfigParam(r *http.Request) database.MCPServerConfig { } // ExtractMCPServerConfigParam reads the "mcpserverconfig" URL parameter. -// Unauthorized reads are concealed as not found, so denied and missing rows -// both return 404. -func ExtractMCPServerConfigParam(db database.Store) func(http.Handler) http.Handler { +// Callers with none of the admitted actions are concealed as not found, so +// denied and missing rows both return 404. +func ExtractMCPServerConfigParam( + db database.Store, + auth func(r *http.Request, action policy.Action, object rbac.Objecter) bool, + actions ...policy.Action, +) func(http.Handler) http.Handler { return func(next http.Handler) http.Handler { return http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() @@ -33,7 +40,10 @@ func ExtractMCPServerConfigParam(db database.Store) func(http.Handler) http.Hand return } - config, err := db.GetMCPServerConfigByID(ctx, configID) + // Authorization follows the raw lookup because mutation-only callers + // may lack the read access enforced by the database wrapper. + //nolint:gocritic // The explicit action checks below own authorization. + config, err := db.GetMCPServerConfigByID(dbauthz.AsSystemRestricted(ctx), configID) if httpapi.Is404Error(err) { httpapi.ResourceNotFound(rw) return @@ -45,7 +55,14 @@ func ExtractMCPServerConfigParam(db database.Store) func(http.Handler) http.Hand }) return } - if config.OrganizationID != OrganizationParam(r).ID { + admitted := false + for _, action := range actions { + if auth(r, action, config) { + admitted = true + break + } + } + if config.OrganizationID != OrganizationParam(r).ID || !admitted { httpapi.ResourceNotFound(rw) return } diff --git a/coderd/mcp.go b/coderd/mcp.go index fd2ee6388c..03d16fab6a 100644 --- a/coderd/mcp.go +++ b/coderd/mcp.go @@ -10,6 +10,7 @@ import ( "io" "net/http" "net/url" + "slices" "strings" "time" @@ -161,22 +162,54 @@ func (api *API) listMCPServerConfigs(rw http.ResponseWriter, r *http.Request) { apiKey := httpmw.APIKey(r) organization := httpmw.OrganizationParam(r) - // Full view: disabled configs included, management fields unredacted. - // Auditors get it to inspect audit-logged resources; their MCP config - // read grant cannot select it because members hold the same read. - // Other members see enabled configs with management fields redacted. + // Full view: disabled configs included, management fields unredacted, + // fetched with system access behind this gate. Auditors get it to + // inspect audit-logged resources but stay subject to per-server ACLs + // elsewhere. Other members see enabled ACL-granted configs, redacted. // The update leg also requires config read so a custom role granting // update without read cannot lift the read filtering below. - hasFullView := (api.Authorize(r, policy.ActionRead, rbac.ResourceMCPServerConfig.InOrg(organization.ID)) && + hasFullView := ((api.Authorize(r, policy.ActionRead, rbac.ResourceMCPServerConfig.InOrg(organization.ID)) && api.Authorize(r, policy.ActionUpdate, rbac.ResourceMCPServerConfig.InOrg(organization.ID))) || - api.Authorize(r, policy.ActionRead, rbac.ResourceAuditLog.InOrg(organization.ID)) + api.Authorize(r, policy.ActionRead, rbac.ResourceAuditLog.InOrg(organization.ID))) && + api.mcpServerConfigReadInKeyScope(r, organization.ID) var configs []database.MCPServerConfig var err error if hasFullView { - configs, err = api.Database.GetMCPServerConfigsByOrganization(ctx, organization.ID) - } else { - configs, err = api.Database.GetEnabledMCPServerConfigsByOrganization(ctx, organization.ID) + //nolint:gocritic // The update-or-audit gate above owns this authorization. + configs, err = api.Database.GetMCPServerConfigsByOrganization(dbauthz.AsSystemRestricted(ctx), organization.ID) + } else if api.mcpServerConfigReadInKeyScope(r, organization.ID) { + seen := make(map[uuid.UUID]struct{}) + for _, action := range []policy.Action{policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionShare} { + prepared, prepareErr := api.HTTPAuth.AuthorizeSQLFilter(r, action, rbac.ResourceMCPServerConfig.Type) + if prepareErr != nil { + httpapi.InternalServerError(rw, prepareErr) + return + } + authorized, queryErr := api.Database.GetAuthorizedMCPServerConfigs(ctx, organization.ID, prepared) + if queryErr != nil { + err = queryErr + break + } + for _, config := range authorized { + if _, ok := seen[config.ID]; ok { + continue + } + seen[config.ID] = struct{}{} + configs = append(configs, config) + } + } + slices.SortFunc(configs, func(a, b database.MCPServerConfig) int { + return strings.Compare(a.DisplayName, b.DisplayName) + }) + // Management-authorized callers keep disabled configs in the redacted + // list so they can still reach and manage them. + configs = slices.DeleteFunc(configs, func(config database.MCPServerConfig) bool { + return !config.Enabled && + !api.Authorize(r, policy.ActionUpdate, config) && + !api.Authorize(r, policy.ActionDelete, config) && + !api.Authorize(r, policy.ActionShare, config) + }) } if err != nil { httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ @@ -231,6 +264,24 @@ func (api *API) listMCPServerConfigs(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, resp) } +// mcpServerConfigReadInKeyScope reports whether the caller's API key scope +// permits reading MCP server configs. The full-view list fetch bypasses row +// authorization with system access, so the key's scope must be enforced here. +// Owner roles make the authorize outcome depend only on the scope dimension. +func (api *API) mcpServerConfigReadInKeyScope(r *http.Request, organizationID uuid.UUID) bool { + caller := httpmw.UserAuthorization(r.Context()) + scopeOnly := rbac.Subject{ + Type: caller.Type, + FriendlyName: caller.FriendlyName, + ID: caller.ID, + Roles: rbac.RoleIdentifiers{rbac.RoleOwner()}, + Scope: caller.Scope, + } + err := api.HTTPAuth.Authorizer.Authorize(r.Context(), scopeOnly, + policy.ActionRead, rbac.ResourceMCPServerConfig.InOrg(organizationID)) + return err == nil +} + // @Summary Create MCP server config // @ID create-mcp-server-config // @Security CoderSessionToken @@ -425,8 +476,12 @@ func (api *API) createMCPServerConfig(rw http.ResponseWriter, r *http.Request) { ModelIntent: req.ModelIntent, AllowInPlanMode: req.AllowInPlanMode, ForwardCoderHeaders: req.ForwardCoderHeaders, - CreatedBy: apiKey.UserID, - UpdatedBy: apiKey.UserID, + GroupACL: database.ChatACL{ + organization.ID.String(): {Permissions: []policy.Action{policy.ActionRead}}, + }, + UserACL: database.ChatACL{}, + CreatedBy: apiKey.UserID, + UpdatedBy: apiKey.UserID, }) if err != nil { switch { @@ -473,11 +528,16 @@ func (api *API) getMCPServerConfig(rw http.ResponseWriter, r *http.Request) { ctx := r.Context() apiKey := httpmw.APIKey(r) config := httpmw.MCPServerConfigParam(r) + if !api.mcpServerConfigReadInKeyScope(r, config.OrganizationID) { + httpapi.ResourceNotFound(rw) + return + } // Same full-view rule as listMCPServerConfigs: admins and auditors. hasFullView := api.Authorize(r, policy.ActionUpdate, config) || api.Authorize(r, policy.ActionRead, rbac.ResourceAuditLog.InOrg(config.OrganizationID)) - if !hasFullView && !config.Enabled { + if !hasFullView && !config.Enabled && + !api.Authorize(r, policy.ActionDelete, config) { httpapi.ResourceNotFound(rw) return } @@ -565,9 +625,8 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) { }) defer commitAudit() - // Set Old before the write-authz check so a write-denied 403 is - // audited. Read-denied callers were already concealed with 404 by - // the param middleware and never reach this handler. + // Set Old before the requested-action check so callers admitted for a + // different action are audited when this handler denies them with 403. aReq.Old = httpmw.MCPServerConfigParam(r) aReq.UpdateOrganizationID(aReq.Old.OrganizationID) @@ -623,7 +682,8 @@ func (api *API) updateMCPServerConfig(rw http.ResponseWriter, r *http.Request) { // Lock and re-fetch the row so omitted fields and the audit baseline // match the row this update replaces, and so grant invalidation // serializes with in-flight OAuth callbacks verifying the config. - current, err := tx.GetMCPServerConfigByIDForUpdate(ctx, existing.ID) + //nolint:gocritic // The update write reauthorizes the locked row. + current, err := tx.GetMCPServerConfigByIDForUpdate(dbauthz.AsSystemRestricted(ctx), existing.ID) if err != nil { return err } @@ -927,9 +987,8 @@ func (api *API) deleteMCPServerConfig(rw http.ResponseWriter, r *http.Request) { }) defer commitAudit() - // Set Old before the write-authz check so a write-denied 403 is - // audited. Read-denied callers were already concealed with 404 by - // the param middleware and never reach this handler. + // Set Old before the requested-action check so callers admitted for a + // different action are audited when this handler denies them with 403. aReq.Old = httpmw.MCPServerConfigParam(r) aReq.UpdateOrganizationID(aReq.Old.OrganizationID) @@ -942,7 +1001,8 @@ func (api *API) deleteMCPServerConfig(rw http.ResponseWriter, r *http.Request) { // Re-fetch under a row lock so the audit record describes the // row this request actually removes, not a middleware snapshot // that a concurrent update may have made stale. - current, err := tx.GetMCPServerConfigByIDForUpdate(ctx, config.ID) + //nolint:gocritic // The delete write reauthorizes the locked row. + current, err := tx.GetMCPServerConfigByIDForUpdate(dbauthz.AsSystemRestricted(ctx), config.ID) if err != nil { return err } diff --git a/coderd/mcp_acl.go b/coderd/mcp_acl.go new file mode 100644 index 0000000000..0227c15cb1 --- /dev/null +++ b/coderd/mcp_acl.go @@ -0,0 +1,330 @@ +package coderd + +import ( + "context" + "fmt" + "maps" + "net/http" + "slices" + + "github.com/google/uuid" + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" + "github.com/coder/coder/v2/coderd/database/dbauthz" + "github.com/coder/coder/v2/coderd/httpapi" + "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/coderd/rbac/acl" + "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/codersdk" +) + +// EXPERIMENTAL: this endpoint is experimental and is subject to change. +// +// @Summary Get MCP server config ACL +// @ID get-mcp-server-config-acl +// @Security CoderSessionToken +// @Tags MCP +// @Produce json +// @Param organization path string true "Organization ID" format(uuid) +// @Param mcpserverconfig path string true "MCP server config ID" format(uuid) +// @Success 200 {object} codersdk.MCPServerConfigACL +// @Router /api/experimental/organizations/{organization}/mcp-servers/{mcpserverconfig}/acl [get] +// @x-apidocgen {"skip": true} +func (api *API) mcpServerConfigACL(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + config := httpmw.MCPServerConfigParam(r) + + // The read gate admits every ACL-granted member, so gate ACL + // enumeration on the same share permission that gates updates. + if !api.Authorize(r, policy.ActionShare, config.RBACObject()) { + httpapi.Forbidden(rw) + return + } + + users, ok := api.mcpServerConfigACLUsers(ctx, rw, config.UserACL) + if !ok { + return + } + groups, ok := api.mcpServerConfigACLGroups(ctx, rw, config.GroupACL) + if !ok { + return + } + httpapi.Write(ctx, rw, http.StatusOK, codersdk.MCPServerConfigACL{ + Users: users, + Groups: groups, + }) +} + +// EXPERIMENTAL: this endpoint is experimental and is subject to change. +// +// @Summary Update MCP server config ACL +// @ID update-mcp-server-config-acl +// @Security CoderSessionToken +// @Tags MCP +// @Accept json +// @Param organization path string true "Organization ID" format(uuid) +// @Param mcpserverconfig path string true "MCP server config ID" format(uuid) +// @Param request body codersdk.UpdateMCPServerConfigACLRequest true "Update MCP server config ACL request" +// @Success 204 +// @Router /api/experimental/organizations/{organization}/mcp-servers/{mcpserverconfig}/acl [patch] +// @x-apidocgen {"skip": true} +func (api *API) patchMCPServerConfigACL(rw http.ResponseWriter, r *http.Request) { + ctx := r.Context() + config := httpmw.MCPServerConfigParam(r) + apiKey := httpmw.APIKey(r) + auditor := api.Auditor.Load() + aReq, commitAudit := audit.InitRequest[database.MCPServerConfig](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: config.OrganizationID, + }) + defer commitAudit() + aReq.Old = config + + if !api.Authorize(r, policy.ActionShare, config.RBACObject()) { + httpapi.Forbidden(rw) + return + } + + var req codersdk.UpdateMCPServerConfigACLRequest + if !httpapi.Read(ctx, rw, r, &req) { + return + } + validations := acl.Validate(ctx, api.Database, MCPServerConfigACLUpdateValidator(req)) + validations = append(validations, api.validateMCPServerConfigACLOrganization(ctx, config.OrganizationID, req)...) + userRoles, dupErrs := canonicalMCPServerConfigACLRoles("user_roles", req.UserRoles) + validations = append(validations, dupErrs...) + groupRoles, dupErrs := canonicalMCPServerConfigACLRoles("group_roles", req.GroupRoles) + validations = append(validations, dupErrs...) + if len(validations) > 0 { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid request to update MCP server config ACL.", + Validations: validations, + }) + return + } + + var updated database.MCPServerConfig + err := api.Database.InTx(func(tx database.Store) error { + //nolint:gocritic // The ACL write below reauthorizes the locked row for share. + current, err := tx.GetMCPServerConfigByIDForUpdate(dbauthz.AsSystemRestricted(ctx), config.ID) + if err != nil { + return xerrors.Errorf("get MCP server config for update: %w", err) + } + aReq.Old = current + userACL := maps.Clone(current.UserACL) + groupACL := maps.Clone(current.GroupACL) + applyMCPServerConfigACLRoles(userACL, userRoles) + applyMCPServerConfigACLRoles(groupACL, groupRoles) + if err := tx.UpdateMCPServerConfigACLByID(ctx, database.UpdateMCPServerConfigACLByIDParams{ + ID: config.ID, + UserACL: userACL, + GroupACL: groupACL, + UpdatedBy: apiKey.UserID, + }); err != nil { + return xerrors.Errorf("update MCP server config ACL: %w", err) + } + updated = current + updated.UserACL = userACL + updated.GroupACL = groupACL + updated.UpdatedBy = uuid.NullUUID{UUID: apiKey.UserID, Valid: true} + return nil + }, nil) + if err != nil { + // A concurrent delete between the middleware fetch and the + // locked re-fetch stays concealed as 404, matching the update + // and delete handlers. + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return + } + httpapi.InternalServerError(rw, err) + return + } + aReq.New = updated + rw.WriteHeader(http.StatusNoContent) +} + +func (api *API) mcpServerConfigACLUsers(ctx context.Context, rw http.ResponseWriter, entries database.ChatACL) ([]codersdk.MCPServerConfigUser, bool) { + ids := parseMCPServerConfigACLIDs(entries) + //nolint:gocritic // ACL managers may resolve principals after the share gate passes. + users, err := api.Database.GetUsersByIDs(dbauthz.AsSystemRestricted(ctx), ids) + if err != nil { + httpapi.InternalServerError(rw, err) + return nil, false + } + result := make([]codersdk.MCPServerConfigUser, 0, len(users)) + for _, user := range users { + result = append(result, codersdk.MCPServerConfigUser{ + MinimalUser: db2sdk.MinimalUser(user), + Role: codersdk.MCPServerConfigRoleRead, + }) + } + return result, true +} + +func (api *API) mcpServerConfigACLGroups(ctx context.Context, rw http.ResponseWriter, entries database.ChatACL) ([]codersdk.MCPServerConfigGroup, bool) { + ids := parseMCPServerConfigACLIDs(entries) + var groups []database.GetGroupsRow + if len(ids) > 0 { + var err error + //nolint:gocritic // ACL managers may resolve principals after the share gate passes. + groups, err = api.Database.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{GroupIds: ids}) + if err != nil { + httpapi.InternalServerError(rw, err) + return nil, false + } + } + countByGroup := make(map[uuid.UUID]int64, len(groups)) + if len(groups) > 0 { + groupIDs := make([]uuid.UUID, 0, len(groups)) + for _, group := range groups { + groupIDs = append(groupIDs, group.Group.ID) + } + //nolint:gocritic // ACL managers may resolve group sizes after the share gate passes. + countRows, err := api.Database.GetGroupMembersCountByGroupIDs(dbauthz.AsSystemRestricted(ctx), database.GetGroupMembersCountByGroupIDsParams{ + GroupIds: groupIDs, + IncludeSystem: false, + }) + if err != nil { + httpapi.InternalServerError(rw, err) + return nil, false + } + for _, row := range countRows { + countByGroup[row.GroupID] = row.MemberCount + } + } + result := make([]codersdk.MCPServerConfigGroup, 0, len(groups)) + for _, group := range groups { + result = append(result, codersdk.MCPServerConfigGroup{ + Group: db2sdk.Group(group, nil, int(countByGroup[group.Group.ID])), + Role: codersdk.MCPServerConfigRoleRead, + }) + } + return result, true +} + +// canonicalMCPServerConfigACLRoles rekeys the request map by canonical +// uuid.String() values so noncanonical spellings hit the same keys RBAC +// reads, and rejects requests where two spellings collapse to one +// principal because map order would decide which role wins. Unparsable +// keys are skipped; acl.Validate already reports them. +func canonicalMCPServerConfigACLRoles(field string, roles map[string]codersdk.MCPServerConfigRole) (map[string]codersdk.MCPServerConfigRole, []codersdk.ValidationError) { + canonical := make(map[string]codersdk.MCPServerConfigRole, len(roles)) + var validErrs []codersdk.ValidationError + for rawID, role := range roles { + parsed, err := uuid.Parse(rawID) + if err != nil { + continue + } + id := parsed.String() + if _, ok := canonical[id]; ok { + validErrs = append(validErrs, codersdk.ValidationError{ + Field: field, + Detail: fmt.Sprintf("duplicate entries for ID %s", id), + }) + continue + } + canonical[id] = role + } + return canonical, validErrs +} + +func applyMCPServerConfigACLRoles(entries database.ChatACL, roles map[string]codersdk.MCPServerConfigRole) { + for id, role := range roles { + if role == codersdk.MCPServerConfigRoleDeleted { + delete(entries, id) + continue + } + entries[id] = database.ChatACLEntry{Permissions: []policy.Action{policy.ActionRead}} + } +} + +func parseMCPServerConfigACLIDs(entries database.ChatACL) []uuid.UUID { + ids := make([]uuid.UUID, 0, len(entries)) + for rawID := range entries { + if id, err := uuid.Parse(rawID); err == nil { + ids = append(ids, id) + } + } + return ids +} + +func (api *API) validateMCPServerConfigACLOrganization(ctx context.Context, organizationID uuid.UUID, req codersdk.UpdateMCPServerConfigACLRequest) []codersdk.ValidationError { + var validations []codersdk.ValidationError + userIDs := activeMCPServerConfigACLIDs(req.UserRoles) + if len(userIDs) > 0 { + //nolint:gocritic // Principal validation requires organization membership visibility. + memberships, err := api.Database.GetOrganizationIDsByMemberIDs(dbauthz.AsSystemRestricted(ctx), userIDs) + if err != nil { + return append(validations, codersdk.ValidationError{Field: "user_roles", Detail: err.Error()}) + } + byUser := make(map[uuid.UUID][]uuid.UUID, len(memberships)) + for _, membership := range memberships { + byUser[membership.UserID] = membership.OrganizationIDs + } + for _, id := range userIDs { + if !slices.Contains(byUser[id], organizationID) { + validations = append(validations, codersdk.ValidationError{ + Field: "user_roles", + Detail: "user " + id.String() + " does not belong to organization " + organizationID.String(), + }) + } + } + } + + groupIDs := activeMCPServerConfigACLIDs(req.GroupRoles) + if len(groupIDs) > 0 { + //nolint:gocritic // Principal validation requires group organization visibility. + groups, err := api.Database.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{GroupIds: groupIDs}) + if err != nil { + return append(validations, codersdk.ValidationError{Field: "group_roles", Detail: err.Error()}) + } + for _, group := range groups { + if group.Group.OrganizationID != organizationID { + validations = append(validations, codersdk.ValidationError{ + Field: "group_roles", + Detail: "group " + group.Group.ID.String() + " does not belong to organization " + organizationID.String(), + }) + } + } + } + return validations +} + +func activeMCPServerConfigACLIDs(roles map[string]codersdk.MCPServerConfigRole) []uuid.UUID { + ids := make([]uuid.UUID, 0, len(roles)) + for rawID, role := range roles { + if role == codersdk.MCPServerConfigRoleDeleted { + continue + } + if id, err := uuid.Parse(rawID); err == nil { + ids = append(ids, id) + } + } + return ids +} + +type MCPServerConfigACLUpdateValidator codersdk.UpdateMCPServerConfigACLRequest + +var _ acl.UpdateValidator[codersdk.MCPServerConfigRole] = MCPServerConfigACLUpdateValidator{} + +func (m MCPServerConfigACLUpdateValidator) Users() (map[string]codersdk.MCPServerConfigRole, string) { + return m.UserRoles, "user_roles" +} + +func (m MCPServerConfigACLUpdateValidator) Groups() (map[string]codersdk.MCPServerConfigRole, string) { + return m.GroupRoles, "group_roles" +} + +func (MCPServerConfigACLUpdateValidator) ValidateRole(role codersdk.MCPServerConfigRole) error { + if role == codersdk.MCPServerConfigRoleDeleted || role == codersdk.MCPServerConfigRoleRead { + return nil + } + return xerrors.Errorf("role %q is not a valid MCP server config role", role) +} diff --git a/coderd/mcp_test.go b/coderd/mcp_test.go index 900d1c85e3..c2ca25c36a 100644 --- a/coderd/mcp_test.go +++ b/coderd/mcp_test.go @@ -533,13 +533,13 @@ func TestMCPServerConfigsAudit(t *testing.T) { require.True(t, byResourceID[deletedID].IsDeleted) }) - t.Run("WriteDeniedAudited", func(t *testing.T) { + t.Run("WriteDeniedConcealed", func(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitLong) client, mAudit := newAuditedMCPClient(t) firstUser := coderdtest.CreateFirstUser(t, client) - memberClient, member := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID) + memberClient, _ := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID) config := createMCPServerConfig(t, client, firstUser.OrganizationID, "audit-denied", true) mAudit.ResetLogs() @@ -549,41 +549,25 @@ func TestMCPServerConfigsAudit(t *testing.T) { }) var sdkErr *codersdk.Error require.ErrorAs(t, err, &sdkErr) - require.Equal(t, http.StatusForbidden, sdkErr.StatusCode()) - - logs := mAudit.AuditLogs() - require.Len(t, logs, 1) - require.Equal(t, database.AuditActionWrite, logs[0].Action) - require.Equal(t, database.ResourceTypeMCPServerConfig, logs[0].ResourceType) - require.Equal(t, config.ID, logs[0].ResourceID) - require.Equal(t, member.ID, logs[0].UserID) - require.Equal(t, firstUser.OrganizationID, logs[0].OrganizationID) - require.EqualValues(t, http.StatusForbidden, logs[0].StatusCode) + require.Equal(t, http.StatusNotFound, sdkErr.StatusCode()) + require.Empty(t, mAudit.AuditLogs()) }) - t.Run("DeleteDeniedAudited", func(t *testing.T) { + t.Run("DeleteDeniedConcealed", func(t *testing.T) { t.Parallel() ctx := testutil.Context(t, testutil.WaitLong) client, mAudit := newAuditedMCPClient(t) firstUser := coderdtest.CreateFirstUser(t, client) - memberClient, member := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID) + memberClient, _ := coderdtest.CreateAnotherUser(t, client, firstUser.OrganizationID) config := createMCPServerConfig(t, client, firstUser.OrganizationID, "audit-delete-denied", true) mAudit.ResetLogs() err := memberClient.DeleteMCPServerConfig(ctx, firstUser.OrganizationID, config.ID) var sdkErr *codersdk.Error require.ErrorAs(t, err, &sdkErr) - require.Equal(t, http.StatusForbidden, sdkErr.StatusCode()) - - logs := mAudit.AuditLogs() - require.Len(t, logs, 1) - require.Equal(t, database.AuditActionDelete, logs[0].Action) - require.Equal(t, database.ResourceTypeMCPServerConfig, logs[0].ResourceType) - require.Equal(t, config.ID, logs[0].ResourceID) - require.Equal(t, member.ID, logs[0].UserID) - require.Equal(t, firstUser.OrganizationID, logs[0].OrganizationID) - require.EqualValues(t, http.StatusForbidden, logs[0].StatusCode) + require.Equal(t, http.StatusNotFound, sdkErr.StatusCode()) + require.Empty(t, mAudit.AuditLogs()) }) } @@ -641,6 +625,15 @@ func TestMCPServerConfigsNonAdmin(t *testing.T) { require.Len(t, memberConfigs, 1) require.Equal(t, "enabled-server", memberConfigs[0].Slug) + // Auditors list ACL-restricted configs but cannot read them row-level. + restricted := createMCPServerConfig(t, adminClient, firstUser.OrganizationID, "restricted-server", true) + err = adminClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, restricted.ID, codersdk.UpdateMCPServerConfigACLRequest{ + GroupRoles: map[string]codersdk.MCPServerConfigRole{ + firstUser.OrganizationID.String(): codersdk.MCPServerConfigRoleDeleted, + }, + }) + require.NoError(t, err) + // Auditors need the full management view of the MCP configs their // audit logs reference. for name, roles := range map[string][]rbac.RoleIdentifier{ @@ -650,7 +643,7 @@ func TestMCPServerConfigsNonAdmin(t *testing.T) { auditorClient, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID, roles...) auditorConfigs, err := auditorClient.MCPServerConfigs(ctx, firstUser.OrganizationID) require.NoError(t, err, name) - require.Len(t, auditorConfigs, 2, name) + require.Len(t, auditorConfigs, 3, name) for _, config := range auditorConfigs { require.NotEmpty(t, config.URL, "%s: %s", name, config.Slug) if !config.Enabled { @@ -659,9 +652,242 @@ func TestMCPServerConfigsNonAdmin(t *testing.T) { require.NotEmpty(t, fetched.URL, name) } } + + _, err = auditorClient.MCPServerConfigByID(ctx, firstUser.OrganizationID, restricted.ID) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr, name) + require.Equal(t, http.StatusNotFound, sdkErr.StatusCode(), name) } } +func TestMCPServerConfigsScopedKeyFullView(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + providerKeys := coderdtest.FakeOpenAICompatProviderAPIKeys(t) + adminClient, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ + DeploymentValues: mcpDeploymentValues(t), + ChatProviderAPIKeys: &providerKeys, + }) + firstUser := coderdtest.CreateFirstUser(t, adminClient) + _ = createMCPServerConfig(t, adminClient, firstUser.OrganizationID, "enabled-server", true) + _ = createMCPServerConfig(t, adminClient, firstUser.OrganizationID, "disabled-server", false) + + _, auditor := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID, + rbac.ScopedRoleOrgAuditor(firstUser.OrganizationID)) + + // MCP config scopes are not user-mintable, so seed scoped keys directly. + newScopedClient := func(userID uuid.UUID, scopes ...database.APIKeyScope) *codersdk.Client { + _, token := dbgen.APIKey(t, db, database.APIKey{ + UserID: userID, + Scopes: append(database.APIKeyScopes{"organization:read"}, scopes...), + }) + client := codersdk.New(adminClient.URL) + client.SetSessionToken(token) + return client + } + + // A key whose scope covers reading MCP configs keeps the full view. + fullViewConfigs, err := newScopedClient(firstUser.UserID, + "mcp_server_config:update", "mcp_server_config:read").MCPServerConfigs(ctx, firstUser.OrganizationID) + require.NoError(t, err) + require.Len(t, fullViewConfigs, 2) + + // Update-or-audit roles make the caller full-view eligible, but a key + // scoped without mcp_server_config:read must not receive config data. + for name, client := range map[string]*codersdk.Client{ + "AdminUpdateScope": newScopedClient(firstUser.UserID, "mcp_server_config:update"), + "AuditorAuditScope": newScopedClient(auditor.ID, "audit_log:read"), + } { + configs, err := client.MCPServerConfigs(ctx, firstUser.OrganizationID) + require.NoError(t, err, name) + require.Empty(t, configs, name) + } +} + +func TestMCPServerConfigACL(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + mAudit := audit.NewMock() + providerKeys := coderdtest.FakeOpenAICompatProviderAPIKeys(t) + adminClient, db := coderdtest.NewWithDatabase(t, &coderdtest.Options{ + DeploymentValues: mcpDeploymentValues(t), + ChatProviderAPIKeys: &providerKeys, + Auditor: mAudit, + }) + firstUser := coderdtest.CreateFirstUser(t, adminClient) + groupMemberClient, groupMember := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + nonMemberClient, nonMember := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + group := dbgen.Group(t, db, database.Group{OrganizationID: firstUser.OrganizationID}) + dbgen.GroupMember(t, db, database.GroupMemberTable{GroupID: group.ID, UserID: groupMember.ID}) + config := createMCPServerConfig(t, adminClient, firstUser.OrganizationID, "acl-server", true) + + mAudit.ResetLogs() + err := adminClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + GroupRoles: map[string]codersdk.MCPServerConfigRole{ + firstUser.OrganizationID.String(): codersdk.MCPServerConfigRoleDeleted, + group.ID.String(): codersdk.MCPServerConfigRoleRead, + }, + }) + require.NoError(t, err) + logs := mAudit.AuditLogs() + require.Len(t, logs, 1) + require.Equal(t, database.AuditActionWrite, logs[0].Action) + require.Equal(t, config.ID, logs[0].ResourceID) + require.EqualValues(t, http.StatusNoContent, logs[0].StatusCode) + + aclResponse, err := adminClient.MCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID) + require.NoError(t, err) + require.Len(t, aclResponse.Groups, 1) + require.Equal(t, group.ID, aclResponse.Groups[0].ID) + require.Equal(t, 1, aclResponse.Groups[0].TotalMemberCount) + + configs, err := groupMemberClient.MCPServerConfigs(ctx, firstUser.OrganizationID) + require.NoError(t, err) + require.Len(t, configs, 1) + _, err = groupMemberClient.MCPServerConfigByID(ctx, firstUser.OrganizationID, config.ID) + require.NoError(t, err) + + configs, err = nonMemberClient.MCPServerConfigs(ctx, firstUser.OrganizationID) + require.NoError(t, err) + require.Empty(t, configs) + _, err = nonMemberClient.MCPServerConfigByID(ctx, firstUser.OrganizationID, config.ID) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusNotFound, sdkErr.StatusCode()) + + _, err = groupMemberClient.MCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID) + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusNotFound, sdkErr.StatusCode()) + + mAudit.ResetLogs() + err = groupMemberClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{}) + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusNotFound, sdkErr.StatusCode()) + require.Empty(t, mAudit.AuditLogs()) + + err = adminClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + UserRoles: map[string]codersdk.MCPServerConfigRole{ + nonMember.ID.String(): codersdk.MCPServerConfigRoleRead, + }, + }) + require.NoError(t, err) + _, err = nonMemberClient.MCPServerConfigByID(ctx, firstUser.OrganizationID, config.ID) + require.NoError(t, err) + + // The sparse user-only update must merge with the existing ACL, not + // clobber the earlier group grant. + aclResponse, err = adminClient.MCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID) + require.NoError(t, err) + require.Len(t, aclResponse.Groups, 1) + require.Equal(t, group.ID, aclResponse.Groups[0].ID) + require.Len(t, aclResponse.Users, 1) + require.Equal(t, nonMember.ID, aclResponse.Users[0].ID) + + // Conflicting roles under two spellings of one principal are + // ambiguous and must be rejected rather than resolved by map order. + err = adminClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + UserRoles: map[string]codersdk.MCPServerConfigRole{ + nonMember.ID.String(): codersdk.MCPServerConfigRoleRead, + "urn:uuid:" + nonMember.ID.String(): codersdk.MCPServerConfigRoleDeleted, + }, + }) + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + require.Contains(t, sdkErr.Error(), "duplicate entries for ID "+nonMember.ID.String()) + + // Noncanonical UUID spellings pass validation, so grants and + // deletions must canonicalize to hit the same ACL keys RBAC reads. + err = adminClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + UserRoles: map[string]codersdk.MCPServerConfigRole{ + strings.ToUpper(groupMember.ID.String()): codersdk.MCPServerConfigRoleRead, + }, + }) + require.NoError(t, err) + aclResponse, err = adminClient.MCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID) + require.NoError(t, err) + require.Len(t, aclResponse.Users, 2) + + err = adminClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + UserRoles: map[string]codersdk.MCPServerConfigRole{ + "urn:uuid:" + nonMember.ID.String(): codersdk.MCPServerConfigRoleDeleted, + strings.ToUpper(groupMember.ID.String()): codersdk.MCPServerConfigRoleDeleted, + }, + }) + require.NoError(t, err) + aclResponse, err = adminClient.MCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID) + require.NoError(t, err) + require.Empty(t, aclResponse.Users) + _, err = nonMemberClient.MCPServerConfigByID(ctx, firstUser.OrganizationID, config.ID) + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusNotFound, sdkErr.StatusCode()) + + otherOrg := dbgen.Organization(t, db, database.Organization{}) + otherGroup := dbgen.Group(t, db, database.Group{OrganizationID: otherOrg.ID}) + err = adminClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + GroupRoles: map[string]codersdk.MCPServerConfigRole{ + otherGroup.ID.String(): codersdk.MCPServerConfigRoleRead, + }, + }) + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + require.Contains(t, sdkErr.Error(), otherGroup.ID.String()) + + foreignUser := dbgen.User(t, db, database.User{}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{OrganizationID: otherOrg.ID, UserID: foreignUser.ID}) + err = adminClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + UserRoles: map[string]codersdk.MCPServerConfigRole{ + foreignUser.ID.String(): codersdk.MCPServerConfigRoleRead, + }, + }) + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusBadRequest, sdkErr.StatusCode()) + require.Contains(t, sdkErr.Error(), foreignUser.ID.String()) +} + +// mcpServerConfigDeleteRaceStore deletes the config right after the param +// middleware read once armed, so the handler's locked re-fetch sees a +// concurrently deleted row. +type mcpServerConfigDeleteRaceStore struct { + database.Store + + armed atomic.Bool +} + +func (s *mcpServerConfigDeleteRaceStore) GetMCPServerConfigByID(ctx context.Context, id uuid.UUID) (database.MCPServerConfig, error) { + config, err := s.Store.GetMCPServerConfigByID(ctx, id) + if err == nil && s.armed.CompareAndSwap(true, false) { + if err := s.Store.DeleteMCPServerConfigByID(ctx, id); err != nil { + return database.MCPServerConfig{}, err + } + } + return config, err +} + +func TestMCPServerConfigACLConcurrentDelete(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + providerKeys := coderdtest.FakeOpenAICompatProviderAPIKeys(t) + db, ps := dbtestutil.NewDB(t) + store := &mcpServerConfigDeleteRaceStore{Store: db} + adminClient := coderdtest.New(t, &coderdtest.Options{ + DeploymentValues: mcpDeploymentValues(t), + ChatProviderAPIKeys: &providerKeys, + Database: store, + Pubsub: ps, + }) + firstUser := coderdtest.CreateFirstUser(t, adminClient) + config := createMCPServerConfig(t, adminClient, firstUser.OrganizationID, "acl-delete-race", true) + + store.armed.Store(true) + err := adminClient.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{}) + var sdkErr *codersdk.Error + require.ErrorAs(t, err, &sdkErr) + require.Equal(t, http.StatusNotFound, sdkErr.StatusCode()) +} + // TestMCPServerConfigsSecretsNeverLeaked is a load-bearing test that // ensures secret fields (OAuth2 client secret, API key value, custom // headers) are never present in API responses for any caller. If this diff --git a/coderd/rbac/acl/updatevalidator.go b/coderd/rbac/acl/updatevalidator.go index a3c0427101..10557b6741 100644 --- a/coderd/rbac/acl/updatevalidator.go +++ b/coderd/rbac/acl/updatevalidator.go @@ -11,7 +11,7 @@ import ( "github.com/coder/coder/v2/codersdk" ) -type UpdateValidator[Role codersdk.WorkspaceRole | codersdk.TemplateRole | codersdk.ChatRole] interface { +type UpdateValidator[Role codersdk.WorkspaceRole | codersdk.TemplateRole | codersdk.ChatRole | codersdk.MCPServerConfigRole] interface { // Users should return a map from user UUIDs (as strings) to the role they // are being assigned. Additionally, it should return a string that will be // used as the field name for the ValidationErrors returned from Validate. @@ -25,7 +25,7 @@ type UpdateValidator[Role codersdk.WorkspaceRole | codersdk.TemplateRole | coder ValidateRole(role Role) error } -func Validate[Role codersdk.WorkspaceRole | codersdk.TemplateRole | codersdk.ChatRole]( +func Validate[Role codersdk.WorkspaceRole | codersdk.TemplateRole | codersdk.ChatRole | codersdk.MCPServerConfigRole]( ctx context.Context, db database.Store, v UpdateValidator[Role], diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index c956750dab..f7cfafc60a 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -224,6 +224,7 @@ var ( // - "ActionCreate" :: create a new MCP server config // - "ActionDelete" :: delete MCP server config // - "ActionRead" :: read MCP server config + // - "ActionShare" :: share MCP server config with other users or groups // - "ActionUpdate" :: update MCP server config ResourceMCPServerConfig = Object{ Type: "mcp_server_config", diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 5909631054..7c75ec4e8d 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -89,6 +89,7 @@ var mcpServerConfigActions = map[Action]ActionDefinition{ ActionCreate: "create a new MCP server config", ActionRead: "read MCP server config", ActionUpdate: "update MCP server config", + ActionShare: "share MCP server config with other users or groups", ActionDelete: "delete MCP server config", } diff --git a/coderd/rbac/regosql/compile_test.go b/coderd/rbac/regosql/compile_test.go index d8842f8325..ddba44b013 100644 --- a/coderd/rbac/regosql/compile_test.go +++ b/coderd/rbac/regosql/compile_test.go @@ -268,6 +268,16 @@ func TestRegoQueries(t *testing.T) { p("user_acl->'me' ? '*'")), VariableConverter: regosql.DefaultVariableConverter(), }, + { + Name: "MCPServerConfigACLAllow", + Queries: []string{ + `"read" in input.object.acl_user_list["d5389ccc-57a4-4b13-8c3f-31747bcdc9f1"]`, + `"read" in input.object.acl_group_list["96c55a0e-73b4-44fc-abac-70d53c35c04c"]`, + }, + ExpectedSQL: "((mcp_server_configs.user_acl#>array['d5389ccc-57a4-4b13-8c3f-31747bcdc9f1', 'permissions'] ? 'read')" + + " OR (mcp_server_configs.group_acl#>array['96c55a0e-73b4-44fc-abac-70d53c35c04c', 'permissions'] ? 'read'))", + VariableConverter: regosql.MCPServerConfigConverter(), + }, { Name: "TemplateOwner", Queries: []string{ diff --git a/coderd/rbac/regosql/configs.go b/coderd/rbac/regosql/configs.go index 0414707fe6..f241e233e0 100644 --- a/coderd/rbac/regosql/configs.go +++ b/coderd/rbac/regosql/configs.go @@ -74,18 +74,15 @@ func ChatNoACLConverter() *sqltypes.VariableConverter { return matcher } -// MCPServerConfigNoACLConverter converts MCP server config permissions to SQL. -// Until sharing adds ACL columns, ACL matchers stay false and only organization -// ownership filters rows. -func MCPServerConfigNoACLConverter() *sqltypes.VariableConverter { +func MCPServerConfigConverter() *sqltypes.VariableConverter { matcher := sqltypes.NewVariableConverter().RegisterMatcher( resourceIDMatcher(), - organizationOwnerMatcher(), + sqltypes.StringVarMatcher("mcp_server_configs.organization_id :: text", []string{"input", "object", "org_owner"}), sqltypes.AlwaysFalse(userOwnerMatcher()), ) matcher.RegisterMatcher( - sqltypes.AlwaysFalse(groupACLMatcher(matcher)), - sqltypes.AlwaysFalse(userACLMatcher(matcher)), + ACLMappingMatcher(matcher, "mcp_server_configs.group_acl", []string{"input", "object", "acl_group_list"}).UsingSubfield("permissions"), + ACLMappingMatcher(matcher, "mcp_server_configs.user_acl", []string{"input", "object", "acl_user_list"}).UsingSubfield("permissions"), ) return matcher } diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index e8cc6af66a..fdb462b7eb 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -476,7 +476,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // Allow auditors to query deployment stats and insights. ResourceDeploymentStats.Type: {policy.ActionRead}, ResourceDeploymentConfig.Type: {policy.ActionRead}, - ResourceMCPServerConfig.Type: {policy.ActionRead}, // Allow auditors to query AI Bridge interceptions. ResourceAibridgeInterception.Type: {policy.ActionRead}, // Allow auditors to read boundary logs. @@ -612,7 +611,6 @@ func ReloadBuiltinRoles(opts *RoleOptions) { ResourceGroupMember.Type: {policy.ActionRead}, ResourceOrganization.Type: {policy.ActionRead}, ResourceOrganizationMember.Type: {policy.ActionRead}, - ResourceMCPServerConfig.Type: {policy.ActionRead}, }), Member: []Permission{}, }, @@ -1159,8 +1157,6 @@ func OrgMemberPermissions(org OrgSettings) OrgRolePermissions { ResourceOrganization.Type: {policy.ActionRead}, // Can read available roles. ResourceAssignOrgRole.Type: {policy.ActionRead}, - // TODO(mafredri): remove once CODAGT-712 adds per-config ACL evaluation. - ResourceMCPServerConfig.Type: {policy.ActionRead}, } // In all modes of workspace sharing but `none`, members need to @@ -1238,8 +1234,6 @@ func OrgServiceAccountPermissions(org OrgSettings) OrgRolePermissions { ResourceOrganization.Type: {policy.ActionRead}, // Can read available roles. ResourceAssignOrgRole.Type: {policy.ActionRead}, - // TODO(mafredri): remove once CODAGT-712 adds per-config ACL evaluation. - ResourceMCPServerConfig.Type: {policy.ActionRead}, } // When workspace sharing is enabled, service accounts need to see diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index fae1db672a..c04a16dfe3 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -825,17 +825,19 @@ func TestRolePermissions(t *testing.T) { }, }, { - Name: "MCPServerConfigRead", - Actions: []policy.Action{policy.ActionRead}, - Resource: rbac.ResourceMCPServerConfig.WithID(uuid.New()).InOrg(orgID), + Name: "MCPServerConfigRead", + Actions: []policy.Action{policy.ActionRead}, + Resource: rbac.ResourceMCPServerConfig.WithID(uuid.New()).InOrg(orgID).WithGroupACL(map[string][]policy.Action{ + orgID.String(): {policy.ActionRead}, + }), AuthorizeMap: map[bool][]hasAuthSubjects{ - true: {owner, orgAdmin, orgAuditor, auditor, orgMemberMe}, - false: {setOtherOrg, memberMe, agentsAccessUser, orgWorkspaceAccessUser, orgUserAdmin, orgTemplateAdmin, templateAdmin, userAdmin}, + true: {owner, orgAdmin, orgAuditor, orgMemberMe, agentsAccessUser, orgWorkspaceAccessUser, orgUserAdmin, orgTemplateAdmin}, + false: {setOtherOrg, memberMe, templateAdmin, userAdmin, auditor}, }, }, { Name: "MCPServerConfigWrite", - Actions: []policy.Action{policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete}, + Actions: []policy.Action{policy.ActionCreate, policy.ActionUpdate, policy.ActionDelete, policy.ActionShare}, Resource: rbac.ResourceMCPServerConfig.WithID(uuid.New()).InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin}, diff --git a/coderd/rbac/scopes_constants_gen.go b/coderd/rbac/scopes_constants_gen.go index 674aa56e7d..33ba267e93 100644 --- a/coderd/rbac/scopes_constants_gen.go +++ b/coderd/rbac/scopes_constants_gen.go @@ -76,6 +76,7 @@ const ( ScopeMcpServerConfigCreate ScopeName = "mcp_server_config:create" ScopeMcpServerConfigDelete ScopeName = "mcp_server_config:delete" ScopeMcpServerConfigRead ScopeName = "mcp_server_config:read" + ScopeMcpServerConfigShare ScopeName = "mcp_server_config:share" ScopeMcpServerConfigUpdate ScopeName = "mcp_server_config:update" ScopeNotificationMessageCreate ScopeName = "notification_message:create" ScopeNotificationMessageDelete ScopeName = "notification_message:delete" @@ -268,6 +269,7 @@ func (e ScopeName) Valid() bool { ScopeMcpServerConfigCreate, ScopeMcpServerConfigDelete, ScopeMcpServerConfigRead, + ScopeMcpServerConfigShare, ScopeMcpServerConfigUpdate, ScopeNotificationMessageCreate, ScopeNotificationMessageDelete, @@ -461,6 +463,7 @@ func AllScopeNameValues() []ScopeName { ScopeMcpServerConfigCreate, ScopeMcpServerConfigDelete, ScopeMcpServerConfigRead, + ScopeMcpServerConfigShare, ScopeMcpServerConfigUpdate, ScopeNotificationMessageCreate, ScopeNotificationMessageDelete, diff --git a/coderd/x/chatd/chatd.go b/coderd/x/chatd/chatd.go index b9888e1739..b54a273c3b 100644 --- a/coderd/x/chatd/chatd.go +++ b/coderd/x/chatd/chatd.go @@ -31,6 +31,7 @@ import ( "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/pubsub" + "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/notifications" coderdpubsub "github.com/coder/coder/v2/coderd/pubsub" "github.com/coder/coder/v2/coderd/rbac" @@ -1203,19 +1204,29 @@ type PromoteQueuedResult struct { PromotedMessage database.ChatMessage } -// enforceForcedMCPServerIDs appends the ID of every enabled Force On -// MCP server config in the chat's organization missing from ids. Force -// On availability is a server-side policy: callers must not be able to -// exclude such servers by stripping IDs from a request (Cure53 -// CDM-02-010). The forced set is read with daemon scope because -// regular users cannot read MCP server configs directly. -func enforceForcedMCPServerIDs(ctx context.Context, store database.Store, organizationID uuid.UUID, ids []uuid.UUID) ([]uuid.UUID, error) { - //nolint:gocritic // Non-admin users need chatd-scoped config reads here. - forced, err := store.GetForcedMCPServerConfigsByOrganization(dbauthz.AsChatd(ctx), organizationID) +// forcedMCPServerConfigsForOwner filters enabled Force On configs +// through the chat owner's ACL so availability cannot widen access. +func forcedMCPServerConfigsForOwner(ctx context.Context, store database.Store, organizationID, ownerID uuid.UUID) ([]database.MCPServerConfig, error) { + owner, _, err := httpmw.UserRBACSubject(ctx, store, ownerID, rbac.ScopeAll) + if err != nil { + return nil, xerrors.Errorf("load chat owner authorization: %w", err) + } + forced, err := store.GetForcedMCPServerConfigsByOrganization(dbauthz.As(ctx, owner), organizationID) + if err != nil { + return nil, xerrors.Errorf("get forced MCP server configs: %w", err) + } + return forced, nil +} + +// enforceForcedMCPServerIDs appends owner-readable Force On config IDs +// missing from ids so callers cannot exclude such servers by stripping +// IDs from a request (Cure53 CDM-02-010). +func enforceForcedMCPServerIDs(ctx context.Context, store database.Store, organizationID, ownerID uuid.UUID, ids []uuid.UUID) ([]uuid.UUID, error) { + forced, err := forcedMCPServerConfigsForOwner(ctx, store, organizationID, ownerID) if err != nil { // Fail closed: proceeding without the forced set would // silently bypass a security policy. - return nil, xerrors.Errorf("get forced MCP server configs: %w", err) + return nil, err } merged := slices.Clone(ids) if merged == nil { @@ -1258,7 +1269,7 @@ func (p *Server) CreateChat(ctx context.Context, opts CreateOptions) (database.C // Force On MCP servers are enforced server-side so a caller // cannot exclude them by stripping IDs from the request // (Cure53 CDM-02-010). - enforcedMCPServerIDs, err := enforceForcedMCPServerIDs(ctx, p.db, opts.OrganizationID, opts.MCPServerIDs) + enforcedMCPServerIDs, err := enforceForcedMCPServerIDs(ctx, p.db, opts.OrganizationID, opts.OwnerID, opts.MCPServerIDs) if err != nil { return database.Chat{}, err } @@ -1516,7 +1527,7 @@ func (p *Server) SendMessage( // Force On MCP servers are enforced server-side so a // caller cannot remove them by tampering with the // update (Cure53 CDM-02-010). - enforcedIDs, enforceErr := enforceForcedMCPServerIDs(ctx, store, lockedChat.OrganizationID, *requestedMCPServerIDs) + enforcedIDs, enforceErr := enforceForcedMCPServerIDs(ctx, store, lockedChat.OrganizationID, lockedChat.OwnerID, *requestedMCPServerIDs) if enforceErr != nil { return enforceErr } diff --git a/coderd/x/chatd/forced_mcp_test.go b/coderd/x/chatd/forced_mcp_test.go index 01aba71d2e..4f8e6d35d0 100644 --- a/coderd/x/chatd/forced_mcp_test.go +++ b/coderd/x/chatd/forced_mcp_test.go @@ -12,11 +12,18 @@ import ( "testing" "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/require" + "cdr.dev/slog/v3/sloggers/slogtest" + "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/httpmw" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/x/chatd" "github.com/coder/coder/v2/coderd/x/chatd/chattest" "github.com/coder/coder/v2/codersdk" @@ -89,14 +96,46 @@ func TestCreateChat_ForceOnMCPServerEnforced(t *testing.T) { CreatedBy: uuid.NullUUID{UUID: user.ID, Valid: true}, UpdatedBy: uuid.NullUUID{UUID: user.ID, Valid: true}, }) + // Grant read access only to an unrelated group. + dbgen.MCPServerConfig(t, db, database.MCPServerConfig{ + OrganizationID: org.ID, + DisplayName: "ACL Denied Forced MCP", + Slug: "acl-denied-forced-mcp", + Url: newEchoMCPTestServer(t, "acl-denied-forced-mcp"), + Availability: "force_on", + GroupACL: database.ChatACL{ + uuid.NewString(): {Permissions: []policy.Action{policy.ActionRead}}, + }, + UserACL: database.ChatACL{}, + CreatedBy: uuid.NullUUID{UUID: user.ID, Valid: true}, + UpdatedBy: uuid.NullUUID{UUID: user.ID, Valid: true}, + }) - server := newActiveTestServer(t, db, ps, func(cfg *chatd.Config) { + // Use the dbauthz wrapper so the MCP ACL post-filter runs. + authzDB := dbauthz.New( + db, + rbac.NewStrictCachingAuthorizer(prometheus.NewRegistry()), + slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}), + coderdtest.AccessControlStorePointer(), + ) + server := newActiveTestServer(t, authzDB, ps, func(cfg *chatd.Config) { withoutMCPToolSearch(cfg) cfg.AIBridgeTransportFactory = chatAIGatewayTransportFactoryPointer(chattest.NewMockAIBridgeTransport(t, openAIURL)) }) + // The seeded member needs agents-access to create a chat. + _, err := db.UpdateMemberRoles(dbauthz.AsSystemRestricted(ctx), database.UpdateMemberRolesParams{ + GrantedRoles: []string{rbac.RoleAgentsAccess()}, + UserID: user.ID, + OrgID: org.ID, + }) + require.NoError(t, err) + ownerSubject, _, err := httpmw.UserRBACSubject(dbauthz.AsSystemRestricted(ctx), db, user.ID, rbac.ScopeAll) + require.NoError(t, err) + ownerCtx := dbauthz.As(ctx, ownerSubject) + // The attacker strips every MCP server ID from the request. - chat, err := server.CreateChat(ctx, chatd.CreateOptions{ + chat, err := server.CreateChat(ownerCtx, chatd.CreateOptions{ OrganizationID: org.ID, OwnerID: user.ID, Title: "forced-mcp-create", @@ -127,6 +166,8 @@ func TestCreateChat_ForceOnMCPServerEnforced(t *testing.T) { require.NotEmpty(t, calls) require.Contains(t, calls[0], "forced-mcp__echo", "force_on MCP tools must be offered to the LLM despite a stripped mcp_server_ids list") + require.NotContains(t, calls[0], "acl-denied-forced-mcp__echo", + "force_on MCP tools must not be offered when the server's ACL denies the chat owner") } // TestSendMessage_ForceOnMCPServerEnforced reproduces CDM-02-010 for diff --git a/coderd/x/chatd/generation_preparer.go b/coderd/x/chatd/generation_preparer.go index b61491d3b4..a154b2ebad 100644 --- a/coderd/x/chatd/generation_preparer.go +++ b/coderd/x/chatd/generation_preparer.go @@ -25,15 +25,10 @@ import ( "github.com/coder/coder/v2/codersdk" ) -// effectiveMCPServerConfigs loads the MCP server configs for a turn: -// the chat's stored selection plus every enabled Force On config. -// Force On inclusion is enforced at generation time, not just at -// write time, so chats persisted before enforcement existed (or -// before an admin marked a server Force On) cannot dodge the policy -// (Cure53 CDM-02-010). Explore chats are exempt: their spawn-time -// snapshot is immutable by design and must never widen after spawn; -// Force On servers reach the snapshot through the parent chat's -// enforced ID list. +// effectiveMCPServerConfigs loads the chat's stored selection plus +// owner-readable Force On configs at generation time, so stored lists +// predating enforcement cannot dodge the policy (Cure53 CDM-02-010). +// Explore chats keep their immutable spawn-time snapshot instead. func (server *Server) effectiveMCPServerConfigs( ctx context.Context, logger slog.Logger, @@ -54,11 +49,11 @@ func (server *Server) effectiveMCPServerConfigs( if isExploreSubagentMode(chat.Mode) { return configs, nil } - forced, err := server.db.GetForcedMCPServerConfigsByOrganization(ctx, chat.OrganizationID) + forced, err := forcedMCPServerConfigsForOwner(ctx, server.db, chat.OrganizationID, chat.OwnerID) if err != nil { // Fail closed: running the turn without the forced set would // silently bypass a security policy. - return nil, xerrors.Errorf("get forced MCP server configs: %w", err) + return nil, err } seen := make(map[uuid.UUID]struct{}, len(configs)) for _, cfg := range configs { @@ -911,6 +906,9 @@ func latestAssistantText(messages []database.ChatMessage) string { return "" } +// ACLs are deliberately not re-checked: revocation blocks new selection but +// leaves already-selected servers usable, like template ACLs for running +// workspaces. Disabling or deleting the config cuts off existing chats. func enabledMCPServerConfigsForChatOrg( ctx context.Context, db database.Store, diff --git a/codersdk/apikey_scopes_gen.go b/codersdk/apikey_scopes_gen.go index 475b2bb76f..3284b0b04b 100644 --- a/codersdk/apikey_scopes_gen.go +++ b/codersdk/apikey_scopes_gen.go @@ -108,6 +108,7 @@ const ( APIKeyScopeMcpServerConfigCreate APIKeyScope = "mcp_server_config:create" APIKeyScopeMcpServerConfigDelete APIKeyScope = "mcp_server_config:delete" APIKeyScopeMcpServerConfigRead APIKeyScope = "mcp_server_config:read" + APIKeyScopeMcpServerConfigShare APIKeyScope = "mcp_server_config:share" APIKeyScopeMcpServerConfigUpdate APIKeyScope = "mcp_server_config:update" APIKeyScopeNotificationMessageAll APIKeyScope = "notification_message:*" APIKeyScopeNotificationMessageCreate APIKeyScope = "notification_message:create" diff --git a/codersdk/mcp.go b/codersdk/mcp.go index a26c855a3f..edb70ca997 100644 --- a/codersdk/mcp.go +++ b/codersdk/mcp.go @@ -102,6 +102,44 @@ type MCPServerConfig struct { AuthConnected bool `json:"auth_connected"` } +// MCPServerConfigRole is a role a user or group holds in an MCP server +// config's access control list. +type MCPServerConfigRole string + +const ( + MCPServerConfigRoleRead MCPServerConfigRole = "read" + // MCPServerConfigRoleDeleted removes the principal's ACL entry when + // used in an update request. + MCPServerConfigRoleDeleted MCPServerConfigRole = "" +) + +// MCPServerConfigACL is the resolved access control list of an MCP server +// config. +type MCPServerConfigACL struct { + Users []MCPServerConfigUser `json:"users"` + Groups []MCPServerConfigGroup `json:"groups"` +} + +// MCPServerConfigUser is a user entry in an MCP server config ACL. +type MCPServerConfigUser struct { + MinimalUser + Role MCPServerConfigRole `json:"role" enums:"read"` +} + +// MCPServerConfigGroup is a group entry in an MCP server config ACL. +type MCPServerConfigGroup struct { + Group + Role MCPServerConfigRole `json:"role" enums:"read"` +} + +// UpdateMCPServerConfigACLRequest is a sparse update of an MCP server +// config ACL: only the listed principals change, and +// MCPServerConfigRoleDeleted removes an entry. +type UpdateMCPServerConfigACLRequest struct { + UserRoles map[string]MCPServerConfigRole `json:"user_roles,omitempty"` + GroupRoles map[string]MCPServerConfigRole `json:"group_roles,omitempty"` +} + // CreateMCPServerConfigRequest is the request to create a new MCP server config. type CreateMCPServerConfigRequest struct { DisplayName string `json:"display_name" validate:"required"` @@ -200,6 +238,34 @@ func (c *Client) MCPServerConfigByID(ctx context.Context, organizationID, id uui return config, ReadBodyAsJSON(res, &config) } +// MCPServerConfigACL returns the resolved ACL of an MCP server config. +func (c *Client) MCPServerConfigACL(ctx context.Context, organizationID, id uuid.UUID) (MCPServerConfigACL, error) { + res, err := c.Request(ctx, http.MethodGet, fmt.Sprintf("/api/experimental/organizations/%s/mcp-servers/%s/acl", organizationID, id), nil) + if err != nil { + return MCPServerConfigACL{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return MCPServerConfigACL{}, ReadBodyAsError(res) + } + var acl MCPServerConfigACL + return acl, ReadBodyAsJSON(res, &acl) +} + +// UpdateMCPServerConfigACL applies a sparse ACL update to an MCP server +// config. +func (c *Client) UpdateMCPServerConfigACL(ctx context.Context, organizationID, id uuid.UUID, req UpdateMCPServerConfigACLRequest) error { + res, err := c.Request(ctx, http.MethodPatch, fmt.Sprintf("/api/experimental/organizations/%s/mcp-servers/%s/acl", organizationID, id), req) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} + func (c *Client) CreateMCPServerConfig(ctx context.Context, organizationID uuid.UUID, req CreateMCPServerConfigRequest) (MCPServerConfig, error) { res, err := c.Request(ctx, http.MethodPost, fmt.Sprintf("/api/experimental/organizations/%s/mcp-servers", organizationID), req) if err != nil { diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index f4302fc155..7858d9558f 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -108,7 +108,7 @@ var RBACResourceActions = map[RBACResource][]RBACAction{ ResourceIdpsyncSettings: {ActionRead, ActionUpdate}, ResourceInboxNotification: {ActionCreate, ActionRead, ActionUpdate}, ResourceLicense: {ActionCreate, ActionDelete, ActionRead}, - ResourceMCPServerConfig: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceMCPServerConfig: {ActionCreate, ActionDelete, ActionRead, ActionShare, ActionUpdate}, ResourceNotificationMessage: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceNotificationPreference: {ActionRead, ActionUpdate}, ResourceNotificationTemplate: {ActionRead, ActionUpdate}, diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 73dce51db7..b08703eb54 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -32,7 +32,7 @@ We track the following resources: | GroupSyncSettings
| |
FieldTracked
auto_create_missing_groupstrue
fieldtrue
legacy_group_name_mappingfalse
mappingtrue
regex_filtertrue
| | HealthSettings
| |
FieldTracked
dismissed_healthcheckstrue
idfalse
| | License
create, delete | |
FieldTracked
exptrue
idfalse
jwtfalse
uploaded_attrue
uuidtrue
| -| MCPServerConfig
create, write, delete | |
FieldTracked
allow_in_plan_modetrue
api_key_headertrue
api_key_valuetrue
api_key_value_key_idfalse
auth_typetrue
availabilitytrue
created_atfalse
created_bytrue
custom_headerstrue
custom_headers_key_idfalse
descriptiontrue
display_nametrue
enabledtrue
forward_coder_headerstrue
icon_urltrue
idfalse
model_intenttrue
oauth2_auth_urltrue
oauth2_client_idtrue
oauth2_client_secrettrue
oauth2_client_secret_key_idfalse
oauth2_revocation_urltrue
oauth2_scopestrue
oauth2_token_urltrue
organization_idfalse
slugtrue
tool_allow_listtrue
tool_deny_listtrue
transporttrue
updated_atfalse
updated_bytrue
urltrue
| +| MCPServerConfig
create, write, delete | |
FieldTracked
allow_in_plan_modetrue
api_key_headertrue
api_key_valuetrue
api_key_value_key_idfalse
auth_typetrue
availabilitytrue
created_atfalse
created_bytrue
custom_headerstrue
custom_headers_key_idfalse
descriptiontrue
display_nametrue
enabledtrue
forward_coder_headerstrue
group_acltrue
icon_urltrue
idfalse
model_intenttrue
oauth2_auth_urltrue
oauth2_client_idtrue
oauth2_client_secrettrue
oauth2_client_secret_key_idfalse
oauth2_revocation_urltrue
oauth2_scopestrue
oauth2_token_urltrue
organization_idfalse
slugtrue
tool_allow_listtrue
tool_deny_listtrue
transporttrue
updated_atfalse
updated_bytrue
urltrue
user_acltrue
| | NotificationTemplate
| |
FieldTracked
actionstrue
body_templatetrue
enabled_by_defaulttrue
grouptrue
idfalse
kindtrue
methodtrue
nametrue
title_templatetrue
| | NotificationsSettings
| |
FieldTracked
idfalse
notifier_pausedtrue
| | OAuth2ProviderApp
| |
FieldTracked
callback_urltrue
client_id_issued_atfalse
client_secret_expires_attrue
client_typetrue
client_uritrue
contactstrue
created_atfalse
dynamically_registeredtrue
grant_typestrue
icontrue
idfalse
jwkstrue
jwks_uritrue
logo_uritrue
nametrue
policy_uritrue
redirect_uristrue
registration_access_tokentrue
registration_client_uritrue
response_typestrue
scopetrue
software_idtrue
software_versiontrue
token_endpoint_auth_methodtrue
tos_uritrue
updated_atfalse
| diff --git a/docs/ai-coder/agents/platform-controls/mcp-servers.md b/docs/ai-coder/agents/platform-controls/mcp-servers.md index 7e871a5295..b4c6ee7b0a 100644 --- a/docs/ai-coder/agents/platform-controls/mcp-servers.md +++ b/docs/ai-coder/agents/platform-controls/mcp-servers.md @@ -44,11 +44,11 @@ organization's servers; other organizations are managed through the API. #### Availability policies -| Policy | Behavior | -|---------------|--------------------------------------------------------| -| `force_on` | Always injected into every chat. Users cannot opt out. | -| `default_on` | Pre-selected in new chats. Users can opt out. | -| `default_off` | Available in the server list but users must opt in. | +| Policy | Behavior | +|---------------|-----------------------------------------------------------------------------------| +| `force_on` | Injected into every chat whose owner has ACL access to the server. No opting out. | +| `default_on` | Pre-selected in new chats. Users can opt out. | +| `default_off` | Available in the server list but users must opt in. | ## Authentication @@ -168,11 +168,17 @@ wins. ## Permissions -| Action | Required role | -|-------------------------------|---------------------| -| Create, update, or delete | Organization admin | -| View enabled servers | Organization member | -| OAuth2 connect and disconnect | Organization member | +| Action | Required role | +|---------------------------|----------------------------| +| Create, update, or delete | Organization admin | +| View enabled servers | Member granted through ACL | +| OAuth2 connect | Member granted through ACL | +| OAuth2 disconnect | Token owner | +| Manage ACLs | Organization admin | + +Disconnect only needs a valid session: users removed from the ACL or the +organization can still delete their stored token and revoke the provider +grant. Members only see enabled servers in their own organizations. Sensitive fields such as API keys and client secrets are redacted in API responses. @@ -180,3 +186,18 @@ such as API keys and client secrets are redacted in API responses. The **MCP servers** settings page is part of deployment settings, so opening it in the dashboard also requires permission to edit deployment configuration. Organization admins without that permission can manage servers through the API. Creating or updating a server with `auth_type` set to `user_oidc` also requires the `deployment_config:update` permission. + +### Access control + +Each server has a group and user ACL that controls which members can see and +use it. New servers grant read access to the organization's **Everyone** group, +so all members have access by default. Admins can remove the Everyone entry and +grant specific groups or users instead through the API +(`GET`/`PATCH /api/experimental/organizations/{organization}/mcp-servers/{id}/acl`); there is no ACL editor +in the settings page. ACL management is available in all editions and does not +require an enterprise entitlement. ACL changes are recorded in the audit log. + +Revoking access stops a member from newly selecting the server in any chat, +but chats that already have the server selected keep using it, the same way +existing workspaces keep running after template access is revoked. To cut +off existing chats as well, disable or delete the server. diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 2ca7cad6d5..d8e48eb64e 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1388,9 +1388,9 @@ None #### Enumerated Values -| Value(s) | -|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `ai_gateway_key:*`, `ai_gateway_key:create`, `ai_gateway_key:delete`, `ai_gateway_key:read`, `ai_gateway_key:update`, `ai_model_price:*`, `ai_model_price:read`, `ai_model_price:update`, `ai_provider:*`, `ai_provider:create`, `ai_provider:delete`, `ai_provider:read`, `ai_provider:update`, `ai_seat:*`, `ai_seat:create`, `ai_seat:read`, `aibridge_interception:*`, `aibridge_interception:create`, `aibridge_interception:read`, `aibridge_interception:update`, `all`, `api_key:*`, `api_key:create`, `api_key:delete`, `api_key:read`, `api_key:update`, `application_connect`, `assign_org_role:*`, `assign_org_role:assign`, `assign_org_role:create`, `assign_org_role:delete`, `assign_org_role:read`, `assign_org_role:unassign`, `assign_org_role:update`, `assign_role:*`, `assign_role:assign`, `assign_role:read`, `assign_role:unassign`, `audit_log:*`, `audit_log:create`, `audit_log:read`, `boundary_log:*`, `boundary_log:create`, `boundary_log:delete`, `boundary_log:read`, `boundary_usage:*`, `boundary_usage:delete`, `boundary_usage:read`, `boundary_usage:update`, `chat:*`, `chat:create`, `chat:delete`, `chat:read`, `chat:share`, `chat:update`, `coder:all`, `coder:apikeys.manage_self`, `coder:application_connect`, `coder:templates.author`, `coder:templates.build`, `coder:workspaces.access`, `coder:workspaces.create`, `coder:workspaces.delete`, `coder:workspaces.operate`, `connection_log:*`, `connection_log:read`, `connection_log:update`, `crypto_key:*`, `crypto_key:create`, `crypto_key:delete`, `crypto_key:read`, `crypto_key:update`, `debug_info:*`, `debug_info:read`, `deployment_config:*`, `deployment_config:read`, `deployment_config:update`, `deployment_stats:*`, `deployment_stats:read`, `file:*`, `file:create`, `file:read`, `group:*`, `group:create`, `group:delete`, `group:read`, `group:update`, `group_member:*`, `group_member:read`, `idpsync_settings:*`, `idpsync_settings:read`, `idpsync_settings:update`, `inbox_notification:*`, `inbox_notification:create`, `inbox_notification:read`, `inbox_notification:update`, `license:*`, `license:create`, `license:delete`, `license:read`, `mcp_server_config:*`, `mcp_server_config:create`, `mcp_server_config:delete`, `mcp_server_config:read`, `mcp_server_config:update`, `notification_message:*`, `notification_message:create`, `notification_message:delete`, `notification_message:read`, `notification_message:update`, `notification_preference:*`, `notification_preference:read`, `notification_preference:update`, `notification_template:*`, `notification_template:read`, `notification_template:update`, `oauth2_app:*`, `oauth2_app:create`, `oauth2_app:delete`, `oauth2_app:read`, `oauth2_app:update`, `oauth2_app_code_token:*`, `oauth2_app_code_token:create`, `oauth2_app_code_token:delete`, `oauth2_app_code_token:read`, `oauth2_app_secret:*`, `oauth2_app_secret:create`, `oauth2_app_secret:delete`, `oauth2_app_secret:read`, `oauth2_app_secret:update`, `organization:*`, `organization:create`, `organization:delete`, `organization:read`, `organization:update`, `organization_member:*`, `organization_member:create`, `organization_member:delete`, `organization_member:read`, `organization_member:update`, `prebuilt_workspace:*`, `prebuilt_workspace:delete`, `prebuilt_workspace:update`, `provisioner_daemon:*`, `provisioner_daemon:create`, `provisioner_daemon:delete`, `provisioner_daemon:read`, `provisioner_daemon:update`, `provisioner_jobs:*`, `provisioner_jobs:create`, `provisioner_jobs:read`, `provisioner_jobs:update`, `replicas:*`, `replicas:read`, `system:*`, `system:create`, `system:delete`, `system:read`, `system:update`, `tailnet_coordinator:*`, `tailnet_coordinator:create`, `tailnet_coordinator:delete`, `tailnet_coordinator:read`, `tailnet_coordinator:update`, `task:*`, `task:create`, `task:delete`, `task:read`, `task:update`, `template:*`, `template:create`, `template:delete`, `template:read`, `template:update`, `template:use`, `template:view_insights`, `usage_event:*`, `usage_event:create`, `usage_event:read`, `usage_event:update`, `user:*`, `user:create`, `user:delete`, `user:read`, `user:read_personal`, `user:update`, `user:update_personal`, `user_secret:*`, `user_secret:create`, `user_secret:delete`, `user_secret:read`, `user_secret:update`, `user_skill:*`, `user_skill:create`, `user_skill:delete`, `user_skill:read`, `user_skill:update`, `webpush_subscription:*`, `webpush_subscription:create`, `webpush_subscription:delete`, `webpush_subscription:read`, `workspace:*`, `workspace:application_connect`, `workspace:create`, `workspace:create_agent`, `workspace:delete`, `workspace:delete_agent`, `workspace:read`, `workspace:share`, `workspace:ssh`, `workspace:start`, `workspace:stop`, `workspace:update`, `workspace:update_agent`, `workspace_agent_devcontainers:*`, `workspace_agent_devcontainers:create`, `workspace_agent_resource_monitor:*`, `workspace_agent_resource_monitor:create`, `workspace_agent_resource_monitor:read`, `workspace_agent_resource_monitor:update`, `workspace_build_orchestration:*`, `workspace_build_orchestration:create`, `workspace_build_orchestration:delete`, `workspace_build_orchestration:read`, `workspace_build_orchestration:update`, `workspace_dormant:*`, `workspace_dormant:application_connect`, `workspace_dormant:create`, `workspace_dormant:create_agent`, `workspace_dormant:delete`, `workspace_dormant:delete_agent`, `workspace_dormant:read`, `workspace_dormant:share`, `workspace_dormant:ssh`, `workspace_dormant:start`, `workspace_dormant:stop`, `workspace_dormant:update`, `workspace_dormant:update_agent`, `workspace_proxy:*`, `workspace_proxy:create`, `workspace_proxy:delete`, `workspace_proxy:read`, `workspace_proxy:update` | +| Value(s) | +|--------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `ai_gateway_key:*`, `ai_gateway_key:create`, `ai_gateway_key:delete`, `ai_gateway_key:read`, `ai_gateway_key:update`, `ai_model_price:*`, `ai_model_price:read`, `ai_model_price:update`, `ai_provider:*`, `ai_provider:create`, `ai_provider:delete`, `ai_provider:read`, `ai_provider:update`, `ai_seat:*`, `ai_seat:create`, `ai_seat:read`, `aibridge_interception:*`, `aibridge_interception:create`, `aibridge_interception:read`, `aibridge_interception:update`, `all`, `api_key:*`, `api_key:create`, `api_key:delete`, `api_key:read`, `api_key:update`, `application_connect`, `assign_org_role:*`, `assign_org_role:assign`, `assign_org_role:create`, `assign_org_role:delete`, `assign_org_role:read`, `assign_org_role:unassign`, `assign_org_role:update`, `assign_role:*`, `assign_role:assign`, `assign_role:read`, `assign_role:unassign`, `audit_log:*`, `audit_log:create`, `audit_log:read`, `boundary_log:*`, `boundary_log:create`, `boundary_log:delete`, `boundary_log:read`, `boundary_usage:*`, `boundary_usage:delete`, `boundary_usage:read`, `boundary_usage:update`, `chat:*`, `chat:create`, `chat:delete`, `chat:read`, `chat:share`, `chat:update`, `coder:all`, `coder:apikeys.manage_self`, `coder:application_connect`, `coder:templates.author`, `coder:templates.build`, `coder:workspaces.access`, `coder:workspaces.create`, `coder:workspaces.delete`, `coder:workspaces.operate`, `connection_log:*`, `connection_log:read`, `connection_log:update`, `crypto_key:*`, `crypto_key:create`, `crypto_key:delete`, `crypto_key:read`, `crypto_key:update`, `debug_info:*`, `debug_info:read`, `deployment_config:*`, `deployment_config:read`, `deployment_config:update`, `deployment_stats:*`, `deployment_stats:read`, `file:*`, `file:create`, `file:read`, `group:*`, `group:create`, `group:delete`, `group:read`, `group:update`, `group_member:*`, `group_member:read`, `idpsync_settings:*`, `idpsync_settings:read`, `idpsync_settings:update`, `inbox_notification:*`, `inbox_notification:create`, `inbox_notification:read`, `inbox_notification:update`, `license:*`, `license:create`, `license:delete`, `license:read`, `mcp_server_config:*`, `mcp_server_config:create`, `mcp_server_config:delete`, `mcp_server_config:read`, `mcp_server_config:share`, `mcp_server_config:update`, `notification_message:*`, `notification_message:create`, `notification_message:delete`, `notification_message:read`, `notification_message:update`, `notification_preference:*`, `notification_preference:read`, `notification_preference:update`, `notification_template:*`, `notification_template:read`, `notification_template:update`, `oauth2_app:*`, `oauth2_app:create`, `oauth2_app:delete`, `oauth2_app:read`, `oauth2_app:update`, `oauth2_app_code_token:*`, `oauth2_app_code_token:create`, `oauth2_app_code_token:delete`, `oauth2_app_code_token:read`, `oauth2_app_secret:*`, `oauth2_app_secret:create`, `oauth2_app_secret:delete`, `oauth2_app_secret:read`, `oauth2_app_secret:update`, `organization:*`, `organization:create`, `organization:delete`, `organization:read`, `organization:update`, `organization_member:*`, `organization_member:create`, `organization_member:delete`, `organization_member:read`, `organization_member:update`, `prebuilt_workspace:*`, `prebuilt_workspace:delete`, `prebuilt_workspace:update`, `provisioner_daemon:*`, `provisioner_daemon:create`, `provisioner_daemon:delete`, `provisioner_daemon:read`, `provisioner_daemon:update`, `provisioner_jobs:*`, `provisioner_jobs:create`, `provisioner_jobs:read`, `provisioner_jobs:update`, `replicas:*`, `replicas:read`, `system:*`, `system:create`, `system:delete`, `system:read`, `system:update`, `tailnet_coordinator:*`, `tailnet_coordinator:create`, `tailnet_coordinator:delete`, `tailnet_coordinator:read`, `tailnet_coordinator:update`, `task:*`, `task:create`, `task:delete`, `task:read`, `task:update`, `template:*`, `template:create`, `template:delete`, `template:read`, `template:update`, `template:use`, `template:view_insights`, `usage_event:*`, `usage_event:create`, `usage_event:read`, `usage_event:update`, `user:*`, `user:create`, `user:delete`, `user:read`, `user:read_personal`, `user:update`, `user:update_personal`, `user_secret:*`, `user_secret:create`, `user_secret:delete`, `user_secret:read`, `user_secret:update`, `user_skill:*`, `user_skill:create`, `user_skill:delete`, `user_skill:read`, `user_skill:update`, `webpush_subscription:*`, `webpush_subscription:create`, `webpush_subscription:delete`, `webpush_subscription:read`, `workspace:*`, `workspace:application_connect`, `workspace:create`, `workspace:create_agent`, `workspace:delete`, `workspace:delete_agent`, `workspace:read`, `workspace:share`, `workspace:ssh`, `workspace:start`, `workspace:stop`, `workspace:update`, `workspace:update_agent`, `workspace_agent_devcontainers:*`, `workspace_agent_devcontainers:create`, `workspace_agent_resource_monitor:*`, `workspace_agent_resource_monitor:create`, `workspace_agent_resource_monitor:read`, `workspace_agent_resource_monitor:update`, `workspace_build_orchestration:*`, `workspace_build_orchestration:create`, `workspace_build_orchestration:delete`, `workspace_build_orchestration:read`, `workspace_build_orchestration:update`, `workspace_dormant:*`, `workspace_dormant:application_connect`, `workspace_dormant:create`, `workspace_dormant:create_agent`, `workspace_dormant:delete`, `workspace_dormant:delete_agent`, `workspace_dormant:read`, `workspace_dormant:share`, `workspace_dormant:ssh`, `workspace_dormant:start`, `workspace_dormant:stop`, `workspace_dormant:update`, `workspace_dormant:update_agent`, `workspace_proxy:*`, `workspace_proxy:create`, `workspace_proxy:delete`, `workspace_proxy:read`, `workspace_proxy:update` | ## codersdk.AddLicenseRequest @@ -8844,6 +8844,159 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith | `updated_at` | string | false | | | | `url` | string | false | | | +## codersdk.MCPServerConfigACL + +```json +{ + "groups": [ + { + "avatar_url": "http://example.com", + "display_name": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "members": [ + { + "avatar_url": "http://example.com", + "created_at": "2019-08-24T14:15:22Z", + "email": "user@example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "is_service_account": true, + "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "", + "name": "string", + "status": "active", + "theme_preference": "string", + "updated_at": "2019-08-24T14:15:22Z", + "username": "string" + } + ], + "name": "string", + "organization_display_name": "string", + "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", + "organization_name": "string", + "quota_allowance": 0, + "role": "read", + "source": "user", + "total_member_count": 0 + } + ], + "users": [ + { + "avatar_url": "http://example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string", + "role": "read", + "username": "string" + } + ] +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|----------|-------------------------------------------------------------------------|----------|--------------|-------------| +| `groups` | array of [codersdk.MCPServerConfigGroup](#codersdkmcpserverconfiggroup) | false | | | +| `users` | array of [codersdk.MCPServerConfigUser](#codersdkmcpserverconfiguser) | false | | | + +## codersdk.MCPServerConfigGroup + +```json +{ + "avatar_url": "http://example.com", + "display_name": "string", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "members": [ + { + "avatar_url": "http://example.com", + "created_at": "2019-08-24T14:15:22Z", + "email": "user@example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "is_service_account": true, + "last_seen_at": "2019-08-24T14:15:22Z", + "login_type": "", + "name": "string", + "status": "active", + "theme_preference": "string", + "updated_at": "2019-08-24T14:15:22Z", + "username": "string" + } + ], + "name": "string", + "organization_display_name": "string", + "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", + "organization_name": "string", + "quota_allowance": 0, + "role": "read", + "source": "user", + "total_member_count": 0 +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|-----------------------------|--------------------------------------------------------------|----------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `avatar_url` | string | false | | | +| `display_name` | string | false | | | +| `id` | string | false | | | +| `members` | array of [codersdk.ReducedUser](#codersdkreduceduser) | false | | | +| `name` | string | false | | | +| `organization_display_name` | string | false | | | +| `organization_id` | string | false | | | +| `organization_name` | string | false | | | +| `quota_allowance` | integer | false | | | +| `role` | [codersdk.MCPServerConfigRole](#codersdkmcpserverconfigrole) | false | | | +| `source` | [codersdk.GroupSource](#codersdkgroupsource) | false | | | +| `total_member_count` | integer | false | | How many members are in this group. Shows the total count, even if the user is not authorized to read group member details. May be greater than `len(Group.Members)`. | + +#### Enumerated Values + +| Property | Value(s) | +|----------|----------| +| `role` | `read` | + +## codersdk.MCPServerConfigRole + +```json +"read" +``` + +### Properties + +#### Enumerated Values + +| Value(s) | +|------------| +| ``, `read` | + +## codersdk.MCPServerConfigUser + +```json +{ + "avatar_url": "http://example.com", + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string", + "role": "read", + "username": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|--------------|--------------------------------------------------------------|----------|--------------|-------------| +| `avatar_url` | string | false | | | +| `id` | string | true | | | +| `name` | string | false | | | +| `role` | [codersdk.MCPServerConfigRole](#codersdkmcpserverconfigrole) | false | | | +| `username` | string | true | | | + +#### Enumerated Values + +| Property | Value(s) | +|----------|----------| +| `role` | `read` | + ## codersdk.MCPServerOAuth2DisconnectResponse ```json @@ -14251,6 +14404,30 @@ Restarts will only happen on weekdays in this list on weeks which line up with W | `url` | string | false | | URL to download the latest release of Coder. | | `version` | string | false | | Version is the semantic version for the latest release of Coder. | +## codersdk.UpdateMCPServerConfigACLRequest + +```json +{ + "group_roles": { + "property1": "read", + "property2": "read" + }, + "user_roles": { + "property1": "read", + "property2": "read" + } +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|--------------------|--------------------------------------------------------------|----------|--------------|-------------| +| `group_roles` | object | false | | | +| » `[any property]` | [codersdk.MCPServerConfigRole](#codersdkmcpserverconfigrole) | false | | | +| `user_roles` | object | false | | | +| » `[any property]` | [codersdk.MCPServerConfigRole](#codersdkmcpserverconfigrole) | false | | | + ## codersdk.UpdateMCPServerConfigRequest ```json diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 2eb419decf..4303eaba86 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -533,6 +533,8 @@ var auditableResourcesTypes = map[any]map[string]Action{ "model_intent": ActionTrack, "allow_in_plan_mode": ActionTrack, "forward_coder_headers": ActionTrack, + "group_acl": ActionTrack, + "user_acl": ActionTrack, "oauth2_revocation_url": ActionTrack, "organization_id": ActionIgnore, }, diff --git a/enterprise/coderd/mcp_test.go b/enterprise/coderd/mcp_test.go index 02801b5e64..1d768483ee 100644 --- a/enterprise/coderd/mcp_test.go +++ b/enterprise/coderd/mcp_test.go @@ -71,6 +71,268 @@ func requireMCPServerConfigRequestStatus( require.Equal(t, wantStatus, res.StatusCode) } +func TestMCPServerConfigUpdateOnlyRoleReachesACLExcludedConfigs(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + owner, firstUser := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }, + }) + + enabled := createMCPServerConfigForOrganization(t, owner, firstUser.OrganizationID, "visible-mcp") + disabled := createMCPServerConfigForOrganization(t, owner, firstUser.OrganizationID, "hidden-mcp") + //nolint:gocritic // Owner access sets up the disabled fixture. + _, err := owner.UpdateMCPServerConfig(ctx, firstUser.OrganizationID, disabled.ID, + codersdk.UpdateMCPServerConfigRequest{Enabled: ptr.Ref(false)}) + require.NoError(t, err) + for _, config := range []codersdk.MCPServerConfig{enabled, disabled} { + //nolint:gocritic // Owner access removes the default ACL grant. + err = owner.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + GroupRoles: map[string]codersdk.MCPServerConfigRole{ + firstUser.OrganizationID.String(): codersdk.MCPServerConfigRoleDeleted, + }, + }) + require.NoError(t, err) + } + + //nolint:gocritic // Owner access isolates custom-role setup from the behavior under test. + role, err := owner.CreateOrganizationRole(ctx, codersdk.Role{ + Name: "mcp-update-only", + OrganizationID: firstUser.OrganizationID.String(), + OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceMCPServerConfig: {codersdk.ActionUpdate}, + }), + }) + require.NoError(t, err) + updateOnly, _ := coderdtest.CreateAnotherUser(t, owner, firstUser.OrganizationID, + rbac.RoleIdentifier{Name: role.Name, OrganizationID: firstUser.OrganizationID}) + + configs, err := updateOnly.MCPServerConfigs(ctx, firstUser.OrganizationID) + require.NoError(t, err) + require.Len(t, configs, 2) + for _, config := range configs { + require.Empty(t, config.URL) + } + + fetched, err := updateOnly.MCPServerConfigByID(ctx, firstUser.OrganizationID, disabled.ID) + require.NoError(t, err) + require.Equal(t, disabled.URL, fetched.URL) + + requireMCPServerConfigRequestStatus(t, updateOnly, http.MethodGet, + "/api/experimental/organizations/"+firstUser.OrganizationID.String()+"/mcp-servers/"+disabled.ID.String()+"/oauth2/connect", + nil, http.StatusNotFound) + + updatedName := "updated-hidden-mcp" + updated, err := updateOnly.UpdateMCPServerConfig(ctx, firstUser.OrganizationID, disabled.ID, + codersdk.UpdateMCPServerConfigRequest{DisplayName: &updatedName}) + require.NoError(t, err) + require.Equal(t, updatedName, updated.DisplayName) +} + +func TestMCPServerConfigDeleteOnlyRoleReachesDisabled(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + owner, firstUser := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }, + }) + + enabled := createMCPServerConfigForOrganization(t, owner, firstUser.OrganizationID, "enabled-mcp") + disabled := createMCPServerConfigForOrganization(t, owner, firstUser.OrganizationID, "disabled-mcp") + //nolint:gocritic // Owner access sets up the disabled fixture. + _, err := owner.UpdateMCPServerConfig(ctx, firstUser.OrganizationID, disabled.ID, + codersdk.UpdateMCPServerConfigRequest{Enabled: ptr.Ref(false)}) + require.NoError(t, err) + for _, config := range []codersdk.MCPServerConfig{enabled, disabled} { + //nolint:gocritic // Owner access removes the default ACL grant. + err = owner.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + GroupRoles: map[string]codersdk.MCPServerConfigRole{ + firstUser.OrganizationID.String(): codersdk.MCPServerConfigRoleDeleted, + }, + }) + require.NoError(t, err) + } + + //nolint:gocritic // Owner access isolates custom-role setup from the behavior under test. + role, err := owner.CreateOrganizationRole(ctx, codersdk.Role{ + Name: "mcp-delete-only", + OrganizationID: firstUser.OrganizationID.String(), + OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceMCPServerConfig: {codersdk.ActionDelete}, + }), + }) + require.NoError(t, err) + deleteOnly, _ := coderdtest.CreateAnotherUser(t, owner, firstUser.OrganizationID, + rbac.RoleIdentifier{Name: role.Name, OrganizationID: firstUser.OrganizationID}) + + configs, err := deleteOnly.MCPServerConfigs(ctx, firstUser.OrganizationID) + require.NoError(t, err) + require.Len(t, configs, 2) + for _, config := range configs { + require.Empty(t, config.URL) + } + + fetched, err := deleteOnly.MCPServerConfigByID(ctx, firstUser.OrganizationID, disabled.ID) + require.NoError(t, err) + require.Equal(t, disabled.ID, fetched.ID) + require.False(t, fetched.Enabled) + require.Empty(t, fetched.URL) + + requireMCPServerConfigRequestStatus(t, deleteOnly, http.MethodGet, + "/api/experimental/organizations/"+firstUser.OrganizationID.String()+"/mcp-servers/"+disabled.ID.String()+"/oauth2/connect", + nil, http.StatusNotFound) + + err = deleteOnly.DeleteMCPServerConfig(ctx, firstUser.OrganizationID, disabled.ID) + require.NoError(t, err) + + configs, err = deleteOnly.MCPServerConfigs(ctx, firstUser.OrganizationID) + require.NoError(t, err) + require.Len(t, configs, 1) + require.Equal(t, enabled.ID, configs[0].ID) +} + +func TestMCPServerConfigShareOnlyRoleRoutes(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + owner, firstUser := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }, + }) + //nolint:gocritic // Owner access creates a secret-bearing redaction fixture. + config, err := owner.CreateMCPServerConfig(ctx, firstUser.OrganizationID, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "share-only-mcp", + Slug: "share-only-mcp", + Transport: "streamable_http", + URL: "https://mcp.example.com/share-only-mcp", + AuthType: "api_key", + APIKeyHeader: "X-Api-Key", + APIKeyValue: "share-only-secret", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + //nolint:gocritic // Owner access removes the default ACL grant. + err = owner.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + GroupRoles: map[string]codersdk.MCPServerConfigRole{ + firstUser.OrganizationID.String(): codersdk.MCPServerConfigRoleDeleted, + }, + }) + require.NoError(t, err) + + //nolint:gocritic // Owner access isolates custom-role setup from the behavior under test. + role, err := owner.CreateOrganizationRole(ctx, codersdk.Role{ + Name: "mcp-share-only", + OrganizationID: firstUser.OrganizationID.String(), + OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceMCPServerConfig: {codersdk.ActionShare}, + }), + }) + require.NoError(t, err) + shareOnly, _ := coderdtest.CreateAnotherUser(t, owner, firstUser.OrganizationID, + rbac.RoleIdentifier{Name: role.Name, OrganizationID: firstUser.OrganizationID}) + + requireListed := func(enabled bool) { + configs, err := shareOnly.MCPServerConfigs(ctx, firstUser.OrganizationID) + require.NoError(t, err) + require.Len(t, configs, 1) + require.Equal(t, config.ID, configs[0].ID) + require.Equal(t, enabled, configs[0].Enabled) + require.Equal(t, "api_key", configs[0].AuthType) + require.True(t, configs[0].HasAPIKey) + require.Empty(t, configs[0].URL) + require.Empty(t, configs[0].Transport) + require.Empty(t, configs[0].APIKeyHeader) + } + requireListed(true) + + //nolint:gocritic // Owner access disables the fixture before share-only listing is retested. + _, err = owner.UpdateMCPServerConfig(ctx, firstUser.OrganizationID, config.ID, + codersdk.UpdateMCPServerConfigRequest{Enabled: ptr.Ref(false)}) + require.NoError(t, err) + requireListed(false) + + _, err = shareOnly.MCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID) + require.NoError(t, err) + err = shareOnly.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{}) + require.NoError(t, err) + + configPath := "/api/experimental/organizations/" + firstUser.OrganizationID.String() + "/mcp-servers/" + config.ID.String() + requireMCPServerConfigRequestStatus(t, shareOnly, http.MethodGet, configPath, nil, http.StatusNotFound) + requireMCPServerConfigRequestStatus(t, shareOnly, http.MethodGet, configPath+"/oauth2/connect", nil, http.StatusNotFound) + requireMCPServerConfigRequestStatus(t, shareOnly, http.MethodPatch, configPath, + codersdk.UpdateMCPServerConfigRequest{DisplayName: ptr.Ref("denied-update")}, http.StatusNotFound) + requireMCPServerConfigRequestStatus(t, shareOnly, http.MethodDelete, configPath, nil, http.StatusNotFound) +} + +func TestMCPServerConfigReadOnlyRoleCanConnect(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitLong) + owner, firstUser := coderdenttest.New(t, &coderdenttest.Options{ + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }, + }) + //nolint:gocritic // Owner access creates the fixture before read-only authorization is tested. + config, err := owner.CreateMCPServerConfig(ctx, firstUser.OrganizationID, codersdk.CreateMCPServerConfigRequest{ + DisplayName: "read-only-connect", + Slug: "read-only-connect", + Transport: "streamable_http", + URL: "https://mcp.example.com/read-only-connect", + AuthType: "oauth2", + OAuth2ClientID: "read-only-client", + OAuth2AuthURL: "https://auth.example.com/authorize", + OAuth2TokenURL: "https://auth.example.com/token", + Availability: "default_on", + Enabled: true, + ToolAllowList: []string{}, + ToolDenyList: []string{}, + }) + require.NoError(t, err) + //nolint:gocritic // Owner access removes the default ACL grant. + err = owner.UpdateMCPServerConfigACL(ctx, firstUser.OrganizationID, config.ID, codersdk.UpdateMCPServerConfigACLRequest{ + GroupRoles: map[string]codersdk.MCPServerConfigRole{ + firstUser.OrganizationID.String(): codersdk.MCPServerConfigRoleDeleted, + }, + }) + require.NoError(t, err) + + //nolint:gocritic // Owner access isolates custom-role setup from the behavior under test. + role, err := owner.CreateOrganizationRole(ctx, codersdk.Role{ + Name: "mcp-read-only", + OrganizationID: firstUser.OrganizationID.String(), + OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceMCPServerConfig: {codersdk.ActionRead}, + }), + }) + require.NoError(t, err) + readOnly, _ := coderdtest.CreateAnotherUser(t, owner, firstUser.OrganizationID, + rbac.RoleIdentifier{Name: role.Name, OrganizationID: firstUser.OrganizationID}) + readOnly.HTTPClient.CheckRedirect = func(_ *http.Request, _ []*http.Request) error { + return http.ErrUseLastResponse + } + + requireMCPServerConfigRequestStatus(t, readOnly, http.MethodGet, + "/api/experimental/organizations/"+firstUser.OrganizationID.String()+"/mcp-servers/"+config.ID.String()+"/oauth2/connect", + nil, http.StatusTemporaryRedirect) +} + func TestMCPServerConfigCollectionOrganizationIsolation(t *testing.T) { t.Parallel() diff --git a/site/src/api/rbacresourcesGenerated.ts b/site/src/api/rbacresourcesGenerated.ts index 3bded6faf1..a7d5fb2799 100644 --- a/site/src/api/rbacresourcesGenerated.ts +++ b/site/src/api/rbacresourcesGenerated.ts @@ -124,6 +124,7 @@ export const RBACResourceActions: Partial< create: "create a new MCP server config", delete: "delete MCP server config", read: "read MCP server config", + share: "share MCP server config with other users or groups", update: "update MCP server config", }, notification_message: { diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 058ae1d859..34c2e89005 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -753,6 +753,7 @@ export type APIKeyScope = | "mcp_server_config:create" | "mcp_server_config:delete" | "mcp_server_config:read" + | "mcp_server_config:share" | "mcp_server_config:update" | "notification_message:*" | "notification_message:create" @@ -998,6 +999,7 @@ export const APIKeyScopes: APIKeyScope[] = [ "mcp_server_config:create", "mcp_server_config:delete", "mcp_server_config:read", + "mcp_server_config:share", "mcp_server_config:update", "notification_message:*", "notification_message:create", @@ -6006,6 +6008,37 @@ export interface MCPServerConfig { readonly auth_connected: boolean; } +// From codersdk/mcp.go +/** + * MCPServerConfigACL is the resolved access control list of an MCP server + * config. + */ +export interface MCPServerConfigACL { + readonly users: readonly MCPServerConfigUser[]; + readonly groups: readonly MCPServerConfigGroup[]; +} + +// From codersdk/mcp.go +/** + * MCPServerConfigGroup is a group entry in an MCP server config ACL. + */ +export interface MCPServerConfigGroup extends Group { + readonly role: MCPServerConfigRole; +} + +// From codersdk/mcp.go +export type MCPServerConfigRole = "" | "read"; + +export const MCPServerConfigRoles: MCPServerConfigRole[] = ["", "read"]; + +// From codersdk/mcp.go +/** + * MCPServerConfigUser is a user entry in an MCP server config ACL. + */ +export interface MCPServerConfigUser extends MinimalUser { + readonly role: MCPServerConfigRole; +} + // From codersdk/mcp.go /** * MCPServerOAuth2DisconnectResponse reports whether the removed token @@ -9773,6 +9806,17 @@ export interface UpdateInboxNotificationReadStatusResponse { readonly unread_count: number; } +// From codersdk/mcp.go +/** + * UpdateMCPServerConfigACLRequest is a sparse update of an MCP server + * config ACL: only the listed principals change, and + * MCPServerConfigRoleDeleted removes an entry. + */ +export interface UpdateMCPServerConfigACLRequest { + readonly user_roles?: Record; + readonly group_roles?: Record; +} + // From codersdk/mcp.go /** * UpdateMCPServerConfigRequest is the request to update an MCP server config.