From 593b8546a2905f6b67e59e8812dd7b09f598d0f1 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <rpm@fthiessen.de>
Date: Wed, 1 Dec 2021 15:31:48 +0100
Subject: [PATCH] fix(roles): Ignore name if it did not change

---
 flaschengeist/controller/roleController.py | 18 ++++++++----------
 flaschengeist/plugins/roles/__init__.py    | 19 +++++++++----------
 flaschengeist/plugins/roles/permissions.py |  7 +++++++
 3 files changed, 24 insertions(+), 20 deletions(-)
 create mode 100644 flaschengeist/plugins/roles/permissions.py

diff --git a/flaschengeist/controller/roleController.py b/flaschengeist/controller/roleController.py
index a81c1bb..9d214b0 100644
--- a/flaschengeist/controller/roleController.py
+++ b/flaschengeist/controller/roleController.py
@@ -27,15 +27,8 @@ def get_permissions():
 
 @Hook
 def update_role(role, new_name):
-    if new_name is None:
-        try:
-            logger.debug(f"Hallo, dies ist die {role.serialize()}")
-            db.session.delete(role)
-            logger.debug(f"Hallo, dies ist die {role.serialize()}")
-            db.session.commit()
-        except IntegrityError:
-            logger.debug("IntegrityError: Role might still be in use", exc_info=True)
-            raise BadRequest("Role still in use")
+    if new_name is None or not isinstance(new_name, str):
+        raise BadRequest("Invalid new name")
     else:
         if role.name == new_name or db.session.query(db.exists().where(Role.name == case_sensitive(new_name))).scalar():
             raise BadRequest("Name already used")
@@ -73,4 +66,9 @@ def create_role(name: str, permissions=[]):
 
 def delete(role):
     role.permissions.clear()
-    update_role(role, None)
+    try:
+        db.session.delete(role)
+        db.session.commit()
+    except IntegrityError:
+        logger.debug("IntegrityError: Role might still be in use", exc_info=True)
+        raise BadRequest("Role still in use")
diff --git a/flaschengeist/plugins/roles/__init__.py b/flaschengeist/plugins/roles/__init__.py
index 54b0547..80deca7 100644
--- a/flaschengeist/plugins/roles/__init__.py
+++ b/flaschengeist/plugins/roles/__init__.py
@@ -10,16 +10,15 @@ from http.client import NO_CONTENT
 from flaschengeist.plugins import Plugin
 from flaschengeist.utils.decorators import login_required
 from flaschengeist.controller import roleController
-from flaschengeist.utils.HTTP import created
+from flaschengeist.utils.HTTP import created, no_content
 
-_permission_edit = "roles_edit"
-_permission_delete = "roles_delete"
+from . import permissions
 
 
 class RolesPlugin(Plugin):
     name = "roles"
     blueprint = Blueprint(name, __name__)
-    permissions = [_permission_edit, _permission_delete]
+    permissions = permissions.permissions
 
 
 @RolesPlugin.blueprint.route("/roles", methods=["GET"])
@@ -40,7 +39,7 @@ def list_roles(current_session):
 
 
 @RolesPlugin.blueprint.route("/roles", methods=["POST"])
-@login_required(permission=_permission_edit)
+@login_required(permission=permissions.EDIT)
 def create_role(current_session):
     """Create new role
 
@@ -98,7 +97,7 @@ def get_role(role_name, current_session):
 
 
 @RolesPlugin.blueprint.route("/roles/<int:role_id>", methods=["PUT"])
-@login_required(permission=_permission_edit)
+@login_required(permission=permissions.EDIT)
 def edit_role(role_id, current_session):
     """Edit role, rename and / or set permissions
 
@@ -118,13 +117,13 @@ def edit_role(role_id, current_session):
     data = request.get_json()
     if "permissions" in data:
         roleController.set_permissions(role, data["permissions"])
-    if "name" in data:
+    if "name" in data and data["name"] != role.name:
         roleController.update_role(role, data["name"])
-    return "", NO_CONTENT
+    return no_content()
 
 
 @RolesPlugin.blueprint.route("/roles/<int:role_id>", methods=["DELETE"])
-@login_required(permission=_permission_delete)
+@login_required(permission=permissions.DELETE)
 def delete_role(role_id, current_session):
     """Delete role
 
@@ -139,4 +138,4 @@ def delete_role(role_id, current_session):
     """
     role = roleController.get(role_id)
     roleController.delete(role)
-    return "", NO_CONTENT
+    return no_content()
diff --git a/flaschengeist/plugins/roles/permissions.py b/flaschengeist/plugins/roles/permissions.py
new file mode 100644
index 0000000..1bb3347
--- /dev/null
+++ b/flaschengeist/plugins/roles/permissions.py
@@ -0,0 +1,7 @@
+EDIT = "roles_edit"
+"""Can edit roles, assign permissions to roles and change names"""
+
+DELETE = "roles_delete"
+"""Can delete roles"""
+
+permissions = [value for key, value in globals().items() if not key.startswith("_")]