From 284429d41c4e82ef55db258e89b79e58b184c98f Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Tue, 3 Feb 2026 13:05:11 +0300 Subject: [PATCH 1/7] Add: introduce new schemas for principal and ktadd requests, refactor related API endpoints to utilize these schemas for better data handling and validation. --- .kerberos/config_server.py | 41 +++++++++++++++++++------ app/api/main/adapters/kerberos.py | 40 ++++++++++++++---------- app/api/main/krb5_router.py | 38 +++++++++-------------- app/api/main/schema.py | 24 +++++++++++++++ app/ldap_protocol/kerberos/base.py | 19 +++++++++--- app/ldap_protocol/kerberos/client.py | 30 +++++++++++++++--- app/ldap_protocol/kerberos/service.py | 29 ++++++++++++++--- app/ldap_protocol/kerberos/stub.py | 15 ++++++--- app/ldap_protocol/ldap_requests/bind.py | 2 +- interface | 2 +- 10 files changed, 170 insertions(+), 70 deletions(-) diff --git a/.kerberos/config_server.py b/.kerberos/config_server.py index 2806c9b86..6657ac6cd 100644 --- a/.kerberos/config_server.py +++ b/.kerberos/config_server.py @@ -61,6 +61,21 @@ class ConfigSchema(BaseModel): stash_password: str +class PrincipalCreateSchema(BaseModel): + """Schema for principal creation.""" + + name: str + password: str | None = None + algorithms: list[str] | None = None + + +class KtaddSchema(BaseModel): + """Schema for ktadd request.""" + + names: list[str] + is_rand_key: bool = False + + class Principal(BaseModel): """Principal kadmin object.""" @@ -300,7 +315,12 @@ async def rename_princ(self, name: str, new_name: str) -> None: new_name, ) - async def ktadd(self, names: list[str], fn: str) -> None: + async def ktadd( + self, + names: list[str], + fn: str, + is_rand_key: bool = False, # noqa: ARG002 + ) -> None: """Create or write to keytab. :param str name: principal @@ -494,16 +514,18 @@ async def reset_setup() -> None: @principal_router.post("", response_class=Response, status_code=201) async def add_princ( kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)], - name: Annotated[str, Body()], - password: Annotated[str | None, Body(embed=True)] = None, + schema: PrincipalCreateSchema, ) -> None: """Add principal. :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract - :param Annotated[str, Body name: principal name - :param Annotated[str, Body password: principal password + :param PrincipalCreateSchema schema: principal data """ - await kadmin.add_princ(name, password) + await kadmin.add_princ( + schema.name, + schema.password, + algorithms=schema.algorithms or {}, + ) @principal_router.get("") @@ -591,16 +613,15 @@ async def rename_princ( @principal_router.post("/ktadd") async def ktadd( kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)], - names: Annotated[list[str], Body()], + schema: KtaddSchema, ) -> FileResponse: """Ktadd principal. :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract - :param Annotated[str, Body name: principal name - :param Annotated[str, Body password: principal password + :param KtaddSchema schema: ktadd request data """ filename = os.path.join(gettempdir(), str(uuid.uuid1())) - await kadmin.ktadd(names, filename) + await kadmin.ktadd(schema.names, filename, is_rand_key=schema.is_rand_key) return FileResponse( filename, diff --git a/app/api/main/adapters/kerberos.py b/app/api/main/adapters/kerberos.py index 1bbe252e2..6124636c4 100644 --- a/app/api/main/adapters/kerberos.py +++ b/app/api/main/adapters/kerberos.py @@ -12,7 +12,12 @@ from starlette.background import BackgroundTask from api.base_adapter import BaseAdapter -from api.main.schema import KerberosSetupRequest +from api.main.schema import ( + KerberosSetupRequest, + KtaddRequest, + PrincipalAddRequest, + PrincipalPutRequest, +) from ldap_protocol.dialogue import LDAPSession, UserSchema from ldap_protocol.kerberos import KerberosState from ldap_protocol.kerberos.service import KerberosService @@ -66,31 +71,29 @@ async def setup_kdc( ) return Response(background=task) - async def add_principal( - self, - primary: str, - instance: str, - ) -> None: + async def add_principal(self, request: PrincipalAddRequest) -> None: """Create principal in Kerberos with given name. :raises HTTPException: on Kerberos errors :return: None """ - return await self._service.add_principal(primary, instance) + return await self._service.add_principal( + request.principal_name, + password=request.password, + algorithms=request.algorithms, + ) - async def rename_principal( - self, - principal_name: str, - principal_new_name: str, - ) -> None: - """Rename principal in Kerberos. + async def rename_principal(self, request: PrincipalPutRequest) -> None: + """Modify principal (rename, password, algorithms). :raises HTTPException: on Kerberos errors :return: None """ return await self._service.rename_principal( - principal_name, - principal_new_name, + principal_name=request.principal_name, + principal_new_name=request.new_principal_name, + algorithms=request.algorithms, + password=request.password, ) async def reset_principal_pw( @@ -121,14 +124,17 @@ async def delete_principal( async def ktadd( self, - names: list[str], + data: KtaddRequest, ) -> StreamingResponse: """Generate keytab and return as streaming response. :raises HTTPException: on Kerberos errors :return: StreamingResponse """ - aiter_bytes, task_struct = await self._service.ktadd(names) + aiter_bytes, task_struct = await self._service.ktadd( + data.names, + is_rand_key=data.is_rand_key, + ) task = BackgroundTask( task_struct.func, *task_struct.args, diff --git a/app/api/main/krb5_router.py b/app/api/main/krb5_router.py index 9ed36515c..8cd32e084 100644 --- a/app/api/main/krb5_router.py +++ b/app/api/main/krb5_router.py @@ -23,7 +23,12 @@ DomainErrorTranslator, ) from api.main.adapters.kerberos import KerberosFastAPIAdapter -from api.main.schema import KerberosSetupRequest +from api.main.schema import ( + KerberosSetupRequest, + KtaddRequest, + PrincipalAddRequest, + PrincipalPutRequest, +) from api.utils import require_master_db from enums import DomainCodes from ldap_protocol.dialogue import LDAPSession @@ -149,15 +154,15 @@ async def setup_kdc( error_map=error_map, ) async def ktadd( - names: Annotated[LIMITED_LIST, Body()], kerberos_adapter: FromDishka[KerberosFastAPIAdapter], + request: KtaddRequest, ) -> StreamingResponse: """Create keytab from kadmin server. :param Annotated[LDAPSession, Depends ldap_session: ldap :return bytes: file """ - return await kerberos_adapter.ktadd(names) + return await kerberos_adapter.ktadd(request) @krb5_router.get( @@ -178,13 +183,12 @@ async def get_krb_status( @krb5_router.post( - "/principal/add", + "/principal", dependencies=[Depends(verify_auth), Depends(require_master_db)], error_map=error_map, ) async def add_principal( - primary: Annotated[LIMITED_STR, Body()], - instance: Annotated[LIMITED_STR, Body()], + request: PrincipalAddRequest, kerberos_adapter: FromDishka[KerberosFastAPIAdapter], ) -> None: """Create principal in kerberos with given name. @@ -194,31 +198,19 @@ async def add_principal( :param Annotated[LDAPSession, Depends ldap_session: ldap :raises HTTPException: on failed kamin request. """ - await kerberos_adapter.add_principal(primary, instance) + await kerberos_adapter.add_principal(request) -@krb5_router.patch( - "/principal/rename", +@krb5_router.put( + "/principal", dependencies=[Depends(verify_auth), Depends(require_master_db)], error_map=error_map, ) async def rename_principal( - principal_name: Annotated[LIMITED_STR, Body()], - principal_new_name: Annotated[LIMITED_STR, Body()], + request: PrincipalPutRequest, kerberos_adapter: FromDishka[KerberosFastAPIAdapter], ) -> None: - """Rename principal in kerberos with given name. - - \f - :param Annotated[str, Body principal_name: upn - :param Annotated[LIMITED_STR, Body principal_new_name: _description_ - :param Annotated[LDAPSession, Depends ldap_session: ldap - :raises HTTPException: on failed kamin request. - """ - await kerberos_adapter.rename_principal( - principal_name, - principal_new_name, - ) + await kerberos_adapter.rename_principal(request) @krb5_router.patch( diff --git a/app/api/main/schema.py b/app/api/main/schema.py index 537b0af7c..96f90d254 100644 --- a/app/api/main/schema.py +++ b/app/api/main/schema.py @@ -70,6 +70,30 @@ class KerberosSetupRequest(BaseModel): stash_password: SecretStr +class PrincipalAddRequest(BaseModel): + """Request schema for POST /principal/add.""" + + principal_name: str + algorithms: list[str] | None = None + password: str | None = None + + +class KtaddRequest(BaseModel): + """Request schema for POST /ktadd.""" + + names: list[str] + is_rand_key: bool = False + + +class PrincipalPutRequest(BaseModel): + """Request schema for PUT /principal (full modify).""" + + principal_name: str + new_principal_name: str + algorithms: list[str] | None = None + password: str | None = None + + class DNSServiceSetupRequest(BaseModel): """DNS setup request schema.""" diff --git a/app/ldap_protocol/kerberos/base.py b/app/ldap_protocol/kerberos/base.py index d70960738..7faf30565 100644 --- a/app/ldap_protocol/kerberos/base.py +++ b/app/ldap_protocol/kerberos/base.py @@ -153,8 +153,9 @@ async def setup( @abstractmethod async def add_principal( self, - name: str, - password: str | None, + principal_name: str, + password: str | None = None, + algorithms: list[str] | None = None, timeout: int | float = 1, ) -> None: ... @@ -179,7 +180,13 @@ async def create_or_update_principal_pw( ) -> None: ... @abstractmethod - async def rename_princ(self, name: str, new_name: str) -> None: ... + async def rename_princ( + self, + name: str, + new_name: str, + algorithms: list[str] | None = None, + password: str | None = None, + ) -> None: ... @backoff.on_exception( backoff.constant, @@ -202,7 +209,11 @@ async def get_status(self, wait_for_positive: bool = False) -> bool: return status @abstractmethod - async def ktadd(self, names: list[str]) -> httpx.Response: ... + async def ktadd( + self, + names: list[str], + is_rand_key: bool, + ) -> httpx.Response: ... @abstractmethod async def lock_principal(self, name: str) -> None: ... diff --git a/app/ldap_protocol/kerberos/client.py b/app/ldap_protocol/kerberos/client.py index 8dfcb8a23..4d35d17c2 100644 --- a/app/ldap_protocol/kerberos/client.py +++ b/app/ldap_protocol/kerberos/client.py @@ -20,12 +20,17 @@ async def add_principal( self, name: str, password: str | None, + algorithms: list[str] | None, timeout: int = 1, ) -> None: """Add request.""" response = await self.client.post( "principal", - json={"name": name, "password": password}, + json={ + "name": name, + "password": password, + "algorithms": algorithms, + }, timeout=timeout, ) @@ -89,17 +94,32 @@ async def create_or_update_principal_pw( raise krb_exc.KRBAPIChangePasswordError(response.text) @logger_wraps() - async def rename_princ(self, name: str, new_name: str) -> None: + async def rename_princ( + self, + name: str, + new_name: str, + algorithms: list[str] | None, + password: str | None, + ) -> None: """Rename request.""" response = await self.client.put( "principal", - json={"name": name, "new_name": new_name}, + json={ + "name": name, + "new_name": new_name, + "algorithms": algorithms, + "password": password, + }, ) if response.status_code != 202: raise krb_exc.KRBAPIRenamePrincipalError(response.text) @logger_wraps() - async def ktadd(self, names: list[str]) -> httpx.Response: + async def ktadd( + self, + names: list[str], + is_rand_key: bool, + ) -> httpx.Response: """Ktadd build request for stream and return response. :param list[str] names: principals @@ -108,7 +128,7 @@ async def ktadd(self, names: list[str]) -> httpx.Response: request = self.client.build_request( "POST", "/principal/ktadd", - json=names, + json={"names": names, "is_rand_key": is_rand_key}, ) response = await self.client.send(request, stream=True) diff --git a/app/ldap_protocol/kerberos/service.py b/app/ldap_protocol/kerberos/service.py index 985d8cdc1..8d3ad1fc8 100644 --- a/app/ldap_protocol/kerberos/service.py +++ b/app/ldap_protocol/kerberos/service.py @@ -358,7 +358,12 @@ async def _schedule_principal_task( ) return TaskStruct(func=func, args=args) - async def add_principal(self, primary: str, instance: str) -> None: + async def add_principal( + self, + principal_name: str, + password: str | None, + algorithms: list[str] | None, + ) -> None: """Create principal in Kerberos with given name. :param str primary: Principal primary name. @@ -367,8 +372,11 @@ async def add_principal(self, primary: str, instance: str) -> None: :return None: None. """ try: - principal_name = f"{primary}/{instance}" - await self._kadmin.add_principal(principal_name, None) + await self._kadmin.add_principal( + principal_name, + password, + algorithms, + ) except KRBAPIAddPrincipalError as exc: raise KerberosDependencyError( f"Error adding principal: {exc}", @@ -378,16 +386,25 @@ async def rename_principal( self, principal_name: str, principal_new_name: str, + algorithms: list[str] | None, + password: str | None, ) -> None: """Rename principal in Kerberos with given name. :param str principal_name: Current principal name. :param str principal_new_name: New principal name. + :param list[str] | None algorithms: Algorithms. + :param str | None password: Password. :raises KerberosDependencyError: On failed kadmin request. :return None: None. """ try: - await self._kadmin.rename_princ(principal_name, principal_new_name) + await self._kadmin.rename_princ( + principal_name, + principal_new_name, + algorithms, + password, + ) except KRBAPIRenamePrincipalError as exc: raise KerberosDependencyError( f"Error renaming principal: {exc}", @@ -432,15 +449,17 @@ async def delete_principal(self, principal_name: str) -> None: async def ktadd( self, names: list[str], + is_rand_key: bool, ) -> tuple[AsyncIterator[bytes], TaskStruct]: """Generate keytab and return (aiter_bytes, TaskStruct). :param list[str] names: List of principal names. + :param bool is_rand_key: If True, generate random key. :raises KerberosNotFoundError: If principal not found. :return tuple: (aiter_bytes, (func, args, kwargs)). """ try: - response = await self._kadmin.ktadd(names) + response = await self._kadmin.ktadd(names, is_rand_key) except KRBAPIPrincipalNotFoundError: raise KerberosNotFoundError("Principal not found") aiter_bytes = response.aiter_bytes() diff --git a/app/ldap_protocol/kerberos/stub.py b/app/ldap_protocol/kerberos/stub.py index 889583c16..b118b09e8 100644 --- a/app/ldap_protocol/kerberos/stub.py +++ b/app/ldap_protocol/kerberos/stub.py @@ -19,8 +19,9 @@ async def setup(self, *args, **kwargs) -> None: # type: ignore @logger_wraps(is_stub=True) async def add_principal( self, - name: str, - password: str | None, + principal_name: str, + password: str | None = None, + algorithms: list[str] | None = None, timeout: int = 1, ) -> None: ... @@ -45,10 +46,16 @@ async def create_or_update_principal_pw( ) -> None: ... @logger_wraps(is_stub=True) - async def rename_princ(self, name: str, new_name: str) -> None: ... + async def rename_princ( + self, + name: str, + new_name: str, + algorithms: list[str] | None = None, + password: str | None = None, + ) -> None: ... @logger_wraps(is_stub=True) - async def ktadd(self, names: list[str]) -> NoReturn: # noqa: ARG002 + async def ktadd(self, names: list[str], is_rand_key: bool) -> NoReturn: # noqa: ARG002 raise KRBAPIPrincipalNotFoundError @logger_wraps(is_stub=True) diff --git a/app/ldap_protocol/ldap_requests/bind.py b/app/ldap_protocol/ldap_requests/bind.py index 7f4af0e5c..a747c26f6 100644 --- a/app/ldap_protocol/ldap_requests/bind.py +++ b/app/ldap_protocol/ldap_requests/bind.py @@ -213,7 +213,7 @@ async def handle( await ctx.kadmin.add_principal( user.sam_account_name, self.authentication_choice.password.get_secret_value(), - 0.1, + timeout=0.1, ) await ctx.ldap_session.set_user(user) diff --git a/interface b/interface index e1ca5656a..f31962020 160000 --- a/interface +++ b/interface @@ -1 +1 @@ -Subproject commit e1ca5656aeabc20a1862aeaf11ded72feaa97403 +Subproject commit f31962020a6689e6a4c61fb3349db5b5c7895f92 From d4962a4a373ec4edbdd8e47efb91e58265b1730a Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Tue, 3 Feb 2026 14:10:00 +0300 Subject: [PATCH 2/7] Refactor: remove unused schemas for principal creation and ktadd requests, update API endpoints to accept parameters directly for improved clarity and simplicity. --- .kerberos/config_server.py | 30 +++++++--------------------- app/ldap_protocol/kerberos/client.py | 4 ++-- 2 files changed, 9 insertions(+), 25 deletions(-) diff --git a/.kerberos/config_server.py b/.kerberos/config_server.py index 6657ac6cd..abc0d9a51 100644 --- a/.kerberos/config_server.py +++ b/.kerberos/config_server.py @@ -61,21 +61,6 @@ class ConfigSchema(BaseModel): stash_password: str -class PrincipalCreateSchema(BaseModel): - """Schema for principal creation.""" - - name: str - password: str | None = None - algorithms: list[str] | None = None - - -class KtaddSchema(BaseModel): - """Schema for ktadd request.""" - - names: list[str] - is_rand_key: bool = False - - class Principal(BaseModel): """Principal kadmin object.""" @@ -319,7 +304,6 @@ async def ktadd( self, names: list[str], fn: str, - is_rand_key: bool = False, # noqa: ARG002 ) -> None: """Create or write to keytab. @@ -514,17 +498,17 @@ async def reset_setup() -> None: @principal_router.post("", response_class=Response, status_code=201) async def add_princ( kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)], - schema: PrincipalCreateSchema, + name: Annotated[str, Body()], + password: Annotated[str | None, Body(embed=True)] = None, ) -> None: """Add principal. :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract - :param PrincipalCreateSchema schema: principal data + """ await kadmin.add_princ( - schema.name, - schema.password, - algorithms=schema.algorithms or {}, + name, + password, ) @@ -613,7 +597,7 @@ async def rename_princ( @principal_router.post("/ktadd") async def ktadd( kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)], - schema: KtaddSchema, + names: Annotated[list[str], Body()], ) -> FileResponse: """Ktadd principal. @@ -621,7 +605,7 @@ async def ktadd( :param KtaddSchema schema: ktadd request data """ filename = os.path.join(gettempdir(), str(uuid.uuid1())) - await kadmin.ktadd(schema.names, filename, is_rand_key=schema.is_rand_key) + await kadmin.ktadd(names, filename) return FileResponse( filename, diff --git a/app/ldap_protocol/kerberos/client.py b/app/ldap_protocol/kerberos/client.py index 4d35d17c2..b7bacfdb8 100644 --- a/app/ldap_protocol/kerberos/client.py +++ b/app/ldap_protocol/kerberos/client.py @@ -20,8 +20,8 @@ async def add_principal( self, name: str, password: str | None, - algorithms: list[str] | None, - timeout: int = 1, + algorithms: list[str] | None = None, + timeout: int | float = 1, ) -> None: """Add request.""" response = await self.client.post( From 0d54a13c550e6337b7a65d809b796f37cfb4b9b1 Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Tue, 3 Feb 2026 14:12:00 +0300 Subject: [PATCH 3/7] Refactor: streamline ktadd and add_princ methods by removing unnecessary line breaks for improved readability. --- .kerberos/config_server.py | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/.kerberos/config_server.py b/.kerberos/config_server.py index abc0d9a51..1c397105a 100644 --- a/.kerberos/config_server.py +++ b/.kerberos/config_server.py @@ -300,11 +300,7 @@ async def rename_princ(self, name: str, new_name: str) -> None: new_name, ) - async def ktadd( - self, - names: list[str], - fn: str, - ) -> None: + async def ktadd(self, names: list[str], fn: str) -> None: """Create or write to keytab. :param str name: principal @@ -506,10 +502,7 @@ async def add_princ( :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract """ - await kadmin.add_princ( - name, - password, - ) + await kadmin.add_princ(name, password) @principal_router.get("") From d08f6643e556682b94fa7369c383b6efdff239fb Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Mon, 9 Feb 2026 11:51:36 +0300 Subject: [PATCH 4/7] Add: enhance principal management with new request models and update API interactions - Introduced new request models: AddPrincipalRequest, KtaddRequest, and ModifyPrincipalRequest for better structure and validation. - Updated API endpoints to use the new request models, ensuring correct JSON structure for principal creation and keytab addition. - Enhanced error handling and logging for keytab operations. - Refactored existing methods to accommodate new parameters for encryption algorithms and random key generation. --- .kerberos/config_server.py | 279 ++++++++++++++++++++---- app/ldap_protocol/kerberos/base.py | 7 +- app/ldap_protocol/kerberos/utils.py | 6 +- tests/test_api/test_main/test_kadmin.py | 10 +- 4 files changed, 250 insertions(+), 52 deletions(-) diff --git a/.kerberos/config_server.py b/.kerberos/config_server.py index 1c397105a..7b9a3dc43 100644 --- a/.kerberos/config_server.py +++ b/.kerberos/config_server.py @@ -31,7 +31,7 @@ ) from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import FileResponse -from pydantic import BaseModel +from pydantic import BaseModel, Field from starlette.background import BackgroundTask KRB5_CONF_PATH = "/etc/krb5.conf" @@ -79,6 +79,40 @@ class PrincipalNotFoundError(Exception): """Not found error.""" +class AddPrincipalRequest(BaseModel): + """Request model for adding principal.""" + + principal_name: str + algorithms: list[str] | None = Field(default=None) + password: str | None = Field(default=None) + + +class KtaddRequest(BaseModel): + """Request model for ktadd.""" + + names: list[str] + is_rand_key: bool = Field(default=False) + + +class ModifyPrincipalRequest(BaseModel): + """Request model for modifying principal.""" + + principal_name: str + new_principal_name: str | None = Field(default=None) + algorithms: list[str] | None = Field(default=None) + password: str | None = Field(default=None) + + +ALGORITHM_MAP = { + "aes128-cts-hmac-sha1-96:normal": kadmv.EncryptionType.aes128_cts_hmac_sha1_96, # noqa: E501 + "aes256-cts-hmac-sha1-96:normal": kadmv.EncryptionType.aes256_cts_hmac_sha1_96, # noqa: E501 + "des-cbc-md5": kadmv.EncryptionType.des_cbc_md5, + "des-cbc-crc": kadmv.EncryptionType.des_cbc_crc, + "arcfour-hmac": kadmv.EncryptionType.arcfour_hmac, + "arcfour-hmac-md5": kadmv.EncryptionType.arcfour_hmac_md5, +} + + class AbstractKRBManager(ABC): """Kadmin manager.""" @@ -95,12 +129,14 @@ async def add_princ( self, name: str, password: str | None, + algorithms: list[str] | None = None, **dbargs, ) -> None: """Create principal. :param str name: principal - :param str | None password: if empty - uses randkey. + :param str | None password: if None - uses randkey. + :param list[str] | None algorithms: encryption algorithms """ @abstractmethod @@ -135,19 +171,17 @@ async def del_princ(self, name: str) -> None: """ @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 ktadd(self, names: list[str], fn: str) -> None: + async def ktadd( + self, + names: list[str], + fn: str, + is_rand_key: bool = False, + ) -> None: """Create or write to keytab. - :param str name: principal + :param list[str] names: principals :param str fn: filename + :param bool is_rand_key: generate random key """ @abstractmethod @@ -164,11 +198,37 @@ async def force_pw_principal(self, name: str, **dbargs) -> None: :param str name: principal """ + @abstractmethod + async def modify_principal( + self, + principal_name: str, + new_principal_name: str | None = None, + algorithms: list[str] | None = None, + password: str | None = None, + **dbargs, + ) -> None: + """Modify principal (rename, change algorithms, password). + + :param str principal_name: current principal name + :param str | None new_principal_name: new name if rename needed + :param list[str] | None algorithms: new encryption algorithms + :param str | None password: new password + """ + class KAdminLocalManager(AbstractKRBManager): """Kadmin manager.""" client: KAdminProtocol + SERVICE_PREFIXES = [ + "host/", + "HTTP/", + "ldap/", + "cifs/", + "dns/", + "krbtgt/", + "kadmin/", + ] def __init__(self, loop: asyncio.AbstractEventLoop | None = None) -> None: """Create threadpool and get loop.""" @@ -202,31 +262,61 @@ async def _init_client(self) -> KAdminProtocol: """Init kadmin local connection.""" return await self.loop.run_in_executor(self.pool, kadmv.local) + def _parse_algorithms(self, algorithms: list[str] | None) -> list | None: + """Parse algorithm strings to kadmv EncryptionType. + + :param algorithms: list of algorithm strings + :return: list of EncryptionType or None + """ + if not algorithms: + return None + + enc_types = [] + for alg in algorithms: + if enc_type := ALGORITHM_MAP.get(alg): + enc_types.append(enc_type) + else: + logging.warning(f"Unknown algorithm: {alg}, skipping") + + return enc_types if enc_types else None + async def add_princ( self, name: str, password: str | None, + algorithms: list[str] | None = None, **dbargs, ) -> None: """Create principal. :param str name: principal - :param str | None password: if empty - uses randkey. + :param str | None password: if None - uses randkey. + :param list[str] | None algorithms: encryption algorithms """ + is_service = any( + name.startswith(prefix) for prefix in self.SERVICE_PREFIXES + ) + if password is None and not is_service: + raise ValueError("Password required for user principals") + await self.loop.run_in_executor( self.pool, - self.client.add_principal, - name, - password, + partial(self.client.add_principal, name, password), ) + princ = await self._get_raw_principal(name) if password: # NOTE: add preauth, attributes == krbticketflags - princ = await self._get_raw_principal(name) await self.loop.run_in_executor( self.pool, partial(princ.modify, attributes=128), ) + enc_types = self._parse_algorithms(algorithms) + if enc_types: + await self.loop.run_in_executor( + self.pool, + partial(princ.modify, enc_types=enc_types), + ) async def _get_raw_principal(self, name: str) -> PrincipalProtocol: principal = await self.loop.run_in_executor( @@ -300,19 +390,67 @@ async def rename_princ(self, name: str, new_name: str) -> None: new_name, ) - async def ktadd(self, names: list[str], fn: str) -> None: + async def ktadd( + self, + names: list[str], + fn: str, + is_rand_key: bool = False, + ) -> None: """Create or write to keytab. - :param str name: principal + :param list[str] names: principals :param str fn: filename - :raises self.PrincipalNotFoundError: on not found princ + :param bool is_rand_key: generate random key + :raises PrincipalNotFoundError: on not found princ """ principals = [await self._get_raw_principal(name) for name in names] if not all(principals): raise PrincipalNotFoundError("Principal not found") - for princ in principals: - await self.loop.run_in_executor(self.pool, princ.ktadd, fn) + try: + if is_rand_key: + for princ in principals: + await self.loop.run_in_executor( + self.pool, + partial(princ.ktadd, fn, randkey=True), + ) + else: + for princ in principals: + await self.loop.run_in_executor(self.pool, princ.ktadd, fn) + except (AttributeError, TypeError): + logging.warning( + "python-kadmv doesn't support randkey in ktadd, " + "using subprocess", + ) + if is_rand_key: + for name in names: + await self._ktadd_with_randkey_via_subprocess(name, fn) + else: + for princ in principals: + await self.loop.run_in_executor(self.pool, princ.ktadd, fn) + + async def _ktadd_with_randkey_via_subprocess( + self, + principal_name: str, + keytab_path: str, + ) -> None: + """Execute ktadd with randkey via subprocess.""" + cmd = [ + "kadmin.local", + "-q", + f"ktadd -k {keytab_path} -randkey {principal_name}", + ] + + proc = await asyncio.create_subprocess_exec( + *cmd, + stdout=asyncio.subprocess.PIPE, + stderr=asyncio.subprocess.PIPE, + ) + + stdout, stderr = await proc.communicate() + + if await proc.wait() != 0: + raise RuntimeError(f"ktadd failed: {stderr.decode()}") async def lock_princ(self, name: str, **dbargs) -> None: """Lock princ. @@ -332,6 +470,50 @@ async def force_pw_principal(self, name: str, **dbargs) -> None: princ.pwexpire = "Now" await self.loop.run_in_executor(self.pool, princ.commit) + async def modify_principal( + self, + principal_name: str, + new_principal_name: str | None = None, + algorithms: list[str] | None = None, + password: str | None = None, + **dbargs, + ) -> None: + """Modify principal (rename, change algorithms, password). + + :param str principal_name: current principal name + :param str | None new_principal_name: new name if rename needed + :param list[str] | None algorithms: new encryption algorithms + :param str | None password: new password + """ + if new_principal_name and new_principal_name != principal_name: + await self.rename_princ(principal_name, new_principal_name) + principal_name = new_principal_name + + princ = await self._get_raw_principal(principal_name) + + if algorithms is not None: + enc_types = self._parse_algorithms(algorithms) + if enc_types: + await self.loop.run_in_executor( + self.pool, + partial(princ.modify, enc_types=enc_types), + ) + + if password is not None: + if password == "": + await self.del_princ(principal_name) + await self.add_princ( + principal_name, + None, + algorithms=algorithms, + ) + else: + await self.change_password(principal_name, password) + await self.loop.run_in_executor( + self.pool, + partial(princ.modify, attributes=128), + ) + @asynccontextmanager async def kadmin_lifespan(app: FastAPI) -> AsyncIterator[None]: @@ -494,15 +676,18 @@ async def reset_setup() -> None: @principal_router.post("", response_class=Response, status_code=201) async def add_princ( kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)], - name: Annotated[str, Body()], - password: Annotated[str | None, Body(embed=True)] = None, + request: AddPrincipalRequest, ) -> None: """Add principal. :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract - + :param AddPrincipalRequest request: request data """ - await kadmin.add_princ(name, password) + await kadmin.add_princ( + request.principal_name, + request.password, + algorithms=request.algorithms, + ) @principal_router.get("") @@ -510,11 +695,10 @@ async def get_princ( kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)], name: str, ) -> Principal: - """Add principal. + """Get principal. :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract - :param Annotated[str, Body name: principal name - :param Annotated[str, Body password: principal password + :param str name: principal name """ return await kadmin.get_princ(name) @@ -524,11 +708,10 @@ async def del_princ( kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)], name: str, ) -> None: - """Add principal. + """Delete principal. :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract - :param Annotated[str, Body name: principal name - :param Annotated[str, Body password: principal password + :param str name: principal name """ await kadmin.del_princ(name) @@ -568,37 +751,47 @@ async def create_or_update_princ_password( @principal_router.put( - "", + "/modify", status_code=status.HTTP_202_ACCEPTED, response_class=Response, ) -async def rename_princ( +async def modify_princ( kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)], - name: Annotated[str, Body()], - new_name: Annotated[str, Body()], + request: ModifyPrincipalRequest, ) -> None: - """Rename principal. + """Modify principal (rename, algorithms, password). :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract - :param Annotated[str, Body name: principal name - :param Annotated[str, Body new_name: principal new name + :param ModifyPrincipalRequest request: request data """ - """""" - await kadmin.rename_princ(name, new_name) + await kadmin.modify_principal( + principal_name=request.principal_name, + new_principal_name=request.new_principal_name, + algorithms=request.algorithms, + password=request.password, + ) @principal_router.post("/ktadd") async def ktadd( kadmin: Annotated[AbstractKRBManager, Depends(get_kadmin)], - names: Annotated[list[str], Body()], + request: KtaddRequest, ) -> FileResponse: """Ktadd principal. :param Annotated[AbstractKRBManager, Depends kadmin: kadmin abstract + <<<<<<< Updated upstream :param KtaddSchema schema: ktadd request data + ======= + :param KtaddRequest request: request data + >>>>>>> Stashed changes """ filename = os.path.join(gettempdir(), str(uuid.uuid1())) - await kadmin.ktadd(names, filename) + await kadmin.ktadd( + request.names, + filename, + is_rand_key=request.is_rand_key, + ) return FileResponse( filename, diff --git a/app/ldap_protocol/kerberos/base.py b/app/ldap_protocol/kerberos/base.py index 7faf30565..8560d6b11 100644 --- a/app/ldap_protocol/kerberos/base.py +++ b/app/ldap_protocol/kerberos/base.py @@ -232,14 +232,17 @@ async def ldap_principal_setup(self, name: str, path: str) -> None: if response.status_code == 200: return - response = await self.client.post("/principal", json={"name": name}) + response = await self.client.post( + "/principal", + json={"principal_name": name}, + ) if response.status_code != 201: log.error(f"Error creating ldap principal: {response.text}") return response = await self.client.post( "/principal/ktadd", - json=[name], + json={"names": [name], "is_rand_key": True}, ) if response.status_code != 200: log.error(f"Error getting keytab: {response.text}") diff --git a/app/ldap_protocol/kerberos/utils.py b/app/ldap_protocol/kerberos/utils.py index c6278ed95..36c5bdebd 100644 --- a/app/ldap_protocol/kerberos/utils.py +++ b/app/ldap_protocol/kerberos/utils.py @@ -63,11 +63,7 @@ async def wrapped(*args: str, **kwargs: str) -> Any: except Exception as err: if isinstance(err, KRBAPIError): logger.error(f"{name} call raised: {err}") - raise - - else: - if not is_stub: - logger.success(f"Executed {name}") + raise return result return wrapped diff --git a/tests/test_api/test_main/test_kadmin.py b/tests/test_api/test_main/test_kadmin.py index b96d6c6c5..2e0668a23 100644 --- a/tests/test_api/test_main/test_kadmin.py +++ b/tests/test_api/test_main/test_kadmin.py @@ -212,7 +212,10 @@ async def test_ktadd( :param LDAPSession ldap_session: ldap """ names = ["test1", "test2"] - response = await http_client.post("/kerberos/ktadd", json=names) + response = await http_client.post( + "/kerberos/ktadd", + json={"names": names, "is_rand_key": False}, + ) kadmin.ktadd.assert_called() # type: ignore assert kadmin.ktadd.call_args.args[0] == names # type: ignore @@ -240,7 +243,10 @@ async def test_ktadd_400( kadmin.ktadd.side_effect = KRBAPIPrincipalNotFoundError() # type: ignore names = ["test1", "test2"] - response = await http_client.post("/kerberos/ktadd", json=names) + response = await http_client.post( + "/kerberos/ktadd", + json={"names": names, "is_rand_key": False}, + ) assert response.status_code == status.HTTP_400_BAD_REQUEST From 77d072035588e24f854310976c4d92a1695b1a40 Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Mon, 9 Feb 2026 12:03:38 +0300 Subject: [PATCH 5/7] Refactor: update API endpoint interactions for principal management to improve parameter handling and request structure - Modified the API endpoints for adding and renaming principals to align with new request models. - Adjusted assertions in tests to reflect changes in the expected arguments for kadmin methods. - Enhanced clarity by consolidating parameters in API requests. --- tests/test_api/test_main/test_kadmin.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/tests/test_api/test_main/test_kadmin.py b/tests/test_api/test_main/test_kadmin.py index 2e0668a23..d1a04d4ca 100644 --- a/tests/test_api/test_main/test_kadmin.py +++ b/tests/test_api/test_main/test_kadmin.py @@ -355,7 +355,7 @@ async def test_bind_create_user( assert await proc.wait() == 0 kadmin_args = kadmin.add_principal.call_args.args # type: ignore - assert kadmin_args == (san, pw, 0.1) + assert kadmin_args == (san, pw) @pytest.mark.asyncio @@ -395,15 +395,15 @@ async def test_add_princ( :param LDAPSession ldap_session: ldap """ response = await http_client.post( - "/kerberos/principal/add", + "/kerberos/principal", json={ - "primary": "host", - "instance": "12345", + "principal_name": "host/12345", + "password": None, }, ) kadmin_args = kadmin.add_principal.call_args.args # type: ignore assert response.status_code == status.HTTP_200_OK - assert kadmin_args == ("host/12345", None) + assert kadmin_args == ("host/12345", None, None) @pytest.mark.asyncio @@ -417,16 +417,16 @@ async def test_rename_princ( :param AsyncClient http_client: http cl :param LDAPSession ldap_session: ldap """ - response = await http_client.patch( - "/kerberos/principal/rename", + response = await http_client.put( + "/kerberos/principal", json={ "principal_name": "name", - "principal_new_name": "nname", + "new_principal_name": "nname", }, ) kadmin_args = kadmin.rename_princ.call_args.args # type: ignore assert response.status_code == status.HTTP_200_OK - assert kadmin_args == ("name", "nname") + assert kadmin_args == ("name", "nname", None, None) @pytest.mark.asyncio From 888fe0ed56afe9fffc6d7ace3851ed192638c94c Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Mon, 9 Feb 2026 12:54:15 +0300 Subject: [PATCH 6/7] Refactor: simplify algorithm parsing in KAdminLocalManager by removing unnecessary None return --- .kerberos/config_server.py | 7 ++----- interface | 2 +- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/.kerberos/config_server.py b/.kerberos/config_server.py index 7b9a3dc43..a9a522bfa 100644 --- a/.kerberos/config_server.py +++ b/.kerberos/config_server.py @@ -262,15 +262,12 @@ async def _init_client(self) -> KAdminProtocol: """Init kadmin local connection.""" return await self.loop.run_in_executor(self.pool, kadmv.local) - def _parse_algorithms(self, algorithms: list[str] | None) -> list | None: + def _parse_algorithms(self, algorithms: list[str] | None) -> list: """Parse algorithm strings to kadmv EncryptionType. :param algorithms: list of algorithm strings :return: list of EncryptionType or None """ - if not algorithms: - return None - enc_types = [] for alg in algorithms: if enc_type := ALGORITHM_MAP.get(alg): @@ -278,7 +275,7 @@ def _parse_algorithms(self, algorithms: list[str] | None) -> list | None: else: logging.warning(f"Unknown algorithm: {alg}, skipping") - return enc_types if enc_types else None + return enc_types async def add_princ( self, diff --git a/interface b/interface index f31962020..e1ca5656a 160000 --- a/interface +++ b/interface @@ -1 +1 @@ -Subproject commit f31962020a6689e6a4c61fb3349db5b5c7895f92 +Subproject commit e1ca5656aeabc20a1862aeaf11ded72feaa97403 From 27f709cf671da9f3efab1954f355bbf5c7b5ba73 Mon Sep 17 00:00:00 2001 From: "m.shvets" Date: Tue, 10 Feb 2026 11:10:08 +0300 Subject: [PATCH 7/7] Refactor: rename principal methods and update related API interactions to modify principal functionality - Removed the rename_princ method from KAdminLocalManager and replaced it with modify_princ. - Updated API endpoint from rename_principal to modify_principal to reflect the new naming convention. - Adjusted related methods and error codes to use modify_principal terminology for consistency across the codebase. --- .kerberos/config_server.py | 13 ------------- app/api/main/adapters/kerberos.py | 6 +++--- app/api/main/krb5_router.py | 4 ++-- app/enums.py | 2 +- app/ldap_protocol/kerberos/base.py | 2 +- app/ldap_protocol/kerberos/exceptions.py | 4 ++-- app/ldap_protocol/kerberos/service.py | 8 ++++---- app/ldap_protocol/kerberos/utils.py | 3 +++ app/ldap_protocol/ldap_requests/modify.py | 6 +++--- 9 files changed, 19 insertions(+), 29 deletions(-) diff --git a/.kerberos/config_server.py b/.kerberos/config_server.py index a9a522bfa..b455735f8 100644 --- a/.kerberos/config_server.py +++ b/.kerberos/config_server.py @@ -374,19 +374,6 @@ async def del_princ(self, name: str) -> None: except kadmv.UnknownPrincipalError: raise PrincipalNotFoundError - 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 ktadd( self, names: list[str], diff --git a/app/api/main/adapters/kerberos.py b/app/api/main/adapters/kerberos.py index 6124636c4..78d9ca4cd 100644 --- a/app/api/main/adapters/kerberos.py +++ b/app/api/main/adapters/kerberos.py @@ -83,13 +83,13 @@ async def add_principal(self, request: PrincipalAddRequest) -> None: algorithms=request.algorithms, ) - async def rename_principal(self, request: PrincipalPutRequest) -> None: - """Modify principal (rename, password, algorithms). + async def modify_principal(self, request: PrincipalPutRequest) -> None: + """Modify principal ( password, algorithms). :raises HTTPException: on Kerberos errors :return: None """ - return await self._service.rename_principal( + return await self._service.modify_principal( principal_name=request.principal_name, principal_new_name=request.new_principal_name, algorithms=request.algorithms, diff --git a/app/api/main/krb5_router.py b/app/api/main/krb5_router.py index 8cd32e084..a7557e6ed 100644 --- a/app/api/main/krb5_router.py +++ b/app/api/main/krb5_router.py @@ -206,11 +206,11 @@ async def add_principal( dependencies=[Depends(verify_auth), Depends(require_master_db)], error_map=error_map, ) -async def rename_principal( +async def modify_principal( request: PrincipalPutRequest, kerberos_adapter: FromDishka[KerberosFastAPIAdapter], ) -> None: - await kerberos_adapter.rename_principal(request) + await kerberos_adapter.modify_principal(request) @krb5_router.patch( diff --git a/app/enums.py b/app/enums.py index b4ef3cde4..b1791a43b 100644 --- a/app/enums.py +++ b/app/enums.py @@ -193,7 +193,7 @@ class AuthorizationRules(IntFlag): KRB_KTADD = auto() KRB_GET_STATUS = auto() KRB_ADD_PRINCIPAL = auto() - KRB_RENAME_PRINCIPAL = auto() + KRB_MODIFY_PRINCIPAL = auto() KRB_RESET_PRINCIPAL_PW = auto() KRB_DELETE_PRINCIPAL = auto() diff --git a/app/ldap_protocol/kerberos/base.py b/app/ldap_protocol/kerberos/base.py index 8560d6b11..432c6c61e 100644 --- a/app/ldap_protocol/kerberos/base.py +++ b/app/ldap_protocol/kerberos/base.py @@ -180,7 +180,7 @@ async def create_or_update_principal_pw( ) -> None: ... @abstractmethod - async def rename_princ( + async def modify_princ( self, name: str, new_name: str, diff --git a/app/ldap_protocol/kerberos/exceptions.py b/app/ldap_protocol/kerberos/exceptions.py index 735149eff..f83a4e237 100644 --- a/app/ldap_protocol/kerberos/exceptions.py +++ b/app/ldap_protocol/kerberos/exceptions.py @@ -31,7 +31,7 @@ class ErrorCodes(IntEnum): KERBEROS_API_GET_PRINCIPAL_ERROR = 16 KERBEROS_API_DELETE_PRINCIPAL_ERROR = 17 KERBEROS_API_CHANGE_PASSWORD_ERROR = 18 - KERBEROS_API_RENAME_PRINCIPAL_ERROR = 19 + KERBEROS_API_MODIFY_PRINCIPAL_ERROR = 19 KERBEROS_API_LOCK_PRINCIPAL_ERROR = 20 KERBEROS_API_FORCE_PASSWORD_CHANGE_ERROR = 21 KERBEROS_API_STATUS_NOT_FOUND_ERROR = 22 @@ -135,7 +135,7 @@ class KRBAPIChangePasswordError(KRBAPIError): class KRBAPIRenamePrincipalError(KRBAPIError): """Rename principal error.""" - code = ErrorCodes.KERBEROS_API_RENAME_PRINCIPAL_ERROR + code = ErrorCodes.KERBEROS_API_MODIFY_PRINCIPAL_ERROR class KRBAPILockPrincipalError(KRBAPIError): diff --git a/app/ldap_protocol/kerberos/service.py b/app/ldap_protocol/kerberos/service.py index 8d3ad1fc8..8fef26326 100644 --- a/app/ldap_protocol/kerberos/service.py +++ b/app/ldap_protocol/kerberos/service.py @@ -382,14 +382,14 @@ async def add_principal( f"Error adding principal: {exc}", ) from exc - async def rename_principal( + async def modify_principal( self, principal_name: str, principal_new_name: str, algorithms: list[str] | None, password: str | None, ) -> None: - """Rename principal in Kerberos with given name. + """Modify principal in Kerberos with given name. :param str principal_name: Current principal name. :param str principal_new_name: New principal name. @@ -399,7 +399,7 @@ async def rename_principal( :return None: None. """ try: - await self._kadmin.rename_princ( + await self._kadmin.modify_princ( principal_name, principal_new_name, algorithms, @@ -488,7 +488,7 @@ async def get_status(self) -> KerberosState: ktadd.__name__: AuthorizationRules.KRB_KTADD, get_status.__name__: AuthorizationRules.KRB_GET_STATUS, add_principal.__name__: AuthorizationRules.KRB_ADD_PRINCIPAL, - rename_principal.__name__: AuthorizationRules.KRB_RENAME_PRINCIPAL, + modify_principal.__name__: AuthorizationRules.KRB_MODIFY_PRINCIPAL, reset_principal_pw.__name__: AuthorizationRules.KRB_RESET_PRINCIPAL_PW, delete_principal.__name__: AuthorizationRules.KRB_DELETE_PRINCIPAL, } diff --git a/app/ldap_protocol/kerberos/utils.py b/app/ldap_protocol/kerberos/utils.py index 36c5bdebd..026b71a1d 100644 --- a/app/ldap_protocol/kerberos/utils.py +++ b/app/ldap_protocol/kerberos/utils.py @@ -64,6 +64,9 @@ async def wrapped(*args: str, **kwargs: str) -> Any: if isinstance(err, KRBAPIError): logger.error(f"{name} call raised: {err}") raise + else: + if not is_stub: + logger.success(f"Executed {name}") return result return wrapped diff --git a/app/ldap_protocol/ldap_requests/modify.py b/app/ldap_protocol/ldap_requests/modify.py index 9f5b8789d..efcf8bb2b 100644 --- a/app/ldap_protocol/ldap_requests/modify.py +++ b/app/ldap_protocol/ldap_requests/modify.py @@ -937,7 +937,7 @@ 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.rename_princ( + await kadmin.modify_princ( directory.user.sam_account_name, new_sam_account_name, ) @@ -1043,11 +1043,11 @@ async def _modify_computer_samaccountname( raise ModifyForbiddenError("Old sAMAccountName value not found.") if old_sam_account_name != new_sam_account_name: - await kadmin.rename_princ( + await kadmin.modify_princ( f"host/{old_sam_account_name}", f"host/{new_sam_account_name}", ) - await kadmin.rename_princ( + await kadmin.modify_princ( f"host/{old_sam_account_name}.{base_dir.name}", f"host/{new_sam_account_name}.{base_dir.name}", )