From ce1ae777013f97dc5276590489e3e59e2a744852 Mon Sep 17 00:00:00 2001 From: Tom Grassmann Date: Mon, 4 Nov 2024 16:25:41 +0100 Subject: [PATCH 1/5] Set passord_change_date in check_pwd_last_set --- authentik/sources/ldap/sync/vendor/freeipa.py | 3 ++- authentik/sources/ldap/sync/vendor/ms_ad.py | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/authentik/sources/ldap/sync/vendor/freeipa.py b/authentik/sources/ldap/sync/vendor/freeipa.py index 44e127e05a51..df08a55a5574 100644 --- a/authentik/sources/ldap/sync/vendor/freeipa.py +++ b/authentik/sources/ldap/sync/vendor/freeipa.py @@ -29,7 +29,7 @@ def check_pwd_last_set(self, attributes: dict[str, Any], user: User, created: bo return pwd_last_set: datetime = attributes.get("krbLastPwdChange", datetime.now()) pwd_last_set = pwd_last_set.replace(tzinfo=UTC) - if created or pwd_last_set >= user.password_change_date: + if created or pwd_last_set > user.password_change_date: self.message(f"'{user.username}': Reset user's password") self._logger.debug( "Reset user's password", @@ -38,6 +38,7 @@ def check_pwd_last_set(self, attributes: dict[str, Any], user: User, created: bo pwd_last_set=pwd_last_set, ) user.set_unusable_password() + user.password_change_date = pwd_last_set user.save() def check_nsaccountlock(self, attributes: dict[str, Any], user: User): diff --git a/authentik/sources/ldap/sync/vendor/ms_ad.py b/authentik/sources/ldap/sync/vendor/ms_ad.py index fd0230897308..5a4557be232a 100644 --- a/authentik/sources/ldap/sync/vendor/ms_ad.py +++ b/authentik/sources/ldap/sync/vendor/ms_ad.py @@ -59,7 +59,7 @@ def ms_check_pwd_last_set(self, attributes: dict[str, Any], user: User, created: return pwd_last_set: datetime = attributes.get("pwdLastSet", datetime.now()) pwd_last_set = pwd_last_set.replace(tzinfo=UTC) - if created or pwd_last_set >= user.password_change_date: + if created or pwd_last_set > user.password_change_date: self.message(f"'{user.username}': Reset user's password") self._logger.debug( "Reset user's password", @@ -68,6 +68,7 @@ def ms_check_pwd_last_set(self, attributes: dict[str, Any], user: User, created: pwd_last_set=pwd_last_set, ) user.set_unusable_password() + user.password_change_date = pwd_last_set user.save() def ms_check_uac(self, attributes: dict[str, Any], user: User): From 42b0799061028b67eb0b7907a108a419aeefb7da Mon Sep 17 00:00:00 2001 From: Tom Grassmann Date: Thu, 7 Nov 2024 10:26:02 +0100 Subject: [PATCH 2/5] Refactor freeipa.py and ms_ad.py to use a common check_pwd_last_set method from the base class --- authentik/sources/ldap/sync/base.py | 38 +++++++++++++++++++ authentik/sources/ldap/sync/vendor/freeipa.py | 21 +--------- authentik/sources/ldap/sync/vendor/ms_ad.py | 20 +--------- 3 files changed, 40 insertions(+), 39 deletions(-) diff --git a/authentik/sources/ldap/sync/base.py b/authentik/sources/ldap/sync/base.py index 5fa7d699bf1a..c95f48191686 100644 --- a/authentik/sources/ldap/sync/base.py +++ b/authentik/sources/ldap/sync/base.py @@ -1,11 +1,14 @@ """Sync LDAP Users and groups into authentik""" from collections.abc import Generator +from datetime import datetime, UTC +from typing import Any from django.conf import settings from ldap3 import DEREF_ALWAYS, SUBTREE, Connection from structlog.stdlib import BoundLogger, get_logger +from authentik.core.models import User from authentik.core.sources.mapper import SourceMapper from authentik.lib.config import CONFIG from authentik.lib.sync.mapper import PropertyMappingManager @@ -65,6 +68,41 @@ def base_dn_groups(self) -> str: return f"{self._source.additional_group_dn},{self._source.base_dn}" return self._source.base_dn + def check_pwd_last_set(self, attribute_name: str, attributes: dict[str, Any], user: User, created: bool): + """ + Test if the ldap password is newer than the authentik password. + If the ldap password is newer set the user password to an unusable password. + This ends all users sessions and forces the user to relogin. + During next user login the used authentication backend MAY choose to write a new usable user password. + + @param attribute_name: The name of the ldap attribute holding the information when the password was changed + @param attributes: All ldap attributes + @param user: The user object we are currently syncing + @param created: True if the user is newly created + @return: + """ + if attribute_name not in attributes: + self._logger.debug( + f"Missing attribute {attribute_name}. Can not test if a newer ldap password is set." + f"Ldap and authentik passwords may be out of sync.", + user=user.username, + created=created + ) + return + + pwd_last_set: datetime = attributes.get(attribute_name, datetime.now()) + pwd_last_set = pwd_last_set.replace(tzinfo=UTC) + if created or pwd_last_set > user.password_change_date: + self.message(f"'{user.username}': Reset user's password") + self._logger.debug( + "Reset user's password", + user=user.username, + created=created, + pwd_last_set=pwd_last_set, + ) + user.set_unusable_password() + user.save() + def message(self, *args, **kwargs): """Add message that is later added to the System Task and shown to the user""" formatted_message = " ".join(args) diff --git a/authentik/sources/ldap/sync/vendor/freeipa.py b/authentik/sources/ldap/sync/vendor/freeipa.py index df08a55a5574..ebca5e356a23 100644 --- a/authentik/sources/ldap/sync/vendor/freeipa.py +++ b/authentik/sources/ldap/sync/vendor/freeipa.py @@ -1,7 +1,6 @@ """FreeIPA specific""" from collections.abc import Generator -from datetime import UTC, datetime from typing import Any from authentik.core.models import User @@ -20,27 +19,9 @@ def get_objects(self, **kwargs) -> Generator: yield None def sync(self, attributes: dict[str, Any], user: User, created: bool): - self.check_pwd_last_set(attributes, user, created) + self.check_pwd_last_set("krbLastPwdChange", attributes, user, created) self.check_nsaccountlock(attributes, user) - def check_pwd_last_set(self, attributes: dict[str, Any], user: User, created: bool): - """Check krbLastPwdChange""" - if "krbLastPwdChange" not in attributes: - return - pwd_last_set: datetime = attributes.get("krbLastPwdChange", datetime.now()) - pwd_last_set = pwd_last_set.replace(tzinfo=UTC) - if created or pwd_last_set > user.password_change_date: - self.message(f"'{user.username}': Reset user's password") - self._logger.debug( - "Reset user's password", - user=user.username, - created=created, - pwd_last_set=pwd_last_set, - ) - user.set_unusable_password() - user.password_change_date = pwd_last_set - user.save() - def check_nsaccountlock(self, attributes: dict[str, Any], user: User): """https://www.port389.org/docs/389ds/howto/howto-account-inactivation.html""" # This is more of a 389-ds quirk rather than FreeIPA, but FreeIPA uses diff --git a/authentik/sources/ldap/sync/vendor/ms_ad.py b/authentik/sources/ldap/sync/vendor/ms_ad.py index 5a4557be232a..520c834c4596 100644 --- a/authentik/sources/ldap/sync/vendor/ms_ad.py +++ b/authentik/sources/ldap/sync/vendor/ms_ad.py @@ -50,27 +50,9 @@ def get_objects(self, **kwargs) -> Generator: yield None def sync(self, attributes: dict[str, Any], user: User, created: bool): - self.ms_check_pwd_last_set(attributes, user, created) + self.check_pwd_last_set("pwdLastSet", attributes, user, created) self.ms_check_uac(attributes, user) - def ms_check_pwd_last_set(self, attributes: dict[str, Any], user: User, created: bool): - """Check pwdLastSet""" - if "pwdLastSet" not in attributes: - return - pwd_last_set: datetime = attributes.get("pwdLastSet", datetime.now()) - pwd_last_set = pwd_last_set.replace(tzinfo=UTC) - if created or pwd_last_set > user.password_change_date: - self.message(f"'{user.username}': Reset user's password") - self._logger.debug( - "Reset user's password", - user=user.username, - created=created, - pwd_last_set=pwd_last_set, - ) - user.set_unusable_password() - user.password_change_date = pwd_last_set - user.save() - def ms_check_uac(self, attributes: dict[str, Any], user: User): """Check userAccountControl""" if "userAccountControl" not in attributes: From 1eb2e1946b8b958b38e5fcc683a8a139c4e1f232 Mon Sep 17 00:00:00 2001 From: Tom Grassmann Date: Thu, 7 Nov 2024 10:30:24 +0100 Subject: [PATCH 3/5] Override set_unusable_password in user model. Our version always updates the user.password_change_date either to now or a given value --- authentik/core/models.py | 11 +++++++++++ authentik/sources/ldap/sync/base.py | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/authentik/core/models.py b/authentik/core/models.py index 85e8901ed1bf..5108933dc877 100644 --- a/authentik/core/models.py +++ b/authentik/core/models.py @@ -356,6 +356,17 @@ def setter(raw_password): return check_password(raw_password, self.password, setter) + def set_unusable_password(self, change_datetime: datetime = None): + """ + In addition to the base version this also updates the password change date. + @param change_datetime: Use this value for the change time instead of now() + """ + if change_datetime is None: + change_datetime = now() + self.password_change_date = change_datetime + super().set_unusable_password() + + @property def uid(self) -> str: """Generate a globally unique UID, based on the user ID and the hashed secret key""" diff --git a/authentik/sources/ldap/sync/base.py b/authentik/sources/ldap/sync/base.py index c95f48191686..934ef9ab83dc 100644 --- a/authentik/sources/ldap/sync/base.py +++ b/authentik/sources/ldap/sync/base.py @@ -78,7 +78,7 @@ def check_pwd_last_set(self, attribute_name: str, attributes: dict[str, Any], us @param attribute_name: The name of the ldap attribute holding the information when the password was changed @param attributes: All ldap attributes @param user: The user object we are currently syncing - @param created: True if the user is newly created + @param created: True, if the user is newly created @return: """ if attribute_name not in attributes: @@ -100,7 +100,7 @@ def check_pwd_last_set(self, attribute_name: str, attributes: dict[str, Any], us created=created, pwd_last_set=pwd_last_set, ) - user.set_unusable_password() + user.set_unusable_password(change_datetime=pwd_last_set) user.save() def message(self, *args, **kwargs): From e0541fee5343f0672ff44fdaa165cc9c24ffac37 Mon Sep 17 00:00:00 2001 From: Tom Grassmann Date: Thu, 7 Nov 2024 11:44:26 +0100 Subject: [PATCH 4/5] lint --- authentik/core/models.py | 1 - authentik/sources/ldap/sync/base.py | 14 +++++++++----- authentik/sources/ldap/sync/vendor/ms_ad.py | 1 - 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/authentik/core/models.py b/authentik/core/models.py index 5108933dc877..c796b8773da7 100644 --- a/authentik/core/models.py +++ b/authentik/core/models.py @@ -366,7 +366,6 @@ def set_unusable_password(self, change_datetime: datetime = None): self.password_change_date = change_datetime super().set_unusable_password() - @property def uid(self) -> str: """Generate a globally unique UID, based on the user ID and the hashed secret key""" diff --git a/authentik/sources/ldap/sync/base.py b/authentik/sources/ldap/sync/base.py index 934ef9ab83dc..4e8a653a14ee 100644 --- a/authentik/sources/ldap/sync/base.py +++ b/authentik/sources/ldap/sync/base.py @@ -1,7 +1,7 @@ """Sync LDAP Users and groups into authentik""" from collections.abc import Generator -from datetime import datetime, UTC +from datetime import UTC, datetime from typing import Any from django.conf import settings @@ -68,14 +68,18 @@ def base_dn_groups(self) -> str: return f"{self._source.additional_group_dn},{self._source.base_dn}" return self._source.base_dn - def check_pwd_last_set(self, attribute_name: str, attributes: dict[str, Any], user: User, created: bool): + def check_pwd_last_set( + self, attribute_name: str, attributes: dict[str, Any], user: User, created: bool + ): """ Test if the ldap password is newer than the authentik password. If the ldap password is newer set the user password to an unusable password. This ends all users sessions and forces the user to relogin. - During next user login the used authentication backend MAY choose to write a new usable user password. + During next user login the used authentication backend MAY choose to write a new usable user + password. - @param attribute_name: The name of the ldap attribute holding the information when the password was changed + @param attribute_name: The name of the ldap attribute holding the information when + the password was changed @param attributes: All ldap attributes @param user: The user object we are currently syncing @param created: True, if the user is newly created @@ -86,7 +90,7 @@ def check_pwd_last_set(self, attribute_name: str, attributes: dict[str, Any], us f"Missing attribute {attribute_name}. Can not test if a newer ldap password is set." f"Ldap and authentik passwords may be out of sync.", user=user.username, - created=created + created=created, ) return diff --git a/authentik/sources/ldap/sync/vendor/ms_ad.py b/authentik/sources/ldap/sync/vendor/ms_ad.py index 520c834c4596..44dd0dd3f3ab 100644 --- a/authentik/sources/ldap/sync/vendor/ms_ad.py +++ b/authentik/sources/ldap/sync/vendor/ms_ad.py @@ -1,7 +1,6 @@ """Active Directory specific""" from collections.abc import Generator -from datetime import UTC, datetime from enum import IntFlag from typing import Any From 701f85d4dd47603b78a7e42a8d848d0fb7ec51ca Mon Sep 17 00:00:00 2001 From: Tom Grassmann Date: Mon, 11 Nov 2024 20:25:48 +0100 Subject: [PATCH 5/5] Ensure that during user create, the changed_password_date does not change after the event is logged --- authentik/core/api/users.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/authentik/core/api/users.py b/authentik/core/api/users.py index 37656933d5ce..a3671377ef15 100644 --- a/authentik/core/api/users.py +++ b/authentik/core/api/users.py @@ -163,7 +163,8 @@ def create(self, validated_data: dict) -> User: ) validated_data["user_permissions"] = permissions instance: User = super().create(validated_data) - self._set_password(instance, password) + # use keep_date=True, so change_password_date is the same as in log event + self._set_password(instance, password, keep_date=True) return instance def update(self, instance: User, validated_data: dict) -> User: @@ -178,14 +179,17 @@ def update(self, instance: User, validated_data: dict) -> User: self._set_password(instance, password) return instance - def _set_password(self, instance: User, password: str | None): + def _set_password(self, instance: User, password: str | None, keep_date: bool = False): """Set password of user if we're in a blueprint context, and if it's an empty string then use an unusable password""" if SERIALIZER_CONTEXT_BLUEPRINT in self.context and password: instance.set_password(password) instance.save() if len(instance.password) == 0: - instance.set_unusable_password() + if keep_date: + instance.set_unusable_password(change_datetime=instance.password_change_date) + else: + instance.set_unusable_password() instance.save() def get_avatar(self, user: User) -> str: