Merge branch 'bugfix/user-creation' into 'dev'

User Creation Fixes See merge request crafty-controller/crafty-4!763
2024-08-30 18:23:09 +00:00 · 2024-07-09 00:48:25 +00:00 · 2024-07-09 00:48:25 +00:00 · 4346f58ff3
commit 4346f58ff3
parent a710c80bb2 0519a36fe1
23 changed files with 121 additions and 21 deletions
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -8,14 +8,19 @@ TBD
 - Bump tornado & requests for sec advisories ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/774))
 - Ensure audit.log exists or create it on Crafty startup ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/771))
 - Fix typing issue on ID comparison causing general users to not be able to delete their own API keys ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/775))
+- Fix user creation bug where it would fail when a role was selected ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/763))
+- Security improvements for general user creations on roles page ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/763))
+- Security improvements for general user creations on user page ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/763))
 ### Tweaks
 - Add info note to default creds file ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/760))
 - Remove navigation label from sidebar ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/766))
 - Add a thread dump to support logs ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/769))
 - Remove text from status page and use symbols ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/770))
+- Add better feedback on when errors appear on user creation ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/763))
 ### Lang
 - Show natural language name instead of country code in User Config Lang select list ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/773))
 - Add remaining `he_IL`, `th_TH` translations for 4.4.0 Release ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/761))
+- Mark `he_IL` incomplete ([Merge Request](https://gitlab.com/crafty-controller/crafty-4/-/merge_requests/763))
 <br><br>

 ## --- [4.4.0] - 2024/05/11
--- a/app/classes/controllers/users_controller.py
+++ b/app/classes/controllers/users_controller.py
@ -55,6 +55,7 @@ class UsersController:
                "minLength": self.helper.minimum_password_length,
                "examples": ["crafty"],
                "title": "Password",
+                "error": "passLength",
            },
            "email": {
                "type": "string",
--- a/app/classes/web/routes/api/roles/index.py
+++ b/app/classes/web/routes/api/roles/index.py
@ -2,6 +2,7 @@ import typing as t
 from jsonschema import ValidationError, validate
 import orjson
 from playhouse.shortcuts import model_to_dict
+from app.classes.models.crafty_permissions import EnumPermissionsCrafty
 from app.classes.web.base_api_handler import BaseApiHandler

 create_role_schema = {
@ -71,7 +72,7 @@ class ApiRolesIndexHandler(BaseApiHandler):
            return
        (
            _,
-            _,
+            exec_user_permissions_crafty,
            _,
            superuser,
            _,
@ -81,7 +82,10 @@ class ApiRolesIndexHandler(BaseApiHandler):
        # GET /api/v2/roles?ids=true
        get_only_ids = self.get_query_argument("ids", None) == "true"

-        if not superuser:
+        if (
+            not superuser
+            and EnumPermissionsCrafty.ROLES_CONFIG not in exec_user_permissions_crafty
+        ):
            return self.finish_json(400, {"status": "error", "error": "NOT_AUTHORIZED"})

        self.finish_json(
@ -104,14 +108,17 @@ class ApiRolesIndexHandler(BaseApiHandler):
            return
        (
            _,
-            _,
+            exec_user_permissions_crafty,
            _,
            superuser,
            user,
            _,
        ) = auth_data

-        if not superuser:
+        if (
+            not superuser
+            and EnumPermissionsCrafty.ROLES_CONFIG not in exec_user_permissions_crafty
+        ):
            return self.finish_json(400, {"status": "error", "error": "NOT_AUTHORIZED"})

        try:
@ -138,6 +145,8 @@ class ApiRolesIndexHandler(BaseApiHandler):

        role_name = data["name"]
        manager = data.get("manager", None)
+        if not superuser and not manager:
+            manager = auth_data[4]["user_id"]
        if manager == self.controller.users.get_id_by_name("SYSTEM") or manager == 0:
            manager = None

--- a/app/classes/web/routes/api/roles/role/index.py
+++ b/app/classes/web/routes/api/roles/role/index.py
@ -1,6 +1,7 @@
 from jsonschema import ValidationError, validate
 import orjson
-from peewee import DoesNotExist
+from peewee import DoesNotExist, IntegrityError
+from app.classes.models.crafty_permissions import EnumPermissionsCrafty
 from app.classes.web.base_api_handler import BaseApiHandler

 modify_role_schema = {
@ -70,14 +71,17 @@ class ApiRolesRoleIndexHandler(BaseApiHandler):
            return
        (
            _,
-            _,
+            exec_user_permissions_crafty,
            _,
            superuser,
            _,
            _,
        ) = auth_data

-        if not superuser:
+        if (
+            not superuser
+            and EnumPermissionsCrafty.ROLES_CONFIG not in exec_user_permissions_crafty
+        ):
            return self.finish_json(400, {"status": "error", "error": "NOT_AUTHORIZED"})

        try:
@ -100,8 +104,11 @@ class ApiRolesRoleIndexHandler(BaseApiHandler):
            user,
            _,
        ) = auth_data
-
-        if not superuser:
+        role = self.controller.roles.get_role(role_id)
+        if (
+            str(role.get("manager", "no manager found")) != str(auth_data[4]["user_id"])
+            and not superuser
+        ):
            return self.finish_json(400, {"status": "error", "error": "NOT_AUTHORIZED"})

        self.controller.roles.remove_role(role_id)
@ -124,7 +131,7 @@ class ApiRolesRoleIndexHandler(BaseApiHandler):
            return
        (
            _,
-            _,
+            exec_user_permissions_crafty,
            _,
            superuser,
            user,
@ -132,7 +139,10 @@ class ApiRolesRoleIndexHandler(BaseApiHandler):
        ) = auth_data

        role = self.controller.roles.get_role(role_id)
-        if not superuser and user["user_id"] != role["manager"]:
+        if not superuser and (
+            user["user_id"] != role["manager"]
+            or EnumPermissionsCrafty.ROLES_CONFIG not in exec_user_permissions_crafty
+        ):
            return self.finish_json(
                400,
                {
@ -179,7 +189,10 @@ class ApiRolesRoleIndexHandler(BaseApiHandler):
            )
        except DoesNotExist:
            return self.finish_json(404, {"status": "error", "error": "ROLE_NOT_FOUND"})
-
+        except IntegrityError:
+            return self.finish_json(
+                404, {"status": "error", "error": "ROLE_NAME_EXISTS"}
+            )
        self.controller.management.add_to_audit_log(
            user["user_id"],
            f"modified role with ID {role_id}",
--- a/app/classes/web/routes/api/users/index.py
+++ b/app/classes/web/routes/api/users/index.py
@ -2,6 +2,7 @@ import logging
 import json
 from jsonschema import validate
 from jsonschema.exceptions import ValidationError
+from app.classes.shared.translation import Translation
 from app.classes.models.crafty_permissions import EnumPermissionsCrafty
 from app.classes.models.roles import Roles, HelperRoles
 from app.classes.models.users import PUBLIC_USER_ATTRS
@ -54,6 +55,7 @@ class ApiUsersIndexHandler(BaseApiHandler):
        )

    def post(self):
+        self.translator = Translation(self.helper)
        new_user_schema = {
            "type": "object",
            "properties": {
@ -87,12 +89,17 @@ class ApiUsersIndexHandler(BaseApiHandler):
        try:
            validate(data, new_user_schema)
        except ValidationError as e:
+            err = self.translator.translate(
+                "validators",
+                e.schema["error"],
+                self.controller.users.get_user_lang_by_id(auth_data[4]["user_id"]),
+            )
            return self.finish_json(
                400,
                {
                    "status": "error",
                    "error": "INVALID_JSON_SCHEMA",
-                    "error_data": str(e),
+                    "error_data": f"{str(err)}",
                },
            )
        username = data["username"]
@ -153,7 +160,11 @@ class ApiUsersIndexHandler(BaseApiHandler):

        for role in roles:
            role = self.controller.roles.get_role(role)
-            if int(role["manager"]) != int(auth_data[4]["user_id"]) and not superuser:
+            if (
+                str(role.get("manager", "no manager found"))
+                != str(auth_data[4]["user_id"])
+                and not superuser
+            ):
                return self.finish_json(
                    400, {"status": "error", "error": "INVALID_ROLES_CREATE"}
                )
--- a/app/classes/web/routes/api/users/user/index.py
+++ b/app/classes/web/routes/api/users/user/index.py
@ -132,7 +132,6 @@ class ApiUsersUserIndexHandler(BaseApiHandler):
            return self.finish_json(
                400, {"status": "error", "error": "INVALID_JSON", "error_data": str(e)}
            )
-
        try:
            validate(data, user_patch_schema)
        except ValidationError as e:
@ -144,10 +143,8 @@ class ApiUsersUserIndexHandler(BaseApiHandler):
                    "error_data": str(e),
                },
            )
-
        if user_id == "@me":
            user_id = user["user_id"]
-
        if (
            EnumPermissionsCrafty.USER_CONFIG not in exec_user_crafty_permissions
            and str(user["user_id"]) != str(user_id)
@ -215,6 +212,25 @@ class ApiUsersUserIndexHandler(BaseApiHandler):
                return self.finish_json(
                    400, {"status": "error", "error": "INVALID_ROLES_MODIFY"}
                )
+            user_modify = self.controller.users.get_user_roles_id(user_id)
+
+            for role in data["roles"]:
+                # Check if user is not a super user and that the exec user is the role
+                # manager or that the role already exists in the user's list
+                if not superuser and (
+                    str(
+                        self.controller.roles.get_role(role).get(
+                            "manager", "no manager found"
+                        )
+                    )
+                    != str(auth_data[4]["user_id"])
+                    and role not in user_modify
+                ):
+                    for item in user_modify:
+                        print(type(role), type(item))
+                    return self.finish_json(
+                        400, {"status": "error", "error": "INVALID_ROLES_MODIFY"}
+                    )

        user_obj = HelperUsers.get_user_model(user_id)
        if "password" in data and str(user["user_id"]) != str(user_id):
--- a/app/frontend/templates/panel/panel_edit_role.html
+++ b/app/frontend/templates/panel/panel_edit_role.html
@ -428,10 +428,13 @@
      if (responseData.status === "ok") {
        window.location.href = "/panel/panel_config";
      } else {
-
+        let errordata = responseData.error;
+        if (responseData.error_data){
+          errordata = responseData.error
+        }
        bootbox.alert({
          title: responseData.error,
-          message: responseData.error_data
+          message: errordata
        });
      }
    });
--- a/app/frontend/templates/panel/panel_edit_user.html
+++ b/app/frontend/templates/panel/panel_edit_user.html
@ -393,6 +393,7 @@ data['lang']) }}{% end %}
  }
  function replacer(key, value) {
      if (typeof value == "boolean" || key === "email" || key === "permissions" || key === "roles") {
+        console.log(key)
        return value
      } else {
        console.log(key, value)
@ -433,6 +434,7 @@ data['lang']) }}{% end %}
      let disabled_flag = false;
      let roles = null;
      if (superuser || userId != edit_id){
+        console.log("ROLES")
        roles = $('.role_check').map(function() {
        if ($(this).attr("disabled")){
          disabled_flag = true;
@ -457,9 +459,7 @@ data['lang']) }}{% end %}
        delete formDataObject.username
      }
      if (superuser || userId != edit_id){
-      if (!disabled_flag){
        formDataObject.roles = roles;
-      }
      if ($("#permissions").length){
        formDataObject.permissions = permissions;
      }
--- a/app/translations/cs_CS.json
+++ b/app/translations/cs_CS.json
@ -672,6 +672,9 @@
        "userTheme": "Motiv UI",
        "uses": "Počet povolených použití (-1==bez omezení)"
    },
+    "validators": {
+        "passLength": "Heslo je příliš krátké. Minimální délka je 8 znaků"
+    },
    "webhooks": {
        "areYouSureDel": "Seš si jistý že chceš smazat tento webhook?",
        "areYouSureRun": "Seš si jistý že chceš otestovat tento webhook?",
--- a/app/translations/de_DE.json
+++ b/app/translations/de_DE.json
@ -653,6 +653,9 @@
        "userTheme": "Design für die Benutzeroberfläche",
        "uses": "Anzahl der erlaubten Verwendungen (-1==Keine Begrenzung)"
    },
+    "validators": {
+        "passLength": "Passwort zu kurz. Mindestlänge: 8"
+    },
    "webhooks": {
        "areYouSureDel": "Sind Sie sicher, dass Sie diesen Webhook löschen möchten?",
        "areYouSureRun": "Sind Sie sicher, dass Sie diesen Webhook testen möchten?",
--- a/app/translations/en_EN.json
+++ b/app/translations/en_EN.json
@ -649,6 +649,9 @@
        "userTheme": "UI Theme",
        "uses": "Number of uses allowed (-1==No Limit)"
    },
+    "validators": {
+        "passLength": "Password Too Short. Minimum Length: 8"
+    },
    "webhooks": {
        "areYouSureDel": "Are you sure you want to delete this webhook?",
        "areYouSureRun": "Are you sure you want to test this webhook?",
--- a/app/translations/es_ES.json
+++ b/app/translations/es_ES.json
@ -653,6 +653,9 @@
        "userTheme": "Tema de Interfaz",
        "uses": "Número de usos permitidos. (Sin límite: -1)"
    },
+    "validators": {
+        "passLength": "Contraseña demasiado corta. Longitud mínima: 8"
+    },
    "webhooks": {
        "areYouSureDel": "¿Estás seguro de que quieres eliminar este webhook?",
        "areYouSureRun": "¿Estás seguro de que quieres probar este webhook?",
--- a/app/translations/fr_FR.json
+++ b/app/translations/fr_FR.json
@ -653,6 +653,9 @@
        "userTheme": "Theme d'Interface Utilisateur",
        "uses": "Nombre d'utilisation Authorisé (-1 == Illimité)"
    },
+    "validators": {
+        "passLength": "Mot de passe trop court. Longueur minimum : 8"
+    },
    "webhooks": {
        "areYouSureDel": "Es-tu sûr de vouloir supprimer ce webhook ?",
        "areYouSureRun": "Es-tu sûr de vouloir tester ce webhook ?",
--- a/app/translations/he_IL_incomplete.json
+++ b/app/translations/he_IL_incomplete.json
--- a/app/translations/it_IT.json
+++ b/app/translations/it_IT.json
@ -653,6 +653,9 @@
        "userTheme": "Tema IU",
        "uses": "Numero di usi permessi (-1==Nessun limite)"
    },
+    "validators": {
+        "passLength": "La password è troppo corta. Lunghezza minima: 8"
+    },
    "webhooks": {
        "areYouSureDel": "Sei sicuro di voler eliminare questo webhook?",
        "areYouSureRun": "Sei sicuro di voler testare questo webhook?",
--- a/app/translations/lol_EN.json
+++ b/app/translations/lol_EN.json
@ -653,6 +653,9 @@
        "userTheme": "THEMEZ",
        "uses": "NUMBER OV USES ALLOWED (-1==NO LIMIT)"
    },
+    "validators": {
+        "passLength": "PASSWRD TOO SMOL. NEEDZ 8 CATZ PLZ"
+    },
    "webhooks": {
        "areYouSureDel": "U SURE U WANTZ TO EATZ DIS WEBHOOK?",
        "areYouSureRun": "U SURE U WANTZ TO TESTZ DIS WEBHOOK?",
--- a/app/translations/lv_LV.json
+++ b/app/translations/lv_LV.json
@ -654,6 +654,9 @@
        "userTheme": "UI Tēma",
        "uses": "Dauzums, cik reizes lietot (-1==Bez Limita)"
    },
+    "validators": {
+        "passLength": "Parole pārāk īsa. Minimālais Garums: 8"
+    },
    "webhooks": {
        "areYouSureDel": "Vai tiešām vēlies noņemt šo webhook?",
        "areYouSureRun": "Vai tiešām vēlies testēt šo webhook?",
--- a/app/translations/nl_BE.json
+++ b/app/translations/nl_BE.json
@ -653,6 +653,9 @@
        "userTheme": "UI-thema",
        "uses": "Aantal toegestane gebruiken (-1==Geen Limiet)"
    },
+    "validators": {
+        "passLength": "Wachtwoord te kort. Minimumlengte: 8 tekens"
+    },
    "webhooks": {
        "areYouSureDel": "Weet u zeker dat u deze webhook wilt verwijderen?",
        "areYouSureRun": "Weet u zeker dat u deze webhook wilt testen?",
--- a/app/translations/pl_PL.json
+++ b/app/translations/pl_PL.json
@ -652,6 +652,9 @@
        "userTheme": "Wygląd interfejsu",
        "uses": "Ilość użyć (-1==Bez limitu)"
    },
+    "validators": {
+        "passLength": "Hasło jest zbyt krótkie. Hasło musi posiadać minimum 8 znaków."
+    },
    "webhooks": {
        "areYouSureDel": "Usunąć ten webhook?",
        "areYouSureRun": "Przetestować ten webhook?",
--- a/app/translations/th_TH.json
+++ b/app/translations/th_TH.json
@ -652,6 +652,9 @@
        "userTheme": "ธีม UI",
        "uses": "จำนวนการใช้งานที่อนุญาต (-1==ไม่มีขีดจำกัด)"
    },
+    "validators": {
+        "passLength": "รหัสผ่านสั้นเกินไป จำนวนตัวอักขระขั้นต่ำ: 8"
+    },
    "webhooks": {
        "areYouSureDel": "คุณแน่ใจหรือไม่ว่าต้องการลบ Webhook นี้?",
        "areYouSureRun": "คุณแน่ใจหรือไม่ว่าต้องการทดสอบ Webhook นี้?",
--- a/app/translations/tr_TR.json
+++ b/app/translations/tr_TR.json
@ -652,6 +652,9 @@
        "userTheme": "UI Teması",
        "uses": "İzin verilen kullanım sayısı (-1==Sınır Yok)"
    },
+    "validators": {
+        "passLength": "Şifre çok kısa. Şifre en az 8 karakter olmalı."
+    },
    "webhooks": {
        "areYouSureDel": "Bu webhooku silmek istediğinizden emin misiniz?",
        "areYouSureRun": "Bu webhooku test etmek istediğinizden emin misiniz?",
--- a/app/translations/uk_UA.json
+++ b/app/translations/uk_UA.json
@ -652,6 +652,9 @@
        "userTheme": "Тема інтерфейсу",
        "uses": "Дозволена кількість використань(-1==Без ліміту)"
    },
+    "validators": {
+        "passLength": "Пароль, надто короткий. Мінімальна довжина: 8 символів"
+    },
    "webhooks": {
        "areYouSureDel": "Ви впевнені, що хочете видалити цей Вебхук?",
        "areYouSureRun": "Ви впевнені, що хочете перевірити цей Вебхук?",
--- a/app/translations/zh_CN.json
+++ b/app/translations/zh_CN.json
@ -653,6 +653,9 @@
        "userTheme": "UI 主题",
        "uses": "使用次数限制（-1==无限制）"
    },
+    "validators": {
+        "passLength": "密码过短。最短长度：8"
+    },
    "webhooks": {
        "areYouSureDel": "您确定要删除此 webhook 吗？",
        "areYouSureRun": "您确定要测试此 webhook 吗？",