From e78e1e7265281cb37610e5f51c519b5f60b7f31b Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Fri, 16 Jan 2026 16:52:52 +0300 Subject: [PATCH 1/7] Refactor: streamline attribute deletion and user creation in ModifyRequest class --- app/ldap_protocol/ldap_requests/modify.py | 43 +++++++++-------------- interface | 2 +- 2 files changed, 17 insertions(+), 28 deletions(-) diff --git a/app/ldap_protocol/ldap_requests/modify.py b/app/ldap_protocol/ldap_requests/modify.py index 1d79fcfd6..ec711d3cb 100644 --- a/app/ldap_protocol/ldap_requests/modify.py +++ b/app/ldap_protocol/ldap_requests/modify.py @@ -275,9 +275,7 @@ async def handle( await self._add(*add_args) await ctx.session.flush() - await ctx.session.execute( - update(Directory).filter_by(id=directory.id), - ) + except MODIFY_EXCEPTION_STACK as err: await ctx.session.rollback() result_code, message = self._match_bad_response(err) @@ -857,17 +855,12 @@ async def _add( # noqa: C901 await session.execute( delete(Attribute) - .filter_by( - name="nsAccountLock", - directory=directory, - ), - ) # fmt: skip - - await session.execute( - delete(Attribute) - .filter_by( - name="shadowExpire", - directory=directory, + .where( + or_( + qa(Attribute.name) == "nsAccountLock", + qa(Attribute.name) == "shadowExpire", + ), + qa(Attribute.directory) == directory, ), ) # fmt: skip @@ -900,18 +893,21 @@ async def _add( # noqa: C901 sam_account_name = create_user_name(directory.id) user_principal_name = f"{sam_account_name}@{base_dn.name}" - user = User( - sam_account_name=sam_account_name, - user_principal_name=user_principal_name, - directory_id=directory.id, - ) + if directory.object_class in ("user", "person"): + user = User( + sam_account_name=sam_account_name, + user_principal_name=user_principal_name, + directory_id=directory.id, + ) + session.add(user) + uac_attr = Attribute( name="userAccountControl", value=str(UserAccountControlFlag.NORMAL_ACCOUNT), directory_id=directory.id, ) - session.add_all([user, uac_attr]) + session.add(uac_attr) await session.flush() await session.refresh(directory) @@ -926,13 +922,6 @@ async def _add( # noqa: C901 .values({name: new_value}), ) - elif name in Group.search_fields and directory.group: - await session.execute( - update(Group) - .filter_by(directory=directory) - .values({name: value}), - ) - elif name in ("userpassword", "unicodepwd") and directory.user: if not settings.USE_CORE_TLS: raise PermissionError("TLS required") diff --git a/interface b/interface index 95ed5e191..f31962020 160000 --- a/interface +++ b/interface @@ -1 +1 @@ -Subproject commit 95ed5e191cdafa07b1dfac96a1659926679ead97 +Subproject commit f31962020a6689e6a4c61fb3349db5b5c7895f92 From bf6095bca2fa211e51503665bf2c89c74e5a6043 Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Fri, 16 Jan 2026 18:08:41 +0300 Subject: [PATCH 2/7] Refactor: simplify user attribute updates in ModifyRequest class by removing unnecessary user creation logic --- app/ldap_protocol/ldap_requests/modify.py | 52 +++++------------------ 1 file changed, 11 insertions(+), 41 deletions(-) diff --git a/app/ldap_protocol/ldap_requests/modify.py b/app/ldap_protocol/ldap_requests/modify.py index ec711d3cb..2cabe8bd2 100644 --- a/app/ldap_protocol/ldap_requests/modify.py +++ b/app/ldap_protocol/ldap_requests/modify.py @@ -37,17 +37,11 @@ from ldap_protocol.policies.password import PasswordPolicyUseCases from ldap_protocol.session_storage import SessionStorage from ldap_protocol.utils.cte import check_root_group_membership_intersection -from ldap_protocol.utils.helpers import ( - create_user_name, - ft_to_dt, - is_dn_in_base_directory, - validate_entry, -) +from ldap_protocol.utils.helpers import ft_to_dt, validate_entry from ldap_protocol.utils.queries import ( add_lock_and_expire_attributes, clear_group_membership, extend_group_membership, - get_base_directories, get_directories, get_directory_by_rid, get_filter_from_path, @@ -884,44 +878,20 @@ async def _add( # noqa: C901 ) elif name in User.search_fields: - if not directory.user: - path_dn = directory.path_dn - for base_directory in await get_base_directories(session): - if is_dn_in_base_directory(base_directory, path_dn): - base_dn = base_directory - break - - sam_account_name = create_user_name(directory.id) - user_principal_name = f"{sam_account_name}@{base_dn.name}" - if directory.object_class in ("user", "person"): - user = User( - sam_account_name=sam_account_name, - user_principal_name=user_principal_name, - directory_id=directory.id, + if directory.user: + if name == "accountexpires": + new_value = ( + ft_to_dt(int(value)) if value != "0" else None ) - session.add(user) + else: + new_value = value # type: ignore - uac_attr = Attribute( - name="userAccountControl", - value=str(UserAccountControlFlag.NORMAL_ACCOUNT), - directory_id=directory.id, + await session.execute( + update(User) + .filter_by(directory=directory) + .values({name: new_value}), ) - session.add(uac_attr) - await session.flush() - await session.refresh(directory) - - if name == "accountexpires": - new_value = ft_to_dt(int(value)) if value != "0" else None - else: - new_value = value # type: ignore - - await session.execute( - update(User) - .filter_by(directory=directory) - .values({name: new_value}), - ) - elif name in ("userpassword", "unicodepwd") and directory.user: if not settings.USE_CORE_TLS: raise PermissionError("TLS required") From 324950dbaffbe8951f1dff169dbf4be361ad6aa8 Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Wed, 21 Jan 2026 10:27:17 +0300 Subject: [PATCH 3/7] Refactor: enhance user creation logic in ModifyRequest class to ensure proper handling of directory users and account expiration updates --- app/ldap_protocol/ldap_requests/modify.py | 47 ++++++++++++++++------- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/app/ldap_protocol/ldap_requests/modify.py b/app/ldap_protocol/ldap_requests/modify.py index 2cabe8bd2..30687cd77 100644 --- a/app/ldap_protocol/ldap_requests/modify.py +++ b/app/ldap_protocol/ldap_requests/modify.py @@ -37,11 +37,17 @@ from ldap_protocol.policies.password import PasswordPolicyUseCases from ldap_protocol.session_storage import SessionStorage from ldap_protocol.utils.cte import check_root_group_membership_intersection -from ldap_protocol.utils.helpers import ft_to_dt, validate_entry +from ldap_protocol.utils.helpers import ( + create_user_name, + ft_to_dt, + is_dn_in_base_directory, + validate_entry, +) from ldap_protocol.utils.queries import ( add_lock_and_expire_attributes, clear_group_membership, extend_group_membership, + get_base_directories, get_directories, get_directory_by_rid, get_filter_from_path, @@ -878,20 +884,35 @@ async def _add( # noqa: C901 ) elif name in User.search_fields: - if directory.user: - if name == "accountexpires": - new_value = ( - ft_to_dt(int(value)) if value != "0" else None - ) - else: - new_value = value # type: ignore - - await session.execute( - update(User) - .filter_by(directory=directory) - .values({name: new_value}), + if not directory.user: + path_dn = directory.path_dn + for base_directory in await get_base_directories(session): + if is_dn_in_base_directory(base_directory, path_dn): + base_dn = base_directory + break + + sam_account_name = create_user_name(directory.id) + user_principal_name = f"{sam_account_name}@{base_dn.name}" + user = User( + sam_account_name=sam_account_name, + user_principal_name=user_principal_name, + directory_id=directory.id, ) + session.add(user) + await session.flush() + await session.refresh(directory) + + if name == "accountexpires": + new_value = ft_to_dt(int(value)) if value != "0" else None + else: + new_value = value # type: ignore + + await session.execute( + update(User) + .filter_by(directory=directory) + .values({name: new_value}), + ) elif name in ("userpassword", "unicodepwd") and directory.user: if not settings.USE_CORE_TLS: raise PermissionError("TLS required") From 576adfbca2863e9998f1c616f23057fd800d2c90 Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Wed, 21 Jan 2026 12:09:31 +0300 Subject: [PATCH 4/7] Refactor: remove user creation logic from ModifyRequest class to streamline account modification process --- app/ldap_protocol/ldap_requests/modify.py | 14 -------------- tests/test_api/test_main/conftest.py | 21 +++++++++++++-------- 2 files changed, 13 insertions(+), 22 deletions(-) diff --git a/app/ldap_protocol/ldap_requests/modify.py b/app/ldap_protocol/ldap_requests/modify.py index 30687cd77..cc14cda32 100644 --- a/app/ldap_protocol/ldap_requests/modify.py +++ b/app/ldap_protocol/ldap_requests/modify.py @@ -38,7 +38,6 @@ from ldap_protocol.session_storage import SessionStorage from ldap_protocol.utils.cte import check_root_group_membership_intersection from ldap_protocol.utils.helpers import ( - create_user_name, ft_to_dt, is_dn_in_base_directory, validate_entry, @@ -888,21 +887,8 @@ async def _add( # noqa: C901 path_dn = directory.path_dn for base_directory in await get_base_directories(session): if is_dn_in_base_directory(base_directory, path_dn): - base_dn = base_directory break - sam_account_name = create_user_name(directory.id) - user_principal_name = f"{sam_account_name}@{base_dn.name}" - user = User( - sam_account_name=sam_account_name, - user_principal_name=user_principal_name, - directory_id=directory.id, - ) - session.add(user) - - await session.flush() - await session.refresh(directory) - if name == "accountexpires": new_value = ft_to_dt(int(value)) if value != "0" else None else: diff --git a/tests/test_api/test_main/conftest.py b/tests/test_api/test_main/conftest.py index bdf9e3f4e..8f1b58dea 100644 --- a/tests/test_api/test_main/conftest.py +++ b/tests/test_api/test_main/conftest.py @@ -47,9 +47,21 @@ async def adding_test_user( "type": "testing_attr", "vals": ["test"], }, + { + "type": "sAMAccountName", + "vals": ["test"], + }, { "type": "objectClass", - "vals": ["organization", "top", "user"], + "vals": [ + "top", + "user", + "person", + "organizationalPerson", + "posixAccount", + "shadowAccount", + "inetOrgPerson", + ], }, ], }, @@ -62,13 +74,6 @@ async def adding_test_user( json={ "object": test_user_dn, "changes": [ - { - "operation": Operation.ADD, - "modification": { - "type": "sAMAccountName", - "vals": ["Test"], - }, - }, { "operation": Operation.ADD, "modification": { From 847725eb393bf94b19b60f7722f6e80e472bd940 Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Wed, 21 Jan 2026 13:11:00 +0300 Subject: [PATCH 5/7] Add user creation logic in test_ldap_modify_with_ap to ensure directory users are properly initialized --- tests/test_ldap/test_util/test_modify.py | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_ldap/test_util/test_modify.py b/tests/test_ldap/test_util/test_modify.py index c141914a4..f66dda70c 100644 --- a/tests/test_ldap/test_util/test_modify.py +++ b/tests/test_ldap/test_util/test_modify.py @@ -17,7 +17,7 @@ from sqlalchemy.orm import joinedload, selectinload, subqueryload from config import Settings -from entities import Directory, Group +from entities import Directory, Group, User from enums import AceType, RoleScope from ldap_protocol.kerberos.base import AbstractKadmin from ldap_protocol.ldap_codes import LDAPCodes @@ -670,6 +670,18 @@ async def test_ldap_modify_with_ap( directory = await session.scalar(query) + if directory and not directory.user: + user = User( + sam_account_name="users_container", + user_principal_name="users_container@md.test", + mail="users@md.test", + display_name="Users Container", + directory_id=directory.id, + ) + session.add(user) + await session.commit() + await session.refresh(directory) + async def try_modify() -> int: with tempfile.NamedTemporaryFile("w") as file: file.write( From 0f97b85c9267b193ddd48e73aead5cb44b921406 Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Wed, 21 Jan 2026 14:23:08 +0300 Subject: [PATCH 6/7] Refactor: remove unused helper functions and streamline user creation in ModifyRequest class --- app/ldap_protocol/ldap_requests/modify.py | 13 +------------ tests/test_ldap/test_util/test_modify.py | 6 +++--- 2 files changed, 4 insertions(+), 15 deletions(-) diff --git a/app/ldap_protocol/ldap_requests/modify.py b/app/ldap_protocol/ldap_requests/modify.py index cc14cda32..676550e3e 100644 --- a/app/ldap_protocol/ldap_requests/modify.py +++ b/app/ldap_protocol/ldap_requests/modify.py @@ -37,16 +37,11 @@ from ldap_protocol.policies.password import PasswordPolicyUseCases from ldap_protocol.session_storage import SessionStorage from ldap_protocol.utils.cte import check_root_group_membership_intersection -from ldap_protocol.utils.helpers import ( - ft_to_dt, - is_dn_in_base_directory, - validate_entry, -) +from ldap_protocol.utils.helpers import ft_to_dt, validate_entry from ldap_protocol.utils.queries import ( add_lock_and_expire_attributes, clear_group_membership, extend_group_membership, - get_base_directories, get_directories, get_directory_by_rid, get_filter_from_path, @@ -883,12 +878,6 @@ async def _add( # noqa: C901 ) elif name in User.search_fields: - if not directory.user: - path_dn = directory.path_dn - for base_directory in await get_base_directories(session): - if is_dn_in_base_directory(base_directory, path_dn): - break - if name == "accountexpires": new_value = ft_to_dt(int(value)) if value != "0" else None else: diff --git a/tests/test_ldap/test_util/test_modify.py b/tests/test_ldap/test_util/test_modify.py index f66dda70c..83ce08bb8 100644 --- a/tests/test_ldap/test_util/test_modify.py +++ b/tests/test_ldap/test_util/test_modify.py @@ -672,10 +672,10 @@ async def test_ldap_modify_with_ap( if directory and not directory.user: user = User( - sam_account_name="users_container", - user_principal_name="users_container@md.test", + sam_account_name="test", + user_principal_name="test@md.test", mail="users@md.test", - display_name="Users Container", + display_name="test", directory_id=directory.id, ) session.add(user) From 240de5d126453d8c04feecf9f74b637149a6d7bd Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Wed, 21 Jan 2026 14:29:46 +0300 Subject: [PATCH 7/7] Refactor: remove user creation assertions in test_ldap_modify_with_ap to simplify test logic --- tests/test_ldap/test_util/test_modify.py | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/tests/test_ldap/test_util/test_modify.py b/tests/test_ldap/test_util/test_modify.py index 83ce08bb8..02b174b4b 100644 --- a/tests/test_ldap/test_util/test_modify.py +++ b/tests/test_ldap/test_util/test_modify.py @@ -17,7 +17,7 @@ from sqlalchemy.orm import joinedload, selectinload, subqueryload from config import Settings -from entities import Directory, Group, User +from entities import Directory, Group from enums import AceType, RoleScope from ldap_protocol.kerberos.base import AbstractKadmin from ldap_protocol.ldap_codes import LDAPCodes @@ -670,18 +670,6 @@ async def test_ldap_modify_with_ap( directory = await session.scalar(query) - if directory and not directory.user: - user = User( - sam_account_name="test", - user_principal_name="test@md.test", - mail="users@md.test", - display_name="test", - directory_id=directory.id, - ) - session.add(user) - await session.commit() - await session.refresh(directory) - async def try_modify() -> int: with tempfile.NamedTemporaryFile("w") as file: file.write( @@ -789,8 +777,6 @@ async def try_modify() -> int: ] assert attributes["jpegPhoto"] == ["modme.jpeg"] - assert directory.user - assert directory.user.mail == "modme@student.of.life.edu" assert "posixEmail" not in attributes