chore: remove organization_id suffix from org_member roles in database (#13473)

Organization member's table is already scoped to an organization.
Rolename should avoid having the org_id appended.

Wipes all existing organization role assignments, which should not be used anyway.

coderd/database/db2sdk/db2sdk.go

@@ -204,13 +204,6 @@ func Group(group database.Group, members []database.User) codersdk.Group {
 	}
 }
 
-func SlimRole(role rbac.Role) codersdk.SlimRole {
-	return codersdk.SlimRole{
-		DisplayName: role.DisplayName,
-		Name:        role.Name,
-	}
-}
-
 func TemplateInsightsParameters(parameterRows []database.GetTemplateParameterInsightsRow) ([]codersdk.TemplateParameterUsage, error) {
 	// Use a stable sort, similarly to how we would sort in the query, note that
 	// we don't sort in the query because order varies depending on the table
@@ -525,6 +518,19 @@ func ProvisionerDaemon(dbDaemon database.ProvisionerDaemon) codersdk.Provisioner
 	return result
 }
 
+func SlimRole(role rbac.Role) codersdk.SlimRole {
+	roleName, orgIDStr, err := rbac.RoleSplit(role.Name)
+	if err != nil {
+		roleName = role.Name
+	}
+
+	return codersdk.SlimRole{
+		DisplayName:    role.DisplayName,
+		Name:           roleName,
+		OrganizationID: orgIDStr,
+	}
+}
+
 func RBACRole(role rbac.Role) codersdk.Role {
 	roleName, orgIDStr, err := rbac.RoleSplit(role.Name)
 	if err != nil {

coderd/database/dbauthz/customroles_test.go

@@ -153,7 +153,7 @@ func TestUpsertCustomRoles(t *testing.T) {
 				UUID:  uuid.New(),
 				Valid: true,
 			},
-			subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID.UUID)),
+			subject: merge(canAssignRole, rbac.ScopedRoleOrgAdmin(orgID.UUID)),
 			org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},
 			}),
@@ -162,7 +162,7 @@ func TestUpsertCustomRoles(t *testing.T) {
 		{
 			name: "user-escalation",
 			// These roles do not grant user perms
-			subject: merge(canAssignRole, rbac.RoleOrgAdmin(orgID.UUID)),
+			subject: merge(canAssignRole, rbac.ScopedRoleOrgAdmin(orgID.UUID)),
 			user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},
 			}),
@@ -190,7 +190,7 @@ func TestUpsertCustomRoles(t *testing.T) {
 		},
 		{
 			name:           "read-workspace-in-org",
-			subject:        merge(canAssignRole, rbac.RoleOrgAdmin(orgID.UUID)),
+			subject:        merge(canAssignRole, rbac.ScopedRoleOrgAdmin(orgID.UUID)),
 			organizationID: orgID,
 			org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},

coderd/database/dbauthz/dbauthz.go

@@ -2472,7 +2472,7 @@ func (q *querier) InsertOrganization(ctx context.Context, arg database.InsertOrg
 
 func (q *querier) InsertOrganizationMember(ctx context.Context, arg database.InsertOrganizationMemberParams) (database.OrganizationMember, error) {
 	// All roles are added roles. Org member is always implied.
-	addedRoles := append(arg.Roles, rbac.RoleOrgMember(arg.OrganizationID))
+	addedRoles := append(arg.Roles, rbac.ScopedRoleOrgMember(arg.OrganizationID))
 	err := q.canAssignRoles(ctx, &arg.OrganizationID, addedRoles, []string{})
 	if err != nil {
 		return database.OrganizationMember{}, err
@@ -2847,8 +2847,22 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb
 		return database.OrganizationMember{}, err
 	}
 
+	// The 'rbac' package expects role names to be scoped.
+	// Convert the argument roles for validation.
+	scopedGranted := make([]string, 0, len(arg.GrantedRoles))
+	for _, grantedRole := range arg.GrantedRoles {
+		// This check is a developer safety check. Old code might try to invoke this code path with
+		// organization id suffixes. Catch this and return a nice error so it can be fixed.
+		_, foundOrg, _ := rbac.RoleSplit(grantedRole)
+		if foundOrg != "" {
+			return database.OrganizationMember{}, xerrors.Errorf("attempt to assign a role %q, remove the ':<organization_id> suffix", grantedRole)
+		}
+
+		scopedGranted = append(scopedGranted, rbac.RoleName(grantedRole, arg.OrgID.String()))
+	}
+
 	// The org member role is always implied.
-	impliedTypes := append(arg.GrantedRoles, rbac.RoleOrgMember(arg.OrgID))
+	impliedTypes := append(scopedGranted, rbac.ScopedRoleOrgMember(arg.OrgID))
 	added, removed := rbac.ChangeRoleSet(member.Roles, impliedTypes)
 	err = q.canAssignRoles(ctx, &arg.OrgID, added, removed)
 	if err != nil {

coderd/database/dbauthz/dbauthz_test.go

@@ -636,7 +636,7 @@ func (s *MethodTestSuite) TestOrganization() {
 		check.Args(database.InsertOrganizationMemberParams{
 			OrganizationID: o.ID,
 			UserID:         u.ID,
-			Roles:          []string{rbac.RoleOrgAdmin(o.ID)},
+			Roles:          []string{rbac.ScopedRoleOrgAdmin(o.ID)},
 		}).Asserts(
 			rbac.ResourceAssignRole.InOrg(o.ID), policy.ActionAssign,
 			rbac.ResourceOrganizationMember.InOrg(o.ID).WithID(u.ID), policy.ActionCreate)
@@ -664,7 +664,7 @@ func (s *MethodTestSuite) TestOrganization() {
 		mem := dbgen.OrganizationMember(s.T(), db, database.OrganizationMember{
 			OrganizationID: o.ID,
 			UserID:         u.ID,
-			Roles:          []string{rbac.RoleOrgAdmin(o.ID)},
+			Roles:          []string{rbac.ScopedRoleOrgAdmin(o.ID)},
 		})
 		out := mem
 		out.Roles = []string{}

coderd/database/dbmem/dbmem.go

@@ -1997,7 +1997,9 @@ func (q *FakeQuerier) GetAuthorizationUserRoles(_ context.Context, userID uuid.U
 
 	for _, mem := range q.organizationMembers {
 		if mem.UserID == userID {
-			roles = append(roles, mem.Roles...)
+			for _, orgRole := range mem.Roles {
+				roles = append(roles, orgRole+":"+mem.OrganizationID.String())
+			}
 			roles = append(roles, "organization-member:"+mem.OrganizationID.String())
 		}
 	}

coderd/database/dump.sql

@@ -576,7 +576,7 @@ CREATE TABLE organization_members (
     organization_id uuid NOT NULL,
     created_at timestamp with time zone NOT NULL,
     updated_at timestamp with time zone NOT NULL,
-    roles text[] DEFAULT '{organization-member}'::text[] NOT NULL
+    roles text[] DEFAULT '{}'::text[] NOT NULL
 );
 
 CREATE TABLE organizations (

coderd/database/migrations/000215_scoped_org_db_roles.down.sql

@@ -0,0 +1 @@
+ALTER TABLE ONLY organization_members ALTER COLUMN roles SET DEFAULT '{organization-member}';

coderd/database/migrations/000215_scoped_org_db_roles.up.sql

@@ -0,0 +1,7 @@
+-- The default was 'organization-member', but we imply that in the
+-- 'GetAuthorizationUserRoles' query.
+ALTER TABLE ONLY organization_members ALTER COLUMN roles SET DEFAULT '{}';
+
+-- No one should be using organization roles yet. If they are, the names in the
+-- database are now incorrect. Just remove them all.
+UPDATE organization_members SET roles = '{}';

coderd/database/queries.sql.go

@@ -8432,12 +8432,14 @@ SELECT
 		array_append(users.rbac_roles, 'member'),
 		(
 			SELECT
-				array_agg(org_roles)
+				-- The roles are returned as a flat array, org scoped and site side.
+				-- Concatenating the organization id scopes the organization roles.
+				array_agg(org_roles || ':' || organization_members.organization_id::text)
 			FROM
 				organization_members,
-				-- All org_members get the org-member role for their orgs
+				-- All org_members get the organization-member role for their orgs
 				unnest(
-					array_append(roles, 'organization-member:' || organization_members.organization_id::text)
+					array_append(roles, 'organization-member')
 				) AS org_roles
 			WHERE
 				user_id = users.id

coderd/database/queries/users.sql

@@ -227,12 +227,14 @@ SELECT
 		array_append(users.rbac_roles, 'member'),
 		(
 			SELECT
-				array_agg(org_roles)
+				-- The roles are returned as a flat array, org scoped and site side.
+				-- Concatenating the organization id scopes the organization roles.
+				array_agg(org_roles || ':' || organization_members.organization_id::text)
 			FROM
 				organization_members,
-				-- All org_members get the org-member role for their orgs
+				-- All org_members get the organization-member role for their orgs
 				unnest(
-					array_append(roles, 'organization-member:' || organization_members.organization_id::text)
+					array_append(roles, 'organization-member')
 				) AS org_roles
 			WHERE
 				user_id = users.id