From 2639ddd03c1d973dd05c2242fb80ecd4fe759d2f Mon Sep 17 00:00:00 2001 From: "Derrick J. Wippler" Date: Tue, 21 May 2019 14:59:50 -0500 Subject: [PATCH] Fixed bugs in webhooks * Added notice in README about go dep bug. * Added endpoints for webhooks in mock server * Change names of some parameters on public methods to make their use clearer. * Changed signature of `GetWebhook()` now returns []string. * Changed signature of `ListWebhooks()` now returns map[string][]string. * Both `GetWebhooks()` and `ListWebhooks()` now handle new and legacy webhooks properly. --- CHANGELOG | 16 +++++++++ README.md | 10 ++++++ credentials.go | 12 +++---- examples/examples.go | 4 +-- mailgun.go | 28 +++++++-------- mock.go | 2 ++ mock_webhooks.go | 83 ++++++++++++++++++++++++++++++++++++++++++++ webhooks.go | 72 +++++++++++++++++++++++++------------- webhooks_test.go | 53 ++++++++++++++++++---------- 9 files changed, 214 insertions(+), 66 deletions(-) create mode 100644 mock_webhooks.go diff --git a/CHANGELOG b/CHANGELOG index 0e452053..9c83c783 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,22 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [3.5.0] - 2019-05-21 +### Added +* Added notice in README about go dep bug. +* Added endpoints for webhooks in mock server +### Changes +* Change names of some parameters on public methods to make their use clearer. +* Changed signature of `GetWebhook()` now returns []string. +* Changed signature of `ListWebhooks()` now returns map[string][]string. +* Both `GetWebhooks()` and `ListWebhooks()` now handle new and legacy webhooks properly. + +## [3.4.0] - 2019-04-23 +### Added +* Added `Message.SetTemplate()` to allow sending with the body of a template. +### Changes +* Changed signature of `CreateDomain()` moved password into `CreateDomainOptions` + ## [3.4.0] - 2019-04-23 ### Added * Added `Message.SetTemplate()` to allow sending with the body of a template. diff --git a/README.md b/README.md index c9c7c3ab..995889f9 100644 --- a/README.md +++ b/README.md @@ -275,6 +275,16 @@ in your code should work fine even if you do not have golang modules enabled for $ go get github.com/mailgun/mailgun-go ``` +**NOTE for go dep users** + +Using version 3 of the mailgun-go library with go dep currently results in the following error +``` +"github.com/mailgun/mailgun-go/v3/events", which contains malformed code: no package exists at ... +``` +This is a known bug in go dep. You can follow the PR to fix this bug [here](https://github.com/golang/dep/pull/1963) +Until this bug is fixed, the best way to use version 3 of the mailgun-go library is to use the golang community +supported [golang modules](https://github.com/golang/go/wiki/Modules). + ## Testing *WARNING* - running the tests will cost you money! diff --git a/credentials.go b/credentials.go index e62aee60..b13fe18d 100644 --- a/credentials.go +++ b/credentials.go @@ -190,11 +190,11 @@ func (mg *MailgunImpl) CreateCredential(ctx context.Context, login, password str } // ChangeCredentialPassword attempts to alter the indicated credential's password. -func (mg *MailgunImpl) ChangeCredentialPassword(ctx context.Context, id, password string) error { - if (id == "") || (password == "") { +func (mg *MailgunImpl) ChangeCredentialPassword(ctx context.Context, login, password string) error { + if (login == "") || (password == "") { return ErrEmptyParam } - r := newHTTPRequest(generateCredentialsUrl(mg, id)) + r := newHTTPRequest(generateCredentialsUrl(mg, login)) r.setClient(mg.Client()) r.setBasicAuth(basicAuthUser, mg.APIKey()) p := newUrlEncodedPayload() @@ -204,11 +204,11 @@ func (mg *MailgunImpl) ChangeCredentialPassword(ctx context.Context, id, passwor } // DeleteCredential attempts to remove the indicated principle from the domain. -func (mg *MailgunImpl) DeleteCredential(ctx context.Context, id string) error { - if id == "" { +func (mg *MailgunImpl) DeleteCredential(ctx context.Context, login string) error { + if login == "" { return ErrEmptyParam } - r := newHTTPRequest(generateCredentialsUrl(mg, id)) + r := newHTTPRequest(generateCredentialsUrl(mg, login)) r.setClient(mg.Client()) r.setBasicAuth(basicAuthUser, mg.APIKey()) _, err := makeDeleteRequest(ctx, r) diff --git a/examples/examples.go b/examples/examples.go index 81a238f6..048e810b 100644 --- a/examples/examples.go +++ b/examples/examples.go @@ -626,7 +626,7 @@ func ValidateEmail(apiKey string) (mailgun.EmailVerification, error) { return mv.ValidateEmail(ctx, "foo@mailgun.net", false) } -func GetWebhook(domain, apiKey string) (string, error) { +func GetWebhook(domain, apiKey string) ([]string, error) { mg := mailgun.NewMailgun(domain, apiKey) ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) @@ -635,7 +635,7 @@ func GetWebhook(domain, apiKey string) (string, error) { return mg.GetWebhook(ctx, "clicked") } -func ListWebhooks(domain, apiKey string) (map[string]string, error) { +func ListWebhooks(domain, apiKey string) (map[string][]string, error) { mg := mailgun.NewMailgun(domain, apiKey) ctx, cancel := context.WithTimeout(context.Background(), time.Second*30) diff --git a/mailgun.go b/mailgun.go index a786de2c..0349b3ca 100644 --- a/mailgun.go +++ b/mailgun.go @@ -159,8 +159,8 @@ type Mailgun interface { ListCredentials(opts *ListOptions) *CredentialsIterator CreateCredential(ctx context.Context, login, password string) error - ChangeCredentialPassword(ctx context.Context, id, password string) error - DeleteCredential(ctx context.Context, id string) error + ChangeCredentialPassword(ctx context.Context, login, password string) error + DeleteCredential(ctx context.Context, login string) error ListUnsubscribes(opts *ListOptions) *UnsubscribesIterator GetUnsubscribe(ctx context.Context, address string) (Unsubscribe, error) @@ -179,10 +179,10 @@ type Mailgun interface { DeleteRoute(ctx context.Context, address string) error UpdateRoute(ctx context.Context, address string, r Route) (Route, error) - ListWebhooks(ctx context.Context) (map[string]string, error) + ListWebhooks(ctx context.Context) (map[string][]string, error) CreateWebhook(ctx context.Context, kind string, url []string) error DeleteWebhook(ctx context.Context, kind string) error - GetWebhook(ctx context.Context, kind string) (string, error) + GetWebhook(ctx context.Context, kind string) ([]string, error) UpdateWebhook(ctx context.Context, kind string, url []string) error VerifyWebhookRequest(req *http.Request) (verified bool, err error) @@ -216,16 +216,16 @@ type Mailgun interface { GetTagLimits(ctx context.Context, domain string) (TagLimits, error) CreateTemplate(ctx context.Context, template *Template) error - GetTemplate(ctx context.Context, id string) (Template, error) + GetTemplate(ctx context.Context, name string) (Template, error) UpdateTemplate(ctx context.Context, template *Template) error - DeleteTemplate(ctx context.Context, id string) error + DeleteTemplate(ctx context.Context, name string) error ListTemplates(opts *ListTemplateOptions) *TemplatesIterator - AddTemplateVersion(ctx context.Context, templateId string, version *TemplateVersion) error - GetTemplateVersion(ctx context.Context, templateId, versionId string) (TemplateVersion, error) - UpdateTemplateVersion(ctx context.Context, templateId string, version *TemplateVersion) error - DeleteTemplateVersion(ctx context.Context, templateId, versionId string) error - ListTemplateVersions(templateId string, opts *ListOptions) *TemplateVersionsIterator + AddTemplateVersion(ctx context.Context, templateName string, version *TemplateVersion) error + GetTemplateVersion(ctx context.Context, templateName, tag string) (TemplateVersion, error) + UpdateTemplateVersion(ctx context.Context, templateName string, version *TemplateVersion) error + DeleteTemplateVersion(ctx context.Context, templateName, tag string) error + ListTemplateVersions(templateName string, opts *ListOptions) *TemplateVersionsIterator } // MailgunImpl bundles data needed by a large number of methods in order to interact with the Mailgun API. @@ -336,10 +336,10 @@ func generateDomainApiUrl(m Mailgun, endpoint string) string { // generateCredentialsUrl renders a URL as generateDomainApiUrl, // but focuses on the SMTP credentials family of API functions. -func generateCredentialsUrl(m Mailgun, id string) string { +func generateCredentialsUrl(m Mailgun, login string) string { tail := "" - if id != "" { - tail = fmt.Sprintf("/%s", id) + if login != "" { + tail = fmt.Sprintf("/%s", login) } return generateDomainApiUrl(m, fmt.Sprintf("credentials%s", tail)) // return fmt.Sprintf("%s/domains/%s/credentials%s", apiBase, m.Domain(), tail) diff --git a/mock.go b/mock.go index 482390b9..f9cf4555 100644 --- a/mock.go +++ b/mock.go @@ -24,6 +24,7 @@ type MockServer struct { mailingList []mailingListContainer routeList []Route events []Event + webhooks WebHooksListResponse } // Create a new instance of the mailgun API mock server @@ -42,6 +43,7 @@ func NewMockServer() MockServer { ms.addMessagesRoutes(r) ms.addValidationRoutes(r) ms.addRoutes(r) + ms.addWebhookRoutes(r) }) // Start the server diff --git a/mock_webhooks.go b/mock_webhooks.go new file mode 100644 index 00000000..82fa56cb --- /dev/null +++ b/mock_webhooks.go @@ -0,0 +1,83 @@ +package mailgun + +import ( + "net/http" + + "github.com/go-chi/chi" +) + +func (ms *MockServer) addWebhookRoutes(r chi.Router) { + r.Route("/domains/{domain}/webhooks", func(r chi.Router) { + r.Get("/", ms.listWebHooks) + r.Post("/", ms.postWebHook) + r.Get("/{webhook}", ms.getWebHook) + r.Put("/{webhook}", ms.putWebHook) + r.Delete("/{webhook}", ms.deleteWebHook) + }) + ms.webhooks = WebHooksListResponse{ + Webhooks: map[string]UrlOrUrls{ + "new-webhook": { + Urls: []string{"http://example.com/new"}, + }, + "legacy-webhook": { + Url: "http://example.com/legacy", + }, + }, + } +} + +func (ms *MockServer) listWebHooks(w http.ResponseWriter, _ *http.Request) { + toJSON(w, ms.webhooks) +} + +func (ms *MockServer) getWebHook(w http.ResponseWriter, r *http.Request) { + resp := WebHookResponse{ + Webhook: UrlOrUrls{ + Urls: ms.webhooks.Webhooks[chi.URLParam(r, "webhook")].Urls, + }, + } + toJSON(w, resp) +} + +func (ms *MockServer) postWebHook(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + w.WriteHeader(http.StatusBadRequest) + toJSON(w, okResp{Message: err.Error()}) + return + } + + var urls []string + for _, url := range r.Form["url"] { + urls = append(urls, url) + } + ms.webhooks.Webhooks[r.FormValue("id")] = UrlOrUrls{Urls: urls} + + toJSON(w, okResp{Message: "success"}) +} + +func (ms *MockServer) putWebHook(w http.ResponseWriter, r *http.Request) { + if err := r.ParseForm(); err != nil { + w.WriteHeader(http.StatusBadRequest) + toJSON(w, okResp{Message: err.Error()}) + return + } + + var urls []string + for _, url := range r.Form["url"] { + urls = append(urls, url) + } + ms.webhooks.Webhooks[chi.URLParam(r, "webhook")] = UrlOrUrls{Urls: urls} + + toJSON(w, okResp{Message: "success"}) +} + +func (ms *MockServer) deleteWebHook(w http.ResponseWriter, r *http.Request) { + _, ok := ms.webhooks.Webhooks[chi.URLParam(r, "webhook")] + if !ok { + w.WriteHeader(http.StatusNotFound) + toJSON(w, okResp{Message: "webhook not found"}) + } + + delete(ms.webhooks.Webhooks, chi.URLParam(r, "webhook")) + toJSON(w, okResp{Message: "success"}) +} diff --git a/webhooks.go b/webhooks.go index 68f34882..b6d9a4f5 100644 --- a/webhooks.go +++ b/webhooks.go @@ -6,41 +6,58 @@ import ( "crypto/sha256" "crypto/subtle" "encoding/hex" + "fmt" "io" "net/http" "github.com/mailgun/mailgun-go/v3/events" ) +type UrlOrUrls struct { + Urls []string `json:"urls"` + Url string `json:"url"` +} + +type WebHooksListResponse struct { + Webhooks map[string]UrlOrUrls `json:"webhooks"` +} + +type WebHookResponse struct { + Webhook UrlOrUrls `json:"webhook"` +} + // ListWebhooks returns the complete set of webhooks configured for your domain. // Note that a zero-length mapping is not an error. -func (mg *MailgunImpl) ListWebhooks(ctx context.Context) (map[string]string, error) { +func (mg *MailgunImpl) ListWebhooks(ctx context.Context) (map[string][]string, error) { r := newHTTPRequest(generateDomainApiUrl(mg, webhooksEndpoint)) r.setClient(mg.Client()) r.setBasicAuth(basicAuthUser, mg.APIKey()) - var envelope struct { - Webhooks map[string]interface{} `json:"webhooks"` - } - err := getResponseFromJSON(ctx, r, &envelope) - hooks := make(map[string]string, 0) + + var body WebHooksListResponse + err := getResponseFromJSON(ctx, r, &body) if err != nil { - return hooks, err + return nil, err } - for k, v := range envelope.Webhooks { - object := v.(map[string]interface{}) - url := object["url"] - hooks[k] = url.(string) + + hooks := make(map[string][]string, 0) + for k, v := range body.Webhooks { + if v.Url != "" { + hooks[k] = []string{v.Url} + } + if len(v.Urls) != 0 { + hooks[k] = append(hooks[k], v.Urls...) + } } return hooks, nil } // CreateWebhook installs a new webhook for your domain. -func (mg *MailgunImpl) CreateWebhook(ctx context.Context, t string, urls []string) error { +func (mg *MailgunImpl) CreateWebhook(ctx context.Context, kind string, urls []string) error { r := newHTTPRequest(generateDomainApiUrl(mg, webhooksEndpoint)) r.setClient(mg.Client()) r.setBasicAuth(basicAuthUser, mg.APIKey()) p := newUrlEncodedPayload() - p.addValue("id", t) + p.addValue("id", kind) for _, url := range urls { p.addValue("url", url) } @@ -49,8 +66,8 @@ func (mg *MailgunImpl) CreateWebhook(ctx context.Context, t string, urls []strin } // DeleteWebhook removes the specified webhook from your domain's configuration. -func (mg *MailgunImpl) DeleteWebhook(ctx context.Context, t string) error { - r := newHTTPRequest(generateDomainApiUrl(mg, webhooksEndpoint) + "/" + t) +func (mg *MailgunImpl) DeleteWebhook(ctx context.Context, kind string) error { + r := newHTTPRequest(generateDomainApiUrl(mg, webhooksEndpoint) + "/" + kind) r.setClient(mg.Client()) r.setBasicAuth(basicAuthUser, mg.APIKey()) _, err := makeDeleteRequest(ctx, r) @@ -58,22 +75,27 @@ func (mg *MailgunImpl) DeleteWebhook(ctx context.Context, t string) error { } // GetWebhook retrieves the currently assigned webhook URL associated with the provided type of webhook. -func (mg *MailgunImpl) GetWebhook(ctx context.Context, t string) (string, error) { - r := newHTTPRequest(generateDomainApiUrl(mg, webhooksEndpoint) + "/" + t) +func (mg *MailgunImpl) GetWebhook(ctx context.Context, kind string) ([]string, error) { + r := newHTTPRequest(generateDomainApiUrl(mg, webhooksEndpoint) + "/" + kind) r.setClient(mg.Client()) r.setBasicAuth(basicAuthUser, mg.APIKey()) - var envelope struct { - Webhook struct { - Url string `json:"url"` - } `json:"webhook"` + var body WebHookResponse + if err := getResponseFromJSON(ctx, r, &body); err != nil { + return nil, err + } + + if body.Webhook.Url != "" { + return []string{body.Webhook.Url}, nil + } + if len(body.Webhook.Urls) != 0 { + return body.Webhook.Urls, nil } - err := getResponseFromJSON(ctx, r, &envelope) - return envelope.Webhook.Url, err + return nil, fmt.Errorf("webhook '%s' returned no urls", kind) } // UpdateWebhook replaces one webhook setting for another. -func (mg *MailgunImpl) UpdateWebhook(ctx context.Context, t string, urls []string) error { - r := newHTTPRequest(generateDomainApiUrl(mg, webhooksEndpoint) + "/" + t) +func (mg *MailgunImpl) UpdateWebhook(ctx context.Context, kind string, urls []string) error { + r := newHTTPRequest(generateDomainApiUrl(mg, webhooksEndpoint) + "/" + kind) r.setClient(mg.Client()) r.setBasicAuth(basicAuthUser, mg.APIKey()) p := newUrlEncodedPayload() diff --git a/webhooks_test.go b/webhooks_test.go index bd17032e..30db4873 100644 --- a/webhooks_test.go +++ b/webhooks_test.go @@ -1,4 +1,4 @@ -package mailgun +package mailgun_test import ( "bytes" @@ -14,27 +14,43 @@ import ( "testing" "github.com/facebookgo/ensure" + "github.com/mailgun/mailgun-go/v3" ) -func TestWebhookCRUD(t *testing.T) { - if reason := SkipNetworkTest(); reason != "" { - t.Skip(reason) - } +func TestGetWebhook(t *testing.T) { + mg := mailgun.NewMailgun(testDomain, testKey) + mg.SetAPIBase(server.URL()) - mg, err := NewMailgunFromEnv() + ctx := context.Background() + list, err := mg.ListWebhooks(ctx) ensure.Nil(t, err) + ensure.DeepEqual(t, len(list), 2) + + urls, err := mg.GetWebhook(ctx, "new-webhook") + ensure.Nil(t, err) + + ensure.DeepEqual(t, urls, []string{"http://example.com/new"}) +} + +func TestWebhookCRUD(t *testing.T) { + mg := mailgun.NewMailgun(testDomain, testKey) + mg.SetAPIBase(server.URL()) + ctx := context.Background() + list, err := mg.ListWebhooks(ctx) + ensure.Nil(t, err) + ensure.DeepEqual(t, len(list), 2) var countHooks = func() int { hooks, err := mg.ListWebhooks(ctx) ensure.Nil(t, err) return len(hooks) } - hookCount := countHooks() - domainURL := "http://api.mailgun.net" - ensure.Nil(t, mg.CreateWebhook(ctx, "deliver", []string{domainURL})) + webHookURLs := []string{"http://api.mailgun.net/webhook"} + ensure.Nil(t, mg.CreateWebhook(ctx, "deliver", webHookURLs)) + defer func() { ensure.Nil(t, mg.DeleteWebhook(ctx, "deliver")) newCount := countHooks() @@ -44,17 +60,16 @@ func TestWebhookCRUD(t *testing.T) { newCount := countHooks() ensure.False(t, newCount <= hookCount) - theURL, err := mg.GetWebhook(ctx, "deliver") + urls, err := mg.GetWebhook(ctx, "deliver") ensure.Nil(t, err) - ensure.DeepEqual(t, theURL, domainURL) + ensure.DeepEqual(t, urls, webHookURLs) - updatedDomainURL := "http://api.mailgun.net/messages" - ensure.Nil(t, mg.UpdateWebhook(ctx, "deliver", []string{updatedDomainURL})) + updatedWebHookURL := []string{"http://api.mailgun.net/messages"} + ensure.Nil(t, mg.UpdateWebhook(ctx, "deliver", updatedWebHookURL)) hooks, err := mg.ListWebhooks(ctx) ensure.Nil(t, err) - - ensure.DeepEqual(t, hooks["deliver"], updatedDomainURL) + ensure.DeepEqual(t, hooks["deliver"], updatedWebHookURL) } var signedTests = []bool{ @@ -63,11 +78,11 @@ var signedTests = []bool{ } func TestVerifyWebhookSignature(t *testing.T) { - mg := NewMailgun(exampleDomain, exampleAPIKey) + mg := mailgun.NewMailgun(testDomain, testKey) for _, v := range signedTests { fields := getSignatureFields(mg.APIKey(), v) - sig := Signature{ + sig := mailgun.Signature{ TimeStamp: fields["timestamp"], Token: fields["token"], Signature: fields["signature"], @@ -83,7 +98,7 @@ func TestVerifyWebhookSignature(t *testing.T) { } func TestVerifyWebhookRequest_Form(t *testing.T) { - mg := NewMailgun(exampleDomain, exampleAPIKey) + mg := mailgun.NewMailgun(testDomain, testKey) for _, v := range signedTests { fields := getSignatureFields(mg.APIKey(), v) @@ -99,7 +114,7 @@ func TestVerifyWebhookRequest_Form(t *testing.T) { } func TestVerifyWebhookRequest_MultipartForm(t *testing.T) { - mg := NewMailgun(exampleDomain, exampleAPIKey) + mg := mailgun.NewMailgun(testDomain, testKey) for _, v := range signedTests { fields := getSignatureFields(mg.APIKey(), v)