Security fixes
Patched some security holes with GetUser and UpdateUser, did some minor cleanup
This commit is contained in:
		
							parent
							
								
									df10417668
								
							
						
					
					
						commit
						4b5057e658
					
				|  | @ -10,7 +10,6 @@ ARG BUILD_DEPENDENCIES="npm \ | |||
| 
 | ||||
| # Get dependencies | ||||
| RUN apk add --update --no-cache ${BUILD_DEPENDENCIES} | ||||
| #RUN apt install ${BUILD_DEPENDENCIES} | ||||
| 
 | ||||
| WORKDIR /build | ||||
| 
 | ||||
|  | @ -21,7 +20,8 @@ COPY package.json /build | |||
| COPY yarn.lock /build | ||||
| 
 | ||||
| # Prepare assets | ||||
| RUN yarn install --pure-lockfile --production && yarn cache clean | ||||
| RUN yarn install --pure-lockfile --production && \ | ||||
|     yarn cache clean | ||||
| 
 | ||||
| # Move admin-lte dist | ||||
| RUN mkdir -p assets/dist/js assets/dist/css && \ | ||||
|  |  | |||
|  | @ -80,10 +80,9 @@ function renderClientList(data) { | |||
| 
 | ||||
| function renderUserList(data) { | ||||
|     $.each(data, function(index, obj) { | ||||
|         // render client status css tag style
 | ||||
|         let clientStatusHtml = '>' | ||||
| 
 | ||||
|         // render client html content
 | ||||
|         // render user html content
 | ||||
|         let html = `<div class="col-sm-6 col-md-6 col-lg-4" id="user_${obj.username}">
 | ||||
|                         <div class="info-box"> | ||||
|                             <div class="info-box-content"> | ||||
|  | @ -101,7 +100,7 @@ function renderUserList(data) { | |||
|                         </div> | ||||
|                     </div>` | ||||
| 
 | ||||
|         // add the client html elements to the list
 | ||||
|         // add the user html elements to the list
 | ||||
|         $('#users-list').append(html); | ||||
|     }); | ||||
| } | ||||
|  |  | |||
|  | @ -107,27 +107,31 @@ func Login(db store.IStore) echo.HandlerFunc { | |||
| 	} | ||||
| } | ||||
| 
 | ||||
| // GetClients handler return a JSON list of Wireguard client data
 | ||||
| // GetUsers handler return a JSON list of all users
 | ||||
| func GetUsers(db store.IStore) echo.HandlerFunc { | ||||
| 	return func(c echo.Context) error { | ||||
| 
 | ||||
| 		clientDataList, err := db.GetUsers() | ||||
| 		usersList, err := db.GetUsers() | ||||
| 		if err != nil { | ||||
| 			return c.JSON(http.StatusInternalServerError, jsonHTTPResponse{ | ||||
| 				false, fmt.Sprintf("Cannot get user list: %v", err), | ||||
| 			}) | ||||
| 		} | ||||
| 
 | ||||
| 		return c.JSON(http.StatusOK, clientDataList) | ||||
| 		return c.JSON(http.StatusOK, usersList) | ||||
| 	} | ||||
| } | ||||
| 
 | ||||
| // GetClient handler returns a JSON object of Wireguard client data
 | ||||
| // GetUser handler returns a JSON object of single user
 | ||||
| func GetUser(db store.IStore) echo.HandlerFunc { | ||||
| 	return func(c echo.Context) error { | ||||
| 
 | ||||
| 		username := c.Param("username") | ||||
| 
 | ||||
| 		if !isAdmin(c) && (username != currentUser(c)) { | ||||
| 			return c.JSON(http.StatusForbidden, jsonHTTPResponse{false, "Manager cannot access other user data"}) | ||||
| 		} | ||||
| 
 | ||||
| 		userData, err := db.GetUserByName(username) | ||||
| 		if err != nil { | ||||
| 			return c.JSON(http.StatusNotFound, jsonHTTPResponse{false, "User not found"}) | ||||
|  | @ -154,7 +158,7 @@ func LoadProfile(db store.IStore) echo.HandlerFunc { | |||
| 	} | ||||
| } | ||||
| 
 | ||||
| // WireGuardClients handler
 | ||||
| // UsersSettings handler
 | ||||
| func UsersSettings(db store.IStore) echo.HandlerFunc { | ||||
| 	return func(c echo.Context) error { | ||||
| 		return c.Render(http.StatusOK, "users_settings.html", map[string]interface{}{ | ||||
|  | @ -163,7 +167,7 @@ func UsersSettings(db store.IStore) echo.HandlerFunc { | |||
| 	} | ||||
| } | ||||
| 
 | ||||
| // UpdateProfile to update user information
 | ||||
| // UpdateUser to update user information
 | ||||
| func UpdateUser(db store.IStore) echo.HandlerFunc { | ||||
| 	return func(c echo.Context) error { | ||||
| 		data := make(map[string]interface{}) | ||||
|  | @ -178,6 +182,14 @@ func UpdateUser(db store.IStore) echo.HandlerFunc { | |||
| 		previousUsername := data["previous_username"].(string) | ||||
| 		admin := data["admin"].(bool) | ||||
| 
 | ||||
| 		if !isAdmin(c) && (previousUsername != currentUser(c)) { | ||||
| 			return c.JSON(http.StatusForbidden, jsonHTTPResponse{false, "Manager cannot access other user data"}) | ||||
| 		} | ||||
| 
 | ||||
| 		if !isAdmin(c) { | ||||
| 			admin = false | ||||
| 		} | ||||
| 
 | ||||
| 		user, err := db.GetUserByName(previousUsername) | ||||
| 		if err != nil { | ||||
| 			return c.JSON(http.StatusNotFound, jsonHTTPResponse{false, err.Error()}) | ||||
|  | @ -221,7 +233,7 @@ func UpdateUser(db store.IStore) echo.HandlerFunc { | |||
| 	} | ||||
| } | ||||
| 
 | ||||
| // UpdateProfile to update user information
 | ||||
| // CreateUser to create new user
 | ||||
| func CreateUser(db store.IStore) echo.HandlerFunc { | ||||
| 	return func(c echo.Context) error { | ||||
| 		data := make(map[string]interface{}) | ||||
|  | @ -266,7 +278,7 @@ func CreateUser(db store.IStore) echo.HandlerFunc { | |||
| 	} | ||||
| } | ||||
| 
 | ||||
| // RemoveClient handler
 | ||||
| // RemoveUser handler
 | ||||
| func RemoveUser(db store.IStore) echo.HandlerFunc { | ||||
| 	return func(c echo.Context) error { | ||||
| 		data := make(map[string]interface{}) | ||||
|  | @ -277,7 +289,7 @@ func RemoveUser(db store.IStore) echo.HandlerFunc { | |||
| 		} | ||||
| 
 | ||||
| 		username := data["username"].(string) | ||||
| 		// delete client from database
 | ||||
| 		// delete user from database
 | ||||
| 
 | ||||
| 		if err := db.DeleteUser(username); err != nil { | ||||
| 			log.Error("Cannot delete user: ", err) | ||||
|  |  | |||
|  | @ -55,7 +55,7 @@ func currentUser(c echo.Context) string { | |||
| 	return username | ||||
| } | ||||
| 
 | ||||
| // currentUser to get username of logged in user
 | ||||
| // isAdmin to get user type: admin or manager
 | ||||
| func isAdmin(c echo.Context) bool { | ||||
| 	if util.DisableLogin { | ||||
| 		return true | ||||
|  |  | |||
|  | @ -132,7 +132,7 @@ func (o *JsonDB) GetUser() (model.User, error) { | |||
| 	return user, o.conn.Read("server", "users", &user) | ||||
| } | ||||
| 
 | ||||
| // GetUsers func to query user info from the database
 | ||||
| // GetUsers func to get all users from the database
 | ||||
| func (o *JsonDB) GetUsers() ([]model.User, error) { | ||||
| 	var users []model.User | ||||
| 	results, err := o.conn.ReadAll("users") | ||||
|  | @ -151,6 +151,7 @@ func (o *JsonDB) GetUsers() ([]model.User, error) { | |||
| 	return users, err | ||||
| } | ||||
| 
 | ||||
| // GetUserByName func to get single user from the database
 | ||||
| func (o *JsonDB) GetUserByName(username string) (model.User, error) { | ||||
| 	user := model.User{} | ||||
| 
 | ||||
|  | @ -161,19 +162,16 @@ func (o *JsonDB) GetUserByName(username string) (model.User, error) { | |||
| 	return user, nil | ||||
| } | ||||
| 
 | ||||
| // SaveUser func to save user in the database
 | ||||
| func (o *JsonDB) SaveUser(user model.User) error { | ||||
| 	return o.conn.Write("users", user.Username, user) | ||||
| } | ||||
| 
 | ||||
| // DeleteUser func to remove user from the database
 | ||||
| func (o *JsonDB) DeleteUser(username string) error { | ||||
| 	return o.conn.Delete("users", username) | ||||
| } | ||||
| 
 | ||||
| //// SaveUser func to user info to the database
 | ||||
| //func (o *JsonDB) SaveUser(user model.User) error {
 | ||||
| //	return o.conn.Write("server", "users", user)
 | ||||
| //}
 | ||||
| 
 | ||||
| // GetGlobalSettings func to query global settings from the database
 | ||||
| func (o *JsonDB) GetGlobalSettings() (model.GlobalSetting, error) { | ||||
| 	settings := model.GlobalSetting{} | ||||
|  |  | |||
|  | @ -83,7 +83,6 @@ Profile | |||
|     function updateUserInfo() { | ||||
|         const username = $("#username").val(); | ||||
|         const password = $("#password").val(); | ||||
| //        const previous_username = $("#previous_username").val(); | ||||
|         const data = {"username": username, "password": password, "previous_username": previous_username, "admin":admin}; | ||||
|         $.ajax({ | ||||
|             cache: false, | ||||
|  |  | |||
|  | @ -14,7 +14,6 @@ Users Settings | |||
| {{end}} | ||||
| 
 | ||||
| {{define "page_content"}} | ||||
| <h1>HUBBA BUBBA BABA YAGA</h1> | ||||
| <section class="content"> | ||||
|     <div class="container-fluid"> | ||||
|         <div class="row" id="users-list"> | ||||
|  | @ -111,7 +110,7 @@ Users Settings | |||
|     } | ||||
| </script> | ||||
| <script> | ||||
|     // load client list | ||||
|     // load user list | ||||
|     $(document).ready(function () { | ||||
|         populateUsersList(); | ||||
|         let newUserHtml = '<div class="col-sm-2 offset-md-4" style=" text-align: right;">' + | ||||
|  | @ -203,10 +202,15 @@ Users Settings | |||
|         const previous_username = $("#_previous_user_name").val(); | ||||
|         const password = $("#_user_password").val(); | ||||
|         let admin = false; | ||||
|         if ($("#_admin").is(':checked')){ | ||||
|         if ($("#_admin").is(':checked')) { | ||||
|             admin = true; | ||||
|         } | ||||
|         const data = {"username": username, "password": password, "previous_username": previous_username, "admin": admin}; | ||||
|         const data = { | ||||
|             "username": username, | ||||
|             "password": password, | ||||
|             "previous_username": previous_username, | ||||
|             "admin": admin | ||||
|         }; | ||||
| 
 | ||||
|         if (previous_username !== "") { | ||||
|             $.ajax({ | ||||
|  | @ -252,7 +256,7 @@ Users Settings | |||
|                 updateUserInfo(); | ||||
|             } | ||||
|         }); | ||||
|         // Edit client form validation | ||||
|         // Edit user form validation | ||||
|         $("#frm_edit_user").validate({ | ||||
|             rules: { | ||||
|                 _user_name: { | ||||
|  | @ -260,7 +264,7 @@ Users Settings | |||
|                 }, | ||||
|                 _user_password: { | ||||
|                     required: function () { | ||||
|                         return $("#_previous_user_name").val()===""; | ||||
|                         return $("#_previous_user_name").val() === ""; | ||||
|                     } | ||||
|                 }, | ||||
|             }, | ||||
|  |  | |||
		Loading…
	
		Reference in New Issue