diff --git a/cli/organizationroles.go b/cli/organizationroles.go index b0cc0d2796c17..dd40d48fb0b36 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -153,6 +153,7 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent return err } + createNewRole := true var customRole codersdk.Role if jsonInput { // JSON Upload mode @@ -174,17 +175,30 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent } return xerrors.Errorf("json input does not appear to be a valid role") } + + existingRoles, err := client.ListOrganizationRoles(ctx, org.ID) + if err != nil { + return xerrors.Errorf("listing existing roles: %w", err) + } + for _, existingRole := range existingRoles { + if strings.EqualFold(customRole.Name, existingRole.Name) { + // Editing an existing role + createNewRole = false + break + } + } } else { if len(inv.Args) == 0 { return xerrors.Errorf("missing role name argument, usage: \"coder organizations roles edit \"") } - interactiveRole, err := interactiveOrgRoleEdit(inv, org.ID, client) + interactiveRole, newRole, err := interactiveOrgRoleEdit(inv, org.ID, client) if err != nil { return xerrors.Errorf("editing role: %w", err) } customRole = *interactiveRole + createNewRole = newRole preview := fmt.Sprintf("permissions: %d site, %d org, %d user", len(customRole.SitePermissions), len(customRole.OrganizationPermissions), len(customRole.UserPermissions)) @@ -203,7 +217,12 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent // Do not actually post updated = customRole } else { - updated, err = client.PatchOrganizationRole(ctx, customRole) + switch createNewRole { + case true: + updated, err = client.CreateOrganizationRole(ctx, customRole) + default: + updated, err = client.UpdateOrganizationRole(ctx, customRole) + } if err != nil { return xerrors.Errorf("patch role: %w", err) } @@ -223,11 +242,12 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent return cmd } -func interactiveOrgRoleEdit(inv *serpent.Invocation, orgID uuid.UUID, client *codersdk.Client) (*codersdk.Role, error) { +func interactiveOrgRoleEdit(inv *serpent.Invocation, orgID uuid.UUID, client *codersdk.Client) (*codersdk.Role, bool, error) { + newRole := false ctx := inv.Context() roles, err := client.ListOrganizationRoles(ctx, orgID) if err != nil { - return nil, xerrors.Errorf("listing roles: %w", err) + return nil, newRole, xerrors.Errorf("listing roles: %w", err) } // Make sure the role actually exists first @@ -246,22 +266,23 @@ func interactiveOrgRoleEdit(inv *serpent.Invocation, orgID uuid.UUID, client *co IsConfirm: true, }) if err != nil { - return nil, xerrors.Errorf("abort: %w", err) + return nil, newRole, xerrors.Errorf("abort: %w", err) } originalRole.Role = codersdk.Role{ Name: inv.Args[0], OrganizationID: orgID.String(), } + newRole = true } // Some checks since interactive mode is limited in what it currently sees if len(originalRole.SitePermissions) > 0 { - return nil, xerrors.Errorf("unable to edit role in interactive mode, it contains site wide permissions") + return nil, newRole, xerrors.Errorf("unable to edit role in interactive mode, it contains site wide permissions") } if len(originalRole.UserPermissions) > 0 { - return nil, xerrors.Errorf("unable to edit role in interactive mode, it contains user permissions") + return nil, newRole, xerrors.Errorf("unable to edit role in interactive mode, it contains user permissions") } role := &originalRole.Role @@ -283,13 +304,13 @@ customRoleLoop: Options: append(permissionPreviews(role, allowedResources), done, abort), }) if err != nil { - return role, xerrors.Errorf("selecting resource: %w", err) + return role, newRole, xerrors.Errorf("selecting resource: %w", err) } switch selected { case done: break customRoleLoop case abort: - return role, xerrors.Errorf("edit role %q aborted", role.Name) + return role, newRole, xerrors.Errorf("edit role %q aborted", role.Name) default: strs := strings.Split(selected, "::") resource := strings.TrimSpace(strs[0]) @@ -300,7 +321,7 @@ customRoleLoop: Defaults: defaultActions(role, resource), }) if err != nil { - return role, xerrors.Errorf("selecting actions for resource %q: %w", resource, err) + return role, newRole, xerrors.Errorf("selecting actions for resource %q: %w", resource, err) } applyOrgResourceActions(role, resource, actions) // back to resources! @@ -309,7 +330,7 @@ customRoleLoop: // This println is required because the prompt ends us on the same line as some text. _, _ = fmt.Println() - return role, nil + return role, newRole, nil } func applyOrgResourceActions(role *codersdk.Role, resource string, actions []string) { diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go index 3c51e33e3e5a4..86c79d772abf3 100644 --- a/coderd/apidoc/docs.go +++ b/coderd/apidoc/docs.go @@ -2500,7 +2500,7 @@ const docTemplate = `{ } } }, - "patch": { + "put": { "security": [ { "CoderSessionToken": [] @@ -2532,7 +2532,55 @@ const docTemplate = `{ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/codersdk.PatchRoleRequest" + "$ref": "#/definitions/codersdk.CustomRoleRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Role" + } + } + } + } + }, + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": [ + "application/json" + ], + "produces": [ + "application/json" + ], + "tags": [ + "Members" + ], + "summary": "Insert a custom organization role", + "operationId": "insert-a-custom-organization-role", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + }, + { + "description": "Insert role request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.CustomRoleRequest" } } ], @@ -9455,6 +9503,36 @@ const docTemplate = `{ } } }, + "codersdk.CustomRoleRequest": { + "type": "object", + "properties": { + "display_name": { + "type": "string" + }, + "name": { + "type": "string" + }, + "organization_permissions": { + "description": "OrganizationPermissions are specific to the organization the role belongs to.", + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Permission" + } + }, + "site_permissions": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Permission" + } + }, + "user_permissions": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Permission" + } + } + } + }, "codersdk.DAUEntry": { "type": "object", "properties": { @@ -11071,36 +11149,6 @@ const docTemplate = `{ } } }, - "codersdk.PatchRoleRequest": { - "type": "object", - "properties": { - "display_name": { - "type": "string" - }, - "name": { - "type": "string" - }, - "organization_permissions": { - "description": "OrganizationPermissions are specific to the organization the role belongs to.", - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.Permission" - } - }, - "site_permissions": { - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.Permission" - } - }, - "user_permissions": { - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.Permission" - } - } - } - }, "codersdk.PatchTemplateVersionRequest": { "type": "object", "properties": { diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json index bf4dc370aa3be..61967f9d5096c 100644 --- a/coderd/apidoc/swagger.json +++ b/coderd/apidoc/swagger.json @@ -2184,7 +2184,7 @@ } } }, - "patch": { + "put": { "security": [ { "CoderSessionToken": [] @@ -2210,7 +2210,49 @@ "in": "body", "required": true, "schema": { - "$ref": "#/definitions/codersdk.PatchRoleRequest" + "$ref": "#/definitions/codersdk.CustomRoleRequest" + } + } + ], + "responses": { + "200": { + "description": "OK", + "schema": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Role" + } + } + } + } + }, + "post": { + "security": [ + { + "CoderSessionToken": [] + } + ], + "consumes": ["application/json"], + "produces": ["application/json"], + "tags": ["Members"], + "summary": "Insert a custom organization role", + "operationId": "insert-a-custom-organization-role", + "parameters": [ + { + "type": "string", + "format": "uuid", + "description": "Organization ID", + "name": "organization", + "in": "path", + "required": true + }, + { + "description": "Insert role request", + "name": "request", + "in": "body", + "required": true, + "schema": { + "$ref": "#/definitions/codersdk.CustomRoleRequest" } } ], @@ -8417,6 +8459,36 @@ } } }, + "codersdk.CustomRoleRequest": { + "type": "object", + "properties": { + "display_name": { + "type": "string" + }, + "name": { + "type": "string" + }, + "organization_permissions": { + "description": "OrganizationPermissions are specific to the organization the role belongs to.", + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Permission" + } + }, + "site_permissions": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Permission" + } + }, + "user_permissions": { + "type": "array", + "items": { + "$ref": "#/definitions/codersdk.Permission" + } + } + } + }, "codersdk.DAUEntry": { "type": "object", "properties": { @@ -9975,36 +10047,6 @@ } } }, - "codersdk.PatchRoleRequest": { - "type": "object", - "properties": { - "display_name": { - "type": "string" - }, - "name": { - "type": "string" - }, - "organization_permissions": { - "description": "OrganizationPermissions are specific to the organization the role belongs to.", - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.Permission" - } - }, - "site_permissions": { - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.Permission" - } - }, - "user_permissions": { - "type": "array", - "items": { - "$ref": "#/definitions/codersdk.Permission" - } - } - } - }, "codersdk.PatchTemplateVersionRequest": { "type": "object", "properties": { diff --git a/coderd/database/dbauthz/customroles_test.go b/coderd/database/dbauthz/customroles_test.go index 4a544989c599e..c5d40b0323185 100644 --- a/coderd/database/dbauthz/customroles_test.go +++ b/coderd/database/dbauthz/customroles_test.go @@ -19,8 +19,8 @@ import ( "github.com/coder/coder/v2/testutil" ) -// TestUpsertCustomRoles verifies creating custom roles cannot escalate permissions. -func TestUpsertCustomRoles(t *testing.T) { +// TestInsertCustomRoles verifies creating custom roles cannot escalate permissions. +func TestInsertCustomRoles(t *testing.T) { t.Parallel() userID := uuid.New() @@ -98,7 +98,7 @@ func TestUpsertCustomRoles(t *testing.T) { org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), - errorContains: "cannot assign both org and site permissions", + errorContains: "organization roles specify site or user permissions", }, { name: "invalid-action", @@ -231,7 +231,7 @@ func TestUpsertCustomRoles(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) ctx = dbauthz.As(ctx, subject) - _, err := az.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{ + _, err := az.InsertCustomRole(ctx, database.InsertCustomRoleParams{ Name: "test-role", DisplayName: "", OrganizationID: tc.organizationID, diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 282de1faccc22..c44d3874978fb 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -815,6 +815,86 @@ func (q *querier) customRoleEscalationCheck(ctx context.Context, actor rbac.Subj return nil } +// customRoleCheck will validate a custom role for inserting or updating. +// If the role is not valid, an error will be returned. +// - Check custom roles are valid for their resource types + actions +// - Check the actor can create the custom role +// - Check the custom role does not grant perms the actor does not have +// - Prevent negative perms +// - Prevent roles with site and org permissions. +func (q *querier) customRoleCheck(ctx context.Context, role database.CustomRole) error { + act, ok := ActorFromContext(ctx) + if !ok { + return NoActorError + } + + // Org permissions require an org role + if role.OrganizationID.UUID == uuid.Nil && len(role.OrgPermissions) > 0 { + return xerrors.Errorf("organization permissions require specifying an organization id") + } + + // Org roles can only specify org permissions + if role.OrganizationID.UUID != uuid.Nil && (len(role.SitePermissions) > 0 || len(role.UserPermissions) > 0) { + return xerrors.Errorf("organization roles specify site or user permissions") + } + + // The rbac.Role has a 'Valid()' function on it that will do a lot + // of checks. + rbacRole, err := rolestore.ConvertDBRole(database.CustomRole{ + Name: role.Name, + DisplayName: role.DisplayName, + SitePermissions: role.SitePermissions, + OrgPermissions: role.OrgPermissions, + UserPermissions: role.UserPermissions, + OrganizationID: role.OrganizationID, + }) + if err != nil { + return xerrors.Errorf("invalid args: %w", err) + } + + err = rbacRole.Valid() + if err != nil { + return xerrors.Errorf("invalid role: %w", err) + } + + if len(rbacRole.Org) > 0 && len(rbacRole.Site) > 0 { + // This is a choice to keep roles simple. If we allow mixing site and org scoped perms, then knowing who can + // do what gets more complicated. + return xerrors.Errorf("invalid custom role, cannot assign both org and site permissions at the same time") + } + + if len(rbacRole.Org) > 1 { + // Again to avoid more complexity in our roles + return xerrors.Errorf("invalid custom role, cannot assign permissions to more than 1 org at a time") + } + + // Prevent escalation + for _, sitePerm := range rbacRole.Site { + err := q.customRoleEscalationCheck(ctx, act, sitePerm, rbac.Object{Type: sitePerm.ResourceType}) + if err != nil { + return xerrors.Errorf("site permission: %w", err) + } + } + + for orgID, perms := range rbacRole.Org { + for _, orgPerm := range perms { + err := q.customRoleEscalationCheck(ctx, act, orgPerm, rbac.Object{OrgID: orgID, Type: orgPerm.ResourceType}) + if err != nil { + return xerrors.Errorf("org=%q: %w", orgID, err) + } + } + } + + for _, userPerm := range rbacRole.User { + err := q.customRoleEscalationCheck(ctx, act, userPerm, rbac.Object{Type: userPerm.ResourceType, Owner: act.ID}) + if err != nil { + return xerrors.Errorf("user permission: %w", err) + } + } + + return nil +} + func (q *querier) AcquireLock(ctx context.Context, id int64) error { return q.db.AcquireLock(ctx, id) } @@ -2551,6 +2631,34 @@ func (q *querier) InsertAuditLog(ctx context.Context, arg database.InsertAuditLo return insert(q.log, q.auth, rbac.ResourceAuditLog, q.db.InsertAuditLog)(ctx, arg) } +func (q *querier) InsertCustomRole(ctx context.Context, arg database.InsertCustomRoleParams) (database.CustomRole, error) { + // Org and site role upsert share the same query. So switch the assertion based on the org uuid. + if arg.OrganizationID.UUID != uuid.Nil { + if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { + return database.CustomRole{}, err + } + } else { + if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignRole); err != nil { + return database.CustomRole{}, err + } + } + + if err := q.customRoleCheck(ctx, database.CustomRole{ + Name: arg.Name, + DisplayName: arg.DisplayName, + SitePermissions: arg.SitePermissions, + OrgPermissions: arg.OrgPermissions, + UserPermissions: arg.UserPermissions, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + OrganizationID: arg.OrganizationID, + ID: uuid.New(), + }); err != nil { + return database.CustomRole{}, err + } + return q.db.InsertCustomRole(ctx, arg) +} + func (q *querier) InsertDBCryptKey(ctx context.Context, arg database.InsertDBCryptKeyParams) error { if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceSystem); err != nil { return err @@ -3002,6 +3110,33 @@ func (q *querier) UpdateAPIKeyByID(ctx context.Context, arg database.UpdateAPIKe return update(q.log, q.auth, fetch, q.db.UpdateAPIKeyByID)(ctx, arg) } +func (q *querier) UpdateCustomRole(ctx context.Context, arg database.UpdateCustomRoleParams) (database.CustomRole, error) { + if arg.OrganizationID.UUID != uuid.Nil { + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { + return database.CustomRole{}, err + } + } else { + if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceAssignRole); err != nil { + return database.CustomRole{}, err + } + } + + if err := q.customRoleCheck(ctx, database.CustomRole{ + Name: arg.Name, + DisplayName: arg.DisplayName, + SitePermissions: arg.SitePermissions, + OrgPermissions: arg.OrgPermissions, + UserPermissions: arg.UserPermissions, + CreatedAt: time.Now(), + UpdatedAt: time.Now(), + OrganizationID: arg.OrganizationID, + ID: uuid.New(), + }); err != nil { + return database.CustomRole{}, err + } + return q.db.UpdateCustomRole(ctx, arg) +} + func (q *querier) UpdateExternalAuthLink(ctx context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) { fetch := func(ctx context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) { return q.db.GetExternalAuthLink(ctx, database.GetExternalAuthLinkParams{UserID: arg.UserID, ProviderID: arg.ProviderID}) @@ -3664,91 +3799,6 @@ func (q *querier) UpsertApplicationName(ctx context.Context, value string) error return q.db.UpsertApplicationName(ctx, value) } -// UpsertCustomRole does a series of authz checks to protect custom roles. -// - Check custom roles are valid for their resource types + actions -// - Check the actor can create the custom role -// - Check the custom role does not grant perms the actor does not have -// - Prevent negative perms -// - Prevent roles with site and org permissions. -func (q *querier) UpsertCustomRole(ctx context.Context, arg database.UpsertCustomRoleParams) (database.CustomRole, error) { - act, ok := ActorFromContext(ctx) - if !ok { - return database.CustomRole{}, NoActorError - } - - // Org and site role upsert share the same query. So switch the assertion based on the org uuid. - if arg.OrganizationID.UUID != uuid.Nil { - if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil { - return database.CustomRole{}, err - } - } else { - if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignRole); err != nil { - return database.CustomRole{}, err - } - } - - if arg.OrganizationID.UUID == uuid.Nil && len(arg.OrgPermissions) > 0 { - return database.CustomRole{}, xerrors.Errorf("organization permissions require specifying an organization id") - } - - // There is quite a bit of validation we should do here. - // The rbac.Role has a 'Valid()' function on it that will do a lot - // of checks. - rbacRole, err := rolestore.ConvertDBRole(database.CustomRole{ - Name: arg.Name, - DisplayName: arg.DisplayName, - SitePermissions: arg.SitePermissions, - OrgPermissions: arg.OrgPermissions, - UserPermissions: arg.UserPermissions, - OrganizationID: arg.OrganizationID, - }) - if err != nil { - return database.CustomRole{}, xerrors.Errorf("invalid args: %w", err) - } - - err = rbacRole.Valid() - if err != nil { - return database.CustomRole{}, xerrors.Errorf("invalid role: %w", err) - } - - if len(rbacRole.Org) > 0 && len(rbacRole.Site) > 0 { - // This is a choice to keep roles simple. If we allow mixing site and org scoped perms, then knowing who can - // do what gets more complicated. - return database.CustomRole{}, xerrors.Errorf("invalid custom role, cannot assign both org and site permissions at the same time") - } - - if len(rbacRole.Org) > 1 { - // Again to avoid more complexity in our roles - return database.CustomRole{}, xerrors.Errorf("invalid custom role, cannot assign permissions to more than 1 org at a time") - } - - // Prevent escalation - for _, sitePerm := range rbacRole.Site { - err := q.customRoleEscalationCheck(ctx, act, sitePerm, rbac.Object{Type: sitePerm.ResourceType}) - if err != nil { - return database.CustomRole{}, xerrors.Errorf("site permission: %w", err) - } - } - - for orgID, perms := range rbacRole.Org { - for _, orgPerm := range perms { - err := q.customRoleEscalationCheck(ctx, act, orgPerm, rbac.Object{OrgID: orgID, Type: orgPerm.ResourceType}) - if err != nil { - return database.CustomRole{}, xerrors.Errorf("org=%q: %w", orgID, err) - } - } - } - - for _, userPerm := range rbacRole.User { - err := q.customRoleEscalationCheck(ctx, act, userPerm, rbac.Object{Type: userPerm.ResourceType, Owner: act.ID}) - if err != nil { - return database.CustomRole{}, xerrors.Errorf("user permission: %w", err) - } - } - - return q.db.UpsertCustomRole(ctx, arg) -} - func (q *querier) UpsertDefaultProxy(ctx context.Context, arg database.UpsertDefaultProxyParams) error { if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceSystem); err != nil { return err diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index d186b789eabf1..0b64bd70fa4f6 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1282,9 +1282,77 @@ func (s *MethodTestSuite) TestUser() { }).Asserts( rbac.ResourceAssignRole, policy.ActionDelete) })) - s.Run("Blank/UpsertCustomRole", s.Subtest(func(db database.Store, check *expects) { + s.Run("Blank/UpdateCustomRole", s.Subtest(func(db database.Store, check *expects) { + customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{}) // Blank is no perms in the role - check.Args(database.UpsertCustomRoleParams{ + check.Args(database.UpdateCustomRoleParams{ + Name: customRole.Name, + DisplayName: "Test Name", + SitePermissions: nil, + OrgPermissions: nil, + UserPermissions: nil, + }).Asserts(rbac.ResourceAssignRole, policy.ActionUpdate) + })) + s.Run("SitePermissions/UpdateCustomRole", 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.UpdateCustomRoleParams{ + Name: customRole.Name, + OrganizationID: customRole.OrganizationID, + DisplayName: "Test Name", + SitePermissions: db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceTemplate: {codersdk.ActionCreate, codersdk.ActionRead, codersdk.ActionUpdate, codersdk.ActionDelete, codersdk.ActionViewInsights}, + }), convertSDKPerm), + OrgPermissions: nil, + UserPermissions: db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, + }), convertSDKPerm), + }).Asserts( + // First check + rbac.ResourceAssignRole, policy.ActionUpdate, + // Escalation checks + rbac.ResourceTemplate, policy.ActionCreate, + rbac.ResourceTemplate, policy.ActionRead, + rbac.ResourceTemplate, policy.ActionUpdate, + rbac.ResourceTemplate, policy.ActionDelete, + rbac.ResourceTemplate, policy.ActionViewInsights, + + rbac.ResourceWorkspace.WithOwner(testActorID.String()), policy.ActionRead, + ) + })) + s.Run("OrgPermissions/UpdateCustomRole", s.Subtest(func(db database.Store, check *expects) { + orgID := uuid.New() + customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{ + OrganizationID: uuid.NullUUID{ + UUID: orgID, + Valid: true, + }, + }) + + check.Args(database.UpdateCustomRoleParams{ + Name: customRole.Name, + DisplayName: "Test Name", + OrganizationID: customRole.OrganizationID, + SitePermissions: nil, + OrgPermissions: db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceTemplate: {codersdk.ActionCreate, codersdk.ActionRead}, + }), convertSDKPerm), + UserPermissions: nil, + }).Asserts( + // First check + rbac.ResourceAssignOrgRole.InOrg(orgID), policy.ActionUpdate, + // Escalation checks + rbac.ResourceTemplate.InOrg(orgID), policy.ActionCreate, + rbac.ResourceTemplate.InOrg(orgID), policy.ActionRead, + ) + })) + s.Run("Blank/InsertCustomRole", s.Subtest(func(db database.Store, check *expects) { + // Blank is no perms in the role + check.Args(database.InsertCustomRoleParams{ Name: "test", DisplayName: "Test Name", SitePermissions: nil, @@ -1292,8 +1360,8 @@ func (s *MethodTestSuite) TestUser() { UserPermissions: nil, }).Asserts(rbac.ResourceAssignRole, policy.ActionCreate) })) - s.Run("SitePermissions/UpsertCustomRole", s.Subtest(func(db database.Store, check *expects) { - check.Args(database.UpsertCustomRoleParams{ + s.Run("SitePermissions/InsertCustomRole", s.Subtest(func(db database.Store, check *expects) { + check.Args(database.InsertCustomRoleParams{ Name: "test", DisplayName: "Test Name", SitePermissions: db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ @@ -1316,9 +1384,9 @@ func (s *MethodTestSuite) TestUser() { rbac.ResourceWorkspace.WithOwner(testActorID.String()), policy.ActionRead, ) })) - s.Run("OrgPermissions/UpsertCustomRole", s.Subtest(func(db database.Store, check *expects) { + s.Run("OrgPermissions/InsertCustomRole", s.Subtest(func(db database.Store, check *expects) { orgID := uuid.New() - check.Args(database.UpsertCustomRoleParams{ + check.Args(database.InsertCustomRoleParams{ Name: "test", DisplayName: "Test Name", OrganizationID: uuid.NullUUID{ @@ -1329,17 +1397,13 @@ func (s *MethodTestSuite) TestUser() { OrgPermissions: db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ codersdk.ResourceTemplate: {codersdk.ActionCreate, codersdk.ActionRead}, }), convertSDKPerm), - UserPermissions: db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ - codersdk.ResourceWorkspace: {codersdk.ActionRead}, - }), convertSDKPerm), + UserPermissions: nil, }).Asserts( // First check rbac.ResourceAssignOrgRole.InOrg(orgID), policy.ActionCreate, // Escalation checks rbac.ResourceTemplate.InOrg(orgID), policy.ActionCreate, rbac.ResourceTemplate.InOrg(orgID), policy.ActionRead, - - rbac.ResourceWorkspace.WithOwner(testActorID.String()), policy.ActionRead, ) })) } diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index ab7cb9bf1b5a3..ccacb0dc0a995 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -880,7 +880,7 @@ func OAuth2ProviderAppToken(t testing.TB, db database.Store, seed database.OAuth } func CustomRole(t testing.TB, db database.Store, seed database.CustomRole) database.CustomRole { - role, err := db.UpsertCustomRole(genCtx, database.UpsertCustomRoleParams{ + role, err := db.InsertCustomRole(genCtx, database.InsertCustomRoleParams{ Name: takeFirst(seed.Name, strings.ToLower(testutil.GetRandomName(t))), DisplayName: testutil.GetRandomName(t), OrganizationID: seed.OrganizationID, diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index 9bbc531dfda9e..39a3adcfd2f4f 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -6161,6 +6161,37 @@ func (q *FakeQuerier) InsertAuditLog(_ context.Context, arg database.InsertAudit return alog, nil } +func (q *FakeQuerier) InsertCustomRole(_ context.Context, arg database.InsertCustomRoleParams) (database.CustomRole, error) { + err := validateDatabaseType(arg) + if err != nil { + return database.CustomRole{}, err + } + + q.mutex.RLock() + defer q.mutex.RUnlock() + for i := range q.customRoles { + if strings.EqualFold(q.customRoles[i].Name, arg.Name) && + q.customRoles[i].OrganizationID.UUID == arg.OrganizationID.UUID { + return database.CustomRole{}, errUniqueConstraint + } + } + + role := database.CustomRole{ + ID: uuid.New(), + Name: arg.Name, + DisplayName: arg.DisplayName, + OrganizationID: arg.OrganizationID, + SitePermissions: arg.SitePermissions, + OrgPermissions: arg.OrgPermissions, + UserPermissions: arg.UserPermissions, + CreatedAt: dbtime.Now(), + UpdatedAt: dbtime.Now(), + } + q.customRoles = append(q.customRoles, role) + + return role, nil +} + func (q *FakeQuerier) InsertDBCryptKey(_ context.Context, arg database.InsertDBCryptKeyParams) error { err := validateDatabaseType(arg) if err != nil { @@ -7531,6 +7562,29 @@ func (q *FakeQuerier) UpdateAPIKeyByID(_ context.Context, arg database.UpdateAPI return sql.ErrNoRows } +func (q *FakeQuerier) UpdateCustomRole(_ context.Context, arg database.UpdateCustomRoleParams) (database.CustomRole, error) { + err := validateDatabaseType(arg) + if err != nil { + return database.CustomRole{}, err + } + + q.mutex.RLock() + defer q.mutex.RUnlock() + for i := range q.customRoles { + if strings.EqualFold(q.customRoles[i].Name, arg.Name) && + q.customRoles[i].OrganizationID.UUID == arg.OrganizationID.UUID { + q.customRoles[i].DisplayName = arg.DisplayName + q.customRoles[i].OrganizationID = arg.OrganizationID + q.customRoles[i].SitePermissions = arg.SitePermissions + q.customRoles[i].OrgPermissions = arg.OrgPermissions + q.customRoles[i].UserPermissions = arg.UserPermissions + q.customRoles[i].UpdatedAt = dbtime.Now() + return q.customRoles[i], nil + } + } + return database.CustomRole{}, sql.ErrNoRows +} + func (q *FakeQuerier) UpdateExternalAuthLink(_ context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) { if err := validateDatabaseType(arg); err != nil { return database.ExternalAuthLink{}, err @@ -8875,42 +8929,6 @@ func (q *FakeQuerier) UpsertApplicationName(_ context.Context, data string) erro return nil } -func (q *FakeQuerier) UpsertCustomRole(_ context.Context, arg database.UpsertCustomRoleParams) (database.CustomRole, error) { - err := validateDatabaseType(arg) - if err != nil { - return database.CustomRole{}, err - } - - q.mutex.RLock() - defer q.mutex.RUnlock() - for i := range q.customRoles { - if strings.EqualFold(q.customRoles[i].Name, arg.Name) { - q.customRoles[i].DisplayName = arg.DisplayName - q.customRoles[i].OrganizationID = arg.OrganizationID - q.customRoles[i].SitePermissions = arg.SitePermissions - q.customRoles[i].OrgPermissions = arg.OrgPermissions - q.customRoles[i].UserPermissions = arg.UserPermissions - q.customRoles[i].UpdatedAt = dbtime.Now() - return q.customRoles[i], nil - } - } - - role := database.CustomRole{ - ID: uuid.New(), - Name: arg.Name, - DisplayName: arg.DisplayName, - OrganizationID: arg.OrganizationID, - SitePermissions: arg.SitePermissions, - OrgPermissions: arg.OrgPermissions, - UserPermissions: arg.UserPermissions, - CreatedAt: dbtime.Now(), - UpdatedAt: dbtime.Now(), - } - q.customRoles = append(q.customRoles, role) - - return role, nil -} - func (q *FakeQuerier) UpsertDefaultProxy(_ context.Context, arg database.UpsertDefaultProxyParams) error { q.defaultProxyDisplayName = arg.DisplayName q.defaultProxyIconURL = arg.IconUrl diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go index eb62316be1902..8c4fd36d8152d 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -1586,6 +1586,13 @@ func (m metricsStore) InsertAuditLog(ctx context.Context, arg database.InsertAud return log, err } +func (m metricsStore) InsertCustomRole(ctx context.Context, arg database.InsertCustomRoleParams) (database.CustomRole, error) { + start := time.Now() + r0, r1 := m.s.InsertCustomRole(ctx, arg) + m.queryLatencies.WithLabelValues("InsertCustomRole").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) InsertDBCryptKey(ctx context.Context, arg database.InsertDBCryptKeyParams) error { start := time.Now() r0 := m.s.InsertDBCryptKey(ctx, arg) @@ -1957,6 +1964,13 @@ func (m metricsStore) UpdateAPIKeyByID(ctx context.Context, arg database.UpdateA return err } +func (m metricsStore) UpdateCustomRole(ctx context.Context, arg database.UpdateCustomRoleParams) (database.CustomRole, error) { + start := time.Now() + r0, r1 := m.s.UpdateCustomRole(ctx, arg) + m.queryLatencies.WithLabelValues("UpdateCustomRole").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) UpdateExternalAuthLink(ctx context.Context, arg database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) { start := time.Now() link, err := m.s.UpdateExternalAuthLink(ctx, arg) @@ -2370,13 +2384,6 @@ func (m metricsStore) UpsertApplicationName(ctx context.Context, value string) e return r0 } -func (m metricsStore) UpsertCustomRole(ctx context.Context, arg database.UpsertCustomRoleParams) (database.CustomRole, error) { - start := time.Now() - r0, r1 := m.s.UpsertCustomRole(ctx, arg) - m.queryLatencies.WithLabelValues("UpsertCustomRole").Observe(time.Since(start).Seconds()) - return r0, r1 -} - func (m metricsStore) UpsertDefaultProxy(ctx context.Context, arg database.UpsertDefaultProxyParams) error { start := time.Now() r0 := m.s.UpsertDefaultProxy(ctx, arg) diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go index 9920dafada324..98103d22168f2 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -3338,6 +3338,21 @@ func (mr *MockStoreMockRecorder) InsertAuditLog(arg0, arg1 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertAuditLog", reflect.TypeOf((*MockStore)(nil).InsertAuditLog), arg0, arg1) } +// InsertCustomRole mocks base method. +func (m *MockStore) InsertCustomRole(arg0 context.Context, arg1 database.InsertCustomRoleParams) (database.CustomRole, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "InsertCustomRole", arg0, arg1) + ret0, _ := ret[0].(database.CustomRole) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// InsertCustomRole indicates an expected call of InsertCustomRole. +func (mr *MockStoreMockRecorder) InsertCustomRole(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "InsertCustomRole", reflect.TypeOf((*MockStore)(nil).InsertCustomRole), arg0, arg1) +} + // InsertDBCryptKey mocks base method. func (m *MockStore) InsertDBCryptKey(arg0 context.Context, arg1 database.InsertDBCryptKeyParams) error { m.ctrl.T.Helper() @@ -4130,6 +4145,21 @@ func (mr *MockStoreMockRecorder) UpdateAPIKeyByID(arg0, arg1 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateAPIKeyByID", reflect.TypeOf((*MockStore)(nil).UpdateAPIKeyByID), arg0, arg1) } +// UpdateCustomRole mocks base method. +func (m *MockStore) UpdateCustomRole(arg0 context.Context, arg1 database.UpdateCustomRoleParams) (database.CustomRole, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "UpdateCustomRole", arg0, arg1) + ret0, _ := ret[0].(database.CustomRole) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// UpdateCustomRole indicates an expected call of UpdateCustomRole. +func (mr *MockStoreMockRecorder) UpdateCustomRole(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpdateCustomRole", reflect.TypeOf((*MockStore)(nil).UpdateCustomRole), arg0, arg1) +} + // UpdateExternalAuthLink mocks base method. func (m *MockStore) UpdateExternalAuthLink(arg0 context.Context, arg1 database.UpdateExternalAuthLinkParams) (database.ExternalAuthLink, error) { m.ctrl.T.Helper() @@ -4980,21 +5010,6 @@ func (mr *MockStoreMockRecorder) UpsertApplicationName(arg0, arg1 any) *gomock.C return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertApplicationName", reflect.TypeOf((*MockStore)(nil).UpsertApplicationName), arg0, arg1) } -// UpsertCustomRole mocks base method. -func (m *MockStore) UpsertCustomRole(arg0 context.Context, arg1 database.UpsertCustomRoleParams) (database.CustomRole, error) { - m.ctrl.T.Helper() - ret := m.ctrl.Call(m, "UpsertCustomRole", arg0, arg1) - ret0, _ := ret[0].(database.CustomRole) - ret1, _ := ret[1].(error) - return ret0, ret1 -} - -// UpsertCustomRole indicates an expected call of UpsertCustomRole. -func (mr *MockStoreMockRecorder) UpsertCustomRole(arg0, arg1 any) *gomock.Call { - mr.mock.ctrl.T.Helper() - return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "UpsertCustomRole", reflect.TypeOf((*MockStore)(nil).UpsertCustomRole), arg0, arg1) -} - // UpsertDefaultProxy mocks base method. func (m *MockStore) UpsertDefaultProxy(arg0 context.Context, arg1 database.UpsertDefaultProxyParams) error { m.ctrl.T.Helper() diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql index c8d360bdf4cae..b34362a33432a 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -1583,7 +1583,7 @@ ALTER TABLE ONLY audit_logs ADD CONSTRAINT audit_logs_pkey PRIMARY KEY (id); ALTER TABLE ONLY custom_roles - ADD CONSTRAINT custom_roles_pkey PRIMARY KEY (name); + ADD CONSTRAINT custom_roles_unique_key UNIQUE (name, organization_id); ALTER TABLE ONLY dbcrypt_keys ADD CONSTRAINT dbcrypt_keys_active_key_digest_key UNIQUE (active_key_digest); diff --git a/coderd/database/migrations/000243_custom_role_pkey_fix.down.sql b/coderd/database/migrations/000243_custom_role_pkey_fix.down.sql new file mode 100644 index 0000000000000..8f0cf0af81740 --- /dev/null +++ b/coderd/database/migrations/000243_custom_role_pkey_fix.down.sql @@ -0,0 +1,5 @@ +ALTER TABLE custom_roles + DROP CONSTRAINT custom_roles_unique_key; + +ALTER TABLE custom_roles + ADD CONSTRAINT custom_roles_pkey PRIMARY KEY (name); diff --git a/coderd/database/migrations/000243_custom_role_pkey_fix.up.sql b/coderd/database/migrations/000243_custom_role_pkey_fix.up.sql new file mode 100644 index 0000000000000..fe84ad118639c --- /dev/null +++ b/coderd/database/migrations/000243_custom_role_pkey_fix.up.sql @@ -0,0 +1,6 @@ +ALTER TABLE custom_roles + DROP CONSTRAINT custom_roles_pkey; + +-- Roles are unique to the organization. +ALTER TABLE custom_roles + ADD CONSTRAINT custom_roles_unique_key UNIQUE (name, organization_id); diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 9ddbf2a74ddf6..a8797de3a8868 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -335,6 +335,7 @@ type sqlcQuerier interface { // every member of the org. InsertAllUsersGroup(ctx context.Context, organizationID uuid.UUID) (Group, error) InsertAuditLog(ctx context.Context, arg InsertAuditLogParams) (AuditLog, error) + InsertCustomRole(ctx context.Context, arg InsertCustomRoleParams) (CustomRole, error) InsertDBCryptKey(ctx context.Context, arg InsertDBCryptKeyParams) error InsertDERPMeshKey(ctx context.Context, value string) error InsertDeploymentID(ctx context.Context, value string) error @@ -402,6 +403,7 @@ type sqlcQuerier interface { UnarchiveTemplateVersion(ctx context.Context, arg UnarchiveTemplateVersionParams) error UnfavoriteWorkspace(ctx context.Context, id uuid.UUID) error UpdateAPIKeyByID(ctx context.Context, arg UpdateAPIKeyByIDParams) error + UpdateCustomRole(ctx context.Context, arg UpdateCustomRoleParams) (CustomRole, error) UpdateExternalAuthLink(ctx context.Context, arg UpdateExternalAuthLinkParams) (ExternalAuthLink, error) UpdateGitSSHKey(ctx context.Context, arg UpdateGitSSHKeyParams) (GitSSHKey, error) UpdateGroupByID(ctx context.Context, arg UpdateGroupByIDParams) (Group, error) @@ -462,7 +464,6 @@ type sqlcQuerier interface { UpsertAnnouncementBanners(ctx context.Context, value string) error UpsertAppSecurityKey(ctx context.Context, value string) error UpsertApplicationName(ctx context.Context, value string) error - UpsertCustomRole(ctx context.Context, arg UpsertCustomRoleParams) (CustomRole, error) // The default proxy is implied and not actually stored in the database. // So we need to store it's configuration here for display purposes. // The functional values are immutable and controlled implicitly. diff --git a/coderd/database/querier_test.go b/coderd/database/querier_test.go index 54225859b3fb9..f42755763ff55 100644 --- a/coderd/database/querier_test.go +++ b/coderd/database/querier_test.go @@ -579,7 +579,7 @@ func TestReadCustomRoles(t *testing.T) { orgID = uuid.NullUUID{} } - role, err := db.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{ + role, err := db.InsertCustomRole(ctx, database.InsertCustomRoleParams{ Name: fmt.Sprintf("role-%d", i), OrganizationID: orgID, }) diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 2de986f9471f9..99e2aaa0e5aa1 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -6547,40 +6547,33 @@ func (q *sqlQuerier) DeleteCustomRole(ctx context.Context, arg DeleteCustomRoleP return err } -const upsertCustomRole = `-- name: UpsertCustomRole :one +const insertCustomRole = `-- name: InsertCustomRole :one INSERT INTO custom_roles ( - name, - display_name, - organization_id, - site_permissions, - org_permissions, - user_permissions, - created_at, - updated_at + name, + display_name, + organization_id, + site_permissions, + org_permissions, + user_permissions, + created_at, + updated_at ) VALUES ( - -- Always force lowercase names - lower($1), - $2, - $3, - $4, - $5, - $6, - now(), - now() + -- Always force lowercase names + lower($1), + $2, + $3, + $4, + $5, + $6, + now(), + now() ) -ON CONFLICT (name) - DO UPDATE SET - display_name = $2, - site_permissions = $4, - org_permissions = $5, - user_permissions = $6, - updated_at = now() RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id ` -type UpsertCustomRoleParams struct { +type InsertCustomRoleParams struct { Name string `db:"name" json:"name"` DisplayName string `db:"display_name" json:"display_name"` OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` @@ -6589,8 +6582,8 @@ type UpsertCustomRoleParams struct { UserPermissions CustomRolePermissions `db:"user_permissions" json:"user_permissions"` } -func (q *sqlQuerier) UpsertCustomRole(ctx context.Context, arg UpsertCustomRoleParams) (CustomRole, error) { - row := q.db.QueryRowContext(ctx, upsertCustomRole, +func (q *sqlQuerier) InsertCustomRole(ctx context.Context, arg InsertCustomRoleParams) (CustomRole, error) { + row := q.db.QueryRowContext(ctx, insertCustomRole, arg.Name, arg.DisplayName, arg.OrganizationID, @@ -6613,6 +6606,54 @@ func (q *sqlQuerier) UpsertCustomRole(ctx context.Context, arg UpsertCustomRoleP return i, err } +const updateCustomRole = `-- name: UpdateCustomRole :one +UPDATE + custom_roles +SET + display_name = $1, + site_permissions = $2, + org_permissions = $3, + user_permissions = $4, + updated_at = now() +WHERE + name = lower($5) + AND organization_id = $6 +RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id +` + +type UpdateCustomRoleParams struct { + DisplayName string `db:"display_name" json:"display_name"` + SitePermissions CustomRolePermissions `db:"site_permissions" json:"site_permissions"` + OrgPermissions CustomRolePermissions `db:"org_permissions" json:"org_permissions"` + UserPermissions CustomRolePermissions `db:"user_permissions" json:"user_permissions"` + Name string `db:"name" json:"name"` + OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` +} + +func (q *sqlQuerier) UpdateCustomRole(ctx context.Context, arg UpdateCustomRoleParams) (CustomRole, error) { + row := q.db.QueryRowContext(ctx, updateCustomRole, + arg.DisplayName, + arg.SitePermissions, + arg.OrgPermissions, + arg.UserPermissions, + arg.Name, + arg.OrganizationID, + ) + var i CustomRole + err := row.Scan( + &i.Name, + &i.DisplayName, + &i.SitePermissions, + &i.OrgPermissions, + &i.UserPermissions, + &i.CreatedAt, + &i.UpdatedAt, + &i.OrganizationID, + &i.ID, + ) + return i, err +} + const getAnnouncementBanners = `-- name: GetAnnouncementBanners :one SELECT value FROM site_configs WHERE key = 'announcement_banners' ` diff --git a/coderd/database/queries/roles.sql b/coderd/database/queries/roles.sql index 21484c056f748..7246ddb6dee2d 100644 --- a/coderd/database/queries/roles.sql +++ b/coderd/database/queries/roles.sql @@ -33,35 +33,41 @@ WHERE AND organization_id = @organization_id ; --- name: UpsertCustomRole :one +-- name: InsertCustomRole :one INSERT INTO custom_roles ( - name, - display_name, - organization_id, - site_permissions, - org_permissions, - user_permissions, - created_at, - updated_at + name, + display_name, + organization_id, + site_permissions, + org_permissions, + user_permissions, + created_at, + updated_at ) VALUES ( - -- Always force lowercase names - lower(@name), - @display_name, - @organization_id, - @site_permissions, - @org_permissions, - @user_permissions, - now(), - now() + -- Always force lowercase names + lower(@name), + @display_name, + @organization_id, + @site_permissions, + @org_permissions, + @user_permissions, + now(), + now() ) -ON CONFLICT (name) - DO UPDATE SET +RETURNING *; + +-- name: UpdateCustomRole :one +UPDATE + custom_roles +SET display_name = @display_name, site_permissions = @site_permissions, org_permissions = @org_permissions, user_permissions = @user_permissions, updated_at = now() -RETURNING * -; +WHERE + name = lower(@name) + AND organization_id = @organization_id +RETURNING *; diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index ab0b3ae90d366..d713b5bba40b2 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -9,7 +9,7 @@ const ( UniqueAgentStatsPkey UniqueConstraint = "agent_stats_pkey" // ALTER TABLE ONLY workspace_agent_stats ADD CONSTRAINT agent_stats_pkey PRIMARY KEY (id); UniqueAPIKeysPkey UniqueConstraint = "api_keys_pkey" // ALTER TABLE ONLY api_keys ADD CONSTRAINT api_keys_pkey PRIMARY KEY (id); UniqueAuditLogsPkey UniqueConstraint = "audit_logs_pkey" // ALTER TABLE ONLY audit_logs ADD CONSTRAINT audit_logs_pkey PRIMARY KEY (id); - UniqueCustomRolesPkey UniqueConstraint = "custom_roles_pkey" // ALTER TABLE ONLY custom_roles ADD CONSTRAINT custom_roles_pkey PRIMARY KEY (name); + UniqueCustomRolesUniqueKey UniqueConstraint = "custom_roles_unique_key" // ALTER TABLE ONLY custom_roles ADD CONSTRAINT custom_roles_unique_key UNIQUE (name, organization_id); UniqueDbcryptKeysActiveKeyDigestKey UniqueConstraint = "dbcrypt_keys_active_key_digest_key" // ALTER TABLE ONLY dbcrypt_keys ADD CONSTRAINT dbcrypt_keys_active_key_digest_key UNIQUE (active_key_digest); UniqueDbcryptKeysPkey UniqueConstraint = "dbcrypt_keys_pkey" // ALTER TABLE ONLY dbcrypt_keys ADD CONSTRAINT dbcrypt_keys_pkey PRIMARY KEY (number); UniqueDbcryptKeysRevokedKeyDigestKey UniqueConstraint = "dbcrypt_keys_revoked_key_digest_key" // ALTER TABLE ONLY dbcrypt_keys ADD CONSTRAINT dbcrypt_keys_revoked_key_digest_key UNIQUE (revoked_key_digest); diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index e75dd76292edb..d270fdad5c1bd 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -28,9 +28,10 @@ var ( // ResourceAssignOrgRole // Valid Actions // - "ActionAssign" :: ability to assign org scoped roles - // - "ActionCreate" :: ability to create/delete/edit custom roles within an organization + // - "ActionCreate" :: ability to create/delete custom roles within an organization // - "ActionDelete" :: ability to delete org scoped roles // - "ActionRead" :: view what roles are assignable + // - "ActionUpdate" :: ability to edit custom roles within an organization ResourceAssignOrgRole = Object{ Type: "assign_org_role", } @@ -41,6 +42,7 @@ var ( // - "ActionCreate" :: ability to create/delete/edit custom roles // - "ActionDelete" :: ability to unassign roles // - "ActionRead" :: view what roles are assignable + // - "ActionUpdate" :: ability to edit custom roles ResourceAssignRole = Object{ Type: "assign_role", } diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 398cec2c829b0..f71a400890a41 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -227,6 +227,7 @@ var RBACPermissions = map[string]PermissionDefinition{ ActionRead: actDef("view what roles are assignable"), ActionDelete: actDef("ability to unassign roles"), ActionCreate: actDef("ability to create/delete/edit custom roles"), + ActionUpdate: actDef("ability to edit custom roles"), }, }, "assign_org_role": { @@ -234,7 +235,8 @@ var RBACPermissions = map[string]PermissionDefinition{ ActionAssign: actDef("ability to assign org scoped roles"), ActionRead: actDef("view what roles are assignable"), ActionDelete: actDef("ability to delete org scoped roles"), - ActionCreate: actDef("ability to create/delete/edit custom roles within an organization"), + ActionCreate: actDef("ability to create/delete custom roles within an organization"), + ActionUpdate: actDef("ability to edit custom roles within an organization"), }, }, "oauth2_app": { diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index b3c81564ff941..a68132ec76ed3 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -281,7 +281,7 @@ func TestRolePermissions(t *testing.T) { }, { Name: "CreateCustomRole", - Actions: []policy.Action{policy.ActionCreate}, + Actions: []policy.Action{policy.ActionCreate, policy.ActionUpdate}, Resource: rbac.ResourceAssignRole, AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner}, @@ -317,7 +317,7 @@ func TestRolePermissions(t *testing.T) { }, { Name: "CreateOrgRoleAssignment", - Actions: []policy.Action{policy.ActionCreate}, + Actions: []policy.Action{policy.ActionCreate, policy.ActionUpdate}, Resource: rbac.ResourceAssignOrgRole.InOrg(orgID), AuthorizeMap: map[bool][]hasAuthSubjects{ true: {owner, orgAdmin}, diff --git a/codersdk/rbacresources_gen.go b/codersdk/rbacresources_gen.go index 67a24bf73e04a..820d4f31b27a7 100644 --- a/codersdk/rbacresources_gen.go +++ b/codersdk/rbacresources_gen.go @@ -58,8 +58,8 @@ const ( var RBACResourceActions = map[RBACResource][]RBACAction{ ResourceWildcard: {}, ResourceApiKey: {ActionCreate, ActionDelete, ActionRead, ActionUpdate}, - ResourceAssignOrgRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, - ResourceAssignRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead}, + ResourceAssignOrgRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead, ActionUpdate}, + ResourceAssignRole: {ActionAssign, ActionCreate, ActionDelete, ActionRead, ActionUpdate}, ResourceAuditLog: {ActionCreate, ActionRead}, ResourceDebugInfo: {ActionRead}, ResourceDeploymentConfig: {ActionRead, ActionUpdate}, diff --git a/codersdk/roles.go b/codersdk/roles.go index 14fc8a88d5333..80f866bbfeadb 100644 --- a/codersdk/roles.go +++ b/codersdk/roles.go @@ -61,8 +61,8 @@ type Role struct { UserPermissions []Permission `json:"user_permissions" table:"user_permissions"` } -// PatchRoleRequest is used to edit custom roles. -type PatchRoleRequest struct { +// CustomRoleRequest is used to edit custom roles. +type CustomRoleRequest struct { Name string `json:"name" table:"name,default_sort" validate:"username"` DisplayName string `json:"display_name" table:"display_name"` SitePermissions []Permission `json:"site_permissions" table:"site_permissions"` @@ -82,9 +82,9 @@ func (r Role) FullName() string { return r.Name + ":" + r.OrganizationID } -// PatchOrganizationRole will upsert a custom organization role -func (c *Client) PatchOrganizationRole(ctx context.Context, role Role) (Role, error) { - req := PatchRoleRequest{ +// CreateOrganizationRole will create a custom organization role +func (c *Client) CreateOrganizationRole(ctx context.Context, role Role) (Role, error) { + req := CustomRoleRequest{ Name: role.Name, DisplayName: role.DisplayName, SitePermissions: role.SitePermissions, @@ -92,7 +92,30 @@ func (c *Client) PatchOrganizationRole(ctx context.Context, role Role) (Role, er UserPermissions: role.UserPermissions, } - res, err := c.Request(ctx, http.MethodPatch, + res, err := c.Request(ctx, http.MethodPost, + fmt.Sprintf("/api/v2/organizations/%s/members/roles", role.OrganizationID), req) + if err != nil { + return Role{}, err + } + defer res.Body.Close() + if res.StatusCode != http.StatusOK { + return Role{}, ReadBodyAsError(res) + } + var r Role + return r, json.NewDecoder(res.Body).Decode(&r) +} + +// UpdateOrganizationRole will update an existing custom organization role +func (c *Client) UpdateOrganizationRole(ctx context.Context, role Role) (Role, error) { + req := CustomRoleRequest{ + Name: role.Name, + DisplayName: role.DisplayName, + SitePermissions: role.SitePermissions, + OrganizationPermissions: role.OrganizationPermissions, + UserPermissions: role.UserPermissions, + } + + res, err := c.Request(ctx, http.MethodPut, fmt.Sprintf("/api/v2/organizations/%s/members/roles", role.OrganizationID), req) if err != nil { return Role{}, err diff --git a/docs/reference/api/members.md b/docs/reference/api/members.md index cecb22340fe99..87b78e23431f5 100644 --- a/docs/reference/api/members.md +++ b/docs/reference/api/members.md @@ -217,13 +217,13 @@ To perform this operation, you must be authenticated. [Learn more](authenticatio ```shell # Example request using curl -curl -X PATCH http://coder-server:8080/api/v2/organizations/{organization}/members/roles \ +curl -X PUT http://coder-server:8080/api/v2/organizations/{organization}/members/roles \ -H 'Content-Type: application/json' \ -H 'Accept: application/json' \ -H 'Coder-Session-Token: API_KEY' ``` -`PATCH /organizations/{organization}/members/roles` +`PUT /organizations/{organization}/members/roles` > Body parameter @@ -257,10 +257,10 @@ curl -X PATCH http://coder-server:8080/api/v2/organizations/{organization}/membe ### Parameters -| Name | In | Type | Required | Description | -| -------------- | ---- | ---------------------------------------------------------------- | -------- | ------------------- | -| `organization` | path | string(uuid) | true | Organization ID | -| `body` | body | [codersdk.PatchRoleRequest](schemas.md#codersdkpatchrolerequest) | true | Upsert role request | +| Name | In | Type | Required | Description | +| -------------- | ---- | ------------------------------------------------------------------ | -------- | ------------------- | +| `organization` | path | string(uuid) | true | Organization ID | +| `body` | body | [codersdk.CustomRoleRequest](schemas.md#codersdkcustomrolerequest) | true | Upsert role request | ### Example responses @@ -369,6 +369,164 @@ Status Code **200** To perform this operation, you must be authenticated. [Learn more](authentication.md). +## Insert a custom organization role + +### Code samples + +```shell +# Example request using curl +curl -X POST http://coder-server:8080/api/v2/organizations/{organization}/members/roles \ + -H 'Content-Type: application/json' \ + -H 'Accept: application/json' \ + -H 'Coder-Session-Token: API_KEY' +``` + +`POST /organizations/{organization}/members/roles` + +> Body parameter + +```json +{ + "display_name": "string", + "name": "string", + "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": "*" + } + ] +} +``` + +### Parameters + +| Name | In | Type | Required | Description | +| -------------- | ---- | ------------------------------------------------------------------ | -------- | ------------------- | +| `organization` | path | string(uuid) | true | Organization ID | +| `body` | body | [codersdk.CustomRoleRequest](schemas.md#codersdkcustomrolerequest) | true | Insert role request | + +### 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` | `group_member` | +| `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). + ## Delete a custom organization role ### Code samples diff --git a/docs/reference/api/schemas.md b/docs/reference/api/schemas.md index a7e5120a1b338..cb7c88af83f2b 100644 --- a/docs/reference/api/schemas.md +++ b/docs/reference/api/schemas.md @@ -1400,6 +1400,46 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `template_version_id` | string | false | | Template version ID can be used to specify a specific version of a template for creating the workspace. | | `ttl_ms` | integer | false | | | +## codersdk.CustomRoleRequest + +```json +{ + "display_name": "string", + "name": "string", + "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": "*" + } + ] +} +``` + +### Properties + +| Name | Type | Required | Restrictions | Description | +| -------------------------- | --------------------------------------------------- | -------- | ------------ | ------------------------------------------------------------------------------ | +| `display_name` | string | false | | | +| `name` | string | false | | | +| `organization_permissions` | array of [codersdk.Permission](#codersdkpermission) | false | | Organization permissions are specific to the organization the role belongs to. | +| `site_permissions` | array of [codersdk.Permission](#codersdkpermission) | false | | | +| `user_permissions` | array of [codersdk.Permission](#codersdkpermission) | false | | | + ## codersdk.DAUEntry ```json @@ -3763,46 +3803,6 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o | `quota_allowance` | integer | false | | | | `remove_users` | array of string | false | | | -## codersdk.PatchRoleRequest - -```json -{ - "display_name": "string", - "name": "string", - "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": "*" - } - ] -} -``` - -### Properties - -| Name | Type | Required | Restrictions | Description | -| -------------------------- | --------------------------------------------------- | -------- | ------------ | ------------------------------------------------------------------------------ | -| `display_name` | string | false | | | -| `name` | string | false | | | -| `organization_permissions` | array of [codersdk.Permission](#codersdkpermission) | false | | Organization permissions are specific to the organization the role belongs to. | -| `site_permissions` | array of [codersdk.Permission](#codersdkpermission) | false | | | -| `user_permissions` | array of [codersdk.Permission](#codersdkpermission) | false | | | - ## codersdk.PatchTemplateVersionRequest ```json diff --git a/enterprise/cli/organizationmembers_test.go b/enterprise/cli/organizationmembers_test.go index f21621d2440f6..ffe536156fcf8 100644 --- a/enterprise/cli/organizationmembers_test.go +++ b/enterprise/cli/organizationmembers_test.go @@ -94,7 +94,7 @@ func TestEnterpriseListOrganizationMembers(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) //nolint:gocritic // only owners can patch roles - customRole, err := ownerClient.PatchOrganizationRole(ctx, codersdk.Role{ + customRole, err := ownerClient.CreateOrganizationRole(ctx, codersdk.Role{ Name: "custom", OrganizationID: owner.OrganizationID.String(), DisplayName: "Custom Role", @@ -147,7 +147,7 @@ func TestAssignOrganizationMemberRole(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) // nolint:gocritic // requires owner role to create - customRole, err := ownerClient.PatchOrganizationRole(ctx, codersdk.Role{ + customRole, err := ownerClient.CreateOrganizationRole(ctx, codersdk.Role{ Name: "custom-role", OrganizationID: owner.OrganizationID.String(), DisplayName: "Custom Role", diff --git a/enterprise/cli/templatecreate_test.go b/enterprise/cli/templatecreate_test.go index 4d3bc266e30c5..f180234d85e85 100644 --- a/enterprise/cli/templatecreate_test.go +++ b/enterprise/cli/templatecreate_test.go @@ -168,7 +168,7 @@ func TestTemplateCreate(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) //nolint:gocritic // owner required to make custom roles - orgTemplateAdminRole, err := ownerClient.PatchOrganizationRole(ctx, codersdk.Role{ + orgTemplateAdminRole, err := ownerClient.CreateOrganizationRole(ctx, codersdk.Role{ Name: "org-template-admin", OrganizationID: secondOrg.ID.String(), OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ diff --git a/enterprise/coderd/coderd.go b/enterprise/coderd/coderd.go index 6196ac32e2598..da33082af7b6e 100644 --- a/enterprise/coderd/coderd.go +++ b/enterprise/coderd/coderd.go @@ -269,7 +269,8 @@ func New(ctx context.Context, options *Options) (_ *API, err error) { httpmw.RequireExperiment(api.AGPL.Experiments, codersdk.ExperimentCustomRoles), httpmw.ExtractOrganizationParam(api.Database), ) - r.Patch("/organizations/{organization}/members/roles", api.patchOrgRoles) + r.Post("/organizations/{organization}/members/roles", api.postOrgRoles) + r.Put("/organizations/{organization}/members/roles", api.putOrgRoles) r.Delete("/organizations/{organization}/members/roles/{roleName}", api.deleteOrgRole) }) diff --git a/enterprise/coderd/roles.go b/enterprise/coderd/roles.go index daef21d716053..50d57e73cae80 100644 --- a/enterprise/coderd/roles.go +++ b/enterprise/coderd/roles.go @@ -1,6 +1,7 @@ package coderd import ( + "context" "fmt" "net/http" @@ -17,19 +18,19 @@ import ( "github.com/coder/coder/v2/codersdk" ) -// patchRole will allow creating a custom organization role +// postOrgRoles will allow creating a custom organization role // -// @Summary Upsert a custom organization role -// @ID upsert-a-custom-organization-role +// @Summary Insert a custom organization role +// @ID insert-a-custom-organization-role // @Security CoderSessionToken // @Accept json // @Produce json // @Param organization path string true "Organization ID" format(uuid) -// @Param request body codersdk.PatchRoleRequest true "Upsert role request" +// @Param request body codersdk.CustomRoleRequest true "Insert role request" // @Tags Members // @Success 200 {array} codersdk.Role -// @Router /organizations/{organization}/members/roles [patch] -func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) { +// @Router /organizations/{organization}/members/roles [post] +func (api *API) postOrgRoles(rw http.ResponseWriter, r *http.Request) { var ( ctx = r.Context() db = api.Database @@ -39,49 +40,82 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) { Audit: *auditor, Log: api.Logger, Request: r, - Action: database.AuditActionWrite, + Action: database.AuditActionCreate, OrganizationID: organization.ID, }) ) defer commitAudit() - var req codersdk.PatchRoleRequest + var req codersdk.CustomRoleRequest if !httpapi.Read(ctx, rw, r, &req) { return } - // This check is not ideal, but we cannot enforce a unique role name in the db against - // the built-in role names. - if rbac.ReservedRoleName(req.Name) { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Reserved role name", - Detail: fmt.Sprintf("%q is a reserved role name, and not allowed to be used", req.Name), - }) + if !validOrganizationRoleRequest(ctx, req, rw) { return } - if err := httpapi.NameValid(req.Name); err != nil { + inserted, err := db.InsertCustomRole(ctx, database.InsertCustomRoleParams{ + Name: req.Name, + DisplayName: req.DisplayName, + OrganizationID: uuid.NullUUID{ + UUID: organization.ID, + Valid: true, + }, + SitePermissions: db2sdk.List(req.SitePermissions, sdkPermissionToDB), + OrgPermissions: db2sdk.List(req.OrganizationPermissions, sdkPermissionToDB), + UserPermissions: db2sdk.List(req.UserPermissions, sdkPermissionToDB), + }) + if httpapi.Is404Error(err) { + httpapi.ResourceNotFound(rw) + return + } + if err != nil { httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid role name", + Message: "Failed to update role permissions", Detail: err.Error(), }) return } + aReq.New = inserted - // Only organization permissions are allowed to be granted - if len(req.SitePermissions) > 0 { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid request, not allowed to assign site wide permissions for an organization role.", - Detail: "organization scoped roles may not contain site wide permissions", + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Role(inserted)) +} + +// patchRole will allow creating a custom organization role +// +// @Summary Upsert a custom organization role +// @ID upsert-a-custom-organization-role +// @Security CoderSessionToken +// @Accept json +// @Produce json +// @Param organization path string true "Organization ID" format(uuid) +// @Param request body codersdk.CustomRoleRequest true "Upsert role request" +// @Tags Members +// @Success 200 {array} codersdk.Role +// @Router /organizations/{organization}/members/roles [put] +func (api *API) putOrgRoles(rw http.ResponseWriter, r *http.Request) { + var ( + ctx = r.Context() + db = api.Database + 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.AuditActionWrite, + OrganizationID: organization.ID, }) + ) + defer commitAudit() + + var req codersdk.CustomRoleRequest + if !httpapi.Read(ctx, rw, r, &req) { return } - if len(req.UserPermissions) > 0 { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid request, not allowed to assign user permissions for an organization role.", - Detail: "organization scoped roles may not contain user permissions", - }) + if !validOrganizationRoleRequest(ctx, req, rw) { return } @@ -106,7 +140,7 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) { aReq.Old = originalRoles[0] } - inserted, err := db.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{ + updated, err := db.UpdateCustomRole(ctx, database.UpdateCustomRoleParams{ Name: req.Name, DisplayName: req.DisplayName, OrganizationID: uuid.NullUUID{ @@ -128,9 +162,9 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) { }) return } - aReq.New = inserted + aReq.New = updated - httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Role(inserted)) + httpapi.Write(ctx, rw, http.StatusOK, db2sdk.Role(updated)) } // deleteOrgRole will remove a custom role from an organization @@ -220,3 +254,42 @@ func sdkPermissionToDB(p codersdk.Permission) database.CustomRolePermission { Action: policy.Action(p.Action), } } + +func validOrganizationRoleRequest(ctx context.Context, req codersdk.CustomRoleRequest, rw http.ResponseWriter) bool { + // This check is not ideal, but we cannot enforce a unique role name in the db against + // the built-in role names. + if rbac.ReservedRoleName(req.Name) { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Reserved role name", + Detail: fmt.Sprintf("%q is a reserved role name, and not allowed to be used", req.Name), + }) + return false + } + + if err := httpapi.NameValid(req.Name); err != nil { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid role name", + Detail: err.Error(), + }) + return false + } + + // Only organization permissions are allowed to be granted + if len(req.SitePermissions) > 0 { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid request, not allowed to assign site wide permissions for an organization role.", + Detail: "organization scoped roles may not contain site wide permissions", + }) + return false + } + + if len(req.UserPermissions) > 0 { + httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ + Message: "Invalid request, not allowed to assign user permissions for an organization role.", + Detail: "organization scoped roles may not contain user permissions", + }) + return false + } + + return true +} diff --git a/enterprise/coderd/roles_test.go b/enterprise/coderd/roles_test.go index afff6dd9f5a25..8c58427a76f0e 100644 --- a/enterprise/coderd/roles_test.go +++ b/enterprise/coderd/roles_test.go @@ -57,7 +57,7 @@ func TestCustomOrganizationRole(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) //nolint:gocritic // owner is required for this - role, err := owner.PatchOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) + role, err := owner.CreateOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) require.NoError(t, err, "upsert role") // Assign the custom template admin role @@ -111,7 +111,7 @@ func TestCustomOrganizationRole(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) //nolint:gocritic // owner is required for this - role, err := owner.PatchOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) + role, err := owner.CreateOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) require.NoError(t, err, "upsert role") // Remove the license to block enterprise functionality @@ -124,7 +124,7 @@ func TestCustomOrganizationRole(t *testing.T) { } // Verify functionality is lost - _, err = owner.PatchOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) + _, err = owner.UpdateOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) require.ErrorContains(t, err, "Custom Roles is an Enterprise feature") // Assign the custom template admin role @@ -152,7 +152,7 @@ func TestCustomOrganizationRole(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) //nolint:gocritic // owner is required for this - role, err := owner.PatchOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) + role, err := owner.CreateOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) require.NoError(t, err, "upsert role") // Assign the custom template admin role @@ -169,7 +169,7 @@ func TestCustomOrganizationRole(t *testing.T) { newRole.SitePermissions = nil newRole.OrganizationPermissions = nil newRole.UserPermissions = nil - _, err = owner.PatchOrganizationRole(ctx, newRole) + _, err = owner.UpdateOrganizationRole(ctx, newRole) require.NoError(t, err, "upsert role with override") // The role should no longer have template perms @@ -203,7 +203,7 @@ func TestCustomOrganizationRole(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) //nolint:gocritic // owner is required for this - _, err := owner.PatchOrganizationRole(ctx, codersdk.Role{ + _, err := owner.CreateOrganizationRole(ctx, codersdk.Role{ Name: "Bad_Name", // No underscores allowed DisplayName: "Testing Purposes", OrganizationID: first.OrganizationID.String(), @@ -232,7 +232,7 @@ func TestCustomOrganizationRole(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) //nolint:gocritic // owner is required for this - _, err := owner.PatchOrganizationRole(ctx, codersdk.Role{ + _, err := owner.CreateOrganizationRole(ctx, codersdk.Role{ Name: "owner", // Reserved DisplayName: "Testing Purposes", OrganizationID: first.OrganizationID.String(), @@ -270,7 +270,7 @@ func TestCustomOrganizationRole(t *testing.T) { } //nolint:gocritic // owner is required for this - _, err := owner.PatchOrganizationRole(ctx, siteRole) + _, err := owner.CreateOrganizationRole(ctx, siteRole) require.ErrorContains(t, err, "site wide permissions") userRole := templateAdminCustom(first.OrganizationID) @@ -282,7 +282,7 @@ func TestCustomOrganizationRole(t *testing.T) { } //nolint:gocritic // owner is required for this - _, err = owner.PatchOrganizationRole(ctx, userRole) + _, err = owner.UpdateOrganizationRole(ctx, userRole) require.ErrorContains(t, err, "not allowed to assign user permissions") }) @@ -307,7 +307,7 @@ func TestCustomOrganizationRole(t *testing.T) { newRole.OrganizationID = "0000" // This is not a valid uuid //nolint:gocritic // owner is required for this - _, err := owner.PatchOrganizationRole(ctx, newRole) + _, err := owner.CreateOrganizationRole(ctx, newRole) require.ErrorContains(t, err, "Resource not found") }) @@ -329,7 +329,7 @@ func TestCustomOrganizationRole(t *testing.T) { 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)) + createdRole, err := orgAdmin.CreateOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) require.NoError(t, err, "upsert role") //nolint:gocritic // org_admin cannot assign to themselves @@ -389,7 +389,7 @@ func TestCustomOrganizationRole(t *testing.T) { 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)) + createdRole, err := orgAdmin.CreateOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) require.NoError(t, err, "upsert role") customRoleIdentifier := rbac.RoleIdentifier{ diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index c43b517ad858a..5bb41cf534e68 100644 --- a/enterprise/coderd/templates_test.go +++ b/enterprise/coderd/templates_test.go @@ -751,7 +751,7 @@ func TestTemplates(t *testing.T) { }) //nolint:gocritic // owner required to make custom roles - orgTemplateAdminRole, err := ownerClient.PatchOrganizationRole(ctx, codersdk.Role{ + orgTemplateAdminRole, err := ownerClient.CreateOrganizationRole(ctx, codersdk.Role{ Name: "org-template-admin", OrganizationID: secondOrg.ID.String(), OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ diff --git a/enterprise/coderd/userauth_test.go b/enterprise/coderd/userauth_test.go index 53a97b6895efd..1f62235e5eb41 100644 --- a/enterprise/coderd/userauth_test.go +++ b/enterprise/coderd/userauth_test.go @@ -705,7 +705,7 @@ func TestEnterpriseUserLogin(t *testing.T) { ctx := testutil.Context(t, testutil.WaitShort) //nolint:gocritic // owner required - customRole, err := ownerClient.PatchOrganizationRole(ctx, codersdk.Role{ + customRole, err := ownerClient.CreateOrganizationRole(ctx, codersdk.Role{ Name: "custom-role", OrganizationID: owner.OrganizationID.String(), OrganizationPermissions: []codersdk.Permission{}, diff --git a/enterprise/coderd/users_test.go b/enterprise/coderd/users_test.go index 91344ceaa12c1..eb0f23b278d92 100644 --- a/enterprise/coderd/users_test.go +++ b/enterprise/coderd/users_test.go @@ -271,7 +271,7 @@ func TestAssignCustomOrgRoles(t *testing.T) { ctx := testutil.Context(t, testutil.WaitShort) // Create a custom role as an organization admin that allows making templates. - auditorRole, err := client.PatchOrganizationRole(ctx, codersdk.Role{ + auditorRole, err := client.CreateOrganizationRole(ctx, codersdk.Role{ Name: "org-template-admin", OrganizationID: owner.OrganizationID.String(), DisplayName: "Template Admin", diff --git a/site/src/api/api.ts b/site/src/api/api.ts index 397f5e0378d75..5c21d64b9c4be 100644 --- a/site/src/api/api.ts +++ b/site/src/api/api.ts @@ -603,11 +603,26 @@ class ApiMethods { /** * @param organization Can be the organization's ID or name */ - patchOrganizationRole = async ( + createOrganizationRole = async ( organization: string, role: TypesGen.Role, ): Promise => { - const response = await this.axios.patch( + const response = await this.axios.post( + `/api/v2/organizations/${organization}/members/roles`, + role, + ); + + return response.data; + }; + + /** + * @param organization Can be the organization's ID or name + */ + updateOrganizationRole = async ( + organization: string, + role: TypesGen.Role, + ): Promise => { + const response = await this.axios.put( `/api/v2/organizations/${organization}/members/roles`, role, ); diff --git a/site/src/api/queries/roles.ts b/site/src/api/queries/roles.ts index b3572efcaed52..e4b555926ca45 100644 --- a/site/src/api/queries/roles.ts +++ b/site/src/api/queries/roles.ts @@ -23,13 +23,27 @@ export const organizationRoles = (organization: string) => { }; }; -export const patchOrganizationRole = ( +export const createOrganizationRole = ( queryClient: QueryClient, organization: string, ) => { return { mutationFn: (request: Role) => - API.patchOrganizationRole(organization, request), + API.createOrganizationRole(organization, request), + onSuccess: async (updatedRole: Role) => + await queryClient.invalidateQueries( + getRoleQueryKey(organization, updatedRole.name), + ), + }; +}; + +export const updateOrganizationRole = ( + queryClient: QueryClient, + organization: string, +) => { + return { + mutationFn: (request: Role) => + API.updateOrganizationRole(organization, request), onSuccess: async (updatedRole: Role) => await queryClient.invalidateQueries( getRoleQueryKey(organization, updatedRole.name), diff --git a/site/src/api/rbacresources_gen.ts b/site/src/api/rbacresources_gen.ts index 0f9578aa226bf..366d81fa00d52 100644 --- a/site/src/api/rbacresources_gen.ts +++ b/site/src/api/rbacresources_gen.ts @@ -16,15 +16,17 @@ export const RBACResourceActions: Partial< }, assign_org_role: { assign: "ability to assign org scoped roles", - create: "ability to create/delete/edit custom roles within an organization", + create: "ability to create/delete custom roles within an organization", delete: "ability to delete org scoped roles", read: "view what roles are assignable", + update: "ability to edit custom roles within an organization", }, assign_role: { assign: "ability to assign roles", create: "ability to create/delete/edit custom roles", delete: "ability to unassign roles", read: "view what roles are assignable", + update: "ability to edit custom roles", }, audit_log: { create: "create new audit log entries", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 370b4f1f90c76..49e0774edfcdb 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -345,6 +345,15 @@ export interface CreateWorkspaceRequest { readonly automatic_updates?: AutomaticUpdates; } +// From codersdk/roles.go +export interface CustomRoleRequest { + readonly name: string; + readonly display_name: string; + readonly site_permissions: readonly Permission[]; + readonly organization_permissions: readonly Permission[]; + readonly user_permissions: readonly Permission[]; +} + // From codersdk/deployment.go export interface DAUEntry { readonly date: string; @@ -925,15 +934,6 @@ export interface PatchGroupRequest { readonly quota_allowance?: number; } -// From codersdk/roles.go -export interface PatchRoleRequest { - readonly name: string; - readonly display_name: string; - readonly site_permissions: readonly Permission[]; - readonly organization_permissions: readonly Permission[]; - readonly user_permissions: readonly Permission[]; -} - // From codersdk/templateversions.go export interface PatchTemplateVersionRequest { readonly name: string; diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx index 6bbe37c58a813..1bb6fd9418820 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePage.tsx @@ -4,8 +4,12 @@ import { useMutation, useQuery, useQueryClient } from "react-query"; import { useNavigate, useParams } from "react-router-dom"; import { getErrorMessage } from "api/errors"; import { organizationPermissions } from "api/queries/organizations"; -import { patchOrganizationRole, organizationRoles } from "api/queries/roles"; -import type { PatchRoleRequest } from "api/typesGenerated"; +import { + organizationRoles, + createOrganizationRole, + updateOrganizationRole, +} from "api/queries/roles"; +import type { CustomRoleRequest } from "api/typesGenerated"; import { displayError } from "components/GlobalSnackbar/utils"; import { Loader } from "components/Loader/Loader"; import { pageTitle } from "utils/page"; @@ -22,8 +26,11 @@ export const CreateEditRolePage: FC = () => { const { organizations } = useOrganizationSettings(); const organization = organizations?.find((o) => o.name === organizationName); const permissionsQuery = useQuery(organizationPermissions(organization?.id)); - const patchOrganizationRoleMutation = useMutation( - patchOrganizationRole(queryClient, organizationName), + const createOrganizationRoleMutation = useMutation( + createOrganizationRole(queryClient, organizationName), + ); + const updateOrganizationRoleMutation = useMutation( + updateOrganizationRole(queryClient, organizationName), ); const { data: roleData, isLoading } = useQuery( organizationRoles(organizationName), @@ -47,9 +54,13 @@ export const CreateEditRolePage: FC = () => { { + onSubmit={async (data: CustomRoleRequest) => { try { - await patchOrganizationRoleMutation.mutateAsync(data); + if (role) { + await updateOrganizationRoleMutation.mutateAsync(data); + } else { + await createOrganizationRoleMutation.mutateAsync(data); + } navigate(`/organizations/${organizationName}/roles`); } catch (error) { displayError( @@ -57,8 +68,16 @@ export const CreateEditRolePage: FC = () => { ); } }} - error={patchOrganizationRoleMutation.error} - isLoading={patchOrganizationRoleMutation.isLoading} + error={ + role + ? updateOrganizationRoleMutation.error + : createOrganizationRoleMutation.error + } + isLoading={ + role + ? updateOrganizationRoleMutation.isLoading + : createOrganizationRoleMutation.isLoading + } organizationName={organizationName} canAssignOrgRole={permissions.assignOrgRole} /> diff --git a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx index 5d23d2e69a500..d1918f48a8b9a 100644 --- a/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx +++ b/site/src/pages/ManagementSettingsPage/CustomRolesPage/CreateEditRolePageView.tsx @@ -20,7 +20,7 @@ import { isApiValidationError } from "api/errors"; import { RBACResourceActions } from "api/rbacresources_gen"; import type { Role, - PatchRoleRequest, + CustomRoleRequest, Permission, AssignableRoles, RBACResource, @@ -38,7 +38,7 @@ const validationSchema = Yup.object({ export type CreateEditRolePageViewProps = { role: AssignableRoles | undefined; - onSubmit: (data: PatchRoleRequest) => void; + onSubmit: (data: CustomRoleRequest) => void; error?: unknown; isLoading: boolean; organizationName: string; @@ -58,7 +58,7 @@ export const CreateEditRolePageView: FC = ({ const navigate = useNavigate(); const onCancel = () => navigate(-1); - const form = useFormik({ + const form = useFormik({ initialValues: { name: role?.name || "", display_name: role?.display_name || "",