users tests

internal/models/user.go

@@ -7,15 +7,11 @@ import (
 // User type that extends gorm.Model
 type User struct {
 	gorm.Model
-	// Unique email for each user
-	// Email string `gorm:"unique"`
-	Email string
-	// Hashed password
-	Password string
-	// The clusters that this user has linked
-	Clusters []ClusterConfig
-	// The raw kubeconfig uploaded by this user
-	RawKubeConfig []byte
+
+	Email         string          `json:"email" gorm:"unique"`
+	Password      string          `json:"password"`
+	Clusters      []ClusterConfig `json:"clusters"`
+	RawKubeConfig []byte          `json:"rawKubeConfig"`
 }
 
 // UserExternal represents the User type that is sent over REST

internal/repository/gorm/user.go

@@ -34,6 +34,15 @@ func (repo *UserRepository) ReadUser(id uint) (*models.User, error) {
 	return user, nil
 }
 
+// ReadUserByEmail finds a single user based on their unique email
+func (repo *UserRepository) ReadUserByEmail(email string) (*models.User, error) {
+	user := &models.User{}
+	if err := repo.db.Where("email = ?", email).First(&user).Error; err != nil {
+		return nil, err
+	}
+	return user, nil
+}
+
 // UpdateUser modifies an existing User in the database
 func (repo *UserRepository) UpdateUser(user *models.User) (*models.User, error) {
 	if err := repo.db.First(&models.User{}, user.ID).Updates(user).Error; err != nil {

internal/repository/test/user.go

@@ -2,7 +2,6 @@ package test
 
 import (
 	"errors"
-	"fmt"
 
 	"github.com/porter-dev/porter/internal/models"
 	"github.com/porter-dev/porter/internal/repository"
@@ -28,8 +27,6 @@ func (repo *UserRepository) CreateUser(user *models.User) (*models.User, error)
 		return nil, errors.New("Cannot write database")
 	}
 
-	fmt.Println(len(repo.users))
-
 	// make sure email doesn't exist
 	for _, u := range repo.users {
 		if u.Email == user.Email {
@@ -50,7 +47,7 @@ func (repo *UserRepository) ReadUser(id uint) (*models.User, error) {
 		return nil, errors.New("Cannot read from database")
 	}
 
-	if int(id-1) >= len(repo.users) || repo.users[id] == nil {
+	if int(id-1) >= len(repo.users) || repo.users[id-1] == nil {
 		return nil, gorm.ErrRecordNotFound
 	}
 
@@ -58,18 +55,36 @@ func (repo *UserRepository) ReadUser(id uint) (*models.User, error) {
 	return repo.users[index], nil
 }
 
+// ReadUserByEmail finds a single user based on their unique email
+func (repo *UserRepository) ReadUserByEmail(email string) (*models.User, error) {
+	if !repo.canQuery {
+		return nil, errors.New("Cannot read from database")
+	}
+
+	for _, u := range repo.users {
+		if u.Email == email {
+			return u, nil
+		}
+	}
+
+	return nil, gorm.ErrRecordNotFound
+}
+
 // UpdateUser modifies an existing User in the database
 func (repo *UserRepository) UpdateUser(user *models.User) (*models.User, error) {
 	if !repo.canQuery {
 		return nil, errors.New("Cannot write database")
 	}
 
-	if int(user.ID-1) >= len(repo.users) || repo.users[user.ID] == nil {
+	if int(user.ID-1) >= len(repo.users) || repo.users[user.ID-1] == nil {
 		return nil, gorm.ErrRecordNotFound
 	}
 
 	index := int(user.ID - 1)
+	oldUser := *repo.users[index]
 	repo.users[index] = user
+	user.Email = oldUser.Email
+	user.Password = oldUser.Password
 
 	return user, nil
 }

internal/repository/user.go

@@ -11,6 +11,7 @@ type WriteUser func(user *models.User) (*models.User, error)
 type UserRepository interface {
 	CreateUser(user *models.User) (*models.User, error)
 	ReadUser(id uint) (*models.User, error)
+	ReadUserByEmail(email string) (*models.User, error)
 	UpdateUser(user *models.User) (*models.User, error)
 	DeleteUser(user *models.User) (*models.User, error)
 }

server/api/errors.go

@@ -8,13 +8,6 @@ import (
 	"gorm.io/gorm"
 )
 
-const (
-	appErrDataWrite    = "data write error"
-	appErrDataRead     = "data read error"
-	appErrFormDecoding = "could not process JSON body"
-	appErrReadNotFound = "could not find requested object"
-)
-
 // HTTPError is the object returned when the API encounters an error: this
 // gets marshaled into JSON
 type HTTPError struct {
@@ -25,6 +18,32 @@ type HTTPError struct {
 // ErrorCode is a custom Porter error code, useful for frontend messages
 type ErrorCode int64
 
+var (
+	// ErrorDataWrite describes an error in writing to the database
+	ErrorDataWrite = HTTPError{
+		Code: 500,
+		Errors: []string{
+			"Could not write to database",
+		},
+	}
+
+	// ErrorDataRead describes an error when reading from the database
+	ErrorDataRead = HTTPError{
+		Code: 500,
+		Errors: []string{
+			"Could not read from database",
+		},
+	}
+
+	// ErrorInternal describes a generic internal server error
+	ErrorInternal = HTTPError{
+		Code: 500,
+		Errors: []string{
+			"Internal server error",
+		},
+	}
+)
+
 // ------------------------ Error helper functions ------------------------ //
 
 // sendExternalError marshals an HTTPError into JSON: this function will return an error if
@@ -38,13 +57,7 @@ func (app *App) sendExternalError(
 	errExt HTTPError,
 	w http.ResponseWriter,
 ) (intErr error) {
-	respBytes, newErr := json.Marshal(errExt)
-
-	if newErr != nil {
-		app.handleErrorInternal(newErr, w)
-		return newErr
-	}
-
+	respBytes, _ := json.Marshal(errExt)
 	respBody := string(respBytes)
 
 	app.logger.Warn().Err(err).
@@ -63,7 +76,7 @@ func (app *App) sendExternalError(
 func (app *App) handleErrorFormDecoding(err error, code ErrorCode, w http.ResponseWriter) {
 	errExt := HTTPError{
 		Code:   code,
-		Errors: []string{appErrFormDecoding},
+		Errors: []string{"Could not process JSON body"},
 	}
 
 	app.sendExternalError(err, http.StatusBadRequest, errExt, w)
@@ -99,7 +112,7 @@ func (app *App) handleErrorRead(err error, code ErrorCode, w http.ResponseWriter
 	if err == gorm.ErrRecordNotFound {
 		errExt := HTTPError{
 			Code:   code,
-			Errors: []string{appErrReadNotFound},
+			Errors: []string{"Could not find requested object"},
 		}
 
 		app.sendExternalError(err, http.StatusNotFound, errExt, w)
@@ -113,29 +126,17 @@ func (app *App) handleErrorRead(err error, code ErrorCode, w http.ResponseWriter
 // handleErrorDataWrite handles a database write error due to either a connection
 // error with the database or failure to write that wasn't caught by the validators
 func (app *App) handleErrorDataWrite(err error, code ErrorCode, w http.ResponseWriter) {
-	errExt := HTTPError{
-		Code:   code,
-		Errors: []string{appErrDataWrite},
-	}
-
-	app.sendExternalError(err, http.StatusInternalServerError, errExt, w)
+	app.sendExternalError(err, http.StatusInternalServerError, ErrorDataWrite, w)
 }
 
 // handleErrorDataRead handles a database read error due to an internal error, such as
 // the database connection or gorm internals
 func (app *App) handleErrorDataRead(err error, code ErrorCode, w http.ResponseWriter) {
-	errExt := HTTPError{
-		Code:   code,
-		Errors: []string{appErrDataRead},
-	}
-
-	app.sendExternalError(err, http.StatusInternalServerError, errExt, w)
+	app.sendExternalError(err, http.StatusInternalServerError, ErrorDataRead, w)
 }
 
 // handleErrorInternalError is a catch-all for internal errors that occur during the
 // processing of a request
 func (app *App) handleErrorInternal(err error, w http.ResponseWriter) {
-	app.logger.Warn().Err(err).Msg("")
-	w.WriteHeader(http.StatusInternalServerError)
-	w.Write([]byte(`{"error": "Internal server error"}`))
+	app.sendExternalError(err, http.StatusInternalServerError, ErrorInternal, w)
 }

server/api/user_handler.go

@@ -2,8 +2,12 @@ package api
 
 import (
 	"encoding/json"
+	"errors"
 	"net/http"
 	"strconv"
+	"strings"
+
+	"gorm.io/gorm"
 
 	"github.com/go-chi/chi"
 	"github.com/porter-dev/porter/internal/forms"
@@ -13,7 +17,7 @@ import (
 
 // Enumeration of user API error codes, represented as int64
 const (
-	ErrUserDecode ErrorCode = iota
+	ErrUserDecode ErrorCode = iota + 600
 	ErrUserValidateFields
 	ErrUserDataWrite
 	ErrUserDataRead
@@ -24,7 +28,13 @@ const (
 func (app *App) HandleCreateUser(w http.ResponseWriter, r *http.Request) {
 	form := &forms.CreateUserForm{}
 
-	user, err := app.writeUser(form, app.repo.User.CreateUser, w, r)
+	user, err := app.writeUser(
+		form,
+		app.repo.User.CreateUser,
+		w,
+		r,
+		doesUserExist,
+	)
 
 	if err == nil {
 		app.logger.Info().Msgf("New user created: %d", user.ID)
@@ -114,6 +124,7 @@ func (app *App) writeUser(
 	dbWrite repository.WriteUser,
 	w http.ResponseWriter,
 	r *http.Request,
+	validators ...func(repo *repository.Repository, user *models.User) *HTTPError,
 ) (*models.User, error) {
 	// decode from JSON to form value
 	if err := json.NewDecoder(r.Body).Decode(form); err != nil {
@@ -135,6 +146,35 @@ func (app *App) writeUser(
 		return nil, err
 	}
 
+	// Check any additional validators for any semantic errors
+	// We have completed all syntax checks, so these will be sent
+	// with http.StatusUnprocessableEntity (422), unless this is
+	// an internal server error
+	for _, validator := range validators {
+		err := validator(app.repo, userModel)
+
+		if err != nil {
+			goErr := errors.New(strings.Join(err.Errors, ", "))
+			if err.Code == 500 {
+				app.sendExternalError(
+					goErr,
+					http.StatusInternalServerError,
+					*err,
+					w,
+				)
+			} else {
+				app.sendExternalError(
+					goErr,
+					http.StatusUnprocessableEntity,
+					*err,
+					w,
+				)
+			}
+
+			return nil, goErr
+		}
+	}
+
 	// handle write to the database
 	user, err := dbWrite(userModel)
 
@@ -145,3 +185,22 @@ func (app *App) writeUser(
 
 	return user, nil
 }
+
+func doesUserExist(repo *repository.Repository, user *models.User) *HTTPError {
+	user, err := repo.User.ReadUserByEmail(user.Email)
+
+	if user != nil && err == nil {
+		return &HTTPError{
+			Code: ErrUserValidateFields,
+			Errors: []string{
+				"Email already taken",
+			},
+		}
+	}
+
+	if err != gorm.ErrRecordNotFound {
+		return &ErrorDataRead
+	}
+
+	return nil
+}

server/api/user_handler_test.go

@@ -1,18 +1,21 @@
 package api_test
 
 import (
+	"encoding/json"
 	"net/http"
 	"net/http/httptest"
+	"reflect"
 	"strings"
 	"testing"
 	"time"
 
+	"github.com/go-chi/chi"
 	"github.com/porter-dev/porter/internal/config"
 	"github.com/porter-dev/porter/internal/models"
 	"github.com/porter-dev/porter/internal/repository"
 	"github.com/porter-dev/porter/internal/repository/test"
 	"github.com/porter-dev/porter/server/api"
-	"github.com/porter-dev/porter/server/requestlog"
+	"github.com/porter-dev/porter/server/router"
 
 	lr "github.com/porter-dev/porter/internal/logger"
 	vr "github.com/porter-dev/porter/internal/validator"
@@ -48,56 +51,57 @@ type userTest struct {
 	expStatus int
 	expBody   string
 	canQuery  bool
+	validate  func(r *chi.Mux, t *testing.T)
 }
 
 var createUserTests = []userTest{
-	// userTest{
-	// 	msg:      "Create user",
-	// 	method:   "POST",
-	// 	endpoint: "/api/users",
-	// 	body: `{
-	// 		"email": "belanger@getporter.dev",
-	// 		"password": "hello"
-	// 	}`,
-	// 	expStatus: http.StatusCreated,
-	// 	expBody:   "",
-	// 	canQuery:  true,
-	// },
-	// userTest{
-	// 	msg:      "Create user invalid email",
-	// 	method:   "POST",
-	// 	endpoint: "/api/users",
-	// 	body: `{
-	// 		"email": "notanemail",
-	// 		"password": "hello"
-	// 	}`,
-	// 	expStatus: http.StatusUnprocessableEntity,
-	// 	expBody:   `{"code":1,"errors":["email validation failed"]}`,
-	// 	canQuery:  true,
-	// },
-	// userTest{
-	// 	msg:      "Create user missing field",
-	// 	method:   "POST",
-	// 	endpoint: "/api/users",
-	// 	body: `{
-	// 		"password": "hello"
-	// 	}`,
-	// 	expStatus: http.StatusUnprocessableEntity,
-	// 	expBody:   `{"code":1,"errors":["required validation failed"]}`,
-	// 	canQuery:  true,
-	// },
-	// userTest{
-	// 	msg:      "Create user cannot write to db",
-	// 	method:   "POST",
-	// 	endpoint: "/api/users",
-	// 	body: `{
-	// 		"email": "belanger@getporter.dev",
-	// 		"password": "hello"
-	// 	}`,
-	// 	expStatus: http.StatusInternalServerError,
-	// 	expBody:   `{"code":2,"errors":["data write error"]}`,
-	// 	canQuery:  false,
-	// },
+	userTest{
+		msg:      "Create user",
+		method:   "POST",
+		endpoint: "/api/users",
+		body: `{
+			"email": "belanger@getporter.dev",
+			"password": "hello"
+		}`,
+		expStatus: http.StatusCreated,
+		expBody:   "",
+		canQuery:  true,
+	},
+	userTest{
+		msg:      "Create user invalid email",
+		method:   "POST",
+		endpoint: "/api/users",
+		body: `{
+			"email": "notanemail",
+			"password": "hello"
+		}`,
+		expStatus: http.StatusUnprocessableEntity,
+		expBody:   `{"code":601,"errors":["email validation failed"]}`,
+		canQuery:  true,
+	},
+	userTest{
+		msg:      "Create user missing field",
+		method:   "POST",
+		endpoint: "/api/users",
+		body: `{
+			"password": "hello"
+		}`,
+		expStatus: http.StatusUnprocessableEntity,
+		expBody:   `{"code":601,"errors":["required validation failed"]}`,
+		canQuery:  true,
+	},
+	userTest{
+		msg:      "Create user cannot write to db",
+		method:   "POST",
+		endpoint: "/api/users",
+		body: `{
+			"email": "belanger@getporter.dev",
+			"password": "hello"
+		}`,
+		expStatus: http.StatusInternalServerError,
+		expBody:   `{"code":500,"errors":["Could not read from database"]}`,
+		canQuery:  false,
+	},
 	userTest{
 		init: func(repo *repository.Repository) {
 			repo.User.CreateUser(&models.User{
@@ -112,8 +116,8 @@ var createUserTests = []userTest{
 			"email": "belanger@getporter.dev",
 			"password": "hello"
 		}`,
-		expStatus: http.StatusInternalServerError,
-		expBody:   "",
+		expStatus: http.StatusUnprocessableEntity,
+		expBody:   `{"code":601,"errors":["Email already taken"]}`,
 		canQuery:  true,
 	},
 }
@@ -122,6 +126,7 @@ func TestHandleCreateUser(t *testing.T) {
 	for _, c := range createUserTests {
 		// create a mock API
 		api, repo := initApi(c.canQuery)
+		r := router.New(api)
 
 		if c.init != nil {
 			c.init(repo)
@@ -138,9 +143,7 @@ func TestHandleCreateUser(t *testing.T) {
 		}
 
 		rr := httptest.NewRecorder()
-		handler := requestlog.NewHandler(api.HandleCreateUser, api.Logger())
-
-		handler.ServeHTTP(rr, req)
+		r.ServeHTTP(rr, req)
 
 		if status := rr.Code; status != c.expStatus {
 			t.Errorf("%s, handler returned wrong status code: got %v want %v",
@@ -154,4 +157,180 @@ func TestHandleCreateUser(t *testing.T) {
 	}
 }
 
-// var readUserTests = []userTest
+var readUserTests = []userTest{
+	userTest{
+		init: func(repo *repository.Repository) {
+			repo.User.CreateUser(&models.User{
+				Email:    "belanger@getporter.dev",
+				Password: "hello",
+			})
+		},
+		msg:       "Read user successful",
+		method:    "GET",
+		endpoint:  "/api/users/1",
+		body:      "",
+		expStatus: http.StatusOK,
+		expBody:   `{"id":1,"email":"belanger@getporter.dev","clusters":[],"rawKubeConfig":""}`,
+		canQuery:  true,
+	},
+	userTest{
+		init: func(repo *repository.Repository) {
+			repo.User.CreateUser(&models.User{
+				Email:    "belanger@getporter.dev",
+				Password: "hello",
+			})
+		},
+		msg:       "Read user bad id field",
+		method:    "GET",
+		endpoint:  "/api/users/aldkfjas",
+		body:      "",
+		expStatus: http.StatusBadRequest,
+		expBody:   `{"code":600,"errors":["Could not process JSON body"]}`,
+		canQuery:  true,
+	},
+	userTest{
+		init: func(repo *repository.Repository) {
+			repo.User.CreateUser(&models.User{
+				Email:    "belanger@getporter.dev",
+				Password: "hello",
+			})
+		},
+		msg:       "Read user not found",
+		method:    "GET",
+		endpoint:  "/api/users/2",
+		body:      "",
+		expStatus: http.StatusNotFound,
+		expBody:   `{"code":603,"errors":["Could not find requested object"]}`,
+		canQuery:  true,
+	},
+}
+
+func TestHandleReadUser(t *testing.T) {
+	for _, c := range readUserTests {
+		// create a mock API
+		api, repo := initApi(c.canQuery)
+		r := router.New(api)
+
+		if c.init != nil {
+			c.init(repo)
+		}
+
+		req, err := http.NewRequest(
+			c.method,
+			c.endpoint,
+			strings.NewReader(c.body),
+		)
+
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		rr := httptest.NewRecorder()
+		r.ServeHTTP(rr, req)
+
+		if status := rr.Code; status != c.expStatus {
+			t.Errorf("%s, handler returned wrong status code: got %v want %v",
+				c.msg, status, c.expStatus)
+		}
+
+		if status := rr.Code; status == 200 {
+			// if status is expected to be 200, parse the body for UserExternal
+			gotBody := &models.UserExternal{}
+			expBody := &models.UserExternal{}
+
+			json.Unmarshal(rr.Body.Bytes(), gotBody)
+			json.Unmarshal([]byte(c.expBody), expBody)
+
+			if !reflect.DeepEqual(gotBody, expBody) {
+				t.Errorf("%s, handler returned wrong body: got %v want %v",
+					c.msg, gotBody, expBody)
+			}
+		} else {
+			// if status is expected to not be 200, look for error
+			if body := rr.Body.String(); body != c.expBody {
+				t.Errorf("%s, handler returned wrong body: got %v want %v",
+					c.msg, body, c.expBody)
+			}
+		}
+	}
+}
+
+var updateUserTests = []userTest{
+	userTest{
+		init: func(repo *repository.Repository) {
+			repo.User.CreateUser(&models.User{
+				Email:    "belanger@getporter.dev",
+				Password: "hello",
+			})
+		},
+		msg:       "Update user successful",
+		method:    "PUT",
+		endpoint:  "/api/users/1",
+		body:      `{"rawKubeConfig":"apiVersion: v1\nkind: Config\npreferences: {}\ncurrent-context: default\nclusters:\n- cluster:\n    server: https://localhost\n  name: cluster-test\ncontexts:\n- context:\n    cluster: cluster-test\n    user: test-admin\n  name: context-test\nusers:\n- name: test-admin", "allowedClusters":[]}`,
+		expStatus: http.StatusAccepted,
+		expBody:   "",
+		canQuery:  true,
+		validate: func(r *chi.Mux, t *testing.T) {
+			req, err := http.NewRequest(
+				"GET",
+				"/api/users/1",
+				strings.NewReader(""),
+			)
+
+			if err != nil {
+				t.Fatal(err)
+			}
+
+			rr := httptest.NewRecorder()
+			r.ServeHTTP(rr, req)
+
+			gotBody := &models.UserExternal{}
+			expBody := &models.UserExternal{}
+
+			json.Unmarshal(rr.Body.Bytes(), gotBody)
+			json.Unmarshal([]byte(`{"id":1,"email":"belanger@getporter.dev","clusters":[],"rawKubeConfig":"apiVersion: v1\nkind: Config\npreferences: {}\ncurrent-context: default\nclusters:\n- cluster:\n    server: https://localhost\n  name: cluster-test\ncontexts:\n- context:\n    cluster: cluster-test\n    user: test-admin\n  name: context-test\nusers:\n- name: test-admin"}`), expBody)
+
+			if !reflect.DeepEqual(gotBody, expBody) {
+				t.Errorf("%s, handler returned wrong body: got %v want %v",
+					"validator failed", gotBody, expBody)
+			}
+		},
+	},
+}
+
+func TestHandleUpdateUser(t *testing.T) {
+	for _, c := range updateUserTests {
+		// create a mock API
+		api, repo := initApi(c.canQuery)
+		r := router.New(api)
+
+		if c.init != nil {
+			c.init(repo)
+		}
+
+		req, err := http.NewRequest(
+			c.method,
+			c.endpoint,
+			strings.NewReader(c.body),
+		)
+
+		if err != nil {
+			t.Fatal(err)
+		}
+
+		rr := httptest.NewRecorder()
+		r.ServeHTTP(rr, req)
+
+		if status := rr.Code; status != c.expStatus {
+			t.Errorf("%s, handler returned wrong status code: got %v want %v",
+				c.msg, status, c.expStatus)
+		}
+
+		if body := rr.Body.String(); body != c.expBody {
+			t.Errorf("%s, handler returned wrong body: got %v want %v",
+				c.msg, body, c.expBody)
+		}
+
+		c.validate(r, t)
+	}
+}