chore: implement databased backend for custom roles (#13295)

Includes db schema and dbauthz layer for upserting custom roles. Unit test in `customroles_test.go` verify against escalating permissions through this feature.

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)
+			}
+		})
+	}
+}

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

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() {

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,