namecheap: fix challenge (#661)

providers/dns/namecheap/client.go

@@ -1,11 +1,18 @@
 package namecheap
 
-import "encoding/xml"
+import (
+	"encoding/xml"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+	"net/url"
+	"strings"
+)
 
-// record describes a DNS record returned by the Namecheap DNS gethosts API.
+// Record describes a DNS record returned by the Namecheap DNS gethosts API.
 // Namecheap uses the term "host" to refer to all DNS records that include
 // a host field (A, AAAA, CNAME, NS, TXT, URL).
-type record struct {
+type Record struct {
 	Type    string `xml:",attr"`
 	Name    string `xml:",attr"`
 	Address string `xml:",attr"`
@@ -32,7 +39,7 @@ type getHostsResponse struct {
 	XMLName xml.Name   `xml:"ApiResponse"`
 	Status  string     `xml:"Status,attr"`
 	Errors  []apierror `xml:"Errors>Error"`
-	Hosts   []record   `xml:"CommandResponse>DomainDNSGetHostsResult>host"`
+	Hosts   []Record   `xml:"CommandResponse>DomainDNSGetHostsResult>host"`
 }
 
 type getTldsResponse struct {
@@ -42,3 +49,177 @@ type getTldsResponse struct {
 		Name string `xml:",attr"`
 	} `xml:"CommandResponse>Tlds>Tld"`
 }
+
+// getTLDs requests the list of available TLDs.
+// https://www.namecheap.com/support/api/methods/domains/get-tld-list.aspx
+func (d *DNSProvider) getTLDs() (map[string]string, error) {
+	request, err := d.newRequestGet("namecheap.domains.getTldList")
+	if err != nil {
+		return nil, err
+	}
+
+	var gtr getTldsResponse
+	err = d.do(request, &gtr)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(gtr.Errors) > 0 {
+		return nil, fmt.Errorf("%s [%d]", gtr.Errors[0].Description, gtr.Errors[0].Number)
+	}
+
+	tlds := make(map[string]string)
+	for _, t := range gtr.Result {
+		tlds[t.Name] = t.Name
+	}
+	return tlds, nil
+}
+
+// getHosts reads the full list of DNS host records.
+// https://www.namecheap.com/support/api/methods/domains-dns/get-hosts.aspx
+func (d *DNSProvider) getHosts(sld, tld string) ([]Record, error) {
+	request, err := d.newRequestGet("namecheap.domains.dns.getHosts",
+		addParam("SLD", sld),
+		addParam("TLD", tld),
+	)
+	if err != nil {
+		return nil, err
+	}
+
+	var ghr getHostsResponse
+	err = d.do(request, &ghr)
+	if err != nil {
+		return nil, err
+	}
+
+	if len(ghr.Errors) > 0 {
+		return nil, fmt.Errorf("%s [%d]", ghr.Errors[0].Description, ghr.Errors[0].Number)
+	}
+
+	return ghr.Hosts, nil
+}
+
+// setHosts writes the full list of DNS host records .
+// https://www.namecheap.com/support/api/methods/domains-dns/set-hosts.aspx
+func (d *DNSProvider) setHosts(sld, tld string, hosts []Record) error {
+	req, err := d.newRequestPost("namecheap.domains.dns.setHosts",
+		addParam("SLD", sld),
+		addParam("TLD", tld),
+		func(values url.Values) {
+			for i, h := range hosts {
+				ind := fmt.Sprintf("%d", i+1)
+				values.Add("HostName"+ind, h.Name)
+				values.Add("RecordType"+ind, h.Type)
+				values.Add("Address"+ind, h.Address)
+				values.Add("MXPref"+ind, h.MXPref)
+				values.Add("TTL"+ind, h.TTL)
+			}
+		},
+	)
+	if err != nil {
+		return err
+	}
+
+	var shr setHostsResponse
+	err = d.do(req, &shr)
+	if err != nil {
+		return err
+	}
+
+	if len(shr.Errors) > 0 {
+		return fmt.Errorf("%s [%d]", shr.Errors[0].Description, shr.Errors[0].Number)
+	}
+	if shr.Result.IsSuccess != "true" {
+		return fmt.Errorf("setHosts failed")
+	}
+
+	return nil
+}
+
+func (d *DNSProvider) do(req *http.Request, out interface{}) error {
+	resp, err := d.config.HTTPClient.Do(req)
+	if err != nil {
+		return err
+	}
+
+	if resp.StatusCode >= 400 {
+		var body []byte
+		body, err = readBody(resp)
+		if err != nil {
+			return fmt.Errorf("HTTP error %d [%s]: %v", resp.StatusCode, http.StatusText(resp.StatusCode), err)
+		}
+		return fmt.Errorf("HTTP error %d [%s]: %s", resp.StatusCode, http.StatusText(resp.StatusCode), string(body))
+	}
+
+	body, err := readBody(resp)
+	if err != nil {
+		return err
+	}
+
+	if err := xml.Unmarshal(body, out); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (d *DNSProvider) newRequestGet(cmd string, params ...func(url.Values)) (*http.Request, error) {
+	query := d.makeQuery(cmd, params...)
+
+	reqURL, err := url.Parse(d.config.BaseURL)
+	if err != nil {
+		return nil, err
+	}
+
+	reqURL.RawQuery = query.Encode()
+
+	return http.NewRequest(http.MethodGet, reqURL.String(), nil)
+}
+
+func (d *DNSProvider) newRequestPost(cmd string, params ...func(url.Values)) (*http.Request, error) {
+	query := d.makeQuery(cmd, params...)
+
+	req, err := http.NewRequest(http.MethodPost, d.config.BaseURL, strings.NewReader(query.Encode()))
+	if err != nil {
+		return nil, err
+	}
+	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
+
+	return req, nil
+}
+
+func (d *DNSProvider) makeQuery(cmd string, params ...func(url.Values)) url.Values {
+	queryParams := make(url.Values)
+	queryParams.Set("ApiUser", d.config.APIUser)
+	queryParams.Set("ApiKey", d.config.APIKey)
+	queryParams.Set("UserName", d.config.APIUser)
+	queryParams.Set("Command", cmd)
+	queryParams.Set("ClientIp", d.config.ClientIP)
+
+	for _, param := range params {
+		param(queryParams)
+	}
+
+	return queryParams
+}
+
+func addParam(key, value string) func(url.Values) {
+	return func(values url.Values) {
+		values.Set(key, value)
+	}
+}
+
+func readBody(resp *http.Response) ([]byte, error) {
+	if resp.Body == nil {
+		return nil, fmt.Errorf("response body is nil")
+	}
+
+	defer resp.Body.Close()
+
+	rawBody, err := ioutil.ReadAll(resp.Body)
+	if err != nil {
+		return nil, err
+	}
+
+	return rawBody, nil
+}

providers/dns/namecheap/namecheap.go

@@ -3,12 +3,10 @@
 package namecheap
 
 import (
-	"encoding/xml"
 	"errors"
 	"fmt"
 	"io/ioutil"
 	"net/http"
-	"net/url"
 	"strconv"
 	"strings"
 	"time"
@@ -147,22 +145,28 @@ func (d *DNSProvider) Present(domain, token, keyAuth string) error {
 		return fmt.Errorf("namecheap: %v", err)
 	}
 
-	hosts, err := d.getHosts(ch)
+	records, err := d.getHosts(ch.sld, ch.tld)
 	if err != nil {
 		return fmt.Errorf("namecheap: %v", err)
 	}
 
-	d.addChallengeRecord(ch, &hosts)
+	record := Record{
+		Name:    ch.key,
+		Type:    "TXT",
+		Address: ch.keyValue,
+		MXPref:  "10",
+		TTL:     strconv.Itoa(d.config.TTL),
+	}
+
+	records = append(records, record)
 
 	if d.config.Debug {
-		for _, h := range hosts {
+		for _, h := range records {
-			log.Printf(
+			log.Printf("%-5.5s %-30.30s %-6s %-70.70s", h.Type, h.Name, h.TTL, h.Address)
-				"%-5.5s %-30.30s %-6s %-70.70s\n",
-				h.Type, h.Name, h.TTL, h.Address)
 		}
 	}
 
-	err = d.setHosts(ch, hosts)
+	err = d.setHosts(ch.sld, ch.tld, records)
 	if err != nil {
 		return fmt.Errorf("namecheap: %v", err)
 	}
@@ -181,16 +185,25 @@ func (d *DNSProvider) CleanUp(domain, token, keyAuth string) error {
 		return fmt.Errorf("namecheap: %v", err)
 	}
 
-	hosts, err := d.getHosts(ch)
+	records, err := d.getHosts(ch.sld, ch.tld)
 	if err != nil {
 		return fmt.Errorf("namecheap: %v", err)
 	}
 
-	if removed := d.removeChallengeRecord(ch, &hosts); !removed {
+	// Find the challenge TXT record and remove it if found.
+	var found bool
+	for i, h := range records {
+		if h.Name == ch.key && h.Type == "TXT" {
+			records = append(records[:i], records[i+1:]...)
+			found = true
+		}
+	}
+
+	if !found {
 		return nil
 	}
 
-	err = d.setHosts(ch, hosts)
+	err = d.setHosts(ch.sld, ch.tld, records)
 	if err != nil {
 		return fmt.Errorf("namecheap: %v", err)
 	}
@@ -243,189 +256,15 @@ func newChallenge(domain, keyAuth string, tlds map[string]string) (*challenge, e
 		host = strings.Join(parts[:longest-1], ".")
 	}
 
-	key, keyValue, _ := acme.DNS01Record(domain, keyAuth)
+	fqdn, value, _ := acme.DNS01Record(domain, keyAuth)
 
 	return &challenge{
 		domain:   domain,
 		key:      "_acme-challenge." + host,
-		keyFqdn:  key,
+		keyFqdn:  fqdn,
-		keyValue: keyValue,
+		keyValue: value,
 		tld:      tld,
 		sld:      sld,
 		host:     host,
 	}, nil
 }
-
-// setGlobalParams adds the namecheap global parameters to the provided url
-// Values record.
-func (d *DNSProvider) setGlobalParams(v *url.Values, cmd string) {
-	v.Set("ApiUser", d.config.APIUser)
-	v.Set("ApiKey", d.config.APIKey)
-	v.Set("UserName", d.config.APIUser)
-	v.Set("Command", cmd)
-	v.Set("ClientIp", d.config.ClientIP)
-}
-
-// getTLDs requests the list of available TLDs from namecheap.
-func (d *DNSProvider) getTLDs() (tlds map[string]string, err error) {
-	values := make(url.Values)
-	d.setGlobalParams(&values, "namecheap.domains.getTldList")
-
-	reqURL, err := url.Parse(d.config.BaseURL)
-	if err != nil {
-		return nil, err
-	}
-	reqURL.RawQuery = values.Encode()
-
-	resp, err := d.config.HTTPClient.Get(reqURL.String())
-	if err != nil {
-		return nil, err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode >= 400 {
-		return nil, fmt.Errorf("getHosts HTTP error %d", resp.StatusCode)
-	}
-
-	body, err := ioutil.ReadAll(resp.Body)
-	if err != nil {
-		return nil, err
-	}
-
-	var gtr getTldsResponse
-	if err := xml.Unmarshal(body, &gtr); err != nil {
-		return nil, err
-	}
-	if len(gtr.Errors) > 0 {
-		return nil, fmt.Errorf("%s [%d]", gtr.Errors[0].Description, gtr.Errors[0].Number)
-	}
-
-	tlds = make(map[string]string)
-	for _, t := range gtr.Result {
-		tlds[t.Name] = t.Name
-	}
-	return tlds, nil
-}
-
-// getHosts reads the full list of DNS host records using the Namecheap API.
-func (d *DNSProvider) getHosts(ch *challenge) (hosts []record, err error) {
-	values := make(url.Values)
-	d.setGlobalParams(&values, "namecheap.domains.dns.getHosts")
-
-	values.Set("SLD", ch.sld)
-	values.Set("TLD", ch.tld)
-
-	reqURL, err := url.Parse(d.config.BaseURL)
-	if err != nil {
-		return nil, err
-	}
-	reqURL.RawQuery = values.Encode()
-
-	resp, err := d.config.HTTPClient.Get(reqURL.String())
-	if err != nil {
-		return nil, err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode >= 400 {
-		return nil, fmt.Errorf("getHosts HTTP error %d", resp.StatusCode)
-	}
-
-	body, err := ioutil.ReadAll(resp.Body)
-	if err != nil {
-		return nil, err
-	}
-
-	var ghr getHostsResponse
-	if err = xml.Unmarshal(body, &ghr); err != nil {
-		return nil, err
-	}
-	if len(ghr.Errors) > 0 {
-		return nil, fmt.Errorf("%s [%d]", ghr.Errors[0].Description, ghr.Errors[0].Number)
-	}
-
-	return ghr.Hosts, nil
-}
-
-// setHosts writes the full list of DNS host records using the Namecheap API.
-func (d *DNSProvider) setHosts(ch *challenge, hosts []record) error {
-	values := make(url.Values)
-	d.setGlobalParams(&values, "namecheap.domains.dns.setHosts")
-
-	values.Set("SLD", ch.sld)
-	values.Set("TLD", ch.tld)
-
-	for i, h := range hosts {
-		ind := fmt.Sprintf("%d", i+1)
-		values.Add("HostName"+ind, h.Name)
-		values.Add("RecordType"+ind, h.Type)
-		values.Add("Address"+ind, h.Address)
-		values.Add("MXPref"+ind, h.MXPref)
-		values.Add("TTL"+ind, h.TTL)
-	}
-
-	resp, err := d.config.HTTPClient.PostForm(d.config.BaseURL, values)
-	if err != nil {
-		return err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode >= 400 {
-		return fmt.Errorf("setHosts HTTP error %d", resp.StatusCode)
-	}
-
-	body, err := ioutil.ReadAll(resp.Body)
-	if err != nil {
-		return err
-	}
-
-	var shr setHostsResponse
-	if err := xml.Unmarshal(body, &shr); err != nil {
-		return err
-	}
-	if len(shr.Errors) > 0 {
-		return fmt.Errorf("%s [%d]", shr.Errors[0].Description, shr.Errors[0].Number)
-	}
-	if shr.Result.IsSuccess != "true" {
-		return fmt.Errorf("setHosts failed")
-	}
-
-	return nil
-}
-
-// addChallengeRecord adds a DNS challenge TXT record to a list of namecheap
-// host records.
-func (d *DNSProvider) addChallengeRecord(ch *challenge, hosts *[]record) {
-	host := record{
-		Name:    ch.key,
-		Type:    "TXT",
-		Address: ch.keyValue,
-		MXPref:  "10",
-		TTL:     strconv.Itoa(d.config.TTL),
-	}
-
-	// If there's already a TXT record with the same name, replace it.
-	for i, h := range *hosts {
-		if h.Name == ch.key && h.Type == "TXT" {
-			(*hosts)[i] = host
-			return
-		}
-	}
-
-	// No record was replaced, so add a new one.
-	*hosts = append(*hosts, host)
-}
-
-// removeChallengeRecord removes a DNS challenge TXT record from a list of
-// namecheap host records. Return true if a record was removed.
-func (d *DNSProvider) removeChallengeRecord(ch *challenge, hosts *[]record) bool {
-	// Find the challenge TXT record and remove it if found.
-	for i, h := range *hosts {
-		if h.Name == ch.key && h.Type == "TXT" {
-			*hosts = append((*hosts)[:i], (*hosts)[i+1:]...)
-			return true
-		}
-	}
-
-	return false
-}

providers/dns/namecheap/namecheap_test.go

@@ -29,7 +29,7 @@ var (
 )
 
 func TestGetHosts(t *testing.T) {
-	for _, test := range testcases {
+	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {
 			mock := httptest.NewServer(http.HandlerFunc(
 				func(w http.ResponseWriter, r *http.Request) {
@@ -47,8 +47,10 @@ func TestGetHosts(t *testing.T) {
 			provider, err := NewDNSProviderConfig(config)
 			require.NoError(t, err)
 
-			ch, _ := newChallenge(test.domain, "", tlds)
+			ch, err := newChallenge(test.domain, "", tlds)
-			hosts, err := provider.getHosts(ch)
+			require.NoError(t, err)
+
+			hosts, err := provider.getHosts(ch.sld, ch.tld)
 			if test.errString != "" {
 				assert.EqualError(t, err, test.errString)
 			} else {
@@ -79,7 +81,7 @@ func TestGetHosts(t *testing.T) {
 }
 
 func TestSetHosts(t *testing.T) {
-	for _, test := range testcases {
+	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {
 			mock := httptest.NewServer(http.HandlerFunc(
 				func(w http.ResponseWriter, r *http.Request) {
@@ -88,8 +90,11 @@ func TestSetHosts(t *testing.T) {
 			defer mock.Close()
 
 			prov := mockDNSProvider(mock.URL)
-			ch, _ := newChallenge(test.domain, "", tlds)
+
-			hosts, err := prov.getHosts(ch)
+			ch, err := newChallenge(test.domain, "", tlds)
+			require.NoError(t, err)
+
+			hosts, err := prov.getHosts(ch.sld, ch.tld)
 			if test.errString != "" {
 				assert.EqualError(t, err, test.errString)
 			} else {
@@ -99,14 +104,14 @@ func TestSetHosts(t *testing.T) {
 				return
 			}
 
-			err = prov.setHosts(ch, hosts)
+			err = prov.setHosts(ch.sld, ch.tld, hosts)
 			require.NoError(t, err)
 		})
 	}
 }
 
 func TestPresent(t *testing.T) {
-	for _, test := range testcases {
+	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {
 			mock := httptest.NewServer(http.HandlerFunc(
 				func(w http.ResponseWriter, r *http.Request) {
@@ -126,7 +131,7 @@ func TestPresent(t *testing.T) {
 }
 
 func TestCleanUp(t *testing.T) {
-	for _, test := range testcases {
+	for _, test := range testCases {
 		t.Run(test.name, func(t *testing.T) {
 			mock := httptest.NewServer(http.HandlerFunc(
 				func(w http.ResponseWriter, r *http.Request) {
@@ -145,7 +150,7 @@ func TestCleanUp(t *testing.T) {
 	}
 }
 
-func TestNamecheapDomainSplit(t *testing.T) {
+func TestDomainSplit(t *testing.T) {
 	tests := []struct {
 		domain string
 		valid  bool
@@ -202,7 +207,7 @@ func assertEq(t *testing.T, variable, got, want string) {
 	}
 }
 
-func assertHdr(tc *testcase, t *testing.T, values *url.Values) {
+func assertHdr(tc *testCase, t *testing.T, values *url.Values) {
 	ch, _ := newChallenge(tc.domain, "", tlds)
 
 	assertEq(t, "ApiUser", values.Get("ApiUser"), fakeUser)
@@ -213,7 +218,7 @@ func assertHdr(tc *testcase, t *testing.T, values *url.Values) {
 	assertEq(t, "TLD", values.Get("TLD"), ch.tld)
 }
 
-func mockServer(tc *testcase, t *testing.T, w http.ResponseWriter, r *http.Request) {
+func mockServer(tc *testCase, t *testing.T, w http.ResponseWriter, r *http.Request) {
 	switch r.Method {
 
 	case http.MethodGet:
@@ -261,24 +266,27 @@ func mockDNSProvider(url string) *DNSProvider {
 	config.ClientIP = fakeClientIP
 	config.HTTPClient = &http.Client{Timeout: 60 * time.Second}
 
-	provider, _ := NewDNSProviderConfig(config)
+	provider, err := NewDNSProviderConfig(config)
+	if err != nil {
+		panic(err)
+	}
 	return provider
 }
 
-type testcase struct {
+type testCase struct {
 	name             string
 	domain           string
-	hosts            []record
+	hosts            []Record
 	errString        string
 	getHostsResponse string
 	setHostsResponse string
 }
 
-var testcases = []testcase{
+var testCases = []testCase{
 	{
 		name:   "Test:Success:1",
 		domain: "test.example.com",
-		hosts: []record{
+		hosts: []Record{
 			{Type: "A", Name: "home", Address: "10.0.0.1", MXPref: "10", TTL: "1799"},
 			{Type: "A", Name: "www", Address: "10.0.0.2", MXPref: "10", TTL: "1200"},
 			{Type: "AAAA", Name: "a", Address: "::0", MXPref: "10", TTL: "1799"},
@@ -292,7 +300,7 @@ var testcases = []testcase{
 	{
 		name:   "Test:Success:2",
 		domain: "example.com",
-		hosts: []record{
+		hosts: []Record{
 			{Type: "A", Name: "@", Address: "10.0.0.2", MXPref: "10", TTL: "1200"},
 			{Type: "A", Name: "www", Address: "10.0.0.3", MXPref: "10", TTL: "60"},
 		},