diff --git a/coderd/database/dbauthz/customroles_test.go b/coderd/database/dbauthz/customroles_test.go new file mode 100644 index 0000000000000..aaa2c7a34bbf3 --- /dev/null +++ b/coderd/database/dbauthz/customroles_test.go @@ -0,0 +1,258 @@ +package dbauthz_test + +import ( + "encoding/json" + "testing" + + "github.com/google/uuid" + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/require" + + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/coderdtest" + "github.com/coder/coder/v2/coderd/database" + "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/testutil" +) + +// TestUpsertCustomRoles verifies creating custom roles cannot escalate permissions. +func TestUpsertCustomRoles(t *testing.T) { + t.Parallel() + + userID := uuid.New() + subjectFromRoles := func(roles rbac.ExpandableRoles) rbac.Subject { + return rbac.Subject{ + FriendlyName: "Test user", + ID: userID.String(), + Roles: roles, + Groups: nil, + Scope: rbac.ScopeAll, + } + } + + canAssignRole := rbac.Role{ + Name: "can-assign", + DisplayName: "", + Site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceAssignRole.Type: {policy.ActionCreate}, + }), + } + + merge := func(u ...interface{}) rbac.Roles { + all := make([]rbac.Role, 0) + for _, v := range u { + v := v + switch t := v.(type) { + case rbac.Role: + all = append(all, t) + case rbac.ExpandableRoles: + all = append(all, must(t.Expand())...) + case string: + all = append(all, must(rbac.RoleByName(t))) + default: + panic("unknown type") + } + } + + return all + } + + orgID := uuid.New() + 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 + }{ + { + // No roles, so no assign role + name: "no-roles", + subject: rbac.RoleNames([]string{}), + errorContains: "forbidden", + }, + { + // This works because the new role has 0 perms + name: "empty", + subject: merge(canAssignRole), + }, + { + name: "mixed-scopes", + subject: merge(canAssignRole, rbac.RoleOwner()), + site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceWorkspace.Type: {policy.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{ + // Action does not go with resource + rbac.ResourceWorkspace.Type: {policy.ActionViewInsights}, + }), + errorContains: "invalid action", + }, + { + name: "invalid-resource", + subject: merge(canAssignRole, rbac.RoleOwner()), + site: rbac.Permissions(map[string][]policy.Action{ + "foobar": {policy.ActionViewInsights}, + }), + errorContains: "invalid resource", + }, + { + // Not allowing these at this time. + name: "negative-permission", + subject: merge(canAssignRole, rbac.RoleOwner()), + site: []rbac.Permission{ + { + Negate: true, + ResourceType: rbac.ResourceWorkspace.Type, + Action: policy.ActionRead, + }, + }, + errorContains: "no negative permissions", + }, + { + name: "wildcard", // not allowed + subject: merge(canAssignRole, rbac.RoleOwner()), + site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceWorkspace.Type: {policy.WildcardSymbol}, + }), + errorContains: "no wildcard symbols", + }, + // escalation checks + { + name: "read-workspace-escalation", + subject: merge(canAssignRole), + site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceWorkspace.Type: {policy.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}, + }), + }, + 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}, + }), + 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! + }), + user: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceWorkspace.Type: {policy.ActionRead}, // ok! + }), + errorContains: "deployment_config", + }, + // ok! + { + name: "read-workspace-template-admin", + subject: merge(canAssignRole, rbac.RoleTemplateAdmin()), + site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceWorkspace.Type: {policy.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: "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}, + }), + }, + { + name: "site+user-perms", + subject: merge(canAssignRole, rbac.RoleMember(), rbac.RoleTemplateAdmin()), + site: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceWorkspace.Type: {policy.ActionRead}, + }), + user: rbac.Permissions(map[string][]policy.Action{ + rbac.ResourceWorkspace.Type: {policy.ActionRead}, + }), + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + db := dbmem.New() + rec := &coderdtest.RecordingAuthorizer{ + Wrapped: rbac.NewAuthorizer(prometheus.NewRegistry()), + } + az := dbauthz.New(db, rec, slog.Make(), coderdtest.AccessControlStorePointer()) + + subject := subjectFromRoles(tc.subject) + ctx := testutil.Context(t, testutil.WaitMedium) + ctx = dbauthz.As(ctx, subject) + + _, 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)), + }) + if tc.errorContains != "" { + require.ErrorContains(t, err, tc.errorContains) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go index a096346f57064..92b9637e9ddf9 100644 --- a/coderd/database/dbauthz/dbauthz.go +++ b/coderd/database/dbauthz/dbauthz.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "strings" "sync/atomic" "time" @@ -17,6 +18,7 @@ import ( "cdr.dev/slog" "github.com/coder/coder/v2/coderd/rbac/policy" + "github.com/coder/coder/v2/coderd/rbac/rolestore" "github.com/coder/coder/v2/coderd/database" "github.com/coder/coder/v2/coderd/database/dbtime" @@ -580,6 +582,7 @@ func (q *querier) authorizeUpdateFileTemplate(ctx context.Context, file database } } +// canAssignRoles handles assigning built in and custom roles. func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, removed []string) error { actor, ok := ActorFromContext(ctx) if !ok { @@ -594,6 +597,7 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r } grantedRoles := append(added, removed...) + customRoles := make([]string, 0) // Validate that the roles being assigned are valid. for _, r := range grantedRoles { _, isOrgRole := rbac.IsOrgRole(r) @@ -606,7 +610,34 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r // All roles should be valid roles if _, err := rbac.RoleByName(r); err != nil { - return xerrors.Errorf("%q is not a supported role", r) + customRoles = append(customRoles, r) + } + } + + customRolesMap := make(map[string]struct{}, len(customRoles)) + for _, r := range customRoles { + customRolesMap[r] = struct{}{} + } + + if len(customRoles) > 0 { + expandedCustomRoles, err := q.CustomRolesByName(ctx, customRoles) + if err != nil { + return xerrors.Errorf("fetching custom roles: %w", err) + } + + // If the lists are not identical, then have a problem, as some roles + // provided do no exist. + if len(customRoles) != len(expandedCustomRoles) { + for _, role := range customRoles { + // Stop at the first one found. We could make a better error that + // returns them all, but then someone could pass in a large list to make us do + // a lot of loop iterations. + if !slices.ContainsFunc(expandedCustomRoles, func(customRole database.CustomRole) bool { + return strings.EqualFold(customRole.Name, role) + }) { + return xerrors.Errorf("%q is not a supported role", role) + } + } } } @@ -623,6 +654,11 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r } for _, roleName := range grantedRoles { + if _, isCustom := customRolesMap[roleName]; isCustom { + // For now, use a constant name so our static assign map still works. + roleName = rbac.CustomSiteRole() + } + if !rbac.CanAssignRole(actor.Roles, roleName) { return xerrors.Errorf("not authorized to assign role %q", roleName) } @@ -704,6 +740,31 @@ func (q *querier) authorizeTemplateInsights(ctx context.Context, templateIDs []u return nil } +// customRoleEscalationCheck checks to make sure the caller has every permission they are adding +// to a custom role. This prevents permission escalation. +func (q *querier) customRoleEscalationCheck(ctx context.Context, actor rbac.Subject, perm rbac.Permission, object rbac.Object) error { + if perm.Negate { + // Users do not need negative permissions. We can include it later if required. + return xerrors.Errorf("invalid permission for action=%q type=%q, no negative permissions", perm.Action, perm.ResourceType) + } + + if perm.Action == policy.WildcardSymbol || perm.ResourceType == policy.WildcardSymbol { + // It is possible to check for supersets with wildcards, but wildcards can also + // include resources and actions that do not exist today. Custom roles should only be allowed + // to include permissions for existing resources. + return xerrors.Errorf("invalid permission for action=%q type=%q, no wildcard symbols", perm.Action, perm.ResourceType) + } + + object.Type = perm.ResourceType + if err := q.auth.Authorize(ctx, actor, perm.Action, object); err != nil { + // This is a forbidden error, but we can provide more context. Since the user can create a role, just not + // with this perm. + return xerrors.Errorf("invalid permission for action=%q type=%q, not allowed to grant this permission", perm.Action, perm.ResourceType) + } + + return nil +} + func (q *querier) AcquireLock(ctx context.Context, id int64) error { return q.db.AcquireLock(ctx, id) } @@ -773,6 +834,13 @@ func (q *querier) CleanTailnetTunnels(ctx context.Context) error { return q.db.CleanTailnetTunnels(ctx) } +func (q *querier) CustomRolesByName(ctx context.Context, lookupRoles []string) ([]database.CustomRole, error) { + if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceAssignRole); err != nil { + return nil, err + } + return q.db.CustomRolesByName(ctx, lookupRoles) +} + func (q *querier) DeleteAPIKeyByID(ctx context.Context, id string) error { return deleteQ(q.log, q.auth, q.db.GetAPIKeyByID, q.db.DeleteAPIKeyByID)(ctx, id) } @@ -3291,6 +3359,78 @@ 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 + } + + // TODO: If this is an org role, check the org assign role type. + if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignRole); err != nil { + 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. + rbacRole, err := rolestore.ConvertDBRole(database.CustomRole{ + Name: arg.Name, + DisplayName: arg.DisplayName, + SitePermissions: arg.SitePermissions, + OrgPermissions: arg.OrgPermissions, + UserPermissions: arg.UserPermissions, + }) + 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 e8dcb2f8ee5bc..7d04a0d20a52e 100644 --- a/coderd/database/dbauthz/dbauthz_test.go +++ b/coderd/database/dbauthz/dbauthz_test.go @@ -1167,6 +1167,67 @@ func (s *MethodTestSuite) TestUser() { b := dbgen.User(s.T(), db, database.User{}) check.Args().Asserts(rbac.ResourceSystem, policy.ActionRead).Returns(slice.New(a.ID, b.ID)) })) + s.Run("CustomRolesByName", s.Subtest(func(db database.Store, check *expects) { + check.Args([]string{}).Asserts(rbac.ResourceAssignRole, policy.ActionRead).Returns([]database.CustomRole{}) + })) + s.Run("Blank/UpsertCustomRole", s.Subtest(func(db database.Store, check *expects) { + // Blank is no perms in the role + check.Args(database.UpsertCustomRoleParams{ + Name: "test", + DisplayName: "Test Name", + SitePermissions: []byte(`[]`), + OrgPermissions: []byte(`{}`), + UserPermissions: []byte(`[]`), + }).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}, + }))), + }).Asserts( + // First check + rbac.ResourceAssignRole, policy.ActionCreate, + // 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/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}, + }))), + }).Asserts( + // First check + rbac.ResourceAssignRole, policy.ActionCreate, + // Escalation checks + rbac.ResourceTemplate.InOrg(orgID), policy.ActionCreate, + rbac.ResourceTemplate.InOrg(orgID), policy.ActionRead, + + rbac.ResourceWorkspace.WithOwner(testActorID.String()), policy.ActionRead, + ) + })) } func (s *MethodTestSuite) TestWorkspace() { diff --git a/coderd/database/dbauthz/setup_test.go b/coderd/database/dbauthz/setup_test.go index 16829cdef669e..3385ca3f3240c 100644 --- a/coderd/database/dbauthz/setup_test.go +++ b/coderd/database/dbauthz/setup_test.go @@ -99,6 +99,8 @@ func (s *MethodTestSuite) TearDownSuite() { }) } +var testActorID = uuid.New() + // Subtest is a helper function that returns a function that can be passed to // s.Run(). This function will run the test case for the method that is being // tested. The check parameter is used to assert the results of the method. @@ -120,7 +122,7 @@ func (s *MethodTestSuite) Subtest(testCaseF func(db database.Store, check *expec } az := dbauthz.New(db, rec, slog.Make(), coderdtest.AccessControlStorePointer()) actor := rbac.Subject{ - ID: uuid.NewString(), + ID: testActorID.String(), Roles: rbac.RoleNames{rbac.RoleOwner()}, Groups: []string{}, Scope: rbac.ScopeAll, diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go index d1bbd6df49492..ea896b28641f4 100644 --- a/coderd/database/dbmem/dbmem.go +++ b/coderd/database/dbmem/dbmem.go @@ -75,6 +75,7 @@ func New() database.Store { workspaces: make([]database.Workspace, 0), licenses: make([]database.License, 0), workspaceProxies: make([]database.WorkspaceProxy, 0), + customRoles: make([]database.CustomRole, 0), locks: map[int64]struct{}{}, }, } @@ -179,6 +180,7 @@ type data struct { workspaceResources []database.WorkspaceResource workspaces []database.Workspace workspaceProxies []database.WorkspaceProxy + customRoles []database.CustomRole // Locks is a map of lock names. Any keys within the map are currently // locked. locks map[int64]struct{} @@ -1172,6 +1174,23 @@ func (*FakeQuerier) CleanTailnetTunnels(context.Context) error { return ErrUnimplemented } +func (q *FakeQuerier) CustomRolesByName(_ context.Context, lookupRoles []string) ([]database.CustomRole, error) { + q.mutex.Lock() + defer q.mutex.Unlock() + + found := make([]database.CustomRole, 0) + for _, role := range q.data.customRoles { + if slices.ContainsFunc(lookupRoles, func(s string) bool { + return strings.EqualFold(s, role.Name) + }) { + role := role + found = append(found, role) + } + } + + return found, nil +} + func (q *FakeQuerier) DeleteAPIKeyByID(_ context.Context, id string) error { q.mutex.Lock() defer q.mutex.Unlock() @@ -8258,6 +8277,39 @@ 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].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{ + Name: arg.Name, + DisplayName: arg.DisplayName, + 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 77ebfd6718757..4e0c2b8fed158 100644 --- a/coderd/database/dbmetrics/dbmetrics.go +++ b/coderd/database/dbmetrics/dbmetrics.go @@ -144,6 +144,13 @@ func (m metricsStore) CleanTailnetTunnels(ctx context.Context) error { return r0 } +func (m metricsStore) CustomRolesByName(ctx context.Context, lookupRoles []string) ([]database.CustomRole, error) { + start := time.Now() + r0, r1 := m.s.CustomRolesByName(ctx, lookupRoles) + m.queryLatencies.WithLabelValues("CustomRolesByName").Observe(time.Since(start).Seconds()) + return r0, r1 +} + func (m metricsStore) DeleteAPIKeyByID(ctx context.Context, id string) error { start := time.Now() err := m.s.DeleteAPIKeyByID(ctx, id) @@ -2153,6 +2160,13 @@ 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 e651c8301c933..69558e884c6a6 100644 --- a/coderd/database/dbmock/dbmock.go +++ b/coderd/database/dbmock/dbmock.go @@ -173,6 +173,21 @@ func (mr *MockStoreMockRecorder) CleanTailnetTunnels(arg0 any) *gomock.Call { return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CleanTailnetTunnels", reflect.TypeOf((*MockStore)(nil).CleanTailnetTunnels), arg0) } +// CustomRolesByName mocks base method. +func (m *MockStore) CustomRolesByName(arg0 context.Context, arg1 []string) ([]database.CustomRole, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "CustomRolesByName", arg0, arg1) + ret0, _ := ret[0].([]database.CustomRole) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// CustomRolesByName indicates an expected call of CustomRolesByName. +func (mr *MockStoreMockRecorder) CustomRolesByName(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "CustomRolesByName", reflect.TypeOf((*MockStore)(nil).CustomRolesByName), arg0, arg1) +} + // DeleteAPIKeyByID mocks base method. func (m *MockStore) DeleteAPIKeyByID(arg0 context.Context, arg1 string) error { m.ctrl.T.Helper() @@ -4507,6 +4522,21 @@ 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 ed400cf82198f..33a9ebbef8139 100644 --- a/coderd/database/dump.sql +++ b/coderd/database/dump.sql @@ -404,6 +404,18 @@ CREATE TABLE audit_logs ( resource_icon text NOT NULL ); +CREATE TABLE custom_roles ( + name text NOT NULL, + display_name text NOT NULL, + site_permissions jsonb DEFAULT '[]'::jsonb NOT NULL, + org_permissions jsonb DEFAULT '{}'::jsonb NOT NULL, + user_permissions jsonb DEFAULT '[]'::jsonb NOT NULL, + created_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL, + updated_at timestamp with time zone DEFAULT CURRENT_TIMESTAMP NOT NULL +); + +COMMENT ON TABLE custom_roles IS 'Custom roles allow dynamic roles expanded at runtime'; + CREATE TABLE dbcrypt_keys ( number integer NOT NULL, active_key_digest text, @@ -1398,6 +1410,9 @@ ALTER TABLE ONLY api_keys 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); + ALTER TABLE ONLY dbcrypt_keys ADD CONSTRAINT dbcrypt_keys_active_key_digest_key UNIQUE (active_key_digest); @@ -1606,6 +1621,8 @@ CREATE INDEX idx_audit_log_user_id ON audit_logs USING btree (user_id); CREATE INDEX idx_audit_logs_time_desc ON audit_logs USING btree ("time" DESC); +CREATE UNIQUE INDEX idx_custom_roles_name_lower ON custom_roles USING btree (lower(name)); + CREATE INDEX idx_organization_member_organization_id_uuid ON organization_members USING btree (organization_id); CREATE INDEX idx_organization_member_user_id_uuid ON organization_members USING btree (user_id); diff --git a/coderd/database/migrations/000209_custom_roles.down.sql b/coderd/database/migrations/000209_custom_roles.down.sql new file mode 100644 index 0000000000000..b0f9b2a8cc76c --- /dev/null +++ b/coderd/database/migrations/000209_custom_roles.down.sql @@ -0,0 +1,2 @@ +DROP INDEX IF EXISTS idx_custom_roles_name_lower; +DROP TABLE IF EXISTS custom_roles; diff --git a/coderd/database/migrations/000209_custom_roles.up.sql b/coderd/database/migrations/000209_custom_roles.up.sql new file mode 100644 index 0000000000000..b55788c16b955 --- /dev/null +++ b/coderd/database/migrations/000209_custom_roles.up.sql @@ -0,0 +1,26 @@ +CREATE TABLE custom_roles ( + -- name is globally unique. Org scoped roles have their orgid appended + -- like: "name":"organization-admin:bbe8c156-c61e-4d36-b91e-697c6b1477e8" + name text primary key, + -- display_name is the actual name of the role displayed to the user. + display_name text NOT NULL, + + -- Unfortunately these values are schemaless json documents. + -- If there was a permission table for these, that would involve + -- many necessary joins to accomplish this simple json. + + -- site_permissions is '[]Permission' + site_permissions jsonb NOT NULL default '[]', + -- org_permissions is 'map[][]Permission' + org_permissions jsonb NOT NULL default '{}', + -- user_permissions is '[]Permission' + user_permissions jsonb NOT NULL default '[]', + + -- extra convenience meta data. + created_at timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at timestamp with time zone NOT NULL DEFAULT CURRENT_TIMESTAMP +); + +-- Ensure no case variants of the same roles +CREATE UNIQUE INDEX idx_custom_roles_name_lower ON custom_roles USING btree (lower(name)); +COMMENT ON TABLE custom_roles IS 'Custom roles allow dynamic roles expanded at runtime'; diff --git a/coderd/database/migrations/testdata/fixtures/000209_custom_roles.up.sql b/coderd/database/migrations/testdata/fixtures/000209_custom_roles.up.sql new file mode 100644 index 0000000000000..c63e119523624 --- /dev/null +++ b/coderd/database/migrations/testdata/fixtures/000209_custom_roles.up.sql @@ -0,0 +1,20 @@ +INSERT INTO + custom_roles ( + name, + display_name, + site_permissions, + org_permissions, + user_permissions, + created_at, + updated_at +) +VALUES + ( + 'custom-role', + 'Custom Role', + '[{"negate":false,"resource_type":"deployment_config","action":"update"},{"negate":false,"resource_type":"workspace","action":"read"}]', + '{}', + '[{"negate":false,"resource_type":"workspace","action":"read"}]', + date_trunc('hour', NOW()), + date_trunc('hour', NOW()) + '30 minute'::interval + ); diff --git a/coderd/database/models.go b/coderd/database/models.go index 18587b05ade1a..33cf1c607939c 100644 --- a/coderd/database/models.go +++ b/coderd/database/models.go @@ -1781,6 +1781,17 @@ type AuditLog struct { ResourceIcon string `db:"resource_icon" json:"resource_icon"` } +// 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"` +} + // A table used to store the keys used to encrypt the database. type DBCryptKey struct { // An integer used to identify the key. diff --git a/coderd/database/querier.go b/coderd/database/querier.go index 405f86bf47688..01615a58e06bd 100644 --- a/coderd/database/querier.go +++ b/coderd/database/querier.go @@ -48,6 +48,7 @@ type sqlcQuerier interface { CleanTailnetCoordinators(ctx context.Context) error CleanTailnetLostPeers(ctx context.Context) error CleanTailnetTunnels(ctx context.Context) error + CustomRolesByName(ctx context.Context, lookupRoles []string) ([]CustomRole, error) DeleteAPIKeyByID(ctx context.Context, id string) error DeleteAPIKeysByUserID(ctx context.Context, userID uuid.UUID) error DeleteAllTailnetClientSubscriptions(ctx context.Context, arg DeleteAllTailnetClientSubscriptionsParams) error @@ -413,6 +414,7 @@ type sqlcQuerier interface { UpdateWorkspacesDormantDeletingAtByTemplateID(ctx context.Context, arg UpdateWorkspacesDormantDeletingAtByTemplateIDParams) 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/queries.sql.go b/coderd/database/queries.sql.go index f4e7d4d70e4b6..7a0b60478f79f 100644 --- a/coderd/database/queries.sql.go +++ b/coderd/database/queries.sql.go @@ -5553,6 +5553,107 @@ func (q *sqlQuerier) UpdateReplica(ctx context.Context, arg UpdateReplicaParams) return i, err } +const customRolesByName = `-- name: CustomRolesByName :many +SELECT + name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at +FROM + custom_roles +WHERE + -- Case insensitive + name ILIKE ANY($1 :: text []) +` + +func (q *sqlQuerier) CustomRolesByName(ctx context.Context, lookupRoles []string) ([]CustomRole, error) { + rows, err := q.db.QueryContext(ctx, customRolesByName, pq.Array(lookupRoles)) + if err != nil { + return nil, err + } + defer rows.Close() + var items []CustomRole + for rows.Next() { + var i CustomRole + if err := rows.Scan( + &i.Name, + &i.DisplayName, + &i.SitePermissions, + &i.OrgPermissions, + &i.UserPermissions, + &i.CreatedAt, + &i.UpdatedAt, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + +const upsertCustomRole = `-- name: UpsertCustomRole :one +INSERT INTO + custom_roles ( + name, + display_name, + site_permissions, + org_permissions, + user_permissions, + created_at, + updated_at +) +VALUES ( + -- Always force lowercase names + lower($1), + $2, + $3, + $4, + $5, + now(), + now() + ) +ON CONFLICT (name) + DO UPDATE SET + display_name = $2, + site_permissions = $3, + org_permissions = $4, + user_permissions = $5, + updated_at = now() +RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at +` + +type UpsertCustomRoleParams 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"` +} + +func (q *sqlQuerier) UpsertCustomRole(ctx context.Context, arg UpsertCustomRoleParams) (CustomRole, error) { + row := q.db.QueryRowContext(ctx, upsertCustomRole, + arg.Name, + arg.DisplayName, + arg.SitePermissions, + arg.OrgPermissions, + arg.UserPermissions, + ) + var i CustomRole + err := row.Scan( + &i.Name, + &i.DisplayName, + &i.SitePermissions, + &i.OrgPermissions, + &i.UserPermissions, + &i.CreatedAt, + &i.UpdatedAt, + ) + return i, err +} + const getAppSecurityKey = `-- name: GetAppSecurityKey :one SELECT value FROM site_configs WHERE key = 'app_signing_key' ` diff --git a/coderd/database/queries/roles.sql b/coderd/database/queries/roles.sql new file mode 100644 index 0000000000000..30ec437e1814e --- /dev/null +++ b/coderd/database/queries/roles.sql @@ -0,0 +1,41 @@ +-- name: CustomRolesByName :many +SELECT + * +FROM + custom_roles +WHERE + -- Case insensitive + name ILIKE ANY(@lookup_roles :: text []) +; + + +-- name: UpsertCustomRole :one +INSERT INTO + custom_roles ( + name, + display_name, + site_permissions, + org_permissions, + user_permissions, + created_at, + updated_at +) +VALUES ( + -- Always force lowercase names + lower(@name), + @display_name, + @site_permissions, + @org_permissions, + @user_permissions, + now(), + now() + ) +ON CONFLICT (name) + DO UPDATE SET + display_name = @display_name, + site_permissions = @site_permissions, + org_permissions = @org_permissions, + user_permissions = @user_permissions, + updated_at = now() +RETURNING * +; diff --git a/coderd/database/unique_constraint.go b/coderd/database/unique_constraint.go index 9db8af72c8cf6..9dfc8c124aa75 100644 --- a/coderd/database/unique_constraint.go +++ b/coderd/database/unique_constraint.go @@ -9,6 +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); 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); @@ -74,6 +75,7 @@ const ( UniqueWorkspaceResourcesPkey UniqueConstraint = "workspace_resources_pkey" // ALTER TABLE ONLY workspace_resources ADD CONSTRAINT workspace_resources_pkey PRIMARY KEY (id); UniqueWorkspacesPkey UniqueConstraint = "workspaces_pkey" // ALTER TABLE ONLY workspaces ADD CONSTRAINT workspaces_pkey PRIMARY KEY (id); UniqueIndexAPIKeyName UniqueConstraint = "idx_api_key_name" // CREATE UNIQUE INDEX idx_api_key_name ON api_keys USING btree (user_id, token_name) WHERE (login_type = 'token'::login_type); + UniqueIndexCustomRolesNameLower UniqueConstraint = "idx_custom_roles_name_lower" // CREATE UNIQUE INDEX idx_custom_roles_name_lower ON custom_roles USING btree (lower(name)); UniqueIndexOrganizationName UniqueConstraint = "idx_organization_name" // CREATE UNIQUE INDEX idx_organization_name ON organizations USING btree (name); UniqueIndexOrganizationNameLower UniqueConstraint = "idx_organization_name_lower" // CREATE UNIQUE INDEX idx_organization_name_lower ON organizations USING btree (lower(name)); UniqueIndexProvisionerDaemonsNameOwnerKey UniqueConstraint = "idx_provisioner_daemons_name_owner_key" // CREATE UNIQUE INDEX idx_provisioner_daemons_name_owner_key ON provisioner_daemons USING btree (name, lower(COALESCE((tags ->> 'owner'::text), ''::text))); diff --git a/coderd/rbac/object_gen.go b/coderd/rbac/object_gen.go index 57ec0982a15ae..9ab848d795b1c 100644 --- a/coderd/rbac/object_gen.go +++ b/coderd/rbac/object_gen.go @@ -37,7 +37,8 @@ var ( // ResourceAssignRole // Valid Actions // - "ActionAssign" :: ability to assign roles - // - "ActionDelete" :: ability to delete roles + // - "ActionCreate" :: ability to create/delete/edit custom roles + // - "ActionDelete" :: ability to unassign roles // - "ActionRead" :: view what roles are assignable ResourceAssignRole = Object{ Type: "assign_role", diff --git a/coderd/rbac/policy/policy.go b/coderd/rbac/policy/policy.go index 26afb0e011ca7..2d3213264a514 100644 --- a/coderd/rbac/policy/policy.go +++ b/coderd/rbac/policy/policy.go @@ -209,7 +209,8 @@ var RBACPermissions = map[string]PermissionDefinition{ Actions: map[Action]ActionDefinition{ ActionAssign: actDef("ability to assign roles"), ActionRead: actDef("view what roles are assignable"), - ActionDelete: actDef("ability to delete roles"), + ActionDelete: actDef("ability to unassign roles"), + ActionCreate: actDef("ability to create/delete/edit custom roles"), }, }, "assign_org_role": { diff --git a/coderd/rbac/roles.go b/coderd/rbac/roles.go index fbac8ddf5379d..7086e2fe0e2a4 100644 --- a/coderd/rbac/roles.go +++ b/coderd/rbac/roles.go @@ -20,6 +20,10 @@ const ( templateAdmin string = "template-admin" userAdmin string = "user-admin" auditor string = "auditor" + // customSiteRole is a placeholder for all custom site roles. + // This is used for what roles can assign other roles. + // TODO: Make this more dynamic to allow other roles to grant. + customSiteRole string = "custom-site-role" orgAdmin string = "organization-admin" orgMember string = "organization-member" @@ -52,6 +56,8 @@ func RoleOwner() string { return roleName(owner, "") } +func CustomSiteRole() string { return roleName(customSiteRole, "") } + func RoleTemplateAdmin() string { return roleName(templateAdmin, "") } @@ -320,22 +326,24 @@ func ReloadBuiltinRoles(opts *RoleOptions) { // map[actor_role][assign_role] var assignRoles = map[string]map[string]bool{ "system": { - owner: true, - auditor: true, - member: true, - orgAdmin: true, - orgMember: true, - templateAdmin: true, - userAdmin: true, + owner: true, + auditor: true, + member: true, + orgAdmin: true, + orgMember: true, + templateAdmin: true, + userAdmin: true, + customSiteRole: true, }, owner: { - owner: true, - auditor: true, - member: true, - orgAdmin: true, - orgMember: true, - templateAdmin: true, - userAdmin: true, + owner: true, + auditor: true, + member: true, + orgAdmin: true, + orgMember: true, + templateAdmin: true, + userAdmin: true, + customSiteRole: true, }, userAdmin: { member: true, diff --git a/coderd/rbac/roles_test.go b/coderd/rbac/roles_test.go index fe589449b8884..d90f045284c5b 100644 --- a/coderd/rbac/roles_test.go +++ b/coderd/rbac/roles_test.go @@ -248,6 +248,15 @@ func TestRolePermissions(t *testing.T) { false: {otherOrgAdmin, otherOrgMember, memberMe, userAdmin}, }, }, + { + Name: "CreateCustomRole", + Actions: []policy.Action{policy.ActionCreate}, + Resource: rbac.ResourceAssignRole, + AuthorizeMap: map[bool][]authSubject{ + true: {owner}, + false: {userAdmin, orgAdmin, orgMemberMe, otherOrgAdmin, otherOrgMember, memberMe, templateAdmin}, + }, + }, { Name: "RoleAssignment", Actions: []policy.Action{policy.ActionAssign, policy.ActionDelete}, @@ -380,7 +389,7 @@ func TestRolePermissions(t *testing.T) { }, // Some admin style resources { - Name: "Licences", + Name: "Licenses", Actions: []policy.Action{policy.ActionCreate, policy.ActionRead, policy.ActionDelete}, Resource: rbac.ResourceLicense, AuthorizeMap: map[bool][]authSubject{ diff --git a/coderd/rbac/rolestore/rolestore.go b/coderd/rbac/rolestore/rolestore.go new file mode 100644 index 0000000000000..5cf69bcd41fde --- /dev/null +++ b/coderd/rbac/rolestore/rolestore.go @@ -0,0 +1,37 @@ +package rolestore + +import ( + "encoding/json" + + "golang.org/x/xerrors" + + "github.com/coder/coder/v2/coderd/database" + "github.com/coder/coder/v2/coderd/rbac" +) + +func ConvertDBRole(dbRole database.CustomRole) (rbac.Role, error) { + role := rbac.Role{ + Name: dbRole.Name, + DisplayName: dbRole.DisplayName, + Site: nil, + 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 +}