diff --git a/internal/api/grpc/auth/password.go b/internal/api/grpc/auth/password.go index 4629fa31cf..664958ff47 100644 --- a/internal/api/grpc/auth/password.go +++ b/internal/api/grpc/auth/password.go @@ -10,7 +10,7 @@ import ( func (s *Server) UpdateMyPassword(ctx context.Context, req *auth_pb.UpdateMyPasswordRequest) (*auth_pb.UpdateMyPasswordResponse, error) { ctxData := authz.GetCtxData(ctx) - objectDetails, err := s.command.ChangePassword(ctx, ctxData.ResourceOwner, ctxData.UserID, req.OldPassword, req.NewPassword, "") + objectDetails, err := s.command.ChangePassword(ctx, ctxData.ResourceOwner, ctxData.UserID, req.OldPassword, req.NewPassword, "", false) if err != nil { return nil, err } diff --git a/internal/api/grpc/user/v2/password.go b/internal/api/grpc/user/v2/password.go index 0651013cd9..a7d6a55f2e 100644 --- a/internal/api/grpc/user/v2/password.go +++ b/internal/api/grpc/user/v2/password.go @@ -51,9 +51,9 @@ func (s *Server) SetPassword(ctx context.Context, req *user.SetPasswordRequest) switch v := req.GetVerification().(type) { case *user.SetPasswordRequest_CurrentPassword: - details, err = s.command.ChangePassword(ctx, "", req.GetUserId(), v.CurrentPassword, req.GetNewPassword().GetPassword(), "") + details, err = s.command.ChangePassword(ctx, "", req.GetUserId(), v.CurrentPassword, req.GetNewPassword().GetPassword(), "", req.GetNewPassword().GetChangeRequired()) case *user.SetPasswordRequest_VerificationCode: - details, err = s.command.SetPasswordWithVerifyCode(ctx, "", req.GetUserId(), v.VerificationCode, req.GetNewPassword().GetPassword(), "") + details, err = s.command.SetPasswordWithVerifyCode(ctx, "", req.GetUserId(), v.VerificationCode, req.GetNewPassword().GetPassword(), "", req.GetNewPassword().GetChangeRequired()) case nil: details, err = s.command.SetPassword(ctx, "", req.GetUserId(), req.GetNewPassword().GetPassword(), req.GetNewPassword().GetChangeRequired()) default: diff --git a/internal/api/grpc/user/v2/user.go b/internal/api/grpc/user/v2/user.go index f6290e7248..d08df3d8f9 100644 --- a/internal/api/grpc/user/v2/user.go +++ b/internal/api/grpc/user/v2/user.go @@ -249,33 +249,12 @@ func SetHumanPasswordToPassword(password *user.SetPassword) *command.Password { if password == nil { return nil } - var changeRequired bool - var passwordStr *string - if password.GetPassword() != nil { - passwordStr = &password.GetPassword().Password - changeRequired = password.GetPassword().GetChangeRequired() - } - var hash *string - if password.GetHashedPassword() != nil { - hash = &password.GetHashedPassword().Hash - changeRequired = password.GetHashedPassword().GetChangeRequired() - } - var code *string - if password.GetVerificationCode() != "" { - codeT := password.GetVerificationCode() - code = &codeT - } - var oldPassword *string - if password.GetCurrentPassword() != "" { - oldPasswordT := password.GetCurrentPassword() - oldPassword = &oldPasswordT - } return &command.Password{ - PasswordCode: code, - OldPassword: oldPassword, - Password: passwordStr, - EncodedPasswordHash: hash, - ChangeRequired: changeRequired, + PasswordCode: password.GetVerificationCode(), + OldPassword: password.GetCurrentPassword(), + Password: password.GetPassword().GetPassword(), + EncodedPasswordHash: password.GetHashedPassword().GetHash(), + ChangeRequired: password.GetPassword().GetChangeRequired() || password.GetHashedPassword().GetChangeRequired(), } } diff --git a/internal/api/grpc/user/v2/user_integration_test.go b/internal/api/grpc/user/v2/user_integration_test.go index db6bdcdccb..8c070caab0 100644 --- a/internal/api/grpc/user/v2/user_integration_test.go +++ b/internal/api/grpc/user/v2/user_integration_test.go @@ -449,6 +449,40 @@ func TestServer_AddHumanUser(t *testing.T) { }, }, }, + { + name: "password not complexity conform", + args: args{ + CTX, + &user.AddHumanUserRequest{ + Organization: &object.Organization{ + Org: &object.Organization_OrgId{ + OrgId: Tester.Organisation.ID, + }, + }, + Profile: &user.SetHumanProfile{ + GivenName: "Donald", + FamilyName: "Duck", + NickName: gu.Ptr("Dukkie"), + DisplayName: gu.Ptr("Donald Duck"), + PreferredLanguage: gu.Ptr("en"), + Gender: user.Gender_GENDER_DIVERSE.Enum(), + }, + Email: &user.SetHumanEmail{}, + Metadata: []*user.SetMetadataEntry{ + { + Key: "somekey", + Value: []byte("somevalue"), + }, + }, + PasswordType: &user.AddHumanUserRequest_Password{ + Password: &user.Password{ + Password: "insufficient", + }, + }, + }, + }, + wantErr: true, + }, { name: "hashed password", args: args{ diff --git a/internal/api/ui/login/change_password_handler.go b/internal/api/ui/login/change_password_handler.go index 89b5ea41ab..1eacdcebd5 100644 --- a/internal/api/ui/login/change_password_handler.go +++ b/internal/api/ui/login/change_password_handler.go @@ -26,7 +26,7 @@ func (l *Login) handleChangePassword(w http.ResponseWriter, r *http.Request) { return } userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) - _, err = l.command.ChangePassword(setContext(r.Context(), authReq.UserOrgID), authReq.UserOrgID, authReq.UserID, data.OldPassword, data.NewPassword, userAgentID) + _, err = l.command.ChangePassword(setContext(r.Context(), authReq.UserOrgID), authReq.UserOrgID, authReq.UserID, data.OldPassword, data.NewPassword, userAgentID, false) if err != nil { l.renderChangePassword(w, r, authReq, err) return diff --git a/internal/api/ui/login/init_password_handler.go b/internal/api/ui/login/init_password_handler.go index 37fb9470c5..d57d1f83ff 100644 --- a/internal/api/ui/login/init_password_handler.go +++ b/internal/api/ui/login/init_password_handler.go @@ -80,7 +80,7 @@ func (l *Login) checkPWCode(w http.ResponseWriter, r *http.Request, authReq *dom userOrg = authReq.UserOrgID } userAgentID, _ := http_mw.UserAgentIDFromCtx(r.Context()) - _, err := l.command.SetPasswordWithVerifyCode(setContext(r.Context(), userOrg), userOrg, data.UserID, data.Code, data.Password, userAgentID) + _, err := l.command.SetPasswordWithVerifyCode(setContext(r.Context(), userOrg), userOrg, data.UserID, data.Code, data.Password, userAgentID, false) if err != nil { l.renderInitPassword(w, r, authReq, data.UserID, "", err) return diff --git a/internal/command/user_human_init.go b/internal/command/user_human_init.go index 60160743c6..83a2b74a9d 100644 --- a/internal/command/user_human_init.go +++ b/internal/command/user_human_init.go @@ -83,7 +83,7 @@ func (c *Commands) HumanVerifyInitCode(ctx context.Context, userID, resourceOwne commands = append(commands, user.NewHumanEmailVerifiedEvent(ctx, userAgg)) } if password != "" { - passwordCommand, err := c.setPasswordCommand(ctx, userAgg, domain.UserStateActive, password, userAgentID, false, false) + passwordCommand, err := c.setPasswordCommand(ctx, userAgg, domain.UserStateActive, password, "", userAgentID, false, nil) if err != nil { return err } diff --git a/internal/command/user_human_password.go b/internal/command/user_human_password.go index d7567c6478..fd8b29a429 100644 --- a/internal/command/user_human_password.go +++ b/internal/command/user_human_password.go @@ -3,6 +3,7 @@ package command import ( "context" "errors" + "time" "github.com/zitadel/logging" "github.com/zitadel/passwap" @@ -25,16 +26,18 @@ func (c *Commands) SetPassword(ctx context.Context, orgID, userID, password stri if err != nil { return nil, err } - if !wm.UserState.Exists() { - return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-3M0fs", "Errors.User.NotFound") - } - if err = c.checkPermission(ctx, domain.PermissionUserWrite, wm.ResourceOwner, userID); err != nil { - return nil, err - } - return c.setPassword(ctx, wm, password, "", oneTime) + return c.setPassword( + ctx, + wm, + password, + "", // current api implementations never provide an encoded password + "", + oneTime, + c.setPasswordWithPermission(wm.AggregateID, wm.ResourceOwner), + ) } -func (c *Commands) SetPasswordWithVerifyCode(ctx context.Context, orgID, userID, code, password, userAgentID string) (objectDetails *domain.ObjectDetails, err error) { +func (c *Commands) SetPasswordWithVerifyCode(ctx context.Context, orgID, userID, code, password, userAgentID string, changeRequired bool) (objectDetails *domain.ObjectDetails, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -48,66 +51,19 @@ func (c *Commands) SetPasswordWithVerifyCode(ctx context.Context, orgID, userID, if err != nil { return nil, err } - - if wm.Code == nil || wm.UserState == domain.UserStateUnspecified || wm.UserState == domain.UserStateDeleted { - return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-2M9fs", "Errors.User.Code.NotFound") - } - - err = crypto.VerifyCode(wm.CodeCreationDate, wm.CodeExpiry, wm.Code, code, c.userEncryption) - if err != nil { - return nil, err - } - - return c.setPassword(ctx, wm, password, userAgentID, false) -} - -// setEncodedPassword add change event from already encoded password to HumanPasswordWriteModel and return the necessary object details for response -func (c *Commands) setEncodedPassword(ctx context.Context, wm *HumanPasswordWriteModel, password, userAgentID string, changeRequired bool) (objectDetails *domain.ObjectDetails, err error) { - agg := user.NewAggregate(wm.AggregateID, wm.ResourceOwner) - command, err := c.setPasswordCommand(ctx, &agg.Aggregate, wm.UserState, password, userAgentID, changeRequired, true) - if err != nil { - return nil, err - } - err = c.pushAppendAndReduce(ctx, wm, command) - if err != nil { - return nil, err - } - return writeModelToObjectDetails(&wm.WriteModel), nil -} - -// setPassword add change event to HumanPasswordWriteModel and return the necessary object details for response -func (c *Commands) setPassword(ctx context.Context, wm *HumanPasswordWriteModel, password, userAgentID string, changeRequired bool) (objectDetails *domain.ObjectDetails, err error) { - agg := user.NewAggregate(wm.AggregateID, wm.ResourceOwner) - command, err := c.setPasswordCommand(ctx, &agg.Aggregate, wm.UserState, password, userAgentID, changeRequired, false) - if err != nil { - return nil, err - } - err = c.pushAppendAndReduce(ctx, wm, command) - if err != nil { - return nil, err - } - return writeModelToObjectDetails(&wm.WriteModel), nil -} - -func (c *Commands) setPasswordCommand(ctx context.Context, agg *eventstore.Aggregate, userState domain.UserState, password, userAgentID string, changeRequired, encoded bool) (_ eventstore.Command, err error) { - if err = c.canUpdatePassword(ctx, password, agg.ResourceOwner, userState); err != nil { - return nil, err - } - - if !encoded { - ctx, span := tracing.NewNamedSpan(ctx, "passwap.Hash") - encodedPassword, err := c.userPasswordHasher.Hash(password) - span.EndWithError(err) - if err = convertPasswapErr(err); err != nil { - return nil, err - } - return user.NewHumanPasswordChangedEvent(ctx, agg, encodedPassword, changeRequired, userAgentID), nil - } - return user.NewHumanPasswordChangedEvent(ctx, agg, password, changeRequired, userAgentID), nil + return c.setPassword( + ctx, + wm, + password, + "", + userAgentID, + changeRequired, + c.setPasswordWithVerifyCode(wm.CodeCreationDate, wm.CodeExpiry, wm.Code, code), + ) } // ChangePassword change password of existing user -func (c *Commands) ChangePassword(ctx context.Context, orgID, userID, oldPassword, newPassword, userAgentID string) (objectDetails *domain.ObjectDetails, err error) { +func (c *Commands) ChangePassword(ctx context.Context, orgID, userID, oldPassword, newPassword, userAgentID string, changeRequired bool) (objectDetails *domain.ObjectDetails, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -121,12 +77,125 @@ func (c *Commands) ChangePassword(ctx context.Context, orgID, userID, oldPasswor if err != nil { return nil, err } + return c.setPassword( + ctx, + wm, + newPassword, + "", + userAgentID, + changeRequired, + c.checkCurrentPassword(newPassword, "", oldPassword, wm.EncodedHash), + ) +} - newPasswordHash, err := c.verifyAndUpdatePassword(ctx, wm.EncodedHash, oldPassword, newPassword) +type setPasswordVerification func(ctx context.Context) (newEncodedPassword string, err error) + +// 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.checkPermission(ctx, domain.PermissionUserWrite, orgID, userID) + } +} + +// setPasswordWithVerifyCode returns a password code check as [setPasswordVerification] implementation +func (c *Commands) setPasswordWithVerifyCode( + passwordCodeCreationDate time.Time, + passwordCodeExpiry time.Duration, + passwordCode *crypto.CryptoValue, + code string, +) setPasswordVerification { + return func(ctx context.Context) (_ string, err error) { + if passwordCode == nil { + return "", zerrors.ThrowPreconditionFailed(nil, "COMMAND-2M9fs", "Errors.User.Code.NotFound") + } + _, spanCrypto := tracing.NewNamedSpan(ctx, "crypto.VerifyCode") + defer func() { + spanCrypto.EndWithError(err) + }() + return "", crypto.VerifyCode(passwordCodeCreationDate, passwordCodeExpiry, passwordCode, code, c.userEncryption) + } +} + +// checkCurrentPassword returns a password check as [setPasswordVerification] implementation +func (c *Commands) checkCurrentPassword( + newPassword, newEncodedPassword, currentPassword, currentEncodePassword string, +) setPasswordVerification { + // in case the new password is already encoded, we only need to verify the current + if newEncodedPassword != "" { + return func(ctx context.Context) (_ string, err error) { + _, spanPasswap := tracing.NewNamedSpan(ctx, "passwap.Verify") + _, err = c.userPasswordHasher.Verify(currentEncodePassword, currentPassword) + spanPasswap.EndWithError(err) + return "", convertPasswapErr(err) + } + } + + // otherwise let's directly verify and return the new generate hash, so we can reuse it in the event + return func(ctx context.Context) (string, error) { + return c.verifyAndUpdatePassword(ctx, currentEncodePassword, currentPassword, newPassword) + } +} + +// setPassword directly pushes the intent of [setPasswordCommand] to the eventstore and returns the [domain.ObjectDetails] +func (c *Commands) setPassword( + ctx context.Context, + wm *HumanPasswordWriteModel, + password, encodedPassword, userAgentID string, + changeRequired bool, + verificationCheck setPasswordVerification, +) (*domain.ObjectDetails, error) { + agg := user.NewAggregate(wm.AggregateID, wm.ResourceOwner) + command, err := c.setPasswordCommand(ctx, &agg.Aggregate, wm.UserState, password, encodedPassword, userAgentID, changeRequired, verificationCheck) if err != nil { return nil, err } - return c.setEncodedPassword(ctx, wm, newPasswordHash, userAgentID, false) + err = c.pushAppendAndReduce(ctx, wm, command) + if err != nil { + return nil, err + } + return writeModelToObjectDetails(&wm.WriteModel), nil +} + +// setPasswordCommand creates the command / intent for changing a user's password. +// It will check the user's [domain.UserState] to be existing and not initial, +// if the caller is allowed to change the password (permission, by code or by providing the current password), +// and it will ensure the new password (if provided as plain) corresponds to the password complexity policy. +// If not already encoded, the new password will be hashed. +func (c *Commands) setPasswordCommand(ctx context.Context, agg *eventstore.Aggregate, userState domain.UserState, password, encodedPassword, userAgentID string, changeRequired bool, verificationCheck setPasswordVerification) (_ eventstore.Command, err error) { + if !isUserStateExists(userState) { + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-G8dh3", "Errors.User.Password.NotFound") + } + if isUserStateInitial(userState) { + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-M9dse", "Errors.User.NotInitialised") + } + if verificationCheck != nil { + newEncodedPassword, err := verificationCheck(ctx) + if err != nil { + return nil, err + } + // use the new hash from the verification in case there is one (e.g. existing pw check) + if newEncodedPassword != "" { + encodedPassword = newEncodedPassword + } + } + // If password is provided, let's check if is compliant with the policy. + // If only a encodedPassword is passed, we can skip this. + if password != "" { + if err = c.checkPasswordComplexity(ctx, password, agg.ResourceOwner); err != nil { + return nil, err + } + } + + // In case only a plain password was passed, we need to hash it. + if encodedPassword == "" { + _, span := tracing.NewNamedSpan(ctx, "passwap.Hash") + encodedPassword, err = c.userPasswordHasher.Hash(password) + span.EndWithError(err) + if err = convertPasswapErr(err); err != nil { + return nil, err + } + } + return user.NewHumanPasswordChangedEvent(ctx, agg, encodedPassword, changeRequired, userAgentID), nil } // verifyAndUpdatePassword verify if the old password is correct with the encoded hash and @@ -142,17 +211,11 @@ func (c *Commands) verifyAndUpdatePassword(ctx context.Context, encodedHash, old return updated, convertPasswapErr(err) } -// canUpdatePassword checks uf the given password can be used to be the password of a user -func (c *Commands) canUpdatePassword(ctx context.Context, newPassword string, resourceOwner string, state domain.UserState) (err error) { +// checkPasswordComplexity checks uf the given password can be used to be the password of a user +func (c *Commands) checkPasswordComplexity(ctx context.Context, newPassword string, resourceOwner string) (err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() - if !isUserStateExists(state) { - return zerrors.ThrowNotFound(nil, "COMMAND-G8dh3", "Errors.User.Password.NotFound") - } - if state == domain.UserStateInitial { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-M9dse", "Errors.User.NotInitialised") - } policy, err := c.getOrgPasswordComplexityPolicy(ctx, resourceOwner) if err != nil { return err diff --git a/internal/command/user_human_password_test.go b/internal/command/user_human_password_test.go index 965309ef50..4e7672d8bc 100644 --- a/internal/command/user_human_password_test.go +++ b/internal/command/user_human_password_test.go @@ -267,12 +267,13 @@ func TestCommandSide_SetPasswordWithVerifyCode(t *testing.T) { userPasswordHasher *crypto.Hasher } type args struct { - ctx context.Context - userID string - code string - resourceOwner string - password string - userAgentID string + ctx context.Context + userID string + code string + resourceOwner string + password string + userAgentID string + changeRequired bool } type res struct { want *domain.ObjectDetails @@ -562,6 +563,84 @@ func TestCommandSide_SetPasswordWithVerifyCode(t *testing.T) { }, }, }, + { + name: "set password with changeRequired, 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, + ), + ), + eventFromEventPusher( + user.NewHumanEmailVerifiedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + ), + ), + eventFromEventPusherWithCreationDateNow( + user.NewHumanPasswordCodeAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("a"), + }, + time.Hour*1, + domain.NotificationTypeEmail, + "", + ), + ), + ), + expectFilter( + eventFromEventPusher( + org.NewPasswordComplexityPolicyAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + 1, + false, + false, + false, + false, + ), + ), + ), + expectPush( + user.NewHumanPasswordChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "$plain$x$password", + true, + "", + ), + ), + ), + userPasswordHasher: mockPasswordHasher("x"), + userEncryption: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + resourceOwner: "org1", + password: "password", + code: "a", + userAgentID: "", + changeRequired: true, + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -570,7 +649,7 @@ func TestCommandSide_SetPasswordWithVerifyCode(t *testing.T) { userPasswordHasher: tt.fields.userPasswordHasher, userEncryption: tt.fields.userEncryption, } - got, err := r.SetPasswordWithVerifyCode(tt.args.ctx, tt.args.resourceOwner, tt.args.userID, tt.args.code, tt.args.password, tt.args.userAgentID) + got, err := r.SetPasswordWithVerifyCode(tt.args.ctx, tt.args.resourceOwner, tt.args.userID, tt.args.code, tt.args.password, tt.args.userAgentID, tt.args.changeRequired) if tt.res.err == nil { assert.NoError(t, err) } @@ -589,12 +668,13 @@ func TestCommandSide_ChangePassword(t *testing.T) { userPasswordHasher *crypto.Hasher } type args struct { - ctx context.Context - userID string - resourceOwner string - oldPassword string - newPassword string - userAgentID string + ctx context.Context + userID string + resourceOwner string + oldPassword string + newPassword string + userAgentID string + changeRequired bool } type res struct { want *domain.ObjectDetails @@ -700,6 +780,64 @@ func TestCommandSide_ChangePassword(t *testing.T) { err: zerrors.IsPreconditionFailed, }, }, + { + name: "password not matching complexity policy, invalid argument error", + fields: fields{ + userPasswordHasher: mockPasswordHasher("x"), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + oldPassword: "password-old", + newPassword: "password1", + resourceOwner: "org1", + }, + expect: []expect{ + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + eventFromEventPusher( + user.NewHumanEmailVerifiedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + ), + ), + eventFromEventPusher( + user.NewHumanPasswordChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "$plain$x$password-old", + false, + "")), + ), + expectFilter( + eventFromEventPusher( + org.NewPasswordComplexityPolicyAddedEvent( + context.Background(), + &org.NewAggregate("org1").Aggregate, + 1, + true, + true, + true, + true, + ), + ), + ), + }, + res: res{ + err: zerrors.IsErrorInvalidArgument, + }, + }, { name: "password not matching, invalid argument error", fields: fields{ @@ -788,7 +926,7 @@ func TestCommandSide_ChangePassword(t *testing.T) { expectFilter( eventFromEventPusher( org.NewPasswordComplexityPolicyAddedEvent(context.Background(), - &user.NewAggregate("user1", "org1").Aggregate, + &org.NewAggregate("org1").Aggregate, 1, false, false, @@ -880,6 +1018,75 @@ func TestCommandSide_ChangePassword(t *testing.T) { }, }, }, + { + name: "change password with changeRequired, ok", + fields: fields{ + userPasswordHasher: mockPasswordHasher("x"), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + resourceOwner: "org1", + oldPassword: "password", + newPassword: "password1", + userAgentID: "", + changeRequired: true, + }, + expect: []expect{ + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + eventFromEventPusher( + user.NewHumanEmailVerifiedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + ), + ), + eventFromEventPusher( + user.NewHumanPasswordChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "$plain$x$password", + false, + "")), + ), + expectFilter( + eventFromEventPusher( + org.NewPasswordComplexityPolicyAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + 1, + false, + false, + false, + false, + ), + ), + ), + expectPush( + user.NewHumanPasswordChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "$plain$x$password1", + true, + "", + ), + ), + }, + res: res{ + want: &domain.ObjectDetails{ + ResourceOwner: "org1", + }, + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -887,7 +1094,7 @@ func TestCommandSide_ChangePassword(t *testing.T) { eventstore: eventstoreExpect(t, tt.expect...), userPasswordHasher: tt.fields.userPasswordHasher, } - got, err := r.ChangePassword(tt.args.ctx, tt.args.resourceOwner, tt.args.userID, tt.args.oldPassword, tt.args.newPassword, tt.args.userAgentID) + got, err := r.ChangePassword(tt.args.ctx, tt.args.resourceOwner, tt.args.userID, tt.args.oldPassword, tt.args.newPassword, tt.args.userAgentID, tt.args.changeRequired) if tt.res.err == nil { assert.NoError(t, err) } diff --git a/internal/command/user_v2_human.go b/internal/command/user_v2_human.go index dfd8470a65..3cd3269e4b 100644 --- a/internal/command/user_v2_human.go +++ b/internal/command/user_v2_human.go @@ -43,10 +43,10 @@ type Profile struct { type Password struct { // Either you have to have permission, a password code or the old password to change - PasswordCode *string - OldPassword *string - Password *string - EncodedPasswordHash *string + PasswordCode string + OldPassword string + Password string + EncodedPasswordHash string ChangeRequired bool } @@ -73,12 +73,12 @@ func (h *ChangeHuman) Validate(hasher *crypto.Hasher) (err error) { } func (p *Password) Validate(hasher *crypto.Hasher) error { - if p.EncodedPasswordHash != nil { - if !hasher.EncodingSupported(*p.EncodedPasswordHash) { + if p.EncodedPasswordHash != "" { + if !hasher.EncodingSupported(p.EncodedPasswordHash) { return zerrors.ThrowInvalidArgument(nil, "USER-oz74onzvqr", "Errors.User.Password.NotSupported") } } - if p.Password == nil && p.EncodedPasswordHash == nil { + if p.Password == "" && p.EncodedPasswordHash == "" { return zerrors.ThrowInvalidArgument(nil, "COMMAND-3klek4sbns", "Errors.User.Password.Empty") } return nil @@ -285,7 +285,7 @@ func (c *Commands) ChangeUserHuman(ctx context.Context, human *ChangeHuman, alg } } if human.Password != nil { - cmds, err = c.changeUserPassword(ctx, cmds, existingHuman, human.Password, alg) + cmds, err = c.changeUserPassword(ctx, cmds, existingHuman, human.Password) if err != nil { return err } @@ -370,57 +370,34 @@ func changeUserProfile(ctx context.Context, cmds []eventstore.Command, wm *UserV return cmds, err } -func (c *Commands) changeUserPassword(ctx context.Context, cmds []eventstore.Command, wm *UserV2WriteModel, password *Password, alg crypto.EncryptionAlgorithm) ([]eventstore.Command, error) { +func (c *Commands) changeUserPassword(ctx context.Context, cmds []eventstore.Command, wm *UserV2WriteModel, password *Password) ([]eventstore.Command, error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.End() }() - // Either have a code to set the password - if password.PasswordCode != nil { - if err := crypto.VerifyCode(wm.PasswordCodeCreationDate, wm.PasswordCodeExpiry, wm.PasswordCode, *password.PasswordCode, alg); err != nil { - return cmds, err - } + // if no verification is set, the user must have the permission to change the password + verification := c.setPasswordWithPermission(wm.AggregateID, wm.ResourceOwner) + // otherwise check the password code... + if password.PasswordCode != "" { + verification = c.setPasswordWithVerifyCode(wm.PasswordCodeCreationDate, wm.PasswordCodeExpiry, wm.PasswordCode, password.PasswordCode) } - var encodedPassword string - // or have the old password to change it - if password.OldPassword != nil { - // newly encode old password if no new and already encoded password is set - pw := *password.OldPassword - if password.Password != nil { - pw = *password.Password - } - alreadyEncodedPassword, err := c.verifyAndUpdatePassword(ctx, wm.PasswordEncodedHash, *password.OldPassword, pw) - if err != nil { - return cmds, err - } - encodedPassword = alreadyEncodedPassword + // ...or old password + if password.OldPassword != "" { + verification = c.checkCurrentPassword(password.Password, password.EncodedPasswordHash, password.OldPassword, wm.PasswordEncodedHash) } - - // password already hashed in request - if password.EncodedPasswordHash != nil { - cmd, err := c.setPasswordCommand(ctx, &wm.Aggregate().Aggregate, wm.UserState, *password.EncodedPasswordHash, "", password.ChangeRequired, true) - if cmd != nil { - return append(cmds, cmd), err - } - return cmds, err + cmd, err := c.setPasswordCommand( + ctx, + &wm.Aggregate().Aggregate, + wm.UserState, + password.Password, + password.EncodedPasswordHash, + "", + password.ChangeRequired, + verification, + ) + if cmd != nil { + return append(cmds, cmd), err } - // password already hashed in verify - if encodedPassword != "" { - cmd, err := c.setPasswordCommand(ctx, &wm.Aggregate().Aggregate, wm.UserState, encodedPassword, "", password.ChangeRequired, true) - if cmd != nil { - return append(cmds, cmd), err - } - return cmds, err - } - // password still to be hashed - if password.Password != nil { - cmd, err := c.setPasswordCommand(ctx, &wm.Aggregate().Aggregate, wm.UserState, *password.Password, "", password.ChangeRequired, false) - if cmd != nil { - return append(cmds, cmd), err - } - return cmds, err - } - // no password changes necessary - return cmds, nil + return cmds, err } func (c *Commands) userExistsWriteModel(ctx context.Context, userID string) (writeModel *UserV2WriteModel, err error) { diff --git a/internal/command/user_v2_human_test.go b/internal/command/user_v2_human_test.go index f0ea86c42a..4a11ba0ea1 100644 --- a/internal/command/user_v2_human_test.go +++ b/internal/command/user_v2_human_test.go @@ -2014,8 +2014,8 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - Password: gu.Ptr("password2"), - OldPassword: gu.Ptr("password"), + Password: "password2", + OldPassword: "password", ChangeRequired: true, }, }, @@ -2061,8 +2061,8 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - Password: gu.Ptr("password2"), - OldPassword: gu.Ptr("password"), + Password: "password2", + OldPassword: "password", ChangeRequired: true, }, }, @@ -2085,7 +2085,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - OldPassword: gu.Ptr("password"), + OldPassword: "password", ChangeRequired: true, }, }, @@ -2119,7 +2119,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - Password: gu.Ptr("password2"), + Password: "password2", ChangeRequired: true, }, }, @@ -2173,7 +2173,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - Password: gu.Ptr("password2"), + Password: "password2", ChangeRequired: true, }, }, @@ -2229,8 +2229,8 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - Password: gu.Ptr("password2"), - OldPassword: gu.Ptr("password"), + Password: "password2", + OldPassword: "password", ChangeRequired: true, }, }, @@ -2266,8 +2266,8 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - Password: gu.Ptr("password2"), - OldPassword: gu.Ptr("wrong"), + Password: "password2", + OldPassword: "wrong", ChangeRequired: true, }, }, @@ -2336,8 +2336,8 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - Password: gu.Ptr("password2"), - PasswordCode: gu.Ptr("code"), + Password: "password2", + PasswordCode: "code", ChangeRequired: true, }, }, @@ -2389,8 +2389,8 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - Password: gu.Ptr("password2"), - PasswordCode: gu.Ptr("wrong"), + Password: "password2", + PasswordCode: "wrong", ChangeRequired: true, }, }, @@ -2403,7 +2403,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { }, }, { - name: "change human password encoded, password code, ok", + name: "change human password, password code, not matching policy", fields: fields{ eventstore: expectEventstore( expectFilter( @@ -2436,9 +2436,58 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { org.NewPasswordComplexityPolicyAddedEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, 1, - false, - false, - false, + true, + true, + true, + true, + ), + ), + ), + ), + checkPermission: newMockPermissionCheckAllowed(), + userPasswordHasher: mockPasswordHasher("x"), + }, + args: args{ + ctx: context.Background(), + orgID: "org1", + human: &ChangeHuman{ + Password: &Password{ + Password: "password2", + PasswordCode: "code", + ChangeRequired: true, + }, + }, + codeAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + res: res{ + err: zerrors.IsErrorInvalidArgument, + }, + }, + { + name: "change human password encoded, password code, ok", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + newAddHumanEvent("$plain$x$password", true, true, "", language.English), + ), + eventFromEventPusher( + user.NewHumanInitializedCheckSucceededEvent(context.Background(), + &userAgg.Aggregate, + ), + ), + eventFromEventPusherWithCreationDateNow( + user.NewHumanPasswordCodeAddedEventV2(context.Background(), + &userAgg.Aggregate, + &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("code"), + }, + time.Hour*1, + domain.NotificationTypeEmail, + "", false, ), ), @@ -2460,8 +2509,8 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - EncodedPasswordHash: gu.Ptr("$plain$x$password2"), - PasswordCode: gu.Ptr("code"), + EncodedPasswordHash: "$plain$x$password2", + PasswordCode: "code", ChangeRequired: true, }, }, @@ -2533,9 +2582,9 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { orgID: "org1", human: &ChangeHuman{ Password: &Password{ - Password: gu.Ptr("passwordnotused"), - EncodedPasswordHash: gu.Ptr("$plain$x$password2"), - PasswordCode: gu.Ptr("code"), + Password: "passwordnotused", + EncodedPasswordHash: "$plain$x$password2", + PasswordCode: "code", ChangeRequired: true, }, }, @@ -2557,6 +2606,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) { userPasswordHasher: tt.fields.userPasswordHasher, newEncryptedCode: tt.fields.newCode, checkPermission: tt.fields.checkPermission, + userEncryption: tt.args.codeAlg, } err := r.ChangeUserHuman(tt.args.ctx, tt.args.human, tt.args.codeAlg) if tt.res.err == nil {