diff --git a/cli/organizationroles.go b/cli/organizationroles.go index d1279656666fa..75cf048198b30 100644 --- a/cli/organizationroles.go +++ b/cli/organizationroles.go @@ -36,7 +36,7 @@ func (r *RootCmd) organizationRoles() *serpent.Command { func (r *RootCmd) showOrganizationRoles() *serpent.Command { formatter := cliui.NewOutputFormatter( cliui.ChangeFormatterData( - cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "org_permissions", "user_permissions"}), + cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "organization_permissions", "user_permissions"}), func(data any) (any, error) { inputs, ok := data.([]codersdk.AssignableRoles) if !ok { @@ -103,7 +103,7 @@ func (r *RootCmd) showOrganizationRoles() *serpent.Command { func (r *RootCmd) editOrganizationRole() *serpent.Command { formatter := cliui.NewOutputFormatter( cliui.ChangeFormatterData( - cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "org_permissions", "user_permissions"}), + cliui.TableFormat([]roleTableRow{}, []string{"name", "display_name", "site_permissions", "organization_permissions", "user_permissions"}), func(data any) (any, error) { typed, _ := data.(codersdk.Role) return []roleTableRow{roleToTableView(typed)}, nil @@ -391,6 +391,6 @@ type roleTableRow struct { OrganizationID string `table:"organization_id"` SitePermissions string ` table:"site_permissions"` // map[] -> Permissions - OrganizationPermissions string `table:"org_permissions"` + OrganizationPermissions string `table:"organization_permissions"` UserPermissions string `table:"user_permissions"` } diff --git a/coderd/database/db2sdk/db2sdk.go b/coderd/database/db2sdk/db2sdk.go index 2fe9ac9af7a3d..402752805bf7b 100644 --- a/coderd/database/db2sdk/db2sdk.go +++ b/coderd/database/db2sdk/db2sdk.go @@ -18,7 +18,6 @@ import ( "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/parameter" "github.com/coder/coder/v2/coderd/rbac" - "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/coderd/workspaceapps/appurl" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/provisionersdk/proto" @@ -526,54 +525,51 @@ func ProvisionerDaemon(dbDaemon database.ProvisionerDaemon) codersdk.Provisioner return result } -func Role(role rbac.Role) codersdk.Role { +func RBACRole(role rbac.Role) codersdk.Role { roleName, orgIDStr, err := rbac.RoleSplit(role.Name) if err != nil { roleName = role.Name } + orgPerms := role.Org[orgIDStr] return codersdk.Role{ - Name: roleName, - OrganizationID: orgIDStr, - DisplayName: role.DisplayName, - SitePermissions: List(role.Site, Permission), - // This is not perfect. If there are organization permissions in another - // organization, they will be omitted. This should not be allowed, so - // should never happen. - OrganizationPermissions: List(role.Org[orgIDStr], Permission), - UserPermissions: List(role.User, Permission), + Name: roleName, + OrganizationID: orgIDStr, + DisplayName: role.DisplayName, + SitePermissions: List(role.Site, RBACPermission), + OrganizationPermissions: List(orgPerms, RBACPermission), + UserPermissions: List(role.User, RBACPermission), } } -func Permission(permission rbac.Permission) codersdk.Permission { - return codersdk.Permission{ - Negate: permission.Negate, - ResourceType: codersdk.RBACResource(permission.ResourceType), - Action: codersdk.RBACAction(permission.Action), +func Role(role database.CustomRole) codersdk.Role { + orgID := "" + if role.OrganizationID.UUID != uuid.Nil { + orgID = role.OrganizationID.UUID.String() } -} -func RoleToRBAC(role codersdk.Role) rbac.Role { - orgPerms := map[string][]rbac.Permission{} - if role.OrganizationID != "" { - orgPerms = map[string][]rbac.Permission{ - role.OrganizationID: List(role.OrganizationPermissions, PermissionToRBAC), - } + return codersdk.Role{ + Name: role.Name, + OrganizationID: orgID, + DisplayName: role.DisplayName, + SitePermissions: List(role.SitePermissions, Permission), + OrganizationPermissions: List(role.OrgPermissions, Permission), + UserPermissions: List(role.UserPermissions, Permission), } +} - return rbac.Role{ - Name: rbac.RoleName(role.Name, role.OrganizationID), - DisplayName: role.DisplayName, - Site: List(role.SitePermissions, PermissionToRBAC), - Org: orgPerms, - User: List(role.UserPermissions, PermissionToRBAC), +func Permission(permission database.CustomRolePermission) codersdk.Permission { + return codersdk.Permission{ + Negate: permission.Negate, + ResourceType: codersdk.RBACResource(permission.ResourceType), + Action: codersdk.RBACAction(permission.Action), } } -func PermissionToRBAC(permission codersdk.Permission) rbac.Permission { - return rbac.Permission{ +func RBACPermission(permission rbac.Permission) codersdk.Permission { + return codersdk.Permission{ Negate: permission.Negate, - ResourceType: string(permission.ResourceType), - Action: policy.Action(permission.Action), + ResourceType: codersdk.RBACResource(permission.ResourceType), + Action: codersdk.RBACAction(permission.Action), } } diff --git a/coderd/database/dbauthz/customroles_test.go b/coderd/database/dbauthz/customroles_test.go index aaa2c7a34bbf3..ddcdca084f7f8 100644 --- a/coderd/database/dbauthz/customroles_test.go +++ b/coderd/database/dbauthz/customroles_test.go @@ -1,7 +1,6 @@ package dbauthz_test import ( - "encoding/json" "testing" "github.com/google/uuid" @@ -11,10 +10,12 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbauthz" "github.com/coder/coder/v2/coderd/database/dbmem" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" ) @@ -60,17 +61,21 @@ func TestUpsertCustomRoles(t *testing.T) { return all } - orgID := uuid.New() + orgID := uuid.NullUUID{ + UUID: uuid.New(), + Valid: true, + } testCases := []struct { name string subject rbac.ExpandableRoles // Perms to create on new custom role - site []rbac.Permission - org map[string][]rbac.Permission - user []rbac.Permission - errorContains string + organizationID uuid.NullUUID + site []codersdk.Permission + org []codersdk.Permission + user []codersdk.Permission + errorContains string }{ { // No roles, so no assign role @@ -84,45 +89,31 @@ func TestUpsertCustomRoles(t *testing.T) { subject: merge(canAssignRole), }, { - name: "mixed-scopes", - subject: merge(canAssignRole, rbac.RoleOwner()), - site: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, + name: "mixed-scopes", + subject: merge(canAssignRole, rbac.RoleOwner()), + organizationID: orgID, + site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, + }), + org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), - org: map[string][]rbac.Permission{ - uuid.New().String(): rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, - }), - }, errorContains: "cannot assign both org and site permissions", }, - { - name: "multiple-org", - subject: merge(canAssignRole, rbac.RoleOwner()), - org: map[string][]rbac.Permission{ - uuid.New().String(): rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, - }), - uuid.New().String(): rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, - }), - }, - errorContains: "cannot assign permissions to more than 1", - }, { name: "invalid-action", subject: merge(canAssignRole, rbac.RoleOwner()), - site: rbac.Permissions(map[string][]policy.Action{ + site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ // Action does not go with resource - rbac.ResourceWorkspace.Type: {policy.ActionViewInsights}, + codersdk.ResourceWorkspace: {codersdk.ActionViewInsights}, }), errorContains: "invalid action", }, { name: "invalid-resource", subject: merge(canAssignRole, rbac.RoleOwner()), - site: rbac.Permissions(map[string][]policy.Action{ - "foobar": {policy.ActionViewInsights}, + site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + "foobar": {codersdk.ActionViewInsights}, }), errorContains: "invalid resource", }, @@ -130,11 +121,11 @@ func TestUpsertCustomRoles(t *testing.T) { // Not allowing these at this time. name: "negative-permission", subject: merge(canAssignRole, rbac.RoleOwner()), - site: []rbac.Permission{ + site: []codersdk.Permission{ { Negate: true, - ResourceType: rbac.ResourceWorkspace.Type, - Action: policy.ActionRead, + ResourceType: codersdk.ResourceWorkspace, + Action: codersdk.ActionRead, }, }, errorContains: "no negative permissions", @@ -142,8 +133,8 @@ func TestUpsertCustomRoles(t *testing.T) { { name: "wildcard", // not allowed subject: merge(canAssignRole, rbac.RoleOwner()), - site: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.WildcardSymbol}, + site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {"*"}, }), errorContains: "no wildcard symbols", }, @@ -151,40 +142,41 @@ func TestUpsertCustomRoles(t *testing.T) { { name: "read-workspace-escalation", subject: merge(canAssignRole), - site: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, + site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), errorContains: "not allowed to grant this permission", }, { - name: "read-workspace-outside-org", - subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID)), - org: map[string][]rbac.Permission{ - // The org admin is for a different org - uuid.NewString(): rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, - }), + name: "read-workspace-outside-org", + organizationID: uuid.NullUUID{ + UUID: uuid.New(), + Valid: true, }, + subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID.UUID)), + org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, + }), errorContains: "not allowed to grant this permission", }, { name: "user-escalation", // These roles do not grant user perms - subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID)), - user: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, + subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID.UUID)), + user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), errorContains: "not allowed to grant this permission", }, { name: "template-admin-escalation", subject: merge(canAssignRole, rbac.RoleTemplateAdmin()), - site: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, // ok! - rbac.ResourceDeploymentConfig.Type: {policy.ActionUpdate}, // not ok! + site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, // ok! + codersdk.ResourceDeploymentConfig: {codersdk.ActionUpdate}, // not ok! }), - user: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, // ok! + user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, // ok! }), errorContains: "deployment_config", }, @@ -192,36 +184,34 @@ func TestUpsertCustomRoles(t *testing.T) { { name: "read-workspace-template-admin", subject: merge(canAssignRole, rbac.RoleTemplateAdmin()), - site: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, + site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), }, { - name: "read-workspace-in-org", - subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID)), - org: map[string][]rbac.Permission{ - // Org admin of this org, this is ok! - orgID.String(): rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, - }), - }, + name: "read-workspace-in-org", + subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID.UUID)), + organizationID: orgID, + org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, + }), }, { name: "user-perms", // This is weird, but is ok subject: merge(canAssignRole, rbac.RoleMember()), - user: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, + user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), }, { name: "site+user-perms", subject: merge(canAssignRole, rbac.RoleMember(), rbac.RoleTemplateAdmin()), - site: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, + site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), - user: rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, + user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{ + codersdk.ResourceWorkspace: {codersdk.ActionRead}, }), }, } @@ -244,9 +234,10 @@ func TestUpsertCustomRoles(t *testing.T) { _, err := az.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{ Name: "test-role", DisplayName: "", - SitePermissions: must(json.Marshal(tc.site)), - OrgPermissions: must(json.Marshal(tc.org)), - UserPermissions: must(json.Marshal(tc.user)), + OrganizationID: tc.organizationID, + SitePermissions: db2sdk.List(tc.site, convertSDKPerm), + OrgPermissions: db2sdk.List(tc.org, convertSDKPerm), + UserPermissions: db2sdk.List(tc.user, convertSDKPerm), }) if tc.errorContains != "" { require.ErrorContains(t, err, tc.errorContains) @@ -256,3 +247,11 @@ func TestUpsertCustomRoles(t *testing.T) { }) } } + +func convertSDKPerm(perm codersdk.Permission) database.CustomRolePermission { + return database.CustomRolePermission{ + Negate: perm.Negate, + ResourceType: string(perm.ResourceType), + Action: policy.Action(perm.Action), + } +} diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index 3a814cfed88d2..a590e272e65fe 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -3441,13 +3441,20 @@ func (q *querier) UpsertCustomRole(ctx context.Context, arg database.UpsertCusto return database.CustomRole{}, err } - // There is quite a bit of validation we should do here. First, let's make sure the json data is correct. + 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) diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go index 9507e1b83c00e..218f73af762ae 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -13,7 +13,9 @@ import ( "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/coderd/coderdtest" "github.com/coder/coder/v2/coderd/database" @@ -1202,22 +1204,22 @@ func (s *MethodTestSuite) TestUser() { check.Args(database.UpsertCustomRoleParams{ Name: "test", DisplayName: "Test Name", - SitePermissions: []byte(`[]`), - OrgPermissions: []byte(`{}`), - UserPermissions: []byte(`[]`), + SitePermissions: nil, + OrgPermissions: nil, + UserPermissions: nil, }).Asserts(rbac.ResourceAssignRole, policy.ActionCreate) })) s.Run("SitePermissions/UpsertCustomRole", s.Subtest(func(db database.Store, check *expects) { check.Args(database.UpsertCustomRoleParams{ Name: "test", DisplayName: "Test Name", - SitePermissions: must(json.Marshal(rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceTemplate.Type: {policy.ActionCreate, policy.ActionRead, policy.ActionUpdate, policy.ActionDelete, policy.ActionViewInsights}, - }))), - OrgPermissions: []byte(`{}`), - UserPermissions: must(json.Marshal(rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, - }))), + 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.ActionCreate, @@ -1234,17 +1236,19 @@ func (s *MethodTestSuite) TestUser() { s.Run("OrgPermissions/UpsertCustomRole", s.Subtest(func(db database.Store, check *expects) { orgID := uuid.New() check.Args(database.UpsertCustomRoleParams{ - Name: "test", - DisplayName: "Test Name", - SitePermissions: []byte(`[]`), - OrgPermissions: must(json.Marshal(map[string][]rbac.Permission{ - orgID.String(): rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceTemplate.Type: {policy.ActionCreate, policy.ActionRead}, - }), - })), - UserPermissions: must(json.Marshal(rbac.Permissions(map[string][]policy.Action{ - rbac.ResourceWorkspace.Type: {policy.ActionRead}, - }))), + Name: "test", + DisplayName: "Test Name", + OrganizationID: uuid.NullUUID{ + UUID: orgID, + Valid: true, + }, + SitePermissions: nil, + 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), }).Asserts( // First check rbac.ResourceAssignRole, policy.ActionCreate, diff --git a/coderd/database/dbgen/dbgen.go b/coderd/database/dbgen/dbgen.go index be612abc333f9..4dea6bdb39f75 100644 --- a/coderd/database/dbgen/dbgen.go +++ b/coderd/database/dbgen/dbgen.go @@ -823,9 +823,9 @@ func CustomRole(t testing.TB, db database.Store, seed database.CustomRole) datab Name: takeFirst(seed.Name, strings.ToLower(namesgenerator.GetRandomName(1))), DisplayName: namesgenerator.GetRandomName(1), OrganizationID: seed.OrganizationID, - SitePermissions: takeFirstSlice(seed.SitePermissions, []byte("[]")), - OrgPermissions: takeFirstSlice(seed.SitePermissions, []byte("{}")), - UserPermissions: takeFirstSlice(seed.SitePermissions, []byte("[]")), + SitePermissions: takeFirstSlice(seed.SitePermissions, []database.CustomRolePermission{}), + OrgPermissions: takeFirstSlice(seed.SitePermissions, []database.CustomRolePermission{}), + UserPermissions: takeFirstSlice(seed.SitePermissions, []database.CustomRolePermission{}), }) require.NoError(t, err, "insert custom role") return role diff --git a/coderd/database/migrations/000214_org_custom_role_array.down.sql b/coderd/database/migrations/000214_org_custom_role_array.down.sql new file mode 100644 index 0000000000000..099389eac58ce --- /dev/null +++ b/coderd/database/migrations/000214_org_custom_role_array.down.sql @@ -0,0 +1 @@ +UPDATE custom_roles SET org_permissions = '{}'; diff --git a/coderd/database/migrations/000214_org_custom_role_array.up.sql b/coderd/database/migrations/000214_org_custom_role_array.up.sql new file mode 100644 index 0000000000000..294d2826fe5f3 --- /dev/null +++ b/coderd/database/migrations/000214_org_custom_role_array.up.sql @@ -0,0 +1,4 @@ +-- Previous custom roles are now invalid, as the json changed. Since this is an +-- experimental feature, there is no point in trying to save the perms. +-- This does not elevate any permissions, so it is not a security issue. +UPDATE custom_roles SET org_permissions = '[]'; diff --git a/coderd/database/models.go b/coderd/database/models.go index 42c41c83bd5dc..e5ba9fcea6841 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1783,13 +1783,13 @@ type AuditLog struct { // Custom roles allow dynamic roles expanded at runtime type CustomRole struct { - Name string `db:"name" json:"name"` - DisplayName string `db:"display_name" json:"display_name"` - SitePermissions json.RawMessage `db:"site_permissions" json:"site_permissions"` - OrgPermissions json.RawMessage `db:"org_permissions" json:"org_permissions"` - UserPermissions json.RawMessage `db:"user_permissions" json:"user_permissions"` - CreatedAt time.Time `db:"created_at" json:"created_at"` - UpdatedAt time.Time `db:"updated_at" json:"updated_at"` + Name string `db:"name" json:"name"` + 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"` + CreatedAt time.Time `db:"created_at" json:"created_at"` + UpdatedAt time.Time `db:"updated_at" json:"updated_at"` // Roles can optionally be scoped to an organization OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` } diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go index 56fcfaf998e4f..5bc7552117b58 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5696,12 +5696,12 @@ RETURNING name, display_name, site_permissions, org_permissions, user_permission ` type UpsertCustomRoleParams struct { - Name string `db:"name" json:"name"` - DisplayName string `db:"display_name" json:"display_name"` - OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` - SitePermissions json.RawMessage `db:"site_permissions" json:"site_permissions"` - OrgPermissions json.RawMessage `db:"org_permissions" json:"org_permissions"` - UserPermissions json.RawMessage `db:"user_permissions" json:"user_permissions"` + Name string `db:"name" json:"name"` + DisplayName string `db:"display_name" json:"display_name"` + OrganizationID uuid.NullUUID `db:"organization_id" json:"organization_id"` + SitePermissions CustomRolePermissions `db:"site_permissions" json:"site_permissions"` + OrgPermissions CustomRolePermissions `db:"org_permissions" json:"org_permissions"` + UserPermissions CustomRolePermissions `db:"user_permissions" json:"user_permissions"` } func (q *sqlQuerier) UpsertCustomRole(ctx context.Context, arg UpsertCustomRoleParams) (CustomRole, error) { diff --git a/coderd/database/sqlc.yaml b/coderd/database/sqlc.yaml index 7913a9acf1627..ff8faf5f7704c 100644 --- a/coderd/database/sqlc.yaml +++ b/coderd/database/sqlc.yaml @@ -28,6 +28,15 @@ sql: emit_enum_valid_method: true emit_all_enum_values: true overrides: + - column: "custom_roles.site_permissions" + go_type: + type: "CustomRolePermissions" + - column: "custom_roles.org_permissions" + go_type: + type: "CustomRolePermissions" + - column: "custom_roles.user_permissions" + go_type: + type: "CustomRolePermissions" - column: "provisioner_daemons.tags" go_type: type: "StringMap" diff --git a/coderd/database/types.go b/coderd/database/types.go index 497446b25abfa..5d0490d0c9020 100644 --- a/coderd/database/types.go +++ b/coderd/database/types.go @@ -112,3 +112,33 @@ func (m *StringMapOfInt) Scan(src interface{}) error { func (m StringMapOfInt) Value() (driver.Value, error) { return json.Marshal(m) } + +type CustomRolePermissions []CustomRolePermission + +func (a *CustomRolePermissions) Scan(src interface{}) error { + switch v := src.(type) { + case string: + return json.Unmarshal([]byte(v), &a) + case []byte: + return json.Unmarshal(v, &a) + } + return xerrors.Errorf("unexpected type %T", src) +} + +func (a CustomRolePermissions) Value() (driver.Value, error) { + return json.Marshal(a) +} + +type CustomRolePermission struct { + Negate bool `json:"negate"` + ResourceType string `json:"resource_type"` + Action policy.Action `json:"action"` +} + +func (a CustomRolePermission) String() string { + str := a.ResourceType + "." + string(a.Action) + if a.Negate { + return "-" + str + } + return str +} diff --git a/coderd/rbac/rolestore/rolestore.go b/coderd/rbac/rolestore/rolestore.go index e0d199241fc9f..083f03877aa83 100644 --- a/coderd/rbac/rolestore/rolestore.go +++ b/coderd/rbac/rolestore/rolestore.go @@ -2,7 +2,6 @@ package rolestore import ( "context" - "encoding/json" "net/http" "github.com/google/uuid" @@ -96,6 +95,20 @@ func Expand(ctx context.Context, db database.Store, names []string) (rbac.Roles, return roles, nil } +func convertPermissions(dbPerms []database.CustomRolePermission) []rbac.Permission { + n := make([]rbac.Permission, 0, len(dbPerms)) + for _, dbPerm := range dbPerms { + n = append(n, rbac.Permission{ + Negate: dbPerm.Negate, + ResourceType: dbPerm.ResourceType, + Action: dbPerm.Action, + }) + } + return n +} + +// ConvertDBRole should not be used by any human facing apis. It is used +// for authz purposes. func ConvertDBRole(dbRole database.CustomRole) (rbac.Role, error) { name := dbRole.Name if dbRole.OrganizationID.Valid { @@ -104,68 +117,21 @@ func ConvertDBRole(dbRole database.CustomRole) (rbac.Role, error) { role := rbac.Role{ Name: name, DisplayName: dbRole.DisplayName, - Site: nil, + Site: convertPermissions(dbRole.SitePermissions), Org: nil, - User: nil, - } - - err := json.Unmarshal(dbRole.SitePermissions, &role.Site) - if err != nil { - return role, xerrors.Errorf("unmarshal site permissions: %w", err) - } - - err = json.Unmarshal(dbRole.OrgPermissions, &role.Org) - if err != nil { - return role, xerrors.Errorf("unmarshal org permissions: %w", err) - } - - err = json.Unmarshal(dbRole.UserPermissions, &role.User) - if err != nil { - return role, xerrors.Errorf("unmarshal user permissions: %w", err) - } - - return role, nil -} - -func ConvertRoleToDB(role rbac.Role) (database.CustomRole, error) { - roleName, orgIDStr, err := rbac.RoleSplit(role.Name) - if err != nil { - return database.CustomRole{}, xerrors.Errorf("split role %q: %w", role.Name, err) + User: convertPermissions(dbRole.UserPermissions), } - dbRole := database.CustomRole{ - Name: roleName, - DisplayName: role.DisplayName, + // Org permissions only make sense if an org id is specified. + if len(dbRole.OrgPermissions) > 0 && dbRole.OrganizationID.UUID == uuid.Nil { + return rbac.Role{}, xerrors.Errorf("role has organization perms without an org id specified") } - if orgIDStr != "" { - orgID, err := uuid.Parse(orgIDStr) - if err != nil { - return database.CustomRole{}, xerrors.Errorf("parse org id %q: %w", orgIDStr, err) - } - dbRole.OrganizationID = uuid.NullUUID{ - UUID: orgID, - Valid: true, + if dbRole.OrganizationID.UUID != uuid.Nil { + role.Org = map[string][]rbac.Permission{ + dbRole.OrganizationID.UUID.String(): convertPermissions(dbRole.OrgPermissions), } } - siteData, err := json.Marshal(role.Site) - if err != nil { - return dbRole, xerrors.Errorf("marshal site permissions: %w", err) - } - dbRole.SitePermissions = siteData - - orgData, err := json.Marshal(role.Org) - if err != nil { - return dbRole, xerrors.Errorf("marshal org permissions: %w", err) - } - dbRole.OrgPermissions = orgData - - userData, err := json.Marshal(role.User) - if err != nil { - return dbRole, xerrors.Errorf("marshal user permissions: %w", err) - } - dbRole.UserPermissions = userData - - return dbRole, nil + return role, nil } diff --git a/coderd/roles.go b/coderd/roles.go index e8505baa4d255..94b121940ed45 100644 --- a/coderd/roles.go +++ b/coderd/roles.go @@ -10,7 +10,6 @@ import ( "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/httpmw" "github.com/coder/coder/v2/coderd/rbac/policy" - "github.com/coder/coder/v2/coderd/rbac/rolestore" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/coderd/httpapi" @@ -91,15 +90,7 @@ func (api *API) AssignableSiteRoles(rw http.ResponseWriter, r *http.Request) { return } - customRoles := make([]rbac.Role, 0, len(dbCustomRoles)) - for _, customRole := range dbCustomRoles { - rbacRole, err := rolestore.ConvertDBRole(customRole) - if err == nil { - customRoles = append(customRoles, rbacRole) - } - } - - httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, rbac.SiteRoles(), customRoles)) + httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, rbac.SiteRoles(), dbCustomRoles)) } // assignableOrgRoles returns all org wide roles that can be assigned. @@ -133,18 +124,10 @@ func (api *API) assignableOrgRoles(rw http.ResponseWriter, r *http.Request) { return } - customRoles := make([]rbac.Role, 0, len(dbCustomRoles)) - for _, customRole := range dbCustomRoles { - rbacRole, err := rolestore.ConvertDBRole(customRole) - if err == nil { - customRoles = append(customRoles, rbacRole) - } - } - - httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles, customRoles)) + httpapi.Write(ctx, rw, http.StatusOK, assignableRoles(actorRoles.Roles, roles, dbCustomRoles)) } -func assignableRoles(actorRoles rbac.ExpandableRoles, roles []rbac.Role, customRoles []rbac.Role) []codersdk.AssignableRoles { +func assignableRoles(actorRoles rbac.ExpandableRoles, roles []rbac.Role, customRoles []database.CustomRole) []codersdk.AssignableRoles { assignable := make([]codersdk.AssignableRoles, 0) for _, role := range roles { // The member role is implied, and not assignable. @@ -154,7 +137,7 @@ func assignableRoles(actorRoles rbac.ExpandableRoles, roles []rbac.Role, customR continue } assignable = append(assignable, codersdk.AssignableRoles{ - Role: db2sdk.Role(role), + Role: db2sdk.RBACRole(role), Assignable: rbac.CanAssignRole(actorRoles, role.Name), BuiltIn: true, }) diff --git a/coderd/roles_test.go b/coderd/roles_test.go index 6d4f4bb6fe789..1b1aa94c6025a 100644 --- a/coderd/roles_test.go +++ b/coderd/roles_test.go @@ -6,14 +6,15 @@ import ( "slices" "testing" + "github.com/google/uuid" "github.com/stretchr/testify/require" "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/db2sdk" "github.com/coder/coder/v2/coderd/database/dbgen" "github.com/coder/coder/v2/coderd/rbac" "github.com/coder/coder/v2/coderd/rbac/policy" - "github.com/coder/coder/v2/coderd/rbac/rolestore" "github.com/coder/coder/v2/codersdk" "github.com/coder/coder/v2/testutil" ) @@ -170,21 +171,23 @@ func TestListCustomRoles(t *testing.T) { owner := coderdtest.CreateFirstUser(t, client) const roleName = "random_role" - dbgen.CustomRole(t, db, must(rolestore.ConvertRoleToDB(rbac.Role{ - Name: rbac.RoleName(roleName, owner.OrganizationID.String()), + dbgen.CustomRole(t, db, database.CustomRole{ + Name: roleName, DisplayName: "Random Role", - Site: nil, - Org: map[string][]rbac.Permission{ - owner.OrganizationID.String(): { - { - Negate: false, - ResourceType: rbac.ResourceWorkspace.Type, - Action: policy.ActionRead, - }, + OrganizationID: uuid.NullUUID{ + UUID: owner.OrganizationID, + Valid: true, + }, + SitePermissions: nil, + OrgPermissions: []database.CustomRolePermission{ + { + Negate: false, + ResourceType: rbac.ResourceWorkspace.Type, + Action: policy.ActionRead, }, }, - User: nil, - }))) + UserPermissions: nil, + }) ctx := testutil.Context(t, testutil.WaitShort) roles, err := client.ListOrganizationRoles(ctx, owner.OrganizationID) @@ -199,7 +202,7 @@ func TestListCustomRoles(t *testing.T) { func convertRole(roleName string) codersdk.Role { role, _ := rbac.RoleByName(roleName) - return db2sdk.Role(role) + return db2sdk.RBACRole(role) } func convertRoles(assignableRoles map[string]bool) []codersdk.AssignableRoles { diff --git a/codersdk/roles.go b/codersdk/roles.go index 8b119e935a6c6..bfab6b15ae391 100644 --- a/codersdk/roles.go +++ b/codersdk/roles.go @@ -40,7 +40,7 @@ type Role struct { DisplayName string `json:"display_name" table:"display_name"` SitePermissions []Permission `json:"site_permissions" table:"site_permissions"` // OrganizationPermissions are specific for the organization in the field 'OrganizationID' above. - OrganizationPermissions []Permission `json:"organization_permissions" table:"org_permissions"` + OrganizationPermissions []Permission `json:"organization_permissions" table:"organization_permissions"` UserPermissions []Permission `json:"user_permissions" table:"user_permissions"` } diff --git a/enterprise/coderd/roles.go b/enterprise/coderd/roles.go index 448ec9f855cc0..b3a24a8a7779f 100644 --- a/enterprise/coderd/roles.go +++ b/enterprise/coderd/roles.go @@ -10,7 +10,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/rolestore" + "github.com/coder/coder/v2/coderd/rbac/policy" "github.com/coder/coder/v2/codersdk" ) @@ -59,27 +59,16 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, return codersdk.Role{}, false } - // Make sure all permissions inputted are valid according to our policy. - rbacRole := db2sdk.RoleToRBAC(role) - args, err := rolestore.ConvertRoleToDB(rbacRole) - if err != nil { - httpapi.Write(ctx, rw, http.StatusBadRequest, codersdk.Response{ - Message: "Invalid request", - Detail: err.Error(), - }) - return codersdk.Role{}, false - } - inserted, err := db.UpsertCustomRole(ctx, database.UpsertCustomRoleParams{ - Name: args.Name, - DisplayName: args.DisplayName, + Name: role.Name, + DisplayName: role.DisplayName, OrganizationID: uuid.NullUUID{ UUID: orgID, Valid: true, }, - SitePermissions: args.SitePermissions, - OrgPermissions: args.OrgPermissions, - UserPermissions: args.UserPermissions, + SitePermissions: db2sdk.List(role.SitePermissions, sdkPermissionToDB), + OrgPermissions: db2sdk.List(role.OrganizationPermissions, sdkPermissionToDB), + UserPermissions: db2sdk.List(role.UserPermissions, sdkPermissionToDB), }) if httpapi.Is404Error(err) { httpapi.ResourceNotFound(rw) @@ -93,14 +82,13 @@ func (h enterpriseCustomRoleHandler) PatchOrganizationRole(ctx context.Context, return codersdk.Role{}, false } - convertedInsert, err := rolestore.ConvertDBRole(inserted) - if err != nil { - httpapi.Write(ctx, rw, http.StatusInternalServerError, codersdk.Response{ - Message: "Permissions were updated, unable to read them back out of the database.", - Detail: err.Error(), - }) - return codersdk.Role{}, false - } + return db2sdk.Role(inserted), true +} - return db2sdk.Role(convertedInsert), true +func sdkPermissionToDB(p codersdk.Permission) database.CustomRolePermission { + return database.CustomRolePermission{ + Negate: p.Negate, + ResourceType: string(p.ResourceType), + Action: policy.Action(p.Action), + } }