stop exposing password to the public

bstaijen · bstaijen · commit 3063de2ab867 · 2017-03-02T15:04:46.000-08:00
diff --git a/authentication-service/app/http/controllers/controllers.go b/authentication-service/app/http/controllers/controllers.go
@@ -63,6 +63,7 @@ func LoginHandler(connection *sql.DB, cnf config.Config) negroni.HandlerFunc {
 
 		// Retrieve the user from the database
 		databaseUser, _ := db.GetUserByUsername(connection, login.Username)
+		databaseUser.Password = "" // trick to prevent password from leaking to client
 
 		// Send the token and user back
 		data := &models.Token{
diff --git a/profile-service/app/http/controllers/controllers.go b/profile-service/app/http/controllers/controllers.go
@@ -27,7 +27,7 @@ import (
 // CreateUserHandler creates a new user in the database. Password is saved as a hash.
 func CreateUserHandler(connection *sql.DB, cnf config.Config) negroni.HandlerFunc {
 	return negroni.HandlerFunc(func(w http.ResponseWriter, r *http.Request, next http.HandlerFunc) {
-		user := &models.User{}
+		user := &models.UserCreate{}
 		err := util.RequestToJSON(r, user)
 		if err != nil {
 			util.SendBadRequest(w, errors.New("Bad json"))
@@ -64,9 +64,9 @@ func CreateUserHandler(connection *sql.DB, cnf config.Config) negroni.HandlerFun
 				}
 
 				type Token struct {
-					Token     string      `json:"token"`
-					ExpiresOn string      `json:"expires_on"`
-					User      models.User `json:"user"`
+					Token     string              `json:"token"`
+					ExpiresOn string              `json:"expires_on"`
+					User      models.UserResponse `json:"user"`
 				}
 
 				util.SendOK(w, &Token{
@@ -98,7 +98,7 @@ func DeleteUserHandler(connection *sql.DB, cnf config.Config) negroni.HandlerFun
 			return
 		}
 
-		user := &models.User{}
+		user := &models.UserResponse{}
 		err := util.RequestToJSON(r, user)
 		if err != nil {
 			util.SendBadRequest(w, errors.New("Bad json"))
@@ -143,7 +143,7 @@ func UpdateUserHandler(connection *sql.DB, cnf config.Config) negroni.HandlerFun
 			return
 		}
 
-		user := &models.User{}
+		user := &models.UserResponse{}
 		err := util.RequestToJSON(r, user)
 		if err != nil {
 			util.SendBadRequest(w, errors.New("bad json"))
@@ -168,15 +168,11 @@ func UpdateUserHandler(connection *sql.DB, cnf config.Config) negroni.HandlerFun
 		}
 
 		if err := user.Validate(); err == nil {
-			if err := user.ValidatePassword(); err == nil {
 
-				db.UpdateUser(connection, user)
+			db.UpdateUser(connection, user)
 
-				util.SendOK(w, user)
+			util.SendOK(w, user)
 
-			} else {
-				util.SendBadRequest(w, err)
-			}
 		} else {
 			util.SendBadRequest(w, err)
 		}
diff --git a/profile-service/app/http/controllers/controllers_test.go b/profile-service/app/http/controllers/controllers_test.go
@@ -35,7 +35,7 @@ func TestCreateUser(t *testing.T) {
 	cnf := config.Config{}
 	cnf.SecretKey = "ABCDEF"
 
-	user := &models.User{}
+	user := &models.UserCreate{}
 	user.ID = 1
 	user.Email = "username@example.com"
 	user.Password = "password"
@@ -83,9 +83,9 @@ func TestCreateUser(t *testing.T) {
 
 	// Make sure response is alright
 	type Token struct {
-		Token     string      `json:"token"`
-		ExpiresOn string      `json:"expires_on"`
-		User      models.User `json:"user"`
+		Token     string              `json:"token"`
+		ExpiresOn string              `json:"expires_on"`
+		User      models.UserResponse `json:"user"`
 	}
 
 	response := &Token{}
@@ -141,7 +141,7 @@ func TestCreateUserWithoutUsername(t *testing.T) {
 	cnf := config.Config{}
 	cnf.SecretKey = "ABCDEF"
 
-	user := &models.User{}
+	user := &models.UserCreate{}
 
 	json, _ := json.Marshal(user)
 
@@ -173,7 +173,7 @@ func TestCreateUserWithoutPassword(t *testing.T) {
 	cnf := config.Config{}
 	cnf.SecretKey = "ABCDEF"
 
-	user := &models.User{}
+	user := &models.UserCreate{}
 	user.Email = "test@example.com"
 	user.Username = "username"
 
@@ -207,7 +207,7 @@ func TestCreateUserWithoutEmail(t *testing.T) {
 	cnf := config.Config{}
 	cnf.SecretKey = "ABCDEF"
 
-	user := &models.User{}
+	user := &models.UserCreate{}
 	user.Username = "username"
 
 	json, _ := json.Marshal(user)
@@ -250,7 +250,7 @@ func TestDeleteUser(t *testing.T) {
 	cnf := config.Config{}
 	cnf.SecretKey = "ABCDEF"
 
-	user := &models.User{}
+	user := &models.UserCreate{}
 	user.ID = 1
 	user.Email = "username@example.com"
 	user.Password = "password"
@@ -532,8 +532,8 @@ func TestTryDeleteOtherUser(t *testing.T) {
 	}
 }
 
-func getTestUser() *models.User {
-	user := &models.User{}
+func getTestUser() *models.UserCreate {
+	user := &models.UserCreate{}
 	user.ID = 1
 	user.Email = "username@example.com"
 	user.Password = "password"
@@ -543,7 +543,7 @@ func getTestUser() *models.User {
 	return user
 }
 
-func getTokenString(cnf config.Config, user *models.User, t *testing.T) string {
+func getTokenString(cnf config.Config, user *models.UserCreate, t *testing.T) string {
 	expiration := time.Now().Add(time.Hour * 24 * 31).Unix()
 	token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
 		"sub": user.ID,
diff --git a/profile-service/app/http/routes/routes_test.go b/profile-service/app/http/routes/routes_test.go
@@ -263,8 +263,8 @@ func doRequest(db *sql.DB, cnf config.Config, method string, url string, body *b
 	return res
 }
 
-func getTestUser() *models.User {
-	user := &models.User{}
+func getTestUser() *models.UserCreate {
+	user := &models.UserCreate{}
 	user.ID = 1
 	user.Email = "username@example.com"
 	user.Password = "password"
@@ -274,7 +274,7 @@ func getTestUser() *models.User {
 	return user
 }
 
-func getTokenString(cnf config.Config, user *models.User, t *testing.T) string {
+func getTokenString(cnf config.Config, user *models.UserCreate, t *testing.T) string {
 	expiration := time.Now().Add(time.Hour * 24 * 31).Unix()
 	token := jwt.NewWithClaims(jwt.SigningMethodHS256, jwt.MapClaims{
 		"sub": user.ID,
diff --git a/profile-service/app/models/user.go b/profile-service/app/models/user.go
@@ -9,28 +9,49 @@ import (
 // lower_case private, upper_case public
 // Uppercase variable is mandatory for exposing to json
 
-// User model
-type User struct {
+// UserResponse model
+type UserResponse struct {
 	ID        int       `json:"id"`
 	Username  string    `json:"username"`
 	Email     string    `json:"email"`
 	CreatedAt time.Time `json:"createdAt"`
-	Password  string    `json:"password,omitempty"`
+}
+
+// UserCreate model
+type UserCreate struct {
+	ID        int       `json:"id"`
+	Username  string    `json:"username"`
+	Email     string    `json:"email"`
+	CreatedAt time.Time `json:"createdAt"`
+	Password  string    `json:"password"`
 	Hash      string
 }
 
 // GetUsername returns the username of an user
-func (u *User) GetUsername() string {
+func (u *UserCreate) GetUsername() string {
 	return u.Username
 }
 
 // Print method of User
-func (u *User) Print() string {
+func (u *UserCreate) Print() string {
 	return fmt.Sprintf("%v (%v) - %v", u.Username, u.ID, u.CreatedAt)
 }
 
 // Validate returns an error when the username or email is to short.
-func (u *User) Validate() error {
+func (u *UserCreate) Validate() error {
+	if len(u.Username) < 1 {
+		return ErrUsernameTooShort
+	}
+
+	if len(u.Email) < 1 {
+		return ErrEmailTooShort
+	}
+
+	return nil
+}
+
+// Validate returns an error when the username or email is to short.
+func (u *UserResponse) Validate() error {
 	if len(u.Username) < 1 {
 		return ErrUsernameTooShort
 	}
@@ -43,7 +64,7 @@ func (u *User) Validate() error {
 }
 
 // ValidatePassword returns an error if the password is to small.
-func (u *User) ValidatePassword() error {
+func (u *UserCreate) ValidatePassword() error {
 	if len(u.Password) < 1 {
 		return ErrPasswordTooShort
 	}
diff --git a/profile-service/app/models/user_test.go b/profile-service/app/models/user_test.go
@@ -8,8 +8,8 @@ import (
 	"github.com/bstaijen/mariadb-for-microservices/profile-service/app/models"
 )
 
-func getSimpleUser() models.User {
-	user := models.User{}
+func getSimpleUser() models.UserCreate {
+	user := models.UserCreate{}
 	user.ID = 1
 	user.Username = "user"
 	user.Email = "user@example.com"
@@ -18,7 +18,7 @@ func getSimpleUser() models.User {
 }
 
 func TestGetUsername(t *testing.T) {
-	user := models.User{}
+	user := models.UserCreate{}
 	user.Username = "user"
 
 	expected := "user"
@@ -58,7 +58,7 @@ func TestValidate(t *testing.T) {
 }
 
 func TestTooShortUsername(t *testing.T) {
-	user := models.User{}
+	user := models.UserCreate{}
 	user.Username = ""
 	user.Password = "pass"
 	err := user.Validate()
@@ -73,7 +73,7 @@ func TestTooShortUsername(t *testing.T) {
 }
 
 func TestTooShortEMail(t *testing.T) {
-	user := models.User{}
+	user := models.UserCreate{}
 	user.Username = "user"
 	user.Email = ""
 	err := user.Validate()
@@ -88,7 +88,7 @@ func TestTooShortEMail(t *testing.T) {
 }
 
 func TestTooShortPassword(t *testing.T) {
-	user := models.User{}
+	user := models.UserCreate{}
 	user.Username = "user"
 	user.Password = ""
 	err := user.ValidatePassword()
diff --git a/profile-service/database/db.go b/profile-service/database/db.go
@@ -46,10 +46,10 @@ func CloseConnection(db *sql.DB) {
 }
 
 // GetUserByID returns an models.User identified by it's ID or a ErrUserNotFound error when the user cannot be found.
-func GetUserByID(db *sql.DB, ID int) (models.User, error) {
-	rows, err := db.Query("SELECT id, username, createdAt, password, email FROM users WHERE id = ?", ID)
+func GetUserByID(db *sql.DB, ID int) (models.UserResponse, error) {
+	rows, err := db.Query("SELECT id, username, createdAt, email FROM users WHERE id = ?", ID)
 	if err != nil {
-		return models.User{}, err
+		return models.UserResponse{}, err
 	}
 
 	if rows.Next() {
@@ -60,16 +60,16 @@ func GetUserByID(db *sql.DB, ID int) (models.User, error) {
 		var email string
 		err = rows.Scan(&id, &username, &createdAt, &password, &email)
 		if err != nil {
-			return models.User{}, err
+			return models.UserResponse{}, err
 		}
 
-		return models.User{ID: id, Username: username, CreatedAt: createdAt, Password: password, Email: email}, nil
+		return models.UserResponse{ID: id, Username: username, CreatedAt: createdAt, Email: email}, nil
 	}
-	return models.User{}, ErrUserNotFound
+	return models.UserResponse{}, ErrUserNotFound
 }
 
 // CreateUser create an user in the database and returns the ID of the user being inserted. This method returns a ErrUsernameIsNotUnique or ErrEmailIsNotUnique when the username or email of an user is not unique.
-func CreateUser(db *sql.DB, user *models.User) (int, error) {
+func CreateUser(db *sql.DB, user *models.UserCreate) (int, error) {
 	// check unique username
 	query := "SELECT * FROM users WHERE username = ?"
 	rows, err := db.Query(query, user.Username)
@@ -112,7 +112,7 @@ func CreateUser(db *sql.DB, user *models.User) (int, error) {
 }
 
 // UpdateUser updates the username and email of an user. (note: this method does not check if user is authorized to update this row)
-func UpdateUser(db *sql.DB, user *models.User) (int, error) {
+func UpdateUser(db *sql.DB, user *models.UserResponse) (int, error) {
 	_, err := db.Exec("UPDATE users SET username = ?, email = ? WHERE id = ?", user.Username, user.Email, user.ID)
 	if err != nil {
 		log.Errorf("Error inserting")
@@ -123,7 +123,7 @@ func UpdateUser(db *sql.DB, user *models.User) (int, error) {
 }
 
 // DeleteUser deletes an user from the database. Method does not check if the caller is authorized to perform this action. Method returns the number of rows affected by query. (should be 1)
-func DeleteUser(db *sql.DB, user *models.User) (int, error) {
+func DeleteUser(db *sql.DB, user *models.UserResponse) (int, error) {
 	if user.ID > 0 {
 		res, err := db.Exec("DELETE from users WHERE id = ?", user.ID)
 		if err != nil {
@@ -145,13 +145,13 @@ func DeleteUser(db *sql.DB, user *models.User) (int, error) {
 }
 
 // GetUsers returns a list of all database-users. Note: Consider implementing a paging function because this method returns EVERY users at once.
-func GetUsers(db *sql.DB) ([]models.User, error) {
+func GetUsers(db *sql.DB) ([]models.UserResponse, error) {
 
 	rows, err := db.Query("SELECT id, username, email, createdAt FROM users")
 	if err != nil {
 		return nil, err
 	}
-	persons := make([]models.User, 0)
+	persons := make([]models.UserResponse, 0)
 
 	for rows.Next() {
 		var id int
@@ -162,7 +162,7 @@ func GetUsers(db *sql.DB) ([]models.User, error) {
 		if err != nil {
 			return nil, err
 		}
-		persons = append(persons, models.User{ID: id, Username: username, CreatedAt: createdAt})
+		persons = append(persons, models.UserResponse{ID: id, Username: username, CreatedAt: createdAt})
 	}
 	return persons, nil
 }
diff --git a/profile-service/database/db_test.go b/profile-service/database/db_test.go

Original file line number	Diff line number	Diff line change
`@@ -46,10 +46,10 @@ func CloseConnection(db *sql.DB) {`
`46`	`46`	`}`
`47`	`47`
`48`	`48`	`// GetUserByID returns an models.User identified by it's ID or a ErrUserNotFound error when the user cannot be found.`
`49`		`-func GetUserByID(db *sql.DB, ID int) (models.User, error) {`
`50`		`- rows, err := db.Query("SELECT id, username, createdAt, password, email FROM users WHERE id = ?", ID)`
	`49`	`+func GetUserByID(db *sql.DB, ID int) (models.UserResponse, error) {`
	`50`	`+ rows, err := db.Query("SELECT id, username, createdAt, email FROM users WHERE id = ?", ID)`
`51`	`51`	`if err != nil {`
`52`		`- return models.User{}, err`
	`52`	`+ return models.UserResponse{}, err`
`53`	`53`	`}`
`54`	`54`
`55`	`55`	`if rows.Next() {`
`@@ -60,16 +60,16 @@ func GetUserByID(db *sql.DB, ID int) (models.User, error) {`
`60`	`60`	`var email string`
`61`	`61`	`err = rows.Scan(&id, &username, &createdAt, &password, &email)`
`62`	`62`	`if err != nil {`
`63`		`- return models.User{}, err`
	`63`	`+ return models.UserResponse{}, err`
`64`	`64`	`}`
`65`	`65`
`66`		`- return models.User{ID: id, Username: username, CreatedAt: createdAt, Password: password, Email: email}, nil`
	`66`	`+ return models.UserResponse{ID: id, Username: username, CreatedAt: createdAt, Email: email}, nil`
`67`	`67`	`}`
`68`		`- return models.User{}, ErrUserNotFound`
	`68`	`+ return models.UserResponse{}, ErrUserNotFound`
`69`	`69`	`}`
`70`	`70`
`71`	`71`	`// CreateUser create an user in the database and returns the ID of the user being inserted. This method returns a ErrUsernameIsNotUnique or ErrEmailIsNotUnique when the username or email of an user is not unique.`
`72`		`-func CreateUser(db sql.DB, user models.User) (int, error) {`
	`72`	`+func CreateUser(db sql.DB, user models.UserCreate) (int, error) {`
`73`	`73`	`// check unique username`
`74`	`74`	`query := "SELECT * FROM users WHERE username = ?"`
`75`	`75`	`rows, err := db.Query(query, user.Username)`
`@@ -112,7 +112,7 @@ func CreateUser(db sql.DB, user models.User) (int, error) {`
`112`	`112`	`}`
`113`	`113`
`114`	`114`	`// UpdateUser updates the username and email of an user. (note: this method does not check if user is authorized to update this row)`
`115`		`-func UpdateUser(db sql.DB, user models.User) (int, error) {`
	`115`	`+func UpdateUser(db sql.DB, user models.UserResponse) (int, error) {`
`116`	`116`	`_, err := db.Exec("UPDATE users SET username = ?, email = ? WHERE id = ?", user.Username, user.Email, user.ID)`
`117`	`117`	`if err != nil {`
`118`	`118`	`log.Errorf("Error inserting")`
`@@ -123,7 +123,7 @@ func UpdateUser(db sql.DB, user models.User) (int, error) {`
`123`	`123`	`}`
`124`	`124`
`125`	`125`	`// DeleteUser deletes an user from the database. Method does not check if the caller is authorized to perform this action. Method returns the number of rows affected by query. (should be 1)`
`126`		`-func DeleteUser(db sql.DB, user models.User) (int, error) {`
	`126`	`+func DeleteUser(db sql.DB, user models.UserResponse) (int, error) {`
`127`	`127`	`if user.ID > 0 {`
`128`	`128`	`res, err := db.Exec("DELETE from users WHERE id = ?", user.ID)`
`129`	`129`	`if err != nil {`
`@@ -145,13 +145,13 @@ func DeleteUser(db sql.DB, user models.User) (int, error) {`
`145`	`145`	`}`
`146`	`146`
`147`	`147`	`// GetUsers returns a list of all database-users. Note: Consider implementing a paging function because this method returns EVERY users at once.`
`148`		`-func GetUsers(db *sql.DB) ([]models.User, error) {`
	`148`	`+func GetUsers(db *sql.DB) ([]models.UserResponse, error) {`
`149`	`149`
`150`	`150`	`rows, err := db.Query("SELECT id, username, email, createdAt FROM users")`
`151`	`151`	`if err != nil {`
`152`	`152`	`return nil, err`
`153`	`153`	`}`
`154`		`- persons := make([]models.User, 0)`
	`154`	`+ persons := make([]models.UserResponse, 0)`
`155`	`155`
`156`	`156`	`for rows.Next() {`
`157`	`157`	`var id int`
`@@ -162,7 +162,7 @@ func GetUsers(db *sql.DB) ([]models.User, error) {`
`162`	`162`	`if err != nil {`
`163`	`163`	`return nil, err`
`164`	`164`	`}`
`165`		`- persons = append(persons, models.User{ID: id, Username: username, CreatedAt: createdAt})`
	`165`	`+ persons = append(persons, models.UserResponse{ID: id, Username: username, CreatedAt: createdAt})`
`166`	`166`	`}`
`167`	`167`	`return persons, nil`
`168`	`168`	`}`