Improve handling duplicate ACME EAB references

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)

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)
 	}

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 {

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)

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"`