diff --git a/internal/api/grpc/session/v2/session_integration_test.go b/internal/api/grpc/session/v2/session_integration_test.go index 689893319..c6fd1b5d4 100644 --- a/internal/api/grpc/session/v2/session_integration_test.go +++ b/internal/api/grpc/session/v2/session_integration_test.go @@ -14,12 +14,15 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/zitadel/logging" + "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" + "google.golang.org/grpc/status" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/durationpb" "google.golang.org/protobuf/types/known/timestamppb" "github.com/zitadel/zitadel/internal/integration" + mgmt "github.com/zitadel/zitadel/pkg/grpc/management" object "github.com/zitadel/zitadel/pkg/grpc/object/v2beta" session "github.com/zitadel/zitadel/pkg/grpc/session/v2beta" user "github.com/zitadel/zitadel/pkg/grpc/user/v2beta" @@ -27,6 +30,7 @@ import ( var ( CTX context.Context + IAMOwnerCTX context.Context Tester *integration.Tester Client session.SessionServiceClient User *user.AddHumanUserResponse @@ -44,6 +48,7 @@ func TestMain(m *testing.M) { Client = Tester.Client.SessionV2 CTX, _ = Tester.WithAuthorization(ctx, integration.OrgOwner), errCtx + IAMOwnerCTX = Tester.WithAuthorization(ctx, integration.IAMOwner) User = createFullUser(CTX) DeactivatedUser = createDeactivatedUser(CTX) LockedUser = createLockedUser(CTX) @@ -341,6 +346,48 @@ func TestServer_CreateSession(t *testing.T) { } } +func TestServer_CreateSession_lock_user(t *testing.T) { + // create a separate org so we don't interfere with any other test + org := Tester.CreateOrganization(IAMOwnerCTX, + fmt.Sprintf("TestServer_CreateSession_lock_user_%d", time.Now().UnixNano()), + fmt.Sprintf("%d@mouse.com", time.Now().UnixNano()), + ) + userID := org.CreatedAdmins[0].GetUserId() + Tester.SetUserPassword(IAMOwnerCTX, userID, integration.UserPassword, false) + + // enable password lockout + maxAttempts := 2 + ctxOrg := metadata.AppendToOutgoingContext(IAMOwnerCTX, "x-zitadel-orgid", org.GetOrganizationId()) + _, err := Tester.Client.Mgmt.AddCustomLockoutPolicy(ctxOrg, &mgmt.AddCustomLockoutPolicyRequest{ + MaxPasswordAttempts: uint32(maxAttempts), + }) + require.NoError(t, err) + + for i := 0; i <= maxAttempts; i++ { + _, err := Client.CreateSession(CTX, &session.CreateSessionRequest{ + Checks: &session.Checks{ + User: &session.CheckUser{ + Search: &session.CheckUser_UserId{ + UserId: userID, + }, + }, + Password: &session.CheckPassword{ + Password: "invalid", + }, + }, + }) + assert.Error(t, err) + statusCode := status.Code(err) + expectedCode := codes.InvalidArgument + // as soon as we hit the limit the user is locked and following request will + // already deny any check with a precondition failed since the user is locked + if i >= maxAttempts { + expectedCode = codes.FailedPrecondition + } + assert.Equal(t, expectedCode, statusCode) + } +} + func TestServer_CreateSession_webauthn(t *testing.T) { // create new session with user and request the webauthn challenge createResp, err := Client.CreateSession(CTX, &session.CreateSessionRequest{ diff --git a/internal/auth/repository/eventsourcing/eventstore/auth_request.go b/internal/auth/repository/eventsourcing/eventstore/auth_request.go index a5105d1f5..e695d8351 100644 --- a/internal/auth/repository/eventsourcing/eventstore/auth_request.go +++ b/internal/auth/repository/eventsourcing/eventstore/auth_request.go @@ -340,11 +340,7 @@ func (repo *AuthRequestRepo) VerifyPassword(ctx context.Context, authReqID, user } return err } - policy, err := repo.getLockoutPolicy(ctx, resourceOwner) - if err != nil { - return err - } - err = repo.Command.HumanCheckPassword(ctx, resourceOwner, userID, password, request.WithCurrentInfo(info), lockoutPolicyToDomain(policy)) + err = repo.Command.HumanCheckPassword(ctx, resourceOwner, userID, password, request.WithCurrentInfo(info)) if isIgnoreUserInvalidPasswordError(err, request) { return zerrors.ThrowInvalidArgument(nil, "EVENT-Jsf32", "Errors.User.UsernameOrPassword.Invalid") } diff --git a/internal/command/instance_policy_password_lockout.go b/internal/command/instance_policy_password_lockout.go index 59766ae38..9e677c573 100644 --- a/internal/command/instance_policy_password_lockout.go +++ b/internal/command/instance_policy_password_lockout.go @@ -32,7 +32,7 @@ func (c *Commands) AddDefaultLockoutPolicy(ctx context.Context, maxPasswordAttem } func (c *Commands) ChangeDefaultLockoutPolicy(ctx context.Context, policy *domain.LockoutPolicy) (*domain.LockoutPolicy, error) { - existingPolicy, err := c.defaultLockoutPolicyWriteModelByID(ctx) + existingPolicy, err := defaultLockoutPolicyWriteModelByID(ctx, c.eventstore.FilterToQueryReducer) if err != nil { return nil, err } @@ -63,12 +63,12 @@ func (c *Commands) ChangeDefaultLockoutPolicy(ctx context.Context, policy *domai return writeModelToLockoutPolicy(&existingPolicy.LockoutPolicyWriteModel), nil } -func (c *Commands) defaultLockoutPolicyWriteModelByID(ctx context.Context) (policy *InstanceLockoutPolicyWriteModel, err error) { +func defaultLockoutPolicyWriteModelByID(ctx context.Context, reducer func(ctx context.Context, r eventstore.QueryReducer) error) (policy *InstanceLockoutPolicyWriteModel, err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() writeModel := NewInstanceLockoutPolicyWriteModel(ctx) - err = c.eventstore.FilterToQueryReducer(ctx, writeModel) + err = reducer(ctx, writeModel) if err != nil { return nil, err } diff --git a/internal/command/org_policy_lockout.go b/internal/command/org_policy_lockout.go index 052fd9e23..d7ace6f69 100644 --- a/internal/command/org_policy_lockout.go +++ b/internal/command/org_policy_lockout.go @@ -4,6 +4,7 @@ import ( "context" "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/repository/org" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -12,7 +13,7 @@ func (c *Commands) AddLockoutPolicy(ctx context.Context, resourceOwner string, p if resourceOwner == "" { return nil, zerrors.ThrowInvalidArgument(nil, "Org-8fJif", "Errors.ResourceOwnerMissing") } - addedPolicy, err := c.orgLockoutPolicyWriteModelByID(ctx, resourceOwner) + addedPolicy, err := orgLockoutPolicyWriteModelByID(ctx, resourceOwner, c.eventstore.FilterToQueryReducer) if err != nil { return nil, err } @@ -42,7 +43,7 @@ func (c *Commands) ChangeLockoutPolicy(ctx context.Context, resourceOwner string if resourceOwner == "" { return nil, zerrors.ThrowInvalidArgument(nil, "Org-3J9fs", "Errors.ResourceOwnerMissing") } - existingPolicy, err := c.orgLockoutPolicyWriteModelByID(ctx, resourceOwner) + existingPolicy, err := orgLockoutPolicyWriteModelByID(ctx, resourceOwner, c.eventstore.FilterToQueryReducer) if err != nil { return nil, err } @@ -71,7 +72,7 @@ func (c *Commands) RemoveLockoutPolicy(ctx context.Context, orgID string) (*doma if orgID == "" { return nil, zerrors.ThrowInvalidArgument(nil, "Org-4J9fs", "Errors.ResourceOwnerMissing") } - existingPolicy, err := c.orgLockoutPolicyWriteModelByID(ctx, orgID) + existingPolicy, err := orgLockoutPolicyWriteModelByID(ctx, orgID, c.eventstore.FilterToQueryReducer) if err != nil { return nil, err } @@ -93,7 +94,7 @@ func (c *Commands) RemoveLockoutPolicy(ctx context.Context, orgID string) (*doma } func (c *Commands) removeLockoutPolicyIfExists(ctx context.Context, orgID string) (*org.LockoutPolicyRemovedEvent, error) { - existingPolicy, err := c.orgLockoutPolicyWriteModelByID(ctx, orgID) + existingPolicy, err := orgLockoutPolicyWriteModelByID(ctx, orgID, c.eventstore.FilterToQueryReducer) if err != nil { return nil, err } @@ -104,24 +105,24 @@ func (c *Commands) removeLockoutPolicyIfExists(ctx context.Context, orgID string return org.NewLockoutPolicyRemovedEvent(ctx, orgAgg), nil } -func (c *Commands) orgLockoutPolicyWriteModelByID(ctx context.Context, orgID string) (*OrgLockoutPolicyWriteModel, error) { +func orgLockoutPolicyWriteModelByID(ctx context.Context, orgID string, queryReducer func(ctx context.Context, r eventstore.QueryReducer) error) (*OrgLockoutPolicyWriteModel, error) { policy := NewOrgLockoutPolicyWriteModel(orgID) - err := c.eventstore.FilterToQueryReducer(ctx, policy) + err := queryReducer(ctx, policy) if err != nil { return nil, err } return policy, nil } -func (c *Commands) getLockoutPolicy(ctx context.Context, orgID string) (*domain.LockoutPolicy, error) { - orgWm, err := c.orgLockoutPolicyWriteModelByID(ctx, orgID) +func getLockoutPolicy(ctx context.Context, orgID string, queryReducer func(ctx context.Context, r eventstore.QueryReducer) error) (*domain.LockoutPolicy, error) { + orgWm, err := orgLockoutPolicyWriteModelByID(ctx, orgID, queryReducer) if err != nil { return nil, err } if orgWm.State == domain.PolicyStateActive { return writeModelToLockoutPolicy(&orgWm.LockoutPolicyWriteModel), nil } - instanceWm, err := c.defaultLockoutPolicyWriteModelByID(ctx) + instanceWm, err := defaultLockoutPolicyWriteModelByID(ctx, queryReducer) if err != nil { return nil, err } diff --git a/internal/command/session.go b/internal/command/session.go index d6fcb072a..ccc224cab 100644 --- a/internal/command/session.go +++ b/internal/command/session.go @@ -7,6 +7,7 @@ import ( "fmt" "time" + "github.com/zitadel/logging" "golang.org/x/text/language" "github.com/zitadel/zitadel/internal/activity" @@ -17,21 +18,18 @@ import ( "github.com/zitadel/zitadel/internal/id" "github.com/zitadel/zitadel/internal/repository/session" "github.com/zitadel/zitadel/internal/repository/user" - "github.com/zitadel/zitadel/internal/telemetry/tracing" "github.com/zitadel/zitadel/internal/zerrors" ) -type SessionCommand func(ctx context.Context, cmd *SessionCommands) error +type SessionCommand func(ctx context.Context, cmd *SessionCommands) ([]eventstore.Command, error) type SessionCommands struct { sessionCommands []SessionCommand - sessionWriteModel *SessionWriteModel - passwordWriteModel *HumanPasswordWriteModel - intentWriteModel *IDPIntentWriteModel - totpWriteModel *HumanTOTPWriteModel - eventstore *eventstore.Eventstore - eventCommands []eventstore.Command + sessionWriteModel *SessionWriteModel + intentWriteModel *IDPIntentWriteModel + eventstore *eventstore.Eventstore + eventCommands []eventstore.Command hasher *crypto.Hasher intentAlg crypto.EncryptionAlgorithm @@ -59,114 +57,92 @@ func (c *Commands) NewSessionCommands(cmds []SessionCommand, session *SessionWri // CheckUser defines a user check to be executed for a session update func CheckUser(id string, resourceOwner string, preferredLanguage *language.Tag) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) error { + return func(ctx context.Context, cmd *SessionCommands) ([]eventstore.Command, error) { if cmd.sessionWriteModel.UserID != "" && id != "" && cmd.sessionWriteModel.UserID != id { - return zerrors.ThrowInvalidArgument(nil, "", "user change not possible") + return nil, zerrors.ThrowInvalidArgument(nil, "", "user change not possible") } - return cmd.UserChecked(ctx, id, resourceOwner, cmd.now(), preferredLanguage) + return nil, cmd.UserChecked(ctx, id, resourceOwner, cmd.now(), preferredLanguage) } } // CheckPassword defines a password check to be executed for a session update func CheckPassword(password string) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) error { - if cmd.sessionWriteModel.UserID == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sfw3f", "Errors.User.UserIDMissing") - } - cmd.passwordWriteModel = NewHumanPasswordWriteModel(cmd.sessionWriteModel.UserID, "") - err := cmd.eventstore.FilterToQueryReducer(ctx, cmd.passwordWriteModel) + return func(ctx context.Context, cmd *SessionCommands) ([]eventstore.Command, error) { + commands, err := checkPassword(ctx, cmd.sessionWriteModel.UserID, password, cmd.eventstore, cmd.hasher, nil) if err != nil { - return err + return commands, err } - if cmd.passwordWriteModel.UserState == domain.UserStateUnspecified || cmd.passwordWriteModel.UserState == domain.UserStateDeleted { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-Df4b3", "Errors.User.NotFound") - } - - if cmd.passwordWriteModel.EncodedHash == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-WEf3t", "Errors.User.Password.NotSet") - } - ctx, spanPasswordComparison := tracing.NewNamedSpan(ctx, "passwap.Verify") - updated, err := cmd.hasher.Verify(cmd.passwordWriteModel.EncodedHash, password) - spanPasswordComparison.EndWithError(err) - if err != nil { - //TODO: maybe we want to reset the session in the future https://github.com/zitadel/zitadel/issues/5807 - return zerrors.ThrowInvalidArgument(err, "COMMAND-SAF3g", "Errors.User.Password.Invalid") - } - if updated != "" { - cmd.eventCommands = append(cmd.eventCommands, user.NewHumanPasswordHashUpdatedEvent(ctx, UserAggregateFromWriteModel(&cmd.passwordWriteModel.WriteModel), updated)) - } - + cmd.eventCommands = append(cmd.eventCommands, commands...) cmd.PasswordChecked(ctx, cmd.now()) - return nil + return nil, nil } } // CheckIntent defines a check for a succeeded intent to be executed for a session update func CheckIntent(intentID, token string) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) error { + return func(ctx context.Context, cmd *SessionCommands) ([]eventstore.Command, error) { if cmd.sessionWriteModel.UserID == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sfw3r", "Errors.User.UserIDMissing") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sfw3r", "Errors.User.UserIDMissing") } if err := crypto.CheckToken(cmd.intentAlg, token, intentID); err != nil { - return err + return nil, err } cmd.intentWriteModel = NewIDPIntentWriteModel(intentID, "") err := cmd.eventstore.FilterToQueryReducer(ctx, cmd.intentWriteModel) if err != nil { - return err + return nil, err } if cmd.intentWriteModel.State != domain.IDPIntentStateSucceeded { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-Df4bw", "Errors.Intent.NotSucceeded") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Df4bw", "Errors.Intent.NotSucceeded") } if cmd.intentWriteModel.UserID != "" { if cmd.intentWriteModel.UserID != cmd.sessionWriteModel.UserID { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-O8xk3w", "Errors.Intent.OtherUser") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-O8xk3w", "Errors.Intent.OtherUser") } } else { linkWriteModel := NewUserIDPLinkWriteModel(cmd.sessionWriteModel.UserID, cmd.intentWriteModel.IDPID, cmd.intentWriteModel.IDPUserID, cmd.sessionWriteModel.UserResourceOwner) err := cmd.eventstore.FilterToQueryReducer(ctx, linkWriteModel) if err != nil { - return err + return nil, err } if linkWriteModel.State != domain.UserIDPLinkStateActive { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-O8xk3w", "Errors.Intent.OtherUser") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-O8xk3w", "Errors.Intent.OtherUser") } } cmd.IntentChecked(ctx, cmd.now()) - return nil + return nil, nil } } func CheckTOTP(code string) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) (err error) { - if cmd.sessionWriteModel.UserID == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-Neil7", "Errors.User.UserIDMissing") - } - cmd.totpWriteModel = NewHumanTOTPWriteModel(cmd.sessionWriteModel.UserID, "") - err = cmd.eventstore.FilterToQueryReducer(ctx, cmd.totpWriteModel) + return func(ctx context.Context, cmd *SessionCommands) (_ []eventstore.Command, err error) { + commands, err := checkTOTP( + ctx, + cmd.sessionWriteModel.UserID, + "", + code, + cmd.eventstore.FilterToQueryReducer, + cmd.totpAlg, + nil, + ) if err != nil { - return err - } - if cmd.totpWriteModel.State != domain.MFAStateReady { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-eej1U", "Errors.User.MFA.OTP.NotReady") - } - err = domain.VerifyTOTP(code, cmd.totpWriteModel.Secret, cmd.totpAlg) - if err != nil { - return err + return commands, err } + cmd.eventCommands = append(cmd.eventCommands, commands...) cmd.TOTPChecked(ctx, cmd.now()) - return nil + return nil, nil } } -// Exec will execute the commands specified and returns an error on the first occurrence -func (s *SessionCommands) Exec(ctx context.Context) error { +// Exec will execute the commands specified and returns an error on the first occurrence. +// In case of an error there might be specific commands returned, e.g. a failed pw check will have to be stored. +func (s *SessionCommands) Exec(ctx context.Context) ([]eventstore.Command, error) { for _, cmd := range s.sessionCommands { - if err := cmd(ctx, s); err != nil { - return err + if cmds, err := cmd(ctx, s); err != nil { + return cmds, err } } - return nil + return nil, nil } func (s *SessionCommands) Start(ctx context.Context, userAgent *domain.UserAgent) { @@ -360,8 +336,11 @@ func (c *Commands) updateSession(ctx context.Context, checks *SessionCommands, m if err = checks.sessionWriteModel.CheckNotInvalidated(); err != nil { return nil, err } - if err := checks.Exec(ctx); err != nil { - // TODO: how to handle failed checks (e.g. pw wrong) https://github.com/zitadel/zitadel/issues/5807 + if cmds, err := checks.Exec(ctx); err != nil { + if len(cmds) > 0 { + _, pushErr := c.eventstore.Push(ctx, cmds...) + logging.OnError(pushErr).Error("unable to store check failures") + } return nil, err } checks.ChangeMetadata(ctx, metadata) diff --git a/internal/command/session_otp.go b/internal/command/session_otp.go index 859f89340..6b7e20fb1 100644 --- a/internal/command/session_otp.go +++ b/internal/command/session_otp.go @@ -6,9 +6,10 @@ import ( "golang.org/x/text/language" - "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/repository/session" + "github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -21,26 +22,26 @@ func (c *Commands) CreateOTPSMSChallenge() SessionCommand { } func (c *Commands) createOTPSMSChallenge(returnCode bool, dst *string) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) error { + return func(ctx context.Context, cmd *SessionCommands) ([]eventstore.Command, error) { if cmd.sessionWriteModel.UserID == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-JKL3g", "Errors.User.UserIDMissing") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-JKL3g", "Errors.User.UserIDMissing") } writeModel := NewHumanOTPSMSWriteModel(cmd.sessionWriteModel.UserID, "") if err := cmd.eventstore.FilterToQueryReducer(ctx, writeModel); err != nil { - return err + return nil, err } if !writeModel.OTPAdded() { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-BJ2g3", "Errors.User.MFA.OTP.NotReady") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-BJ2g3", "Errors.User.MFA.OTP.NotReady") } code, err := cmd.createCode(ctx, cmd.eventstore.Filter, domain.SecretGeneratorTypeOTPSMS, cmd.otpAlg, c.defaultSecretGenerators.OTPSMS) if err != nil { - return err + return nil, err } if returnCode { *dst = code.Plain } cmd.OTPSMSChallenged(ctx, code.Crypted, code.Expiry, returnCode) - return nil + return nil, nil } } @@ -74,26 +75,26 @@ func (c *Commands) CreateOTPEmailChallenge() SessionCommand { } func (c *Commands) createOTPEmailChallenge(returnCode bool, urlTmpl string, dst *string) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) error { + return func(ctx context.Context, cmd *SessionCommands) ([]eventstore.Command, error) { if cmd.sessionWriteModel.UserID == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-JK3gp", "Errors.User.UserIDMissing") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-JK3gp", "Errors.User.UserIDMissing") } writeModel := NewHumanOTPEmailWriteModel(cmd.sessionWriteModel.UserID, "") if err := cmd.eventstore.FilterToQueryReducer(ctx, writeModel); err != nil { - return err + return nil, err } if !writeModel.OTPAdded() { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-JKLJ3", "Errors.User.MFA.OTP.NotReady") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-JKLJ3", "Errors.User.MFA.OTP.NotReady") } code, err := cmd.createCode(ctx, cmd.eventstore.Filter, domain.SecretGeneratorTypeOTPEmail, cmd.otpAlg, c.defaultSecretGenerators.OTPEmail) if err != nil { - return err + return nil, err } if returnCode { *dst = code.Plain } cmd.OTPEmailChallenged(ctx, code.Crypted, code.Expiry, returnCode, urlTmpl) - return nil + return nil, nil } } @@ -112,37 +113,57 @@ func (c *Commands) OTPEmailSent(ctx context.Context, sessionID, resourceOwner st } func CheckOTPSMS(code string) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) (err error) { - if cmd.sessionWriteModel.UserID == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-VDrh3", "Errors.User.UserIDMissing") + return func(ctx context.Context, cmd *SessionCommands) (_ []eventstore.Command, err error) { + writeModel := func(ctx context.Context, userID string, resourceOwner string) (OTPCodeWriteModel, error) { + otpWriteModel := NewHumanOTPSMSCodeWriteModel(cmd.sessionWriteModel.UserID, "") + err := cmd.eventstore.FilterToQueryReducer(ctx, otpWriteModel) + if err != nil { + return nil, err + } + // explicitly set the challenge from the session write model since the code write model will only check user events + otpWriteModel.otpCode = cmd.sessionWriteModel.OTPSMSCodeChallenge + return otpWriteModel, nil } - challenge := cmd.sessionWriteModel.OTPSMSCodeChallenge - if challenge == nil { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-SF3tv", "Errors.User.Code.NotFound") + succeededEvent := func(ctx context.Context, aggregate *eventstore.Aggregate, info *user.AuthRequestInfo) eventstore.Command { + return user.NewHumanOTPSMSCheckSucceededEvent(ctx, aggregate, nil) } - err = crypto.VerifyCode(challenge.CreationDate, challenge.Expiry, challenge.Code, code, cmd.otpAlg) + failedEvent := func(ctx context.Context, aggregate *eventstore.Aggregate, info *user.AuthRequestInfo) eventstore.Command { + return user.NewHumanOTPSMSCheckFailedEvent(ctx, aggregate, nil) + } + commands, err := checkOTP(ctx, cmd.sessionWriteModel.UserID, code, "", nil, writeModel, cmd.eventstore.FilterToQueryReducer, cmd.otpAlg, succeededEvent, failedEvent) if err != nil { - return err + return commands, err } + cmd.eventCommands = append(cmd.eventCommands, commands...) cmd.OTPSMSChecked(ctx, cmd.now()) - return nil + return nil, nil } } func CheckOTPEmail(code string) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) (err error) { - if cmd.sessionWriteModel.UserID == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-ejo2w", "Errors.User.UserIDMissing") + return func(ctx context.Context, cmd *SessionCommands) (_ []eventstore.Command, err error) { + writeModel := func(ctx context.Context, userID string, resourceOwner string) (OTPCodeWriteModel, error) { + otpWriteModel := NewHumanOTPEmailCodeWriteModel(cmd.sessionWriteModel.UserID, "") + err := cmd.eventstore.FilterToQueryReducer(ctx, otpWriteModel) + if err != nil { + return nil, err + } + // explicitly set the challenge from the session write model since the code write model will only check user events + otpWriteModel.otpCode = cmd.sessionWriteModel.OTPEmailCodeChallenge + return otpWriteModel, nil } - challenge := cmd.sessionWriteModel.OTPEmailCodeChallenge - if challenge == nil { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-zF3g3", "Errors.User.Code.NotFound") + succeededEvent := func(ctx context.Context, aggregate *eventstore.Aggregate, info *user.AuthRequestInfo) eventstore.Command { + return user.NewHumanOTPEmailCheckSucceededEvent(ctx, aggregate, nil) } - err = crypto.VerifyCode(challenge.CreationDate, challenge.Expiry, challenge.Code, code, cmd.otpAlg) + failedEvent := func(ctx context.Context, aggregate *eventstore.Aggregate, info *user.AuthRequestInfo) eventstore.Command { + return user.NewHumanOTPEmailCheckFailedEvent(ctx, aggregate, nil) + } + commands, err := checkOTP(ctx, cmd.sessionWriteModel.UserID, code, "", nil, writeModel, cmd.eventstore.FilterToQueryReducer, cmd.otpAlg, succeededEvent, failedEvent) if err != nil { - return err + return commands, err } + cmd.eventCommands = append(cmd.eventCommands, commands...) cmd.OTPEmailChecked(ctx, cmd.now()) - return nil + return nil, nil } } diff --git a/internal/command/session_otp_test.go b/internal/command/session_otp_test.go index c5be0aeca..d6934d047 100644 --- a/internal/command/session_otp_test.go +++ b/internal/command/session_otp_test.go @@ -11,6 +11,7 @@ import ( "github.com/zitadel/zitadel/internal/crypto" "github.com/zitadel/zitadel/internal/domain" "github.com/zitadel/zitadel/internal/eventstore" + "github.com/zitadel/zitadel/internal/repository/org" "github.com/zitadel/zitadel/internal/repository/session" "github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/zerrors" @@ -110,8 +111,9 @@ func TestCommands_CreateOTPSMSChallengeReturnCode(t *testing.T) { now: time.Now, } - err := cmd(context.Background(), cmds) + gotCmds, err := cmd(context.Background(), cmds) assert.ErrorIs(t, err, tt.res.err) + assert.Empty(t, gotCmds) assert.Equal(t, tt.res.returnCode, dst) assert.Equal(t, tt.res.commands, cmds.eventCommands) }) @@ -210,8 +212,9 @@ func TestCommands_CreateOTPSMSChallenge(t *testing.T) { now: time.Now, } - err := cmd(context.Background(), cmds) + gotCmds, err := cmd(context.Background(), cmds) assert.ErrorIs(t, err, tt.res.err) + assert.Empty(t, gotCmds) assert.Equal(t, tt.res.commands, cmds.eventCommands) }) } @@ -410,8 +413,9 @@ func TestCommands_CreateOTPEmailChallengeURLTemplate(t *testing.T) { now: time.Now, } - err = cmd(context.Background(), cmds) + gotCmds, err := cmd(context.Background(), cmds) assert.ErrorIs(t, err, tt.res.err) + assert.Empty(t, gotCmds) assert.Equal(t, tt.res.commands, cmds.eventCommands) }) } @@ -511,8 +515,9 @@ func TestCommands_CreateOTPEmailChallengeReturnCode(t *testing.T) { now: time.Now, } - err := cmd(context.Background(), cmds) + gotCmds, err := cmd(context.Background(), cmds) assert.ErrorIs(t, err, tt.res.err) + assert.Empty(t, gotCmds) assert.Equal(t, tt.res.returnCode, dst) assert.Equal(t, tt.res.commands, cmds.eventCommands) }) @@ -611,8 +616,9 @@ func TestCommands_CreateOTPEmailChallenge(t *testing.T) { now: time.Now, } - err := cmd(context.Background(), cmds) + gotCmds, err := cmd(context.Background(), cmds) assert.ErrorIs(t, err, tt.res.err) + assert.Empty(t, gotCmds) assert.Equal(t, tt.res.commands, cmds.eventCommands) }) } @@ -701,8 +707,9 @@ func TestCheckOTPSMS(t *testing.T) { code string } type res struct { - err error - commands []eventstore.Command + err error + commands []eventstore.Command + errorCommands []eventstore.Command } tests := []struct { name string @@ -720,13 +727,43 @@ func TestCheckOTPSMS(t *testing.T) { code: "code", }, res: res{ - err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-VDrh3", "Errors.User.UserIDMissing"), + err: zerrors.ThrowInvalidArgument(nil, "COMMAND-S453v", "Errors.User.UserIDMissing"), + }, + }, + { + name: "missing code", + fields: fields{ + eventstore: expectEventstore(), + userID: "userID", + }, + args: args{}, + res: res{ + err: zerrors.ThrowInvalidArgument(nil, "COMMAND-SJl2g", "Errors.User.Code.Empty"), + }, + }, + { + name: "not set up", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), + ), + userID: "userID", + }, + args: args{ + code: "code", + }, + res: res{ + err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-d2r52", "Errors.User.MFA.OTP.NotReady"), }, }, { name: "missing challenge", fields: fields{ - eventstore: expectEventstore(), + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + ), userID: "userID", otpCodeChallenge: nil, }, @@ -734,14 +771,26 @@ func TestCheckOTPSMS(t *testing.T) { code: "code", }, res: res{ - err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-SF3tv", "Errors.User.Code.NotFound"), + err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-S34gh", "Errors.User.Code.NotFound"), }, }, { name: "invalid code", fields: fields{ - eventstore: expectEventstore(), - userID: "userID", + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + expectFilter(), // recheck + expectFilter( + eventFromEventPusher( + org.NewLockoutPolicyAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + 0, 0, false, + ), + ), + ), + ), + userID: "userID", otpCodeChallenge: &OTPCode{ Code: &crypto.CryptoValue{ CryptoType: crypto.TypeEncryption, @@ -759,13 +808,61 @@ func TestCheckOTPSMS(t *testing.T) { }, res: res{ err: zerrors.ThrowPreconditionFailed(nil, "CODE-QvUQ4P", "Errors.User.Code.Expired"), + errorCommands: []eventstore.Command{ + user.NewHumanOTPSMSCheckFailedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil), + }, + }, + }, + { + name: "invalid code, locked", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + expectFilter(), // recheck + expectFilter( + eventFromEventPusher( + org.NewLockoutPolicyAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + 0, 1, false, + ), + ), + ), + ), + userID: "userID", + otpCodeChallenge: &OTPCode{ + Code: &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("code"), + }, + Expiry: 5 * time.Minute, + CreationDate: testNow.Add(-10 * time.Minute), + }, + otpAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args: args{ + code: "code", + }, + res: res{ + err: zerrors.ThrowPreconditionFailed(nil, "CODE-QvUQ4P", "Errors.User.Code.Expired"), + errorCommands: []eventstore.Command{ + user.NewHumanOTPSMSCheckFailedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil), + user.NewUserLockedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate), + }, }, }, { name: "check ok", fields: fields{ - eventstore: expectEventstore(), - userID: "userID", + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + expectFilter(), // recheck + ), + userID: "userID", otpCodeChallenge: &OTPCode{ Code: &crypto.CryptoValue{ CryptoType: crypto.TypeEncryption, @@ -783,12 +880,44 @@ func TestCheckOTPSMS(t *testing.T) { }, res: res{ commands: []eventstore.Command{ + user.NewHumanOTPSMSCheckSucceededEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil), session.NewOTPSMSCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instanceID").Aggregate, testNow, ), }, }, }, + { + name: "check ok, locked in the meantime", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + expectFilter( + user.NewUserLockedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate), + ), + ), + userID: "userID", + otpCodeChallenge: &OTPCode{ + Code: &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("code"), + }, + Expiry: 5 * time.Minute, + CreationDate: testNow, + }, + otpAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args: args{ + code: "code", + }, + res: res{ + err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-S6h4R", "Errors.User.Locked"), + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -811,8 +940,9 @@ func TestCheckOTPSMS(t *testing.T) { }, } - err := cmd(context.Background(), cmds) + gotCmds, err := cmd(context.Background(), cmds) assert.ErrorIs(t, err, tt.res.err) + assert.Equal(t, tt.res.errorCommands, gotCmds) assert.Equal(t, tt.res.commands, cmds.eventCommands) }) } @@ -829,8 +959,9 @@ func TestCheckOTPEmail(t *testing.T) { code string } type res struct { - err error - commands []eventstore.Command + err error + commands []eventstore.Command + errorCommands []eventstore.Command } tests := []struct { name string @@ -848,13 +979,43 @@ func TestCheckOTPEmail(t *testing.T) { code: "code", }, res: res{ - err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-ejo2w", "Errors.User.UserIDMissing"), + err: zerrors.ThrowInvalidArgument(nil, "COMMAND-S453v", "Errors.User.UserIDMissing"), + }, + }, + { + name: "missing code", + fields: fields{ + eventstore: expectEventstore(), + userID: "userID", + }, + args: args{}, + res: res{ + err: zerrors.ThrowInvalidArgument(nil, "COMMAND-SJl2g", "Errors.User.Code.Empty"), + }, + }, + { + name: "not set up", + fields: fields{ + eventstore: expectEventstore( + expectFilter(), + ), + userID: "userID", + }, + args: args{ + code: "code", + }, + res: res{ + err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-d2r52", "Errors.User.MFA.OTP.NotReady"), }, }, { name: "missing challenge", fields: fields{ - eventstore: expectEventstore(), + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + ), userID: "userID", otpCodeChallenge: nil, }, @@ -862,14 +1023,26 @@ func TestCheckOTPEmail(t *testing.T) { code: "code", }, res: res{ - err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-zF3g3", "Errors.User.Code.NotFound"), + err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-S34gh", "Errors.User.Code.NotFound"), }, }, { name: "invalid code", fields: fields{ - eventstore: expectEventstore(), - userID: "userID", + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + expectFilter(), // recheck + expectFilter( + eventFromEventPusher( + org.NewLockoutPolicyAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + 0, 0, false, + ), + ), + ), + ), + userID: "userID", otpCodeChallenge: &OTPCode{ Code: &crypto.CryptoValue{ CryptoType: crypto.TypeEncryption, @@ -887,13 +1060,61 @@ func TestCheckOTPEmail(t *testing.T) { }, res: res{ err: zerrors.ThrowPreconditionFailed(nil, "CODE-QvUQ4P", "Errors.User.Code.Expired"), + errorCommands: []eventstore.Command{ + user.NewHumanOTPEmailCheckFailedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil), + }, + }, + }, + { + name: "invalid code, locked", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + expectFilter(), // recheck + expectFilter( + eventFromEventPusher( + org.NewLockoutPolicyAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, + 0, 1, false, + ), + ), + ), + ), + userID: "userID", + otpCodeChallenge: &OTPCode{ + Code: &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("code"), + }, + Expiry: 5 * time.Minute, + CreationDate: testNow.Add(-10 * time.Minute), + }, + otpAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args: args{ + code: "code", + }, + res: res{ + err: zerrors.ThrowPreconditionFailed(nil, "CODE-QvUQ4P", "Errors.User.Code.Expired"), + errorCommands: []eventstore.Command{ + user.NewHumanOTPEmailCheckFailedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil), + user.NewUserLockedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate), + }, }, }, { name: "check ok", fields: fields{ - eventstore: expectEventstore(), - userID: "userID", + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + expectFilter(), // recheck + ), + userID: "userID", otpCodeChallenge: &OTPCode{ Code: &crypto.CryptoValue{ CryptoType: crypto.TypeEncryption, @@ -911,12 +1132,44 @@ func TestCheckOTPEmail(t *testing.T) { }, res: res{ commands: []eventstore.Command{ + user.NewHumanOTPEmailCheckSucceededEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil), session.NewOTPEmailCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instanceID").Aggregate, testNow, ), }, }, }, + { + name: "check ok, locked in the meantime", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher(user.NewHumanOTPEmailAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)), + ), + expectFilter( + user.NewUserLockedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate), + ), + ), + userID: "userID", + otpCodeChallenge: &OTPCode{ + Code: &crypto.CryptoValue{ + CryptoType: crypto.TypeEncryption, + Algorithm: "enc", + KeyID: "id", + Crypted: []byte("code"), + }, + Expiry: 5 * time.Minute, + CreationDate: testNow, + }, + otpAlg: crypto.CreateMockEncryptionAlg(gomock.NewController(t)), + }, + args: args{ + code: "code", + }, + res: res{ + err: zerrors.ThrowPreconditionFailed(nil, "COMMAND-S6h4R", "Errors.User.Locked"), + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -939,8 +1192,9 @@ func TestCheckOTPEmail(t *testing.T) { }, } - err := cmd(context.Background(), cmds) + gotCmds, err := cmd(context.Background(), cmds) assert.ErrorIs(t, err, tt.res.err) + assert.Equal(t, tt.res.errorCommands, gotCmds) assert.Equal(t, tt.res.commands, cmds.eventCommands) }) } diff --git a/internal/command/session_test.go b/internal/command/session_test.go index feaf395c2..46cbaeafc 100644 --- a/internal/command/session_test.go +++ b/internal/command/session_test.go @@ -22,6 +22,7 @@ import ( "github.com/zitadel/zitadel/internal/id" "github.com/zitadel/zitadel/internal/id/mock" "github.com/zitadel/zitadel/internal/repository/idpintent" + "github.com/zitadel/zitadel/internal/repository/org" "github.com/zitadel/zitadel/internal/repository/session" "github.com/zitadel/zitadel/internal/repository/user" "github.com/zitadel/zitadel/internal/zerrors" @@ -430,8 +431,8 @@ func TestCommands_updateSession(t *testing.T) { checks: &SessionCommands{ sessionWriteModel: NewSessionWriteModel("sessionID", "instance1"), sessionCommands: []SessionCommand{ - func(ctx context.Context, cmd *SessionCommands) error { - return zerrors.ThrowInternal(nil, "id", "check failed") + func(ctx context.Context, cmd *SessionCommands) ([]eventstore.Command, error) { + return nil, zerrors.ThrowInternal(nil, "id", "check failed") }, }, }, @@ -525,6 +526,55 @@ func TestCommands_updateSession(t *testing.T) { }, }, }, + { + "set user, invalid password", + fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, + "username", "", "", "", "", language.English, domain.GenderUnspecified, "", false), + ), + eventFromEventPusher( + user.NewHumanPasswordChangedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, + "$plain$x$password", false, ""), + ), + ), + expectFilter(), // recheck + expectFilter( + org.NewLockoutPolicyAddedEvent(context.Background(), &org.NewAggregate("org1").Aggregate, 0, 0, false), + ), + expectPush( + user.NewHumanPasswordCheckFailedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil), + ), + ), + }, + args{ + ctx: authz.NewMockContext("instance1", "", ""), + checks: &SessionCommands{ + sessionWriteModel: NewSessionWriteModel("sessionID", "instance1"), + sessionCommands: []SessionCommand{ + CheckUser("userID", "org1", &language.Afrikaans), + CheckPassword("invalid password"), + }, + createToken: func(sessionID string) (string, string, error) { + return "tokenID", + "token", + nil + }, + hasher: mockPasswordHasher("x"), + now: func() time.Time { + return testNow + }, + }, + metadata: map[string][]byte{ + "key": []byte("value"), + }, + }, + res{ + err: zerrors.ThrowInvalidArgument(nil, "COMMAND-3M0fs", "Errors.User.Password.Invalid"), + }, + }, { "set user, password, metadata and token", fields{ @@ -539,10 +589,12 @@ func TestCommands_updateSession(t *testing.T) { "$plain$x$password", false, ""), ), ), + expectFilter(), // recheck expectPush( session.NewUserCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instance1").Aggregate, "userID", "org1", testNow, &language.Afrikaans, ), + user.NewHumanPasswordCheckSucceededEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil), session.NewPasswordCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instance1").Aggregate, testNow, ), @@ -872,6 +924,7 @@ func TestCheckTOTP(t *testing.T) { sessAgg := &session.NewAggregate("session1", "instance1").Aggregate userAgg := &user.NewAggregate("user1", "org1").Aggregate + orgAgg := &org.NewAggregate("org1").Aggregate code, err := totp.GenerateCode(key.Secret(), testNow) require.NoError(t, err) @@ -886,6 +939,7 @@ func TestCheckTOTP(t *testing.T) { code string fields fields wantEventCommands []eventstore.Command + wantErrorCommands []eventstore.Command wantErr error }{ { @@ -897,7 +951,7 @@ func TestCheckTOTP(t *testing.T) { }, eventstore: expectEventstore(), }, - wantErr: zerrors.ThrowPreconditionFailed(nil, "COMMAND-Neil7", "Errors.User.UserIDMissing"), + wantErr: zerrors.ThrowInvalidArgument(nil, "COMMAND-8N9ds", "Errors.User.UserIDMissing"), }, { name: "filter error", @@ -931,7 +985,7 @@ func TestCheckTOTP(t *testing.T) { ), ), }, - wantErr: zerrors.ThrowPreconditionFailed(nil, "COMMAND-eej1U", "Errors.User.MFA.OTP.NotReady"), + wantErr: zerrors.ThrowPreconditionFailed(nil, "COMMAND-3Mif9s", "Errors.User.MFA.OTP.NotReady"), }, { name: "otp verify error", @@ -951,8 +1005,45 @@ func TestCheckTOTP(t *testing.T) { user.NewHumanOTPVerifiedEvent(ctx, userAgg, "agent1"), ), ), + expectFilter(), // recheck + expectFilter( + eventFromEventPusher(org.NewLockoutPolicyAddedEvent(ctx, orgAgg, 0, 0, false)), + ), ), }, + wantErrorCommands: []eventstore.Command{ + user.NewHumanOTPCheckFailedEvent(ctx, userAgg, nil), + }, + wantErr: zerrors.ThrowInvalidArgument(nil, "EVENT-8isk2", "Errors.User.MFA.OTP.InvalidCode"), + }, + { + name: "otp verify error, locked", + code: "foobar", + fields: fields{ + sessionWriteModel: &SessionWriteModel{ + UserID: "user1", + UserCheckedAt: testNow, + aggregate: sessAgg, + }, + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + ), + eventFromEventPusher( + user.NewHumanOTPVerifiedEvent(ctx, userAgg, "agent1"), + ), + ), + expectFilter(), // recheck + expectFilter( + eventFromEventPusher(org.NewLockoutPolicyAddedEvent(ctx, orgAgg, 1, 1, false)), + ), + ), + }, + wantErrorCommands: []eventstore.Command{ + user.NewHumanOTPCheckFailedEvent(ctx, userAgg, nil), + user.NewUserLockedEvent(ctx, userAgg), + }, wantErr: zerrors.ThrowInvalidArgument(nil, "EVENT-8isk2", "Errors.User.MFA.OTP.InvalidCode"), }, { @@ -973,12 +1064,39 @@ func TestCheckTOTP(t *testing.T) { user.NewHumanOTPVerifiedEvent(ctx, userAgg, "agent1"), ), ), + expectFilter(), // recheck ), }, wantEventCommands: []eventstore.Command{ + user.NewHumanOTPCheckSucceededEvent(ctx, userAgg, nil), session.NewTOTPCheckedEvent(ctx, sessAgg, testNow), }, }, + { + name: "ok, but locked in the meantime", + code: code, + fields: fields{ + sessionWriteModel: &SessionWriteModel{ + UserID: "user1", + UserCheckedAt: testNow, + aggregate: sessAgg, + }, + eventstore: expectEventstore( + expectFilter( + eventFromEventPusher( + user.NewHumanOTPAddedEvent(ctx, userAgg, secret), + ), + eventFromEventPusher( + user.NewHumanOTPVerifiedEvent(ctx, userAgg, "agent1"), + ), + ), + expectFilter( + user.NewUserLockedEvent(ctx, userAgg), + ), + ), + }, + wantErr: zerrors.ThrowPreconditionFailed(nil, "COMMAND-SF3fg", "Errors.User.Locked"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -988,8 +1106,9 @@ func TestCheckTOTP(t *testing.T) { totpAlg: cryptoAlg, now: func() time.Time { return testNow }, } - err := CheckTOTP(tt.code)(ctx, cmd) + gotCmds, err := CheckTOTP(tt.code)(ctx, cmd) require.ErrorIs(t, err, tt.wantErr) + assert.Equal(t, tt.wantErrorCommands, gotCmds) assert.Equal(t, tt.wantEventCommands, cmd.eventCommands) }) } diff --git a/internal/command/session_webauhtn.go b/internal/command/session_webauhtn.go index 680b641a5..0bbb68c99 100644 --- a/internal/command/session_webauhtn.go +++ b/internal/command/session_webauhtn.go @@ -5,6 +5,7 @@ import ( "encoding/json" "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/eventstore" "github.com/zitadel/zitadel/internal/zerrors" ) @@ -41,49 +42,49 @@ func (s *SessionCommands) getHumanWebAuthNTokenReadModel(ctx context.Context, us } func (c *Commands) CreateWebAuthNChallenge(userVerification domain.UserVerificationRequirement, rpid string, dst json.Unmarshaler) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) error { + return func(ctx context.Context, cmd *SessionCommands) ([]eventstore.Command, error) { humanPasskeys, err := cmd.getHumanWebAuthNTokens(ctx, userVerification) if err != nil { - return err + return nil, err } webAuthNLogin, err := c.webauthnConfig.BeginLogin(ctx, humanPasskeys.human, userVerification, rpid, humanPasskeys.tokens...) if err != nil { - return err + return nil, err } if err = json.Unmarshal(webAuthNLogin.CredentialAssertionData, dst); err != nil { - return zerrors.ThrowInternal(err, "COMMAND-Yah6A", "Errors.Internal") + return nil, zerrors.ThrowInternal(err, "COMMAND-Yah6A", "Errors.Internal") } cmd.WebAuthNChallenged(ctx, webAuthNLogin.Challenge, webAuthNLogin.AllowedCredentialIDs, webAuthNLogin.UserVerification, rpid) - return nil + return nil, nil } } func (c *Commands) CheckWebAuthN(credentialAssertionData json.Marshaler) SessionCommand { - return func(ctx context.Context, cmd *SessionCommands) error { + return func(ctx context.Context, cmd *SessionCommands) ([]eventstore.Command, error) { credentialAssertionData, err := json.Marshal(credentialAssertionData) if err != nil { - return zerrors.ThrowInternal(err, "COMMAND-ohG2o", "Errors.Internal") + return nil, zerrors.ThrowInternal(err, "COMMAND-ohG2o", "Errors.Internal") } challenge := cmd.sessionWriteModel.WebAuthNChallenge if challenge == nil { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-Ioqu5", "Errors.Session.WebAuthN.NoChallenge") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Ioqu5", "Errors.Session.WebAuthN.NoChallenge") } webAuthNTokens, err := cmd.getHumanWebAuthNTokens(ctx, challenge.UserVerification) if err != nil { - return err + return nil, err } webAuthN := challenge.WebAuthNLogin(webAuthNTokens.human, credentialAssertionData) credential, err := c.webauthnConfig.FinishLogin(ctx, webAuthNTokens.human, webAuthN, credentialAssertionData, webAuthNTokens.tokens...) if err != nil && (credential == nil || credential.ID == nil) { - return err + return nil, err } _, token := domain.GetTokenByKeyID(webAuthNTokens.tokens, credential.ID) if token == nil { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-Aej7i", "Errors.User.WebAuthN.NotFound") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Aej7i", "Errors.User.WebAuthN.NotFound") } cmd.WebAuthNChecked(ctx, cmd.now(), token.WebAuthNTokenID, credential.Authenticator.SignCount, credential.Flags.UserVerified) - return nil + return nil, nil } } diff --git a/internal/command/user_human_otp.go b/internal/command/user_human_otp.go index c368167b3..39abab3b8 100644 --- a/internal/command/user_human_otp.go +++ b/internal/command/user_human_otp.go @@ -161,48 +161,67 @@ func (c *Commands) HumanCheckMFATOTPSetup(ctx context.Context, userID, code, use } func (c *Commands) HumanCheckMFATOTP(ctx context.Context, userID, code, resourceOwner string, authRequest *domain.AuthRequest) error { + commands, err := checkTOTP( + ctx, + userID, + resourceOwner, + code, + c.eventstore.FilterToQueryReducer, + c.multifactors.OTP.CryptoMFA, + authRequestDomainToAuthRequestInfo(authRequest), + ) + + _, pushErr := c.eventstore.Push(ctx, commands...) + logging.OnError(pushErr).Error("error create password check failed event") + return err +} + +func checkTOTP( + ctx context.Context, + userID, resourceOwner, code string, + queryReducer func(ctx context.Context, r eventstore.QueryReducer) error, + alg crypto.EncryptionAlgorithm, + optionalAuthRequestInfo *user.AuthRequestInfo, +) ([]eventstore.Command, error) { if userID == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-8N9ds", "Errors.User.UserIDMissing") + return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-8N9ds", "Errors.User.UserIDMissing") } - existingOTP, err := c.totpWriteModelByID(ctx, userID, resourceOwner) + existingOTP := NewHumanTOTPWriteModel(userID, resourceOwner) + err := queryReducer(ctx, existingOTP) if err != nil { - return err + return nil, err } if existingOTP.State != domain.MFAStateReady { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-3Mif9s", "Errors.User.MFA.OTP.NotReady") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-3Mif9s", "Errors.User.MFA.OTP.NotReady") } userAgg := UserAggregateFromWriteModel(&existingOTP.WriteModel) - verifyErr := domain.VerifyTOTP(code, existingOTP.Secret, c.multifactors.OTP.CryptoMFA) + verifyErr := domain.VerifyTOTP(code, existingOTP.Secret, alg) // recheck for additional events (failed OTP checks or locks) - recheckErr := c.eventstore.FilterToQueryReducer(ctx, existingOTP) + recheckErr := queryReducer(ctx, existingOTP) if recheckErr != nil { - return recheckErr + return nil, recheckErr } if existingOTP.UserLocked { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-SF3fg", "Errors.User.Locked") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-SF3fg", "Errors.User.Locked") } // the OTP check succeeded and the user was not locked in the meantime if verifyErr == nil { - _, err = c.eventstore.Push(ctx, user.NewHumanOTPCheckSucceededEvent(ctx, userAgg, authRequestDomainToAuthRequestInfo(authRequest))) - return err + return []eventstore.Command{user.NewHumanOTPCheckSucceededEvent(ctx, userAgg, optionalAuthRequestInfo)}, nil } // the OTP check failed, therefore check if the limit was reached and the user must additionally be locked commands := make([]eventstore.Command, 0, 2) - commands = append(commands, user.NewHumanOTPCheckFailedEvent(ctx, userAgg, authRequestDomainToAuthRequestInfo(authRequest))) - lockoutPolicy, err := c.getLockoutPolicy(ctx, resourceOwner) + commands = append(commands, user.NewHumanOTPCheckFailedEvent(ctx, userAgg, optionalAuthRequestInfo)) + lockoutPolicy, err := getLockoutPolicy(ctx, existingOTP.ResourceOwner, queryReducer) if err != nil { - return err + return nil, err } if lockoutPolicy.MaxOTPAttempts > 0 && existingOTP.CheckFailedCount+1 >= lockoutPolicy.MaxOTPAttempts { commands = append(commands, user.NewUserLockedEvent(ctx, userAgg)) } - - _, pushErr := c.eventstore.Push(ctx, commands...) - logging.OnError(pushErr).Error("error create password check failed event") - return verifyErr + return commands, verifyErr } func (c *Commands) HumanRemoveTOTP(ctx context.Context, userID, resourceOwner string) (*domain.ObjectDetails, error) { @@ -342,16 +361,23 @@ func (c *Commands) HumanCheckOTPSMS(ctx context.Context, userID, code, resourceO failedEvent := func(ctx context.Context, aggregate *eventstore.Aggregate, info *user.AuthRequestInfo) eventstore.Command { return user.NewHumanOTPSMSCheckFailedEvent(ctx, aggregate, authRequestDomainToAuthRequestInfo(authRequest)) } - return c.humanCheckOTP( + commands, err := checkOTP( ctx, userID, code, resourceOwner, authRequest, writeModel, + c.eventstore.FilterToQueryReducer, + c.userEncryption, succeededEvent, failedEvent, ) + if len(commands) > 0 { + _, pushErr := c.eventstore.Push(ctx, commands...) + logging.WithFields("userID", userID).OnError(pushErr).Error("otp failure check push failed") + } + return err } // AddHumanOTPEmail adds the OTP Email factor to a user. @@ -467,16 +493,23 @@ func (c *Commands) HumanCheckOTPEmail(ctx context.Context, userID, code, resourc failedEvent := func(ctx context.Context, aggregate *eventstore.Aggregate, info *user.AuthRequestInfo) eventstore.Command { return user.NewHumanOTPEmailCheckFailedEvent(ctx, aggregate, authRequestDomainToAuthRequestInfo(authRequest)) } - return c.humanCheckOTP( + commands, err := checkOTP( ctx, userID, code, resourceOwner, authRequest, writeModel, + c.eventstore.FilterToQueryReducer, + c.userEncryption, succeededEvent, failedEvent, ) + if len(commands) > 0 { + _, pushErr := c.eventstore.Push(ctx, commands...) + logging.WithFields("userID", userID).OnError(pushErr).Error("otp failure check push failed") + } + return err } // sendHumanOTP creates a code for a registered mechanism (sms / email), which is used for a check (during login) @@ -534,62 +567,57 @@ func (c *Commands) humanOTPSent( return err } -func (c *Commands) humanCheckOTP( +func checkOTP( ctx context.Context, userID, code, resourceOwner string, authRequest *domain.AuthRequest, writeModelByID func(ctx context.Context, userID string, resourceOwner string) (OTPCodeWriteModel, error), - checkSucceededEvent func(ctx context.Context, aggregate *eventstore.Aggregate, info *user.AuthRequestInfo) eventstore.Command, - checkFailedEvent func(ctx context.Context, aggregate *eventstore.Aggregate, info *user.AuthRequestInfo) eventstore.Command, -) error { + queryReducer func(ctx context.Context, r eventstore.QueryReducer) error, + alg crypto.EncryptionAlgorithm, + checkSucceededEvent, checkFailedEvent func(ctx context.Context, aggregate *eventstore.Aggregate, info *user.AuthRequestInfo) eventstore.Command, +) ([]eventstore.Command, error) { if userID == "" { - return zerrors.ThrowInvalidArgument(nil, "COMMAND-S453v", "Errors.User.UserIDMissing") + return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-S453v", "Errors.User.UserIDMissing") } if code == "" { - return zerrors.ThrowInvalidArgument(nil, "COMMAND-SJl2g", "Errors.User.Code.Empty") + return nil, zerrors.ThrowInvalidArgument(nil, "COMMAND-SJl2g", "Errors.User.Code.Empty") } existingOTP, err := writeModelByID(ctx, userID, resourceOwner) if err != nil { - return err + return nil, err } if !existingOTP.OTPAdded() { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-d2r52", "Errors.User.MFA.OTP.NotReady") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-d2r52", "Errors.User.MFA.OTP.NotReady") } if existingOTP.Code() == nil { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-S34gh", "Errors.User.Code.NotFound") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-S34gh", "Errors.User.Code.NotFound") } userAgg := &user.NewAggregate(userID, existingOTP.ResourceOwner()).Aggregate - verifyErr := crypto.VerifyCode(existingOTP.CodeCreationDate(), existingOTP.CodeExpiry(), existingOTP.Code(), code, c.userEncryption) + verifyErr := crypto.VerifyCode(existingOTP.CodeCreationDate(), existingOTP.CodeExpiry(), existingOTP.Code(), code, alg) // recheck for additional events (failed OTP checks or locks) - recheckErr := c.eventstore.FilterToQueryReducer(ctx, existingOTP) + recheckErr := queryReducer(ctx, existingOTP) if recheckErr != nil { - return recheckErr + return nil, recheckErr } if existingOTP.UserLocked() { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-S6h4R", "Errors.User.Locked") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-S6h4R", "Errors.User.Locked") } // the OTP check succeeded and the user was not locked in the meantime if verifyErr == nil { - _, err = c.eventstore.Push(ctx, checkSucceededEvent(ctx, userAgg, authRequestDomainToAuthRequestInfo(authRequest))) - return err + return []eventstore.Command{checkSucceededEvent(ctx, userAgg, authRequestDomainToAuthRequestInfo(authRequest))}, nil } // the OTP check failed, therefore check if the limit was reached and the user must additionally be locked commands := make([]eventstore.Command, 0, 2) commands = append(commands, checkFailedEvent(ctx, userAgg, authRequestDomainToAuthRequestInfo(authRequest))) - lockoutPolicy, err := c.getLockoutPolicy(ctx, resourceOwner) - if err != nil { - return err - } - if lockoutPolicy.MaxOTPAttempts > 0 && existingOTP.CheckFailedCount()+1 >= lockoutPolicy.MaxOTPAttempts { + lockoutPolicy, lockoutErr := getLockoutPolicy(ctx, existingOTP.ResourceOwner(), queryReducer) + logging.OnError(lockoutErr).Error("unable to get lockout policy") + if lockoutPolicy != nil && lockoutPolicy.MaxOTPAttempts > 0 && existingOTP.CheckFailedCount()+1 >= lockoutPolicy.MaxOTPAttempts { commands = append(commands, user.NewUserLockedEvent(ctx, userAgg)) } - - _, pushErr := c.eventstore.Push(ctx, commands...) - logging.WithFields("userID", userID).OnError(pushErr).Error("otp failure check push failed") - return verifyErr + return commands, verifyErr } func (c *Commands) totpWriteModelByID(ctx context.Context, userID, resourceOwner string) (writeModel *HumanTOTPWriteModel, err error) { diff --git a/internal/command/user_human_otp_model.go b/internal/command/user_human_otp_model.go index 43abc1434..2780a52d7 100644 --- a/internal/command/user_human_otp_model.go +++ b/internal/command/user_human_otp_model.go @@ -157,24 +157,31 @@ func (wm *HumanOTPSMSWriteModel) Query() *eventstore.SearchQueryBuilder { type HumanOTPSMSCodeWriteModel struct { *HumanOTPSMSWriteModel - code *crypto.CryptoValue - codeCreationDate time.Time - codeExpiry time.Duration + otpCode *OTPCode checkFailedCount uint64 userLocked bool } func (wm *HumanOTPSMSCodeWriteModel) CodeCreationDate() time.Time { - return wm.codeCreationDate + if wm.otpCode == nil { + return time.Time{} + } + return wm.otpCode.CreationDate } func (wm *HumanOTPSMSCodeWriteModel) CodeExpiry() time.Duration { - return wm.codeExpiry + if wm.otpCode == nil { + return 0 + } + return wm.otpCode.Expiry } func (wm *HumanOTPSMSCodeWriteModel) Code() *crypto.CryptoValue { - return wm.code + if wm.otpCode == nil { + return nil + } + return wm.otpCode.Code } func (wm *HumanOTPSMSCodeWriteModel) CheckFailedCount() uint64 { @@ -195,9 +202,11 @@ func (wm *HumanOTPSMSCodeWriteModel) Reduce() error { for _, event := range wm.Events { switch e := event.(type) { case *user.HumanOTPSMSCodeAddedEvent: - wm.code = e.Code - wm.codeCreationDate = e.CreationDate() - wm.codeExpiry = e.Expiry + wm.otpCode = &OTPCode{ + Code: e.Code, + CreationDate: e.CreationDate(), + Expiry: e.Expiry, + } case *user.HumanOTPSMSCheckSucceededEvent: wm.checkFailedCount = 0 case *user.HumanOTPSMSCheckFailedEvent: @@ -300,24 +309,31 @@ func (wm *HumanOTPEmailWriteModel) Query() *eventstore.SearchQueryBuilder { type HumanOTPEmailCodeWriteModel struct { *HumanOTPEmailWriteModel - code *crypto.CryptoValue - codeCreationDate time.Time - codeExpiry time.Duration + otpCode *OTPCode checkFailedCount uint64 userLocked bool } func (wm *HumanOTPEmailCodeWriteModel) CodeCreationDate() time.Time { - return wm.codeCreationDate + if wm.otpCode == nil { + return time.Time{} + } + return wm.otpCode.CreationDate } func (wm *HumanOTPEmailCodeWriteModel) CodeExpiry() time.Duration { - return wm.codeExpiry + if wm.otpCode == nil { + return 0 + } + return wm.otpCode.Expiry } func (wm *HumanOTPEmailCodeWriteModel) Code() *crypto.CryptoValue { - return wm.code + if wm.otpCode == nil { + return nil + } + return wm.otpCode.Code } func (wm *HumanOTPEmailCodeWriteModel) CheckFailedCount() uint64 { @@ -338,9 +354,11 @@ func (wm *HumanOTPEmailCodeWriteModel) Reduce() error { for _, event := range wm.Events { switch e := event.(type) { case *user.HumanOTPEmailCodeAddedEvent: - wm.code = e.Code - wm.codeCreationDate = e.CreationDate() - wm.codeExpiry = e.Expiry + wm.otpCode = &OTPCode{ + Code: e.Code, + CreationDate: e.CreationDate(), + Expiry: e.Expiry, + } case *user.HumanOTPEmailCheckSucceededEvent: wm.checkFailedCount = 0 case *user.HumanOTPEmailCheckFailedEvent: diff --git a/internal/command/user_human_password.go b/internal/command/user_human_password.go index fd8b29a42..839977b38 100644 --- a/internal/command/user_human_password.go +++ b/internal/command/user_human_password.go @@ -295,8 +295,8 @@ func (c *Commands) PasswordChangeSent(ctx context.Context, orgID, userID string) return err } -// HumanCheckPassword check password for user with additional informations from authRequest -func (c *Commands) HumanCheckPassword(ctx context.Context, orgID, userID, password string, authRequest *domain.AuthRequest, lockoutPolicy *domain.LockoutPolicy) (err error) { +// HumanCheckPassword check password for user with additional information from authRequest +func (c *Commands) HumanCheckPassword(ctx context.Context, orgID, userID, password string, authRequest *domain.AuthRequest) (err error) { ctx, span := tracing.NewSpan(ctx) defer func() { span.EndWithError(err) }() @@ -314,56 +314,66 @@ func (c *Commands) HumanCheckPassword(ctx context.Context, orgID, userID, passwo if !loginPolicy.AllowUsernamePassword { return zerrors.ThrowPreconditionFailed(err, "COMMAND-Dft32", "Errors.Org.LoginPolicy.UsernamePasswordNotAllowed") } - - wm, err := c.passwordWriteModel(ctx, userID, orgID) - if err != nil { + commands, err := checkPassword(ctx, userID, password, c.eventstore, c.userPasswordHasher, authRequestDomainToAuthRequestInfo(authRequest)) + if len(commands) == 0 { return err } + _, pushErr := c.eventstore.Push(ctx, commands...) + logging.OnError(pushErr).Error("error create password check failed event") + return err +} - if !isUserStateExists(wm.UserState) { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-3n77z", "Errors.User.NotFound") +func checkPassword(ctx context.Context, userID, password string, es *eventstore.Eventstore, hasher *crypto.Hasher, optionalAuthRequestInfo *user.AuthRequestInfo) ([]eventstore.Command, error) { + if userID == "" { + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-Sfw3f", "Errors.User.UserIDMissing") + } + wm := NewHumanPasswordWriteModel(userID, "") + err := es.FilterToQueryReducer(ctx, wm) + if err != nil { + return nil, err + } + if !wm.UserState.Exists() { + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-3n77z", "Errors.User.NotFound") } if wm.UserState == domain.UserStateLocked { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-JLK35", "Errors.User.Locked") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-JLK35", "Errors.User.Locked") } if wm.EncodedHash == "" { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-3nJ4t", "Errors.User.Password.NotSet") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-3nJ4t", "Errors.User.Password.NotSet") } userAgg := UserAggregateFromWriteModel(&wm.WriteModel) ctx, spanPasswordComparison := tracing.NewNamedSpan(ctx, "passwap.Verify") - updated, err := c.userPasswordHasher.Verify(wm.EncodedHash, password) + updated, err := hasher.Verify(wm.EncodedHash, password) spanPasswordComparison.EndWithError(err) err = convertPasswapErr(err) commands := make([]eventstore.Command, 0, 2) // recheck for additional events (failed password checks or locks) - recheckErr := c.eventstore.FilterToQueryReducer(ctx, wm) + recheckErr := es.FilterToQueryReducer(ctx, wm) if recheckErr != nil { - return recheckErr + return nil, recheckErr } if wm.UserState == domain.UserStateLocked { - return zerrors.ThrowPreconditionFailed(nil, "COMMAND-SFA3t", "Errors.User.Locked") + return nil, zerrors.ThrowPreconditionFailed(nil, "COMMAND-SFA3t", "Errors.User.Locked") } if err == nil { - commands = append(commands, user.NewHumanPasswordCheckSucceededEvent(ctx, userAgg, authRequestDomainToAuthRequestInfo(authRequest))) + commands = append(commands, user.NewHumanPasswordCheckSucceededEvent(ctx, userAgg, optionalAuthRequestInfo)) if updated != "" { commands = append(commands, user.NewHumanPasswordHashUpdatedEvent(ctx, userAgg, updated)) } - _, err = c.eventstore.Push(ctx, commands...) - return err + return commands, nil } - commands = append(commands, user.NewHumanPasswordCheckFailedEvent(ctx, userAgg, authRequestDomainToAuthRequestInfo(authRequest))) - if lockoutPolicy != nil && lockoutPolicy.MaxPasswordAttempts > 0 { - if wm.PasswordCheckFailedCount+1 >= lockoutPolicy.MaxPasswordAttempts { - commands = append(commands, user.NewUserLockedEvent(ctx, userAgg)) - } + commands = append(commands, user.NewHumanPasswordCheckFailedEvent(ctx, userAgg, optionalAuthRequestInfo)) + + lockoutPolicy, lockoutErr := getLockoutPolicy(ctx, wm.ResourceOwner, es.FilterToQueryReducer) + logging.OnError(lockoutErr).Error("unable to get lockout policy") + if lockoutPolicy != nil && lockoutPolicy.MaxPasswordAttempts > 0 && wm.PasswordCheckFailedCount+1 >= lockoutPolicy.MaxPasswordAttempts { + commands = append(commands, user.NewUserLockedEvent(ctx, userAgg)) } - _, pushErr := c.eventstore.Push(ctx, commands...) - logging.OnError(pushErr).Error("error create password check failed event") - return err + return commands, err } func (c *Commands) passwordWriteModel(ctx context.Context, userID, resourceOwner string) (writeModel *HumanPasswordWriteModel, err error) { diff --git a/internal/command/user_human_password_test.go b/internal/command/user_human_password_test.go index 4e7672d8b..d944f5e1c 100644 --- a/internal/command/user_human_password_test.go +++ b/internal/command/user_human_password_test.go @@ -1456,7 +1456,6 @@ func TestCommandSide_CheckPassword(t *testing.T) { resourceOwner string password string authReq *domain.AuthRequest - lockoutPolicy *domain.LockoutPolicy } type res struct { err func(error) bool @@ -1768,6 +1767,13 @@ func TestCommandSide_CheckPassword(t *testing.T) { "")), ), expectFilter(), + expectFilter( + eventFromEventPusher( + org.NewLockoutPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + 0, 0, false, + )), + ), expectPush( user.NewHumanPasswordCheckFailedEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -1789,7 +1795,6 @@ func TestCommandSide_CheckPassword(t *testing.T) { ID: "request1", AgentID: "agent1", }, - lockoutPolicy: &domain.LockoutPolicy{}, }, res: res{ err: zerrors.IsErrorInvalidArgument, @@ -1852,6 +1857,13 @@ func TestCommandSide_CheckPassword(t *testing.T) { ), ), expectFilter(), + expectFilter( + eventFromEventPusher( + org.NewLockoutPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + 1, 1, false, + )), + ), expectPush( user.NewHumanPasswordCheckFailedEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -1876,10 +1888,6 @@ func TestCommandSide_CheckPassword(t *testing.T) { ID: "request1", AgentID: "agent1", }, - lockoutPolicy: &domain.LockoutPolicy{ - MaxPasswordAttempts: 1, - MaxOTPAttempts: 1, - }, }, res: res{ err: zerrors.IsErrorInvalidArgument, @@ -2230,7 +2238,7 @@ func TestCommandSide_CheckPassword(t *testing.T) { eventstore: tt.fields.eventstore(t), userPasswordHasher: tt.fields.userPasswordHasher, } - err := r.HumanCheckPassword(tt.args.ctx, tt.args.resourceOwner, tt.args.userID, tt.args.password, tt.args.authReq, tt.args.lockoutPolicy) + err := r.HumanCheckPassword(tt.args.ctx, tt.args.resourceOwner, tt.args.userID, tt.args.password, tt.args.authReq) if tt.res.err == nil { assert.NoError(t, err) }