Skip to content

Commit

Permalink
Merge pull request from GHSA-7h8m-vrxx-vr4m
Browse files Browse the repository at this point in the history 
* fix: handle locking policy correctly for multiple simultaneous password checks

* recheck events
  • Loading branch information
@livio-a
livio-a committed Nov 8, 2023
1 parent 9a708b1 commit 22e2d55
Show file tree
Hide file tree
Showing 4 changed files with 178 additions and 4 deletions.
17 changes: 14 additions & 3 deletions internal/command/user_human_password.go
View file
Expand Up @@ -233,18 +233,30 @@ 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)
ctx, spanPasswordComparison := tracing.NewNamedSpan(ctx, "passwap.Verify")
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 != "" {
Expand All @@ -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")
Expand Down
9 changes: 9 additions & 0 deletions internal/command/user_human_password_model.go
View file
Expand Up @@ -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:
Expand All @@ -92,6 +97,7 @@ func (wm *HumanPasswordWriteModel) Query() *eventstore.SearchQueryBuilder {
user.HumanPasswordCheckSucceededType,
user.HumanPasswordHashUpdatedType,
user.UserRemovedType,
user.UserLockedType,
user.UserUnlockedType,
user.UserV1AddedType,
user.UserV1RegisteredType,
Expand All @@ -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
}
150 changes: 149 additions & 1 deletion internal/command/user_human_password_test.go
View file
Expand Up @@ -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{
Expand Down Expand Up @@ -1336,6 +1398,7 @@ func TestCommandSide_CheckPassword(t *testing.T) {
false,
"")),
),
expectFilter(),
expectPush(
user.NewHumanPasswordCheckFailedEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1507,6 +1572,7 @@ func TestCommandSide_CheckPassword(t *testing.T) {
false,
"")),
),
expectFilter(),
expectPush(
user.NewHumanPasswordCheckSucceededEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
Expand Down Expand Up @@ -1587,6 +1653,7 @@ func TestCommandSide_CheckPassword(t *testing.T) {
false,
"")),
),
expectFilter(),
expectPush(
user.NewHumanPasswordCheckSucceededEvent(
context.Background(),
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -1682,6 +1829,7 @@ func TestCommandSide_CheckPassword(t *testing.T) {
},
),
),
expectFilter(),
expectPush(
user.NewHumanPasswordCheckSucceededEvent(context.Background(),
&user.NewAggregate("user1", "org1").Aggregate,
Expand Down
6 changes: 6 additions & 0 deletions internal/query/user_password.go
View file
Expand Up @@ -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:
Expand Down Expand Up @@ -129,6 +134,7 @@ func (wm *HumanPasswordReadModel) Query() *eventstore.SearchQueryBuilder {
user.HumanPasswordCheckSucceededType,
user.HumanPasswordHashUpdatedType,
user.UserRemovedType,
user.UserLockedType,
user.UserUnlockedType,
user.UserV1AddedType,
user.UserV1RegisteredType,
Expand Down

1 comment on commit 22e2d55

@vercel
Copy link

@vercel vercel bot commented on 22e2d55 Nov 8, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

docs – ./

zitadel-docs.vercel.app
docs-git-main-zitadel.vercel.app
docs-zitadel.vercel.app

Please sign in to comment.