From db7f5a770354b9c4eb4908fe6214b3bcc92037c6 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:24:45 +0200 Subject: [PATCH] feat: add organization skill API routes (#30610) ## Stack Part of [CODAGT-1341](https://linear.app/codercom/issue/CODAGT-1341/support-organization-level-skills-in-coder-agents). This stack adds organization skills to Coder Agents: admins publish a skill once per organization, share it with groups or members, and chats in that organization load it next to personal and workspace skills. It replaces the 21-PR draft stack #30584 to #30604 with five PRs, bottom to top: 1. #30609 data model: one migration, queries, RBAC and dbauthz 2. **#30610** organization skill API routes and the scope-neutral SDK (this PR) 3. #30611 sharing ACL routes 4. #30612 settings UI: shared skills table, admin page, member section 5. #30613 chats: chatd loading, `/` menu, docs ## What changes This PR adds the organization skill CRUD routes and makes the skill SDK types owner-neutral so personal and organization skills share them. **API contract change (experimental API, personal and organization skills):** skill list and get responses gain `enabled`, and `PATCH .../skills/{skillName}` now takes `{content?, enabled?}`, so `content` is optional and a toggle doesn't resend it. A PATCH with neither field returns 400; `content`, when present, is validated as before. In the SDK, `UserSkill`, `UserSkillMetadata`, `CreateUserSkillRequest` and `UpdateUserSkillRequest` become `Skill`, `SkillMetadata`, `CreateSkillRequest` and `UpdateSkillRequest` (`codersdk/userskills.go` moves to `codersdk/skills.go`), and `UpdateSkillRequest.Content` is now `*string`. The personal client method names are unchanged. The site changes are the handwritten edits that rename forces (`api.ts`, `queries/userSkills.ts`, `testHelpers/skills.ts` and three call sites), with no UI change. Organization routes under `/api/experimental/organizations/{organization}/skills`, not gated by an experiment or license, like the personal ones: - `GET` lists with the caller's own permissions, so admins, the site owner and org auditors see every skill and members see only skills shared with them. `GET /{skillName}` goes through the new `httpmw.ExtractOrganizationSkillParam` and admits only read, so a missing skill and a denied one both return 404. - `POST` checks `create` on the organization before reading the body, then seeds `group_acl` with the Everyone group (whose ID is the organization ID) so a new skill reaches every member until an admin narrows it. The 100-skill cap and a duplicate name return 409. - `PATCH` admits only callers who can read the skill, because its response includes the content; `DELETE` admits callers who can read or delete it. A reader who can't change the skill gets a write-specific 403 from the database wrapper, and everyone else gets 404. PATCH locks the row as system, updates as the caller in the same transaction, and audits the locked row only after the transaction commits. - Audit: `database.AuditableOrganizationSkill` records resource type `organization_skill` with the organization ID; personal skills keep `user_skill`. Personal and organization handlers share one create and update path in `coderd/userskills.go` (`createSkill`, `updateSkill`, and a `skillWriteErrors` table per owner and action). ## Commits The first commit reproduces the reviewed code from the old stack; each later commit is a labeled trim or fix. - `49cb2c84276` feat: add organization skill API routes. Reproduction: every file this commit changes, including the SDK layer's site edits, equals the reviewed old `write-routes` head `aa597100f28`; the rest of the tree differs only by #30609's migration merge and main. - `dc893bc60d6` refactor(coderd): share the skill create and update path across owners (production +164/-188). It keeps the organization create check before the body read, the lock as system then update as caller order, and audit assignment after the transaction. No swagger, `codersdk` or `typesGenerated.ts` change; `make gen` is clean. - `f94925425ef` test(coderd): merge duplicate personal and organization skill tests. `TestSkillsCRUD`, `TestSkillValidationAndConflicts` and `TestSkillAudit` are table-driven over Personal, OrgAdmin and SiteOwner, alongside `TestOrganizationSkillAuditsDeniedWrites` and one `ScopedAPIKeys` subtest; `TestPatchUserSkill` is deleted because none of its blocks were unique. Coverage of `userskills.go`, `organizationskills.go` and `httpmw/organizationskillparam.go` is 101 of 117 blocks before and after, the same block set. - `ad0a7a37504` fix(coderd): require read access to patch an organization skill. The organization PATCH route now admits only `read`, because its response carries the skill's content: a key scoped to `organization_skill:update` without read gets 404, like GET, instead of 200 with the content. Readers without update still get 403, and DELETE is unchanged. `TestOrganizationSkillAccess/ScopedAPIKeys` asserts the 404, that the response has no content, and that the skill is unchanged; against the old route it fails with 200. - `69d00ded0b6` and `f67b79343ec` merge the updated #30609 layer, including main; `69d00ded0b6` resolves a conflict in the personal skills page with main. No other changes of their own. - `18373ffd59e` merges the updated #30609 layer (main at `e7b76872098`) and reapplies the `AuditableOrganizationSkill` entry in `enterprise/audit/table.go` alongside main's new `AuditableChatProjectInstructions` entries. ## Size Production +838/-308, tests +733/-322, generated +801/-274; total +2372/-904. This is above the 500-line reviewer-friendly limit on purpose: Mike asked for 4 to 5 PRs instead of the 21 small ones this stack replaces. ## Replaces #30587 (scope-neutral SDK and personal enabled state), #30588 (read routes), #30589 (write routes and audit). ## Validation - The coverage comparison above, before and after the test merge. - `make gen` leaves no diff after the shared create and update refactor. - On `18373ffd59e`: `make gen` is clean; build, vet, the `coderd` and `enterprise/coderd` skill tests, `rbac`, `httpmw`, `dbauthz` and `enterprise/audit` pass; `pnpm check`, `pnpm lint` and `make pre-commit` pass. - Remote UAT round 7 through Coder Agents passed on the whole stack; see #30613's Validation for scope and findings. > Xum opened this PR on behalf of @ibetitsmike. --- coderd/apidoc/docs.go | 376 +++++++-- coderd/apidoc/swagger.json | 354 +++++++-- coderd/audit/diff.go | 1 + coderd/audit/request.go | 8 + coderd/coderd.go | 19 + coderd/database/db2sdk/db2sdk.go | 52 +- coderd/database/dbauthz/dbauthz.go | 4 + coderd/database/dbauthz/dbauthz_test.go | 5 + coderd/database/dbmetrics/querymetrics.go | 8 + coderd/database/dbmock/dbmock.go | 15 + coderd/database/modelmethods.go | 7 + coderd/database/querier.go | 1 + coderd/database/queries.sql.go | 48 +- coderd/database/queries/skills.sql | 13 +- coderd/httpmw/organizationskillparam.go | 75 ++ coderd/organizationskills.go | 240 ++++++ coderd/organizationskills_test.go | 325 ++++++++ coderd/userskills.go | 330 ++++---- coderd/userskills_test.go | 712 ++++++++++-------- codersdk/skills.go | 204 +++++ codersdk/userskills.go | 119 --- docs/admin/security/audit-logs.md | 22 + docs/reference/api/schemas.md | 154 ++-- enterprise/audit/table.go | 15 + enterprise/coderd/userskills_audit_test.go | 7 +- site/src/api/api.ts | 22 +- site/src/api/queries/userSkills.test.ts | 11 +- site/src/api/queries/userSkills.ts | 33 +- site/src/api/typesGenerated.ts | 97 +-- .../AgentsPage/AgentChatPage.stories.tsx | 1 + .../AgentSettingsPersonalSkillsPage.tsx | 6 +- .../AgentSettingsPersonalSkillsPageView.tsx | 10 +- .../ChatMessageInput.stories.tsx | 4 +- .../ChatMessageInput/ChatMessageInput.tsx | 2 +- .../AgentsPage/utils/personalSkills.test.ts | 3 +- site/src/testHelpers/skills.ts | 7 +- 36 files changed, 2389 insertions(+), 921 deletions(-) create mode 100644 coderd/httpmw/organizationskillparam.go create mode 100644 coderd/organizationskills.go create mode 100644 coderd/organizationskills_test.go create mode 100644 codersdk/skills.go delete mode 100644 codersdk/userskills.go 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` + + + + + + + + + + + + + + + + + +
FieldTracked
contentYes
created_atNo
descriptionYes
enabledYes
group_aclYes
idYes
nameYes
organization_idYes
project_idYes
updated_atNo
user_aclYes
user_idYes
+ ### AuditableUserAIBudgetOverride Actions: `write`, `delete` diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index a45355f34c..269a22055e 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -7028,6 +7028,20 @@ AuthorizationObject can represent a "set" of objects, such as: all workspaces in |-------|--------|----------|--------------|-------------| | `key` | string | false | | | +## codersdk.CreateSkillRequest + +```json +{ + "content": "string" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|-----------|--------|----------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `content` | string | false | | 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. | + ## codersdk.CreateTemplateRequest ```json @@ -7341,20 +7355,6 @@ This is required on creation to enable a user-flow of validating a template work | `name` | string | false | | | | `value` | string | false | | | -## codersdk.CreateUserSkillRequest - -```json -{ - "content": "string" -} -``` - -### Properties - -| Name | Type | Required | Restrictions | Description | -|-----------|--------|----------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `content` | string | false | | 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. | - ## codersdk.CreateWorkspaceBuildOnSuccessRequest ```json @@ -14528,6 +14528,56 @@ Git clone makes use of this by parsing the URL from: 'Username for "https://gith |-----------------| | `group`, `user` | +## codersdk.Skill + +```json +{ + "content": "string", + "created_at": "2019-08-24T14:15:22Z", + "description": "string", + "enabled": true, + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string", + "updated_at": "2019-08-24T14:15:22Z" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|---------------|---------|----------|--------------|-------------| +| `content` | string | false | | | +| `created_at` | string | false | | | +| `description` | string | false | | | +| `enabled` | boolean | false | | | +| `id` | string | false | | | +| `name` | string | false | | | +| `updated_at` | string | false | | | + +## codersdk.SkillMetadata + +```json +{ + "created_at": "2019-08-24T14:15:22Z", + "description": "string", + "enabled": true, + "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", + "name": "string", + "updated_at": "2019-08-24T14:15:22Z" +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|---------------|---------|----------|--------------|-------------| +| `created_at` | string | false | | | +| `description` | string | false | | | +| `enabled` | boolean | false | | | +| `id` | string | false | | | +| `name` | string | false | | | +| `updated_at` | string | false | | | + ## codersdk.SlimRole ```json @@ -16859,6 +16909,22 @@ The cursor is owner-scoped, so only the chat owner may set this. Opening a chat' |---------|-----------------|----------|--------------|-------------| | `roles` | array of string | false | | | +## codersdk.UpdateSkillRequest + +```json +{ + "content": "string", + "enabled": true +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +|-----------|---------|----------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `content` | string | false | | 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. | +| `enabled` | boolean | false | | | + ## codersdk.UpdateTemplateACL ```json @@ -17160,20 +17226,6 @@ If the schedule is empty, the user will be updated to use the default schedule.| | `file_path` | string | false | | | | `value` | string | false | | | -## codersdk.UpdateUserSkillRequest - -```json -{ - "content": "string" -} -``` - -### Properties - -| Name | Type | Required | Restrictions | Description | -|-----------|--------|----------|--------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------| -| `content` | string | false | | 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. | - ## codersdk.UpdateWorkspaceACL ```json @@ -18042,52 +18094,6 @@ If the schedule is empty, the user will be updated to use the default schedule.| |------------------------------|---------|----------|--------------|-----------------------------------------------------------------------------------------------------------------------------------| | `file_path_delivery_enabled` | boolean | false | | File path delivery enabled reports whether Coder writes stored file paths into workspaces. Stored paths are preserved either way. | -## codersdk.UserSkill - -```json -{ - "content": "string", - "created_at": "2019-08-24T14:15:22Z", - "description": "string", - "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", - "name": "string", - "updated_at": "2019-08-24T14:15:22Z" -} -``` - -### Properties - -| Name | Type | Required | Restrictions | Description | -|---------------|--------|----------|--------------|-------------| -| `content` | string | false | | | -| `created_at` | string | false | | | -| `description` | string | false | | | -| `id` | string | false | | | -| `name` | string | false | | | -| `updated_at` | string | false | | | - -## codersdk.UserSkillMetadata - -```json -{ - "created_at": "2019-08-24T14:15:22Z", - "description": "string", - "id": "497f6eca-6276-4993-bfeb-53cbbbba6f08", - "name": "string", - "updated_at": "2019-08-24T14:15:22Z" -} -``` - -### Properties - -| Name | Type | Required | Restrictions | Description | -|---------------|--------|----------|--------------|-------------| -| `created_at` | string | false | | | -| `description` | string | false | | | -| `id` | string | false | | | -| `name` | string | false | | | -| `updated_at` | string | false | | | - ## codersdk.UserStatus ```json diff --git a/enterprise/audit/table.go b/enterprise/audit/table.go index 404620f40e..a87309f8c5 100644 --- a/enterprise/audit/table.go +++ b/enterprise/audit/table.go @@ -41,6 +41,7 @@ var AuditActionMap = map[string][]codersdk.AuditAction{ "MCPServerConfig": {codersdk.AuditActionCreate, codersdk.AuditActionWrite, codersdk.AuditActionDelete}, "UserSecret": {codersdk.AuditActionCreate, codersdk.AuditActionWrite, codersdk.AuditActionDelete}, "Skill": {codersdk.AuditActionCreate, codersdk.AuditActionWrite, codersdk.AuditActionDelete}, + "AuditableOrganizationSkill": {codersdk.AuditActionCreate, codersdk.AuditActionWrite, codersdk.AuditActionDelete}, "ChatInstructionSettings": {codersdk.AuditActionWrite}, "ChatOperationalSettings": {codersdk.AuditActionWrite}, "ChatOrganizationSystemPrompt": {codersdk.AuditActionWrite}, @@ -604,6 +605,20 @@ var auditableResourcesTypes = map[any]map[string]Action{ "created_at": ActionIgnore, "updated_at": ActionIgnore, }, + &database.AuditableOrganizationSkill{}: { + "id": ActionTrack, + "user_id": ActionTrack, + "organization_id": ActionTrack, + "project_id": ActionTrack, + "name": ActionTrack, + "description": ActionTrack, + "content": ActionTrack, + "enabled": ActionTrack, + "group_acl": ActionTrack, + "user_acl": ActionTrack, + "created_at": ActionIgnore, + "updated_at": ActionIgnore, + }, &database.ChatOperationalSettings{}: { "id": ActionIgnore, "chat_retention_days": ActionTrack, diff --git a/enterprise/coderd/userskills_audit_test.go b/enterprise/coderd/userskills_audit_test.go index 4394b28ac6..1dc4dcc9de 100644 --- a/enterprise/coderd/userskills_audit_test.go +++ b/enterprise/coderd/userskills_audit_test.go @@ -13,6 +13,7 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbtestutil" + "github.com/coder/coder/v2/coderd/util/ptr" "github.com/coder/coder/v2/codersdk" entaudit "github.com/coder/coder/v2/enterprise/audit" "github.com/coder/coder/v2/enterprise/audit/backends" @@ -51,14 +52,14 @@ func TestUserSkillAuditDiffTracksContent(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) initialContent := userSkillMarkdown("audit-tracking", "initial", "initial body") - skill, err := member.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateUserSkillRequest{ + skill, err := member.CreateUserSkill(ctx, codersdk.Me, codersdk.CreateSkillRequest{ Content: initialContent, }) require.NoError(t, err) newContent := userSkillMarkdown("audit-tracking", "after", "new body") - _, err = member.UpdateUserSkill(ctx, codersdk.Me, skill.Name, codersdk.UpdateUserSkillRequest{ - Content: newContent, + _, err = member.UpdateUserSkill(ctx, codersdk.Me, skill.Name, codersdk.UpdateSkillRequest{ + Content: ptr.Ref(newContent), }) require.NoError(t, err) diff --git a/site/src/api/api.ts b/site/src/api/api.ts index a3fd3b2e6b..5fdf20e62e 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -3852,19 +3852,17 @@ class ExperimentalApiMethods { createUserSkill = async ( user: string, - req: TypesGen.CreateUserSkillRequest, - ): Promise => { - const response = await this.axios.post( + req: TypesGen.CreateSkillRequest, + ): Promise => { + const response = await this.axios.post( userSkillsPath(user), req, ); return response.data; }; - getUserSkills = async ( - user: string, - ): Promise => { - const response = await this.axios.get( + getUserSkills = async (user: string): Promise => { + const response = await this.axios.get( userSkillsPath(user), ); return response.data; @@ -3873,8 +3871,8 @@ class ExperimentalApiMethods { getUserSkillByName = async ( user: string, name: string, - ): Promise => { - const response = await this.axios.get( + ): Promise => { + const response = await this.axios.get( userSkillPath(user, name), ); return response.data; @@ -3883,9 +3881,9 @@ class ExperimentalApiMethods { updateUserSkill = async ( user: string, name: string, - req: TypesGen.UpdateUserSkillRequest, - ): Promise => { - const response = await this.axios.patch( + req: TypesGen.UpdateSkillRequest, + ): Promise => { + const response = await this.axios.patch( userSkillPath(user, name), req, ); diff --git a/site/src/api/queries/userSkills.test.ts b/site/src/api/queries/userSkills.test.ts index c8792aecbe..d883f15d9b 100644 --- a/site/src/api/queries/userSkills.test.ts +++ b/site/src/api/queries/userSkills.test.ts @@ -1,6 +1,6 @@ import { QueryClient } from "react-query"; import { describe, expect, it } from "vitest"; -import type { UserSkill, UserSkillMetadata } from "#/api/typesGenerated"; +import type { Skill, SkillMetadata } from "#/api/typesGenerated"; import { createUserSkill, deleteUserSkill, @@ -21,23 +21,22 @@ const createTestQueryClient = (): QueryClient => }, }); -const makeSkill = ( - name: string, - overrides: Partial = {}, -): UserSkill => ({ +const makeSkill = (name: string, overrides: Partial = {}): Skill => ({ id: `${name}-id`, name, description: `${name} description`, + enabled: true, content: `---\nname: ${name}\n---\nBody\n`, created_at: "2026-05-21T00:00:00Z", updated_at: "2026-05-21T00:00:00Z", ...overrides, }); -const toMetadata = (skill: UserSkill): UserSkillMetadata => ({ +const toMetadata = (skill: Skill): SkillMetadata => ({ id: skill.id, name: skill.name, description: skill.description, + enabled: skill.enabled, created_at: skill.created_at, updated_at: skill.updated_at, }); diff --git a/site/src/api/queries/userSkills.ts b/site/src/api/queries/userSkills.ts index 2c85a7f3ad..db23939018 100644 --- a/site/src/api/queries/userSkills.ts +++ b/site/src/api/queries/userSkills.ts @@ -8,45 +8,46 @@ const userSkillKey = (name: string, user = "me") => [...userSkillsKey(user), name] as const; const toUserSkillMetadata = ( - skill: TypesGen.UserSkill, -): TypesGen.UserSkillMetadata => ({ + skill: TypesGen.Skill, +): TypesGen.SkillMetadata => ({ id: skill.id, name: skill.name, description: skill.description, + enabled: skill.enabled, created_at: skill.created_at, updated_at: skill.updated_at, }); const sortUserSkillMetadata = ( - skills: TypesGen.UserSkillMetadata[], -): TypesGen.UserSkillMetadata[] => + skills: TypesGen.SkillMetadata[], +): TypesGen.SkillMetadata[] => skills.toSorted((a, b) => a.name.localeCompare(b.name, "en-US")); const upsertUserSkillMetadata = ( - skills: TypesGen.UserSkillMetadata[] | undefined, - skill: TypesGen.UserSkillMetadata, -): TypesGen.UserSkillMetadata[] => { + skills: TypesGen.SkillMetadata[] | undefined, + skill: TypesGen.SkillMetadata, +): TypesGen.SkillMetadata[] => { const withoutSkill = skills?.filter(({ name }) => name !== skill.name) ?? []; return sortUserSkillMetadata([...withoutSkill, skill]); }; export const userSkills = (user = "me") => ({ queryKey: userSkillsKey(user), - queryFn: (): Promise => + queryFn: (): Promise => API.experimental.getUserSkills(user), }); export const userSkill = (name: string, user = "me") => ({ queryKey: userSkillKey(name, user), - queryFn: (): Promise => + queryFn: (): Promise => API.experimental.getUserSkillByName(user, name), }); export const createUserSkill = (queryClient: QueryClient, user = "me") => ({ - mutationFn: (req: TypesGen.CreateUserSkillRequest) => + mutationFn: (req: TypesGen.CreateSkillRequest) => API.experimental.createUserSkill(user, req), - onSuccess: (skill: TypesGen.UserSkill) => { - queryClient.setQueryData( + onSuccess: (skill: TypesGen.Skill) => { + queryClient.setQueryData( userSkillsKey(user), (skills) => upsertUserSkillMetadata(skills, toUserSkillMetadata(skill)), ); @@ -56,15 +57,15 @@ export const createUserSkill = (queryClient: QueryClient, user = "me") => ({ type UpdateUserSkillArgs = { name: string; - req: TypesGen.UpdateUserSkillRequest; + req: TypesGen.UpdateSkillRequest; }; export const updateUserSkill = (queryClient: QueryClient, user = "me") => ({ mutationFn: ({ name, req }: UpdateUserSkillArgs) => API.experimental.updateUserSkill(user, name, req), - onSuccess: (skill: TypesGen.UserSkill, { name }: UpdateUserSkillArgs) => { + onSuccess: (skill: TypesGen.Skill, { name }: UpdateUserSkillArgs) => { queryClient.setQueryData(userSkillKey(name, user), skill); - queryClient.setQueryData( + queryClient.setQueryData( userSkillsKey(user), (skills) => skills @@ -81,7 +82,7 @@ export const deleteUserSkill = (queryClient: QueryClient, user = "me") => ({ queryKey: userSkillKey(name, user), exact: true, }); - queryClient.setQueryData( + queryClient.setQueryData( userSkillsKey(user), (skills) => skills?.filter((skill) => skill.name !== name), ); diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 212c1ed054..4af8c3aa4b 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -4433,6 +4433,19 @@ export interface CreateProvisionerKeyResponse { readonly key: string; } +// From codersdk/skills.go +/** + * CreateSkillRequest is the payload for creating a skill. + */ +export interface CreateSkillRequest { + /** + * 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. + */ + readonly content: string; +} + // From codersdk/organizations.go /** * CreateTemplateRequest provides options when creating a template. @@ -4708,19 +4721,6 @@ export interface CreateUserSecretRequest { readonly enabled?: boolean; } -// From codersdk/userskills.go -/** - * CreateUserSkillRequest is the payload for creating a user skill. - */ -export interface CreateUserSkillRequest { - /** - * 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. - */ - readonly content: string; -} - // From codersdk/workspaces.go /** * CreateWorkspaceBuildOnSuccessRequest queues a follow-up build that @@ -9619,6 +9619,29 @@ export const SignedAppTokenCookie = "coder_signed_app_token"; */ export const SignedAppTokenQueryParameter = "coder_signed_app_token_23db1dde"; +// From codersdk/skills.go +/** + * Skill represents a personal or organization skill with its raw Markdown + * content. + */ +export interface Skill extends SkillMetadata { + readonly content: string; +} + +// From codersdk/skills.go +/** + * SkillMetadata represents a personal or organization skill without its raw + * Markdown content. + */ +export interface SkillMetadata { + readonly id: string; + readonly name: string; + readonly description: string; + readonly enabled: boolean; + readonly created_at: string; + readonly updated_at: string; +} + // From codersdk/roles.go /** * SlimRole omits permission information from a role. @@ -10850,6 +10873,21 @@ export interface UpdateRoles { readonly roles: readonly string[]; } +// From codersdk/skills.go +/** + * UpdateSkillRequest is the payload for updating a skill. At least one field + * must be set. + */ +export interface UpdateSkillRequest { + /** + * 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. + */ + readonly content?: string; + readonly enabled?: boolean; +} + // From codersdk/templates.go export interface UpdateTemplateACL { /** @@ -11103,19 +11141,6 @@ export interface UpdateUserSecretRequest { readonly enabled?: boolean; } -// From codersdk/userskills.go -/** - * UpdateUserSkillRequest is the payload for updating a user skill. - */ -export interface UpdateUserSkillRequest { - /** - * 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. - */ - readonly content: string; -} - // From codersdk/workspaces.go export interface UpdateWorkspaceACL { /** @@ -11693,26 +11718,6 @@ export interface UserSecretsCapabilities { readonly file_path_delivery_enabled: boolean; } -// From codersdk/userskills.go -/** - * UserSkill represents a user skill with its raw Markdown content. - */ -export interface UserSkill extends UserSkillMetadata { - readonly content: string; -} - -// From codersdk/userskills.go -/** - * UserSkillMetadata represents a user skill without its raw Markdown content. - */ -export interface UserSkillMetadata { - readonly id: string; - readonly name: string; - readonly description: string; - readonly created_at: string; - readonly updated_at: string; -} - // From codersdk/users.go export type UserStatus = "active" | "dormant" | "suspended"; diff --git a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx index ef0dd1da23..1b5f36dcb8 100644 --- a/site/src/pages/AgentsPage/AgentChatPage.stories.tsx +++ b/site/src/pages/AgentsPage/AgentChatPage.stories.tsx @@ -3050,6 +3050,7 @@ export const SlashCompactYieldsToPersonalSkill: Story = { id: "5f3f847b-6e77-4be4-a591-6c04ee0e0e78", name: "compact", description: "Personal compact skill", + enabled: true, created_at: "2024-01-01T00:00:00Z", updated_at: "2024-01-01T00:00:00Z", }, diff --git a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPage.tsx b/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPage.tsx index 68e5e194fd..559f67cca4 100644 --- a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPage.tsx +++ b/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPage.tsx @@ -11,7 +11,7 @@ import { userSkill, userSkills, } from "#/api/queries/userSkills"; -import type { UserSkillMetadata } from "#/api/typesGenerated"; +import type { SkillMetadata } from "#/api/typesGenerated"; import { toast } from "#/components/Toaster/toast"; import { AgentSettingsPersonalSkillsPageView, @@ -34,7 +34,7 @@ const emptySkillFormValues: PersonalSkillFormValues = { type DialogState = | { type: "create"; submittedContent?: string } | { type: "edit"; name: string; submittedContent?: string } - | { type: "delete"; skill: UserSkillMetadata; submittedName?: string } + | { type: "delete"; skill: SkillMetadata; submittedName?: string } | null; const personalSkillError = ( @@ -75,7 +75,7 @@ const downloadPersonalSkillFile = async ( }; const exportPersonalSkillsArchive = async ( - skills: readonly UserSkillMetadata[], + skills: readonly SkillMetadata[], fetchContent: (name: string) => Promise, ): Promise => { const contents = await Promise.all( diff --git a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.tsx b/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.tsx index 82939d3bc3..a0386b4bbd 100644 --- a/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.tsx +++ b/site/src/pages/AgentsPage/AgentSettingsPersonalSkillsPageView.tsx @@ -1,5 +1,5 @@ import { EllipsisVerticalIcon, PlusIcon } from "lucide-react"; -import type { UserSkillMetadata } from "#/api/typesGenerated"; +import type { SkillMetadata } from "#/api/typesGenerated"; import { Alert, AlertDescription } from "#/components/Alert/Alert"; import { ErrorAlert } from "#/components/Alert/ErrorAlert"; import { Button } from "#/components/Button/Button"; @@ -69,7 +69,7 @@ export type PersonalSkillEditorState = }; export type PersonalSkillDeleteState = { - skill: UserSkillMetadata; + skill: SkillMetadata; error?: PersonalSkillErrorDisplay; isDeleting: boolean; onConfirm: () => void; @@ -77,15 +77,15 @@ export type PersonalSkillDeleteState = { }; export type AgentSettingsPersonalSkillsPageViewProps = { - skills: readonly UserSkillMetadata[]; + skills: readonly SkillMetadata[]; error: unknown; isLoading: boolean; isRetrying: boolean; onRetry: () => void; onCreate: () => void; onEdit: (name: string) => void; - onDelete: (skill: UserSkillMetadata) => void; - onDownload: (skill: UserSkillMetadata) => void; + onDelete: (skill: SkillMetadata) => void; + onDownload: (skill: SkillMetadata) => void; onExportAll: () => void; downloadingSkillName?: string; isExportingAll: boolean; diff --git a/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.stories.tsx b/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.stories.tsx index 99da6b5d91..9dde8ecb3a 100644 --- a/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.stories.tsx +++ b/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.stories.tsx @@ -158,7 +158,7 @@ export const ArrowKeysSelectHighlightedSkill: Story = { // Enough skills to overflow the menu's max height so arrow-key // navigation has to scroll the list. -const manyPersonalSkills: TypesGen.UserSkillMetadata[] = Array.from( +const manyPersonalSkills: TypesGen.SkillMetadata[] = Array.from( { length: 15 }, (_, index) => ({ ...MockSkill, @@ -473,7 +473,7 @@ const mockMobileMatchMedia = (): (() => void) => { }; }; -const longSkillList: TypesGen.UserSkillMetadata[] = Array.from( +const longSkillList: TypesGen.SkillMetadata[] = Array.from( { length: 30 }, (_, index) => ({ ...MockSkill, diff --git a/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.tsx b/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.tsx index 766723de99..292f84ccf9 100644 --- a/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.tsx +++ b/site/src/pages/AgentsPage/components/ChatMessageInput/ChatMessageInput.tsx @@ -547,7 +547,7 @@ type ChatMessageInputProps = Omit< /** * Story and test seam for deterministic personal skill menu data. */ - personalSkillsOverride?: readonly TypesGen.UserSkillMetadata[]; + personalSkillsOverride?: readonly TypesGen.SkillMetadata[]; /** * Workspace skill menu data from the chat's pinned context, so the * menu matches read_skill resolution. Undefined while the chat diff --git a/site/src/pages/AgentsPage/utils/personalSkills.test.ts b/site/src/pages/AgentsPage/utils/personalSkills.test.ts index bd4ccb828d..c649e9bd8b 100644 --- a/site/src/pages/AgentsPage/utils/personalSkills.test.ts +++ b/site/src/pages/AgentsPage/utils/personalSkills.test.ts @@ -20,10 +20,11 @@ const skill = ( name: string, description: string, index: number, -): TypesGen.UserSkillMetadata => ({ +): TypesGen.SkillMetadata => ({ id: `skill-${index}`, name, description, + enabled: true, created_at: now, updated_at: now, }); diff --git a/site/src/testHelpers/skills.ts b/site/src/testHelpers/skills.ts index 0f7fd6481e..67a4e13772 100644 --- a/site/src/testHelpers/skills.ts +++ b/site/src/testHelpers/skills.ts @@ -1,15 +1,16 @@ -import type { UserSkillMetadata } from "#/api/typesGenerated"; +import type { SkillMetadata } from "#/api/typesGenerated"; import { MOCK_TIMESTAMP } from "./chatEntities"; -export const MockSkill: UserSkillMetadata = { +export const MockSkill: SkillMetadata = { id: "skill-1", name: "skill", description: "", + enabled: true, created_at: MOCK_TIMESTAMP, updated_at: MOCK_TIMESTAMP, }; -export const MockSkills: UserSkillMetadata[] = [ +export const MockSkills: SkillMetadata[] = [ { ...MockSkill, id: "skill-reviewer",