From ef98ee7d117995492ec7c29b340fd1987b10a3c0 Mon Sep 17 00:00:00 2001 From: Nicola Murino Date: Sat, 9 Nov 2024 20:24:35 +0100 Subject: [PATCH] don't allow admins to change their own permissions Signed-off-by: Nicola Murino --- internal/httpd/api_admin.go | 4 ++-- internal/httpd/auth_utils.go | 13 ------------- internal/httpd/httpd_test.go | 4 ++-- internal/httpd/webadmin.go | 4 ++-- internal/util/util.go | 28 ++++++++++++++++------------ static/locales/en/translation.json | 2 +- static/locales/it/translation.json | 2 +- 7 files changed, 24 insertions(+), 33 deletions(-) diff --git a/internal/httpd/api_admin.go b/internal/httpd/api_admin.go index 646863e2..5a7b0afe 100644 --- a/internal/httpd/api_admin.go +++ b/internal/httpd/api_admin.go @@ -149,8 +149,8 @@ func updateAdmin(w http.ResponseWriter, r *http.Request) { http.StatusBadRequest) return } - if claims.isCriticalPermRemoved(updatedAdmin.Permissions) { - sendAPIResponse(w, r, errors.New("you cannot remove these permissions to yourself"), "", http.StatusBadRequest) + if !util.SlicesEqual(admin.Permissions, updatedAdmin.Permissions) { + sendAPIResponse(w, r, errors.New("you cannot change your permissions"), "", http.StatusBadRequest) return } if updatedAdmin.Status == 0 { diff --git a/internal/httpd/auth_utils.go b/internal/httpd/auth_utils.go index 6183578b..7cbdfc7d 100644 --- a/internal/httpd/auth_utils.go +++ b/internal/httpd/auth_utils.go @@ -226,19 +226,6 @@ func (c *jwtTokenClaims) Decode(token map[string]any) { } } -func (c *jwtTokenClaims) isCriticalPermRemoved(permissions []string) bool { - if slices.Contains(permissions, dataprovider.PermAdminAny) { - return false - } - if (slices.Contains(c.Permissions, dataprovider.PermAdminManageAdmins) || - slices.Contains(c.Permissions, dataprovider.PermAdminAny)) && - !slices.Contains(permissions, dataprovider.PermAdminManageAdmins) && - !slices.Contains(permissions, dataprovider.PermAdminAny) { - return true - } - return false -} - func (c *jwtTokenClaims) hasPerm(perm string) bool { if slices.Contains(c.Permissions, dataprovider.PermAdminAny) { return true diff --git a/internal/httpd/httpd_test.go b/internal/httpd/httpd_test.go index 8e4fe3ae..9219a448 100644 --- a/internal/httpd/httpd_test.go +++ b/internal/httpd/httpd_test.go @@ -11843,7 +11843,7 @@ func TestUpdateAdminMock(t *testing.T) { setBearerForReq(req, token) rr = executeRequest(req) checkResponseCode(t, http.StatusBadRequest, rr) - assert.Contains(t, rr.Body.String(), "you cannot remove these permissions to yourself") + assert.Contains(t, rr.Body.String(), "you cannot change your permissions") admin.Permissions = []string{dataprovider.PermAdminAny} admin.Role = "missing role" asJSON, err = json.Marshal(admin) @@ -11858,7 +11858,7 @@ func TestUpdateAdminMock(t *testing.T) { altToken, err := getJWTAPITokenFromTestServer(altAdminUsername, defaultTokenAuthPass) assert.NoError(t, err) admin.Password = "" // it must remain unchanged - admin.Permissions = []string{dataprovider.PermAdminManageAdmins, dataprovider.PermAdminCloseConnections} + admin.Permissions = []string{dataprovider.PermAdminManageAdmins} asJSON, err = json.Marshal(admin) assert.NoError(t, err) req, _ = http.NewRequest(http.MethodPut, path.Join(adminPath, altAdminUsername), bytes.NewBuffer(asJSON)) diff --git a/internal/httpd/webadmin.go b/internal/httpd/webadmin.go index e6022764..8d858fb8 100644 --- a/internal/httpd/webadmin.go +++ b/internal/httpd/webadmin.go @@ -3156,9 +3156,9 @@ func (s *httpdServer) handleWebUpdateAdminPost(w http.ResponseWriter, r *http.Re return } if username == claims.Username { - if claims.isCriticalPermRemoved(updatedAdmin.Permissions) { + if !util.SlicesEqual(admin.Permissions, updatedAdmin.Permissions) { s.renderAddUpdateAdminPage(w, r, &updatedAdmin, - util.NewI18nError(errors.New("you cannot remove these permissions to yourself"), + util.NewI18nError(errors.New("you cannot change your permissions"), util.I18nErrorAdminSelfPerms, ), false) return diff --git a/internal/util/util.go b/internal/util/util.go index e9eddb62..b439b385 100644 --- a/internal/util/util.go +++ b/internal/util/util.go @@ -40,6 +40,7 @@ import ( "path/filepath" "regexp" "runtime" + "slices" "strconv" "strings" "time" @@ -127,18 +128,6 @@ var bytesSizeTable = map[string]uint64{ "e": eByte, } -// Remove removes an element from a string slice and -// returns the modified slice -func Remove(elems []string, val string) []string { - for idx, v := range elems { - if v == val { - elems[idx] = elems[len(elems)-1] - return elems[:len(elems)-1] - } - } - return elems -} - // IsStringPrefixInSlice searches a string prefix in a slice and returns true // if a matching prefix is found func IsStringPrefixInSlice(obj string, list []string) bool { @@ -921,3 +910,18 @@ func ReadConfigFromFile(name, configDir string) (string, error) { } return strings.TrimSpace(BytesToString(val)), nil } + +// SlicesEqual checks if the provided slices contain the same elements, +// also in different order. +func SlicesEqual(s1, s2 []string) bool { + if len(s1) != len(s2) { + return false + } + for _, v := range s1 { + if !slices.Contains(s2, v) { + return false + } + } + + return true +} diff --git a/static/locales/en/translation.json b/static/locales/en/translation.json index 89f302d3..02b99373 100644 --- a/static/locales/en/translation.json +++ b/static/locales/en/translation.json @@ -762,7 +762,7 @@ "role_permissions": "A role admin cannot have the following permissions: {{val}}", "view_manage": "View and manage admins", "self_delete": "You cannot delete yourself", - "self_permissions": "You cannot remove these permissions to yourself", + "self_permissions": "You cannot change your permissions", "self_disable": "You cannot disable yourself", "self_role": "You cannot add/change your role", "password_help": "If blank the current password will not be changed", diff --git a/static/locales/it/translation.json b/static/locales/it/translation.json index 004ac04c..1bf69e64 100644 --- a/static/locales/it/translation.json +++ b/static/locales/it/translation.json @@ -762,7 +762,7 @@ "role_permissions": "Un amministratore di ruolo non può avere le seguenti autorizzazioni: {{val}}", "view_manage": "Visualizza e gestisci amministratori", "self_delete": "Non puoi eliminare te stesso", - "self_permissions": "Non puoi rimuovere questi permessi a te stesso", + "self_permissions": "Non puoi cambiare i tuoi permessi", "self_disable": "Non puoi disabilitare te stesso", "self_role": "Non puoi aggiungere/modificare il tuo ruolo", "password_help": "Se vuoto la password corrente non verrà modificata",