From 976c8f82c67a3cadf7caaf802a7983f9ad050cb9 Mon Sep 17 00:00:00 2001
From: David Cowden <dcow@smallstep.com>
Date: Wed, 13 May 2020 07:55:38 -0700
Subject: [PATCH] acme/authority: Fix tests

Also, return early from ValidateChallenge if the challenge is already
valid. Interestingly, we aren't actually testing most of the
ValidateChallenge func, just the early error and return conditions. We
should add some more coverage here.
---
 acme/authority.go      |  8 ++++++++
 acme/authority_test.go | 20 ++++++++++++--------
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/acme/authority.go b/acme/authority.go
index cc1a6daa..9a522e8d 100644
--- a/acme/authority.go
+++ b/acme/authority.go
@@ -317,6 +317,14 @@ func (a *Authority) ValidateChallenge(p provisioner.Interface, accID, chID strin
 	if err != nil {
 		return nil, err
 	}
+	switch ch.getStatus() {
+	case StatusPending, StatusProcessing:
+		break
+	case StatusInvalid, StatusValid:
+		return ch.toACME(a.dir, p)
+	default:
+		panic("unknown challenge state: " + ch.getStatus())
+	}
 
 	// Validate the challenge belongs to the account owned by the requester.
 	if accID != ch.getAccountID() {
diff --git a/acme/authority_test.go b/acme/authority_test.go
index f798053b..ff6cec0a 100644
--- a/acme/authority_test.go
+++ b/acme/authority_test.go
@@ -1224,6 +1224,7 @@ func TestAuthorityValidateChallenge(t *testing.T) {
 				err:  ServerInternalErr(errors.Errorf("error loading challenge %s: force", id)),
 			}
 		},
+
 		"fail/challenge-not-owned-by-account": func(t *testing.T) test {
 			ch, err := newHTTPCh()
 			assert.FatalError(t, err)
@@ -1244,6 +1245,7 @@ func TestAuthorityValidateChallenge(t *testing.T) {
 				err:   UnauthorizedErr(errors.New("account does not own challenge")),
 			}
 		},
+
 		"fail/validate-error": func(t *testing.T) test {
 			ch, err := newHTTPCh()
 			assert.FatalError(t, err)
@@ -1269,15 +1271,16 @@ func TestAuthorityValidateChallenge(t *testing.T) {
 				err:   ServerInternalErr(errors.New("error saving challenge: error saving acme challenge: force")),
 			}
 		},
-		"ok": func(t *testing.T) test {
+
+		"ok/already-valid": func(t *testing.T) test {
 			ch, err := newHTTPCh()
 			assert.FatalError(t, err)
-			_ch, ok := ch.(*http01Challenge)
-			assert.Fatal(t, ok)
-			_ch.baseChallenge.Status = StatusValid
-			_ch.baseChallenge.Validated = clock.Now()
-			_ch.baseChallenge.Retry = nil
-			b, err := json.Marshal(ch)
+			bc := ch.clone()
+			bc.Status = StatusValid
+			bc.Validated = clock.Now()
+			bc.Retry = nil
+			rch := bc.morph()
+			b, err := json.Marshal(rch)
 			assert.FatalError(t, err)
 			auth, err := NewAuthority(&db.MockNoSQLDB{
 				MGet: func(bucket, key []byte) ([]byte, error) {
@@ -1291,10 +1294,11 @@ func TestAuthorityValidateChallenge(t *testing.T) {
 				auth:  auth,
 				id:    ch.getID(),
 				accID: ch.getAccountID(),
-				ch:    ch,
+				ch:    rch,
 			}
 		},
 	}
+
 	for name, run := range tests {
 		t.Run(name, func(t *testing.T) {
 			tc := run(t)