Security fixes

Patched some security holes with GetUser and UpdateUser, did some minor cleanup
This commit is contained in:
armisss4 2022-12-29 12:54:31 +02:00 committed by GitHub
parent df10417668
commit 4b5057e658
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 40 additions and 28 deletions

View file

@ -10,7 +10,6 @@ ARG BUILD_DEPENDENCIES="npm \
# Get dependencies # Get dependencies
RUN apk add --update --no-cache ${BUILD_DEPENDENCIES} RUN apk add --update --no-cache ${BUILD_DEPENDENCIES}
#RUN apt install ${BUILD_DEPENDENCIES}
WORKDIR /build WORKDIR /build
@ -21,7 +20,8 @@ COPY package.json /build
COPY yarn.lock /build COPY yarn.lock /build
# Prepare assets # Prepare assets
RUN yarn install --pure-lockfile --production && yarn cache clean RUN yarn install --pure-lockfile --production && \
yarn cache clean
# Move admin-lte dist # Move admin-lte dist
RUN mkdir -p assets/dist/js assets/dist/css && \ RUN mkdir -p assets/dist/js assets/dist/css && \

View file

@ -80,10 +80,9 @@ function renderClientList(data) {
function renderUserList(data) { function renderUserList(data) {
$.each(data, function(index, obj) { $.each(data, function(index, obj) {
// render client status css tag style
let clientStatusHtml = '>' 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}"> 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">
<div class="info-box-content"> <div class="info-box-content">
@ -101,7 +100,7 @@ function renderUserList(data) {
</div> </div>
</div>` </div>`
// add the client html elements to the list // add the user html elements to the list
$('#users-list').append(html); $('#users-list').append(html);
}); });
} }

View file

@ -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 { func GetUsers(db store.IStore) echo.HandlerFunc {
return func(c echo.Context) error { return func(c echo.Context) error {
clientDataList, err := db.GetUsers() usersList, err := db.GetUsers()
if err != nil { if err != nil {
return c.JSON(http.StatusInternalServerError, jsonHTTPResponse{ return c.JSON(http.StatusInternalServerError, jsonHTTPResponse{
false, fmt.Sprintf("Cannot get user list: %v", err), 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 { func GetUser(db store.IStore) echo.HandlerFunc {
return func(c echo.Context) error { return func(c echo.Context) error {
username := c.Param("username") 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) userData, err := db.GetUserByName(username)
if err != nil { if err != nil {
return c.JSON(http.StatusNotFound, jsonHTTPResponse{false, "User not found"}) 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 { func UsersSettings(db store.IStore) echo.HandlerFunc {
return func(c echo.Context) error { return func(c echo.Context) error {
return c.Render(http.StatusOK, "users_settings.html", map[string]interface{}{ 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 { func UpdateUser(db store.IStore) echo.HandlerFunc {
return func(c echo.Context) error { return func(c echo.Context) error {
data := make(map[string]interface{}) data := make(map[string]interface{})
@ -178,6 +182,14 @@ func UpdateUser(db store.IStore) echo.HandlerFunc {
previousUsername := data["previous_username"].(string) previousUsername := data["previous_username"].(string)
admin := data["admin"].(bool) 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) user, err := db.GetUserByName(previousUsername)
if err != nil { if err != nil {
return c.JSON(http.StatusNotFound, jsonHTTPResponse{false, err.Error()}) 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 { func CreateUser(db store.IStore) echo.HandlerFunc {
return func(c echo.Context) error { return func(c echo.Context) error {
data := make(map[string]interface{}) 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 { func RemoveUser(db store.IStore) echo.HandlerFunc {
return func(c echo.Context) error { return func(c echo.Context) error {
data := make(map[string]interface{}) data := make(map[string]interface{})
@ -277,7 +289,7 @@ func RemoveUser(db store.IStore) echo.HandlerFunc {
} }
username := data["username"].(string) username := data["username"].(string)
// delete client from database // delete user from database
if err := db.DeleteUser(username); err != nil { if err := db.DeleteUser(username); err != nil {
log.Error("Cannot delete user: ", err) log.Error("Cannot delete user: ", err)

View file

@ -55,7 +55,7 @@ func currentUser(c echo.Context) string {
return username return username
} }
// currentUser to get username of logged in user // isAdmin to get user type: admin or manager
func isAdmin(c echo.Context) bool { func isAdmin(c echo.Context) bool {
if util.DisableLogin { if util.DisableLogin {
return true return true

View file

@ -132,7 +132,7 @@ func (o *JsonDB) GetUser() (model.User, error) {
return user, o.conn.Read("server", "users", &user) 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) { func (o *JsonDB) GetUsers() ([]model.User, error) {
var users []model.User var users []model.User
results, err := o.conn.ReadAll("users") results, err := o.conn.ReadAll("users")
@ -151,6 +151,7 @@ func (o *JsonDB) GetUsers() ([]model.User, error) {
return users, err return users, err
} }
// GetUserByName func to get single user from the database
func (o *JsonDB) GetUserByName(username string) (model.User, error) { func (o *JsonDB) GetUserByName(username string) (model.User, error) {
user := model.User{} user := model.User{}
@ -161,19 +162,16 @@ func (o *JsonDB) GetUserByName(username string) (model.User, error) {
return user, nil return user, nil
} }
// SaveUser func to save user in the database
func (o *JsonDB) SaveUser(user model.User) error { func (o *JsonDB) SaveUser(user model.User) error {
return o.conn.Write("users", user.Username, user) return o.conn.Write("users", user.Username, user)
} }
// DeleteUser func to remove user from the database
func (o *JsonDB) DeleteUser(username string) error { func (o *JsonDB) DeleteUser(username string) error {
return o.conn.Delete("users", username) 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 // GetGlobalSettings func to query global settings from the database
func (o *JsonDB) GetGlobalSettings() (model.GlobalSetting, error) { func (o *JsonDB) GetGlobalSettings() (model.GlobalSetting, error) {
settings := model.GlobalSetting{} settings := model.GlobalSetting{}

View file

@ -83,7 +83,6 @@ Profile
function updateUserInfo() { function updateUserInfo() {
const username = $("#username").val(); const username = $("#username").val();
const password = $("#password").val(); const password = $("#password").val();
// const previous_username = $("#previous_username").val();
const data = {"username": username, "password": password, "previous_username": previous_username, "admin":admin}; const data = {"username": username, "password": password, "previous_username": previous_username, "admin":admin};
$.ajax({ $.ajax({
cache: false, cache: false,

View file

@ -14,7 +14,6 @@ Users Settings
{{end}} {{end}}
{{define "page_content"}} {{define "page_content"}}
<h1>HUBBA BUBBA BABA YAGA</h1>
<section class="content"> <section class="content">
<div class="container-fluid"> <div class="container-fluid">
<div class="row" id="users-list"> <div class="row" id="users-list">
@ -111,7 +110,7 @@ Users Settings
} }
</script> </script>
<script> <script>
// load client list // load user list
$(document).ready(function () { $(document).ready(function () {
populateUsersList(); populateUsersList();
let newUserHtml = '<div class="col-sm-2 offset-md-4" style=" text-align: right;">' + 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 previous_username = $("#_previous_user_name").val();
const password = $("#_user_password").val(); const password = $("#_user_password").val();
let admin = false; let admin = false;
if ($("#_admin").is(':checked')){ if ($("#_admin").is(':checked')) {
admin = true; 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 !== "") { if (previous_username !== "") {
$.ajax({ $.ajax({
@ -252,7 +256,7 @@ Users Settings
updateUserInfo(); updateUserInfo();
} }
}); });
// Edit client form validation // Edit user form validation
$("#frm_edit_user").validate({ $("#frm_edit_user").validate({
rules: { rules: {
_user_name: { _user_name: {
@ -260,7 +264,7 @@ Users Settings
}, },
_user_password: { _user_password: {
required: function () { required: function () {
return $("#_previous_user_name").val()===""; return $("#_previous_user_name").val() === "";
} }
}, },
}, },