From 38733dab95b380157950863031f916dd7502212f Mon Sep 17 00:00:00 2001 From: Marten Seemann Date: Thu, 20 Jun 2024 12:13:51 +0800 Subject: [PATCH] api: output a more helpful error message when root is not found --- api/api.go | 2 +- api/api_test.go | 21 +++++++++++++-------- test/integration/requestid_test.go | 6 +++--- 3 files changed, 17 insertions(+), 12 deletions(-) diff --git a/api/api.go b/api/api.go index 0b139a710..4619bd1e4 100644 --- a/api/api.go +++ b/api/api.go @@ -372,7 +372,7 @@ func Root(w http.ResponseWriter, r *http.Request) { // Load root certificate with the cert, err := mustAuthority(r.Context()).Root(sum) if err != nil { - render.Error(w, r, errs.Wrapf(http.StatusNotFound, err, "%s was not found", r.RequestURI)) + render.Error(w, r, errs.NotFoundErr(err, errs.WithMessage("root with fingerprint %s was not found", sum))) return } diff --git a/api/api_test.go b/api/api_test.go index 8090c6d43..fe5e0f63a 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -835,20 +835,22 @@ func Test_Health(t *testing.T) { } func Test_Root(t *testing.T) { + const sha = "efc7d6b475a56fe587650bcdb999a4a308f815ba44db4bf0371ea68a786ccd36" tests := []struct { - name string - root *x509.Certificate - err error - statusCode int + name string + root *x509.Certificate + err error + expectedMsg string + statusCode int }{ - {"ok", parseCertificate(rootPEM), nil, 200}, - {"fail", nil, fmt.Errorf("not found"), 404}, + {"ok", parseCertificate(rootPEM), nil, "", 200}, + {"fail", nil, fmt.Errorf("not found"), fmt.Sprintf("root with fingerprint %s was not found", sha), 404}, } // Request with chi context chiCtx := chi.NewRouteContext() - chiCtx.URLParams.Add("sha", "efc7d6b475a56fe587650bcdb999a4a308f815ba44db4bf0371ea68a786ccd36") - req := httptest.NewRequest("GET", "http://example.com/root/efc7d6b475a56fe587650bcdb999a4a308f815ba44db4bf0371ea68a786ccd36", http.NoBody) + chiCtx.URLParams.Add("sha", sha) + req := httptest.NewRequest("GET", "http://example.com/root/"+sha, http.NoBody) req = req.WithContext(context.WithValue(context.Background(), chi.RouteCtxKey, chiCtx)) expected := []byte(`{"ca":"` + strings.ReplaceAll(rootPEM, "\n", `\n`) + `\n"}`) @@ -866,6 +868,7 @@ func Test_Root(t *testing.T) { body, err := io.ReadAll(res.Body) res.Body.Close() + fmt.Println("body:", string(body)) if err != nil { t.Errorf("caHandler.Root unexpected error = %v", err) } @@ -873,6 +876,8 @@ func Test_Root(t *testing.T) { if !bytes.Equal(bytes.TrimSpace(body), expected) { t.Errorf("caHandler.Root Body = %s, wants %s", body, expected) } + } else { + require.Contains(t, string(body), tt.expectedMsg) } }) } diff --git a/test/integration/requestid_test.go b/test/integration/requestid_test.go index 54fd2eb07..70949ba7f 100644 --- a/test/integration/requestid_test.go +++ b/test/integration/requestid_test.go @@ -146,11 +146,11 @@ func Test_reflectRequestID(t *testing.T) { var firstErr *errs.Error if assert.ErrorAs(t, err, &firstErr) { assert.Equal(t, 404, firstErr.StatusCode()) - assert.Equal(t, "The requested resource could not be found. Please see the certificate authority logs for more info.", firstErr.Err.Error()) + assert.Equal(t, "root with fingerprint invalid was not found", firstErr.Err.Error()) assert.NotEmpty(t, firstErr.RequestID) // TODO: include the below error in the JSON? It's currently only output to the CA logs. Also see https://github.com/smallstep/certificates/pull/759 - //assert.Equal(t, "/root/invalid was not found: certificate with fingerprint invalid was not found", apiErr.Msg) + // assert.Equal(t, "/root/invalid was not found: certificate with fingerprint invalid was not found", apiErr.Msg) } assert.Nil(t, rootResponse) @@ -159,7 +159,7 @@ func Test_reflectRequestID(t *testing.T) { var secondErr *errs.Error if assert.ErrorAs(t, err, &secondErr) { assert.Equal(t, 404, secondErr.StatusCode()) - assert.Equal(t, "The requested resource could not be found. Please see the certificate authority logs for more info.", secondErr.Err.Error()) + assert.Equal(t, "root with fingerprint invalid was not found", secondErr.Err.Error()) assert.Equal(t, "reqID", secondErr.RequestID) } assert.Nil(t, rootResponse)