diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index fd03b85062..69ead44274 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -1196,6 +1196,167 @@ const docTemplate = `{ } } }, + "/api/experimental/organizations/{organization}/skills/{skillName}/acl": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "Organizations" + ], + "summary": "Get an organization skill ACL", + "operationId": "get-an-organization-skill-acl", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Skill name", + "name": "skillName", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSkillACL" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "patch": { + "consumes": [ + "application/json" + ], + "tags": [ + "Organizations" + ], + "summary": "Update an organization skill ACL", + "operationId": "update-an-organization-skill-acl", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Skill name", + "name": "skillName", + "in": "path", + "required": true + }, + { + "description": "Update organization skill ACL request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.UpdateOrganizationSkillACLRequest" + } + } + ], + "responses": { + "204": { + "description": "No Content" + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, + "/api/experimental/organizations/{organization}/skills/{skillName}/acl/available": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "Organizations" + ], + "summary": "Get available organization skill ACL users and groups", + "operationId": "get-available-organization-skill-acl-users-and-groups", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Skill name", + "name": "skillName", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "User search query; free-text search also applies to groups", + "name": "q", + "in": "query" + }, + { + "type": "string", + "format": "uuid", + "description": "User after ID", + "name": "after_id", + "in": "query" + }, + { + "type": "integer", + "description": "Page limit for users and groups, if 0 returns all candidates", + "name": "limit", + "in": "query" + }, + { + "type": "integer", + "description": "User page offset", + "name": "offset", + "in": "query" + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.ACLAvailable" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, "/api/experimental/users/email": { "put": { "consumes": [ @@ -27917,6 +28078,122 @@ const docTemplate = `{ } } }, + "codersdk.OrganizationSkillACL": { + "type": "object", + "properties": { + "groups": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.OrganizationSkillGroup" + } + }, + "users": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.OrganizationSkillUser" + } + } + } + }, + "codersdk.OrganizationSkillGroup": { + "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.OrganizationSkillRole" + } + ] + }, + "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.OrganizationSkillRole": { + "type": "string", + "enum": [ + "read", + "" + ], + "x-enum-varnames": [ + "OrganizationSkillRoleRead", + "OrganizationSkillRoleDeleted" + ] + }, + "codersdk.OrganizationSkillUser": { + "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.OrganizationSkillRole" + } + ] + }, + "username": { + "type": "string" + } + } + }, "codersdk.OrganizationSyncSettings": { "type": "object", "properties": { @@ -31775,6 +32052,23 @@ const docTemplate = `{ } } }, + "codersdk.UpdateOrganizationSkillACLRequest": { + "type": "object", + "properties": { + "group_roles": { + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/codersdk.OrganizationSkillRole" + } + }, + "user_roles": { + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/codersdk.OrganizationSkillRole" + } + } + } + }, "codersdk.UpdateRoles": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index a4e5661749..910d06afbe 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -1075,6 +1075,155 @@ } } }, + "/api/experimental/organizations/{organization}/skills/{skillName}/acl": { + "get": { + "produces": ["application/json"], + "tags": ["Organizations"], + "summary": "Get an organization skill ACL", + "operationId": "get-an-organization-skill-acl", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Skill name", + "name": "skillName", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.OrganizationSkillACL" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "patch": { + "consumes": ["application/json"], + "tags": ["Organizations"], + "summary": "Update an organization skill ACL", + "operationId": "update-an-organization-skill-acl", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Skill name", + "name": "skillName", + "in": "path", + "required": true + }, + { + "description": "Update organization skill ACL request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.UpdateOrganizationSkillACLRequest" + } + } + ], + "responses": { + "204": { + "description": "No Content" + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, + "/api/experimental/organizations/{organization}/skills/{skillName}/acl/available": { + "get": { + "produces": ["application/json"], + "tags": ["Organizations"], + "summary": "Get available organization skill ACL users and groups", + "operationId": "get-available-organization-skill-acl-users-and-groups", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Skill name", + "name": "skillName", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "User search query; free-text search also applies to groups", + "name": "q", + "in": "query" + }, + { + "type": "string", + "format": "uuid", + "description": "User after ID", + "name": "after_id", + "in": "query" + }, + { + "type": "integer", + "description": "Page limit for users and groups, if 0 returns all candidates", + "name": "limit", + "in": "query" + }, + { + "type": "integer", + "description": "User page offset", + "name": "offset", + "in": "query" + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.ACLAvailable" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, "/api/experimental/users/email": { "put": { "consumes": ["application/json"], @@ -25591,6 +25740,112 @@ } } }, + "codersdk.OrganizationSkillACL": { + "type": "object", + "properties": { + "groups": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.OrganizationSkillGroup" + } + }, + "users": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.OrganizationSkillUser" + } + } + } + }, + "codersdk.OrganizationSkillGroup": { + "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.OrganizationSkillRole" + } + ] + }, + "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.OrganizationSkillRole": { + "type": "string", + "enum": ["read", ""], + "x-enum-varnames": [ + "OrganizationSkillRoleRead", + "OrganizationSkillRoleDeleted" + ] + }, + "codersdk.OrganizationSkillUser": { + "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.OrganizationSkillRole" + } + ] + }, + "username": { + "type": "string" + } + } + }, "codersdk.OrganizationSyncSettings": { "type": "object", "properties": { @@ -29279,6 +29534,23 @@ } } }, + "codersdk.UpdateOrganizationSkillACLRequest": { + "type": "object", + "properties": { + "group_roles": { + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/codersdk.OrganizationSkillRole" + } + }, + "user_roles": { + "type": "object", + "additionalProperties": { + "$ref": "#/definitions/codersdk.OrganizationSkillRole" + } + } + } + }, "codersdk.UpdateRoles": { "type": "object", "properties": { diff --git a/coderd/coderd.go b/coderd/coderd.go index 4c11d75ea3..67a354f245 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1388,6 +1388,12 @@ func New(options *Options) *API { policy.ActionRead)).Patch("/", api.patchOrganizationSkill) r.With(httpmw.ExtractOrganizationSkillParam(options.Database, api.HTTPAuth.Authorize, policy.ActionRead, policy.ActionDelete)).Delete("/", api.deleteOrganizationSkill) + r.Route("/acl", func(r chi.Router) { + r.Use(httpmw.ExtractOrganizationSkillParam(options.Database, api.HTTPAuth.Authorize, policy.ActionShare)) + r.Get("/", api.getOrganizationSkillACL) + r.Patch("/", api.patchOrganizationSkillACL) + r.Get("/available", api.getOrganizationSkillACLAvailable) + }) }) }) api.registerExperimentalChatRoutes(r, apiKeyMiddleware) diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index b65c9e202d..65bca38184 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -7303,6 +7303,13 @@ func (q *querier) LockChatByID(ctx context.Context, id uuid.UUID) (uuid.UUID, er return q.db.LockChatByID(ctx, id) } +func (q *querier) LockOrganizationMembersByUserIDsForShare(ctx context.Context, arg database.LockOrganizationMembersByUserIDsForShareParams) ([]uuid.UUID, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceOrganizationMember.InOrg(arg.OrganizationID)); err != nil { + return nil, err + } + return q.db.LockOrganizationMembersByUserIDsForShare(ctx, arg) +} + func (q *querier) LockProvisionerKeyByIDForShare(ctx context.Context, id uuid.UUID) (uuid.UUID, error) { // The lock query returns only the key ID, so fetch the key to authorize // the read against its RBAC object. @@ -8220,6 +8227,13 @@ func (q *querier) UpdateOrganizationDeletedByID(ctx context.Context, arg databas return deleteQ(q.log, q.auth, q.db.GetOrganizationByID, deleteF)(ctx, arg.ID) } +func (q *querier) UpdateOrganizationSkillACLByID(ctx context.Context, arg database.UpdateOrganizationSkillACLByIDParams) (database.Skill, error) { + fetch := func(ctx context.Context, arg database.UpdateOrganizationSkillACLByIDParams) (database.Skill, error) { + return q.db.GetOrganizationSkillByIDForUpdate(ctx, arg.ID) + } + return fetchAndQuery(q.log, q.auth, policy.ActionShare, fetch, q.db.UpdateOrganizationSkillACLByID)(ctx, arg) +} + func (q *querier) UpdateOrganizationSkillByOrganizationIDAndName(ctx context.Context, arg database.UpdateOrganizationSkillByOrganizationIDAndNameParams) (database.Skill, error) { fetch := func(ctx context.Context, arg database.UpdateOrganizationSkillByOrganizationIDAndNameParams) (database.Skill, error) { return q.db.GetOrganizationSkillByOrganizationIDAndName(ctx, database.GetOrganizationSkillByOrganizationIDAndNameParams{ diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index fe92378b43..88933c542f 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -2738,6 +2738,13 @@ func (s *MethodTestSuite) TestOrganization() { dbm.EXPECT().GetOrganizationByName(gomock.Any(), arg).Return(o, nil).AnyTimes() check.Args(arg).Asserts(o, policy.ActionRead).Returns(o) })) + s.Run("LockOrganizationMembersByUserIDsForShare", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { + o := testutil.Fake(s.T(), faker, database.Organization{}) + ids := []uuid.UUID{uuid.New(), uuid.New()} + arg := database.LockOrganizationMembersByUserIDsForShareParams{OrganizationID: o.ID, UserIds: ids} + dbm.EXPECT().LockOrganizationMembersByUserIDsForShare(gomock.Any(), arg).Return(ids, nil).AnyTimes() + check.Args(arg).Asserts(rbac.ResourceOrganizationMember.InOrg(o.ID), policy.ActionRead).Returns(ids) + })) s.Run("GetOrganizationIDsByMemberIDs", s.Mocked(func(dbm *dbmock.MockStore, faker *gofakeit.Faker, check *expects) { oa := testutil.Fake(s.T(), faker, database.Organization{}) ob := testutil.Fake(s.T(), faker, database.Organization{}) @@ -6974,6 +6981,20 @@ func (s *MethodTestSuite) TestOrganizationSkills() { dbm.EXPECT().GetOrganizationSkillByIDForUpdate(gomock.Any(), skill.ID).Return(skill, nil).AnyTimes() check.Args(skill.ID).Asserts(skill, policy.ActionRead).Returns(skill) })) + s.Run("UpdateOrganizationSkillACLByID", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + skill := orgSkill(uuid.New(), "test") + arg := database.UpdateOrganizationSkillACLByIDParams{ + ID: skill.ID, + GroupACL: database.ChatACL{}, + UserACL: database.ChatACL{uuid.NewString(): {Permissions: []policy.Action{policy.ActionRead}}}, + } + updated := skill + updated.GroupACL = arg.GroupACL + updated.UserACL = arg.UserACL + dbm.EXPECT().GetOrganizationSkillByIDForUpdate(gomock.Any(), skill.ID).Return(skill, nil).AnyTimes() + dbm.EXPECT().UpdateOrganizationSkillACLByID(gomock.Any(), arg).Return(updated, nil).AnyTimes() + check.Args(arg).Asserts(skill, policy.ActionShare).Returns(updated) + })) } func (s *MethodTestSuite) TestUsageEvents() { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 7b72fc7058..56fb590a58 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -5152,6 +5152,14 @@ func (m queryMetricsStore) LockChatByID(ctx context.Context, id uuid.UUID) (uuid return r0, r1 } +func (m queryMetricsStore) LockOrganizationMembersByUserIDsForShare(ctx context.Context, arg database.LockOrganizationMembersByUserIDsForShareParams) ([]uuid.UUID, error) { + start := time.Now() + r0, r1 := m.s.LockOrganizationMembersByUserIDsForShare(ctx, arg) + m.queryLatencies.WithLabelValues("LockOrganizationMembersByUserIDsForShare").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "LockOrganizationMembersByUserIDsForShare").Inc() + return r0, r1 +} + func (m queryMetricsStore) LockProvisionerKeyByIDForShare(ctx context.Context, id uuid.UUID) (uuid.UUID, error) { start := time.Now() r0, r1 := m.s.LockProvisionerKeyByIDForShare(ctx, id) @@ -5825,6 +5833,14 @@ func (m queryMetricsStore) UpdateOrganizationDeletedByID(ctx context.Context, ar return r0 } +func (m queryMetricsStore) UpdateOrganizationSkillACLByID(ctx context.Context, arg database.UpdateOrganizationSkillACLByIDParams) (database.Skill, error) { + start := time.Now() + r0, r1 := m.s.UpdateOrganizationSkillACLByID(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateOrganizationSkillACLByID").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "UpdateOrganizationSkillACLByID").Inc() + return r0, r1 +} + func (m queryMetricsStore) UpdateOrganizationSkillByOrganizationIDAndName(ctx context.Context, arg database.UpdateOrganizationSkillByOrganizationIDAndNameParams) (database.Skill, error) { start := time.Now() r0, r1 := m.s.UpdateOrganizationSkillByOrganizationIDAndName(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 9a0914a9c3..d16fe737ff 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -9774,6 +9774,21 @@ func (mr *MockStoreMockRecorder) LockChatByID(ctx, id any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockChatByID", reflect.TypeOf((*MockStore)(nil).LockChatByID), ctx, id) } +// LockOrganizationMembersByUserIDsForShare mocks base method. +func (m *MockStore) LockOrganizationMembersByUserIDsForShare(ctx context.Context, arg database.LockOrganizationMembersByUserIDsForShareParams) ([]uuid.UUID, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "LockOrganizationMembersByUserIDsForShare", ctx, arg) + ret0, _ := ret[0].([]uuid.UUID) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// LockOrganizationMembersByUserIDsForShare indicates an expected call of LockOrganizationMembersByUserIDsForShare. +func (mr *MockStoreMockRecorder) LockOrganizationMembersByUserIDsForShare(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "LockOrganizationMembersByUserIDsForShare", reflect.TypeOf((*MockStore)(nil).LockOrganizationMembersByUserIDsForShare), ctx, arg) +} + // LockProvisionerKeyByIDForShare mocks base method. func (m *MockStore) LockProvisionerKeyByIDForShare(ctx context.Context, id uuid.UUID) (uuid.UUID, error) { m.ctrl.T.Helper() @@ -11036,6 +11051,21 @@ func (mr *MockStoreMockRecorder) UpdateOrganizationDeletedByID(ctx, arg any) *go return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrganizationDeletedByID", reflect.TypeOf((*MockStore)(nil).UpdateOrganizationDeletedByID), ctx, arg) } +// UpdateOrganizationSkillACLByID mocks base method. +func (m *MockStore) UpdateOrganizationSkillACLByID(ctx context.Context, arg database.UpdateOrganizationSkillACLByIDParams) (database.Skill, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateOrganizationSkillACLByID", ctx, arg) + ret0, _ := ret[0].(database.Skill) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateOrganizationSkillACLByID indicates an expected call of UpdateOrganizationSkillACLByID. +func (mr *MockStoreMockRecorder) UpdateOrganizationSkillACLByID(ctx, arg any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateOrganizationSkillACLByID", reflect.TypeOf((*MockStore)(nil).UpdateOrganizationSkillACLByID), ctx, arg) +} + // UpdateOrganizationSkillByOrganizationIDAndName mocks base method. func (m *MockStore) UpdateOrganizationSkillByOrganizationIDAndName(ctx context.Context, arg database.UpdateOrganizationSkillByOrganizationIDAndNameParams) (database.Skill, error) { m.ctrl.T.Helper() diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 56e593359b..41429317c6 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -1399,6 +1399,11 @@ type sqlcQuerier interface { // allocate a new snapshot version in one round trip. LockChatAndBumpSnapshotVersion(ctx context.Context, id uuid.UUID) (Chat, error) LockChatByID(ctx context.Context, id uuid.UUID) (uuid.UUID, error) + // Locks the listed users' membership rows with FOR KEY SHARE for the rest of + // the current transaction. FOR KEY SHARE conflicts with DELETE, so a member + // cannot be removed until the transaction ends, and a removal committed + // earlier is observed as a missing row. + LockOrganizationMembersByUserIDsForShare(ctx context.Context, arg LockOrganizationMembersByUserIDsForShareParams) ([]uuid.UUID, error) // Locks the provisioner key row with FOR KEY SHARE for the remainder of the // current transaction. FOR KEY SHARE conflicts with DELETE, so while the lock // is held the key cannot be deleted, and a committed deletion is observed as @@ -1649,6 +1654,7 @@ type sqlcQuerier interface { UpdateOAuth2ProviderAppByID(ctx context.Context, arg UpdateOAuth2ProviderAppByIDParams) (OAuth2ProviderApp, error) UpdateOrganization(ctx context.Context, arg UpdateOrganizationParams) (Organization, error) UpdateOrganizationDeletedByID(ctx context.Context, arg UpdateOrganizationDeletedByIDParams) error + UpdateOrganizationSkillACLByID(ctx context.Context, arg UpdateOrganizationSkillACLByIDParams) (Skill, error) UpdateOrganizationSkillByOrganizationIDAndName(ctx context.Context, arg UpdateOrganizationSkillByOrganizationIDAndNameParams) (Skill, error) UpdateOrganizationWorkspaceSharingSettings(ctx context.Context, arg UpdateOrganizationWorkspaceSharingSettingsParams) (Organization, error) // Cancels all pending provisioner jobs for prebuilt workspaces on a specific preset from an diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2f838824d1..aef0c47acd 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -22694,6 +22694,51 @@ func (q *sqlQuerier) InsertOrganizationMember(ctx context.Context, arg InsertOrg return i, err } +const lockOrganizationMembersByUserIDsForShare = `-- name: LockOrganizationMembersByUserIDsForShare :many +SELECT + user_id +FROM + organization_members +WHERE + organization_id = $1 + AND user_id = ANY($2 :: uuid [ ]) +ORDER BY + user_id +FOR KEY SHARE +` + +type LockOrganizationMembersByUserIDsForShareParams struct { + OrganizationID uuid.UUID `db:"organization_id" json:"organization_id"` + UserIds []uuid.UUID `db:"user_ids" json:"user_ids"` +} + +// Locks the listed users' membership rows with FOR KEY SHARE for the rest of +// the current transaction. FOR KEY SHARE conflicts with DELETE, so a member +// cannot be removed until the transaction ends, and a removal committed +// earlier is observed as a missing row. +func (q *sqlQuerier) LockOrganizationMembersByUserIDsForShare(ctx context.Context, arg LockOrganizationMembersByUserIDsForShareParams) ([]uuid.UUID, error) { + rows, err := q.db.QueryContext(ctx, lockOrganizationMembersByUserIDsForShare, arg.OrganizationID, pq.Array(arg.UserIds)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []uuid.UUID + for rows.Next() { + var user_id uuid.UUID + if err := rows.Scan(&user_id); err != nil { + return nil, err + } + items = append(items, user_id) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const organizationMembers = `-- name: OrganizationMembers :many SELECT organization_members.user_id, organization_members.organization_id, organization_members.created_at, organization_members.updated_at, organization_members.roles, @@ -28782,6 +28827,42 @@ func (q *sqlQuerier) ListUserSkillMetadataByUserID(ctx context.Context, userID u return items, nil } +const updateOrganizationSkillACLByID = `-- name: UpdateOrganizationSkillACLByID :one +UPDATE skills +SET + group_acl = $1, + user_acl = $2, + updated_at = now() +WHERE id = $3::uuid AND organization_id IS NOT NULL +RETURNING id, user_id, name, description, content, created_at, updated_at, organization_id, project_id, enabled, group_acl, user_acl +` + +type UpdateOrganizationSkillACLByIDParams struct { + GroupACL ChatACL `db:"group_acl" json:"group_acl"` + UserACL ChatACL `db:"user_acl" json:"user_acl"` + ID uuid.UUID `db:"id" json:"id"` +} + +func (q *sqlQuerier) UpdateOrganizationSkillACLByID(ctx context.Context, arg UpdateOrganizationSkillACLByIDParams) (Skill, error) { + row := q.db.QueryRowContext(ctx, updateOrganizationSkillACLByID, arg.GroupACL, arg.UserACL, arg.ID) + var i Skill + err := row.Scan( + &i.ID, + &i.UserID, + &i.Name, + &i.Description, + &i.Content, + &i.CreatedAt, + &i.UpdatedAt, + &i.OrganizationID, + &i.ProjectID, + &i.Enabled, + &i.GroupACL, + &i.UserACL, + ) + return i, err +} + const updateOrganizationSkillByOrganizationIDAndName = `-- name: UpdateOrganizationSkillByOrganizationIDAndName :one UPDATE skills SET diff --git a/coderd/database/queries/organizationmembers.sql b/coderd/database/queries/organizationmembers.sql index 2857e61efd..ade0afda7d 100644 --- a/coderd/database/queries/organizationmembers.sql +++ b/coderd/database/queries/organizationmembers.sql @@ -70,6 +70,22 @@ WHERE GROUP BY user_id; +-- name: LockOrganizationMembersByUserIDsForShare :many +-- Locks the listed users' membership rows with FOR KEY SHARE for the rest of +-- the current transaction. FOR KEY SHARE conflicts with DELETE, so a member +-- cannot be removed until the transaction ends, and a removal committed +-- earlier is observed as a missing row. +SELECT + user_id +FROM + organization_members +WHERE + organization_id = @organization_id + AND user_id = ANY(@user_ids :: uuid [ ]) +ORDER BY + user_id +FOR KEY SHARE; + -- name: UpdateMemberRoles :one UPDATE organization_members diff --git a/coderd/database/queries/skills.sql b/coderd/database/queries/skills.sql index 14cc876107..73d1b33c39 100644 --- a/coderd/database/queries/skills.sql +++ b/coderd/database/queries/skills.sql @@ -69,3 +69,12 @@ SELECT * FROM skills WHERE id = @id::uuid AND organization_id IS NOT NULL FOR UPDATE; + +-- name: UpdateOrganizationSkillACLByID :one +UPDATE skills +SET + group_acl = @group_acl, + user_acl = @user_acl, + updated_at = now() +WHERE id = @id::uuid AND organization_id IS NOT NULL +RETURNING *; diff --git a/coderd/mcp_acl.go b/coderd/mcp_acl.go index 572c9ae20a..64d2af4604 100644 --- a/coderd/mcp_acl.go +++ b/coderd/mcp_acl.go @@ -44,12 +44,18 @@ func (api *API) mcpServerConfigACL(rw http.ResponseWriter, r *http.Request) { return } - users, ok := api.mcpServerConfigACLUsers(ctx, rw, config.UserACL) - if !ok { + users, err := resolveACLUsers(ctx, api.Database, config.UserACL, func(user codersdk.MinimalUser) codersdk.MCPServerConfigUser { + return codersdk.MCPServerConfigUser{MinimalUser: user, Role: codersdk.MCPServerConfigRoleRead} + }) + if err != nil { + httpapi.InternalServerError(rw, err) return } - groups, ok := api.mcpServerConfigACLGroups(ctx, rw, config.GroupACL) - if !ok { + groups, err := resolveACLGroups(ctx, api.Database, config.GroupACL, func(group codersdk.Group) codersdk.MCPServerConfigGroup { + return codersdk.MCPServerConfigGroup{Group: group, Role: codersdk.MCPServerConfigRoleRead} + }) + if err != nil { + httpapi.InternalServerError(rw, err) return } httpapi.Write(ctx, rw, http.StatusOK, codersdk.MCPServerConfigACL{ @@ -73,13 +79,20 @@ func (api *API) mcpServerConfigACL(rw http.ResponseWriter, r *http.Request) { // @Router /api/v2/organizations/{organization}/mcp-servers/{mcpserverconfig}/acl/available [get] // @x-apidocgen {"skip": true} func (api *API) mcpServerConfigACLAvailable(rw http.ResponseWriter, r *http.Request) { - ctx := r.Context() config := httpmw.MCPServerConfigParam(r) if !api.Authorize(r, policy.ActionShare, config.RBACObject()) { httpapi.ResourceNotFound(rw) return } + api.writeOrganizationACLAvailable(rw, r, config.OrganizationID) +} + +// writeOrganizationACLAvailable writes the organization members and groups that +// can be granted access to a resource in organizationID. Callers must have +// authorized share on that resource. +func (api *API) writeOrganizationACLAvailable(rw http.ResponseWriter, r *http.Request, organizationID uuid.UUID) { + ctx := r.Context() userFilter, validations := searchquery.Users(r.URL.Query().Get("q")) if len(validations) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ @@ -93,13 +106,13 @@ func (api *API) mcpServerConfigACLAvailable(rw http.ResponseWriter, r *http.Requ return } - //nolint:gocritic // The MCP server config share permission authorizes this - // bounded organization-scoped lookup even when the caller cannot browse the - // ordinary directories. + //nolint:gocritic // The resource share permission checked by the caller + // authorizes this bounded organization-scoped lookup even when the caller + // cannot browse the ordinary directories. restrictedCtx := dbauthz.AsSystemRestricted(ctx) members, err := api.Database.PaginatedOrganizationMembers(restrictedCtx, database.PaginatedOrganizationMembersParams{ AfterID: pagination.AfterID, - OrganizationID: config.OrganizationID, + OrganizationID: organizationID, Search: userFilter.Search, Name: userFilter.Name, ExactUsername: userFilter.ExactUsername, @@ -120,18 +133,18 @@ func (api *API) mcpServerConfigACLAvailable(rw http.ResponseWriter, r *http.Requ LimitOpt: int32(pagination.Limit), }) if err != nil { - httpapi.InternalServerError(rw, xerrors.Errorf("list MCP server config ACL users: %w", err)) + httpapi.InternalServerError(rw, xerrors.Errorf("list ACL candidate users: %w", err)) return } groups, err := api.Database.GetGroups(restrictedCtx, database.GetGroupsParams{ - OrganizationID: config.OrganizationID, + OrganizationID: organizationID, Search: userFilter.Search, // #nosec G115 - Pagination limits are small and fit in int32. LimitOpt: int32(pagination.Limit), }) if err != nil && !xerrors.Is(err, sql.ErrNoRows) { - httpapi.InternalServerError(rw, xerrors.Errorf("list MCP server config ACL groups: %w", err)) + httpapi.InternalServerError(rw, xerrors.Errorf("list ACL candidate groups: %w", err)) return } @@ -139,14 +152,15 @@ func (api *API) mcpServerConfigACLAvailable(rw http.ResponseWriter, r *http.Requ for i, group := range groups { groupIDs[i] = group.Group.ID } - countByGroup, ok := api.mcpServerConfigACLGroupMemberCounts(restrictedCtx, rw, groupIDs) - if !ok { + countByGroup, err := aclGroupMemberCounts(restrictedCtx, api.Database, groupIDs) + if err != nil { + httpapi.InternalServerError(rw, err) return } sdkUsers := make([]codersdk.ReducedUser, 0, len(members)) for _, member := range members { - sdkUsers = append(sdkUsers, mcpServerConfigACLReducedUser(member)) + sdkUsers = append(sdkUsers, reducedUserFromPaginatedOrganizationMember(member)) } sdkGroups := make([]codersdk.Group, 0, len(groups)) for _, group := range groups { @@ -194,12 +208,7 @@ func (api *API) patchMCPServerConfigACL(rw http.ResponseWriter, r *http.Request) 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...) + userRoles, groupRoles, validations := validateOrganizationACLUpdate(ctx, api.Database, config.OrganizationID, mcpServerConfigACLUpdateValidator(req)) if len(validations) > 0 { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ Message: "Invalid request to update MCP server config ACL.", @@ -218,8 +227,8 @@ func (api *API) patchMCPServerConfigACL(rw http.ResponseWriter, r *http.Request) aReq.Old = current userACL := maps.Clone(current.UserACL) groupACL := maps.Clone(current.GroupACL) - applyMCPServerConfigACLRoles(userACL, userRoles) - applyMCPServerConfigACLRoles(groupACL, groupRoles) + applyACLReadRoles(userACL, userRoles) + applyACLReadRoles(groupACL, groupRoles) if err := tx.UpdateMCPServerConfigACLByID(ctx, database.UpdateMCPServerConfigACLByIDParams{ ID: config.ID, UserACL: userACL, @@ -249,34 +258,33 @@ func (api *API) patchMCPServerConfigACL(rw http.ResponseWriter, r *http.Request) rw.WriteHeader(http.StatusNoContent) } -func (api *API) mcpServerConfigACLUsers(ctx context.Context, rw http.ResponseWriter, entries database.ChatACL) ([]codersdk.MCPServerConfigUser, bool) { - ids := parseMCPServerConfigACLIDs(entries) +// resolveACLUsers resolves user ACL entries for callers that already passed +// the resource's share check. +func resolveACLUsers[T any](ctx context.Context, db database.Store, entries database.ChatACL, entry func(codersdk.MinimalUser) T) ([]T, error) { + ids := parseACLIDs(entries) //nolint:gocritic // ACL managers may resolve principals after the share gate passes. - users, err := api.Database.GetUsersByIDs(dbauthz.AsSystemRestricted(ctx), ids) + users, err := db.GetUsersByIDs(dbauthz.AsSystemRestricted(ctx), ids) if err != nil { - httpapi.InternalServerError(rw, err) - return nil, false + return nil, xerrors.Errorf("get ACL users: %w", err) } - result := make([]codersdk.MCPServerConfigUser, 0, len(users)) + result := make([]T, 0, len(users)) for _, user := range users { - result = append(result, codersdk.MCPServerConfigUser{ - MinimalUser: db2sdk.MinimalUser(user), - Role: codersdk.MCPServerConfigRoleRead, - }) + result = append(result, entry(db2sdk.MinimalUser(user))) } - return result, true + return result, nil } -func (api *API) mcpServerConfigACLGroups(ctx context.Context, rw http.ResponseWriter, entries database.ChatACL) ([]codersdk.MCPServerConfigGroup, bool) { - ids := parseMCPServerConfigACLIDs(entries) +// resolveACLGroups resolves group ACL entries, including member counts, for +// callers that already passed the resource's share check. +func resolveACLGroups[T any](ctx context.Context, db database.Store, entries database.ChatACL, entry func(codersdk.Group) T) ([]T, error) { + ids := parseACLIDs(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}) + groups, err = db.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{GroupIds: ids}) if err != nil { - httpapi.InternalServerError(rw, err) - return nil, false + return nil, xerrors.Errorf("get ACL groups: %w", err) } } groupIDs := make([]uuid.UUID, 0, len(groups)) @@ -284,65 +292,57 @@ func (api *API) mcpServerConfigACLGroups(ctx context.Context, rw http.ResponseWr groupIDs = append(groupIDs, group.Group.ID) } //nolint:gocritic // ACL managers may resolve group sizes after the share gate passes. - countByGroup, ok := api.mcpServerConfigACLGroupMemberCounts(dbauthz.AsSystemRestricted(ctx), rw, groupIDs) - if !ok { - return nil, false + countByGroup, err := aclGroupMemberCounts(dbauthz.AsSystemRestricted(ctx), db, groupIDs) + if err != nil { + return nil, err } - result := make([]codersdk.MCPServerConfigGroup, 0, len(groups)) + result := make([]T, 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, - }) + result = append(result, entry(db2sdk.Group(group, nil, int(countByGroup[group.Group.ID])))) } - return result, true + return result, nil } -func (api *API) mcpServerConfigACLGroupMemberCounts(ctx context.Context, rw http.ResponseWriter, groupIDs []uuid.UUID) (map[uuid.UUID]int64, bool) { +func aclGroupMemberCounts(ctx context.Context, db database.Store, groupIDs []uuid.UUID) (map[uuid.UUID]int64, error) { countByGroup := make(map[uuid.UUID]int64, len(groupIDs)) if len(groupIDs) == 0 { - return countByGroup, true + return countByGroup, nil } - countRows, err := api.Database.GetGroupMembersCountByGroupIDs(ctx, database.GetGroupMembersCountByGroupIDsParams{ + countRows, err := db.GetGroupMembersCountByGroupIDs(ctx, database.GetGroupMembersCountByGroupIDsParams{ GroupIds: groupIDs, IncludeSystem: false, }) if err != nil && !xerrors.Is(err, sql.ErrNoRows) { - httpapi.InternalServerError(rw, xerrors.Errorf("count MCP server config ACL group members: %w", err)) - return nil, false + return nil, xerrors.Errorf("count ACL group members: %w", err) } for _, row := range countRows { countByGroup[row.GroupID] = row.MemberCount } - return countByGroup, true + return countByGroup, nil } -func mcpServerConfigACLReducedUser(member database.PaginatedOrganizationMembersRow) codersdk.ReducedUser { - return codersdk.ReducedUser{ - MinimalUser: codersdk.MinimalUser{ - ID: member.OrganizationMember.UserID, - Username: member.Username, - Name: member.Name, - AvatarURL: member.AvatarURL, - }, - Email: member.Email, - CreatedAt: member.UserCreatedAt, - UpdatedAt: member.UserUpdatedAt, - LastSeenAt: member.LastSeenAt, - Status: codersdk.UserStatus(member.Status), - LoginType: codersdk.LoginType(member.LoginType), - IsServiceAccount: member.IsServiceAccount, - } +// validateOrganizationACLUpdate validates a sparse ACL update for a resource +// in organizationID and returns its roles rekeyed by canonical UUID. +func validateOrganizationACLUpdate[R acl.Role](ctx context.Context, db database.Store, organizationID uuid.UUID, v acl.UpdateValidator[R]) (userRoles, groupRoles map[string]R, validations []codersdk.ValidationError) { + users, usersField := v.Users() + groups, groupsField := v.Groups() + validations = acl.Validate(ctx, db, v) + validations = append(validations, validateACLOrganization(ctx, db, organizationID, usersField, users, groupsField, groups)...) + userRoles, dupErrs := canonicalACLRoles(usersField, users) + validations = append(validations, dupErrs...) + groupRoles, dupErrs = canonicalACLRoles(groupsField, groups) + validations = append(validations, dupErrs...) + return userRoles, groupRoles, validations } -// 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)) +// canonicalACLRoles 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 canonicalACLRoles[R acl.Role](field string, roles map[string]R) (map[string]R, []codersdk.ValidationError) { + canonical := make(map[string]R, len(roles)) var validErrs []codersdk.ValidationError for rawID, role := range roles { parsed, err := uuid.Parse(rawID) @@ -362,9 +362,11 @@ func canonicalMCPServerConfigACLRoles(field string, roles map[string]codersdk.MC return canonical, validErrs } -func applyMCPServerConfigACLRoles(entries database.ChatACL, roles map[string]codersdk.MCPServerConfigRole) { +// applyACLReadRoles applies sparse roles to entries: the empty role removes an +// entry and every other validated role grants read. +func applyACLReadRoles[R acl.Role](entries database.ChatACL, roles map[string]R) { for id, role := range roles { - if role == codersdk.MCPServerConfigRoleDeleted { + if string(role) == "" { delete(entries, id) continue } @@ -372,7 +374,7 @@ func applyMCPServerConfigACLRoles(entries database.ChatACL, roles map[string]cod } } -func parseMCPServerConfigACLIDs(entries database.ChatACL) []uuid.UUID { +func parseACLIDs(entries database.ChatACL) []uuid.UUID { ids := make([]uuid.UUID, 0, len(entries)) for rawID := range entries { if id, err := uuid.Parse(rawID); err == nil { @@ -382,14 +384,14 @@ func parseMCPServerConfigACLIDs(entries database.ChatACL) []uuid.UUID { return ids } -func (api *API) validateMCPServerConfigACLOrganization(ctx context.Context, organizationID uuid.UUID, req codersdk.UpdateMCPServerConfigACLRequest) []codersdk.ValidationError { +func validateACLOrganization[R acl.Role](ctx context.Context, db database.Store, organizationID uuid.UUID, usersField string, userRoles map[string]R, groupsField string, groupRoles map[string]R) []codersdk.ValidationError { var validations []codersdk.ValidationError - userIDs := activeMCPServerConfigACLIDs(req.UserRoles) + userIDs := activeACLIDs(userRoles) if len(userIDs) > 0 { //nolint:gocritic // Principal validation requires organization membership visibility. - memberships, err := api.Database.GetOrganizationIDsByMemberIDs(dbauthz.AsSystemRestricted(ctx), userIDs) + memberships, err := db.GetOrganizationIDsByMemberIDs(dbauthz.AsSystemRestricted(ctx), userIDs) if err != nil { - return append(validations, codersdk.ValidationError{Field: "user_roles", Detail: err.Error()}) + return append(validations, codersdk.ValidationError{Field: usersField, Detail: err.Error()}) } byUser := make(map[uuid.UUID][]uuid.UUID, len(memberships)) for _, membership := range memberships { @@ -398,24 +400,24 @@ func (api *API) validateMCPServerConfigACLOrganization(ctx context.Context, orga for _, id := range userIDs { if !slices.Contains(byUser[id], organizationID) { validations = append(validations, codersdk.ValidationError{ - Field: "user_roles", + Field: usersField, Detail: "user " + id.String() + " does not belong to organization " + organizationID.String(), }) } } } - groupIDs := activeMCPServerConfigACLIDs(req.GroupRoles) + groupIDs := activeACLIDs(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}) + groups, err := db.GetGroups(dbauthz.AsSystemRestricted(ctx), database.GetGroupsParams{GroupIds: groupIDs}) if err != nil { - return append(validations, codersdk.ValidationError{Field: "group_roles", Detail: err.Error()}) + return append(validations, codersdk.ValidationError{Field: groupsField, Detail: err.Error()}) } for _, group := range groups { if group.Group.OrganizationID != organizationID { validations = append(validations, codersdk.ValidationError{ - Field: "group_roles", + Field: groupsField, Detail: "group " + group.Group.ID.String() + " does not belong to organization " + organizationID.String(), }) } @@ -424,10 +426,10 @@ func (api *API) validateMCPServerConfigACLOrganization(ctx context.Context, orga return validations } -func activeMCPServerConfigACLIDs(roles map[string]codersdk.MCPServerConfigRole) []uuid.UUID { +func activeACLIDs[R acl.Role](roles map[string]R) []uuid.UUID { ids := make([]uuid.UUID, 0, len(roles)) for rawID, role := range roles { - if role == codersdk.MCPServerConfigRoleDeleted { + if string(role) == "" { continue } if id, err := uuid.Parse(rawID); err == nil { @@ -437,19 +439,19 @@ func activeMCPServerConfigACLIDs(roles map[string]codersdk.MCPServerConfigRole) return ids } -type MCPServerConfigACLUpdateValidator codersdk.UpdateMCPServerConfigACLRequest +type mcpServerConfigACLUpdateValidator codersdk.UpdateMCPServerConfigACLRequest -var _ acl.UpdateValidator[codersdk.MCPServerConfigRole] = MCPServerConfigACLUpdateValidator{} +var _ acl.UpdateValidator[codersdk.MCPServerConfigRole] = mcpServerConfigACLUpdateValidator{} -func (m MCPServerConfigACLUpdateValidator) Users() (map[string]codersdk.MCPServerConfigRole, string) { +func (m mcpServerConfigACLUpdateValidator) Users() (map[string]codersdk.MCPServerConfigRole, string) { return m.UserRoles, "user_roles" } -func (m MCPServerConfigACLUpdateValidator) Groups() (map[string]codersdk.MCPServerConfigRole, string) { +func (m mcpServerConfigACLUpdateValidator) Groups() (map[string]codersdk.MCPServerConfigRole, string) { return m.GroupRoles, "group_roles" } -func (MCPServerConfigACLUpdateValidator) ValidateRole(role codersdk.MCPServerConfigRole) error { +func (mcpServerConfigACLUpdateValidator) ValidateRole(role codersdk.MCPServerConfigRole) error { if role == codersdk.MCPServerConfigRoleDeleted || role == codersdk.MCPServerConfigRoleRead { return nil } diff --git a/coderd/organizationskills_acl.go b/coderd/organizationskills_acl.go new file mode 100644 index 0000000000..ac78ac5810 --- /dev/null +++ b/coderd/organizationskills_acl.go @@ -0,0 +1,191 @@ +package coderd + +import ( + "errors" + "maps" + "net/http" + + "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/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/codersdk" +) + +// @Summary Get an organization skill ACL +// @ID get-an-organization-skill-acl +// @Security CoderSessionToken +// @Produce json +// @Tags Organizations +// @Param organization path string true "Organization ID or name" +// @Param skillName path string true "Skill name" +// @Success 200 {object} codersdk.OrganizationSkillACL +// @Router /api/experimental/organizations/{organization}/skills/{skillName}/acl [get] +// @x-apidocgen {"skip": true} +func (api *API) getOrganizationSkillACL(rw http.ResponseWriter, r *http.Request) { //nolint:revive // Method name matches route. + ctx := r.Context() + skill := httpmw.OrganizationSkillParam(r) + + users, err := resolveACLUsers(ctx, api.Database, skill.UserACL, func(user codersdk.MinimalUser) codersdk.OrganizationSkillUser { + return codersdk.OrganizationSkillUser{MinimalUser: user, Role: codersdk.OrganizationSkillRoleRead} + }) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + groups, err := resolveACLGroups(ctx, api.Database, skill.GroupACL, func(group codersdk.Group) codersdk.OrganizationSkillGroup { + return codersdk.OrganizationSkillGroup{Group: group, Role: codersdk.OrganizationSkillRoleRead} + }) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + httpapi.Write(ctx, rw, http.StatusOK, codersdk.OrganizationSkillACL{ + Users: users, + Groups: groups, + }) +} + +// @Summary Get available organization skill ACL users and groups +// @ID get-available-organization-skill-acl-users-and-groups +// @Security CoderSessionToken +// @Produce json +// @Tags Organizations +// @Param organization path string true "Organization ID or name" +// @Param skillName path string true "Skill name" +// @Param q query string false "User search query; free-text search also applies to groups" +// @Param after_id query string false "User after ID" format(uuid) +// @Param limit query int false "Page limit for users and groups, if 0 returns all candidates" +// @Param offset query int false "User page offset" +// @Success 200 {object} codersdk.ACLAvailable +// @Router /api/experimental/organizations/{organization}/skills/{skillName}/acl/available [get] +// @x-apidocgen {"skip": true} +func (api *API) getOrganizationSkillACLAvailable(rw http.ResponseWriter, r *http.Request) { //nolint:revive // Method name matches route. + api.writeOrganizationACLAvailable(rw, r, httpmw.OrganizationSkillParam(r).OrganizationID.UUID) +} + +// @Summary Update an organization skill ACL +// @ID update-an-organization-skill-acl +// @Security CoderSessionToken +// @Accept json +// @Tags Organizations +// @Param organization path string true "Organization ID or name" +// @Param skillName path string true "Skill name" +// @Param request body codersdk.UpdateOrganizationSkillACLRequest true "Update organization skill ACL request" +// @Success 204 +// @Router /api/experimental/organizations/{organization}/skills/{skillName}/acl [patch] +// @x-apidocgen {"skip": true} +func (api *API) patchOrganizationSkillACL(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + skill = httpmw.OrganizationSkillParam(r) + organizationID = skill.OrganizationID.UUID + auditor = api.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.AuditableOrganizationSkill](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: organizationID, + }) + ) + defer commitAudit() + aReq.Old = database.AuditableOrganizationSkill{Skill: skill} + + var req codersdk.UpdateOrganizationSkillACLRequest + if !httpapi.Read(ctx, rw, r, &req) { + return + } + + var updated database.Skill + err := api.Database.InTx(func(tx database.Store) error { + // Lock granted users' memberships before the skill row: a member + // removal locks its membership row and then, through its trigger, the + // skill rows, so the reverse order could deadlock. Holding the lock + // keeps a removal from missing the grant written below. + if userIDs := activeACLIDs(req.UserRoles); len(userIDs) > 0 { + //nolint:gocritic // Validation below reports users that are not members. + _, err := tx.LockOrganizationMembersByUserIDsForShare(dbauthz.AsSystemRestricted(ctx), database.LockOrganizationMembersByUserIDsForShareParams{ + OrganizationID: organizationID, + UserIds: userIDs, + }) + if err != nil { + return xerrors.Errorf("lock organization members: %w", err) + } + } + //nolint:gocritic // The ACL write below reauthorizes the locked row for share. + current, err := tx.GetOrganizationSkillByIDForUpdate(dbauthz.AsSystemRestricted(ctx), skill.ID) + if err != nil { + return xerrors.Errorf("get organization skill for update: %w", err) + } + userRoles, groupRoles, validations := validateOrganizationACLUpdate(ctx, tx, organizationID, organizationSkillACLUpdateValidator(req)) + if len(validations) > 0 { + return &organizationSkillACLValidationError{validations: validations} + } + aReq.Old = database.AuditableOrganizationSkill{Skill: current} + userACL := maps.Clone(current.UserACL) + groupACL := maps.Clone(current.GroupACL) + applyACLReadRoles(userACL, userRoles) + applyACLReadRoles(groupACL, groupRoles) + updated, err = tx.UpdateOrganizationSkillACLByID(ctx, database.UpdateOrganizationSkillACLByIDParams{ + ID: skill.ID, + UserACL: userACL, + GroupACL: groupACL, + }) + if err != nil { + return xerrors.Errorf("update organization skill ACL: %w", err) + } + return nil + }, nil) + if err != nil { + var validationErr *organizationSkillACLValidationError + if errors.As(err, &validationErr) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid request to update organization skill ACL.", + Validations: validationErr.validations, + }) + return + } + // A delete between the middleware read and the row lock 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 = database.AuditableOrganizationSkill{Skill: updated} + rw.WriteHeader(http.StatusNoContent) +} + +type organizationSkillACLValidationError struct { + validations []codersdk.ValidationError +} + +func (*organizationSkillACLValidationError) Error() string { + return "invalid organization skill ACL" +} + +type organizationSkillACLUpdateValidator codersdk.UpdateOrganizationSkillACLRequest + +var _ acl.UpdateValidator[codersdk.OrganizationSkillRole] = organizationSkillACLUpdateValidator{} + +func (v organizationSkillACLUpdateValidator) Users() (map[string]codersdk.OrganizationSkillRole, string) { + return v.UserRoles, "user_roles" +} + +func (v organizationSkillACLUpdateValidator) Groups() (map[string]codersdk.OrganizationSkillRole, string) { + return v.GroupRoles, "group_roles" +} + +func (organizationSkillACLUpdateValidator) ValidateRole(role codersdk.OrganizationSkillRole) error { + if role == codersdk.OrganizationSkillRoleDeleted || role == codersdk.OrganizationSkillRoleRead { + return nil + } + return xerrors.Errorf("role %q is not a valid organization skill role", role) +} diff --git a/coderd/organizationskills_acl_test.go b/coderd/organizationskills_acl_test.go new file mode 100644 index 0000000000..bcba8fcf31 --- /dev/null +++ b/coderd/organizationskills_acl_test.go @@ -0,0 +1,356 @@ +package coderd_test + +import ( + "context" + "encoding/json" + "net/http" + "sync/atomic" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/require" + + "github.com/coder/coder/v2/coderd/audit" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/dbgen" + "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/rbac" + "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" +) + +func TestOrganizationSkillACL(t *testing.T) { + t.Parallel() + + mAudit := audit.NewMockWithDiffFn(func(old, newVal any) audit.Map { + oldSkill, oldOK := old.(database.AuditableOrganizationSkill) + newSkill, newOK := newVal.(database.AuditableOrganizationSkill) + if !oldOK || !newOK { + return audit.Map{} + } + return audit.Map{ + "group_acl": {Old: oldSkill.GroupACL, New: newSkill.GroupACL}, + "user_acl": {Old: oldSkill.UserACL, New: newSkill.UserACL}, + } + }) + db, ps := dbtestutil.NewDB(t) + ownerRawClient := coderdtest.New(t, &coderdtest.Options{Database: db, Pubsub: ps, Auditor: mAudit}) + firstUser := coderdtest.CreateFirstUser(t, ownerRawClient) + orgID := firstUser.OrganizationID + owner := codersdk.NewExperimentalClient(ownerRawClient) + orgAdminRawClient, _ := coderdtest.CreateAnotherUser(t, ownerRawClient, orgID, rbac.ScopedRoleOrgAdmin(orgID)) + memberRawClient, member := coderdtest.CreateAnotherUser(t, ownerRawClient, orgID) + memberClient := codersdk.NewExperimentalClient(memberRawClient) + + otherOrg := dbgen.Organization(t, db, database.Organization{}) + foreignUser := dbgen.User(t, db, database.User{}) + dbgen.OrganizationMember(t, db, database.OrganizationMember{OrganizationID: otherOrg.ID, UserID: foreignUser.ID}) + + readACL := func(ids ...uuid.UUID) database.ChatACL { + entries := database.ChatACL{} + for _, id := range ids { + entries[id.String()] = database.ChatACLEntry{Permissions: []policy.Action{policy.ActionRead}} + } + return entries + } + createSkill := func(ctx context.Context, t *testing.T, name string) codersdk.Skill { + t.Helper() + skill, err := owner.CreateOrganizationSkill(ctx, orgID, codersdk.CreateSkillRequest{ + Content: userSkillMarkdown(name, "ACL", "Body."), + }) + require.NoError(t, err) + return skill + } + + // The site owner shares in every other subtest. Listing candidates is + // shared with MCP server configs and covered by + // TestMCPServerConfigACLAvailable. + t.Run("OrgAdminManagesACL", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + client := codersdk.NewExperimentalClient(orgAdminRawClient) + skill := createSkill(ctx, t, "org-admin-sharer") + + acl, err := client.OrganizationSkillACL(ctx, orgID, skill.Name) + require.NoError(t, err) + require.Empty(t, acl.Users) + require.Len(t, acl.Groups, 1) + require.Equal(t, orgID, acl.Groups[0].ID) + require.Equal(t, codersdk.OrganizationSkillRoleRead, acl.Groups[0].Role) + + available, err := client.OrganizationSkillACLAvailable(ctx, orgID, skill.Name, codersdk.UsersRequest{}) + require.NoError(t, err) + require.Contains(t, aclAvailableUserIDs(available), member.ID) + require.NotContains(t, aclAvailableUserIDs(available), foreignUser.ID) + + err = client.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, codersdk.UpdateOrganizationSkillACLRequest{ + UserRoles: map[string]codersdk.OrganizationSkillRole{member.ID.String(): codersdk.OrganizationSkillRoleRead}, + }) + require.NoError(t, err) + acl, err = client.OrganizationSkillACL(ctx, orgID, skill.Name) + require.NoError(t, err) + require.Len(t, acl.Users, 1) + require.Equal(t, member.ID, acl.Users[0].ID) + require.Equal(t, codersdk.OrganizationSkillRoleRead, acl.Users[0].Role) + }) + + t.Run("ReadersGetNotFound", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + skill := createSkill(ctx, t, "non-sharer-skill") + _, err := memberClient.OrganizationSkillByName(ctx, orgID, skill.Name) + require.NoError(t, err) + + _, err = memberClient.OrganizationSkillACL(ctx, orgID, skill.Name) + requireSDKErrorStatus(t, err, http.StatusNotFound) + err = memberClient.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, codersdk.UpdateOrganizationSkillACLRequest{ + UserRoles: map[string]codersdk.OrganizationSkillRole{member.ID.String(): codersdk.OrganizationSkillRoleRead}, + }) + requireSDKErrorStatus(t, err, http.StatusNotFound) + _, err = memberClient.OrganizationSkillACLAvailable(ctx, orgID, skill.Name, codersdk.UsersRequest{}) + requireSDKErrorStatus(t, err, http.StatusNotFound) + }) + + t.Run("SparseUpdateChangesReaders", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + group := dbgen.Group(t, db, database.Group{OrganizationID: orgID}) + groupMemberRawClient, groupMember := coderdtest.CreateAnotherUser(t, ownerRawClient, orgID) + dbgen.GroupMember(t, db, database.GroupMemberTable{GroupID: group.ID, UserID: groupMember.ID}) + addedRawClient, added := coderdtest.CreateAnotherUser(t, ownerRawClient, orgID) + skill := createSkill(ctx, t, "sparse-acl-skill") + requireOrganizationSkillListed(ctx, t, memberClient, orgID, skill.Name) + + err := owner.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, codersdk.UpdateOrganizationSkillACLRequest{ + UserRoles: map[string]codersdk.OrganizationSkillRole{added.ID.String(): codersdk.OrganizationSkillRoleRead}, + GroupRoles: map[string]codersdk.OrganizationSkillRole{ + group.ID.String(): codersdk.OrganizationSkillRoleRead, + orgID.String(): codersdk.OrganizationSkillRoleDeleted, + }, + }) + require.NoError(t, err) + + requireOrganizationSkillNotListed(ctx, t, memberClient, orgID, skill.Name) + _, err = memberClient.OrganizationSkillByName(ctx, orgID, skill.Name) + requireSDKErrorStatus(t, err, http.StatusNotFound) + for _, rawClient := range []*codersdk.Client{groupMemberRawClient, addedRawClient} { + client := codersdk.NewExperimentalClient(rawClient) + requireOrganizationSkillListed(ctx, t, client, orgID, skill.Name) + _, err = client.OrganizationSkillByName(ctx, orgID, skill.Name) + require.NoError(t, err) + } + + var log database.AuditLog + for _, entry := range mAudit.AuditLogs() { + if entry.ResourceID == skill.ID && entry.Action == database.AuditActionWrite { + log = entry + } + } + require.Equal(t, database.ResourceTypeOrganizationSkill, log.ResourceType) + require.Equal(t, orgID, log.OrganizationID) + var diff map[string]struct { + Old database.ChatACL `json:"old"` + New database.ChatACL `json:"new"` + } + require.NoError(t, json.Unmarshal(log.Diff, &diff)) + require.Equal(t, readACL(orgID), diff["group_acl"].Old) + require.Equal(t, readACL(group.ID), diff["group_acl"].New) + require.Equal(t, database.ChatACL{}, diff["user_acl"].Old) + require.Equal(t, readACL(added.ID), diff["user_acl"].New) + + // Removing principals that are no longer granted, or no longer exist, + // leaves the ACL unchanged. + err = owner.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, codersdk.UpdateOrganizationSkillACLRequest{ + UserRoles: map[string]codersdk.OrganizationSkillRole{uuid.NewString(): codersdk.OrganizationSkillRoleDeleted}, + GroupRoles: map[string]codersdk.OrganizationSkillRole{orgID.String(): codersdk.OrganizationSkillRoleDeleted}, + }) + require.NoError(t, err) + acl, err := owner.OrganizationSkillACL(ctx, orgID, skill.Name) + require.NoError(t, err) + require.Len(t, acl.Users, 1) + require.Equal(t, added.ID, acl.Users[0].ID) + require.Len(t, acl.Groups, 1) + require.Equal(t, group.ID, acl.Groups[0].ID) + require.Equal(t, 1, acl.Groups[0].TotalMemberCount) + }) + + // Membership, duplicate-spelling and cross-organization validation is + // shared with MCP server configs and covered by TestMCPServerConfigACL. + t.Run("RejectsUnsupportedRole", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + skill := createSkill(ctx, t, "invalid-acl-skill") + err := owner.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, codersdk.UpdateOrganizationSkillACLRequest{ + UserRoles: map[string]codersdk.OrganizationSkillRole{member.ID.String(): "use"}, + }) + sdkErr := requireSDKErrorStatus(t, err, http.StatusBadRequest) + require.Contains(t, sdkErr.Error(), `role "use" is not a valid organization skill role`) + + acl, err := owner.OrganizationSkillACL(ctx, orgID, skill.Name) + require.NoError(t, err) + require.Empty(t, acl.Users) + require.Len(t, acl.Groups, 1) + require.Equal(t, orgID, acl.Groups[0].ID) + }) + + t.Run("ScopedAPIKeys", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + skill := createSkill(ctx, t, "scoped-acl-skill") + scopedClient := func(scopes ...database.APIKeyScope) *codersdk.ExperimentalClient { + _, token := dbgen.APIKey(t, db, database.APIKey{UserID: firstUser.UserID, Scopes: scopes}) + rawClient := codersdk.New(ownerRawClient.URL) + rawClient.SetSessionToken(token) + return codersdk.NewExperimentalClient(rawClient) + } + req := codersdk.UpdateOrganizationSkillACLRequest{ + UserRoles: map[string]codersdk.OrganizationSkillRole{member.ID.String(): codersdk.OrganizationSkillRoleRead}, + } + + reader := scopedClient("organization:read", "organization_skill:read") + _, err := reader.OrganizationSkillByName(ctx, orgID, skill.Name) + require.NoError(t, err) + err = reader.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, req) + requireSDKErrorStatus(t, err, http.StatusNotFound) + + sharer := scopedClient("organization:read", "organization_skill:share") + require.NoError(t, sharer.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, req)) + }) +} + +// organizationSkillDeleteRaceStore deletes the skill right after the param +// middleware read once armed, so the handler's locked re-fetch sees a +// concurrently deleted row. +type organizationSkillDeleteRaceStore struct { + database.Store + + armed atomic.Bool +} + +func (s *organizationSkillDeleteRaceStore) GetOrganizationSkillByOrganizationIDAndName(ctx context.Context, arg database.GetOrganizationSkillByOrganizationIDAndNameParams) (database.Skill, error) { + skill, err := s.Store.GetOrganizationSkillByOrganizationIDAndName(ctx, arg) + if err == nil && s.armed.CompareAndSwap(true, false) { + if _, err := s.DeleteOrganizationSkillByOrganizationIDAndName(ctx, database.DeleteOrganizationSkillByOrganizationIDAndNameParams(arg)); err != nil { + return database.Skill{}, err + } + } + return skill, err +} + +func TestOrganizationSkillACLConcurrentDelete(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + db, ps := dbtestutil.NewDB(t) + store := &organizationSkillDeleteRaceStore{Store: db} + ownerRawClient := coderdtest.New(t, &coderdtest.Options{Database: store, Pubsub: ps}) + firstUser := coderdtest.CreateFirstUser(t, ownerRawClient) + client := codersdk.NewExperimentalClient(ownerRawClient) + skill, err := client.CreateOrganizationSkill(ctx, firstUser.OrganizationID, codersdk.CreateSkillRequest{ + Content: userSkillMarkdown("acl-delete-race", "ACL", "Body."), + }) + require.NoError(t, err) + + store.armed.Store(true) + err = client.UpdateOrganizationSkillACL(ctx, firstUser.OrganizationID, skill.Name, codersdk.UpdateOrganizationSkillACLRequest{}) + requireSDKErrorStatus(t, err, http.StatusNotFound) +} + +func TestOrganizationSkillACLMemberRemoval(t *testing.T) { + t.Parallel() + + db, ps, sqlDB := dbtestutil.NewDBWithSQLDB(t) + ownerRawClient := coderdtest.New(t, &coderdtest.Options{Database: db, Pubsub: ps}) + firstUser := coderdtest.CreateFirstUser(t, ownerRawClient) + orgID := firstUser.OrganizationID + owner := codersdk.NewExperimentalClient(ownerRawClient) + + // Each case starts from a skill without the Everyone grant so a rejoined + // member can only read it through a leftover direct grant. + setup := func(ctx context.Context, t *testing.T, name string) (codersdk.Skill, *codersdk.ExperimentalClient, uuid.UUID) { + t.Helper() + skill, err := owner.CreateOrganizationSkill(ctx, orgID, codersdk.CreateSkillRequest{ + Content: userSkillMarkdown(name, "ACL", "Body."), + }) + require.NoError(t, err) + require.NoError(t, owner.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, codersdk.UpdateOrganizationSkillACLRequest{ + GroupRoles: map[string]codersdk.OrganizationSkillRole{orgID.String(): codersdk.OrganizationSkillRoleDeleted}, + })) + rawClient, user := coderdtest.CreateAnotherUser(t, ownerRawClient, orgID) + return skill, codersdk.NewExperimentalClient(rawClient), user.ID + } + requireRejoinedMemberCannotRead := func(ctx context.Context, t *testing.T, skill codersdk.Skill, client *codersdk.ExperimentalClient, userID uuid.UUID) { + t.Helper() + var granted bool + require.NoError(t, sqlDB.QueryRowContext(ctx, `SELECT user_acl ? $2 FROM skills WHERE id = $1`, skill.ID, userID.String()).Scan(&granted)) + require.False(t, granted) + dbgen.OrganizationMember(t, db, database.OrganizationMember{OrganizationID: orgID, UserID: userID}) + _, err := client.OrganizationSkillByName(ctx, orgID, skill.Name) + requireSDKErrorStatus(t, err, http.StatusNotFound) + } + + t.Run("RemovalBeforeGrant", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + skill, client, userID := setup(ctx, t, "removal-before-grant") + + tx, err := sqlDB.BeginTx(ctx, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = tx.Rollback() }) + _, err = tx.ExecContext(ctx, `DELETE FROM organization_members WHERE organization_id = $1 AND user_id = $2`, orgID, userID) + require.NoError(t, err) + + patchErr := make(chan error, 1) + go func() { + patchErr <- owner.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, codersdk.UpdateOrganizationSkillACLRequest{ + UserRoles: map[string]codersdk.OrganizationSkillRole{userID.String(): codersdk.OrganizationSkillRoleRead}, + }) + }() + testutil.Eventually(ctx, t, func(ctx context.Context) bool { + if len(patchErr) > 0 { + return true + } + var waits int + err := sqlDB.QueryRowContext(ctx, `SELECT COUNT(*) FROM pg_stat_activity +WHERE datname = current_database() AND pid <> pg_backend_pid() AND wait_event_type = 'Lock'`).Scan(&waits) + return err == nil && waits > 0 + }, testutil.IntervalFast, "PATCH returns or waits for the membership lock") + require.NoError(t, tx.Commit()) + + err = testutil.RequireReceive(ctx, t, patchErr) + sdkErr := requireSDKErrorStatus(t, err, http.StatusBadRequest) + require.Contains(t, sdkErr.Error(), "user "+userID.String()+" does not belong to organization") + requireRejoinedMemberCannotRead(ctx, t, skill, client, userID) + }) + + t.Run("RemovalAfterGrant", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + skill, client, userID := setup(ctx, t, "removal-after-grant") + require.NoError(t, owner.UpdateOrganizationSkillACL(ctx, orgID, skill.Name, codersdk.UpdateOrganizationSkillACLRequest{ + UserRoles: map[string]codersdk.OrganizationSkillRole{userID.String(): codersdk.OrganizationSkillRoleRead}, + })) + _, err := client.OrganizationSkillByName(ctx, orgID, skill.Name) + require.NoError(t, err) + + require.NoError(t, ownerRawClient.DeleteOrganizationMember(ctx, orgID, userID.String())) + requireRejoinedMemberCannotRead(ctx, t, skill, client, userID) + }) +} + +func aclAvailableUserIDs(available codersdk.ACLAvailable) []uuid.UUID { + ids := make([]uuid.UUID, 0, len(available.Users)) + for _, user := range available.Users { + ids = append(ids, user.ID) + } + return ids +} diff --git a/coderd/rbac/acl/updatevalidator.go b/coderd/rbac/acl/updatevalidator.go index 10557b6741..63352036e0 100644 --- a/coderd/rbac/acl/updatevalidator.go +++ b/coderd/rbac/acl/updatevalidator.go @@ -11,24 +11,29 @@ import ( "github.com/coder/coder/v2/codersdk" ) -type UpdateValidator[Role codersdk.WorkspaceRole | codersdk.TemplateRole | codersdk.ChatRole | codersdk.MCPServerConfigRole] interface { +// Role is the set of ACL role types Validate accepts. +type Role interface { + codersdk.WorkspaceRole | codersdk.TemplateRole | codersdk.ChatRole | codersdk.MCPServerConfigRole | codersdk.OrganizationSkillRole +} + +type UpdateValidator[R Role] 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. - Users() (map[string]Role, string) + Users() (map[string]R, string) // Groups should return a map from group 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. - Groups() (map[string]Role, string) + Groups() (map[string]R, string) // ValidateRole should return an error that will be used in the // ValidationError if the role is invalid for the corresponding resource type. - ValidateRole(role Role) error + ValidateRole(role R) error } -func Validate[Role codersdk.WorkspaceRole | codersdk.TemplateRole | codersdk.ChatRole | codersdk.MCPServerConfigRole]( +func Validate[R Role]( ctx context.Context, db database.Store, - v UpdateValidator[Role], + v UpdateValidator[R], ) []codersdk.ValidationError { // nolint:gocritic // Validate requires full read access to users and groups ctx = dbauthz.AsSystemRestricted(ctx) diff --git a/codersdk/skills.go b/codersdk/skills.go index f78fd5bbe7..11fbe65a85 100644 --- a/codersdk/skills.go +++ b/codersdk/skills.go @@ -46,6 +46,44 @@ type UpdateSkillRequest struct { Enabled *bool `json:"enabled,omitempty"` } +// OrganizationSkillRole is a role a user or group holds in an organization +// skill's access control list. +type OrganizationSkillRole string + +const ( + OrganizationSkillRoleRead OrganizationSkillRole = "read" + // OrganizationSkillRoleDeleted removes the principal's ACL entry when used + // in an update request. + OrganizationSkillRoleDeleted OrganizationSkillRole = "" +) + +// OrganizationSkillACL is the resolved access control list of an +// organization skill. +type OrganizationSkillACL struct { + Users []OrganizationSkillUser `json:"users"` + Groups []OrganizationSkillGroup `json:"groups"` +} + +// OrganizationSkillUser is a user entry in an organization skill ACL. +type OrganizationSkillUser struct { + MinimalUser + Role OrganizationSkillRole `json:"role" enums:"read"` +} + +// OrganizationSkillGroup is a group entry in an organization skill ACL. +type OrganizationSkillGroup struct { + Group + Role OrganizationSkillRole `json:"role" enums:"read"` +} + +// UpdateOrganizationSkillACLRequest is a sparse update of an organization +// skill ACL: only the listed principals change, and +// OrganizationSkillRoleDeleted removes an entry. +type UpdateOrganizationSkillACLRequest struct { + UserRoles map[string]OrganizationSkillRole `json:"user_roles,omitempty"` + GroupRoles map[string]OrganizationSkillRole `json:"group_roles,omitempty"` +} + func userSkillsPath(user string) string { return fmt.Sprintf("/api/experimental/users/%s/skills", url.PathEscape(user)) } @@ -202,3 +240,51 @@ func (c *ExperimentalClient) DeleteOrganizationSkill(ctx context.Context, organi } return nil } + +// OrganizationSkillACL returns the resolved ACL of an organization skill. +func (c *ExperimentalClient) OrganizationSkillACL(ctx context.Context, organizationID uuid.UUID, name string) (OrganizationSkillACL, error) { + res, err := c.Request(ctx, http.MethodGet, organizationSkillPath(organizationID, name)+"/acl", nil) + if err != nil { + return OrganizationSkillACL{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return OrganizationSkillACL{}, ReadBodyAsError(res) + } + var acl OrganizationSkillACL + return acl, ReadBodyAsJSON(res, &acl) +} + +// UpdateOrganizationSkillACL applies a sparse ACL update to an organization +// skill. +func (c *ExperimentalClient) UpdateOrganizationSkillACL(ctx context.Context, organizationID uuid.UUID, name string, req UpdateOrganizationSkillACLRequest) error { + res, err := c.Request(ctx, http.MethodPatch, organizationSkillPath(organizationID, name)+"/acl", req) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} + +// OrganizationSkillACLAvailable returns the organization members and groups +// that can be added to an organization skill ACL. +func (c *ExperimentalClient) OrganizationSkillACLAvailable(ctx context.Context, organizationID uuid.UUID, name string, req UsersRequest) (ACLAvailable, error) { + res, err := c.Request(ctx, http.MethodGet, + organizationSkillPath(organizationID, name)+"/acl/available", + nil, + req.Pagination.asRequestOption(), + req.asRequestOption(), + ) + if err != nil { + return ACLAvailable{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return ACLAvailable{}, ReadBodyAsError(res) + } + var available ACLAvailable + return available, ReadBodyAsJSON(res, &available) +} diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index 269a22055e..9d635d6d0e 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -12629,6 +12629,159 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith |----------|-----------------------| | `status` | `active`, `suspended` | +## codersdk.OrganizationSkillACL + +```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.OrganizationSkillGroup](#codersdkorganizationskillgroup) | false | | | +| `users` | array of [codersdk.OrganizationSkillUser](#codersdkorganizationskilluser) | false | | | + +## codersdk.OrganizationSkillGroup + +```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.OrganizationSkillRole](#codersdkorganizationskillrole) | 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.OrganizationSkillRole + +```json +"read" +``` + +### Properties + +#### Enumerated Values + +| Value(s) | +|------------| +| ``, `read` | + +## codersdk.OrganizationSkillUser + +```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.OrganizationSkillRole](#codersdkorganizationskillrole) | false | | | +| `username` | string | true | | | + +#### Enumerated Values + +| Property | Value(s) | +|----------|----------| +| `role` | `read` | + ## codersdk.OrganizationSyncSettings ```json @@ -16893,6 +17046,30 @@ The cursor is owner-scoped, so only the chat owner may set this. Opening a chat' | `icon` | string | false | | | | `name` | string | false | | | +## codersdk.UpdateOrganizationSkillACLRequest + +```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.OrganizationSkillRole](#codersdkorganizationskillrole) | false | | | +| `user_roles` | object | false | | | +| » `[any property]` | [codersdk.OrganizationSkillRole](#codersdkorganizationskillrole) | false | | | + ## codersdk.UpdateRoles ```json diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 4af8c3aa4b..3679d774ac 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -7945,6 +7945,37 @@ export interface OrganizationProvisionerJobsOptions { readonly Initiator: string; } +// From codersdk/skills.go +/** + * OrganizationSkillACL is the resolved access control list of an + * organization skill. + */ +export interface OrganizationSkillACL { + readonly users: readonly OrganizationSkillUser[]; + readonly groups: readonly OrganizationSkillGroup[]; +} + +// From codersdk/skills.go +/** + * OrganizationSkillGroup is a group entry in an organization skill ACL. + */ +export interface OrganizationSkillGroup extends Group { + readonly role: OrganizationSkillRole; +} + +// From codersdk/skills.go +export type OrganizationSkillRole = "" | "read"; + +export const OrganizationSkillRoles: OrganizationSkillRole[] = ["", "read"]; + +// From codersdk/skills.go +/** + * OrganizationSkillUser is a user entry in an organization skill ACL. + */ +export interface OrganizationSkillUser extends MinimalUser { + readonly role: OrganizationSkillRole; +} + // From codersdk/idpsync.go export interface OrganizationSyncSettings { /** @@ -10868,6 +10899,17 @@ export interface UpdateOrganizationRequest { readonly default_org_member_roles?: string[]; } +// From codersdk/skills.go +/** + * UpdateOrganizationSkillACLRequest is a sparse update of an organization + * skill ACL: only the listed principals change, and + * OrganizationSkillRoleDeleted removes an entry. + */ +export interface UpdateOrganizationSkillACLRequest { + readonly user_roles?: Record; + readonly group_roles?: Record; +} + // From codersdk/users.go export interface UpdateRoles { readonly roles: readonly string[];