fix: Force v2 permission checks on user listing

# Which Problems Are Solved

When the feature flag for enabling permission checks v2 is disabled, a user without permission could list users across instances and get the total number of users available.

# How the Problems Are Solved

Disregard the state of the feature flag and always enforce permission checks v2 on v2 APIs.

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>
(cherry picked from commit 826039c620)

(cherry picked from commit 0e17d0005a)

internal/api/grpc/user/v2/integration_test/query_test.go

@@ -4,7 +4,6 @@ package user_test
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"slices"
 	"testing"
@@ -17,61 +16,11 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"github.com/zitadel/zitadel/internal/integration"
-	"github.com/zitadel/zitadel/pkg/grpc/feature/v2"
 	"github.com/zitadel/zitadel/pkg/grpc/object/v2"
 	"github.com/zitadel/zitadel/pkg/grpc/session/v2"
 	"github.com/zitadel/zitadel/pkg/grpc/user/v2"
 )
 
-var (
-	permissionCheckV2SetFlagInital bool
-	permissionCheckV2SetFlag       bool
-)
-
-type permissionCheckV2SettingsStruct struct {
-	TestNamePrependString string
-	SetFlag               bool
-}
-
-var permissionCheckV2Settings []permissionCheckV2SettingsStruct = []permissionCheckV2SettingsStruct{
-	{
-		SetFlag:               false,
-		TestNamePrependString: "permission_check_v2 IS NOT SET" + " ",
-	},
-	{
-		SetFlag:               true,
-		TestNamePrependString: "permission_check_v2 IS SET" + " ",
-	},
-}
-
-func setPermissionCheckV2Flag(t *testing.T, setFlag bool) {
-	if permissionCheckV2SetFlagInital && permissionCheckV2SetFlag == setFlag {
-		return
-	}
-
-	_, err := Instance.Client.FeatureV2.SetInstanceFeatures(IamCTX, &feature.SetInstanceFeaturesRequest{
-		PermissionCheckV2: &setFlag,
-	})
-	require.NoError(t, err)
-
-	var flagSet bool
-	for i := 0; !flagSet || i < 6; i++ {
-		res, err := Instance.Client.FeatureV2.GetInstanceFeatures(IamCTX, &feature.GetInstanceFeaturesRequest{})
-		require.NoError(t, err)
-		if res.PermissionCheckV2.Enabled == setFlag {
-			flagSet = true
-			continue
-		}
-		time.Sleep(10 * time.Second)
-	}
-
-	if !flagSet {
-		require.NoError(t, errors.New("unable to set permission_check_v2 flag"))
-	}
-	permissionCheckV2SetFlagInital = true
-	permissionCheckV2SetFlag = setFlag
-}
-
 func TestServer_GetUserByID(t *testing.T) {
 	orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetUserByIDOrg-%s", gofakeit.AppName()), gofakeit.Email())
 	type args struct {
@@ -433,11 +382,6 @@ func createUserWithUserName(ctx context.Context, username string, orgID string,
 }
 
 func TestServer_ListUsers(t *testing.T) {
-	defer func() {
-		_, err := Instance.Client.FeatureV2.ResetInstanceFeatures(IamCTX, &feature.ResetInstanceFeaturesRequest{})
-		require.NoError(t, err)
-	}()
-
 	orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListUsersOrg-%s", gofakeit.AppName()), gofakeit.Email())
 	type args struct {
 		ctx context.Context
@@ -1119,7 +1063,7 @@ func TestServer_ListUsers(t *testing.T) {
 			},
 			want: &user.ListUsersResponse{
 				Details: &object.ListDetails{
-					TotalResult: 0,
+					TotalResult: 1,
 					Timestamp:   timestamppb.Now(),
 				},
 				SortingColumn: 0,
@@ -1130,65 +1074,54 @@ func TestServer_ListUsers(t *testing.T) {
 			},
 		},
 	}
-	for _, f := range permissionCheckV2Settings {
-		f := f
-		for _, tt := range tests {
-			t.Run(f.TestNamePrependString+tt.name, func(t *testing.T) {
-				setPermissionCheckV2Flag(t, f.SetFlag)
-				infos := tt.args.dep(IamCTX, tt.args.req)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			infos := tt.args.dep(IamCTX, tt.args.req)
 
-				retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 10*time.Minute)
-				require.EventuallyWithT(t, func(ttt *assert.CollectT) {
-					got, err := Client.ListUsers(tt.args.ctx, tt.args.req)
-					if tt.wantErr {
-						require.Error(ttt, err)
-						return
-					}
-					require.NoError(ttt, err)
+			retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 10*time.Minute)
+			require.EventuallyWithT(t, func(ttt *assert.CollectT) {
+				got, err := Client.ListUsers(tt.args.ctx, tt.args.req)
+				if tt.wantErr {
+					require.Error(ttt, err)
+					return
+				}
+				require.NoError(ttt, err)
 
-					// always only give back dependency infos which are required for the response
-					require.Len(ttt, tt.want.Result, len(infos))
-					if assert.Len(ttt, got.Result, len(tt.want.Result)) {
-						tt.want.Details.TotalResult = got.Details.TotalResult
-
-						// fill in userid and username as it is generated
-						for i := range infos {
-							if tt.want.Result[i] == nil {
-								continue
-							}
-							tt.want.Result[i].UserId = infos[i].UserID
-							tt.want.Result[i].Username = infos[i].Username
-							tt.want.Result[i].PreferredLoginName = infos[i].Username
-							tt.want.Result[i].LoginNames = []string{infos[i].Username}
-							if human := tt.want.Result[i].GetHuman(); human != nil {
-								human.Email.Email = infos[i].Username
-								human.Phone.Phone = infos[i].Phone
-								if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil {
-									human.PasswordChanged = infos[i].Changed
-								}
-							}
-							tt.want.Result[i].Details = infos[i].Details
+				// always only give back dependency infos which are required for the response
+				require.Len(ttt, tt.want.Result, len(infos))
+				if assert.Len(ttt, got.Result, len(tt.want.Result)) {
+					// fill in userid and username as it is generated
+					for i := range infos {
+						if tt.want.Result[i] == nil {
+							continue
 						}
-						for i := range tt.want.Result {
-							if tt.want.Result[i] == nil {
-								continue
+						tt.want.Result[i].UserId = infos[i].UserID
+						tt.want.Result[i].Username = infos[i].Username
+						tt.want.Result[i].PreferredLoginName = infos[i].Username
+						tt.want.Result[i].LoginNames = []string{infos[i].Username}
+						if human := tt.want.Result[i].GetHuman(); human != nil {
+							human.Email.Email = infos[i].Username
+							human.Phone.Phone = infos[i].Phone
+							if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil {
+								human.PasswordChanged = infos[i].Changed
 							}
-							assert.EqualExportedValues(ttt, got.Result[i], tt.want.Result[i])
 						}
+						tt.want.Result[i].Details = infos[i].Details
 					}
-					integration.AssertListDetails(ttt, tt.want, got)
-				}, retryDuration, tick, "timeout waiting for expected user result")
-			})
-		}
+					for i := range tt.want.Result {
+						if tt.want.Result[i] == nil {
+							continue
+						}
+						assert.EqualExportedValues(ttt, got.Result[i], tt.want.Result[i])
+					}
+				}
+				integration.AssertListDetails(ttt, tt.want, got)
+			}, retryDuration, tick, "timeout waiting for expected user result")
+		})
 	}
 }
 
 func TestServer_SystemUsers_ListUsers(t *testing.T) {
-	defer func() {
-		_, err := Instance.Client.FeatureV2.ResetInstanceFeatures(IamCTX, &feature.ResetInstanceFeaturesRequest{})
-		require.NoError(t, err)
-	}()
-
 	org1 := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListUsersOrg-%s", gofakeit.AppName()), gofakeit.Email())
 	org2 := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListUsersOrg-%s", gofakeit.AppName()), "org2@zitadel.com")
 	org3 := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListUsersOrg-%s", gofakeit.AppName()), gofakeit.Email())
@@ -1239,38 +1172,33 @@ func TestServer_SystemUsers_ListUsers(t *testing.T) {
 		},
 	}
 
-	for _, f := range permissionCheckV2Settings {
-		f := f
-		for _, tt := range tests {
-			t.Run(f.TestNamePrependString+tt.name, func(t *testing.T) {
-				setPermissionCheckV2Flag(t, f.SetFlag)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.ctx, 1*time.Minute)
+			require.EventuallyWithT(t, func(ttt *assert.CollectT) {
+				got, err := Client.ListUsers(tt.ctx, tt.req)
+				require.NoError(ttt, err)
 
-				retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.ctx, 1*time.Minute)
-				require.EventuallyWithT(t, func(ttt *assert.CollectT) {
-					got, err := Client.ListUsers(tt.ctx, tt.req)
-					require.NoError(ttt, err)
+				if tt.checkNumberOfUsersReturned {
+					require.Equal(t, len(tt.expectedFoundUsernames), len(got.Result))
+				}
 
-					if tt.checkNumberOfUsersReturned {
-						require.Equal(t, len(tt.expectedFoundUsernames), len(got.Result))
-					}
-
-					if tt.expectedFoundUsernames != nil {
-						for _, user := range got.Result {
-							for i, username := range tt.expectedFoundUsernames {
-								if username == user.Username {
-									tt.expectedFoundUsernames = tt.expectedFoundUsernames[i+1:]
-									break
-								}
-							}
-							if len(tt.expectedFoundUsernames) == 0 {
-								return
+				if tt.expectedFoundUsernames != nil {
+					for _, user := range got.Result {
+						for i, username := range tt.expectedFoundUsernames {
+							if username == user.Username {
+								tt.expectedFoundUsernames = tt.expectedFoundUsernames[i+1:]
+								break
 							}
 						}
-						require.FailNow(t, "unable to find all users with specified usernames")
+						if len(tt.expectedFoundUsernames) == 0 {
+							return
+						}
 					}
-				}, retryDuration, tick, "timeout waiting for expected user result")
-			})
-		}
+					require.FailNow(t, "unable to find all users with specified usernames")
+				}
+			}, retryDuration, tick, "timeout waiting for expected user result")
+		})
 	}
 }
 

internal/api/grpc/user/v2beta/integration_test/query_test.go

@@ -4,7 +4,6 @@ package user_test
 
 import (
 	"context"
-	"errors"
 	"fmt"
 	"slices"
 	"testing"
@@ -17,7 +16,6 @@ import (
 	"google.golang.org/protobuf/types/known/timestamppb"
 
 	"github.com/zitadel/zitadel/internal/integration"
-	"github.com/zitadel/zitadel/pkg/grpc/feature/v2"
 	"github.com/zitadel/zitadel/pkg/grpc/object/v2"
 	object_v2beta "github.com/zitadel/zitadel/pkg/grpc/object/v2beta"
 	"github.com/zitadel/zitadel/pkg/grpc/session/v2"
@@ -32,55 +30,6 @@ func detailsV2ToV2beta(obj *object.Details) *object_v2beta.Details {
 	}
 }
 
-var (
-	permissionCheckV2SetFlagInital bool
-	permissionCheckV2SetFlag       bool
-)
-
-type permissionCheckV2SettingsStruct struct {
-	TestNamePrependString string
-	SetFlag               bool
-}
-
-var permissionCheckV2Settings []permissionCheckV2SettingsStruct = []permissionCheckV2SettingsStruct{
-	{
-		SetFlag:               false,
-		TestNamePrependString: "permission_check_v2 IS NOT SET" + " ",
-	},
-	{
-		SetFlag:               true,
-		TestNamePrependString: "permission_check_v2 IS SET" + " ",
-	},
-}
-
-func setPermissionCheckV2Flag(t *testing.T, setFlag bool) {
-	if permissionCheckV2SetFlagInital && permissionCheckV2SetFlag == setFlag {
-		return
-	}
-
-	_, err := Instance.Client.FeatureV2.SetInstanceFeatures(IamCTX, &feature.SetInstanceFeaturesRequest{
-		PermissionCheckV2: &setFlag,
-	})
-	require.NoError(t, err)
-
-	var flagSet bool
-	for i := 0; !flagSet || i < 6; i++ {
-		res, err := Instance.Client.FeatureV2.GetInstanceFeatures(IamCTX, &feature.GetInstanceFeaturesRequest{})
-		require.NoError(t, err)
-		if res.PermissionCheckV2.Enabled == setFlag {
-			flagSet = true
-			continue
-		}
-		time.Sleep(10 * time.Second)
-	}
-
-	if !flagSet {
-		require.NoError(t, errors.New("unable to set permission_check_v2 flag"))
-	}
-	permissionCheckV2SetFlagInital = true
-	permissionCheckV2SetFlag = setFlag
-}
-
 func TestServer_GetUserByID(t *testing.T) {
 	orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("GetUserByIDOrg-%s", gofakeit.AppName()), gofakeit.Email())
 	type args struct {
@@ -433,11 +382,6 @@ func createUser(ctx context.Context, orgID string, passwordChangeRequired bool)
 }
 
 func TestServer_ListUsers(t *testing.T) {
-	defer func() {
-		_, err := Instance.Client.FeatureV2.ResetInstanceFeatures(IamCTX, &feature.ResetInstanceFeaturesRequest{})
-		require.NoError(t, err)
-	}()
-
 	orgResp := Instance.CreateOrganization(IamCTX, fmt.Sprintf("ListUsersOrg-%s", gofakeit.AppName()), gofakeit.Email())
 	type args struct {
 		ctx context.Context
@@ -1122,7 +1066,7 @@ func TestServer_ListUsers(t *testing.T) {
 			},
 			want: &user.ListUsersResponse{
 				Details: &object_v2beta.ListDetails{
-					TotalResult: 0,
+					TotalResult: 1,
 					Timestamp:   timestamppb.Now(),
 				},
 				SortingColumn: 0,
@@ -1133,59 +1077,52 @@ func TestServer_ListUsers(t *testing.T) {
 			},
 		},
 	}
-	for _, f := range permissionCheckV2Settings {
-		f := f
-		for _, tt := range tests {
-			t.Run(f.TestNamePrependString+tt.name, func(t *testing.T) {
-				setPermissionCheckV2Flag(t, f.SetFlag)
-				infos := tt.args.dep(IamCTX, tt.args.req)
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			infos := tt.args.dep(IamCTX, tt.args.req)
 
-				// retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 10*time.Minute)
-				retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 20*time.Second)
-				require.EventuallyWithT(t, func(ttt *assert.CollectT) {
-					got, err := Client.ListUsers(tt.args.ctx, tt.args.req)
-					if tt.wantErr {
-						require.Error(ttt, err)
-						return
-					}
-					require.NoError(ttt, err)
+			// retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 10*time.Minute)
+			retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 20*time.Second)
+			require.EventuallyWithT(t, func(ttt *assert.CollectT) {
+				got, err := Client.ListUsers(tt.args.ctx, tt.args.req)
+				if tt.wantErr {
+					require.Error(ttt, err)
+					return
+				}
+				require.NoError(ttt, err)
 
-					// always only give back dependency infos which are required for the response
-					require.Len(ttt, tt.want.Result, len(infos))
-					// always first check length, otherwise its failed anyway
-					if assert.Len(ttt, got.Result, len(tt.want.Result)) {
-						// totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions
-						tt.want.Details.TotalResult = got.Details.TotalResult
-
-						// fill in userid and username as it is generated
-						for i := range infos {
-							if tt.want.Result[i] == nil {
-								continue
-							}
-							tt.want.Result[i].UserId = infos[i].UserID
-							tt.want.Result[i].Username = infos[i].Username
-							tt.want.Result[i].PreferredLoginName = infos[i].Username
-							tt.want.Result[i].LoginNames = []string{infos[i].Username}
-							if human := tt.want.Result[i].GetHuman(); human != nil {
-								human.Email.Email = infos[i].Username
-								human.Phone.Phone = infos[i].Phone
-								if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil {
-									human.PasswordChanged = infos[i].Changed
-								}
-							}
-							tt.want.Result[i].Details = detailsV2ToV2beta(infos[i].Details)
+				// always only give back dependency infos which are required for the response
+				require.Len(ttt, tt.want.Result, len(infos))
+				// always first check length, otherwise its failed anyway
+				if assert.Len(ttt, got.Result, len(tt.want.Result)) {
+					// fill in userid and username as it is generated
+					for i := range infos {
+						if tt.want.Result[i] == nil {
+							continue
 						}
-						for i := range tt.want.Result {
-							if tt.want.Result[i] == nil {
-								continue
+						tt.want.Result[i].UserId = infos[i].UserID
+						tt.want.Result[i].Username = infos[i].Username
+						tt.want.Result[i].PreferredLoginName = infos[i].Username
+						tt.want.Result[i].LoginNames = []string{infos[i].Username}
+						if human := tt.want.Result[i].GetHuman(); human != nil {
+							human.Email.Email = infos[i].Username
+							human.Phone.Phone = infos[i].Phone
+							if tt.want.Result[i].GetHuman().GetPasswordChanged() != nil {
+								human.PasswordChanged = infos[i].Changed
 							}
-							assert.EqualExportedValues(ttt, got.Result[i], tt.want.Result[i])
 						}
+						tt.want.Result[i].Details = detailsV2ToV2beta(infos[i].Details)
 					}
-					integration.AssertListDetails(ttt, tt.want, got)
-				}, retryDuration, tick, "timeout waiting for expected user result")
-			})
-		}
+					for i := range tt.want.Result {
+						if tt.want.Result[i] == nil {
+							continue
+						}
+						assert.EqualExportedValues(ttt, got.Result[i], tt.want.Result[i])
+					}
+				}
+				integration.AssertListDetails(ttt, tt.want, got)
+			}, retryDuration, tick, "timeout waiting for expected user result")
+		})
 	}
 }
 

internal/query/user.go

@@ -5,7 +5,6 @@ import (
 	"database/sql"
 	_ "embed"
 	"errors"
-	"slices"
 	"strings"
 	"time"
 
@@ -124,14 +123,6 @@ type NotifyUser struct {
 	PasswordSet        bool
 }
 
-func usersCheckPermission(ctx context.Context, users *Users, permissionCheck domain.PermissionCheck) {
-	users.Users = slices.DeleteFunc(users.Users,
-		func(user *User) bool {
-			return userCheckPermission(ctx, user.ResourceOwner, user.ID, permissionCheck) != nil
-		},
-	)
-}
-
 type UserSearchQueries struct {
 	SearchRequest
 	Queries []SearchQuery
@@ -601,13 +592,11 @@ func (q *Queries) CountUsers(ctx context.Context, queries *UserSearchQueries) (c
 }
 
 func (q *Queries) SearchUsers(ctx context.Context, queries *UserSearchQueries, filterOrgIds string, permissionCheck domain.PermissionCheck) (*Users, error) {
-	users, err := q.searchUsers(ctx, queries, filterOrgIds, permissionCheck != nil && authz.GetFeatures(ctx).PermissionCheckV2)
+	users, err := q.searchUsers(ctx, queries, filterOrgIds, permissionCheck != nil)
 	if err != nil {
 		return nil, err
 	}
-	if permissionCheck != nil && !authz.GetFeatures(ctx).PermissionCheckV2 {
-		usersCheckPermission(ctx, users, permissionCheck)
-	}
+
 	return users, nil
 }
 

internal/query/user_test.go

@@ -10,7 +10,6 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"golang.org/x/text/language"
 
 	"github.com/zitadel/zitadel/internal/api/authz"
@@ -19,129 +18,6 @@ import (
 	"github.com/zitadel/zitadel/internal/zerrors"
 )
 
-func TestUser_usersCheckPermission(t *testing.T) {
-	type want struct {
-		users []*User
-	}
-	tests := []struct {
-		name        string
-		want        want
-		users       *Users
-		permissions []string
-	}{
-		{
-			"permissions for all users",
-			want{
-				users: []*User{
-					{ID: "first"}, {ID: "second"}, {ID: "third"},
-				},
-			},
-			&Users{
-				Users: []*User{
-					{ID: "first"}, {ID: "second"}, {ID: "third"},
-				},
-			},
-			[]string{"first", "second", "third"},
-		},
-		{
-			"permissions for one user, first",
-			want{
-				users: []*User{
-					{ID: "first"},
-				},
-			},
-			&Users{
-				Users: []*User{
-					{ID: "first"}, {ID: "second"}, {ID: "third"},
-				},
-			},
-			[]string{"first"},
-		},
-		{
-			"permissions for one user, second",
-			want{
-				users: []*User{
-					{ID: "second"},
-				},
-			},
-			&Users{
-				Users: []*User{
-					{ID: "first"}, {ID: "second"}, {ID: "third"},
-				},
-			},
-			[]string{"second"},
-		},
-		{
-			"permissions for one user, third",
-			want{
-				users: []*User{
-					{ID: "third"},
-				},
-			},
-			&Users{
-				Users: []*User{
-					{ID: "first"}, {ID: "second"}, {ID: "third"},
-				},
-			},
-			[]string{"third"},
-		},
-		{
-			"permissions for two users, first",
-			want{
-				users: []*User{
-					{ID: "first"}, {ID: "third"},
-				},
-			},
-			&Users{
-				Users: []*User{
-					{ID: "first"}, {ID: "second"}, {ID: "third"},
-				},
-			},
-			[]string{"first", "third"},
-		},
-		{
-			"permissions for two users, second",
-			want{
-				users: []*User{
-					{ID: "second"}, {ID: "third"},
-				},
-			},
-			&Users{
-				Users: []*User{
-					{ID: "first"}, {ID: "second"}, {ID: "third"},
-				},
-			},
-			[]string{"second", "third"},
-		},
-		{
-			"no permissions",
-			want{
-				users: []*User{},
-			},
-			&Users{
-				Users: []*User{
-					{ID: "first"}, {ID: "second"}, {ID: "third"},
-				},
-			},
-			[]string{},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			checkPermission := func(ctx context.Context, permission, orgID, resourceID string) (err error) {
-				for _, perm := range tt.permissions {
-					if resourceID == perm {
-						return nil
-					}
-				}
-				return errors.New("failed")
-			}
-			usersCheckPermission(context.Background(), tt.users, checkPermission)
-			require.Equal(t, tt.want.users, tt.users.Users)
-		})
-	}
-}
-
 func TestUser_userCheckPermission(t *testing.T) {
 	type args struct {
 		ctxData       string