diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index d5b5b781fc..fd03b85062 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -975,6 +975,227 @@ const docTemplate = `{ } } }, + "/api/experimental/organizations/{organization}/skills": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "Organizations" + ], + "summary": "List organization skills", + "operationId": "list-organization-skills", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.SkillMetadata" + } + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "post": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Organizations" + ], + "summary": "Create an organization skill", + "operationId": "create-an-organization-skill", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + }, + { + "description": "Create organization skill request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.CreateSkillRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/codersdk.Skill" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, + "/api/experimental/organizations/{organization}/skills/{skillName}": { + "get": { + "produces": [ + "application/json" + ], + "tags": [ + "Organizations" + ], + "summary": "Get an organization skill by name", + "operationId": "get-an-organization-skill-by-name", + "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.Skill" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "delete": { + "tags": [ + "Organizations" + ], + "summary": "Delete an organization skill", + "operationId": "delete-an-organization-skill", + "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": { + "204": { + "description": "No Content" + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "patch": { + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Organizations" + ], + "summary": "Update an organization skill", + "operationId": "update-an-organization-skill", + "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 request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.UpdateSkillRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.Skill" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, "/api/experimental/users/email": { "put": { "consumes": [ @@ -1036,7 +1257,7 @@ const docTemplate = `{ "schema": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.UserSkillMetadata" + "$ref": "#/definitions/codersdk.SkillMetadata" } } } @@ -1076,7 +1297,7 @@ const docTemplate = `{ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/codersdk.CreateUserSkillRequest" + "$ref": "#/definitions/codersdk.CreateSkillRequest" } } ], @@ -1084,7 +1305,7 @@ const docTemplate = `{ "201": { "description": "Created", "schema": { - "$ref": "#/definitions/codersdk.UserSkill" + "$ref": "#/definitions/codersdk.Skill" } } }, @@ -1128,7 +1349,7 @@ const docTemplate = `{ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.UserSkill" + "$ref": "#/definitions/codersdk.Skill" } } }, @@ -1210,7 +1431,7 @@ const docTemplate = `{ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/codersdk.UpdateUserSkillRequest" + "$ref": "#/definitions/codersdk.UpdateSkillRequest" } } ], @@ -1218,7 +1439,7 @@ const docTemplate = `{ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.UserSkill" + "$ref": "#/definitions/codersdk.Skill" } } }, @@ -23476,6 +23697,15 @@ const docTemplate = `{ } } }, + "codersdk.CreateSkillRequest": { + "type": "object", + "properties": { + "content": { + "description": "Content must be SKILL.md-format Markdown with YAML frontmatter. The\nfrontmatter must include name, may include description, and must be\nfollowed by a non-empty body.", + "type": "string" + } + } + }, "codersdk.CreateTemplateRequest": { "type": "object", "required": [ @@ -23929,15 +24159,6 @@ const docTemplate = `{ } } }, - "codersdk.CreateUserSkillRequest": { - "type": "object", - "properties": { - "content": { - "description": "Content must be SKILL.md-format Markdown with YAML frontmatter. The\nfrontmatter must include name, may include description, and must be\nfollowed by a non-empty body.", - "type": "string" - } - } - }, "codersdk.CreateWorkspaceBuildOnSuccessRequest": { "type": "object", "required": [ @@ -29700,6 +29921,61 @@ const docTemplate = `{ "SharedWorkspaceActorTypeUser" ] }, + "codersdk.Skill": { + "type": "object", + "properties": { + "content": { + "type": "string" + }, + "created_at": { + "type": "string", + "format": "date-time" + }, + "description": { + "type": "string" + }, + "enabled": { + "type": "boolean" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + }, + "updated_at": { + "type": "string", + "format": "date-time" + } + } + }, + "codersdk.SkillMetadata": { + "type": "object", + "properties": { + "created_at": { + "type": "string", + "format": "date-time" + }, + "description": { + "type": "string" + }, + "enabled": { + "type": "boolean" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + }, + "updated_at": { + "type": "string", + "format": "date-time" + } + } + }, "codersdk.SlimRole": { "type": "object", "properties": { @@ -31510,6 +31786,18 @@ const docTemplate = `{ } } }, + "codersdk.UpdateSkillRequest": { + "type": "object", + "properties": { + "content": { + "description": "Content must be SKILL.md-format Markdown with YAML frontmatter. The\nfrontmatter must include name, may include description, and must be\nfollowed by a non-empty body.", + "type": "string" + }, + "enabled": { + "type": "boolean" + } + } + }, "codersdk.UpdateTemplateACL": { "type": "object", "properties": { @@ -31832,15 +32120,6 @@ const docTemplate = `{ } } }, - "codersdk.UpdateUserSkillRequest": { - "type": "object", - "properties": { - "content": { - "description": "Content must be SKILL.md-format Markdown with YAML frontmatter. The\nfrontmatter must include name, may include description, and must be\nfollowed by a non-empty body.", - "type": "string" - } - } - }, "codersdk.UpdateWorkspaceACL": { "type": "object", "properties": { @@ -32552,55 +32831,6 @@ const docTemplate = `{ } } }, - "codersdk.UserSkill": { - "type": "object", - "properties": { - "content": { - "type": "string" - }, - "created_at": { - "type": "string", - "format": "date-time" - }, - "description": { - "type": "string" - }, - "id": { - "type": "string", - "format": "uuid" - }, - "name": { - "type": "string" - }, - "updated_at": { - "type": "string", - "format": "date-time" - } - } - }, - "codersdk.UserSkillMetadata": { - "type": "object", - "properties": { - "created_at": { - "type": "string", - "format": "date-time" - }, - "description": { - "type": "string" - }, - "id": { - "type": "string", - "format": "uuid" - }, - "name": { - "type": "string" - }, - "updated_at": { - "type": "string", - "format": "date-time" - } - } - }, "codersdk.UserStatus": { "type": "string", "enum": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index 09d2abce42..a4e5661749 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -876,6 +876,205 @@ } } }, + "/api/experimental/organizations/{organization}/skills": { + "get": { + "produces": ["application/json"], + "tags": ["Organizations"], + "summary": "List organization skills", + "operationId": "list-organization-skills", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.SkillMetadata" + } + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "post": { + "consumes": ["application/json"], + "produces": ["application/json"], + "tags": ["Organizations"], + "summary": "Create an organization skill", + "operationId": "create-an-organization-skill", + "parameters": [ + { + "type": "string", + "description": "Organization ID or name", + "name": "organization", + "in": "path", + "required": true + }, + { + "description": "Create organization skill request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.CreateSkillRequest" + } + } + ], + "responses": { + "201": { + "description": "Created", + "schema": { + "$ref": "#/definitions/codersdk.Skill" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, + "/api/experimental/organizations/{organization}/skills/{skillName}": { + "get": { + "produces": ["application/json"], + "tags": ["Organizations"], + "summary": "Get an organization skill by name", + "operationId": "get-an-organization-skill-by-name", + "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.Skill" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "delete": { + "tags": ["Organizations"], + "summary": "Delete an organization skill", + "operationId": "delete-an-organization-skill", + "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": { + "204": { + "description": "No Content" + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + }, + "patch": { + "consumes": ["application/json"], + "produces": ["application/json"], + "tags": ["Organizations"], + "summary": "Update an organization skill", + "operationId": "update-an-organization-skill", + "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 request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.UpdateSkillRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "$ref": "#/definitions/codersdk.Skill" + } + } + }, + "security": [ + { + "CoderSessionToken": [] + } + ], + "x-apidocgen": { + "skip": true + } + } + }, "/api/experimental/users/email": { "put": { "consumes": ["application/json"], @@ -929,7 +1128,7 @@ "schema": { "type": "array", "items": { - "$ref": "#/definitions/codersdk.UserSkillMetadata" + "$ref": "#/definitions/codersdk.SkillMetadata" } } } @@ -963,7 +1162,7 @@ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/codersdk.CreateUserSkillRequest" + "$ref": "#/definitions/codersdk.CreateSkillRequest" } } ], @@ -971,7 +1170,7 @@ "201": { "description": "Created", "schema": { - "$ref": "#/definitions/codersdk.UserSkill" + "$ref": "#/definitions/codersdk.Skill" } } }, @@ -1011,7 +1210,7 @@ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.UserSkill" + "$ref": "#/definitions/codersdk.Skill" } } }, @@ -1085,7 +1284,7 @@ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/codersdk.UpdateUserSkillRequest" + "$ref": "#/definitions/codersdk.UpdateSkillRequest" } } ], @@ -1093,7 +1292,7 @@ "200": { "description": "OK", "schema": { - "$ref": "#/definitions/codersdk.UserSkill" + "$ref": "#/definitions/codersdk.Skill" } } }, @@ -21323,6 +21522,15 @@ } } }, + "codersdk.CreateSkillRequest": { + "type": "object", + "properties": { + "content": { + "description": "Content must be SKILL.md-format Markdown with YAML frontmatter. The\nfrontmatter must include name, may include description, and must be\nfollowed by a non-empty body.", + "type": "string" + } + } + }, "codersdk.CreateTemplateRequest": { "type": "object", "required": ["name", "template_version_id"], @@ -21753,15 +21961,6 @@ } } }, - "codersdk.CreateUserSkillRequest": { - "type": "object", - "properties": { - "content": { - "description": "Content must be SKILL.md-format Markdown with YAML frontmatter. The\nfrontmatter must include name, may include description, and must be\nfollowed by a non-empty body.", - "type": "string" - } - } - }, "codersdk.CreateWorkspaceBuildOnSuccessRequest": { "type": "object", "required": ["transition"], @@ -27314,6 +27513,61 @@ "SharedWorkspaceActorTypeUser" ] }, + "codersdk.Skill": { + "type": "object", + "properties": { + "content": { + "type": "string" + }, + "created_at": { + "type": "string", + "format": "date-time" + }, + "description": { + "type": "string" + }, + "enabled": { + "type": "boolean" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + }, + "updated_at": { + "type": "string", + "format": "date-time" + } + } + }, + "codersdk.SkillMetadata": { + "type": "object", + "properties": { + "created_at": { + "type": "string", + "format": "date-time" + }, + "description": { + "type": "string" + }, + "enabled": { + "type": "boolean" + }, + "id": { + "type": "string", + "format": "uuid" + }, + "name": { + "type": "string" + }, + "updated_at": { + "type": "string", + "format": "date-time" + } + } + }, "codersdk.SlimRole": { "type": "object", "properties": { @@ -29036,6 +29290,18 @@ } } }, + "codersdk.UpdateSkillRequest": { + "type": "object", + "properties": { + "content": { + "description": "Content must be SKILL.md-format Markdown with YAML frontmatter. The\nfrontmatter must include name, may include description, and must be\nfollowed by a non-empty body.", + "type": "string" + }, + "enabled": { + "type": "boolean" + } + } + }, "codersdk.UpdateTemplateACL": { "type": "object", "properties": { @@ -29343,15 +29609,6 @@ } } }, - "codersdk.UpdateUserSkillRequest": { - "type": "object", - "properties": { - "content": { - "description": "Content must be SKILL.md-format Markdown with YAML frontmatter. The\nfrontmatter must include name, may include description, and must be\nfollowed by a non-empty body.", - "type": "string" - } - } - }, "codersdk.UpdateWorkspaceACL": { "type": "object", "properties": { @@ -30041,55 +30298,6 @@ } } }, - "codersdk.UserSkill": { - "type": "object", - "properties": { - "content": { - "type": "string" - }, - "created_at": { - "type": "string", - "format": "date-time" - }, - "description": { - "type": "string" - }, - "id": { - "type": "string", - "format": "uuid" - }, - "name": { - "type": "string" - }, - "updated_at": { - "type": "string", - "format": "date-time" - } - } - }, - "codersdk.UserSkillMetadata": { - "type": "object", - "properties": { - "created_at": { - "type": "string", - "format": "date-time" - }, - "description": { - "type": "string" - }, - "id": { - "type": "string", - "format": "uuid" - }, - "name": { - "type": "string" - }, - "updated_at": { - "type": "string", - "format": "date-time" - } - } - }, "codersdk.UserStatus": { "type": "string", "enum": ["active", "dormant", "suspended"], diff --git a/coderd/audit/diff.go b/coderd/audit/diff.go index 6099aec61b..1a8f31299f 100644 --- a/coderd/audit/diff.go +++ b/coderd/audit/diff.go @@ -47,6 +47,7 @@ type Auditable interface { database.AuditableUserAIBudgetOverride | database.UserSecret | database.Skill | + database.AuditableOrganizationSkill | database.ChatInstructionSettings | database.ChatOperationalSettings | database.ChatOrganizationSystemPrompt | diff --git a/coderd/audit/request.go b/coderd/audit/request.go index 00383ddb87..bbac6b5f1f 100644 --- a/coderd/audit/request.go +++ b/coderd/audit/request.go @@ -169,6 +169,8 @@ func ResourceTarget[T Auditable](tgt T) string { return typed.Name case database.Skill: return typed.Name + case database.AuditableOrganizationSkill: + return typed.Name case database.ChatInstructionSettings: return typed.Name case database.ChatOperationalSettings: @@ -287,6 +289,8 @@ func ResourceID[T Auditable](tgt T) uuid.UUID { return typed.ID case database.Skill: return typed.ID + case database.AuditableOrganizationSkill: + return typed.ID case database.ChatInstructionSettings: // Fixed ID per setting; see ChatInstructionSettings IDs. return typed.ID @@ -380,6 +384,8 @@ func ResourceType[T Auditable](tgt T) database.ResourceType { return database.ResourceTypeUserSecret case database.Skill: return database.ResourceTypeUserSkill + case database.AuditableOrganizationSkill: + return database.ResourceTypeOrganizationSkill case database.ChatInstructionSettings: return database.ResourceTypeChatInstructionSettings case database.ChatOperationalSettings: @@ -487,6 +493,8 @@ func ResourceRequiresOrgID[T Auditable]() bool { case database.Skill: // User skills are global to the user across organizations. return false + case database.AuditableOrganizationSkill: + return true case database.ChatInstructionSettings: // Deployment settings, not scoped to any organization. return false diff --git a/coderd/coderd.go b/coderd/coderd.go index 7ef5fb2073..4c11d75ea3 100644 --- a/coderd/coderd.go +++ b/coderd/coderd.go @@ -1371,6 +1371,25 @@ func New(options *Options) *API { r.Delete("/", api.deleteUserSkill) }) }) + r.Route("/organizations/{organization}/skills", func(r chi.Router) { + r.Use( + apiKeyMiddleware, + httpmw.ExtractOrganizationParam(options.Database), + ) + r.Get("/", api.getOrganizationSkills) + r.Post("/", api.postOrganizationSkill) + r.Route("/{skillName}", func(r chi.Router) { + r.With(httpmw.ExtractOrganizationSkillParam(options.Database, api.HTTPAuth.Authorize, + policy.ActionRead)).Get("/", api.getOrganizationSkill) + // PATCH returns the skill's content, so it requires read like GET. + // Readers reach the handlers, where the database wrapper denies + // the mutation with 403 rather than concealing the skill. + r.With(httpmw.ExtractOrganizationSkillParam(options.Database, api.HTTPAuth.Authorize, + policy.ActionRead)).Patch("/", api.patchOrganizationSkill) + r.With(httpmw.ExtractOrganizationSkillParam(options.Database, api.HTTPAuth.Authorize, + policy.ActionRead, policy.ActionDelete)).Delete("/", api.deleteOrganizationSkill) + }) + }) api.registerExperimentalChatRoutes(r, apiKeyMiddleware) r.Route("/organizations/{organization}", func(r chi.Router) { r.Use( diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index a6729da394..3a6ea5ed65 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -2426,13 +2426,14 @@ func UserSecrets(secrets []database.ListUserSecretsRow) []codersdk.UserSecret { return result } -// UserSkill converts a database UserSkill to an SDK UserSkill. -func UserSkill(skill database.Skill) codersdk.UserSkill { - return codersdk.UserSkill{ - UserSkillMetadata: codersdk.UserSkillMetadata{ +// Skill converts a database Skill to an SDK Skill. +func Skill(skill database.Skill) codersdk.Skill { + return codersdk.Skill{ + SkillMetadata: codersdk.SkillMetadata{ ID: skill.ID, Name: skill.Name, Description: skill.Description, + Enabled: skill.Enabled, CreatedAt: skill.CreatedAt, UpdatedAt: skill.UpdatedAt, }, @@ -2440,22 +2441,35 @@ func UserSkill(skill database.Skill) codersdk.UserSkill { } } -// UserSkillMetadata converts database user skill metadata to an SDK UserSkillMetadata. -func UserSkillMetadata(skill database.ListUserSkillMetadataByUserIDRow) codersdk.UserSkillMetadata { - return codersdk.UserSkillMetadata{ - ID: skill.ID, - Name: skill.Name, - Description: skill.Description, - CreatedAt: skill.CreatedAt, - UpdatedAt: skill.UpdatedAt, - } -} - -// UserSkillMetadataList converts database user skill metadata rows to SDK values. -func UserSkillMetadataList(rows []database.ListUserSkillMetadataByUserIDRow) []codersdk.UserSkillMetadata { - metadata := make([]codersdk.UserSkillMetadata, 0, len(rows)) +// OrganizationSkillMetadataList converts organization skill metadata rows to +// SDK values. +func OrganizationSkillMetadataList(rows []database.ListOrganizationSkillMetadataByOrganizationIDRow) []codersdk.SkillMetadata { + metadata := make([]codersdk.SkillMetadata, 0, len(rows)) for _, row := range rows { - metadata = append(metadata, UserSkillMetadata(row)) + metadata = append(metadata, codersdk.SkillMetadata{ + ID: row.ID, + Name: row.Name, + Description: row.Description, + Enabled: row.Enabled, + CreatedAt: row.CreatedAt, + UpdatedAt: row.UpdatedAt, + }) + } + return metadata +} + +// UserSkillMetadataList converts personal skill metadata rows to SDK values. +func UserSkillMetadataList(rows []database.ListUserSkillMetadataByUserIDRow) []codersdk.SkillMetadata { + metadata := make([]codersdk.SkillMetadata, 0, len(rows)) + for _, row := range rows { + metadata = append(metadata, codersdk.SkillMetadata{ + ID: row.ID, + Name: row.Name, + Description: row.Description, + Enabled: row.Enabled, + CreatedAt: row.CreatedAt, + UpdatedAt: row.UpdatedAt, + }) } return metadata } diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 9ac2fb15f5..b65c9e202d 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -4651,6 +4651,10 @@ func (q *querier) GetOrganizationResourceCountByID(ctx context.Context, organiza return q.db.GetOrganizationResourceCountByID(ctx, organizationID) } +func (q *querier) GetOrganizationSkillByIDForUpdate(ctx context.Context, id uuid.UUID) (database.Skill, error) { + return fetch(q.log, q.auth, q.db.GetOrganizationSkillByIDForUpdate)(ctx, id) +} + func (q *querier) GetOrganizationSkillByOrganizationIDAndName(ctx context.Context, arg database.GetOrganizationSkillByOrganizationIDAndNameParams) (database.Skill, error) { return fetch(q.log, q.auth, q.db.GetOrganizationSkillByOrganizationIDAndName)(ctx, arg) } diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 26e4f872b2..fe92378b43 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -6969,6 +6969,11 @@ func (s *MethodTestSuite) TestOrganizationSkills() { dbm.EXPECT().DeleteOrganizationSkillByOrganizationIDAndName(gomock.Any(), arg).Return(skill, nil).AnyTimes() check.Args(arg).Asserts(skill, policy.ActionDelete).Returns(skill) })) + s.Run("GetOrganizationSkillByIDForUpdate", s.Mocked(func(dbm *dbmock.MockStore, _ *gofakeit.Faker, check *expects) { + skill := orgSkill(uuid.New(), "test") + dbm.EXPECT().GetOrganizationSkillByIDForUpdate(gomock.Any(), skill.ID).Return(skill, nil).AnyTimes() + check.Args(skill.ID).Asserts(skill, policy.ActionRead).Returns(skill) + })) } func (s *MethodTestSuite) TestUsageEvents() { diff --git a/coderd/database/dbmetrics/querymetrics.go b/coderd/database/dbmetrics/querymetrics.go index 93964f9a78..7b72fc7058 100644 --- a/coderd/database/dbmetrics/querymetrics.go +++ b/coderd/database/dbmetrics/querymetrics.go @@ -2816,6 +2816,14 @@ func (m queryMetricsStore) GetOrganizationResourceCountByID(ctx context.Context, return r0, r1 } +func (m queryMetricsStore) GetOrganizationSkillByIDForUpdate(ctx context.Context, id uuid.UUID) (database.Skill, error) { + start := time.Now() + r0, r1 := m.s.GetOrganizationSkillByIDForUpdate(ctx, id) + m.queryLatencies.WithLabelValues("GetOrganizationSkillByIDForUpdate").Observe(time.Since(start).Seconds()) + m.queryCounts.WithLabelValues(httpmw.ExtractHTTPRoute(ctx), httpmw.ExtractHTTPMethod(ctx), "GetOrganizationSkillByIDForUpdate").Inc() + return r0, r1 +} + func (m queryMetricsStore) GetOrganizationSkillByOrganizationIDAndName(ctx context.Context, arg database.GetOrganizationSkillByOrganizationIDAndNameParams) (database.Skill, error) { start := time.Now() r0, r1 := m.s.GetOrganizationSkillByOrganizationIDAndName(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index b7e96d9935..9a0914a9c3 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -5291,6 +5291,21 @@ func (mr *MockStoreMockRecorder) GetOrganizationResourceCountByID(ctx, organizat return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationResourceCountByID", reflect.TypeOf((*MockStore)(nil).GetOrganizationResourceCountByID), ctx, organizationID) } +// GetOrganizationSkillByIDForUpdate mocks base method. +func (m *MockStore) GetOrganizationSkillByIDForUpdate(ctx context.Context, id uuid.UUID) (database.Skill, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetOrganizationSkillByIDForUpdate", ctx, id) + ret0, _ := ret[0].(database.Skill) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetOrganizationSkillByIDForUpdate indicates an expected call of GetOrganizationSkillByIDForUpdate. +func (mr *MockStoreMockRecorder) GetOrganizationSkillByIDForUpdate(ctx, id any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetOrganizationSkillByIDForUpdate", reflect.TypeOf((*MockStore)(nil).GetOrganizationSkillByIDForUpdate), ctx, id) +} + // GetOrganizationSkillByOrganizationIDAndName mocks base method. func (m *MockStore) GetOrganizationSkillByOrganizationIDAndName(ctx context.Context, arg database.GetOrganizationSkillByOrganizationIDAndNameParams) (database.Skill, error) { m.ctrl.T.Helper() diff --git a/coderd/database/modelmethods.go b/coderd/database/modelmethods.go index e496187834..088299f38d 100644 --- a/coderd/database/modelmethods.go +++ b/coderd/database/modelmethods.go @@ -79,6 +79,13 @@ func (m OrganizationMember) Auditable(username string) AuditableOrganizationMemb } } +// AuditableOrganizationSkill is the audit-log representation of an +// organization skill. It is a separate type because audit decides per Go type +// whether a resource requires an organization ID. +type AuditableOrganizationSkill struct { + Skill +} + type AuditableGroup struct { Group Members []GroupMemberTable `json:"members"` diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 2f5d55e37b..56e593359b 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -790,6 +790,7 @@ type sqlcQuerier interface { GetOrganizationGroupsAISpend(ctx context.Context, arg GetOrganizationGroupsAISpendParams) ([]GetOrganizationGroupsAISpendRow, error) GetOrganizationIDsByMemberIDs(ctx context.Context, ids []uuid.UUID) ([]GetOrganizationIDsByMemberIDsRow, error) GetOrganizationResourceCountByID(ctx context.Context, organizationID uuid.UUID) (GetOrganizationResourceCountByIDRow, error) + GetOrganizationSkillByIDForUpdate(ctx context.Context, id uuid.UUID) (Skill, error) GetOrganizationSkillByOrganizationIDAndName(ctx context.Context, arg GetOrganizationSkillByOrganizationIDAndNameParams) (Skill, error) GetOrganizations(ctx context.Context, arg GetOrganizationsParams) ([]Organization, error) GetOrganizationsByUserID(ctx context.Context, arg GetOrganizationsByUserIDParams) ([]Organization, error) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 1ff5aebe0a..2f838824d1 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -28509,6 +28509,33 @@ func (q *sqlQuerier) DeleteUserSkillByUserIDAndName(ctx context.Context, arg Del return i, err } +const getOrganizationSkillByIDForUpdate = `-- name: GetOrganizationSkillByIDForUpdate :one +SELECT id, user_id, name, description, content, created_at, updated_at, organization_id, project_id, enabled, group_acl, user_acl +FROM skills +WHERE id = $1::uuid AND organization_id IS NOT NULL +FOR UPDATE +` + +func (q *sqlQuerier) GetOrganizationSkillByIDForUpdate(ctx context.Context, id uuid.UUID) (Skill, error) { + row := q.db.QueryRowContext(ctx, getOrganizationSkillByIDForUpdate, 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 getOrganizationSkillByOrganizationIDAndName = `-- name: GetOrganizationSkillByOrganizationIDAndName :one SELECT id, user_id, name, description, content, created_at, updated_at, organization_id, project_id, enabled, group_acl, user_acl FROM skills @@ -28708,7 +28735,7 @@ func (q *sqlQuerier) ListOrganizationSkillMetadataByOrganizationID(ctx context.C const listUserSkillMetadataByUserID = `-- name: ListUserSkillMetadataByUserID :many SELECT - id, user_id, name, description, created_at, updated_at + id, user_id, name, description, enabled, created_at, updated_at FROM skills WHERE user_id = $1::uuid ORDER BY name ASC @@ -28719,6 +28746,7 @@ type ListUserSkillMetadataByUserIDRow struct { UserID uuid.NullUUID `db:"user_id" json:"user_id"` Name string `db:"name" json:"name"` Description string `db:"description" json:"description"` + Enabled bool `db:"enabled" json:"enabled"` CreatedAt time.Time `db:"created_at" json:"created_at"` UpdatedAt time.Time `db:"updated_at" json:"updated_at"` } @@ -28737,6 +28765,7 @@ func (q *sqlQuerier) ListUserSkillMetadataByUserID(ctx context.Context, userID u &i.UserID, &i.Name, &i.Description, + &i.Enabled, &i.CreatedAt, &i.UpdatedAt, ); err != nil { @@ -28801,24 +28830,27 @@ func (q *sqlQuerier) UpdateOrganizationSkillByOrganizationIDAndName(ctx context. const updateUserSkillByUserIDAndName = `-- name: UpdateUserSkillByUserIDAndName :one UPDATE skills SET - description = $1, - content = $2, + description = COALESCE($1::text, description), + content = COALESCE($2::text, content), + enabled = COALESCE($3::boolean, enabled), updated_at = now() -WHERE user_id = $3::uuid AND name = $4 +WHERE user_id = $4::uuid AND name = $5 RETURNING id, user_id, name, description, content, created_at, updated_at, organization_id, project_id, enabled, group_acl, user_acl ` type UpdateUserSkillByUserIDAndNameParams struct { - Description string `db:"description" json:"description"` - Content string `db:"content" json:"content"` - UserID uuid.UUID `db:"user_id" json:"user_id"` - Name string `db:"name" json:"name"` + Description sql.NullString `db:"description" json:"description"` + Content sql.NullString `db:"content" json:"content"` + Enabled sql.NullBool `db:"enabled" json:"enabled"` + UserID uuid.UUID `db:"user_id" json:"user_id"` + Name string `db:"name" json:"name"` } func (q *sqlQuerier) UpdateUserSkillByUserIDAndName(ctx context.Context, arg UpdateUserSkillByUserIDAndNameParams) (Skill, error) { row := q.db.QueryRowContext(ctx, updateUserSkillByUserIDAndName, arg.Description, arg.Content, + arg.Enabled, arg.UserID, arg.Name, ) diff --git a/coderd/database/queries/skills.sql b/coderd/database/queries/skills.sql index 005d2ac356..14cc876107 100644 --- a/coderd/database/queries/skills.sql +++ b/coderd/database/queries/skills.sql @@ -10,7 +10,7 @@ WHERE user_id = @user_id::uuid AND name = @name; -- name: ListUserSkillMetadataByUserID :many SELECT - id, user_id, name, description, created_at, updated_at + id, user_id, name, description, enabled, created_at, updated_at FROM skills WHERE user_id = @user_id::uuid ORDER BY name ASC; @@ -18,8 +18,9 @@ ORDER BY name ASC; -- name: UpdateUserSkillByUserIDAndName :one UPDATE skills SET - description = @description, - content = @content, + description = COALESCE(sqlc.narg('description')::text, description), + content = COALESCE(sqlc.narg('content')::text, content), + enabled = COALESCE(sqlc.narg('enabled')::boolean, enabled), updated_at = now() WHERE user_id = @user_id::uuid AND name = @name RETURNING *; @@ -62,3 +63,9 @@ RETURNING *; DELETE FROM skills WHERE organization_id = @organization_id::uuid AND name = @name RETURNING *; + +-- name: GetOrganizationSkillByIDForUpdate :one +SELECT * +FROM skills +WHERE id = @id::uuid AND organization_id IS NOT NULL +FOR UPDATE; diff --git a/coderd/httpmw/organizationskillparam.go b/coderd/httpmw/organizationskillparam.go new file mode 100644 index 0000000000..48fd0df075 --- /dev/null +++ b/coderd/httpmw/organizationskillparam.go @@ -0,0 +1,75 @@ +package httpmw + +import ( + "context" + "net/http" + + "github.com/go-chi/chi/v5" + + "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" +) + +type organizationSkillParamContextKey struct{} + +// OrganizationSkillParam returns the organization skill from the +// ExtractOrganizationSkillParam handler. +func OrganizationSkillParam(r *http.Request) database.Skill { + skill, ok := r.Context().Value(organizationSkillParamContextKey{}).(database.Skill) + if !ok { + panic("developer error: organization skill param middleware not provided") + } + return skill +} + +// ExtractOrganizationSkillParam reads the "skillName" URL parameter within the +// organization from ExtractOrganizationParam. Callers with none of the +// admitted actions are concealed as not found, so denied and missing rows both +// return 404. +func ExtractOrganizationSkillParam( + 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() + // 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. + skill, err := db.GetOrganizationSkillByOrganizationIDAndName(dbauthz.AsSystemRestricted(ctx), database.GetOrganizationSkillByOrganizationIDAndNameParams{ + OrganizationID: OrganizationParam(r).ID, + Name: chi.URLParam(r, "skillName"), + }) + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return + } + if err != nil { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: "Internal error fetching organization skill.", + Detail: err.Error(), + }) + return + } + admitted := false + for _, action := range actions { + if auth(r, action, skill) { + admitted = true + break + } + } + if !admitted { + httpapi.ResourceNotFound(rw) + return + } + + ctx = context.WithValue(ctx, organizationSkillParamContextKey{}, skill) + next.ServeHTTP(rw, r.WithContext(ctx)) + }) + } +} diff --git a/coderd/organizationskills.go b/coderd/organizationskills.go new file mode 100644 index 0000000000..6979abfa07 --- /dev/null +++ b/coderd/organizationskills.go @@ -0,0 +1,240 @@ +package coderd + +import ( + "fmt" + "net/http" + + "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" + "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/coderd/x/skills" + "github.com/coder/coder/v2/codersdk" +) + +const ( + // skillsPerOrganizationLimitConstraint is raised by the skill cap trigger + // with USING CONSTRAINT, so dbgen does not emit it. + skillsPerOrganizationLimitConstraint database.CheckConstraint = "skills_per_organization_limit" + // maxSkillsPerOrganization matches the cap trigger, which applies one + // limit to every owner kind. + maxSkillsPerOrganization = skills.MaxPersonalSkillsPerUser +) + +// @Summary Create an organization skill +// @ID create-an-organization-skill +// @Security CoderSessionToken +// @Accept json +// @Produce json +// @Tags Organizations +// @Param organization path string true "Organization ID or name" +// @Param request body codersdk.CreateSkillRequest true "Create organization skill request" +// @Success 201 {object} codersdk.Skill +// @Router /api/experimental/organizations/{organization}/skills [post] +// @x-apidocgen {"skip": true} +func (api *API) postOrganizationSkill(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + organization = httpmw.OrganizationParam(r) + auditor = api.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.AuditableOrganizationSkill](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionCreate, + OrganizationID: organization.ID, + }) + ) + defer commitAudit() + if !api.Authorize(r, policy.ActionCreate, rbac.ResourceOrganizationSkill.InOrg(organization.ID)) { + httpapi.Write(ctx, rw, http.StatusForbidden, organizationSkillCreateErrors.forbidden) + return + } + + skill, ok := createSkill(ctx, rw, r, organizationSkillCreateErrors, func(parsed skills.ParsedSkill, content string) (database.Skill, error) { + return api.Database.InsertOrganizationSkill(ctx, database.InsertOrganizationSkillParams{ + ID: uuid.New(), + OrganizationID: organization.ID, + Name: parsed.Name, + Description: parsed.Description, + Content: content, + // The Everyone group shares the organization's ID, so new skills + // reach every member until an admin narrows the ACL. + GroupACL: database.ChatACL{ + organization.ID.String(): {Permissions: []policy.Action{policy.ActionRead}}, + }, + UserACL: database.ChatACL{}, + }) + }) + if !ok { + return + } + aReq.New = database.AuditableOrganizationSkill{Skill: skill} +} + +// @Summary List organization skills +// @ID list-organization-skills +// @Security CoderSessionToken +// @Produce json +// @Tags Organizations +// @Param organization path string true "Organization ID or name" +// @Success 200 {array} codersdk.SkillMetadata +// @Router /api/experimental/organizations/{organization}/skills [get] +// @x-apidocgen {"skip": true} +func (api *API) getOrganizationSkills(rw http.ResponseWriter, r *http.Request) { //nolint:revive // Method name matches route. + ctx := r.Context() + organization := httpmw.OrganizationParam(r) + + rows, err := api.Database.ListOrganizationSkillMetadataByOrganizationID(ctx, organization.ID) + if err != nil { + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return + } + httpapi.InternalServerError(rw, err) + return + } + + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.OrganizationSkillMetadataList(rows)) +} + +// @Summary Get an organization skill by name +// @ID get-an-organization-skill-by-name +// @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.Skill +// @Router /api/experimental/organizations/{organization}/skills/{skillName} [get] +// @x-apidocgen {"skip": true} +func (api *API) getOrganizationSkill(rw http.ResponseWriter, r *http.Request) { //nolint:revive // Method name matches route. + httpapi.Write(r.Context(), rw, http.StatusOK, db2sdk.Skill(httpmw.OrganizationSkillParam(r))) +} + +// @Summary Update an organization skill +// @ID update-an-organization-skill +// @Security CoderSessionToken +// @Accept json +// @Produce json +// @Tags Organizations +// @Param organization path string true "Organization ID or name" +// @Param skillName path string true "Skill name" +// @Param request body codersdk.UpdateSkillRequest true "Update organization skill request" +// @Success 200 {object} codersdk.Skill +// @Router /api/experimental/organizations/{organization}/skills/{skillName} [patch] +// @x-apidocgen {"skip": true} +func (api *API) patchOrganizationSkill(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + oldSkill = httpmw.OrganizationSkillParam(r) + auditor = api.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.AuditableOrganizationSkill](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionWrite, + OrganizationID: oldSkill.OrganizationID.UUID, + }) + ) + defer commitAudit() + aReq.Old = database.AuditableOrganizationSkill{Skill: oldSkill} + + current, skill, ok := api.updateSkill(ctx, rw, r, oldSkill.Name, organizationSkillUpdateErrors, func(tx database.Store, update skillUpdate) (database.Skill, database.Skill, error) { + //nolint:gocritic // The update below reauthorizes the locked row for update. + locked, err := tx.GetOrganizationSkillByIDForUpdate(dbauthz.AsSystemRestricted(ctx), oldSkill.ID) + if err != nil { + return database.Skill{}, database.Skill{}, xerrors.Errorf("lock organization skill: %w", err) + } + updated, err := tx.UpdateOrganizationSkillByOrganizationIDAndName(ctx, database.UpdateOrganizationSkillByOrganizationIDAndNameParams{ + Description: update.Description, + Content: update.Content, + Enabled: update.Enabled, + OrganizationID: locked.OrganizationID.UUID, + Name: locked.Name, + }) + if err != nil { + return database.Skill{}, database.Skill{}, xerrors.Errorf("update organization skill: %w", err) + } + return locked, updated, nil + }) + if !ok { + return + } + // The audit diff starts from the locked row, not the middleware's read. + aReq.Old = database.AuditableOrganizationSkill{Skill: current} + aReq.New = database.AuditableOrganizationSkill{Skill: skill} +} + +// @Summary Delete an organization skill +// @ID delete-an-organization-skill +// @Security CoderSessionToken +// @Tags Organizations +// @Param organization path string true "Organization ID or name" +// @Param skillName path string true "Skill name" +// @Success 204 +// @Router /api/experimental/organizations/{organization}/skills/{skillName} [delete] +// @x-apidocgen {"skip": true} +func (api *API) deleteOrganizationSkill(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + skill = httpmw.OrganizationSkillParam(r) + auditor = api.Auditor.Load() + aReq, commitAudit = audit.InitRequest[database.AuditableOrganizationSkill](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionDelete, + OrganizationID: skill.OrganizationID.UUID, + }) + ) + defer commitAudit() + aReq.Old = database.AuditableOrganizationSkill{Skill: skill} + + deleted, err := api.Database.DeleteOrganizationSkillByOrganizationIDAndName(ctx, database.DeleteOrganizationSkillByOrganizationIDAndNameParams{ + OrganizationID: skill.OrganizationID.UUID, + Name: skill.Name, + }) + if err != nil { + switch { + case httpapi.IsUnauthorizedError(err): + httpapi.Write(ctx, rw, http.StatusForbidden, organizationSkillForbidden("delete this organization skill")) + case httpapi.Is404Error(err): + httpapi.ResourceNotFound(rw) + default: + httpapi.InternalServerError(rw, err) + } + return + } + aReq.Old = database.AuditableOrganizationSkill{Skill: deleted} + + rw.WriteHeader(http.StatusNoContent) +} + +var ( + // These replace httpapi.Forbidden, whose detail says the caller cannot + // view content these callers may be able to read. + organizationSkillCreateErrors = skillWriteErrors{ + forbidden: organizationSkillForbidden("create organization skills"), + limit: skillsPerOrganizationLimitConstraint, + limitReached: codersdk.Response{ + Message: "Organization skill limit reached.", + Detail: fmt.Sprintf("Each organization can have at most %d skills.", maxSkillsPerOrganization), + }, + nameIndex: database.UniqueSkillsOrganizationIDNameIndex, + } + organizationSkillUpdateErrors = skillWriteErrors{ + forbidden: organizationSkillForbidden("update this organization skill"), + } +) + +func organizationSkillForbidden(action string) codersdk.Response { + return codersdk.Response{Message: fmt.Sprintf("You don't have permission to %s.", action)} +} diff --git a/coderd/organizationskills_test.go b/coderd/organizationskills_test.go new file mode 100644 index 0000000000..1bb9f36d61 --- /dev/null +++ b/coderd/organizationskills_test.go @@ -0,0 +1,325 @@ +package coderd_test + +import ( + "context" + "encoding/json" + "fmt" + "io" + "net/http" + "testing" + + "github.com/google/uuid" + "github.com/stretchr/testify/assert" + "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/coderd/util/ptr" + "github.com/coder/coder/v2/coderd/x/skills" + "github.com/coder/coder/v2/codersdk" + "github.com/coder/coder/v2/testutil" +) + +func TestOrganizationSkillAccess(t *testing.T) { + t.Parallel() + + db, ps := dbtestutil.NewDB(t) + ownerClient := coderdtest.New(t, &coderdtest.Options{Database: db, Pubsub: ps}) + firstUser := coderdtest.CreateFirstUser(t, ownerClient) + orgID := firstUser.OrganizationID + admin := codersdk.NewExperimentalClient(ownerClient) + memberRawClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, orgID) + member := codersdk.NewExperimentalClient(memberRawClient) + sharedRawClient, sharedUser := coderdtest.CreateAnotherUser(t, ownerClient, orgID) + shared := codersdk.NewExperimentalClient(sharedRawClient) + auditorRawClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, orgID, rbac.ScopedRoleOrgAuditor(orgID)) + auditor := codersdk.NewExperimentalClient(auditorRawClient) + + everyoneACL := func() database.ChatACL { + return database.ChatACL{orgID.String(): {Permissions: []policy.Action{policy.ActionRead}}} + } + everyone := dbgen.OrganizationSkill(t, db, database.Skill{ + OrganizationID: uuid.NullUUID{UUID: orgID, Valid: true}, + Name: "everyone-skill", + GroupACL: everyoneACL(), + UserACL: database.ChatACL{}, + }) + userShared := dbgen.OrganizationSkill(t, db, database.Skill{ + OrganizationID: uuid.NullUUID{UUID: orgID, Valid: true}, + Name: "user-shared-skill", + GroupACL: database.ChatACL{}, + UserACL: database.ChatACL{ + sharedUser.ID.String(): {Permissions: []policy.Action{policy.ActionRead}}, + }, + }) + + t.Run("MemberReadsThroughEveryone", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + requireOrganizationSkillListed(ctx, t, member, orgID, everyone.Name) + _, err := member.OrganizationSkillByName(ctx, orgID, everyone.Name) + require.NoError(t, err) + + _, err = member.CreateOrganizationSkill(ctx, orgID, codersdk.CreateSkillRequest{ + Content: userSkillMarkdown("member-created", "Denied", "Body."), + }) + sdkErr := requireSDKErrorStatus(t, err, http.StatusForbidden) + assert.Equal(t, "You don't have permission to create organization skills.", sdkErr.Message) + _, err = member.UpdateOrganizationSkill(ctx, orgID, everyone.Name, codersdk.UpdateSkillRequest{Enabled: ptr.Ref(false)}) + requireSDKErrorStatus(t, err, http.StatusForbidden) + err = member.DeleteOrganizationSkill(ctx, orgID, everyone.Name) + requireSDKErrorStatus(t, err, http.StatusForbidden) + }) + + t.Run("CreateSeedsEveryoneGrant", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + _, err := admin.CreateOrganizationSkill(ctx, orgID, codersdk.CreateSkillRequest{ + Content: userSkillMarkdown("api-created-skill", "Created", "Body."), + }) + require.NoError(t, err) + requireOrganizationSkillListed(ctx, t, member, orgID, "api-created-skill") + _, err = member.OrganizationSkillByName(ctx, orgID, "api-created-skill") + require.NoError(t, err) + }) + + t.Run("UserACLGrantsOnlyThatUser", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + requireOrganizationSkillListed(ctx, t, shared, orgID, userShared.Name) + _, err := shared.OrganizationSkillByName(ctx, orgID, userShared.Name) + require.NoError(t, err) + + requireOrganizationSkillNotListed(ctx, t, member, orgID, userShared.Name) + _, err = member.OrganizationSkillByName(ctx, orgID, userShared.Name) + requireSDKErrorStatus(t, err, http.StatusNotFound) + _, err = member.UpdateOrganizationSkill(ctx, orgID, userShared.Name, codersdk.UpdateSkillRequest{Enabled: ptr.Ref(false)}) + requireSDKErrorStatus(t, err, http.StatusNotFound) + err = member.DeleteOrganizationSkill(ctx, orgID, userShared.Name) + requireSDKErrorStatus(t, err, http.StatusNotFound) + }) + + t.Run("OrgAuditorReads", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + requireOrganizationSkillListed(ctx, t, auditor, orgID, userShared.Name) + _, err := auditor.OrganizationSkillByName(ctx, orgID, userShared.Name) + require.NoError(t, err) + _, err = auditor.UpdateOrganizationSkill(ctx, orgID, userShared.Name, codersdk.UpdateSkillRequest{Enabled: ptr.Ref(false)}) + sdkErr := requireSDKErrorStatus(t, err, http.StatusForbidden) + assert.Equal(t, "You don't have permission to update this organization skill.", sdkErr.Message) + err = auditor.DeleteOrganizationSkill(ctx, orgID, userShared.Name) + sdkErr = requireSDKErrorStatus(t, err, http.StatusForbidden) + assert.Equal(t, "You don't have permission to delete this organization skill.", sdkErr.Message) + }) + + t.Run("OtherOrganizationMember", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + 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}) + _, token := dbgen.APIKey(t, db, database.APIKey{UserID: foreignUser.ID}) + foreignRawClient := codersdk.New(ownerClient.URL) + foreignRawClient.SetSessionToken(token) + foreign := codersdk.NewExperimentalClient(foreignRawClient) + + _, err := foreign.OrganizationSkills(ctx, orgID) + requireSDKErrorStatus(t, err, http.StatusNotFound) + _, err = foreign.OrganizationSkillByName(ctx, orgID, everyone.Name) + requireSDKErrorStatus(t, err, http.StatusNotFound) + }) + + t.Run("ScopedAPIKeys", func(t *testing.T) { + t.Parallel() + + scopedClient := func(scope database.APIKeyScope) *codersdk.ExperimentalClient { + _, token := dbgen.APIKey(t, db, database.APIKey{ + UserID: firstUser.UserID, + Scopes: database.APIKeyScopes{"organization:read", scope}, + }) + client := codersdk.New(ownerClient.URL) + client.SetSessionToken(token) + return codersdk.NewExperimentalClient(client) + } + + ctx := testutil.Context(t, testutil.WaitMedium) + reader := scopedClient("organization_skill:read") + requireOrganizationSkillListed(ctx, t, reader, orgID, everyone.Name) + _, err := reader.OrganizationSkillByName(ctx, orgID, everyone.Name) + require.NoError(t, err) + _, err = reader.CreateOrganizationSkill(ctx, orgID, codersdk.CreateSkillRequest{ + Content: userSkillMarkdown("scoped-created", "Denied", "Body."), + }) + requireSDKErrorStatus(t, err, http.StatusForbidden) + _, err = reader.UpdateOrganizationSkill(ctx, orgID, everyone.Name, codersdk.UpdateSkillRequest{Enabled: ptr.Ref(false)}) + requireSDKErrorStatus(t, err, http.StatusForbidden) + + for _, scope := range []database.APIKeyScope{"organization_skill:update", "organization_skill:delete"} { + _, err := scopedClient(scope).OrganizationSkillByName(ctx, orgID, everyone.Name) + requireSDKErrorStatus(t, err, http.StatusNotFound, scope) + } + + // A PATCH response carries the skill's content, so an update-only key + // must not reach the handler. + updater := scopedClient("organization_skill:update") + res, err := updater.Request(ctx, http.MethodPatch, + fmt.Sprintf("/api/experimental/organizations/%s/skills/%s", orgID, everyone.Name), + codersdk.UpdateSkillRequest{Enabled: ptr.Ref(false)}) + require.NoError(t, err) + defer res.Body.Close() + require.Equal(t, http.StatusNotFound, res.StatusCode) + body, err := io.ReadAll(res.Body) + require.NoError(t, err) + assert.NotContains(t, string(body), everyone.Content) + got, err := admin.OrganizationSkillByName(ctx, orgID, everyone.Name) + require.NoError(t, err) + assert.True(t, got.Enabled, "denied PATCH must not apply") + }) +} + +func TestOrganizationSkillLimit(t *testing.T) { + t.Parallel() + + ownerClient, db := coderdtest.NewWithDatabase(t, nil) + firstUser := coderdtest.CreateFirstUser(t, ownerClient) + client := codersdk.NewExperimentalClient(ownerClient) + ctx := testutil.Context(t, testutil.WaitLong) + + for i := range skills.MaxPersonalSkillsPerUser { + dbgen.OrganizationSkill(t, db, database.Skill{ + OrganizationID: uuid.NullUUID{UUID: firstUser.OrganizationID, Valid: true}, + Name: fmt.Sprintf("org-limit-skill-%03d", i), + }) + } + + _, err := client.CreateOrganizationSkill(ctx, firstUser.OrganizationID, codersdk.CreateSkillRequest{ + Content: userSkillMarkdown("org-limit-overflow", "Limit", "Body."), + }) + sdkErr := requireSDKErrorStatus(t, err, http.StatusConflict) + assert.Equal(t, "Organization skill limit reached.", sdkErr.Message) + assert.Equal(t, + fmt.Sprintf("Each organization can have at most %d skills.", skills.MaxPersonalSkillsPerUser), + sdkErr.Detail, + ) +} + +// TestOrganizationSkillAuditsDeniedWrites covers denied writes, which the +// middleware admits for readers and the handler audits before rejecting. +func TestOrganizationSkillAuditsDeniedWrites(t *testing.T) { + t.Parallel() + + auditor := audit.NewMock() + ownerClient := coderdtest.New(t, &coderdtest.Options{Auditor: auditor}) + firstUser := coderdtest.CreateFirstUser(t, ownerClient) + orgID := firstUser.OrganizationID + client := codersdk.NewExperimentalClient(ownerClient) + memberRawClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, orgID) + member := codersdk.NewExperimentalClient(memberRawClient) + ctx := testutil.Context(t, testutil.WaitMedium) + + skill, err := client.CreateOrganizationSkill(ctx, orgID, codersdk.CreateSkillRequest{ + Content: userSkillMarkdown("audited-skill", "Audit", "Body."), + }) + require.NoError(t, err) + auditor.ResetLogs() + _, err = member.UpdateOrganizationSkill(ctx, orgID, skill.Name, codersdk.UpdateSkillRequest{Enabled: ptr.Ref(false)}) + requireSDKErrorStatus(t, err, http.StatusForbidden) + err = member.DeleteOrganizationSkill(ctx, orgID, skill.Name) + requireSDKErrorStatus(t, err, http.StatusForbidden) + + logs := auditor.AuditLogs() + require.Len(t, logs, 2) + for i, action := range []database.AuditAction{database.AuditActionWrite, database.AuditActionDelete} { + assert.Equal(t, action, logs[i].Action) + assert.EqualValues(t, http.StatusForbidden, logs[i].StatusCode) + assert.Equal(t, database.ResourceTypeOrganizationSkill, logs[i].ResourceType) + assert.Equal(t, skill.ID, logs[i].ResourceID) + assert.Equal(t, skill.Name, logs[i].ResourceTarget) + assert.Equal(t, orgID, logs[i].OrganizationID) + } +} + +func TestOrganizationSkillUpdateAuditsLockedRow(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + auditor := 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{"description": {Old: oldSkill.Description, New: newSkill.Description}} + }) + db, ps, sqlDB := dbtestutil.NewDBWithSQLDB(t) + ownerClient := coderdtest.New(t, &coderdtest.Options{Database: db, Pubsub: ps, Auditor: auditor}) + firstUser := coderdtest.CreateFirstUser(t, ownerClient) + client := codersdk.NewExperimentalClient(ownerClient) + skill, err := client.CreateOrganizationSkill(ctx, firstUser.OrganizationID, codersdk.CreateSkillRequest{ + Content: userSkillMarkdown("locked-audit-skill", "Before", "Body."), + }) + require.NoError(t, err) + + tx, err := sqlDB.BeginTx(ctx, nil) + require.NoError(t, err) + t.Cleanup(func() { _ = tx.Rollback() }) + _, err = tx.ExecContext(ctx, `UPDATE skills SET description = 'Concurrent' WHERE id = $1`, skill.ID) + require.NoError(t, err) + + patchErr := make(chan error, 1) + go func() { + _, err := client.UpdateOrganizationSkill(ctx, firstUser.OrganizationID, skill.Name, codersdk.UpdateSkillRequest{Enabled: ptr.Ref(false)}) + patchErr <- err + }() + testutil.Eventually(ctx, t, func(ctx context.Context) bool { + 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 waits for the skill row lock") + require.NoError(t, tx.Commit()) + require.NoError(t, testutil.RequireReceive(ctx, t, patchErr)) + + var log database.AuditLog + for _, entry := range auditor.AuditLogs() { + if entry.ResourceID == skill.ID && entry.Action == database.AuditActionWrite { + log = entry + } + } + var diff map[string]codersdk.AuditDiffField + require.NoError(t, json.Unmarshal(log.Diff, &diff)) + require.Equal(t, "Concurrent", diff["description"].Old) +} + +func requireOrganizationSkillListed(ctx context.Context, t *testing.T, client *codersdk.ExperimentalClient, orgID uuid.UUID, name string) codersdk.SkillMetadata { + t.Helper() + list, err := client.OrganizationSkills(ctx, orgID) + require.NoError(t, err) + for _, skill := range list { + if skill.Name == name { + return skill + } + } + require.Failf(t, "organization skill not listed", "%q missing from %v", name, list) + return codersdk.SkillMetadata{} +} + +func requireOrganizationSkillNotListed(ctx context.Context, t *testing.T, client *codersdk.ExperimentalClient, orgID uuid.UUID, name string) { + t.Helper() + list, err := client.OrganizationSkills(ctx, orgID) + require.NoError(t, err) + for _, skill := range list { + require.NotEqual(t, name, skill.Name, "organization skill unexpectedly listed") + } +} diff --git a/coderd/userskills.go b/coderd/userskills.go index 6ad654e317..24208b5a40 100644 --- a/coderd/userskills.go +++ b/coderd/userskills.go @@ -2,6 +2,7 @@ package coderd import ( "context" + "database/sql" "errors" "fmt" "net/http" @@ -20,13 +21,13 @@ import ( ) const ( - // personalSkillJSONEscapeExpansion is the maximum expansion for one byte in a JSON string. - personalSkillJSONEscapeExpansion = 6 - // personalSkillRequestEnvelopeBytes leaves room for the surrounding JSON object. - personalSkillRequestEnvelopeBytes = 1024 - // maxPersonalSkillRequestBytes allows worst-case JSON string escaping for + // skillJSONEscapeExpansion is the maximum expansion for one byte in a JSON string. + skillJSONEscapeExpansion = 6 + // skillRequestEnvelopeBytes leaves room for the surrounding JSON object. + skillRequestEnvelopeBytes = 1024 + // maxSkillRequestBytes allows worst-case JSON string escaping for // otherwise valid raw skill content. - maxPersonalSkillRequestBytes = skills.MaxPersonalSkillSizeBytes*personalSkillJSONEscapeExpansion + personalSkillRequestEnvelopeBytes + maxSkillRequestBytes = skills.MaxPersonalSkillSizeBytes*skillJSONEscapeExpansion + skillRequestEnvelopeBytes // These names are raised by trigger functions with USING CONSTRAINT. // They are not table CHECK constraints, so dbgen does not emit them in @@ -42,8 +43,8 @@ const ( // @Produce json // @Tags Users // @Param user path string true "User ID, username, or me" -// @Param request body codersdk.CreateUserSkillRequest true "Create user skill request" -// @Success 201 {object} codersdk.UserSkill +// @Param request body codersdk.CreateSkillRequest true "Create user skill request" +// @Success 201 {object} codersdk.Skill // @Router /api/experimental/users/{user}/skills [post] // @x-apidocgen {"skip": true} func (api *API) postUserSkill(rw http.ResponseWriter, r *http.Request) { @@ -60,55 +61,19 @@ func (api *API) postUserSkill(rw http.ResponseWriter, r *http.Request) { ) defer commitAudit() - var req codersdk.CreateUserSkillRequest - if !httpapi.ReadLimit(ctx, rw, r, maxPersonalSkillRequestBytes, &req) { - return - } - - parsedSkill, err := skills.ParsePersonalSkillMarkdown([]byte(req.Content)) - if err != nil { - writeInvalidUserSkillContent(ctx, rw, err) - return - } - - params := database.InsertUserSkillParams{ - ID: uuid.New(), - UserID: user.ID, - Name: parsedSkill.Name, - Description: parsedSkill.Description, - Content: req.Content, - } - skill, err := api.Database.InsertUserSkill(ctx, params) - if err != nil { - if httpapi.IsUnauthorizedError(err) { - httpapi.Forbidden(rw) - return - } - if database.IsCheckViolation(err, userSkillUserDeletedConstraint) { - writeCannotCreateUserSkillForDeletedUser(ctx, rw) - return - } - if httpapi.Is404Error(err) { - httpapi.ResourceNotFound(rw) - return - } - if database.IsCheckViolation(err, skillsPerUserLimitConstraint) { - writeUserSkillLimitReached(ctx, rw) - return - } - if database.IsUniqueViolation(err, database.UniqueSkillsUserIDNameIndex) { - httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ - Message: "A skill with that name already exists.", - Detail: err.Error(), - }) - return - } - httpapi.InternalServerError(rw, err) + skill, ok := createSkill(ctx, rw, r, userSkillCreateErrors, func(parsed skills.ParsedSkill, content string) (database.Skill, error) { + return api.Database.InsertUserSkill(ctx, database.InsertUserSkillParams{ + ID: uuid.New(), + UserID: user.ID, + Name: parsed.Name, + Description: parsed.Description, + Content: content, + }) + }) + if !ok { return } aReq.New = skill - - httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.UserSkill(skill)) } // @Summary List user skills @@ -117,7 +82,7 @@ func (api *API) postUserSkill(rw http.ResponseWriter, r *http.Request) { // @Produce json // @Tags Users // @Param user path string true "User ID, username, or me" -// @Success 200 {array} codersdk.UserSkillMetadata +// @Success 200 {array} codersdk.SkillMetadata // @Router /api/experimental/users/{user}/skills [get] // @x-apidocgen {"skip": true} func (api *API) getUserSkills(rw http.ResponseWriter, r *http.Request) { //nolint:revive // Method name matches route. @@ -144,7 +109,7 @@ func (api *API) getUserSkills(rw http.ResponseWriter, r *http.Request) { //nolin // @Tags Users // @Param user path string true "User ID, username, or me" // @Param skillName path string true "Skill name" -// @Success 200 {object} codersdk.UserSkill +// @Success 200 {object} codersdk.Skill // @Router /api/experimental/users/{user}/skills/{skillName} [get] // @x-apidocgen {"skip": true} func (api *API) getUserSkill(rw http.ResponseWriter, r *http.Request) { //nolint:revive // Method name matches route. @@ -165,7 +130,7 @@ func (api *API) getUserSkill(rw http.ResponseWriter, r *http.Request) { //nolint return } - httpapi.Write(ctx, rw, http.StatusOK, db2sdk.UserSkill(skill)) + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Skill(skill)) } // @Summary Update a user skill @@ -176,8 +141,8 @@ func (api *API) getUserSkill(rw http.ResponseWriter, r *http.Request) { //nolint // @Tags Users // @Param user path string true "User ID, username, or me" // @Param skillName path string true "Skill name" -// @Param request body codersdk.UpdateUserSkillRequest true "Update user skill request" -// @Success 200 {object} codersdk.UserSkill +// @Param request body codersdk.UpdateSkillRequest true "Update user skill request" +// @Success 200 {object} codersdk.Skill // @Router /api/experimental/users/{user}/skills/{skillName} [patch] // @x-apidocgen {"skip": true} func (api *API) patchUserSkill(rw http.ResponseWriter, r *http.Request) { @@ -195,75 +160,31 @@ func (api *API) patchUserSkill(rw http.ResponseWriter, r *http.Request) { ) defer commitAudit() - var req codersdk.UpdateUserSkillRequest - if !httpapi.ReadLimit(ctx, rw, r, maxPersonalSkillRequestBytes, &req) { - return - } - - parsedSkill, err := skills.ParsePersonalSkillMarkdown([]byte(req.Content)) - if err != nil { - writeInvalidUserSkillContent(ctx, rw, err) - return - } - if parsedSkill.Name != name { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Skill name in path does not match frontmatter name.", - Detail: fmt.Sprintf("path has %q, frontmatter has %q", name, parsedSkill.Name), - }) - return - } - - params := database.UpdateUserSkillByUserIDAndNameParams{ - UserID: user.ID, - Name: name, - Description: parsedSkill.Description, - Content: req.Content, - } - - var ( - skill database.Skill - oldSkill database.Skill - ) - err = api.Database.InTx(func(tx database.Store) error { + oldSkill, skill, ok := api.updateSkill(ctx, rw, r, name, userSkillUpdateErrors, func(tx database.Store, update skillUpdate) (database.Skill, database.Skill, error) { fetched, err := tx.GetUserSkillByUserIDAndName(ctx, database.GetUserSkillByUserIDAndNameParams{ UserID: user.ID, Name: name, }) if err != nil { - return xerrors.Errorf("fetch user skill: %w", err) + return database.Skill{}, database.Skill{}, xerrors.Errorf("fetch user skill: %w", err) } - - updated, err := tx.UpdateUserSkillByUserIDAndName(ctx, params) + updated, err := tx.UpdateUserSkillByUserIDAndName(ctx, database.UpdateUserSkillByUserIDAndNameParams{ + UserID: user.ID, + Name: name, + Description: update.Description, + Content: update.Content, + Enabled: update.Enabled, + }) if err != nil { - return xerrors.Errorf("update user skill: %w", err) + return database.Skill{}, database.Skill{}, xerrors.Errorf("update user skill: %w", err) } - oldSkill = fetched - skill = updated - return nil - }, nil) - if err != nil { - if httpapi.IsUnauthorizedError(err) { - httpapi.Forbidden(rw) - return - } - if database.IsCheckViolation(err, userSkillUserDeletedConstraint) { - writeCannotModifyUserSkillForDeletedUser(ctx, rw) - return - } - if httpapi.Is404Error(err) { - httpapi.ResourceNotFound(rw) - return - } - httpapi.InternalServerError(rw, err) + return fetched, updated, nil + }) + if !ok { return } - - // Assign audit state after InTx returns so the audit log can never - // claim a rolled-back update was committed. aReq.Old = oldSkill aReq.New = skill - - httpapi.Write(ctx, rw, http.StatusOK, db2sdk.UserSkill(skill)) } // @Summary Delete a user skill @@ -307,31 +228,166 @@ func (api *API) deleteUserSkill(rw http.ResponseWriter, r *http.Request) { rw.WriteHeader(http.StatusNoContent) } -func writeCannotCreateUserSkillForDeletedUser(ctx context.Context, rw http.ResponseWriter) { +var ( + userSkillCreateErrors = skillWriteErrors{ + ownerDeleted: userDeletedSkillResponse("Cannot create skills for deleted users."), + limit: skillsPerUserLimitConstraint, + limitReached: codersdk.Response{ + Message: "Personal skill limit reached.", + Detail: fmt.Sprintf("Each user can have at most %d personal skills.", skills.MaxPersonalSkillsPerUser), + }, + nameIndex: database.UniqueSkillsUserIDNameIndex, + } + userSkillUpdateErrors = skillWriteErrors{ + ownerDeleted: userDeletedSkillResponse("Cannot modify skills for deleted users."), + } +) + +func userDeletedSkillResponse(message string) codersdk.Response { + return codersdk.Response{Message: message, Detail: "This user has been deleted and cannot be modified."} +} + +// skillWriteErrors holds the owner-specific responses for errors from a +// skill insert or update. Empty responses leave their error to the generic +// mapping, and an empty forbidden response uses httpapi.Forbidden. +type skillWriteErrors struct { + forbidden codersdk.Response + ownerDeleted codersdk.Response + limit database.CheckConstraint + limitReached codersdk.Response + nameIndex database.UniqueConstraint +} + +func (e skillWriteErrors) write(ctx context.Context, rw http.ResponseWriter, err error) { + switch { + case httpapi.IsUnauthorizedError(err) && e.forbidden.Message == "": + httpapi.Forbidden(rw) + case httpapi.IsUnauthorizedError(err): + httpapi.Write(ctx, rw, http.StatusForbidden, e.forbidden) + case e.ownerDeleted.Message != "" && database.IsCheckViolation(err, userSkillUserDeletedConstraint): + httpapi.Write(ctx, rw, http.StatusConflict, e.ownerDeleted) + case httpapi.Is404Error(err): + httpapi.ResourceNotFound(rw) + case e.limitReached.Message != "" && database.IsCheckViolation(err, e.limit): + httpapi.Write(ctx, rw, http.StatusConflict, e.limitReached) + case e.nameIndex != "" && database.IsUniqueViolation(err, e.nameIndex): + writeSkillNameConflict(ctx, rw) + default: + httpapi.InternalServerError(rw, err) + } +} + +// createSkill parses a skill create request, stores the skill with insert, +// and writes the response. It returns false when it wrote an error. +func createSkill( + ctx context.Context, + rw http.ResponseWriter, + r *http.Request, + errs skillWriteErrors, + insert func(parsed skills.ParsedSkill, content string) (database.Skill, error), +) (database.Skill, bool) { + var req codersdk.CreateSkillRequest + if !httpapi.ReadLimit(ctx, rw, r, maxSkillRequestBytes, &req) { + return database.Skill{}, false + } + parsed, err := skills.ParsePersonalSkillMarkdown([]byte(req.Content)) + if err != nil { + writeInvalidSkillContent(ctx, rw, err) + return database.Skill{}, false + } + skill, err := insert(parsed, req.Content) + if err != nil { + errs.write(ctx, rw, err) + return database.Skill{}, false + } + httpapi.Write(ctx, rw, http.StatusCreated, db2sdk.Skill(skill)) + return skill, true +} + +// updateSkill applies a skill PATCH request for the skill named name and +// writes the response. apply runs in a transaction and returns the rows +// before and after the update. Callers assign audit state only after it +// returns true, so the audit log never claims a rolled-back update. +func (api *API) updateSkill( + ctx context.Context, + rw http.ResponseWriter, + r *http.Request, + name string, + errs skillWriteErrors, + apply func(tx database.Store, update skillUpdate) (old database.Skill, updated database.Skill, err error), +) (old database.Skill, updated database.Skill, ok bool) { + update, ok := readSkillUpdate(ctx, rw, r, name) + if !ok { + return database.Skill{}, database.Skill{}, false + } + err := api.Database.InTx(func(tx database.Store) error { + var err error + old, updated, err = apply(tx, update) + return err + }, nil) + if err != nil { + errs.write(ctx, rw, err) + return database.Skill{}, database.Skill{}, false + } + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Skill(updated)) + return old, updated, true +} + +// skillUpdate holds the optional columns a skill PATCH writes. +type skillUpdate struct { + Description sql.NullString + Content sql.NullString + Enabled sql.NullBool +} + +// readSkillUpdate reads and validates a PATCH request for the skill named +// name. It writes the error response and returns false when the request is +// invalid. +func readSkillUpdate(ctx context.Context, rw http.ResponseWriter, r *http.Request, name string) (skillUpdate, bool) { + var req codersdk.UpdateSkillRequest + if !httpapi.ReadLimit(ctx, rw, r, maxSkillRequestBytes, &req) { + return skillUpdate{}, false + } + if req.Content == nil && req.Enabled == nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "No skill fields to update.", + Detail: "Set content, enabled, or both.", + }) + return skillUpdate{}, false + } + + var update skillUpdate + if req.Enabled != nil { + update.Enabled = sql.NullBool{Bool: *req.Enabled, Valid: true} + } + if req.Content == nil { + return update, true + } + parsed, err := skills.ParsePersonalSkillMarkdown([]byte(*req.Content)) + if err != nil { + writeInvalidSkillContent(ctx, rw, err) + return skillUpdate{}, false + } + if parsed.Name != name { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Skill name in path does not match frontmatter name.", + Detail: fmt.Sprintf("path has %q, frontmatter has %q", name, parsed.Name), + }) + return skillUpdate{}, false + } + update.Description = sql.NullString{String: parsed.Description, Valid: true} + update.Content = sql.NullString{String: *req.Content, Valid: true} + return update, true +} + +func writeSkillNameConflict(ctx context.Context, rw http.ResponseWriter) { httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ - Message: "Cannot create skills for deleted users.", - Detail: "This user has been deleted and cannot be modified.", + Message: "A skill with that name already exists.", + Detail: "Choose a different name, or edit the existing skill.", }) } -func writeCannotModifyUserSkillForDeletedUser(ctx context.Context, rw http.ResponseWriter) { - httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ - Message: "Cannot modify skills for deleted users.", - Detail: "This user has been deleted and cannot be modified.", - }) -} - -func writeUserSkillLimitReached(ctx context.Context, rw http.ResponseWriter) { - httpapi.Write(ctx, rw, http.StatusConflict, codersdk.Response{ - Message: "Personal skill limit reached.", - Detail: fmt.Sprintf( - "Each user can have at most %d personal skills.", - skills.MaxPersonalSkillsPerUser, - ), - }) -} - -func writeInvalidUserSkillContent(ctx context.Context, rw http.ResponseWriter, err error) { +func writeInvalidSkillContent(ctx context.Context, rw http.ResponseWriter, err error) { message := "Invalid skill content." switch { case errors.Is(err, skills.ErrInvalidSkillName): diff --git a/coderd/userskills_test.go b/coderd/userskills_test.go index 753bfa8a84..7e2a5f192f 100644 --- a/coderd/userskills_test.go +++ b/coderd/userskills_test.go @@ -1,6 +1,7 @@ package coderd_test import ( + "context" "database/sql" "encoding/json" "fmt" @@ -16,193 +17,319 @@ import ( "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/rbac" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/coderd/x/skills" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" ) -func TestPatchUserSkill(t *testing.T) { - t.Parallel() - - ownerRawClient := coderdtest.New(t, nil) - firstUser := coderdtest.CreateFirstUser(t, ownerRawClient) - memberRawClient, member := coderdtest.CreateAnotherUser(t, ownerRawClient, firstUser.OrganizationID) - memberClient := codersdk.NewExperimentalClient(memberRawClient) - auditorRawClient, _ := coderdtest.CreateAnotherUser(t, ownerRawClient, firstUser.OrganizationID, rbac.RoleAuditor()) - auditorClient := codersdk.NewExperimentalClient(auditorRawClient) - ctx := testutil.Context(t, testutil.WaitMedium) - - _, err := memberClient.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ - Content: userSkillMarkdown("forbidden-skill", "Test skill", "Original body."), - }) - require.NoError(t, err) - - _, err = auditorClient.UpdateUserSkill(ctx, member.ID.String(), "forbidden-skill", codersdk.UpdateUserSkillRequest{ - Content: userSkillMarkdown("forbidden-skill", "Test skill", "Updated body."), - }) - requireSDKErrorStatus(t, err, http.StatusForbidden) +// skillOwnerAPI drives the personal or organization skill routes through the +// same calls, so behavior both owners share is asserted for each of them. +type skillOwnerAPI struct { + path string + resourceType database.ResourceType + auditOrgID uuid.UUID + client *codersdk.ExperimentalClient + create func(context.Context, codersdk.CreateSkillRequest) (codersdk.Skill, error) + list func(context.Context) ([]codersdk.SkillMetadata, error) + get func(context.Context, string) (codersdk.Skill, error) + update func(context.Context, string, codersdk.UpdateSkillRequest) (codersdk.Skill, error) + delete func(context.Context, string) error } -func TestUserSkillsCRUD(t *testing.T) { +func personalSkillAPI(client *codersdk.ExperimentalClient) skillOwnerAPI { + return skillOwnerAPI{ + path: "/api/experimental/users/me/skills", + resourceType: database.ResourceTypeUserSkill, + client: client, + create: func(ctx context.Context, req codersdk.CreateSkillRequest) (codersdk.Skill, error) { + return client.CreateUserSkill(ctx, codersdk.Me, req) + }, + list: func(ctx context.Context) ([]codersdk.SkillMetadata, error) { + return client.UserSkills(ctx, codersdk.Me) + }, + get: func(ctx context.Context, name string) (codersdk.Skill, error) { + return client.UserSkillByName(ctx, codersdk.Me, name) + }, + update: func(ctx context.Context, name string, req codersdk.UpdateSkillRequest) (codersdk.Skill, error) { + return client.UpdateUserSkill(ctx, codersdk.Me, name, req) + }, + delete: func(ctx context.Context, name string) error { return client.DeleteUserSkill(ctx, codersdk.Me, name) }, + } +} + +func organizationSkillAPI(client *codersdk.ExperimentalClient, orgID uuid.UUID) skillOwnerAPI { + return skillOwnerAPI{ + path: fmt.Sprintf("/api/experimental/organizations/%s/skills", orgID), + resourceType: database.ResourceTypeOrganizationSkill, + auditOrgID: orgID, + client: client, + create: func(ctx context.Context, req codersdk.CreateSkillRequest) (codersdk.Skill, error) { + return client.CreateOrganizationSkill(ctx, orgID, req) + }, + list: func(ctx context.Context) ([]codersdk.SkillMetadata, error) { + return client.OrganizationSkills(ctx, orgID) + }, + get: func(ctx context.Context, name string) (codersdk.Skill, error) { + return client.OrganizationSkillByName(ctx, orgID, name) + }, + update: func(ctx context.Context, name string, req codersdk.UpdateSkillRequest) (codersdk.Skill, error) { + return client.UpdateOrganizationSkill(ctx, orgID, name, req) + }, + delete: func(ctx context.Context, name string) error { return client.DeleteOrganizationSkill(ctx, orgID, name) }, + } +} + +// skillOwnerCases each start a deployment and return a caller that manages +// one owner's skills, plus a peer that manages another owner of the same kind. +var skillOwnerCases = []struct { + name string + setup func(t *testing.T, opts *coderdtest.Options) (owner, peer skillOwnerAPI) +}{ + { + name: "Personal", + setup: func(t *testing.T, opts *coderdtest.Options) (skillOwnerAPI, skillOwnerAPI) { + adminClient := coderdtest.New(t, opts) + firstUser := coderdtest.CreateFirstUser(t, adminClient) + ownerClient, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + peerClient, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) + return personalSkillAPI(codersdk.NewExperimentalClient(ownerClient)), personalSkillAPI(codersdk.NewExperimentalClient(peerClient)) + }, + }, + { + name: "OrgAdmin", + setup: func(t *testing.T, opts *coderdtest.Options) (skillOwnerAPI, skillOwnerAPI) { + ownerClient, db := coderdtest.NewWithDatabase(t, opts) + firstUser := coderdtest.CreateFirstUser(t, ownerClient) + orgAdminClient, _ := coderdtest.CreateAnotherUser(t, ownerClient, firstUser.OrganizationID, rbac.ScopedRoleOrgAdmin(firstUser.OrganizationID)) + peerOrg := dbgen.Organization(t, db, database.Organization{}) + return organizationSkillAPI(codersdk.NewExperimentalClient(orgAdminClient), firstUser.OrganizationID), + organizationSkillAPI(codersdk.NewExperimentalClient(ownerClient), peerOrg.ID) + }, + }, + { + name: "SiteOwner", + setup: func(t *testing.T, opts *coderdtest.Options) (skillOwnerAPI, skillOwnerAPI) { + ownerClient, db := coderdtest.NewWithDatabase(t, opts) + firstUser := coderdtest.CreateFirstUser(t, ownerClient) + client := codersdk.NewExperimentalClient(ownerClient) + peerOrg := dbgen.Organization(t, db, database.Organization{}) + return organizationSkillAPI(client, firstUser.OrganizationID), organizationSkillAPI(client, peerOrg.ID) + }, + }, +} + +func TestSkillsCRUD(t *testing.T) { t.Parallel() - adminClient := coderdtest.New(t, nil) - firstUser := coderdtest.CreateFirstUser(t, adminClient) - ownerClient, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) - owner := codersdk.NewExperimentalClient(ownerClient) - ctx := testutil.Context(t, testutil.WaitMedium) + for _, tc := range skillOwnerCases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() - emptyList, err := owner.UserSkills(ctx, codersdk.Me) - require.NoError(t, err) - assert.NotNil(t, emptyList) - assert.Empty(t, emptyList) + owner, _ := tc.setup(t, nil) + ctx := testutil.Context(t, testutil.WaitMedium) - emptyRes, err := owner.Request(ctx, http.MethodGet, "/api/experimental/users/me/skills", nil) - require.NoError(t, err) - defer emptyRes.Body.Close() - require.Equal(t, http.StatusOK, emptyRes.StatusCode) - var rawEmptyList []map[string]json.RawMessage - require.NoError(t, json.NewDecoder(emptyRes.Body).Decode(&rawEmptyList)) - assert.NotNil(t, rawEmptyList) - assert.Empty(t, rawEmptyList) + emptyList, err := owner.list(ctx) + require.NoError(t, err) + assert.NotNil(t, emptyList) + assert.Empty(t, emptyList) + rawEmptyList := requireRawSkillList(ctx, t, owner) + assert.NotNil(t, rawEmptyList) + assert.Empty(t, rawEmptyList) - content := userSkillMarkdown("crud-skill", "Initial description", "Use this skill for CRUD tests.") - created, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{Content: content}) - require.NoError(t, err) - assert.NotZero(t, created.ID) - assert.Equal(t, "crud-skill", created.Name) - assert.Equal(t, "Initial description", created.Description) - assert.Equal(t, content, created.Content) - assert.NotZero(t, created.CreatedAt) - assert.NotZero(t, created.UpdatedAt) + content := userSkillMarkdown("crud-skill", "Initial description", "Use this skill for CRUD tests.") + created, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: content}) + require.NoError(t, err) + assert.NotZero(t, created.ID) + assert.Equal(t, "crud-skill", created.Name) + assert.Equal(t, "Initial description", created.Description) + assert.Equal(t, content, created.Content) + assert.True(t, created.Enabled) + assert.NotZero(t, created.CreatedAt) + assert.NotZero(t, created.UpdatedAt) - list, err := owner.UserSkills(ctx, codersdk.Me) - require.NoError(t, err) - require.Len(t, list, 1) - assert.Equal(t, created.ID, list[0].ID) - assert.Equal(t, "crud-skill", list[0].Name) - assert.Equal(t, "Initial description", list[0].Description) + list, err := owner.list(ctx) + require.NoError(t, err) + require.Len(t, list, 1) + assert.Equal(t, created.ID, list[0].ID) + assert.Equal(t, "crud-skill", list[0].Name) + assert.Equal(t, "Initial description", list[0].Description) + assert.True(t, list[0].Enabled) + rawList := requireRawSkillList(ctx, t, owner) + require.Len(t, rawList, 1) + assert.NotContains(t, rawList[0], "content") - res, err := owner.Request(ctx, http.MethodGet, "/api/experimental/users/me/skills", nil) + got, err := owner.get(ctx, "crud-skill") + require.NoError(t, err) + assert.Equal(t, created.ID, got.ID) + assert.Equal(t, content, got.Content) + + updatedContent := userSkillMarkdown("crud-skill", "Updated description", "Updated body.") + updated, err := owner.update(ctx, "crud-skill", codersdk.UpdateSkillRequest{Content: ptr.Ref(updatedContent)}) + require.NoError(t, err) + assert.Equal(t, created.ID, updated.ID) + assert.Equal(t, "Updated description", updated.Description) + assert.Equal(t, updatedContent, updated.Content) + assert.True(t, updated.Enabled) + + disabled, err := owner.update(ctx, "crud-skill", codersdk.UpdateSkillRequest{Enabled: ptr.Ref(false)}) + require.NoError(t, err) + assert.False(t, disabled.Enabled) + assert.Equal(t, updatedContent, disabled.Content) + assert.Equal(t, "Updated description", disabled.Description) + list, err = owner.list(ctx) + require.NoError(t, err) + require.Len(t, list, 1) + assert.False(t, list[0].Enabled) + got, err = owner.get(ctx, "crud-skill") + require.NoError(t, err) + assert.False(t, got.Enabled) + + require.NoError(t, owner.delete(ctx, "crud-skill")) + _, err = owner.get(ctx, "crud-skill") + requireSDKErrorStatus(t, err, http.StatusNotFound) + }) + } +} + +func requireRawSkillList(ctx context.Context, t *testing.T, owner skillOwnerAPI) []map[string]json.RawMessage { + t.Helper() + res, err := owner.client.Request(ctx, http.MethodGet, owner.path, nil) require.NoError(t, err) defer res.Body.Close() require.Equal(t, http.StatusOK, res.StatusCode) - var rawList []map[string]json.RawMessage - require.NoError(t, json.NewDecoder(res.Body).Decode(&rawList)) - require.Len(t, rawList, 1) - assert.NotContains(t, rawList[0], "content") - - got, err := owner.UserSkillByName(ctx, codersdk.Me, "crud-skill") - require.NoError(t, err) - assert.Equal(t, created.ID, got.ID) - assert.Equal(t, content, got.Content) - - updatedContent := userSkillMarkdown("crud-skill", "Updated description", "Updated body.") - updated, err := owner.UpdateUserSkill(ctx, codersdk.Me, "crud-skill", codersdk.UpdateUserSkillRequest{Content: updatedContent}) - require.NoError(t, err) - assert.Equal(t, created.ID, updated.ID) - assert.Equal(t, "Updated description", updated.Description) - assert.Equal(t, updatedContent, updated.Content) - - require.NoError(t, owner.DeleteUserSkill(ctx, codersdk.Me, "crud-skill")) - _, err = owner.UserSkillByName(ctx, codersdk.Me, "crud-skill") - requireSDKErrorStatus(t, err, http.StatusNotFound) + var raw []map[string]json.RawMessage + require.NoError(t, json.NewDecoder(res.Body).Decode(&raw)) + return raw } -func TestUserSkillValidationAndConflicts(t *testing.T) { +func TestSkillValidationAndConflicts(t *testing.T) { t.Parallel() - adminClient := coderdtest.New(t, nil) - firstUser := coderdtest.CreateFirstUser(t, adminClient) - ownerClient, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) - otherClient, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) - owner := codersdk.NewExperimentalClient(ownerClient) - other := codersdk.NewExperimentalClient(otherClient) - - tests := []struct { - name string - content string - expectedMessage string - }{ - { - name: "MissingFrontmatterDelimiters", - content: "name: missing-frontmatter\n\nBody.", - expectedMessage: "Invalid skill content.", - }, - { - name: "MissingName", - content: "---\n" + - "description: Missing name\n" + - "---\n\nBody.", - expectedMessage: "Invalid skill name.", - }, - { - name: "NonKebabCaseName", - content: userSkillMarkdown("NotKebab", "Invalid", "Body."), - expectedMessage: "Invalid skill name.", - }, - { - name: "NameTooLong", - content: userSkillMarkdown(strings.Repeat("a", skills.MaxPersonalSkillNameBytes+1), "Invalid", "Body."), - expectedMessage: "Invalid skill name.", - }, - { - name: "EmptyBody", - content: userSkillMarkdown("empty-body", "Invalid", " \n"), - expectedMessage: "Skill body is required.", - }, - { - name: "TooLarge", - content: strings.Repeat("a", skills.MaxPersonalSkillSizeBytes+1), - expectedMessage: "Skill content is too large.", - }, - } - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { + for _, tc := range skillOwnerCases { + t.Run(tc.name, func(t *testing.T) { t.Parallel() - subCtx := testutil.Context(t, testutil.WaitMedium) - _, err := owner.CreateUserSkill(subCtx, codersdk.Me, codersdk.CreateUserSkillRequest{Content: tt.content}) - sdkErr := requireSDKErrorStatus(t, err, http.StatusBadRequest) - assert.Equal(t, tt.expectedMessage, sdkErr.Message) + owner, peer := tc.setup(t, nil) + tests := []struct { + name string + content string + expectedMessage string + }{ + { + name: "MissingFrontmatterDelimiters", + content: "name: missing-frontmatter\n\nBody.", + expectedMessage: "Invalid skill content.", + }, + { + name: "MissingName", + content: "---\n" + + "description: Missing name\n" + + "---\n\nBody.", + expectedMessage: "Invalid skill name.", + }, + { + name: "NonKebabCaseName", + content: userSkillMarkdown("NotKebab", "Invalid", "Body."), + expectedMessage: "Invalid skill name.", + }, + { + name: "NameTooLong", + content: userSkillMarkdown(strings.Repeat("a", skills.MaxPersonalSkillNameBytes+1), "Invalid", "Body."), + expectedMessage: "Invalid skill name.", + }, + { + name: "EmptyBody", + content: userSkillMarkdown("empty-body", "Invalid", " \n"), + expectedMessage: "Skill body is required.", + }, + { + name: "TooLarge", + content: strings.Repeat("a", skills.MaxPersonalSkillSizeBytes+1), + expectedMessage: "Skill content is too large.", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + _, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: tt.content}) + sdkErr := requireSDKErrorStatus(t, err, http.StatusBadRequest) + assert.Equal(t, tt.expectedMessage, sdkErr.Message) + }) + } + + t.Run("PatchEmptyBody", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + _, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: userSkillMarkdown("patch-validation", "Valid", "Body.")}) + require.NoError(t, err) + _, err = owner.update(ctx, "patch-validation", codersdk.UpdateSkillRequest{ + Content: ptr.Ref(userSkillMarkdown("patch-validation", "Invalid", " \n")), + }) + sdkErr := requireSDKErrorStatus(t, err, http.StatusBadRequest) + assert.Equal(t, "Skill body is required.", sdkErr.Message) + }) + + t.Run("PatchNoFields", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + _, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: userSkillMarkdown("patch-no-fields", "Valid", "Body.")}) + require.NoError(t, err) + _, err = owner.update(ctx, "patch-no-fields", codersdk.UpdateSkillRequest{}) + sdkErr := requireSDKErrorStatus(t, err, http.StatusBadRequest) + assert.Equal(t, "No skill fields to update.", sdkErr.Message) + }) + + t.Run("PatchNameMismatch", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + _, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: userSkillMarkdown("old-name", "Old", "Body.")}) + require.NoError(t, err) + _, err = owner.update(ctx, "old-name", codersdk.UpdateSkillRequest{ + Content: ptr.Ref(userSkillMarkdown("new-name", "New", "Body.")), + }) + sdkErr := requireSDKErrorStatus(t, err, http.StatusBadRequest) + assert.Equal(t, "Skill name in path does not match frontmatter name.", sdkErr.Message) + assert.Equal(t, `path has "old-name", frontmatter has "new-name"`, sdkErr.Detail) + }) + + t.Run("MissingSkill", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + _, err := owner.get(ctx, "missing-skill") + requireSDKErrorStatus(t, err, http.StatusNotFound) + _, err = owner.update(ctx, "missing-skill", codersdk.UpdateSkillRequest{ + Content: ptr.Ref(userSkillMarkdown("missing-skill", "Missing", "Body.")), + }) + requireSDKErrorStatus(t, err, http.StatusNotFound) + err = owner.delete(ctx, "missing-skill") + requireSDKErrorStatus(t, err, http.StatusNotFound) + }) + + t.Run("SameNameConflictsOnlyWithinOwner", func(t *testing.T) { + t.Parallel() + + ctx := testutil.Context(t, testutil.WaitMedium) + sharedContent := userSkillMarkdown("shared-skill", "Shared", "Shared body.") + _, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: sharedContent}) + require.NoError(t, err) + _, err = owner.create(ctx, codersdk.CreateSkillRequest{Content: sharedContent}) + sdkErr := requireSDKErrorStatus(t, err, http.StatusConflict) + assert.Equal(t, "A skill with that name already exists.", sdkErr.Message) + _, err = peer.create(ctx, codersdk.CreateSkillRequest{Content: sharedContent}) + require.NoError(t, err) + }) }) } - - t.Run("PatchEmptyBody", func(t *testing.T) { - t.Parallel() - - subCtx := testutil.Context(t, testutil.WaitMedium) - patchValidationContent := userSkillMarkdown("patch-validation", "Valid", "Body.") - _, err := owner.CreateUserSkill(subCtx, codersdk.Me, codersdk.CreateUserSkillRequest{Content: patchValidationContent}) - require.NoError(t, err) - _, err = owner.UpdateUserSkill(subCtx, codersdk.Me, "patch-validation", codersdk.UpdateUserSkillRequest{ - Content: userSkillMarkdown("patch-validation", "Invalid", " \n"), - }) - sdkErr := requireSDKErrorStatus(t, err, http.StatusBadRequest) - assert.Equal(t, "Skill body is required.", sdkErr.Message) - }) - - t.Run("DuplicateNameConflict", func(t *testing.T) { - t.Parallel() - - subCtx := testutil.Context(t, testutil.WaitMedium) - sharedContent := userSkillMarkdown("shared-skill", "Shared", "Shared body.") - _, err := owner.CreateUserSkill(subCtx, codersdk.Me, codersdk.CreateUserSkillRequest{Content: sharedContent}) - require.NoError(t, err) - _, err = owner.CreateUserSkill(subCtx, codersdk.Me, codersdk.CreateUserSkillRequest{Content: sharedContent}) - requireSDKErrorStatus(t, err, http.StatusConflict) - }) - - t.Run("CrossUserSameNameAllowed", func(t *testing.T) { - t.Parallel() - - subCtx := testutil.Context(t, testutil.WaitMedium) - sharedContent := userSkillMarkdown("shared-skill", "Shared", "Shared body.") - _, err := other.CreateUserSkill(subCtx, codersdk.Me, codersdk.CreateUserSkillRequest{Content: sharedContent}) - require.NoError(t, err) - }) } func TestUserSkillLimit(t *testing.T) { @@ -216,13 +343,13 @@ func TestUserSkillLimit(t *testing.T) { for i := range skills.MaxPersonalSkillsPerUser { name := fmt.Sprintf("limit-skill-%03d", i) - _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ + _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateSkillRequest{ Content: userSkillMarkdown(name, "Limit", "Body."), }) require.NoError(t, err) } - _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ + _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateSkillRequest{ Content: userSkillMarkdown("limit-skill-overflow", "Limit", "Body."), }) sdkErr := requireSDKErrorStatus(t, err, http.StatusConflict) @@ -244,7 +371,7 @@ func TestUserSkillLimitConcurrentCreates(t *testing.T) { for i := range skills.MaxPersonalSkillsPerUser - 1 { name := fmt.Sprintf("concurrent-limit-skill-%03d", i) - _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ + _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateSkillRequest{ Content: userSkillMarkdown(name, "Limit", "Body."), }) require.NoError(t, err) @@ -257,7 +384,7 @@ func TestUserSkillLimitConcurrentCreates(t *testing.T) { go func() { <-start name := fmt.Sprintf("concurrent-limit-overflow-%03d", i) - _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ + _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateSkillRequest{ Content: userSkillMarkdown(name, "Limit", "Body."), }) results <- err @@ -297,49 +424,17 @@ func TestUserSkillRequestAllowsEscapedMaxSizeContent(t *testing.T) { content := prefix + strings.Repeat(`"`, bodyLen) + suffix require.Len(t, []byte(content), skills.MaxPersonalSkillSizeBytes) - raw, err := json.Marshal(codersdk.CreateUserSkillRequest{Content: content}) + raw, err := json.Marshal(codersdk.CreateSkillRequest{Content: content}) require.NoError(t, err) require.Greater(t, len(raw), skills.MaxPersonalSkillSizeBytes+1024) - created, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ + created, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateSkillRequest{ Content: content, }) require.NoError(t, err) assert.Equal(t, "escaped-limit-skill", created.Name) } -func TestUserSkillMissingAndUpdateMismatch(t *testing.T) { - t.Parallel() - - adminClient := coderdtest.New(t, nil) - firstUser := coderdtest.CreateFirstUser(t, adminClient) - ownerClient, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) - owner := codersdk.NewExperimentalClient(ownerClient) - ctx := testutil.Context(t, testutil.WaitMedium) - - _, err := owner.UserSkillByName(ctx, codersdk.Me, "missing-skill") - requireSDKErrorStatus(t, err, http.StatusNotFound) - - _, err = owner.UpdateUserSkill(ctx, codersdk.Me, "missing-skill", codersdk.UpdateUserSkillRequest{ - Content: userSkillMarkdown("missing-skill", "Missing", "Body."), - }) - requireSDKErrorStatus(t, err, http.StatusNotFound) - - err = owner.DeleteUserSkill(ctx, codersdk.Me, "missing-skill") - requireSDKErrorStatus(t, err, http.StatusNotFound) - - _, err = owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ - Content: userSkillMarkdown("old-name", "Old", "Body."), - }) - require.NoError(t, err) - _, err = owner.UpdateUserSkill(ctx, codersdk.Me, "old-name", codersdk.UpdateUserSkillRequest{ - Content: userSkillMarkdown("new-name", "New", "Body."), - }) - sdkErr := requireSDKErrorStatus(t, err, http.StatusBadRequest) - assert.Equal(t, "Skill name in path does not match frontmatter name.", sdkErr.Message) - assert.Equal(t, `path has "old-name", frontmatter has "new-name"`, sdkErr.Detail) -} - func TestUserSkillAuthorization(t *testing.T) { t.Parallel() @@ -355,7 +450,7 @@ func TestUserSkillAuthorization(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) targetUser := ownerUser.Username - _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ + _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateSkillRequest{ Content: userSkillMarkdown("auth-skill", "Auth", "Body."), }) require.NoError(t, err) @@ -364,12 +459,12 @@ func TestUserSkillAuthorization(t *testing.T) { requireSDKErrorStatus(t, err, http.StatusNotFound) _, err = other.UserSkillByName(ctx, targetUser, "auth-skill") requireSDKErrorStatus(t, err, http.StatusNotFound) - _, err = other.CreateUserSkill(ctx, targetUser, codersdk.CreateUserSkillRequest{ + _, err = other.CreateUserSkill(ctx, targetUser, codersdk.CreateSkillRequest{ Content: userSkillMarkdown("denied-create", "Denied", "Body."), }) requireSDKErrorStatus(t, err, http.StatusNotFound) - _, err = other.UpdateUserSkill(ctx, targetUser, "auth-skill", codersdk.UpdateUserSkillRequest{ - Content: userSkillMarkdown("auth-skill", "Denied", "Body."), + _, err = other.UpdateUserSkill(ctx, targetUser, "auth-skill", codersdk.UpdateSkillRequest{ + Content: ptr.Ref(userSkillMarkdown("auth-skill", "Denied", "Body.")), }) requireSDKErrorStatus(t, err, http.StatusNotFound) err = other.DeleteUserSkill(ctx, targetUser, "auth-skill") @@ -379,18 +474,18 @@ func TestUserSkillAuthorization(t *testing.T) { requireSDKErrorStatus(t, err, http.StatusNotFound) _, err = userAdmin.UserSkillByName(ctx, targetUser, "auth-skill") requireSDKErrorStatus(t, err, http.StatusNotFound) - _, err = userAdmin.CreateUserSkill(ctx, targetUser, codersdk.CreateUserSkillRequest{ + _, err = userAdmin.CreateUserSkill(ctx, targetUser, codersdk.CreateSkillRequest{ Content: userSkillMarkdown("denied-admin-create", "Denied", "Body."), }) requireSDKErrorStatus(t, err, http.StatusForbidden) - _, err = userAdmin.UpdateUserSkill(ctx, targetUser, "auth-skill", codersdk.UpdateUserSkillRequest{ - Content: userSkillMarkdown("auth-skill", "Denied", "Body."), + _, err = userAdmin.UpdateUserSkill(ctx, targetUser, "auth-skill", codersdk.UpdateSkillRequest{ + Content: ptr.Ref(userSkillMarkdown("auth-skill", "Denied", "Body.")), }) requireSDKErrorStatus(t, err, http.StatusForbidden) err = userAdmin.DeleteUserSkill(ctx, targetUser, "auth-skill") requireSDKErrorStatus(t, err, http.StatusNotFound) - _, err = admin.CreateUserSkill(ctx, targetUser, codersdk.CreateUserSkillRequest{ + _, err = admin.CreateUserSkill(ctx, targetUser, codersdk.CreateSkillRequest{ Content: userSkillMarkdown("admin-created", "Admin create", "Created by admin."), }) requireSDKErrorStatus(t, err, http.StatusForbidden) @@ -402,8 +497,8 @@ func TestUserSkillAuthorization(t *testing.T) { got, err := admin.UserSkillByName(ctx, targetUser, "auth-skill") require.NoError(t, err) assert.Equal(t, "auth-skill", got.Name) - _, err = admin.UpdateUserSkill(ctx, targetUser, "auth-skill", codersdk.UpdateUserSkillRequest{ - Content: userSkillMarkdown("auth-skill", "Admin update", "Updated by admin."), + _, err = admin.UpdateUserSkill(ctx, targetUser, "auth-skill", codersdk.UpdateSkillRequest{ + Content: ptr.Ref(userSkillMarkdown("auth-skill", "Admin update", "Updated by admin.")), }) requireSDKErrorStatus(t, err, http.StatusForbidden) require.NoError(t, admin.DeleteUserSkill(ctx, targetUser, "auth-skill")) @@ -418,7 +513,7 @@ func TestUserSkillSoftDeleteCleanup(t *testing.T) { owner := codersdk.NewExperimentalClient(ownerClient) ctx := testutil.Context(t, testutil.WaitMedium) - _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ + _, err := owner.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateSkillRequest{ Content: userSkillMarkdown("soft-delete-skill", "Soft delete", "Body."), }) require.NoError(t, err) @@ -546,118 +641,101 @@ func TestUserSkillSchemaConstants(t *testing.T) { } } -//nolint:paralleltest,tparallel // Subtests share one auditor and run sequentially. -func TestUserSkillAudit(t *testing.T) { +func TestSkillAudit(t *testing.T) { t.Parallel() - auditor := audit.NewMock() - adminClient := coderdtest.New(t, &coderdtest.Options{Auditor: auditor}) - firstUser := coderdtest.CreateFirstUser(t, adminClient) - memberClient, _ := coderdtest.CreateAnotherUser(t, adminClient, firstUser.OrganizationID) - member := codersdk.NewExperimentalClient(memberClient) - ctx := testutil.Context(t, testutil.WaitMedium) - auditor.ResetLogs() + for _, tc := range skillOwnerCases { + //nolint:paralleltest,tparallel // Subtests share one auditor and run sequentially. + t.Run(tc.name, func(t *testing.T) { + t.Parallel() - genName := func(t *testing.T) string { - return strings.ToLower(strings.ReplaceAll(t.Name(), "/", "-")) + auditor := audit.NewMock() + owner, _ := tc.setup(t, &coderdtest.Options{Auditor: auditor}) + ctx := testutil.Context(t, testutil.WaitMedium) + auditor.ResetLogs() + + genName := func(t *testing.T) string { + return strings.ToLower(strings.ReplaceAll(t.Name(), "/", "-")) + } + requireLog := func(t *testing.T, log database.AuditLog, action database.AuditAction, status int, skill codersdk.Skill) { + t.Helper() + assert.Equal(t, action, log.Action) + assert.EqualValues(t, status, log.StatusCode) + assert.Equal(t, owner.resourceType, log.ResourceType) + assert.Equal(t, owner.auditOrgID, log.OrganizationID) + assert.Equal(t, skill.ID, log.ResourceID) + assert.Equal(t, skill.Name, log.ResourceTarget) + } + + t.Run("CreateEmitsLog", func(t *testing.T) { + auditor.ResetLogs() + skill, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: userSkillMarkdown(genName(t), "Audit", "Body.")}) + require.NoError(t, err) + + logs := auditor.AuditLogs() + require.Len(t, logs, 1) + requireLog(t, logs[0], database.AuditActionCreate, http.StatusCreated, skill) + }) + + t.Run("UpdateEmitsLog", func(t *testing.T) { + name := genName(t) + skill, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: userSkillMarkdown(name, "Initial", "Body.")}) + require.NoError(t, err) + auditor.ResetLogs() + _, err = owner.update(ctx, name, codersdk.UpdateSkillRequest{ + Content: ptr.Ref(userSkillMarkdown(name, "Updated", "Updated body.")), + }) + require.NoError(t, err) + + logs := auditor.AuditLogs() + require.Len(t, logs, 1) + requireLog(t, logs[0], database.AuditActionWrite, http.StatusOK, skill) + }) + + t.Run("DeleteEmitsLog", func(t *testing.T) { + name := genName(t) + skill, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: userSkillMarkdown(name, "Delete", "Body.")}) + require.NoError(t, err) + auditor.ResetLogs() + require.NoError(t, owner.delete(ctx, name)) + + logs := auditor.AuditLogs() + require.Len(t, logs, 1) + requireLog(t, logs[0], database.AuditActionDelete, http.StatusNoContent, skill) + }) + + t.Run("ReadsDoNotEmitLogs", func(t *testing.T) { + name := genName(t) + _, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: userSkillMarkdown(name, "Read", "Body.")}) + require.NoError(t, err) + auditor.ResetLogs() + + _, err = owner.list(ctx) + require.NoError(t, err) + _, err = owner.get(ctx, name) + require.NoError(t, err) + assert.Empty(t, auditor.AuditLogs()) + }) + + t.Run("ValidationFailureDoesNotEmitLog", func(t *testing.T) { + auditor.ResetLogs() + _, err := owner.create(ctx, codersdk.CreateSkillRequest{Content: userSkillMarkdown("bad-name", "Invalid", " \n")}) + requireSDKErrorStatus(t, err, http.StatusBadRequest) + assert.Empty(t, auditor.AuditLogs()) + }) + + t.Run("MissingSkillFailuresDoNotEmitLogs", func(t *testing.T) { + auditor.ResetLogs() + _, err := owner.update(ctx, "missing-audit-skill", codersdk.UpdateSkillRequest{ + Content: ptr.Ref(userSkillMarkdown("missing-audit-skill", "Missing", "Body.")), + }) + requireSDKErrorStatus(t, err, http.StatusNotFound) + err = owner.delete(ctx, "missing-audit-skill") + requireSDKErrorStatus(t, err, http.StatusNotFound) + assert.Empty(t, auditor.AuditLogs()) + }) + }) } - - t.Run("CreateEmitsLog", func(t *testing.T) { - auditor.ResetLogs() - name := genName(t) - - skill, err := member.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ - Content: userSkillMarkdown(name, "Audit", "Body."), - }) - require.NoError(t, err) - - logs := auditor.AuditLogs() - require.Len(t, logs, 1) - assert.Equal(t, database.AuditActionCreate, logs[0].Action) - assert.Equal(t, skill.ID, logs[0].ResourceID) - assert.Equal(t, skill.Name, logs[0].ResourceTarget) - assert.EqualValues(t, http.StatusCreated, logs[0].StatusCode) - }) - - t.Run("UpdateEmitsLog", func(t *testing.T) { - auditor.ResetLogs() - name := genName(t) - - skill, err := member.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ - Content: userSkillMarkdown(name, "Initial", "Body."), - }) - require.NoError(t, err) - _, err = member.UpdateUserSkill(ctx, codersdk.Me, name, codersdk.UpdateUserSkillRequest{ - Content: userSkillMarkdown(name, "Updated", "Updated body."), - }) - require.NoError(t, err) - - logs := auditor.AuditLogs() - require.Len(t, logs, 2) - assert.Equal(t, database.AuditActionCreate, logs[0].Action) - assert.Equal(t, database.AuditActionWrite, logs[1].Action) - assert.Equal(t, skill.ID, logs[1].ResourceID) - assert.Equal(t, skill.Name, logs[1].ResourceTarget) - assert.EqualValues(t, http.StatusOK, logs[1].StatusCode) - }) - - t.Run("DeleteEmitsLog", func(t *testing.T) { - auditor.ResetLogs() - name := genName(t) - - skill, err := member.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ - Content: userSkillMarkdown(name, "Delete", "Body."), - }) - require.NoError(t, err) - require.NoError(t, member.DeleteUserSkill(ctx, codersdk.Me, name)) - - logs := auditor.AuditLogs() - require.Len(t, logs, 2) - assert.Equal(t, database.AuditActionCreate, logs[0].Action) - assert.Equal(t, database.AuditActionDelete, logs[1].Action) - assert.Equal(t, skill.ID, logs[1].ResourceID) - assert.Equal(t, skill.Name, logs[1].ResourceTarget) - assert.EqualValues(t, http.StatusNoContent, logs[1].StatusCode) - }) - - t.Run("ReadsDoNotEmitLogs", func(t *testing.T) { - auditor.ResetLogs() - name := genName(t) - - _, err := member.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ - Content: userSkillMarkdown(name, "Read", "Body."), - }) - require.NoError(t, err) - auditor.ResetLogs() - - _, err = member.UserSkills(ctx, codersdk.Me) - require.NoError(t, err) - _, err = member.UserSkillByName(ctx, codersdk.Me, name) - require.NoError(t, err) - assert.Empty(t, auditor.AuditLogs()) - }) - - t.Run("ValidationFailureDoesNotEmitLog", func(t *testing.T) { - auditor.ResetLogs() - - _, err := member.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ - Content: userSkillMarkdown("bad-name", "Invalid", " \n"), - }) - requireSDKErrorStatus(t, err, http.StatusBadRequest) - assert.Empty(t, auditor.AuditLogs()) - }) - - t.Run("MissingSkillFailuresDoNotEmitLogs", func(t *testing.T) { - auditor.ResetLogs() - - _, err := member.UpdateUserSkill(ctx, codersdk.Me, "missing-audit-skill", codersdk.UpdateUserSkillRequest{ - Content: userSkillMarkdown("missing-audit-skill", "Missing", "Body."), - }) - requireSDKErrorStatus(t, err, http.StatusNotFound) - err = member.DeleteUserSkill(ctx, codersdk.Me, "missing-audit-skill") - requireSDKErrorStatus(t, err, http.StatusNotFound) - assert.Empty(t, auditor.AuditLogs()) - }) } func userSkillMarkdown(name string, description string, body string) string { diff --git a/codersdk/skills.go b/codersdk/skills.go new file mode 100644 index 0000000000..f78fd5bbe7 --- /dev/null +++ b/codersdk/skills.go @@ -0,0 +1,204 @@ +package codersdk + +import ( + "context" + "fmt" + "net/http" + "net/url" + "time" + + "github.com/google/uuid" +) + +// SkillMetadata represents a personal or organization skill without its raw +// Markdown content. +type SkillMetadata struct { + ID uuid.UUID `json:"id" format:"uuid"` + Name string `json:"name"` + Description string `json:"description"` + Enabled bool `json:"enabled"` + CreatedAt time.Time `json:"created_at" format:"date-time"` + UpdatedAt time.Time `json:"updated_at" format:"date-time"` +} + +// Skill represents a personal or organization skill with its raw Markdown +// content. +type Skill struct { + SkillMetadata + Content string `json:"content"` +} + +// CreateSkillRequest is the payload for creating a skill. +type CreateSkillRequest struct { + // Content must be SKILL.md-format Markdown with YAML frontmatter. The + // frontmatter must include name, may include description, and must be + // followed by a non-empty body. + Content string `json:"content"` +} + +// UpdateSkillRequest is the payload for updating a skill. At least one field +// must be set. +type UpdateSkillRequest struct { + // Content must be SKILL.md-format Markdown with YAML frontmatter. The + // frontmatter must include name, may include description, and must be + // followed by a non-empty body. + Content *string `json:"content,omitempty"` + Enabled *bool `json:"enabled,omitempty"` +} + +func userSkillsPath(user string) string { + return fmt.Sprintf("/api/experimental/users/%s/skills", url.PathEscape(user)) +} + +func userSkillPath(user string, name string) string { + return fmt.Sprintf("%s/%s", userSkillsPath(user), url.PathEscape(name)) +} + +// CreateUserSkill creates a user skill from raw Markdown content. +func (c *ExperimentalClient) CreateUserSkill(ctx context.Context, user string, req CreateSkillRequest) (Skill, error) { + res, err := c.Request(ctx, http.MethodPost, userSkillsPath(user), req) + if err != nil { + return Skill{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusCreated { + return Skill{}, ReadBodyAsError(res) + } + var skill Skill + return skill, ReadBodyAsJSON(res, &skill) +} + +// UserSkills lists user skill metadata for the specified user. +func (c *ExperimentalClient) UserSkills(ctx context.Context, user string) ([]SkillMetadata, error) { + res, err := c.Request(ctx, http.MethodGet, userSkillsPath(user), nil) + if err != nil { + return nil, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return nil, ReadBodyAsError(res) + } + var skills []SkillMetadata + return skills, ReadBodyAsJSON(res, &skills) +} + +// UserSkillByName returns a user skill by name. +func (c *ExperimentalClient) UserSkillByName(ctx context.Context, user string, name string) (Skill, error) { + res, err := c.Request(ctx, http.MethodGet, userSkillPath(user, name), nil) + if err != nil { + return Skill{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return Skill{}, ReadBodyAsError(res) + } + var skill Skill + return skill, ReadBodyAsJSON(res, &skill) +} + +// UpdateUserSkill updates a user skill's raw Markdown content or enabled state. +func (c *ExperimentalClient) UpdateUserSkill(ctx context.Context, user string, name string, req UpdateSkillRequest) (Skill, error) { + res, err := c.Request(ctx, http.MethodPatch, userSkillPath(user, name), req) + if err != nil { + return Skill{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return Skill{}, ReadBodyAsError(res) + } + var skill Skill + return skill, ReadBodyAsJSON(res, &skill) +} + +// DeleteUserSkill deletes a user skill by name. +func (c *ExperimentalClient) DeleteUserSkill(ctx context.Context, user string, name string) error { + res, err := c.Request(ctx, http.MethodDelete, userSkillPath(user, name), nil) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} + +func organizationSkillsPath(organizationID uuid.UUID) string { + return fmt.Sprintf("/api/experimental/organizations/%s/skills", organizationID) +} + +func organizationSkillPath(organizationID uuid.UUID, name string) string { + return fmt.Sprintf("%s/%s", organizationSkillsPath(organizationID), url.PathEscape(name)) +} + +// CreateOrganizationSkill creates an organization skill from raw Markdown +// content. +func (c *ExperimentalClient) CreateOrganizationSkill(ctx context.Context, organizationID uuid.UUID, req CreateSkillRequest) (Skill, error) { + res, err := c.Request(ctx, http.MethodPost, organizationSkillsPath(organizationID), req) + if err != nil { + return Skill{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusCreated { + return Skill{}, ReadBodyAsError(res) + } + var skill Skill + return skill, ReadBodyAsJSON(res, &skill) +} + +// OrganizationSkills lists the organization skill metadata the caller can +// read. +func (c *ExperimentalClient) OrganizationSkills(ctx context.Context, organizationID uuid.UUID) ([]SkillMetadata, error) { + res, err := c.Request(ctx, http.MethodGet, organizationSkillsPath(organizationID), nil) + if err != nil { + return nil, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return nil, ReadBodyAsError(res) + } + var skills []SkillMetadata + return skills, ReadBodyAsJSON(res, &skills) +} + +// OrganizationSkillByName returns an organization skill by name. +func (c *ExperimentalClient) OrganizationSkillByName(ctx context.Context, organizationID uuid.UUID, name string) (Skill, error) { + res, err := c.Request(ctx, http.MethodGet, organizationSkillPath(organizationID, name), nil) + if err != nil { + return Skill{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return Skill{}, ReadBodyAsError(res) + } + var skill Skill + return skill, ReadBodyAsJSON(res, &skill) +} + +// UpdateOrganizationSkill updates an organization skill's raw Markdown content +// or enabled state. +func (c *ExperimentalClient) UpdateOrganizationSkill(ctx context.Context, organizationID uuid.UUID, name string, req UpdateSkillRequest) (Skill, error) { + res, err := c.Request(ctx, http.MethodPatch, organizationSkillPath(organizationID, name), req) + if err != nil { + return Skill{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return Skill{}, ReadBodyAsError(res) + } + var skill Skill + return skill, ReadBodyAsJSON(res, &skill) +} + +// DeleteOrganizationSkill deletes an organization skill by name. +func (c *ExperimentalClient) DeleteOrganizationSkill(ctx context.Context, organizationID uuid.UUID, name string) error { + res, err := c.Request(ctx, http.MethodDelete, organizationSkillPath(organizationID, name), nil) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} diff --git a/codersdk/userskills.go b/codersdk/userskills.go deleted file mode 100644 index 5586b76231..0000000000 --- a/codersdk/userskills.go +++ /dev/null @@ -1,119 +0,0 @@ -package codersdk - -import ( - "context" - "fmt" - "net/http" - "net/url" - "time" - - "github.com/google/uuid" -) - -// UserSkillMetadata represents a user skill without its raw Markdown content. -type UserSkillMetadata struct { - ID uuid.UUID `json:"id" format:"uuid"` - Name string `json:"name"` - Description string `json:"description"` - CreatedAt time.Time `json:"created_at" format:"date-time"` - UpdatedAt time.Time `json:"updated_at" format:"date-time"` -} - -// UserSkill represents a user skill with its raw Markdown content. -type UserSkill struct { - UserSkillMetadata - Content string `json:"content"` -} - -// CreateUserSkillRequest is the payload for creating a user skill. -type CreateUserSkillRequest struct { - // Content must be SKILL.md-format Markdown with YAML frontmatter. The - // frontmatter must include name, may include description, and must be - // followed by a non-empty body. - Content string `json:"content"` -} - -// UpdateUserSkillRequest is the payload for updating a user skill. -type UpdateUserSkillRequest struct { - // Content must be SKILL.md-format Markdown with YAML frontmatter. The - // frontmatter must include name, may include description, and must be - // followed by a non-empty body. - Content string `json:"content"` -} - -func userSkillsPath(user string) string { - return fmt.Sprintf("/api/experimental/users/%s/skills", url.PathEscape(user)) -} - -func userSkillPath(user string, name string) string { - return fmt.Sprintf("%s/%s", userSkillsPath(user), url.PathEscape(name)) -} - -// CreateUserSkill creates a user skill from raw Markdown content. -func (c *ExperimentalClient) CreateUserSkill(ctx context.Context, user string, req CreateUserSkillRequest) (UserSkill, error) { - res, err := c.Request(ctx, http.MethodPost, userSkillsPath(user), req) - if err != nil { - return UserSkill{}, err - } - defer res.Body.Close() - if res.StatusCode != http.StatusCreated { - return UserSkill{}, ReadBodyAsError(res) - } - var skill UserSkill - return skill, ReadBodyAsJSON(res, &skill) -} - -// UserSkills lists user skill metadata for the specified user. -func (c *ExperimentalClient) UserSkills(ctx context.Context, user string) ([]UserSkillMetadata, error) { - res, err := c.Request(ctx, http.MethodGet, userSkillsPath(user), nil) - if err != nil { - return nil, err - } - defer res.Body.Close() - if res.StatusCode != http.StatusOK { - return nil, ReadBodyAsError(res) - } - var skills []UserSkillMetadata - return skills, ReadBodyAsJSON(res, &skills) -} - -// UserSkillByName returns a user skill by name. -func (c *ExperimentalClient) UserSkillByName(ctx context.Context, user string, name string) (UserSkill, error) { - res, err := c.Request(ctx, http.MethodGet, userSkillPath(user, name), nil) - if err != nil { - return UserSkill{}, err - } - defer res.Body.Close() - if res.StatusCode != http.StatusOK { - return UserSkill{}, ReadBodyAsError(res) - } - var skill UserSkill - return skill, ReadBodyAsJSON(res, &skill) -} - -// UpdateUserSkill replaces a user skill's raw Markdown content. -func (c *ExperimentalClient) UpdateUserSkill(ctx context.Context, user string, name string, req UpdateUserSkillRequest) (UserSkill, error) { - res, err := c.Request(ctx, http.MethodPatch, userSkillPath(user, name), req) - if err != nil { - return UserSkill{}, err - } - defer res.Body.Close() - if res.StatusCode != http.StatusOK { - return UserSkill{}, ReadBodyAsError(res) - } - var skill UserSkill - return skill, ReadBodyAsJSON(res, &skill) -} - -// DeleteUserSkill deletes a user skill by name. -func (c *ExperimentalClient) DeleteUserSkill(ctx context.Context, user string, name string) error { - res, err := c.Request(ctx, http.MethodDelete, userSkillPath(user, name), nil) - if err != nil { - return err - } - defer res.Body.Close() - if res.StatusCode != http.StatusNoContent { - return ReadBodyAsError(res) - } - return nil -} diff --git a/docs/admin/security/audit-logs.md b/docs/admin/security/audit-logs.md index 1cc325c637..1806cb74d2 100644 --- a/docs/admin/security/audit-logs.md +++ b/docs/admin/security/audit-logs.md @@ -189,6 +189,28 @@ Actions: `write`, `delete` +### AuditableOrganizationSkill + +Actions: `create`, `write`, `delete` + +
| Field | Tracked |
|---|---|
content | Yes |
created_at | No |
description | Yes |
enabled | Yes |
group_acl | Yes |
id | Yes |
name | Yes |
organization_id | Yes |
project_id | Yes |
updated_at | No |
user_acl | Yes |
user_id | Yes |