From 86ce546a178cd8a1272264dbaad3187240aae0d0 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 22:05:29 -0500 Subject: [PATCH] Profile: read-only name for cn-named directory entries slapd refuses to remove an RDN value, and every legacy account is cn=,ou=People, so a name edit there always failed. Those accounts now see their name read-only (the view rejects the profile action and update_profile refuses as a backstop); uid-named accounts from website registration can still edit it. README documents the cn/mail self-write ACL and mail uniqueness overlay now applied in production. Co-Authored-By: Claude Opus 5.5 --- README.md | 8 +++++- membership/members/profiles.py | 21 +++++++++++++++ membership/members/tests/test_profiles.py | 31 +++++++++++++++++++++++ membership/members/views.py | 4 +-- membership/templates/profile.html | 8 ++++++ 5 files changed, 69 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 41bb1e7..aa97ea4 100644 --- a/README.md +++ b/README.md @@ -394,7 +394,10 @@ Other directory attributes (photos, SSH keys, addresses, `displayName`, etc.) are neither shown nor modified, so existing values are left in place. If the connector's `nameAttr` is changed, change `profiles.TEXT_ATTRIBUTES` to match. Saving the name replaces every `cn` value with the single entered value; Dex -already uses only the first. Account DNs must not use `cn` as their RDN. +already uses only the first. slapd refuses to remove an RDN value, so accounts +whose DN is named by `cn` (the legacy `cn=,ou=People` entries) see their +name read-only; renaming them would change DNs other services store. Accounts +created by website registration are `uid`-named and can edit their name. **Directory prerequisites:** @@ -404,6 +407,9 @@ already uses only the first. Account DNs must not use `cn` as their RDN. Accounts are pinned to immutable UUIDs; missing/deleted entries do not fall back to a reused username. LDAP unavailability disables editing rather than presenting an empty form that could erase values. +- Example ACL (insert before the catch-all rule), plus the `unique` overlay with + `olcUniqueURI: ldap:///dc=eom,dc=dev?mail?sub`: + `to attrs=cn,mail by self write by dn="cn=admin,dc=eom,dc=dev" write by dn="cn=readonly,dc=eom,dc=dev" read by * none` - Every write binds with the member's **current LDAP password** and checks that entry's UUID again. Configure directory **self-write ACLs** only for `cn`, `mail` and self Password Modify; do not grant writes to `uid`, diff --git a/membership/members/profiles.py b/membership/members/profiles.py index 27ca425..1582ecf 100755 --- a/membership/members/profiles.py +++ b/membership/members/profiles.py @@ -16,6 +16,7 @@ from ldap3.core.exceptions import LDAPException from ldap3.operation.search import compile_filter, parse_filter from ldap3.protocol.controls import build_control from ldap3.utils.conv import escape_filter_chars +from ldap3.utils.dn import parse_dn from . import directory from .models import Account @@ -83,6 +84,24 @@ def read_profile(account): return values +def name_editable(dn): + """False when cn names the entry: slapd refuses to remove an RDN value. + + Legacy accounts are cn=,ou=People; renaming them would change DNs + that other services store, so their name is shown read-only instead. + """ + try: + components = parse_dn(dn) + except Exception: + return False + for attribute, _value, separator in components: + if attribute.casefold() in {"cn", "2.5.4.3"}: + return False + if separator != "+": + return True + return False + + def revision(account, values): return signing.dumps({"account": str(account.pk), "uuid": account.ldap_uuid, "digest": _digest(values)}, salt="ldap-profile") @@ -118,6 +137,8 @@ def update_profile(account, cleaned, expected_revision): with _authorized(account, cleaned["current_password"]) as (conn, values): if proof != {"account": str(account.pk), "uuid": account.ldap_uuid, "digest": _digest(values)}: raise ValueError("Your profile changed. Reload it before saving.") + if not name_editable(account.ldap_dn): + raise ValueError("This account's name is part of its directory entry name.") changes = {} for name in TEXT_ATTRIBUTES: new_values = cleaned[name] diff --git a/membership/members/tests/test_profiles.py b/membership/members/tests/test_profiles.py index f71170c..f32561f 100755 --- a/membership/members/tests/test_profiles.py +++ b/membership/members/tests/test_profiles.py @@ -81,6 +81,17 @@ class ProfileFormTests(SimpleTestCase): self.assertEqual(profiles.initial(profile_values(cn=["Alice", "Alice Example"])), {"cn": "Alice"}) self.assertEqual(profiles.initial(profile_values(cn=[])), {"cn": ""}) + def test_name_editable_only_when_cn_does_not_name_the_entry(self): + for dn, editable in ( + (ALICE_DN, True), ("uid=bob,ou=People,dc=eom,dc=dev", True), + ("uid=bob,cn=Group,dc=eom,dc=dev", True), + ("cn=Logan Fick,ou=People,dc=eom,dc=dev", False), ("CN=x,dc=eom,dc=dev", False), + ("uid=bob+cn=Bob,ou=People,dc=eom,dc=dev", False), ("2.5.4.3=x,dc=eom,dc=dev", False), + ("", False), ("not a dn", False), + ): + with self.subTest(dn=dn): + self.assertIs(profiles.name_editable(dn), editable) + def test_field_groups_and_unique_password_ids(self): form = self.form() grouped = [*form.identity_fields(), *form.authorization_fields()] @@ -481,6 +492,19 @@ class ProfileViewTests(TestCase): self.account.refresh_from_db() self.assertEqual(self.account.email, "alice@example.org") + def test_cn_named_account_sees_read_only_name_and_cannot_post(self): + Account.objects.filter(pk=self.account.pk).update(ldap_dn="cn=Alice Example," + SEARCH_DN) + self.read.side_effect = lambda account: copy.deepcopy(self.values) + response = self.client.get(self.profile_url) + self.assertFalse(response.context["name_editable"]) + self.assertContains(response, "Alice Example") + self.assertNotContains(response, 'name="cn"') + self.assertNotContains(response, 'value="profile"') + self.assertContains(response, 'value="email"') + self.assertContains(response, 'value="password"') + self.assertEqual(self.client.post(self.profile_url, self.profile_data(cn="New Name")).status_code, 400) + self.assert_no_writes() + def test_photo_endpoint_removed(self): with self.assertRaises(NoReverseMatch): reverse("members:profile_photo") @@ -640,6 +664,13 @@ class ProfileDirectoryTests(TestCase): self.assertEqual(conn.modify.call_args.args, (ALICE_DN, {"cn": [(MODIFY_REPLACE, ["Changed"])]})) self.assert_critical_assertion(conn.modify.call_args) + def test_profile_update_refuses_cn_named_entry(self): + conn, authorized = self.authorized() + self.account.ldap_dn = "cn=Alice Example," + SEARCH_DN + with patch("members.profiles._authorized", authorized), self.assertRaisesRegex(ValueError, "entry name"): + profiles.update_profile(self.account, self.cleaned(cn=["Changed"]), profiles.revision(self.account, self.values)) + conn.modify.assert_not_called() + def test_profile_edit_without_timestamp_fails_closed(self): values = profile_values(modifyTimestamp=[]) conn, authorized = self.authorized(values=values) diff --git a/membership/members/views.py b/membership/members/views.py index 0db01e2..cf08618 100755 --- a/membership/members/views.py +++ b/membership/members/views.py @@ -274,7 +274,7 @@ def logout(request): def _profile_context(account, values): - return {"account": account, "profile_loaded": True, "form": ProfileForm(initial=profiles.initial(values)), "email_form": ProfileEmailForm(), "password_form": ProfilePasswordForm(), "email_values": values["mail"], "revision": profiles.revision(account, values)} + return {"account": account, "profile_loaded": True, "name_editable": profiles.name_editable(account.ldap_dn), "name_values": values["cn"], "form": ProfileForm(initial=profiles.initial(values)), "email_form": ProfileEmailForm(), "password_form": ProfilePasswordForm(), "email_values": values["mail"], "revision": profiles.revision(account, values)} @sensitive_post_parameters() @@ -298,7 +298,7 @@ def profile(request): return HttpResponse("Please try again later.", status=429) action = request.POST.get("action") forms = {"profile": ("form", ProfileForm), "email": ("email_form", ProfileEmailForm), "password": ("password_form", ProfilePasswordForm)} - if action not in forms: + if action not in forms or (action == "profile" and not context["name_editable"]): return HttpResponse(status=400) form_key, form_class = forms[action] form = form_class(request.POST) diff --git a/membership/templates/profile.html b/membership/templates/profile.html index 3ca2810..00f6145 100755 --- a/membership/templates/profile.html +++ b/membership/templates/profile.html @@ -45,6 +45,7 @@ + {% if name_editable %}
{% csrf_token %} @@ -55,6 +56,13 @@ {% include "profile_fields.html" with fields=form.authorization_fields only %}
+ {% else %} +
+

Name

+

{{ name_values|first|default:account.uid }}

+

Your name is part of how your account is stored, so it can't be changed here. Contact eric@eom.dev if it needs updating.

+
+ {% endif %}

Email address

Current email address: