diff --git a/cli/organizationroles.go b/cli/organizationroles.go index 70ff90ab5bee1..b0cc0d2796c17 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -203,7 +203,7 @@ func (r *RootCmd) editOrganizationRole(orgContext *OrganizationContext) *serpent // Do not actually post updated = customRole } else { - updated, err = client.PatchOrganizationRole(ctx, org.ID, customRole) + updated, err = client.PatchOrganizationRole(ctx, customRole) if err != nil { return xerrors.Errorf("patch role: %w", err) } diff --git a/coderd/roles.go b/coderd/roles.go index 8c066f5fecbb3..7bc67df7d8a52 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -20,12 +20,12 @@ import ( // roles. Ideally only included in the enterprise package, but the routes are // intermixed with AGPL endpoints. type CustomRoleHandler interface { - PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool) + PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.PatchRoleRequest) (codersdk.Role, bool) } type agplCustomRoleHandler struct{} -func (agplCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, _ *http.Request, _ uuid.UUID, _ codersdk.Role) (codersdk.Role, bool) { +func (agplCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, _ *http.Request, _ uuid.UUID, _ codersdk.PatchRoleRequest) (codersdk.Role, bool) { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ Message: "Creating and updating custom roles is an Enterprise feature. Contact sales!", }) @@ -49,7 +49,7 @@ func (api *API) patchOrgRoles(rw http.ResponseWriter, r *http.Request) { organization = httpmw.OrganizationParam(r) ) - var req codersdk.Role + var req codersdk.PatchRoleRequest if !httpapi.Read(ctx, rw, r, &req) { return } diff --git a/codersdk/roles.go b/codersdk/roles.go index 7d1f007cc7463..0ad05ee679167 100644 --- a/codersdk/roles.go +++ b/codersdk/roles.go @@ -50,7 +50,7 @@ type Permission struct { Action RBACAction `json:"action"` } -// Role is a longer form of SlimRole used to edit custom roles. +// Role is a longer form of SlimRole that includes permissions details. type Role struct { Name string `json:"name" table:"name,default_sort" validate:"username"` OrganizationID string `json:"organization_id,omitempty" table:"organization_id" format:"uuid"` @@ -61,6 +61,16 @@ type Role struct { UserPermissions []Permission `json:"user_permissions" table:"user_permissions"` } +// PatchRoleRequest is used to edit custom roles. +type PatchRoleRequest 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"` + // OrganizationPermissions are specific to the organization the role belongs to. + OrganizationPermissions []Permission `json:"organization_permissions" table:"organization_permissions"` + UserPermissions []Permission `json:"user_permissions" table:"user_permissions"` +} + // FullName returns the role name scoped to the organization ID. This is useful if // printing a set of roles from different scopes, as duplicated names across multiple // scopes will become unique. @@ -73,9 +83,17 @@ func (r Role) FullName() string { } // PatchOrganizationRole will upsert a custom organization role -func (c *Client) PatchOrganizationRole(ctx context.Context, organizationID uuid.UUID, req Role) (Role, error) { +func (c *Client) PatchOrganizationRole(ctx context.Context, role Role) (Role, error) { + req := PatchRoleRequest{ + Name: role.Name, + DisplayName: role.DisplayName, + SitePermissions: role.SitePermissions, + OrganizationPermissions: role.OrganizationPermissions, + UserPermissions: role.UserPermissions, + } + res, err := c.Request(ctx, http.MethodPatch, - fmt.Sprintf("/api/v2/organizations/%s/members/roles", organizationID.String()), req) + fmt.Sprintf("/api/v2/organizations/%s/members/roles", role.OrganizationID), req) if err != nil { return Role{}, err } @@ -83,8 +101,8 @@ func (c *Client) PatchOrganizationRole(ctx context.Context, organizationID uuid. if res.StatusCode != http.StatusOK { return Role{}, ReadBodyAsError(res) } - var role Role - return role, json.NewDecoder(res.Body).Decode(&role) + var r Role + return r, json.NewDecoder(res.Body).Decode(&r) } // ListSiteRoles lists all assignable site wide roles. diff --git a/enterprise/cli/organizationmembers_test.go b/enterprise/cli/organizationmembers_test.go index fb6a7cb286058..e8944862738e9 100644 --- a/enterprise/cli/organizationmembers_test.go +++ b/enterprise/cli/organizationmembers_test.go @@ -36,7 +36,7 @@ func TestEnterpriseListOrganizationMembers(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) //nolint:gocritic // only owners can patch roles - customRole, err := ownerClient.PatchOrganizationRole(ctx, owner.OrganizationID, codersdk.Role{ + customRole, err := ownerClient.PatchOrganizationRole(ctx, codersdk.Role{ Name: "custom", OrganizationID: owner.OrganizationID.String(), DisplayName: "Custom Role", @@ -89,7 +89,7 @@ func TestAssignOrganizationMemberRole(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) // nolint:gocritic // requires owner role to create - customRole, err := ownerClient.PatchOrganizationRole(ctx, owner.OrganizationID, codersdk.Role{ + customRole, err := ownerClient.PatchOrganizationRole(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 f67648cfefc14..4d3bc266e30c5 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, secondOrg.ID, codersdk.Role{ + orgTemplateAdminRole, err := ownerClient.PatchOrganizationRole(ctx, codersdk.Role{ Name: "org-template-admin", OrganizationID: secondOrg.ID.String(), OrganizationPermissions: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ diff --git a/enterprise/coderd/roles.go b/enterprise/coderd/roles.go index b080f01df2d4f..bebd36da0de14 100644 --- a/enterprise/coderd/roles.go +++ b/enterprise/coderd/roles.go @@ -21,7 +21,7 @@ type enterpriseCustomRoleHandler struct { Enabled bool } -func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.Role) (codersdk.Role, bool) { +func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, rw http.ResponseWriter, r *http.Request, orgID uuid.UUID, role codersdk.PatchRoleRequest) (codersdk.Role, bool) { if !h.Enabled { httpapi.Write(ctx, rw, http.StatusForbidden, codersdk.Response{ Message: "Custom roles are not enabled", @@ -77,14 +77,6 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, return codersdk.Role{}, false } - if role.OrganizationID != orgID.String() { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid request, organization in role and url must match", - Detail: fmt.Sprintf("role organization=%q does not match URL=%q", role.OrganizationID, orgID.String()), - }) - return codersdk.Role{}, false - } - originalRoles, err := db.CustomRoles(ctx, database.CustomRolesParams{ LookupRoles: []database.NameOrganizationPair{ { diff --git a/enterprise/coderd/roles_test.go b/enterprise/coderd/roles_test.go index b6fa32e128a0c..50c7c5cad08bb 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, first.OrganizationID, templateAdminCustom(first.OrganizationID)) + role, err := owner.PatchOrganizationRole(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, first.OrganizationID, templateAdminCustom(first.OrganizationID)) + role, err := owner.PatchOrganizationRole(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, first.OrganizationID, templateAdminCustom(first.OrganizationID)) + _, err = owner.PatchOrganizationRole(ctx, templateAdminCustom(first.OrganizationID)) require.ErrorContains(t, err, "roles are not enabled") // 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, first.OrganizationID, templateAdminCustom(first.OrganizationID)) + role, err := owner.PatchOrganizationRole(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, first.OrganizationID, newRole) + _, err = owner.PatchOrganizationRole(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, first.OrganizationID, codersdk.Role{ + _, err := owner.PatchOrganizationRole(ctx, codersdk.Role{ Name: "Bad_Name", // No underscores allowed DisplayName: "Testing Purposes", OrganizationID: first.OrganizationID.String(), @@ -232,9 +232,10 @@ func TestCustomOrganizationRole(t *testing.T) { ctx := testutil.Context(t, testutil.WaitMedium) //nolint:gocritic // owner is required for this - _, err := owner.PatchOrganizationRole(ctx, first.OrganizationID, codersdk.Role{ + _, err := owner.PatchOrganizationRole(ctx, codersdk.Role{ Name: "owner", // Reserved DisplayName: "Testing Purposes", + OrganizationID: first.OrganizationID.String(), SitePermissions: nil, OrganizationPermissions: nil, UserPermissions: nil, @@ -242,28 +243,6 @@ func TestCustomOrganizationRole(t *testing.T) { require.ErrorContains(t, err, "Reserved") }) - t.Run("MismatchedOrganizations", 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, - }, - }, - }) - - ctx := testutil.Context(t, testutil.WaitMedium) - - //nolint:gocritic // owner is required for this - _, err := owner.PatchOrganizationRole(ctx, first.OrganizationID, templateAdminCustom(uuid.New())) - require.ErrorContains(t, err, "does not match") - }) - // Attempt to add site & user permissions, which is not allowed t.Run("ExcessPermissions", func(t *testing.T) { t.Parallel() @@ -291,7 +270,7 @@ func TestCustomOrganizationRole(t *testing.T) { } //nolint:gocritic // owner is required for this - _, err := owner.PatchOrganizationRole(ctx, first.OrganizationID, siteRole) + _, err := owner.PatchOrganizationRole(ctx, siteRole) require.ErrorContains(t, err, "site wide permissions") userRole := templateAdminCustom(first.OrganizationID) @@ -303,11 +282,11 @@ func TestCustomOrganizationRole(t *testing.T) { } //nolint:gocritic // owner is required for this - _, err = owner.PatchOrganizationRole(ctx, first.OrganizationID, userRole) + _, err = owner.PatchOrganizationRole(ctx, userRole) require.ErrorContains(t, err, "not allowed to assign user permissions") }) - t.Run("InvalidUUID", func(t *testing.T) { + t.Run("NotFound", func(t *testing.T) { t.Parallel() dv := coderdtest.DeploymentValues(t) dv.Experiments = []string{string(codersdk.ExperimentCustomRoles)} @@ -328,8 +307,8 @@ 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, first.OrganizationID, newRole) - require.ErrorContains(t, err, "Invalid request") + _, err := owner.PatchOrganizationRole(ctx, newRole) + require.ErrorContains(t, err, "Resource not found") }) } diff --git a/enterprise/coderd/templates_test.go b/enterprise/coderd/templates_test.go index 99e99fe6cb31b..c43b517ad858a 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, secondOrg.ID, codersdk.Role{ + orgTemplateAdminRole, err := ownerClient.PatchOrganizationRole(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 a5f4214db48b5..53a97b6895efd 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, owner.OrganizationID, codersdk.Role{ + customRole, err := ownerClient.PatchOrganizationRole(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 88d845c58c52a..91344ceaa12c1 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, owner.OrganizationID, codersdk.Role{ + auditorRole, err := client.PatchOrganizationRole(ctx, codersdk.Role{ Name: "org-template-admin", OrganizationID: owner.OrganizationID.String(), DisplayName: "Template Admin", diff --git a/site/src/api/typesGenerated.ts b/site/src/api/typesGenerated.ts index 0ad30e1310cff..b9217a681b5e6 100644 --- a/site/src/api/typesGenerated.ts +++ b/site/src/api/typesGenerated.ts @@ -898,6 +898,15 @@ 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;