ui/profile: display proper errors in send test dialog

notification/store.go

@@ -248,10 +248,18 @@ func (s *Store) Code(ctx context.Context, id string) (int, error) {
 }
 
 func (s *Store) SendContactMethodTest(ctx context.Context, id string) error {
-	_, err := s.cmUserID(ctx, id)
+	cmUserID, err := s.cmUserID(ctx, id)
 	if err != nil {
 		return err
 	}
+
+	// due to potential regulations around consent with phone calls and SMS, we
+	// only allow users to send test messages to their own contact methods
+	err = permission.LimitCheckAny(ctx, permission.MatchUser(cmUserID))
+	if err != nil {
+		return err
+	}
+
 	tx, err := s.db.BeginTx(ctx, nil)
 	if err != nil {
 		return err

test/smoke/twilioenablebysms_test.go

@@ -12,9 +12,9 @@ func TestTwilioEnableBySMS(t *testing.T) {
 	t.Parallel()
 
 	sqlQuery := `
-		insert into users (id, name, email) 
+		insert into users (id, name, email, role) 
 		values 
-			({{uuid "user"}}, 'bob', 'joe');
+			({{uuid "user"}}, 'bob', 'joe', 'user');
 		insert into user_contact_methods (id, user_id, name, type, value, disabled) 
 		values
 			({{uuid "cm1"}}, {{uuid "user"}}, 'personal', 'SMS', {{phone "1"}}, true),
@@ -30,7 +30,7 @@ func TestTwilioEnableBySMS(t *testing.T) {
 	defer h.Close()
 
 	doQL := func(query string, expectErr bool) {
-		g := h.GraphQLQuery2(query)
+		g := h.GraphQLQueryUserT(t, h.UUID("user"), query)
 		if expectErr {
 			if len(g.Errors) == 0 {
 				t.Fatal("expected error")

test/smoke/twilioenablebyvoice_test.go

@@ -12,9 +12,9 @@ func TestTwilioEnablebyVoice(t *testing.T) {
 	t.Parallel()
 
 	sqlQuery := `
-		insert into users (id, name, email) 
+		insert into users (id, name, email, role) 
 		values 
-			({{uuid "user"}}, 'bob', 'joe');
+			({{uuid "user"}}, 'bob', 'joe', 'user');
 		insert into user_contact_methods (id, user_id, name, type, value, disabled) 
 		values
 			({{uuid "cm1"}}, {{uuid "user"}}, 'personal', 'SMS', {{phone "1"}}, true),
@@ -30,7 +30,7 @@ func TestTwilioEnablebyVoice(t *testing.T) {
 	defer h.Close()
 
 	doQL := func(query string, expectErr bool) {
-		g := h.GraphQLQuery2(query)
+		g := h.GraphQLQueryUserT(t, h.UUID("user"), query)
 		if expectErr {
 			if len(g.Errors) == 0 {
 				t.Fatal("expected error")

test/smoke/twiliosmsverification_test.go

@@ -15,9 +15,9 @@ func TestTwilioSMSVerification(t *testing.T) {
 	t.Parallel()
 
 	sqlQuery := `
-		insert into users (id, name, email) 
+		insert into users (id, name, email, role) 
 		values 
-			({{uuid "user"}}, 'bob', 'joe');
+			({{uuid "user"}}, 'bob', 'joe', 'user');
 		insert into user_contact_methods (id, user_id, name, type, value, disabled) 
 		values
 			({{uuid "cm1"}}, {{uuid "user"}}, 'personal', 'SMS', {{phone "1"}}, true);
@@ -30,7 +30,7 @@ func TestTwilioSMSVerification(t *testing.T) {
 	defer h.Close()
 
 	doQL := func(query string) {
-		g := h.GraphQLQuery2(query)
+		g := h.GraphQLQueryUserT(t, h.UUID("user"), query)
 		for _, err := range g.Errors {
 			t.Error("GraphQL Error:", err.Message)
 		}

test/smoke/twiliotestsms_test.go

@@ -12,9 +12,9 @@ func TestTwilioSMS(t *testing.T) {
 	t.Parallel()
 
 	sqlQuery := `
-	insert into users (id, name, email) 
+	insert into users (id, name, email, role) 
 	values 
-		({{uuid "user"}}, 'bob', 'joe');
+		({{uuid "user"}}, 'bob', 'joe', 'user');
 	insert into user_contact_methods (id, user_id, name, type, value) 
 	values
 	    ({{uuid "cm1"}}, {{uuid "user"}}, 'personal', 'SMS', {{phone "1"}});
@@ -23,7 +23,7 @@ func TestTwilioSMS(t *testing.T) {
 	defer h.Close()
 
 	doQL := func(query string) {
-		g := h.GraphQLQuery2(query)
+		g := h.GraphQLQueryUserT(t, h.UUID("user"), query)
 		for _, err := range g.Errors {
 			t.Error("GraphQL Error:", err.Message)
 		}

test/smoke/twiliotestsmsnotcmowner_test.go

@@ -0,0 +1,36 @@
+package smoke
+
+import (
+	"fmt"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+	"github.com/target/goalert/test/smoke/harness"
+)
+
+// TestTwilioSMSNotCMOwner checks that a test sent from a user who is not the
+// owner of the contact method returns an error.
+func TestTwilioSMSNotCMOwner(t *testing.T) {
+	t.Parallel()
+
+	sqlQuery := `
+	insert into users (id, name, email) 
+	values 
+		({{uuid "user"}}, 'bob', 'joe');
+	insert into user_contact_methods (id, user_id, name, type, value) 
+	values
+	    ({{uuid "cm1"}}, {{uuid "user"}}, 'personal', 'SMS', {{phone "1"}});
+`
+	h := harness.NewHarness(t, sqlQuery, "add-verification-code")
+	defer h.Close()
+
+	cm1 := h.UUID("cm1")
+
+	g := h.GraphQLQuery2(fmt.Sprintf(`
+		mutation {
+			testContactMethod(id: "%s")
+		}
+		`, cm1))
+	require.Len(t, g.Errors, 1, "errors returned from GraphQL")
+	require.Equal(t, "access denied", g.Errors[0].Message)
+}

test/smoke/twiliotestvoice_test.go

@@ -12,9 +12,9 @@ func TestTwilioVoice(t *testing.T) {
 	t.Parallel()
 
 	sqlQuery := `
-	insert into users (id, name, email) 
+	insert into users (id, name, email, role) 
 	values 
-		({{uuid "user"}}, 'bob', 'joe');
+		({{uuid "user"}}, 'bob', 'joe', 'user');
 	insert into user_contact_methods (id, user_id, name, type, value) 
 	values
 	    ({{uuid "cm1"}}, {{uuid "user"}}, 'personal', 'VOICE', {{phone "1"}});
@@ -23,7 +23,7 @@ func TestTwilioVoice(t *testing.T) {
 	defer h.Close()
 
 	doQL := func(query string) {
-		g := h.GraphQLQuery2(query)
+		g := h.GraphQLQueryUserT(t, h.UUID("user"), query)
 		for _, err := range g.Errors {
 			t.Error("GraphQL Error:", err.Message)
 		}

test/smoke/twiliovoiceverification_test.go

@@ -16,9 +16,9 @@ func TestTwilioVoiceVerification(t *testing.T) {
 	t.Parallel()
 
 	sqlQuery := `
-		insert into users (id, name, email) 
+		insert into users (id, name, email, role) 
 		values 
-			({{uuid "user"}}, 'bob', 'joe');
+			({{uuid "user"}}, 'bob', 'joe', 'user');
 		insert into user_contact_methods (id, user_id, name, type, value, disabled) 
 		values
 			({{uuid "cm2"}}, {{uuid "user"}}, 'personal', 'VOICE', {{phone "1"}}, true);
@@ -44,7 +44,7 @@ func TestTwilioVoiceVerification(t *testing.T) {
 	defer h.Close()
 
 	doQL := func(query string) {
-		g := h.GraphQLQuery2(query)
+		g := h.GraphQLQueryUserT(t, h.UUID("user"), query)
 		for _, err := range g.Errors {
 			t.Error("GraphQL Error:", err.Message)
 		}

web/src/app/users/SendTestDialog.tsx

@@ -1,5 +1,5 @@
-import React, { useEffect, useState, MouseEvent } from 'react'
-import { useQuery, useMutation, gql } from '@apollo/client'
+import React, { useEffect, MouseEvent, useState } from 'react'
+import { gql, useQuery, useMutation } from 'urql'
 
 import Spinner from '../loading/components/Spinner'
 
@@ -43,47 +43,49 @@ export default function SendTestDialog(
 ): JSX.Element {
   const { title = 'Test Delivery Status', onClose, messageID } = props
 
-  const [now] = useState(DateTime.local())
+  const [sendTestStatus, sendTest] = useMutation(mutation)
 
-  const [sendTest, sendTestStatus] = useMutation(mutation, {
+  const [{ data, fetching, error }] = useQuery({
+    query,
     variables: {
       id: messageID,
     },
+    requestPolicy: 'network-only',
   })
 
-  const { data, loading, error } = useQuery(query, {
-    variables: {
-      id: messageID,
-    },
-    fetchPolicy: 'network-only',
-  })
-
+  // We keep a stable timestampe to track how long the dialog has been open
+  const [now] = useState(DateTime.utc())
   const status = data?.userContactMethod?.lastTestMessageState?.status ?? ''
   const cmDestValue = data?.userContactMethod?.formattedValue ?? ''
   const cmType: ContactMethodType = data?.userContactMethod?.type ?? ''
   const lastTestVerifyAt = data?.userContactMethod?.lastTestVerifyAt ?? ''
   const timeSinceLastVerified = now.diff(DateTime.fromISO(lastTestVerifyAt))
   const fromValue =
     data?.userContactMethod?.lastTestMessageState?.formattedSrcValue ?? ''
-  const errorMessage = error?.message ?? ''
+  const errorMessage = (error?.message || sendTestStatus.error?.message) ?? ''
 
   useEffect(() => {
-    if (loading || error || sendTestStatus.called) {
+    if (fetching || errorMessage || sendTestStatus.data) {
       return
     }
     if (
-      data?.userContactMethod?.lastTestMessageState == null ||
+      data?.lastTestMessageState == null ||
       !(timeSinceLastVerified.as('seconds') < 60)
     ) {
-      sendTest()
+      sendTest({ id: messageID })
     }
-  }, [lastTestVerifyAt, loading])
+  }, [lastTestVerifyAt, fetching])
 
-  let details
-  if (sendTestStatus.called && lastTestVerifyAt > now.toISO()) {
+  let details = ''
+  if (sendTestStatus.data && lastTestVerifyAt > now.toISO()) {
     details = data?.userContactMethod?.lastTestMessageState?.details ?? ''
   }
 
+  const isLoading =
+    sendTestStatus.fetching ||
+    (!!details && !!errorMessage) ||
+    status === 'pending'
+
   const getTestStatusColor = (status: string): string => {
     switch (status) {
       case 'OK':
@@ -95,8 +97,6 @@ export default function SendTestDialog(
     }
   }
 
-  if (loading || sendTestStatus.loading) return <Spinner text='Loading...' />
-
   const msg = (): string => {
     switch (cmType) {
       case 'SMS':
@@ -119,21 +119,17 @@ export default function SendTestDialog(
         <DialogContentText>
           GoAlert is sending a test {msg()}.
         </DialogContentText>
+        {isLoading && <Spinner text='Sending Test...' />}
         {fromValue && (
           <DialogContentText>
             The test message was sent from {fromValue}.
           </DialogContentText>
         )}
-        {details && (
+        {!!details && (
           <DialogContentText color={getTestStatusColor(status)}>
             {toTitleCase(details)}
           </DialogContentText>
         )}
-        {!details && (
-          <DialogContentText color='error'>
-            Couldn't send a message yet, please try again after about a minute.
-          </DialogContentText>
-        )}
       </DialogContent>
 
       {errorMessage && <DialogContentError error={errorMessage} />}