Merge commit from fork

* Force v2 permission checks on user listing

* Remove duplicated tests

* cleanup and fix tests

---------

Co-authored-by: Livio Spring <livio.a@gmail.com>

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

@@ -4,7 +4,6 @@ package user_test
 
 import (
 	"context"
-	"errors"
 	"slices"
 	"testing"
 	"time"
@@ -15,7 +14,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"
@@ -30,55 +28,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, integration.OrganizationName(), integration.Email())
 	type args struct {
@@ -431,11 +380,6 @@ func createUser(ctx context.Context, orgID string, passwordChangeRequired bool)
 }
 
 func TestServer_ListUsers(t *testing.T) {
-	t.Cleanup(func() {
-		_, err := Instance.Client.FeatureV2.ResetInstanceFeatures(IamCTX, &feature.ResetInstanceFeaturesRequest{})
-		require.NoError(t, err)
-	})
-
 	orgResp := Instance.CreateOrganization(IamCTX, integration.OrganizationName(), integration.Email())
 	type args struct {
 		ctx context.Context
@@ -1111,7 +1055,6 @@ func TestServer_ListUsers(t *testing.T) {
 				func(ctx context.Context, request *user_v2beta.ListUsersRequest) userAttrs {
 					orgRespForOrgTests := Instance.CreateOrganization(IamCTX, integration.OrganizationName(), integration.Email())
 					orgRespForOrgTests2 := Instance.CreateOrganization(IamCTX, integration.OrganizationName(), integration.Email())
-					// info := createUser(ctx, orgRespForOrgTests.OrganizationId, false)
 					createUser(ctx, orgRespForOrgTests.OrganizationId, false)
 					request.Queries = []*user_v2beta.SearchQuery{}
 					request.Queries = append(request.Queries, OrganizationIdQuery(orgRespForOrgTests2.OrganizationId))
@@ -1120,7 +1063,7 @@ func TestServer_ListUsers(t *testing.T) {
 			},
 			want: &user_v2beta.ListUsersResponse{
 				Details: &object_v2beta.ListDetails{
-					TotalResult: 0,
+					TotalResult: 1,
 					Timestamp:   timestamppb.Now(),
 				},
 				SortingColumn: 0,
@@ -1131,58 +1074,51 @@ func TestServer_ListUsers(t *testing.T) {
 			},
 		},
 	}
-	for _, f := range permissionCheckV2Settings {
-		for _, tc := range tt {
-			t.Run(f.TestNamePrependString+tc.name, func(t1 *testing.T) {
-				setPermissionCheckV2Flag(t1, f.SetFlag)
-				infos := tc.args.dep(IamCTX, tc.args.req)
+	for _, tc := range tt {
+		t.Run(tc.name, func(t1 *testing.T) {
+			infos := tc.args.dep(IamCTX, tc.args.req)
 
-				// retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tt.args.ctx, 10*time.Minute)
-				retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tc.args.ctx, time.Minute)
-				require.EventuallyWithT(t1, func(ttt *assert.CollectT) {
-					got, err := Client.ListUsers(tc.args.ctx, tc.args.req)
-					if tc.wantErr {
-						require.Error(ttt, err)
-						return
-					}
-					require.NoError(ttt, err)
+			retryDuration, tick := integration.WaitForAndTickWithMaxDuration(tc.args.ctx, time.Minute)
+			require.EventuallyWithT(t1, func(ttt *assert.CollectT) {
+				got, err := Client.ListUsers(tc.args.ctx, tc.args.req)
+				if tc.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, tc.want.Result, len(infos))
-					// always first check length, otherwise its failed anyway
-					if assert.Len(ttt, got.Result, len(tc.want.Result)) {
-						// totalResult is unrelated to the tests here so gets carried over, can vary from the count of results due to permissions
-						tc.want.Details.TotalResult = got.Details.TotalResult
-
-						// fill in userid and username as it is generated
-						for i := range infos {
-							if tc.want.Result[i] == nil {
-								continue
-							}
-							tc.want.Result[i].UserId = infos[i].UserID
-							tc.want.Result[i].Username = infos[i].Username
-							tc.want.Result[i].PreferredLoginName = infos[i].Username
-							tc.want.Result[i].LoginNames = []string{infos[i].Username}
-							if human := tc.want.Result[i].GetHuman(); human != nil {
-								human.Email.Email = infos[i].Username
-								human.Phone.Phone = infos[i].Phone
-								if tc.want.Result[i].GetHuman().GetPasswordChanged() != nil {
-									human.PasswordChanged = infos[i].Changed
-								}
-							}
-							tc.want.Result[i].Details = detailsV2ToV2beta(infos[i].Details)
+				// always only give back dependency infos which are required for the response
+				require.Len(ttt, tc.want.Result, len(infos))
+				// always first check length, otherwise its failed anyway
+				if assert.Len(ttt, got.Result, len(tc.want.Result)) {
+					// fill in userid and username as it is generated
+					for i := range infos {
+						if tc.want.Result[i] == nil {
+							continue
 						}
-						for i := range tc.want.Result {
-							if tc.want.Result[i] == nil {
-								continue
+						tc.want.Result[i].UserId = infos[i].UserID
+						tc.want.Result[i].Username = infos[i].Username
+						tc.want.Result[i].PreferredLoginName = infos[i].Username
+						tc.want.Result[i].LoginNames = []string{infos[i].Username}
+						if human := tc.want.Result[i].GetHuman(); human != nil {
+							human.Email.Email = infos[i].Username
+							human.Phone.Phone = infos[i].Phone
+							if tc.want.Result[i].GetHuman().GetPasswordChanged() != nil {
+								human.PasswordChanged = infos[i].Changed
 							}
-							assert.EqualExportedValues(ttt, got.Result[i], tc.want.Result[i])
 						}
+						tc.want.Result[i].Details = detailsV2ToV2beta(infos[i].Details)
 					}
-					integration.AssertListDetails(ttt, tc.want, got)
-				}, retryDuration, tick, "timeout waiting for expected user result")
-			})
-		}
+					for i := range tc.want.Result {
+						if tc.want.Result[i] == nil {
+							continue
+						}
+						assert.EqualExportedValues(ttt, got.Result[i], tc.want.Result[i])
+					}
+				}
+				integration.AssertListDetails(ttt, tc.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
-		},
-	)
-}
-
 func userPermissionCheckV2(ctx context.Context, query sq.SelectBuilder, enabled bool, filters []SearchQuery) sq.SelectBuilder {
 	return userPermissionCheckV2WithCustomColumns(ctx, query, enabled, filters, UserResourceOwnerCol, UserIDCol)
 }
@@ -619,14 +610,12 @@ func (q *Queries) CountUsers(ctx context.Context, queries *UserSearchQueries) (c
 }
 
 func (q *Queries) SearchUsers(ctx context.Context, queries *UserSearchQueries, permissionCheck domain.PermissionCheck) (*Users, error) {
-	permissionCheckV2 := PermissionV2(ctx, permissionCheck)
+	permissionCheckV2 := permissionCheck != nil
 	users, err := q.searchUsers(ctx, queries, permissionCheckV2)
 	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

@@ -11,7 +11,6 @@ import (
 
 	sq "github.com/Masterminds/squirrel"
 	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"golang.org/x/text/language"
 
 	"github.com/zitadel/zitadel/internal/api/authz"
@@ -20,129 +19,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