refactor user CRUD operations

controllers/network_test.go

@@ -89,27 +89,27 @@ func TestSecurityCheck(t *testing.T) {
 
 	os.Setenv("MASTER_KEY", "secretkey")
 	t.Run("NoNetwork", func(t *testing.T) {
-		networks, username, err := logic.UserPermissions(false, "", "Bearer secretkey")
+		username, err := logic.UserPermissions(false, "", "Bearer secretkey")
 		assert.Nil(t, err)
-		t.Log(networks, username)
+		t.Log(username)
 	})
 	t.Run("WithNetwork", func(t *testing.T) {
-		networks, username, err := logic.UserPermissions(false, "skynet", "Bearer secretkey")
+		username, err := logic.UserPermissions(false, "skynet", "Bearer secretkey")
 		assert.Nil(t, err)
-		t.Log(networks, username)
+		t.Log(username)
 	})
 	t.Run("BadNet", func(t *testing.T) {
 		t.Skip()
-		networks, username, err := logic.UserPermissions(false, "badnet", "Bearer secretkey")
+		username, err := logic.UserPermissions(false, "badnet", "Bearer secretkey")
 		assert.NotNil(t, err)
 		t.Log(err)
-		t.Log(networks, username)
+		t.Log(username)
 	})
 	t.Run("BadToken", func(t *testing.T) {
-		networks, username, err := logic.UserPermissions(false, "skynet", "Bearer badkey")
+		username, err := logic.UserPermissions(false, "skynet", "Bearer badkey")
 		assert.NotNil(t, err)
 		t.Log(err)
-		t.Log(networks, username)
+		t.Log(username)
 	})
 }
 

controllers/user.go

@@ -29,10 +29,8 @@ func userHandlers(r *mux.Router) {
 	r.HandleFunc("/api/users/adm/authenticate", authenticateUser).Methods(http.MethodPost)
 	r.HandleFunc("/api/users/{username}/remote_access_gw", attachUserToRemoteAccessGw).Methods(http.MethodPost)
 	r.HandleFunc("/api/users/{username}/remote_access_gw", removeUserFromRemoteAccessGW).Methods(http.MethodDelete)
-	r.HandleFunc("/api/nodes/user/remote_access_gws", logic.SecurityCheck(false, http.HandlerFunc(getUserRemoteAccessGws))).Methods(http.MethodGet)
-	r.HandleFunc("/api/users/{username}", logic.SecurityCheck(false, logic.ContinueIfUserMatch(http.HandlerFunc(updateUser)))).Methods(http.MethodPut)
-	r.HandleFunc("/api/users/networks/{username}", logic.SecurityCheck(true, http.HandlerFunc(updateUserNetworks))).Methods(http.MethodPut)
-	r.HandleFunc("/api/users/{username}/adm", logic.SecurityCheck(true, http.HandlerFunc(updateUserAdm))).Methods(http.MethodPut)
+	r.HandleFunc("/api/users/{username}/remote_access_gw", logic.SecurityCheck(false, http.HandlerFunc(getUserRemoteAccessGws))).Methods(http.MethodGet)
+	r.HandleFunc("/api/users/{username}", logic.SecurityCheck(true, http.HandlerFunc(updateUser))).Methods(http.MethodPut)
 	r.HandleFunc("/api/users/{username}", logic.SecurityCheck(true, checkFreeTierLimits(limitChoiceUsers, http.HandlerFunc(createUser)))).Methods(http.MethodPost)
 	r.HandleFunc("/api/users/{username}", logic.SecurityCheck(true, http.HandlerFunc(deleteUser))).Methods(http.MethodDelete)
 	r.HandleFunc("/api/users/{username}", logic.SecurityCheck(false, logic.ContinueIfUserMatch(http.HandlerFunc(getUser)))).Methods(http.MethodGet)
@@ -290,7 +288,7 @@ func getUserRemoteAccessGws(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	userGws := []UserRemoteGws{}
+	userGws := make(map[string][]UserRemoteGws)
 	user, err := logic.GetUser(username)
 	if err != nil {
 		logger.Log(0, username, "failed to fetch user: ", err.Error())
@@ -316,14 +314,18 @@ func getUserRemoteAccessGws(w http.ResponseWriter, r *http.Request) {
 				if err != nil {
 					continue
 				}
+
 				if _, ok := user.RemoteGwIDs[node.ID.String()]; ok {
-					userGws = append(userGws, UserRemoteGws{
+					gws := userGws[node.Network]
+
+					gws = append(gws, UserRemoteGws{
 						GwID:      node.ID.String(),
 						GWName:    host.Name,
 						Network:   node.Network,
 						GwClient:  extClient,
 						Connected: true,
 					})
+					userGws[node.Network] = gws
 					delete(user.RemoteGwIDs, node.ID.String())
 				}
 			}
@@ -343,11 +345,14 @@ func getUserRemoteAccessGws(w http.ResponseWriter, r *http.Request) {
 		if err != nil {
 			continue
 		}
-		userGws = append(userGws, UserRemoteGws{
+		gws := userGws[node.Network]
+
+		gws = append(gws, UserRemoteGws{
 			GwID:    node.ID.String(),
 			GWName:  host.Name,
 			Network: node.Network,
 		})
+		userGws[node.Network] = gws
 	}
 
 	w.WriteHeader(http.StatusOK)
@@ -437,69 +442,40 @@ func createSuperAdmin(w http.ResponseWriter, r *http.Request) {
 //				200: userBodyResponse
 func createUser(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
-
+	caller, err := logic.GetUser(r.Header.Get("user"))
+	if err != nil {
+		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
+	}
 	var user models.User
-	err := json.NewDecoder(r.Body).Decode(&user)
+	err = json.NewDecoder(r.Body).Decode(&user)
 	if err != nil {
 		logger.Log(0, user.UserName, "error decoding request body: ",
 			err.Error())
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest"))
 		return
 	}
-
-	err = logic.CreateUser(&user)
-	if err != nil {
-		logger.Log(0, user.UserName, "error creating new user: ",
-			err.Error())
+	if !caller.IsSuperAdmin && user.IsAdmin {
+		slog.Error("error creating new user: ", "user", user.UserName, "error",
+			errors.New("only superadmin can create admin users"))
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest"))
 		return
 	}
-	logger.Log(1, user.UserName, "was created")
-	json.NewEncoder(w).Encode(logic.ToReturnUser(user))
-}
-
-// swagger:route PUT /api/users/networks/{username} user updateUserNetworks
-//
-// Updates the networks of the given user.
-//
-//			Schemes: https
-//
-//			Security:
-//	  		oauth
-//
-//			Responses:
-//				200: userBodyResponse
-func updateUserNetworks(w http.ResponseWriter, r *http.Request) {
-	w.Header().Set("Content-Type", "application/json")
-	var params = mux.Vars(r)
-	// start here
-	username := params["username"]
-	_, err := logic.GetUser(username)
-	if err != nil {
-		logger.Log(0, username,
-			"failed to update user networks: ", err.Error())
-		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
-		return
-	}
-	userChange := &models.User{}
-	// we decode our body request params
-	err = json.NewDecoder(r.Body).Decode(userChange)
-	if err != nil {
-		logger.Log(0, username, "error decoding request body: ",
-			err.Error())
+	if user.IsSuperAdmin {
+		slog.Error("error creating new user: ", "user", user.UserName, "error",
+			errors.New("additional superadmins cannot be created"))
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest"))
 		return
 	}
 
-	logger.Log(1, username, "status was updated")
-	// re-read and return the new user struct
-	returnUser, err := logic.GetReturnUser(username)
+	err = logic.CreateUser(&user)
 	if err != nil {
-		logger.Log(0, username, "failed to fetch user: ", err.Error())
-		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
+		logger.Log(0, user.UserName, "error creating new user: ",
+			err.Error())
+		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest"))
 		return
 	}
-	json.NewEncoder(w).Encode(returnUser)
+	logger.Log(1, user.UserName, "was created")
+	json.NewEncoder(w).Encode(logic.ToReturnUser(user))
 }
 
 // swagger:route PUT /api/users/{username} user updateUser
@@ -517,18 +493,12 @@ func updateUser(w http.ResponseWriter, r *http.Request) {
 	w.Header().Set("Content-Type", "application/json")
 	var params = mux.Vars(r)
 	// start here
-	jwtUser, _, isadmin, err := verifyJWT(r.Header.Get("Authorization"))
+
+	caller, err := logic.GetUser(r.Header.Get("user"))
 	if err != nil {
-		logger.Log(0, "verifyJWT error", err.Error())
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
-		return
 	}
 	username := params["username"]
-	if username != jwtUser && !isadmin {
-		logger.Log(0, "non-admin user", jwtUser, "attempted to update user", username)
-		logic.ReturnErrorResponse(w, r, logic.FormatError(errors.New("not authorizied"), "unauthorized"))
-		return
-	}
 	user, err := logic.GetUser(username)
 	if err != nil {
 		logger.Log(0, username,
@@ -536,12 +506,6 @@ func updateUser(w http.ResponseWriter, r *http.Request) {
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
 		return
 	}
-	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"))
-		return
-	}
 	var userchange models.User
 	// we decode our body request params
 	err = json.NewDecoder(r.Body).Decode(&userchange)
@@ -551,11 +515,44 @@ func updateUser(w http.ResponseWriter, r *http.Request) {
 		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "badrequest"))
 		return
 	}
-	if userchange.IsAdmin && !isadmin {
-		logger.Log(0, "non-admin user", jwtUser, "attempted get admin privilages")
-		logic.ReturnErrorResponse(w, r, logic.FormatError(errors.New("not authorizied"), "unauthorized"))
+	selfUpdate := false
+	if caller.UserName == user.UserName {
+		selfUpdate = true
+	}
+
+	if !caller.IsSuperAdmin {
+		if user.IsSuperAdmin {
+			slog.Error("non-superadmin user", "caller", caller.UserName, "attempted to update superadmin user", username)
+			logic.ReturnErrorResponse(w, r, logic.FormatError(errors.New("cannot update superadmin user"), "forbidden"))
+			return
+		}
+		if !selfUpdate && !(caller.IsAdmin) {
+			slog.Error("non-admin user", "caller", caller.UserName, "attempted to update  user", username)
+			logic.ReturnErrorResponse(w, r, logic.FormatError(errors.New("not authorized"), "forbidden"))
+			return
+		}
+
+		if userchange.IsAdmin != user.IsAdmin || userchange.IsSuperAdmin != user.IsSuperAdmin {
+			if selfUpdate {
+				slog.Error("user cannot change his own role", "caller", caller.UserName, "attempted to update user role", username)
+				logic.ReturnErrorResponse(w, r, logic.FormatError(errors.New("user not allowed to self assign role"), "forbidden"))
+				return
+			}
+		}
+		if caller.IsAdmin && user.IsAdmin {
+			slog.Error("admin user cannot update another admin", "caller", caller.UserName, "attempted to update admin user", username)
+			logic.ReturnErrorResponse(w, r, logic.FormatError(errors.New("admin user cannot update another admin"), "forbidden"))
+			return
+		}
+	}
+
+	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"))
 		return
 	}
+
 	user, err = logic.UpdateUser(&userchange, user)
 	if err != nil {
 		logger.Log(0, username,
@@ -635,9 +632,32 @@ func deleteUser(w http.ResponseWriter, r *http.Request) {
 
 	// get params
 	var params = mux.Vars(r)
-
+	caller, err := logic.GetUser(r.Header.Get("user"))
+	if err != nil {
+		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
+	}
 	username := params["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 user.IsSuperAdmin {
+		slog.Error(
+			"failed to delete user: ", "user", username, "error", "superadmin cannot be deleted")
+		logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
+		return
+	}
+	if !caller.IsSuperAdmin {
+		if caller.IsAdmin && user.IsAdmin {
+			slog.Error(
+				"failed to delete user: ", "user", username, "error", "admin cannot delete another admin user")
+			logic.ReturnErrorResponse(w, r, logic.FormatError(err, "internal"))
+			return
+		}
+	}
 	success, err := logic.DeleteUser(username)
 	if err != nil {
 		logger.Log(0, username,

controllers/user_test.go

@@ -65,20 +65,6 @@ func TestCreateUserNoHashedPassword(t *testing.T) {
 	assertUserNameButNoPassword(t, rec.Body, user.UserName)
 }
 
-func TestUpdateUserNetworksNoHashedPassword(t *testing.T) {
-	// prepare existing user base
-	user1 := models.User{UserName: "joestar", Password: "jonathan"}
-	haveOnlyOneUser(t, user1)
-
-	// prepare request
-	user2 := models.User{UserName: "joestar", Password: "joseph"}
-	rec, req := prepareUserRequest(t, user2, user1.UserName)
-
-	// test response
-	updateUserNetworks(rec, req)
-	assertUserNameButNoPassword(t, rec.Body, user1.UserName)
-}
-
 func TestUpdateUserNoHashedPassword(t *testing.T) {
 	// prepare existing user base
 	user1 := models.User{UserName: "dio", Password: "brando"}

logic/auth.go

@@ -206,10 +206,7 @@ func UpdateUser(userchange, user *models.User) (*models.User, error) {
 
 		user.Password = userchange.Password
 	}
-
-	if (userchange.IsAdmin != user.IsAdmin) && !user.IsAdmin {
-		user.IsAdmin = userchange.IsAdmin
-	}
+	user.IsAdmin = userchange.IsAdmin
 
 	err := ValidateUser(user)
 	if err != nil {

logic/security.go

@@ -1,12 +1,10 @@
 package logic
 
 import (
-	"encoding/json"
 	"net/http"
 	"strings"
 
 	"github.com/gorilla/mux"
-	"github.com/gravitl/netmaker/database"
 	"github.com/gravitl/netmaker/models"
 	"github.com/gravitl/netmaker/servercfg"
 )
@@ -30,68 +28,45 @@ func SecurityCheck(reqAdmin bool, next http.Handler) http.HandlerFunc {
 			Code: http.StatusForbidden, Message: Forbidden_Msg,
 		}
 		r.Header.Set("ismaster", "no")
-
-		var params = mux.Vars(r)
 		bearerToken := r.Header.Get("Authorization")
-		// to have a custom DNS service adding entries
-		// we should refactor this, but is for the special case of an external service to query the DNS api
-		if strings.Contains(r.RequestURI, "/dns") && strings.ToUpper(r.Method) == "GET" && authenticateDNSToken(bearerToken) {
-			// do dns stuff
-			r.Header.Set("user", "nameserver")
-			networks, _ := json.Marshal([]string{ALL_NETWORK_ACCESS})
-			r.Header.Set("networks", string(networks))
-			next.ServeHTTP(w, r)
-			return
-		}
-		var networkName = params["networkname"]
-		if len(networkName) == 0 {
-			networkName = params["network"]
-		}
-		networks, username, err := UserPermissions(reqAdmin, networkName, bearerToken)
+		username, err := UserPermissions(reqAdmin, bearerToken)
 		if err != nil {
 			ReturnErrorResponse(w, r, errorResponse)
 			return
 		}
 		// detect masteradmin
-		if len(networks) > 0 && networks[0] == ALL_NETWORK_ACCESS {
+		if username == master_uname {
 			r.Header.Set("ismaster", "yes")
 		}
-		networksJson, err := json.Marshal(&networks)
-		if err != nil {
-			ReturnErrorResponse(w, r, errorResponse)
-			return
-		}
 		r.Header.Set("user", username)
-		r.Header.Set("networks", string(networksJson))
 		next.ServeHTTP(w, r)
 	}
 }
 
 // UserPermissions - checks token stuff
-func UserPermissions(reqAdmin bool, netname string, token string) ([]string, string, error) {
+func UserPermissions(reqAdmin bool, token string) (string, error) {
 	var tokenSplit = strings.Split(token, " ")
 	var authToken = ""
-	userNetworks := []string{}
 
 	if len(tokenSplit) < 2 {
-		return userNetworks, "", Unauthorized_Err
+		return "", Unauthorized_Err
 	} else {
 		authToken = tokenSplit[1]
 	}
 	//all endpoints here require master so not as complicated
 	if authenticateMaster(authToken) {
 		// TODO log in as an actual admin user
-		return []string{ALL_NETWORK_ACCESS}, master_uname, nil
+		return master_uname, nil
 	}
-	username, _, isadmin, err := VerifyUserToken(authToken)
+	username, issuperadmin, isadmin, err := VerifyUserToken(authToken)
 	if err != nil {
-		return nil, username, Unauthorized_Err
+		return username, Unauthorized_Err
 	}
-	if !isadmin && reqAdmin {
-		return nil, username, Forbidden_Err
+	if reqAdmin && !(issuperadmin || isadmin) {
+		return username, Forbidden_Err
 	}
 
-	return userNetworks, username, nil
+	return username, nil
 }
 
 // Consider a more secure way of setting master key
@@ -99,23 +74,6 @@ func authenticateMaster(tokenString string) bool {
 	return tokenString == servercfg.GetMasterKey() && servercfg.GetMasterKey() != ""
 }
 
-func authenticateNetworkUser(network string, userNetworks []string) bool {
-	networkexists, err := NetworkExists(network)
-	if (err != nil && !database.IsEmptyRecord(err)) || !networkexists {
-		return false
-	}
-	return StringSliceContains(userNetworks, network)
-}
-
-// Consider a more secure way of setting master key
-func authenticateDNSToken(tokenString string) bool {
-	tokens := strings.Split(tokenString, " ")
-	if len(tokens) < 2 {
-		return false
-	}
-	return len(servercfg.GetDNSKey()) > 0 && tokens[1] == servercfg.GetDNSKey()
-}
-
 func ContinueIfUserMatch(next http.Handler) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		var errorResponse = models.ErrorResponse{

servercfg/serverconf.go

@@ -320,17 +320,6 @@ func GetMasterKey() string {
 	return key
 }
 
-// GetDNSKey - gets the configured dns key of server
-func GetDNSKey() string {
-	key := ""
-	if os.Getenv("DNS_KEY") != "" {
-		key = os.Getenv("DNS_KEY")
-	} else if config.Config.Server.DNSKey != "" {
-		key = config.Config.Server.DNSKey
-	}
-	return key
-}
-
 // GetAllowedOrigin - get the allowed origin
 func GetAllowedOrigin() string {
 	allowedorigin := "*"