mirror of
https://github.com/zitadel/zitadel.git
synced 2025-02-28 16:57:23 +00:00
fix: handle locking policy correctly for multiple simultaneous password checks
Merge pull request from GHSA-7h8m-vrxx-vr4m * fix: handle locking policy correctly for multiple simultaneous password checks * recheck events (cherry picked from commit 22e2d5599918864877e054ebe82fb834a5aa1077)
This commit is contained in:
parent
0090868b46
commit
393f711ca7
@ -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")
|
||||
|
@ -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
|
||||
}
|
||||
|
@ -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,
|
||||
|
@ -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,
|
||||
|
Loading…
x
Reference in New Issue
Block a user