Fix modify samaccountname (#969)

Author: Naksen

.kerberos/config_server.py

@@ -188,6 +188,14 @@ async def force_pw_principal(self, name: str, **dbargs) -> None:
         :param str name: principal
         """
 
+    @abstractmethod
+    async def rename_princ(self, name: str, new_name: str) -> None:
+        """Rename principal.
+
+        :param str name: original name
+        :param str new_name: new name
+        """
+
     @abstractmethod
     async def modify_principal(
         self,
@@ -272,6 +280,19 @@ async def add_princ(
                 partial(princ.modify, attributes=128),
             )
 
+    async def rename_princ(self, name: str, new_name: str) -> None:
+        """Rename principal.
+
+        :param str name: original name
+        :param str new_name: new name
+        """
+        await self.loop.run_in_executor(
+            self.pool,
+            self.client.rename_principal,
+            name,
+            new_name,
+        )
+
     async def _get_raw_principal(self, name: str) -> PrincipalProtocol:
         principal = await self.loop.run_in_executor(
             self.pool,
@@ -640,6 +661,25 @@ async def create_or_update_princ_password(
     await kadmin.create_or_update_princ_pw(name, password)
 
 
+@principal_router.put(
+    "/rename",
+    status_code=status.HTTP_202_ACCEPTED,
+    response_class=Response,
+)
+async def rename_princ(
+    kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)],
+    name: Annotated[str, Body()],
+    new_name: Annotated[str, Body()],
+) -> None:
+    """Rename principal.
+
+    :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract
+    :param Annotated[str, Body name: principal name
+    :param Annotated[str, Body new_name: principal new name
+    """
+    await kadmin.rename_princ(name, new_name)
+
+
 @principal_router.put(
     "/modify",
     status_code=status.HTTP_202_ACCEPTED,

app/ldap_protocol/kerberos/base.py

@@ -188,6 +188,13 @@ async def modify_princ(
         password: str | None = None,
     ) -> None: ...
 
+    @abstractmethod
+    async def rename_princ(
+        self,
+        name: str,
+        new_name: str,
+    ) -> None: ...
+
     @backoff.on_exception(
         backoff.constant,
         (

app/ldap_protocol/kerberos/client.py

@@ -103,9 +103,9 @@ async def modify_princ(
     ) -> None:
         """Rename request."""
         response = await self.client.put(
-            "principal",
+            "principal/modify",
             json={
-                "name": name,
+                "principal_name": name,
                 "new_name": new_name,
                 "algorithms": algorithms,
                 "password": password,
@@ -114,6 +114,20 @@ async def modify_princ(
         if response.status_code != 202:
             raise krb_exc.KRBAPIModifyPrincipalError(response.text)
 
+    @logger_wraps()
+    async def rename_princ(
+        self,
+        name: str,
+        new_name: str,
+    ) -> None:
+        """Rename request."""
+        response = await self.client.put(
+            "principal/rename",
+            json={"name": name, "new_name": new_name},
+        )
+        if response.status_code != 202:
+            raise krb_exc.KRBAPIModifyPrincipalError(response.text)
+
     @logger_wraps()
     async def ktadd(
         self,

app/ldap_protocol/kerberos/stub.py

@@ -54,6 +54,13 @@ async def modify_princ(
         password: str | None = None,
     ) -> None: ...
 
+    @logger_wraps(is_stub=True)
+    async def rename_princ(
+        self,
+        name: str,
+        new_name: str,
+    ) -> None: ...
+
     @logger_wraps(is_stub=True)
     async def ktadd(self, names: list[str], is_rand_key: bool) -> NoReturn:  # noqa: ARG002
         raise KRBAPIPrincipalNotFoundError

app/ldap_protocol/ldap_requests/modify.py

@@ -216,7 +216,10 @@ async def handle(
             )
             return
 
-        if directory.rdname in names:
+        if (
+            directory.rdname != "krbprincipalname"
+            and directory.rdname in names
+        ):
             yield ModifyResponse(result_code=LDAPCodes.NOT_ALLOWED_ON_RDN)
             return
 
@@ -935,11 +938,9 @@ async def _add(  # noqa: C901
                         new_user_principal_name = f"{new_sam_account_name}@{base_dir.name}"  # noqa: E501  # fmt: skip
 
                     if directory.user.sam_account_name != new_sam_account_name:
-                        await kadmin.modify_princ(
+                        await kadmin.rename_princ(
                             directory.user.sam_account_name,
                             new_sam_account_name,
-                            algorithms=None,
-                            password=None,
                         )
 
                         directory.user.user_principal_name = new_user_principal_name  # noqa: E501  # fmt: skip
@@ -1043,17 +1044,13 @@ async def _modify_computer_samaccountname(
             raise ModifyForbiddenError("Old sAMAccountName value not found.")
 
         if old_sam_account_name != new_sam_account_name:
-            await kadmin.modify_princ(
+            await kadmin.rename_princ(
                 f"host/{old_sam_account_name}",
                 f"host/{new_sam_account_name}",
-                algorithms=None,
-                password=None,
             )
-            await kadmin.modify_princ(
+            await kadmin.rename_princ(
                 f"host/{old_sam_account_name}.{base_dir.name}",
                 f"host/{new_sam_account_name}.{base_dir.name}",
-                algorithms=None,
-                password=None,
             )
 
     async def _get_base_dir(

tests/conftest.py

@@ -195,6 +195,7 @@ async def get_kadmin(self) -> AsyncIterator[AsyncMock]:
         kadmin.add_principal = AsyncMock()
         kadmin.del_principal = AsyncMock()
         kadmin.modify_princ = AsyncMock()
+        kadmin.rename_princ = AsyncMock()
         kadmin.create_or_update_principal_pw = AsyncMock()
         kadmin.change_principal_password = AsyncMock()
         kadmin.lock_principal = AsyncMock()

tests/test_api/test_main/test_router/test_modify.py

@@ -101,7 +101,7 @@ async def test_api_correct_modify_user_samaccountname(
     data = response.json()
     assert isinstance(data, dict)
     assert data.get("resultCode") == LDAPCodes.SUCCESS
-    assert kadmin.modify_princ.call_args.args == ("new_user", "NEW user name")  # type: ignore
+    assert kadmin.rename_princ.call_args.args == ("new_user", "NEW user name")  # type: ignore
 
     response = await http_client.post(
         "entry/search",
@@ -160,7 +160,7 @@ async def test_api_correct_modify_user_userprincipalname(
     data = response.json()
     assert isinstance(data, dict)
     assert data.get("resultCode") == LDAPCodes.SUCCESS
-    assert kadmin.modify_princ.call_args.args == ("new_user", "newbiguser")  # type: ignore
+    assert kadmin.rename_princ.call_args.args == ("new_user", "newbiguser")  # type: ignore
 
     response = await http_client.post(
         "entry/search",
@@ -219,12 +219,12 @@ async def test_api_correct_modify_computer_samaccountname_replace(
 
     assert isinstance(data, dict)
     assert data.get("resultCode") == LDAPCodes.SUCCESS
-    assert kadmin.modify_princ.call_count == 2  # type: ignore
-    assert kadmin.modify_princ.call_args_list[0].args == (  # type: ignore
+    assert kadmin.rename_princ.call_count == 2  # type: ignore
+    assert kadmin.rename_princ.call_args_list[0].args == (  # type: ignore
         "host/mycomputer",
         "host/maincomputer",
     )
-    assert kadmin.modify_princ.call_args_list[1].args == (  # type: ignore
+    assert kadmin.rename_princ.call_args_list[1].args == (  # type: ignore
         "host/mycomputer.md.test",
         "host/maincomputer.md.test",
     )