Browse Source

Refactored user functions to use refrences rather than values

walkerwmanuel 2 years ago
parent
commit
71d66b7f93
11 changed files with 68 additions and 103 deletions
  1. 1 0
      .gitignore
  2. 2 2
      auth/auth.go
  3. 1 1
      auth/nodecallback.go
  4. 1 1
      controllers/network_test.go
  5. 1 1
      controllers/node.go
  6. 9 27
      controllers/user.go
  7. 23 43
      controllers/user_test.go
  8. 24 22
      logic/auth.go
  9. 1 1
      logic/jwts.go
  10. 4 4
      logic/users.go
  11. 1 1
      serverctl/serverctl.go

+ 1 - 0
.gitignore

@@ -22,3 +22,4 @@ data/
 .vscode/
 .idea/
 netmaker.exe
+netmaker.code-workspace

+ 2 - 2
auth/auth.go

@@ -171,7 +171,7 @@ func addUser(email string) error {
 		Password: newPass,
 	}
 	if !hasAdmin { // must be first attempt, create an admin
-		if newUser, err = logic.CreateAdmin(newUser); err != nil {
+		if err = logic.CreateAdmin(&newUser); err != nil {
 			logger.Log(1, "error creating admin from user,", email, "; user not added")
 		} else {
 			logger.Log(1, "admin created from user,", email, "; was first user added")
@@ -179,7 +179,7 @@ func addUser(email string) error {
 	} else { // otherwise add to db as admin..?
 		// TODO: add ability to add users with preemptive permissions
 		newUser.IsAdmin = false
-		if newUser, err = logic.CreateUser(newUser); err != nil {
+		if err = logic.CreateUser(&newUser); err != nil {
 			logger.Log(1, "error creating user,", email, "; user not added")
 		} else {
 			logger.Log(0, "user created from ", email)

+ 1 - 1
auth/nodecallback.go

@@ -257,5 +257,5 @@ func isUserIsAllowed(username, network string, shouldAddUser bool) (*models.User
 		}
 	}
 
-	return &user, nil
+	return user, nil
 }

+ 1 - 1
controllers/network_test.go

@@ -327,7 +327,7 @@ func initialize() {
 }
 
 func createAdminUser() {
-	logic.CreateAdmin(models.User{
+	logic.CreateAdmin(&models.User{
 		UserName: "admin",
 		Password: "password",
 		IsAdmin:  true,

+ 1 - 1
controllers/node.go

@@ -408,7 +408,7 @@ func getAllNodes(w http.ResponseWriter, r *http.Request) {
 			return
 		}
 	} else {
-		nodes, err = getUsersNodes(user)
+		nodes, err = getUsersNodes(*user)
 		if err != nil {
 			logger.Log(0, r.Header.Get("user"),
 				"error fetching nodes: ", err.Error())

+ 9 - 27
controllers/user.go

@@ -9,7 +9,6 @@ import (
 	"github.com/gorilla/mux"
 	"github.com/gorilla/websocket"
 	"github.com/gravitl/netmaker/auth"
-	"github.com/gravitl/netmaker/database"
 	"github.com/gravitl/netmaker/logger"
 	"github.com/gravitl/netmaker/logic"
 	"github.com/gravitl/netmaker/models"
@@ -136,20 +135,6 @@ func hasAdmin(w http.ResponseWriter, r *http.Request) {
 
 }
 
-// GetUserInternal - gets an internal user
-func GetUserInternal(username string) (models.User, error) {
-
-	var user models.User
-	record, err := database.FetchRecord(database.USERS_TABLE_NAME, username)
-	if err != nil {
-		return user, err
-	}
-	if err = json.Unmarshal([]byte(record), &user); err != nil {
-		return models.User{}, err
-	}
-	return user, err
-}
-
 // swagger:route GET /api/users/{username} user getUser
 //
 // Get an individual user.
@@ -235,7 +220,7 @@ func createAdmin(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	admin, err = logic.CreateAdmin(admin)
+	err = logic.CreateAdmin(&admin)
 	if err != nil {
 		logger.Log(0, admin.UserName, "failed to create admin: ",
 			err.Error())
@@ -270,7 +255,7 @@ func createUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	user, err = logic.CreateUser(user)
+	err = logic.CreateUser(&user)
 	if err != nil {
 		logger.Log(0, user.UserName, "error creating new user: ",
 			err.Error())
@@ -295,10 +280,9 @@ func createUser(w http.ResponseWriter, r *http.Request) {
 func updateUserNetworks(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 	var params = mux.Vars(r)
-	var user models.User
 	// start here
 	username := params["username"]
-	user, err := GetUserInternal(username)
+	user, err := logic.GetUser(username)
 	if err != nil {
 		logger.Log(0, username,
 			"failed to update user networks: ", err.Error())
@@ -345,17 +329,16 @@ func updateUserNetworks(w http.ResponseWriter, r *http.Request) {
 func updateUser(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 	var params = mux.Vars(r)
-	var user models.User
 	// start here
 	username := params["username"]
-	user, err := GetUserInternal(username)
+	user, err := logic.GetUser(username)
 	if err != nil {
 		logger.Log(0, username,
 			"failed to update user info: ", err.Error())
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
 		return
 	}
-	if auth.IsOauthUser(&user) == nil {
+	if auth.IsOauthUser(user) == nil {
 		err := fmt.Errorf("cannot update user info for oauth user %s", username)
 		logger.Log(0, err.Error())
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "forbidden"))
@@ -371,7 +354,7 @@ func updateUser(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	userchange.Networks = nil
-	user, err = logic.UpdateUser(userchange, user)
+	user, err = logic.UpdateUser(&userchange, user)
 	if err != nil {
 		logger.Log(0, username,
 			"failed to update user info: ", err.Error())
@@ -396,15 +379,14 @@ func updateUser(w http.ResponseWriter, r *http.Request) {
 func updateUserAdm(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 	var params = mux.Vars(r)
-	var user models.User
 	// start here
 	username := params["username"]
-	user, err := GetUserInternal(username)
+	user, err := logic.GetUser(username)
 	if err != nil {
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
 		return
 	}
-	if auth.IsOauthUser(&user) != nil {
+	if auth.IsOauthUser(user) != nil {
 		err := fmt.Errorf("cannot update user info for oauth user %s", username)
 		logger.Log(0, err.Error())
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "forbidden"))
@@ -423,7 +405,7 @@ func updateUserAdm(w http.ResponseWriter, r *http.Request) {
 		logger.Log(0, username, "not an admin user")
 		logic.ReturnErrorResponse(w, r, logic.FormatError(errors.New("not a admin user"), "badrequest"))
 	}
-	user, err = logic.UpdateUser(userchange, user)
+	user, err = logic.UpdateUser(&userchange, user)
 	if err != nil {
 		logger.Log(0, username,
 			"failed to update user (admin) info: ", err.Error())

+ 23 - 43
controllers/user_test.go

@@ -32,7 +32,7 @@ func TestHasAdmin(t *testing.T) {
 	})
 	t.Run("No admin user", func(t *testing.T) {
 		var user = models.User{"noadmin", "password", nil, false, nil}
-		_, err := logic.CreateUser(user)
+		err := logic.CreateUser(&user)
 		assert.Nil(t, err)
 		found, err := logic.HasAdmin()
 		assert.Nil(t, err)
@@ -40,7 +40,7 @@ func TestHasAdmin(t *testing.T) {
 	})
 	t.Run("admin user", func(t *testing.T) {
 		var user = models.User{"admin", "password", nil, true, nil}
-		_, err := logic.CreateUser(user)
+		err := logic.CreateUser(&user)
 		assert.Nil(t, err)
 		found, err := logic.HasAdmin()
 		assert.Nil(t, err)
@@ -48,7 +48,7 @@ func TestHasAdmin(t *testing.T) {
 	})
 	t.Run("multiple admins", func(t *testing.T) {
 		var user = models.User{"admin1", "password", nil, true, nil}
-		_, err := logic.CreateUser(user)
+		 err := logic.CreateUser(&user)
 		assert.Nil(t, err)
 		found, err := logic.HasAdmin()
 		assert.Nil(t, err)
@@ -61,12 +61,11 @@ func TestCreateUser(t *testing.T) {
 	deleteAllUsers()
 	user := models.User{"admin", "password", nil, true, nil}
 	t.Run("NoUser", func(t *testing.T) {
-		admin, err := logic.CreateUser(user)
+		err := logic.CreateUser(&user)
 		assert.Nil(t, err)
-		assert.Equal(t, user.UserName, admin.UserName)
 	})
 	t.Run("UserExists", func(t *testing.T) {
-		_, err := logic.CreateUser(user)
+		err := logic.CreateUser(&user)
 		assert.NotNil(t, err)
 		assert.EqualError(t, err, "user exists")
 	})
@@ -79,16 +78,14 @@ func TestCreateAdmin(t *testing.T) {
 	t.Run("NoAdmin", func(t *testing.T) {
 		user.UserName = "admin"
 		user.Password = "password"
-		admin, err := logic.CreateAdmin(user)
+		err := logic.CreateAdmin(&user)
 		assert.Nil(t, err)
-		assert.Equal(t, user.UserName, admin.UserName)
 	})
 	t.Run("AdminExists", func(t *testing.T) {
 		user.UserName = "admin2"
 		user.Password = "password1"
-		admin, err := logic.CreateAdmin(user)
+		err := logic.CreateAdmin(&user)
 		assert.EqualError(t, err, "admin user already exists")
-		assert.Equal(t, admin, models.User{})
 	})
 }
 
@@ -102,7 +99,7 @@ func TestDeleteUser(t *testing.T) {
 	})
 	t.Run("Existing User", func(t *testing.T) {
 		user := models.User{"admin", "password", nil, true, nil}
-		logic.CreateUser(user)
+		logic.CreateUser(&user)
 		deleted, err := logic.DeleteUser("admin")
 		assert.Nil(t, err)
 		assert.True(t, deleted)
@@ -115,44 +112,44 @@ func TestValidateUser(t *testing.T) {
 	t.Run("Valid Create", func(t *testing.T) {
 		user.UserName = "admin"
 		user.Password = "validpass"
-		err := logic.ValidateUser(user)
+		err := logic.ValidateUser(&user)
 		assert.Nil(t, err)
 	})
 	t.Run("Valid Update", func(t *testing.T) {
 		user.UserName = "admin"
 		user.Password = "password"
-		err := logic.ValidateUser(user)
+		err := logic.ValidateUser(&user)
 		assert.Nil(t, err)
 	})
 	t.Run("Invalid UserName", func(t *testing.T) {
 		t.Skip()
 		user.UserName = "*invalid"
-		err := logic.ValidateUser(user)
+		err := logic.ValidateUser(&user)
 		assert.Error(t, err)
 		//assert.Contains(t, err.Error(), "Field validation for 'UserName' failed")
 	})
 	t.Run("Short UserName", func(t *testing.T) {
 		t.Skip()
 		user.UserName = "1"
-		err := logic.ValidateUser(user)
+		err := logic.ValidateUser(&user)
 		assert.NotNil(t, err)
 		//assert.Contains(t, err.Error(), "Field validation for 'UserName' failed")
 	})
 	t.Run("Empty UserName", func(t *testing.T) {
 		t.Skip()
 		user.UserName = ""
-		err := logic.ValidateUser(user)
+		err := logic.ValidateUser(&user)
 		assert.EqualError(t, err, "some string")
 		//assert.Contains(t, err.Error(), "Field validation for 'UserName' failed")
 	})
 	t.Run("EmptyPassword", func(t *testing.T) {
 		user.Password = ""
-		err := logic.ValidateUser(user)
+		err := logic.ValidateUser(&user)
 		assert.EqualError(t, err, "Key: 'User.Password' Error:Field validation for 'Password' failed on the 'required' tag")
 	})
 	t.Run("ShortPassword", func(t *testing.T) {
 		user.Password = "123"
-		err := logic.ValidateUser(user)
+		err := logic.ValidateUser(&user)
 		assert.EqualError(t, err, "Key: 'User.Password' Error:Field validation for 'Password' failed on the 'min' tag")
 	})
 }
@@ -167,30 +164,13 @@ func TestGetUser(t *testing.T) {
 	})
 	t.Run("UserExisits", func(t *testing.T) {
 		user := models.User{"admin", "password", nil, true, nil}
-		logic.CreateUser(user)
+		logic.CreateUser(&user)
 		admin, err := logic.GetUser("admin")
 		assert.Nil(t, err)
 		assert.Equal(t, user.UserName, admin.UserName)
 	})
 }
 
-func TestGetUserInternal(t *testing.T) {
-	database.InitializeDatabase()
-	deleteAllUsers()
-	t.Run("NonExistantUser", func(t *testing.T) {
-		admin, err := GetUserInternal("admin")
-		assert.EqualError(t, err, "could not find any records")
-		assert.Equal(t, "", admin.UserName)
-	})
-	t.Run("UserExisits", func(t *testing.T) {
-		user := models.User{"admin", "password", nil, true, nil}
-		logic.CreateUser(user)
-		admin, err := GetUserInternal("admin")
-		assert.Nil(t, err)
-		assert.Equal(t, user.UserName, admin.UserName)
-	})
-}
-
 func TestGetUsers(t *testing.T) {
 	database.InitializeDatabase()
 	deleteAllUsers()
@@ -201,14 +181,14 @@ func TestGetUsers(t *testing.T) {
 	})
 	t.Run("UserExisits", func(t *testing.T) {
 		user := models.User{"admin", "password", nil, true, nil}
-		logic.CreateUser(user)
+		logic.CreateUser(&user)
 		admins, err := logic.GetUsers()
 		assert.Nil(t, err)
 		assert.Equal(t, user.UserName, admins[0].UserName)
 	})
 	t.Run("MulipleUsers", func(t *testing.T) {
 		user := models.User{"user", "password", nil, true, nil}
-		logic.CreateUser(user)
+		logic.CreateUser(&user)
 		admins, err := logic.GetUsers()
 		assert.Nil(t, err)
 		for _, u := range admins {
@@ -228,14 +208,14 @@ func TestUpdateUser(t *testing.T) {
 	user := models.User{"admin", "password", nil, true, nil}
 	newuser := models.User{"hello", "world", []string{"wirecat, netmaker"}, true, []string{}}
 	t.Run("NonExistantUser", func(t *testing.T) {
-		admin, err := logic.UpdateUser(newuser, user)
+		admin, err := logic.UpdateUser(&newuser, &user)
 		assert.EqualError(t, err, "could not find any records")
 		assert.Equal(t, "", admin.UserName)
 	})
 
 	t.Run("UserExists", func(t *testing.T) {
-		logic.CreateUser(user)
-		admin, err := logic.UpdateUser(newuser, user)
+		logic.CreateUser(&user)
+		admin, err := logic.UpdateUser(&newuser, &user)
 		assert.Nil(t, err)
 		assert.Equal(t, newuser.UserName, admin.UserName)
 	})
@@ -292,7 +272,7 @@ func TestVerifyAuthRequest(t *testing.T) {
 	})
 	t.Run("Non-Admin", func(t *testing.T) {
 		user := models.User{"nonadmin", "somepass", nil, false, []string{}}
-		logic.CreateUser(user)
+		logic.CreateUser(&user)
 		authRequest := models.UserAuthParams{"nonadmin", "somepass"}
 		jwt, err := logic.VerifyAuthRequest(authRequest)
 		assert.NotNil(t, jwt)
@@ -300,7 +280,7 @@ func TestVerifyAuthRequest(t *testing.T) {
 	})
 	t.Run("WrongPassword", func(t *testing.T) {
 		user := models.User{"admin", "password", nil, false, []string{}}
-		logic.CreateUser(user)
+		logic.CreateUser(&user)
 		authRequest := models.UserAuthParams{"admin", "badpass"}
 		jwt, err := logic.VerifyAuthRequest(authRequest)
 		assert.Equal(t, "", jwt)

+ 24 - 22
logic/auth.go

@@ -81,20 +81,20 @@ func GetUsers() ([]models.ReturnUser, error) {
 }
 
 // CreateUser - creates a user
-func CreateUser(user models.User) (models.User, error) {
+func CreateUser(user *models.User) error {
 	// check if user exists
 	if _, err := GetUser(user.UserName); err == nil {
-		return models.User{}, errors.New("user exists")
+		return errors.New("user exists")
 	}
 	var err = ValidateUser(user)
 	if err != nil {
-		return models.User{}, err
+		return err
 	}
 
 	// encrypt that password so we never see it again
 	hash, err := bcrypt.GenerateFromPassword([]byte(user.Password), 5)
 	if err != nil {
-		return user, err
+		return err
 	}
 	// set password to encrypted password
 	user.Password = string(hash)
@@ -102,19 +102,19 @@ func CreateUser(user models.User) (models.User, error) {
 	tokenString, _ := CreateProUserJWT(user.UserName, user.Networks, user.Groups, user.IsAdmin)
 	if tokenString == "" {
 		// logic.ReturnErrorResponse(w, r, errorResponse)
-		return user, err
+		return err
 	}
 
-	SetUserDefaults(&user)
+	SetUserDefaults(user)
 
 	// connect db
-	data, err := json.Marshal(&user)
+	data, err := json.Marshal(user)
 	if err != nil {
-		return user, err
+		return err
 	}
 	err = database.Insert(user.UserName, string(data), database.USERS_TABLE_NAME)
 	if err != nil {
-		return user, err
+		return err
 	}
 
 	// == PRO == Add user to every network as network user ==
@@ -153,17 +153,17 @@ func CreateUser(user models.User) (models.User, error) {
 	}
 	// == END PRO ==
 
-	return user, nil
+	return nil
 }
 
 // CreateAdmin - creates an admin user
-func CreateAdmin(admin models.User) (models.User, error) {
+func CreateAdmin(admin *models.User) error {
 	hasadmin, err := HasAdmin()
 	if err != nil {
-		return models.User{}, err
+		return err
 	}
 	if hasadmin {
-		return models.User{}, errors.New("admin user already exists")
+		return errors.New("admin user already exists")
 	}
 	admin.IsAdmin = true
 	return CreateUser(admin)
@@ -242,22 +242,24 @@ func UpdateUserNetworks(newNetworks, newGroups []string, isadmin bool, currentUs
 		currentUser.Networks = newNetworks
 	}
 
-	_, err = UpdateUser(models.User{
+	userChange := models.User{
 		UserName: currentUser.UserName,
 		Networks: currentUser.Networks,
 		IsAdmin:  currentUser.IsAdmin,
 		Password: "",
 		Groups:   currentUser.Groups,
-	}, returnedUser)
+	}
+
+	_, err = UpdateUser(&userChange, returnedUser)
 
 	return err
 }
 
 // UpdateUser - updates a given user
-func UpdateUser(userchange models.User, user models.User) (models.User, error) {
+func UpdateUser(userchange, user *models.User) (*models.User, error) {
 	// check if user exists
 	if _, err := GetUser(user.UserName); err != nil {
-		return models.User{}, err
+		return &models.User{}, err
 	}
 
 	queryUser := user.UserName
@@ -290,25 +292,25 @@ func UpdateUser(userchange models.User, user models.User) (models.User, error) {
 
 	err := ValidateUser(user)
 	if err != nil {
-		return models.User{}, err
+		return &models.User{}, err
 	}
 
 	if err = database.DeleteRecord(database.USERS_TABLE_NAME, queryUser); err != nil {
-		return models.User{}, err
+		return &models.User{}, err
 	}
 	data, err := json.Marshal(&user)
 	if err != nil {
-		return models.User{}, err
+		return &models.User{}, err
 	}
 	if err = database.Insert(user.UserName, string(data), database.USERS_TABLE_NAME); err != nil {
-		return models.User{}, err
+		return &models.User{}, err
 	}
 	logger.Log(1, "updated user", queryUser)
 	return user, nil
 }
 
 // ValidateUser - validates a user model
-func ValidateUser(user models.User) error {
+func ValidateUser(user *models.User) error {
 
 	v := validator.New()
 	_ = v.RegisterValidation("in_charset", func(fl validator.FieldLevel) bool {

+ 1 - 1
logic/jwts.go

@@ -114,7 +114,7 @@ func VerifyUserToken(tokenString string) (username string, networks []string, is
 	})
 
 	if token != nil && token.Valid {
-		var user models.User
+		var user *models.User
 		// check that user exists
 		user, err = GetUser(claims.UserName)
 		if err != nil {

+ 4 - 4
logic/users.go

@@ -11,17 +11,17 @@ import (
 )
 
 // GetUser - gets a user
-func GetUser(username string) (models.User, error) {
+func GetUser(username string) (*models.User, error) {
 
 	var user models.User
 	record, err := database.FetchRecord(database.USERS_TABLE_NAME, username)
 	if err != nil {
-		return user, err
+		return &user, err
 	}
 	if err = json.Unmarshal([]byte(record), &user); err != nil {
-		return models.User{}, err
+		return &models.User{}, err
 	}
-	return user, err
+	return &user, err
 }
 
 // GetGroupUsers - gets users in a group

+ 1 - 1
serverctl/serverctl.go

@@ -173,7 +173,7 @@ func setUserDefaults() error {
 		if err != nil {
 			logger.Log(0, "could not update user", updateUser.UserName)
 		}
-		logic.SetUserDefaults(&updateUser)
+		logic.SetUserDefaults(updateUser)
 		copyUser := updateUser
 		copyUser.Password = ""
 		if _, err = logic.UpdateUser(copyUser, updateUser); err != nil {