From d354d55e7fc250508b50eb571923adf96e919f0d Mon Sep 17 00:00:00 2001
From: Herman Slatman <hermanslatman@hotmail.com>
Date: Sat, 16 Oct 2021 14:44:56 +0200
Subject: [PATCH] Improve handling duplicate ACME EAB references

---
 acme/db.go                    |  2 +-
 acme/db/nosql/account.go      |  2 +-
 acme/db/nosql/account_test.go |  2 +-
 authority/admin/api/acme.go   | 13 +++++++++++--
 authority/admin/errors.go     |  2 +-
 5 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/acme/db.go b/acme/db.go
index 726aa937..a4173ce5 100644
--- a/acme/db.go
+++ b/acme/db.go
@@ -154,7 +154,7 @@ func (m *MockDB) GetExternalAccountKeys(ctx context.Context, provisionerName str
 	return m.MockRet1.([]*ExternalAccountKey), m.MockError
 }
 
-// GetExtrnalAccountKeyByReference mock
+// GetExternalAccountKeyByReference mock
 func (m *MockDB) GetExternalAccountKeyByReference(ctx context.Context, provisionerName, reference string) (*ExternalAccountKey, error) {
 	if m.MockGetExternalAccountKeys != nil {
 		return m.GetExternalAccountKeyByReference(ctx, provisionerName, reference)
diff --git a/acme/db/nosql/account.go b/acme/db/nosql/account.go
index c00185be..aca85a76 100644
--- a/acme/db/nosql/account.go
+++ b/acme/db/nosql/account.go
@@ -295,7 +295,7 @@ func (db *DB) GetExternalAccountKeyByReference(ctx context.Context, provisionerN
 	}
 	k, err := db.db.Get(externalAccountKeysByReferenceTable, []byte(reference))
 	if nosqlDB.IsErrNotFound(err) {
-		return nil, errors.Errorf("ACME EAB key for reference %s not found", reference)
+		return nil, acme.ErrNotFound
 	} else if err != nil {
 		return nil, errors.Wrapf(err, "error loading ACME EAB key for reference %s", reference)
 	}
diff --git a/acme/db/nosql/account_test.go b/acme/db/nosql/account_test.go
index b06ac6bb..4b94e40f 100644
--- a/acme/db/nosql/account_test.go
+++ b/acme/db/nosql/account_test.go
@@ -992,7 +992,7 @@ func TestDB_GetExternalAccountKeyByReference(t *testing.T) {
 						return nil, nosqldb.ErrNotFound
 					},
 				},
-				err: errors.New("ACME EAB key for reference ref not found"),
+				err: errors.New("not found"),
 			}
 		},
 		"fail/reference-load-error": func(t *testing.T) test {
diff --git a/authority/admin/api/acme.go b/authority/admin/api/acme.go
index 9283dfc5..8f3cdf2b 100644
--- a/authority/admin/api/acme.go
+++ b/authority/admin/api/acme.go
@@ -92,15 +92,24 @@ func (h *Handler) CreateExternalAccountKey(w http.ResponseWriter, r *http.Reques
 	prov := chi.URLParam(r, "prov")
 	reference := body.Reference
 
+	// check if a key with the reference does not exist (only when a reference was in the request)
 	if reference != "" {
 		k, err := h.acmeDB.GetExternalAccountKeyByReference(r.Context(), prov, reference)
-		// retrieving an EAB key from DB results in error if it doesn't exist, which is what we're looking for
-		if err == nil || k != nil {
+		// retrieving an EAB key from DB results in an error if it doesn't exist, which is what we're looking for,
+		// but other errors can also happen. Return early if that happens; continuing if it was acme.ErrNotFound.
+		shouldWriteError := err != nil && !(acme.ErrNotFound == err)
+		if shouldWriteError {
+			api.WriteError(w, err)
+			return
+		}
+		// if a key was found, return HTTP 409 conflict
+		if k != nil {
 			err := admin.NewError(admin.ErrorBadRequestType, "an ACME EAB key for provisioner %s with reference %s already exists", prov, reference)
 			err.Status = 409
 			api.WriteError(w, err)
 			return
 		}
+		// continue execution if no key was found for the reference
 	}
 
 	eak, err := h.acmeDB.CreateExternalAccountKey(r.Context(), prov, reference)
diff --git a/authority/admin/errors.go b/authority/admin/errors.go
index 607093b0..217227ca 100644
--- a/authority/admin/errors.go
+++ b/authority/admin/errors.go
@@ -104,7 +104,7 @@ var (
 	}
 )
 
-// Error represents an Admin
+// Error represents an Admin error
 type Error struct {
 	Type    string `json:"type"`
 	Detail  string `json:"detail"`