fix: mr comment fix

courier/sms.go

@@ -19,7 +19,6 @@ type sendSMSRequestBody struct {
 
 type smsClient struct {
 	*http.Client
-	Host          string
 	RequestConfig json.RawMessage
 
 	GetTemplateType        func(t SMSTemplate) (TemplateType, error)
@@ -81,7 +80,7 @@ func (c *courier) dispatchSMS(ctx context.Context, msg Message) error {
 		return err
 	}
 
-	builder, err := request.NewBuilder(c.smsClient.RequestConfig, c.deps.Logger())
+	builder, err := request.NewBuilder(c.smsClient.RequestConfig, c.deps.HTTPClient(ctx), c.deps.Logger())
 	if err != nil {
 		return err
 	}

courier/sms_test.go

@@ -10,14 +10,16 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gofrs/uuid"
+	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/ory/kratos/courier/template/sms"
 	"github.com/ory/kratos/driver/config"
 	"github.com/ory/kratos/internal"
-	"github.com/ory/kratos/x"
+	"github.com/ory/x/resilience"
 )
 
 func TestQueueSMS(t *testing.T) {
@@ -89,14 +91,20 @@ func TestQueueSMS(t *testing.T) {
 	for _, message := range expectedSMS {
 		id, err := c.QueueSMS(ctx, sms.NewTestStub(reg, message))
 		require.NoError(t, err)
-		x.RequireNotNilUUID(t, id)
+		require.NotEqual(t, uuid.Nil, id)
 	}
 
 	go func() {
 		require.NoError(t, c.Work(ctx))
 	}()
 
-	time.Sleep(time.Second)
+	require.NoError(t, resilience.Retry(reg.Logger(), time.Millisecond*250, time.Second*10, func() error {
+		if len(actual) == len(expectedSMS) {
+			return nil
+		}
+		return errors.New("capacity not reached")
+	}))
+
 	for i, message := range actual {
 		expected := expectedSMS[i]
 

courier/smtp_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/gofrs/uuid"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 	"github.com/stretchr/testify/assert"
@@ -70,7 +71,7 @@ func TestQueueEmail(t *testing.T) {
 	conf.MustSet(config.ViperKeyCourierSMTPFrom, "[email protected]")
 	reg.Logger().Level = logrus.TraceLevel
 
-	c := reg.Courier(ctx) //???
+	c := reg.Courier(ctx)
 
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
@@ -81,15 +82,15 @@ func TestQueueEmail(t *testing.T) {
 		Body:    "test-body-1",
 	}))
 	require.NoError(t, err)
-	x.RequireNotNilUUID(t, id)
+	require.NotEqual(t, uuid.Nil, id)
 
 	id, err = c.QueueEmail(ctx, templates.NewTestStub(reg, &templates.TestStubModel{
 		To:      "[email protected]",
 		Subject: "test-subject-2",
 		Body:    "test-body-2",
 	}))
 	require.NoError(t, err)
-	x.RequireNotNilUUID(t, id)
+	require.NotEqual(t, uuid.Nil, id)
 
 	// The third email contains a sender name and custom headers
 	conf.MustSet(config.ViperKeyCourierSMTPFromName, "Bob")
@@ -103,7 +104,7 @@ func TestQueueEmail(t *testing.T) {
 		Body:    "test-body-3",
 	}))
 	require.NoError(t, err)
-	x.RequireNotNilUUID(t, id)
+	require.NotEqual(t, uuid.Nil, id)
 
 	go func() {
 		require.NoError(t, c.Work(ctx))

courier/template/sms/otp.go

@@ -15,8 +15,9 @@ type (
 	}
 
 	OTPMessageModel struct {
-		To   string
-		Code string
+		To       string
+		Code     string
+		Identity map[string]interface{}
 	}
 )
 

courier/template/sms/stub.go

@@ -15,8 +15,9 @@ type (
 	}
 
 	TestStubModel struct {
-		To   string
-		Body string
+		To       string
+		Body     string
+		Identity map[string]interface{}
 	}
 )
 

embedx/config.schema.json

@@ -1478,26 +1478,23 @@
                   "type": "string",
                   "description": "The HTTP method to use (GET, POST, etc)."
                 },
+                "header": {
+                  "type": "object",
+                  "description": "The HTTP headers that must be applied to request",
+                  "additionalProperties": {
+                    "type": "string"
+                  }
+                },
                 "body": {
                   "type": "string",
-                  "oneOf": [
-                    {
-                      "format": "uri",
-                      "pattern": "^(http|https|file|base64)://",
-                      "description": "URI pointing to the jsonnet template used for payload generation. Only used for those HTTP methods, which support HTTP body payloads",
-                      "examples": [
-                        "file:///path/to/body.jsonnet",
-                        "file://./body.jsonnet",
-                        "base64://ZnVuY3Rpb24oY3R4KSB7CiAgaWRlbnRpdHlfaWQ6IGlmIGN0eFsiaWRlbnRpdHkiXSAhPSBudWxsIHRoZW4gY3R4LmlkZW50aXR5LmlkLAp9=",
-                        "https://oryapis.com/default_body.jsonnet"
-                      ]
-                    },
-                    {
-                      "description": "DEPRECATED: please use a URI instead (i.e. prefix your filepath with 'file://')",
-                      "not": {
-                        "pattern": "^(http|https|file|base64)://"
-                      }
-                    }
+                  "format": "uri",
+                  "pattern": "^(http|https|file|base64)://",
+                  "description": "URI pointing to the jsonnet template used for payload generation. Only used for those HTTP methods, which support HTTP body payloads",
+                  "examples": [
+                    "file:///path/to/body.jsonnet",
+                    "file://./body.jsonnet",
+                    "base64://ZnVuY3Rpb24oY3R4KSB7CiAgaWRlbnRpdHlfaWQ6IGlmIGN0eFsiaWRlbnRpdHkiXSAhPSBudWxsIHRoZW4gY3R4LmlkZW50aXR5LmlkLAp9=",
+                    "https://oryapis.com/default_body.jsonnet"
                   ]
                 },
                 "auth": {
@@ -1514,7 +1511,12 @@
                   ]
                 },
                 "additionalProperties": false
-              }
+              },
+              "required": [
+                "url",
+                "method"
+              ],
+              "additionalProperties": false
             }
           },
           "additionalProperties": false

request/builder.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 
 	"github.com/google/go-jsonnet"
+	"github.com/hashicorp/go-retryablehttp"
 
 	"github.com/ory/x/fetcher"
 	"github.com/ory/x/logrusx"
@@ -22,12 +23,13 @@ const (
 )
 
 type Builder struct {
-	r    *http.Request
-	log  *logrusx.Logger
-	conf *Config
+	r           *http.Request
+	log         *logrusx.Logger
+	conf        *Config
+	fetchClient *retryablehttp.Client
 }
 
-func NewBuilder(config json.RawMessage, l *logrusx.Logger) (*Builder, error) {
+func NewBuilder(config json.RawMessage, client *retryablehttp.Client, l *logrusx.Logger) (*Builder, error) {
 	c, err := parseConfig(config)
 	if err != nil {
 		return nil, err
@@ -39,9 +41,10 @@ func NewBuilder(config json.RawMessage, l *logrusx.Logger) (*Builder, error) {
 	}
 
 	return &Builder{
-		r:    r,
-		log:  l,
-		conf: c,
+		r:           r,
+		log:         l,
+		conf:        c,
+		fetchClient: client,
 	}, nil
 }
 
@@ -69,13 +72,18 @@ func (b *Builder) addBody(body interface{}) error {
 		return errors.New("got empty template path for request with body")
 	}
 
+	tpl, err := b.readTemplate()
+	if err != nil {
+		return err
+	}
+
 	switch contentType {
 	case ContentTypeForm:
-		if err := b.addURLEncodedBody(body); err != nil {
+		if err := b.addURLEncodedBody(tpl, body); err != nil {
 			return err
 		}
 	case ContentTypeJSON:
-		if err := b.addJSONBody(body); err != nil {
+		if err := b.addJSONBody(tpl, body); err != nil {
 			return err
 		}
 	default:
@@ -85,14 +93,7 @@ func (b *Builder) addBody(body interface{}) error {
 	return nil
 }
 
-func (b *Builder) addJSONBody(body interface{}) error {
-	tURL := b.conf.TemplateURI
-
-	tpl, err := readTemplate(tURL, b.log)
-	if err != nil {
-		return err
-	}
-
+func (b *Builder) addJSONBody(template *bytes.Buffer, body interface{}) error {
 	buf := new(bytes.Buffer)
 	enc := json.NewEncoder(buf)
 	enc.SetEscapeHTML(false)
@@ -105,7 +106,7 @@ func (b *Builder) addJSONBody(body interface{}) error {
 	vm := jsonnet.MakeVM()
 	vm.TLACode("ctx", buf.String())
 
-	res, err := vm.EvaluateAnonymousSnippet(tURL, tpl.String())
+	res, err := vm.EvaluateAnonymousSnippet(b.conf.TemplateURI, template.String())
 	if err != nil {
 		return err
 	}
@@ -117,13 +118,7 @@ func (b *Builder) addJSONBody(body interface{}) error {
 	return nil
 }
 
-func (b *Builder) addURLEncodedBody(body interface{}) error {
-	tURL := b.conf.TemplateURI
-	tpl, err := readTemplate(tURL, b.log)
-	if err != nil {
-		return err
-	}
-
+func (b *Builder) addURLEncodedBody(template *bytes.Buffer, body interface{}) error {
 	buf := new(bytes.Buffer)
 	enc := json.NewEncoder(buf)
 	enc.SetEscapeHTML(false)
@@ -136,7 +131,7 @@ func (b *Builder) addURLEncodedBody(body interface{}) error {
 	vm := jsonnet.MakeVM()
 	vm.TLACode("ctx", buf.String())
 
-	res, err := vm.EvaluateAnonymousSnippet(tURL, tpl.String())
+	res, err := vm.EvaluateAnonymousSnippet(b.conf.TemplateURI, template.String())
 	if err != nil {
 		return err
 	}
@@ -175,18 +170,20 @@ func (b *Builder) BuildRequest(body interface{}) (*http.Request, error) {
 	return b.r, nil
 }
 
-func readTemplate(templateURI string, l *logrusx.Logger) (*bytes.Buffer, error) {
+func (b *Builder) readTemplate() (*bytes.Buffer, error) {
+	templateURI := b.conf.TemplateURI
+
 	if templateURI == "" {
 		return nil, nil
 	}
 
-	f := fetcher.NewFetcher()
+	f := fetcher.NewFetcher(fetcher.WithClient(b.fetchClient))
 
 	tpl, err := f.Fetch(templateURI)
 	if errors.Is(err, fetcher.ErrUnknownScheme) {
 		// legacy filepath
 		templateURI = "file://" + templateURI
-		l.WithError(err).Warnf("support for filepaths without a 'file://' scheme will be dropped in the next release, please use %s instead in your config", templateURI)
+		b.log.WithError(err).Warnf("support for filepaths without a 'file://' scheme will be dropped in the next release, please use %s instead in your config", templateURI)
 
 		tpl, err = f.Fetch(templateURI)
 	}

request/builder_test.go

@@ -238,7 +238,7 @@ func TestBuildRequest(t *testing.T) {
 		t.Run("request-type="+tc.name, func(t *testing.T) {
 			l := logrusx.New("kratos", "test")
 
-			rb, err := NewBuilder(json.RawMessage(tc.rawConfig), l)
+			rb, err := NewBuilder(json.RawMessage(tc.rawConfig), nil, l)
 			require.NoError(t, err)
 
 			assert.Equal(t, tc.bodyTemplateURI, rb.conf.TemplateURI)

selfservice/hook/web_hook.go

@@ -117,7 +117,7 @@ func (e *WebHook) ExecuteSettingsPostPersistHook(_ http.ResponseWriter, req *htt
 }
 
 func (e *WebHook) execute(ctx context.Context, data *templateContext) error {
-	builder, err := request.NewBuilder(e.conf, e.deps.Logger())
+	builder, err := request.NewBuilder(e.conf, e.deps.HTTPClient(ctx), e.deps.Logger())
 	if err != nil {
 		return err
 	}

test/schema/fixtures/config.schema.test.success/root.courierSMS.yaml

@@ -0,0 +1,23 @@
+selfservice:
+  default_browser_return_url: "#/definitions/defaultReturnTo"
+
+dsn: foo
+
+identity:
+  schemas:
+    - id: default
+      url: https://example.com
+
+courier:
+  smtp:
+    connection_uri: smtps://foo:bar@my-mailserver:1234/
+    from_address: [email protected]
+  sms:
+    enabled: true
+    from: "+19592155527"
+    request_config:
+      url: https://sms.example.com
+      method: POST
+      body: file://request.config.twilio.jsonnet
+      header:
+        'Content-Type': "application/x-www-form-urlencoded"

x/require.go

@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"testing"
 
-	"github.com/gofrs/uuid"
 	"github.com/stretchr/testify/require"
 )
 
@@ -14,7 +13,3 @@ func RequireJSONMarshal(t *testing.T, in interface{}) []byte {
 	require.NoError(t, json.NewEncoder(&b).Encode(in))
 	return b.Bytes()
 }
-
-func RequireNotNilUUID(t *testing.T, id uuid.UUID) {
-	require.NotEqual(t, uuid.Nil, id)
-}