Move Twilio to twilio/ package

docs/releases.md

@@ -2013,6 +2013,10 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release
 
 * Allow logging in with your verified primary email address (in addition to your username), so a password reset no longer leaves you unable to sign in when you only remember the email you signed up with
 
+**Bug fixes + maintenance:**
+
+* Fix Twilio phone calls and phone number verifications failing silently when Twilio rejected the request, and move the Twilio integration into its own `twilio` package
+
 ### ntfy Android v1.25.2 (UNRELEASED)
 
 This release makes the "connection lost" alert configurable and turns it off by default. Folks did not like it and many reached out

server/server.go

@@ -38,6 +38,7 @@ import (
 	"heckel.io/ntfy/v2/message"
 	"heckel.io/ntfy/v2/model"
 	"heckel.io/ntfy/v2/payments"
+	"heckel.io/ntfy/v2/twilio"
 	"heckel.io/ntfy/v2/user"
 	"heckel.io/ntfy/v2/util"
 	"heckel.io/ntfy/v2/webpush"
@@ -58,7 +59,7 @@ type Server struct {
 	topics            map[string]*topic
 	visitors          map[string]*visitor // ip:<ip> or user:<user>
 	firebaseClient    *firebaseClient
-	twilio            *twilioClient
+	twilio            *twilio.Client
 	messages          int64                               // Total number of messages (persisted if messageCache enabled)
 	messagesHistory   []int64                             // Last n values of the messages counter, used to determine rate
 	userManager       *user.Manager                       // Might be nil!
@@ -252,6 +253,16 @@ func New(conf *Config) (*Server, error) {
 	if err != nil {
 		return nil, err
 	}
+	twilioClient := twilio.NewClient(&twilio.Config{
+		Account:       conf.TwilioAccount,
+		AuthToken:     conf.TwilioAuthToken,
+		PhoneNumber:   conf.TwilioPhoneNumber,
+		CallsBaseURL:  conf.TwilioCallsBaseURL,
+		VerifyBaseURL: conf.TwilioVerifyBaseURL,
+		VerifyService: conf.TwilioVerifyService,
+		CallFormat:    conf.TwilioCallFormat,
+		BuildVersion:  conf.BuildVersion,
+	})
 	var userManager *user.Manager
 	if conf.AuthFile != "" || pool != nil {
 		authConfig := &user.Config{
@@ -298,7 +309,7 @@ func New(conf *Config) (*Server, error) {
 		webPush:         wp,
 		attachment:      attachmentStore,
 		firebaseClient:  firebaseClient,
-		twilio:          newTwilioClient(conf, userManager),
+		twilio:          twilioClient,
 		mailer:          sender,
 		topics:          topics,
 		userManager:     userManager,
@@ -844,7 +855,7 @@ func (s *Server) handlePublishInternal(r *http.Request, v *visitor) (*model.Mess
 	}
 	if call != "" {
 		var httpErr *errHTTP
-		call, httpErr = s.twilio.convertPhoneNumber(v.User(), call)
+		call, httpErr = s.convertPhoneNumber(v.User(), call)
 		if httpErr != nil {
 			return nil, httpErr.With(t)
 		} else if !vrate.CallAllowed() {
@@ -893,7 +904,7 @@ func (s *Server) handlePublishInternal(r *http.Request, v *visitor) (*model.Mess
 			go s.sendEmail(v, m, email)
 		}
 		if s.config.TwilioAccount != "" && call != "" {
-			go s.twilio.callPhone(v, r, m, call)
+			go s.callPhone(v, m, call)
 		}
 		if s.config.UpstreamBaseURL != "" && !unifiedpush { // UP messages are not sent to upstream
 			go s.forwardPollRequest(v, m)

server/server_account.go

@@ -10,6 +10,7 @@ import (
 
 	"heckel.io/ntfy/v2/log"
 	"heckel.io/ntfy/v2/model"
+	"heckel.io/ntfy/v2/twilio"
 	"heckel.io/ntfy/v2/user"
 	"heckel.io/ntfy/v2/util"
 )
@@ -631,7 +632,7 @@ func (s *Server) handleAccountPhoneNumberVerify(w http.ResponseWriter, r *http.R
 	}
 	// Actually add the unverified number, and send verification
 	logvr(v, r).Tag(tagAccount).Field("phone_number", req.Number).Debug("Sending phone number verification")
-	if err := s.twilio.verifyPhoneNumber(v, r, req.Number, req.Channel); err != nil {
+	if err := s.twilio.Verify(req.Number, req.Channel); err != nil {
 		return err
 	}
 	return s.writeJSON(w, newSuccessResponse())
@@ -646,7 +647,10 @@ func (s *Server) handleAccountPhoneNumberAdd(w http.ResponseWriter, r *http.Requ
 	if !phoneNumberRegex.MatchString(req.Number) {
 		return errHTTPBadRequestPhoneNumberInvalid
 	}
-	if err := s.twilio.verifyPhoneNumberCheck(v, r, req.Number, req.Code); err != nil {
+	if err := s.twilio.CheckVerify(req.Number, req.Code); err != nil {
+		if errors.Is(err, twilio.ErrVerificationExpired) {
+			return errHTTPGonePhoneVerificationExpired
+		}
 		return err
 	}
 	logvr(v, r).Tag(tagAccount).Field("phone_number", req.Number).Debug("Adding phone number as verified")

server/server_twilio.go

@@ -1,77 +1,20 @@
 package server
 
 import (
-	"bytes"
-	"encoding/xml"
-	"fmt"
-	"io"
-	"net/http"
-	"net/url"
-	"strings"
-	"text/template"
-
-	"heckel.io/ntfy/v2/log"
 	"heckel.io/ntfy/v2/model"
+	"heckel.io/ntfy/v2/twilio"
 	"heckel.io/ntfy/v2/user"
 	"heckel.io/ntfy/v2/util"
 )
 
-// twilioClient talks to the Twilio API to make phone calls (for the "Call" feature) and to verify
-// phone numbers. It holds the Twilio configuration and the user manager (used to look up a user's
-// verified phone numbers), so that this functionality is decoupled from the main Server.
-type twilioClient struct {
-	config      *Config
-	userManager *user.Manager // May be nil!
-}
-
-func newTwilioClient(conf *Config, userManager *user.Manager) *twilioClient {
-	return &twilioClient{
-		config:      conf,
-		userManager: userManager,
-	}
-}
-
-// defaultTwilioCallFormatTemplate is the default TwiML template used for Twilio calls.
-// It can be overridden in the server configuration's twilio-call-format field.
-//
-// The format uses Go template syntax with the following fields:
-// {{.Topic}}, {{.Title}}, {{.Message}}, {{.Priority}}, {{.Tags}}, {{.Sender}}
-// String fields are automatically XML-escaped.
-var defaultTwilioCallFormatTemplate = template.Must(template.New("twiml").Parse(`
-<Response>
-	<Pause length="1"/>
-	<Say loop="3">
-		You have a message from notify on topic {{.Topic}}. Message:
-		<break time="1s"/>
-		{{.Message}}
-		<break time="1s"/>
-		End of message.
-		<break time="1s"/>
-		This message was sent by user {{.Sender}}. It will be repeated three times.
-		To unsubscribe from calls like this, remove your phone number in the notify web app.
-		<break time="3s"/>
-	</Say>
-	<Say>Goodbye.</Say>
-</Response>`))
-
-// twilioCallData holds the data passed to the Twilio call format template
-type twilioCallData struct {
-	Topic    string
-	Title    string
-	Message  string
-	Priority int
-	Tags     []string
-	Sender   string
-}
-
 // convertPhoneNumber checks if the given phone number is verified for the given user, and if so, returns the verified
 // phone number. It also converts a boolean string ("yes", "1", "true") to the first verified phone number.
 // If the user is anonymous, it will return an error.
-func (c *twilioClient) convertPhoneNumber(u *user.User, phoneNumber string) (string, *errHTTP) {
+func (s *Server) convertPhoneNumber(u *user.User, phoneNumber string) (string, *errHTTP) {
 	if u == nil {
 		return "", errHTTPBadRequestAnonymousCallsNotAllowed
 	}
-	phoneNumbers, err := c.userManager.PhoneNumbers(u.ID)
+	phoneNumbers, err := s.userManager.PhoneNumbers(u.ID)
 	if err != nil {
 		return "", errHTTPInternalError
 	} else if len(phoneNumbers) == 0 {
@@ -87,139 +30,24 @@ func (c *twilioClient) convertPhoneNumber(u *user.User, phoneNumber string) (str
 
 // callPhone calls the Twilio API to make a phone call to the given phone number, using the given message.
 // Failures will be logged, but not returned to the caller.
-func (c *twilioClient) callPhone(v *visitor, r *http.Request, m *model.Message, to string) {
+func (s *Server) callPhone(v *visitor, m *model.Message, to string) {
 	u, sender := v.User(), m.Sender.String()
 	if u != nil {
 		sender = u.Name
 	}
-	tmpl := defaultTwilioCallFormatTemplate
+	logvm(v, m).Tag(tagTwilio).Field("twilio_to", to).Info("Making phone call to %s", to)
-	if c.config.TwilioCallFormat != nil {
+	err := s.twilio.Call(to, &twilio.CallData{
-		tmpl = c.config.TwilioCallFormat
+		Topic:    m.Topic,
-	}
+		Title:    m.Title,
-	tags := make([]string, len(m.Tags))
+		Message:  m.Message,
-	for i, tag := range m.Tags {
-		tags[i] = xmlEscapeText(tag)
-	}
-	templateData := &twilioCallData{
-		Topic:    xmlEscapeText(m.Topic),
-		Title:    xmlEscapeText(m.Title),
-		Message:  xmlEscapeText(m.Message),
 		Priority: m.Priority,
-		Tags:     tags,
+		Tags:     m.Tags,
-		Sender:   xmlEscapeText(sender),
+		Sender:   sender,
-	}
+	})
-	var bodyBuf bytes.Buffer
-	if err := tmpl.Execute(&bodyBuf, templateData); err != nil {
-		logvrm(v, r, m).Tag(tagTwilio).Err(err).Warn("Error executing Twilio call format template")
-		minc(metricCallsMadeFailure)
-		return
-	}
-	body := bodyBuf.String()
-	data := url.Values{}
-	data.Set("From", c.config.TwilioPhoneNumber)
-	data.Set("To", to)
-	data.Set("Twiml", body)
-	ev := logvrm(v, r, m).Tag(tagTwilio).Field("twilio_to", to).FieldIf("twilio_body", body, log.TraceLevel).Debug("Sending Twilio request")
-	response, err := c.callPhoneInternal(data)
 	if err != nil {
-		ev.Field("twilio_response", response).Err(err).Warn("Error sending Twilio request")
+		logvm(v, m).Tag(tagTwilio).Field("twilio_to", to).Err(err).Warn("Unable to call phone %s: %v", to, err.Error())
 		minc(metricCallsMadeFailure)
 		return
 	}
-	ev.FieldIf("twilio_response", response, log.TraceLevel).Debug("Received successful Twilio response")
 	minc(metricCallsMadeSuccess)
 }
-
-func (c *twilioClient) callPhoneInternal(data url.Values) (string, error) {
-	requestURL := fmt.Sprintf("%s/2010-04-01/Accounts/%s/Calls.json", c.config.TwilioCallsBaseURL, c.config.TwilioAccount)
-	req, err := http.NewRequest(http.MethodPost, requestURL, strings.NewReader(data.Encode()))
-	if err != nil {
-		return "", err
-	}
-	req.Header.Set("User-Agent", "ntfy/"+c.config.BuildVersion)
-	req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
-	req.Header.Set("Authorization", util.BasicAuth(c.config.TwilioAccount, c.config.TwilioAuthToken))
-	resp, err := http.DefaultClient.Do(req)
-	if err != nil {
-		return "", err
-	}
-	response, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return "", err
-	}
-	return string(response), nil
-}
-
-func (c *twilioClient) verifyPhoneNumber(v *visitor, r *http.Request, phoneNumber, channel string) error {
-	ev := logvr(v, r).Tag(tagTwilio).Field("twilio_to", phoneNumber).Field("twilio_channel", channel).Debug("Sending phone verification")
-	data := url.Values{}
-	data.Set("To", phoneNumber)
-	data.Set("Channel", channel)
-	requestURL := fmt.Sprintf("%s/v2/Services/%s/Verifications", c.config.TwilioVerifyBaseURL, c.config.TwilioVerifyService)
-	req, err := http.NewRequest(http.MethodPost, requestURL, strings.NewReader(data.Encode()))
-	if err != nil {
-		return err
-	}
-	req.Header.Set("User-Agent", "ntfy/"+c.config.BuildVersion)
-	req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
-	req.Header.Set("Authorization", util.BasicAuth(c.config.TwilioAccount, c.config.TwilioAuthToken))
-	resp, err := http.DefaultClient.Do(req)
-	if err != nil {
-		return err
-	}
-	response, err := io.ReadAll(resp.Body)
-	if err != nil {
-		ev.Err(err).Warn("Error sending Twilio phone verification request")
-		return err
-	}
-	ev.FieldIf("twilio_response", string(response), log.TraceLevel).Debug("Received Twilio phone verification response")
-	return nil
-}
-
-func (c *twilioClient) verifyPhoneNumberCheck(v *visitor, r *http.Request, phoneNumber, code string) error {
-	ev := logvr(v, r).Tag(tagTwilio).Field("twilio_to", phoneNumber).Debug("Checking phone verification")
-	data := url.Values{}
-	data.Set("To", phoneNumber)
-	data.Set("Code", code)
-	requestURL := fmt.Sprintf("%s/v2/Services/%s/VerificationCheck", c.config.TwilioVerifyBaseURL, c.config.TwilioVerifyService)
-	req, err := http.NewRequest(http.MethodPost, requestURL, strings.NewReader(data.Encode()))
-	if err != nil {
-		return err
-	}
-	req.Header.Set("User-Agent", "ntfy/"+c.config.BuildVersion)
-	req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
-	req.Header.Set("Authorization", util.BasicAuth(c.config.TwilioAccount, c.config.TwilioAuthToken))
-	resp, err := http.DefaultClient.Do(req)
-	if err != nil {
-		return err
-	} else if resp.StatusCode != http.StatusOK {
-		if ev.IsTrace() {
-			response, err := io.ReadAll(resp.Body)
-			if err != nil {
-				return err
-			}
-			ev.Field("twilio_response", string(response))
-		}
-		ev.Warn("Twilio phone verification failed with status code %d", resp.StatusCode)
-		if resp.StatusCode == http.StatusNotFound {
-			return errHTTPGonePhoneVerificationExpired
-		}
-		return errHTTPInternalError
-	}
-	response, err := io.ReadAll(resp.Body)
-	if err != nil {
-		return err
-	}
-	if ev.IsTrace() {
-		ev.Field("twilio_response", string(response)).Trace("Received successful Twilio phone verification response")
-	} else if ev.IsDebug() {
-		ev.Debug("Received successful Twilio phone verification response")
-	}
-	return nil
-}
-
-func xmlEscapeText(text string) string {
-	var buf bytes.Buffer
-	_ = xml.EscapeText(&buf, []byte(text))
-	return buf.String()
-}

twilio/client.go

@@ -0,0 +1,169 @@
+// Package twilio talks to the Twilio API to make phone calls (for the "Call" feature) and to
+// verify phone numbers. It holds the Twilio configuration, so that this functionality is
+// decoupled from the ntfy server.
+package twilio
+
+import (
+	"bytes"
+	"fmt"
+	"io"
+	"net/http"
+	"net/url"
+	"strings"
+
+	"heckel.io/ntfy/v2/log"
+	"heckel.io/ntfy/v2/util"
+)
+
+const (
+	tagTwilio = "twilio"
+)
+
+// Client is the Twilio API client
+type Client struct {
+	config *Config
+}
+
+// NewClient creates a new Twilio Client with the given config
+func NewClient(config *Config) *Client {
+	return &Client{config: config}
+}
+
+// Call calls the Twilio API to make a phone call to the given phone number, using the given data
+func (c *Client) Call(to string, data *CallData) error {
+	tmpl := defaultCallFormatTemplate
+	if c.config.CallFormat != nil {
+		tmpl = c.config.CallFormat
+	}
+	var bodyBuf bytes.Buffer
+	if err := tmpl.Execute(&bodyBuf, data.escaped()); err != nil {
+		log.Tag(tagTwilio).Err(err).Warn("Error executing Twilio call format template")
+		return err
+	}
+	body := bodyBuf.String()
+	form := url.Values{}
+	form.Set("From", c.config.PhoneNumber)
+	form.Set("To", to)
+	form.Set("Twiml", body)
+	ev := log.Tag(tagTwilio).
+		Field("twilio_to", to).
+		FieldIf("twilio_body", body, log.TraceLevel).
+		Debug("Sending Twilio request")
+	requestURL := fmt.Sprintf("%s/2010-04-01/Accounts/%s/Calls.json", c.config.CallsBaseURL, c.config.Account)
+	response, code, err := c.request(requestURL, form)
+	if err != nil {
+		ev.Field("twilio_response", response).Err(err).Warn("Error sending Twilio request")
+		return err
+	} else if !success(code) {
+		// Twilio rejects calls with a 4xx, e.g. for an invalid phone number, or if the account
+		// is out of funds. Without this check, a rejected call would be counted as a success.
+		ev.Field("twilio_status", code).Field("twilio_response", response).Warn("Twilio call failed with status code %d", code)
+		return err
+	}
+	ev.FieldIf("twilio_response", response, log.TraceLevel).Debug("Received successful Twilio response")
+	return nil
+}
+
+// Verify calls the Twilio Verify API to send a verification code to the given phone
+// number, via the given channel ("sms" or "call")
+func (c *Client) Verify(phoneNumber, channel string) error {
+	ev := log.Tag(tagTwilio).Field("twilio_to", phoneNumber).Field("twilio_channel", channel).Debug("Sending phone verification")
+	form := url.Values{}
+	form.Set("To", phoneNumber)
+	form.Set("Channel", channel)
+	requestURL := fmt.Sprintf("%s/v2/Services/%s/Verifications", c.config.VerifyBaseURL, c.config.VerifyService)
+	response, code, err := c.request(requestURL, form)
+	if err != nil {
+		ev.Err(err).Warn("Error sending Twilio phone verification request")
+		return err
+	} else if !success(code) {
+		// Without this check, a rejected verification would look like a success to the caller,
+		// and the user would be told to wait for an SMS that was never sent.
+		ev.Field("twilio_status", code).Field("twilio_response", response).Warn("Twilio phone verification request failed with status code %d", code)
+		return err
+	}
+	ev.FieldIf("twilio_response", response, log.TraceLevel).Debug("Received Twilio phone verification response")
+	return nil
+}
+
+// CheckVerify calls the Twilio Verify API to check the verification code for the given
+// phone number. It returns ErrVerificationExpired if the code has expired or never existed.
+func (c *Client) CheckVerify(phoneNumber, code string) error {
+	ev := log.Tag(tagTwilio).Field("twilio_to", phoneNumber).Debug("Checking phone verification")
+	form := url.Values{}
+	form.Set("To", phoneNumber)
+	form.Set("Code", code)
+	requestURL := fmt.Sprintf("%s/v2/Services/%s/VerificationCheck", c.config.VerifyBaseURL, c.config.VerifyService)
+	req, err := c.newRequest(requestURL, form)
+	if err != nil {
+		return err
+	}
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return err
+	}
+	defer resp.Body.Close()
+	if resp.StatusCode != http.StatusOK {
+		if ev.IsTrace() {
+			response, err := io.ReadAll(resp.Body)
+			if err != nil {
+				return err
+			}
+			ev.Field("twilio_response", string(response))
+		}
+		ev.Warn("Twilio phone verification failed with status code %d", resp.StatusCode)
+		if resp.StatusCode == http.StatusNotFound {
+			return ErrVerificationExpired
+		}
+		return fmt.Errorf("twilio phone verification failed with status code %d", resp.StatusCode)
+	}
+	response, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return err
+	}
+	if ev.IsTrace() {
+		ev.Field("twilio_response", string(response)).Trace("Received successful Twilio phone verification response")
+	} else if ev.IsDebug() {
+		ev.Debug("Received successful Twilio phone verification response")
+	}
+	return nil
+}
+
+// request POSTs the given form to the given Twilio API URL, and returns the raw response body
+// and status code. It does not treat a non-2xx status code as an error; that is up to the
+// caller. The response body is returned even if the request failed, so that it can be logged.
+func (c *Client) request(requestURL string, form url.Values) (string, int, error) {
+	req, err := c.newRequest(requestURL, form)
+	if err != nil {
+		return "", 0, err
+	}
+	resp, err := http.DefaultClient.Do(req)
+	if err != nil {
+		return "", 0, err
+	}
+	defer resp.Body.Close()
+	response, err := io.ReadAll(resp.Body)
+	if err != nil {
+		return "", resp.StatusCode, err
+	}
+	return string(response), resp.StatusCode, nil
+}
+
+// success reports whether the given HTTP status code indicates success. Note that the Twilio
+// Calls API returns 201 Created (not 200 OK) for a successfully queued call.
+func success(code int) bool {
+	return code >= 200 && code <= 299
+}
+
+// newRequest creates a form-encoded POST request against the Twilio API, with the auth and
+// User-Agent headers set
+func (c *Client) newRequest(requestURL string, form url.Values) (*http.Request, error) {
+	req, err := http.NewRequest(http.MethodPost, requestURL, strings.NewReader(form.Encode()))
+	if err != nil {
+		return nil, err
+	}
+	req.Header.Set("User-Agent", "ntfy/"+c.config.BuildVersion)
+	req.Header.Add("Content-Type", "application/x-www-form-urlencoded")
+	req.Header.Set("Authorization", util.BasicAuth(c.config.Account, c.config.AuthToken))
+	return req, nil
+}

twilio/client_test.go

@@ -0,0 +1,301 @@
+package twilio
+
+import (
+	"errors"
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"testing"
+	"text/template"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestClient_Call_Success(t *testing.T) {
+	var body string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		require.Equal(t, "/2010-04-01/Accounts/AC1234567890/Calls.json", r.URL.Path)
+		require.Equal(t, "Basic QUMxMjM0NTY3ODkwOkFBRUFBMTIzNDU2Nzg5MA==", r.Header.Get("Authorization"))
+		require.Equal(t, "application/x-www-form-urlencoded", r.Header.Get("Content-Type"))
+		require.Equal(t, "ntfy/1.2.3", r.Header.Get("User-Agent"))
+		b, err := io.ReadAll(r.Body)
+		require.Nil(t, err)
+		body = string(b)
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	require.Nil(t, c.Call("+11122233344", &CallData{Topic: "mytopic", Message: "hi there", Sender: "phil"}))
+
+	form, err := url.ParseQuery(body)
+	require.Nil(t, err)
+	require.Equal(t, "+1234567890", form.Get("From"))
+	require.Equal(t, "+11122233344", form.Get("To"))
+	require.Contains(t, form.Get("Twiml"), "You have a message from notify on topic mytopic. Message:")
+	require.Contains(t, form.Get("Twiml"), "hi there")
+	require.Contains(t, form.Get("Twiml"), "This message was sent by user phil.")
+}
+
+// TestClient_Call_EscapesXML ensures that user-controlled fields cannot break out of the
+// TwiML document, i.e. that a message containing XML is escaped rather than interpreted
+func TestClient_Call_EscapesXML(t *testing.T) {
+	var body string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		b, err := io.ReadAll(r.Body)
+		require.Nil(t, err)
+		body = string(b)
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	data := &CallData{
+		Topic:   "mytopic",
+		Message: `</Say><Say>evil</Say>`,
+		Tags:    []string{"<tag>"},
+		Sender:  `phil & "friends"`,
+	}
+	require.Nil(t, c.Call("+11122233344", data))
+
+	form, err := url.ParseQuery(body)
+	require.Nil(t, err)
+	twiml := form.Get("Twiml")
+	require.NotContains(t, twiml, "<Say>evil</Say>")
+	require.Contains(t, twiml, "&lt;/Say&gt;&lt;Say&gt;evil&lt;/Say&gt;")
+	require.Contains(t, twiml, "phil &amp; &#34;friends&#34;")
+	// The caller's data must not be modified by the escaping
+	require.Equal(t, `</Say><Say>evil</Say>`, data.Message)
+	require.Equal(t, []string{"<tag>"}, data.Tags)
+}
+
+func TestClient_Call_CustomCallFormat(t *testing.T) {
+	var body string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		b, err := io.ReadAll(r.Body)
+		require.Nil(t, err)
+		body = string(b)
+	}))
+	defer server.Close()
+
+	conf := testConfig(server.URL)
+	conf.CallFormat = template.Must(template.New("twiml").Parse(`<Response><Say>{{.Message}} von {{.Sender}}</Say></Response>`))
+	c := NewClient(conf)
+	require.Nil(t, c.Call("+11122233344", &CallData{Topic: "mytopic", Message: "hi there", Sender: "phil"}))
+
+	form, err := url.ParseQuery(body)
+	require.Nil(t, err)
+	require.Equal(t, "<Response><Say>hi there von phil</Say></Response>", form.Get("Twiml"))
+}
+
+// TestClient_Call_RendersAllFields covers the fields that the default TwiML template does not
+// use, i.e. Title, Priority and Tags, including the escaping of every tag
+func TestClient_Call_RendersAllFields(t *testing.T) {
+	var body string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		b, err := io.ReadAll(r.Body)
+		require.Nil(t, err)
+		body = string(b)
+	}))
+	defer server.Close()
+
+	conf := testConfig(server.URL)
+	conf.CallFormat = template.Must(template.New("twiml").Parse(`<Response><Say>{{.Title}}/{{.Priority}}{{range .Tags}}/{{.}}{{end}}</Say></Response>`))
+	c := NewClient(conf)
+	data := &CallData{
+		Topic:    "mytopic",
+		Title:    "a <title>",
+		Priority: 5,
+		Tags:     []string{"<one>", "two & three"},
+	}
+	require.Nil(t, c.Call("+11122233344", data))
+
+	form, err := url.ParseQuery(body)
+	require.Nil(t, err)
+	require.Equal(t, "<Response><Say>a &lt;title&gt;/5/&lt;one&gt;/two &amp; three</Say></Response>", form.Get("Twiml"))
+}
+
+func TestClient_Call_TemplateError(t *testing.T) {
+	conf := testConfig("http://dummy.invalid")
+	conf.CallFormat = template.Must(template.New("twiml").Parse(`{{.DoesNotExist}}`))
+	c := NewClient(conf)
+	require.Error(t, c.Call("+11122233344", &CallData{Topic: "mytopic"}))
+}
+
+// TestClient_Call_Created ensures that a 201 Created is treated as a success. The Twilio Calls
+// API returns 201 (not 200) for a successfully queued call, so this must not be an error.
+func TestClient_Call_Created(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusCreated)
+		w.Write([]byte(`{"status":"queued"}`))
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	require.Nil(t, c.Call("+11122233344", &CallData{Topic: "mytopic", Message: "hi there"}))
+}
+
+// TestClient_Call_TwilioError ensures that a non-2xx response from Twilio is returned as an
+// error, so that the server counts it as a failure instead of a success. Twilio rejects calls
+// with a 4xx, e.g. for an invalid "To" number, or when the account is out of funds.
+func TestClient_Call_TwilioError(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusBadRequest)
+		w.Write([]byte(`{"code":21211,"message":"Invalid 'To' Phone Number: +invalid"}`))
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	err := c.Call("+invalid", &CallData{Topic: "mytopic", Message: "hi there"})
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "400")
+}
+
+func TestClient_Call_TwilioServerError(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	require.Error(t, c.Call("+11122233344", &CallData{Topic: "mytopic", Message: "hi there"}))
+}
+
+// TestClient_Call_TransportError ensures that a call to an unreachable Twilio API returns an
+// error, so that the server can count it as a failure
+func TestClient_Call_TransportError(t *testing.T) {
+	c := NewClient(testConfig(closedServerURL(t)))
+	require.Error(t, c.Call("+11122233344", &CallData{Topic: "mytopic", Message: "hi there"}))
+}
+
+func TestClient_Call_InvalidBaseURL(t *testing.T) {
+	c := NewClient(testConfig("://invalid"))
+	require.Error(t, c.Call("+11122233344", &CallData{Topic: "mytopic", Message: "hi there"}))
+}
+
+// TestClient_Verify_Created ensures that a 201 Created is treated as a success. The Twilio
+// Verify API returns 201 (not 200) when it creates a verification, so this must not be an error.
+func TestClient_Verify_Created(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusCreated)
+		w.Write([]byte(`{"status":"pending"}`))
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	require.Nil(t, c.Verify("+12223334444", "sms"))
+}
+
+// TestClient_Verify_TwilioError ensures that a non-2xx response from Twilio is returned as an
+// error. Without this, no SMS is sent, but the user is still told to check their phone.
+func TestClient_Verify_TwilioError(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusBadRequest)
+		w.Write([]byte(`{"code":60200,"message":"Invalid parameter"}`))
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	err := c.Verify("+12223334444", "sms")
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "400")
+}
+
+func TestClient_Verify_Unauthorized(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusUnauthorized)
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	require.Error(t, c.Verify("+12223334444", "sms"))
+}
+
+func TestClient_Verify_TransportError(t *testing.T) {
+	c := NewClient(testConfig(closedServerURL(t)))
+	require.Error(t, c.Verify("+12223334444", "sms"))
+}
+
+func TestClient_CheckVerify_TransportError(t *testing.T) {
+	c := NewClient(testConfig(closedServerURL(t)))
+	err := c.CheckVerify("+12223334444", "123456")
+	require.Error(t, err)
+	require.False(t, errors.Is(err, ErrVerificationExpired))
+}
+
+func TestClient_Verify_Success(t *testing.T) {
+	var body string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		require.Equal(t, "/v2/Services/VA1234567890/Verifications", r.URL.Path)
+		require.Equal(t, "Basic QUMxMjM0NTY3ODkwOkFBRUFBMTIzNDU2Nzg5MA==", r.Header.Get("Authorization"))
+		b, err := io.ReadAll(r.Body)
+		require.Nil(t, err)
+		body = string(b)
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	require.Nil(t, c.Verify("+12223334444", "sms"))
+	require.Equal(t, "Channel=sms&To=%2B12223334444", body)
+}
+
+func TestClient_CheckVerify_Success(t *testing.T) {
+	var body string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		require.Equal(t, "/v2/Services/VA1234567890/VerificationCheck", r.URL.Path)
+		b, err := io.ReadAll(r.Body)
+		require.Nil(t, err)
+		body = string(b)
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	require.Nil(t, c.CheckVerify("+12223334444", "123456"))
+	require.Equal(t, "Code=123456&To=%2B12223334444", body)
+}
+
+// TestClient_CheckVerify_Expired ensures that a 404 from the Twilio Verify API is
+// mapped to ErrVerificationExpired, which the server turns into an HTTP 410
+func TestClient_CheckVerify_Expired(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusNotFound)
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	err := c.CheckVerify("+12223334444", "123456")
+	require.True(t, errors.Is(err, ErrVerificationExpired))
+}
+
+func TestClient_CheckVerify_OtherError(t *testing.T) {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusInternalServerError)
+	}))
+	defer server.Close()
+
+	c := NewClient(testConfig(server.URL))
+	err := c.CheckVerify("+12223334444", "123456")
+	require.Error(t, err)
+	require.False(t, errors.Is(err, ErrVerificationExpired))
+}
+
+// closedServerURL returns the URL of a server that is not listening anymore, to simulate an
+// unreachable Twilio API
+func closedServerURL(t *testing.T) string {
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		t.Fatal("Should not be called")
+	}))
+	server.Close()
+	return server.URL
+}
+
+func testConfig(baseURL string) *Config {
+	return &Config{
+		Account:       "AC1234567890",
+		AuthToken:     "AAEAA1234567890",
+		PhoneNumber:   "+1234567890",
+		CallsBaseURL:  baseURL,
+		VerifyBaseURL: baseURL,
+		VerifyService: "VA1234567890",
+		BuildVersion:  "1.2.3",
+	}
+}

twilio/types.go

@@ -0,0 +1,80 @@
+package twilio
+
+import (
+	"bytes"
+	"encoding/xml"
+	"errors"
+	"text/template"
+)
+
+// ErrVerificationExpired is returned by CheckVerify if the verification code has
+// expired, or if it never existed in the first place
+var ErrVerificationExpired = errors.New("phone number verification expired or does not exist")
+
+// Config holds the Twilio configuration for the client
+type Config struct {
+	Account       string             // Twilio account SID, e.g. AC123...
+	AuthToken     string             // Twilio auth token
+	PhoneNumber   string             // Twilio number to use for outgoing calls
+	CallsBaseURL  string             // Base URL of the Twilio Calls API
+	VerifyBaseURL string             // Base URL of the Twilio Verify API
+	VerifyService string             // Twilio Verify service ID, e.g. VA123...
+	CallFormat    *template.Template // TwiML template for calls; if nil, defaultCallFormatTemplate is used
+	BuildVersion  string             // ntfy version, used for the User-Agent header
+}
+
+// defaultCallFormatTemplate is the default TwiML template used for Twilio calls.
+// It can be overridden in the server configuration's twilio-call-format field.
+//
+// The format uses Go template syntax with the following fields:
+// {{.Topic}}, {{.Title}}, {{.Message}}, {{.Priority}}, {{.Tags}}, {{.Sender}}
+// String fields are automatically XML-escaped.
+var defaultCallFormatTemplate = template.Must(template.New("twiml").Parse(`
+<Response>
+	<Pause length="1"/>
+	<Say loop="3">
+		You have a message from notify on topic {{.Topic}}. Message:
+		<break time="1s"/>
+		{{.Message}}
+		<break time="1s"/>
+		End of message.
+		<break time="1s"/>
+		This message was sent by user {{.Sender}}. It will be repeated three times.
+		To unsubscribe from calls like this, remove your phone number in the notify web app.
+		<break time="3s"/>
+	</Say>
+	<Say>Goodbye.</Say>
+</Response>`))
+
+// CallData holds the data passed to the Twilio call format template. String fields are
+// XML-escaped before the template is executed, so callers pass them unescaped.
+type CallData struct {
+	Topic    string
+	Title    string
+	Message  string
+	Priority int
+	Tags     []string
+	Sender   string
+}
+
+// escaped returns a copy of the call data with all string fields XML-escaped
+func (d *CallData) escaped() *CallData {
+	tags := make([]string, len(d.Tags))
+	for i, tag := range d.Tags {
+		tags[i] = xmlEscapeText(tag)
+	}
+	return &CallData{
+		Topic:    xmlEscapeText(d.Topic),
+		Title:    xmlEscapeText(d.Title),
+		Message:  xmlEscapeText(d.Message),
+		Priority: d.Priority,
+		Tags:     tags,
+		Sender:   xmlEscapeText(d.Sender),
+	}
+}
+
+func xmlEscapeText(text string) string {
+	var buf bytes.Buffer
+	_ = xml.EscapeText(&buf, []byte(text))
+	return buf.String()
+}