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) {