From 8e2bfe3f47754942056d0c6973d8fb6ae3628a7e Mon Sep 17 00:00:00 2001 From: smavros Date: Mon, 10 Jun 2019 13:59:29 +0200 Subject: [PATCH 1/4] Changes in getUsers for GET /users endpoint: - User role can only update a user --- common/roles.go | 3 ++- routes/user/userEndpoints.go | 15 +++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/common/roles.go b/common/roles.go index a495eaf..ed7f37d 100644 --- a/common/roles.go +++ b/common/roles.go @@ -39,6 +39,7 @@ type RoleActions map[string]ModelActions // Predefined CRUD operations permissions to be used in Roles var crud = Permission{Create: true, Read: true, Update: true, Delete: true} var _ru_ = Permission{Create: false, Read: true, Update: true, Delete: false} +var __u_ = Permission{Create: false, Read: false, Update: true, Delete: false} var _r__ = Permission{Create: false, Read: true, Update: false, Delete: false} // Roles is used as a look up variable to determine if a certain user is @@ -51,7 +52,7 @@ var Roles = RoleActions{ ModelSimulator: crud, }, "User": { - ModelUser: _ru_, + ModelUser: __u_, ModelSimulation: crud, ModelSimulationModel: crud, ModelSimulator: _r__, diff --git a/routes/user/userEndpoints.go b/routes/user/userEndpoints.go index b41ea76..f89bdbd 100644 --- a/routes/user/userEndpoints.go +++ b/routes/user/userEndpoints.go @@ -134,17 +134,16 @@ func authenticate(c *gin.Context) { // @Failure 500 "Internal server error" // @Router /users [get] func getUsers(c *gin.Context) { - //// dummy TODO: check in the middleware if the user is authorized - //authorized := false - //// TODO: move this redirect in the authentication middleware - //if !authorized { - //c.Redirect(http.StatusSeeOther, "/authenticate") - //return - //} + + err := common.ValidateRole(c, common.ModelUser, common.Read) + if err != nil { + c.JSON(http.StatusUnprocessableEntity, fmt.Sprintf("%v", err)) + return + } db := common.GetDB() var users []common.User - err := db.Order("ID asc").Find(&users).Error + err = db.Order("ID asc").Find(&users).Error if common.ProvideErrorResponse(c, err) { return } From b74b5f3a44569a0177cfea45ebbc6a41f3f47354 Mon Sep 17 00:00:00 2001 From: smavros Date: Mon, 10 Jun 2019 16:47:46 +0200 Subject: [PATCH 2/4] Fixes bug in getUsers(). Adds helper funnction. --- common/utilities.go | 9 +++++++++ routes/user/userEndpoints.go | 15 ++++++++------- 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/common/utilities.go b/common/utilities.go index b432c52..91deb42 100644 --- a/common/utilities.go +++ b/common/utilities.go @@ -125,3 +125,12 @@ func AuthenticateForTest(t *testing.T, router *gin.Engine, url string, method st return body_data["token"].(string) } + +// Read the parameter with name paramName from the gin Context and +// return it as uint variable +func UintParamFromCtx(c *gin.Context, paramName string) (uint, error) { + + param, err := strconv.Atoi(c.Param(paramName)) + + return uint(param), err +} diff --git a/routes/user/userEndpoints.go b/routes/user/userEndpoints.go index f89bdbd..363300d 100644 --- a/routes/user/userEndpoints.go +++ b/routes/user/userEndpoints.go @@ -5,7 +5,6 @@ import ( "github.com/dgrijalva/jwt-go" "github.com/gin-gonic/gin" "net/http" - "strconv" "time" "git.rwth-aachen.de/acs/public/villas/villasweb-backend-go/common" @@ -238,7 +237,7 @@ func addUser(c *gin.Context) { // @Router /users/{userID} [put] func updateUser(c *gin.Context) { - err := common.ValidateRole(c, common.ModelUser, common.Read) + err := common.ValidateRole(c, common.ModelUser, common.Update) if err != nil { c.JSON(http.StatusUnprocessableEntity, fmt.Sprintf("%v", err)) return @@ -246,8 +245,8 @@ func updateUser(c *gin.Context) { // Find the user var user User - toBeUpdatedID, _ := strconv.ParseInt(c.Param("UserID"), 10, 64) - err = user.ByID(uint(toBeUpdatedID)) + toBeUpdatedID, _ := common.UintParamFromCtx(c, "UserID") + err = user.ByID(toBeUpdatedID) if err != nil { c.JSON(http.StatusNotFound, fmt.Sprintf("%v", err)) return @@ -258,11 +257,13 @@ func updateUser(c *gin.Context) { // in the context from the Authentication middleware) userID, _ := c.Get(common.UserIDCtx) userRole, _ := c.Get(common.UserRoleCtx) + if toBeUpdatedID != userID && userRole != "Admin" { c.JSON(http.StatusForbidden, gin.H{ "success": false, "message": "Invalid authorization", }) + return } // Bind the (context) with the User struct @@ -335,9 +336,9 @@ func getUser(c *gin.Context) { } var user User - id, _ := strconv.ParseInt(c.Param("UserID"), 10, 64) + id, _ := common.UintParamFromCtx(c, "UserID") - err = user.ByID(uint(id)) + err = user.ByID(id) if err != nil { c.JSON(http.StatusNotFound, fmt.Sprintf("%v", err)) return @@ -370,7 +371,7 @@ func deleteUser(c *gin.Context) { } var user User - id, _ := strconv.ParseInt(c.Param("UserID"), 10, 64) + id, _ := common.UintParamFromCtx(c, "UserID") // Check that the user exist err = user.ByID(uint(id)) From c68510fa99457cc6ce86265fdea3ac16823d18b7 Mon Sep 17 00:00:00 2001 From: smavros Date: Wed, 12 Jun 2019 16:04:15 +0200 Subject: [PATCH 3/4] Adds validation of login request: - Renames type Credentials to loginRequest --- go.mod | 4 ++++ go.sum | 8 ++++++++ routes/user/userEndpoints.go | 32 +++++++++++++++++++++++++------- routes/user/userMethods.go | 8 -------- 4 files changed, 37 insertions(+), 15 deletions(-) diff --git a/go.mod b/go.mod index 25c82c2..2ac4679 100644 --- a/go.mod +++ b/go.mod @@ -4,11 +4,15 @@ require ( github.com/alecthomas/template v0.0.0-20160405071501-a0175ee3bccc github.com/dgrijalva/jwt-go v3.2.0+incompatible github.com/gin-gonic/gin v1.4.0 + github.com/go-playground/locales v0.12.1 // indirect + github.com/go-playground/universal-translator v0.16.0 // indirect github.com/jinzhu/gorm v1.9.8 + github.com/leodido/go-urn v1.1.0 // indirect github.com/stretchr/testify v1.3.0 github.com/swaggo/gin-swagger v1.1.0 github.com/swaggo/swag v1.5.0 golang.org/x/crypto v0.0.0-20190325154230-a5d413f7728c + gopkg.in/go-playground/validator.v9 v9.29.0 ) replace github.com/ugorji/go v1.1.4 => github.com/ugorji/go/codec v0.0.0-20190204201341-e444a5086c43 diff --git a/go.sum b/go.sum index 39ebcf0..bea7b43 100644 --- a/go.sum +++ b/go.sum @@ -49,6 +49,10 @@ github.com/go-openapi/spec v0.18.0/go.mod h1:XkF/MOi14NmjsfZ8VtAKf8pIlbZzyoTvZsd github.com/go-openapi/swag v0.17.0/go.mod h1:AByQ+nYG6gQg71GINrmuDXCPWdL640yX49/kXLo40Tg= github.com/go-openapi/swag v0.18.0 h1:1DU8Km1MRGv9Pj7BNLmkA+umwTStwDHttXvx3NhJA70= github.com/go-openapi/swag v0.18.0/go.mod h1:AByQ+nYG6gQg71GINrmuDXCPWdL640yX49/kXLo40Tg= +github.com/go-playground/locales v0.12.1 h1:2FITxuFt/xuCNP1Acdhv62OzaCiviiE4kotfhkmOqEc= +github.com/go-playground/locales v0.12.1/go.mod h1:IUMDtCfWo/w/mtMfIE/IG2K+Ey3ygWanZIBtBW0W2TM= +github.com/go-playground/universal-translator v0.16.0 h1:X++omBR/4cE2MNg91AoC3rmGrCjJ8eAeUP/K/EKx4DM= +github.com/go-playground/universal-translator v0.16.0/go.mod h1:1AnU7NaIRDWWzGEKwgtJRd2xk99HeFyHw3yid4rvQIY= github.com/go-sql-driver/mysql v1.4.1 h1:g24URVg0OFbNUTx9qqY1IRZ9D9z3iPyi5zKhQZpNwpA= github.com/go-sql-driver/mysql v1.4.1/go.mod h1:zAC/RDZ24gD3HViQzih4MyKcchzm+sOG5ZlKdlhCg5w= github.com/go-stack/stack v1.8.0/go.mod h1:v0f6uXyyMGvRgIKkXu+yp6POWl0qKG85gN/melR3HDY= @@ -89,6 +93,8 @@ github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORN github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= github.com/kr/text v0.1.0 h1:45sCR5RtlFHMR4UwH9sdQ5TC8v0qDQCHnXt+kaKSTVE= github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= +github.com/leodido/go-urn v1.1.0 h1:Sm1gr51B1kKyfD2BlRcLSiEkffoG96g6TPv6eRoEiB8= +github.com/leodido/go-urn v1.1.0/go.mod h1:+cyI34gQWZcE1eQU7NVgKkkzdXDQHr1dBMtdAPozLkw= github.com/lib/pq v1.1.0 h1:/5u4a+KGJptBRqGzPvYQL9p0d/tPR4S31+Tnzj9lEO4= github.com/lib/pq v1.1.0/go.mod h1:5WUZQaWbwv1U+lTReE5YruASi9Al49XbQIvNi/34Woo= github.com/mailru/easyjson v0.0.0-20180823135443-60711f1a8329/go.mod h1:C1wdFJiN94OJF2b5HbByQZoLdCWB1Yqtg26g4irojpc= @@ -210,6 +216,8 @@ gopkg.in/go-playground/assert.v1 v1.2.1 h1:xoYuJVE7KT85PYWrN730RguIQO0ePzVRfFMXa gopkg.in/go-playground/assert.v1 v1.2.1/go.mod h1:9RXL0bg/zibRAgZUYszZSwO/z8Y/a8bDuhia5mkpMnE= gopkg.in/go-playground/validator.v8 v8.18.2 h1:lFB4DoMU6B626w8ny76MV7VX6W2VHct2GVOI3xgiMrQ= gopkg.in/go-playground/validator.v8 v8.18.2/go.mod h1:RX2a/7Ha8BgOhfk7j780h4/u/RRjR0eouCJSH80/M2Y= +gopkg.in/go-playground/validator.v9 v9.29.0 h1:5ofssLNYgAA/inWn6rTZ4juWpRJUwEnXc1LG2IeXwgQ= +gopkg.in/go-playground/validator.v9 v9.29.0/go.mod h1:+c9/zcJMFNgbLvly1L1V+PpxWdVbfP1avr/N00E2vyQ= gopkg.in/tomb.v1 v1.0.0-20141024135613-dd632973f1e7/go.mod h1:dt/ZhP58zS4L8KSrWDmTeBkI65Dw0HsyUHuEVlX15mw= gopkg.in/yaml.v2 v2.2.1/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI= gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw= diff --git a/routes/user/userEndpoints.go b/routes/user/userEndpoints.go index 363300d..6d7c68a 100644 --- a/routes/user/userEndpoints.go +++ b/routes/user/userEndpoints.go @@ -4,16 +4,24 @@ import ( "fmt" "github.com/dgrijalva/jwt-go" "github.com/gin-gonic/gin" + "gopkg.in/go-playground/validator.v9" "net/http" "time" "git.rwth-aachen.de/acs/public/villas/villasweb-backend-go/common" ) +var validate *validator.Validate + // TODO: the signing secret must be environmental variable const jwtSigningSecret = "This should NOT be here!!@33$8&" const weekHours = time.Hour * 24 * 7 +type loginRequest struct { + Username string `form:"Username" validate:"required"` + Password string `form:"Password" validate:"required,min=8"` +} + type tokenClaims struct { UserID uint `json:"id"` Role string `json:"role"` @@ -44,7 +52,7 @@ func RegisterUserEndpoints(r *gin.RouterGroup) { // @Accept json // @Produce json // @Tags users -// @Param inputUser body user.Credentials true "Credentials of user" +// @Param inputUser body user.loginRequest true "loginRequest of user" // @Success 200 {object} user.AuthResponse "JSON web token and message" // @Failure 401 "Unauthorized Access" // @Failure 404 "Not found" @@ -52,9 +60,9 @@ func RegisterUserEndpoints(r *gin.RouterGroup) { // @Router /authenticate [post] func authenticate(c *gin.Context) { - // Bind the response (context) with the Credentials struct - var loginRequest Credentials - if err := c.ShouldBindJSON(&loginRequest); err != nil { + // Bind the response (context) with the loginRequest struct + var credentials loginRequest + if err := c.ShouldBindJSON(&credentials); err != nil { c.JSON(http.StatusUnprocessableEntity, gin.H{ "success": false, "message": fmt.Sprintf("%v", err), @@ -62,8 +70,18 @@ func authenticate(c *gin.Context) { return } + validate = validator.New() + errs := validate.Struct(credentials) + if errs != nil { + c.JSON(http.StatusBadRequest, gin.H{ + "success": false, + "message": "Invalid credentials", + }) + return + } + // Check if the Username or Password are empty - if loginRequest.Username == "" || loginRequest.Password == "" { + if credentials.Username == "" || credentials.Password == "" { c.JSON(http.StatusUnauthorized, gin.H{ "success": false, "message": "Invalid credentials", @@ -73,7 +91,7 @@ func authenticate(c *gin.Context) { // Find the username in the database var user User - err := user.ByUsername(loginRequest.Username) + err := user.ByUsername(credentials.Username) if err != nil { c.JSON(http.StatusNotFound, gin.H{ "success": false, @@ -83,7 +101,7 @@ func authenticate(c *gin.Context) { } // Validate the password - err = user.validatePassword(loginRequest.Password) + err = user.validatePassword(credentials.Password) if err != nil { c.JSON(http.StatusUnauthorized, gin.H{ "success": false, diff --git a/routes/user/userMethods.go b/routes/user/userMethods.go index c8f2999..850ba25 100644 --- a/routes/user/userMethods.go +++ b/routes/user/userMethods.go @@ -8,14 +8,6 @@ import ( const bcryptCost = 10 -// TODO: use validator -type Credentials struct { - Username string `form:"Username"` - Password string `form:"Password"` - Role string `form:"Role"` - Mail string `form:"Mail"` -} - // This is ugly but no other way to keep each model on the corresponding // package since we have circular dependencies. Methods of a type must // live in the same package. From c33bcd0202f35d6f2a44808141a0d0dfc9b2de5c Mon Sep 17 00:00:00 2001 From: Sonja Happ Date: Thu, 4 Jul 2019 15:50:48 +0200 Subject: [PATCH 4/4] Fix password validation to work with testing DB entries --- common/utilities.go | 1 + routes/user/userEndpoints.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/common/utilities.go b/common/utilities.go index 95bb040..06a8a79 100644 --- a/common/utilities.go +++ b/common/utilities.go @@ -72,6 +72,7 @@ func AuthenticateForTest(t *testing.T, router *gin.Engine, url string, method st success := body_data["success"].(bool) if !success { + fmt.Println("Authentication not successful: ", body_data["message"]) panic(-1) } diff --git a/routes/user/userEndpoints.go b/routes/user/userEndpoints.go index 04e3b30..d062940 100644 --- a/routes/user/userEndpoints.go +++ b/routes/user/userEndpoints.go @@ -19,7 +19,7 @@ const weekHours = time.Hour * 24 * 7 type loginRequest struct { Username string `form:"Username" validate:"required"` - Password string `form:"Password" validate:"required,min=8"` + Password string `form:"Password" validate:"required,min=6"` } type tokenClaims struct {