[Plugin] Use plugin function instead of HookCall

flaschengeist/controller/roleController.py

@@ -1,10 +1,10 @@
-from flask import current_app
 from sqlalchemy.exc import IntegrityError
 from werkzeug.exceptions import BadRequest, NotFound
 
 from flaschengeist.models.user import Role, Permission
 from flaschengeist.database import db
 from flaschengeist import logger
+from flaschengeist.utils.hook import Hook
 
 
 def get_all():
@@ -25,14 +25,21 @@ def get_permissions():
     return Permission.query.all()
 
 
+@Hook
+def role_updated(role, old_name):
+    """Hook used when roles are updated"""
+    pass
+
+
 def rename(role, new_name):
     if role.name == new_name:
         return
     if db.session.query(db.exists().where(Role.name == new_name)).scalar():
         raise BadRequest("Name already used")
 
-    current_app.config["FG_AUTH_BACKEND"].modify_role(role.name, new_name)
+    old_name = role.name
     role.name = new_name
+    role_updated(role, old_name)
     db.session.commit()
 
 
@@ -70,4 +77,4 @@ def delete(role):
     except IntegrityError:
         logger.debug("IntegrityError: Role might still be in use", exc_info=True)
         raise BadRequest("Role still in use")
-    current_app.config["FG_AUTH_BACKEND"].modify_role(role.name, None)
+    role_updated(None, role.name)

flaschengeist/controller/userController.py

@@ -8,21 +8,6 @@ from flaschengeist.database import db
 from flaschengeist import logger
 
 
-class Avatar:
-    mimetype = ""
-    binary = bytearray()
-
-
-@Hook
-def load_avatar(avatar: Avatar, user: User):
-    pass
-
-
-@Hook
-def save_avatar(avatar: Avatar, user: User):
-    pass
-
-
 def login_user(username, password):
     logger.info("login user {{ {} }}".format(username))
     try:
@@ -106,3 +91,11 @@ def register(data):
     db.session.add(user)
     db.session.commit()
     return user
+
+
+def load_avatar(user: User):
+    return current_app.config["FG_AUTH_BACKEND"].get_avatar(user)
+
+
+def save_avatar(user, avatar):
+    return current_app.config["FG_AUTH_BACKEND"].set_avatar(user, avatar)

flaschengeist/models/user.py

@@ -96,3 +96,10 @@ class _UserAttribute(db.Model, ModelSerializeMixin):
     user: User = db.Column("user", db.Integer, db.ForeignKey("user.id"), nullable=False)
     name: str = db.Column(db.String(30))
     value: any = db.Column(db.PickleType(protocol=4))
+
+
+class _Avatar:
+    """Wrapper class for avatar binaries"""
+
+    mimetype = ""
+    binary = bytearray()

flaschengeist/plugins/__init__.py

@@ -1,7 +1,7 @@
 import pkg_resources
-from typing import Optional
+from werkzeug.exceptions import MethodNotAllowed, NotFound
-from werkzeug.exceptions import MethodNotAllowed
 
+from flaschengeist.models.user import _Avatar
 from flaschengeist.utils.hook import HookCall
 
 send_message_hook = HookCall("send_message")
@@ -10,18 +10,11 @@ Args:
     message: Message object to send
 """
 
-load_avatar_hook = HookCall("load_avatar")
+role_updated = HookCall("role_updated")
-"""Hook decorator for loading Avatar data
+"""Hook decorator for when roles are modified
 Args:
-    avatar: Avatar object
+    role: Role object containing the modified role (None if deleted)
-    user: User object to load from
+    old_name: Old name if the name was changed
-"""
-
-save_avatar_hook = HookCall("save_avatar")
-"""Hook decorator for saving Avatar data
-Args:
-    avatar: Avatar object
-    user: User object to save
 """
 
 update_user_hook = HookCall("update_user")
@@ -95,16 +88,6 @@ class AuthPlugin(Plugin):
         """
         raise NotImplemented
 
-    def modify_role(self, old_name: str, new_name: Optional[str]):
-        """A call to this function indicated that a role was deleted (and has no users)
-        Might be used if modify_user is implemented.
-
-        Args:
-            old_name: Name of the modified role
-            new_name: New role name or None if deleted
-        """
-        pass
-
     def create_user(self, user, password):
         """If backend is using (writeable) external data, then create a new user on the external database.
 
@@ -123,3 +106,24 @@ class AuthPlugin(Plugin):
 
         """
         raise MethodNotAllowed
+
+    def get_avatar(self, user) -> _Avatar:
+        """Retrieve avatar for given user (if supported by auth backend)
+
+        Args:
+            user: User to retrieve the avatar for
+        Raises:
+            NotFound: If no avatar found or not implemented
+        """
+        raise NotFound
+
+    def set_avatar(self, user, avatar: _Avatar):
+        """Set the avatar for given user (if supported by auth backend)
+
+        Args:
+            user: User to set the avatar for
+            avatar: Avatar to set
+        Raises:
+            MethodNotAllowed: If not supported by Backend
+        """
+        raise MethodNotAllowed

flaschengeist/plugins/auth_ldap/__init__.py

@@ -7,11 +7,11 @@ from flask import current_app as app
 from ldap3.utils.hashed import hashed
 from ldap3.core.exceptions import LDAPPasswordIsMandatoryError, LDAPBindError
 from ldap3 import SUBTREE, MODIFY_REPLACE, MODIFY_ADD, MODIFY_DELETE, HASHED_SALTED_MD5
-from werkzeug.exceptions import BadRequest, InternalServerError
+from werkzeug.exceptions import BadRequest, InternalServerError, NotFound
 
 from flaschengeist import logger
-from flaschengeist.plugins import AuthPlugin, load_avatar_hook, save_avatar_hook
+from flaschengeist.plugins import AuthPlugin, role_updated
-from flaschengeist.models.user import User
+from flaschengeist.models.user import User, Role, _Avatar
 import flaschengeist.controller.userController as userController
 
 
@@ -43,13 +43,9 @@ class AuthLDAP(AuthPlugin):
         else:
             self.admin_dn = None
 
-        @load_avatar_hook
+        @role_updated
-        def load_avatar(avatar, user):
+        def _role_updated(role, old_name):
-            self.__load_avatar(avatar, user)
+            self.__modify_role(role, old_name)
-
-        @save_avatar_hook
-        def load_avatar(avatar, user):
-            self.__save_avatar(avatar, user)
 
     def login(self, user, password):
         if not user:
@@ -88,12 +84,12 @@ class AuthLDAP(AuthPlugin):
                 SUBTREE,
                 attributes=["uidNumber"],
             )
-            uidNumbers = sorted(
+            uid_number = (
-                self.ldap.response(),
+                sorted(self.ldap.response(), key=lambda i: i["attributes"]["uidNumber"], reverse=True,)[0][
-                key=lambda i: i["attributes"]["uidNumber"],
+                    "attributes"
-                reverse=True,
+                ]["uidNumber"]
+                + 1
             )
-            uidNumber = uidNumbers[0]["attributes"]["uidNumber"] + 1
             dn = f"cn={user.firstname} {user.lastname},ou=user,{self.dn}"
             object_class = [
                 "inetOrgPerson",
@@ -109,30 +105,13 @@ class AuthLDAP(AuthPlugin):
                 "loginShell": "/bin/bash",
                 "uid": user.userid,
                 "userPassword": hashed(HASHED_SALTED_MD5, password),
-                "uidNumber": uidNumber,
+                "uidNumber": uid_number,
             }
             ldap_conn.add(dn, object_class, attributes)
             self._set_roles(user)
         except (LDAPPasswordIsMandatoryError, LDAPBindError):
             raise BadRequest
 
-    def modify_role(self, old_name: str, new_name: Optional[str]):
-        if self.admin_dn is None:
-            logger.error("admin_dn missing in ldap config!")
-            raise InternalServerError
-        try:
-            ldap_conn = self.ldap.connect(self.admin_dn, self.admin_secret)
-            ldap_conn.search(f"ou=group,{self.dn}", f"(cn={old_name})", SUBTREE, attributes=["cn"])
-            if len(ldap_conn.response) >= 0:
-                dn = ldap_conn.response[0]["dn"]
-                if new_name:
-                    ldap_conn.modify_dn(dn, f"cn={new_name}")
-                else:
-                    ldap_conn.delete(dn)
-
-        except (LDAPPasswordIsMandatoryError, LDAPBindError):
-            raise BadRequest
-
     def modify_user(self, user: User, password=None, new_password=None):
         try:
             dn = user.get_attribute("DN")
@@ -161,7 +140,7 @@ class AuthLDAP(AuthPlugin):
         except (LDAPPasswordIsMandatoryError, LDAPBindError):
             raise BadRequest
 
-    def __load_avatar(self, avatar, user):
+    def get_avatar(self, user):
         self.ldap.connection.search(
             "ou=user,{}".format(self.dn),
             "(uid={})".format(user.userid),
@@ -169,16 +148,21 @@ class AuthLDAP(AuthPlugin):
             attributes=["uid", "jpegPhoto"],
         )
         r = self.ldap.connection.response[0]["attributes"]
-        if r["uid"][0] == user.userid:
-            avatar.mimetype = "image/jpeg"
-            avatar.binary.clear()
-            avatar.binary.extend(r["jpegPhoto"][0])
 
-    def __save_avatar(self, avatar, user):
+        if r["uid"][0] == user.userid:
+            avatar = _Avatar()
+            avatar.mimetype = "image/jpeg"
+            avatar.binary.extend(r["jpegPhoto"][0])
+            return avatar
+        else:
+            raise NotFound
+
+    def set_avatar(self, user, avatar: _Avatar):
         if avatar.mimetype != "image/jpeg":
             # Try converting using Pillow (if installed)
             try:
                 from PIL import Image
+
                 image = Image.open(io.BytesIO(avatar.binary))
                 image_bytes = io.BytesIO()
                 image.save(image_bytes, format="JPEG")
@@ -198,6 +182,27 @@ class AuthLDAP(AuthPlugin):
         ldap_conn = self.ldap.connect(self.admin_dn, self.admin_secret)
         ldap_conn.modify(dn, {"jpegPhoto": [(MODIFY_REPLACE, [avatar.binary])]})
 
+    def __modify_role(
+        self,
+        role: Optional[Role],
+        old_name: str,
+    ):
+        if self.admin_dn is None:
+            logger.error("admin_dn missing in ldap config!")
+            raise InternalServerError
+        try:
+            ldap_conn = self.ldap.connect(self.admin_dn, self.admin_secret)
+            ldap_conn.search(f"ou=group,{self.dn}", f"(cn={old_name})", SUBTREE, attributes=["cn"])
+            if len(ldap_conn.response) >= 0:
+                dn = ldap_conn.response[0]["dn"]
+                if role:
+                    ldap_conn.modify_dn(dn, f"cn={role.name}")
+                else:
+                    ldap_conn.delete(dn)
+
+        except (LDAPPasswordIsMandatoryError, LDAPBindError):
+            raise BadRequest
+
     def _get_groups(self, uid):
         groups = []
         self.ldap.connection.search(
@@ -211,7 +216,7 @@ class AuthLDAP(AuthPlugin):
             groups.append(data["attributes"]["cn"][0])
         return groups
 
-    def _get_all_roles(self, ldap_conn):
+    def _get_all_roles(self):
         self.ldap.connection.search(
             f"ou=group,{self.dn}",
             "(cn=*)",
@@ -224,7 +229,7 @@ class AuthLDAP(AuthPlugin):
         try:
             ldap_conn = self.ldap.connect(self.admin_dn, self.admin_secret)
 
-            ldap_roles = self._get_all_roles(ldap_conn)
+            ldap_roles = self._get_all_roles()
 
             gid_numbers = sorted(ldap_roles, key=lambda i: i["attributes"]["gidNumber"], reverse=True)
             gid_number = gid_numbers[0]["attributes"]["gidNumber"] + 1
@@ -237,7 +242,7 @@ class AuthLDAP(AuthPlugin):
                         attributes={"gidNumber": gid_number},
                     )
 
-            ldap_roles = self._get_all_roles(ldap_conn)
+            ldap_roles = self._get_all_roles()
 
             for ldap_role in ldap_roles:
                 if ldap_role["attributes"]["cn"][0] in user.roles:

flaschengeist/plugins/users/__init__.py

@@ -9,7 +9,7 @@ from flask import Blueprint, request, jsonify, make_response, Response
 from werkzeug.exceptions import BadRequest, Forbidden, MethodNotAllowed, NotFound
 
 from flaschengeist import logger
-from flaschengeist.models.user import User
+from flaschengeist.models.user import User, _Avatar
 from flaschengeist.plugins import Plugin
 from flaschengeist.decorator import login_required, extract_session
 from flaschengeist.controller import userController
@@ -97,8 +97,7 @@ def get_user(userid, current_session):
 @users_bp.route("/users/<userid>/avatar", methods=["GET"])
 def get_avatar(userid):
     user = userController.get_user(userid)
-    avatar = userController.Avatar()
+    avatar = userController.load_avatar(user)
-    userController.load_avatar(avatar, user)
     if len(avatar.binary) > 0:
         response = Response(avatar.binary, mimetype=avatar.mimetype)
         response.add_etag()
@@ -115,10 +114,10 @@ def set_avatar(userid, current_session):
 
     file = request.files.get("file")
     if file:
-        avatar = userController.Avatar()
+        avatar = _Avatar()
         avatar.mimetype = file.content_type
         avatar.binary = bytearray(file.stream.read())
-        userController.save_avatar(avatar, user)
+        userController.save_avatar(user, avatar)
         return created()
     else:
         raise BadRequest