From 196eaa84d2d0c64794319c9c333b9b254c26db59 Mon Sep 17 00:00:00 2001 From: Gayathri Vijayan <66356931+grvijayan@users.noreply.github.com> Date: Tue, 28 Oct 2025 12:24:50 +0100 Subject: [PATCH] fix(user): auth option while listing user metadata (#10968) # Which Problems Are Solved A user from `org A` with `ORG_USER_MANAGER` role in `org B` is unable to list user metadata for a user in `org B`. # How the Problems Are Solved The `auth.option` is set to a specific permission (`user.read`) in the API definition of `ListUserMetadata`, which causes the interceptors to check for this specific permission. In this case, there is no specific check for org membership of a user (from org A) in a target organization (org B), and hence the call fails even though the user has the necessary permissions. This has been fixed by setting the `auth.option` to `authenticated`, and the necessary [permission checks are handled in the query-layer](https://github.com/zitadel/zitadel/blob/main/internal/query/user_metadata.go#L173). # Additional Changes N/A # Additional Context - Closes #10925 --------- Co-authored-by: Marco A. --- .../user/v2/integration_test/metadata_test.go | 167 ++++++++++++++++-- proto/zitadel/user/v2/user_service.proto | 2 +- 2 files changed, 152 insertions(+), 17 deletions(-) diff --git a/internal/api/grpc/user/v2/integration_test/metadata_test.go b/internal/api/grpc/user/v2/integration_test/metadata_test.go index 31931fab8..fcced5e6b 100644 --- a/internal/api/grpc/user/v2/integration_test/metadata_test.go +++ b/internal/api/grpc/user/v2/integration_test/metadata_test.go @@ -140,13 +140,12 @@ func TestServer_ListUserMetadata(t *testing.T) { } tests := []struct { - name string - args args - want *user.ListUserMetadataResponse - wantErr bool + name string + args args + want *user.ListUserMetadataResponse }{ { - name: "missing permission", + name: "missing permission, actual TotalResult count returned", args: args{ ctx: Instance.WithAuthorizationToken(context.Background(), integration.UserTypeNoPermission), dep: func(ctx context.Context, request *user.ListUserMetadataRequest, response *user.ListUserMetadataResponse) { @@ -156,7 +155,12 @@ func TestServer_ListUserMetadata(t *testing.T) { }, req: &user.ListUserMetadataRequest{}, }, - wantErr: true, + want: &user.ListUserMetadataResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + AppliedLimit: 100, + }, + }, }, { name: "list request", @@ -194,7 +198,7 @@ func TestServer_ListUserMetadata(t *testing.T) { userID := Instance.CreateUserTypeHuman(iamOwnerCTX, integration.Email()).GetId() request.UserId = userID key := "key1" - response.Metadata[0] = setUserMetadata(iamOwnerCTX, userID, key, "value1") + response.Metadata[0] = setUserMetadata(iamOwnerCTX, Instance, userID, key, "value1") Instance.SetUserMetadata(iamOwnerCTX, userID, "key2", "value2") Instance.SetUserMetadata(iamOwnerCTX, userID, "key3", "value3") request.Filters[0] = &metadata.MetadataSearchFilter{ @@ -223,9 +227,9 @@ func TestServer_ListUserMetadata(t *testing.T) { userID := Instance.CreateUserTypeHuman(iamOwnerCTX, integration.Email()).GetId() request.UserId = userID - response.Metadata[2] = setUserMetadata(iamOwnerCTX, userID, "key1", "value1") - response.Metadata[1] = setUserMetadata(iamOwnerCTX, userID, "key2", "value2") - response.Metadata[0] = setUserMetadata(iamOwnerCTX, userID, "key3", "value3") + response.Metadata[2] = setUserMetadata(iamOwnerCTX, Instance, userID, "key1", "value1") + response.Metadata[1] = setUserMetadata(iamOwnerCTX, Instance, userID, "key2", "value2") + response.Metadata[0] = setUserMetadata(iamOwnerCTX, Instance, userID, "key3", "value3") }, req: &user.ListUserMetadataRequest{}, }, @@ -249,10 +253,6 @@ func TestServer_ListUserMetadata(t *testing.T) { retryDuration, tick := integration.WaitForAndTickWithMaxDuration(iamOwnerCTX, time.Minute) require.EventuallyWithT(t, func(ttt *assert.CollectT) { got, listErr := Instance.Client.UserV2.ListUserMetadata(tt.args.ctx, tt.args.req) - if tt.wantErr { - require.Error(ttt, listErr) - return - } require.NoError(ttt, listErr) // always first check length, otherwise its failed anyway if assert.Len(ttt, got.Metadata, len(tt.want.Metadata)) { @@ -264,8 +264,143 @@ func TestServer_ListUserMetadata(t *testing.T) { } } -func setUserMetadata(ctx context.Context, userID, key, value string) *metadata.Metadata { - metadataResp := Instance.SetUserMetadata(ctx, userID, key, value) +func TestServer_ListUserMetadata_WithPermissionV2(t *testing.T) { + ensureFeaturePermissionV2Enabled(t, InstancePermissionV2) + iamOwnerCTX := InstancePermissionV2.WithAuthorizationToken(OrgCTX, integration.UserTypeIAMOwner) + + type args struct { + ctx context.Context + dep func(context.Context, *user.ListUserMetadataRequest, *user.ListUserMetadataResponse) + req *user.ListUserMetadataRequest + } + + tests := []struct { + name string + args args + want *user.ListUserMetadataResponse + }{ + { + name: "missing permission, TotalResult set to 0", + args: args{ + ctx: InstancePermissionV2.WithAuthorizationToken(context.Background(), integration.UserTypeNoPermission), + dep: func(ctx context.Context, request *user.ListUserMetadataRequest, response *user.ListUserMetadataResponse) { + userID := InstancePermissionV2.CreateUserTypeHuman(iamOwnerCTX, integration.Email()).GetId() + request.UserId = userID + InstancePermissionV2.SetUserMetadata(iamOwnerCTX, userID, "key1", "value1") + }, + req: &user.ListUserMetadataRequest{}, + }, + want: &user.ListUserMetadataResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 0, + AppliedLimit: 100, + }, + }, + }, + { + name: "list request", + args: args{ + ctx: iamOwnerCTX, + dep: func(ctx context.Context, request *user.ListUserMetadataRequest, response *user.ListUserMetadataResponse) { + userID := InstancePermissionV2.CreateUserTypeHuman(iamOwnerCTX, integration.Email()).GetId() + request.UserId = userID + metadataResp := InstancePermissionV2.SetUserMetadata(iamOwnerCTX, userID, "key1", "value1") + + response.Metadata[0] = &metadata.Metadata{ + CreationDate: metadataResp.GetSetDate(), + ChangeDate: metadataResp.GetSetDate(), + Key: "key1", + Value: []byte(base64.StdEncoding.EncodeToString([]byte("value1"))), + } + }, + req: &user.ListUserMetadataRequest{}, + }, + want: &user.ListUserMetadataResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + AppliedLimit: 100, + }, + Metadata: []*metadata.Metadata{ + {}, + }, + }, + }, + { + name: "list request single key", + args: args{ + ctx: iamOwnerCTX, + dep: func(ctx context.Context, request *user.ListUserMetadataRequest, response *user.ListUserMetadataResponse) { + userID := InstancePermissionV2.CreateUserTypeHuman(iamOwnerCTX, integration.Email()).GetId() + request.UserId = userID + key := "key1" + response.Metadata[0] = setUserMetadata(iamOwnerCTX, InstancePermissionV2, userID, key, "value1") + InstancePermissionV2.SetUserMetadata(iamOwnerCTX, userID, "key2", "value2") + InstancePermissionV2.SetUserMetadata(iamOwnerCTX, userID, "key3", "value3") + request.Filters[0] = &metadata.MetadataSearchFilter{ + Filter: &metadata.MetadataSearchFilter_KeyFilter{KeyFilter: &metadata.MetadataKeyFilter{Key: key}}, + } + }, + req: &user.ListUserMetadataRequest{ + Filters: []*metadata.MetadataSearchFilter{{}}, + }, + }, + want: &user.ListUserMetadataResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 1, + AppliedLimit: 100, + }, + Metadata: []*metadata.Metadata{ + {}, + }, + }, + }, + { + name: "list multiple keys", + args: args{ + ctx: iamOwnerCTX, + dep: func(ctx context.Context, request *user.ListUserMetadataRequest, response *user.ListUserMetadataResponse) { + userID := InstancePermissionV2.CreateUserTypeHuman(iamOwnerCTX, integration.Email()).GetId() + request.UserId = userID + + response.Metadata[2] = setUserMetadata(iamOwnerCTX, InstancePermissionV2, userID, "key1", "value1") + response.Metadata[1] = setUserMetadata(iamOwnerCTX, InstancePermissionV2, userID, "key2", "value2") + response.Metadata[0] = setUserMetadata(iamOwnerCTX, InstancePermissionV2, userID, "key3", "value3") + }, + req: &user.ListUserMetadataRequest{}, + }, + want: &user.ListUserMetadataResponse{ + Pagination: &filter.PaginationResponse{ + TotalResult: 3, + AppliedLimit: 100, + }, + Metadata: []*metadata.Metadata{ + {}, {}, {}, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if tt.args.dep != nil { + tt.args.dep(tt.args.ctx, tt.args.req, tt.want) + } + + retryDuration, tick := integration.WaitForAndTickWithMaxDuration(iamOwnerCTX, time.Minute) + require.EventuallyWithT(t, func(ttt *assert.CollectT) { + got, listErr := InstancePermissionV2.Client.UserV2.ListUserMetadata(tt.args.ctx, tt.args.req) + require.NoError(ttt, listErr) + // always first check length, otherwise its failed anyway + if assert.Len(ttt, got.Metadata, len(tt.want.Metadata)) { + assert.EqualExportedValues(ttt, got.Metadata, tt.want.Metadata) + } + assertPaginationResponse(ttt, tt.want.Pagination, got.Pagination) + }, retryDuration, tick, "timeout waiting for expected execution result") + }) + } +} + +func setUserMetadata(ctx context.Context, instance *integration.Instance, userID, key, value string) *metadata.Metadata { + metadataResp := instance.SetUserMetadata(ctx, userID, key, value) return &metadata.Metadata{ CreationDate: metadataResp.GetSetDate(), ChangeDate: metadataResp.GetSetDate(), diff --git a/proto/zitadel/user/v2/user_service.proto b/proto/zitadel/user/v2/user_service.proto index 36546e2b7..1c9bb5f8e 100644 --- a/proto/zitadel/user/v2/user_service.proto +++ b/proto/zitadel/user/v2/user_service.proto @@ -1907,7 +1907,7 @@ service UserService { }; option (zitadel.protoc_gen_zitadel.v2.options) = {auth_option: { - permission: "user.read" + permission: "authenticated" } };