diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index e92fb75c4fb6dbfa942799a43bcd260a08e30be4..cf68dff51a9aa59c825fa21573aa4f3bc899223c 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -14,6 +14,7 @@ Fixed
* Remove left-over reference to preferences in a form definition that caused
form extensions in downstream apps to break
+* Allow non-LDAP users to authenticate if LDAP is used with password handling
`2.4`_ – 2021-12-24
-------------------
diff --git a/aleksis/core/migrations/0030_user_attributes.py b/aleksis/core/migrations/0030_user_attributes.py
new file mode 100644
index 0000000000000000000000000000000000000000..38434842aae54f5f4ad004094c33c524dd0a69ba
--- /dev/null
+++ b/aleksis/core/migrations/0030_user_attributes.py
@@ -0,0 +1,46 @@
+# Generated by Django 3.2.10 on 2021-12-25 10:59
+
+import aleksis.core.mixins
+from django.conf import settings
+from django.contrib.auth import get_user_model
+from django.db import migrations, models
+import django.db.models.deletion
+
+
+def assume_ldap_authenticated_true(apps, schema_editor):
+ """Set ldap_authenticated user attribute to True to protect existing sites."""
+ if not hasattr(settings, "AUTH_LDAP_SERVER_URI"):
+ # Skip if LDAP is not used on site
+ return
+
+ User = get_user_model()
+ UserAdditionalAttributes = apps.get_model("core", "UserAdditionalAttributes")
+
+ db_alias = schema_editor.connection.alias
+
+ attributes = [
+ UserAdditionalAttributes(user_id=user.pk, attributes={"ldap_authenticated": True})
+ for user in User.objects.using(db_alias).all()
+ ]
+ UserAdditionalAttributes.objects.using(db_alias).bulk_create(attributes)
+
+
+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),
+ ),
+ migrations.RunPython(assume_ldap_authenticated_true, lambda a, s: None),
+ ]
diff --git a/aleksis/core/models.py b/aleksis/core/models.py
index aa20122bff05c234df71ef99db5a6fda318e10cd..73ffab18ab217e1698ccc4fbdea9b25c1de97dfb 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/tests/views/test_account.py b/aleksis/core/tests/views/test_account.py
index 28686eabf8290a7a72444d8fe5f4b71bb74f4626..ae598ab72826d403e94531ef2bf7194b4ee5225c 100644
--- a/aleksis/core/tests/views/test_account.py
+++ b/aleksis/core/tests/views/test_account.py
@@ -1,10 +1,23 @@
from django.conf import settings
+from django.test import override_settings
from django.urls import reverse
+import ldap
import pytest
+from django_auth_ldap.config import LDAPSearch
+
+from aleksis.core.models import UserAdditionalAttributes
pytestmark = pytest.mark.django_db
+LDAP_BASE = "dc=example,dc=com"
+LDAP_SETTINGS = {
+ "AUTH_LDAP_GLOBAL_OPTIONS": {
+ ldap.OPT_NETWORK_TIMEOUT: 1,
+ },
+ "AUTH_LDAP_USER_SEARCH": LDAPSearch(LDAP_BASE, ldap.SCOPE_SUBTREE),
+}
+
def test_index_not_logged_in(client):
response = client.get("/")
@@ -40,3 +53,34 @@ def test_logout(client, django_user_model):
assert response.status_code == 200
assert "Please login to see this page." in response.content.decode("utf-8")
+
+
+@override_settings(
+ AUTHENTICATION_BACKENDS=[
+ "aleksis.core.util.ldap.LDAPBackend",
+ "django.contrib.auth.backends.ModelBackend",
+ ],
+ AUTH_LDAP_SERVER_URI="ldap://[100::0]",
+ AUTH_LDAP_SET_USABLE_PASSWORD=True,
+ **LDAP_SETTINGS
+)
+def test_login_ldap_fail_if_previously_ldap_authenticated(client, django_user_model):
+ username = "foo"
+ password = "bar"
+
+ django_user_model.objects.create_user(username=username, password=password)
+
+ # Logging in with a fresh account should success
+ res = client.login(username=username, password=password)
+ assert res
+ client.get(reverse("logout"), follow=True)
+
+ # Logging in with a previously LDAP-authenticated account should fail
+ UserAdditionalAttributes.set_user_attribute(username, "ldap_authenticated", True)
+ res = client.login(username=username, password=password)
+ assert not res
+
+ # Explicitly noting account has not been used with LDAP should succeed
+ UserAdditionalAttributes.set_user_attribute(username, "ldap_authenticated", False)
+ res = client.login(username=username, password=password)
+ assert res
diff --git a/aleksis/core/util/ldap.py b/aleksis/core/util/ldap.py
index 96b058ac0060975cdd08281be94ba4df371aa7db..b60c9de98a48ca8b34a1cbd758301f0542cc9116 100644
--- a/aleksis/core/util/ldap.py
+++ b/aleksis/core/util/ldap.py
@@ -4,6 +4,8 @@ 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 +26,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
diff --git a/tox.ini b/tox.ini
index 6ba5d926ea520a574125f4a9717b93ba0020a2c3..4819e5b95946e3f021429e75c36fe13e7dd61d25 100644
--- a/tox.ini
+++ b/tox.ini
@@ -9,7 +9,7 @@ whitelist_externals = poetry
skip_install = true
envdir = {toxworkdir}/globalenv
commands_pre =
- poetry install
+ poetry install -E ldap
poetry run aleksis-admin yarn install
poetry run aleksis-admin collectstatic --no-input
commands =