From 824ffc86758c7b9637a4a8458c80a537a7183057 Mon Sep 17 00:00:00 2001
From: Ferdinand Thiessen <rpm@fthiessen.de>
Date: Mon, 9 Nov 2020 03:44:35 +0100
Subject: [PATCH] [Plugin] Roles: Fixed controller and Model

* Identify role by id not name, as name might change
* Set permissions and Delete Role are fixed (db exception was thrown)
---
 flaschengeist/controller/roleController.py | 19 ++++++++++++-------
 flaschengeist/models/user.py               |  5 ++---
 flaschengeist/plugins/roles/__init__.py    | 22 +++++++++++-----------
 3 files changed, 25 insertions(+), 21 deletions(-)

diff --git a/flaschengeist/controller/roleController.py b/flaschengeist/controller/roleController.py
index 7100570..185bffa 100644
--- a/flaschengeist/controller/roleController.py
+++ b/flaschengeist/controller/roleController.py
@@ -11,9 +11,13 @@ def get_all():
 
 
 def get(role_name):
-    role = Role.query.filter(Role.name == role_name).one_or_none()
+    if type(role_name) is int:
+        role = Role.query.get(role_name)
+    else:
+        role = Role.query.filter(Role.name == role_name).one_or_none()
     if not role:
         raise NotFound
+    return role
 
 
 def get_permissions():
@@ -25,11 +29,12 @@ def update_role(role):
 
 
 def set_permissions(role, permissions):
+    role.permissions.clear()
     for name in permissions:
-        p = Permission.query.filter(Permission.name == name).one_or_none()
-        if not p:
+        p = Permission.query.filter(Permission.name.in_(permissions)).all()
+        if not p or len(p) < len(permissions):
             raise BadRequest("Invalid permission name >{}<".format(name))
-        role.permissions.append(p)
+        role.permissions.extend(p)
     db.session.commit()
 
 
@@ -50,10 +55,10 @@ def create_role(name: str, permissions=[]):
 
 
 def delete(role):
+    role.permissions.clear()
     try:
-        num = Role.query.filter(Role.id == role.id).delete()
+        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")
-    db.session.commit()
-    return num == 1
diff --git a/flaschengeist/models/user.py b/flaschengeist/models/user.py
index fe55ee8..ae55369 100644
--- a/flaschengeist/models/user.py
+++ b/flaschengeist/models/user.py
@@ -25,13 +25,12 @@ class Permission(db.Model, ModelSerializeMixin):
 
 class Role(db.Model, ModelSerializeMixin):
     __tablename__ = "role"
+    id: int = db.Column(db.Integer, primary_key=True)
     name: str = db.Column(db.String(30), unique=True)
     permissions: [Permission] = db.relationship(
-        "Permission", secondary=role_permission_association_table, cascade="all, delete"
+        "Permission", secondary=role_permission_association_table
     )
 
-    _id = db.Column("id", db.Integer, primary_key=True)
-
 
 class User(db.Model, ModelSerializeMixin):
     """Database Object for User
diff --git a/flaschengeist/plugins/roles/__init__.py b/flaschengeist/plugins/roles/__init__.py
index be7ab4f..ee8c6d4 100644
--- a/flaschengeist/plugins/roles/__init__.py
+++ b/flaschengeist/plugins/roles/__init__.py
@@ -50,7 +50,7 @@ def create_role(current_session):
         current_session: Session sent with Authorization Header
 
     Returns:
-        HTTP-200 or HTTP error
+        HTTP-201 or HTTP error
     """
     data = request.get_json()
     if not data or "name" not in data:
@@ -96,23 +96,23 @@ def get_role(role_name, current_session):
     return jsonify(role)
 
 
-@roles_bp.route("/roles/<role_name>", methods=["PUT"])
+@roles_bp.route("/roles/<int:role_id>", methods=["PUT"])
 @login_required(permission=_permission_edit)
-def edit_role(role_name, current_session):
+def edit_role(role_id, current_session):
     """Edit role, rename and / or set permissions
 
-    Route: ``/roles/<role_name>`` | Method: ``PUT``
+    Route: ``/roles/<role_id>`` | Method: ``PUT``
 
     POST-data: ``{name?: string, permissions?: string[]}``
 
     Args:
-        role_name: Name of role
+        role_id: Identifier of the role
         current_session: Session sent with Authorization Header
 
     Returns:
             HTTP-200 or HTTP error
     """
-    role = roleController.get(role_name)
+    role = roleController.get(role_id)
 
     data = request.get_json()
     if "name" in data:
@@ -123,20 +123,20 @@ def edit_role(role_name, current_session):
     return "", NO_CONTENT
 
 
-@roles_bp.route("/roles/<role_name>", methods=["DELETE"])
+@roles_bp.route("/roles/<int:role_id>", methods=["DELETE"])
 @login_required(permission=_permission_edit)
-def delete_role(role_name, current_session):
+def delete_role(role_id, current_session):
     """Delete role
 
-    Route: ``/roles/<role_name>`` | Method: ``DELETE``
+    Route: ``/roles/<role_id>`` | Method: ``DELETE``
 
     Args:
-        role_name: Name of role
+        role_id: Identifier of the role
         current_session: Session sent with Authorization Header
 
     Returns:
         HTTP-204 or HTTP error
     """
-    role = roleController.get(role_name)
+    role = roleController.get(role_id)
     roleController.delete(role)
     return "", NO_CONTENT