Browse Source

don't allow admins to change their own permissions

Signed-off-by: Nicola Murino <[email protected]>
Nicola Murino 1 year ago
parent
commit
65e8e2c1d4

+ 2 - 2
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 {

+ 0 - 13
internal/httpd/auth_utils.go

@@ -211,19 +211,6 @@ func (c *jwtTokenClaims) Decode(token map[string]any) {
 	}
 }
 
-func (c *jwtTokenClaims) isCriticalPermRemoved(permissions []string) bool {
-	if util.Contains(permissions, dataprovider.PermAdminAny) {
-		return false
-	}
-	if (util.Contains(c.Permissions, dataprovider.PermAdminManageAdmins) ||
-		util.Contains(c.Permissions, dataprovider.PermAdminAny)) &&
-		!util.Contains(permissions, dataprovider.PermAdminManageAdmins) &&
-		!util.Contains(permissions, dataprovider.PermAdminAny) {
-		return true
-	}
-	return false
-}
-
 func (c *jwtTokenClaims) hasPerm(perm string) bool {
 	if util.Contains(c.Permissions, dataprovider.PermAdminAny) {
 		return true

+ 2 - 2
internal/httpd/httpd_test.go

@@ -11667,7 +11667,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)
@@ -11682,7 +11682,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))

+ 2 - 2
internal/httpd/webadmin.go

@@ -3046,9 +3046,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

+ 16 - 12
internal/util/util.go

@@ -40,6 +40,7 @@ import (
 	"path/filepath"
 	"regexp"
 	"runtime"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -137,18 +138,6 @@ func Contains[T comparable](elems []T, v T) bool {
 	return false
 }
 
-// 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 {
@@ -912,3 +901,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
+}

+ 1 - 1
static/locales/en/translation.json

@@ -737,7 +737,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",

+ 1 - 1
static/locales/it/translation.json

@@ -737,7 +737,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",