From c47993feae1af37b53873812ca0d0da02290c0ad Mon Sep 17 00:00:00 2001
From: Roland Bracewell Shoemaker <roland@bracewel.net>
Date: Mon, 22 Oct 2018 06:27:54 -0700
Subject: [PATCH] Fix new reg GetRegistrationByKey error check (#3895)

Fixes #3877.
---
 wfe/wfe.go      | 10 ++++++----
 wfe/wfe_test.go | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 38 insertions(+), 4 deletions(-)

diff --git a/wfe/wfe.go b/wfe/wfe.go
index d657401a39b..594e45e5297 100644
--- a/wfe/wfe.go
+++ b/wfe/wfe.go
@@ -570,7 +570,6 @@ func link(url, relation string) string {
 
 // NewRegistration is used by clients to submit a new registration/account
 func (wfe *WebFrontEndImpl) NewRegistration(ctx context.Context, logEvent *web.RequestEvent, response http.ResponseWriter, request *http.Request) {
-
 	body, key, _, prob := wfe.verifyPOST(ctx, logEvent, request, false, core.ResourceNewReg)
 	addRequesterHeader(response, logEvent.Requester)
 	if prob != nil {
@@ -579,15 +578,18 @@ func (wfe *WebFrontEndImpl) NewRegistration(ctx context.Context, logEvent *web.R
 		return
 	}
 
-	if existingReg, err := wfe.SA.GetRegistrationByKey(ctx, key); err == nil {
+	existingReg, err := wfe.SA.GetRegistrationByKey(ctx, key)
+	if err != nil && !berrors.Is(err, berrors.NotFound) {
+		wfe.sendError(response, logEvent, probs.ServerInternal("couldn't retrieve the registration"), err)
+		return
+	} else if err == nil || !berrors.Is(err, berrors.NotFound) {
 		response.Header().Set("Location", web.RelativeEndpoint(request, fmt.Sprintf("%s%d", regPath, existingReg.ID)))
-		// TODO(#595): check for missing registration err
 		wfe.sendError(response, logEvent, probs.Conflict("Registration key is already in use"), err)
 		return
 	}
 
 	var init core.Registration
-	err := json.Unmarshal(body, &init)
+	err = json.Unmarshal(body, &init)
 	if err != nil {
 		wfe.sendError(response, logEvent, probs.Malformed("Error unmarshaling JSON"), err)
 		return
diff --git a/wfe/wfe_test.go b/wfe/wfe_test.go
index fc3b3fb0ede..f3ad2724335 100644
--- a/wfe/wfe_test.go
+++ b/wfe/wfe_test.go
@@ -7,6 +7,7 @@ import (
 	"crypto/x509"
 	"encoding/json"
 	"encoding/pem"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -2499,3 +2500,34 @@ func TestNewCertificateSCTError(t *testing.T) {
 		responseWriter.Body.String(),
 		`{"type":"`+probs.V1ErrorNS+`serverInternal","detail":"Error creating new cert :: Unable to meet CA SCT embedding requirements","status":500}`)
 }
+
+type mockSAGetRegByKeyNotFoundAfterVerify struct {
+	core.StorageGetter
+	verified bool
+}
+
+func (sa *mockSAGetRegByKeyNotFoundAfterVerify) GetRegistrationByKey(ctx context.Context, jwk *jose.JSONWebKey) (core.Registration, error) {
+	if !sa.verified {
+		sa.verified = true
+		return sa.StorageGetter.GetRegistrationByKey(ctx, jwk)
+	}
+	return core.Registration{}, errors.New("broke")
+}
+
+// If GetRegistrationByKey returns a non berrors.NotFound error NewRegistration should fail
+// out with an internal server error instead of continuing on and attempting to create a new
+// account.
+func TestNewRegistrationGetKeyBroken(t *testing.T) {
+	wfe, fc := setupWFE(t)
+	wfe.SA = &mockSAGetRegByKeyNotFoundAfterVerify{mocks.NewStorageAuthority(fc), false}
+	payload := `{"resource":"new-reg","contact":["mailto:person@mail.com"],"agreement":"` + agreementURL + `"}`
+	responseWriter := httptest.NewRecorder()
+	wfe.NewRegistration(ctx, newRequestEvent(), responseWriter,
+		makePostRequest(signRequest(t, payload, wfe.nonceService)))
+	var prob probs.ProblemDetails
+	err := json.Unmarshal(responseWriter.Body.Bytes(), &prob)
+	test.AssertNotError(t, err, "unmarshalling response")
+	if prob.Type != probs.V1ErrorNS+probs.ServerInternalProblem {
+		t.Errorf("Wrong type for returned problem: %#v", prob.Type)
+	}
+}