From 20f8d950c473071ad410f981213c7d67e98260a1 Mon Sep 17 00:00:00 2001
From: max furman <mx.furman@gmail.com>
Date: Fri, 18 Dec 2020 11:17:12 -0500
Subject: [PATCH] Fix broken ValidateChallenge test

---
 acme/authority_test.go | 42 ++++++++++++++++++++++++++++++++++++------
 acme/challenge.go      |  5 ++++-
 acme/challenge_test.go | 16 ++++++++++++++++
 3 files changed, 56 insertions(+), 7 deletions(-)

diff --git a/acme/authority_test.go b/acme/authority_test.go
index d411ca06..961805fa 100644
--- a/acme/authority_test.go
+++ b/acme/authority_test.go
@@ -2,9 +2,14 @@ package acme
 
 import (
 	"context"
+	"crypto"
+	"encoding/base64"
 	"encoding/json"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"net/url"
+	"strings"
 	"testing"
 	"time"
 
@@ -1331,11 +1336,14 @@ func TestAuthorityValidateChallenge(t *testing.T) {
 	prov := newProv()
 	ctx := context.WithValue(context.Background(), ProvisionerContextKey, prov)
 	ctx = context.WithValue(ctx, BaseURLContextKey, "https://test.ca.smallstep.com:8080")
+
 	type test struct {
 		auth      *Authority
 		id, accID string
 		err       *Error
 		ch        challenge
+		jwk       *jose.JSONWebKey
+		server    *httptest.Server
 	}
 	tests := map[string]func(t *testing.T) test{
 		"fail/getChallenge-error": func(t *testing.T) test {
@@ -1375,8 +1383,24 @@ func TestAuthorityValidateChallenge(t *testing.T) {
 			}
 		},
 		"fail/validate-error": func(t *testing.T) test {
-			ch, err := newHTTPCh()
+			keyauth := "temp"
+			keyauthp := &keyauth
+			// Create test server that returns challenge auth
+			ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+				fmt.Fprintf(w, "%s\r\n", *keyauthp)
+			}))
+
+			ch, err := newHTTPChWithServer(strings.TrimPrefix(ts.URL, "http://"))
+			assert.FatalError(t, err)
+
+			jwk, _, err := jose.GenerateDefaultKeyPair([]byte("pass"))
+			assert.FatalError(t, err)
+
+			thumbprint, err := jwk.Thumbprint(crypto.SHA256)
 			assert.FatalError(t, err)
+			encPrint := base64.RawURLEncoding.EncodeToString(thumbprint)
+			*keyauthp = fmt.Sprintf("%s.%s", ch.getToken(), encPrint)
+
 			b, err := json.Marshal(ch)
 			assert.FatalError(t, err)
 			auth, err := NewAuthority(&db.MockNoSQLDB{
@@ -1393,10 +1417,12 @@ func TestAuthorityValidateChallenge(t *testing.T) {
 			}, "ca.smallstep.com", "acme", nil)
 			assert.FatalError(t, err)
 			return test{
-				auth:  auth,
-				id:    ch.getID(),
-				accID: ch.getAccountID(),
-				err:   ServerInternalErr(errors.New("error attempting challenge validation: error saving acme challenge: force")),
+				auth:   auth,
+				id:     ch.getID(),
+				accID:  ch.getAccountID(),
+				jwk:    jwk,
+				server: ts,
+				err:    ServerInternalErr(errors.New("error attempting challenge validation: error saving acme challenge: force")),
 			}
 		},
 		"ok": func(t *testing.T) test {
@@ -1427,7 +1453,11 @@ func TestAuthorityValidateChallenge(t *testing.T) {
 	for name, run := range tests {
 		t.Run(name, func(t *testing.T) {
 			tc := run(t)
-			if acmeCh, err := tc.auth.ValidateChallenge(ctx, tc.accID, tc.id, nil); err != nil {
+
+			if tc.server != nil {
+				defer tc.server.Close()
+			}
+			if acmeCh, err := tc.auth.ValidateChallenge(ctx, tc.accID, tc.id, tc.jwk); err != nil {
 				if assert.NotNil(t, tc.err) {
 					ae, ok := err.(*Error)
 					assert.True(t, ok)
diff --git a/acme/challenge.go b/acme/challenge.go
index a032bc00..6d2d13d1 100644
--- a/acme/challenge.go
+++ b/acme/challenge.go
@@ -238,7 +238,10 @@ func (bc *baseChallenge) validate(db nosql.DB, jwk *jose.JSONWebKey, vo validate
 func (bc *baseChallenge) storeError(db nosql.DB, err *Error) error {
 	clone := bc.clone()
 	clone.Error = err.ToACME()
-	return clone.save(db, bc)
+	if err := clone.save(db, bc); err != nil {
+		return ServerInternalErr(errors.Wrap(err, "failure saving error to acme challenge"))
+	}
+	return nil
 }
 
 // unmarshalChallenge unmarshals a challenge type into the correct sub-type.
diff --git a/acme/challenge_test.go b/acme/challenge_test.go
index c3d97f9f..87ec0c4c 100644
--- a/acme/challenge_test.go
+++ b/acme/challenge_test.go
@@ -69,6 +69,22 @@ func newHTTPCh() (challenge, error) {
 	return newHTTP01Challenge(mockdb, testOps)
 }
 
+func newHTTPChWithServer(host string) (challenge, error) {
+	mockdb := &db.MockNoSQLDB{
+		MCmpAndSwap: func(bucket, key, old, newval []byte) ([]byte, bool, error) {
+			return []byte("foo"), true, nil
+		},
+	}
+	return newHTTP01Challenge(mockdb, ChallengeOptions{
+		AccountID: "accID",
+		AuthzID:   "authzID",
+		Identifier: Identifier{
+			Type:  "", // will get set correctly depending on the "new.." method.
+			Value: host,
+		},
+	})
+}
+
 func TestNewHTTP01Challenge(t *testing.T) {
 	ops := ChallengeOptions{
 		AccountID: "accID",