Skip to content

Commit a5f3b0a

Browse files
asraaasraa
authored
fix retrieve endpoint response code and add testing (sigstore#1043)
Signed-off-by: Asra Ali <asraa@google.com> Signed-off-by: Asra Ali <asraa@google.com>
1 parent d925bc3 commit a5f3b0a

File tree

3 files changed

+129
-14
lines changed

3 files changed

+129
-14
lines changed

‎pkg/api/entries.go‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -361,42 +361,44 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
361361
g, _ := errgroup.WithContext(httpReqCtx)
362362

363363
var searchHashes [][]byte
364+
code := http.StatusBadRequest
364365
for _, entryID := range params.Entry.EntryUUIDs {
365366
if sharding.ValidateEntryID(entryID) == nil {
366367
logEntry, err := retrieveLogEntry(httpReqCtx, entryID)
368+
if errors.Is(err, ErrNotFound) {
369+
code = http.StatusNotFound
370+
}
367371
if err != nil {
368-
return handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf("error getting log entry for %s", entryID))
372+
return handleRekorAPIError(params, code, err, fmt.Sprintf("error getting log entry for %s", entryID))
369373
}
370374
resultPayload = append(resultPayload, logEntry)
371375
continue
372376
}
373377
// At this point, check if we got a uuid instead of an EntryID, so search for the hash later
374378
uuid := entryID
375379
if err := sharding.ValidateUUID(uuid); err != nil {
376-
return handleRekorAPIError(params, http.StatusBadRequest, err, fmt.Sprintf("validating uuid %s", uuid))
380+
return handleRekorAPIError(params, code, err, fmt.Sprintf("validating uuid %s", uuid))
377381
}
378382
hash, err := hex.DecodeString(uuid)
379383
if err != nil {
380-
return handleRekorAPIError(params, http.StatusBadRequest, err, malformedUUID)
384+
return handleRekorAPIError(params, code, err, malformedUUID)
381385
}
382386
searchHashes = append(searchHashes, hash)
383387
}
384388

385-
code := http.StatusBadRequest
386389
entries := params.Entry.Entries()
387390
searchHashesChan := make(chan []byte, len(entries))
388391
for _, e := range entries {
389392
e := e // https://golang.org/doc/faq#closures_and_goroutines
390393
g.Go(func() error {
391394
entry, err := types.UnmarshalEntry(e)
392395
if err != nil {
393-
return err
396+
return fmt.Errorf("unmarshalling entry: %w", err)
394397
}
395398

396399
leaf, err := types.CanonicalizeEntry(httpReqCtx, entry)
397400
if err != nil {
398-
code = http.StatusInternalServerError
399-
return err
401+
return fmt.Errorf("canonicalizing entry: %w", err)
400402
}
401403
hasher := rfc6962.DefaultHasher
402404
leafHash := hasher.HashLeaf(leaf)
@@ -420,9 +422,11 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
420422
g.Go(func() error {
421423
resp := tc.getLeafAndProofByHash(hash)
422424
switch resp.status {
423-
case codes.OK, codes.NotFound:
424-
default:
425+
case codes.OK:
426+
case codes.NotFound:
427+
code = http.StatusNotFound
425428
return resp.err
429+
default:
426430
}
427431
leafResult := resp.getLeafAndProofResult
428432
if leafResult != nil && leafResult.Leaf != nil {
@@ -440,6 +444,7 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
440444
if leafResp != nil {
441445
logEntry, err := logEntryFromLeaf(httpReqCtx, api.signer, tc, leafResp.Leaf, leafResp.SignedLogRoot, leafResp.Proof, api.logRanges.ActiveTreeID(), api.logRanges)
442446
if err != nil {
447+
code = http.StatusInternalServerError
443448
return handleRekorAPIError(params, code, err, err.Error())
444449
}
445450

@@ -451,11 +456,18 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
451456
if len(params.Entry.LogIndexes) > 0 {
452457
g, _ := errgroup.WithContext(httpReqCtx)
453458
resultPayloadChan := make(chan models.LogEntry, len(params.Entry.LogIndexes))
459+
460+
code := http.StatusInternalServerError
454461
for _, logIndex := range params.Entry.LogIndexes {
455462
logIndex := logIndex // https://golang.org/doc/faq#closures_and_goroutines
456463
g.Go(func() error {
457464
logEntry, err := retrieveLogEntryByIndex(httpReqCtx, int(swag.Int64Value(logIndex)))
458465
if err != nil {
466+
switch {
467+
case errors.Is(err, ErrNotFound):
468+
code = http.StatusNotFound
469+
default:
470+
}
459471
return err
460472
}
461473
resultPayloadChan <- logEntry
@@ -464,7 +476,7 @@ func SearchLogQueryHandler(params entries.SearchLogQueryParams) middleware.Respo
464476
}
465477

466478
if err := g.Wait(); err != nil {
467-
return handleRekorAPIError(params, http.StatusInternalServerError, fmt.Errorf("grpc error: %w", err), trillianUnexpectedResult)
479+
return handleRekorAPIError(params, code, err, err.Error())
468480
}
469481
close(resultPayloadChan)
470482
for result := range resultPayloadChan {

‎tests/canonical_rekor.json‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
{
2+
"kind": "rekord",
3+
"apiVersion": "0.0.1",
4+
"spec": {
5+
"signature": {
6+
"format": "pgp",
7+
"content": "iQHKBAABCAA0FiEEcgCUXG78adj6hGUJJrfBoJ04pHoFAl+86RwWHGxoaW5kc0Bwcm90b25tYWlsLmNvbQAKCRAmt8GgnTikejcHC/9yyGEPh2D+MnNR8I8w0sfWChc6pGAQoS6qk/sfC/9GvF4OC7RIy6OwLr/lxyEZbOP2ngYjh/s5KjKxhZyApwwg13LmcbazGnXc3E76J55LoTfwoRa9fupH/M6HI56VFKwnu+AbMNW1s+DM47r7i5nIN6IX9kMpDe3B9XTUULff/yNUv0XtXU+VAf8ndF1w117YVWxf8TnU/HWvX74URQPN+syuyqK/NO1H1KhBVTzcIYd5H6kJu300jgkDypyyqQpd/pJYVwfeY8fCOaeCpfIPjKQ/4enCsAeBgKsAwfIbor8WiE86KoANYqROaW7uqiN+VPadbWVeN6bMpRIdEq8+NKQGlepSCRqbkVg4VKGOPgB3h5WbY9U1O1FVDnXyt7kWdEPEZjBX+V4DawshvNe5LIyqH5hJ1QNAFd0UStqKQt8EUZ/gAtQiXSGbxM1ACoYL9HblKW5b+kj/onKghekFoCoAfhMwRRqR5g/TS/Pc2/ztwYTIuhpQQfMXziTm64g=",
8+
"publicKey": {
9+
"content":"LS0tLS1CRUdJTiBQR1AgUFVCTElDIEtFWSBCTE9DSy0tLS0tCgptUUdOQkYrY0lNMEJEQUNhOEc3UkQydjNtaXdNdHhWYVppM0pCVnVlVkFxSEtDNGVLb01TNUhNQ1JvK0haVlJBCjcwWG1zVHBYMVoxZ1pRdXVDMEdEWTI2aEJoZWpBcTNoeDJydjYvOHE5MEJ2V0dIOXRWZUdwTDFzYUltNTJnRVIKWHlWZ2d6NWtBQzBTNnZNbjdkcjJldEJrV1dQK09qMDVTMDJZWkJUWWd4cE9ieWVjVVNjcUtOVGpzbFpRQkgyZApTSHVrM28yWjdoTTQ5VTBsN3piV3c0b0lUK2xBUmNzYWpRVHdXamxpYVBEL0hSalQyblJPaEloaXRlZC93Z3l6CnlkSXE1ZTZzMThWTGNUNzVxV25yWlhOUFdGd2YyNVJYWTN1dGtXK0dXNW5RZU44MFEya1JFZ2t4RnM1QWQ1V1oKdkU3dDgvaHg1em1zbFo0dGZGNHNpM1FaZUlRQmFjWWJ3eE1QU0RmOW9GR3hkR0ZUODg5d0pMR2dXbXIxVGtQTQpjTjA2d3hBUkd0eE4wejYxRkpUaWpMV1JialczdW5JOWhjUWNVbE4vUSsxNm90SHBlS1ZnNG9XMDBDcnZXT0Q2CnFrTGNNRDQ5eVVEOGZTR0IrUkVuaUJhODlDOWtRVTRTS2Rnc0xML1ErSksrU3k5S21JRHJtMWE0RGZQMXBzZmUKTGphcnpzVlpmS1VIZndjQUVRRUFBYlFpVEhWclpTQklhVzVrY3lBOGJHaHBibVJ6UUhCeWIzUnZibTFoYVd3dQpZMjl0UG9rQjFBUVRBUWdBUGhZaEJISUFsRnh1L0duWStvUmxDU2Ezd2FDZE9LUjZCUUpmbkNETkFoc0RCUWtECndtY0FCUXNKQ0FjQ0JoVUtDUWdMQWdRV0FnTUJBaDRCQWhlQUFBb0pFQ2Ezd2FDZE9LUjZaMWtMLzFJSzB2ZGUKWlg1cjVTZWJOeFRJTlNBQXZZa3JLUnlKNWY3bE9NOWdMR0l1YzJGb05VbmpWUVQwcklHOTAxOWg0OHBDeTkxZgpYakREUk1ZOWd6RldXQ2dHblhoMWhXSTNNN0JKRjZZRTZ1NkRYR3N2dVVwR3JOZVpBRzZra2F6QXVBbm5WMGtDCjA4em9SckFaQ3ZscGFacnlkOGl0YityVitRS3A3QXcybEFJSDFlNmR3TTRSTEZqdmZrOExKWHhqSkFvUG13NmwKTHcxOGM3b1c2UkxPOVFYUThlTTZyMnZISHBtMFR1ZHZaeWFmTnVDMzJHRGxNWTR1MFYxRGI4THN5bVBzQWh1QQoySno0L0tQcTZ1S3dJdG1WSzRwbmRmRUR1NkQxVG9vRFlYaXB0WWFmZHZVMzNwVVF4d0hvZlRUZkU1elp3MlBlCmxIM25aZHNnSFhHUHhKTExNcU9wVzRDL2NNNlpRVmdZU3RWcjBudlU2NitRalF2c2tVWlIwNmRkRXpuQnBHSnMKdHBtajlBZS9HUlk4RU5uTjkvMkdmRXVydHozZEtOVVpvak15MTUzamNHMFUxenpoMTE1V0o3dDh3SEJ1NFM0cAowZ0UrUkFxeXRBY0laRGQyTlNOcno4VnI5RkU5eCtmYXQ5RVJsYm5kQUJFNWlWOHNLMCtGYW5Xd2dia0JqUVJmCm5DRE5BUXdBdEJvdGhmY1J6cjN4cjNQOXA3UUNNd0t1aW9udk1DbThXZ3dOUzRDcGhxbzVOT3IyaU1qa0xQMEoKb21nSkxWWDVOK2Jydjh5NEg4cllQd0tCMTZvL2hBOEliR2JwWXltM0ZjeWtUd2NiV2J0UFRMRXRkQ1VQTFlURApOQzVMR0pwZzNlODZZZlF0QU42L01uWnlZT21sRHgyV0d0dExkbXNBU0dWdXg2QVZKcUl2K3gwNlVLSkVtSzN0CmpsRVZLeWcxMlJFenllNUlUNnFFU0dwT3pvMllsV1VxSVR3L0FhUFEyWnhVYXh2WUZvVU9jd2djZG5Ia2dzaEkKT245aC9OSFVtUDMyV1F2cWtRTXVVYVBJTlJzQzgzS3ZUREdseWZTSFZGek1hNGhETWhFY1h6NGFjaW5kNVdUZQp6eUxnWmhPYjdjTmVDeDR4Y3J0UEI2VTdCUi9GVkx6TEJsQXp1emppRWhZd0pvM0FPTXFGb1I1bUFxaGx1dE5PCnNzeW9mYnFUZ0diU0xkamJYUC9hRXRnejJNVjluL29jMVNCOEhlWk8vMTdKeWduenJ1SUt5Ky9sT1dPenQralYKVkZwVnloMXVlOGxGN3ltS1I0dHNsK2lJVmJxblB2cE1oTE9JQnFYRm4yZ01Da0dvSkx5N09IbzJXQUVKR2x0MwpTd3BicmpqMUFCRUJBQUdKQWJ3RUdBRUlBQ1lXSVFSeUFKUmNidnhwMlBxRVpRa210OEdnblRpa2VnVUNYNXdnCnpRSWJEQVVKQThKbkFBQUtDUkFtdDhHZ25UaWtlaW5pREFDRUFma1pxLzRScDJhTkE0ZGJvSjdVRlhET2FSa1YKOU1Lb0VaRnFUTU5vdkRMNXhoTWxnbFBQdS9sK2RoVGd4ZGVKOUVWSG9lenRiODk2VS9wT3VCUnNuOVZ0VzRZLwpqZWlXN0V5TlhBZC9PcnZuRmJ4KzdpWExxdXBaSkpGVGkvajlSaFZZTnNtbDdzZWJUUGVCbkdEQTkxcWJDNHhICnBRVkRDdWp4NjlWeE81RTFMU29oQ00rTy81dkxCbThpMW8vbmJGbWJ5N1ZDeUtlUkRmaHRmOW5DODRxc0U5R3EKVTcvTFNpazliZnhNV2JwcTh5a250bVMzYTBzemM0YlZGcGV6QnBtTmIwQVZjQitUbTlnV21FemhpTHM2RktBTgpJbnFOdVh1Qkw5UENhYzcrbVUrYzJtQmdHT1JHZDFkWk8zUkM4OXpGM3hCQlluQ09lNWNBTUZsYzFYR3NsbHNJCmR6ZHJkWHZiTkJ6L2o3MXB1TjhvRlltL1hiVmNpZU8wVGZRaURjVHQ4S2lpUjlUQUQ5L1A1OTNSTWxMT0dTOHAKaHZKYmlGb1pmWEhjbHNaRkhtOERRUWE5NElad1RCOG00Z0JWME0yWFN2ZEhvMzBsc3FqdFphWmlTclJoNHJzaApuMTRwYkFhVGRhS0VQY3Z0dWZiVXVXMElqWWQya3BJVC90Zz0KPU9naHIKLS0tLS1FTkQgUEdQIFBVQkxJQyBLRVkgQkxPQ0stLS0tLQo="
10+
}
11+
},
12+
"data": {
13+
"content": "aGVsbG8gcmVrb3IK",
14+
"hash": {
15+
"algorithm": "sha256",
16+
"value": "45c7b11fcbf07dec1694adecd8c5b85770a12a6c8dfdcf2580a2db0c47c31779"
17+
}
18+
}
19+
}
20+
}

‎tests/e2e_test.go‎

Lines changed: 87 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -142,10 +142,11 @@ func TestUploadVerifyRekord(t *testing.T) {
142142
}
143143

144144
// Verify should fail initially
145-
runCliErr(t, "verify", "--artifact", artifactPath, "--signature", sigPath, "--public-key", pubPath)
145+
out := runCliErr(t, "verify", "--artifact", artifactPath, "--signature", sigPath, "--public-key", pubPath)
146+
outputContains(t, out, "404")
146147

147148
// It should upload successfully.
148-
out := runCli(t, "upload", "--artifact", artifactPath, "--signature", sigPath, "--public-key", pubPath)
149+
out = runCli(t, "upload", "--artifact", artifactPath, "--signature", sigPath, "--public-key", pubPath)
149150
outputContains(t, out, "Created entry at")
150151

151152
// Now we should be able to verify it.
@@ -996,12 +997,45 @@ func TestGetNonExistantIndex(t *testing.T) {
996997
outputContains(t, out, "404")
997998
}
998999

1000+
func TestVerifyNonExistantIndex(t *testing.T) {
1001+
// this index is extremely likely to not exist
1002+
out := runCliErr(t, "verify", "--log-index", "100000000")
1003+
outputContains(t, out, "404")
1004+
}
1005+
9991006
func TestGetNonExistantUUID(t *testing.T) {
10001007
// this uuid is extremely likely to not exist
10011008
out := runCliErr(t, "get", "--uuid", "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
10021009
outputContains(t, out, "404")
10031010
}
10041011

1012+
func TestVerifyNonExistantUUID(t *testing.T) {
1013+
// this uuid is extremely likely to not exist
1014+
out := runCliErr(t, "verify", "--uuid", "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff")
1015+
outputContains(t, out, "404")
1016+
1017+
// Check response code
1018+
tid := getTreeID(t)
1019+
h := sha256.Sum256([]byte("123"))
1020+
entryID, err := sharding.CreateEntryIDFromParts(fmt.Sprintf("%x", tid),
1021+
hex.EncodeToString(h[:]))
1022+
if err != nil {
1023+
t.Fatal(err)
1024+
}
1025+
body := fmt.Sprintf("{\"entryUUIDs\":[\"%s\"]}", entryID.ReturnEntryIDString())
1026+
resp, err := http.Post("http://localhost:3000/api/v1/log/entries/retrieve",
1027+
"application/json",
1028+
bytes.NewReader([]byte(body)))
1029+
if err != nil {
1030+
t.Fatal(err)
1031+
}
1032+
c, _ := ioutil.ReadAll(resp.Body)
1033+
t.Log(string(c))
1034+
if resp.StatusCode != 404 {
1035+
t.Fatal("expected 404 status")
1036+
}
1037+
}
1038+
10051039
func TestEntryUpload(t *testing.T) {
10061040
artifactPath := filepath.Join(t.TempDir(), "artifact")
10071041
sigPath := filepath.Join(t.TempDir(), "signature.asc")
@@ -1193,6 +1227,54 @@ func TestSearchQueryLimit(t *testing.T) {
11931227
}
11941228
}
11951229

1230+
func TestSearchQueryMalformedEntry(t *testing.T) {
1231+
wd, err := os.Getwd()
1232+
if err != nil {
1233+
t.Fatal(err)
1234+
}
1235+
b, err := ioutil.ReadFile(filepath.Join(wd, "rekor.json"))
1236+
if err != nil {
1237+
t.Fatal(err)
1238+
}
1239+
body := fmt.Sprintf("{\"entries\":[\"%s\"]}", b)
1240+
resp, err := http.Post("http://localhost:3000/api/v1/log/entries/retrieve",
1241+
"application/json",
1242+
bytes.NewBuffer([]byte(body)))
1243+
if err != nil {
1244+
t.Fatal(err)
1245+
}
1246+
c, _ := ioutil.ReadAll(resp.Body)
1247+
t.Log(string(c))
1248+
if resp.StatusCode != 400 {
1249+
t.Fatal("expected status 400")
1250+
}
1251+
}
1252+
1253+
func TestSearchQueryNonExistentEntry(t *testing.T) {
1254+
// Nonexistent but well-formed entry results in 404 not found.
1255+
wd, err := os.Getwd()
1256+
if err != nil {
1257+
t.Fatal(err)
1258+
}
1259+
b, err := ioutil.ReadFile(filepath.Join(wd, "canonical_rekor.json"))
1260+
if err != nil {
1261+
t.Fatal(err)
1262+
}
1263+
body := fmt.Sprintf("{\"entries\":[%s]}", b)
1264+
t.Log(string(body))
1265+
resp, err := http.Post("http://localhost:3000/api/v1/log/entries/retrieve",
1266+
"application/json",
1267+
bytes.NewBuffer([]byte(body)))
1268+
if err != nil {
1269+
t.Fatal(err)
1270+
}
1271+
c, _ := ioutil.ReadAll(resp.Body)
1272+
t.Log(string(c))
1273+
if resp.StatusCode != 404 {
1274+
t.Fatal("expected 404 status")
1275+
}
1276+
}
1277+
11961278
func getBody(t *testing.T, limit int) []byte {
11971279
t.Helper()
11981280
s := fmt.Sprintf("{\"logIndexes\": [%d", limit)
@@ -1260,7 +1342,8 @@ func TestSearchValidateTreeID(t *testing.T) {
12601342
if err != nil {
12611343
t.Fatal(err)
12621344
}
1263-
if resp.StatusCode != 400 {
1264-
t.Fatalf("expected 400 status code but got %d", resp.StatusCode)
1345+
// Not Found because currently we don't detect that an unused random tree ID is invalid.
1346+
if resp.StatusCode != 404 {
1347+
t.Fatalf("expected 404 status code but got %d", resp.StatusCode)
12651348
}
12661349
}

0 commit comments

Comments
 (0)