Add more context to errors

8 files changed, 55 insertions, 51 deletions

diff --git a/client/client.go b/client/client.go
index b13c4cb..382c53d 100644
--- a/client/client.go
+++ b/client/client.go
@@ -11,6 +11,7 @@ import (
 	"time"
 
 	"github.com/nsheridan/cashier/lib"
+	"github.com/pkg/errors"
 	"golang.org/x/crypto/ssh"
 	"golang.org/x/crypto/ssh/agent"
 )
@@ -27,7 +28,7 @@ func InstallCert(a agent.Agent, cert *ssh.Certificate, key Key) error {
 		LifetimeSecs: uint32(lifetime),
 	}
 	if err := a.Add(pubcert); err != nil {
-		return fmt.Errorf("error importing certificate: %s", err)
+		return errors.Wrap(err, "unable to add cert to ssh agent")
 	}
 	privkey := agent.AddedKey{
 		PrivateKey:   key,
@@ -35,7 +36,7 @@ func InstallCert(a agent.Agent, cert *ssh.Certificate, key Key) error {
 		LifetimeSecs: uint32(lifetime),
 	}
 	if err := a.Add(privkey); err != nil {
-		return fmt.Errorf("error importing key: %s", err)
+		return errors.Wrap(err, "unable to add private key to ssh agent")
 	}
 	return nil
 }
@@ -48,7 +49,7 @@ func send(s []byte, token, ca string, ValidateTLSCertificate bool) (*lib.SignRes
 	client := &http.Client{Transport: transport}
 	u, err := url.Parse(ca)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "unable to parse CA url")
 	}
 	u.Path = path.Join(u.Path, "/sign")
 	req, err := http.NewRequest("POST", u.String(), bytes.NewReader(s))
@@ -68,7 +69,7 @@ func send(s []byte, token, ca string, ValidateTLSCertificate bool) (*lib.SignRes
 	defer resp.Body.Close()
 	c := &lib.SignResponse{}
 	if err := json.NewDecoder(resp.Body).Decode(c); err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "unable to decode server response")
 	}
 	return c, nil
 }
@@ -84,22 +85,22 @@ func Sign(pub ssh.PublicKey, token string, conf *Config) (*ssh.Certificate, erro
 		ValidUntil: time.Now().Add(validity),
 	})
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "unable to create sign request")
 	}
 	resp, err := send(s, token, conf.CA, conf.ValidateTLSCertificate)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "error sending request to CA")
 	}
 	if resp.Status != "ok" {
-		return nil, fmt.Errorf("error: %s", resp.Response)
+		return nil, fmt.Errorf("bad response from CA: %s", resp.Response)
 	}
 	k, _, _, _, err := ssh.ParseAuthorizedKey([]byte(resp.Response))
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "unable to parse response")
 	}
 	cert, ok := k.(*ssh.Certificate)
 	if !ok {
-		return nil, fmt.Errorf("did not receive a certificate from server")
+		return nil, fmt.Errorf("did not receive a valid certificate from server")
 	}
 	return cert, nil
 }
diff --git a/client/keys.go b/client/keys.go
index 3d2fb31..73983a8 100644
--- a/client/keys.go
+++ b/client/keys.go
@@ -8,6 +8,8 @@ import (
 	"crypto/rsa"
 	"fmt"
 
+	"github.com/pkg/errors"
+
 	"golang.org/x/crypto/ed25519"
 	"golang.org/x/crypto/ssh"
 )
@@ -68,7 +70,7 @@ func generateECDSAKey(size int) (Key, error) {
 	case 521:
 		curve = elliptic.P521()
 	default:
-		return nil, fmt.Errorf("Unsupported key size: %d. Valid sizes are '256', '384', '521'", size)
+		return nil, fmt.Errorf("Unsupported ECDSA key size: %d. Valid sizes are '256', '384', '521'", size)
 	}
 	return ecdsa.GenerateKey(curve, rand.Reader)
 }
@@ -101,8 +103,8 @@ func GenerateKey(options ...func(*options)) (Key, ssh.PublicKey, error) {
 		privkey, err = generateRSAKey(config.size)
 	}
 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errors.Wrapf(err, "unable to generate %s key-pair", config.keytype)
 	}
 	pubkey, err = ssh.NewPublicKey(privkey.Public())
-	return privkey, pubkey, err
+	return privkey, pubkey, errors.Wrap(err, "error parsing public key")
 }
diff --git a/cmd/cashierd/main.go b/cmd/cashierd/main.go
index 83627ad..fc03171 100644
--- a/cmd/cashierd/main.go
+++ b/cmd/cashierd/main.go
@@ -5,7 +5,6 @@ import (
 	"crypto/tls"
 	"encoding/hex"
 	"encoding/json"
-	"errors"
 	"flag"
 	"fmt"
 	"html/template"
@@ -17,6 +16,8 @@ import (
 	"strconv"
 	"strings"
 
+	"github.com/pkg/errors"
+
 	"go4.org/wkfs"
 	"golang.org/x/crypto/acme/autocert"
 	"golang.org/x/oauth2"
@@ -125,7 +126,7 @@ func (a *appContext) login(w http.ResponseWriter, r *http.Request) (int, error)
 }
 
 // parseKey retrieves and unmarshals the signing request.
-func parseKey(r *http.Request) (*lib.SignRequest, error) {
+func extractKey(r *http.Request) (*lib.SignRequest, error) {
 	var s lib.SignRequest
 	if err := json.NewDecoder(r.Body).Decode(&s); err != nil {
 		return nil, err
@@ -154,23 +155,25 @@ func signHandler(a *appContext, w http.ResponseWriter, r *http.Request) (int, er
 	}
 
 	// Sign the pubkey and issue the cert.
-	req, err := parseKey(r)
+	req, err := extractKey(r)
 	if err != nil {
-		return http.StatusInternalServerError, err
+		return http.StatusBadRequest, errors.Wrap(err, "unable to extract key from request")
 	}
 	username := a.authprovider.Username(token)
 	a.authprovider.Revoke(token) // We don't need this anymore.
 	cert, err := a.sshKeySigner.SignUserKey(req, username)
 	if err != nil {
-		return http.StatusInternalServerError, err
+		return http.StatusInternalServerError, errors.Wrap(err, "error signing key")
 	}
 	if err := a.certstore.SetCert(cert); err != nil {
 		log.Printf("Error recording cert: %v", err)
 	}
-	json.NewEncoder(w).Encode(&lib.SignResponse{
+	if err := json.NewEncoder(w).Encode(&lib.SignResponse{
 		Status:   "ok",
 		Response: lib.GetPublicKey(cert),
-	})
+	}); err != nil {
+		return http.StatusInternalServerError, errors.Wrap(err, "error encoding response")
+	}
 	return http.StatusOK, nil
 }
 
@@ -219,7 +222,7 @@ func listRevokedCertsHandler(a *appContext, w http.ResponseWriter, r *http.Reque
 	}
 	rl, err := a.sshKeySigner.GenerateRevocationList(revoked)
 	if err != nil {
-		return http.StatusInternalServerError, err
+		return http.StatusInternalServerError, errors.Wrap(err, "unable to generate KRL")
 	}
 	w.Header().Set("Content-Type", "application/octet-stream")
 	w.Write(rl)
@@ -258,7 +261,7 @@ func revokeCertHandler(a *appContext, w http.ResponseWriter, r *http.Request) (i
 	r.ParseForm()
 	for _, id := range r.Form["cert_id"] {
 		if err := a.certstore.Revoke(id); err != nil {
-			return http.StatusInternalServerError, err
+			return http.StatusInternalServerError, errors.Wrap(err, "unable to revoke")
 		}
 	}
 	http.Redirect(w, r, "/admin/certs", http.StatusSeeOther)
@@ -292,7 +295,7 @@ func newState() string {
 func readConfig(filename string) (*config.Config, error) {
 	f, err := os.Open(filename)
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "failed to parse config file")
 	}
 	defer f.Close()
 	return config.ReadConfig(f)
@@ -301,11 +304,11 @@ func readConfig(filename string) (*config.Config, error) {
 func loadCerts(certFile, keyFile string) (tls.Certificate, error) {
 	key, err := wkfs.ReadFile(keyFile)
 	if err != nil {
-		return tls.Certificate{}, err
+		return tls.Certificate{}, errors.Wrap(err, "error reading TLS private key")
 	}
 	cert, err := wkfs.ReadFile(certFile)
 	if err != nil {
-		return tls.Certificate{}, err
+		return tls.Certificate{}, errors.Wrap(err, "error reading TLS certificate")
 	}
 	return tls.X509KeyPair(cert, key)
 }
@@ -338,14 +341,15 @@ func main() {
 	if conf.Server.HTTPLogFile != "" {
 		logfile, err = os.OpenFile(conf.Server.HTTPLogFile, os.O_WRONLY|os.O_APPEND|os.O_CREATE, 0640)
 		if err != nil {
-			log.Fatal(err)
+			log.Printf("unable to open %s for writing. logging to stdout", conf.Server.HTTPLogFile)
+			logfile = os.Stderr
 		}
 	}
 
 	laddr := fmt.Sprintf("%s:%d", conf.Server.Addr, conf.Server.Port)
 	l, err := net.Listen("tcp", laddr)
 	if err != nil {
-		log.Fatal(err)
+		log.Fatal(errors.Wrapf(err, "unable to listen on %s:%d", conf.Server.Addr, conf.Server.Port))
 	}
 
 	tlsConfig := &tls.Config{}
@@ -364,7 +368,7 @@ func main() {
 			tlsConfig.Certificates = make([]tls.Certificate, 1)
 			tlsConfig.Certificates[0], err = loadCerts(conf.Server.TLSCert, conf.Server.TLSKey)
 			if err != nil {
-				log.Fatal(err)
+				log.Fatal(errors.Wrap(err, "unable to create TLS listener"))
 			}
 		}
 		l = tls.NewListener(l, tlsConfig)
@@ -373,7 +377,7 @@ func main() {
 	if conf.Server.User != "" {
 		log.Print("Dropping privileges...")
 		if err := drop.DropPrivileges(conf.Server.User); err != nil {
-			log.Fatal(err)
+			log.Fatal(errors.Wrap(err, "unable to drop privileges"))
 		}
 	}
 
@@ -388,7 +392,7 @@ func main() {
 		log.Fatalf("Unknown provider %s\n", conf.Auth.Provider)
 	}
 	if err != nil {
-		log.Fatal(err)
+		log.Fatal(errors.Wrapf(err, "unable to use provider '%s'", conf.Auth.Provider))
 	}
 
 	certstore, err := store.New(conf.Server.Database)
diff --git a/server/auth/github/github.go b/server/auth/github/github.go
index 24a4bbf..7628526 100644
--- a/server/auth/github/github.go
+++ b/server/auth/github/github.go
@@ -32,7 +32,7 @@ func New(c *config.Auth) (auth.Provider, error) {
 		uw[u] = true
 	}
 	if c.ProviderOpts["organization"] == "" && len(uw) == 0 {
-		return nil, errors.New("github_opts organization and the users whitelist must not be both empty")
+		return nil, errors.New("either GitHub organization or users whitelist must be specified")
 	}
 	return &Config{
 		config: &oauth2.Config{
diff --git a/server/auth/github/github_test.go b/server/auth/github/github_test.go
index c0b26a4..b0c97d2 100644
--- a/server/auth/github/github_test.go
+++ b/server/auth/github/github_test.go
@@ -29,11 +29,9 @@ func TestNew(t *testing.T) {
 
 func TestNewEmptyOrganization(t *testing.T) {
 	organization = ""
-	a := assert.New(t)
-
-	_, err := newGithub()
-	a.EqualError(err, "github_opts organization and the users whitelist must not be both empty")
-
+	if _, err := newGithub(); err == nil {
+		t.Error("creating a provider without an organization set should return an error")
+	}
 	organization = "exampleorg"
 }
 
diff --git a/server/auth/google/google.go b/server/auth/google/google.go
index 08a4083..643ecfe 100644
--- a/server/auth/google/google.go
+++ b/server/auth/google/google.go
@@ -34,7 +34,7 @@ func New(c *config.Auth) (auth.Provider, error) {
 		uw[u] = true
 	}
 	if c.ProviderOpts["domain"] == "" && len(uw) == 0 {
-		return nil, errors.New("google_opts domain and the users whitelist must not be both empty")
+		return nil, errors.New("either Google Apps domain or users whitelist must be specified")
 	}
 
 	return &Config{
diff --git a/server/auth/google/google_test.go b/server/auth/google/google_test.go
index b80c4bf..781cf6f 100644
--- a/server/auth/google/google_test.go
+++ b/server/auth/google/google_test.go
@@ -28,12 +28,11 @@ func TestNew(t *testing.T) {
 }
 
 func TestNewWithoutDomain(t *testing.T) {
-	a := assert.New(t)
-
 	domain = ""
 
-	_, err := newGoogle()
-	a.EqualError(err, "google_opts domain and the users whitelist must not be both empty")
+	if _, err := newGoogle(); err == nil {
+		t.Error("creating a provider without a domain set should return an error")
+	}
 
 	domain = "example.com"
 }
diff --git a/server/config/config.go b/server/config/config.go
index 5f3f458..f2598a0 100644
--- a/server/config/config.go
+++ b/server/config/config.go
@@ -1,7 +1,6 @@
 package config
 
 import (
-	"errors"
 	"fmt"
 	"io"
 	"log"
@@ -12,6 +11,7 @@ import (
 	"github.com/hashicorp/go-multierror"
 	"github.com/mitchellh/mapstructure"
 	"github.com/nsheridan/cashier/server/helpers/vault"
+	"github.com/pkg/errors"
 	"github.com/spf13/viper"
 )
 
@@ -156,14 +156,14 @@ func setFromVault(c *Config) error {
 	}
 	v, err := vault.NewClient(c.Vault.Address, c.Vault.Token)
 	if err != nil {
-		return err
+		return errors.Wrap(err, "vault error")
 	}
-	var errors error
+	var errs error
 	get := func(value string) string {
 		if strings.HasPrefix(value, "/vault/") {
 			s, err := v.Read(value)
 			if err != nil {
-				errors = multierror.Append(errors, err)
+				errs = multierror.Append(errs, err)
 			}
 			return s
 		}
@@ -180,12 +180,12 @@ func setFromVault(c *Config) error {
 		c.AWS.AccessKey = get(c.AWS.AccessKey)
 		c.AWS.SecretKey = get(c.AWS.SecretKey)
 	}
-	return errors
+	return errors.Wrap(errs, "errors reading from vault")
 }
 
 // Unmarshal the config into a *Config
 func decode() (*Config, error) {
-	var errors error
+	var errs error
 	config := &Config{}
 	configPieces := map[string]interface{}{
 		"auth":   &config.Auth,
@@ -200,21 +200,21 @@ func decode() (*Config, error) {
 			continue
 		}
 		if err := mapstructure.WeakDecode(conf[0], val); err != nil {
-			errors = multierror.Append(errors, err)
+			errs = multierror.Append(errs, err)
 		}
 	}
-	return config, errors
+	return config, errs
 }
 
 // ReadConfig parses a hcl configuration file into a Config struct.
 func ReadConfig(r io.Reader) (*Config, error) {
 	viper.SetConfigType("hcl")
 	if err := viper.ReadConfig(r); err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "unable to read config")
 	}
 	config, err := decode()
 	if err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "unable to parse config")
 	}
 	if err := setFromVault(config); err != nil {
 		return nil, err
@@ -222,7 +222,7 @@ func ReadConfig(r io.Reader) (*Config, error) {
 	setFromEnvironment(config)
 	convertDatastoreConfig(config)
 	if err := verifyConfig(config); err != nil {
-		return nil, err
+		return nil, errors.Wrap(err, "unable to verify config")
 	}
 	return config, nil
 }