diff --git a/internal/api/grpc/user/v2/human.go b/internal/api/grpc/user/v2/human.go index 05391bce1f..b4c4bb0a4d 100644 --- a/internal/api/grpc/user/v2/human.go +++ b/internal/api/grpc/user/v2/human.go @@ -80,8 +80,8 @@ func setMetadataEntries(metadata []*user.Metadata) []*user.SetMetadataEntry { return metadataEntries } -func (s *Server) updateUserTypeHuman(ctx context.Context, humanPb *user.UpdateUserRequest_Human, userId string, userName *string) (*connect.Response[user.UpdateUserResponse], error) { - cmd, err := updateHumanUserToCommand(userId, userName, humanPb) +func (s *Server) updateUserTypeHuman(ctx context.Context, humanPb *user.UpdateUserRequest_Human, userId string, userName *string, reqMetadata []*user.Metadata) (*connect.Response[user.UpdateUserResponse], error) { + cmd, err := updateHumanUserToCommand(userId, userName, humanPb, reqMetadata) if err != nil { return nil, err } @@ -95,7 +95,7 @@ func (s *Server) updateUserTypeHuman(ctx context.Context, humanPb *user.UpdateUs }), nil } -func updateHumanUserToCommand(userId string, userName *string, human *user.UpdateUserRequest_Human) (*command.ChangeHuman, error) { +func updateHumanUserToCommand(userId string, userName *string, human *user.UpdateUserRequest_Human, reqMetadata []*user.Metadata) (*command.ChangeHuman, error) { phone := human.GetPhone() if phone != nil && phone.Phone == "" && phone.GetVerification() != nil { return nil, zerrors.ThrowInvalidArgument(nil, "USERv2-4f3d6", "Errors.User.Phone.VerifyingRemovalIsNotSupported") @@ -111,6 +111,7 @@ func updateHumanUserToCommand(userId string, userName *string, human *user.Updat Email: email, Phone: setHumanPhoneToPhone(human.Phone, true), Password: setHumanPasswordToPassword(human.Password), + Metadata: setUserMetadataToDomain(reqMetadata), }, nil } diff --git a/internal/api/grpc/user/v2/human_test.go b/internal/api/grpc/user/v2/human_test.go index 52e5371dcc..f6cd0ea0ec 100644 --- a/internal/api/grpc/user/v2/human_test.go +++ b/internal/api/grpc/user/v2/human_test.go @@ -20,6 +20,7 @@ func Test_patchHumanUserToCommand(t *testing.T) { userId string userName *string human *user.UpdateUserRequest_Human + metadata []*user.Metadata } tests := []struct { name string @@ -81,6 +82,16 @@ func Test_patchHumanUserToCommand(t *testing.T) { }, }, }, + metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + }, }, want: &command.ChangeHuman{ ID: "userId", @@ -106,6 +117,16 @@ func Test_patchHumanUserToCommand(t *testing.T) { Password: "newPassword", ChangeRequired: true, }, + Metadata: []*domain.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + }, }, wantErr: assert.NoError, }, { @@ -242,7 +263,7 @@ func Test_patchHumanUserToCommand(t *testing.T) { }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := updateHumanUserToCommand(tt.args.userId, tt.args.userName, tt.args.human) + got, err := updateHumanUserToCommand(tt.args.userId, tt.args.userName, tt.args.human, tt.args.metadata) if !tt.wantErr(t, err, fmt.Sprintf("patchHumanUserToCommand(%v, %v, %v)", tt.args.userId, tt.args.userName, tt.args.human)) { return } diff --git a/internal/api/grpc/user/v2/integration_test/user_test.go b/internal/api/grpc/user/v2/integration_test/user_test.go index c95b5c0d5d..62467bb2b7 100644 --- a/internal/api/grpc/user/v2/integration_test/user_test.go +++ b/internal/api/grpc/user/v2/integration_test/user_test.go @@ -4409,146 +4409,405 @@ func TestServer_UpdateUser_And_Compare(t *testing.T) { } type testCase struct { args args - assert func(t *testing.T, getResponse *user.GetUserByIDResponse) + assert func( + t *testing.T, + getResponse *user.GetUserByIDResponse, + getMetadataResponse *user.ListUserMetadataResponse, + ) } tests := []struct { name string testCase func(runId string) testCase - }{{ - name: "human remove phone", - testCase: func(runId string) testCase { - username := fmt.Sprintf("donald.duck+%s", runId) - email := username + "@example.com" - return testCase{ - args: args{ - ctx: OrgCTX, - create: &user.CreateUserRequest{ - OrganizationId: Instance.DefaultOrg.Id, - UserId: &runId, - UserType: &user.CreateUserRequest_Human_{ - Human: &user.CreateUserRequest_Human{ - Profile: &user.SetHumanProfile{ - GivenName: "Donald", - FamilyName: "Duck", + }{ + { + name: "human remove phone", + testCase: func(runId string) testCase { + username := fmt.Sprintf("donald.duck+%s", runId) + email := username + "@example.com" + return testCase{ + args: args{ + ctx: OrgCTX, + create: &user.CreateUserRequest{ + OrganizationId: Instance.DefaultOrg.Id, + UserId: &runId, + UserType: &user.CreateUserRequest_Human_{ + Human: &user.CreateUserRequest_Human{ + Profile: &user.SetHumanProfile{ + GivenName: "Donald", + FamilyName: "Duck", + }, + Email: &user.SetHumanEmail{ + Email: email, + }, + Phone: &user.SetHumanPhone{ + Phone: "+1234567890", + }, }, - Email: &user.SetHumanEmail{ - Email: email, - }, - Phone: &user.SetHumanPhone{ - Phone: "+1234567890", + }, + }, + update: &user.UpdateUserRequest{ + UserId: runId, + UserType: &user.UpdateUserRequest_Human_{ + Human: &user.UpdateUserRequest_Human{ + Phone: &user.SetHumanPhone{}, }, }, }, }, - update: &user.UpdateUserRequest{ - UserId: runId, - UserType: &user.UpdateUserRequest_Human_{ - Human: &user.UpdateUserRequest_Human{ - Phone: &user.SetHumanPhone{}, - }, - }, + assert: func(t *testing.T, getResponse *user.GetUserByIDResponse, _ *user.ListUserMetadataResponse) { + assert.Empty(t, getResponse.GetUser().GetHuman().GetPhone().GetPhone(), "phone is not empty") }, - }, - assert: func(t *testing.T, getResponse *user.GetUserByIDResponse) { - assert.Empty(t, getResponse.GetUser().GetHuman().GetPhone().GetPhone(), "phone is not empty") - }, - } + } + }, }, - }, { - name: "human username", - testCase: func(runId string) testCase { - username := fmt.Sprintf("donald.duck+%s", runId) - email := username + "@example.com" - return testCase{ - args: args{ - ctx: OrgCTX, - create: &user.CreateUserRequest{ - OrganizationId: Instance.DefaultOrg.Id, - UserId: &runId, - UserType: &user.CreateUserRequest_Human_{ - Human: &user.CreateUserRequest_Human{ - Profile: &user.SetHumanProfile{ - GivenName: "Donald", - FamilyName: "Duck", + { + name: "human username", + testCase: func(runId string) testCase { + username := fmt.Sprintf("donald.duck+%s", runId) + email := username + "@example.com" + return testCase{ + args: args{ + ctx: OrgCTX, + create: &user.CreateUserRequest{ + OrganizationId: Instance.DefaultOrg.Id, + UserId: &runId, + UserType: &user.CreateUserRequest_Human_{ + Human: &user.CreateUserRequest_Human{ + Profile: &user.SetHumanProfile{ + GivenName: "Donald", + FamilyName: "Duck", + }, + Email: &user.SetHumanEmail{ + Email: email, + }, }, - Email: &user.SetHumanEmail{ - Email: email, + }, + }, + update: &user.UpdateUserRequest{ + UserId: runId, + Username: &username, + UserType: &user.UpdateUserRequest_Human_{ + Human: &user.UpdateUserRequest_Human{}, + }, + }, + }, + assert: func(t *testing.T, getResponse *user.GetUserByIDResponse, _ *user.ListUserMetadataResponse) { + assert.Equal(t, username, getResponse.GetUser().GetUsername()) + }, + } + }, + }, + { + name: "service accountname", + testCase: func(runId string) testCase { + username := fmt.Sprintf("donald.duck+%s", runId) + return testCase{ + args: args{ + ctx: OrgCTX, + create: &user.CreateUserRequest{ + OrganizationId: Instance.DefaultOrg.Id, + UserId: &runId, + UserType: &user.CreateUserRequest_Machine_{ + Machine: &user.CreateUserRequest_Machine{ + Name: "Donald", + }, + }, + }, + update: &user.UpdateUserRequest{ + UserId: runId, + Username: &username, + UserType: &user.UpdateUserRequest_Machine_{ + Machine: &user.UpdateUserRequest_Machine{}, + }, + }, + }, + assert: func(t *testing.T, getResponse *user.GetUserByIDResponse, _ *user.ListUserMetadataResponse) { + assert.Equal(t, username, getResponse.GetUser().GetUsername()) + }, + } + }, + }, + { + name: "machine accessTokenType", + testCase: func(runId string) testCase { + return testCase{ + args: args{ + ctx: OrgCTX, + create: &user.CreateUserRequest{ + OrganizationId: Instance.DefaultOrg.Id, + UserId: &runId, + UserType: &user.CreateUserRequest_Machine_{ + Machine: &user.CreateUserRequest_Machine{ + Name: "Donald", + }, + }, + }, + update: &user.UpdateUserRequest{ + UserId: runId, + UserType: &user.UpdateUserRequest_Machine_{ + Machine: &user.UpdateUserRequest_Machine{ + AccessTokenType: gu.Ptr(user.AccessTokenType_ACCESS_TOKEN_TYPE_JWT), }, }, }, }, - update: &user.UpdateUserRequest{ - UserId: runId, - Username: &username, - UserType: &user.UpdateUserRequest_Human_{ - Human: &user.UpdateUserRequest_Human{}, - }, + assert: func(t *testing.T, getResponse *user.GetUserByIDResponse, _ *user.ListUserMetadataResponse) { + assert.Equal(t, user.AccessTokenType_ACCESS_TOKEN_TYPE_JWT, getResponse.GetUser().GetMachine().GetAccessTokenType()) }, - }, - assert: func(t *testing.T, getResponse *user.GetUserByIDResponse) { - assert.Equal(t, username, getResponse.GetUser().GetUsername()) - }, - } + } + }, }, - }, { - name: "service accountname", - testCase: func(runId string) testCase { - username := fmt.Sprintf("donald.duck+%s", runId) - return testCase{ - args: args{ - ctx: OrgCTX, - create: &user.CreateUserRequest{ - OrganizationId: Instance.DefaultOrg.Id, - UserId: &runId, - UserType: &user.CreateUserRequest_Machine_{ - Machine: &user.CreateUserRequest_Machine{ - Name: "Donald", + { + name: "human metadata", + testCase: func(runId string) testCase { + username := fmt.Sprintf("donald.duck+%s", runId) + email := username + "@example.com" + return testCase{ + args: args{ + ctx: OrgCTX, + create: &user.CreateUserRequest{ + OrganizationId: Instance.DefaultOrg.Id, + UserId: &runId, + UserType: &user.CreateUserRequest_Human_{ + Human: &user.CreateUserRequest_Human{ + Profile: &user.SetHumanProfile{ + GivenName: "Donald", + FamilyName: "Duck", + }, + Email: &user.SetHumanEmail{ + Email: email, + }, + }, + }, + Metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + }, + }, + update: &user.UpdateUserRequest{ + UserId: runId, + Username: &username, + UserType: &user.UpdateUserRequest_Human_{ + Human: &user.UpdateUserRequest_Human{}, + }, + Metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("updated_value1"), // updating key1 + }, + { + Key: "key3", + Value: []byte("value3"), // adding key3 + }, }, }, }, - update: &user.UpdateUserRequest{ - UserId: runId, - Username: &username, - UserType: &user.UpdateUserRequest_Machine_{ - Machine: &user.UpdateUserRequest_Machine{}, - }, + assert: func(t *testing.T, getResponse *user.GetUserByIDResponse, listMetadataResponse *user.ListUserMetadataResponse) { + assert.Equal(t, username, getResponse.GetUser().GetUsername()) + expectedMetadata := []*metadata.Metadata{ + {Key: "key1", Value: []byte("updated_value1")}, + {Key: "key2", Value: []byte("value2")}, + {Key: "key3", Value: []byte("value3")}, + } + assertMetadataEquals(t, expectedMetadata, listMetadataResponse.GetMetadata()) }, - }, - assert: func(t *testing.T, getResponse *user.GetUserByIDResponse) { - assert.Equal(t, username, getResponse.GetUser().GetUsername()) - }, - } + } + }, }, - }, { - name: "machine accessTokenType", - testCase: func(runId string) testCase { - return testCase{ - args: args{ - ctx: OrgCTX, - create: &user.CreateUserRequest{ - OrganizationId: Instance.DefaultOrg.Id, - UserId: &runId, - UserType: &user.CreateUserRequest_Machine_{ - Machine: &user.CreateUserRequest_Machine{ - Name: "Donald", + { + name: "human metadata - update and delete, ok", + testCase: func(runId string) testCase { + username := fmt.Sprintf("donald.duck+%s", runId) + email := username + "@example.com" + return testCase{ + args: args{ + ctx: OrgCTX, + create: &user.CreateUserRequest{ + OrganizationId: Instance.DefaultOrg.Id, + UserId: &runId, + UserType: &user.CreateUserRequest_Human_{ + Human: &user.CreateUserRequest_Human{ + Profile: &user.SetHumanProfile{ + GivenName: "Donald", + FamilyName: "Duck", + }, + Email: &user.SetHumanEmail{ + Email: email, + }, + }, + }, + Metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + }, + }, + update: &user.UpdateUserRequest{ + UserId: runId, + Username: &username, + UserType: &user.UpdateUserRequest_Human_{ + Human: &user.UpdateUserRequest_Human{}, + }, + Metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("updated_value1"), // updating key1 + }, + { + Key: "key2", // removing key2 + }, + { + Key: "key3", // removing a non-existing key + }, + { + Key: "key4", // adding a new key + Value: []byte("value4"), + }, }, }, }, - update: &user.UpdateUserRequest{ - UserId: runId, - UserType: &user.UpdateUserRequest_Machine_{ - Machine: &user.UpdateUserRequest_Machine{ - AccessTokenType: gu.Ptr(user.AccessTokenType_ACCESS_TOKEN_TYPE_JWT), + assert: func(t *testing.T, getResponse *user.GetUserByIDResponse, listMetadataResponse *user.ListUserMetadataResponse) { + assert.Equal(t, username, getResponse.GetUser().GetUsername()) + expectedMetadata := []*metadata.Metadata{ + {Key: "key1", Value: []byte("updated_value1")}, + {Key: "key4", Value: []byte("value4")}, + } + assertMetadataEquals(t, expectedMetadata, listMetadataResponse.GetMetadata()) + }, + } + }, + }, + { + name: "service account metadata", + testCase: func(runId string) testCase { + username := fmt.Sprintf("service.account+%s", runId) + return testCase{ + args: args{ + ctx: OrgCTX, + create: &user.CreateUserRequest{ + OrganizationId: Instance.DefaultOrg.Id, + UserId: &runId, + UserType: &user.CreateUserRequest_Machine_{ + Machine: &user.CreateUserRequest_Machine{ + Name: "service_account", + AccessTokenType: user.AccessTokenType_ACCESS_TOKEN_TYPE_JWT, + }, + }, + Metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + }, + }, + update: &user.UpdateUserRequest{ + UserId: runId, + Username: &username, + UserType: &user.UpdateUserRequest_Machine_{ + Machine: &user.UpdateUserRequest_Machine{}, + }, + Metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("updated_value1"), // updating key1 + }, + { + Key: "key3", + Value: []byte("value3"), // adding key3 + }, }, }, }, - }, - assert: func(t *testing.T, getResponse *user.GetUserByIDResponse) { - assert.Equal(t, user.AccessTokenType_ACCESS_TOKEN_TYPE_JWT, getResponse.GetUser().GetMachine().GetAccessTokenType()) - }, - } + assert: func(t *testing.T, getResponse *user.GetUserByIDResponse, listMetadataResponse *user.ListUserMetadataResponse) { + assert.Equal(t, username, getResponse.GetUser().GetUsername()) + expectedMetadata := []*metadata.Metadata{ + {Key: "key1", Value: []byte("updated_value1")}, + {Key: "key2", Value: []byte("value2")}, + {Key: "key3", Value: []byte("value3")}, + } + assertMetadataEquals(t, expectedMetadata, listMetadataResponse.GetMetadata()) + }, + } + }, }, - }} + { + name: "service account metadata - update and delete, ok", + testCase: func(runId string) testCase { + username := fmt.Sprintf("service.account+%s", runId) + return testCase{ + args: args{ + ctx: OrgCTX, + create: &user.CreateUserRequest{ + OrganizationId: Instance.DefaultOrg.Id, + UserId: &runId, + UserType: &user.CreateUserRequest_Machine_{ + Machine: &user.CreateUserRequest_Machine{ + Name: "service_account", + AccessTokenType: user.AccessTokenType_ACCESS_TOKEN_TYPE_JWT, + }, + }, + Metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + }, + }, + update: &user.UpdateUserRequest{ + UserId: runId, + Username: &username, + UserType: &user.UpdateUserRequest_Machine_{ + Machine: &user.UpdateUserRequest_Machine{}, + }, + Metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("updated_value1"), // updating key1 + }, + { + Key: "key2", // removing key2 + }, + { + Key: "key3", // removing a non-existing key + }, + { + Key: "key4", // adding a new key + Value: []byte("value4"), + }, + }, + }, + }, + assert: func(t *testing.T, getResponse *user.GetUserByIDResponse, listMetadataResponse *user.ListUserMetadataResponse) { + assert.Equal(t, username, getResponse.GetUser().GetUsername()) + expectedMetadata := []*metadata.Metadata{ + {Key: "key1", Value: []byte("updated_value1")}, + {Key: "key4", Value: []byte("value4")}, + } + assertMetadataEquals(t, expectedMetadata, listMetadataResponse.GetMetadata()) + }, + } + }, + }, + } for i, tt := range tests { t.Run(tt.name, func(t *testing.T) { now := time.Now() @@ -4563,7 +4822,11 @@ func TestServer_UpdateUser_And_Compare(t *testing.T) { UserId: createResponse.GetId(), }) require.NoError(t, err) - test.assert(t, getResponse) + gotMetadataResponse, err := Client.ListUserMetadata(test.args.ctx, &user.ListUserMetadataRequest{ + UserId: createResponse.GetId(), + }) + require.NoError(t, err) + test.assert(t, getResponse, gotMetadataResponse) }) } } diff --git a/internal/api/grpc/user/v2/machine.go b/internal/api/grpc/user/v2/machine.go index 74645387da..c64fec9850 100644 --- a/internal/api/grpc/user/v2/machine.go +++ b/internal/api/grpc/user/v2/machine.go @@ -47,8 +47,8 @@ func (s *Server) createUserTypeMachine(ctx context.Context, machinePb *user.Crea }), nil } -func (s *Server) updateUserTypeMachine(ctx context.Context, machinePb *user.UpdateUserRequest_Machine, userId string, userName *string) (*connect.Response[user.UpdateUserResponse], error) { - cmd := updateMachineUserToCommand(userId, userName, machinePb) +func (s *Server) updateUserTypeMachine(ctx context.Context, machinePb *user.UpdateUserRequest_Machine, userId string, userName *string, reqMetadata []*user.Metadata) (*connect.Response[user.UpdateUserResponse], error) { + cmd := updateMachineUserToCommand(userId, userName, machinePb, reqMetadata) err := s.command.ChangeUserMachine(ctx, cmd) if err != nil { return nil, err @@ -58,7 +58,7 @@ func (s *Server) updateUserTypeMachine(ctx context.Context, machinePb *user.Upda }), nil } -func updateMachineUserToCommand(userId string, userName *string, machine *user.UpdateUserRequest_Machine) *command.ChangeMachine { +func updateMachineUserToCommand(userId string, userName *string, machine *user.UpdateUserRequest_Machine, reqMetadata []*user.Metadata) *command.ChangeMachine { var accessTokenType *domain.OIDCTokenType if machine.AccessTokenType != nil { tokenType := accessTokenTypeToDomain(*machine.AccessTokenType) @@ -70,6 +70,7 @@ func updateMachineUserToCommand(userId string, userName *string, machine *user.U Name: machine.Name, Description: machine.Description, AccessTokenType: accessTokenType, + Metadata: setUserMetadataToDomain(reqMetadata), } } diff --git a/internal/api/grpc/user/v2/machine_test.go b/internal/api/grpc/user/v2/machine_test.go index a5dc710b1f..189e65d2e0 100644 --- a/internal/api/grpc/user/v2/machine_test.go +++ b/internal/api/grpc/user/v2/machine_test.go @@ -18,6 +18,7 @@ func Test_patchMachineUserToCommand(t *testing.T) { userId string userName *string machine *user.UpdateUserRequest_Machine + metadata []*user.Metadata } tests := []struct { name string @@ -45,6 +46,16 @@ func Test_patchMachineUserToCommand(t *testing.T) { Description: gu.Ptr("description"), AccessTokenType: gu.Ptr(user.AccessTokenType_ACCESS_TOKEN_TYPE_JWT), }, + metadata: []*user.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + }, }, want: &command.ChangeMachine{ ID: "userId", @@ -52,11 +63,21 @@ func Test_patchMachineUserToCommand(t *testing.T) { Name: gu.Ptr("name"), Description: gu.Ptr("description"), AccessTokenType: gu.Ptr(domain.OIDCTokenTypeJWT), + Metadata: []*domain.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte("value2"), + }, + }, }, }} for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := updateMachineUserToCommand(tt.args.userId, tt.args.userName, tt.args.machine) + got := updateMachineUserToCommand(tt.args.userId, tt.args.userName, tt.args.machine, tt.args.metadata) if diff := cmp.Diff(tt.want, got, cmpopts.EquateComparable(language.Tag{})); diff != "" { t.Errorf("patchMachineUserToCommand() mismatch (-want +got):\n%s", diff) } diff --git a/internal/api/grpc/user/v2/metadata.go b/internal/api/grpc/user/v2/metadata.go index c7ce97e38e..3ac9776088 100644 --- a/internal/api/grpc/user/v2/metadata.go +++ b/internal/api/grpc/user/v2/metadata.go @@ -49,7 +49,7 @@ func (s *Server) listUserMetadataRequestToModel(req *user.ListUserMetadataReques } func (s *Server) SetUserMetadata(ctx context.Context, req *connect.Request[user.SetUserMetadataRequest]) (*connect.Response[user.SetUserMetadataResponse], error) { - result, err := s.command.BulkSetUserMetadata(ctx, req.Msg.UserId, "", s.command.NewPermissionCheckUserWrite(ctx, false), setUserMetadataToDomain(req.Msg)...) + result, err := s.command.BulkSetUserMetadata(ctx, req.Msg.UserId, "", s.command.NewPermissionCheckUserWrite(ctx, false), setUserMetadataToDomain(req.Msg.GetMetadata())...) if err != nil { return nil, err } @@ -58,15 +58,18 @@ func (s *Server) SetUserMetadata(ctx context.Context, req *connect.Request[user. }), nil } -func setUserMetadataToDomain(req *user.SetUserMetadataRequest) []*domain.Metadata { - metadata := make([]*domain.Metadata, len(req.Metadata)) - for i, data := range req.Metadata { - metadata[i] = &domain.Metadata{ - Key: data.Key, - Value: data.Value, +func setUserMetadataToDomain(reqMetadata []*user.Metadata) []*domain.Metadata { + if len(reqMetadata) == 0 { + return nil + } + metadataEntries := make([]*domain.Metadata, len(reqMetadata)) + for i, data := range reqMetadata { + metadataEntries[i] = &domain.Metadata{ + Key: data.GetKey(), + Value: data.GetValue(), } } - return metadata + return metadataEntries } func (s *Server) DeleteUserMetadata(ctx context.Context, req *connect.Request[user.DeleteUserMetadataRequest]) (*connect.Response[user.DeleteUserMetadataResponse], error) { diff --git a/internal/api/grpc/user/v2/user.go b/internal/api/grpc/user/v2/user.go index f99ee6d621..ece77bc249 100644 --- a/internal/api/grpc/user/v2/user.go +++ b/internal/api/grpc/user/v2/user.go @@ -425,9 +425,9 @@ func (s *Server) CreateUser(ctx context.Context, req *connect.Request[user.Creat func (s *Server) UpdateUser(ctx context.Context, req *connect.Request[user.UpdateUserRequest]) (*connect.Response[user.UpdateUserResponse], error) { switch userType := req.Msg.GetUserType().(type) { case *user.UpdateUserRequest_Human_: - return s.updateUserTypeHuman(ctx, userType.Human, req.Msg.GetUserId(), req.Msg.Username) + return s.updateUserTypeHuman(ctx, userType.Human, req.Msg.GetUserId(), req.Msg.Username, req.Msg.GetMetadata()) case *user.UpdateUserRequest_Machine_: - return s.updateUserTypeMachine(ctx, userType.Machine, req.Msg.GetUserId(), req.Msg.Username) + return s.updateUserTypeMachine(ctx, userType.Machine, req.Msg.GetUserId(), req.Msg.Username, req.Msg.GetMetadata()) default: return nil, zerrors.ThrowUnimplemented(nil, "", "user type is not implemented") } diff --git a/internal/command/user_metadata.go b/internal/command/user_metadata.go index ebc5b1619f..c08b52ab24 100644 --- a/internal/command/user_metadata.go +++ b/internal/command/user_metadata.go @@ -67,36 +67,14 @@ func (c *Commands) BulkSetUserMetadata(ctx context.Context, userID, resourceOwne } } - events := make([]eventstore.Command, 0) setMetadata, err := c.getUserMetadataListModelByID(ctx, userID, userResourceOwner) if err != nil { return nil, err } userAgg := UserAggregateFromWriteModel(&setMetadata.WriteModel) - for _, data := range metadatas { - existingValue, keyExists := setMetadata.metadataList[data.Key] - - // if value is empty, a metadata remove event has to be pushed - if len(data.Value) == 0 { - // Ignore deletion if key does not exist - if !keyExists { - continue - } - - event := user.NewMetadataRemovedEvent(ctx, userAgg, data.Key) - events = append(events, event) - continue - } - - // if no change to metadata no event has to be pushed - if keyExists && bytes.Equal(existingValue, data.Value) { - continue - } - event, err := c.setUserMetadata(ctx, userAgg, data) - if err != nil { - return nil, err - } - events = append(events, event) + events, err := c.createMetadataEvents(ctx, metadatas, setMetadata.metadataList, userAgg) + if err != nil { + return nil, err } // no changes for the metadata if len(events) == 0 { @@ -115,6 +93,37 @@ func (c *Commands) BulkSetUserMetadata(ctx context.Context, userID, resourceOwne return writeModelToObjectDetails(&setMetadata.WriteModel), nil } +// createMetadataEvents compares the existing metadata key-value pair before creating metadata set/remove events +func (c *Commands) createMetadataEvents(ctx context.Context, reqMetadata []*domain.Metadata, currentMetadataMap map[string][]byte, userAgg *eventstore.Aggregate) ([]eventstore.Command, error) { + events := make([]eventstore.Command, 0) + for _, data := range reqMetadata { + existingValue, keyExists := currentMetadataMap[data.Key] + + // if the value is empty, a metadata remove event has to be pushed + if len(data.Value) == 0 { + // Ignore deletion if the key does not exist + if !keyExists { + continue + } + + event := user.NewMetadataRemovedEvent(ctx, userAgg, data.Key) + events = append(events, event) + continue + } + + // if no change to metadata, no event has to be pushed + if keyExists && bytes.Equal(existingValue, data.Value) { + continue + } + event, err := c.setUserMetadata(ctx, userAgg, data) + if err != nil { + return nil, err + } + events = append(events, event) + } + return events, nil +} + func (c *Commands) setUserMetadata(ctx context.Context, userAgg *eventstore.Aggregate, metadata *domain.Metadata) (command eventstore.Command, err error) { if !metadata.IsValid() { return nil, zerrors.ThrowInvalidArgument(nil, "META-2m00f", "Errors.Metadata.Invalid") diff --git a/internal/command/user_metadata_test.go b/internal/command/user_metadata_test.go index e134a54394..d658a202fb 100644 --- a/internal/command/user_metadata_test.go +++ b/internal/command/user_metadata_test.go @@ -704,6 +704,121 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) { }, }, }, + { + name: "add and delete metadata, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "", + "firstname lastname", + language.Und, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key1", + []byte("value1"), + ), + ), + eventFromEventPusher( + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key2", + []byte("value2"), + ), + ), + ), + expectPush( + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key1", + []byte("updated_value1"), + ), + user.NewMetadataRemovedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key2", + ), + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key3", + []byte("value3"), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + userID: "user1", + metadataList: []*domain.Metadata{ + {Key: "key1", Value: []byte("updated_value1")}, + {Key: "key2"}, + {Key: "key3", Value: []byte("value3")}, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "delete non existing key, ok (ignored)", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "", + "firstname lastname", + language.Und, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewMetadataSetEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "key1", + []byte("value1"), + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + userID: "user1", + metadataList: []*domain.Metadata{ + {Key: "key2"}, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/command/user_v2_human.go b/internal/command/user_v2_human.go index 9936f637f6..de4386d4c8 100644 --- a/internal/command/user_v2_human.go +++ b/internal/command/user_v2_human.go @@ -337,13 +337,12 @@ func (c *Commands) ChangeUserHuman(ctx context.Context, human *ChangeHuman, alg } } - for _, md := range human.Metadata { - cmd, err := c.setUserMetadata(ctx, userAgg, md) + if len(human.Metadata) > 0 { + metadataCmds, err := c.createMetadataEvents(ctx, human.Metadata, existingHuman.Metadata, userAgg) if err != nil { return err } - - cmds = append(cmds, cmd) + cmds = append(cmds, metadataCmds...) } for _, mdKey := range human.MetadataKeysToRemove { diff --git a/internal/command/user_v2_human_test.go b/internal/command/user_v2_human_test.go index 9c14590977..4aa23cf2fc 100644 --- a/internal/command/user_v2_human_test.go +++ b/internal/command/user_v2_human_test.go @@ -3919,6 +3919,148 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { }, }, }, + { + name: "change human metadata, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + newAddHumanEvent("$plain$x$password", true, true, "", language.English), + ), + ), + expectPush( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key1", + []byte("value1"), + ), + user.NewMetadataSetEvent(context.Background(), + &userAgg.Aggregate, + "key3", + []byte("value3"), + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + tarpit: expectTarpit(0), + loginPaths: expectLoginPathsNoCall, + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &ChangeHuman{ + Metadata: []*domain.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key3", + Value: []byte("value3"), + }, + }, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + Sequence: 0, + EventDate: time.Time{}, + ResourceOwner: "org1", + }, + }, + }, + { + name: "change human metadata, empty value, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + newAddHumanEvent("$plain$x$password", true, true, "", language.English), + ), + eventFromEventPusher( // pre-existing metadata + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key1", + []byte("value1")), + ), + eventFromEventPusher( // pre-existing metadata + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key2", + []byte("value2")), + ), + ), + expectPush( + user.NewMetadataRemovedEvent( + context.Background(), + &userAgg.Aggregate, + "key2", + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + tarpit: expectTarpit(0), + loginPaths: expectLoginPathsNoCall, + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &ChangeHuman{ + Metadata: []*domain.Metadata{ + { + Key: "key1", + Value: []byte("value1"), // no update + }, + { + Key: "key2", + Value: []byte(""), // delete the key for an empty value + }, + }, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + Sequence: 0, + EventDate: time.Time{}, + ResourceOwner: "org1", + }, + }, + }, + { + name: "change human metadata, no permission", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + newAddHumanEvent("$plain$x$password", true, true, "", language.English), + ), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + tarpit: expectTarpit(0), + loginPaths: expectLoginPathsNoCall, + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &ChangeHuman{ + Metadata: []*domain.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + }, + }, + }, + res: res{ + err: func(err error) bool { + return errors.Is(err, zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied")) + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/internal/command/user_v2_machine.go b/internal/command/user_v2_machine.go index c391d6a3ac..3653cb47a7 100644 --- a/internal/command/user_v2_machine.go +++ b/internal/command/user_v2_machine.go @@ -20,6 +20,8 @@ type ChangeMachine struct { // Details are set after a successful execution of the command Details *domain.ObjectDetails + + Metadata []*domain.Metadata } func (h *ChangeMachine) Changed() bool { @@ -35,25 +37,35 @@ func (h *ChangeMachine) Changed() bool { if h.AccessTokenType != nil { return true } + if len(h.Metadata) > 0 { + return true + } return false } func (c *Commands) ChangeUserMachine(ctx context.Context, machine *ChangeMachine) (err error) { + // get the existing user existingMachine, err := c.UserMachineWriteModel( ctx, machine.ID, machine.ResourceOwner, - false, + len(machine.Metadata) > 0, ) if err != nil { return err } - if machine.Changed() { - if err := c.checkPermissionUpdateUser(ctx, existingMachine.ResourceOwner, existingMachine.AggregateID, true); err != nil { - return err - } + // check whether the user has permissions to make this change + if err := c.checkPermissionUpdateUser(ctx, existingMachine.ResourceOwner, existingMachine.AggregateID, true); err != nil { + return err } + // if there are no changes, return the existing object details + if !machine.Changed() { + machine.Details = writeModelToObjectDetails(&existingMachine.WriteModel) + return nil + } + + // create the events for the change cmds := make([]eventstore.Command, 0) if machine.Username != nil { cmds, err = c.changeUsername(ctx, cmds, existingMachine, *machine.Username) @@ -74,6 +86,14 @@ func (c *Commands) ChangeUserMachine(ctx context.Context, machine *ChangeMachine if len(machineChanges) > 0 { cmds = append(cmds, user.NewMachineChangedEvent(ctx, &existingMachine.Aggregate().Aggregate, machineChanges)) } + if len(machine.Metadata) > 0 { + metadataCmds, err := c.createMetadataEvents(ctx, machine.Metadata, existingMachine.Metadata, &existingMachine.Aggregate().Aggregate) + if err != nil { + return err + } + cmds = append(cmds, metadataCmds...) + } + if len(cmds) == 0 { machine.Details = writeModelToObjectDetails(&existingMachine.WriteModel) return nil diff --git a/internal/command/user_v2_machine_test.go b/internal/command/user_v2_machine_test.go index 831ebdd3d4..c66ec72494 100644 --- a/internal/command/user_v2_machine_test.go +++ b/internal/command/user_v2_machine_test.go @@ -338,6 +338,133 @@ func TestCommandSide_ChangeUserMachine(t *testing.T) { }, }, }, + { + name: "change machine metadata, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(userAddedEvent), + ), + expectPush( + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key1", + []byte("value1"), + ), + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key3", + []byte("value3"), + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + machine: &ChangeMachine{ + Metadata: []*domain.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key3", + Value: []byte("value3"), + }, + }, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "change machine metadata, delete metadata, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(userAddedEvent), + eventFromEventPusher( // pre-existing metadata + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key1", + []byte("value1")), + ), + eventFromEventPusher( // pre-existing metadata + user.NewMetadataSetEvent( + context.Background(), + &userAgg.Aggregate, + "key2", + []byte("value2")), + ), + ), + expectPush( + user.NewMetadataRemovedEvent( + context.Background(), + &userAgg.Aggregate, + "key2", + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + machine: &ChangeMachine{ + Metadata: []*domain.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + { + Key: "key2", + Value: []byte(""), + }, + }, + }, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "change machine metadata, no permission", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(userAddedEvent), + ), + ), + checkPermission: newMockPermissionCheckNotAllowed(), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + machine: &ChangeMachine{ + Metadata: []*domain.Metadata{ + { + Key: "key1", + Value: []byte("value1"), + }, + }, + }, + }, + res: res{ + err: func(err error) bool { + return errors.Is(err, zerrors.ThrowPermissionDenied(nil, "AUTHZ-HKJD33", "Errors.PermissionDenied")) + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { diff --git a/proto/zitadel/user/v2/user_service.proto b/proto/zitadel/user/v2/user_service.proto index 73b1cc82a7..b5815c4c21 100644 --- a/proto/zitadel/user/v2/user_service.proto +++ b/proto/zitadel/user/v2/user_service.proto @@ -2190,7 +2190,7 @@ message CreateUserRequest{ ]; option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_schema) = { - example: "{\"organizationId\":\"69629026806489455\",\"userId\":\"163840776835432345\",\"username\":\"minnie-mouse\",\"human\":{\"profile\":{\"givenName\":\"Minnie\",\"familyName\":\"Mouse\",\"nickName\":\"Mini\",\"displayName\":\"Minnie Mouse\",\"preferredLanguage\":\"en\",\"gender\":\"GENDER_FEMALE\"},\"email\":{\"email\":\"mini@mouse.com\",\"sendCode\":{\"urlTemplate\":\"https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}\"}},\"phone\":{\"phone\":\"+41791234567\",\"isVerified\":true},\"password\":{\"password\":\"Secr3tP4ssw0rd!\",\"changeRequired\":true},\"idpLinks\":[{\"idpId\":\"d654e6ba-70a3-48ef-a95d-37c8d8a7901a\",\"userId\":\"6516849804890468048461403518\",\"userName\":\"user@external.com\"}],\"totpSecret\":\"TJOPWSDYILLHXFV4MLKNNJOWFG7VSDCK\"}}"; + example: "{\"organizationId\":\"69629026806489455\",\"userId\":\"163840776835432345\",\"username\":\"minnie-mouse\",\"human\":{\"profile\":{\"givenName\":\"Minnie\",\"familyName\":\"Mouse\",\"nickName\":\"Mini\",\"displayName\":\"Minnie Mouse\",\"preferredLanguage\":\"en\",\"gender\":\"GENDER_FEMALE\"},\"email\":{\"email\":\"mini@mouse.com\",\"sendCode\":{\"urlTemplate\":\"https://example.com/email/verify?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}\"}},\"phone\":{\"phone\":\"+41791234567\",\"isVerified\":true},\"password\":{\"password\":\"Secr3tP4ssw0rd!\",\"changeRequired\":true},\"idpLinks\":[{\"idpId\":\"d654e6ba-70a3-48ef-a95d-37c8d8a7901a\",\"userId\":\"6516849804890468048461403518\",\"userName\":\"user@external.com\"}],\"totpSecret\":\"TJOPWSDYILLHXFV4MLKNNJOWFG7VSDCK\"},\"metadata\":[{\"key\":\"test1\",\"value\":\"VGhpcyBpcyBteSBmaXJzdCB2YWx1ZQ==\"},{\"key\":\"test2\",\"value\":\"VGhpcyBpcyBteSBzZWNvbmQgdmFsdWU=\"}]}"; }; } @@ -2607,8 +2607,17 @@ message UpdateUserRequest{ Human human = 3; Machine machine = 4; } + + // Metadata to be set. + // Metadata is expected as key-value pairs, and the value has to be base64 encoded. + repeated Metadata metadata = 5 [ + (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_field) = { + example: "[{\"key\": \"test1\", \"value\": \"VGhpcyBpcyBteSBmaXJzdCB2YWx1ZQ==\"}, {\"key\": \"test2\", \"value\": \"VGhpcyBpcyBteSBzZWNvbmQgdmFsdWU=\"}]" + } + ]; + option (grpc.gateway.protoc_gen_openapiv2.options.openapiv2_schema) = { - example: "{\"username\":\"minnie-mouse\",\"human\":{\"profile\":{\"givenName\":\"Minnie\",\"familyName\":\"Mouse\",\"displayName\":\"Minnie Mouse\"},\"email\":{\"email\":\"mini@mouse.com\",\"returnCode\":{}},\"phone\":{\"phone\":\"+41791234567\",\"isVerified\":true},\"password\":{\"password\":{\"password\":\"Secr3tP4ssw0rd!\",\"changeRequired\":true},\"verificationCode\":\"SKJd342k\"},\"totpSecret\":\"TJOPWSDYILLHXFV4MLKNNJOWFG7VSDCK\"}}"; + example: "{\"userId\":\"123456789012345678\",\"username\":\"minnie-mouse\",\"human\":{\"profile\":{\"givenName\":\"Minnie\",\"familyName\":\"Mouse\",\"displayName\":\"Minnie Mouse\"},\"email\":{\"email\":\"mini@mouse.com\",\"returnCode\":{}},\"phone\":{\"phone\":\"+41791234567\",\"isVerified\":true},\"password\":{\"password\":{\"password\":\"Secr3tP4ssw0rd!\",\"changeRequired\":true},\"verificationCode\":\"SKJd342k\"},\"totpSecret\":\"TJOPWSDYILLHXFV4MLKNNJOWFG7VSDCK\"},\"metadata\":[{\"key\": \"test1\", \"value\": \"VGhpcyBpcyBteSBmaXJzdCB2YWx1ZQ==\"}, {\"key\": \"test2\", \"value\": \"VGhpcyBpcyBteSBzZWNvbmQgdmFsdWU=\"}]}"; }; }