nicer validation errors (#180)

server/service_users_test.go

@@ -128,7 +128,7 @@ func TestCreateUser(t *testing.T) {
 		{
 			Username: stringPtr("admin1"),
 			Password: stringPtr("foobar"),
-			wantErr:  invalidArgumentError{field: "email", required: true},
+			wantErr:  invalidArgumentError{invalidArgument{name: "email", reason: "missing required argument"}},
 		},
 		{
 			Username:           stringPtr("admin1"),
@@ -151,7 +151,7 @@ func TestCreateUser(t *testing.T) {
 			Email:              stringPtr("admin1@example.com"),
 			NeedsPasswordReset: boolPtr(true),
 			Admin:              boolPtr(false),
-			wantErr:            invalidArgumentError{field: "username", required: true},
+			wantErr:            invalidArgumentError{invalidArgument{name: "username", reason: "'@' character not allowed in usernames"}},
 		},
 	}
 
@@ -204,11 +204,11 @@ func TestChangeUserPassword(t *testing.T) {
 		},
 		{ // missing token
 			newPassword: "123cat!",
-			wantErr:     invalidArgumentError{field: "token", required: true},
+			wantErr:     invalidArgumentError{invalidArgument{name: "token", reason: "cannot be empty field"}},
 		},
 		{ // missing password
 			token:   "abcd",
-			wantErr: invalidArgumentError{field: "password", required: true},
+			wantErr: invalidArgumentError{invalidArgument{name: "new_password", reason: "cannot be empty field"}},
 		},
 	}
 

server/transport.go

@@ -3,7 +3,6 @@ package server
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"net/http"
 	"strconv"
 
@@ -17,20 +16,6 @@ var (
 	errBadRoute = errors.New("bad route")
 )
 
-type invalidArgumentError struct {
-	field    string
-	required bool
-}
-
-// invalidArgumentError is returned when one or more arguments are invalid.
-func (e invalidArgumentError) Error() string {
-	req := "optional"
-	if e.required {
-		req = "required"
-	}
-	return fmt.Sprintf("%s argument invalid or missing: %s", req, e.field)
-}
-
 func encodeResponse(ctx context.Context, w http.ResponseWriter, response interface{}) error {
 	if e, ok := response.(errorer); ok && e.error() != nil {
 		encodeError(ctx, e.error(), w)
@@ -72,6 +57,24 @@ func encodeError(_ context.Context, err error, w http.ResponseWriter) {
 	}
 	enc := json.NewEncoder(w)
 	enc.SetIndent("", "  ")
+
+	// decide on error type and encode proper JSON format
+	type validator interface {
+		Invalid() []map[string]string
+	}
+	if e, ok := err.(validator); ok {
+		var ve = struct {
+			Message string              `json:"message"`
+			Errors  []map[string]string `json:"errors"`
+		}{
+			Message: "Validation Failed",
+			Errors:  e.Invalid(),
+		}
+		enc.Encode(ve)
+		return
+	}
+
+	// other errors
 	enc.Encode(map[string]interface{}{
 		"error": err.Error(),
 	})

server/validation_users.go

@@ -12,36 +12,56 @@ type validationMiddleware struct {
 }
 
 func (mw validationMiddleware) NewUser(ctx context.Context, p kolide.UserPayload) (*kolide.User, error) {
+	var invalid []invalidArgument
 	if p.Username == nil {
-		return nil, invalidArgumentError{field: "username", required: true}
+		invalid = append(invalid, invalidArgument{name: "username", reason: "missing required argument"})
 	}
-
 	if p.Username != nil {
 		if strings.Contains(*p.Username, "@") {
-			// TODO @groob this makes it obvious that the
-			// validation error needs a "reason" field
-			return nil, invalidArgumentError{field: "username", required: true}
+			invalid = append(invalid, invalidArgument{name: "username", reason: "'@' character not allowed in usernames"})
 		}
 	}
-
 	if p.Password == nil {
-		return nil, invalidArgumentError{field: "password", required: true}
+		invalid = append(invalid, invalidArgument{name: "password", reason: "missing required argument"})
 	}
-
 	if p.Email == nil {
-		return nil, invalidArgumentError{field: "email", required: true}
+		invalid = append(invalid, invalidArgument{name: "email", reason: "missing required argument"})
+	}
+	if len(invalid) != 0 {
+		return nil, invalidArgumentError(invalid)
 	}
-
 	return mw.Service.NewUser(ctx, p)
 }
 
 func (mw validationMiddleware) ResetPassword(ctx context.Context, token, password string) error {
+	var invalid []invalidArgument
 	if token == "" {
-		return invalidArgumentError{field: "token", required: true}
+		invalid = append(invalid, invalidArgument{name: "token", reason: "cannot be empty field"})
 	}
-
 	if password == "" {
-		return invalidArgumentError{field: "password", required: true}
+		invalid = append(invalid, invalidArgument{name: "new_password", reason: "cannot be empty field"})
+	}
+	if len(invalid) != 0 {
+		return invalidArgumentError(invalid)
 	}
 	return mw.Service.ResetPassword(ctx, token, password)
 }
+
+type invalidArgumentError []invalidArgument
+type invalidArgument struct {
+	name   string
+	reason string
+}
+
+// invalidArgumentError is returned when one or more arguments are invalid.
+func (e invalidArgumentError) Error() string {
+	return "validation failed"
+}
+
+func (e invalidArgumentError) Invalid() []map[string]string {
+	var invalid []map[string]string
+	for _, i := range e {
+		invalid = append(invalid, map[string]string{"name": i.name, "reason": i.reason})
+	}
+	return invalid
+}