From 54a8ac52658de4cbe0573da03fc214844609f3ca Mon Sep 17 00:00:00 2001 From: "Teodoro B. Mendes" Date: Tue, 14 Jul 2026 17:22:32 -0300 Subject: [PATCH 1/2] feat(2440): add v3 commit email card feat(2440): wire commit email card into v3 profile edit fix(2440): improve commit email add rows and error handling fix(2440): order commit emails verified first, then pending fix(2440): withdraw pending claim instead of deleting the email record feat(2440): bind commit email claims at verification via claimed_by feat(2440): v3 confirm pages for commit email verification link feat(2440): surface claim expiry in email and confirm pages fix(2440): show concrete expiry datetime on confirm page fix(2440): keep verification failure page generic fix(2440): anonymous wording when claimant has no display name fix(2440): single claimed-emails queryset so profile render can't clobber it feat(2440): second-person copy when the claimant opens their own link feat(2440): verify link only works in the claimant's own session feat(2440): hide expired pending claims from the commit email lists fix: isolate test cache from dev Redis so waffle flags survive test runs fix: require login on legacy commit email claim endpoint fix: only the claimant's active sibling claims keep legacy attribution fix: serialize claim lifecycle transitions with row locks fix: include spinner border inside its 24px box test: cover resend auth boundaries and foreign verified email exclusion fix: restore legacy verify screen and flow behind the v3 flag fix: restore legacy verification email when the v3 flag is off fix(2440): restore legacy claim flow verbatim, v3 gets its own form and ask method fix(2440): legacy ask also records claimed_by so claims survive the flag flip fix(2440): address coderabbit feedback fix(2440): render generic failure page for malformed verify tokens fix(2440): always render the add-email row per Figma and for no-JS fix(2440): associate the Email label with the add-row input fix(2440): use space token for 8px margins in commit email card fix(2440): scope ask_to_claim save to the claim fields --- config/test_settings.py | 12 + config/urls.py | 18 + libraries/constants.py | 3 + libraries/forms.py | 76 ++ .../0042_commitauthoremail_claimed_by.py | 28 + ...3_backfill_commitauthoremail_claimed_by.py | 30 + libraries/models.py | 182 ++- libraries/tasks.py | 72 +- libraries/tests/test_v3_commit_email_views.py | 1019 +++++++++++++++++ libraries/utils.py | 12 + libraries/views.py | 206 +++- static/css/v3/commit-email-card.css | 192 ++++ static/css/v3/components.css | 1 + templates/includes/icon.html | 3 + templates/v3/includes/_commit_email_card.html | 26 + .../v3/includes/_commit_email_card_body.html | 84 ++ .../v3/libraries/commit_email_confirm.html | 134 +++ .../libraries/email/verify_commit_email.txt | 11 + templates/v3/user_profile_edit.html | 21 +- users/forms.py | 28 - users/tests/test_v3_profile_edit.py | 125 ++ users/views.py | 19 +- 22 files changed, 2221 insertions(+), 81 deletions(-) create mode 100644 libraries/migrations/0042_commitauthoremail_claimed_by.py create mode 100644 libraries/migrations/0043_backfill_commitauthoremail_claimed_by.py create mode 100644 libraries/tests/test_v3_commit_email_views.py create mode 100644 static/css/v3/commit-email-card.css create mode 100644 templates/v3/includes/_commit_email_card.html create mode 100644 templates/v3/includes/_commit_email_card_body.html create mode 100644 templates/v3/libraries/commit_email_confirm.html create mode 100644 templates/v3/libraries/email/verify_commit_email.txt diff --git a/config/test_settings.py b/config/test_settings.py index 3ad22f331..ac01f8f66 100644 --- a/config/test_settings.py +++ b/config/test_settings.py @@ -26,6 +26,18 @@ def __getitem__(self, item): EMAIL_BACKEND = "django.core.mail.backends.locmem.EmailBackend" +# Tests must not share the dev server's Redis: waffle caches Flag rows from +# whichever database wrote last, so a test run would poison the dev cache +# with test-database flag state (e.g. the v3 flag going inactive after every +# pytest run until re-saved in the admin) +CACHES = { + "default": {"BACKEND": "django.core.cache.backends.locmem.LocMemCache"}, + "static_content": { + "BACKEND": "django.core.cache.backends.locmem.LocMemCache", + "LOCATION": "static_content", + }, +} + MIGRATION_MODULES = DisableMigrations() # User a faster password hasher diff --git a/config/urls.py b/config/urls.py index 387074a54..725d242b9 100644 --- a/config/urls.py +++ b/config/urls.py @@ -52,6 +52,9 @@ CommitAuthorEmailCreateView, VerifyCommitEmailView, CommitEmailResendView, + V3CommitAuthorEmailCreateView, + V3CommitAuthorEmailWithdrawView, + V3CommitAuthorEmailResendView, ) from news.feeds import AtomNewsFeed, RSSNewsFeed from news.views import ( @@ -264,6 +267,21 @@ CommitEmailResendView.as_view(), name="commit-author-email-verify-resend", ), + path( + "libraries/commit_author_email_create_v3/", + V3CommitAuthorEmailCreateView.as_view(), + name="v3-commit-author-email-create", + ), + path( + "libraries/commit_author_email_v3//withdraw/", + V3CommitAuthorEmailWithdrawView.as_view(), + name="v3-commit-author-email-withdraw", + ), + path( + "libraries/commit_author_email_v3//resend/", + V3CommitAuthorEmailResendView.as_view(), + name="v3-commit-author-email-resend", + ), # Redirect for '/libs/' legacy boost.org urls. re_path( r"^libs/(?P[-\w]+)/?$", diff --git a/libraries/constants.py b/libraries/constants.py index 51d2fbcbc..009ba3946 100644 --- a/libraries/constants.py +++ b/libraries/constants.py @@ -369,3 +369,6 @@ DOCKER_CONTAINER_URL_WEB = "http://web:8000" RELEASE_REPORT_AUTHORS_PER_PAGE_THRESHOLD = 6 + +# How long a commit email verification link stays valid (seconds). +COMMIT_EMAIL_CLAIM_MAX_AGE = 24 * 60 * 60 # 24 hours diff --git a/libraries/forms.py b/libraries/forms.py index 28d8f8a33..fd592ba76 100644 --- a/libraries/forms.py +++ b/libraries/forms.py @@ -601,3 +601,79 @@ def clean_email(self): raise forms.ValidationError(msg) return email + + +class V3CommitAuthorEmailForm(Form): + """The v3 claim form: same field as the legacy CommitAuthorEmailForm + above (which is preserved untouched), but validating against the + claimed_by claim model where attribution is only bound at verification. + """ + + email = forms.EmailField() + + class Meta: + fields = ["email"] + + def __init__(self, *args, user=None, **kwargs): + self.user = user + self.commit_author_email = None + super().__init__(*args, **kwargs) + + def clean_email(self): + """Emails should have been created by the commit import process, so we + need to ensure the email exists, then check the claim state of the row + itself before the author-level binding: author.user is also set by the + email/github matching heuristics, so on its own it only blocks a claim + when a different user has verified a sibling email - otherwise + verification is what settles ownership.""" + email = self.cleaned_data.get("email") + commit_author_email = CommitAuthorEmail.objects.filter( + email__iexact=email + ).first() + + if not commit_author_email: + raise forms.ValidationError( + "Email address is not associated with any commits." + ) + + claimant = commit_author_email.claimed_by + if commit_author_email.claim_verified: + owner = claimant or commit_author_email.author.user + if self.user is not None and owner == self.user: + msg = "This email address is already associated with your account." + else: + msg = ( + "This email address has already been claimed by another user. " + "Report an issue if this is incorrect." + ) + raise forms.ValidationError(msg) + + has_open_claim = ( + commit_author_email.claim_hash is not None + and claimant is not None + and not commit_author_email.is_verification_email_expired() + ) + if has_open_claim: + if self.user is not None and claimant == self.user: + msg = "A verification email is already pending for this address." + else: + msg = "This email address has a verification pending by another user." + raise forms.ValidationError(msg) + + author_user = commit_author_email.author.user + if author_user is not None and author_user != self.user: + other_verified_sibling = ( + commit_author_email.author.commitauthoremail_set.filter( + claim_verified=True + ) + .exclude(pk=commit_author_email.pk) + .exists() + ) + if other_verified_sibling: + raise forms.ValidationError( + "This email address is already associated with another user. " + "Report an issue if this is incorrect." + ) + + self.commit_author_email = commit_author_email + return email diff --git a/libraries/migrations/0042_commitauthoremail_claimed_by.py b/libraries/migrations/0042_commitauthoremail_claimed_by.py new file mode 100644 index 000000000..d7b9b9ec8 --- /dev/null +++ b/libraries/migrations/0042_commitauthoremail_claimed_by.py @@ -0,0 +1,28 @@ +# Generated by Django 6.0.2 on 2026-07-15 13:30 + +import django.db.models.deletion +from django.conf import settings +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("libraries", "0041_category_short_description"), + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ] + + operations = [ + migrations.AddField( + model_name="commitauthoremail", + name="claimed_by", + field=models.ForeignKey( + blank=True, + help_text="Who asked to claim this email. Public attribution (author.user) is only bound at verification.", + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name="commit_email_claims", + to=settings.AUTH_USER_MODEL, + ), + ), + ] diff --git a/libraries/migrations/0043_backfill_commitauthoremail_claimed_by.py b/libraries/migrations/0043_backfill_commitauthoremail_claimed_by.py new file mode 100644 index 000000000..73cb6396a --- /dev/null +++ b/libraries/migrations/0043_backfill_commitauthoremail_claimed_by.py @@ -0,0 +1,30 @@ +from django.db import migrations +from django.db.models import OuterRef, Subquery + + +def backfill_claimed_by(apps, schema_editor): + """Historically a claim was recorded by binding author.user at ask time, + so for every row that has a claim token and a bound author, that user is + the best available signal of who asked. Covers verified claims and open + pending ones. Rows bound only by the email/github matching heuristics + have no claim_hash and are deliberately left alone. + """ + CommitAuthorEmail = apps.get_model("libraries", "CommitAuthorEmail") + CommitAuthor = apps.get_model("libraries", "CommitAuthor") + CommitAuthorEmail.objects.filter( + author__user__isnull=False, claim_hash__isnull=False + ).update( + claimed_by_id=Subquery( + CommitAuthor.objects.filter(pk=OuterRef("author_id")).values("user_id")[:1] + ) + ) + + +class Migration(migrations.Migration): + dependencies = [ + ("libraries", "0042_commitauthoremail_claimed_by"), + ] + + operations = [ + migrations.RunPython(backfill_claimed_by, migrations.RunPython.noop), + ] diff --git a/libraries/models.py b/libraries/models.py index f6da98e97..8f29b194a 100644 --- a/libraries/models.py +++ b/libraries/models.py @@ -1,3 +1,4 @@ +import logging import os import re import uuid @@ -7,7 +8,7 @@ from django.core.cache import caches from django.db import models, transaction -from django.db.models import Sum +from django.db.models import Q, Sum from django.db.models.signals import pre_delete from django.dispatch import receiver from django.urls import reverse @@ -31,7 +32,7 @@ ) from mailing_list.models import EmailData from versions.models import ReportConfiguration -from .constants import LIBRARY_GITHUB_URL_OVERRIDES +from .constants import COMMIT_EMAIL_CLAIM_MAX_AGE, LIBRARY_GITHUB_URL_OVERRIDES from .utils import ( generate_random_string, @@ -39,6 +40,8 @@ generate_release_report_filename, ) +logger = logging.getLogger(__name__) + class Category(models.Model): """ @@ -168,6 +171,35 @@ class CommitAuthorEmail(models.Model): claim_hash = models.UUIDField(null=True, blank=True) claim_hash_expiration = models.DateTimeField(default=timezone.now) claim_verified = models.BooleanField(default=False) + claimed_by = models.ForeignKey( + settings.AUTH_USER_MODEL, + null=True, + blank=True, + on_delete=models.SET_NULL, + related_name="commit_email_claims", + help_text="Who asked to claim this email. Public attribution " + "(author.user) is only bound at verification.", + ) + + @classmethod + def claimed_by_user(cls, user): + """Emails the user actually claimed: verified ones, or pending ones + with an open, unexpired ask, verified first. Expired asks are not + cleared anywhere - they are simply never shown, their token is dead, + and the next ask (by anyone) overwrites the claim fields. A bound + author's other imported emails are not the user's business. + """ + return ( + cls.objects.filter(claimed_by=user) + .filter( + Q(claim_verified=True) + | Q( + claim_hash__isnull=False, + claim_hash_expiration__gt=timezone.now(), + ) + ) + .order_by("-claim_verified", "email") + ) def is_verification_email_expired(self): return timezone.now() > self.claim_hash_expiration @@ -175,6 +207,10 @@ def is_verification_email_expired(self): def trigger_verification_email(self, request): self.author.user = request.user self.author.save(update_fields=["user"]) + # the one delta from the pre-v3 flow: record the claimant so claims + # made while the v3 flag is off still show in the v3 card once it + # flips on + self.claimed_by = request.user self.claim_hash = uuid.uuid4() self.claim_hash_expiration = timezone.now() + timedelta(days=1) self.save() @@ -192,6 +228,148 @@ def trigger_verification_email(self, request): return CommitAuthorEmail.objects.filter(author__user=self.author.user) + def ask_to_claim(self, request): + """Ask to claim this email for request.user - the v3 flow. The legacy + flow is trigger_verification_email above, preserved untouched. + + Side-effect-free beyond this row: records the claimant and a fresh + token and emails the address. author.user - the field driving public + attribution - is only bound once the inbox owner confirms + (see verify_claim). + + Returns None without side effects if the row is no longer claimable + by request.user - the form validated an unlocked row, so a concurrent + request may have verified it or opened a competing claim since. + """ + with transaction.atomic(): + locked = type(self).objects.select_for_update().get(pk=self.pk) + claimed_by_other = ( + locked.claimed_by_id is not None + and locked.claimed_by_id != request.user.pk + and locked.claim_hash is not None + and locked.claim_hash_expiration > timezone.now() + ) + if locked.claim_verified or claimed_by_other: + return None + + self.claimed_by = request.user + self.claim_hash = uuid.uuid4() + self.claim_hash_expiration = timezone.now() + timedelta( + seconds=COMMIT_EMAIL_CLAIM_MAX_AGE + ) + # self predates the lock; a full-row save would clobber + # concurrent writes to unrelated fields with the stale snapshot + self.save( + update_fields=["claimed_by", "claim_hash", "claim_hash_expiration"] + ) + + url = request.build_absolute_uri( + reverse( + "commit-author-email-verify", + kwargs={"token": self.claim_hash}, + ) + ) + # here to avoid circular import + from .tasks import send_commit_author_email_verify_mail + + # may be blank - the email falls back to an anonymous variant rather + # than leaking the claimant's account email + send_commit_author_email_verify_mail.delay( + self.email, + url, + request.user.display_name, + v3=True, + ) + + return CommitAuthorEmail.objects.filter(claimed_by=request.user) + + def verify_claim(self): + """Complete a claim: mark this row verified and bind public + attribution (author.user) to the claimant. + + A verified claim outranks the email/github matching heuristics that + also set author.user (see tasks.update_commit_author_user), but it + never steals an author on which a different user holds another + verified claim - in that conflict the row is verified without + rebinding and the conflict is logged for admins. + + Returns True when attribution was bound to the claimant, False + when a conflicting verified sibling claim left author.user alone. + """ + with transaction.atomic(): + # locking the author serializes sibling verifications, so two + # can't both pass the conflict check before either commits + author = CommitAuthor.objects.select_for_update().get(pk=self.author_id) + self.claim_verified = True + self.claim_hash_expiration = timezone.now() + self.save(update_fields=["claim_verified", "claim_hash_expiration"]) + + conflicting_claim = ( + author.commitauthoremail_set.exclude(pk=self.pk) + .filter(claim_verified=True, claimed_by__isnull=False) + .exclude(claimed_by=self.claimed_by) + .exists() + ) + if conflicting_claim: + logger.warning( + "Verified commit email claim %s (user %s) conflicts with a " + "verified sibling claim on author %s; author.user left as %s", + self.pk, + self.claimed_by_id, + self.author_id, + author.user_id, + ) + return False + author.user = self.claimed_by + author.save(update_fields=["user"]) + return True + + def withdraw_claim(self): + """Undo a pending claim without touching the imported email record. + + The commit importer owns CommitAuthorEmail rows, so withdrawing an + unverified claim must never delete one; it only clears the claim + fields on this row. Unbinding author.user remains solely as a + fallback for legacy rows claimed before the claimed_by field + existed, which bound the author at ask time. + """ + with transaction.atomic(): + claimant = self.claimed_by + self.claimed_by = None + self.claim_hash = None + self.claim_hash_expiration = timezone.now() + self.save( + update_fields=["claimed_by", "claim_hash", "claim_hash_expiration"] + ) + + if claimant is None: + return + # same author lock as verify_claim, so the sibling check below + # can't race a sibling verification into a stale unbind + author = CommitAuthor.objects.select_for_update().get(pk=self.author_id) + if author.user != claimant: + return + # only the claimant's own active claims justify keeping the + # binding: expired tokens are never cleared (so they must not + # count forever), and other users' asks never bound this author + # to this claimant + author_still_bound = ( + author.commitauthoremail_set.exclude(pk=self.pk) + .filter( + Q(claim_verified=True, claimed_by=claimant) + | Q( + claim_verified=False, + claimed_by=claimant, + claim_hash__isnull=False, + claim_hash_expiration__gt=timezone.now(), + ) + ) + .exists() + ) + if not author_still_bound: + author.user = None + author.save(update_fields=["user"]) + def __str__(self): return f"{self.author.name}: {self.email}" diff --git a/libraries/tasks.py b/libraries/tasks.py index d740e8b51..05f91aa1c 100644 --- a/libraries/tasks.py +++ b/libraries/tasks.py @@ -1,7 +1,8 @@ from datetime import date, timedelta from celery import shared_task, chain -from django.core.mail import EmailMultiAlternatives +from django.core.mail import EmailMultiAlternatives, send_mail +from django.template.loader import render_to_string from django.core.management import call_command import structlog from django.db.models.functions import ExtractWeek, ExtractIsoYear @@ -30,12 +31,14 @@ from users.tasks import User from versions.models import Version from .constants import ( + COMMIT_EMAIL_CLAIM_MAX_AGE, LIBRARY_DOCS_EXCEPTIONS, LIBRARY_DOCS_MISSING, VERSION_DOCS_MISSING, DOCKER_CONTAINER_URL_WEB, ) from .utils import ( + format_duration, version_within_range, update_base_tag, generate_release_report_filename, @@ -498,28 +501,51 @@ def update_commit_author_user(author_id: int): @shared_task -def send_commit_author_email_verify_mail(commit_author_email, url): - logger.info(f"Sending verification email to {commit_author_email} with {url=}") - - text_content = ( - "Please verify your email address by clicking the following link: \n" - f"\n\n {url}\n\n If you did not request a commit author verification " - "you can safely ignore this email.\n" - ) - html_content = ( - "

Please verify your email address at the following link:

" - f"

Verify Email

" - "

If you did not request a commit author verification you can safely ignore " - "this email.

" - ) - msg = EmailMultiAlternatives( - subject="Please verify your email address", - body=text_content, - from_email=settings.DEFAULT_FROM_EMAIL, - to=[commit_author_email], - ) - msg.attach_alternative(html_content, "text/html") - msg.send() +def send_commit_author_email_verify_mail( + commit_author_email, url, requester=None, v3=False +): + if v3: + # unlike the legacy log line below, no address (PII) or url (which + # embeds the claim token) may end up in the logs + logger.info("Sending commit email verification message") + message = render_to_string( + "v3/libraries/email/verify_commit_email.txt", + { + "email": commit_author_email, + "requester": requester, + "confirm_url": url, + "expiry_label": format_duration(COMMIT_EMAIL_CLAIM_MAX_AGE), + }, + ) + send_mail( + subject="Confirm your commit email address", + message=message, + from_email=settings.DEFAULT_FROM_EMAIL, + recipient_list=[commit_author_email], + fail_silently=False, + ) + else: + # pre-v3 email, preserved verbatim + logger.info(f"Sending verification email to {commit_author_email} with {url=}") + text_content = ( + "Please verify your email address by clicking the following link: \n" + f"\n\n {url}\n\n If you did not request a commit author verification " + "you can safely ignore this email.\n" + ) + html_content = ( + "

Please verify your email address at the following link:

" + f"

Verify Email

" + "

If you did not request a commit author verification you can safely " + "ignore this email.

" + ) + msg = EmailMultiAlternatives( + subject="Please verify your email address", + body=text_content, + from_email=settings.DEFAULT_FROM_EMAIL, + to=[commit_author_email], + ) + msg.attach_alternative(html_content, "text/html") + msg.send() logger.info(f"Verification email to {commit_author_email} sent") diff --git a/libraries/tests/test_v3_commit_email_views.py b/libraries/tests/test_v3_commit_email_views.py new file mode 100644 index 000000000..296555f77 --- /dev/null +++ b/libraries/tests/test_v3_commit_email_views.py @@ -0,0 +1,1019 @@ +import uuid +from datetime import timedelta + +import pytest +import waffle.testutils +from django.utils import timezone +from django.utils.formats import date_format +from model_bakery import baker + +from ..models import CommitAuthorEmail + +pytestmark = pytest.mark.django_db + + +def _unclaimed_commit_email(email="dev@example.com"): + return baker.make( + "libraries.CommitAuthorEmail", + email=email, + claim_verified=False, + ) + + +def _pending_claim(user, email="dev@example.com"): + """An email mid-claim: verification requested by `user` (claimed_by + + claim_hash set) but not yet confirmed - what ask_to_claim leaves + behind. author.user is untouched until verification.""" + return baker.make( + "libraries.CommitAuthorEmail", + email=email, + claim_verified=False, + claim_hash=uuid.uuid4(), + claim_hash_expiration=timezone.now() + timedelta(days=1), + claimed_by=user, + ) + + +def _verified_claim(user, email="dev@example.com"): + """A completed claim - what verify_claim leaves behind.""" + commit_author_email = _pending_claim(user, email=email) + commit_author_email.claim_verified = True + commit_author_email.save() + commit_author_email.author.user = user + commit_author_email.author.save() + return commit_author_email + + +# --- create (ask to claim) --- + + +def test_v3_commit_email_create_requires_login(tp): + commit_author_email = _unclaimed_commit_email() + response = tp.post( + tp.reverse("v3-commit-author-email-create"), + data={"email": commit_author_email.email}, + ) + tp.response_302(response) + assert "login" in response.url + + +@waffle.testutils.override_flag("v3", active=True) +def test_v3_commit_email_create_claims_and_sends_verification(user, tp, mailoutbox): + commit_author_email = _unclaimed_commit_email() + with tp.login(user): + response = tp.post( + tp.reverse("v3-commit-author-email-create"), + data={"email": commit_author_email.email}, + ) + tp.response_302(response) + + commit_author_email.refresh_from_db() + assert commit_author_email.claimed_by == user + assert commit_author_email.claim_verified is False + assert commit_author_email.claim_hash is not None + # asking is side-effect-free: public attribution is only bound at + # verification + assert commit_author_email.author.user is None + assert len(mailoutbox) == 1 + assert commit_author_email.email in mailoutbox[0].to + body = mailoutbox[0].body + assert commit_author_email.email in body + assert str(commit_author_email.claim_hash) in body + assert "It expires in 1\xa0day." in body + assert "only works while signed in to the account that requested it" in body + assert "you can safely ignore this email" in body + + +@waffle.testutils.override_flag("v3", active=True) +def test_v3_commit_email_verification_email_names_the_requester(user, tp, mailoutbox): + user.display_name = "Requesting User" + user.save() + commit_author_email = _unclaimed_commit_email() + with tp.login(user): + tp.post( + tp.reverse("v3-commit-author-email-create"), + data={"email": commit_author_email.email}, + ) + assert "The user Requesting User has asked to link" in mailoutbox[0].body + + +def test_v3_commit_email_create_htmx_returns_updated_card(user, tp): + commit_author_email = _unclaimed_commit_email() + with tp.login(user): + response = tp.post( + tp.reverse("v3-commit-author-email-create"), + data={"email": commit_author_email.email}, + extra={"HTTP_HX_REQUEST": "true"}, + ) + tp.response_200(response) + content = response.content.decode() + assert commit_author_email.email in content + # the base add row stays rendered below the new pending row so another + # email can be added without JS + # resend, withdraw, base add row, extra-row template + assert content.count("", form_start) + assert form_end != -1 + assert "csrfmiddlewaretoken" in content[form_start:form_end] + + commit_author_email.refresh_from_db() + assert commit_author_email.claim_verified is False + assert commit_author_email.author.user is None + + +@waffle.testutils.override_flag("v3", active=True) +def test_verify_get_outside_claimant_session_shows_generic_failure(user, tp): + """A valid token viewed logged out or from a different account renders + the exact same failure page as an unknown token: no confirm button, no + claim details, so a leaked or forwarded link neither works nor reveals + anything.""" + commit_author_email = _pending_claim(user) + other_user = baker.make("users.User") + other_user.set_password("password") + other_user.save() + + def _body(response): + """The confirm component only - the base-template chrome around it + legitimately differs between logged-in and anonymous sessions.""" + content = response.content.decode() + start = content.find("mailing-list-confirm__header") + end = content.find("", start) + assert start != -1 + assert end != -1 + return content[start:end] + + unknown_response = tp.get( + tp.reverse("commit-author-email-verify", token=str(uuid.uuid4())) + ) + + anonymous_response = tp.get( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + with tp.login(other_user): + other_user_response = tp.get( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + + for response in (anonymous_response, other_user_response): + tp.response_200(response) + content = response.content.decode() + assert commit_author_email.email not in content + assert "You asked to link" not in content + assert _body(response) == _body(unknown_response) + + +@waffle.testutils.override_flag("v3", active=True) +def test_verify_post_outside_claimant_session_does_not_verify(user, tp): + """The confirm action never runs on the claimant's behalf: POSTing a + valid token logged out or as a different user changes nothing and + renders the generic failure page.""" + commit_author_email = _pending_claim(user) + other_user = baker.make("users.User") + other_user.set_password("password") + other_user.save() + + response = tp.post( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + tp.response_200(response) + assert "could not be confirmed" in response.content.decode() + + with tp.login(other_user): + response = tp.post( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + tp.response_200(response) + assert "could not be confirmed" in response.content.decode() + + commit_author_email.refresh_from_db() + assert commit_author_email.claim_verified is False + assert commit_author_email.claimed_by == user + assert commit_author_email.author.user is None + + +@waffle.testutils.override_flag("v3", active=True) +def test_verify_email_falls_back_when_claimant_has_no_display_name( + user, tp, mailoutbox +): + user.display_name = "" + user.save() + commit_author_email = _unclaimed_commit_email() + with tp.login(user): + tp.post( + tp.reverse("v3-commit-author-email-create"), + data={"email": commit_author_email.email}, + ) + body = mailoutbox[0].body + assert "A user has asked to link" in body + assert user.email not in body + + +@waffle.testutils.override_flag("v3", active=True) +def test_verify_post_by_claimant_verifies_and_binds(user, tp): + """The confirm button in the claimant's own session completes the claim + and binds attribution to them, with second-person success copy.""" + user.display_name = "Test Claimant" + user.save() + commit_author_email = _pending_claim(user) + + with tp.login(user): + response = tp.post( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + tp.response_200(response) + content = response.content.decode() + assert "successfully confirmed" in content + assert "associated with your account" in content + assert "Test Claimant's" not in content + + commit_author_email.refresh_from_db() + assert commit_author_email.claim_verified is True + assert commit_author_email.author.user == user + + +@waffle.testutils.override_flag("v3", active=True) +def test_verify_post_conflicting_verified_sibling_does_not_rebind(user, tp): + """A verified claim never steals an author on which a different user + holds another verified claim - and the page must say so instead of + reporting the commit history as associated.""" + other_user = baker.make("users.User") + verified = _verified_claim(other_user, email="theirs@example.com") + pending = baker.make( + "libraries.CommitAuthorEmail", + email="contested@example.com", + author=verified.author, + claim_verified=False, + claim_hash=uuid.uuid4(), + claim_hash_expiration=timezone.now() + timedelta(days=1), + claimed_by=user, + ) + + with tp.login(user): + response = tp.post( + tp.reverse("commit-author-email-verify", token=str(pending.claim_hash)) + ) + tp.response_200(response) + content = response.content.decode() + assert "Commit history not linked" in content + assert "claimed by another user, so it was not linked" in content + assert "associated with your account" not in content + + pending.refresh_from_db() + assert pending.claim_verified is True + pending.author.refresh_from_db() + assert pending.author.user == other_user + + +@waffle.testutils.override_flag("v3", active=True) +def test_verify_get_already_verified_shows_friendly_message_to_claimant(user, tp): + """The 'already verified' state is itself claimant-only; anyone else + gets the generic failure page so the token reveals nothing.""" + commit_author_email = _pending_claim(user) + commit_author_email.verify_claim() + + with tp.login(user): + response = tp.get( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + tp.response_200(response) + assert "already been verified" in response.content.decode() + + response = tp.get( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + tp.response_200(response) + assert "already been verified" not in response.content.decode() + assert "could not be confirmed" in response.content.decode() + + +@waffle.testutils.override_flag("v3", active=True) +def test_verify_expired_token_shows_failure(user, tp): + """An expired token fails even in the claimant's own session.""" + commit_author_email = _pending_claim(user) + commit_author_email.claim_hash_expiration = timezone.now() - timedelta(hours=1) + commit_author_email.save() + + with tp.login(user): + response = tp.get( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + tp.response_200(response) + content = response.content.decode() + assert "could not be confirmed" in content + + # POSTing the expired token must not verify either + response = tp.post( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + tp.response_200(response) + commit_author_email.refresh_from_db() + assert commit_author_email.claim_verified is False + + +@waffle.testutils.override_flag("v3", active=True) +def test_verify_unknown_token_and_expired_token_render_the_same_message(user, tp): + """The failure page is deliberately generic so it does not reveal + whether a token exists or merely expired, even to the claimant.""" + expired = _pending_claim(user) + expired.claim_hash_expiration = timezone.now() - timedelta(hours=1) + expired.save() + + def _body(response): + content = response.content.decode() + return content[content.find("mailing-list-confirm__header") :] + + with tp.login(user): + unknown_response = tp.get( + tp.reverse("commit-author-email-verify", token=str(uuid.uuid4())) + ) + expired_response = tp.get( + tp.reverse("commit-author-email-verify", token=str(expired.claim_hash)) + ) + tp.response_200(unknown_response) + tp.response_200(expired_response) + assert "could not be confirmed" in _body(unknown_response) + assert _body(unknown_response) == _body(expired_response) + + +@waffle.testutils.override_flag("v3", active=True) +def test_verify_malformed_token_shows_generic_failure(user, tp): + """The route accepts any string, so a truncated or mangled link must + render the same generic failure page as an unknown token instead of + crashing on the UUID lookup - including for logged-in users, whose + session triggers the claim queries.""" + _pending_claim(user) + + with tp.login(user): + response = tp.get(tp.reverse("commit-author-email-verify", token="not-a-uuid")) + tp.response_200(response) + assert "could not be confirmed" in response.content.decode() + + response = tp.post(tp.reverse("commit-author-email-verify", token="not-a-uuid")) + tp.response_200(response) + assert "could not be confirmed" in response.content.decode() + + +# --- verify with the v3 flag off (legacy flow, preserved untouched) --- + + +@waffle.testutils.override_flag("v3", active=False) +def test_verify_legacy_get_verifies_immediately(user, tp): + """With the flag off the pre-v3 behavior is intact: opening the link + while logged in completes the verification on the spot, binds the author + to the visitor, and renders the legacy template.""" + commit_author_email = _pending_claim(user) + + with tp.login(user): + response = tp.get( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + tp.response_200(response) + content = response.content.decode() + assert "Commit Author Email Address Confirmation" in content + assert "successfully confirmed" in content + assert commit_author_email.email in content + + commit_author_email.refresh_from_db() + assert commit_author_email.claim_verified is True + assert commit_author_email.author.user == user + + +@waffle.testutils.override_flag("v3", active=False) +def test_verify_legacy_invalid_token_shows_reason(user, tp): + with tp.login(user): + response = tp.get( + tp.reverse("commit-author-email-verify", token=str(uuid.uuid4())) + ) + tp.response_200(response) + content = response.content.decode() + assert "could not be confirmed" in content + assert "No valid commit author found or the token has expired" in content + + +@waffle.testutils.override_flag("v3", active=False) +def test_verify_legacy_email_content(user, tp, mailoutbox): + """With the flag off, asking to claim sends the pre-v3 email: the plain + 'Please verify' copy with an HTML alternative, not the v3 template.""" + commit_author_email = _unclaimed_commit_email() + + with tp.login(user): + tp.post( + tp.reverse("commit-author-email-create"), + data={"email": commit_author_email.email}, + ) + + assert len(mailoutbox) == 1 + message = mailoutbox[0] + assert message.subject == "Please verify your email address" + assert "Please verify your email address by clicking the following link" in ( + message.body + ) + assert "You asked to link" not in message.body + html_alternatives = [c for c, t in message.alternatives if t == "text/html"] + assert html_alternatives and "Verify Email" in html_alternatives[0] + + +@waffle.testutils.override_flag("v3", active=False) +def test_legacy_create_binds_author_at_ask_time(user, tp): + """The legacy ask flow is intact: asking to claim immediately binds + author.user to the asker. claimed_by is recorded too, so the claim + carries over to the v3 card when the flag flips on.""" + commit_author_email = _unclaimed_commit_email() + + with tp.login(user): + response = tp.post( + tp.reverse("commit-author-email-create"), + data={"email": commit_author_email.email}, + ) + tp.response_200(response) + + commit_author_email.refresh_from_db() + assert commit_author_email.author.user == user + assert commit_author_email.claim_hash is not None + assert commit_author_email.claim_verified is False + assert commit_author_email.claimed_by == user + + +@waffle.testutils.override_flag("v3", active=False) +def test_legacy_create_rejects_author_bound_email(user, tp): + """The legacy form rules are intact: any author.user binding blocks the + claim, with the pre-v3 message.""" + other_user = baker.make("users.User") + commit_author_email = _unclaimed_commit_email() + commit_author_email.author.user = other_user + commit_author_email.author.save() + + with tp.login(user): + response = tp.post( + tp.reverse("commit-author-email-create"), + data={"email": commit_author_email.email}, + ) + assert response.status_code == 422 + assert "already associated with a user" in response.content.decode() + + +@waffle.testutils.override_flag("v3", active=False) +def test_legacy_resend_sends_new_verification(user, tp, mailoutbox): + """The legacy resend is intact: keyed by the author.user binding made at + ask time, no claimed_by involved.""" + commit_author_email = _unclaimed_commit_email() + commit_author_email.author.user = user + commit_author_email.author.save() + commit_author_email.claim_hash = uuid.uuid4() + commit_author_email.claim_hash_expiration = timezone.now() + timedelta(days=1) + commit_author_email.save() + old_hash = commit_author_email.claim_hash + + with tp.login(user): + response = tp.post( + tp.reverse("commit-author-email-verify-resend", claim_hash=str(old_hash)) + ) + tp.response_200(response) + + commit_author_email.refresh_from_db() + assert commit_author_email.claim_hash != old_hash + assert len(mailoutbox) == 1 + assert mailoutbox[0].subject == "Please verify your email address" + + +@waffle.testutils.override_flag("v3", active=False) +def test_verify_legacy_post_not_allowed(user, tp): + """The legacy view was GET-only; the v3 confirm POST must not exist + behind the flag.""" + commit_author_email = _pending_claim(user) + + with tp.login(user): + response = tp.post( + tp.reverse( + "commit-author-email-verify", token=str(commit_author_email.claim_hash) + ) + ) + assert response.status_code == 405 diff --git a/libraries/utils.py b/libraries/utils.py index 0b45fa3fb..e4a239cd9 100644 --- a/libraries/utils.py +++ b/libraries/utils.py @@ -18,7 +18,9 @@ from django.db.models import Count, F, QuerySet from django.db.models.functions import Lower from django.urls import reverse +from django.utils import timezone as django_timezone from django.utils.text import slugify +from django.utils.timesince import timesince from libraries.constants import ( DEFAULT_LIBRARIES_LANDING_VIEW, @@ -127,6 +129,16 @@ def generate_random_string(length=4): return random_string +def format_duration(seconds: int) -> str: + """Human label for a duration, e.g. 86400 -> "1 day". + + Mirrors the mailing-list confirm flow's formatting so expiry copy reads + the same across both email verification flows. + """ + now = django_timezone.now() + return timesince(now - relativedelta(seconds=seconds), now, depth=1) + + def version_within_range( version: str, min_version: str = None, max_version: str = None ): diff --git a/libraries/views.py b/libraries/views.py index d1561543c..0a8035aad 100644 --- a/libraries/views.py +++ b/libraries/views.py @@ -1,7 +1,11 @@ import datetime +import uuid + import structlog from django.contrib import messages +from django.contrib.auth.mixins import LoginRequiredMixin +from django.db import transaction from django.db.models import Prefetch from django.http import Http404, HttpResponse from django.shortcuts import get_object_or_404, redirect @@ -12,6 +16,7 @@ from django.views import View from django.views.decorators.csrf import csrf_exempt from django.views.generic import DetailView, ListView, FormView, TemplateView +from waffle import flag_is_active from core.constants import SLACK_URL from core.githubhelper import GithubAPIClient @@ -24,7 +29,7 @@ from versions.models import Version from .constants import README_MISSING -from .forms import CommitAuthorEmailForm +from .forms import CommitAuthorEmailForm, V3CommitAuthorEmailForm from .mixins import VersionAlertMixin, BoostVersionMixin, ContributorMixin from .models import ( Category, @@ -712,14 +717,57 @@ def form_invalid(self, form): class VerifyCommitEmailView(TemplateView): """ - View to verify commit email addresses. - This is used to ensure that commit authors have verified their email addresses. + Landing page for the commit email verification link. + + Behind the v3 flag, the link is only usable inside the claimant's own + authenticated session: proving the claim takes both the token (inbox + access) and being logged in as the account that asked. Any other + visitor - anonymous, or a different account - gets the same generic + failure page as an invalid token, so nothing ever happens on the + claimant's behalf and tokens cannot be probed. GET never changes state + (mail scanners prefetch links); the claim completes on the explicit + POST from the confirm button. + + With the flag off, the legacy behavior is preserved untouched: the GET + itself completes the verification and binds author.user to the visitor, + rendered with the legacy template. """ template_name = "libraries/profile_confirm_email_address.html" + v3_template_name = "v3/libraries/commit_email_confirm.html" - def get_context_data(self, **kwargs): - context = super().get_context_data(**kwargs) + def get_template_names(self): + if flag_is_active(self.request, "v3"): + return [self.v3_template_name] + return super().get_template_names() + + def _parsed_token(self): + # the route accepts any string; a mangled link must land on the + # generic failure page, not 500 on the UUIDField lookup + try: + return uuid.UUID(str(self.kwargs.get("token"))) + except (TypeError, ValueError): + return None + + def _get_viewers_open_claim(self, for_update=False): + if not self.request.user.is_authenticated: + return None + token = self._parsed_token() + if token is None: + return None + queryset = CommitAuthorEmail.objects.filter( + claim_hash=token, + claim_hash_expiration__gt=timezone.now(), + claim_verified=False, + claimed_by=self.request.user, + ).select_related("author") + if for_update: + queryset = queryset.select_for_update(of=("self",)) + return queryset.first() + + def _legacy_verify_context(self, context): + # pre-v3 behavior, restored verbatim: the GET completes the + # verification and binds the author to whoever opened the link token = self.kwargs.get("token") commit_author_email = ( CommitAuthorEmail.objects.filter( @@ -746,6 +794,51 @@ def get_context_data(self, **kwargs): return context + def get_context_data(self, **kwargs): + context = super().get_context_data(**kwargs) + if not flag_is_active(self.request, "v3"): + return self._legacy_verify_context(context) + context["home_url"] = "/" + commit_author_email = self._get_viewers_open_claim() + if commit_author_email: + context["commit_author_email"] = commit_author_email + elif ( + self.request.user.is_authenticated + and self._parsed_token() is not None + and CommitAuthorEmail.objects.filter( + claim_hash=self._parsed_token(), + claim_verified=True, + claimed_by=self.request.user, + ).exists() + ): + context["already_verified"] = True + return context + + def post(self, request, *args, **kwargs): + if not flag_is_active(request, "v3"): + # the legacy view was GET-only + return self.http_method_not_allowed(request, *args, **kwargs) + + # locked read: the token, claimant, expiry, and verification state + # are re-evaluated under the row lock, so a concurrent withdraw or + # resend can't slip between the check and verify_claim + with transaction.atomic(): + commit_author_email = self._get_viewers_open_claim(for_update=True) + if commit_author_email: + attribution_bound = commit_author_email.verify_claim() + + if not commit_author_email: + return self.get(request, *args, **kwargs) + return self.render_to_response( + { + "view": self, + "commit_email": commit_author_email.email, + "confirmed": attribution_bound, + "conflict": not attribution_bound, + "home_url": "/", + } + ) + class CommitEmailResendView(TemplateView): def post(self, request, *args, **kwargs): @@ -761,3 +854,106 @@ def post(self, request, *args, **kwargs): commit_author_email.trigger_verification_email(request) return HttpResponse('') + + +def _is_htmx(request) -> bool: + return request.headers.get("HX-Request") == "true" + + +class V3CommitAuthorEmailCardMixin: + """Shared rendering for the v3 profile page's commit-email card body, so + the create/delete/resend views below all hand back the same up-to-date + markup after making their change. + """ + + def _card_body(self, request, form=None): + return TemplateResponse( + request, + "v3/includes/_commit_email_card_body.html", + { + "commit_email_addresses": CommitAuthorEmail.claimed_by_user( + request.user + ), + "commit_email_form": form or V3CommitAuthorEmailForm(), + }, + ) + + def _redirect_to_profile(self, request): + return redirect(f"{reverse('profile-account')}?edit=true") + + +class V3CommitAuthorEmailCreateView( + LoginRequiredMixin, V3CommitAuthorEmailCardMixin, View +): + def post(self, request, *args, **kwargs): + form = V3CommitAuthorEmailForm(request.POST, user=request.user) + if not form.is_valid(): + if _is_htmx(request): + return self._card_body(request, form=form) + for error in form.errors.get("email", []): + messages.error(request, error) + return self._redirect_to_profile(request) + + if form.commit_author_email.ask_to_claim(request) is None: + # the row was verified or claimed by someone else between the + # form's (unlocked) validation and the locked re-check + form.add_error("email", "This email address can no longer be claimed.") + if _is_htmx(request): + return self._card_body(request, form=form) + for error in form.errors.get("email", []): + messages.error(request, error) + return self._redirect_to_profile(request) + + if _is_htmx(request): + return self._card_body(request) + return self._redirect_to_profile(request) + + +class V3CommitAuthorEmailWithdrawView( + LoginRequiredMixin, V3CommitAuthorEmailCardMixin, View +): + """Withdraw a pending (unverified) claim. The CommitAuthorEmail row + belongs to the commit importer and is never deleted; only the claim + fields on the row are cleared (see withdraw_claim). Verified claims + are historical record-keeping for contribution stats, so they are + refused even if the request is forged past a hidden button, as are + emails with no open ask. + """ + + def post(self, request, *args, **kwargs): + # locked read so the guards can't race a concurrent verify: whoever + # locks first wins and the loser re-evaluates (verified -> 404 here) + with transaction.atomic(): + commit_author_email = get_object_or_404( + CommitAuthorEmail.objects.select_for_update(), + pk=kwargs["pk"], + claimed_by=request.user, + claim_verified=False, + claim_hash__isnull=False, + ) + commit_author_email.withdraw_claim() + + if _is_htmx(request): + return self._card_body(request) + return self._redirect_to_profile(request) + + +class V3CommitAuthorEmailResendView( + LoginRequiredMixin, V3CommitAuthorEmailCardMixin, View +): + def post(self, request, *args, **kwargs): + # no outer transaction: ask_to_claim re-checks its preconditions + # under its own row lock, and the verification email must be + # enqueued after that lock commits, not while it is held + commit_author_email = get_object_or_404( + CommitAuthorEmail, + pk=kwargs["pk"], + claimed_by=request.user, + claim_verified=False, + claim_hash__isnull=False, + ) + commit_author_email.ask_to_claim(request) + + if _is_htmx(request): + return self._card_body(request) + return self._redirect_to_profile(request) diff --git a/static/css/v3/commit-email-card.css b/static/css/v3/commit-email-card.css new file mode 100644 index 000000000..3206ee843 --- /dev/null +++ b/static/css/v3/commit-email-card.css @@ -0,0 +1,192 @@ +/* + Commit Email Card + Lists the user's associated commit-author email addresses as read-only + field rows. A pending email shows an inline "Pending" status plus resend + (reload) and remove (close) icon buttons outside the field; a verified + email is a plain read-only row. New emails are added via editable field + rows with a Submit button that swaps to a spinner while the HTMX request + is in flight. Extends the base .card component; the Submit button reuses + .btn.btn-primary and the add-row input reuses .field from forms.css. + + commit-email__items — vertical list of saved email rows + commit-email__item — one saved email: field + outside action icons + commit-email__field — the field-styled box around the email value + commit-email__address — the email address text inside the field + commit-email__status — "Pending" inline text inside the field + commit-email__action-btn — icon-only resend/remove button outside the field + commit-email__add-row — an add-new-email form: field + Submit (+ close) + commit-email__spinner — replaces Submit while the request is in flight + commit-email__add-another — wrapper for the "+ Add Another" button +*/ + +.commit-email__body { + display: flex; + flex-direction: column; + gap: var(--space-default); + width: 100%; +} + +.commit-email__items { + display: flex; + flex-direction: column; + gap: var(--space-default); + width: 100%; + list-style: none; + margin: 0; + padding: 0; +} + +.commit-email__item { + display: flex; + align-items: center; + gap: var(--space-default); + width: 100%; +} + +.commit-email__field { + display: flex; + flex: 1 1 auto; + align-items: center; + gap: var(--space-default); + min-width: 0; + height: 40px; + padding: 0 var(--space-default) 0 var(--space-large); + background-color: var(--color-surface-weak); + border: 1px solid var(--color-stroke-weak); + border-radius: var(--border-radius-xl); +} + +.commit-email__address { + flex: 1 1 auto; + min-width: 0; + overflow: hidden; + text-overflow: ellipsis; + white-space: nowrap; + font-family: var(--font-sans); + font-size: var(--font-size-small); + font-weight: var(--font-weight-regular); + line-height: var(--line-height-relaxed); + color: var(--color-text-tertiary); +} + +.commit-email__status { + flex-shrink: 0; + font-family: var(--font-sans); + font-size: var(--font-size-small); + font-weight: var(--font-weight-regular); + color: var(--color-text-tertiary); +} + +.commit-email__action-btn { + display: flex; + align-items: center; + justify-content: center; + flex-shrink: 0; + width: 24px; + height: 24px; + padding: 0; + border: none; + background: transparent; + color: var(--color-icon-primary); + cursor: pointer; +} + +.commit-email__action-btn:hover { + color: var(--color-text-link-accent); +} + +.commit-email__action-form { + display: flex; + margin: 0; +} + +.commit-email__add-row { + display: flex; + align-items: flex-start; + gap: var(--space-default); + width: 100%; +} + +.commit-email__add-field { + flex: 1 1 auto; + /* .field is a column with gap; the include's always-rendered (usually + empty) error container would otherwise add a phantom gap below the + input, doubling the spacing between rows */ + gap: 0; +} + +.commit-email__add-field .field__error { + margin-top: var(--space-default); +} + +/* centers the 24px close button against the 40px field control (the row + is top-aligned so an inline error doesn't push the buttons down) */ +.commit-email__add-row .commit-email__action-btn { + margin-top: var(--space-default); +} + +.commit-email__add-row .btn { + height: 40px; + flex-shrink: 0; +} + +/* Swap Submit for the spinner while this row's HTMX request is in flight. + htmx toggles the htmx-request class on the form carrying hx-post. */ +.commit-email__spinner { + display: none; + /* keeps the 3px border inside the 24px box, so the footprint math + below holds without relying on a global border-box reset */ + box-sizing: border-box; + flex-shrink: 0; + width: 24px; + height: 24px; + /* centers the 24px spinner in the 128x40 footprint the hidden Submit + button leaves behind (Figma frame 2147238281 is FIXED 128x40) */ + margin: var(--space-default) 52px; + border: 3px solid var(--color-surface-strong); + border-top-color: var(--color-icon-brand-accent); + border-radius: 50%; + animation: commit-email-spin 0.8s linear infinite; +} + +.commit-email__add-row.htmx-request .btn { + display: none; +} + +.commit-email__add-row.htmx-request .commit-email__spinner { + display: block; +} + +@keyframes commit-email-spin { + to { + transform: rotate(360deg); + } +} + +@media (prefers-reduced-motion: reduce) { + .commit-email__spinner { + animation: none; + } +} + +.commit-email__add-another { + padding-top: var(--space-default); +} + +.commit-email__add-another .btn-underline { + padding: 0; + border: none; + background: none; + font-family: var(--font-sans); + font-size: var(--font-size-xs); + font-weight: var(--font-weight-medium); + line-height: var(--line-height-default); + color: var(--color-text-primary); + cursor: pointer; +} + +@media (max-width: 767px) { + .commit-email__add-row { + flex-wrap: wrap; + } +} diff --git a/static/css/v3/components.css b/static/css/v3/components.css index 290733966..d97153ad7 100644 --- a/static/css/v3/components.css +++ b/static/css/v3/components.css @@ -32,6 +32,7 @@ @import "./banner.css"; @import "./animations.css"; @import "./account-connections.css"; +@import "./commit-email-card.css"; @import "./dialog.css"; @import "./content-modal.css"; @import "./achivement-card.css"; diff --git a/templates/includes/icon.html b/templates/includes/icon.html index 7e6fe0b46..c1fde08bb 100644 --- a/templates/includes/icon.html +++ b/templates/includes/icon.html @@ -89,6 +89,9 @@ {% elif icon_name == "close" %} + {% elif icon_name == "reload" %} + {% comment %} Pixel-art repeat/reload arrows (Figma icon Type=reload, 16-grid — pass icon_viewbox="0 0 16 16"). {% endcomment %} + {% elif icon_name == "arrow-right" %} {% elif icon_name == "link" %} diff --git a/templates/v3/includes/_commit_email_card.html b/templates/v3/includes/_commit_email_card.html new file mode 100644 index 000000000..aaffc55c5 --- /dev/null +++ b/templates/v3/includes/_commit_email_card.html @@ -0,0 +1,26 @@ +{% comment %} + V3 Commit Email Card + Lets the user associate multiple commit-author email addresses with their + account so contribution history across those addresses aggregates onto + one profile. A pending (unverified) email can be removed or re-sent; a + verified email is permanent, since removing it would affect historical + contribution records. + + Variables: + commit_email_addresses (queryset of CommitAuthorEmail) — the user's + current commit emails, verified and pending + csrf_token (required when included with `only` — the body's + forms cannot post without it) + + Usage: + {% include 'v3/includes/_commit_email_card.html' with commit_email_addresses=commit_email_addresses csrf_token=csrf_token only %} +{% endcomment %} + diff --git a/templates/v3/includes/_commit_email_card_body.html b/templates/v3/includes/_commit_email_card_body.html new file mode 100644 index 000000000..4666dc483 --- /dev/null +++ b/templates/v3/includes/_commit_email_card_body.html @@ -0,0 +1,84 @@ +{% comment %} + V3 commit email card body — the list of the user's commit emails plus the + add-new-email rows. Returned wholesale by the create/withdraw/resend views + (see libraries/views.py) so every action re-renders from current DB state. + + Row model (per Figma variants 6873:25111 / 6741:68235 / 6741:68469 / + 7059:18111 / 6741:68703): + - verified email → read-only field, no controls + - pending email → read-only field with inline "Pending", resend (reload) + and withdraw (close) icon buttons outside the field + - base add row → always server-rendered below the existing rows (per + the Figma variants, and so adding works without JS); + Submit is Alpine-disabled while the input is empty + - extra add rows → appended client-side via "+ Add Another" (Alpine), + each removable with a close button before submitting + + Variables: + commit_email_addresses (queryset of CommitAuthorEmail, required) + commit_email_form (CommitAuthorEmailForm, optional — bound with + errors when re-rendering after a failed add) + csrf_token (required when included with `only`; provided by the + request context when rendered from a view) + + Usage: + {% include 'v3/includes/_commit_email_card_body.html' with commit_email_addresses=commit_email_addresses only %} +{% endcomment %} +
+ {# field-email is the id _field_text.html generates for name='email' in the always-rendered base add row #} + + {% if commit_email_addresses %} +
    + {% for cea in commit_email_addresses %} +
  • +
    + {{ cea.email }} + {% if not cea.claim_verified %} + Pending + {% endif %} +
    + {% if not cea.claim_verified %} +
    + {% csrf_token %} + +
    +
    + {% csrf_token %} + +
    + {% endif %} +
    • + {% endfor %} +
+ {% endif %} +
+ {% csrf_token %} + {% include 'v3/includes/_field_text.html' with name='email' type='email' placeholder='Your email' value=commit_email_form.email.value error=commit_email_form.email.errors.0 extra_class='commit-email__add-field' only %} + + +
+ +
+ +
+
Add other email addresses you've used in commits to Boost libraries.
+
diff --git a/templates/v3/libraries/commit_email_confirm.html b/templates/v3/libraries/commit_email_confirm.html new file mode 100644 index 000000000..d80022a12 --- /dev/null +++ b/templates/v3/libraries/commit_email_confirm.html @@ -0,0 +1,134 @@ +{% extends "base.html" %} +{% load static %} + +{% comment %} + Landing page for the commit email verification link (VerifyCommitEmailView). + Reuses the mailing-list confirm-page component for visual consistency with + the mailing list confirmation pages. + + The view only surfaces the non-failure states to the logged-in claimant + themselves, so the copy is always second person ("You asked..."). + + States (mutually exclusive): + confirmed - the POST just completed the claim + conflict - the POST verified the email but another account + already holds a verified claim on the author, so + the commit history was not linked + already_verified - the token belongs to the viewer's verified claim + commit_author_email - the viewer's open claim: explicit Confirm button + (fallback) - anything else: invalid, expired, logged out, or a + different account (single generic message) +{% endcomment %} + +{% block extra_head %} + {{ block.super }} + +{% endblock %} + +{% block content %} +
+
+ + {% if confirmed %} + + +
+

Email confirmed

+

+ + has been successfully confirmed and its commit history is now + associated with your account. +

+
+ + {% include "v3/includes/_button.html" with url=home_url label="Go to homepage" only %} + + {% elif conflict %} + + +
+

Commit history not linked

+

+ + has been confirmed, but its commit history has already been + claimed by another user, so it was not linked to your account. + Report an issue if this is incorrect. +

+
+ + {% include "v3/includes/_button.html" with url=home_url label="Go to homepage" only %} + + {% elif already_verified %} + + +
+

Already verified

+

+ This email address has already been verified. No further action is needed. +

+
+ + {% include "v3/includes/_button.html" with url=home_url label="Go to homepage" only %} + + {% elif commit_author_email %} + + +
+

Confirm your commit email

+

+ You asked to link + + and its commit history to your boost.org account. +

+

+ This link expires on {{ commit_author_email.claim_hash_expiration|date:"F j, Y, g:i A T" }}. +

+
+ +
+ {% csrf_token %} + {% include "v3/includes/_button.html" with label="Confirm and link this email" type="submit" only %} +
+ + {% else %} + + + {# deliberately generic: identical for unknown, expired, logged-out, and wrong-account visits, so it reveals nothing about the token #} +
+

Invalid or expired link

+

+ Your email address could not be confirmed. No valid commit author found or the token has expired. Please make sure you are signed in to the account that requested the link, or request a new verification email. +

+
+ + {% include "v3/includes/_button.html" with url=home_url label="Go to homepage" only %} + {% endif %} + +
+
+{% endblock %} diff --git a/templates/v3/libraries/email/verify_commit_email.txt b/templates/v3/libraries/email/verify_commit_email.txt new file mode 100644 index 000000000..9361a2bbe --- /dev/null +++ b/templates/v3/libraries/email/verify_commit_email.txt @@ -0,0 +1,11 @@ +Hi, + +{% if requester %}The user {{ requester }} has{% else %}A user has{% endif %} asked to link {{ email }} and its commit history to their boost.org account. + +To confirm, sign in to that boost.org account, then open the link below and use the confirmation button there: + + {{ confirm_url }} + +The link only works while signed in to the account that requested it. It expires in {{ expiry_label }}. + +If you did not request this, you can safely ignore this email - nothing will be linked. diff --git a/templates/v3/user_profile_edit.html b/templates/v3/user_profile_edit.html index 144adaec7..e7e84d23e 100644 --- a/templates/v3/user_profile_edit.html +++ b/templates/v3/user_profile_edit.html @@ -215,26 +215,7 @@ - + {% include 'v3/includes/_commit_email_card.html' with commit_email_addresses=commit_email_addresses csrf_token=csrf_token only %}