diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index fcea8168d0000..8fa9d0893421c 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2549,6 +2549,51 @@ const docTemplate = `{ } } }, + "/organizations/{organization}/members/roles/{roleName}": { + "delete": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": [ + "application/json" + ], + "tags": [ + "Members" + ], + "summary": "Delete a custom organization role", + "operationId": "delete-a-custom-organization-role", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Role name", + "name": "roleName", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Role" + } + } + } + } + } + }, "/organizations/{organization}/members/{user}": { "post": { "security": [ diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index f519081f6d425..4fbb0f86e195a 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2227,6 +2227,47 @@ } } }, + "/organizations/{organization}/members/roles/{roleName}": { + "delete": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "produces": ["application/json"], + "tags": ["Members"], + "summary": "Delete a custom organization role", + "operationId": "delete-a-custom-organization-role", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + }, + { + "type": "string", + "description": "Role name", + "name": "roleName", + "in": "path", + "required": true + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Role" + } + } + } + } + } + }, "/organizations/{organization}/members/{user}": { "post": { "security": [ diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 28812b4eae026..1ab1183985098 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -958,6 +958,20 @@ func (q *querier) DeleteCoordinator(ctx context.Context, id uuid.UUID) error { return q.db.DeleteCoordinator(ctx, id) } +func (q *querier) DeleteCustomRole(ctx context.Context, arg database.DeleteCustomRoleParams) error { + if arg.OrganizationID.UUID != uuid.Nil { + if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { + return err + } + } else { + if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignRole); err != nil { + return err + } + } + + return q.db.DeleteCustomRole(ctx, arg) +} + func (q *querier) DeleteExternalAuthLink(ctx context.Context, arg database.DeleteExternalAuthLinkParams) error { return fetchAndExec(q.log, q.auth, policy.ActionUpdatePersonal, func(ctx context.Context, arg database.DeleteExternalAuthLinkParams) (database.ExternalAuthLink, error) { //nolint:gosimple diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 282143df5e505..4eb764fde57b1 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1247,6 +1247,31 @@ func (s *MethodTestSuite) TestUser() { s.Run("CustomRoles", s.Subtest(func(db database.Store, check *expects) { check.Args(database.CustomRolesParams{}).Asserts(rbac.ResourceAssignRole, policy.ActionRead).Returns([]database.CustomRole{}) })) + s.Run("Organization/DeleteCustomRole", s.Subtest(func(db database.Store, check *expects) { + customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{ + OrganizationID: uuid.NullUUID{ + UUID: uuid.New(), + Valid: true, + }, + }) + check.Args(database.DeleteCustomRoleParams{ + Name: customRole.Name, + OrganizationID: customRole.OrganizationID, + }).Asserts( + rbac.ResourceAssignOrgRole.InOrg(customRole.OrganizationID.UUID), policy.ActionDelete) + })) + s.Run("Site/DeleteCustomRole", s.Subtest(func(db database.Store, check *expects) { + customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{ + OrganizationID: uuid.NullUUID{ + UUID: uuid.Nil, + Valid: false, + }, + }) + check.Args(database.DeleteCustomRoleParams{ + Name: customRole.Name, + }).Asserts( + rbac.ResourceAssignRole, policy.ActionDelete) + })) s.Run("Blank/UpsertCustomRole", s.Subtest(func(db database.Store, check *expects) { // Blank is no perms in the role check.Args(database.UpsertCustomRoleParams{ diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 5768379535668..387263fbe18c4 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -1381,6 +1381,35 @@ func (*FakeQuerier) DeleteCoordinator(context.Context, uuid.UUID) error { return ErrUnimplemented } +func (q *FakeQuerier) DeleteCustomRole(_ context.Context, arg database.DeleteCustomRoleParams) error { + err := validateDatabaseType(arg) + if err != nil { + return err + } + + q.mutex.RLock() + defer q.mutex.RUnlock() + + initial := len(q.data.customRoles) + q.data.customRoles = slices.DeleteFunc(q.data.customRoles, func(role database.CustomRole) bool { + return role.OrganizationID.UUID == arg.OrganizationID.UUID && role.Name == arg.Name + }) + if initial == len(q.data.customRoles) { + return sql.ErrNoRows + } + + // Emulate the trigger 'remove_organization_member_custom_role' + for i, mem := range q.organizationMembers { + if mem.OrganizationID == arg.OrganizationID.UUID { + mem.Roles = slices.DeleteFunc(mem.Roles, func(role string) bool { + return role == arg.Name + }) + q.organizationMembers[i] = mem + } + } + return nil +} + func (q *FakeQuerier) DeleteExternalAuthLink(_ context.Context, arg database.DeleteExternalAuthLinkParams) error { err := validateDatabaseType(arg) if err != nil { diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index 7b6cdb147dcf9..207f02d241e99 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -214,6 +214,13 @@ func (m metricsStore) DeleteCoordinator(ctx context.Context, id uuid.UUID) error return r0 } +func (m metricsStore) DeleteCustomRole(ctx context.Context, arg database.DeleteCustomRoleParams) error { + start := time.Now() + r0 := m.s.DeleteCustomRole(ctx, arg) + m.queryLatencies.WithLabelValues("DeleteCustomRole").Observe(time.Since(start).Seconds()) + return r0 +} + func (m metricsStore) DeleteExternalAuthLink(ctx context.Context, arg database.DeleteExternalAuthLinkParams) error { start := time.Now() r0 := m.s.DeleteExternalAuthLink(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index bda8186a26a4f..11c1dcd86c831 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -317,6 +317,20 @@ func (mr *MockStoreMockRecorder) DeleteCoordinator(arg0, arg1 any) *gomock.Call return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteCoordinator", reflect.TypeOf((*MockStore)(nil).DeleteCoordinator), arg0, arg1) } +// DeleteCustomRole mocks base method. +func (m *MockStore) DeleteCustomRole(arg0 context.Context, arg1 database.DeleteCustomRoleParams) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteCustomRole", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// DeleteCustomRole indicates an expected call of DeleteCustomRole. +func (mr *MockStoreMockRecorder) DeleteCustomRole(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteCustomRole", reflect.TypeOf((*MockStore)(nil).DeleteCustomRole), arg0, arg1) +} + // DeleteExternalAuthLink mocks base method. func (m *MockStore) DeleteExternalAuthLink(arg0 context.Context, arg1 database.DeleteExternalAuthLinkParams) error { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index 527c7df3c7959..a16d4bc31847b 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -302,6 +302,26 @@ BEGIN END; $$; +CREATE FUNCTION remove_organization_member_role() RETURNS trigger + LANGUAGE plpgsql + AS $$ +BEGIN + -- Delete the role from all organization members that have it. + -- TODO: When site wide custom roles are supported, if the + -- organization_id is null, we should remove the role from the 'users' + -- table instead. + IF OLD.organization_id IS NOT NULL THEN + UPDATE organization_members + -- this is a noop if the role is not assigned to the member + SET roles = array_remove(roles, OLD.name) + WHERE + -- Scope to the correct organization + organization_members.organization_id = OLD.organization_id; + END IF; + RETURN OLD; +END; +$$; + CREATE FUNCTION tailnet_notify_agent_change() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -1838,6 +1858,10 @@ CREATE UNIQUE INDEX workspaces_owner_id_lower_idx ON workspaces USING btree (own CREATE TRIGGER inhibit_enqueue_if_disabled BEFORE INSERT ON notification_messages FOR EACH ROW EXECUTE FUNCTION inhibit_enqueue_if_disabled(); +CREATE TRIGGER remove_organization_member_custom_role BEFORE DELETE ON custom_roles FOR EACH ROW EXECUTE FUNCTION remove_organization_member_role(); + +COMMENT ON TRIGGER remove_organization_member_custom_role ON custom_roles IS 'When a custom_role is deleted, this trigger removes the role from all organization members.'; + CREATE TRIGGER tailnet_notify_agent_change AFTER INSERT OR DELETE OR UPDATE ON tailnet_agents FOR EACH ROW EXECUTE FUNCTION tailnet_notify_agent_change(); CREATE TRIGGER tailnet_notify_client_change AFTER INSERT OR DELETE OR UPDATE ON tailnet_clients FOR EACH ROW EXECUTE FUNCTION tailnet_notify_client_change(); diff --git a/coderd/database/migrations/000241_delete_user_roles.down.sql b/coderd/database/migrations/000241_delete_user_roles.down.sql new file mode 100644 index 0000000000000..dea9ce0cf7c1b --- /dev/null +++ b/coderd/database/migrations/000241_delete_user_roles.down.sql @@ -0,0 +1,2 @@ +DROP TRIGGER IF EXISTS remove_organization_member_custom_role ON custom_roles; +DROP FUNCTION IF EXISTS remove_organization_member_role; diff --git a/coderd/database/migrations/000241_delete_user_roles.up.sql b/coderd/database/migrations/000241_delete_user_roles.up.sql new file mode 100644 index 0000000000000..d09f555abc633 --- /dev/null +++ b/coderd/database/migrations/000241_delete_user_roles.up.sql @@ -0,0 +1,35 @@ +-- When a custom role is deleted, we need to remove the assigned role +-- from all organization members that have it. +-- This action cannot be reverted, so deleting a custom role should be +-- done with caution. +CREATE OR REPLACE FUNCTION remove_organization_member_role() + RETURNS TRIGGER AS +$$ +BEGIN + -- Delete the role from all organization members that have it. + -- TODO: When site wide custom roles are supported, if the + -- organization_id is null, we should remove the role from the 'users' + -- table instead. + IF OLD.organization_id IS NOT NULL THEN + UPDATE organization_members + -- this is a noop if the role is not assigned to the member + SET roles = array_remove(roles, OLD.name) + WHERE + -- Scope to the correct organization + organization_members.organization_id = OLD.organization_id; + END IF; + RETURN OLD; +END; +$$ LANGUAGE plpgsql; + + +-- Attach the function to deleting the custom role +CREATE TRIGGER remove_organization_member_custom_role + BEFORE DELETE ON custom_roles FOR EACH ROW + EXECUTE PROCEDURE remove_organization_member_role(); + + +COMMENT ON TRIGGER + remove_organization_member_custom_role + ON custom_roles IS + 'When a custom_role is deleted, this trigger removes the role from all organization members.'; diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 2d45e154b532d..9feb5266cc2a2 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -69,6 +69,7 @@ type sqlcQuerier interface { DeleteAllTailnetTunnels(ctx context.Context, arg DeleteAllTailnetTunnelsParams) error DeleteApplicationConnectAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error DeleteCoordinator(ctx context.Context, id uuid.UUID) error + DeleteCustomRole(ctx context.Context, arg DeleteCustomRoleParams) error DeleteExternalAuthLink(ctx context.Context, arg DeleteExternalAuthLinkParams) error DeleteGitSSHKey(ctx context.Context, userID uuid.UUID) error DeleteGroupByID(ctx context.Context, id uuid.UUID) error diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index c99ed1441aa87..421fcc9a864d5 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -6520,6 +6520,24 @@ func (q *sqlQuerier) CustomRoles(ctx context.Context, arg CustomRolesParams) ([] return items, nil } +const deleteCustomRole = `-- name: DeleteCustomRole :exec +DELETE FROM + custom_roles +WHERE + name = lower($1) + AND organization_id = $2 +` + +type DeleteCustomRoleParams struct { + Name string `db:"name" json:"name"` + OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` +} + +func (q *sqlQuerier) DeleteCustomRole(ctx context.Context, arg DeleteCustomRoleParams) error { + _, err := q.db.ExecContext(ctx, deleteCustomRole, arg.Name, arg.OrganizationID) + return err +} + const upsertCustomRole = `-- name: UpsertCustomRole :one INSERT INTO custom_roles ( diff --git a/coderd/database/queries/roles.sql b/coderd/database/queries/roles.sql index ec5566a3d0dbb..21484c056f748 100644 --- a/coderd/database/queries/roles.sql +++ b/coderd/database/queries/roles.sql @@ -25,6 +25,13 @@ WHERE END ; +-- name: DeleteCustomRole :exec +DELETE FROM + custom_roles +WHERE + name = lower(@name) + AND organization_id = @organization_id +; -- name: UpsertCustomRole :one INSERT INTO diff --git a/coderd/httpapi/httpapi.go b/coderd/httpapi/httpapi.go index c1267d1720e17..c0dcc7cde985e 100644 --- a/coderd/httpapi/httpapi.go +++ b/coderd/httpapi/httpapi.go @@ -106,12 +106,24 @@ func Is404Error(err error) bool { return false } + // This tests for dbauthz.IsNotAuthorizedError and rbac.IsUnauthorizedError. + if IsUnauthorizedError(err) { + return true + } + return xerrors.Is(err, sql.ErrNoRows) +} + +func IsUnauthorizedError(err error) bool { + if err == nil { + return false + } + // This tests for dbauthz.IsNotAuthorizedError and rbac.IsUnauthorizedError. var unauthorized httpapiconstraints.IsUnauthorizedError if errors.As(err, &unauthorized) && unauthorized.IsUnauthorized() { return true } - return xerrors.Is(err, sql.ErrNoRows) + return false } // Convenience error functions don't take contexts since their responses are diff --git a/codersdk/roles.go b/codersdk/roles.go index 0ad05ee679167..14fc8a88d5333 100644 --- a/codersdk/roles.go +++ b/codersdk/roles.go @@ -105,6 +105,20 @@ func (c *Client) PatchOrganizationRole(ctx context.Context, role Role) (Role, er return r, json.NewDecoder(res.Body).Decode(&r) } +// DeleteOrganizationRole will delete a custom organization role +func (c *Client) DeleteOrganizationRole(ctx context.Context, organizationID uuid.UUID, roleName string) error { + res, err := c.Request(ctx, http.MethodDelete, + fmt.Sprintf("/api/v2/organizations/%s/members/roles/%s", organizationID.String(), roleName), nil) + if err != nil { + return err + } + defer res.Body.Close() + if res.StatusCode != http.StatusNoContent { + return ReadBodyAsError(res) + } + return nil +} + // ListSiteRoles lists all assignable site wide roles. func (c *Client) ListSiteRoles(ctx context.Context) ([]AssignableRoles, error) { res, err := c.Request(ctx, http.MethodGet, "/api/v2/users/roles", nil) diff --git a/docs/api/members.md b/docs/api/members.md index 182fa99ee5d04..472d3dcc90d43 100644 --- a/docs/api/members.md +++ b/docs/api/members.md @@ -367,6 +367,132 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Delete a custom organization role + +### Code samples + +```shell +# Example request using curl +curl -X DELETE http://coder-server:8080/api/v2/organizations/{organization}/members/roles/{roleName} \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`DELETE /organizations/{organization}/members/roles/{roleName}` + +### Parameters + +| Name | In | Type | Required | Description | +| -------------- | ---- | ------------ | -------- | --------------- | +| `organization` | path | string(uuid) | true | Organization ID | +| `roleName` | path | string | true | Role name | + +### Example responses + +> 200 Response + +```json +[ + { + "display_name": "string", + "name": "string", + "organization_id": "7c60d51f-b44e-4682-87d6-449835ea4de6", + "organization_permissions": [ + { + "action": "application_connect", + "negate": true, + "resource_type": "*" + } + ], + "site_permissions": [ + { + "action": "application_connect", + "negate": true, + "resource_type": "*" + } + ], + "user_permissions": [ + { + "action": "application_connect", + "negate": true, + "resource_type": "*" + } + ] + } +] +``` + +### Responses + +| Status | Meaning | Description | Schema | +| ------ | ------------------------------------------------------- | ----------- | ------------------------------------------------- | +| 200 | [OK](https://tools.ietf.org/html/rfc7231#section-6.3.1) | OK | array of [codersdk.Role](schemas.md#codersdkrole) | + +

Response Schema

+ +Status Code **200** + +| Name | Type | Required | Restrictions | Description | +| ---------------------------- | -------------------------------------------------------- | -------- | ------------ | ----------------------------------------------------------------------------------------------- | +| `[array item]` | array | false | | | +| `» display_name` | string | false | | | +| `» name` | string | false | | | +| `» organization_id` | string(uuid) | false | | | +| `» organization_permissions` | array | false | | Organization permissions are specific for the organization in the field 'OrganizationID' above. | +| `»» action` | [codersdk.RBACAction](schemas.md#codersdkrbacaction) | false | | | +| `»» negate` | boolean | false | | Negate makes this a negative permission | +| `»» resource_type` | [codersdk.RBACResource](schemas.md#codersdkrbacresource) | false | | | +| `» site_permissions` | array | false | | | +| `» user_permissions` | array | false | | | + +#### Enumerated Values + +| Property | Value | +| --------------- | ------------------------- | +| `action` | `application_connect` | +| `action` | `assign` | +| `action` | `create` | +| `action` | `delete` | +| `action` | `read` | +| `action` | `read_personal` | +| `action` | `ssh` | +| `action` | `update` | +| `action` | `update_personal` | +| `action` | `use` | +| `action` | `view_insights` | +| `action` | `start` | +| `action` | `stop` | +| `resource_type` | `*` | +| `resource_type` | `api_key` | +| `resource_type` | `assign_org_role` | +| `resource_type` | `assign_role` | +| `resource_type` | `audit_log` | +| `resource_type` | `debug_info` | +| `resource_type` | `deployment_config` | +| `resource_type` | `deployment_stats` | +| `resource_type` | `file` | +| `resource_type` | `group` | +| `resource_type` | `license` | +| `resource_type` | `notification_preference` | +| `resource_type` | `notification_template` | +| `resource_type` | `oauth2_app` | +| `resource_type` | `oauth2_app_code_token` | +| `resource_type` | `oauth2_app_secret` | +| `resource_type` | `organization` | +| `resource_type` | `organization_member` | +| `resource_type` | `provisioner_daemon` | +| `resource_type` | `provisioner_keys` | +| `resource_type` | `replicas` | +| `resource_type` | `system` | +| `resource_type` | `tailnet_coordinator` | +| `resource_type` | `template` | +| `resource_type` | `user` | +| `resource_type` | `workspace` | +| `resource_type` | `workspace_dormant` | +| `resource_type` | `workspace_proxy` | + +To perform this operation, you must be authenticated. [Learn more](authentication.md). + ## Add organization member ### Code samples diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 9b6d2ec24a9ac..501e6086eaeb3 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -269,6 +269,7 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { httpmw.ExtractOrganizationParam(api.Database), ) r.Patch("/organizations/{organization}/members/roles", api.patchOrgRoles) + r.Delete("/organizations/{organization}/members/roles/{roleName}", api.deleteOrgRole) }) r.Route("/organizations/{organization}/groups", func(r chi.Router) { diff --git a/enterprise/coderd/roles.go b/enterprise/coderd/roles.go index 80bce8a1e975b..daef21d716053 100644 --- a/enterprise/coderd/roles.go +++ b/enterprise/coderd/roles.go @@ -4,6 +4,7 @@ import ( "fmt" "net/http" + "github.com/go-chi/chi/v5" "github.com/google/uuid" "github.com/coder/coder/v2/coderd/audit" @@ -92,7 +93,8 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) { }, }, ExcludeOrgRoles: false, - OrganizationID: organization.ID, + // Linter requires all fields to be set. This field is not actually required. + OrganizationID: organization.ID, }) // If it is a 404 (not found) error, ignore it. if err != nil && !httpapi.Is404Error(err) { @@ -131,6 +133,86 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) { httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Role(inserted)) } +// deleteOrgRole will remove a custom role from an organization +// +// @Summary Delete a custom organization role +// @ID delete-a-custom-organization-role +// @Security CoderSessionToken +// @Produce json +// @Param organization path string true "Organization ID" format(uuid) +// @Param roleName path string true "Role name" +// @Tags Members +// @Success 200 {array} codersdk.Role +// @Router /organizations/{organization}/members/roles/{roleName} [delete] +func (api *API) deleteOrgRole(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + auditor = api.AGPL.Auditor.Load() + organization = httpmw.OrganizationParam(r) + aReq, commitAudit = audit.InitRequest[database.CustomRole](rw, &audit.RequestParams{ + Audit: *auditor, + Log: api.Logger, + Request: r, + Action: database.AuditActionDelete, + OrganizationID: organization.ID, + }) + ) + defer commitAudit() + + rolename := chi.URLParam(r, "roleName") + roles, err := api.Database.CustomRoles(ctx, database.CustomRolesParams{ + LookupRoles: []database.NameOrganizationPair{ + { + Name: rolename, + OrganizationID: organization.ID, + }, + }, + ExcludeOrgRoles: false, + // Linter requires all fields to be set. This field is not actually required. + OrganizationID: organization.ID, + }) + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + if len(roles) == 0 { + httpapi.Write(ctx, rw, http.StatusNotFound, codersdk.Response{ + Message: fmt.Sprintf("No custom role with the name %s found", rolename), + Detail: "no role found", + Validations: nil, + }) + return + } + if len(roles) > 1 { + httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ + Message: fmt.Sprintf("Multiple roles with the name %s found", rolename), + Detail: "multiple roles found, this should never happen", + Validations: nil, + }) + return + } + aReq.Old = roles[0] + + err = api.Database.DeleteCustomRole(ctx, database.DeleteCustomRoleParams{ + Name: rolename, + OrganizationID: uuid.NullUUID{ + UUID: organization.ID, + Valid: true, + }, + }) + if httpapi.IsUnauthorizedError(err) { + httpapi.Forbidden(rw) + return + } + if err != nil { + httpapi.InternalServerError(rw, err) + return + } + aReq.New = database.CustomRole{} + + httpapi.Write(ctx, rw, http.StatusNoContent, nil) +} + func sdkPermissionToDB(p codersdk.Permission) database.CustomRolePermission { return database.CustomRolePermission{ Negate: p.Negate, diff --git a/enterprise/coderd/roles_test.go b/enterprise/coderd/roles_test.go index 05073efd05986..afff6dd9f5a25 100644 --- a/enterprise/coderd/roles_test.go +++ b/enterprise/coderd/roles_test.go @@ -310,6 +310,134 @@ func TestCustomOrganizationRole(t *testing.T) { _, err := owner.PatchOrganizationRole(ctx, newRole) require.ErrorContains(t, err, "Resource not found") }) + + t.Run("Delete", func(t *testing.T) { + t.Parallel() + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{string(codersdk.ExperimentCustomRoles)} + owner, first := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }, + }) + + orgAdmin, orgAdminUser := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID)) + ctx := testutil.Context(t, testutil.WaitMedium) + + createdRole, err := orgAdmin.PatchOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) + require.NoError(t, err, "upsert role") + + //nolint:gocritic // org_admin cannot assign to themselves + _, err = owner.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, orgAdminUser.ID.String(), codersdk.UpdateRoles{ + // Give the user this custom role, to ensure when it is deleted, the user + // is ok to be used. + Roles: []string{createdRole.Name, rbac.ScopedRoleOrgAdmin(first.OrganizationID).Name}, + }) + require.NoError(t, err, "assign custom role to user") + + existingRoles, err := orgAdmin.ListOrganizationRoles(ctx, first.OrganizationID) + require.NoError(t, err) + + exists := slices.ContainsFunc(existingRoles, func(role codersdk.AssignableRoles) bool { + return role.Name == createdRole.Name + }) + require.True(t, exists, "custom role should exist") + + // Delete the role + err = orgAdmin.DeleteOrganizationRole(ctx, first.OrganizationID, createdRole.Name) + require.NoError(t, err) + + existingRoles, err = orgAdmin.ListOrganizationRoles(ctx, first.OrganizationID) + require.NoError(t, err) + + exists = slices.ContainsFunc(existingRoles, func(role codersdk.AssignableRoles) bool { + return role.Name == createdRole.Name + }) + require.False(t, exists, "custom role should be deleted") + + // Verify you can still assign roles. + // There used to be a bug that if a member had a delete role, they + // could not be assigned roles anymore. + //nolint:gocritic // org_admin cannot assign to themselves + _, err = owner.UpdateOrganizationMemberRoles(ctx, first.OrganizationID, orgAdminUser.ID.String(), codersdk.UpdateRoles{ + Roles: []string{rbac.ScopedRoleOrgAdmin(first.OrganizationID).Name}, + }) + require.NoError(t, err) + }) + + // Verify deleting a custom role cascades to all members + t.Run("DeleteRoleCascadeMembers", func(t *testing.T) { + t.Parallel() + dv := coderdtest.DeploymentValues(t) + dv.Experiments = []string{string(codersdk.ExperimentCustomRoles)} + owner, first := coderdenttest.New(t, &coderdenttest.Options{ + Options: &coderdtest.Options{ + DeploymentValues: dv, + }, + LicenseOptions: &coderdenttest.LicenseOptions{ + Features: license.Features{ + codersdk.FeatureCustomRoles: 1, + }, + }, + }) + + orgAdmin, orgAdminUser := coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID)) + ctx := testutil.Context(t, testutil.WaitMedium) + + createdRole, err := orgAdmin.PatchOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) + require.NoError(t, err, "upsert role") + + customRoleIdentifier := rbac.RoleIdentifier{ + Name: createdRole.Name, + OrganizationID: first.OrganizationID, + } + + // Create a few members with the role + coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, customRoleIdentifier) + coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgAdmin(first.OrganizationID), customRoleIdentifier) + coderdtest.CreateAnotherUser(t, owner, first.OrganizationID, rbac.ScopedRoleOrgTemplateAdmin(first.OrganizationID), rbac.ScopedRoleOrgAuditor(first.OrganizationID), customRoleIdentifier) + + // Verify members have the custom role + originalMembers, err := orgAdmin.OrganizationMembers(ctx, first.OrganizationID) + require.NoError(t, err) + require.Len(t, originalMembers, 5) // 3 members + org admin + owner + for _, member := range originalMembers { + if member.UserID == orgAdminUser.ID || member.UserID == first.UserID { + continue + } + + require.True(t, slices.ContainsFunc(member.Roles, func(role codersdk.SlimRole) bool { + return role.Name == customRoleIdentifier.Name + }), "member should have custom role") + } + + err = orgAdmin.DeleteOrganizationRole(ctx, first.OrganizationID, createdRole.Name) + require.NoError(t, err) + + // Verify the role was removed from all members + members, err := orgAdmin.OrganizationMembers(ctx, first.OrganizationID) + require.NoError(t, err) + require.Len(t, members, 5) // 3 members + org admin + owner + for _, member := range members { + require.False(t, slices.ContainsFunc(member.Roles, func(role codersdk.SlimRole) bool { + return role.Name == customRoleIdentifier.Name + }), "role should be removed from all users") + + // Verify the rest of the member's roles are unchanged + original := originalMembers[slices.IndexFunc(originalMembers, func(haystack codersdk.OrganizationMemberWithUserData) bool { + return haystack.UserID == member.UserID + })] + originalWithoutCustom := slices.DeleteFunc(original.Roles, func(role codersdk.SlimRole) bool { + return role.Name == customRoleIdentifier.Name + }) + require.ElementsMatch(t, originalWithoutCustom, member.Roles, "original roles are unchanged") + } + }) } func TestListRoles(t *testing.T) {