Login
This commit is contained in:
@@ -22,6 +22,7 @@ LDAP_URL=ldaps://ldap.eom.dev
|
||||
# Set to 1 ONLY with ldap:// on a trusted network to skip StartTLS.
|
||||
# Bind and registration passwords will be sent unencrypted. LDAPS stays verified.
|
||||
LDAP_ALLOW_INSECURE=0
|
||||
# Service credentials for registration/worker group sync, not Dex login.
|
||||
LDAP_BIND_DN=
|
||||
LDAP_BIND_PASSWORD=
|
||||
LDAP_CA_CERT_FILE=
|
||||
|
||||
@@ -16,8 +16,10 @@ Both payment methods feed the same LDAP group reconciliation and local expiry.
|
||||
2. Sign-in uses the single Dex LDAP option with Authorization Code + PKCE, state
|
||||
and nonce. Authlib verifies the RS256 ID token, issuer, audience, signature
|
||||
and lifetime. The signed connector/UID must match the configured connector
|
||||
and LDAP identity; account ownership is anchored to immutable `entryUUID`,
|
||||
never email alone.
|
||||
and username. Local account ownership is pinned to the verified Dex issuer
|
||||
and subject, never email alone. Membership makes **no LDAP connection during
|
||||
login**, including first login by an existing Dex user. The worker separately
|
||||
resolves and pins LDAP `entryUUID` before synchronizing paid groups.
|
||||
3. Authenticated users choose a server-allowlisted recurring Stripe Price.
|
||||
Stripe hosts Checkout and the billing portal. Return URLs never grant access.
|
||||
4. Stripe webhooks are signature-verified and durably deduplicated. The worker
|
||||
@@ -116,8 +118,8 @@ of these five prices when inspected.
|
||||
preserve subjects in other apps. Add the website `inetOrgPerson` branch to the
|
||||
LDAP filter; this allows those entries to authenticate to all Dex clients, so
|
||||
review that scope. Dex uses signed `federated:id` claims, while the app resolves
|
||||
UID to LDAP and checks immutable `entryUUID`. RS256 and all discovery endpoints
|
||||
must be HTTPS on the issuer's origin. Existing lowercase LDAP usernames
|
||||
UID from verified claims without querying LDAP during login. RS256 and all
|
||||
discovery endpoints must be HTTPS on the issuer's origin. Existing lowercase LDAP usernames
|
||||
matching the registration syntax can sign in without signup.
|
||||
5. The supplied tier file contains live Price IDs. Set `STRIPE_LIVE_MODE=1` only
|
||||
with the matching live Stripe API key. For sandbox testing, override
|
||||
@@ -167,6 +169,64 @@ No production credentials or live account changes are made by the test suite.
|
||||
|
||||
## Operations and limitations
|
||||
|
||||
### Dex-only login and upgrade
|
||||
|
||||
Deploy the new membership image and apply migration `0004_dex_login_identity`
|
||||
before serving it. **Keep `DEX_ISSUER` unchanged during the upgrade**: the
|
||||
migration pins existing non-null subjects to the currently configured issuer,
|
||||
because older records did not store their issuer. Back up the database and
|
||||
coordinate replacing web and worker together; older code cannot handle accounts
|
||||
whose directory UUID has not yet been resolved. Do not roll back to the old
|
||||
schema while unresolved accounts exist.
|
||||
|
||||
- Returning users sign in using their existing issuer/subject binding, without
|
||||
LDAP or payment-provider calls. Signed verified email updates their contact
|
||||
address; it never identifies an account or attaches billing history.
|
||||
- First-time Dex users with no local username collision receive a free local
|
||||
account. The worker discovers their LDAP DN/UUID on its next successful group
|
||||
sync and thereafter uses only that UUID. Missing/ambiguous directory entries
|
||||
stop synchronization; a missing pinned UUID never falls back to username.
|
||||
- Website registration still provisions LDAP. Its browser session retains a
|
||||
one-hour, server-side proof permitting the first Dex login to bind only the
|
||||
just-created local account, with the same signed connector/username. Successful
|
||||
login consumes that proof. Without it, existing unbound accounts require an
|
||||
administrator mapping; matching email or username alone is insufficient.
|
||||
- For an existing **unbound** account, the management command `bind_dex_identity`
|
||||
requires `--account` (local account UUID), `--uid` and `--subject` (the exact
|
||||
Dex `sub` for this issuer/client/connector). Independently verify the identity
|
||||
with trusted Dex/directory administration before running it. Do not accept
|
||||
subjects asserted by users or decode an unverified token as proof. It makes
|
||||
no LDAP calls, preserves directory/billing data and refuses to replace an
|
||||
existing binding or reuse another account's subject. Lost registration proof
|
||||
can be recovered through this reviewed mapping, not by repeating registration.
|
||||
- Issuer, connector, username and subject changes require an explicit identity
|
||||
migration. The old automatic rebinding of retired `www-ldap` subjects is
|
||||
intentionally removed; review those bindings separately before deployment.
|
||||
|
||||
The existing Dex LDAP connector uses `idAttr: uid`. **Do not recycle usernames**:
|
||||
Dex itself cannot distinguish a recreated LDAP entry with the same connector/uid
|
||||
from the previous identity. Directory UUID pinning still protects group writes,
|
||||
but membership no longer checks directory recreation during login. Preserve the
|
||||
connector mapping for other services; migrating Dex to immutable user IDs is a
|
||||
separate coordinated change, not an automatic part of this release.
|
||||
|
||||
### LDAP service credentials still required for directory writes
|
||||
|
||||
LDAP is still needed for registration and worker group synchronization. Configure
|
||||
the least-privilege `LDAP_BIND_DN` and its **plaintext bind password** in
|
||||
`LDAP_BIND_PASSWORD`, not a stored `{CRYPT}$1$...` hash or already-base64-encoded
|
||||
value. Kubernetes Secret `data` encodes original values once. Check service
|
||||
password rotation/account locks and redeploy web and worker after Secret updates;
|
||||
existing pods do not refresh environment credentials automatically. Never use
|
||||
root/admin credentials or disable TLS verification to bypass failures.
|
||||
|
||||
Older images may log `LDAP service bind rejected during Dex callback` or
|
||||
`Dex callback rejected kind=LDAPInvalidCredentialsResult`. These login-time
|
||||
service binds are removed by this release. OpenLDAP remains responsible for
|
||||
verifying users' existing password hashes through Dex. `LDAP_PASSWORD_SCHEME`
|
||||
only gates new registration; its Argon2 requirement is unchanged. Plan migration
|
||||
away from weak legacy password hashes separately.
|
||||
|
||||
### SHKeeper prepaid crypto
|
||||
|
||||
Stripe remains the automatically recurring payment method. SHKeeper accepts
|
||||
|
||||
@@ -105,7 +105,7 @@ def apply_groups(account):
|
||||
account.ldap_dn = reconcile_groups(account, desired, managed) if managed else account.ldap_dn
|
||||
account.last_ldap_sync = now
|
||||
account.membership_level = membership_level
|
||||
account.save(update_fields=["ldap_dn", "last_ldap_sync", "membership_level"])
|
||||
account.save(update_fields=["ldap_dn", "ldap_uuid", "last_ldap_sync", "membership_level"])
|
||||
return len(desired)
|
||||
|
||||
|
||||
|
||||
@@ -8,10 +8,16 @@ from urllib.parse import urlsplit
|
||||
from django.conf import settings
|
||||
from django.core.exceptions import ImproperlyConfigured
|
||||
from ldap3 import BASE, MODIFY_ADD, MODIFY_DELETE, Connection, Server, Tls
|
||||
from ldap3.core.exceptions import LDAPException
|
||||
from ldap3.core.exceptions import LDAPException, LDAPInvalidCredentialsResult
|
||||
from ldap3.utils.conv import escape_filter_chars
|
||||
from ldap3.utils.dn import escape_rdn
|
||||
|
||||
from .models import Account
|
||||
|
||||
|
||||
class LDAPServiceBindError(LDAPException):
|
||||
"""The application's LDAP service credentials were rejected, not a user's."""
|
||||
|
||||
|
||||
@contextmanager
|
||||
def connection():
|
||||
@@ -25,7 +31,12 @@ def connection():
|
||||
# Plaintext requires explicit operator consent; LDAPS is never downgraded.
|
||||
if url.scheme == "ldap" and not settings.LDAP_ALLOW_INSECURE and not conn.start_tls():
|
||||
raise LDAPException("StartTLS failed; refusing to bind.")
|
||||
if not conn.bind():
|
||||
try:
|
||||
bound = conn.bind()
|
||||
except LDAPInvalidCredentialsResult:
|
||||
# Do not expose server diagnostics or retry with a user's password.
|
||||
raise LDAPServiceBindError("Check LDAP_BIND_DN and LDAP_BIND_PASSWORD: use the service account's plaintext password, not its stored hash.") from None
|
||||
if not bound:
|
||||
raise LDAPException("LDAP bind failed.")
|
||||
yield conn
|
||||
finally:
|
||||
@@ -41,16 +52,6 @@ def _identity(entry):
|
||||
return {"uid": str(entry.uid.value), "email": str(entry.mail.value), "ldap_dn": entry.entry_dn, "ldap_uuid": str(entry.entryUUID.value)}
|
||||
|
||||
|
||||
def identity_for_login(uid):
|
||||
if not isinstance(uid, str) or not re.fullmatch(r"[a-z][a-z0-9._-]{2,47}", uid):
|
||||
raise ValueError("Dex must return a valid LDAP uid as preferred_username.")
|
||||
with connection() as conn:
|
||||
entries = _find(conn, f"(&(objectClass=inetOrgPerson)(uid={escape_filter_chars(uid)}))")
|
||||
if len(entries) != 1:
|
||||
raise ValueError("LDAP identity is missing or ambiguous.")
|
||||
return _identity(entries[0])
|
||||
|
||||
|
||||
def create_account(uid, email, password):
|
||||
if settings.LDAP_PASSWORD_SCHEME != "{ARGON2}":
|
||||
raise ImproperlyConfigured("Verify slapd's Argon2 password policy before enabling registration.")
|
||||
@@ -82,9 +83,22 @@ def reconcile_groups(account, desired, managed):
|
||||
if not placeholder:
|
||||
raise ImproperlyConfigured("LDAP_GROUP_PLACEHOLDER_DN is required to revoke the final member.")
|
||||
with connection() as conn:
|
||||
entries = _find(conn, f"(entryUUID={escape_filter_chars(account.ldap_uuid)})")
|
||||
if account.ldap_uuid:
|
||||
query = f"(entryUUID={escape_filter_chars(account.ldap_uuid)})"
|
||||
else:
|
||||
# Only the worker resolves first-time Dex users. Never fall back to
|
||||
# username once an entryUUID has been pinned (including deletions).
|
||||
if account.oidc_issuer != settings.DEX_ISSUER or not account.oidc_sub or not re.fullmatch(r"[a-z][a-z0-9._-]{2,47}", account.uid):
|
||||
raise ValueError("A verified Dex mapping is required to resolve the directory identity.")
|
||||
query = f"(&(objectClass=inetOrgPerson)(uid={escape_filter_chars(account.uid)}))"
|
||||
entries = _find(conn, query)
|
||||
if len(entries) != 1:
|
||||
raise ValueError("Membership identity missing or ambiguous; refusing to modify groups.")
|
||||
identity = _identity(entries[0])
|
||||
if not identity["ldap_uuid"] or identity["ldap_uuid"] == "None" or len(identity["ldap_uuid"]) > 64:
|
||||
raise ValueError("A directory entryUUID is required to synchronize groups.")
|
||||
if not account.ldap_uuid and Account.objects.filter(ldap_uuid=identity["ldap_uuid"]).exclude(pk=account.pk).exists():
|
||||
raise ValueError("Directory entry is already pinned to another local account; refusing to modify groups.")
|
||||
dn = entries[0].entry_dn
|
||||
if placeholder.casefold() == dn.casefold():
|
||||
raise ImproperlyConfigured("The LDAP sentinel must not be a login account.")
|
||||
@@ -107,4 +121,6 @@ def reconcile_groups(account, desired, managed):
|
||||
changes.append((MODIFY_ADD, [dn]))
|
||||
if changes and not conn.modify(group_dn, {"uniqueMember": changes}):
|
||||
raise LDAPException("LDAP group synchronization failed.")
|
||||
# Caller holds the account row lock and persists this pin with sync state.
|
||||
account.ldap_uuid = identity["ldap_uuid"]
|
||||
return dn
|
||||
|
||||
@@ -0,0 +1,38 @@
|
||||
from django.conf import settings
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.core.management.base import BaseCommand, CommandError
|
||||
from django.db import IntegrityError, transaction
|
||||
|
||||
from members.models import Account
|
||||
|
||||
|
||||
class Command(BaseCommand):
|
||||
help = "Explicitly bind an unlinked local account to an independently verified Dex subject."
|
||||
|
||||
def add_arguments(self, parser):
|
||||
parser.add_argument("--account", required=True, help="Local account UUID, not an email address.")
|
||||
parser.add_argument("--uid", required=True, help="Independently verified LDAP connector uid.")
|
||||
parser.add_argument("--subject", required=True, help="Exact verified Dex sub for the configured issuer/connector.")
|
||||
|
||||
def handle(self, *args, **options):
|
||||
subject = options["subject"]
|
||||
if not subject or len(subject) > 512:
|
||||
raise CommandError("A nonempty Dex subject of at most 512 characters is required.")
|
||||
try:
|
||||
with transaction.atomic():
|
||||
account = Account.objects.select_for_update().get(pk=options["account"])
|
||||
if account.uid != options["uid"]:
|
||||
raise CommandError("The verified uid does not match the selected account.")
|
||||
if account.oidc_sub:
|
||||
if account.oidc_sub == subject and account.oidc_issuer == settings.DEX_ISSUER:
|
||||
self.stdout.write("Dex identity already bound.")
|
||||
return
|
||||
raise CommandError("Account already has a Dex binding; automatic rebinding is forbidden.")
|
||||
account.oidc_issuer = settings.DEX_ISSUER
|
||||
account.oidc_sub = subject
|
||||
account.save(update_fields=["oidc_issuer", "oidc_sub"])
|
||||
except (Account.DoesNotExist, ValidationError, ValueError) as exc:
|
||||
raise CommandError("Select an existing account by UUID.") from exc
|
||||
except IntegrityError:
|
||||
raise CommandError("Dex subject is already bound to another account.") from None
|
||||
self.stdout.write(self.style.SUCCESS("Dex identity bound. No LDAP or billing data changed."))
|
||||
@@ -0,0 +1,29 @@
|
||||
from django.conf import settings
|
||||
from django.db import migrations, models
|
||||
|
||||
|
||||
def pin_existing_issuer(apps, schema_editor):
|
||||
# Upgrade with the same issuer that authenticated the existing subjects.
|
||||
Account = apps.get_model("members", "Account")
|
||||
Account.objects.using(schema_editor.connection.alias).filter(
|
||||
oidc_sub__isnull=False,
|
||||
).update(oidc_issuer=settings.DEX_ISSUER)
|
||||
|
||||
|
||||
class Migration(migrations.Migration):
|
||||
dependencies = [("members", "0003_membership_levels")]
|
||||
operations = [
|
||||
migrations.AlterField(
|
||||
model_name="account", name="ldap_dn",
|
||||
field=models.CharField(blank=True, max_length=1024),
|
||||
),
|
||||
migrations.AlterField(
|
||||
model_name="account", name="ldap_uuid",
|
||||
field=models.CharField(blank=True, max_length=64, null=True, unique=True),
|
||||
),
|
||||
migrations.AddField(
|
||||
model_name="account", name="oidc_issuer",
|
||||
field=models.CharField(blank=True, default="", max_length=2048),
|
||||
),
|
||||
migrations.RunPython(pin_existing_issuer, migrations.RunPython.noop),
|
||||
]
|
||||
@@ -7,8 +7,9 @@ class Account(models.Model):
|
||||
id = models.UUIDField(primary_key=True, default=uuid.uuid4, editable=False)
|
||||
uid = models.CharField(max_length=48, unique=True)
|
||||
email = models.EmailField()
|
||||
ldap_dn = models.CharField(max_length=1024)
|
||||
ldap_uuid = models.CharField(max_length=64, unique=True)
|
||||
ldap_dn = models.CharField(max_length=1024, blank=True)
|
||||
ldap_uuid = models.CharField(max_length=64, unique=True, null=True, blank=True)
|
||||
oidc_issuer = models.CharField(max_length=2048, blank=True, default="")
|
||||
oidc_sub = models.CharField(max_length=512, unique=True, null=True)
|
||||
membership_level = models.PositiveSmallIntegerField(default=0)
|
||||
stripe_customer = models.CharField(max_length=128, unique=True, null=True)
|
||||
|
||||
@@ -5,11 +5,14 @@ import re
|
||||
import time
|
||||
from contextlib import contextmanager
|
||||
from datetime import timedelta
|
||||
from io import StringIO
|
||||
from unittest.mock import MagicMock, patch
|
||||
|
||||
from django.conf import settings
|
||||
from django.core import mail
|
||||
from django.test import Client, SimpleTestCase, TestCase, override_settings
|
||||
from django.core.management import call_command
|
||||
from django.core.management.base import CommandError
|
||||
from django.test import Client, TestCase, TransactionTestCase, override_settings
|
||||
from django.utils import timezone
|
||||
from ldap3 import Connection, MOCK_SYNC, OFFLINE_SLAPD_2_4, Server
|
||||
|
||||
@@ -28,6 +31,8 @@ TIERS = {
|
||||
|
||||
|
||||
def account(**kwargs):
|
||||
if kwargs.get("oidc_sub"):
|
||||
kwargs.setdefault("oidc_issuer", settings.DEX_ISSUER)
|
||||
return Account.objects.create(uid="alice", email="alice@example.org", ldap_dn=DN, ldap_uuid="11111111-1111-1111-1111-111111111111", **kwargs)
|
||||
|
||||
|
||||
@@ -70,6 +75,7 @@ class RegistrationTests(TestCase):
|
||||
self.assertFalse(Subscription.objects.exists())
|
||||
self.assertFalse(Registration.objects.exists())
|
||||
self.assertNotIn("account_id", self.client.session)
|
||||
self.assertEqual(self.client.session["registration_identity"]["account_id"], str(Account.objects.get().pk))
|
||||
self.assertNotIn(password, str(list(Account.objects.values())))
|
||||
self.client.post("/members/register/finish/", {"password": password, "confirm_password": password})
|
||||
self.assertEqual(provision.call_count, 1)
|
||||
@@ -134,46 +140,118 @@ class IdentityTests(TestCase):
|
||||
def claims(self, **changes):
|
||||
return {"iss": settings.DEX_ISSUER, "sub": "dex-alice", "preferred_username": "alice", "email_verified": True, "email": "alice@example.org", "federated_claims": {"connector_id": settings.DEX_CONNECTOR_ID, "user_id": "alice"}, **changes}
|
||||
|
||||
@patch("members.views.directory.identity_for_login")
|
||||
def test_existing_directory_account_links_without_password(self, identity):
|
||||
identity.return_value = {"uid": "alice", "email": "alice@example.org", "ldap_dn": DN, "ldap_uuid": "uuid-alice"}
|
||||
@patch("members.directory.connection", side_effect=AssertionError("Login must not contact LDAP"))
|
||||
def test_first_and_returning_dex_login_need_no_ldap(self, connection):
|
||||
first = views.link_identity(self.claims())
|
||||
second = views.link_identity(self.claims(sub="ldap-alice"))
|
||||
second = views.link_identity(self.claims(email="updated@example.org"))
|
||||
self.assertEqual(first.pk, second.pk)
|
||||
first.refresh_from_db()
|
||||
self.assertEqual(first.oidc_sub, "ldap-alice")
|
||||
second.refresh_from_db()
|
||||
self.assertEqual(second.oidc_sub, "dex-alice")
|
||||
self.assertEqual(second.oidc_issuer, settings.DEX_ISSUER)
|
||||
self.assertEqual(second.email, "updated@example.org")
|
||||
self.assertIsNone(second.ldap_uuid)
|
||||
self.assertEqual(second.ldap_dn, "")
|
||||
self.assertEqual(second.membership_level, 0)
|
||||
self.assertEqual(ManagedGroup.objects.filter(account=second).count(), len(TIERS))
|
||||
self.assertFalse(Subscription.objects.exists())
|
||||
views.link_identity(self.claims(sub="next-verified-subject"))
|
||||
first.refresh_from_db()
|
||||
self.assertEqual(first.oidc_sub, "next-verified-subject")
|
||||
connection.assert_not_called()
|
||||
|
||||
@patch("members.views.directory.identity_for_login")
|
||||
def test_no_email_only_or_unverified_link(self, identity):
|
||||
for changes in ({"email_verified": False}, {"iss": "https://evil.example"}, {"sub": ""}):
|
||||
with self.assertRaises(ValueError):
|
||||
def test_invalid_claims_rejected(self):
|
||||
from django.core.exceptions import ValidationError
|
||||
|
||||
for changes in ({"email_verified": False}, {"iss": "https://evil.example"}, {"sub": ""}, {"sub": "x" * 513}, {"preferred_username": "*)(uid=*)"}, {"email": None}, {"email": "invalid"}):
|
||||
with self.subTest(changes=changes), self.assertRaises((ValueError, ValidationError)):
|
||||
views.link_identity(self.claims(**changes))
|
||||
identity.assert_not_called()
|
||||
identity.return_value = {"uid": "alice", "email": "other@example.org", "ldap_dn": DN, "ldap_uuid": "uuid-alice"}
|
||||
with self.assertRaises(ValueError):
|
||||
views.link_identity(self.claims())
|
||||
self.assertFalse(Account.objects.exists())
|
||||
|
||||
@patch("members.views.directory.identity_for_login")
|
||||
def test_other_connector_or_recreated_ldap_account_rejected(self, identity):
|
||||
identity.return_value = {"uid": "alice", "email": "alice@example.org", "ldap_dn": DN, "ldap_uuid": "uuid-alice"}
|
||||
for federated in ({"connector_id": "github", "user_id": "alice"}, {"connector_id": settings.DEX_CONNECTOR_ID, "user_id": "different-uid"}, {}):
|
||||
def test_wrong_or_missing_signed_connector_identity_rejected(self):
|
||||
for federated in ({"connector_id": "github", "user_id": "alice"}, {"connector_id": settings.DEX_CONNECTOR_ID, "user_id": "different-uid"}, {}, ["malformed"]):
|
||||
with self.assertRaises(ValueError):
|
||||
views.link_identity(self.claims(federated_claims=federated))
|
||||
self.assertFalse(Account.objects.exists())
|
||||
|
||||
@patch("members.views.directory.identity_for_login")
|
||||
def test_same_uid_different_uuid_cannot_take_over_account(self, identity):
|
||||
def test_existing_subject_and_directory_pin_preserved(self):
|
||||
original = account(oidc_sub="dex-alice", stripe_customer="cus_existing")
|
||||
linked = views.link_identity(self.claims())
|
||||
self.assertEqual(linked.pk, original.pk)
|
||||
self.assertEqual(linked.ldap_uuid, original.ldap_uuid)
|
||||
self.assertEqual(linked.ldap_dn, original.ldap_dn)
|
||||
self.assertEqual(linked.stripe_customer, "cus_existing")
|
||||
|
||||
def test_different_subject_cannot_take_over_existing_account(self):
|
||||
original = account(oidc_sub="original")
|
||||
identity.return_value = {"uid": "alice", "email": "alice@example.org", "ldap_dn": DN, "ldap_uuid": "new-directory-entry"}
|
||||
with self.assertRaises(ValueError):
|
||||
views.link_identity(self.claims(), registration_account_id=str(original.pk))
|
||||
original.refresh_from_db()
|
||||
self.assertEqual(original.oidc_sub, "original")
|
||||
|
||||
def test_issuer_change_does_not_reuse_subject(self):
|
||||
original = account(oidc_sub="dex-alice", oidc_issuer="https://previous.example/")
|
||||
with self.assertRaises(ValueError):
|
||||
views.link_identity(self.claims())
|
||||
original.refresh_from_db()
|
||||
self.assertEqual(original.oidc_sub, "original")
|
||||
self.assertEqual(original.oidc_issuer, "https://previous.example/")
|
||||
|
||||
def test_email_match_does_not_link_different_identity(self):
|
||||
original = account(oidc_sub="original")
|
||||
other = views.link_identity(self.claims(sub="dex-bob", preferred_username="bob", federated_claims={"connector_id": settings.DEX_CONNECTOR_ID, "user_id": "bob"}))
|
||||
self.assertNotEqual(other.pk, original.pk)
|
||||
self.assertIsNone(other.stripe_customer)
|
||||
|
||||
def test_subject_cannot_be_reused_with_another_uid(self):
|
||||
account(oidc_sub="dex-alice")
|
||||
with self.assertRaises(ValueError):
|
||||
views.link_identity(self.claims(preferred_username="bob", federated_claims={"connector_id": settings.DEX_CONNECTOR_ID, "user_id": "bob"}))
|
||||
self.assertEqual(Account.objects.count(), 1)
|
||||
|
||||
def test_unbound_account_requires_registration_proof(self):
|
||||
original = account()
|
||||
for proof in (None, "another-account"):
|
||||
with self.assertRaises(ValueError):
|
||||
views.link_identity(self.claims(), registration_account_id=proof)
|
||||
linked = views.link_identity(self.claims(), registration_account_id=str(original.pk))
|
||||
self.assertEqual(linked.pk, original.pk)
|
||||
self.assertEqual(linked.ldap_uuid, original.ldap_uuid)
|
||||
self.assertEqual(linked.oidc_sub, "dex-alice")
|
||||
|
||||
@patch("members.views.dex")
|
||||
def test_callback_consumes_registration_proof_and_rejects_expired_proof(self, dex):
|
||||
original = account()
|
||||
dex.return_value.authorize_access_token.return_value = {"id_token": "private-token", "userinfo": self.claims()}
|
||||
session = self.client.session
|
||||
session["registration_identity"] = {"account_id": str(original.pk), "expires_at": time.time() - 10}
|
||||
session.save()
|
||||
self.assertContains(self.client.get("/members/oidc/callback/"), "could not be verified")
|
||||
original.refresh_from_db()
|
||||
self.assertIsNone(original.oidc_sub)
|
||||
session = self.client.session
|
||||
session["registration_identity"] = {"account_id": str(original.pk), "expires_at": time.time() + 300}
|
||||
session.save()
|
||||
self.assertRedirects(self.client.get("/members/oidc/callback/"), "/members/")
|
||||
self.assertEqual(dict(self.client.session), {"account_id": str(original.pk)})
|
||||
|
||||
def test_operator_binding_is_explicit_idempotent_and_cannot_rebind(self):
|
||||
original = account()
|
||||
options = {"account": str(original.pk), "uid": "alice", "subject": "dex-alice", "stdout": StringIO()}
|
||||
with self.assertRaises(CommandError):
|
||||
call_command("bind_dex_identity", **{**options, "uid": "bob"})
|
||||
call_command("bind_dex_identity", **options)
|
||||
call_command("bind_dex_identity", **options)
|
||||
self.assertEqual(views.link_identity(self.claims()).pk, original.pk)
|
||||
with self.assertRaises(CommandError):
|
||||
call_command("bind_dex_identity", **{**options, "subject": "different-sub"})
|
||||
|
||||
def test_operator_cannot_bind_subject_owned_by_another_account(self):
|
||||
original = account()
|
||||
Account.objects.create(uid="bob", email="bob@example.org", oidc_sub="dex-alice", oidc_issuer=settings.DEX_ISSUER)
|
||||
with self.assertRaises(CommandError):
|
||||
call_command("bind_dex_identity", account=str(original.pk), uid="alice", subject="dex-alice", stdout=StringIO())
|
||||
original.refresh_from_db()
|
||||
self.assertIsNone(original.oidc_sub)
|
||||
|
||||
def test_operator_binding_rejects_invalid_account_ids(self):
|
||||
with self.assertRaises(CommandError):
|
||||
call_command("bind_dex_identity", account="not-a-uuid", uid="alice", subject="dex-alice", stdout=StringIO())
|
||||
|
||||
@patch("members.views.dex")
|
||||
@patch("members.views.link_identity")
|
||||
@@ -196,6 +274,18 @@ class IdentityTests(TestCase):
|
||||
self.assertContains(response, "could not be verified")
|
||||
self.assertNotIn("account_id", self.client.session)
|
||||
|
||||
@patch("members.views.dex")
|
||||
@patch("members.directory.Connection")
|
||||
@override_settings(LDAP_URL="", LDAP_BIND_DN="", LDAP_BIND_PASSWORD="", LDAP_PASSWORD_SCHEME="{CRYPT}")
|
||||
def test_callback_works_without_ldap_configuration(self, conn_factory, dex):
|
||||
from ldap3.core.exceptions import LDAPInvalidCredentialsResult
|
||||
|
||||
dex.return_value.authorize_access_token.return_value = {"id_token": "private-token", "userinfo": self.claims()}
|
||||
conn_factory.return_value.bind.side_effect = LDAPInvalidCredentialsResult(description="private LDAP diagnostic")
|
||||
self.assertRedirects(self.client.get("/members/oidc/callback/?code=private-code"), "/members/")
|
||||
self.assertEqual(dict(self.client.session), {"account_id": str(Account.objects.get().pk)})
|
||||
conn_factory.assert_not_called()
|
||||
|
||||
|
||||
@override_settings(MEMBERSHIP_TIERS=TIERS, STRIPE_WEBHOOK_SECRET="whsec_test_only", STRIPE_PORTAL_CONFIGURATION="bpc_test")
|
||||
class BillingViewsTests(TestCase):
|
||||
@@ -438,7 +528,7 @@ class ReconciliationTests(TestCase):
|
||||
|
||||
|
||||
@override_settings(LDAP_SEARCH_BASE_DN=BASE, LDAP_GROUP_PLACEHOLDER_DN=SENTINEL)
|
||||
class DirectoryTests(SimpleTestCase):
|
||||
class DirectoryTests(TestCase):
|
||||
def setUp(self):
|
||||
# ldap3 supports offline schemas; its third-party annotation omits them.
|
||||
server = Server("mock", get_info=OFFLINE_SLAPD_2_4) # pyright: ignore[reportArgumentType]
|
||||
@@ -476,9 +566,54 @@ class DirectoryTests(SimpleTestCase):
|
||||
with self.assertRaises(ValueError):
|
||||
directory.reconcile_groups(self.member, {GROUP}, {GROUP})
|
||||
|
||||
def test_login_uid_rejects_filter_injection(self):
|
||||
with self.assertRaises(ValueError):
|
||||
directory.identity_for_login("*)(uid=*)")
|
||||
def test_worker_resolves_and_pins_first_time_dex_identity(self):
|
||||
self.member.ldap_uuid = None
|
||||
self.member.ldap_dn = ""
|
||||
self.member.oidc_issuer = settings.DEX_ISSUER
|
||||
self.member.oidc_sub = "dex-alice"
|
||||
with patch("members.directory.connection", self.mock_connection), patch.object(self.conn, "search", wraps=self.conn.search) as search:
|
||||
self.assertEqual(directory.reconcile_groups(self.member, {GROUP}, {GROUP}), DN)
|
||||
self.assertEqual(search.call_args_list[0].kwargs["attributes"], ["uid", "mail", "entryUUID"])
|
||||
self.assertIn("(uid=alice)", search.call_args_list[0].args[1])
|
||||
self.assertEqual(self.member.ldap_uuid, "11111111-1111-1111-1111-111111111111")
|
||||
# A recreated uid must never substitute for the now-pinned LDAP entry.
|
||||
self.conn.delete(DN)
|
||||
self.conn.strategy.add_entry(DN, {"objectClass": ["inetOrgPerson"], "uid": "alice", "cn": "alice", "sn": "alice", "mail": "alice@example.org", "entryUUID": "22222222-2222-2222-2222-222222222222"})
|
||||
with patch("members.directory.connection", self.mock_connection), patch.object(self.conn, "modify", wraps=self.conn.modify) as modify:
|
||||
with self.assertRaises(ValueError):
|
||||
directory.reconcile_groups(self.member, {GROUP}, {GROUP})
|
||||
modify.assert_not_called()
|
||||
|
||||
def test_worker_unpinned_identity_requires_trusted_mapping(self):
|
||||
self.member.ldap_uuid = None
|
||||
for uid, issuer, subject in (("alice", "", None), ("alice", "https://other.example", "sub"), ("*)(uid=*)", settings.DEX_ISSUER, "sub")):
|
||||
self.member.uid, self.member.oidc_issuer, self.member.oidc_sub = uid, issuer, subject
|
||||
with patch("members.directory.connection", self.mock_connection):
|
||||
with self.assertRaises(ValueError):
|
||||
directory.reconcile_groups(self.member, {GROUP}, {GROUP})
|
||||
|
||||
def test_worker_refuses_directory_uuid_already_owned_by_another_account(self):
|
||||
account()
|
||||
self.member.ldap_uuid = None
|
||||
self.member.oidc_issuer = settings.DEX_ISSUER
|
||||
self.member.oidc_sub = "dex-alice"
|
||||
with patch("members.directory.connection", self.mock_connection), patch.object(self.conn, "modify", wraps=self.conn.modify) as modify:
|
||||
with self.assertRaises(ValueError):
|
||||
directory.reconcile_groups(self.member, {GROUP}, {GROUP})
|
||||
modify.assert_not_called()
|
||||
|
||||
@override_settings(LDAP_URL="ldaps://ldap.example.org", LDAP_BIND_DN="cn=service", LDAP_BIND_PASSWORD="test-only", LDAP_ACCOUNT_BASE_DN=BASE)
|
||||
@patch("members.directory.Connection")
|
||||
def test_service_bind_failure_is_redacted_and_unbound(self, conn_factory):
|
||||
from ldap3.core.exceptions import LDAPInvalidCredentialsResult
|
||||
|
||||
conn_factory.return_value.bind.side_effect = LDAPInvalidCredentialsResult(description="private diagnostic")
|
||||
with self.assertRaises(directory.LDAPServiceBindError) as raised:
|
||||
with directory.connection():
|
||||
self.fail("Rejected credentials must not reach LDAP operations")
|
||||
self.assertIn("LDAP_BIND_PASSWORD", str(raised.exception))
|
||||
self.assertNotIn("private diagnostic", str(raised.exception))
|
||||
conn_factory.return_value.unbind.assert_called_once()
|
||||
|
||||
@override_settings(LDAP_URL="ldap://ldap.example.org", LDAP_ALLOW_INSECURE=False, LDAP_BIND_DN="cn=service", LDAP_BIND_PASSWORD="test-only", LDAP_ACCOUNT_BASE_DN=BASE)
|
||||
@patch("members.directory.Connection")
|
||||
@@ -515,3 +650,58 @@ class DirectoryTests(SimpleTestCase):
|
||||
self.assertEqual(options["use_ssl"], scheme == "ldaps")
|
||||
self.assertEqual(options["port"], port)
|
||||
self.assertEqual(options["tls"].validate, ssl.CERT_REQUIRED)
|
||||
|
||||
|
||||
@override_settings(LDAP_SEARCH_BASE_DN=BASE, LDAP_GROUP_PLACEHOLDER_DN=SENTINEL, MEMBERSHIP_TIERS=TIERS)
|
||||
class DeferredDirectoryPersistenceTests(TestCase):
|
||||
def test_worker_persists_uuid_and_dn_after_real_offline_ldap_sync(self):
|
||||
member = views.link_identity({"iss": settings.DEX_ISSUER, "sub": "dex-alice", "preferred_username": "alice", "email_verified": True, "email": "alice@example.org", "federated_claims": {"connector_id": settings.DEX_CONNECTOR_ID, "user_id": "alice"}})
|
||||
server = Server("mock", get_info=OFFLINE_SLAPD_2_4) # pyright: ignore[reportArgumentType]
|
||||
conn = Connection(server, client_strategy=MOCK_SYNC)
|
||||
conn.bind()
|
||||
conn.strategy.add_entry(DN, {"objectClass": ["inetOrgPerson"], "uid": "alice", "cn": "alice", "sn": "alice", "mail": "alice@example.org", "entryUUID": "11111111-1111-1111-1111-111111111111"})
|
||||
conn.strategy.add_entry(SENTINEL, {"objectClass": ["organizationalRole"], "cn": "sentinel"})
|
||||
for group in (GROUP, OTHER_GROUP):
|
||||
conn.strategy.add_entry(group, {"objectClass": ["groupOfUniqueNames"], "cn": "paid", "uniqueMember": [SENTINEL]})
|
||||
|
||||
@contextmanager
|
||||
def mock_connection():
|
||||
yield conn
|
||||
|
||||
with patch("members.directory.connection", mock_connection):
|
||||
self.assertTrue(billing.sync_account(member.pk, refresh_stripe=False))
|
||||
member.refresh_from_db()
|
||||
self.assertEqual(member.ldap_uuid, "11111111-1111-1111-1111-111111111111")
|
||||
self.assertEqual(member.ldap_dn, DN)
|
||||
self.assertIsNotNone(member.last_ldap_sync)
|
||||
self.assertEqual(member.membership_level, 0)
|
||||
|
||||
|
||||
class DexIdentityMigrationTests(TransactionTestCase):
|
||||
@override_settings(DEX_ISSUER="https://original-issuer.example/")
|
||||
def test_upgrade_preserves_existing_bindings_and_allows_unresolved_users(self):
|
||||
from django.db import connection
|
||||
from django.db.migrations.executor import MigrationExecutor
|
||||
|
||||
previous = [("members", "0003_membership_levels")]
|
||||
current = [("members", "0004_dex_login_identity")]
|
||||
executor = MigrationExecutor(connection)
|
||||
executor.migrate(previous)
|
||||
try:
|
||||
OldAccount = executor.loader.project_state(previous).apps.get_model("members", "Account")
|
||||
linked = OldAccount.objects.create(uid="alice", email="alice@example.org", ldap_dn=DN, ldap_uuid="uuid-alice", oidc_sub="existing-sub", stripe_customer="cus_existing")
|
||||
unlinked = OldAccount.objects.create(uid="bob", email="bob@example.org", ldap_dn="uid=bob," + BASE, ldap_uuid="uuid-bob")
|
||||
executor = MigrationExecutor(connection)
|
||||
executor.migrate(current)
|
||||
NewAccount = executor.loader.project_state(current).apps.get_model("members", "Account")
|
||||
restored = NewAccount.objects.values("oidc_issuer", "oidc_sub", "ldap_uuid", "stripe_customer").get(pk=linked.pk)
|
||||
self.assertEqual(restored["oidc_issuer"], "https://original-issuer.example/")
|
||||
self.assertEqual(restored["oidc_sub"], "existing-sub")
|
||||
self.assertEqual(restored["ldap_uuid"], "uuid-alice")
|
||||
self.assertEqual(restored["stripe_customer"], "cus_existing")
|
||||
self.assertEqual(NewAccount.objects.values_list("oidc_issuer", flat=True).get(pk=unlinked.pk), "")
|
||||
for uid in ("charlie", "david"):
|
||||
new = NewAccount.objects.create(uid=uid, email=f"{uid}@example.org", oidc_issuer=settings.DEX_ISSUER, oidc_sub=f"dex-{uid}")
|
||||
self.assertIsNone(NewAccount.objects.values_list("ldap_uuid", flat=True).get(pk=new.pk))
|
||||
finally:
|
||||
MigrationExecutor(connection).migrate(current)
|
||||
|
||||
@@ -48,6 +48,19 @@ class AuthlibFlowTests(TestCase):
|
||||
with self.assertRaises(Exception):
|
||||
self.exchange(remote, callback, token)
|
||||
|
||||
def test_verified_callback_creates_account_without_ldap(self):
|
||||
from members import views
|
||||
from members.models import Account
|
||||
|
||||
remote, callback, token = self.flow(preferred_username="alice", email="alice@example.org", email_verified=True, federated_claims={"connector_id": settings.DEX_CONNECTOR_ID, "user_id": "alice"})
|
||||
with patch("members.views.dex", return_value=remote), patch.object(remote, "fetch_access_token", return_value=token), patch.object(remote, "fetch_jwk_set", return_value={"keys": [self.key.as_dict(private=False)]}), patch("members.directory.Connection", side_effect=AssertionError("Login must not bind to LDAP")) as ldap:
|
||||
response = views.callback(callback)
|
||||
self.assertEqual(response.status_code, 302)
|
||||
member = Account.objects.get(oidc_issuer=settings.DEX_ISSUER, oidc_sub="test-sub")
|
||||
self.assertIsNone(member.ldap_uuid)
|
||||
self.assertEqual(dict(callback.session), {"account_id": str(member.pk)})
|
||||
ldap.assert_not_called()
|
||||
|
||||
def test_wrong_nonce_issuer_audience_expiry_rejected(self):
|
||||
for changes in ({"nonce": "attacker"}, {"iss": "https://evil.example"}, {"aud": "another-client"}, {"exp": int(time.time()) - 100}):
|
||||
with self.subTest(changes=changes):
|
||||
|
||||
+34
-21
@@ -1,5 +1,6 @@
|
||||
import hashlib
|
||||
import logging
|
||||
import re
|
||||
import secrets
|
||||
import uuid
|
||||
from datetime import timedelta
|
||||
@@ -9,6 +10,7 @@ from urllib.parse import urlsplit
|
||||
import stripe
|
||||
from authlib.integrations.django_client import OAuth
|
||||
from django.conf import settings
|
||||
from django.core.validators import validate_email
|
||||
from django.core.mail import send_mail
|
||||
from django.db import transaction
|
||||
from django.http import HttpResponse
|
||||
@@ -129,6 +131,9 @@ def finish_registration(request):
|
||||
remember_groups(account)
|
||||
Registration.objects.filter(uid=registration.uid).delete()
|
||||
request.session.pop("registration_hash", None)
|
||||
# Server-side, short-lived proof permits first binding of ONLY the
|
||||
# account just provisioned in this browser, never an email match.
|
||||
request.session["registration_identity"] = {"account_id": str(account.pk), "expires_at": (timezone.now() + timedelta(hours=1)).timestamp()}
|
||||
return page(request, title="Account created", message="Your free eom.dev account is ready. Sign in to explore the community and its services, or choose an optional paid membership for additional benefits.")
|
||||
except ValueError:
|
||||
form.add_error(None, "An account with those details may already exist. Try signing in or contact us for help.")
|
||||
@@ -163,32 +168,37 @@ def login(request):
|
||||
return page(request, title="Sign-in unavailable", message="Please try again later.")
|
||||
|
||||
|
||||
def link_identity(claims):
|
||||
def link_identity(claims, *, registration_account_id=None):
|
||||
if claims.get("iss") != settings.DEX_ISSUER or claims.get("email_verified") is not True or not isinstance(claims.get("sub"), str) or not claims["sub"] or len(claims["sub"]) > 512:
|
||||
raise ValueError("A verified Dex identity is required.")
|
||||
identity = directory.identity_for_login(claims.get("preferred_username"))
|
||||
uid, email = claims.get("preferred_username"), claims.get("email")
|
||||
if not isinstance(uid, str) or not re.fullmatch(r"[a-z][a-z0-9._-]{2,47}", uid):
|
||||
raise ValueError("Dex must return a valid uid as preferred_username.")
|
||||
if not isinstance(email, str) or len(email) > 254:
|
||||
raise ValueError("A verified Dex email is required.")
|
||||
validate_email(email)
|
||||
federated = claims.get("federated_claims") or {}
|
||||
if federated.get("connector_id") != settings.DEX_CONNECTOR_ID or federated.get("user_id") != identity["uid"]:
|
||||
if not isinstance(federated, dict) or federated.get("connector_id") != settings.DEX_CONNECTOR_ID or federated.get("user_id") != uid:
|
||||
raise ValueError("Dex must authenticate the configured LDAP connector with uid as idAttr.")
|
||||
if identity["email"].casefold() != str(claims.get("email", "")).casefold():
|
||||
raise ValueError("Dex and LDAP email disagree.")
|
||||
# Resolve by immutable LDAP entryUUID, never by email. Dex's federated user
|
||||
# ID remains uid so the unified connector preserves existing OIDC subjects.
|
||||
# Authentication is entirely OIDC. Never contact LDAP or silently rebind an
|
||||
# existing account to a different subject based on username/email matches.
|
||||
with transaction.atomic():
|
||||
uid_owner = Account.objects.filter(uid=identity["uid"]).first()
|
||||
if uid_owner and uid_owner.ldap_uuid != identity["ldap_uuid"]:
|
||||
raise ValueError("Username is already linked to another directory entry.")
|
||||
account, _ = Account.objects.get_or_create(ldap_uuid=identity["ldap_uuid"], defaults=identity)
|
||||
account = Account.objects.select_for_update().get(pk=account.pk)
|
||||
subject_owner = Account.objects.filter(oidc_sub=claims["sub"]).exclude(pk=account.pk).exists()
|
||||
if subject_owner:
|
||||
raise ValueError("Dex subject is already linked to another directory entry.")
|
||||
# A previous login may have used the retired www-ldap connector, whose
|
||||
# subjects used entryUUID. Rebind after validating the same LDAP entry.
|
||||
account = Account.objects.select_for_update().filter(oidc_issuer=claims["iss"], oidc_sub=claims["sub"]).first()
|
||||
if account:
|
||||
if account.uid != uid:
|
||||
raise ValueError("Dex identity username changed; an explicit migration is required.")
|
||||
else:
|
||||
account, created = Account.objects.select_for_update().get_or_create(uid=uid, defaults={"email": email, "oidc_issuer": claims["iss"], "oidc_sub": claims["sub"]})
|
||||
if not created:
|
||||
if account.oidc_sub:
|
||||
if account.oidc_issuer != claims["iss"] or account.oidc_sub != claims["sub"]:
|
||||
raise ValueError("Existing account requires an explicit Dex identity mapping.")
|
||||
elif str(account.pk) != registration_account_id:
|
||||
raise ValueError("Existing account requires an explicit Dex identity mapping.")
|
||||
account.oidc_issuer = claims["iss"]
|
||||
account.oidc_sub = claims["sub"]
|
||||
account.email = identity["email"]
|
||||
account.ldap_dn = identity["ldap_dn"]
|
||||
account.save(update_fields=["oidc_sub", "email", "ldap_dn"])
|
||||
account.email = email
|
||||
account.save(update_fields=["oidc_issuer", "oidc_sub", "email"])
|
||||
remember_groups(account)
|
||||
return account
|
||||
|
||||
@@ -199,7 +209,10 @@ def callback(request):
|
||||
token = dex().authorize_access_token(request, claims_options={"iss": {"essential": True, "value": settings.DEX_ISSUER}}, leeway=30)
|
||||
if not token.get("id_token"):
|
||||
raise ValueError("ID token is required.")
|
||||
account = link_identity(token.get("userinfo") or {})
|
||||
proof = request.session.get("registration_identity") or {}
|
||||
expires_at = proof.get("expires_at") if isinstance(proof, dict) else None
|
||||
registration_account_id = proof.get("account_id") if isinstance(expires_at, (int, float)) and expires_at > timezone.now().timestamp() else None
|
||||
account = link_identity(token.get("userinfo") or {}, registration_account_id=registration_account_id)
|
||||
except Exception as exc:
|
||||
logger.error("Dex callback rejected kind=%s", type(exc).__name__)
|
||||
return page(request, title="Sign-in could not be verified", message="Try signing in again, or contact the administrator.")
|
||||
|
||||
Reference in New Issue
Block a user