Skip to content

Commit

Permalink
feat: add test OTP support for mobile app reviews (#1166)
Browse files Browse the repository at this point in the history
When developers build mobile apps that use phone login, they need to
provide pre-determined phone numbers and OTPs that will work so that
automated and manual app reviewers (that work at Apple's AppStore or
Google's Play Store) can test and confirm compliance with the phone
system.

Those reviewers / systems cannot be expected to provide their own phone
number.

Developers can thus set up the following environment variable:

```
GOTRUE_EXTERNAL_SMS_TEST_OTP="<phone-1>=<otp-1>, <phone-2>=<otp-2>..."
GOTRUE_EXTERNAL_SMS_TEST_OTP_VALID_UNTIL="<ISO date time>"
```

SMS messages are not sent to those test phone numbers. Furthermore after
the validity period has expired, they will automatically not be used.
This enhances the security so that people don't forget test OTPs
accidentally.

Incidentally this makes it possible to use phone number logins when
developing locally.
  • Loading branch information
hf authored Aug 1, 2023
1 parent 669ce97 commit 2fb0cf5
Show file tree
Hide file tree
Showing 3 changed files with 77 additions and 24 deletions.
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)) {
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

0 comments on commit 2fb0cf5

Please sign in to comment.