Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sources/kerberos: use new python-kadmin implementation #11932

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
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
20 changes: 10 additions & 10 deletions authentik/sources/kerberos/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@
from typing import Any

import gssapi
import kadmin
import pglock
from django.db import connection, models
from django.db.models.fields import b64decode
from django.http import HttpRequest
from django.shortcuts import reverse
from django.templatetags.static import static
from django.utils.translation import gettext_lazy as _
from kadmin import KAdmin
from kadmin.exceptions import PyKAdminException
from rest_framework.serializers import Serializer
from structlog.stdlib import get_logger

Expand All @@ -30,9 +31,8 @@
LOGGER = get_logger()


# python-kadmin leaks file descriptors. As such, this global is used to reuse
# existing kadmin connections instead of creating new ones, which results in less to no file
# descriptors leaks
# Creating kadmin connections is expensive. As such, this global is used to reuse
# existing kadmin connections instead of creating new ones
_kadmin_connections: dict[str, Any] = {}


Expand Down Expand Up @@ -198,13 +198,13 @@
conf_path.write_text(self.krb5_conf)
return str(conf_path)

def _kadmin_init(self) -> "kadmin.KAdmin | None":
def _kadmin_init(self) -> KAdmin | None:
# kadmin doesn't use a ccache for its connection
# as such, we don't need to create a separate ccache for each source
if not self.sync_principal:
return None
if self.sync_password:
return kadmin.init_with_password(
return KAdmin.with_password(
self.sync_principal,
self.sync_password,
)
Expand All @@ -215,18 +215,18 @@
keytab_path.touch(mode=0o600)
keytab_path.write_bytes(b64decode(self.sync_keytab))
keytab = f"FILE:{keytab_path}"
return kadmin.init_with_keytab(
return KAdmin.with_keytab(

Check warning on line 218 in authentik/sources/kerberos/models.py

View check run for this annotation

Codecov / codecov/patch

authentik/sources/kerberos/models.py#L218

Added line #L218 was not covered by tests
self.sync_principal,
keytab,
)
if self.sync_ccache:
return kadmin.init_with_ccache(
return KAdmin.with_ccache(

Check warning on line 223 in authentik/sources/kerberos/models.py

View check run for this annotation

Codecov / codecov/patch

authentik/sources/kerberos/models.py#L223

Added line #L223 was not covered by tests
self.sync_principal,
self.sync_ccache,
)
return None

def connection(self) -> "kadmin.KAdmin | None":
def connection(self) -> KAdmin | None:
"""Get kadmin connection"""
if str(self.pk) not in _kadmin_connections:
kadm = self._kadmin_init()
Expand All @@ -246,7 +246,7 @@
status["status"] = "no connection"
return status
status["principal_exists"] = kadm.principal_exists(self.sync_principal)
except kadmin.KAdminError as exc:
except PyKAdminException as exc:

Check warning on line 249 in authentik/sources/kerberos/models.py

View check run for this annotation

Codecov / codecov/patch

authentik/sources/kerberos/models.py#L249

Added line #L249 was not covered by tests
status["status"] = str(exc)
return status

Expand Down
4 changes: 2 additions & 2 deletions authentik/sources/kerberos/signals.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
"""authentik kerberos source signals"""

import kadmin
from django.db.models.signals import post_save
from django.dispatch import receiver
from kadmin.exceptions import PyKAdminException
from rest_framework.serializers import ValidationError
from structlog.stdlib import get_logger

Expand Down Expand Up @@ -48,7 +48,7 @@
source.connection().getprinc(user_source_connection.identifier).change_password(
password
)
except kadmin.KAdminError as exc:
except PyKAdminException as exc:

Check warning on line 51 in authentik/sources/kerberos/signals.py

View check run for this annotation

Codecov / codecov/patch

authentik/sources/kerberos/signals.py#L51

Added line #L51 was not covered by tests
LOGGER.warning("failed to set Kerberos password", exc=exc, source=source)
Event.new(
EventAction.CONFIGURATION_ERROR,
Expand Down
6 changes: 3 additions & 3 deletions authentik/sources/kerberos/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@

from typing import Any

import kadmin
from django.core.exceptions import FieldError
from django.db import IntegrityError, transaction
from kadmin import KAdmin
from structlog.stdlib import BoundLogger, get_logger

from authentik.core.expression.exceptions import (
Expand All @@ -30,7 +30,7 @@ class KerberosSync:

_source: KerberosSource
_logger: BoundLogger
_connection: "kadmin.KAdmin"
_connection: KAdmin
mapper: SourceMapper
user_manager: PropertyMappingManager
group_manager: PropertyMappingManager
Expand Down Expand Up @@ -161,7 +161,7 @@ def sync(self) -> int:

user_count = 0
with Krb5ConfContext(self._source):
for principal in self._connection.principals():
for principal in self._connection.list_principals("*"):
if self._handle_principal(principal):
user_count += 1
return user_count
1 change: 1 addition & 0 deletions authentik/sources/kerberos/tests/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ def setUp(self):
)
self.user = User.objects.create(username=generate_id())
self.user.set_unusable_password()
self.user.save()
UserKerberosSourceConnection.objects.create(
source=self.source, user=self.user, identifier=self.realm.user_princ
)
Expand Down
66 changes: 49 additions & 17 deletions poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -131,8 +131,7 @@ pydantic-scim = "*"
pyjwt = "*"
pyrad = "*"
python = "~3.12"
# Fork of python-kadmin with compilation fixes as it's unmaintained
python-kadmin = { git = "https://github.com/authentik-community/python-kadmin.git", tag = "v0.2.0" }
python-kadmin-rs = "0.0.4"
pyyaml = "*"
requests-oauthlib = "*"
scim2-filter-parser = "*"
Expand Down
Loading