From 2d6b86e2eb8029e063974efe45e2f68d219c54c5 Mon Sep 17 00:00:00 2001 From: Ferdinand Thiessen Date: Wed, 18 Nov 2020 00:39:25 +0100 Subject: [PATCH] [System][Plugin] Improved Hooks, roles and auth_ldap improvements * Hooks now allow multiple hooked functions * Hooks can now be called before and after a function call * Fixed issue in datetime util when string is None or empty * Roles: Return new created role as json * auth_ldap: Use new Hooks * auth_ldap: Fixed an issue where ldap response is not checked (when role gets renamed) --- flaschengeist/controller/roleController.py | 41 ++++++++++----------- flaschengeist/controller/userController.py | 2 +- flaschengeist/plugins/__init__.py | 23 ++++++------ flaschengeist/plugins/auth_ldap/__init__.py | 20 +++++----- flaschengeist/plugins/roles/__init__.py | 10 ++--- flaschengeist/utils/datetime.py | 2 + flaschengeist/utils/hook.py | 31 ++++++++++++---- 7 files changed, 72 insertions(+), 57 deletions(-) diff --git a/flaschengeist/controller/roleController.py b/flaschengeist/controller/roleController.py index e478eec..501da56 100644 --- a/flaschengeist/controller/roleController.py +++ b/flaschengeist/controller/roleController.py @@ -26,21 +26,21 @@ def get_permissions(): @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") - - old_name = role.name - role.name = new_name - role_updated(role, old_name) - db.session.commit() +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") + elif role.name != new_name: + if db.session.query(db.exists().where(Role.name == new_name)).scalar(): + raise BadRequest("Name already used") + role.name = new_name + db.session.commit() def set_permissions(role, permissions): @@ -63,18 +63,15 @@ def create_permissions(permissions): def create_role(name: str, permissions=[]): + logger.debug(f"Create new role with name: {name}") role = Role(name=name) db.session.add(role) set_permissions(role, permissions) + db.session.commit() + logger.debug(f"Created role: {role.serialize()}") return role def delete(role): role.permissions.clear() - 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") - role_updated(None, role.name) + update_role(role, None) diff --git a/flaschengeist/controller/userController.py b/flaschengeist/controller/userController.py index 96c52ab..910951e 100644 --- a/flaschengeist/controller/userController.py +++ b/flaschengeist/controller/userController.py @@ -101,5 +101,5 @@ def save_avatar(user, avatar): user.avatar_url = "" r = current_app.config["FG_AUTH_BACKEND"].set_avatar(user, avatar) if not user.avatar_url: - user.avatar_url = url_for('users.get_avatar', userid=user.userid) + user.avatar_url = url_for("users.get_avatar", userid=user.userid) db.session.commit() diff --git a/flaschengeist/plugins/__init__.py b/flaschengeist/plugins/__init__.py index bc66893..a61e7f3 100644 --- a/flaschengeist/plugins/__init__.py +++ b/flaschengeist/plugins/__init__.py @@ -2,22 +2,21 @@ import pkg_resources from werkzeug.exceptions import MethodNotAllowed, NotFound from flaschengeist.models.user import _Avatar -from flaschengeist.utils.hook import HookCall +from flaschengeist.utils.hook import HookBefore, HookAfter -send_message_hook = HookCall("send_message") -"""Hook decorator for sending messages, register to send the message -Args: - message: Message object to send -""" - -role_updated = HookCall("role_updated") +before_role_updated = HookBefore("update_role") """Hook decorator for when roles are modified Args: - role: Role object containing the modified role (None if deleted) - old_name: Old name if the name was changed + role: Role object to modify + new_name: New name if the name was changed (None if delete) """ - -update_user_hook = HookCall("update_user") +after_role_updated = HookAfter("update_role") +"""Hook decorator for when roles are modified +Args: + role: Role object containing the modified role + new_name: New name if the name was changed (None if deleted) +""" +before_update_user = HookBefore("update_user") """Hook decorator, when ever an user update is done, this is called before. Args: user: User object diff --git a/flaschengeist/plugins/auth_ldap/__init__.py b/flaschengeist/plugins/auth_ldap/__init__.py index 1d22822..619ac94 100644 --- a/flaschengeist/plugins/auth_ldap/__init__.py +++ b/flaschengeist/plugins/auth_ldap/__init__.py @@ -10,7 +10,7 @@ from ldap3 import SUBTREE, MODIFY_REPLACE, MODIFY_ADD, MODIFY_DELETE, HASHED_SAL from werkzeug.exceptions import BadRequest, InternalServerError, NotFound from flaschengeist import logger -from flaschengeist.plugins import AuthPlugin, role_updated +from flaschengeist.plugins import AuthPlugin, after_role_updated from flaschengeist.models.user import User, Role, _Avatar import flaschengeist.controller.userController as userController @@ -43,9 +43,9 @@ class AuthLDAP(AuthPlugin): else: self.admin_dn = None - @role_updated - def _role_updated(role, old_name): - self.__modify_role(role, old_name) + @after_role_updated + def _role_updated(role, new_name): + self.__modify_role(role, new_name) def login(self, user, password): if not user: @@ -183,19 +183,19 @@ class AuthLDAP(AuthPlugin): def __modify_role( self, - role: Optional[Role], - old_name: str, + role: Role, + 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: + ldap_conn.search(f"ou=group,{self.dn}", f"(cn={role.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}") + if new_name: + ldap_conn.modify_dn(dn, f"cn={new_name}") else: ldap_conn.delete(dn) diff --git a/flaschengeist/plugins/roles/__init__.py b/flaschengeist/plugins/roles/__init__.py index fd93a03..de0c505 100644 --- a/flaschengeist/plugins/roles/__init__.py +++ b/flaschengeist/plugins/roles/__init__.py @@ -10,6 +10,7 @@ from http.client import CREATED, NO_CONTENT from flaschengeist.plugins import Plugin from flaschengeist.decorator import login_required from flaschengeist.controller import roleController +from flaschengeist.utils.HTTP import created roles_bp = Blueprint("roles", __name__) _permission_edit = "roles_edit" @@ -51,15 +52,14 @@ def create_role(current_session): current_session: Session sent with Authorization Header Returns: - HTTP-201 or HTTP error + HTTP-201 and json encoded created Role or HTTP error """ data = request.get_json() if not data or "name" not in data: raise BadRequest if "permissions" in data: permissions = data["permissions"] - roleController.create_role(data["name"], permissions) - return "", CREATED + return created(roleController.create_role(data["name"], permissions)) @roles_bp.route("/roles/permissions", methods=["GET"]) @@ -116,10 +116,10 @@ def edit_role(role_id, current_session): role = roleController.get(role_id) data = request.get_json() - if "name" in data: - roleController.rename(role, data["name"]) if "permissions" in data: roleController.set_permissions(role, data["permissions"]) + if "name" in data: + roleController.update_role(role, data["name"]) return "", NO_CONTENT diff --git a/flaschengeist/utils/datetime.py b/flaschengeist/utils/datetime.py index dc62371..cf97a00 100644 --- a/flaschengeist/utils/datetime.py +++ b/flaschengeist/utils/datetime.py @@ -6,6 +6,8 @@ MonkeyPatch.patch_fromisoformat() def from_iso_format(date_str): """Z-suffix aware version of `datetime.datetime.fromisoformat`""" + if not date_str: + return None time = datetime.datetime.fromisoformat(date_str.replace("Z", "+00:00")) if time.tzinfo: return time.astimezone(datetime.timezone.utc) diff --git a/flaschengeist/utils/hook.py b/flaschengeist/utils/hook.py index 0af84f3..02db156 100644 --- a/flaschengeist/utils/hook.py +++ b/flaschengeist/utils/hook.py @@ -1,4 +1,4 @@ -_hook_dict = {} +_hook_dict = ({}, {}) class Hook(object): @@ -10,17 +10,34 @@ class Hook(object): self.function = function def __call__(self, *args, **kwargs): - if self.function.__name__ in _hook_dict: - _hook_dict[self.function.__name__](*args, **kwargs) - self.function(*args, **kwargs) + # Hooks before + for function in _hook_dict[0].get(self.function.__name__, []): + function(*args, **kwargs) + # Main function + ret = self.function(*args, **kwargs) + # Hooks after + for function in _hook_dict[1].get(self.function.__name__, []): + function(*args, **kwargs) + return ret -class HookCall(object): - """Decorator for functions to be called if a Hook is called""" +class HookBefore(object): + """Decorator for functions to be called before a Hook-Function is called""" def __init__(self, name): self.name = name def __call__(self, function): - _hook_dict[self.name] = function + _hook_dict[0].setdefault(self.name, []).append(function) + return function + + +class HookAfter(object): + """Decorator for functions to be called after a Hook-Function is called""" + + def __init__(self, name): + self.name = name + + def __call__(self, function): + _hook_dict[1].setdefault(self.name, []).append(function) return function