coder · Emyrk · May 21, 2024 · May 16, 2024 · May 16, 2024 · May 21, 2024
diff --git a/coderd/apidoc/docs.go b/coderd/apidoc/docs.go
diff --git a/coderd/apidoc/swagger.json b/coderd/apidoc/swagger.json
diff --git a/coderd/database/dbauthz/dbauthz.go b/coderd/database/dbauthz/dbauthz.go
@@ -835,11 +835,12 @@ 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) {
+// TODO: Handle org scoped lookups
+func (q *querier) CustomRoles(ctx context.Context, arg database.CustomRolesParams) ([]database.CustomRole, error) {
 	if err := q.authorizeContext(ctx, policy.ActionRead, rbac.ResourceAssignRole); err != nil {
 		return nil, err
 	}
-	return q.db.CustomRolesByName(ctx, lookupRoles)
+	return q.db.CustomRoles(ctx, arg)
 }
 
 func (q *querier) DeleteAPIKeyByID(ctx context.Context, id string) error {

diff --git a/coderd/database/dbauthz/dbauthz_test.go b/coderd/database/dbauthz/dbauthz_test.go
@@ -1177,8 +1177,8 @@ 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("CustomRoles", s.Subtest(func(db database.Store, check *expects) {
+		check.Args(database.CustomRolesParams{}).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

diff --git a/coderd/database/dbmem/dbmem.go b/coderd/database/dbmem/dbmem.go
@@ -1175,18 +1175,26 @@ func (*FakeQuerier) CleanTailnetTunnels(context.Context) error {
 	return ErrUnimplemented
 }
 
-func (q *FakeQuerier) CustomRolesByName(_ context.Context, lookupRoles []string) ([]database.CustomRole, error) {
+func (q *FakeQuerier) CustomRoles(_ context.Context, arg database.CustomRolesParams) ([]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)
+		role := role
+		if len(arg.LookupRoles) > 0 {
+			if !slices.ContainsFunc(arg.LookupRoles, func(s string) bool {
+				return strings.EqualFold(s, role.Name)
+			}) {
+				continue
+			}
 		}
+
+		if arg.ExcludeOrgRoles && role.OrganizationID.Valid {
+			continue
+		}
+
+		found = append(found, role)
 	}
 
 	return found, nil

diff --git a/coderd/database/dbmetrics/dbmetrics.go b/coderd/database/dbmetrics/dbmetrics.go
diff --git a/coderd/database/dbmock/dbmock.go b/coderd/database/dbmock/dbmock.go
diff --git a/coderd/database/dump.sql b/coderd/database/dump.sql
diff --git a/coderd/database/migrations/000212_custom_role_orgs.down.sql b/coderd/database/migrations/000212_custom_role_orgs.down.sql
@@ -0,0 +1,3 @@
+ALTER TABLE custom_roles
+	-- This column is nullable, meaning no organization scope
+	DROP COLUMN organization_id;
diff --git a/coderd/database/migrations/000212_custom_role_orgs.up.sql b/coderd/database/migrations/000212_custom_role_orgs.up.sql
@@ -0,0 +1,5 @@
+ALTER TABLE custom_roles
+	-- This column is nullable, meaning no organization scope
+	ADD COLUMN organization_id uuid;
+
+COMMENT ON COLUMN custom_roles.organization_id IS 'Roles can optionally be scoped to an organization'
diff --git a/coderd/database/models.go b/coderd/database/models.go
diff --git a/coderd/database/querier.go b/coderd/database/querier.go
diff --git a/coderd/database/queries.sql.go b/coderd/database/queries.sql.go
diff --git a/coderd/database/queries/roles.sql b/coderd/database/queries/roles.sql
@@ -1,14 +1,23 @@
--- name: CustomRolesByName :many
+-- name: CustomRoles :many
 SELECT
 	*
 FROM
 	custom_roles
 WHERE
+  true
+  -- Lookup roles filter
+  AND CASE WHEN array_length(@lookup_roles :: text[], 1) > 0  THEN
 	-- Case insensitive
 	name ILIKE ANY(@lookup_roles :: text [])
+    ELSE true
+  END
+  -- Org scoping filter, to only fetch site wide roles
+  AND CASE WHEN @exclude_org_roles :: boolean  THEN
+	organization_id IS null
+	ELSE true
+  END
 ;
 
-
 -- name: UpsertCustomRole :one
 INSERT INTO
 	custom_roles (

diff --git a/coderd/httpapi/name.go b/coderd/httpapi/name.go
@@ -38,7 +38,7 @@ func UsernameFrom(str string) string {
 }
 
 // NameValid returns whether the input string is a valid name.
-// It is a generic validator for any name (user, workspace, template, etc.).
+// It is a generic validator for any name (user, workspace, template, role name, etc.).
 func NameValid(str string) error {
 	if len(str) > 32 {
 		return xerrors.New("must be <= 32 characters")

diff --git a/coderd/rbac/rolestore/rolestore.go b/coderd/rbac/rolestore/rolestore.go
@@ -72,7 +72,10 @@ func Expand(ctx context.Context, db database.Store, names []string) (rbac.Roles,
 		// If some roles are missing from the database, they are omitted from
 		// the expansion. These roles are no-ops. Should we raise some kind of
 		// warning when this happens?
-		dbroles, err := db.CustomRolesByName(ctx, lookup)
+		dbroles, err := db.CustomRoles(ctx, database.CustomRolesParams{
+			LookupRoles:     lookup,
+			ExcludeOrgRoles: false,
+		})
 		if err != nil {
 			return nil, xerrors.Errorf("fetch custom roles: %w", err)
 		}
@@ -81,7 +84,7 @@ func Expand(ctx context.Context, db database.Store, names []string) (rbac.Roles,
 		for _, dbrole := range dbroles {
 			converted, err := ConvertDBRole(dbrole)
 			if err != nil {
-				return nil, xerrors.Errorf("convert db role %q: %w", dbrole, err)
+				return nil, xerrors.Errorf("convert db role %q: %w", dbrole.Name, err)
 			}
 			roles = append(roles, converted)
 			cache.Store(dbrole.Name, converted)