TheArchive/zitadel
1
0
Fork 0
mirror of https://github.com/zitadel/zitadel.git synced 2025-08-11 23:27:31 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: twilio code generation and verification (#8728)

Browse Source 
# 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>
(cherry picked from commit f653589609)
This commit is contained in:
Livio Spring
2024-10-07 07:12:44 +02:00
parent 1f07d4128f
commit eb59893344
8 changed files with 188 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
internal/command/session.go
View File

@@ -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,
}
}

2
internal/command/session_otp.go
View File

@@ -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,
)

38
internal/command/session_otp_test.go
View File

@@ -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
},

5
internal/command/user_v2_human_test.go
View File

@@ -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,
"",
),
),
),

2
internal/command/user_v2_model_test.go
View File

@@ -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(

12
internal/command/user_v2_password.go
View File

@@ -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

140
internal/command/user_v2_password_test.go
View File

@@ -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
eventstore func(t *testing.T) *eventstore.Eventstore
userEncryption crypto.EncryptionAlgorithm
newCode encrypedCodeFunc
checkPermission domain.PermissionCheck
eventstore func(t *testing.T) *eventstore.Eventstore
userEncryption crypto.EncryptionAlgorithm
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(),
newCode: mockEncryptedCode("code", 10*time.Minute),
defaultSecretGenerators: defaultGenerators,
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,
eventstore: tt.fields.eventstore(t),
userEncryption: tt.fields.userEncryption,
newEncryptedCode: tt.fields.newCode,
checkPermission: tt.fields.checkPermission,
eventstore: tt.fields.eventstore(t),
userEncryption: tt.fields.userEncryption,
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)

2
internal/repository/user/human_password.go
View File

@@ -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,
}
}