chore: protect reserved builtin rolenames (#13571)

Conflicting built-in and database role names makes it hard to
disambiguate
This commit is contained in:
Steven Masley
2024-06-13 10:12:37 -10:00
committed by GitHub
parent 7d51515f9d
commit 3d30c8dc68
3 changed files with 46 additions and 0 deletions

View File

@ -195,6 +195,13 @@ type RoleOptions struct {
NoOwnerWorkspaceExec bool
}
// ReservedRoleName exists because the database should only allow unique role
// names, but some roles are built in. So these names are reserved
func ReservedRoleName(name string) bool {
_, ok := builtInRoles[name]
return ok
}
// ReloadBuiltinRoles loads the static roles into the builtInRoles map.
// This can be called again with a different config to change the behavior.
//

View File

@ -11,6 +11,7 @@ import (
"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/database/db2sdk"
"github.com/coder/coder/v2/coderd/httpapi"
"github.com/coder/coder/v2/coderd/rbac"
"github.com/coder/coder/v2/coderd/rbac/policy"
"github.com/coder/coder/v2/codersdk"
)
@ -41,6 +42,16 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context,
)
defer commitAudit()
// 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(role.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", role.Name),
})
return codersdk.Role{}, false
}
if err := httpapi.NameValid(role.Name); err != nil {
httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{
Message: "Invalid role name",

View File

@ -210,6 +210,34 @@ func TestCustomOrganizationRole(t *testing.T) {
require.ErrorContains(t, err, "Validation")
})
t.Run("ReservedName", 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, codersdk.Role{
Name: "owner", // Reserved
DisplayName: "Testing Purposes",
SitePermissions: nil,
OrganizationPermissions: nil,
UserPermissions: nil,
})
require.ErrorContains(t, err, "Reserved")
})
t.Run("MismatchedOrganizations", func(t *testing.T) {
t.Parallel()
dv := coderdtest.DeploymentValues(t)