diff --git a/src/permission.py b/src/permission.py index b416b05aba..9654d57cdc 100644 --- a/src/permission.py +++ b/src/permission.py @@ -580,10 +580,10 @@ def permission_url( ldap.update( f"cn={permission},ou=permission", { - "URL": [url] if url is not None else [], + "URL": url if url is not None else set(), "additionalUrls": new_additional_urls, - "authHeader": [str(auth_header).upper()], - "showTile": [str(show_tile).upper()], + "authHeader": str(auth_header).upper(), + "showTile": str(show_tile).upper(), }, ) except Exception as e: @@ -670,9 +670,9 @@ def permission_sync_to_user(): continue new_inherited_perms = { - "inheritPermission": [ + "inheritPermission": { f"uid={u},ou=users,dc=yunohost,dc=org" for u in should_be_allowed_users - ], + }, "memberUid": should_be_allowed_users, } @@ -727,15 +727,15 @@ def _update_ldap_group_permission( allowed = [allowed] if not isinstance(allowed, list) else allowed # Guarantee uniqueness of values in allowed, which would otherwise make ldap.update angry. allowed = set(allowed) - update["groupPermission"] = [ + update["groupPermission"] = { "cn=" + g + ",ou=groups,dc=yunohost,dc=org" for g in allowed - ] + } if label is not None: - update["label"] = [str(label)] + update["label"] = str(label) if protected is not None: - update["isProtected"] = [str(protected).upper()] + update["isProtected"] = str(protected).upper() if show_tile is not None: if show_tile is True: @@ -752,7 +752,7 @@ def _update_ldap_group_permission( m18n.n("show_tile_cant_be_enabled_for_regex", permission=permission) ) show_tile = False - update["showTile"] = [str(show_tile).upper()] + update["showTile"] = str(show_tile).upper() try: ldap.update(f"cn={permission},ou=permission", update) diff --git a/src/settings.py b/src/settings.py index 4f42183be8..c07b096299 100644 --- a/src/settings.py +++ b/src/settings.py @@ -240,7 +240,7 @@ def _apply(self): ldap = _get_ldap_interface() ldap.update( "cn=admins,ou=sudo", - {"sudoOption": ["!authenticate"] if passwordless_sudo else []}, + {"sudoOption": "!authenticate" if passwordless_sudo else set()}, ) super()._apply() diff --git a/src/user.py b/src/user.py index 606e8abd54..2a5872773f 100644 --- a/src/user.py +++ b/src/user.py @@ -293,9 +293,9 @@ def user_create( # Create group for user and add to group 'all_users' user_group_create(groupname=username, gid=uid, primary_group=True, sync_perm=False) - user_group_update(groupname="all_users", add=username, force=True, sync_perm=True) if admin: - user_group_update(groupname="admins", add=username, sync_perm=True) + user_group_update(groupname="admins", add=username, sync_perm=False) + user_group_update(groupname="all_users", add=username, force=True, sync_perm=True) # Trigger post_user_create hooks env_dict = { @@ -416,23 +416,23 @@ def user_update( # Get modifications from arguments new_attr_dict = {} if firstname: - new_attr_dict["givenName"] = [firstname] # TODO: Validate - new_attr_dict["cn"] = new_attr_dict["displayName"] = [ - (firstname + " " + user["sn"][0]).strip() - ] + new_attr_dict["givenName"] = firstname # TODO: Validate + new_attr_dict["cn"] = new_attr_dict["displayName"] = ( + firstname + " " + user["sn"][0] + ).strip() env_dict["YNH_USER_FIRSTNAME"] = firstname if lastname: - new_attr_dict["sn"] = [lastname] # TODO: Validate - new_attr_dict["cn"] = new_attr_dict["displayName"] = [ - (user["givenName"][0] + " " + lastname).strip() - ] + new_attr_dict["sn"] = lastname # TODO: Validate + new_attr_dict["cn"] = new_attr_dict["displayName"] = ( + user["givenName"][0] + " " + lastname + ).strip() env_dict["YNH_USER_LASTNAME"] = lastname if lastname and firstname: - new_attr_dict["cn"] = new_attr_dict["displayName"] = [ - (firstname + " " + lastname).strip() - ] + new_attr_dict["cn"] = new_attr_dict["displayName"] = ( + firstname + " " + lastname + ).strip() # change_password is None if user_update is not called to change the password if change_password is not None and change_password != "": @@ -451,13 +451,14 @@ def user_update( "admin" if is_admin else "user", change_password ) - new_attr_dict["userPassword"] = [_hash_user_password(change_password)] + new_attr_dict["userPassword"] = _hash_user_password(change_password) env_dict["YNH_USER_PASSWORD"] = change_password if mail: # If the requested mail address is already as main address or as an alias by this user if mail in user["mail"]: - user["mail"].remove(mail) + if mail != user["mail"][0]: + user["mail"].remove(mail) # Othewise, check that this mail address is not already used by this user else: try: @@ -483,7 +484,7 @@ def user_update( # (c.f. similar stuff as before) if mail in user["mail"]: - user["mail"].remove(mail) + continue else: try: ldap.validate_uniqueness({"mail": mail}) @@ -512,33 +513,27 @@ def user_update( if add_mailforward: if not isinstance(add_mailforward, list): add_mailforward = [add_mailforward] - for mail in add_mailforward: - if mail in user["maildrop"][1:]: - continue - user["maildrop"].append(mail) - new_attr_dict["maildrop"] = user["maildrop"] + new_attr_dict["maildrop"] = set(user["maildrop"]) + set(add_mailforward) if remove_mailforward: if not isinstance(remove_mailforward, list): remove_mailforward = [remove_mailforward] - for mail in remove_mailforward: - if len(user["maildrop"]) > 1 and mail in user["maildrop"][1:]: - user["maildrop"].remove(mail) - else: - raise YunohostValidationError("mail_forward_remove_failed", mail=mail) - new_attr_dict["maildrop"] = user["maildrop"] + new_attr_dict["maildrop"] = set(user["maildrop"]) - set(remove_mailforward) + + if len(user["maildrop"]) - len(remove_mailforward) != len(new_attr_dict["maildrop"]): + raise YunohostValidationError("mail_forward_remove_failed", mail=mail) if "maildrop" in new_attr_dict: env_dict["YNH_USER_MAILFORWARDS"] = ",".join(new_attr_dict["maildrop"]) if mailbox_quota is not None: - new_attr_dict["mailuserquota"] = [mailbox_quota] + new_attr_dict["mailuserquota"] = mailbox_quota env_dict["YNH_USER_MAILQUOTA"] = mailbox_quota if loginShell is not None: if not shellexists(loginShell) or loginShell not in list_shells(): raise YunohostValidationError("invalid_shell", shell=loginShell) - new_attr_dict["loginShell"] = [loginShell] + new_attr_dict["loginShell"] = loginShell env_dict["YNH_USER_LOGINSHELL"] = loginShell if not from_import: @@ -601,7 +596,8 @@ def user_info(username): result_dict["mail-aliases"] = user["mail"][1:] if len(user["maildrop"]) > 1: - result_dict["mail-forward"] = user["maildrop"][1:] + user["maildrop"].remove(username) + result_dict["mail-forward"] = user["maildrop"] if "mailuserquota" in user: userquota = user["mailuserquota"][0] @@ -1275,14 +1271,10 @@ def user_group_update( logger.info(m18n.n("group_update_aliases", group=groupname)) new_attr_dict["mail"] = set(new_group_mail) - if new_attr_dict["mail"] and "mailGroup" not in group["objectClass"]: - new_attr_dict["objectClass"] = group["objectClass"] + ["mailGroup"] - if not new_attr_dict["mail"] and "mailGroup" in group["objectClass"]: - new_attr_dict["objectClass"] = [ - c - for c in group["objectClass"] - if c != "mailGroup" and c != "mailAccount" - ] + if new_attr_dict["mail"]: + new_attr_dict["objectClass"] = set(group["objectClass"]) + {"mailGroup"} + else: + new_attr_dict["objectClass"] = set(group["objectClass"]) - {"mailGroup", "mailAccount"} if new_attr_dict: if not from_import: diff --git a/src/utils/ldap.py b/src/utils/ldap.py index 98ab356cc7..7166a3744e 100644 --- a/src/utils/ldap.py +++ b/src/utils/ldap.py @@ -68,6 +68,50 @@ def _destroy_ldap_interface(): atexit.register(_destroy_ldap_interface) +def modifyModlist_finegrained(old_entry: dict, new_entry: dict) -> list: + """ + Prepare an optimized modification list to give to ldap.modify_ext() + """ + ldif = [] + for attribute, value in new_entry.items(): + if not isinstance(value, (set, list)): + value = {value} + old_value = old_entry.get(attribute, set()) + if not isinstance(old_value, (set, list)): + old_value = {old_value} + if value == set(old_value): + continue + + if not old_value: + ldif.append((ldap.MOD_ADD, attribute, list(value))) + # Add or/and delete only needed values with unordered set + elif isinstance(value, set): + values_to_del = set(old_value) - value + if values_to_del == set(old_value): + ldif.append((ldap.MOD_REPLACE, attribute, list(value))) + continue + elif values_to_del: + ldif.append((ldap.MOD_DELETE, attribute, list(values_to_del))) + + values_to_add = value - set(old_value) + if values_to_add: + ldif.append((ldap.MOD_ADD, attribute, list(values_to_add))) + + # Add or/and delete only needed values with ordered list + else: + for i, v in enumerate(value): + if i >= len(old_value) or old_value[i] != v: + break + if i == 0: + ldif.append((ldap.MOD_REPLACE, attribute, value)) + else: + if old_value[i:]: + ldif.append((ldap.MOD_DELETE, attribute, old_value[i:])) + if value[i:]: + ldif.append((ldap.MOD_ADD, attribute, value[i:])) + + return ldif + class LDAPInterface: def __init__(self): @@ -242,10 +286,19 @@ def update(self, rdn, attr_dict, new_rdn=False): """ dn = rdn + "," + self.basedn - actual_entry = self.search(rdn, attrs=None) - ldif = modlist.modifyModlist(actual_entry[0], attr_dict, ignore_oldexistent=1) + current_entry = self.search(rdn, attrs=None) + + + # Previously, we used modifyModlist, which directly uses the lib system libldap + # supplied with openldap. Unfortunately, the output of this command was not + # optimal with attributes containing lists (complete deletion then complete + # rewriting of the list). In view of the major performance problems associated + # with our inherited permissions system, we decided to rewrite this part to + # optimize the output. + # ldif = modlist.modifyModlist(current_entry[0], attr_dict, ignore_oldexistent=1) + ldif = modifyModlist_finegrained(current_entry[0], attr_dict) - if ldif == []: + if not ldif: logger.debug("Nothing to update in LDAP") return True @@ -255,12 +308,13 @@ def update(self, rdn, attr_dict, new_rdn=False): new_base = dn.split(",", 1)[1] dn = new_rdn + "," + new_base - for i, (a, k, vs) in enumerate(ldif): - if isinstance(vs, list): - vs = [v.encode("utf-8") for v in vs] - elif isinstance(vs, str): - vs = [vs.encode("utf-8")] - ldif[i] = (a, k, vs) + # mod_op : 0 ADD, 1 DELETE, 2 REPLACE + for i, (mod_op, attribute, values) in enumerate(ldif): + if isinstance(values, list): + values = [v.encode("utf-8") for v in values] + elif isinstance(values, str): + values = [values.encode("utf-8")] + ldif[i] = (mod_op, attribute, values) self.con.modify_ext_s(dn, ldif) except Exception as e: