From 251de1f0a48506a81b2d253ef58fbcdd4ea88600 Mon Sep 17 00:00:00 2001 From: connorwalsh Date: Fri, 16 Feb 2018 16:57:38 -0500 Subject: [PATCH 1/3] [cw,am|#9] extract uuid validation into separate utils package, fix postgres sql parameter syntax (positional arguments), begin writing user validation on a per action basis, include test fore user create --- server/core/middleware.go | 13 ++----------- server/types/user.go | 38 +++++++++++++++++++++++++++++++------- server/types/user_test.go | 16 ++++++++++++++++ server/utils/uuid.go | 17 +++++++++++++++++ 4 files changed, 66 insertions(+), 18 deletions(-) create mode 100644 server/utils/uuid.go diff --git a/server/core/middleware.go b/server/core/middleware.go index 7111b20..11e65c4 100644 --- a/server/core/middleware.go +++ b/server/core/middleware.go @@ -2,15 +2,11 @@ package core import ( "fmt" - "regexp" + "github.com/connorwalsh/new-yorken-poesry-magazine/server/utils" "github.com/gocraft/web" ) -var ( - uuidRegexp = regexp.MustCompile("^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}$") -) - // Validate incoming requests. // ensure path parameters are valid, etc. func (a *API) Validate(rw web.ResponseWriter, req *web.Request, next web.NextMiddlewareFunc) { @@ -31,7 +27,7 @@ func ValidateParams(params map[string]string) error { switch k { case API_ID_PATH_PARAM: // id path params MUST be V4 UUIDs - if !IsValidUUIDV4(v) { + if !utils.IsValidUUIDV4(v) { return fmt.Errorf("Id parameter must be a UUID V4 (given %s)", v) } default: @@ -43,11 +39,6 @@ func ValidateParams(params map[string]string) error { return nil } -// check to see if a string is a UUID V4 -func IsValidUUIDV4(uuid string) bool { - return uuidRegexp.MatchString(uuid) -} - // Authorize requests. // ensure a user cannot delete other users, etc. func (*API) Authorize(rw web.ResponseWriter, req *web.Request, next web.NextMiddlewareFunc) { diff --git a/server/types/user.go b/server/types/user.go index 3178daf..6415dc9 100644 --- a/server/types/user.go +++ b/server/types/user.go @@ -3,6 +3,9 @@ package types import ( "database/sql" "fmt" + + "github.com/connorwalsh/new-yorken-poesry-magazine/server/utils" + _ "github.com/lib/pq" ) type User struct { @@ -13,8 +16,20 @@ type User struct { Poets []*Poet `json:"poets"` } -func (u *User) Validate() error { - // TODO +func (u *User) Validate(action string) error { + // make sure id, if not an empty string, is a uuid + if !utils.IsValidUUIDV4(u.Id) && u.Id != "" { + return fmt.Errorf("User Id must be a valid uuid, given %s", u.Id) + } + + // perform validation on a per action basis + switch action { + case "CREATE": + case "UPDATE": + default: + // only ensure that the id is present + // this aplies to the READ and DELETE cases + } return nil } @@ -48,30 +63,39 @@ func (*User) CreateTable(db *sql.DB) error { return nil } -func (u *User) CreateUser(db *sql.DB) error { +func (u *User) CreateUser(id string, db *sql.DB) error { var ( - err error + rows *sql.Rows + err error ) // we assume that all validation/sanitization has already been called + // assign id + u.Id = id + // prepare statement if not already done so. if userCreateStmt == nil { // create statement stmt := `INSERT INTO users ( - id, username, password, email, poets - ) VALUES (?, ?, ?, ?, ?)` + id, username, password, email + ) VALUES ($1, $2, $3, $4)` userCreateStmt, err = db.Prepare(stmt) if err != nil { return err } } - _, err = userCreateStmt.Exec(u.Id, u.Username, u.Password, u.Email, u.Poets) + rows, err = userCreateStmt.Query(u.Id, u.Username, u.Password, u.Email) if err != nil { return err } + // read created result + for rows.Next() { + rows.Scan(u.Id, u.Username, u.Password, u.Email) + } + return nil } diff --git a/server/types/user_test.go b/server/types/user_test.go index 2228ede..a303028 100644 --- a/server/types/user_test.go +++ b/server/types/user_test.go @@ -3,6 +3,7 @@ package types import ( "database/sql" + uuid "github.com/satori/go.uuid" "github.com/stretchr/testify/suite" ) @@ -51,3 +52,18 @@ func (s *UserTestSuite) TestCreateTable() { err := user.CreateTable(testDB) s.NoError(err) } + +func (s *UserTestSuite) TestCreateUser() { + user := &User{Username: "c", Password: "axaxaxax", Email: "hr@worst.nightmare"} + id := uuid.NewV4().String() + expectedResult := &User{ + Id: id, + Username: "c", + Password: "axaxaxax", + Email: "hr@worst.nightmare", + } + + err := user.CreateUser(id, testDB) + s.NoError(err) + s.EqualValues(user, expectedResult) +} diff --git a/server/utils/uuid.go b/server/utils/uuid.go new file mode 100644 index 0000000..b49be71 --- /dev/null +++ b/server/utils/uuid.go @@ -0,0 +1,17 @@ +package utils + +import "regexp" + +var ( + uuidRegexp = regexp.MustCompile("^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-4[a-fA-F0-9]{3}-[8|9|aA|bB][a-fA-F0-9]{3}-[a-fA-F0-9]{12}$") +) + +// check to see if a string is a UUID V4 +func IsValidUUIDV4(uuid string) bool { + return uuidRegexp.MatchString(uuid) +} + +func ScanForSQLInjection(s string) error { + // maybe we need this in the future, but we don't need this when using prepared sql statements + return nil +} From 1705beadd0a382ed034e9a76c273eded7a8294ea Mon Sep 17 00:00:00 2001 From: connorwalsh Date: Sat, 17 Feb 2018 13:38:30 -0500 Subject: [PATCH 2/3] [cw|#9] add more cases to user validation, test user Read and Delete, begin working on user update (which might be tricky because we need to be able to detect provided fields) --- server/consts/actions.go | 8 +++++ server/core/handlers.go | 7 ++-- server/core/middleware_test.go | 6 ++-- server/types/user.go | 59 ++++++++++++++++++++-------------- server/types/user_test.go | 47 ++++++++++++++++++++++++++- 5 files changed, 96 insertions(+), 31 deletions(-) create mode 100644 server/consts/actions.go diff --git a/server/consts/actions.go b/server/consts/actions.go new file mode 100644 index 0000000..317fae3 --- /dev/null +++ b/server/consts/actions.go @@ -0,0 +1,8 @@ +package consts + +const ( + CREATE = "create" + READ = "read" + UPDATE = "update" + DELETE = "delete" +) diff --git a/server/core/handlers.go b/server/core/handlers.go index df64307..3f86522 100644 --- a/server/core/handlers.go +++ b/server/core/handlers.go @@ -4,6 +4,7 @@ import ( "encoding/json" "fmt" + "github.com/connorwalsh/new-yorken-poesry-magazine/server/consts" "github.com/connorwalsh/new-yorken-poesry-magazine/server/types" "github.com/gocraft/web" ) @@ -70,7 +71,7 @@ func (a *API) CreateUser(rw web.ResponseWriter, req *web.Request) { // TODO send failure response to client } - err = user.Validate() + err = user.Validate(consts.CREATE) if err != nil { a.Error(err.Error()) } @@ -97,13 +98,13 @@ func (a *API) GetUser(rw web.ResponseWriter, req *web.Request) { // assigning said id to id of user struct user := &types.User{Id: id} - err = user.Validate() + err = user.Validate(consts.READ) if err != nil { a.Error(err.Error()) } // invoke read - err = user.ReadUser(a.db) + err = user.Read(a.db) if err != nil { a.Error(err.Error()) } diff --git a/server/core/middleware_test.go b/server/core/middleware_test.go index 240312f..e1fc706 100644 --- a/server/core/middleware_test.go +++ b/server/core/middleware_test.go @@ -4,6 +4,7 @@ import ( "os" "testing" + "github.com/connorwalsh/new-yorken-poesry-magazine/server/utils" "github.com/satori/go.uuid" "github.com/stretchr/testify/assert" ) @@ -14,17 +15,18 @@ func MainTest(m *testing.M) { os.Exit(retcode) } +// move this test to utils func TestIsValidUUIDV4_WithValidUUID(t *testing.T) { id := uuid.NewV4().String() - assert.True(t, IsValidUUIDV4(id)) + assert.True(t, utils.IsValidUUIDV4(id)) } func TestIsValidUUIDV4_WithInValidUUID(t *testing.T) { id := uuid.NewV4().String() // say no to sql injections - assert.False(t, IsValidUUIDV4(id+" OR 1=1 --see u in h3ll")) + assert.False(t, utils.IsValidUUIDV4(id+" OR 1=1 --see u in h3ll")) } func TestValidateParams_NoParams(t *testing.T) { diff --git a/server/types/user.go b/server/types/user.go index 6415dc9..2d8dee0 100644 --- a/server/types/user.go +++ b/server/types/user.go @@ -4,6 +4,7 @@ import ( "database/sql" "fmt" + "github.com/connorwalsh/new-yorken-poesry-magazine/server/consts" "github.com/connorwalsh/new-yorken-poesry-magazine/server/utils" _ "github.com/lib/pq" ) @@ -24,11 +25,21 @@ func (u *User) Validate(action string) error { // perform validation on a per action basis switch action { - case "CREATE": - case "UPDATE": + case consts.CREATE: + case consts.UPDATE: + case consts.DELETE: + // TODO ensure that only a user can delete themselves + fallthrough default: // only ensure that the id is present // this aplies to the READ and DELETE cases + // we minimally need the Id to exist in these cases + if u.Id == "" { + return fmt.Errorf( + "User Id is a required field to fulfill a %s", + action, + ) + } } return nil @@ -63,7 +74,7 @@ func (*User) CreateTable(db *sql.DB) error { return nil } -func (u *User) CreateUser(id string, db *sql.DB) error { +func (u *User) Create(id string, db *sql.DB) error { var ( rows *sql.Rows err error @@ -99,7 +110,7 @@ func (u *User) CreateUser(id string, db *sql.DB) error { return nil } -func (u *User) ReadUser(db *sql.DB) error { +func (u *User) Read(db *sql.DB) error { var ( err error ) @@ -107,7 +118,7 @@ func (u *User) ReadUser(db *sql.DB) error { // prepare statement if not already done so. if userReadStmt == nil { // read statement - stmt := `SELECT * FROM users WHERE id = ?` + stmt := `SELECT * FROM users WHERE id = $1` userReadStmt, err = db.Prepare(stmt) if err != nil { return err @@ -118,35 +129,28 @@ func (u *User) ReadUser(db *sql.DB) error { // run prepared query over arguments // NOTE: we are not joining from the poets tables - rows, err := userReadStmt.Query(u.Id) - if err != nil { - return err - } - - // decode results into user struct - defer rows.Close() - for rows.Next() { - err = rows.Scan(&u.Id, &u.Username, &u.Password, &u.Email) - if err != nil { - return err - } - } - err = rows.Err() - if err != nil { + err = userReadStmt. + QueryRow(u.Id). + Scan(&u.Id, &u.Username, &u.Password, &u.Email) + switch { + case err == sql.ErrNoRows: + return fmt.Errorf("No user with id %s", u.Id) + case err != nil: return err } - fmt.Println(u) - return nil } -func (u *User) UpdateUser(db *sql.DB) error { +func (u *User) Update(db *sql.DB) error { + // var ( + // err error + // ) return nil } -func (u *User) DeleteUser(db *sql.DB) error { +func (u *User) Delete(db *sql.DB) error { var ( err error ) @@ -154,13 +158,18 @@ func (u *User) DeleteUser(db *sql.DB) error { // prepare statement if not already done so. if userDeleteStmt == nil { // delete statement - stmt := `DELETE FROM users WHERE id = ?` + stmt := `DELETE FROM users WHERE id = $1` userDeleteStmt, err = db.Prepare(stmt) if err != nil { return err } } + _, err = userDeleteStmt.Exec(u.Id) + if err != nil { + return err + } + return nil } diff --git a/server/types/user_test.go b/server/types/user_test.go index a303028..7a32a48 100644 --- a/server/types/user_test.go +++ b/server/types/user_test.go @@ -63,7 +63,52 @@ func (s *UserTestSuite) TestCreateUser() { Email: "hr@worst.nightmare", } - err := user.CreateUser(id, testDB) + err := user.Create(id, testDB) s.NoError(err) s.EqualValues(user, expectedResult) } + +func (s *UserTestSuite) TestReadUser() { + id := uuid.NewV4().String() + expectedUser := &User{ + Username: "dagon", + Password: "bl4ckr33f", + Email: "gasp@unknowable.horror", + } + + // create expected user in db + err := expectedUser.Create(id, testDB) + s.NoError(err) + + user := &User{Id: id} + err = user.Read(testDB) + s.NoError(err) + + s.EqualValues(expectedUser, user) +} + +func (s *UserTestSuite) TestReadUser_NonExistent() { + user := &User{Id: uuid.NewV4().String()} + err := user.Read(testDB) + s.Error(err) +} + +func (s *UserTestSuite) TestDeleteUser() { + id := uuid.NewV4().String() + // create a user + user := &User{ + Username: "colonel_buendias", + Password: "g0ld3nf15h", + Email: "rogue@macondo.gov", + } + err := user.Create(id, testDB) + s.NoError(err) + + // delete a user + err = user.Delete(testDB) + s.NoError(err) + + // read a user (shouldn't exist) + err = user.Read(testDB) + s.Error(err) +} From 3bd002fcff87801ac792864ccba9131cdf6d16ae Mon Sep 17 00:00:00 2001 From: connorwalsh Date: Sat, 17 Feb 2018 14:22:52 -0500 Subject: [PATCH 3/3] [cw|#9] complete ReadAllUsers and include test --- server/types/user.go | 32 +++++++++++++++++++++++--------- server/types/user_test.go | 39 +++++++++++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/server/types/user.go b/server/types/user.go index 2d8dee0..54dd60b 100644 --- a/server/types/user.go +++ b/server/types/user.go @@ -76,8 +76,7 @@ func (*User) CreateTable(db *sql.DB) error { func (u *User) Create(id string, db *sql.DB) error { var ( - rows *sql.Rows - err error + err error ) // we assume that all validation/sanitization has already been called @@ -97,16 +96,11 @@ func (u *User) Create(id string, db *sql.DB) error { } } - rows, err = userCreateStmt.Query(u.Id, u.Username, u.Password, u.Email) + _, err = userCreateStmt.Exec(u.Id, u.Username, u.Password, u.Email) if err != nil { return err } - // read created result - for rows.Next() { - rows.Scan(u.Id, u.Username, u.Password, u.Email) - } - return nil } @@ -183,12 +177,32 @@ func ReadUsers(db *sql.DB) ([]*User, error) { if userReadAllStmt == nil { // readAll statement // TODO pagination - stmt := `SELECT * FROM users` + stmt := `SELECT id, username, email FROM users` userReadAllStmt, err = db.Prepare(stmt) if err != nil { return users, nil } } + rows, err := userReadAllStmt.Query() + if err != nil { + return users, err + } + + defer rows.Close() + for rows.Next() { + user := &User{} + err = rows.Scan(&user.Id, &user.Username, &user.Email) + if err != nil { + return users, err + } + + // append scanned user into list of all users + users = append(users, user) + } + if err := rows.Err(); err != nil { + return users, err + } + return users, nil } diff --git a/server/types/user_test.go b/server/types/user_test.go index 7a32a48..4feb912 100644 --- a/server/types/user_test.go +++ b/server/types/user_test.go @@ -43,6 +43,15 @@ func (s *UserTestSuite) BeforeTest(suiteName, testName string) { if err != nil { panic(err) } + case "TestReadAllUsers": + _, err = s.db.Exec(`DROP TABLE IF EXISTS users CASCADE`) + if err != nil { + panic(err) + } + err := (&User{}).CreateTable(testDB) + if err != nil { + panic(err) + } } } @@ -54,11 +63,11 @@ func (s *UserTestSuite) TestCreateTable() { } func (s *UserTestSuite) TestCreateUser() { - user := &User{Username: "c", Password: "axaxaxax", Email: "hr@worst.nightmare"} + user := &User{Username: "tlon", Password: "axaxaxax", Email: "hr@worst.nightmare"} id := uuid.NewV4().String() expectedResult := &User{ Id: id, - Username: "c", + Username: "tlon", Password: "axaxaxax", Email: "hr@worst.nightmare", } @@ -112,3 +121,29 @@ func (s *UserTestSuite) TestDeleteUser() { err = user.Read(testDB) s.Error(err) } + +func (s *UserTestSuite) TestReadAllUsers() { + var err error + + // create three users + ids := []string{uuid.NewV4().String(), uuid.NewV4().String(), uuid.NewV4().String()} + expectedUsers := []*User{ + &User{Username: "a", Password: "passwd", Email: "a@idk.org"}, + &User{Username: "b", Password: "passwd", Email: "b@idk.org"}, + &User{Username: "c", Password: "passwd", Email: "c@idk.org"}, + } + + for i := 0; i < len(ids); i++ { + err = expectedUsers[i].Create(ids[i], testDB) + s.NoError(err) + + // since we do not read passwords of users, we set them to empty string + expectedUsers[i].Password = "" + } + + // read all users + users, err := ReadUsers(testDB) + s.NoError(err) + + s.EqualValues(expectedUsers, users) +}