chore: add an unassign action for roles (#16728)

coderd/database/dbauthz/customroles_test.go

@@ -34,11 +34,12 @@ func TestInsertCustomRoles(t *testing.T) {
 		}
 	}
 
-	canAssignRole := rbac.Role{
+	canCreateCustomRole := rbac.Role{
 		Identifier:  rbac.RoleIdentifier{Name: "can-assign"},
 		DisplayName: "",
 		Site: rbac.Permissions(map[string][]policy.Action{
-			rbac.ResourceAssignRole.Type: {policy.ActionRead, policy.ActionCreate},
+			rbac.ResourceAssignRole.Type:    {policy.ActionRead},
+			rbac.ResourceAssignOrgRole.Type: {policy.ActionRead, policy.ActionCreate},
 		}),
 	}
 
@@ -61,17 +62,15 @@ func TestInsertCustomRoles(t *testing.T) {
 		return all
 	}
 
-	orgID := uuid.NullUUID{
-		UUID:  uuid.New(),
-		Valid: true,
-	}
+	orgID := uuid.New()
+
 	testCases := []struct {
 		name string
 
 		subject rbac.ExpandableRoles
 
 		// Perms to create on new custom role
-		organizationID uuid.NullUUID
+		organizationID uuid.UUID
 		site           []codersdk.Permission
 		org            []codersdk.Permission
 		user           []codersdk.Permission
@@ -79,19 +78,21 @@ func TestInsertCustomRoles(t *testing.T) {
 	}{
 		{
 			// No roles, so no assign role
-			name:          "no-roles",
-			subject:       rbac.RoleIdentifiers{},
-			errorContains: "forbidden",
+			name:           "no-roles",
+			organizationID: orgID,
+			subject:        rbac.RoleIdentifiers{},
+			errorContains:  "forbidden",
 		},
 		{
 			// This works because the new role has 0 perms
-			name:    "empty",
-			subject: merge(canAssignRole),
+			name:           "empty",
+			organizationID: orgID,
+			subject:        merge(canCreateCustomRole),
 		},
 		{
 			name:           "mixed-scopes",
-			subject:        merge(canAssignRole, rbac.RoleOwner()),
 			organizationID: orgID,
+			subject:        merge(canCreateCustomRole, rbac.RoleOwner()),
 			site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},
 			}),
@@ -101,27 +102,30 @@ func TestInsertCustomRoles(t *testing.T) {
 			errorContains: "organization roles specify site or user permissions",
 		},
 		{
-			name:    "invalid-action",
-			subject: merge(canAssignRole, rbac.RoleOwner()),
-			site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
+			name:           "invalid-action",
+			organizationID: orgID,
+			subject:        merge(canCreateCustomRole, rbac.RoleOwner()),
+			org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				// Action does not go with resource
 				codersdk.ResourceWorkspace: {codersdk.ActionViewInsights},
 			}),
 			errorContains: "invalid action",
 		},
 		{
-			name:    "invalid-resource",
-			subject: merge(canAssignRole, rbac.RoleOwner()),
-			site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
+			name:           "invalid-resource",
+			organizationID: orgID,
+			subject:        merge(canCreateCustomRole, rbac.RoleOwner()),
+			org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				"foobar": {codersdk.ActionViewInsights},
 			}),
 			errorContains: "invalid resource",
 		},
 		{
 			// Not allowing these at this time.
-			name:    "negative-permission",
-			subject: merge(canAssignRole, rbac.RoleOwner()),
-			site: []codersdk.Permission{
+			name:           "negative-permission",
+			organizationID: orgID,
+			subject:        merge(canCreateCustomRole, rbac.RoleOwner()),
+			org: []codersdk.Permission{
 				{
 					Negate:       true,
 					ResourceType: codersdk.ResourceWorkspace,
@@ -131,89 +135,69 @@ func TestInsertCustomRoles(t *testing.T) {
 			errorContains: "no negative permissions",
 		},
 		{
-			name:    "wildcard", // not allowed
-			subject: merge(canAssignRole, rbac.RoleOwner()),
-			site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
+			name:           "wildcard", // not allowed
+			organizationID: orgID,
+			subject:        merge(canCreateCustomRole, rbac.RoleOwner()),
+			org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceWorkspace: {"*"},
 			}),
 			errorContains: "no wildcard symbols",
 		},
 		// escalation checks
 		{
-			name:    "read-workspace-escalation",
-			subject: merge(canAssignRole),
-			site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
+			name:           "read-workspace-escalation",
+			organizationID: orgID,
+			subject:        merge(canCreateCustomRole),
+			org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},
 			}),
 			errorContains: "not allowed to grant this permission",
 		},
 		{
-			name: "read-workspace-outside-org",
-			organizationID: uuid.NullUUID{
-				UUID:  uuid.New(),
-				Valid: true,
-			},
-			subject: merge(canAssignRole, rbac.ScopedRoleOrgAdmin(orgID.UUID)),
+			name:           "read-workspace-outside-org",
+			organizationID: uuid.New(),
+			subject:        merge(canCreateCustomRole, rbac.ScopedRoleOrgAdmin(orgID)),
 			org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},
 			}),
-			errorContains: "forbidden",
+			errorContains: "not allowed to grant this permission",
 		},
 		{
 			name: "user-escalation",
 			// These roles do not grant user perms
-			subject: merge(canAssignRole, rbac.ScopedRoleOrgAdmin(orgID.UUID)),
+			organizationID: orgID,
+			subject:        merge(canCreateCustomRole, rbac.ScopedRoleOrgAdmin(orgID)),
 			user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},
 			}),
-			errorContains: "not allowed to grant this permission",
+			errorContains: "organization roles specify site or user permissions",
 		},
 		{
-			name:    "template-admin-escalation",
-			subject: merge(canAssignRole, rbac.RoleTemplateAdmin()),
+			name:           "site-escalation",
+			organizationID: orgID,
+			subject:        merge(canCreateCustomRole, rbac.RoleTemplateAdmin()),
 			site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
-				codersdk.ResourceWorkspace:        {codersdk.ActionRead},   // ok!
 				codersdk.ResourceDeploymentConfig: {codersdk.ActionUpdate}, // not ok!
 			}),
-			user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
-				codersdk.ResourceWorkspace: {codersdk.ActionRead}, // ok!
-			}),
-			errorContains: "deployment_config",
+			errorContains: "organization roles specify site or user permissions",
 		},
 		// ok!
 		{
-			name:    "read-workspace-template-admin",
-			subject: merge(canAssignRole, rbac.RoleTemplateAdmin()),
-			site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
+			name:           "read-workspace-template-admin",
+			organizationID: orgID,
+			subject:        merge(canCreateCustomRole, rbac.RoleTemplateAdmin()),
+			org: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},
 			}),
 		},
 		{
 			name:           "read-workspace-in-org",
-			subject:        merge(canAssignRole, rbac.ScopedRoleOrgAdmin(orgID.UUID)),
 			organizationID: orgID,
+			subject:        merge(canCreateCustomRole, rbac.ScopedRoleOrgAdmin(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: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
-				codersdk.ResourceWorkspace: {codersdk.ActionRead},
-			}),
-		},
-		{
-			name:    "site+user-perms",
-			subject: merge(canAssignRole, rbac.RoleMember(), rbac.RoleTemplateAdmin()),
-			site: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
-				codersdk.ResourceWorkspace: {codersdk.ActionRead},
-			}),
-			user: codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
-				codersdk.ResourceWorkspace: {codersdk.ActionRead},
-			}),
-		},
 	}
 
 	for _, tc := range testCases {
@@ -234,7 +218,7 @@ func TestInsertCustomRoles(t *testing.T) {
 			_, err := az.InsertCustomRole(ctx, database.InsertCustomRoleParams{
 				Name:            "test-role",
 				DisplayName:     "",
-				OrganizationID:  tc.organizationID,
+				OrganizationID:  uuid.NullUUID{UUID: tc.organizationID, Valid: true},
 				SitePermissions: db2sdk.List(tc.site, convertSDKPerm),
 				OrgPermissions:  db2sdk.List(tc.org, convertSDKPerm),
 				UserPermissions: db2sdk.List(tc.user, convertSDKPerm),
@@ -249,11 +233,11 @@ func TestInsertCustomRoles(t *testing.T) {
 					LookupRoles: []database.NameOrganizationPair{
 						{
 							Name:           "test-role",
-							OrganizationID: tc.organizationID.UUID,
+							OrganizationID: tc.organizationID,
 						},
 					},
 					ExcludeOrgRoles: false,
-					OrganizationID:  uuid.UUID{},
+					OrganizationID:  uuid.Nil,
 				})
 				require.NoError(t, err)
 				require.Len(t, roles, 1)

coderd/database/dbauthz/dbauthz.go

@@ -747,7 +747,7 @@ func (*querier) convertToDeploymentRoles(names []string) []rbac.RoleIdentifier {
 }
 
 // canAssignRoles handles assigning built in and custom roles.
-func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, removed []rbac.RoleIdentifier) error {
+func (q *querier) canAssignRoles(ctx context.Context, orgID uuid.UUID, added, removed []rbac.RoleIdentifier) error {
 	actor, ok := ActorFromContext(ctx)
 	if !ok {
 		return NoActorError
@@ -755,12 +755,14 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
 
 	roleAssign := rbac.ResourceAssignRole
 	shouldBeOrgRoles := false
-	if orgID != nil {
-		roleAssign = rbac.ResourceAssignOrgRole.InOrg(*orgID)
+	if orgID != uuid.Nil {
+		roleAssign = rbac.ResourceAssignOrgRole.InOrg(orgID)
 		shouldBeOrgRoles = true
 	}
 
-	grantedRoles := append(added, removed...)
+	grantedRoles := make([]rbac.RoleIdentifier, 0, len(added)+len(removed))
+	grantedRoles = append(grantedRoles, added...)
+	grantedRoles = append(grantedRoles, removed...)
 	customRoles := make([]rbac.RoleIdentifier, 0)
 	// Validate that the roles being assigned are valid.
 	for _, r := range grantedRoles {
@@ -774,11 +776,11 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
 		}
 
 		if shouldBeOrgRoles {
-			if orgID == nil {
+			if orgID == uuid.Nil {
 				return xerrors.Errorf("should never happen, orgID is nil, but trying to assign an organization role")
 			}
 
-			if r.OrganizationID != *orgID {
+			if r.OrganizationID != orgID {
 				return xerrors.Errorf("attempted to assign role from a different org, role %q to %q", r, orgID.String())
 			}
 		}
@@ -824,7 +826,7 @@ func (q *querier) canAssignRoles(ctx context.Context, orgID *uuid.UUID, added, r
 	}
 
 	if len(removed) > 0 {
-		if err := q.authorizeContext(ctx, policy.ActionDelete, roleAssign); err != nil {
+		if err := q.authorizeContext(ctx, policy.ActionUnassign, roleAssign); err != nil {
 			return err
 		}
 	}
@@ -1124,11 +1126,15 @@ func (q *querier) CleanTailnetTunnels(ctx context.Context) error {
 	return q.db.CleanTailnetTunnels(ctx)
 }
 
-// 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 {
+	roleObject := rbac.ResourceAssignRole
+	if arg.OrganizationID != uuid.Nil {
+		roleObject = rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID)
+	}
+	if err := q.authorizeContext(ctx, policy.ActionRead, roleObject); err != nil {
 		return nil, err
 	}
+
 	return q.db.CustomRoles(ctx, arg)
 }
 
@@ -1185,14 +1191,11 @@ func (q *querier) DeleteCryptoKey(ctx context.Context, arg database.DeleteCrypto
 }
 
 func (q *querier) DeleteCustomRole(ctx context.Context, arg database.DeleteCustomRoleParams) error {
-	if arg.OrganizationID.UUID != uuid.Nil {
-		if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
-			return err
-		}
-	} else {
-		if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignRole); err != nil {
-			return err
-		}
+	if !arg.OrganizationID.Valid || arg.OrganizationID.UUID == uuid.Nil {
+		return NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")}
+	}
+	if err := q.authorizeContext(ctx, policy.ActionDelete, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
+		return err
 	}
 
 	return q.db.DeleteCustomRole(ctx, arg)
@@ -3009,14 +3012,11 @@ func (q *querier) InsertCryptoKey(ctx context.Context, arg database.InsertCrypto
 
 func (q *querier) InsertCustomRole(ctx context.Context, arg database.InsertCustomRoleParams) (database.CustomRole, error) {
 	// Org and site role upsert share the same query. So switch the assertion based on the org uuid.
-	if arg.OrganizationID.UUID != uuid.Nil {
-		if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
-			return database.CustomRole{}, err
-		}
-	} else {
-		if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignRole); err != nil {
-			return database.CustomRole{}, err
-		}
+	if !arg.OrganizationID.Valid || arg.OrganizationID.UUID == uuid.Nil {
+		return database.CustomRole{}, NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")}
+	}
+	if err := q.authorizeContext(ctx, policy.ActionCreate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
+		return database.CustomRole{}, err
 	}
 
 	if err := q.customRoleCheck(ctx, database.CustomRole{
@@ -3146,7 +3146,7 @@ func (q *querier) InsertOrganizationMember(ctx context.Context, arg database.Ins
 
 	// All roles are added roles. Org member is always implied.
 	addedRoles := append(orgRoles, rbac.ScopedRoleOrgMember(arg.OrganizationID))
-	err = q.canAssignRoles(ctx, &arg.OrganizationID, addedRoles, []rbac.RoleIdentifier{})
+	err = q.canAssignRoles(ctx, arg.OrganizationID, addedRoles, []rbac.RoleIdentifier{})
 	if err != nil {
 		return database.OrganizationMember{}, err
 	}
@@ -3270,7 +3270,7 @@ func (q *querier) InsertTemplateVersionWorkspaceTag(ctx context.Context, arg dat
 func (q *querier) InsertUser(ctx context.Context, arg database.InsertUserParams) (database.User, error) {
 	// Always check if the assigned roles can actually be assigned by this actor.
 	impliedRoles := append([]rbac.RoleIdentifier{rbac.RoleMember()}, q.convertToDeploymentRoles(arg.RBACRoles)...)
-	err := q.canAssignRoles(ctx, nil, impliedRoles, []rbac.RoleIdentifier{})
+	err := q.canAssignRoles(ctx, uuid.Nil, impliedRoles, []rbac.RoleIdentifier{})
 	if err != nil {
 		return database.User{}, err
 	}
@@ -3608,14 +3608,11 @@ func (q *querier) UpdateCryptoKeyDeletesAt(ctx context.Context, arg database.Upd
 }
 
 func (q *querier) UpdateCustomRole(ctx context.Context, arg database.UpdateCustomRoleParams) (database.CustomRole, error) {
-	if arg.OrganizationID.UUID != uuid.Nil {
-		if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
-			return database.CustomRole{}, err
-		}
-	} else {
-		if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceAssignRole); err != nil {
-			return database.CustomRole{}, err
-		}
+	if !arg.OrganizationID.Valid || arg.OrganizationID.UUID == uuid.Nil {
+		return database.CustomRole{}, NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")}
+	}
+	if err := q.authorizeContext(ctx, policy.ActionUpdate, rbac.ResourceAssignOrgRole.InOrg(arg.OrganizationID.UUID)); err != nil {
+		return database.CustomRole{}, err
 	}
 
 	if err := q.customRoleCheck(ctx, database.CustomRole{
@@ -3695,7 +3692,7 @@ func (q *querier) UpdateMemberRoles(ctx context.Context, arg database.UpdateMemb
 	impliedTypes := append(scopedGranted, rbac.ScopedRoleOrgMember(arg.OrgID))
 
 	added, removed := rbac.ChangeRoleSet(originalRoles, impliedTypes)
-	err = q.canAssignRoles(ctx, &arg.OrgID, added, removed)
+	err = q.canAssignRoles(ctx, arg.OrgID, added, removed)
 	if err != nil {
 		return database.OrganizationMember{}, err
 	}
@@ -4102,7 +4099,7 @@ func (q *querier) UpdateUserRoles(ctx context.Context, arg database.UpdateUserRo
 	impliedTypes := append(q.convertToDeploymentRoles(arg.GrantedRoles), rbac.RoleMember())
 	// If the changeset is nothing, less rbac checks need to be done.
 	added, removed := rbac.ChangeRoleSet(q.convertToDeploymentRoles(user.RBACRoles), impliedTypes)
-	err = q.canAssignRoles(ctx, nil, added, removed)
+	err = q.canAssignRoles(ctx, uuid.Nil, added, removed)
 	if err != nil {
 		return database.User{}, err
 	}

coderd/database/dbauthz/dbauthz_test.go

@@ -1011,7 +1011,7 @@ func (s *MethodTestSuite) TestOrganization() {
 			Asserts(
 				mem, policy.ActionRead,
 				rbac.ResourceAssignOrgRole.InOrg(o.ID), policy.ActionAssign, // org-mem
-				rbac.ResourceAssignOrgRole.InOrg(o.ID), policy.ActionDelete, // org-admin
+				rbac.ResourceAssignOrgRole.InOrg(o.ID), policy.ActionUnassign, // org-admin
 			).Returns(out)
 	}))
 }
@@ -1619,7 +1619,7 @@ func (s *MethodTestSuite) TestUser() {
 		}).Asserts(
 			u, policy.ActionRead,
 			rbac.ResourceAssignRole, policy.ActionAssign,
-			rbac.ResourceAssignRole, policy.ActionDelete,
+			rbac.ResourceAssignRole, policy.ActionUnassign,
 		).Returns(o)
 	}))
 	s.Run("AllUserIDs", s.Subtest(func(db database.Store, check *expects) {
@@ -1653,30 +1653,28 @@ func (s *MethodTestSuite) TestUser() {
 		check.Args(database.DeleteCustomRoleParams{
 			Name: customRole.Name,
 		}).Asserts(
-			rbac.ResourceAssignRole, policy.ActionDelete)
+		// fails immediately, missing organization id
+		).Errors(dbauthz.NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")})
 	}))
 	s.Run("Blank/UpdateCustomRole", s.Subtest(func(db database.Store, check *expects) {
 		dbtestutil.DisableForeignKeysAndTriggers(s.T(), db)
-		customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{})
+		customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{
+			OrganizationID: uuid.NullUUID{UUID: uuid.New(), Valid: true},
+		})
 		// Blank is no perms in the role
 		check.Args(database.UpdateCustomRoleParams{
 			Name:            customRole.Name,
 			DisplayName:     "Test Name",
+			OrganizationID:  customRole.OrganizationID,
 			SitePermissions: nil,
 			OrgPermissions:  nil,
 			UserPermissions: nil,
-		}).Asserts(rbac.ResourceAssignRole, policy.ActionUpdate).ErrorsWithPG(sql.ErrNoRows)
+		}).Asserts(rbac.ResourceAssignOrgRole.InOrg(customRole.OrganizationID.UUID), policy.ActionUpdate)
 	}))
 	s.Run("SitePermissions/UpdateCustomRole", s.Subtest(func(db database.Store, check *expects) {
-		customRole := dbgen.CustomRole(s.T(), db, database.CustomRole{
-			OrganizationID: uuid.NullUUID{
-				UUID:  uuid.Nil,
-				Valid: false,
-			},
-		})
 		check.Args(database.UpdateCustomRoleParams{
-			Name:           customRole.Name,
-			OrganizationID: customRole.OrganizationID,
+			Name:           "",
+			OrganizationID: uuid.NullUUID{UUID: uuid.Nil, Valid: false},
 			DisplayName:    "Test Name",
 			SitePermissions: db2sdk.List(codersdk.CreatePermissions(map[codersdk.RBACResource][]codersdk.RBACAction{
 				codersdk.ResourceTemplate: {codersdk.ActionCreate, codersdk.ActionRead, codersdk.ActionUpdate, codersdk.ActionDelete, codersdk.ActionViewInsights},
@@ -1686,17 +1684,8 @@ func (s *MethodTestSuite) TestUser() {
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},
 			}), convertSDKPerm),
 		}).Asserts(
-			// First check
-			rbac.ResourceAssignRole, policy.ActionUpdate,
-			// 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,
-		).ErrorsWithPG(sql.ErrNoRows)
+		// fails immediately, missing organization id
+		).Errors(dbauthz.NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")})
 	}))
 	s.Run("OrgPermissions/UpdateCustomRole", s.Subtest(func(db database.Store, check *expects) {
 		orgID := uuid.New()
@@ -1726,13 +1715,15 @@ func (s *MethodTestSuite) TestUser() {
 	}))
 	s.Run("Blank/InsertCustomRole", s.Subtest(func(db database.Store, check *expects) {
 		// Blank is no perms in the role
+		orgID := uuid.New()
 		check.Args(database.InsertCustomRoleParams{
 			Name:            "test",
 			DisplayName:     "Test Name",
+			OrganizationID:  uuid.NullUUID{UUID: orgID, Valid: true},
 			SitePermissions: nil,
 			OrgPermissions:  nil,
 			UserPermissions: nil,
-		}).Asserts(rbac.ResourceAssignRole, policy.ActionCreate)
+		}).Asserts(rbac.ResourceAssignOrgRole.InOrg(orgID), policy.ActionCreate)
 	}))
 	s.Run("SitePermissions/InsertCustomRole", s.Subtest(func(db database.Store, check *expects) {
 		check.Args(database.InsertCustomRoleParams{
@@ -1746,17 +1737,8 @@ func (s *MethodTestSuite) TestUser() {
 				codersdk.ResourceWorkspace: {codersdk.ActionRead},
 			}), convertSDKPerm),
 		}).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,
-		)
+		// fails immediately, missing organization id
+		).Errors(dbauthz.NotAuthorizedError{Err: xerrors.New("custom roles must belong to an organization")})
 	}))
 	s.Run("OrgPermissions/InsertCustomRole", s.Subtest(func(db database.Store, check *expects) {
 		orgID := uuid.New()

coderd/database/queries.sql.go

@@ -7775,25 +7775,25 @@ SELECT
 FROM
 	custom_roles
 WHERE
-  true
-  -- @lookup_roles will filter for exact (role_name, org_id) pairs
-  -- To do this manually in SQL, you can construct an array and cast it:
-  -- cast(ARRAY[('customrole','ece79dac-926e-44ca-9790-2ff7c5eb6e0c')] AS name_organization_pair[])
-  AND CASE WHEN array_length($1 :: name_organization_pair[], 1) > 0  THEN
-    -- Using 'coalesce' to avoid troubles with null literals being an empty string.
-	(name, coalesce(organization_id, '00000000-0000-0000-0000-000000000000' ::uuid)) = ANY ($1::name_organization_pair[])
-    ELSE true
-  END
-  -- This allows fetching all roles, or just site wide roles
-  AND CASE WHEN $2 :: boolean  THEN
-	organization_id IS null
+	true
+	-- @lookup_roles will filter for exact (role_name, org_id) pairs
+	-- To do this manually in SQL, you can construct an array and cast it:
+	-- cast(ARRAY[('customrole','ece79dac-926e-44ca-9790-2ff7c5eb6e0c')] AS name_organization_pair[])
+	AND CASE WHEN array_length($1 :: name_organization_pair[], 1) > 0  THEN
+		-- Using 'coalesce' to avoid troubles with null literals being an empty string.
+		(name, coalesce(organization_id, '00000000-0000-0000-0000-000000000000' ::uuid)) = ANY ($1::name_organization_pair[])
 	ELSE true
-  END
-  -- Allows fetching all roles to a particular organization
-  AND CASE WHEN $3 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid  THEN
-      organization_id = $3
-    ELSE true
-  END
+	END
+	-- This allows fetching all roles, or just site wide roles
+	AND CASE WHEN $2 :: boolean  THEN
+		organization_id IS null
+	ELSE true
+	END
+	-- Allows fetching all roles to a particular organization
+	AND CASE WHEN $3 :: uuid != '00000000-0000-0000-0000-000000000000'::uuid  THEN
+		organization_id = $3
+	ELSE true
+	END
 `
 
 type CustomRolesParams struct {
@@ -7866,16 +7866,16 @@ INSERT INTO
 	updated_at
 )
 VALUES (
-		   -- Always force lowercase names
-		   lower($1),
-		   $2,
-		   $3,
-		   $4,
-		   $5,
-		   $6,
-		   now(),
-		   now()
-	   )
+	-- Always force lowercase names
+	lower($1),
+	$2,
+	$3,
+	$4,
+	$5,
+	$6,
+	now(),
+	now()
+)
 RETURNING name, display_name, site_permissions, org_permissions, user_permissions, created_at, updated_at, organization_id, id
 `
 

coderd/database/queries/roles.sql

@@ -4,25 +4,25 @@ SELECT
 FROM
 	custom_roles
 WHERE
-  true
-  -- @lookup_roles will filter for exact (role_name, org_id) pairs
-  -- To do this manually in SQL, you can construct an array and cast it:
-  -- cast(ARRAY[('customrole','ece79dac-926e-44ca-9790-2ff7c5eb6e0c')] AS name_organization_pair[])
-  AND CASE WHEN array_length(@lookup_roles :: name_organization_pair[], 1) > 0  THEN
-    -- Using 'coalesce' to avoid troubles with null literals being an empty string.
-	(name, coalesce(organization_id, '00000000-0000-0000-0000-000000000000' ::uuid)) = ANY (@lookup_roles::name_organization_pair[])
-    ELSE true
-  END
-  -- This allows fetching all roles, or just site wide roles
-  AND CASE WHEN @exclude_org_roles :: boolean  THEN
-	organization_id IS null
+	true
+	-- @lookup_roles will filter for exact (role_name, org_id) pairs
+	-- To do this manually in SQL, you can construct an array and cast it:
+	-- cast(ARRAY[('customrole','ece79dac-926e-44ca-9790-2ff7c5eb6e0c')] AS name_organization_pair[])
+	AND CASE WHEN array_length(@lookup_roles :: name_organization_pair[], 1) > 0  THEN
+		-- Using 'coalesce' to avoid troubles with null literals being an empty string.
+		(name, coalesce(organization_id, '00000000-0000-0000-0000-000000000000' ::uuid)) = ANY (@lookup_roles::name_organization_pair[])
 	ELSE true
-  END
-  -- Allows fetching all roles to a particular organization
-  AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid  THEN
-      organization_id = @organization_id
-    ELSE true
-  END
+	END
+	-- This allows fetching all roles, or just site wide roles
+	AND CASE WHEN @exclude_org_roles :: boolean  THEN
+		organization_id IS null
+	ELSE true
+	END
+	-- Allows fetching all roles to a particular organization
+	AND CASE WHEN @organization_id :: uuid != '00000000-0000-0000-0000-000000000000'::uuid  THEN
+		organization_id = @organization_id
+	ELSE true
+	END
 ;
 
 -- name: DeleteCustomRole :exec
@@ -46,16 +46,16 @@ INSERT INTO
 	updated_at
 )
 VALUES (
-		   -- Always force lowercase names
-		   lower(@name),
-		   @display_name,
-		   @organization_id,
-		   @site_permissions,
-		   @org_permissions,
-		   @user_permissions,
-		   now(),
-		   now()
-	   )
+	-- Always force lowercase names
+	lower(@name),
+	@display_name,
+	@organization_id,
+	@site_permissions,
+	@org_permissions,
+	@user_permissions,
+	now(),
+	now()
+)
 RETURNING *;
 
 -- name: UpdateCustomRole :one