From 534e3ec13254a5a4f517914fb25c20bca0816e56 Mon Sep 17 00:00:00 2001
From: Dominik George <dominik.george@teckids.org>
Date: Sat, 25 Dec 2021 12:06:17 +0100
Subject: [PATCH] Allow using local Django accounts and LDAP accounts at the
same time
This fixes #470, where local Django accoutns were generally locked if
LDAP accoutns were used together with password handling to protect
against deleted/locked LDAP users being able to still login using a
shadow copy of their account in the Django database.
The fix introduces user account attributes, and the LDAP
authentication code keeps a record of users who used to authenticate
with LDAP in the past. If a suer is known to have been using LDAP in
the past, they are denied if they cannot be authenticated in the
future; if a user tries to authenticate who has not used LDAP in the
past, they are allowed in.
---
.../core/migrations/0030_user_attributes.py | 26 ++++++++++++
aleksis/core/models.py | 42 ++++++++++++++++++-
aleksis/core/util/ldap.py | 24 ++++++++++-
3 files changed, 89 insertions(+), 3 deletions(-)
create mode 100644 aleksis/core/migrations/0030_user_attributes.py
diff --git a/aleksis/core/migrations/0030_user_attributes.py b/aleksis/core/migrations/0030_user_attributes.py
new file mode 100644
index 000000000..eb9bac333
--- /dev/null
+++ b/aleksis/core/migrations/0030_user_attributes.py
@@ -0,0 +1,26 @@
+# Generated by Django 3.2.10 on 2021-12-25 10:59
+
+import aleksis.core.mixins
+from django.conf import settings
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+class Migration(migrations.Migration):
+
+ dependencies = [
+ migrations.swappable_dependency(settings.AUTH_USER_MODEL),
+ ('core', '0029_invitations'),
+ ]
+
+ operations = [
+ migrations.CreateModel(
+ name='UserAdditionalAttributes',
+ fields=[
+ ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
+ ('attributes', models.JSONField(default=dict, verbose_name='Additional attributes')),
+ ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, related_name='additional_attributes', to=settings.AUTH_USER_MODEL, verbose_name='Linked user')),
+ ],
+ bases=(models.Model, aleksis.core.mixins.PureDjangoModel),
+ ),
+ ]
diff --git a/aleksis/core/models.py b/aleksis/core/models.py
index aa20122bf..73ffab18a 100644
--- a/aleksis/core/models.py
+++ b/aleksis/core/models.py
@@ -1,7 +1,7 @@
# flake8: noqa: DJ01
import hmac
from datetime import date, datetime, timedelta
-from typing import Iterable, List, Optional, Sequence, Union
+from typing import Any, Iterable, List, Optional, Sequence, Union
from urllib.parse import urlparse
from django.conf import settings
@@ -1131,6 +1131,46 @@ class TaskUserAssignment(ExtensibleModel):
verbose_name_plural = _("Task user assignments")
+class UserAdditionalAttributes(models.Model, PureDjangoModel):
+ """Additional attributes for Django user accounts.
+
+ These attributes are explicitly linked to a User, not to a Person.
+ """
+
+ user = models.OneToOneField(
+ get_user_model(),
+ on_delete=models.CASCADE,
+ related_name="additional_attributes",
+ verbose_name=_("Linked user"),
+ )
+
+ attributes = models.JSONField(verbose_name=_("Additional attributes"), default=dict)
+
+ @classmethod
+ def get_user_attribute(
+ cls, username: str, attribute: str, default: Optional[Any] = None
+ ) -> Any:
+ """Get a user attribute for a user by name."""
+ try:
+ attributes = cls.objects.get(user__username=username)
+ except cls.DoesNotExist:
+ return default
+
+ return attributes.attributes.get(attribute, default)
+
+ @classmethod
+ def set_user_attribute(cls, username: str, attribute: str, value: Any):
+ """Set a user attribute for a user by name.
+
+ Raises DoesNotExist if a username for a non-existing Django user is passed.
+ """
+ user = get_user_model().objects.get(username=username)
+ attributes, __ = cls.objects.update_or_create(user=user)
+
+ attributes.attributes[attribute] = value
+ attributes.save()
+
+
class OAuthApplication(AbstractApplication):
"""Modified OAuth application class that supports Grant Flows configured in preferences."""
diff --git a/aleksis/core/util/ldap.py b/aleksis/core/util/ldap.py
index 96b058ac0..8dc0b8db4 100644
--- a/aleksis/core/util/ldap.py
+++ b/aleksis/core/util/ldap.py
@@ -1,9 +1,12 @@
"""Utilities and extensions for django_auth_ldap."""
+from django.contrib.auth import get_user_model
from django.core.exceptions import PermissionDenied
from django_auth_ldap.backend import LDAPBackend as _LDAPBackend
+from ..models import UserAdditionalAttributes
+
class LDAPBackend(_LDAPBackend):
default_settings = {"SET_USABLE_PASSWORD": False}
@@ -24,11 +27,28 @@ class LDAPBackend(_LDAPBackend):
if self.settings.SET_USABLE_PASSWORD:
if not user:
- # Fail early and do not try other backends
- raise PermissionDenied("LDAP failed to authenticate user")
+ # The user could not be authenticated against LDAP.
+ # We need to make sure to let other backends handle it, but also that
+ # we do not let actually deleted/locked LDAP users fall through to a
+ # backend that cached a valid password
+ if UserAdditionalAttributes.get_user_attribute(
+ ldap_user._username, "ldap_authenticated", False
+ ):
+ # User was LDAP-authenticated in the past, so we fail authentication now
+ # to not let other backends override a legitimate deletion
+ raise PermissionDenied("LDAP failed to authenticate user")
+ else:
+ # No note about LDAP authentication in the past
+ # The user can continue authentication like before if they exist
+ return user
# Set a usable password so users can change their LDAP password
user.set_password(password)
user.save()
+ # Not that we LDAP-autenticated the user so we can check this in the future
+ UserAdditionalAttributes.set_user_attribute(
+ ldap_user._username, "ldap_authenticated", True
+ )
+
return user
--
GitLab