fix: twilio code generation and verification (#8728)

# Which Problems Are Solved

The recently added possibility to generate and verify codes through
Twilio verification service did failed on checking OTP SMS code through
the session API. Additionally, password codes generated by the V2 API
and sent through phone would always use the internal generator and
verification mechanism rather than the configured.

# How the Problems Are Solved

- Correctly set the verifier for OTP SMS for the session API
  - Always use the internal verifier for OTP Email (for now)
- Select the generator / verifier based on the configuration for
password codes with notification type SMS for V2 APIs

# Additional Changes

None

# Additional Context

- relates to #8678 
- reported by customer

---------

Co-authored-by: Stefan Benz <46600784+stebenz@users.noreply.github.com>

internal/command/session.go

@@ -55,6 +55,7 @@ func (c *Commands) NewSessionCommands(cmds []SessionCommand, session *SessionWri
 		createCode:        c.newEncryptedCodeWithDefault,
 		createPhoneCode:   c.newPhoneCode,
 		createToken:       c.sessionTokenCreator,
+		getCodeVerifier:   c.phoneCodeVerifierFromConfig,
 		now:               time.Now,
 	}
 }

internal/command/session_otp.go

@@ -180,7 +180,7 @@ func CheckOTPEmail(code string) SessionCommand {
 			writeModel,
 			cmd.eventstore.FilterToQueryReducer,
 			cmd.otpAlg,
-			cmd.getCodeVerifier,
+			nil, // email currently always uses local code checks
 			succeededEvent,
 			failedEvent,
 		)

internal/command/session_otp_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/zitadel/zitadel/internal/domain"
 	"github.com/zitadel/zitadel/internal/eventstore"
 	"github.com/zitadel/zitadel/internal/notification/senders"
+	"github.com/zitadel/zitadel/internal/notification/senders/mock"
 	"github.com/zitadel/zitadel/internal/repository/org"
 	"github.com/zitadel/zitadel/internal/repository/session"
 	"github.com/zitadel/zitadel/internal/repository/user"
@@ -763,6 +764,7 @@ func TestCheckOTPSMS(t *testing.T) {
 		userID           string
 		otpCodeChallenge *OTPCode
 		otpAlg           crypto.EncryptionAlgorithm
+		getCodeVerifier  func(ctx context.Context, id string) (senders.CodeGenerator, error)
 	}
 	type args struct {
 		code string
@@ -948,6 +950,41 @@ func TestCheckOTPSMS(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "check ok (external)",
+			fields: fields{
+				eventstore: expectEventstore(
+					expectFilter(
+						eventFromEventPusher(user.NewHumanOTPSMSAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate)),
+					),
+					expectFilter(), // recheck
+				),
+				userID: "userID",
+				otpCodeChallenge: &OTPCode{
+					Code:           nil,
+					Expiry:         0,
+					GeneratorID:    "generatorID",
+					VerificationID: "verificationID",
+					CreationDate:   testNow,
+				},
+				getCodeVerifier: func(ctx context.Context, id string) (senders.CodeGenerator, error) {
+					sender := mock.NewMockCodeGenerator(gomock.NewController(t))
+					sender.EXPECT().VerifyCode("verificationID", "code").Return(nil)
+					return sender, nil
+				},
+			},
+			args: args{
+				code: "code",
+			},
+			res: res{
+				commands: []eventstore.Command{
+					user.NewHumanOTPSMSCheckSucceededEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate, nil),
+					session.NewOTPSMSCheckedEvent(context.Background(), &session.NewAggregate("sessionID", "instanceID").Aggregate,
+						testNow,
+					),
+				},
+			},
+		},
 		{
 			name: "check ok, locked in the meantime",
 			fields: fields{
@@ -996,6 +1033,7 @@ func TestCheckOTPSMS(t *testing.T) {
 				sessionWriteModel: sessionModel,
 				eventstore:        tt.fields.eventstore(t),
 				otpAlg:            tt.fields.otpAlg,
+				getCodeVerifier:   tt.fields.getCodeVerifier,
 				now: func() time.Time {
 					return testNow
 				},

internal/command/user_v2_human_test.go

@@ -2967,6 +2967,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) {
 								domain.NotificationTypeEmail,
 								"",
 								false,
+								"",
 							),
 						),
 					),
@@ -3040,6 +3041,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) {
 								domain.NotificationTypeEmail,
 								"",
 								false,
+								"",
 							),
 						),
 					),
@@ -3091,6 +3093,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) {
 								domain.NotificationTypeEmail,
 								"",
 								false,
+								"",
 							),
 						),
 					),
@@ -3152,6 +3155,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) {
 								domain.NotificationTypeEmail,
 								"",
 								false,
+								"",
 							),
 						),
 					),
@@ -3213,6 +3217,7 @@ func TestCommandSide_ChangeUserHuman(t *testing.T) {
 								domain.NotificationTypeEmail,
 								"",
 								false,
+								"",
 							),
 						),
 					),

internal/command/user_v2_model_test.go

@@ -1494,6 +1494,7 @@ func TestCommandSide_userHumanWriteModel_password(t *testing.T) {
 								domain.NotificationTypeEmail,
 								"",
 								false,
+								"",
 							),
 						),
 					),
@@ -1556,6 +1557,7 @@ func TestCommandSide_userHumanWriteModel_password(t *testing.T) {
 								domain.NotificationTypeEmail,
 								"",
 								false,
+								"",
 							),
 						),
 						eventFromEventPusher(

internal/command/user_v2_password.go

@@ -55,14 +55,20 @@ func (c *Commands) requestPasswordReset(ctx context.Context, userID string, retu
 			return nil, nil, err
 		}
 	}
-	code, err := c.newEncryptedCode(ctx, c.eventstore.Filter, domain.SecretGeneratorTypePasswordResetCode, c.userEncryption) //nolint:staticcheck
+	var passwordCode *EncryptedCode
+	var generatorID string
+	if notificationType == domain.NotificationTypeSms {
+		passwordCode, generatorID, err = c.newPhoneCode(ctx, c.eventstore.Filter, domain.SecretGeneratorTypePasswordResetCode, c.userEncryption, c.defaultSecretGenerators.PasswordVerificationCode) //nolint:staticcheck
+	} else {
+		passwordCode, err = c.newEncryptedCode(ctx, c.eventstore.Filter, domain.SecretGeneratorTypePasswordResetCode, c.userEncryption) //nolint:staticcheck
+	}
 	if err != nil {
 		return nil, nil, err
 	}
-	cmd := user.NewHumanPasswordCodeAddedEventV2(ctx, UserAggregateFromWriteModel(&model.WriteModel), code.Crypted, code.Expiry, notificationType, urlTmpl, returnCode)
+	cmd := user.NewHumanPasswordCodeAddedEventV2(ctx, UserAggregateFromWriteModelCtx(ctx, &model.WriteModel), passwordCode.CryptedCode(), passwordCode.CodeExpiry(), notificationType, urlTmpl, returnCode, generatorID)
 
 	if returnCode {
-		plainCode = &code.Plain
+		plainCode = &passwordCode.Plain
 	}
 	if err = c.pushAppendAndReduce(ctx, model, cmd); err != nil {
 		return nil, nil, err

internal/command/user_v2_password_test.go

@@ -14,6 +14,7 @@ import (
 	"github.com/zitadel/zitadel/internal/crypto"
 	"github.com/zitadel/zitadel/internal/domain"
 	"github.com/zitadel/zitadel/internal/eventstore"
+	"github.com/zitadel/zitadel/internal/repository/instance"
 	"github.com/zitadel/zitadel/internal/repository/user"
 	"github.com/zitadel/zitadel/internal/zerrors"
 )
@@ -327,11 +328,23 @@ func TestCommands_RequestPasswordResetURLTemplate(t *testing.T) {
 }
 
 func TestCommands_requestPasswordReset(t *testing.T) {
+	defaultGenerators := &SecretGenerators{
+		OTPSMS: &crypto.GeneratorConfig{
+			Length:              8,
+			Expiry:              time.Hour,
+			IncludeLowerLetters: true,
+			IncludeUpperLetters: true,
+			IncludeDigits:       true,
+			IncludeSymbols:      true,
+		},
+	}
 	type fields struct {
-		checkPermission domain.PermissionCheck
+		checkPermission             domain.PermissionCheck
-		eventstore      func(t *testing.T) *eventstore.Eventstore
+		eventstore                  func(t *testing.T) *eventstore.Eventstore
-		userEncryption  crypto.EncryptionAlgorithm
+		userEncryption              crypto.EncryptionAlgorithm
-		newCode         encrypedCodeFunc
+		newCode                     encrypedCodeFunc
+		newEncryptedCodeWithDefault encryptedCodeWithDefaultFunc
+		defaultSecretGenerators     *SecretGenerators
 	}
 	type args struct {
 		ctx              context.Context
@@ -449,6 +462,7 @@ func TestCommands_requestPasswordReset(t *testing.T) {
 							domain.NotificationTypeEmail,
 							"",
 							false,
+							"",
 						),
 					),
 				),
@@ -489,6 +503,7 @@ func TestCommands_requestPasswordReset(t *testing.T) {
 							domain.NotificationTypeEmail,
 							"https://example.com/password/changey?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}",
 							false,
+							"",
 						),
 					),
 				),
@@ -518,6 +533,36 @@ func TestCommands_requestPasswordReset(t *testing.T) {
 								language.English, domain.GenderUnspecified, "email", false),
 						),
 					),
+					expectFilter(
+						eventFromEventPusher(
+							instance.NewSMSConfigActivatedEvent(
+								context.Background(),
+								&instance.NewAggregate("instanceID").Aggregate,
+								"id",
+							),
+						),
+					),
+					expectFilter(
+						eventFromEventPusher(
+							instance.NewSMSConfigTwilioAddedEvent(
+								context.Background(),
+								&instance.NewAggregate("instanceID").Aggregate,
+								"id",
+								"",
+								"sid",
+								"senderNumber",
+								&crypto.CryptoValue{CryptoType: crypto.TypeEncryption, Algorithm: "enc", KeyID: "id", Crypted: []byte("crypted")},
+								"",
+							),
+						),
+						eventFromEventPusher(
+							instance.NewSMSConfigActivatedEvent(
+								context.Background(),
+								&instance.NewAggregate("instanceID").Aggregate,
+								"id",
+							),
+						),
+					),
 					expectPush(
 						user.NewHumanPasswordCodeAddedEventV2(context.Background(), &user.NewAggregate("userID", "org1").Aggregate,
 							&crypto.CryptoValue{
@@ -530,11 +575,82 @@ func TestCommands_requestPasswordReset(t *testing.T) {
 							domain.NotificationTypeSms,
 							"https://example.com/password/changey?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}",
 							false,
+							"",
 						),
 					),
 				),
-				checkPermission: newMockPermissionCheckAllowed(),
+				defaultSecretGenerators:     defaultGenerators,
-				newCode:         mockEncryptedCode("code", 10*time.Minute),
+				checkPermission:             newMockPermissionCheckAllowed(),
+				newEncryptedCodeWithDefault: mockEncryptedCodeWithDefault("code", 10*time.Minute),
+			},
+			args: args{
+				ctx:              context.Background(),
+				userID:           "userID",
+				urlTmpl:          "https://example.com/password/changey?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}",
+				notificationType: domain.NotificationTypeSms,
+			},
+			res: res{
+				details: &domain.ObjectDetails{
+					ResourceOwner: "org1",
+				},
+				code: nil,
+			},
+		},
+		{
+			name: "code generated template sms external",
+			fields: fields{
+				eventstore: expectEventstore(
+					expectFilter(
+						eventFromEventPusher(
+							user.NewHumanAddedEvent(context.Background(), &user.NewAggregate("userID", "org1").Aggregate,
+								"username", "firstname", "lastname", "nickname", "displayname",
+								language.English, domain.GenderUnspecified, "email", false),
+						),
+					),
+					expectFilter(
+						eventFromEventPusher(
+							instance.NewSMSConfigActivatedEvent(
+								context.Background(),
+								&instance.NewAggregate("instanceID").Aggregate,
+								"id",
+							),
+						),
+					),
+					expectFilter(
+						eventFromEventPusher(
+							instance.NewSMSConfigTwilioAddedEvent(
+								context.Background(),
+								&instance.NewAggregate("instanceID").Aggregate,
+								"id",
+								"",
+								"sid",
+								"senderNumber",
+								&crypto.CryptoValue{CryptoType: crypto.TypeEncryption, Algorithm: "enc", KeyID: "id", Crypted: []byte("crypted")},
+								"verifyServiceSid",
+							),
+						),
+						eventFromEventPusher(
+							instance.NewSMSConfigActivatedEvent(
+								context.Background(),
+								&instance.NewAggregate("instanceID").Aggregate,
+								"id",
+							),
+						),
+					),
+					expectPush(
+						user.NewHumanPasswordCodeAddedEventV2(context.Background(), &user.NewAggregate("userID", "org1").Aggregate,
+							nil,
+							0,
+							domain.NotificationTypeSms,
+							"https://example.com/password/changey?userID={{.UserID}}&code={{.Code}}&orgID={{.OrgID}}",
+							false,
+							"id",
+						),
+					),
+				),
+				checkPermission:         newMockPermissionCheckAllowed(),
+				newCode:                 mockEncryptedCode("code", 10*time.Minute),
+				defaultSecretGenerators: defaultGenerators,
 			},
 			args: args{
 				ctx:              context.Background(),
@@ -572,6 +688,7 @@ func TestCommands_requestPasswordReset(t *testing.T) {
 							domain.NotificationTypeEmail,
 							"",
 							true,
+							"",
 						),
 					),
 				),
@@ -594,11 +711,14 @@ func TestCommands_requestPasswordReset(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			c := &Commands{
-				checkPermission:  tt.fields.checkPermission,
+				checkPermission:             tt.fields.checkPermission,
-				eventstore:       tt.fields.eventstore(t),
+				eventstore:                  tt.fields.eventstore(t),
-				userEncryption:   tt.fields.userEncryption,
+				userEncryption:              tt.fields.userEncryption,
-				newEncryptedCode: tt.fields.newCode,
+				newEncryptedCode:            tt.fields.newCode,
+				newEncryptedCodeWithDefault: tt.fields.newEncryptedCodeWithDefault,
+				defaultSecretGenerators:     tt.fields.defaultSecretGenerators,
 			}
+
 			got, gotPlainCode, err := c.requestPasswordReset(tt.args.ctx, tt.args.userID, tt.args.returnCode, tt.args.urlTmpl, tt.args.notificationType)
 			require.ErrorIs(t, err, tt.res.err)
 			assertObjectDetails(t, tt.res.details, got)

internal/repository/user/human_password.go

@@ -137,6 +137,7 @@ func NewHumanPasswordCodeAddedEventV2(
 	notificationType domain.NotificationType,
 	urlTemplate string,
 	codeReturned bool,
+	generatorID string,
 ) *HumanPasswordCodeAddedEvent {
 	return &HumanPasswordCodeAddedEvent{
 		BaseEvent: *eventstore.NewBaseEventForPush(
@@ -150,6 +151,7 @@ func NewHumanPasswordCodeAddedEventV2(
 		URLTemplate:       urlTemplate,
 		CodeReturned:      codeReturned,
 		TriggeredAtOrigin: http.DomainContext(ctx).Origin(),
+		GeneratorID:       generatorID,
 	}
 }