From edd7f32efc3d725b3f12512d21a7f3d1e21c98ab Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Mon, 12 Sep 2022 14:50:37 +0800 Subject: [PATCH 1/4] refactor: use custom HttpClient interface --- api/sms_provider/sms_provider.go | 8 ++++++++ api/sms_provider/twilio.go | 2 -- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/api/sms_provider/sms_provider.go b/api/sms_provider/sms_provider.go index 81cc02e2c3..fbaf2895a1 100644 --- a/api/sms_provider/sms_provider.go +++ b/api/sms_provider/sms_provider.go @@ -3,6 +3,7 @@ package sms_provider import ( "fmt" "log" + "net/http" "os" "time" @@ -10,6 +11,7 @@ import ( ) var defaultTimeout time.Duration = time.Second * 10 +var client HttpClient func init() { timeoutStr := os.Getenv("GOTRUE_INTERNAL_HTTP_TIMEOUT") @@ -20,8 +22,14 @@ func init() { defaultTimeout = timeout } } + client = &http.Client{ + Timeout: defaultTimeout, + } } +type HttpClient interface { + Do(req *http.Request) (*http.Response, error) +} type SmsProvider interface { SendSms(phone, message string) error } diff --git a/api/sms_provider/twilio.go b/api/sms_provider/twilio.go index ce29db825e..1426ba57b3 100644 --- a/api/sms_provider/twilio.go +++ b/api/sms_provider/twilio.go @@ -61,8 +61,6 @@ func (t *TwilioProvider) SendSms(phone string, message string) error { "From": {t.Config.MessageServiceSid}, "Body": {message}, } - - client := &http.Client{Timeout: defaultTimeout} r, err := http.NewRequest("POST", t.APIPath, strings.NewReader(body.Encode())) if err != nil { return err From e1beb1941e3a3ee399197df50e986d81feb029cd Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Mon, 12 Sep 2022 14:50:53 +0800 Subject: [PATCH 2/4] test: add test for twilio SendSms --- api/sms_provider/sms_provider_test.go | 83 +++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 api/sms_provider/sms_provider_test.go diff --git a/api/sms_provider/sms_provider_test.go b/api/sms_provider/sms_provider_test.go new file mode 100644 index 0000000000..6eda2a139d --- /dev/null +++ b/api/sms_provider/sms_provider_test.go @@ -0,0 +1,83 @@ +package sms_provider + +import ( + "bytes" + "encoding/json" + "io" + "net/http" + "testing" + + "github.com/netlify/gotrue/conf" + "github.com/stretchr/testify/mock" + "github.com/stretchr/testify/require" + "github.com/stretchr/testify/suite" +) + +var handleApiRequest func(*http.Request) (*http.Response, error) + +type SmsProviderTestSuite struct { + suite.Suite + Config *conf.GlobalConfiguration + Client HttpClient +} + +type MockHttpClient struct { + mock.Mock +} + +func (m *MockHttpClient) Do(req *http.Request) (*http.Response, error) { + return handleApiRequest(req) +} + +func TestSmsProvider(t *testing.T) { + + ts := &SmsProviderTestSuite{ + Config: &conf.GlobalConfiguration{ + Sms: conf.SmsProviderConfiguration{ + Twilio: conf.TwilioProviderConfiguration{ + AccountSid: "test_account_sid", + AuthToken: "test_auth_token", + MessageServiceSid: "test_message_service_id", + }, + }, + }, + Client: &MockHttpClient{}, + } + suite.Run(t, ts) +} + +func (ts *SmsProviderTestSuite) TestTwilioSendSms() { + twilioProvider, err := NewTwilioProvider(ts.Config.Sms.Twilio) + require.NoError(ts.T(), err) + + client = ts.Client + + handleApiRequest = func(req *http.Request) (*http.Response, error) { + // check authorization header + require.Contains(ts.T(), req.Header, "Authorization") + + // check request body sent + require.NoError(ts.T(), req.ParseForm()) + require.Contains(ts.T(), req.Form, "To") + require.Contains(ts.T(), req.Form, "Channel") + require.Contains(ts.T(), req.Form, "From") + require.Contains(ts.T(), req.Form, "Body") + + b, err := json.Marshal(&SmsStatus{ + To: req.Form["To"][0], + From: req.Form["From"][0], + Status: "sent", + Body: req.Form["Body"][0], + }) + require.NoError(ts.T(), err) + respBody := io.NopCloser(bytes.NewReader(b)) + + return &http.Response{ + StatusCode: http.StatusOK, + Body: respBody, + }, nil + } + + err = twilioProvider.SendSms("123456789", "This is a test message") + require.NoError(ts.T(), err) +} From 85397467ce25ec8eace6b97b626902b4f6b99eb0 Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Fri, 16 Sep 2022 14:33:39 +0800 Subject: [PATCH 3/4] use gock for mocking requests --- api/sms_provider/sms_provider.go | 8 -- api/sms_provider/sms_provider_test.go | 156 ++++++++++++++++++++------ api/sms_provider/textlocal.go | 2 +- api/sms_provider/twilio.go | 1 + go.mod | 2 + go.sum | 5 + 6 files changed, 131 insertions(+), 43 deletions(-) diff --git a/api/sms_provider/sms_provider.go b/api/sms_provider/sms_provider.go index fbaf2895a1..81cc02e2c3 100644 --- a/api/sms_provider/sms_provider.go +++ b/api/sms_provider/sms_provider.go @@ -3,7 +3,6 @@ package sms_provider import ( "fmt" "log" - "net/http" "os" "time" @@ -11,7 +10,6 @@ import ( ) var defaultTimeout time.Duration = time.Second * 10 -var client HttpClient func init() { timeoutStr := os.Getenv("GOTRUE_INTERNAL_HTTP_TIMEOUT") @@ -22,14 +20,8 @@ func init() { defaultTimeout = timeout } } - client = &http.Client{ - Timeout: defaultTimeout, - } } -type HttpClient interface { - Do(req *http.Request) (*http.Response, error) -} type SmsProvider interface { SendSms(phone, message string) error } diff --git a/api/sms_provider/sms_provider_test.go b/api/sms_provider/sms_provider_test.go index 6eda2a139d..fdc023c019 100644 --- a/api/sms_provider/sms_provider_test.go +++ b/api/sms_provider/sms_provider_test.go @@ -1,16 +1,16 @@ package sms_provider import ( - "bytes" - "encoding/json" - "io" + "encoding/base64" "net/http" + "net/url" "testing" "github.com/netlify/gotrue/conf" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" + "gopkg.in/h2non/gock.v1" ) var handleApiRequest func(*http.Request) (*http.Response, error) @@ -18,7 +18,6 @@ var handleApiRequest func(*http.Request) (*http.Response, error) type SmsProviderTestSuite struct { suite.Suite Config *conf.GlobalConfiguration - Client HttpClient } type MockHttpClient struct { @@ -30,7 +29,6 @@ func (m *MockHttpClient) Do(req *http.Request) (*http.Response, error) { } func TestSmsProvider(t *testing.T) { - ts := &SmsProviderTestSuite{ Config: &conf.GlobalConfiguration{ Sms: conf.SmsProviderConfiguration{ @@ -39,45 +37,135 @@ func TestSmsProvider(t *testing.T) { AuthToken: "test_auth_token", MessageServiceSid: "test_message_service_id", }, + Messagebird: conf.MessagebirdProviderConfiguration{ + AccessKey: "test_access_key", + Originator: "test_originator", + }, + Vonage: conf.VonageProviderConfiguration{ + ApiKey: "test_api_key", + ApiSecret: "test_api_secret", + From: "test_from", + }, + Textlocal: conf.TextlocalProviderConfiguration{ + ApiKey: "test_api_key", + Sender: "test_sender", + }, }, }, - Client: &MockHttpClient{}, } suite.Run(t, ts) } func (ts *SmsProviderTestSuite) TestTwilioSendSms() { - twilioProvider, err := NewTwilioProvider(ts.Config.Sms.Twilio) + defer gock.Off() + provider, err := NewTwilioProvider(ts.Config.Sms.Twilio) + require.NoError(ts.T(), err) + + twilioProvider, ok := provider.(*TwilioProvider) + require.Equal(ts.T(), true, ok) + + phone := "123456789" + message := "This is the sms code: 123456" + + body := url.Values{ + "To": {"+" + phone}, + "Channel": {"sms"}, + "From": {twilioProvider.Config.MessageServiceSid}, + "Body": {message}, + } + + gock.New(twilioProvider.APIPath).Post(""). + MatchHeader("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(twilioProvider.Config.AccountSid+":"+twilioProvider.Config.AuthToken))). + MatchType("url").BodyString(body.Encode()). + Reply(200).JSON(SmsStatus{ + To: "+" + phone, + From: twilioProvider.Config.MessageServiceSid, + Status: "sent", + Body: message, + }) + + err = twilioProvider.SendSms(phone, message) + require.NoError(ts.T(), err) +} + +func (ts *SmsProviderTestSuite) TestMessagebirdSendSms() { + defer gock.Off() + provider, err := NewMessagebirdProvider(ts.Config.Sms.Messagebird) + require.NoError(ts.T(), err) + + messagebirdProvider, ok := provider.(*MessagebirdProvider) + require.Equal(ts.T(), true, ok) + + phone := "123456789" + message := "This is the sms code: 123456" + body := url.Values{ + "originator": {messagebirdProvider.Config.Originator}, + "body": {message}, + "recipients": {phone}, + "type": {"sms"}, + "datacoding": {"unicode"}, + } + gock.New(messagebirdProvider.APIPath).Post("").MatchHeader("Authorization", "AccessKey "+messagebirdProvider.Config.AccessKey).MatchType("url").BodyString(body.Encode()).Reply(200).JSON(MessagebirdResponse{ + Recipients: MessagebirdResponseRecipients{ + TotalSentCount: 1, + }, + }) + + err = messagebirdProvider.SendSms(phone, message) + require.NoError(ts.T(), err) +} + +func (ts *SmsProviderTestSuite) TestVonageSendSms() { + defer gock.Off() + provider, err := NewVonageProvider(ts.Config.Sms.Vonage) + require.NoError(ts.T(), err) + + vonageProvider, ok := provider.(*VonageProvider) + require.Equal(ts.T(), true, ok) + + phone := "123456789" + message := "This is the sms code: 123456" + + body := url.Values{ + "from": {vonageProvider.Config.From}, + "to": {phone}, + "text": {message}, + "api_key": {vonageProvider.Config.ApiKey}, + "api_secret": {vonageProvider.Config.ApiSecret}, + } + + gock.New(vonageProvider.APIPath).Post("").MatchType("url").BodyString(body.Encode()).Reply(200).JSON(VonageResponse{ + Messages: []VonageResponseMessage{ + {Status: "0"}, + }, + }) + + err = vonageProvider.SendSms(phone, message) require.NoError(ts.T(), err) +} - client = ts.Client - - handleApiRequest = func(req *http.Request) (*http.Response, error) { - // check authorization header - require.Contains(ts.T(), req.Header, "Authorization") - - // check request body sent - require.NoError(ts.T(), req.ParseForm()) - require.Contains(ts.T(), req.Form, "To") - require.Contains(ts.T(), req.Form, "Channel") - require.Contains(ts.T(), req.Form, "From") - require.Contains(ts.T(), req.Form, "Body") - - b, err := json.Marshal(&SmsStatus{ - To: req.Form["To"][0], - From: req.Form["From"][0], - Status: "sent", - Body: req.Form["Body"][0], - }) - require.NoError(ts.T(), err) - respBody := io.NopCloser(bytes.NewReader(b)) - - return &http.Response{ - StatusCode: http.StatusOK, - Body: respBody, - }, nil +func (ts *SmsProviderTestSuite) TestTextLocalSendSms() { + defer gock.Off() + provider, err := NewTextlocalProvider(ts.Config.Sms.Textlocal) + require.NoError(ts.T(), err) + + textlocalProvider, ok := provider.(*TextlocalProvider) + require.Equal(ts.T(), true, ok) + + phone := "123456789" + message := "This is the sms code: 123456" + body := url.Values{ + "sender": {textlocalProvider.Config.Sender}, + "apikey": {textlocalProvider.Config.ApiKey}, + "message": {message}, + "numbers": {phone}, } - err = twilioProvider.SendSms("123456789", "This is a test message") + gock.New(textlocalProvider.APIPath).Post("").MatchType("url").BodyString(body.Encode()).Reply(200).JSON(TextlocalResponse{ + Status: "success", + Errors: []TextlocalError{}, + }) + + err = textlocalProvider.SendSms(phone, message) require.NoError(ts.T(), err) } diff --git a/api/sms_provider/textlocal.go b/api/sms_provider/textlocal.go index ac0f4b164a..29f88bdca5 100644 --- a/api/sms_provider/textlocal.go +++ b/api/sms_provider/textlocal.go @@ -71,7 +71,7 @@ func (t *TextlocalProvider) SendSms(phone string, message string) error { return derr } - if len(resp.Errors) == 0 { + if len(resp.Errors) > 0 { return errors.New("Textlocal error: Internal Error") } diff --git a/api/sms_provider/twilio.go b/api/sms_provider/twilio.go index 1426ba57b3..55ba9a8ce9 100644 --- a/api/sms_provider/twilio.go +++ b/api/sms_provider/twilio.go @@ -61,6 +61,7 @@ func (t *TwilioProvider) SendSms(phone string, message string) error { "From": {t.Config.MessageServiceSid}, "Body": {message}, } + client := &http.Client{Timeout: defaultTimeout} r, err := http.NewRequest("POST", t.APIPath, strings.NewReader(body.Encode())) if err != nil { return err diff --git a/go.mod b/go.mod index 8919f73648..7f077e284e 100644 --- a/go.mod +++ b/go.mod @@ -57,6 +57,7 @@ require ( github.com/google/go-cmp v0.5.2 // indirect github.com/gorilla/context v1.1.1 // indirect github.com/gorilla/css v1.0.0 // indirect + github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 // indirect github.com/inconshreveable/mousetrap v1.0.0 // indirect github.com/jackc/chunkreader/v2 v2.0.1 // indirect github.com/jackc/pgio v1.0.0 // indirect @@ -93,6 +94,7 @@ require ( google.golang.org/protobuf v1.25.0 // indirect gopkg.in/alexcesaro/quotedprintable.v3 v3.0.0-20150716171945-2caba252f4dc // indirect gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect + gopkg.in/h2non/gock.v1 v1.1.2 // indirect gopkg.in/square/go-jose.v2 v2.5.1 // indirect gopkg.in/yaml.v2 v2.4.0 // indirect gopkg.in/yaml.v3 v3.0.0 // indirect diff --git a/go.sum b/go.sum index ef19630dc3..3e4fc9df13 100644 --- a/go.sum +++ b/go.sum @@ -235,6 +235,8 @@ github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/ad github.com/grpc-ecosystem/go-grpc-middleware v1.0.0/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs= github.com/grpc-ecosystem/go-grpc-prometheus v1.2.0/go.mod h1:8NvIoxWQoOIhqOTXgfV/d3M/q6VIi02HzZEHgUlZvzk= github.com/grpc-ecosystem/grpc-gateway v1.9.0/go.mod h1:vNeuVxBJEsws4ogUvrchl83t/GYV9WGTSLVdBhOQFDY= +github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542 h1:2VTzZjLZBgl62/EtslCrtky5vbi9dd7HrQPQIx6wqiw= +github.com/h2non/parth v0.0.0-20190131123155-b4df798d6542/go.mod h1:Ow0tF8D4Kplbc8s8sSb3V2oUCygFHVp8gC3Dn6U4MNI= github.com/hashicorp/consul/api v1.1.0/go.mod h1:VmuI/Lkw1nC05EYQWNKwWGbkg+FbDBtguAZLlVdkD9Q= github.com/hashicorp/consul/sdk v0.1.1/go.mod h1:VKf9jXwCTEY1QZP2MOLRhb5i/I/ssyNV1vwHyQBF0x8= github.com/hashicorp/errwrap v1.0.0/go.mod h1:YH+1FKiLXxHSkmPseP+kNlulaMuP3n2brvKWEqk/Jc4= @@ -421,6 +423,7 @@ github.com/nats-io/nkeys v0.0.2/go.mod h1:dab7URMsZm6Z/jp9Z5UGa87Uutgc2mVpXLC4B7 github.com/nats-io/nuid v1.0.1/go.mod h1:19wcPz3Ph3q0Jbyiqsd0kePYG7A95tJPxeL+1OSON2c= github.com/nats-io/stan.go v0.4.5/go.mod h1:Ji7mK6gRZJSH1nc3ZJH6vi7zn/QnZhpR9Arm4iuzsUQ= github.com/nats-io/stan.go v0.5.0/go.mod h1:dYqB+vMN3C2F9pT1FRQpg9eHbjPj6mP0yYuyBNuXHZE= +github.com/nbio/st v0.0.0-20140626010706-e9e8d9816f32/go.mod h1:9wM+0iRr9ahx58uYLpLIr5fm8diHn0JbqRycJi6w0Ms= github.com/netlify/mailme v1.1.1 h1:S/ANl+Hy/EIoJUgGiLJYYLZJ2QOTG452R73qTQudMns= github.com/netlify/mailme v1.1.1/go.mod h1:8g03BJmU+ps7ma5vcH+t8aMtaicQTMX3ffP7RJ8xY8g= github.com/netlify/netlify-commons v0.32.0 h1:IgpqedBa6aFrc+daRgGZ+SmU9eBXlDXzKSAjevWmshM= @@ -869,6 +872,8 @@ gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/gomail.v2 v2.0.0-20150902115704-41f357289737/go.mod h1:LRQQ+SO6ZHR7tOkpBDuZnXENFzX8qRjMDMyPD6BRkCw= gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df h1:n7WqCuqOuCbNr617RXOY0AWRXxgwEyPp2z+p0+hgMuE= gopkg.in/gomail.v2 v2.0.0-20160411212932-81ebce5c23df/go.mod h1:LRQQ+SO6ZHR7tOkpBDuZnXENFzX8qRjMDMyPD6BRkCw= +gopkg.in/h2non/gock.v1 v1.1.2 h1:jBbHXgGBK/AoPVfJh5x4r/WxIrElvbLel8TCZkkZJoY= +gopkg.in/h2non/gock.v1 v1.1.2/go.mod h1:n7UGz/ckNChHiK05rDoiC4MYSunEC/lyaUm2WWaDva0= gopkg.in/inconshreveable/log15.v2 v2.0.0-20180818164646-67afb5ed74ec/go.mod h1:aPpfJ7XW+gOuirDoZ8gHhLh3kZ1B08FtV2bbmy7Jv3s= gopkg.in/ini.v1 v1.51.0/go.mod h1:pNLf8WUiyNEtQjuu5G5vTm06TEv9tsIgeAvK8hOrP4k= gopkg.in/resty.v1 v1.12.0/go.mod h1:mDo4pnntr5jdWRML875a/NmxYqAlA73dVijT2AXvQQo= From e8c5455ade202f01311589931ceabc62dc4b772f Mon Sep 17 00:00:00 2001 From: Kang Ming Date: Fri, 16 Sep 2022 14:56:22 +0800 Subject: [PATCH 4/4] test twilio error edge cases --- api/sms_provider/sms_provider_test.go | 67 ++++++++++++++++++++++----- 1 file changed, 56 insertions(+), 11 deletions(-) diff --git a/api/sms_provider/sms_provider_test.go b/api/sms_provider/sms_provider_test.go index fdc023c019..b13bd9cd0f 100644 --- a/api/sms_provider/sms_provider_test.go +++ b/api/sms_provider/sms_provider_test.go @@ -2,6 +2,7 @@ package sms_provider import ( "encoding/base64" + "fmt" "net/http" "net/url" "testing" @@ -74,18 +75,62 @@ func (ts *SmsProviderTestSuite) TestTwilioSendSms() { "Body": {message}, } - gock.New(twilioProvider.APIPath).Post(""). - MatchHeader("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(twilioProvider.Config.AccountSid+":"+twilioProvider.Config.AuthToken))). - MatchType("url").BodyString(body.Encode()). - Reply(200).JSON(SmsStatus{ - To: "+" + phone, - From: twilioProvider.Config.MessageServiceSid, - Status: "sent", - Body: message, - }) + cases := []struct { + Desc string + TwilioResponse *gock.Response + ExpectedError error + }{ + { + Desc: "Successfully sent sms", + TwilioResponse: gock.New(twilioProvider.APIPath).Post(""). + MatchHeader("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(twilioProvider.Config.AccountSid+":"+twilioProvider.Config.AuthToken))). + MatchType("url").BodyString(body.Encode()). + Reply(200).JSON(SmsStatus{ + To: "+" + phone, + From: twilioProvider.Config.MessageServiceSid, + Status: "sent", + Body: message, + }), + ExpectedError: nil, + }, + { + Desc: "Sms status is failed / undelivered", + TwilioResponse: gock.New(twilioProvider.APIPath).Post(""). + MatchHeader("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(twilioProvider.Config.AccountSid+":"+twilioProvider.Config.AuthToken))). + MatchType("url").BodyString(body.Encode()). + Reply(200).JSON(SmsStatus{ + ErrorMessage: "failed to send sms", + ErrorCode: "401", + Status: "failed", + }), + ExpectedError: fmt.Errorf("twilio error: %v %v", "failed to send sms", "401"), + }, + { + Desc: "Non-2xx status code returned", + TwilioResponse: gock.New(twilioProvider.APIPath).Post(""). + MatchHeader("Authorization", "Basic "+base64.StdEncoding.EncodeToString([]byte(twilioProvider.Config.AccountSid+":"+twilioProvider.Config.AuthToken))). + MatchType("url").BodyString(body.Encode()). + Reply(500).JSON(twilioErrResponse{ + Code: 500, + Message: "Internal server error", + MoreInfo: "error", + Status: 500, + }), + ExpectedError: &twilioErrResponse{ + Code: 500, + Message: "Internal server error", + MoreInfo: "error", + Status: 500, + }, + }, + } - err = twilioProvider.SendSms(phone, message) - require.NoError(ts.T(), err) + for _, c := range cases { + ts.Run(c.Desc, func() { + err = twilioProvider.SendSms(phone, message) + require.Equal(ts.T(), c.ExpectedError, err) + }) + } } func (ts *SmsProviderTestSuite) TestMessagebirdSendSms() {