From 48c110b32b2b96da570570b3beb7427759edf211 Mon Sep 17 00:00:00 2001 From: Raphael Michel Date: Wed, 21 Dec 2022 13:09:14 +0100 Subject: [PATCH] Allow the automatic creation of customer accounts for guest users --- src/pretix/base/services/orders.py | 44 +++++++++--- src/pretix/base/settings.py | 36 ++++++++-- src/pretix/control/logdisplay.py | 3 + .../pretixcontrol/organizers/customers.html | 4 +- src/pretix/control/views/organizer.py | 4 +- src/pretix/presale/checkoutflow.py | 2 +- src/pretix/presale/forms/customer.py | 50 +++++++++----- src/pretix/presale/views/customer.py | 4 +- src/tests/presale/test_checkout.py | 68 +++++++++++++++++++ src/tests/presale/test_customer.py | 28 +++++++- 10 files changed, 200 insertions(+), 43 deletions(-) diff --git a/src/pretix/base/services/orders.py b/src/pretix/base/services/orders.py index 63b0d7a2f9..3a38339973 100644 --- a/src/pretix/base/services/orders.py +++ b/src/pretix/base/services/orders.py @@ -64,9 +64,9 @@ from pretix.base.i18n import ( LazyLocaleException, get_language_without_region, language, ) from pretix.base.models import ( - CartPosition, Device, Event, GiftCard, Item, ItemVariation, Membership, - Order, OrderPayment, OrderPosition, Quota, Seat, SeatCategoryMapping, User, - Voucher, + CartPosition, Customer, Device, Event, GiftCard, Item, ItemVariation, + Membership, Order, OrderPayment, OrderPosition, Quota, Seat, + SeatCategoryMapping, User, Voucher, ) from pretix.base.models.event import SubEvent from pretix.base.models.orders import ( @@ -849,7 +849,7 @@ def _get_fees(positions: List[CartPosition], payment_requests: List[dict], addre def _create_order(event: Event, email: str, positions: List[CartPosition], now_dt: datetime, payment_requests: List[dict], locale: str=None, address: InvoiceAddress=None, meta_info: dict=None, sales_channel: str='web', shown_total=None, - customer=None): + customer=None, customer_attached=False): payments = [] sales_channel = get_all_sales_channels()[sales_channel] @@ -931,6 +931,8 @@ def _create_order(event: Event, email: str, positions: List[CartPosition], now_d if meta_info: for msg in meta_info.get('confirm_messages', []): order.log_action('pretix.event.order.consent', data={'msg': msg}) + if customer and customer_attached: + customer.log_action('pretix.customer.order.attached', data={'order': order.code}) order_placed.send(event, order=order) return order, payments @@ -981,9 +983,6 @@ def _perform_order(event: Event, payment_requests: List[dict], position_ids: Lis if not p['pprov']: raise OrderError(error_messages['internal']) - if customer: - customer = event.organizer.customers.get(pk=customer) - if email == settings.PRETIX_EMAIL_NONE_VALUE: email = None @@ -995,6 +994,30 @@ def _perform_order(event: Event, payment_requests: List[dict], position_ids: Lis except InvoiceAddress.DoesNotExist: pass + customer_attached = False + if customer: + customer = event.organizer.customers.get(pk=customer) + elif event.settings.customer_accounts_link_by_email in ('attach', 'create') and email: + try: + customer = event.organizer.customers.get(email__iexact=email) + customer_attached = True + except Customer.MultipleObjectsReturned: + logger.warning(f'Multiple customer accounts found for {email}, not attaching.') + except Customer.DoesNotExist: + if event.settings.customer_accounts_link_by_email == 'create': + customer = event.organizer.customers.create( + email=email, + name_parts=addr.name_parts if addr else None, + phone=(meta_info or {}).get('contact_form_data', {}).get('phone'), + is_active=True, + is_verified=False, + locale=locale, + ) + customer.set_unusable_password() + customer.save() + customer.log_action('pretix.customer.created.auto', {}) + customer_attached = True + requires_seat = Exists( SeatCategoryMapping.objects.filter( Q(product=OuterRef('item')) @@ -1018,7 +1041,7 @@ def _perform_order(event: Event, payment_requests: List[dict], position_ids: Lis locale=locale, invoice_address=addr, meta_info=meta_info, - customer=customer, + customer=customer if not customer_attached else None, ) lockfn = NoLockManager @@ -1041,10 +1064,11 @@ def _perform_order(event: Event, payment_requests: List[dict], position_ids: Lis raise OrderError(error_messages['empty']) if len(position_ids) != len(positions): raise OrderError(error_messages['internal']) - _check_positions(event, now_dt, positions, address=addr, sales_channel=sales_channel, customer=customer) + _check_positions(event, now_dt, positions, address=addr, sales_channel=sales_channel, + customer=customer if not customer_attached else None) order, payment_objs = _create_order(event, email, positions, now_dt, payment_requests, locale=locale, address=addr, meta_info=meta_info, sales_channel=sales_channel, - shown_total=shown_total, customer=customer) + shown_total=shown_total, customer=customer, customer_attached=customer_attached) try: for p in payment_objs: if p.provider == 'free': diff --git a/src/pretix/base/settings.py b/src/pretix/base/settings.py index 7518ddf685..35022034c9 100644 --- a/src/pretix/base/settings.py +++ b/src/pretix/base/settings.py @@ -159,13 +159,37 @@ DEFAULTS = { }, 'customer_accounts_link_by_email': { 'default': 'False', - 'type': bool, - 'form_class': forms.BooleanField, - 'serializer_class': serializers.BooleanField, + 'type': str, + 'form_class': forms.ChoiceField, + 'serializer_class': serializers.ChoiceField, + 'serializer_kwargs': dict( + choices=( + ('False', _('Keep guest orders separate and do not link them to customer accounts.')), + ('True', _('Do not attach guest orders to customer accounts, but show them in customer ' + 'accounts with a verified matching email address.')), + ('attach', _('Attach guest orders to existing customer accounts with a matching email address if such ' + 'an account exists.')), + ('create', _('Attach guest orders to existing customer accounts with a matching email and ' + 'automatically create a customer account for all others.')), + ('forbidden', _('Do not allow guest orders.')), + ), + ), 'form_kwargs': dict( - label=_("Match orders based on email address"), - help_text=_("This will allow registered customers to access orders made with the same email address, even if the customer " - "was not logged in during the purchase.") + label=_("Guest order handling"), + widget=forms.RadioSelect, + choices=( + ('False', _('Keep guest orders separate and do not link them to customer accounts.')), + ('True', _('Do not attach guest orders to customer accounts, but show them in customer ' + 'accounts with a verified matching email address.')), + ('attach', _('Attach guest orders to existing customer accounts with a matching email address if ' + 'such an account exists.')), + ('create', _('Attach guest orders to existing customer accounts with a matching email address and ' + 'automatically create a customer account for all others.')), + ('forbidden', _('Do not allow guest orders.')), + ), + help_text=_('Please be aware that creating customer accounts without a users consent might not be a ' + 'good idea for privacy reasons in some jurisdictions or situations. We always recommend to ' + 'let the user choose if they want an account.'), ) }, 'max_items_per_order': { diff --git a/src/pretix/control/logdisplay.py b/src/pretix/control/logdisplay.py index 89b26e782a..c100d49b49 100644 --- a/src/pretix/control/logdisplay.py +++ b/src/pretix/control/logdisplay.py @@ -331,7 +331,10 @@ def pretixcontrol_logentry_display(sender: Event, logentry: LogEntry, **kwargs): 'pretix.membershiptype.changed': _('The membership type has been changed.'), 'pretix.membershiptype.deleted': _('The membership type has been deleted.'), 'pretix.customer.created': _('The account has been created.'), + 'pretix.customer.created.auto': _('The account has been automatically created during an order.'), + 'pretix.customer.claimed': _('The automatically created account has been claimed by the customer.'), 'pretix.customer.changed': _('The account has been changed.'), + 'pretix.customer.order.attached': _('An order was automatically attached to the customer.'), 'pretix.customer.membership.created': _('A membership for this account has been added.'), 'pretix.customer.membership.changed': _('A membership of this account has been changed.'), 'pretix.customer.membership.deleted': _('A membership of this account has been deleted.'), diff --git a/src/pretix/control/templates/pretixcontrol/organizers/customers.html b/src/pretix/control/templates/pretixcontrol/organizers/customers.html index 3bba675fcb..a3d9c283fe 100644 --- a/src/pretix/control/templates/pretixcontrol/organizers/customers.html +++ b/src/pretix/control/templates/pretixcontrol/organizers/customers.html @@ -79,9 +79,9 @@ - {% if not c.is_verified %}{% endif %} + {% if not c.is_verified %}{% endif %} {{ c.email|default_if_none:"" }} - {% if not c.is_verified %}{% endif %} + {% if not c.is_verified %}{% endif %} {{ c.name }} {% if c.external_identifier %}{{ c.external_identifier }}{% endif %} diff --git a/src/pretix/control/views/organizer.py b/src/pretix/control/views/organizer.py index d13c0a8413..f0839f5846 100644 --- a/src/pretix/control/views/organizer.py +++ b/src/pretix/control/views/organizer.py @@ -2215,9 +2215,9 @@ class CustomerDetailView(OrganizerDetailViewMixin, OrganizerPermissionRequiredMi def get_queryset(self): q = Q(customer=self.customer) - if self.request.organizer.settings.customer_accounts_link_by_email and self.customer.email: + if self.request.organizer.settings.customer_accounts_link_by_email == 'True' and self.customer.email: # This is safe because we only let customers with verified emails log in - q |= Q(email__iexact=self.customer.email) + q |= Q(customer__isnull=True, email__iexact=self.customer.email) qs = Order.objects.filter( q ).select_related('event').order_by('-datetime') diff --git a/src/pretix/presale/checkoutflow.py b/src/pretix/presale/checkoutflow.py index f40c8fced6..cbd3641754 100644 --- a/src/pretix/presale/checkoutflow.py +++ b/src/pretix/presale/checkoutflow.py @@ -269,7 +269,7 @@ class CustomerStep(CartMixin, TemplateFlowStep): @cached_property def guest_allowed(self): - return not any( + return self.request.event.settings.customer_accounts_link_by_email != 'forbidden' and not any( p.item.require_membership or (p.variation and p.variation.require_membership) or p.item.grant_membership_type_id diff --git a/src/pretix/presale/forms/customer.py b/src/pretix/presale/forms/customer.py index f3db3e8d8d..8c09d813f9 100644 --- a/src/pretix/presale/forms/customer.py +++ b/src/pretix/presale/forms/customer.py @@ -144,6 +144,7 @@ class RegistrationForm(forms.Form): self.standalone = kwargs.pop('standalone') self.signer = signing.TimestampSigner(salt=f'customer-registration-captcha-{get_client_ip(request)}') super().__init__(*args, **kwargs) + self.instance = None event = getattr(request, "event", None) if event and event.settings.order_phone_asked: @@ -214,14 +215,17 @@ class RegistrationForm(forms.Form): if email is not None: try: - self.request.organizer.customers.get(email=email.lower()) + existing = self.request.organizer.customers.get(email=email.lower()) except Customer.DoesNotExist: pass else: - raise forms.ValidationError( - {'email': self.error_messages['duplicate']}, - code='duplicate', - ) + if not existing.is_active or existing.is_verified or existing.has_usable_password(): + raise forms.ValidationError( + {'email': self.error_messages['duplicate']}, + code='duplicate', + ) + else: + self.instance = existing if self.standalone: expect = -1 @@ -256,19 +260,29 @@ class RegistrationForm(forms.Form): return self.cleaned_data def create(self): - customer = self.request.organizer.customers.create( - email=self.cleaned_data['email'], - name_parts=self.cleaned_data['name_parts'], - phone=self.cleaned_data.get('phone'), - is_active=True, - is_verified=False, - locale=get_language_without_region(), - ) - customer.set_unusable_password() - customer.save() - customer.log_action('pretix.customer.created', {}) - customer.send_activation_mail() - return customer + if self.instance: + self.instance.name_parts = self.cleaned_data['name_parts'] + if self.cleaned_data.get('phone'): + self.instance.phone = self.cleaned_data.get('phone') + self.instance.locale = get_language_without_region() + self.instance.save() + self.instance.log_action('pretix.customer.claimed', {}) + self.instance.send_activation_mail() + return self.instance + else: + customer = self.request.organizer.customers.create( + email=self.cleaned_data['email'], + name_parts=self.cleaned_data['name_parts'], + phone=self.cleaned_data.get('phone'), + is_active=True, + is_verified=False, + locale=get_language_without_region(), + ) + customer.set_unusable_password() + customer.save() + customer.log_action('pretix.customer.created', {}) + customer.send_activation_mail() + return customer class SetPasswordForm(forms.Form): diff --git a/src/pretix/presale/views/customer.py b/src/pretix/presale/views/customer.py index 878e0de74e..eb041127fe 100644 --- a/src/pretix/presale/views/customer.py +++ b/src/pretix/presale/views/customer.py @@ -352,9 +352,9 @@ class ProfileView(CustomerRequiredMixin, ListView): def get_queryset(self): q = Q(customer=self.request.customer) - if self.request.organizer.settings.customer_accounts_link_by_email and self.request.customer.email: + if self.request.organizer.settings.customer_accounts_link_by_email == 'True' and self.request.customer.email: # This is safe because we only let customers with verified emails log in - q |= Q(email__iexact=self.request.customer.email) + q |= Q(customer__isnull=True, email__iexact=self.request.customer.email) qs = Order.objects.filter( q ).prefetch_related( diff --git a/src/tests/presale/test_checkout.py b/src/tests/presale/test_checkout.py index 02fa5ea97b..5501c9d113 100644 --- a/src/tests/presale/test_checkout.py +++ b/src/tests/presale/test_checkout.py @@ -4315,6 +4315,56 @@ class CustomerCheckoutTestCase(BaseCheckoutTestCase, TestCase): assert order.email == 'admin@localhost' assert not order.customer + def test_guest_not_allowed(self): + self.orga.settings.customer_accounts_link_by_email = 'forbidden' + response = self.client.get('/%s/%s/checkout/start' % (self.orga.slug, self.event.slug), follow=True) + self.assertRedirects(response, '/%s/%s/checkout/customer/' % (self.orga.slug, self.event.slug), + target_status_code=200) + + response = self.client.post('/%s/%s/checkout/customer/' % (self.orga.slug, self.event.slug), { + 'customer_mode': 'guest' + }, follow=True) + self.assertEqual(response.status_code, 200) + + def test_guest_attach(self): + self.orga.settings.customer_accounts_link_by_email = 'attach' + response = self.client.get('/%s/%s/checkout/start' % (self.orga.slug, self.event.slug), follow=True) + self.assertRedirects(response, '/%s/%s/checkout/customer/' % (self.orga.slug, self.event.slug), + target_status_code=200) + + self.client.post('/%s/%s/checkout/customer/' % (self.orga.slug, self.event.slug), { + 'customer_mode': 'guest' + }, follow=True) + self.client.post('/%s/%s/checkout/questions/' % (self.orga.slug, self.event.slug), { + 'email': 'john@example.org' + }, follow=True) + + order = self._finish() + assert order.email == 'john@example.org' + assert order.customer == self.customer + + def test_guest_create(self): + self.orga.settings.customer_accounts_link_by_email = 'create' + response = self.client.get('/%s/%s/checkout/start' % (self.orga.slug, self.event.slug), follow=True) + self.assertRedirects(response, '/%s/%s/checkout/customer/' % (self.orga.slug, self.event.slug), + target_status_code=200) + + self.client.post('/%s/%s/checkout/customer/' % (self.orga.slug, self.event.slug), { + 'customer_mode': 'guest' + }, follow=True) + self.client.post('/%s/%s/checkout/questions/' % (self.orga.slug, self.event.slug), { + 'email': 'jack@example.org' + }, follow=True) + + order = self._finish() + assert order.email == 'jack@example.org' + assert order.customer + c = order.customer + assert c.email == order.email + assert not c.has_usable_password() + assert c.is_active + assert not c.is_verified + def test_guest_even_if_logged_in(self): self.client.post('/%s/account/login' % self.orga.slug, { 'email': 'john@example.org', @@ -4410,6 +4460,24 @@ class CustomerCheckoutTestCase(BaseCheckoutTestCase, TestCase): assert response.status_code == 200 assert b'alert-danger' in response.content + def test_register_valid_account_previously_autocrated(self): + with scopes_disabled(): + c = self.orga.customers.create(email='foo@example.com', is_active=True, is_verified=False) + c.set_unusable_password() + c.save() + response = self.client.get('/%s/%s/checkout/start' % (self.orga.slug, self.event.slug), follow=True) + self.assertRedirects(response, '/%s/%s/checkout/customer/' % (self.orga.slug, self.event.slug), + target_status_code=200) + + response = self.client.post('/%s/%s/checkout/customer/' % (self.orga.slug, self.event.slug), { + 'customer_mode': 'register', + 'register-email': 'foo@example.com', + 'register-name_parts_0': 'John Doe', + }, follow=False) + self.assertRedirects(response, '/%s/%s/checkout/questions/' % (self.orga.slug, self.event.slug), + target_status_code=200) + assert len(djmail.outbox) == 1 + def test_register_valid(self): response = self.client.get('/%s/%s/checkout/start' % (self.orga.slug, self.event.slug), follow=True) self.assertRedirects(response, '/%s/%s/checkout/customer/' % (self.orga.slug, self.event.slug), diff --git a/src/tests/presale/test_customer.py b/src/tests/presale/test_customer.py index c267d18c99..16a96d44b9 100644 --- a/src/tests/presale/test_customer.py +++ b/src/tests/presale/test_customer.py @@ -124,16 +124,40 @@ def test_org_register(env, client): @pytest.mark.django_db def test_org_register_duplicate_email(env, client): + signer = signing.TimestampSigner(salt='customer-registration-captcha-127.0.0.1') with scopes_disabled(): env[0].customers.create(email='john@example.org') r = client.post('/bigevents/account/register', { 'email': 'john@example.org', 'name_parts_0': 'John Doe', - }) + 'challenge': signer.sign('1+2'), + 'response': '3', + }, REMOTE_ADDR='127.0.0.1') assert b'already registered' in r.content assert r.status_code == 200 +@pytest.mark.django_db +def test_org_register_duplicate_email_ignored_if_previously_autocreated(env, client): + signer = signing.TimestampSigner(salt='customer-registration-captcha-127.0.0.1') + with scopes_disabled(): + c = env[0].customers.create(email='john@example.org', is_active=True, is_verified=False) + c.set_unusable_password() + c.save() + r = client.post('/bigevents/account/register', { + 'email': 'john@example.org', + 'name_parts_0': 'John Doe', + 'challenge': signer.sign('1+2'), + 'response': '3', + }, REMOTE_ADDR='127.0.0.1') + assert r.status_code == 302 + assert len(djmail.outbox) == 1 + with scopes_disabled(): + customer = env[0].customers.get(email='john@example.org') + assert not customer.is_verified + assert customer.is_active + + @pytest.mark.django_db def test_org_resetpw(env, client): with scopes_disabled(): @@ -479,7 +503,7 @@ def test_org_order_list(env, client): assert o2.code not in content assert o3.code in content - env[0].settings.customer_accounts_link_by_email = True + env[0].settings.customer_accounts_link_by_email = 'True' r = client.get('/bigevents/account/') assert r.status_code == 200