From 64ee480f84cd6df89f0edf4e4f10a1d5fea8c5c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20M=C3=B6hlmann?= Date: Wed, 11 Mar 2026 08:25:21 +0100 Subject: [PATCH] Merge commit from fork When an expired invite code was used for webauthn (passkeys), the change date got updated by the failed event. This change date was used to test for expiry, meaning failed events would reset the expiry timeout. This fix adds a Code Creation Date to the writemodel which gets set by the first event. This can be the added or requested event. Other changes: - Expiry renamed to CodeExpiry so it's consistent with similar write models using secret codes. - humanVerifyPasswordlessInitCode takes an algorithm instead of the complete generator, so the method can be unit tested easier - Added tests that reproduced the original issue --- internal/command/main_test.go | 6 +- internal/command/user_converter.go | 2 +- internal/command/user_human_webauthn.go | 8 +- internal/command/user_human_webauthn_model.go | 17 +- internal/command/user_human_webauthn_test.go | 181 ++++++++++++++++++ internal/command/user_v2_passkey.go | 2 +- internal/command/user_v2_passkey_test.go | 62 ++++++ 7 files changed, 264 insertions(+), 14 deletions(-) create mode 100644 internal/command/user_human_webauthn_test.go diff --git a/internal/command/main_test.go b/internal/command/main_test.go index cd67a8615..784b20eee 100644 --- a/internal/command/main_test.go +++ b/internal/command/main_test.go @@ -158,8 +158,12 @@ func eventFromEventPusherWithInstanceID(instanceID string, event eventstore.Comm } func eventFromEventPusherWithCreationDateNow(event eventstore.Command) *repository.Event { + return eventFromEventPusherWithCreationDate(event, time.Now()) +} + +func eventFromEventPusherWithCreationDate(event eventstore.Command, creationDate time.Time) *repository.Event { e := eventFromEventPusher(event) - e.CreationDate = time.Now() + e.CreationDate = creationDate return e } diff --git a/internal/command/user_converter.go b/internal/command/user_converter.go index 484cdc35f..8fa293091 100644 --- a/internal/command/user_converter.go +++ b/internal/command/user_converter.go @@ -135,7 +135,7 @@ func writeModelToPasswordlessInitCode(initCodeModel *HumanPasswordlessInitCodeWr ObjectRoot: writeModelToObjectRoot(initCodeModel.WriteModel), CodeID: initCodeModel.CodeID, Code: code, - Expiration: initCodeModel.Expiration, + Expiration: initCodeModel.CodeExpiration, State: initCodeModel.State, } } diff --git a/internal/command/user_human_webauthn.go b/internal/command/user_human_webauthn.go index f6a85f73f..60614d3e6 100644 --- a/internal/command/user_human_webauthn.go +++ b/internal/command/user_human_webauthn.go @@ -130,7 +130,7 @@ func (c *Commands) HumanAddPasswordlessSetup(ctx context.Context, userID, resour } func (c *Commands) HumanAddPasswordlessSetupInitCode(ctx context.Context, userID, resourceowner, codeID, verificationCode string, preferredPlatformType domain.AuthenticatorAttachment, passwordlessCodeGenerator crypto.Generator) (*domain.WebAuthNToken, error) { - err := c.humanVerifyPasswordlessInitCode(ctx, userID, resourceowner, codeID, verificationCode, passwordlessCodeGenerator) + err := c.humanVerifyPasswordlessInitCode(ctx, userID, resourceowner, codeID, verificationCode, passwordlessCodeGenerator.Alg()) if err != nil { return nil, err } @@ -212,7 +212,7 @@ func (c *Commands) HumanVerifyU2FSetup(ctx context.Context, userID, resourceowne } func (c *Commands) HumanPasswordlessSetupInitCode(ctx context.Context, userID, resourceowner, tokenName, userAgentID, codeID, verificationCode string, credentialData []byte, passwordlessCodeGenerator crypto.Generator) (*domain.ObjectDetails, error) { - err := c.humanVerifyPasswordlessInitCode(ctx, userID, resourceowner, codeID, verificationCode, passwordlessCodeGenerator) + err := c.humanVerifyPasswordlessInitCode(ctx, userID, resourceowner, codeID, verificationCode, passwordlessCodeGenerator.Alg()) if err != nil { return nil, err } @@ -569,7 +569,7 @@ func (c *Commands) HumanPasswordlessInitCodeSent(ctx context.Context, userID, re return err } -func (c *Commands) humanVerifyPasswordlessInitCode(ctx context.Context, userID, resourceOwner, codeID, verificationCode string, passwordlessCodeGenerator crypto.Generator) error { +func (c *Commands) humanVerifyPasswordlessInitCode(ctx context.Context, userID, resourceOwner, codeID, verificationCode string, alg crypto.EncryptionAlgorithm) error { if userID == "" || codeID == "" { return zerrors.ThrowPreconditionFailed(nil, "COMMAND-GVfg3", "Errors.IDMissing") } @@ -578,7 +578,7 @@ func (c *Commands) humanVerifyPasswordlessInitCode(ctx context.Context, userID, if err != nil { return err } - err = crypto.VerifyCode(initCode.ChangeDate, initCode.Expiration, initCode.CryptoCode, verificationCode, passwordlessCodeGenerator.Alg()) + err = crypto.VerifyCode(initCode.CodeCreationDate, initCode.CodeExpiration, initCode.CryptoCode, verificationCode, alg) if err != nil || initCode.State != domain.PasswordlessInitCodeStateActive { userAgg := UserAggregateFromWriteModel(&initCode.WriteModel) _, err = c.eventstore.Push(ctx, usr_repo.NewHumanPasswordlessInitCodeCheckFailedEvent(ctx, userAgg, codeID)) diff --git a/internal/command/user_human_webauthn_model.go b/internal/command/user_human_webauthn_model.go index aab9e1e57..d2fa39a59 100644 --- a/internal/command/user_human_webauthn_model.go +++ b/internal/command/user_human_webauthn_model.go @@ -452,11 +452,12 @@ func (rm *HumanPasswordlessLoginReadModel) Query() *eventstore.SearchQueryBuilde type HumanPasswordlessInitCodeWriteModel struct { eventstore.WriteModel - CodeID string - Attempts uint8 - CryptoCode *crypto.CryptoValue - Expiration time.Duration - State domain.PasswordlessInitCodeState + CodeID string + Attempts uint8 + CryptoCode *crypto.CryptoValue + CodeCreationDate time.Time + CodeExpiration time.Duration + State domain.PasswordlessInitCodeState } func NewHumanPasswordlessInitCodeWriteModel(userID, codeID, resourceOwner string) *HumanPasswordlessInitCodeWriteModel { @@ -520,13 +521,15 @@ func (wm *HumanPasswordlessInitCodeWriteModel) Reduce() error { func (wm *HumanPasswordlessInitCodeWriteModel) appendAddedEvent(e *user.HumanPasswordlessInitCodeAddedEvent) { wm.CryptoCode = e.Code - wm.Expiration = e.Expiry + wm.CodeExpiration = e.Expiry + wm.CodeCreationDate = e.CreationDate() wm.State = domain.PasswordlessInitCodeStateActive } func (wm *HumanPasswordlessInitCodeWriteModel) appendRequestedEvent(e *user.HumanPasswordlessInitCodeRequestedEvent) { wm.CryptoCode = e.Code - wm.Expiration = e.Expiry + wm.CodeExpiration = e.Expiry + wm.CodeCreationDate = e.CreationDate() wm.State = domain.PasswordlessInitCodeStateRequested if e.CodeReturned { wm.State = domain.PasswordlessInitCodeStateActive diff --git a/internal/command/user_human_webauthn_test.go b/internal/command/user_human_webauthn_test.go new file mode 100644 index 000000000..1f458a980 --- /dev/null +++ b/internal/command/user_human_webauthn_test.go @@ -0,0 +1,181 @@ +package command + +import ( + "context" + "io" + "testing" + "time" + + "github.com/stretchr/testify/require" + "go.uber.org/mock/gomock" + + http_util "github.com/zitadel/zitadel/internal/api/http" + "github.com/zitadel/zitadel/internal/crypto" + "github.com/zitadel/zitadel/internal/domain" + "github.com/zitadel/zitadel/internal/eventstore" + "github.com/zitadel/zitadel/internal/repository/user" + "github.com/zitadel/zitadel/internal/zerrors" +) + +func TestCommands_humanVerifyPasswordlessInitCode(t *testing.T) { + ctx := http_util.WithRequestedHost(context.Background(), "example.com") + alg := crypto.CreateMockEncryptionAlg(gomock.NewController(t)) + es := expectEventstore( + expectFilter(eventFromEventPusher(testSecretGeneratorAddedEvent(domain.SecretGeneratorTypePasswordlessInitCode))), + )(t) + code, err := newEncryptedCode(ctx, es.Filter, domain.SecretGeneratorTypePasswordlessInitCode, alg) //nolint:staticcheck + require.NoError(t, err) + userAgg := &user.NewAggregate("user1", "org1").Aggregate + + type fields struct { + eventstore func(*testing.T) *eventstore.Eventstore + } + type args struct { + userID string + resourceOwner string + codeID string + code string + } + tests := []struct { + name string + fields fields + args args + wantErr error + }{ + { + name: "filter error", + fields: fields{ + eventstore: expectEventstore( + expectFilterError(io.ErrClosedPipe), + ), + }, + args: args{ + userID: "user1", + resourceOwner: "org1", + codeID: "123", + }, + wantErr: io.ErrClosedPipe, + }, + { + name: "code verification error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusherWithCreationDateNow( + user.NewHumanPasswordlessInitCodeAddedEvent(context.Background(), + userAgg, "123", code.Crypted, time.Minute, + ), + ), + eventFromEventPusherWithCreationDateNow( + user.NewHumanPasswordlessInitCodeSentEvent(ctx, userAgg, "123"), + ), + ), + expectPush( + user.NewHumanPasswordlessInitCodeCheckFailedEvent(ctx, userAgg, "123"), + ), + ), + }, + args: args{ + userID: "user1", + resourceOwner: "org1", + codeID: "123", + code: "wrong", + }, + wantErr: zerrors.ThrowInvalidArgument(err, "COMMAND-Dhz8i", "Errors.User.Code.Invalid"), + }, + { + name: "success", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusherWithCreationDateNow( + user.NewHumanPasswordlessInitCodeAddedEvent(context.Background(), + userAgg, "123", code.Crypted, time.Minute, + ), + ), + eventFromEventPusherWithCreationDateNow( + user.NewHumanPasswordlessInitCodeSentEvent(ctx, userAgg, "123"), + ), + ), + ), + }, + args: args{ + userID: "user1", + resourceOwner: "org1", + codeID: "123", + code: code.Plain, + }, + }, + { + name: "expired error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusherWithCreationDate( + user.NewHumanPasswordlessInitCodeAddedEvent(context.Background(), + userAgg, "123", code.Crypted, time.Minute, + ), + time.Now().Add(-2*time.Minute), + ), + eventFromEventPusherWithCreationDate( + user.NewHumanPasswordlessInitCodeSentEvent(ctx, userAgg, "123"), + time.Now().Add(-2*time.Minute), + ), + ), + expectPush( + user.NewHumanPasswordlessInitCodeCheckFailedEvent(ctx, userAgg, "123"), + ), + ), + }, + args: args{ + userID: "user1", + resourceOwner: "org1", + codeID: "123", + code: code.Plain, + }, + wantErr: zerrors.ThrowInvalidArgument(err, "COMMAND-Dhz8i", "Errors.User.Code.Invalid"), + }, + { + // https://github.com/zitadel/zitadel/security/advisories/GHSA-2x66-r53r-9r86 + name: "expired, fail, check again", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusherWithCreationDate( + user.NewHumanPasswordlessInitCodeAddedEvent(context.Background(), + userAgg, "123", code.Crypted, time.Minute, + ), + time.Now().Add(-2*time.Minute), + ), + eventFromEventPusherWithCreationDate( + user.NewHumanPasswordlessInitCodeSentEvent(ctx, userAgg, "123"), + time.Now().Add(-2*time.Minute), + ), + eventFromEventPusherWithCreationDateNow( + user.NewHumanPasswordlessInitCodeCheckFailedEvent(ctx, userAgg, "123"), + ), + ), + expectPush( + user.NewHumanPasswordlessInitCodeCheckFailedEvent(ctx, userAgg, "123"), + ), + ), + }, + args: args{ + userID: "user1", + resourceOwner: "org1", + codeID: "123", + code: code.Plain, + }, + wantErr: zerrors.ThrowInvalidArgument(err, "COMMAND-Dhz8i", "Errors.User.Code.Invalid"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + c := &Commands{ + eventstore: tt.fields.eventstore(t), + } + err := c.humanVerifyPasswordlessInitCode(ctx, tt.args.userID, tt.args.resourceOwner, tt.args.codeID, tt.args.code, alg) + require.ErrorIs(t, err, tt.wantErr) + }) + } +} diff --git a/internal/command/user_v2_passkey.go b/internal/command/user_v2_passkey.go index 0f88b8837..a2d59f7a3 100644 --- a/internal/command/user_v2_passkey.go +++ b/internal/command/user_v2_passkey.go @@ -47,7 +47,7 @@ func (c *Commands) verifyUserPasskeyCode(ctx context.Context, userID, resourceOw if err != nil { return nil, err } - err = crypto.VerifyCode(wm.ChangeDate, wm.Expiration, wm.CryptoCode, code, alg) + err = crypto.VerifyCode(wm.CodeCreationDate, wm.CodeExpiration, wm.CryptoCode, code, alg) if err != nil || wm.State != domain.PasswordlessInitCodeStateActive { c.verifyUserPasskeyCodeFailed(ctx, wm) return nil, zerrors.ThrowInvalidArgument(err, "COMMAND-Eeb2a", "Errors.User.Code.Invalid") diff --git a/internal/command/user_v2_passkey_test.go b/internal/command/user_v2_passkey_test.go index 1609f1ab4..13182cd13 100644 --- a/internal/command/user_v2_passkey_test.go +++ b/internal/command/user_v2_passkey_test.go @@ -326,6 +326,68 @@ func TestCommands_verifyUserPasskeyCode(t *testing.T) { }, want: user.NewHumanPasswordlessInitCodeCheckSucceededEvent(ctx, userAgg, "123"), }, + { + name: "expired error", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusherWithCreationDate( + user.NewHumanPasswordlessInitCodeRequestedEvent(context.Background(), + userAgg, "123", code.Crypted, time.Minute, "", false, + ), + time.Now().Add(-2*time.Minute), + ), + eventFromEventPusherWithCreationDate( + user.NewHumanPasswordlessInitCodeSentEvent(ctx, userAgg, "123"), + time.Now().Add(-2*time.Minute), + ), + ), + expectPush( + user.NewHumanPasswordlessInitCodeCheckFailedEvent(ctx, userAgg, "123"), + ), + ), + }, + args: args{ + userID: "user1", + resourceOwner: "org1", + codeID: "123", + code: code.Plain, + }, + wantErr: zerrors.ThrowInvalidArgument(err, "COMMAND-Eeb2a", "Errors.User.Code.Invalid"), + }, + { + // https://github.com/zitadel/zitadel/security/advisories/GHSA-2x66-r53r-9r86 + name: "expired, fail, check again", + fields: fields{ + eventstore: expectEventstore( + expectFilter( + eventFromEventPusherWithCreationDate( + user.NewHumanPasswordlessInitCodeRequestedEvent(context.Background(), + userAgg, "123", code.Crypted, time.Minute, "", false, + ), + time.Now().Add(-2*time.Minute), + ), + eventFromEventPusherWithCreationDate( + user.NewHumanPasswordlessInitCodeSentEvent(ctx, userAgg, "123"), + time.Now().Add(-2*time.Minute), + ), + eventFromEventPusherWithCreationDateNow( + user.NewHumanPasswordlessInitCodeCheckFailedEvent(ctx, userAgg, "123"), + ), + ), + expectPush( + user.NewHumanPasswordlessInitCodeCheckFailedEvent(ctx, userAgg, "123"), + ), + ), + }, + args: args{ + userID: "user1", + resourceOwner: "org1", + codeID: "123", + code: code.Plain, + }, + wantErr: zerrors.ThrowInvalidArgument(err, "COMMAND-Eeb2a", "Errors.User.Code.Invalid"), + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {