diff --git a/.vscode/tasks.json b/.vscode/tasks.json index 4867050..269cbcd 100755 --- a/.vscode/tasks.json +++ b/.vscode/tasks.json @@ -10,6 +10,7 @@ "test", "members.tests.test_membership", "members.tests.test_oidc", + "members.tests.test_profiles", "members.tests.test_postgres", "members.tests.test_shkeeper", "--settings=config.test_settings", @@ -28,6 +29,7 @@ "test", "members.tests.test_membership", "members.tests.test_oidc", + "members.tests.test_profiles", "members.tests.test_postgres", "members.tests.test_shkeeper", "--settings=config.test_settings", @@ -118,6 +120,21 @@ "isBackground": false, "problemMatcher": [], "group": "test" + }, + { + "label": "Test membership profiles", + "type": "shell", + "command": "C:/Users/ermeehan/AppData/Local/Microsoft/WindowsApps/python3.12.exe", + "args": [ + "${workspaceFolder}/membership/manage.py", + "test", + "members.tests.test_profiles", + "--settings=config.test_settings", + "--verbosity=1" + ], + "isBackground": false, + "problemMatcher": [], + "group": "test" } ] -} \ No newline at end of file +} diff --git a/README.md b/README.md index 60ca555..145ecd8 100644 --- a/README.md +++ b/README.md @@ -210,6 +210,103 @@ 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. +### Self-service account profile + +Signed-in members can choose **Edit profile** from their membership page. +Deploy the new membership image, apply migration `0005_profile_email_change`, +and deploy the updated static CSS/proxy configuration. No new public settings +or credentials are required. Login remains Dex-only: the profile editor reads +LDAP **only when opened**, because the configured Dex LDAP connector exposes +username/email/name/groups, not arbitrary LDAP attributes. Cached OIDC claims +are neither a complete nor a current editable profile. Do not map the gamer tag +to `preferred_username` or change `idAttr` to expose it. + +| Profile label | LDAP attribute | +| --- | --- | +| About you | `description` | +| Gamer tag (one value) | `displayName` | +| Email address (verified separately) | `mail` | +| First name / Last name | `givenName` / `sn` | +| Profile photo | `jpegPhoto` | +| City/town / State/province | `l` / `st` | +| Websites | `labeledURI` | +| Mobile numbers / Phone numbers | `mobile` / `telephoneNumber` | +| Organizations / Job titles | `o` / `title` | +| Postal addresses / Postal codes | `postalAddress` / `postalCode` | +| Street addresses / Room numbers | `street` / `roomNumber` | +| SSH public keys | `sshPublicKey` | +| Password | RFC 3062 Password Modify (never read back) | + +The form preserves existing multi-value attributes. Enter one value per line; +separate multiline postal addresses with a blank line. Blank optional fields +are removed on save; `sn` remains required by the directory schema. JPEGs are +limited to 1 MiB and 4096 pixels per side/16 million pixels, resized to fit 512 +pixels and reencoded without metadata. Existing photos are sanitized again +before private, authenticated delivery. No arbitrary URLs are fetched. Supported +SSH keys are Ed25519, RSA >=2048 bits and ECDSA NIST P-256/P-384/P-521; private +keys, authorized_keys options and malformed values are rejected. + +**Directory prerequisites:** + +- The service account needs read/search access to the profile attributes above, + plus `uid`, `entryUUID`, `objectClass` and `modifyTimestamp`, across the + intended account search tree. It never needs read access to `userPassword`. + 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. +- Every write binds with the member's **current LDAP password** and checks that + entry's UUID again. Configure directory **self-write ACLs** only for these + profile attributes and self Password Modify; do not grant writes to `uid`, + `entryUUID`, account DNs, POSIX IDs, group membership or administrative fields. + This supports existing Dex users outside the website registration OU without + granting the membership service unrestricted directory write permissions. +- Install the `ldapPublicKey` auxiliary schema before SSH editing. When a user + supplies keys and the class is absent, the application adds **only** the fixed + `ldapPublicKey` class in the same modify. Either pre-add that class to editable + accounts or permit precisely that self-add with value-scoped `objectClass` + ACLs. Never grant arbitrary self-write to `objectClass`. No schema/ACL changes + are automatically applied by the website or OpenLDAP role. +- OpenLDAP must support critical RFC 4528 assertion controls. Profile/email + writes assert UUID and last-modified time, and profile forms carry a signed, + one-hour revision to reject stale edits. A server rejecting the critical + control must fail closed; do not silently retry without it. +- Maintain directory-side unique `mail` and `uid` enforcement across all + registrars. A client-side email preflight cannot prevent concurrent claims + and may be restricted by self-read ACLs; the server uniqueness overlay is + required. Review which services use `mail` as an account/recovery identifier. + +Email is changed separately: the user enters their current password, receives +a one-hour verification link at the new mailbox, then signs into the same local +account and enters their password again. GET/scanner visits never modify LDAP. +Tokens are stored only as hashes, bound to account/LDAP UUID, and consumed once. +Confirming replaces **all existing mail values** with the verified address and +updates the local contact address; it does not attach another user's billing +history. Old Dex/downstream tokens may retain the prior email until refreshed. + +Password changes send current/new passwords through Password Modify as the +authenticated user, never a service-account reset or direct `userPassword` +write. **Hashing remains OpenLDAP's policy**: no MD5-Crypt hash is generated by +the application, no stored hash is exposed, and the application does not force +or silently change the server's scheme. A legacy MD5-Crypt directory can still +verify current passwords; do not introduce MD5-Crypt as a new application-side +hashing scheme. Audit/migrate weak server hashing separately. The existing +Argon2 gate for new registration remains unchanged. Password changes sign out +the current membership browser; other membership/Dex/service sessions are not +automatically revoked—use each service's session controls when necessary. + +All writes are CSRF-protected and rate-limited per account/IP. Passwords and +profile form bodies must never be logged by ingress/APM/LDAP debug logging. +Profile data can be shared by directory-consuming services: the page is private, +but it does not make phone/address/SSH fields private in the directory. Enforce +directory read ACLs and tell users what other services can access. TLS defaults +are unchanged; the existing explicit plaintext opt-in also exposes profile +passwords/data and should not be used on untrusted networks. + +The supplied Kubernetes ingress accepts up to 2 MiB; Apache allows that limit +only under `/members/profile/` and retains 256 KiB for other member routes. +Mirror the limits at any additional edge proxy. Oversized or invalid uploads +must be rejected before reaching image processing. + ### Database ProgrammingError during callback `Dex callback rejected kind=ProgrammingError` in older images is a database diff --git a/conf/httpd.conf b/conf/httpd.conf index 92a214c..04801bb 100644 --- a/conf/httpd.conf +++ b/conf/httpd.conf @@ -251,6 +251,10 @@ ProxyPassReverse "/members/" "http://membership:8000/members/" RequestHeader set X-Membership-Client-IP "expr=%{REMOTE_ADDR}" LimitRequestBody 262144 + + # 1 MiB JPEG plus form/multipart overhead; other member routes stay smaller. + LimitRequestBody 2097152 + # # Deny access to the entirety of your server's filesystem. You must diff --git a/htdocs/static/membership.css b/htdocs/static/membership.css index 0957ed9..37f057f 100755 --- a/htdocs/static/membership.css +++ b/htdocs/static/membership.css @@ -40,3 +40,33 @@ button.button { cursor: pointer; font-size: inherit; line-height: inherit; } .member-faq details p { color: var(--muted); margin: 1rem 0 .5rem; } .member-footer { padding-block: 2rem; border-top: 1px solid var(--border); } @media (max-width: 540px) { .member-nav, .member-main, .member-footer { width: calc(100% - 2rem); } .member-nav nav { gap: .8rem; font-size: .85rem; } .member-panel { padding: 1.4rem; } .member-actions { align-items: stretch; flex-direction: column; } .member-actions .button { width: 100%; } } + +/* Shared-account profile: use the home page's Catppuccin theme variables. */ +.profile-main { max-width: 880px; } +.profile-heading { padding-block: clamp(2rem, 6vw, 4rem) 1rem; } +.profile-heading h1 { font-size: clamp(2.5rem, 6vw, 4rem); } +.profile-heading > p { max-width: 70ch; } +.profile-jump-links { display: flex; flex-wrap: wrap; gap: .6rem; margin-block: 1rem 2rem; } +.profile-jump-links a { padding: .45rem .85rem; border: 1px solid var(--border); border-radius: 999px; background: var(--surface); text-decoration: none; } +.profile-section { min-width: 0; margin: 0 0 1.5rem; scroll-margin-top: 2rem; } +.profile-section legend { padding-inline: .5rem; color: var(--accent); font-size: 1.35rem; font-weight: 700; } +.profile-section h2, .profile-errors h2 { font-size: clamp(1.5rem, 3vw, 2rem); } +.profile-field { min-width: 0; margin-block: 0 1.5rem; } +.profile-field label { display: inline-block; margin-bottom: .35rem; font-weight: 650; } +.profile-required { color: var(--muted); font-size: .85rem; } +.profile-field input:not([type="checkbox"]), .profile-field textarea { display: block; width: 100%; min-width: 0; padding: .8rem 1rem; border: 1px solid var(--border); border-radius: .65rem; background: var(--page-deep); color: var(--text); font: inherit; } +.profile-field textarea { min-height: 7rem; resize: vertical; line-height: 1.5; } +.profile-field input:focus-visible, .profile-field textarea:focus-visible, .profile-section button:focus-visible { outline: 3px solid var(--accent); outline-offset: 3px; } +.profile-field .helptext { margin-block: 0 .6rem; color: var(--muted); font-size: .9rem; } +.profile-field-invalid input, .profile-field-invalid textarea, .profile-field [aria-invalid="true"] { border-color: var(--red); } +.profile-errors { margin-block: 1rem 1.5rem; padding: 1.25rem; border: 1px solid var(--red); border-radius: .8rem; background: var(--surface); } +.profile-field .errorlist { margin-block: .5rem 0; padding-left: 1.25rem; } +.profile-checkbox input { width: 1.2rem; height: 1.2rem; margin-right: .4rem; accent-color: var(--accent); vertical-align: middle; } +.profile-checkbox label { font-weight: 500; } +.profile-photo { margin: 0 0 1.5rem; } +.profile-photo img { display: block; object-fit: contain; border: 1px solid var(--border); border-radius: 1rem; background: var(--page-deep); } +.profile-photo figcaption { margin-top: .5rem; color: var(--muted); font-size: .85rem; } +.profile-email-values { padding-left: 1.4rem; overflow-wrap: anywhere; } +.profile-back { margin-top: 2rem; } +@media (max-width: 540px) { .profile-section { padding: 1.1rem; } .profile-section .button { width: 100%; } .profile-jump-links { gap: .45rem; } .profile-jump-links a { font-size: .9rem; } } +@media (prefers-reduced-motion: reduce) { html:has(.profile-main) { scroll-behavior: auto; } } diff --git a/membership/config/settings.py b/membership/config/settings.py index 0945112..7c2e577 100755 --- a/membership/config/settings.py +++ b/membership/config/settings.py @@ -63,6 +63,8 @@ SECURE_HSTS_SECONDS = 31536000 if not ALLOW_HTTP else 0 SECURE_CONTENT_TYPE_NOSNIFF = True X_FRAME_OPTIONS = "DENY" DATA_UPLOAD_MAX_MEMORY_SIZE = 262144 +FILE_UPLOAD_MAX_MEMORY_SIZE = 2 * 1024 * 1024 +DATA_UPLOAD_MAX_NUMBER_FILES = 1 AUTH_PASSWORD_VALIDATORS = [ {"NAME": "django.contrib.auth.password_validation.MinimumLengthValidator", "OPTIONS": {"min_length": 12}}, {"NAME": "django.contrib.auth.password_validation.CommonPasswordValidator"}, diff --git a/membership/members/directory.py b/membership/members/directory.py index c6e74d4..9f672ef 100755 --- a/membership/members/directory.py +++ b/membership/members/directory.py @@ -21,11 +21,17 @@ class LDAPServiceBindError(LDAPException): @contextmanager def connection(): + with _connection(settings.LDAP_BIND_DN, settings.LDAP_BIND_PASSWORD, service=True) as conn: + yield conn + + +@contextmanager +def _connection(bind_dn, password, *, service=False): url = urlsplit(settings.LDAP_URL) - if url.scheme not in {"ldap", "ldaps"} or not url.hostname or not settings.LDAP_BIND_DN or not settings.LDAP_BIND_PASSWORD or not settings.LDAP_ACCOUNT_BASE_DN: + if url.scheme not in {"ldap", "ldaps"} or not url.hostname or not bind_dn or not password or not settings.LDAP_ACCOUNT_BASE_DN: raise ImproperlyConfigured("Configure LDAP URL, service bind, and account base DN.") server = Server(url.hostname, port=url.port or (636 if url.scheme == "ldaps" else 389), use_ssl=url.scheme == "ldaps", tls=Tls(validate=ssl.CERT_REQUIRED, ca_certs_file=settings.LDAP_CA_CERT_FILE, version=ssl.PROTOCOL_TLS_CLIENT), connect_timeout=5) - conn = Connection(server, user=settings.LDAP_BIND_DN, password=settings.LDAP_BIND_PASSWORD, raise_exceptions=True, receive_timeout=8, auto_referrals=False) + conn = Connection(server, user=bind_dn, password=password, raise_exceptions=True, receive_timeout=8, auto_referrals=False) try: conn.open() # Plaintext requires explicit operator consent; LDAPS is never downgraded. @@ -34,6 +40,8 @@ def connection(): try: bound = conn.bind() except LDAPInvalidCredentialsResult: + if not service: + raise ValueError("Current password could not be verified.") from None # 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: diff --git a/membership/members/management/commands/sync_memberships.py b/membership/members/management/commands/sync_memberships.py index 82df9e0..d9f17d5 100755 --- a/membership/members/management/commands/sync_memberships.py +++ b/membership/members/management/commands/sync_memberships.py @@ -7,7 +7,7 @@ from django.contrib.sessions.models import Session from django.utils import timezone from members.billing import sync_account -from members.models import Account, RateWindow, Registration, WebhookEvent +from members.models import Account, ProfileEmailChange, RateWindow, Registration, WebhookEvent logger = logging.getLogger(__name__) @@ -44,6 +44,7 @@ class Command(BaseCommand): now = timezone.now() Session.objects.filter(expire_date__lte=now).delete() Registration.objects.filter(expires_at__lte=now).delete() + ProfileEmailChange.objects.filter(expires_at__lte=now).delete() RateWindow.objects.filter(expires_at__lte=now).delete() WebhookEvent.objects.filter(processed_at__lt=now - timedelta(days=30)).delete() if not options["loop"]: diff --git a/membership/members/migrations/0005_profile_email_change.py b/membership/members/migrations/0005_profile_email_change.py new file mode 100755 index 0000000..7fb6616 --- /dev/null +++ b/membership/members/migrations/0005_profile_email_change.py @@ -0,0 +1,15 @@ +import django.db.models.deletion +from django.db import migrations, models + + +class Migration(migrations.Migration): + dependencies = [("members", "0004_dex_login_identity")] + operations = [ + migrations.CreateModel(name="ProfileEmailChange", fields=[ + ("token_hash", models.CharField(max_length=64, primary_key=True, serialize=False)), + ("email", models.EmailField(max_length=254)), + ("ldap_uuid", models.CharField(max_length=64)), + ("expires_at", models.DateTimeField(db_index=True)), + ("account", models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to="members.account")), + ]), + ] diff --git a/membership/members/models.py b/membership/members/models.py index 08eff0d..5922317 100755 --- a/membership/members/models.py +++ b/membership/members/models.py @@ -28,6 +28,14 @@ class Registration(models.Model): expires_at = models.DateTimeField(db_index=True) +class ProfileEmailChange(models.Model): + token_hash = models.CharField(max_length=64, primary_key=True) + account = models.ForeignKey(Account, on_delete=models.CASCADE) + email = models.EmailField() + ldap_uuid = models.CharField(max_length=64) + expires_at = models.DateTimeField(db_index=True) + + class RateWindow(models.Model): key = models.CharField(max_length=64, primary_key=True) count = models.PositiveIntegerField(default=0) diff --git a/membership/members/profile_forms.py b/membership/members/profile_forms.py new file mode 100755 index 0000000..391c0bb --- /dev/null +++ b/membership/members/profile_forms.py @@ -0,0 +1,236 @@ +"""Validation-only forms for the shared LDAP profile; no directory writes.""" + +import unicodedata +import warnings +from io import BytesIO +from urllib.parse import urlsplit + +from cryptography.exceptions import UnsupportedAlgorithm +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import ec, ed25519, rsa +from django import forms +from django.core.exceptions import ValidationError +from django.core.validators import URLValidator + +from .forms import PasswordForm + + +MAX_INPUT_LENGTH = 32768 +MAX_VALUES = 16 +MAX_PHOTO_BYTES = 1024 * 1024 + + +def _safe_text(value: str) -> str: + # Normalize browser line endings before removing unsafe control characters. + value = value.replace("\r\n", "\n").replace("\r", "\n").replace("\t", " ") + return "".join( + char for char in value + if char == "\n" or unicodedata.category(char) not in {"Cc", "Cf", "Cs"} + ) + + +class ProfileValuesField(forms.CharField): + """Text input becomes an ordered, deduplicated list of LDAP strings.""" + + def __init__(self, *, value_length: int = 2048, postal: bool = False, max_values: int = MAX_VALUES, **kwargs): + self.value_length = value_length + self.postal = postal + self.max_values = max_values + kwargs.setdefault("required", False) + kwargs.setdefault("help_text", "One value per line. Up to 16 values; leave blank to clear.") + super().__init__( + max_length=MAX_INPUT_LENGTH, + widget=forms.Textarea(attrs={"rows": 3, "maxlength": MAX_INPUT_LENGTH}), + **kwargs, + ) + + def clean(self, value) -> list[str]: + if value is not None and len(str(value)) > MAX_INPUT_LENGTH: + raise ValidationError("Enter no more than 32,768 characters in this field.") + text = super().clean(_safe_text(str(value)) if value is not None else "") + if self.postal: + entries = [] + lines = [] + for line in text.split("\n") + [""]: + if line.strip(): + # RFC 4517 Postal Address escapes literal '$' and '\'. + lines.append(line.strip().replace("\\", "\\5c").replace("$", "\\24")) + elif lines: + entries.append("$".join(lines)) + lines = [] + else: + entries = [line.strip() for line in text.split("\n") if line.strip()] + if len(entries) > self.max_values: + raise ValidationError(f"Enter no more than {self.max_values} value(s).") + if any(len(entry) > self.value_length for entry in entries): + raise ValidationError(f"Each value must be at most {self.value_length:,} characters.") + if self.required and not entries: + raise ValidationError("Enter at least one value.") + return list(dict.fromkeys(entries)) + + +def _current_password_field() -> forms.CharField: + return forms.CharField( + label="Current password", max_length=1024, strip=False, + widget=forms.PasswordInput( + render_value=False, attrs={"autocomplete": "current-password"} + ), + help_text="Enter your current eom.dev password to authorize this change.", + ) + + +class CurrentPasswordForm(forms.Form): + current_password = _current_password_field() + + def __init__(self, *args, **kwargs): + kwargs.setdefault("auto_id", "id_confirmation_%s") + super().__init__(*args, **kwargs) + + +class ProfileForm(CurrentPasswordForm): + description = ProfileValuesField(label="About you", value_length=4096) + displayName = ProfileValuesField(label="Gamer tag", max_values=1, help_text="One gamer tag, reused by gaming services. Leave blank to clear.") + givenName = ProfileValuesField(label="First name") + l = ProfileValuesField(label="City/town") + labeledURI = ProfileValuesField(label="Websites", help_text="One HTTP or HTTPS URL per line, optionally followed by a space and a label.") + mobile = ProfileValuesField(label="Mobile numbers") + o = ProfileValuesField(label="Organizations") + postalAddress = ProfileValuesField(label="Postal addresses", postal=True, help_text="Use a blank line between addresses. Line breaks within each address are preserved.") + postalCode = ProfileValuesField(label="Postal codes") + roomNumber = ProfileValuesField(label="Room numbers") + sn = ProfileValuesField(label="Last name", required=True, help_text="Enter at least one last name. One value per line.") + sshPublicKey = ProfileValuesField(label="SSH public keys", help_text="One public key per line: Ed25519, RSA (at least 2048 bits), or ECDSA NIST P-256/P-384/P-521. No private keys or authorized_keys options.") + st = ProfileValuesField(label="State/province") + street = ProfileValuesField(label="Street addresses") + telephoneNumber = ProfileValuesField(label="Phone numbers") + title = ProfileValuesField(label="Job titles") + jpegPhoto = forms.FileField( + label="Profile photo", required=False, + widget=forms.FileInput(attrs={"accept": "image/jpeg"}), + help_text="JPEG only, up to 1 MiB and 4096 pixels per side (16 million pixels total). Resized to fit 512 × 512; metadata is removed.", + ) + remove_photo = forms.BooleanField(label="Remove current photo", required=False) + + def __init__(self, *args, **kwargs): + # Initial text values are formatted by the caller, including postal $ decoding. + kwargs.setdefault("auto_id", "id_profile_%s") + super().__init__(*args, **kwargs) + + def identity_fields(self): + return [self[name] for name in ("givenName", "sn", "displayName", "description", "o", "title")] + + def contact_fields(self): + return [self[name] for name in ("labeledURI", "telephoneNumber", "mobile")] + + def address_fields(self): + return [self[name] for name in ("postalAddress", "street", "roomNumber", "l", "st", "postalCode")] + + def authorization_fields(self): + return [self["current_password"]] + + def clean_labeledURI(self) -> list[str]: + entries = self.cleaned_data["labeledURI"] + validator = URLValidator(schemes=["https", "http"]) + for entry in entries: + url = entry.partition(" ")[0] + try: + validator(url) + parsed = urlsplit(url) + if parsed.username is not None or parsed.password is not None or "\\" in url: + raise ValueError("Credentials and backslashes are not allowed.") + if any(char.isspace() for char in url): + raise ValueError("Whitespace is not allowed in URLs.") + except (ValidationError, ValueError): + raise ValidationError("Use an HTTP or HTTPS URL without credentials, optionally followed by a label.") from None + return entries + + def clean_sshPublicKey(self) -> list[str]: + supported = {"ssh-ed25519", "ssh-rsa", "ecdsa-sha2-nistp256", "ecdsa-sha2-nistp384", "ecdsa-sha2-nistp521"} + entries = [] + seen = set() + for value in self.cleaned_data["sshPublicKey"]: + parts = value.split() + if len(parts) < 2 or parts[0] not in supported: + raise ValidationError("Enter a supported SSH public key without options or private-key material.") + try: + key = serialization.load_ssh_public_key(value.encode("utf-8")) + except (ValueError, TypeError, UnsupportedAlgorithm): + raise ValidationError("Enter a valid supported SSH public key.") from None + valid = ( + isinstance(key, ed25519.Ed25519PublicKey) + or isinstance(key, rsa.RSAPublicKey) and key.key_size >= 2048 + or isinstance(key, ec.EllipticCurvePublicKey) + and isinstance(key.curve, (ec.SECP256R1, ec.SECP384R1, ec.SECP521R1)) + ) + if not valid: + raise ValidationError("Use Ed25519, RSA of at least 2048 bits, or ECDSA NIST P-256/P-384/P-521.") + identity = tuple(parts[:2]) + if identity not in seen: + seen.add(identity) + entries.append(value) + return entries + + def clean_jpegPhoto(self) -> bytes | None: + upload = self.cleaned_data.get("jpegPhoto") + if upload is None: + return None + if upload.size > MAX_PHOTO_BYTES: + raise ValidationError("Choose a JPEG no larger than 1 MiB.") + raw = upload.read(MAX_PHOTO_BYTES + 1) + if len(raw) > MAX_PHOTO_BYTES: + raise ValidationError("Choose a JPEG no larger than 1 MiB.") + + # Import only on upload; Pillow is supplied by the application's dependencies. + from PIL import Image, ImageOps, UnidentifiedImageError + + try: + with warnings.catch_warnings(): + warnings.simplefilter("error", Image.DecompressionBombWarning) + with Image.open(BytesIO(raw), formats=["JPEG"]) as image: + width, height = image.size + if image.format != "JPEG" or not (0 < width <= 4096 and 0 < height <= 4096) or width * height > 16_000_000: + raise ValidationError("Choose a JPEG up to 4096 pixels per side and 16 million pixels total.") + image.verify() + with Image.open(BytesIO(raw), formats=["JPEG"]) as image: + image.load() + rgb = ImageOps.exif_transpose(image).convert("RGB") + rgb.thumbnail((512, 512), Image.Resampling.LANCZOS) + # A fresh pixel-only image prevents EXIF/ICC/comment metadata retention. + clean = Image.new("RGB", rgb.size) + clean.paste(rgb) + output = BytesIO() + clean.save(output, format="JPEG", quality=85) + return output.getvalue() + except (UnidentifiedImageError, OSError, ValueError, SyntaxError, Image.DecompressionBombWarning, Image.DecompressionBombError): + raise ValidationError("Choose a valid, safely sized JPEG image.") from None + + def clean(self): + data = super().clean() + if data.get("jpegPhoto") is not None and data.get("remove_photo"): + self.add_error("remove_photo", "Choose either a new photo or removal, not both.") + return data + + +class ProfileEmailForm(CurrentPasswordForm): + email = forms.EmailField(label="New email address", max_length=254, widget=forms.EmailInput(attrs={"autocomplete": "email"})) + field_order = ["email", "current_password"] + + def __init__(self, *args, **kwargs): + kwargs.setdefault("auto_id", "id_email_%s") + super().__init__(*args, **kwargs) + + def clean_email(self) -> str: + return self.cleaned_data["email"].casefold() + + +class ProfilePasswordForm(PasswordForm, CurrentPasswordForm): + field_order = ["current_password", "password", "confirm_password"] + + def __init__(self, *args, **kwargs): + kwargs.setdefault("auto_id", "id_password_%s") + super().__init__(*args, **kwargs) + for name in ("password", "confirm_password"): + self.fields[name].strip = False + self.fields[name].widget = forms.PasswordInput( + render_value=False, attrs={"autocomplete": "new-password"} + ) diff --git a/membership/members/profiles.py b/membership/members/profiles.py new file mode 100755 index 0000000..bd243ec --- /dev/null +++ b/membership/members/profiles.py @@ -0,0 +1,161 @@ +"""Self-service directory operations: pinned identity, user binds, fixed attributes. + +Dex authenticates the session; LDAP remains authoritative for editable values. +No service-account writes, password hashes, tokens or profile snapshots are stored. +""" + +import hashlib +import json +import re +from contextlib import contextmanager + +from django.conf import settings +from django.core import signing +from django.db import transaction +from ldap3 import BASE, MODIFY_ADD, MODIFY_REPLACE +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 . import directory +from .models import Account + + +TEXT_ATTRIBUTES = ( + "description", "displayName", "givenName", "l", "labeledURI", "mobile", "o", + "postalAddress", "postalCode", "roomNumber", "sn", "sshPublicKey", "st", + "street", "telephoneNumber", "title", +) +READ_ATTRIBUTES = (*TEXT_ATTRIBUTES, "mail", "jpegPhoto", "entryUUID", "uid", "modifyTimestamp", "objectClass") + + +def _values(entry): + values = {name: list(entry[name].values) if name in entry else [] for name in READ_ATTRIBUTES} + # Schema-aware ldap3 may convert this to datetime; assertion filters need + # the original LDAP GeneralizedTime syntax rather than Python's rendering. + if "modifyTimestamp" in entry: + values["modifyTimestamp"] = [value.decode("ascii") for value in entry["modifyTimestamp"].raw_values] + if len(values["entryUUID"]) != 1 or len(values["uid"]) != 1: + raise ValueError("A unique directory identity is required.") + return values + + +def _assertion(values): + # Critical RFC 4528 assertion: concurrent edits or a recreated DN fail + # atomically rather than modifying the wrong entry or overwriting new data. + if len(values["modifyTimestamp"]) != 1: + raise ValueError("Read access to modifyTimestamp is required for safe profile edits.") + query = f"(entryUUID={escape_filter_chars(str(values['entryUUID'][0]))})" + query = f"(&{query}(modifyTimestamp={escape_filter_chars(str(values['modifyTimestamp'][0]))}))" + node = parse_filter(query, None, False, False, None, False) + return build_control("1.3.6.1.1.12", True, compile_filter(node.elements[0])) + + +def _digest(values): + def encoded(value): + return hashlib.sha256(value).hexdigest() if isinstance(value, bytes) else str(value) + return hashlib.sha256(json.dumps({key: [encoded(v) for v in values[key]] for key in READ_ATTRIBUTES}, sort_keys=True).encode()).hexdigest() + + +def read_profile(account): + """Resolve once from the signed uid, then pin UUID; never accept a browser DN.""" + with transaction.atomic(): + member = Account.objects.select_for_update().get(pk=account.pk) + if not member.oidc_sub or member.oidc_issuer != settings.DEX_ISSUER: + raise ValueError("A verified Dex identity is required.") + with directory.connection() as conn: + query = f"(entryUUID={escape_filter_chars(member.ldap_uuid)})" if member.ldap_uuid else f"(&(objectClass=inetOrgPerson)(uid={escape_filter_chars(member.uid)}))" + conn.search(settings.LDAP_SEARCH_BASE_DN, query, attributes=list(READ_ATTRIBUTES)) + if len(conn.entries) != 1: + raise ValueError("Profile is missing or ambiguous.") + entry = conn.entries[0] + values = _values(entry) + entry_uuid = str(values["entryUUID"][0]) + if str(values["uid"][0]) != member.uid or not entry_uuid or entry_uuid == "None" or len(entry_uuid) > 64: + raise ValueError("Directory identity does not match the account.") + if Account.objects.filter(ldap_uuid=entry_uuid).exclude(pk=member.pk).exists(): + raise ValueError("Directory identity belongs to another account.") + if entry.entry_dn.casefold() == settings.LDAP_GROUP_PLACEHOLDER_DN.casefold(): + raise ValueError("System entries cannot be edited.") + member.ldap_uuid, member.ldap_dn = entry_uuid, entry.entry_dn + member.save(update_fields=["ldap_uuid", "ldap_dn"]) + account.ldap_uuid, account.ldap_dn = member.ldap_uuid, member.ldap_dn + return values + + +def revision(account, values): + return signing.dumps({"account": str(account.pk), "uuid": account.ldap_uuid, "digest": _digest(values)}, salt="ldap-profile") + + +def initial(values): + def postal(value): + # Decode once: a literal '\24' represented by '\5c24' must not become '$'. + return re.sub(r"\\(24|5[cC])", lambda match: "$" if match.group(1) == "24" else "\\", str(value).replace("$", "\n")) + return {name: ("\n\n".join(postal(value) for value in values[name]) if name == "postalAddress" else "\n".join(str(value) for value in values[name])) for name in TEXT_ATTRIBUTES} + + +@contextmanager +def _authorized(account, password): + if not password or not account.ldap_uuid or not account.ldap_dn: + raise ValueError("A pinned directory entry and current password are required.") + # Authenticate as this user, not as the service identity. slapd self ACLs + # authorize each modification; a compromised service cannot reset passwords. + with directory._connection(account.ldap_dn, password) as conn: + conn.search(account.ldap_dn, f"(entryUUID={escape_filter_chars(account.ldap_uuid)})", search_scope=BASE, attributes=list(READ_ATTRIBUTES)) + if len(conn.entries) != 1: + raise ValueError("Directory identity changed; refusing to edit.") + values = _values(conn.entries[0]) + if str(values["entryUUID"][0]) != account.ldap_uuid or str(values["uid"][0]) != account.uid: + raise ValueError("Directory identity changed; refusing to edit.") + yield conn, values + + +def verify_password(account, password): + with _authorized(account, password): + pass + + +def update_profile(account, cleaned, expected_revision): + proof = signing.loads(expected_revision, salt="ldap-profile", max_age=3600) + 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.") + changes = {} + for name in TEXT_ATTRIBUTES: + new_values = cleaned[name] + if new_values != values[name]: + changes[name] = [(MODIFY_REPLACE, new_values)] + if cleaned["sshPublicKey"] and "ldappublickey" not in {str(value).casefold() for value in values["objectClass"]}: + # Fixed auxiliary class only, never arbitrary browser-supplied + # schema/classes. Install the SSH schema before enabling edits. + changes["objectClass"] = [(MODIFY_ADD, ["ldapPublicKey"])] + photo = cleaned.get("jpegPhoto") + if photo is not None: + changes["jpegPhoto"] = [(MODIFY_REPLACE, [photo])] + elif cleaned.get("remove_photo") and values["jpegPhoto"]: + changes["jpegPhoto"] = [(MODIFY_REPLACE, [])] + if changes and not conn.modify(account.ldap_dn, changes, controls=[_assertion(values)]): + raise LDAPException("Profile modification rejected.") + + +def change_password(account, current_password, new_password): + with _authorized(account, current_password) as (conn, _values): + # RFC 3062 delegates hashing to slapd (including existing server policies). + # No client-side MD5-Crypt, plaintext attribute write or admin reset. + # No userIdentity targets: operate on the authenticated user. RFC 4528 + # assertions are not defined for this extended operation; supplying a + # critical assertion here can make slapd reject an otherwise valid change. + if not conn.extend.standard.modify_password(old_password=current_password, new_password=new_password): + raise LDAPException("Password modification rejected.") + + +def change_email(account, current_password, email): + with _authorized(account, current_password) as (conn, values): + # Enforce directory-side mail uniqueness too: only an LDAP uniqueness + # overlay can prevent another registrar racing this preflight search. + conn.search(settings.LDAP_SEARCH_BASE_DN, f"(mail={escape_filter_chars(email)})", attributes=["entryUUID"]) + if any(str(entry.entryUUID.value) != account.ldap_uuid for entry in conn.entries): + raise ValueError("Email change cannot be completed.") + if not conn.modify(account.ldap_dn, {"mail": [(MODIFY_REPLACE, [email])]}, controls=[_assertion(values)]): + raise LDAPException("Email modification rejected.") diff --git a/membership/members/tests/test_membership.py b/membership/members/tests/test_membership.py index c1d7108..9e0d80f 100755 --- a/membership/members/tests/test_membership.py +++ b/membership/members/tests/test_membership.py @@ -715,6 +715,7 @@ class DexIdentityMigrationTests(TransactionTestCase): previous = [("members", "0003_membership_levels")] current = [("members", "0004_dex_login_identity")] executor = MigrationExecutor(connection) + latest = executor.loader.graph.leaf_nodes() executor.migrate(previous) try: OldAccount = executor.loader.project_state(previous).apps.get_model("members", "Account") @@ -733,4 +734,4 @@ class DexIdentityMigrationTests(TransactionTestCase): 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) + MigrationExecutor(connection).migrate(latest) diff --git a/membership/members/tests/test_profiles.py b/membership/members/tests/test_profiles.py new file mode 100755 index 0000000..7784efe --- /dev/null +++ b/membership/members/tests/test_profiles.py @@ -0,0 +1,892 @@ +"""Offline self-service profile tests; never bind LDAP or deliver real email. + +Run only this module with config.test_settings. Credentials and SSH keys are +ephemeral test data, not service credentials. Directory writes use mocks; LDAP +reads additionally exercise ldap3's in-memory strategy. +""" + +import copy +import hashlib +import secrets +from datetime import timedelta +from io import BytesIO +from unittest.mock import MagicMock, patch +from urllib.parse import urlsplit + +from cryptography.hazmat.primitives import serialization +from cryptography.hazmat.primitives.asymmetric import ec, ed25519, rsa +from django.conf import settings +from django.core import mail, signing +from django.core.exceptions import ValidationError +from django.core.files.uploadedfile import SimpleUploadedFile +from django.test import Client, SimpleTestCase, TestCase, override_settings +from django.urls import Resolver404, resolve, reverse +from django.utils import timezone +from ldap3 import BASE, MOCK_SYNC, MODIFY_REPLACE, Connection, Server +from ldap3.core.exceptions import LDAPException +from PIL import Image + +from members import profiles +from members.models import Account, ProfileEmailChange +from members.profile_forms import ( + MAX_INPUT_LENGTH, + MAX_PHOTO_BYTES, + CurrentPasswordForm, + ProfileEmailForm, + ProfileForm, + ProfilePasswordForm, + ProfileValuesField, +) + + +ALICE_DN = "uid=alice,ou=people,dc=example,dc=org" +SEARCH_DN = "ou=people,dc=example,dc=org" + + +def profile_values(**overrides): + """The complete READ_ATTRIBUTES shape returned by the real reader.""" + values = {name: [] for name in profiles.READ_ATTRIBUTES} + values.update({ + "description": ["Community member"], + "displayName": ["AliceInGame"], + "givenName": ["Alice"], + "sn": ["Example"], + "mail": ["alice@example.org", "alice.other@example.org"], + "postalAddress": ["1 Example Street$Example City", "2 Other Road$Other City"], + "labeledURI": ["https://example.org/ Personal website"], + "entryUUID": ["uuid-alice"], + "uid": ["alice"], + "modifyTimestamp": ["20261029120000Z"], + "objectClass": ["top", "person", "organizationalPerson", "inetOrgPerson"], + }) + values.update(overrides) + return values + + +def jpeg_bytes(size=(800, 400), *, metadata=True): + output = BytesIO() + image = Image.new("RGB", size, (50, 80, 120)) + options = {} + if metadata: + exif = Image.Exif() + exif[0x010E] = "private image description" + options = {"exif": exif, "icc_profile": b"private-test-icc"} + image.save(output, format="JPEG", **options) + return output.getvalue() + + +def photo_upload(raw=None): + return SimpleUploadedFile( + "photo.jpg", jpeg_bytes() if raw is None else raw, + content_type="image/jpeg", + ) + + +class ProfileFormTests(SimpleTestCase): + def setUp(self): + self.password = " " + secrets.token_urlsafe(24) + " " + + def form(self, data=None, files=None): + return ProfileForm( + data={"sn": "Example", "current_password": self.password, **(data or {})}, + files=files, + ) + + def test_friendly_labels_and_single_gamer_tag(self): + form = self.form({"displayName": "AliceInGame"}) + self.assertTrue(form.is_valid(), form.errors) + self.assertEqual(form.fields["displayName"].label, "Gamer tag") + self.assertEqual(form.fields["sn"].label, "Last name") + self.assertEqual(form.fields["l"].label, "City/town") + self.assertEqual(form.cleaned_data["displayName"], ["AliceInGame"]) + invalid = self.form({"displayName": "FirstTag\nSecondTag"}) + self.assertFalse(invalid.is_valid()) + self.assertIn("displayName", invalid.errors) + + def test_all_editable_text_attributes_have_fields_but_identity_is_read_only(self): + self.assertTrue(set(profiles.TEXT_ATTRIBUTES) <= set(ProfileForm.base_fields)) + for name in ("uid", "dn", "ldap_dn", "entryUUID", "mail", "userPassword", "objectClass"): + self.assertNotIn(name, ProfileForm.base_fields) + + def test_field_groups_and_unique_password_ids(self): + form = self.form() + grouped = [ + *form.identity_fields(), *form.contact_fields(), + *form.address_fields(), *form.authorization_fields(), + ] + self.assertEqual(len({field.name for field in grouped}), len(grouped)) + ids = [cls()["current_password"].auto_id for cls in ( + ProfileForm, ProfileEmailForm, ProfilePasswordForm, CurrentPasswordForm, + )] + self.assertEqual(len(set(ids)), 4) + + def test_multivalues_deduplicate_and_strip_control_characters(self): + field = ProfileValuesField() + self.assertEqual(field.clean(" Alice\r\nBob\rAlice\n\tCarol\x00\u200b "), ["Alice", "Bob", "Carol"]) + self.assertEqual(field.clean(None), []) + self.assertEqual(field.clean(" \n "), []) + + def test_value_count_length_and_total_input_limits(self): + field = ProfileValuesField(value_length=3) + self.assertEqual(field.clean("abc"), ["abc"]) + for value in ("abcd", "\n".join(str(i) for i in range(17)), "x" * (MAX_INPUT_LENGTH + 1)): + with self.subTest(length=len(value)), self.assertRaises(ValidationError): + field.clean(value) + self.assertEqual(len(ProfileValuesField().clean("\n".join(str(i) for i in range(16)))), 16) + + def test_surname_required_even_when_only_unsafe_or_blank_text(self): + for surname in ("", " \n ", "\x00\u200b"): + with self.subTest(surname=repr(surname)): + form = self.form({"sn": surname}) + self.assertFalse(form.is_valid()) + self.assertIn("sn", form.errors) + + def test_postal_addresses_round_trip_multiple_multiline_values(self): + values = profile_values() + initial = profiles.initial(values) + self.assertEqual(initial["postalAddress"], "1 Example Street\nExample City\n\n2 Other Road\nOther City") + form = self.form(initial) + self.assertTrue(form.is_valid(), form.errors) + self.assertEqual(form.cleaned_data["postalAddress"], values["postalAddress"]) + self.assertEqual(ProfileValuesField(postal=True).clean(" A \r\n B \r\n\r\n A\nB "), ["A$B"]) + + def test_postal_literal_dollar_and_backslash_round_trip(self): + stored = [r"Unit \24 Five$Backslash \5c24", r"Another\5cAddress"] + text = profiles.initial(profile_values(postalAddress=stored))["postalAddress"] + self.assertEqual(ProfileValuesField(postal=True).clean(text), stored) + + def test_http_https_urls_with_optional_labels(self): + form = self.form({"labeledURI": "https://example.org/a?q=b Website\nhttp://example.net/"}) + self.assertTrue(form.is_valid(), form.errors) + self.assertEqual(form.cleaned_data["labeledURI"], ["https://example.org/a?q=b Website", "http://example.net/"]) + + def test_urls_reject_other_schemes_credentials_and_backslashes(self): + for url in ( + "javascript:alert(1)", "data:text/html,test", "file:///etc/passwd", + "ftp://example.org/file", "//example.org/", "https://user@example.org/", + "https://user:secret@example.org/", "https://example.org/\\path", "not-a-url", + ): + with self.subTest(url=url): + form = self.form({"labeledURI": url}) + self.assertFalse(form.is_valid()) + self.assertIn("labeledURI", form.errors) + + def test_supported_ssh_keys_multivalued_and_deduplicated_by_key_not_comment(self): + keys = [ + ed25519.Ed25519PrivateKey.generate().public_key(), + rsa.generate_private_key(public_exponent=65537, key_size=2048).public_key(), + ec.generate_private_key(ec.SECP256R1()).public_key(), + ec.generate_private_key(ec.SECP384R1()).public_key(), + ec.generate_private_key(ec.SECP521R1()).public_key(), + ] + lines = [key.public_bytes(serialization.Encoding.OpenSSH, serialization.PublicFormat.OpenSSH).decode() for key in keys] + form = self.form({"sshPublicKey": "\n".join([lines[0] + " laptop", lines[0] + " other comment", *lines[1:]])}) + self.assertTrue(form.is_valid(), form.errors) + self.assertEqual(form.cleaned_data["sshPublicKey"], [lines[0] + " laptop", *lines[1:]]) + + def test_invalid_ssh_options_or_private_material_rejected(self): + public = ed25519.Ed25519PrivateKey.generate().public_key().public_bytes( + serialization.Encoding.OpenSSH, serialization.PublicFormat.OpenSSH, + ).decode() + for value in ("ssh-ed25519 invalid", 'command="id" ' + public, "ssh-dss invalid", "-----BEGIN OPENSSH PRIVATE KEY-----"): + with self.subTest(value=value): + form = self.form({"sshPublicKey": value}) + self.assertFalse(form.is_valid()) + self.assertIn("sshPublicKey", form.errors) + + def test_jpeg_is_resized_reencoded_and_metadata_removed(self): + raw = jpeg_bytes() + form = self.form(files={"jpegPhoto": photo_upload(raw)}) + self.assertTrue(form.is_valid(), form.errors) + clean = form.cleaned_data["jpegPhoto"] + self.assertIsInstance(clean, bytes) + self.assertNotEqual(clean, raw) + with Image.open(BytesIO(clean)) as image: + self.assertEqual(image.format, "JPEG") + self.assertEqual(image.mode, "RGB") + self.assertEqual(image.size, (512, 256)) + self.assertFalse(image.getexif()) + self.assertNotIn("icc_profile", image.info) + self.assertNotIn("comment", image.info) + + def test_fake_truncated_empty_and_non_jpeg_uploads_rejected(self): + png = BytesIO() + Image.new("RGB", (2, 2)).save(png, "PNG") + for raw in (b"", b"not an image", b"\xff\xd8fake JPEG", jpeg_bytes()[:30], png.getvalue()): + with self.subTest(length=len(raw)): + form = self.form(files={"jpegPhoto": photo_upload(raw)}) + self.assertFalse(form.is_valid()) + self.assertIn("jpegPhoto", form.errors) + + def test_oversize_upload_size_and_actual_read_limits(self): + for declared_size in (MAX_PHOTO_BYTES + 1, 1): + with self.subTest(declared_size=declared_size): + upload = photo_upload(b"x" * (MAX_PHOTO_BYTES + 1)) + upload.size = declared_size + form = self.form(files={"jpegPhoto": upload}) + self.assertFalse(form.is_valid()) + self.assertIn("jpegPhoto", form.errors) + + def test_photo_dimension_limit(self): + form = self.form(files={"jpegPhoto": photo_upload(jpeg_bytes((4097, 1)))}) + self.assertFalse(form.is_valid()) + self.assertIn("jpegPhoto", form.errors) + + def test_photo_omission_removal_and_conflicting_replacement(self): + form = self.form() + self.assertTrue(form.is_valid(), form.errors) + self.assertIsNone(form.cleaned_data["jpegPhoto"]) + form = self.form({"remove_photo": "on"}) + self.assertTrue(form.is_valid(), form.errors) + self.assertTrue(form.cleaned_data["remove_photo"]) + form = self.form({"remove_photo": "on"}, {"jpegPhoto": photo_upload()}) + self.assertFalse(form.is_valid()) + self.assertIn("remove_photo", form.errors) + + def test_current_and_new_password_whitespace_preserved_not_rendered(self): + new_password = " " + secrets.token_urlsafe(24) + " " + form = ProfilePasswordForm(data={ + "current_password": self.password, + "password": new_password, "confirm_password": new_password, + }) + self.assertTrue(form.is_valid(), form.errors) + self.assertEqual(form.cleaned_data["current_password"], self.password) + self.assertEqual(form.cleaned_data["password"], new_password) + self.assertEqual(form.cleaned_data["confirm_password"], new_password) + self.assertNotIn(self.password, form.as_p()) + self.assertNotIn(new_password, form.as_p()) + + def test_password_required_length_and_confirmation(self): + for data, field in ( + ({"current_password": ""}, "current_password"), + ({"current_password": "x" * 1025}, "current_password"), + ): + with self.subTest(field=field, length=len(data[field])): + form = CurrentPasswordForm(data=data) + self.assertFalse(form.is_valid()) + self.assertIn(field, form.errors) + for password, confirm in (("short", "short"), (self.password, self.password.strip())): + form = ProfilePasswordForm(data={"current_password": self.password, "password": password, "confirm_password": confirm}) + self.assertFalse(form.is_valid()) + + def test_email_casefold_and_current_password_required(self): + form = ProfileEmailForm(data={"email": "Alice.New@EXAMPLE.ORG", "current_password": self.password}) + self.assertTrue(form.is_valid(), form.errors) + self.assertEqual(form.cleaned_data["email"], "alice.new@example.org") + form = ProfileEmailForm(data={"email": "alice.new@example.org"}) + self.assertFalse(form.is_valid()) + self.assertIn("current_password", form.errors) + + +class ProfileViewTests(TestCase): + def setUp(self): + self.account = Account.objects.create( + uid="alice", email="alice@example.org", oidc_issuer=settings.DEX_ISSUER, + oidc_sub="sub-alice", ldap_uuid="uuid-alice", ldap_dn=ALICE_DN, + ) + self.password = " " + secrets.token_urlsafe(24) + " " + self.values = profile_values() + self.sign_in(self.client, self.account) + self.profile_url = reverse("members:profile") + self.photo_url = reverse("members:profile_photo") + self.confirm_url = reverse("members:confirm_profile_email") + # Unlike a loose read mock, this checks the saved pin seen by the view. + self.read = self.start_patch("members.views.profiles.read_profile", side_effect=self.read_profile) + self.update = self.start_patch("members.views.profiles.update_profile") + self.change_password = self.start_patch("members.views.profiles.change_password") + self.verify_password = self.start_patch("members.views.profiles.verify_password") + self.change_email = self.start_patch("members.views.profiles.change_email") + self.start_patch("members.directory.connection", side_effect=AssertionError("Offline test attempted service LDAP")) + self.start_patch("members.directory._connection", side_effect=AssertionError("Offline test attempted user LDAP")) + + def start_patch(self, name, **kwargs): + patcher = patch(name, **kwargs) + mock = patcher.start() + self.addCleanup(patcher.stop) + return mock + + def sign_in(self, client, account): + session = client.session + session["account_id"] = str(account.pk) + session.save() + + def read_profile(self, account): + self.assertEqual(account.pk, self.account.pk) + saved = Account.objects.get(pk=account.pk) + self.assertEqual((account.uid, account.ldap_uuid, account.ldap_dn), (saved.uid, "uuid-alice", ALICE_DN)) + return copy.deepcopy(self.values) + + def profile_data(self, **overrides): + return { + **profiles.initial(self.values), "action": "profile", + "revision": profiles.revision(self.account, self.values), + "current_password": self.password, **overrides, + } + + def assert_no_writes(self): + for operation in (self.update, self.change_password, self.verify_password, self.change_email): + operation.assert_not_called() + + def pending(self, *, account=None, expires_at=None, ldap_uuid="uuid-alice"): + token = secrets.token_urlsafe(32) + pending = ProfileEmailChange.objects.create( + token_hash=hashlib.sha256(token.encode()).hexdigest(), + account=account or self.account, email="alice.new@example.org", + ldap_uuid=ldap_uuid, expires_at=expires_at or timezone.now() + timedelta(hours=1), + ) + return token, pending + + def exchange(self, token): + return self.client.get(reverse("members:verify_profile_email", args=[token])) + + def test_anonymous_endpoints_redirect_to_sign_in_page(self): + client = Client() + urls = (self.profile_url, self.photo_url, self.confirm_url, reverse("members:verify_profile_email", args=[secrets.token_urlsafe(32)])) + for url in urls: + with self.subTest(url=url): + self.assertRedirects(client.get(url), reverse("members:home"), fetch_redirect_response=False) + self.read.assert_not_called() + self.assert_no_writes() + + def test_get_loads_full_profile_and_revision_for_signed_in_account_only(self): + response = self.client.get(self.profile_url, {"uid": "mallory", "dn": "cn=admin", "account_id": str(secrets.token_hex(16))}) + self.assertEqual(response.status_code, 200) + self.assertTrue(response.context["profile_loaded"]) + self.assertEqual(response.context["email_values"], self.values["mail"]) + self.assertEqual(response.context["form"].initial, profiles.initial(self.values)) + proof = signing.loads(response.context["revision"], salt="ldap-profile", max_age=3600) + self.assertEqual(proof["account"], str(self.account.pk)) + self.assertEqual(proof["uuid"], "uuid-alice") + self.assertContains(response, 'name="csrfmiddlewaretoken"') + self.assert_no_writes() + + def test_csrf_required_for_each_mutating_action_and_email_confirmation(self): + client = Client(enforce_csrf_checks=True) + self.sign_in(client, self.account) + token, pending = self.pending() + self.assertEqual(client.get(reverse("members:verify_profile_email", args=[token])).status_code, 302) + for action in ("profile", "email", "password"): + with self.subTest(action=action): + self.assertEqual(client.post(self.profile_url, self.profile_data(action=action)).status_code, 403) + self.assertEqual(client.post(self.confirm_url, {"current_password": self.password}).status_code, 403) + self.assertTrue(ProfileEmailChange.objects.filter(pk=pending.pk).exists()) + self.assert_no_writes() + # A real cookie/token pair passes middleware; unknown action reaches 400. + client.get(self.profile_url) + csrf = client.cookies[settings.CSRF_COOKIE_NAME].value + self.assertEqual(client.post(self.profile_url, {"action": "unknown", "csrfmiddlewaretoken": csrf}).status_code, 400) + + def test_unknown_or_missing_action_is_bad_request(self): + for data in ({}, {"action": "delete"}, {"action": "userPassword"}): + with self.subTest(data=data): + self.assertEqual(self.client.post(self.profile_url, data).status_code, 400) + self.assert_no_writes() + + def test_load_failure_disables_all_editing_get_and_post_and_redacts_logs(self): + self.read.side_effect = LDAPException(self.password) + with self.assertLogs("members.views", level="ERROR") as captured: + for method in (self.client.get, self.client.post): + response = method(self.profile_url, self.profile_data()) + self.assertEqual(response.status_code, 503) + self.assertFalse(response.context["profile_loaded"]) + self.assertNotContains(response, "/", views.verify_profile_email, name="verify_profile_email"), + path("profile/email/confirm/", views.confirm_profile_email, name="confirm_profile_email"), path("checkout//", views.checkout, name="checkout"), path("billing/", views.portal, name="portal"), path("stripe/webhook/", views.webhook, name="webhook"), diff --git a/membership/members/views.py b/membership/members/views.py index 2bf1751..8410ec3 100755 --- a/membership/members/views.py +++ b/membership/members/views.py @@ -11,6 +11,7 @@ import stripe from authlib.integrations.django_client import OAuth from django.conf import settings from django.core.validators import validate_email +from django.core.files.uploadedfile import SimpleUploadedFile from django.core.mail import send_mail from django.db import ProgrammingError, transaction from django.http import HttpResponse @@ -23,9 +24,10 @@ from django.views.decorators.csrf import csrf_exempt from django.views.decorators.debug import sensitive_post_parameters from django.views.decorators.http import require_GET, require_POST, require_http_methods -from . import billing, directory, shkeeper +from . import billing, directory, profiles, shkeeper from .forms import PasswordForm, RegistrationForm -from .models import Account, CryptoInvoice, ManagedGroup, RateWindow, Registration, Subscription, WebhookEvent +from .profile_forms import CurrentPasswordForm, ProfileEmailForm, ProfileForm, ProfilePasswordForm +from .models import Account, CryptoInvoice, ManagedGroup, ProfileEmailChange, RateWindow, Registration, Subscription, WebhookEvent logger = logging.getLogger(__name__) @@ -240,6 +242,139 @@ def logout(request): return redirect("members:home") +def _profile_context(account, values): + return {"account": account, "profile_loaded": True, "form": ProfileForm(initial=profiles.initial(values)), "email_form": ProfileEmailForm(), "password_form": ProfilePasswordForm(), "has_photo": bool(values["jpegPhoto"]), "email_values": values["mail"], "revision": profiles.revision(account, values)} + + +@sensitive_post_parameters() +@require_http_methods(["GET", "POST"]) +@account_required +def profile(request): + member = request.member + # Bound before any password verification; never log posted values or LDAP + # diagnostic messages. Account and IP limits protect directory binds. + if limited("profile-read", str(member.pk), 120): + return HttpResponse("Please try again later.", status=429) + try: + values = profiles.read_profile(member) + except Exception as exc: + logger.error("Profile read failed account=%s kind=%s", member.pk, type(exc).__name__) + return render(request, "profile.html", {"account": member, "profile_loaded": False}, status=503) + context = _profile_context(member, values) + context.update(saved=request.GET.get("saved") == "1", email_sent=request.GET.get("email_sent") == "1") + if request.method == "POST": + if limited("profile-write", str(member.pk), 20) or limited("profile-write-ip", client_ip(request), 40): + 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: + return HttpResponse(status=400) + form_key, form_class = forms[action] + form = form_class(request.POST, request.FILES) if action == "profile" else form_class(request.POST) + context[form_key] = form + if form.is_valid(): + try: + # Serialize directory pinning and edits against the worker. + with transaction.atomic(): + member = Account.objects.select_for_update().get(pk=member.pk) + if action == "profile": + profiles.update_profile(member, form.cleaned_data, request.POST.get("revision", "")) + elif action == "password": + profiles.change_password(member, form.cleaned_data["current_password"], form.cleaned_data["password"]) + else: + profiles.verify_password(member, form.cleaned_data["current_password"]) + token = secrets.token_urlsafe(32) + digest = hashlib.sha256(token.encode()).hexdigest() + ProfileEmailChange.objects.filter(account=member).delete() + ProfileEmailChange.objects.create(token_hash=digest, account=member, email=form.cleaned_data["email"], ldap_uuid=member.ldap_uuid, expires_at=timezone.now() + timedelta(hours=1)) + url = settings.PUBLIC_URL + reverse("members:verify_profile_email", args=[token]) + send_mail("Verify your new eom.dev email", f"Confirm this email address for your account (expires in one hour):\n\n{url}\n\nYou must sign in to the same account and enter your current password to confirm. If you did not request this, ignore this message.", settings.DEFAULT_FROM_EMAIL, [form.cleaned_data["email"]]) + logger.info("Profile change completed account=%s action=%s", member.pk, action) + if action == "password": + request.session.flush() + return page(request, title="Password changed", message="Your password was changed. Sign in again. Existing Dex and other service sessions are not automatically signed out.") + return redirect(reverse("members:profile") + ("?email_sent=1" if action == "email" else "?saved=1")) + except Exception as exc: + logger.error("Profile change failed account=%s kind=%s", member.pk, type(exc).__name__) + form.add_error(None, "Changes could not be completed. Check your current password and reload your profile before retrying. Contact the administrator if this continues.") + # Keep the original revision on validation errors, never silently allow + # a stale bound form to overwrite concurrently updated directory data. + if action == "profile": + context["revision"] = request.POST.get("revision", "") + return render(request, "profile.html", context) + + +@require_GET +@account_required +def profile_photo(request): + if limited("profile-photo", str(request.member.pk), 120): + return HttpResponse(status=429) + try: + values = profiles.read_profile(request.member) + if not values["jpegPhoto"]: + return HttpResponse(status=404) + # Existing LDAP photos are untrusted too: rewrite before browser delivery. + raw = values["jpegPhoto"][0] + if not isinstance(raw, bytes): + raise ValueError("Invalid stored image.") + form = ProfileForm(files={"jpegPhoto": SimpleUploadedFile("photo.jpg", raw, content_type="image/jpeg")}) + form.is_valid() + image = form.cleaned_data.get("jpegPhoto") + if not image: + return HttpResponse(status=404) + response = HttpResponse(image, content_type="image/jpeg") + response["Content-Disposition"] = 'inline; filename="profile.jpg"' + return response + except Exception as exc: + logger.error("Profile photo failed account=%s kind=%s", request.member.pk, type(exc).__name__) + return HttpResponse(status=404) + + +@require_GET +@account_required +def verify_profile_email(request, token): + if limited("profile-email-verify", client_ip(request), 60): + return HttpResponse(status=429) + digest = hashlib.sha256(token.encode()).hexdigest() + if not ProfileEmailChange.objects.filter(pk=digest, account=request.member, expires_at__gt=timezone.now()).exists(): + return page(request, title="Link unavailable", message="Request a new email change from your profile.") + request.session["profile_email_hash"] = digest + return redirect("members:confirm_profile_email") + + +@sensitive_post_parameters("current_password") +@require_http_methods(["GET", "POST"]) +@account_required +def confirm_profile_email(request): + digest = request.session.get("profile_email_hash") + pending = ProfileEmailChange.objects.filter(pk=digest, account=request.member, expires_at__gt=timezone.now()).first() + if not pending: + return page(request, title="Link unavailable", message="Request a new email change from your profile.") + form = CurrentPasswordForm(request.POST if request.method == "POST" else None) + if request.method == "POST": + if limited("profile-write", str(request.member.pk), 20) or limited("profile-write-ip", client_ip(request), 40): + return HttpResponse("Please try again later.", status=429) + if form.is_valid(): + try: + with transaction.atomic(): + member = Account.objects.select_for_update().get(pk=request.member.pk) + pending = ProfileEmailChange.objects.select_for_update().get(pk=digest, account=member, expires_at__gt=timezone.now()) + profiles.read_profile(member) + if member.ldap_uuid != pending.ldap_uuid: + raise ValueError("Directory identity changed.") + profiles.change_email(member, form.cleaned_data["current_password"], pending.email) + member.email = pending.email + member.save(update_fields=["email"]) + ProfileEmailChange.objects.filter(account=member).delete() + request.session.pop("profile_email_hash", None) + request.session.cycle_key() + return redirect(reverse("members:profile") + "?saved=1") + except Exception as exc: + logger.error("Profile email confirmation failed account=%s kind=%s", request.member.pk, type(exc).__name__) + form.add_error(None, "Email change could not be completed. Check your current password or request a new verification link.") + return render(request, "profile.html", {"account": request.member, "confirm_email": True, "confirmation_email": pending.email, "confirmation_form": form}) + + def stripe_redirect(url, host): parsed = urlsplit(url or "") if parsed.scheme != "https" or parsed.hostname != host or parsed.username: diff --git a/membership/requirements.txt b/membership/requirements.txt index 415afd1..6de352c 100755 --- a/membership/requirements.txt +++ b/membership/requirements.txt @@ -5,3 +5,5 @@ ldap3==2.9.1 stripe==14.4.1 psycopg[binary]>=3.2,<4 gunicorn>=23,<24 +Pillow>=12.0.0,<13 +cryptography>=50,<51 diff --git a/membership/templates/members.html b/membership/templates/members.html index 60f68e1..e21de9c 100755 --- a/membership/templates/members.html +++ b/membership/templates/members.html @@ -48,6 +48,7 @@