fix: correct user self management on metadata and delete (#10666)

# Which Problems Are Solved

This PR fixes the self-management of users for metadata and own removal
and improves the corresponding permission checks.
While looking into the problems, I also noticed that there's a bug in
the metadata mapping when using `api.metadata.push` in actions v1 and
that re-adding a previously existing key after its removal was not
possible.

# How the Problems Are Solved

- Added a parameter `allowSelfManagement` to checkPermissionOnUser to
not require a permission if a user is changing its own data.
- Updated use of `NewPermissionCheckUserWrite` including prevention of
self-management for metadata.
- Pass permission check to the command side (for metadata functions) to
allow it implicitly for login v1 and actions v1.
- Use of json.Marshal for the metadata mapping (as with
`AppendMetadata`)
- Check the metadata state when comparing the value.

# Additional Changes

- added a variadic `roles` parameter to the `CreateOrgMembership`
integration test helper function to allow defining specific roles.

# Additional Context

- noted internally while testing v4.1.x
- requires backport to v4.x
- closes https://github.com/zitadel/zitadel/issues/10470
- relates to https://github.com/zitadel/zitadel/pull/10426

internal/command/permission_checks.go

@@ -39,29 +39,36 @@ func (c *Commands) newPermissionCheck(ctx context.Context, permission string, ag
 	}
 }
 
-func (c *Commands) checkPermissionOnUser(ctx context.Context, permission string) PermissionCheck {
+func (c *Commands) checkPermissionOnUser(ctx context.Context, permission string, allowSelfManagement bool) PermissionCheck {
 	return func(resourceOwner, aggregateID string) error {
-		if aggregateID != "" && aggregateID == authz.GetCtxData(ctx).UserID {
+		if allowSelfManagement && aggregateID != "" && aggregateID == authz.GetCtxData(ctx).UserID {
 			return nil
 		}
 		return c.newPermissionCheck(ctx, permission, user.AggregateType)(resourceOwner, aggregateID)
 	}
 }
 
-func (c *Commands) NewPermissionCheckUserWrite(ctx context.Context) PermissionCheck {
-	return c.checkPermissionOnUser(ctx, domain.PermissionUserWrite)
+func (c *Commands) NewPermissionCheckUserWrite(ctx context.Context, allowSelfManagement bool) PermissionCheck {
+	return c.checkPermissionOnUser(ctx, domain.PermissionUserWrite, allowSelfManagement)
 }
 
 func (c *Commands) checkPermissionDeleteUser(ctx context.Context, resourceOwner, userID string) error {
-	return c.checkPermissionOnUser(ctx, domain.PermissionUserDelete)(resourceOwner, userID)
+	err := c.checkPermissionOnUser(ctx, domain.PermissionUserDelete, false)(resourceOwner, userID)
+	if err == nil {
+		return nil
+	}
+	if userID != authz.GetCtxData(ctx).UserID {
+		return err
+	}
+	return c.checkPermissionOnUser(ctx, domain.PermissionUserDeleteSelf, false)(resourceOwner, userID)
 }
 
-func (c *Commands) checkPermissionUpdateUser(ctx context.Context, resourceOwner, userID string) error {
-	return c.NewPermissionCheckUserWrite(ctx)(resourceOwner, userID)
+func (c *Commands) checkPermissionUpdateUser(ctx context.Context, resourceOwner, userID string, allowSelfManagement bool) error {
+	return c.NewPermissionCheckUserWrite(ctx, allowSelfManagement)(resourceOwner, userID)
 }
 
 func (c *Commands) checkPermissionUpdateUserCredentials(ctx context.Context, resourceOwner, userID string) error {
-	return c.checkPermissionOnUser(ctx, domain.PermissionUserCredentialWrite)(resourceOwner, userID)
+	return c.checkPermissionOnUser(ctx, domain.PermissionUserCredentialWrite, true)(resourceOwner, userID)
 }
 
 func (c *Commands) checkPermissionCreateProject(ctx context.Context, resourceOwner, projectID string) error {

internal/command/permission_checks_test.go

@@ -147,6 +147,7 @@ func TestCommands_CheckPermissionUserWrite(t *testing.T) {
 	type args struct {
 		ctx                        context.Context
 		resourceOwner, aggregateID string
+		allowSelfManagement        bool
 	}
 	type want struct {
 		err func(error) bool
@@ -163,8 +164,29 @@ func TestCommands_CheckPermissionUserWrite(t *testing.T) {
 				ctx: authz.SetCtxData(context.Background(), authz.CtxData{
 					UserID: "aggregateID",
 				}),
-				resourceOwner: "resourceOwner",
-				aggregateID:   "aggregateID",
+				resourceOwner:       "resourceOwner",
+				aggregateID:         "aggregateID",
+				allowSelfManagement: true,
+			},
+		},
+		{
+			name: "self, no selfManagementAllowed, permission check",
+			fields: fields{
+				domainPermissionCheck: mockDomainPermissionCheck(
+					authz.SetCtxData(context.Background(), authz.CtxData{
+						UserID: "aggregateID",
+					}),
+					"user.write",
+					"resourceOwner",
+					"aggregateID"),
+			},
+			args: args{
+				ctx: authz.SetCtxData(context.Background(), authz.CtxData{
+					UserID: "aggregateID",
+				}),
+				resourceOwner:       "resourceOwner",
+				aggregateID:         "aggregateID",
+				allowSelfManagement: false,
 			},
 		},
 		{
@@ -194,7 +216,7 @@ func TestCommands_CheckPermissionUserWrite(t *testing.T) {
 			if tt.fields.domainPermissionCheck != nil {
 				c.checkPermission = tt.fields.domainPermissionCheck(t)
 			}
-			err := c.NewPermissionCheckUserWrite(tt.args.ctx)(tt.args.resourceOwner, tt.args.aggregateID)
+			err := c.NewPermissionCheckUserWrite(tt.args.ctx, tt.args.allowSelfManagement)(tt.args.resourceOwner, tt.args.aggregateID)
 			if tt.want.err != nil {
 				assert.True(t, tt.want.err(err))
 			}
@@ -223,7 +245,40 @@ func TestCommands_CheckPermissionUserDelete(t *testing.T) {
 		want   want
 	}{
 		{
-			name: "self, no permission check",
+			name: "self permission allowed, permission check",
+			fields: fields{
+				domainPermissionCheck: mockDomainPermissionCheck(
+					userCtx,
+					"user.delete",
+					"resourceOwner",
+					"aggregateID"),
+			},
+			args: args{
+				ctx:           userCtx,
+				resourceOwner: "resourceOwner",
+				aggregateID:   "aggregateID",
+			},
+		},
+		{
+			name: "self user.delete not allowed, user.self.delete permission check",
+			fields: fields{
+				domainPermissionCheck: mockDomainPermissionChecks(
+					expectedCheck{
+						userCtx,
+						"user.delete",
+						"resourceOwner",
+						"aggregateID",
+						zerrors.ThrowPermissionDenied(nil, "id", "permission denied"),
+					},
+					expectedCheck{
+						userCtx,
+						"user.self.delete",
+						"resourceOwner",
+						"aggregateID",
+						nil,
+					},
+				),
+			},
 			args: args{
 				ctx:           userCtx,
 				resourceOwner: "resourceOwner",
@@ -276,3 +331,41 @@ func mockDomainPermissionCheck(expectCtx context.Context, expectPermission, expe
 		}
 	}
 }
+
+type expectedCheck struct {
+	ctx           context.Context
+	permission    string
+	resourceOwner string
+	resourceID    string
+	err           error
+}
+
+func mockDomainPermissionChecks(checks ...expectedCheck) func(t *testing.T) domain.PermissionCheck {
+	var i int
+	return func(t *testing.T) domain.PermissionCheck {
+		t.Cleanup(func() {
+			t.Helper()
+			if i != len(checks) {
+				t.Logf("not all expected checks were called, expected: %d, got: %d", len(checks), i)
+				for ; i < len(checks); i++ {
+					t.Logf("missing call: %+v", checks[i])
+				}
+				t.Fail()
+			}
+		})
+
+		return func(ctx context.Context, permission, orgID, resourceID string) (err error) {
+			if i >= len(checks) {
+				assert.Fail(t, "no more checks expected")
+				return nil
+			}
+			expect := checks[i]
+			assert.Equal(t, expect.ctx, ctx)
+			assert.Equal(t, expect.permission, permission)
+			assert.Equal(t, expect.resourceOwner, orgID)
+			assert.Equal(t, expect.resourceID, resourceID)
+			i++
+			return expect.err
+		}
+	}
+}

internal/command/user_human_otp.go

@@ -237,7 +237,7 @@ func (c *Commands) HumanRemoveTOTP(ctx context.Context, userID, resourceOwner st
 	if existingOTP.State == domain.MFAStateUnspecified || existingOTP.State == domain.MFAStateRemoved {
 		return nil, zerrors.ThrowNotFound(nil, "COMMAND-Hd9sd", "Errors.User.MFA.OTP.NotExisting")
 	}
-	if err := c.checkPermissionUpdateUser(ctx, existingOTP.ResourceOwner, userID); err != nil {
+	if err := c.checkPermissionUpdateUser(ctx, existingOTP.ResourceOwner, userID, true); err != nil {
 		return nil, err
 	}
 	userAgg := UserAggregateFromWriteModel(&existingOTP.WriteModel)
@@ -309,7 +309,7 @@ func (c *Commands) RemoveHumanOTPSMS(ctx context.Context, userID, resourceOwner
 	if err != nil {
 		return nil, err
 	}
-	if err := c.checkPermissionUpdateUser(ctx, existingOTP.WriteModel.ResourceOwner, userID); err != nil {
+	if err := c.checkPermissionUpdateUser(ctx, existingOTP.WriteModel.ResourceOwner, userID, true); err != nil {
 		return nil, err
 	}
 	if !existingOTP.otpAdded {
@@ -439,7 +439,7 @@ func (c *Commands) RemoveHumanOTPEmail(ctx context.Context, userID, resourceOwne
 	if err != nil {
 		return nil, err
 	}
-	if err := c.checkPermissionUpdateUser(ctx, existingOTP.WriteModel.ResourceOwner, userID); err != nil {
+	if err := c.checkPermissionUpdateUser(ctx, existingOTP.WriteModel.ResourceOwner, userID, true); err != nil {
 		return nil, err
 	}
 	if !existingOTP.otpAdded {

internal/command/user_human_password.go

@@ -112,7 +112,7 @@ type setPasswordVerification func(ctx context.Context) (newEncodedPassword strin
 // setPasswordWithPermission returns a permission check as [setPasswordVerification] implementation
 func (c *Commands) setPasswordWithPermission(userID, orgID string) setPasswordVerification {
 	return func(ctx context.Context) (_ string, err error) {
-		return "", c.checkPermissionUpdateUser(ctx, orgID, userID)
+		return "", c.checkPermissionUpdateUser(ctx, orgID, userID, false)
 	}
 }
 

internal/command/user_human_webauthn.go

@@ -601,7 +601,7 @@ func (c *Commands) removeHumanWebAuthN(ctx context.Context, userID, webAuthNID,
 		return nil, zerrors.ThrowNotFound(nil, "COMMAND-DAfb2", "Errors.User.WebAuthN.NotFound")
 	}
 
-	if err := c.checkPermissionUpdateUser(ctx, existingWebAuthN.ResourceOwner, existingWebAuthN.AggregateID); err != nil {
+	if err := c.checkPermissionUpdateUser(ctx, existingWebAuthN.ResourceOwner, existingWebAuthN.AggregateID, true); err != nil {
 		return nil, err
 	}
 

internal/command/user_metadata.go

@@ -11,7 +11,7 @@ import (
 	"github.com/zitadel/zitadel/internal/zerrors"
 )
 
-func (c *Commands) SetUserMetadata(ctx context.Context, metadata *domain.Metadata, userID, resourceOwner string) (_ *domain.Metadata, err error) {
+func (c *Commands) SetUserMetadata(ctx context.Context, metadata *domain.Metadata, userID, resourceOwner string, check PermissionCheck) (_ *domain.Metadata, err error) {
 	ctx, span := tracing.NewSpan(ctx)
 	defer func() { span.EndWithError(err) }()
 
@@ -20,8 +20,10 @@ func (c *Commands) SetUserMetadata(ctx context.Context, metadata *domain.Metadat
 		return nil, err
 	}
 
-	if err := c.checkPermissionUpdateUser(ctx, userResourceOwner, userID); err != nil {
-		return nil, err
+	if check != nil {
+		if err := check(userResourceOwner, userID); err != nil {
+			return nil, err
+		}
 	}
 
 	setMetadata, err := c.getUserMetadataModelByID(ctx, userID, userResourceOwner, metadata.Key)
@@ -30,7 +32,7 @@ func (c *Commands) SetUserMetadata(ctx context.Context, metadata *domain.Metadat
 	}
 	userAgg := UserAggregateFromWriteModel(&setMetadata.WriteModel)
 	// return if no change in the metadata
-	if bytes.Equal(setMetadata.Value, metadata.Value) {
+	if setMetadata.State == domain.MetadataStateActive && bytes.Equal(setMetadata.Value, metadata.Value) {
 		return writeModelToUserMetadata(setMetadata), nil
 	}
 
@@ -50,7 +52,7 @@ func (c *Commands) SetUserMetadata(ctx context.Context, metadata *domain.Metadat
 	return writeModelToUserMetadata(setMetadata), nil
 }
 
-func (c *Commands) BulkSetUserMetadata(ctx context.Context, userID, resourceOwner string, metadatas ...*domain.Metadata) (_ *domain.ObjectDetails, err error) {
+func (c *Commands) BulkSetUserMetadata(ctx context.Context, userID, resourceOwner string, check PermissionCheck, metadatas ...*domain.Metadata) (_ *domain.ObjectDetails, err error) {
 	if len(metadatas) == 0 {
 		return nil, zerrors.ThrowPreconditionFailed(nil, "META-9mm2d", "Errors.Metadata.NoData")
 	}
@@ -59,8 +61,10 @@ func (c *Commands) BulkSetUserMetadata(ctx context.Context, userID, resourceOwne
 		return nil, err
 	}
 
-	if err := c.checkPermissionUpdateUser(ctx, userResourceOwner, userID); err != nil {
-		return nil, err
+	if check != nil {
+		if err := check(userResourceOwner, userID); err != nil {
+			return nil, err
+		}
 	}
 
 	events := make([]eventstore.Command, 0)
@@ -109,7 +113,7 @@ func (c *Commands) setUserMetadata(ctx context.Context, userAgg *eventstore.Aggr
 	), nil
 }
 
-func (c *Commands) RemoveUserMetadata(ctx context.Context, metadataKey, userID, resourceOwner string) (_ *domain.ObjectDetails, err error) {
+func (c *Commands) RemoveUserMetadata(ctx context.Context, metadataKey, userID, resourceOwner string, check PermissionCheck) (_ *domain.ObjectDetails, err error) {
 	if metadataKey == "" {
 		return nil, zerrors.ThrowInvalidArgument(nil, "META-2n0fs", "Errors.Metadata.Invalid")
 	}
@@ -117,9 +121,10 @@ func (c *Commands) RemoveUserMetadata(ctx context.Context, metadataKey, userID,
 	if err != nil {
 		return nil, err
 	}
-
-	if err := c.checkPermissionUpdateUser(ctx, userResourceOwner, userID); err != nil {
-		return nil, err
+	if check != nil {
+		if err := check(userResourceOwner, userID); err != nil {
+			return nil, err
+		}
 	}
 
 	removeMetadata, err := c.getUserMetadataModelByID(ctx, userID, userResourceOwner, metadataKey)
@@ -146,7 +151,7 @@ func (c *Commands) RemoveUserMetadata(ctx context.Context, metadataKey, userID,
 	return writeModelToObjectDetails(&removeMetadata.WriteModel), nil
 }
 
-func (c *Commands) BulkRemoveUserMetadata(ctx context.Context, userID, resourceOwner string, metadataKeys ...string) (_ *domain.ObjectDetails, err error) {
+func (c *Commands) BulkRemoveUserMetadata(ctx context.Context, userID, resourceOwner string, check PermissionCheck, metadataKeys ...string) (_ *domain.ObjectDetails, err error) {
 	if len(metadataKeys) == 0 {
 		return nil, zerrors.ThrowPreconditionFailed(nil, "META-9mm2d", "Errors.Metadata.NoData")
 	}
@@ -154,9 +159,10 @@ func (c *Commands) BulkRemoveUserMetadata(ctx context.Context, userID, resourceO
 	if err != nil {
 		return nil, err
 	}
-
-	if err := c.checkPermissionUpdateUser(ctx, userResourceOwner, userID); err != nil {
-		return nil, err
+	if check != nil {
+		if err := check(userResourceOwner, userID); err != nil {
+			return nil, err
+		}
 	}
 
 	events := make([]eventstore.Command, len(metadataKeys))

internal/command/user_metadata_test.go

@@ -16,8 +16,7 @@ import (
 
 func TestCommandSide_SetUserMetadata(t *testing.T) {
 	type fields struct {
-		eventstore      func(t *testing.T) *eventstore.Eventstore
-		checkPermission domain.PermissionCheck
+		eventstore func(t *testing.T) *eventstore.Eventstore
 	}
 	type (
 		args struct {
@@ -25,6 +24,7 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 			orgID    string
 			userID   string
 			metadata *domain.Metadata
+			check    PermissionCheck
 		}
 	)
 	type res struct {
@@ -43,7 +43,6 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 				eventstore: expectEventstore(
 					expectFilter(),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:    context.Background(),
@@ -88,7 +87,6 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:    context.Background(),
@@ -123,7 +121,6 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckNotAllowed(),
 			},
 			args: args{
 				ctx:    context.Background(),
@@ -133,6 +130,9 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 					Key:   "key",
 					Value: []byte("value"),
 				},
+				check: func(resourceOwner, aggregateID string) error {
+					return zerrors.ThrowPermissionDenied(nil, "id", "permission denied")
+				},
 			},
 			res: res{
 				err: zerrors.IsPermissionDenied,
@@ -167,7 +167,6 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:    context.Background(),
@@ -218,9 +217,13 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 								[]byte("value"),
 							),
 						),
+						eventFromEventPusher(
+							user.NewMetadataRemovedAllEvent(context.Background(),
+								&user.NewAggregate("user1", "org1").Aggregate,
+							),
+						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:    context.Background(),
@@ -235,7 +238,7 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 			},
 		},
 		{
-			name: "add metadata, reset, ok",
+			name: "add metadata with same key, ok",
 			fields: fields{
 				eventstore: expectEventstore(
 					expectFilter(
@@ -271,7 +274,6 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:    context.Background(),
@@ -294,14 +296,130 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "add metadata with same key and value, ok (ignore)",
+			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 protected]",
+								true,
+							),
+						),
+					),
+					expectFilter(
+						eventFromEventPusher(
+							user.NewMetadataSetEvent(context.Background(),
+								&user.NewAggregate("user1", "org1").Aggregate,
+								"key",
+								[]byte("value"),
+							),
+						),
+					),
+				),
+			},
+			args: args{
+				ctx:    context.Background(),
+				orgID:  "org1",
+				userID: "user1",
+				metadata: &domain.Metadata{
+					Key:   "key",
+					Value: []byte("value"),
+				},
+			},
+			res: res{
+				want: &domain.Metadata{
+					ObjectRoot: models.ObjectRoot{
+						AggregateID:   "user1",
+						ResourceOwner: "org1",
+					},
+					Key:   "key",
+					Value: []byte("value"),
+					State: domain.MetadataStateActive,
+				},
+			},
+		},
+		{
+			name: "add deleted metadata with same value, 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 protected]",
+								true,
+							),
+						),
+					),
+					expectFilter(
+						eventFromEventPusher(
+							user.NewMetadataSetEvent(context.Background(),
+								&user.NewAggregate("user1", "org1").Aggregate,
+								"key",
+								[]byte("value"),
+							),
+						),
+						eventFromEventPusher(
+							user.NewMetadataRemovedEvent(context.Background(),
+								&user.NewAggregate("user1", "org1").Aggregate,
+								"key",
+							),
+						),
+					),
+					expectPush(
+						user.NewMetadataSetEvent(context.Background(),
+							&user.NewAggregate("user1", "org1").Aggregate,
+							"key",
+							[]byte("value"),
+						),
+					),
+				),
+			},
+			args: args{
+				ctx:    context.Background(),
+				orgID:  "org1",
+				userID: "user1",
+				metadata: &domain.Metadata{
+					Key:   "key",
+					Value: []byte("value"),
+				},
+			},
+			res: res{
+				want: &domain.Metadata{
+					ObjectRoot: models.ObjectRoot{
+						AggregateID:   "user1",
+						ResourceOwner: "org1",
+					},
+					Key:   "key",
+					Value: []byte("value"),
+					State: domain.MetadataStateActive,
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			r := &Commands{
-				eventstore:      tt.fields.eventstore(t),
-				checkPermission: tt.fields.checkPermission,
+				eventstore: tt.fields.eventstore(t),
 			}
-			got, err := r.SetUserMetadata(tt.args.ctx, tt.args.metadata, tt.args.userID, tt.args.orgID)
+			got, err := r.SetUserMetadata(tt.args.ctx, tt.args.metadata, tt.args.userID, tt.args.orgID, tt.args.check)
 			if tt.res.err == nil {
 				assert.NoError(t, err)
 			}
@@ -317,14 +435,14 @@ func TestCommandSide_SetUserMetadata(t *testing.T) {
 
 func TestCommandSide_BulkSetUserMetadata(t *testing.T) {
 	type fields struct {
-		eventstore      func(t *testing.T) *eventstore.Eventstore
-		checkPermission domain.PermissionCheck
+		eventstore func(t *testing.T) *eventstore.Eventstore
 	}
 	type (
 		args struct {
 			ctx          context.Context
 			orgID        string
 			userID       string
+			check        PermissionCheck
 			metadataList []*domain.Metadata
 		}
 	)
@@ -394,7 +512,6 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) {
 					),
 					expectFilter(),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:    context.Background(),
@@ -430,12 +547,14 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckNotAllowed(),
 			},
 			args: args{
 				ctx:    context.Background(),
 				orgID:  "org1",
 				userID: "user1",
+				check: func(resourceOwner, aggregateID string) error {
+					return zerrors.ThrowPermissionDenied(nil, "id", "permission-denied")
+				},
 				metadataList: []*domain.Metadata{
 					{Key: "key", Value: []byte("value")},
 					{Key: "key1", Value: []byte("value1")},
@@ -479,7 +598,6 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:    context.Background(),
@@ -496,14 +614,82 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "re add deleted 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 protected]",
+								true,
+							),
+						),
+					),
+					expectFilter(
+						eventFromEventPusher(
+							user.NewMetadataSetEvent(context.Background(),
+								&user.NewAggregate("user1", "org1").Aggregate,
+								"key",
+								[]byte("value"),
+							)),
+						eventFromEventPusher(
+							user.NewMetadataSetEvent(context.Background(),
+								&user.NewAggregate("user1", "org1").Aggregate,
+								"key1",
+								[]byte("value1"),
+							)),
+						eventFromEventPusher(
+							user.NewMetadataRemovedEvent(context.Background(),
+								&user.NewAggregate("user1", "org1").Aggregate,
+								"key",
+							)),
+					),
+					expectPush(
+						user.NewMetadataSetEvent(context.Background(),
+							&user.NewAggregate("user1", "org1").Aggregate,
+							"key",
+							[]byte("value"),
+						),
+						user.NewMetadataSetEvent(context.Background(),
+							&user.NewAggregate("user1", "org1").Aggregate,
+							"key2",
+							[]byte("value2"),
+						),
+					),
+				),
+			},
+			args: args{
+				ctx:    context.Background(),
+				orgID:  "org1",
+				userID: "user1",
+				metadataList: []*domain.Metadata{
+					{Key: "key", Value: []byte("value")},
+					{Key: "key1", Value: []byte("value1")},
+					{Key: "key2", Value: []byte("value2")},
+				},
+			},
+			res: res{
+				want: &domain.ObjectDetails{
+					ResourceOwner: "org1",
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			r := &Commands{
-				eventstore:      tt.fields.eventstore(t),
-				checkPermission: tt.fields.checkPermission,
+				eventstore: tt.fields.eventstore(t),
 			}
-			got, err := r.BulkSetUserMetadata(tt.args.ctx, tt.args.userID, tt.args.orgID, tt.args.metadataList...)
+			got, err := r.BulkSetUserMetadata(tt.args.ctx, tt.args.userID, tt.args.orgID, tt.args.check, tt.args.metadataList...)
 			if tt.res.err == nil {
 				assert.NoError(t, err)
 			}
@@ -519,8 +705,7 @@ func TestCommandSide_BulkSetUserMetadata(t *testing.T) {
 
 func TestCommandSide_UserRemoveMetadata(t *testing.T) {
 	type fields struct {
-		eventstore      func(t *testing.T) *eventstore.Eventstore
-		checkPermission domain.PermissionCheck
+		eventstore func(t *testing.T) *eventstore.Eventstore
 	}
 	type (
 		args struct {
@@ -528,6 +713,7 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) {
 			orgID       string
 			userID      string
 			metadataKey string
+			check       PermissionCheck
 		}
 	)
 	type res struct {
@@ -594,7 +780,6 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) {
 					),
 					expectFilter(),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:         context.Background(),
@@ -627,13 +812,15 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckNotAllowed(),
 			},
 			args: args{
 				ctx:         context.Background(),
 				orgID:       "org1",
 				userID:      "user1",
 				metadataKey: "key",
+				check: func(resourceOwner, aggregateID string) error {
+					return zerrors.ThrowPermissionDenied(nil, "id", "permission denied")
+				},
 			},
 			res: res{
 				err: zerrors.IsPermissionDenied,
@@ -675,7 +862,6 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:         context.Background(),
@@ -693,10 +879,9 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			r := &Commands{
-				eventstore:      tt.fields.eventstore(t),
-				checkPermission: tt.fields.checkPermission,
+				eventstore: tt.fields.eventstore(t),
 			}
-			got, err := r.RemoveUserMetadata(tt.args.ctx, tt.args.metadataKey, tt.args.userID, tt.args.orgID)
+			got, err := r.RemoveUserMetadata(tt.args.ctx, tt.args.metadataKey, tt.args.userID, tt.args.orgID, tt.args.check)
 			if tt.res.err == nil {
 				assert.NoError(t, err)
 			}
@@ -712,14 +897,14 @@ func TestCommandSide_UserRemoveMetadata(t *testing.T) {
 
 func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) {
 	type fields struct {
-		eventstore      func(t *testing.T) *eventstore.Eventstore
-		checkPermission domain.PermissionCheck
+		eventstore func(t *testing.T) *eventstore.Eventstore
 	}
 	type (
 		args struct {
 			ctx          context.Context
 			orgID        string
 			userID       string
+			check        PermissionCheck
 			metadataList []string
 		}
 	)
@@ -794,7 +979,6 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:          context.Background(),
@@ -843,7 +1027,6 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:          context.Background(),
@@ -876,13 +1059,15 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckNotAllowed(),
 			},
 			args: args{
 				ctx:          context.Background(),
 				orgID:        "org1",
 				userID:       "user1",
 				metadataList: []string{"key", "key1"},
+				check: func(resourceOwner, aggregateID string) error {
+					return zerrors.ThrowPermissionDenied(nil, "id", "permission denied")
+				},
 			},
 			res: res{
 				err: zerrors.IsPermissionDenied,
@@ -935,7 +1120,6 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) {
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
 			},
 			args: args{
 				ctx:          context.Background(),
@@ -953,10 +1137,9 @@ func TestCommandSide_BulkRemoveUserMetadata(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			r := &Commands{
-				eventstore:      tt.fields.eventstore(t),
-				checkPermission: tt.fields.checkPermission,
+				eventstore: tt.fields.eventstore(t),
 			}
-			got, err := r.BulkRemoveUserMetadata(tt.args.ctx, tt.args.userID, tt.args.orgID, tt.args.metadataList...)
+			got, err := r.BulkRemoveUserMetadata(tt.args.ctx, tt.args.userID, tt.args.orgID, tt.args.check, tt.args.metadataList...)
 			if tt.res.err == nil {
 				assert.NoError(t, err)
 			}

internal/command/user_v2.go

@@ -28,7 +28,7 @@ func (c *Commands) LockUserV2(ctx context.Context, userID string) (*domain.Objec
 		return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-lgws8wtsqf", "Errors.User.ShouldBeActiveOrInitial")
 	}
 
-	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID); err != nil {
+	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID, false); err != nil {
 		return nil, err
 	}
 
@@ -53,7 +53,7 @@ func (c *Commands) UnlockUserV2(ctx context.Context, userID string) (*domain.Obj
 	if !hasUserState(existingHuman.UserState, domain.UserStateLocked) {
 		return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-olb9vb0oca", "Errors.User.NotLocked")
 	}
-	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID); err != nil {
+	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID, false); err != nil {
 		return nil, err
 	}
 
@@ -81,7 +81,7 @@ func (c *Commands) DeactivateUserV2(ctx context.Context, userID string) (*domain
 	if isUserStateInactive(existingHuman.UserState) {
 		return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-5gunjw0cd7", "Errors.User.AlreadyInactive")
 	}
-	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID); err != nil {
+	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID, false); err != nil {
 		return nil, err
 	}
 
@@ -106,7 +106,7 @@ func (c *Commands) ReactivateUserV2(ctx context.Context, userID string) (*domain
 	if !isUserStateInactive(existingHuman.UserState) {
 		return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-s5qqcz97hf", "Errors.User.NotInactive")
 	}
-	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID); err != nil {
+	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID, false); err != nil {
 		return nil, err
 	}
 

internal/command/user_v2_email.go

@@ -148,7 +148,7 @@ func (c *Commands) changeUserEmailWithGeneratorEvents(ctx context.Context, userI
 	if err != nil {
 		return nil, err
 	}
-	if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil {
+	if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID, true); err != nil {
 		return nil, err
 	}
 	if err = cmd.Change(ctx, domain.EmailAddress(email)); err != nil {
@@ -170,7 +170,7 @@ func (c *Commands) sendUserEmailCodeWithGeneratorEvents(ctx context.Context, use
 	if err != nil {
 		return nil, err
 	}
-	if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil {
+	if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID, true); err != nil {
 		return nil, err
 	}
 	if existingCheck && cmd.model.Code == nil {

internal/command/user_v2_human.go

@@ -138,7 +138,7 @@ func (c *Commands) AddUserHuman(ctx context.Context, resourceOwner string, human
 	}
 	// check for permission to create user on resourceOwner
 	if !human.Register {
-		if err := c.checkPermissionUpdateUser(ctx, resourceOwner, human.ID); err != nil {
+		if err := c.checkPermissionUpdateUser(ctx, resourceOwner, human.ID, true); err != nil {
 			return err
 		}
 	}
@@ -279,6 +279,7 @@ func (c *Commands) ChangeUserHuman(ctx context.Context, human *ChangeHuman, alg
 		return err
 	}
 
+	metadataChanged := len(human.Metadata) > 0 || len(human.MetadataKeysToRemove) > 0
 	existingHuman, err := c.UserHumanWriteModel(
 		ctx,
 		human.ID,
@@ -289,14 +290,14 @@ func (c *Commands) ChangeUserHuman(ctx context.Context, human *ChangeHuman, alg
 		human.Password != nil,
 		false, // avatar not updateable
 		false, // IDPLinks not updateable
-		len(human.Metadata) > 0 || len(human.MetadataKeysToRemove) > 0,
+		metadataChanged,
 	)
 	if err != nil {
 		return err
 	}
 
 	if human.Changed() {
-		if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID); err != nil {
+		if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID, !metadataChanged); err != nil {
 			return err
 		}
 	}
@@ -522,7 +523,7 @@ func (c *Commands) HumanMFAInitSkippedV2(ctx context.Context, userID string) (*d
 	if !isUserStateExists(existingHuman.UserState) {
 		return nil, zerrors.ThrowNotFound(nil, "COMMAND-auj6jeBei4", "Errors.User.NotFound")
 	}
-	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID); err != nil {
+	if err := c.checkPermissionUpdateUser(ctx, existingHuman.ResourceOwner, existingHuman.AggregateID, true); err != nil {
 		return nil, err
 	}
 

internal/command/user_v2_machine.go

@@ -45,7 +45,7 @@ func (c *Commands) ChangeUserMachine(ctx context.Context, machine *ChangeMachine
 		return err
 	}
 	if machine.Changed() {
-		if err := c.checkPermissionUpdateUser(ctx, existingMachine.ResourceOwner, existingMachine.AggregateID); err != nil {
+		if err := c.checkPermissionUpdateUser(ctx, existingMachine.ResourceOwner, existingMachine.AggregateID, true); err != nil {
 			return err
 		}
 	}

internal/command/user_v2_password.go

@@ -49,7 +49,7 @@ func (c *Commands) requestPasswordReset(ctx context.Context, userID string, retu
 	if model.UserState == domain.UserStateInitial {
 		return nil, nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sfe4g", "Errors.User.NotInitialised")
 	}
-	if err = c.checkPermissionUpdateUser(ctx, model.ResourceOwner, userID); err != nil {
+	if err = c.checkPermissionUpdateUser(ctx, model.ResourceOwner, userID, true); err != nil {
 		return nil, nil, err
 	}
 	var passwordCode *EncryptedCode

internal/command/user_v2_phone.go

@@ -82,7 +82,7 @@ func (c *Commands) changeUserPhoneWithGenerator(ctx context.Context, userID, pho
 	if err != nil {
 		return nil, err
 	}
-	if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil {
+	if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID, true); err != nil {
 		return nil, err
 	}
 	if err = cmd.Change(ctx, domain.PhoneNumber(phone)); err != nil {
@@ -102,7 +102,7 @@ func (c *Commands) resendUserPhoneCodeWithGenerator(ctx context.Context, userID
 	if err != nil {
 		return nil, err
 	}
-	if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID); err != nil {
+	if err = c.checkPermissionUpdateUser(ctx, cmd.aggregate.ResourceOwner, userID, true); err != nil {
 		return nil, err
 	}
 	if cmd.model.Code == nil && cmd.model.GeneratorID == "" {

internal/command/user_v2_test.go

@@ -1352,7 +1352,7 @@ func TestCommandSide_RemoveUserV2(t *testing.T) {
 			},
 		},
 		{
-			name: "remove self, ok",
+			name: "remove self, permission denied",
 			fields: fields{
 				eventstore: expectEventstore(
 					expectFilter(
@@ -1371,25 +1371,6 @@ func TestCommandSide_RemoveUserV2(t *testing.T) {
 							),
 						),
 					),
-					expectFilter(
-						eventFromEventPusher(
-							org.NewDomainPolicyAddedEvent(ctx,
-								orgAgg,
-								true,
-								true,
-								true,
-							),
-						),
-					),
-					expectFilterOrganizationSettings("org1", false, false),
-					expectPush(
-						user.NewUserRemovedEvent(ctx,
-							&user.NewAggregate(ctxUserID, "org1").Aggregate,
-							"username",
-							nil,
-							true,
-						),
-					),
 				),
 				checkPermission: newMockPermissionCheckNotAllowed(),
 			},
@@ -1397,9 +1378,7 @@ func TestCommandSide_RemoveUserV2(t *testing.T) {
 				userID: ctxUserID,
 			},
 			res: res{
-				want: &domain.ObjectDetails{
-					ResourceOwner: "org1",
-				},
+				err: zerrors.IsPermissionDenied,
 			},
 		},
 	}