From aabefb938227925aadce8e496ffff3f2cd05d838 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Fri, 31 May 2024 00:08:48 +0200 Subject: [PATCH] feat(session api): respect lockout policy (#8027) # Which Problems Are Solved The session API was designed to be flexible enough for multiple use cases / login scenarios, where the login could respect the login policy or not. The session API itself does not have a corresponding policy and would not check for a required MFA or alike. It therefore also did not yet respect the lockout policy and would leave it to the login UI to handle that. Since the lockout policy is related to the user and not the login itself, we decided to handle the lockout also on calls of the session API. # How the Problems Are Solved If a lockout policy is set for either password or (T)OTP checks, the corresponding check on the session API be run against the lockout check. This means that any failed check, regardless if occurred in the session API or the current hosted login will be counted against the maximum allowed checks of that authentication mechanism. TOTP, OTP SMS and OTP Email are each treated as a separate mechanism. For implementation: - The existing lockout check functions were refactored to be usable for session API calls. - `SessionCommand` type now returns not only an error, but also `[]eventstore.Command` - these will be executed in case of an error # Additional Changes None. # Additional Context Closes #7967 --------- Co-authored-by: Elio Bischof --- .../session/v2/session_integration_test.go | 47 +++ .../eventsourcing/eventstore/auth_request.go | 6 +- .../instance_policy_password_lockout.go | 6 +- internal/command/org_policy_lockout.go | 19 +- internal/command/session.go | 115 +++---- internal/command/session_otp.go | 83 +++-- internal/command/session_otp_test.go | 304 ++++++++++++++++-- internal/command/session_test.go | 129 +++++++- internal/command/session_webauhtn.go | 25 +- internal/command/user_human_otp.go | 114 ++++--- internal/command/user_human_otp_model.go | 54 ++-- internal/command/user_human_password.go | 58 ++-- internal/command/user_human_password_test.go | 22 +- 13 files changed, 732 insertions(+), 250 deletions(-) diff --git a/internal/api/grpc/session/v2/session_integration_test.go b/internal/api/grpc/session/v2/session_integration_test.go index 689893319b..c6fd1b5d42 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 a5105d1f5b..e695d83511 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 59766ae38f..9e677c573c 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 052fd9e239..d7ace6f69e 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 d6fcb072ad..ccc224cab7 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 859f893400..6b7e20fb1a 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 c5be0aecae..d6934d0472 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 feaf395c22..46cbaeafc3 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 680b641a55..0bbb68c99d 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 c368167b3d..39abab3b86 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 43abc14349..2780a52d7c 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 fd8b29a429..839977b384 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 4e7672d8bc..d944f5e1cc 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) }