From b4faddc45bd15cc60486fdd5ea38b912cc5a0c05 Mon Sep 17 00:00:00 2001
From: Bob Callaway <bobcallaway@users.noreply.github.com>
Date: Mon, 4 Jan 2021 09:07:40 -0500
Subject: [PATCH] Checks correct response code to prevent duplicate insertion
 (#79)

QueuedLeafResponse holds the actual insertion response code, so check that instead of just the base GRPC status code.

also fixes middleware ordering issue

Signed-off-by: Bob Callaway <bcallawa@redhat.com>
---
 go.mod                                        |  1 +
 pkg/api/entries.go                            | 21 +++++++++++++------
 .../restapi/configure_rekor_server.go         |  4 ++--
 3 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/go.mod b/go.mod
index 81b30d9..562f970 100644
--- a/go.mod
+++ b/go.mod
@@ -46,6 +46,7 @@ require (
 	golang.org/x/sys v0.0.0-20201207223542-d4d67f95c62d // indirect
 	golang.org/x/tools v0.0.0-20201208062317-e652b2f42cc7 // indirect
 	google.golang.org/appengine v1.6.7
+	google.golang.org/genproto v0.0.0-20200825200019-8632dd797987
 	google.golang.org/grpc v1.32.0
 	gopkg.in/ini.v1 v1.62.0 // indirect
 )
diff --git a/pkg/api/entries.go b/pkg/api/entries.go
index 72dbc8d..17e44a9 100644
--- a/pkg/api/entries.go
+++ b/pkg/api/entries.go
@@ -27,6 +27,7 @@ import (
 
 	"github.com/go-openapi/swag"
 
+	"google.golang.org/genproto/googleapis/rpc/code"
 	"google.golang.org/grpc/codes"
 
 	"github.com/projectrekor/rekor/pkg/types"
@@ -85,16 +86,24 @@ func CreateLogEntryHandler(params entries.CreateLogEntryParams) middleware.Respo
 	tc := NewTrillianClient(httpReq.Context())
 
 	resp := tc.addLeaf(leaf)
-	switch resp.status {
-	case codes.OK:
-	case codes.AlreadyExists, codes.FailedPrecondition:
-		return handleRekorAPIError(params, http.StatusConflict, fmt.Errorf("grpc error: %w", resp.err), entryAlreadyExists)
-	default:
+	//this represents overall GRPC response state (not the results of insertion into the log)
+	if resp.status != codes.OK {
 		return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("grpc error: %w", resp.err), trillianUnexpectedResult)
 	}
 
-	queuedLeaf := resp.getAddResult.QueuedLeaf.Leaf
+	//this represents the results of inserting the proposed leaf into the log; status is nil in success path
+	insertionStatus := resp.getAddResult.QueuedLeaf.Status
+	if insertionStatus != nil {
+		switch insertionStatus.Code {
+		case int32(code.Code_OK):
+		case int32(code.Code_ALREADY_EXISTS), int32(code.Code_FAILED_PRECONDITION):
+			return handleRekorAPIError(params, http.StatusConflict, fmt.Errorf("grpc error: %v", insertionStatus.String()), entryAlreadyExists)
+		default:
+			return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("grpc error: %v", insertionStatus.String()), trillianUnexpectedResult)
+		}
+	}
 
+	queuedLeaf := resp.getAddResult.QueuedLeaf.Leaf
 	uuid := hex.EncodeToString(queuedLeaf.GetMerkleLeafHash())
 
 	logEntry := models.LogEntry{
diff --git a/pkg/generated/restapi/configure_rekor_server.go b/pkg/generated/restapi/configure_rekor_server.go
index 0429556..b1d002f 100644
--- a/pkg/generated/restapi/configure_rekor_server.go
+++ b/pkg/generated/restapi/configure_rekor_server.go
@@ -109,8 +109,8 @@ func setupMiddlewares(handler http.Handler) http.Handler {
 // The middleware configuration happens before anything, this middleware also applies to serving the swagger.json document.
 // So this is a good place to plug in a panic handling middleware, logging and metrics
 func setupGlobalMiddleware(handler http.Handler) http.Handler {
-	returnHandler := middleware.Recoverer(handler)
-	returnHandler = middleware.Logger(returnHandler)
+	returnHandler := middleware.Logger(handler)
+	returnHandler = middleware.Recoverer(returnHandler)
 	returnHandler = middleware.Heartbeat("/ping")(returnHandler)
 
 	return middleware.RequestID(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-- 
GitLab