Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add test OTP support for mobile app reviews #1166

Merged
merged 1 commit into from
Aug 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 27 additions & 16 deletions internal/api/phone.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,30 +67,41 @@ func (a *API) sendPhoneConfirmation(ctx context.Context, tx *storage.Connection,
return "", internalServerError("invalid otp type")
}

// intentionally keeping this before the test OTP, so that the behavior
// of regular and test OTPs is similar
if sentAt != nil && !sentAt.Add(config.Sms.MaxFrequency).Before(time.Now()) {
return "", MaxFrequencyLimitError
}
oldToken := *token
otp, err := crypto.GenerateOtp(config.Sms.OtpLength)
if err != nil {
return "", internalServerError("error generating otp").WithInternalError(err)
}
*token = crypto.GenerateTokenHash(phone, otp)

var message string
if config.Sms.Template == "" {
message = fmt.Sprintf(defaultSmsMessage, otp)
} else {
message = strings.Replace(config.Sms.Template, "{{ .Code }}", otp, -1)
now := time.Now()

var otp, messageID string

if testOTP, ok := config.Sms.GetTestOTP(phone, now); ok {
otp = testOTP
messageID = "test-otp"
}

messageID, serr := smsProvider.SendMessage(phone, message, channel)
if serr != nil {
*token = oldToken
return messageID, serr
if otp == "" { // not using test OTPs
otp, err := crypto.GenerateOtp(config.Sms.OtpLength)
if err != nil {
return "", internalServerError("error generating otp").WithInternalError(err)
}

var message string
if config.Sms.Template == "" {
message = fmt.Sprintf(defaultSmsMessage, otp)
} else {
message = strings.Replace(config.Sms.Template, "{{ .Code }}", otp, -1)
}

messageID, err = smsProvider.SendMessage(phone, message, channel)
if err != nil {
return messageID, err
}
}

now := time.Now()
*token = crypto.GenerateTokenHash(phone, otp)

switch otpType {
case phoneConfirmationOtp:
Expand Down
34 changes: 32 additions & 2 deletions internal/api/phone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,12 @@ type PhoneTestSuite struct {

type TestSmsProvider struct {
mock.Mock

SentMessages int
}

func (t *TestSmsProvider) SendMessage(phone string, message string, channel string) (string, error) {
t.SentMessages += 1
return "", nil
}

Expand Down Expand Up @@ -66,7 +69,7 @@ func (ts *PhoneTestSuite) TestFormatPhoneNumber() {
assert.Equal(ts.T(), "123456789", actual)
}

func (ts *PhoneTestSuite) TestSendPhoneConfirmation() {
func doTestSendPhoneConfirmation(ts *PhoneTestSuite, useTestOTP bool) {
u, err := models.FindUserByPhoneAndAudience(ts.API.db, "123456789", ts.Config.JWT.Aud)
require.NoError(ts.T(), err)
ctx := context.Background()
Expand Down Expand Up @@ -97,13 +100,31 @@ func (ts *PhoneTestSuite) TestSendPhoneConfirmation() {
},
}

if useTestOTP {
ts.API.config.Sms.TestOTP = map[string]string{
"123456789": "123456",
}
} else {
ts.API.config.Sms.TestOTP = nil
}

for _, c := range cases {
ts.Run(c.desc, func() {
_, err = ts.API.sendPhoneConfirmation(ctx, ts.API.db, u, "123456789", c.otpType, &TestSmsProvider{}, sms_provider.SMSProvider)
provider := &TestSmsProvider{}

_, err = ts.API.sendPhoneConfirmation(ctx, ts.API.db, u, "123456789", c.otpType, provider, sms_provider.SMSProvider)
require.Equal(ts.T(), c.expected, err)
u, err = models.FindUserByPhoneAndAudience(ts.API.db, "123456789", ts.Config.JWT.Aud)
require.NoError(ts.T(), err)

if c.expected == nil {
if useTestOTP {
require.Equal(ts.T(), provider.SentMessages, 0)
} else {
require.Equal(ts.T(), provider.SentMessages, 1)
}
}

switch c.otpType {
case phoneConfirmationOtp:
require.NotEmpty(ts.T(), u.ConfirmationToken)
Expand All @@ -120,6 +141,14 @@ func (ts *PhoneTestSuite) TestSendPhoneConfirmation() {
}
}

func (ts *PhoneTestSuite) TestSendPhoneConfirmation() {
doTestSendPhoneConfirmation(ts, false)
}

func (ts *PhoneTestSuite) TestSendPhoneConfirmationWithTestOTP() {
doTestSendPhoneConfirmation(ts, true)
}

func (ts *PhoneTestSuite) TestMissingSmsProviderConfig() {
u, err := models.FindUserByPhoneAndAudience(ts.API.db, "123456789", ts.Config.JWT.Aud)
require.NoError(ts.T(), err)
Expand Down Expand Up @@ -198,6 +227,7 @@ func (ts *PhoneTestSuite) TestMissingSmsProviderConfig() {
ts.Config.Sms.Messagebird.AccessKey = ""
ts.Config.Sms.Textlocal.ApiKey = ""
ts.Config.Sms.Vonage.ApiKey = ""

for _, c := range cases {
for _, provider := range smsProviders {
ts.Config.Sms.Provider = provider
Expand Down
24 changes: 18 additions & 6 deletions internal/conf/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,19 +221,31 @@ type PhoneProviderConfiguration struct {
}

type SmsProviderConfiguration struct {
Autoconfirm bool `json:"autoconfirm"`
MaxFrequency time.Duration `json:"max_frequency" split_words:"true"`
OtpExp uint `json:"otp_exp" split_words:"true"`
OtpLength int `json:"otp_length" split_words:"true"`
Provider string `json:"provider"`
Template string `json:"template"`
Autoconfirm bool `json:"autoconfirm"`
MaxFrequency time.Duration `json:"max_frequency" split_words:"true"`
OtpExp uint `json:"otp_exp" split_words:"true"`
OtpLength int `json:"otp_length" split_words:"true"`
Provider string `json:"provider"`
Template string `json:"template"`
TestOTP map[string]string `json:"test_otp" split_words:"true"`
TestOTPValidUntil time.Time `json:"test_otp_valid_until" split_words:"true"`

Twilio TwilioProviderConfiguration `json:"twilio"`
TwilioVerify TwilioVerifyProviderConfiguration `json:"twilio_verify" split_words:"true"`
Messagebird MessagebirdProviderConfiguration `json:"messagebird"`
Textlocal TextlocalProviderConfiguration `json:"textlocal"`
Vonage VonageProviderConfiguration `json:"vonage"`
}

func (c *SmsProviderConfiguration) GetTestOTP(phone string, now time.Time) (string, bool) {
if c.TestOTP != nil && (c.TestOTPValidUntil.IsZero() || now.Before(c.TestOTPValidUntil)) {
kangmingtay marked this conversation as resolved.
Show resolved Hide resolved
testOTP, ok := c.TestOTP[phone]
return testOTP, ok
}

return "", false
}

type TwilioProviderConfiguration struct {
AccountSid string `json:"account_sid" split_words:"true"`
AuthToken string `json:"auth_token" split_words:"true"`
Expand Down