fix(twilio): rename RCSSenderID to RCSSenderSID and update related validation

Author: mastercactapus

config/config.go

@@ -116,7 +116,7 @@ type Config struct {
 
 		MessagingServiceSID string `public:"true" info:"If set, replaces the use of From Number for SMS notifications."`
 
-		RCSSenderID string `info:"The sender ID for RCS messages. Required if RCS is enabled for the FromNumber or MessagingServiceSID."`
+		RCSSenderSID string `info:"The sender SID for RCS messages. Required if RCS is enabled for the MessagingServiceSID."`
 
 		DisableTwoWaySMS      bool     `info:"Disables SMS reply codes for alert messages."`
 		SMSCarrierLookup      bool     `info:"Perform carrier lookup of SMS contact methods (required for SMSFromNumberOverride). Extra charges may apply."`
@@ -510,6 +510,12 @@ func (cfg Config) Validate() error {
 	if cfg.Twilio.MessagingServiceSID != "" {
 		err = validate.Many(err, validate.TwilioSID("Twilio.MessagingServiceSID", "MG", cfg.Twilio.MessagingServiceSID))
 	}
+	if cfg.Twilio.RCSSenderSID != "" {
+		err = validate.Many(err, validate.TwilioSID("Twilio.RCSSenderSID", "XE", cfg.Twilio.RCSSenderSID))
+		if cfg.Twilio.MessagingServiceSID == "" {
+			err = validate.Many(err, validation.NewFieldError("Twilio.MessagingServiceSID", "required when Twilio.RCSSenderSID is set"))
+		}
+	}
 	if cfg.Mailgun.EmailDomain != "" {
 		err = validate.Many(err, validate.Email("Mailgun.EmailDomain", "example@"+cfg.Mailgun.EmailDomain))
 	}

graphql2/mapconfig.go

@@ -7,8 +7,10 @@ import (
 	"encoding/binary"
 	"fmt"
 	"math/rand"
+	"strings"
 	"time"
 
+	"github.com/target/goalert/config"
 	"github.com/target/goalert/gadb"
 	"github.com/target/goalert/permission"
 	"github.com/target/goalert/search"
@@ -136,10 +138,12 @@ func (s *Store) OriginalMessageStatus(ctx context.Context, alertID int, dstID De
 		return nil, err
 	}
 
-	return outgoingMessageToSendResult(row.OutgoingMessage, row.CmDest, row.ChDest)
+	return outgoingMessageToSendResult(ctx, row.OutgoingMessage, row.CmDest, row.ChDest)
 }
 
-func outgoingMessageToSendResult(msg gadb.OutgoingMessage, cm, ch gadb.NullDestV1) (*SendResult, error) {
+func outgoingMessageToSendResult(ctx context.Context, msg gadb.OutgoingMessage, cm, ch gadb.NullDestV1) (*SendResult, error) {
+	cfg := config.FromContext(ctx)
+
 	res := SendResult{
 		ID:                msg.ID.String(),
 		ProviderMessageID: msg.ProviderMsgID,
@@ -157,11 +161,19 @@ func outgoingMessageToSendResult(msg gadb.OutgoingMessage, cm, ch gadb.NullDestV
 		return nil, err
 	}
 
+	srcValue := msg.SrcValue.String
+	if cm.DestV1.Type == "builtin-twilio-sms" && strings.HasPrefix(srcValue, "rcs:") {
+		// Hack: Twilio currently has no way to fetch the Display Name of the RCS sender, so we use the application name as a fallback.
+		//
+		// In the future, we can use the RCSSenderSID to fetch the actual display name.
+		srcValue = cfg.ApplicationName()
+	}
+
 	res.Status = Status{
 		State:    state,
 		Details:  msg.StatusDetails,
 		Sequence: int(msg.ProviderSeq),
-		SrcValue: msg.SrcValue.String,
+		SrcValue: srcValue,
 	}
 	if msg.LastStatusAt.Valid {
 		res.Status.Age = msg.LastStatusAt.Time.Sub(msg.CreatedAt)
@@ -381,7 +393,7 @@ func (s *Store) FindManyMessageStatuses(ctx context.Context, strIDs []string) ([
 
 	var result []SendResult
 	for _, r := range rows {
-		res, err := outgoingMessageToSendResult(r.OutgoingMessage, r.CmDest, r.ChDest)
+		res, err := outgoingMessageToSendResult(ctx, r.OutgoingMessage, r.CmDest, r.ChDest)
 		if err != nil {
 			return nil, err
 		}
@@ -416,7 +428,7 @@ func (s *Store) LastMessageStatus(ctx context.Context, typ gadb.EnumOutgoingMess
 		return nil, time.Time{}, err
 	}
 
-	sendRes, err := outgoingMessageToSendResult(row.OutgoingMessage, row.CmDest, row.ChDest)
+	sendRes, err := outgoingMessageToSendResult(ctx, row.OutgoingMessage, row.CmDest, row.ChDest)
 	if err != nil {
 		return nil, time.Time{}, err
 	}

notification/store.go

@@ -168,24 +168,17 @@ func (s *SMS) ServeStatusCallback(w http.ResponseWriter, req *http.Request) {
 		return
 	}
 	ctx := req.Context()
-	cfg := config.FromContext(ctx)
 	status := MessageStatus(req.FormValue("MessageStatus"))
 	sid := validSID(req.FormValue("MessageSid"))
-	var number string
-	if cfg.Twilio.RCSSenderID != "" && req.FormValue("From") == "rcs:"+cfg.Twilio.RCSSenderID {
-		number = req.FormValue("To")
-	} else {
-		number = validPhone(req.FormValue("To"))
-	}
-	if status == "" || sid == "" || number == "" {
+	if status == "" || sid == "" {
 		http.Error(w, "", http.StatusBadRequest)
 		return
 	}
 
 	ctx = log.WithFields(ctx, log.Fields{
 		"Status": status,
 		"SID":    sid,
-		"Phone":  number,
+		"Phone":  req.FormValue("To"),
 		"Type":   "TwilioSMS",
 	})
 	msg := Message{SID: sid, Status: status, From: strings.TrimPrefix(req.FormValue("From"), "rcs:")}
@@ -228,8 +221,12 @@ func (s *SMS) ServeMessage(w http.ResponseWriter, req *http.Request) {
 	ctx := req.Context()
 	cfg := config.FromContext(ctx)
 	from := validPhone(strings.TrimPrefix(req.FormValue("From"), "rcs:"))
-	if from == "" || from == cfg.Twilio.FromNumber || from == cfg.Twilio.RCSSenderID {
-		http.Error(w, "", http.StatusBadRequest)
+	if from == cfg.Twilio.FromNumber || req.FormValue("From") == req.FormValue("To") {
+		http.Error(w, "ignoring message loop", http.StatusBadRequest)
+		return
+	}
+	if from == "" {
+		http.Error(w, "invalid from number", http.StatusBadRequest)
 		return
 	}
 

notification/twilio/sms.go

@@ -1652,7 +1652,7 @@ type ConfigID =
   | 'Twilio.AlternateAuthToken'
   | 'Twilio.FromNumber'
   | 'Twilio.MessagingServiceSID'
-  | 'Twilio.RCSSenderID'
+  | 'Twilio.RCSSenderSID'
   | 'Twilio.DisableTwoWaySMS'
   | 'Twilio.SMSCarrierLookup'
   | 'Twilio.SMSFromNumberOverride'