diff --git a/api/api.go b/api/api.go index 16e24bb2..611e6c84 100644 --- a/api/api.go +++ b/api/api.go @@ -324,7 +324,7 @@ func (h *caHandler) Provisioners(w http.ResponseWriter, r *http.Request) { p, next, err := h.Authority.GetProvisioners(cursor, limit) if err != nil { - WriteError(w, errs.InternalServerErr(err)) + WriteError(w, errs.InternalServerErr(err, "error getting provisioners")) return } JSON(w, &ProvisionersResponse{ diff --git a/api/api_test.go b/api/api_test.go index 5cbce8b3..e8952463 100644 --- a/api/api_test.go +++ b/api/api_test.go @@ -1090,7 +1090,7 @@ func Test_caHandler_Provisioners(t *testing.T) { expectedError400 := errs.BadRequest("limit 'abc' is not an integer") expectedError400Bytes, err := json.Marshal(expectedError400) assert.FatalError(t, err) - expectedError500 := errs.InternalServer("force") + expectedError500 := errs.InternalServer("error getting provisioners") expectedError500Bytes, err := json.Marshal(expectedError500) assert.FatalError(t, err) for _, tt := range tests { diff --git a/api/rekey.go b/api/rekey.go index b7958844..9b5016fc 100644 --- a/api/rekey.go +++ b/api/rekey.go @@ -44,7 +44,7 @@ func (h *caHandler) Rekey(w http.ResponseWriter, r *http.Request) { certChain, err := h.Authority.Rekey(r.TLS.PeerCertificates[0], body.CsrPEM.CertificateRequest.PublicKey) if err != nil { - WriteError(w, errs.Wrap(http.StatusInternalServerError, err, "cahandler.Rekey")) + WriteError(w, errs.InternalServerErr(err, "error rekeying certificate")) return } certChainPEM := certChainToPEM(certChain) diff --git a/api/renew.go b/api/renew.go index 725322ee..a76c672b 100644 --- a/api/renew.go +++ b/api/renew.go @@ -16,7 +16,7 @@ func (h *caHandler) Renew(w http.ResponseWriter, r *http.Request) { certChain, err := h.Authority.Renew(r.TLS.PeerCertificates[0]) if err != nil { - WriteError(w, errs.Wrap(http.StatusInternalServerError, err, "cahandler.Renew")) + WriteError(w, errs.InternalServerErr(err, "error renewing certificate")) return } certChainPEM := certChainToPEM(certChain) diff --git a/api/ssh.go b/api/ssh.go index c9be1527..cb9e98ba 100644 --- a/api/ssh.go +++ b/api/ssh.go @@ -344,7 +344,7 @@ func (h *caHandler) SSHSign(w http.ResponseWriter, r *http.Request) { func (h *caHandler) SSHRoots(w http.ResponseWriter, r *http.Request) { keys, err := h.Authority.GetSSHRoots(r.Context()) if err != nil { - WriteError(w, errs.InternalServerErr(err)) + WriteError(w, errs.InternalServerErr(err, "error getting ssh roots")) return } @@ -369,7 +369,7 @@ func (h *caHandler) SSHRoots(w http.ResponseWriter, r *http.Request) { func (h *caHandler) SSHFederation(w http.ResponseWriter, r *http.Request) { keys, err := h.Authority.GetSSHFederation(r.Context()) if err != nil { - WriteError(w, errs.InternalServerErr(err)) + WriteError(w, errs.InternalServerErr(err, "error getting federated ssh roots")) return } @@ -404,7 +404,7 @@ func (h *caHandler) SSHConfig(w http.ResponseWriter, r *http.Request) { ts, err := h.Authority.GetSSHConfig(r.Context(), body.Type, body.Data) if err != nil { - WriteError(w, errs.InternalServerErr(err)) + WriteError(w, errs.InternalServerErr(err, "error getting ssh config")) return } @@ -415,7 +415,7 @@ func (h *caHandler) SSHConfig(w http.ResponseWriter, r *http.Request) { case provisioner.SSHHostCert: cfg.HostTemplates = ts default: - WriteError(w, errs.InternalServer("it should hot get here")) + WriteError(w, errs.Internal("it should hot get here")) return } @@ -436,7 +436,7 @@ func (h *caHandler) SSHCheckHost(w http.ResponseWriter, r *http.Request) { exists, err := h.Authority.CheckSSHHost(r.Context(), body.Principal, body.Token) if err != nil { - WriteError(w, errs.InternalServerErr(err)) + WriteError(w, errs.InternalServerErr(err, "error checking for host")) return } JSON(w, &SSHCheckPrincipalResponse{ @@ -453,7 +453,7 @@ func (h *caHandler) SSHGetHosts(w http.ResponseWriter, r *http.Request) { hosts, err := h.Authority.GetSSHHosts(r.Context(), cert) if err != nil { - WriteError(w, errs.InternalServerErr(err)) + WriteError(w, errs.InternalServerErr(err, "error getting ssh hosts")) return } JSON(w, &SSHGetHostsResponse{ @@ -475,7 +475,7 @@ func (h *caHandler) SSHBastion(w http.ResponseWriter, r *http.Request) { bastion, err := h.Authority.GetSSHBastion(r.Context(), body.User, body.Hostname) if err != nil { - WriteError(w, errs.InternalServerErr(err)) + WriteError(w, errs.InternalServerErr(err, "error getting ssh bastion")) return } diff --git a/api/sshRekey.go b/api/sshRekey.go index b2c55509..c2aa492f 100644 --- a/api/sshRekey.go +++ b/api/sshRekey.go @@ -63,7 +63,7 @@ func (h *caHandler) SSHRekey(w http.ResponseWriter, r *http.Request) { } oldCert, _, err := provisioner.ExtractSSHPOPCert(body.OTT) if err != nil { - WriteError(w, errs.InternalServerErr(err)) + WriteError(w, errs.InternalServerErr(err, "error exacting ssh certificate")) } newCert, err := h.Authority.RekeySSH(ctx, oldCert, publicKey, signOpts...) diff --git a/api/sshRenew.go b/api/sshRenew.go index 8d07ba01..5796d60f 100644 --- a/api/sshRenew.go +++ b/api/sshRenew.go @@ -55,7 +55,7 @@ func (h *caHandler) SSHRenew(w http.ResponseWriter, r *http.Request) { } oldCert, _, err := provisioner.ExtractSSHPOPCert(body.OTT) if err != nil { - WriteError(w, errs.InternalServerErr(err)) + WriteError(w, errs.InternalServerErr(err, "error extraction ssh certificate")) } newCert, err := h.Authority.RenewSSH(ctx, oldCert) diff --git a/authority/admin/api/provisioner.go b/authority/admin/api/provisioner.go index fd1a02d5..7361fc21 100644 --- a/authority/admin/api/provisioner.go +++ b/authority/admin/api/provisioner.go @@ -53,14 +53,13 @@ func (h *Handler) GetProvisioner(w http.ResponseWriter, r *http.Request) { func (h *Handler) GetProvisioners(w http.ResponseWriter, r *http.Request) { cursor, limit, err := api.ParseCursor(r) if err != nil { - api.WriteError(w, admin.WrapError(admin.ErrorBadRequestType, err, - "error parsing cursor & limit query params")) + api.WriteError(w, err) return } p, next, err := h.auth.GetProvisioners(cursor, limit) if err != nil { - api.WriteError(w, errs.InternalServerErr(err)) + api.WriteError(w, errs.InternalServerErr(err, "error getting provisioners")) return } api.JSON(w, &GetProvisionersResponse{ diff --git a/authority/authorize.go b/authority/authorize.go index a4e7e591..e8d03d8b 100644 --- a/authority/authorize.go +++ b/authority/authorize.go @@ -227,7 +227,10 @@ func (a *Authority) Authorize(ctx context.Context, token string) ([]provisioner. _, signOpts, err := a.authorizeSSHRekey(ctx, token) return signOpts, errs.Wrap(http.StatusInternalServerError, err, "authority.Authorize", opts...) default: - return nil, errs.InternalServer("authority.Authorize; method %d is not supported", append([]interface{}{m}, opts...)...) + return nil, errs.ApplyOptions( + errs.InternalServer("method %d is not supported", m), + opts..., + ) } } diff --git a/authority/authorize_test.go b/authority/authorize_test.go index 6d524a25..97905a41 100644 --- a/authority/authorize_test.go +++ b/authority/authorize_test.go @@ -722,7 +722,7 @@ func TestAuthority_Authorize(t *testing.T) { auth: a, token: "foo", ctx: provisioner.NewContextWithMethod(context.Background(), 15), - err: errors.New("authority.Authorize; method 15 is not supported"), + err: errors.New("method 15 is not supported"), code: http.StatusInternalServerError, } }, diff --git a/authority/provisioner/aws.go b/authority/provisioner/aws.go index fdad7b4a..ae08ba54 100644 --- a/authority/provisioner/aws.go +++ b/authority/provisioner/aws.go @@ -619,7 +619,7 @@ func (p *AWS) authorizeToken(token string) (*awsPayload, error) { return nil, errs.Wrapf(http.StatusUnauthorized, err, "aws.authorizeToken; error parsing aws token") } if len(jwt.Headers) == 0 { - return nil, errs.InternalServer("aws.authorizeToken; error parsing token, header is missing") + return nil, errs.BadRequest("error parsing token, header is missing") } var unsafeClaims awsPayload diff --git a/authority/provisioner/sshpop.go b/authority/provisioner/sshpop.go index 3039d2a3..2a09d00b 100644 --- a/authority/provisioner/sshpop.go +++ b/authority/provisioner/sshpop.go @@ -126,7 +126,7 @@ func (p *SSHPOP) authorizeToken(token string, audiences []string) (*sshPOPPayloa } sshCryptoPubKey, ok := sshCert.Key.(ssh.CryptoPublicKey) if !ok { - return nil, errs.InternalServer("sshpop.authorizeToken; sshpop public key could not be cast to ssh CryptoPublicKey") + return nil, errs.Internal("sshpop public key could not be cast to ssh CryptoPublicKey") } pubKey := sshCryptoPubKey.CryptoPublicKey() diff --git a/authority/root.go b/authority/root.go index f391997f..5d86e082 100644 --- a/authority/root.go +++ b/authority/root.go @@ -15,7 +15,7 @@ func (a *Authority) Root(sum string) (*x509.Certificate, error) { crt, ok := val.(*x509.Certificate) if !ok { - return nil, errs.InternalServer("stored value is not a *x509.Certificate") + return nil, errs.Internal("stored value is not a *x509.Certificate") } return crt, nil } @@ -49,7 +49,7 @@ func (a *Authority) GetFederation() (federation []*x509.Certificate, err error) crt, ok := v.(*x509.Certificate) if !ok { federation = nil - err = errs.InternalServer("stored value is not a *x509.Certificate") + err = errs.Internal("stored value is not a *x509.Certificate") return false } federation = append(federation, crt) diff --git a/authority/root_test.go b/authority/root_test.go index 6e5f1932..82217286 100644 --- a/authority/root_test.go +++ b/authority/root_test.go @@ -22,7 +22,7 @@ func TestRoot(t *testing.T) { code int }{ "not-found": {"foo", errors.New("certificate with fingerprint foo was not found"), http.StatusNotFound}, - "invalid-stored-certificate": {"invaliddata", errors.New("stored value is not a *x509.Certificate"), http.StatusInternalServerError}, + "invalid-stored-certificate": {"invaliddata", errors.New(errs.InternalServerErrorDefaultMsg), http.StatusInternalServerError}, "success": {"189f573cfa159251e445530847ef80b1b62a3a380ee670dcb49e33ed34da0616", nil, http.StatusOK}, } diff --git a/authority/ssh.go b/authority/ssh.go index 7b8f26e2..627f21ce 100644 --- a/authority/ssh.go +++ b/authority/ssh.go @@ -177,7 +177,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi } default: - return nil, errs.InternalServer("authority.SignSSH: invalid extra option type %T", o) + return nil, errs.Internal("invalid extra SignOption of type %T", o) } } @@ -231,7 +231,7 @@ func (a *Authority) SignSSH(ctx context.Context, key ssh.PublicKey, opts provisi } signer = a.sshCAHostCertSignKey default: - return nil, errs.InternalServer("authority.SignSSH: unexpected ssh certificate type: %d", certTpl.CertType) + return nil, errs.Internal("invalid ssh certificate of type '%d'", certTpl.CertType) } // Sign certificate. @@ -297,7 +297,7 @@ func (a *Authority) RenewSSH(ctx context.Context, oldCert *ssh.Certificate) (*ss } signer = a.sshCAHostCertSignKey default: - return nil, errs.InternalServer("renewSSH: unexpected ssh certificate type: %d", certTpl.CertType) + return nil, errs.Internal("invalid ssh certificate of type '%d'", certTpl.CertType) } // Sign certificate. @@ -323,7 +323,7 @@ func (a *Authority) RekeySSH(ctx context.Context, oldCert *ssh.Certificate, pub case provisioner.SSHCertValidator: validators = append(validators, o) default: - return nil, errs.InternalServer("rekeySSH; invalid extra option type %T", o) + return nil, errs.Internal("invalid extra SignOption of type %T", o) } } diff --git a/authority/ssh_test.go b/authority/ssh_test.go index b0907a79..322640f1 100644 --- a/authority/ssh_test.go +++ b/authority/ssh_test.go @@ -901,7 +901,7 @@ func TestAuthority_RekeySSH(t *testing.T) { hostSigner: signer, key: pub, signOpts: []provisioner.SignOption{userOptions}, - err: errors.New("rekeySSH; invalid extra option type"), + err: errors.New(errs.InternalServerErrorDefaultMsg), code: http.StatusInternalServerError, } }, diff --git a/authority/tls.go b/authority/tls.go index 7853198e..8a535a52 100644 --- a/authority/tls.go +++ b/authority/tls.go @@ -113,7 +113,10 @@ func (a *Authority) Sign(csr *x509.CertificateRequest, signOpts provisioner.Sign certEnforcers = append(certEnforcers, k) default: - return nil, errs.InternalServer("authority.Sign; invalid extra option type %T", append([]interface{}{k}, opts...)...) + return nil, errs.ApplyOptions( + errs.Internal("invalid extra SignOption of type %T", k), + opts..., + ) } } diff --git a/authority/tls_test.go b/authority/tls_test.go index 1b5e4d01..cce78e18 100644 --- a/authority/tls_test.go +++ b/authority/tls_test.go @@ -268,7 +268,7 @@ func TestAuthority_Sign(t *testing.T) { csr: csr, extraOpts: append(extraOpts, "42"), signOpts: signOpts, - err: errors.New("authority.Sign; invalid extra option type string"), + err: errors.New(errs.InternalServerErrorDefaultMsg), code: http.StatusInternalServerError, } }, diff --git a/ca/client_test.go b/ca/client_test.go index 29a4848d..5822e6b0 100644 --- a/ca/client_test.go +++ b/ca/client_test.go @@ -163,7 +163,7 @@ func TestClient_Version(t *testing.T) { expectedErr error }{ {"ok", ok, 200, false, nil}, - {"500", errs.InternalServer("force"), 500, true, errors.New(errs.InternalServerErrorDefaultMsg)}, + {"500", errs.InternalServer("force"), 500, true, errors.New(errs.InternalServerPrefix)}, {"404", errs.NotFound("force"), 404, true, errors.New(errs.NotFoundDefaultMsg)}, } @@ -193,7 +193,7 @@ func TestClient_Version(t *testing.T) { if got != nil { t.Errorf("Client.Version() = %v, want nil", got) } - assert.HasPrefix(t, tt.expectedErr.Error(), err.Error()) + assert.HasPrefix(t, err.Error(), tt.expectedErr.Error()) default: if !reflect.DeepEqual(got, tt.response) { t.Errorf("Client.Version() = %v, want %v", got, tt.response) @@ -214,7 +214,7 @@ func TestClient_Health(t *testing.T) { expectedErr error }{ {"ok", ok, 200, false, nil}, - {"not ok", errs.InternalServer("force"), 500, true, errors.New(errs.InternalServerErrorDefaultMsg)}, + {"not ok", errs.InternalServer("force"), 500, true, errors.New(errs.InternalServerPrefix)}, } srv := httptest.NewServer(nil) @@ -244,7 +244,7 @@ func TestClient_Health(t *testing.T) { if got != nil { t.Errorf("Client.Health() = %v, want nil", got) } - assert.HasPrefix(t, tt.expectedErr.Error(), err.Error()) + assert.HasPrefix(t, err.Error(), tt.expectedErr.Error()) default: if !reflect.DeepEqual(got, tt.response) { t.Errorf("Client.Health() = %v, want %v", got, tt.response) @@ -648,7 +648,7 @@ func TestClient_Provisioners(t *testing.T) { if got != nil { t.Errorf("Client.Provisioners() = %v, want nil", got) } - assert.HasPrefix(t, errs.InternalServerErrorDefaultMsg, err.Error()) + assert.HasPrefix(t, err.Error(), errs.InternalServerPrefix) default: if !reflect.DeepEqual(got, tt.response) { t.Errorf("Client.Provisioners() = %v, want %v", got, tt.response) diff --git a/errs/error.go b/errs/error.go index 2c1fe6a9..4d5040c2 100644 --- a/errs/error.go +++ b/errs/error.go @@ -172,9 +172,11 @@ func StatusCodeError(code int, e error, opts ...Option) error { opts = append(opts, withDefaultMessage(ForbiddenDefaultMsg)) return NewErr(http.StatusForbidden, e, opts...) case http.StatusInternalServerError: - return InternalServerErr(e, opts...) + opts = append(opts, withDefaultMessage(InternalServerErrorDefaultMsg)) + return NewErr(http.StatusInternalServerError, e, opts...) case http.StatusNotImplemented: - return NotImplementedErr(e, opts...) + opts = append(opts, withDefaultMessage(NotImplementedDefaultMsg)) + return NewErr(http.StatusNotImplemented, e, opts...) default: return UnexpectedErr(code, e, opts...) } @@ -201,9 +203,13 @@ var ( // directly sent to the cli. BadRequestPrefix = "The request could not be completed: " - // ForbiddenPrefix is the prefix added to the forbidden messates that are + // ForbiddenPrefix is the prefix added to the forbidden messages that are // sent to the cli. ForbiddenPrefix = "The request was forbidden by the certificate authority: " + + // InternalServerPrefix is the prefix added to the internal server error + // messages that are sent to the cli. + InternalServerPrefix = "The certificate authority encountered an Internal Server Error: " ) func formatMessage(status int, msg string) string { @@ -212,6 +218,8 @@ func formatMessage(status int, msg string) string { return BadRequestPrefix + msg + "." case http.StatusForbidden: return ForbiddenPrefix + msg + "." + case http.StatusInternalServerError: + return InternalServerPrefix + msg + "." default: return msg } @@ -315,16 +323,26 @@ func ApplyOptions(err error, opts ...interface{}) error { return err } +// Internal creates a generic 500 error message with the a formatted error in +// the logs. +func Internal(format string, args ...interface{}) error { + return InternalErr(fmt.Errorf(format, args...)) +} + +// Internal creates a generic 500 error message with the given error in the +// logs. +func InternalErr(err error) error { + return NewError(http.StatusInternalServerError, err, InternalServerErrorDefaultMsg) +} + // InternalServer creates a 500 error with the given format and arguments. func InternalServer(format string, args ...interface{}) error { - args = append(args, withDefaultMessage(InternalServerErrorDefaultMsg)) - return Errorf(http.StatusInternalServerError, format, args...) + return New(http.StatusInternalServerError, format, args...) } // InternalServerErr returns a 500 error with the given error. -func InternalServerErr(err error, opts ...Option) error { - opts = append(opts, withDefaultMessage(InternalServerErrorDefaultMsg)) - return NewErr(http.StatusInternalServerError, err, opts...) +func InternalServerErr(err error, format string, args ...interface{}) error { + return NewError(http.StatusInternalServerError, err, format, args...) } // NotImplemented creates a 501 error with the given format and arguments.