Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .kerberos/entrypoint.sh
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ set -e

sed -i 's/ou=users/cn=users/g' /etc/kdc/krb5.d/stash.keyfile || true
sed -i 's/ou=users/cn=users/g' /etc/kdc/krb5.conf || true

cd /server

uvicorn --factory config_server:create_app \
Expand Down
182 changes: 182 additions & 0 deletions app/alembic/versions/a1b2c3d4e5f6_rename_services_to_system.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,182 @@
"""Rename services container to System for AD compatibility.
Revision ID: a1b2c3d4e5f6
Revises: 6c858cc05da7
Create Date: 2026-01-13 12:00:00.000000
"""

from alembic import op
from dishka import AsyncContainer
from sqlalchemy import and_, exists, select
from sqlalchemy.ext.asyncio import AsyncConnection, AsyncSession

from entities import Attribute, Directory
from ldap_protocol.utils.queries import get_base_directories
from repo.pg.tables import queryable_attr as qa

# revision identifiers, used by Alembic.
revision: None | str = "a1b2c3d4e5f6"
down_revision: None | str = "6c858cc05da7"
branch_labels: None | list[str] = None
depends_on: None | list[str] = None


def upgrade(container: AsyncContainer) -> None: # noqa: ARG001
Copy link
Collaborator

@milov-dmitriy milov-dmitriy Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

еще что не могу понять, а что если в лдап дереве нет директории с именем service ? мы же тогда просто выйдем из миграции, и кажется , что это не совсем правильно, наверное стоит создавать директорию System в таком случае

не знаю, стоит ли мне лезть в бизнеслогику, но не знаю в общем...

"""Upgrade: Rename 'services' container to 'System'."""
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] название System именно в upper кейсе?
просто у нас везде lower, и это как будто выбивается из общей практики, я бы уточнил у автора задачи


async def _rename_services_to_system(connection: AsyncConnection) -> None:
session = AsyncSession(bind=connection)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

из контейнера берем теперь сесиию

await session.begin()

try:
base_directories = await get_base_directories(session)
if not base_directories:
await session.commit()
return
Comment on lines +33 to +36
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

это можно наружу, зачем это внутри try

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

или может вообще уберем try? у нас это не очень распространенная практика


services_dirs = await session.scalars(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

services_dirs -> service_dirs

select(Directory).where(qa(Directory.name) == "services"),
)

for services_dir in services_dirs:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

services_dir -> service_dir

system_exists = await session.scalar(
select(exists(Directory)).where(
and_(
qa(Directory.name) == "System",
qa(Directory.parent_id) == services_dir.parent_id,
),
),
)
Comment on lines +43 to +50
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

select(exists().where())
а у тебя так:
select(exists()).where()

проверь работоспособность этого решения в реалтайме плз

Copy link
Collaborator

@milov-dmitriy milov-dmitriy Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

можно создать любые директории-контейнеры и прочее руками, а затем сделать make downgrade чтоб откатить и накатить эту миграцию или можно просто сделать чекаут с dev на твою ветку

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. тут точно нельзя никак по другому запрос написать?

можно в запрос встроить parent_id.in_(service_dir_ids) (псевдокод)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. предлагаю сделать "команда с отдельной строки" как у нас везде принято:
select
where


if system_exists:
continue

services_dir.name = "System"
services_dir.path = [
"ou=System" if p == "ou=services" else p
for p in services_dir.path
]

await session.flush()
await _update_descendants(session, services_dir.id)

await _update_attributes(session, "ou=services", "ou=System")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

предлагаю строго привязывать к id той директории которую мы обновили, т.е. передавать в _update_attributes еще и directory_id, чтобы не сделать "лишних" изменений у тех директорий, которые в отбор не попали, но атрибуты которых попали под фильтр атрибутов. думаю, это повысит надежность миграции

await session.commit()

except Exception:
await session.rollback()
raise

async def _update_descendants(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

обе этих протектед функции предлагаю перенести вверх, чтобы сначала разработчик ознакамливался с ними, а только потом видел их использование в коде

session: AsyncSession,
parent_id: int,
) -> None:
"""Recursively update paths of all descendants."""
children = await session.scalars(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

children -> child_dirs

select(Directory).where(qa(Directory.parent_id) == parent_id),
)
Comment on lines +76 to +78
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

предлагаю сделать "команда с отдельной строки" как у нас везде принято:

select
where


for child in children:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

child -> child_dir

child.path = [
"ou=System" if p == "ou=services" else p for p in child.path
]
await session.flush()
await _update_descendants(session, child.id)

async def _update_attributes(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

обе этих протектед функции предлагаю перенести вверх, чтобы сначала разработчик ознакамливался с ними, а только потом видел их использование в коде

session: AsyncSession,
old_value: str,
new_value: str,
) -> None:
"""Update attribute values containing old DN."""
result = await session.execute(
select(Attribute).where(
Attribute.value.ilike(f"%{old_value}%"), # type: ignore
),
)
Comment on lines +93 to +97
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

предлагаю сделать "команда с отдельной строки" как у нас везде принято:

select
where

attributes = result.scalars().all()

for attr in attributes:
if attr.value and old_value in attr.value:
attr.value = attr.value.replace(old_value, new_value)

await session.flush()

op.run_async(_rename_services_to_system)


def downgrade(container: AsyncContainer) -> None: # noqa: ARG001
"""Downgrade: Rename 'System' container back to 'services'."""

Comment on lines +109 to +111
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

проверь downgrade на аналогичные комментарии что и upgrade

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. неминг
  2. форматирование запросов
  3. необходимость try-блока

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. сессия из контейнера

async def _rename_system_to_services(connection: AsyncConnection) -> None:
session = AsyncSession(bind=connection)
await session.begin()

try:
base_directories = await get_base_directories(session)
if not base_directories:
await session.commit()
return

system_dirs = await session.scalars(
select(Directory).where(qa(Directory.name) == "System"),
)

for system_dir in system_dirs:
system_dir.name = "services"
system_dir.path = [
"ou=services" if p == "ou=System" else p
for p in system_dir.path
]

await session.flush()
await _update_descendants_downgrade(session, system_dir.id)

await _update_attributes_downgrade(
session,
"ou=System",
"ou=services",
)
await session.commit()

except Exception:
await session.rollback()
raise

async def _update_descendants_downgrade(
session: AsyncSession,
parent_id: int,
) -> None:
"""Recursively update paths of all descendants."""
children = await session.scalars(
select(Directory).where(qa(Directory.parent_id) == parent_id),
)

for child in children:
child.path = [
"ou=services" if p == "ou=System" else p for p in child.path
]
await session.flush()
await _update_descendants_downgrade(session, child.id)

async def _update_attributes_downgrade(
session: AsyncSession,
old_value: str,
new_value: str,
) -> None:
"""Update attribute values during downgrade."""
result = await session.execute(
select(Attribute).where(
Attribute.value.ilike(f"%{old_value}%"), # type: ignore
),
)
attributes = result.scalars().all()

for attr in attributes:
if attr.value and old_value in attr.value:
attr.value = attr.value.replace(old_value, new_value)

await session.flush()

op.run_async(_rename_system_to_services)
77 changes: 60 additions & 17 deletions app/extra/scripts/update_krb5_config.py
Original file line number Diff line number Diff line change
@@ -1,46 +1,89 @@
"""Kerberos update config.
"""Kerberos configuration update script.

Copyright (c) 2025 MultiFactor
License: https://github.com/MultiDirectoryLab/MultiDirectory/blob/main/LICENSE
"""

from pathlib import Path

from loguru import logger
from sqlalchemy.ext.asyncio import AsyncSession

from config import Settings
from ldap_protocol.kerberos import AbstractKadmin
from ldap_protocol.kerberos.utils import get_services_container_dn
from ldap_protocol.utils.queries import get_base_directories

KRB5_CONF_PATH = Path("/etc/krb5kdc/krb5.conf")
KDC_CONF_PATH = Path("/etc/krb5kdc/kdc.conf")
STASH_FILE_PATH = Path("/etc/krb5kdc/krb5.d/stash.keyfile")


def _migrate_legacy_dns(content: str) -> str:
"""Replace legacy DN formats with current ones.

:param content: File content to migrate.
:return: Migrated content.
"""
return content.replace("ou=services", "ou=System").replace(
"ou=users",
"cn=users",
)


async def update_krb5_config(
kadmin: AbstractKadmin,
session: AsyncSession,
settings: Settings,
) -> None:
"""Update kerberos config."""
if not (await kadmin.get_status(wait_for_positive=True)):
logger.error("kadmin_api is not running")
return
"""Update Kerberos configuration files via direct write to shared volume.

base_dn_list = await get_base_directories(session)
base_dn = base_dn_list[0].path_dn
domain: str = base_dn_list[0].name
Renders krb5.conf and kdc.conf from templates and writes them directly
to the shared volume. Also migrates legacy DN formats in stash.keyfile
if present (ou=services -> ou=System, ou=users -> cn=users).

krbadmin = "cn=krbadmin,cn=users," + base_dn
services_container = "ou=services," + base_dn
:param session: Database session for fetching base directories.
:param settings: Application settings with template environment.
:raises Exception: If config rendering or writing fails.
"""
if not KRB5_CONF_PATH.parent.exists():
logger.error(
f"Config directory {KRB5_CONF_PATH.parent} not found, "
"kerberos volume not mounted",
)
return

krb5_template = settings.TEMPLATES.get_template("krb5.conf")
kdc_template = settings.TEMPLATES.get_template("kdc.conf")
base_dn_list = await get_base_directories(session)
if not base_dn_list:
logger.error("No base directories found")
return

kdc_config = await kdc_template.render_async(domain=domain)
base_dn = base_dn_list[0].path_dn
domain = base_dn_list[0].name
krbadmin = f"cn=krbadmin,cn=users,{base_dn}"
services_container = get_services_container_dn(base_dn)

krb5_config = await krb5_template.render_async(
krb5_config = await settings.TEMPLATES.get_template(
"krb5.conf",
).render_async(
domain=domain,
krbadmin=krbadmin,
services_container=services_container,
ldap_uri=settings.KRB5_LDAP_URI,
mfa_push_url=settings.KRB5_MFA_PUSH_URL,
sync_password_url=settings.KRB5_SYNC_PASSWORD_URL,
)
kdc_config = await settings.TEMPLATES.get_template(
"kdc.conf",
).render_async(
domain=domain,
)

KRB5_CONF_PATH.write_text(krb5_config, encoding="utf-8")
KDC_CONF_PATH.write_text(kdc_config, encoding="utf-8")

await kadmin.setup_configs(krb5_config, kdc_config)
if STASH_FILE_PATH.exists():
stash_content = STASH_FILE_PATH.read_text(encoding="utf-8")
if "ou=services" in stash_content or "ou=users" in stash_content:
STASH_FILE_PATH.write_text(
_migrate_legacy_dns(stash_content),
encoding="utf-8",
)
11 changes: 8 additions & 3 deletions app/ldap_protocol/kerberos/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,12 @@
from .ldap_structure import KRBLDAPStructureManager
from .schemas import AddRequests, KDCContext, KerberosAdminDnGroup, TaskStruct
from .template_render import KRBTemplateRenderer
from .utils import KerberosState, get_krb_server_state, set_state
from .utils import (
KerberosState,
get_krb_server_state,
get_services_container_dn,
set_state,
)


class KerberosService(AbstractService):
Expand Down Expand Up @@ -140,7 +145,7 @@ def _build_kerberos_admin_dns(self, base_dn: str) -> KerberosAdminDnGroup:
dataclass with DN for krbadmin, services_container, krbadmin_group.
"""
krbadmin = f"cn=krbadmin,cn=users,{base_dn}"
services_container = f"ou=services,{base_dn}"
services_container = get_services_container_dn(base_dn)
krbgroup = f"cn=krbadmin,cn=groups,{base_dn}"
return KerberosAdminDnGroup(
krbadmin_dn=krbadmin,
Expand Down Expand Up @@ -288,7 +293,7 @@ async def _get_kdc_context(self) -> KDCContext:
base_dn, domain = await self._get_base_dn()
krbadmin = f"cn=krbadmin,cn=users,{base_dn}"
krbgroup = f"cn=krbadmin,cn=groups,{base_dn}"
services_container = f"ou=services,{base_dn}"
services_container = get_services_container_dn(base_dn)
Comment on lines 293 to +296
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] можем сделать для krbadmin, krbgroup, ... аналогичный метод что и для services_container

м?

return KDCContext(
base_dn=base_dn,
domain=domain,
Expand Down
5 changes: 5 additions & 0 deletions app/ldap_protocol/kerberos/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,3 +131,8 @@ async def unlock_principal(name: str, session: AsyncSession) -> None:
.filter_by(directory_id=subquery, name="krbprincipalexpiration")
.execution_options(synchronize_session=False),
)


def get_services_container_dn(base_dn: str) -> str:
"""Get System container DN for services."""
return f"ou=System,{base_dn}"
Comment on lines +136 to +138
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а это точно к керберосу относится а не к лдапу?

3 changes: 2 additions & 1 deletion app/ldap_protocol/roles/role_use_case.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

from entities import AccessControlEntry, AceType, Directory, Role
from enums import AuthorizationRules, RoleConstants, RoleScope
from ldap_protocol.kerberos.utils import get_services_container_dn
from ldap_protocol.utils.queries import get_base_directories
from repo.pg.tables import (
access_control_entries_table,
Expand Down Expand Up @@ -211,7 +212,7 @@ async def create_kerberos_system_role(self) -> None:

aces = self._get_full_access_aces(
role_id=self._role_dao.get_last_id(),
base_dn="ou=services," + base_dn_list[0].path_dn,
base_dn=get_services_container_dn(base_dn_list[0].path_dn),
)
await self._access_control_entry_dao.create_bulk(aces)

Expand Down
1 change: 1 addition & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -424,6 +424,7 @@ services:
- ./certs:/certs
- ./app:/app
- ldap_keytab:/LDAP_keytab/
- kdc:/etc/krb5kdc/
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

а это нужная для этого ПР правка? не могу разобраться

env_file: local.env
command: python multidirectory.py --scheduler
tty: true
Expand Down
2 changes: 1 addition & 1 deletion interface
Loading