Fix NPE when queue is full (#2307)

Author: sethvargo

pkg/controller/issueapi/send_sms.go

@@ -73,7 +73,7 @@ func (c *Controller) SendSMS(ctx context.Context, realm *database.Realm, smsProv
 	if err := c.doSend(ctx, realm, smsProvider, signer, keyID, request, result); err != nil {
 		result.HTTPCode = http.StatusBadRequest
 		if sms.IsSMSQueueFull(err) {
-			result.ErrorReturn = result.ErrorReturn.WithCode(api.ErrSMSQueueFull)
+			result.ErrorReturn = api.Errorf("failed to send sms: queue is full: %s", err).WithCode(api.ErrSMSQueueFull)
 		} else {
 			result.ErrorReturn = api.Errorf("failed to send sms: %s", err).WithCode(api.ErrSMSFailure)
 		}

pkg/controller/issueapi/send_sms_test.go

@@ -22,6 +22,7 @@ import (
 	"fmt"
 	"io"
 	"net/http"
+	"os"
 	"strings"
 	"testing"
 	"time"
@@ -219,3 +220,97 @@ func TestSMS_sendSMS(t *testing.T) {
 		t.Errorf("expected SMS failure to roll-back and delete code. got %v", err)
 	}
 }
+
+func TestSMS_sendSMS_integration(t *testing.T) {
+	t.Parallel()
+
+	if testing.Short() {
+		t.Skipf("🚧 Skipping twilio tests (short)!")
+	}
+
+	accountSid := os.Getenv("TWILIO_ACCOUNT_SID")
+	authToken := os.Getenv("TWILIO_AUTH_TOKEN")
+	if accountSid == "" || authToken == "" {
+		t.Skipf("🚧 🚧 Skipping twilio tests (missing TWILIO_ACCOUNT_SID/TWILIO_AUTH_TOKEN)")
+	}
+
+	ctx := project.TestContext(t)
+
+	harness := envstest.NewServerConfig(t, testDatabaseInstance)
+	db := harness.Database
+
+	realm, err := db.FindRealm(1)
+	if err != nil {
+		t.Fatal(err)
+	}
+	realm.AllowBulkUpload = true
+	if err := db.SaveRealm(realm, database.SystemTest); err != nil {
+		t.Fatalf("failed to save realm: %v", err)
+	}
+
+	smsConfig := &database.SMSConfig{
+		RealmID:          realm.ID,
+		ProviderType:     sms.ProviderTypeTwilio,
+		TwilioAccountSid: accountSid,
+		TwilioFromNumber: "+15005550008", // magic number
+		TwilioAuthToken:  authToken,
+	}
+	if err := db.SaveSMSConfig(smsConfig); err != nil {
+		t.Fatal(err)
+	}
+
+	smsProvider, err := realm.SMSProvider(harness.Database)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	membership := &database.Membership{
+		RealmID:     realm.ID,
+		Realm:       realm,
+		Permissions: rbac.CodeBulkIssue,
+	}
+
+	ctx = controller.WithMembership(ctx, membership)
+
+	harness.Config.SMSSigning.FailClosed = true
+	c := issueapi.New(harness.Config, db, harness.RateLimiter, harness.KeyManager, harness.Renderer)
+
+	t.Run("queue_full", func(t *testing.T) {
+		t.Parallel()
+
+		request := &api.IssueCodeRequest{
+			TestType:    "confirmed",
+			SymptomDate: time.Now().UTC().Add(-48 * time.Hour).Format(project.RFC3339Date),
+			TZOffset:    0,
+			Phone:       project.TestPhoneNumber,
+		}
+		result := &issueapi.IssueResult{
+			HTTPCode: http.StatusOK,
+			VerCode: &database.VerificationCode{
+				RealmID:       realm.ID,
+				Code:          "00000001",
+				LongCode:      "00000001ABC",
+				Claimed:       true,
+				TestType:      "confirmed",
+				ExpiresAt:     time.Now().Add(time.Hour),
+				LongExpiresAt: time.Now().Add(time.Hour),
+			},
+		}
+		if err := realm.SaveVerificationCode(db, result.VerCode); err != nil {
+			t.Fatal(err)
+		}
+		// un-hmac the codes so rollback can find them.
+		result.VerCode.Code = "00000001"
+		result.VerCode.LongCode = "00000001ABC"
+
+		c.SendSMS(ctx, realm, smsProvider, nil, "", request, result)
+		if result.ErrorReturn == nil {
+			t.Fatal("expected failed SMS, but got no error response")
+		} else if result.ErrorReturn.ErrorCode != api.ErrSMSQueueFull {
+			t.Fatal("expected SMS failure code")
+		}
+		if _, err := realm.FindVerificationCodeByUUID(db, result.VerCode.UUID); !database.IsNotFound(err) {
+			t.Errorf("expected SMS failure to roll-back and delete code. got %v", err)
+		}
+	})
+}