From 22e2d5599918864877e054ebe82fb834a5aa1077 Mon Sep 17 00:00:00 2001 From: Livio Spring Date: Wed, 8 Nov 2023 15:19:13 +0200 Subject: [PATCH] Merge pull request from GHSA-7h8m-vrxx-vr4m * fix: handle locking policy correctly for multiple simultaneous password checks * recheck events --- internal/command/user_human_password.go | 17 +- internal/command/user_human_password_model.go | 9 ++ internal/command/user_human_password_test.go | 150 +++++++++++++++++- internal/query/user_password.go | 6 + 4 files changed, 178 insertions(+), 4 deletions(-) diff --git a/internal/command/user_human_password.go b/internal/command/user_human_password.go index 487907212fa..fa87770a5f4 100644 --- a/internal/command/user_human_password.go +++ b/internal/command/user_human_password.go @@ -233,9 +233,12 @@ func (c *Commands) HumanCheckPassword(ctx context.Context, orgID, userID, passwo if wm.UserState == domain.UserStateUnspecified || wm.UserState == domain.UserStateDeleted { return caos_errs.ThrowPreconditionFailed(nil, "COMMAND-3n77z", "Errors.User.NotFound") } + if wm.UserState == domain.UserStateLocked { + return caos_errs.ThrowPreconditionFailed(nil, "COMMAND-JLK35", "Errors.User.Locked") + } if wm.EncodedHash == "" { - return caos_errs.ThrowPreconditionFailed(nil, "COMMAND-3n77z", "Errors.User.Password.NotSet") + return caos_errs.ThrowPreconditionFailed(nil, "COMMAND-3nJ4t", "Errors.User.Password.NotSet") } userAgg := UserAggregateFromWriteModel(&wm.WriteModel) @@ -243,8 +246,17 @@ func (c *Commands) HumanCheckPassword(ctx context.Context, orgID, userID, passwo updated, err := c.userPasswordHasher.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) + if recheckErr != nil { + return recheckErr + } + if wm.UserState == domain.UserStateLocked { + return caos_errs.ThrowPreconditionFailed(nil, "COMMAND-SFA3t", "Errors.User.Locked") + } + if err == nil { commands = append(commands, user.NewHumanPasswordCheckSucceededEvent(ctx, userAgg, authRequestDomainToAuthRequestInfo(authRequest))) if updated != "" { @@ -259,7 +271,6 @@ func (c *Commands) HumanCheckPassword(ctx context.Context, orgID, userID, passwo if 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") diff --git a/internal/command/user_human_password_model.go b/internal/command/user_human_password_model.go index c4064ba157c..ec4d11da24c 100644 --- a/internal/command/user_human_password_model.go +++ b/internal/command/user_human_password_model.go @@ -65,8 +65,13 @@ func (wm *HumanPasswordWriteModel) Reduce() error { wm.PasswordCheckFailedCount += 1 case *user.HumanPasswordCheckSucceededEvent: wm.PasswordCheckFailedCount = 0 + case *user.UserLockedEvent: + wm.UserState = domain.UserStateLocked case *user.UserUnlockedEvent: wm.PasswordCheckFailedCount = 0 + if wm.UserState != domain.UserStateDeleted { + wm.UserState = domain.UserStateActive + } case *user.UserRemovedEvent: wm.UserState = domain.UserStateDeleted case *user.HumanPasswordHashUpdatedEvent: @@ -92,6 +97,7 @@ func (wm *HumanPasswordWriteModel) Query() *eventstore.SearchQueryBuilder { user.HumanPasswordCheckSucceededType, user.HumanPasswordHashUpdatedType, user.UserRemovedType, + user.UserLockedType, user.UserUnlockedType, user.UserV1AddedType, user.UserV1RegisteredType, @@ -108,5 +114,8 @@ func (wm *HumanPasswordWriteModel) Query() *eventstore.SearchQueryBuilder { if wm.ResourceOwner != "" { query.ResourceOwner(wm.ResourceOwner) } + if wm.WriteModel.ProcessedSequence != 0 { + query.SequenceGreater(wm.WriteModel.ProcessedSequence) + } return query } diff --git a/internal/command/user_human_password_test.go b/internal/command/user_human_password_test.go index 2af86bc7282..a90d972502b 100644 --- a/internal/command/user_human_password_test.go +++ b/internal/command/user_human_password_test.go @@ -1222,6 +1222,68 @@ func TestCommandSide_CheckPassword(t *testing.T) { err: caos_errs.IsPreconditionFailed, }, }, + { + name: "user locked, precondition error", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + true, + false, + false, + false, + false, + false, + false, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + "", + time.Hour*1, + time.Hour*2, + time.Hour*3, + time.Hour*4, + time.Hour*5, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + eventFromEventPusher( + user.NewUserLockedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + ), + ), + ), + ), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + resourceOwner: "org1", + password: "password", + }, + res: res{ + err: caos_errs.IsPreconditionFailed, + }, + }, { name: "existing password empty, precondition error", fields: fields{ @@ -1336,6 +1398,7 @@ func TestCommandSide_CheckPassword(t *testing.T) { false, "")), ), + expectFilter(), expectPush( user.NewHumanPasswordCheckFailedEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -1417,8 +1480,10 @@ func TestCommandSide_CheckPassword(t *testing.T) { &user.NewAggregate("user1", "org1").Aggregate, "$plain$x$password", false, - "")), + ""), + ), ), + expectFilter(), expectPush( user.NewHumanPasswordCheckFailedEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -1507,6 +1572,7 @@ func TestCommandSide_CheckPassword(t *testing.T) { false, "")), ), + expectFilter(), expectPush( user.NewHumanPasswordCheckSucceededEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, @@ -1587,6 +1653,7 @@ func TestCommandSide_CheckPassword(t *testing.T) { false, "")), ), + expectFilter(), expectPush( user.NewHumanPasswordCheckSucceededEvent( context.Background(), @@ -1616,6 +1683,86 @@ func TestCommandSide_CheckPassword(t *testing.T) { }, res: res{}, }, + { + name: "check password ok, locked in the mean time", + fields: fields{ + eventstore: eventstoreExpect( + t, + expectFilter( + eventFromEventPusher( + org.NewLoginPolicyAddedEvent(context.Background(), + &org.NewAggregate("org1").Aggregate, + true, + false, + false, + false, + false, + false, + false, + false, + false, + false, + domain.PasswordlessTypeNotAllowed, + "", + time.Hour*1, + time.Hour*2, + time.Hour*3, + time.Hour*4, + time.Hour*5, + ), + ), + ), + expectFilter( + eventFromEventPusher( + user.NewHumanAddedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "username", + "firstname", + "lastname", + "nickname", + "displayname", + language.German, + domain.GenderUnspecified, + "email@test.ch", + true, + ), + ), + eventFromEventPusher( + user.NewHumanEmailVerifiedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + ), + ), + eventFromEventPusher( + user.NewHumanPasswordChangedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + "$plain$x$password", + false, + "")), + ), + expectFilter( + eventFromEventPusher( + user.NewUserLockedEvent(context.Background(), + &user.NewAggregate("user1", "org1").Aggregate, + ), + ), + ), + ), + userPasswordHasher: mockPasswordHasher("x"), + }, + args: args{ + ctx: context.Background(), + userID: "user1", + resourceOwner: "org1", + password: "password", + authReq: &domain.AuthRequest{ + ID: "request1", + AgentID: "agent1", + }, + }, + res: res{ + err: caos_errs.IsPreconditionFailed, + }, + }, { name: "regression test old version event", fields: fields{ @@ -1682,6 +1829,7 @@ func TestCommandSide_CheckPassword(t *testing.T) { }, ), ), + expectFilter(), expectPush( user.NewHumanPasswordCheckSucceededEvent(context.Background(), &user.NewAggregate("user1", "org1").Aggregate, diff --git a/internal/query/user_password.go b/internal/query/user_password.go index 0e9da8a5e5b..5f991cf002b 100644 --- a/internal/query/user_password.go +++ b/internal/query/user_password.go @@ -100,8 +100,13 @@ func (wm *HumanPasswordReadModel) Reduce() error { wm.PasswordCheckFailedCount += 1 case *user.HumanPasswordCheckSucceededEvent: wm.PasswordCheckFailedCount = 0 + case *user.UserLockedEvent: + wm.UserState = domain.UserStateLocked case *user.UserUnlockedEvent: wm.PasswordCheckFailedCount = 0 + if wm.UserState != domain.UserStateDeleted { + wm.UserState = domain.UserStateActive + } case *user.UserRemovedEvent: wm.UserState = domain.UserStateDeleted case *user.HumanPasswordHashUpdatedEvent: @@ -129,6 +134,7 @@ func (wm *HumanPasswordReadModel) Query() *eventstore.SearchQueryBuilder { user.HumanPasswordCheckSucceededType, user.HumanPasswordHashUpdatedType, user.UserRemovedType, + user.UserLockedType, user.UserUnlockedType, user.UserV1AddedType, user.UserV1RegisteredType,