gandiv5: fix DNS Challenge (#664)

providers/dns/gandiv5/client.go

@@ -1,18 +1,123 @@
 package gandiv5
 
-// types for JSON method calls and parameters
+import (
+	"bytes"
+	"encoding/json"
+	"fmt"
+	"io/ioutil"
+	"net/http"
+)
 
-type addFieldRequest struct {
+const apiKeyHeader = "X-Api-Key"
+
+// types for JSON responses with only a message
+type apiResponse struct {
+	Message string `json:"message"`
+	UUID    string `json:"uuid,omitempty"`
+}
+
+// Record TXT record representation
+type Record struct {
 	RRSetTTL    int      `json:"rrset_ttl"`
 	RRSetValues []string `json:"rrset_values"`
+	RRSetName   string   `json:"rrset_name,omitempty"`
+	RRSetType   string   `json:"rrset_type,omitempty"`
 }
 
-type deleteFieldRequest struct {
+func (d *DNSProvider) newRequest(method, resource string, body interface{}) (*http.Request, error) {
-	Delete bool `json:"delete"`
+	u := fmt.Sprintf("%s/%s", d.config.BaseURL, resource)
+
+	if body == nil {
+		req, err := http.NewRequest(method, u, nil)
+		if err != nil {
+			return nil, err
 		}
 
-// types for JSON responses
+		return req, nil
-
+	}
-type responseStruct struct {
+
-	Message string `json:"message"`
+	reqBody, err := json.Marshal(body)
+	if err != nil {
+		return nil, err
+	}
+
+	req, err := http.NewRequest(method, u, bytes.NewBuffer(reqBody))
+	if err != nil {
+		return nil, err
+	}
+
+	req.Header.Set("Content-Type", "application/json")
+
+	return req, nil
+}
+
+func (d *DNSProvider) do(req *http.Request, v interface{}) error {
+	if len(d.config.APIKey) > 0 {
+		req.Header.Set(apiKeyHeader, d.config.APIKey)
+	}
+
+	resp, err := d.config.HTTPClient.Do(req)
+	if err != nil {
+		return err
+	}
+
+	err = checkResponse(resp)
+	if err != nil {
+		return err
+	}
+
+	if v == nil {
+		return nil
+	}
+
+	raw, err := readBody(resp)
+	if err != nil {
+		return fmt.Errorf("failed to read body: %v", err)
+	}
+
+	if len(raw) > 0 {
+		err = json.Unmarshal(raw, v)
+		if err != nil {
+			return fmt.Errorf("unmarshaling error: %v: %s", err, string(raw))
+		}
+	}
+
+	return nil
+}
+
+func checkResponse(resp *http.Response) error {
+	if resp.StatusCode == 404 && resp.Request.Method == http.MethodGet {
+		return nil
+	}
+
+	if resp.StatusCode >= 400 {
+		data, err := readBody(resp)
+		if err != nil {
+			return fmt.Errorf("%d [%s] request failed: %v", resp.StatusCode, http.StatusText(resp.StatusCode), err)
+		}
+
+		message := &apiResponse{}
+		err = json.Unmarshal(data, message)
+		if err != nil {
+			return fmt.Errorf("%d [%s] request failed: %v: %s", resp.StatusCode, http.StatusText(resp.StatusCode), err, data)
+		}
+		return fmt.Errorf("%d [%s] request failed: %s", resp.StatusCode, http.StatusText(resp.StatusCode), message.Message)
+	}
+
+	return nil
+}
+
+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/gandiv5/gandiv5.go

@@ -3,8 +3,6 @@
 package gandiv5
 
 import (
-	"bytes"
-	"encoding/json"
 	"errors"
 	"fmt"
 	"net/http"
@@ -184,61 +182,75 @@ func (d *DNSProvider) Timeout() (timeout, interval time.Duration) {
 // functions to perform API actions
 
 func (d *DNSProvider) addTXTRecord(domain string, name string, value string, ttl int) error {
-	target := fmt.Sprintf("domains/%s/records/%s/TXT", domain, name)
+	// Get exiting values for the TXT records
-	response, err := d.sendRequest(http.MethodPut, target, addFieldRequest{
+	// Needed to create challenges for both wildcard and base name domains
-		RRSetTTL:    ttl,
+	txtRecord, err := d.getTXTRecord(domain, name)
-		RRSetValues: []string{value},
+	if err != nil {
-	})
-	if response != nil {
-		log.Infof("gandiv5: %s", response.Message)
-	}
 		return err
 	}
 
+	values := []string{value}
+	if len(txtRecord.RRSetValues) > 0 {
+		values = append(values, txtRecord.RRSetValues...)
+	}
+
+	target := fmt.Sprintf("domains/%s/records/%s/TXT", domain, name)
+
+	newRecord := &Record{RRSetTTL: ttl, RRSetValues: values}
+	req, err := d.newRequest(http.MethodPut, target, newRecord)
+	if err != nil {
+		return err
+	}
+
+	message := &apiResponse{}
+	err = d.do(req, message)
+	if err != nil {
+		return fmt.Errorf("unable to create TXT record for domain %s and name %s: %v", domain, name, err)
+	}
+
+	if message != nil && len(message.Message) > 0 {
+		log.Infof("API response: %s", message.Message)
+	}
+
+	return nil
+}
+
+func (d *DNSProvider) getTXTRecord(domain, name string) (*Record, error) {
+	target := fmt.Sprintf("domains/%s/records/%s/TXT", domain, name)
+
+	// Get exiting values for the TXT records
+	// Needed to create challenges for both wildcard and base name domains
+	req, err := d.newRequest(http.MethodGet, target, nil)
+	if err != nil {
+		return nil, err
+	}
+
+	txtRecord := &Record{}
+	err = d.do(req, txtRecord)
+	if err != nil {
+		return nil, fmt.Errorf("unable to get TXT records for domain %s and name %s: %v", domain, name, err)
+	}
+
+	return txtRecord, nil
+}
+
 func (d *DNSProvider) deleteTXTRecord(domain string, name string) error {
 	target := fmt.Sprintf("domains/%s/records/%s/TXT", domain, name)
-	response, err := d.sendRequest(http.MethodDelete, target, deleteFieldRequest{
+
-		Delete: true,
+	req, err := d.newRequest(http.MethodDelete, target, nil)
-	})
+	if err != nil {
-	if response != nil && response.Message == "" {
-		log.Infof("gandiv5: Zone record deleted")
-	}
 		return err
 	}
 
-func (d *DNSProvider) sendRequest(method string, resource string, payload interface{}) (*responseStruct, error) {
+	message := &apiResponse{}
-	url := fmt.Sprintf("%s/%s", d.config.BaseURL, resource)
+	err = d.do(req, message)
-
-	body, err := json.Marshal(payload)
 	if err != nil {
-		return nil, err
+		return fmt.Errorf("unable to delete TXT record for domain %s and name %s: %v", domain, name, err)
 	}
 
-	req, err := http.NewRequest(method, url, bytes.NewReader(body))
+	if message != nil && len(message.Message) > 0 {
-	if err != nil {
+		log.Infof("API response: %s", message.Message)
-		return nil, err
 	}
 
-	req.Header.Set("Content-Type", "application/json")
+	return nil
-	if len(d.config.APIKey) > 0 {
-		req.Header.Set("X-Api-Key", d.config.APIKey)
-	}
-
-	resp, err := d.config.HTTPClient.Do(req)
-	if err != nil {
-		return nil, err
-	}
-	defer resp.Body.Close()
-
-	if resp.StatusCode >= 400 {
-		return nil, fmt.Errorf("request failed with HTTP status code %d", resp.StatusCode)
-	}
-
-	var response responseStruct
-	err = json.NewDecoder(resp.Body).Decode(&response)
-	if err != nil && method != http.MethodDelete {
-		return nil, err
-	}
-
-	return &response, nil
 }

providers/dns/gandiv5/gandiv5_test.go

@@ -1,14 +1,15 @@
 package gandiv5
 
 import (
-	"io"
+	"fmt"
 	"io/ioutil"
 	"net/http"
 	"net/http/httptest"
 	"regexp"
-	"strings"
 	"testing"
 
+	"github.com/xenolf/lego/log"
+
 	"github.com/stretchr/testify/require"
 )
 
@@ -21,21 +22,49 @@ func TestDNSProvider(t *testing.T) {
 	require.NoError(t, err)
 
 	// start fake RPC server
-	fakeServer := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+	handler := http.NewServeMux()
-		require.Equal(t, "application/json", r.Header.Get("Content-Type"), "invalid content type")
+	handler.HandleFunc("/domains/example.com/records/_acme-challenge.abc.def/TXT", func(rw http.ResponseWriter, req *http.Request) {
+		log.Infof("request: %s %s", req.Method, req.URL)
 
-		req, errS := ioutil.ReadAll(r.Body)
+		if req.Header.Get(apiKeyHeader) == "" {
-		require.NoError(t, errS)
+			http.Error(rw, `{"message": "missing API key"}`, http.StatusUnauthorized)
+			return
+		}
 
-		req = regexpToken.ReplaceAllLiteral(req, []byte(`"rrset_values":["TOKEN"]`))
+		if req.Method == http.MethodPost && req.Header.Get("Content-Type") != "application/json" {
+			http.Error(rw, `{"message": "invalid content type"}`, http.StatusBadRequest)
+			return
+		}
 
-		resp, ok := serverResponses[string(req)]
+		body, errS := ioutil.ReadAll(req.Body)
-		require.True(t, ok, "Server response for request not found")
+		if errS != nil {
+			http.Error(rw, fmt.Sprintf(`{"message": "read body error: %v"}`, errS), http.StatusInternalServerError)
+			return
+		}
 
-		_, errS = io.Copy(w, strings.NewReader(resp))
+		body = regexpToken.ReplaceAllLiteral(body, []byte(`"rrset_values":["TOKEN"]`))
-		require.NoError(t, errS)
+
-	}))
+		responses, ok := serverResponses[req.Method]
-	defer fakeServer.Close()
+		if !ok {
+			http.Error(rw, fmt.Sprintf(`{"message": "Server response for request not found: %#q"}`, string(body)), http.StatusInternalServerError)
+			return
+		}
+
+		resp := responses[string(body)]
+
+		_, errS = rw.Write([]byte(resp))
+		if errS != nil {
+			http.Error(rw, fmt.Sprintf(`{"message": "failed to write response: %v"}`, errS), http.StatusInternalServerError)
+			return
+		}
+	})
+	handler.HandleFunc("/", func(rw http.ResponseWriter, req *http.Request) {
+		log.Infof("request: %s %s", req.Method, req.URL)
+		http.Error(rw, fmt.Sprintf(`{"message": "URL doesn't match: %s"}`, req.URL), http.StatusNotFound)
+	})
+
+	server := httptest.NewServer(handler)
+	defer server.Close()
 
 	// define function to override findZoneByFqdn with
 	fakeFindZoneByFqdn := func(fqdn string, nameserver []string) (string, error) {
@@ -44,7 +73,7 @@ func TestDNSProvider(t *testing.T) {
 
 	config := NewDefaultConfig()
 	config.APIKey = "123412341234123412341234"
-	config.BaseURL = fakeServer.URL
+	config.BaseURL = server.URL
 
 	provider, err := NewDNSProviderConfig(config)
 	require.NoError(t, err)
@@ -67,9 +96,14 @@ func TestDNSProvider(t *testing.T) {
 
 // serverResponses is the JSON Request->Response map used by the
 // fake JSON server.
-var serverResponses = map[string]string{
+var serverResponses = map[string]map[string]string{
-	// Present Request->Response (addTXTRecord)
+	http.MethodGet: {
+		``: `{"rrset_ttl":300,"rrset_values":[],"rrset_name":"_acme-challenge.abc.def","rrset_type":"TXT"}`,
+	},
+	http.MethodPut: {
 		`{"rrset_ttl":300,"rrset_values":["TOKEN"]}`: `{"message": "Zone Record Created"}`,
-	// CleanUp Request->Response (deleteTXTRecord)
+	},
-	`{"delete":true}`: ``,
+	http.MethodDelete: {
+		``: ``,
+	},
 }