acme-dns: use new registred account (#2445)

Author: ldez

providers/dns/acmedns/acmedns.go

@@ -178,7 +178,7 @@ func (d *DNSProvider) Present(domain, _, keyAuth string) error {
 
 		// The account did not exist.
 		// Create a new one and return an error indicating the required one-time manual CNAME setup.
-		err = d.register(ctx, domain, info.FQDN)
+		account, err = d.register(ctx, domain, info.FQDN)
 		if err != nil {
 			return err
 		}
@@ -200,10 +200,10 @@ func (d *DNSProvider) CleanUp(_, _, _ string) error {
 // If account creation works as expected a ErrCNAMERequired error is returned describing
 // the one-time manual CNAME setup required to complete setup of the ACME-DNS hook for the domain.
 // If any other error occurs it is returned as-is.
-func (d *DNSProvider) register(ctx context.Context, domain, fqdn string) error {
+func (d *DNSProvider) register(ctx context.Context, domain, fqdn string) (goacmedns.Account, error) {
 	newAcct, err := d.client.RegisterAccount(ctx, d.config.AllowList)
 	if err != nil {
-		return err
+		return goacmedns.Account{}, err
 	}
 
 	var cnameCreated bool
@@ -213,23 +213,23 @@ func (d *DNSProvider) register(ctx context.Context, domain, fqdn string) error {
 	if err != nil {
 		cnameCreated = errors.Is(err, internal.ErrCNAMEAlreadyCreated)
 		if !cnameCreated {
-			return err
+			return goacmedns.Account{}, err
 		}
 	}
 
 	err = d.storage.Save(ctx)
 	if err != nil {
-		return err
+		return goacmedns.Account{}, err
 	}
 
 	if cnameCreated {
-		return nil
+		return newAcct, nil
 	}
 
 	// Stop issuance by returning an error.
 	// The user needs to perform a manual one-time CNAME setup in their DNS zone
 	// to complete the setup of the new account we created.
-	return ErrCNAMERequired{
+	return goacmedns.Account{}, ErrCNAMERequired{
 		Domain: domain,
 		FQDN:   fqdn,
 		Target: newAcct.FullDomain,

providers/dns/acmedns/acmedns_test.go

@@ -2,6 +2,8 @@ package acmedns
 
 import (
 	"context"
+	"net/http"
+	"net/http/httptest"
 	"testing"
 
 	"github.com/nrdcg/goacmedns"
@@ -10,27 +12,17 @@ import (
 )
 
 const (
-	// Fixed test data for unit tests.
 	egDomain  = "example.com"
 	egFQDN    = "_acme-challenge." + egDomain + "."
 	egKeyAuth = "⚷"
 )
 
-// TestPresent tests that the ACME-DNS Present function for updating a DNS-01
-// challenge response TXT record works as expected.
 func TestPresent(t *testing.T) {
 	// validAccountStorage is a mockStorage configured to return the egTestAccount.
-	validAccountStorage := mockStorage{
-		map[string]goacmedns.Account{
-			egDomain: egTestAccount,
-		},
-	}
-	// validUpdateClient is a mockClient configured with the egTestAccount that will
-	// track TXT updates in a map.
-	validUpdateClient := mockUpdateClient{
-		mockClient{egTestAccount},
-		make(map[goacmedns.Account]string),
-	}
+	validAccountStorage := newMockStorage().WithAccount(egDomain, egTestAccount)
+
+	// validUpdateClient is a mockClient configured with the egTestAccount that will track TXT updates in a map.
+	validUpdateClient := newMockClient()
 
 	testCases := []struct {
 		Name          string
@@ -40,13 +32,13 @@ func TestPresent(t *testing.T) {
 	}{
 		{
 			Name:          "present when client storage returns unexpected error",
-			Client:        mockClient{egTestAccount},
-			Storage:       errorFetchStorage{},
+			Client:        newMockClient().WithRegisterAccount(egTestAccount),
+			Storage:       newMockStorage().WithFetchError(errorStorageErr),
 			ExpectedError: errorStorageErr,
 		},
 		{
 			Name:   "present when client storage returns ErrDomainNotFound",
-			Client: mockClient{egTestAccount},
+			Client: newMockClient().WithRegisterAccount(egTestAccount),
 			ExpectedError: ErrCNAMERequired{
 				Domain: egDomain,
 				FQDN:   egFQDN,
@@ -55,7 +47,7 @@ func TestPresent(t *testing.T) {
 		},
 		{
 			Name:          "present when client UpdateTXTRecord returns unexpected error",
-			Client:        errorUpdateClient{},
+			Client:        newMockClient().WithUpdateTXTRecordError(errorClientErr),
 			Storage:       validAccountStorage,
 			ExpectedError: errorClientErr,
 		},
@@ -71,16 +63,13 @@ func TestPresent(t *testing.T) {
 			p := &DNSProvider{
 				config:  NewDefaultConfig(),
 				client:  test.Client,
-				storage: mockStorage{make(map[string]goacmedns.Account)},
+				storage: newMockStorage(),
 			}
 
-			// override the storage mock if required by the test case.
 			if test.Storage != nil {
 				p.storage = test.Storage
 			}
 
-			// call Present. The token argument can be garbage because the ACME-DNS
-			// provider does not use it.
 			err := p.Present(egDomain, "foo", egKeyAuth)
 			if test.ExpectedError != nil {
 				assert.Equal(t, test.ExpectedError, err)
@@ -97,36 +86,33 @@ func TestPresent(t *testing.T) {
 	assert.Len(t, validUpdateClient.records[egTestAccount], 43)
 }
 
-// TestRegister tests that the ACME-DNS register function works correctly.
 func TestRegister(t *testing.T) {
 	testCases := []struct {
 		Name          string
 		Client        acmeDNSClient
 		Storage       goacmedns.Storage
-		Domain        string
-		FQDN          string
 		ExpectedError error
 	}{
 		{
 			Name:          "register when acme-dns client returns an error",
-			Client:        errorRegisterClient{},
+			Client:        newMockClient().WithRegisterAccountError(errorClientErr),
 			ExpectedError: errorClientErr,
 		},
 		{
 			Name:          "register when acme-dns storage put returns an error",
-			Client:        mockClient{egTestAccount},
-			Storage:       errorPutStorage{mockStorage{make(map[string]goacmedns.Account)}},
+			Client:        newMockClient().WithRegisterAccount(egTestAccount),
+			Storage:       newMockStorage().WithPutError(errorStorageErr),
 			ExpectedError: errorStorageErr,
 		},
 		{
 			Name:          "register when acme-dns storage save returns an error",
-			Client:        mockClient{egTestAccount},
-			Storage:       errorSaveStorage{mockStorage{make(map[string]goacmedns.Account)}},
+			Client:        newMockClient().WithRegisterAccount(egTestAccount),
+			Storage:       newMockStorage().WithSaveError(errorStorageErr),
 			ExpectedError: errorStorageErr,
 		},
 		{
 			Name:   "register when everything works",
-			Client: mockClient{egTestAccount},
+			Client: newMockClient().WithRegisterAccount(egTestAccount),
 			ExpectedError: ErrCNAMERequired{
 				Domain: egDomain,
 				FQDN:   egFQDN,
@@ -140,20 +126,128 @@ func TestRegister(t *testing.T) {
 			p := &DNSProvider{
 				config:  NewDefaultConfig(),
 				client:  test.Client,
-				storage: mockStorage{make(map[string]goacmedns.Account)},
+				storage: newMockStorage(),
 			}
 
-			// override the storage mock if required by the testcase.
 			if test.Storage != nil {
 				p.storage = test.Storage
 			}
 
-			// Call register for the example domain/fqdn.
-			err := p.register(context.Background(), egDomain, egFQDN)
+			acc, err := p.register(context.Background(), egDomain, egFQDN)
+			if test.ExpectedError != nil {
+				assert.Equal(t, test.ExpectedError, err)
+			} else {
+				assert.Equal(t, goacmedns.Account{}, acc)
+				require.NoError(t, err)
+			}
+		})
+	}
+}
+
+func TestPresent_httpStorage(t *testing.T) {
+	testCases := []struct {
+		desc          string
+		StatusCode    int
+		ExpectedError error
+	}{
+		{
+			desc:       "the CNAME is not handled by the storage",
+			StatusCode: http.StatusOK,
+			ExpectedError: ErrCNAMERequired{
+				Domain: egDomain,
+				FQDN:   egFQDN,
+				Target: egTestAccount.FullDomain,
+			},
+		},
+		{
+			desc:       "the CNAME is handled by the storage",
+			StatusCode: http.StatusCreated,
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.desc, func(t *testing.T) {
+			mux := http.NewServeMux()
+			server := httptest.NewServer(mux)
+
+			config := NewDefaultConfig()
+			config.StorageBaseURL = server.URL
+
+			p, err := NewDNSProviderConfig(config)
+			require.NoError(t, err)
+
+			client := newMockClient().WithRegisterAccount(egTestAccount)
+			p.client = client
+
+			// Fetch
+			mux.HandleFunc("GET /example.com", func(rw http.ResponseWriter, reg *http.Request) {
+				rw.WriteHeader(http.StatusNotFound)
+			})
+
+			// Put
+			mux.HandleFunc("POST /example.com", func(rw http.ResponseWriter, req *http.Request) {
+				rw.WriteHeader(test.StatusCode)
+			})
+
+			err = p.Present(egDomain, "foo", egKeyAuth)
+			if test.ExpectedError != nil {
+				assert.Equal(t, test.ExpectedError, err)
+				assert.True(t, client.registerAccountCalled)
+				assert.False(t, client.updateTXTRecordCalled)
+			} else {
+				require.NoError(t, err)
+				assert.True(t, client.registerAccountCalled)
+				assert.True(t, client.updateTXTRecordCalled)
+			}
+		})
+	}
+}
+
+func TestRegister_httpStorage(t *testing.T) {
+	testCases := []struct {
+		Name          string
+		StatusCode    int
+		ExpectedError error
+	}{
+		{
+			Name:       "status code 200",
+			StatusCode: http.StatusOK,
+			ExpectedError: ErrCNAMERequired{
+				Domain: egDomain,
+				FQDN:   egFQDN,
+				Target: egTestAccount.FullDomain,
+			},
+		},
+		{
+			Name:       "status code 201",
+			StatusCode: http.StatusCreated,
+		},
+	}
+
+	for _, test := range testCases {
+		t.Run(test.Name, func(t *testing.T) {
+			mux := http.NewServeMux()
+			server := httptest.NewServer(mux)
+
+			config := NewDefaultConfig()
+			config.StorageBaseURL = server.URL
+
+			p, err := NewDNSProviderConfig(config)
+			require.NoError(t, err)
+
+			p.client = newMockClient().WithRegisterAccount(egTestAccount)
+
+			// Put
+			mux.HandleFunc("POST /example.com", func(w http.ResponseWriter, r *http.Request) {
+				w.WriteHeader(test.StatusCode)
+			})
+
+			acc, err := p.register(context.Background(), egDomain, egFQDN)
 			if test.ExpectedError != nil {
 				assert.Equal(t, test.ExpectedError, err)
 			} else {
 				require.NoError(t, err)
+				assert.Equal(t, egTestAccount, acc)
 			}
 		})
 	}