diff --git a/api/errors.go b/api/errors.go index 522fa955..49efd486 100644 --- a/api/errors.go +++ b/api/errors.go @@ -9,6 +9,7 @@ import ( "github.com/pkg/errors" "github.com/smallstep/certificates/acme" + "github.com/smallstep/certificates/api/log" "github.com/smallstep/certificates/authority/admin" "github.com/smallstep/certificates/errs" "github.com/smallstep/certificates/logging" @@ -60,6 +61,6 @@ func WriteError(w http.ResponseWriter, err error) { } if err := json.NewEncoder(w).Encode(err); err != nil { - LogError(w, err) + log.Error(w, err) } } diff --git a/api/log/log.go b/api/log/log.go new file mode 100644 index 00000000..78dae506 --- /dev/null +++ b/api/log/log.go @@ -0,0 +1,47 @@ +// Package log implements API-related logging helpers. +package log + +import ( + "log" + "net/http" + + "github.com/smallstep/certificates/logging" +) + +// Error adds to the response writer the given error if it implements +// logging.ResponseLogger. If it does not implement it, then writes the error +// using the log package. +func Error(rw http.ResponseWriter, err error) { + if rl, ok := rw.(logging.ResponseLogger); ok { + rl.WithFields(map[string]interface{}{ + "error": err, + }) + } else { + log.Println(err) + } +} + +// EnabledResponse log the response object if it implements the EnableLogger +// interface. +func EnabledResponse(rw http.ResponseWriter, v interface{}) { + type enableLogger interface { + ToLog() (interface{}, error) + } + + if el, ok := v.(enableLogger); ok { + out, err := el.ToLog() + if err != nil { + Error(rw, err) + + return + } + + if rl, ok := rw.(logging.ResponseLogger); ok { + rl.WithFields(map[string]interface{}{ + "response": out, + }) + } else { + log.Println(out) + } + } +} diff --git a/api/log/log_test.go b/api/log/log_test.go new file mode 100644 index 00000000..fcd3ea2b --- /dev/null +++ b/api/log/log_test.go @@ -0,0 +1,44 @@ +package log + +import ( + "errors" + "net/http" + "net/http/httptest" + "reflect" + "testing" + + "github.com/smallstep/certificates/logging" +) + +func TestError(t *testing.T) { + theError := errors.New("the error") + + type args struct { + rw http.ResponseWriter + err error + } + tests := []struct { + name string + args args + withFields bool + }{ + {"normalLogger", args{httptest.NewRecorder(), theError}, false}, + {"responseLogger", args{logging.NewResponseLogger(httptest.NewRecorder()), theError}, true}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + Error(tt.args.rw, tt.args.err) + if tt.withFields { + if rl, ok := tt.args.rw.(logging.ResponseLogger); ok { + fields := rl.Fields() + if !reflect.DeepEqual(fields["error"], theError) { + t.Errorf("ResponseLogger[\"error\"] = %s, wants %s", fields["error"], theError) + } + } else { + t.Error("ResponseWriter does not implement logging.ResponseLogger") + } + } + }) + } +} diff --git a/api/utils.go b/api/utils.go index 9daa0cd2..e3fcc9c4 100644 --- a/api/utils.go +++ b/api/utils.go @@ -2,52 +2,14 @@ package api import ( "encoding/json" - "log" "net/http" "google.golang.org/protobuf/encoding/protojson" "google.golang.org/protobuf/proto" - "github.com/smallstep/certificates/logging" + "github.com/smallstep/certificates/api/log" ) -// EnableLogger is an interface that enables response logging for an object. -type EnableLogger interface { - ToLog() (interface{}, error) -} - -// LogError adds to the response writer the given error if it implements -// logging.ResponseLogger. If it does not implement it, then writes the error -// using the log package. -func LogError(rw http.ResponseWriter, err error) { - if rl, ok := rw.(logging.ResponseLogger); ok { - rl.WithFields(map[string]interface{}{ - "error": err, - }) - } else { - log.Println(err) - } -} - -// LogEnabledResponse log the response object if it implements the EnableLogger -// interface. -func LogEnabledResponse(rw http.ResponseWriter, v interface{}) { - if el, ok := v.(EnableLogger); ok { - out, err := el.ToLog() - if err != nil { - LogError(rw, err) - return - } - if rl, ok := rw.(logging.ResponseLogger); ok { - rl.WithFields(map[string]interface{}{ - "response": out, - }) - } else { - log.Println(out) - } - } -} - // JSON writes the passed value into the http.ResponseWriter. func JSON(w http.ResponseWriter, v interface{}) { JSONStatus(w, v, http.StatusOK) @@ -59,10 +21,12 @@ func JSONStatus(w http.ResponseWriter, v interface{}, status int) { w.Header().Set("Content-Type", "application/json") w.WriteHeader(status) if err := json.NewEncoder(w).Encode(v); err != nil { - LogError(w, err) + log.Error(w, err) + return } - LogEnabledResponse(w, v) + + log.EnabledResponse(w, v) } // ProtoJSON writes the passed value into the http.ResponseWriter. @@ -78,12 +42,16 @@ func ProtoJSONStatus(w http.ResponseWriter, m proto.Message, status int) { b, err := protojson.Marshal(m) if err != nil { - LogError(w, err) + log.Error(w, err) + return } + if _, err := w.Write(b); err != nil { - LogError(w, err) + log.Error(w, err) + return } - //LogEnabledResponse(w, v) + + // log.EnabledResponse(w, v) } diff --git a/api/utils_test.go b/api/utils_test.go index 12350c97..f5e1e1cb 100644 --- a/api/utils_test.go +++ b/api/utils_test.go @@ -3,46 +3,11 @@ package api import ( "net/http" "net/http/httptest" - "reflect" "testing" - "github.com/pkg/errors" - "github.com/smallstep/certificates/logging" ) -func TestLogError(t *testing.T) { - theError := errors.New("the error") - type args struct { - rw http.ResponseWriter - err error - } - tests := []struct { - name string - args args - withFields bool - }{ - {"normalLogger", args{httptest.NewRecorder(), theError}, false}, - {"responseLogger", args{logging.NewResponseLogger(httptest.NewRecorder()), theError}, true}, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - LogError(tt.args.rw, tt.args.err) - if tt.withFields { - if rl, ok := tt.args.rw.(logging.ResponseLogger); ok { - fields := rl.Fields() - if !reflect.DeepEqual(fields["error"], theError) { - t.Errorf("ResponseLogger[\"error\"] = %s, wants %s", fields["error"], theError) - } - } else { - t.Error("ResponseWriter does not implement logging.ResponseLogger") - } - } - }) - } -} - func TestJSON(t *testing.T) { type args struct { rw http.ResponseWriter diff --git a/scep/api/api.go b/scep/api/api.go index 77c683ee..a326ea92 100644 --- a/scep/api/api.go +++ b/scep/api/api.go @@ -10,14 +10,14 @@ import ( "strings" "github.com/go-chi/chi" - "github.com/smallstep/certificates/api" - "github.com/smallstep/certificates/authority/provisioner" - "github.com/smallstep/certificates/scep" + microscep "github.com/micromdm/scep/v2/scep" + "github.com/pkg/errors" "go.mozilla.org/pkcs7" - "github.com/pkg/errors" - - microscep "github.com/micromdm/scep/v2/scep" + "github.com/smallstep/certificates/api" + "github.com/smallstep/certificates/api/log" + "github.com/smallstep/certificates/authority/provisioner" + "github.com/smallstep/certificates/scep" ) const ( @@ -337,7 +337,7 @@ func formatCapabilities(caps []string) []byte { func writeSCEPResponse(w http.ResponseWriter, response SCEPResponse) { if response.Error != nil { - api.LogError(w, response.Error) + log.Error(w, response.Error) } if response.Certificate != nil {