From 9266f8f00b209da88000ce01b3c2a6bda8d7a707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Fri, 15 Sep 2023 18:29:29 +0300 Subject: [PATCH] fix(command): allow email as username (#6565) Fixes #6460 Made the username checks consistent with create human user. --- internal/command/user.go | 14 ++- internal/command/user_model.go | 13 --- internal/command/user_test.go | 179 +++++++++++++++++++++++++++------ 3 files changed, 160 insertions(+), 46 deletions(-) diff --git a/internal/command/user.go b/internal/command/user.go index 77b9acbcc9..15631a9609 100644 --- a/internal/command/user.go +++ b/internal/command/user.go @@ -42,9 +42,17 @@ func (c *Commands) ChangeUsername(ctx context.Context, orgID, userID, userName s if err != nil { return nil, errors.ThrowPreconditionFailed(err, "COMMAND-38fnu", "Errors.Org.DomainPolicy.NotExisting") } - - if err := CheckDomainPolicyForUserName(userName, domainPolicy); err != nil { - return nil, err + if !domainPolicy.UserLoginMustBeDomain { + index := strings.LastIndex(userName, "@") + if index > 1 { + domainCheck := NewOrgDomainVerifiedWriteModel(userName[index+1:]) + if err := c.eventstore.FilterToQueryReducer(ctx, domainCheck); err != nil { + return nil, err + } + if domainCheck.Verified && domainCheck.ResourceOwner != orgID { + return nil, errors.ThrowInvalidArgument(nil, "COMMAND-Di2ei", "Errors.User.DomainNotAllowedAsUsername") + } + } } userAgg := UserAggregateFromWriteModel(&existingUser.WriteModel) diff --git a/internal/command/user_model.go b/internal/command/user_model.go index d8f671b462..7edfacfebf 100644 --- a/internal/command/user_model.go +++ b/internal/command/user_model.go @@ -1,12 +1,9 @@ package command import ( - "strings" - "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/domain" - caos_errors "github.com/zitadel/zitadel/internal/errors" "github.com/zitadel/zitadel/internal/repository/user" ) @@ -126,16 +123,6 @@ func UserAggregateFromWriteModel(wm *eventstore.WriteModel) *eventstore.Aggregat return eventstore.AggregateFromWriteModel(wm, user.AggregateType, user.AggregateVersion) } -func CheckDomainPolicyForUserName(userName string, policy *domain.DomainPolicy) error { - if policy == nil { - return caos_errors.ThrowPreconditionFailed(nil, "COMMAND-3Mb9s", "Errors.Users.DomainPolicyNil") - } - if policy.UserLoginMustBeDomain && strings.Contains(userName, "@") { - return caos_errors.ThrowPreconditionFailed(nil, "COMMAND-2k9fD", "Errors.User.EmailAsUsernameNotAllowed") - } - return nil -} - func isUserStateExists(state domain.UserState) bool { return !hasUserState(state, domain.UserStateDeleted, domain.UserStateUnspecified) } diff --git a/internal/command/user_test.go b/internal/command/user_test.go index 23dae95062..a6f16ee32e 100644 --- a/internal/command/user_test.go +++ b/internal/command/user_test.go @@ -23,7 +23,7 @@ import ( func TestCommandSide_UsernameChange(t *testing.T) { type fields struct { - eventstore *eventstore.Eventstore + eventstore func(*testing.T) *eventstore.Eventstore } type ( args struct { @@ -46,9 +46,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { { name: "userid missing, invalid argument error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -63,9 +61,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { { name: "orgid missing, invalid argument error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -80,9 +76,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { { name: "username missing, invalid argument error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -97,9 +91,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { { name: "username only spaces, invalid argument error", fields: fields{ - eventstore: eventstoreExpect( - t, - ), + eventstore: expectEventstore(), }, args: args{ ctx: context.Background(), @@ -114,8 +106,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { { name: "user removed, not found error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter(), ), }, @@ -132,8 +123,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { { name: "username not changed, precondition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -165,8 +155,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { { name: "username not changed (spaces), precondition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -198,8 +187,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { { name: "org iam policy not found, precondition error", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -229,10 +217,60 @@ func TestCommandSide_UsernameChange(t *testing.T) { }, }, { - name: "invalid username, precondition error", + name: "domain verified, wrong org", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter(), + expectFilter( + eventFromEventPusher( + instance.NewDomainPolicyAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + false, + true, + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainVerifiedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "test.ch", + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "wrong", + userID: "user1", + username: "test@test.ch", + }, + res: res{ + err: errors.IsErrorInvalidArgument, + }, + }, + { + name: "email as username, ok", + fields: fields{ + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -260,6 +298,20 @@ func TestCommandSide_UsernameChange(t *testing.T) { ), ), ), + expectPush( + []*repository.Event{ + eventFromEventPusher( + user.NewUsernameChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "test@test.ch", + true, + ), + ), + }, + uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("username", "org1", true)), + uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("test@test.ch", "org1", true)), + ), ), }, args: args{ @@ -269,14 +321,82 @@ func TestCommandSide_UsernameChange(t *testing.T) { username: "test@test.ch", }, res: res{ - err: errors.IsPreconditionFailed, + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, + { + name: "email as username, verified domain, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + ), + expectFilter(), + expectFilter( + eventFromEventPusher( + instance.NewDomainPolicyAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + false, + true, + true, + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewDomainVerifiedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + "test.ch", + ), + ), + ), + expectPush( + []*repository.Event{ + eventFromEventPusher( + user.NewUsernameChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "test@test.ch", + true, + ), + ), + }, + uniqueConstraintsFromEventConstraint(user.NewRemoveUsernameUniqueConstraint("username", "org1", false)), + uniqueConstraintsFromEventConstraint(user.NewAddUsernameUniqueConstraint("test@test.ch", "org1", false)), + ), + ), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + userID: "user1", + username: "test@test.ch", + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, }, }, { name: "change username, ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -335,8 +455,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { { name: "change username (remove spaces), ok", fields: fields{ - eventstore: eventstoreExpect( - t, + eventstore: expectEventstore( expectFilter( eventFromEventPusher( user.NewHumanAddedEvent(context.Background(), @@ -396,7 +515,7 @@ func TestCommandSide_UsernameChange(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { r := &Commands{ - eventstore: tt.fields.eventstore, + eventstore: tt.fields.eventstore(t), } got, err := r.ChangeUsername(tt.args.ctx, tt.args.orgID, tt.args.userID, tt.args.username) if tt.res.err == nil {