diff --git a/doc/development/api/auth.rst b/doc/development/api/auth.rst index 71add64cdd..9111dc1c7d 100644 --- a/doc/development/api/auth.rst +++ b/doc/development/api/auth.rst @@ -20,20 +20,31 @@ Basically, three pre-defined flows are supported: * Authentication mechanisms that rely on **redirection**, e.g. to an OAuth provider. These can be implemented by supplying a ``authentication_url`` method and implementing a custom return view. -Authentication backends are *not* collected through a signal. Instead, they must explicitly be set through the -``auth_backends`` directive in the ``pretix.cfg`` :ref:`configuration file `. +For security reasons, authentication backends are *not* automatically discovered through a signal. Instead, they must +explicitly be set through the ``auth_backends`` directive in the ``pretix.cfg`` :ref:`configuration file `. -In each of these methods (``form_authenticate``, ``request_authenticate`` or your custom view) you are supposed to -either get an existing :py:class:`pretix.base.models.User` object from the database or create a new one. There are a -few rules you need to follow: +In each of these methods (``form_authenticate``, ``request_authenticate``, or your custom view) you are supposed to +use ``User.objects.get_or_create_for_backend`` to get a :py:class:`pretix.base.models.User` object from the database +or create a new one. -* You **MUST** only return users with the ``auth_backend`` attribute set to the ``identifier`` value of your backend. +There are a few rules you need to follow: -* You **MUST** create new users with the ``auth_backend`` attribute set to the ``identifier`` value of your backend. +* You **MUST** have some kind of identifier for a user that is globally unique and **SHOULD** never change, even if the + user's name or email address changes. This could e.g. be the ID of the user in an external database. The identifier + must not be longer than 190 characters. If you worry your backend might generated longer identifiers, consider + using a hash function to trim them to a constant length. + +* You **SHOULD** not allow users created by other authentication backends to log in through your code, and you **MUST** + only create, modify or return users with ``auth_backend`` set to your backend. * Every user object **MUST** have an email address. Email addresses are globally unique. If the email address is already registered to a user who signs in through a different backend, you **SHOULD** refuse the login. +``User.objects.get_or_create_for_backend`` will follow these rules for you automatically. It works like this: + +.. autoclass:: pretix.base.models.UserManager + :members: get_or_create_for_backend + The backend interface --------------------- @@ -59,6 +70,7 @@ The backend interface .. automethod:: authentication_url + Logging users in ---------------- @@ -68,3 +80,45 @@ recommend that you use the following utility method to correctly set session val authentication (if activated): .. autofunction:: pretix.control.views.auth.process_login + +A custom view that is called after a redirect from an external identity provider could look like this:: + + from django.contrib import messages + from django.shortcuts import redirect + from django.urls import reverse + + from pretix.base.models import User + from pretix.base.models.auth import EmailAddressTakenError + from pretix.control.views.auth import process_login + + + def return_view(request): + # Verify validity of login with the external provider's API + api_response = my_verify_login_function( + code=request.GET.get('code') + ) + + try: + u = User.objects.get_or_create_for_backend( + 'my_backend_name', + api_response['userid'], + api_response['email'], + set_always={ + 'fullname': '{} {}'.format( + api_response.get('given_name', ''), + api_response.get('family_name', ''), + ), + }, + set_on_creation={ + 'locale': api_response.get('locale').lower()[:2], + 'timezone': api_response.get('zoneinfo', 'UTC'), + } + ) + except EmailAddressTakenError: + messages.error( + request, _('We cannot create your user account as a user account in this system ' + 'already exists with the same email address.') + ) + return redirect(reverse('control:auth.login')) + else: + return process_login(request, u, keep_logged_in=False) diff --git a/src/pretix/base/auth.py b/src/pretix/base/auth.py index c0e94b7ccc..fbdb1415ae 100644 --- a/src/pretix/base/auth.py +++ b/src/pretix/base/auth.py @@ -94,6 +94,9 @@ class BaseAuthBackend: This method will be called after the user filled in the login form. ``request`` will contain the current request and ``form_data`` the input for the form fields defined in ``login_form_fields``. You are expected to either return a ``User`` object (if login was successful) or ``None``. + + You are expected to either return a ``User`` object (if login was successful) or ``None``. You should + obtain this user object using ``User.objects.get_or_create_for_backend``. """ return @@ -104,7 +107,9 @@ class BaseAuthBackend: reverse proxy, you can directly return a ``User`` object that will be logged in. ``request`` will contain the current request. - You are expected to either return a ``User`` object (if login was successful) or ``None``. + + You are expected to either return a ``User`` object (if login was successful) or ``None``. You should + obtain this user object using ``User.objects.get_or_create_for_backend``. """ return diff --git a/src/pretix/base/migrations/0208_auto_20220214_1632.py b/src/pretix/base/migrations/0208_auto_20220214_1632.py new file mode 100644 index 0000000000..b25f4ca445 --- /dev/null +++ b/src/pretix/base/migrations/0208_auto_20220214_1632.py @@ -0,0 +1,22 @@ +# Generated by Django 3.2.4 on 2022-02-14 16:32 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('pretixbase', '0207_auto_20220119_1427'), + ] + + operations = [ + migrations.AddField( + model_name='user', + name='auth_backend_identifier', + field=models.CharField(db_index=True, max_length=190, null=True), + ), + migrations.AlterUniqueTogether( + name='user', + unique_together={('auth_backend', 'auth_backend_identifier')}, + ), + ] diff --git a/src/pretix/base/models/auth.py b/src/pretix/base/models/auth.py index 10f7a84d34..117212437e 100644 --- a/src/pretix/base/models/auth.py +++ b/src/pretix/base/models/auth.py @@ -44,7 +44,7 @@ from django.contrib.auth.models import ( ) from django.contrib.auth.tokens import default_token_generator from django.contrib.contenttypes.models import ContentType -from django.db import models +from django.db import IntegrityError, models, transaction from django.db.models import Q from django.utils.crypto import get_random_string, salted_hmac from django.utils.timezone import now @@ -61,6 +61,10 @@ from pretix.helpers.urls import build_absolute_uri from .base import LoggingMixin +class EmailAddressTakenError(IntegrityError): + pass + + class UserManager(BaseUserManager): """ This is the user manager for our custom user model. See the User @@ -83,6 +87,116 @@ class UserManager(BaseUserManager): user.save() return user + def get_or_create_for_backend(self, backend, identifier, email, set_always, set_on_creation): + """ + This method should be used by third-party authentication backends to log in a user. + It either returns an already existing user or creates a new user. + + In pretix 4.7 and earlier, email addresses were the only property to identify a user with. + Starting with pretix 4.8, backends SHOULD instead use a unique, immutable identifier + based on their backend data store to allow for changing email addresses. + + This method transparently handles the conversion of old user accounts and adds the + backend identifier to their database record. + + This method will never return users managed by a different authentication backend. + If you try to create an account with an email address already blocked by a different + authentication backend, :py:class:`EmailAddressTakenError` will be raised. In this case, + you should display a message to the user. + + :param backend: The `identifier` attribute of the authentication backend + :param identifier: The unique, immutable identifier of this user, max. 190 characters + :param email: The user's email address + :param set_always: A dictionary of fields to update on the user model on every login + :param set_on_creation: A dictionary of fields to set on the user model if it's newly created + :return: A `User` instance. + """ + if identifier is None: + raise ValueError('You need to supply a custom, unique identifier for this user.') + if email is None: + raise ValueError('You need to supply an email address for this user.') + if 'auth_backend_identifier' in set_always or 'auth_backend_identifier' in set_on_creation or \ + 'auth_backend' in set_always or 'auth_backend' in set_on_creation: + raise ValueError('You may not update auth_backend/auth_backend_identifier.') + if len(identifier) > 190: + raise ValueError('The user identifier must not be more than 190 characters.') + + # Always update the email address + set_always.update({'email': email}) + + # First, check if we find the user based on it's backend-specific authenticator + try: + u = self.get( + auth_backend=backend, + auth_backend_identifier=identifier, + ) + dirty = False + for k, v in set_always.items(): + if getattr(u, k) != v: + setattr(u, k, v) + dirty = True + if dirty: + try: + with transaction.atomic(): + u.save(update_fields=set_always.keys()) + except IntegrityError: + # This might only raise IntegrityError if the email address is used + # by someone else + raise EmailAddressTakenError() + return u + except self.model.DoesNotExist: + pass + + # Second, check if we find the user based on their email address and this backend + try: + u = self.get( + auth_backend=backend, + auth_backend_identifier__isnull=True, + email=email, + ) + u.auth_backend_identifier = identifier + for k, v in set_always.items(): + setattr(u, k, v) + try: + with transaction.atomic(): + u.save(update_fields=['auth_backend_identifier'] + list(set_always.keys())) + return u + except IntegrityError: + # This might only raise IntegrityError if this code is being executed twice + # and runs into a race condition, this mechanism is taken from Django's + # get_or_create + try: + return self.get( + auth_backend=backend, + auth_backend_identifier=identifier, + ) + except self.model.DoesNotExist: + pass + raise + except self.model.DoesNotExist: + pass + + # Third, create a new user + u = User( + auth_backend=backend, + auth_backend_identifier=identifier, + **set_on_creation, + **set_always, + ) + try: + u.save(force_insert=True) + return u + except IntegrityError: + # This might either be a race condition or the email address is taken + # by a different backend + try: + return self.get( + auth_backend=backend, + auth_backend_identifier=identifier, + ) + except self.model.DoesNotExist: + raise EmailAddressTakenError() + def generate_notifications_token(): return get_random_string(length=32) @@ -117,6 +231,10 @@ class User(AbstractBaseUser, PermissionsMixin, LoggingMixin): :type needs_password_change: bool :param timezone: The user's preferred timezone. :type timezone: str + :param auth_backend: The identifier of the authentication backend plugin responsible for managing this user. + :type auth_backend: str + :param auth_backend_identifier: The native identifier of the user provided by a non-native authentication backend. + :type auth_backend_identifier: str """ USERNAME_FIELD = 'email' @@ -152,6 +270,7 @@ class User(AbstractBaseUser, PermissionsMixin, LoggingMixin): ) notifications_token = models.CharField(max_length=255, default=generate_notifications_token) auth_backend = models.CharField(max_length=255, default='native') + auth_backend_identifier = models.CharField(max_length=190, db_index=True, null=True, blank=True) session_token = models.CharField(max_length=32, default=generate_session_token) objects = UserManager() @@ -164,6 +283,7 @@ class User(AbstractBaseUser, PermissionsMixin, LoggingMixin): verbose_name = _("User") verbose_name_plural = _("Users") ordering = ('email',) + unique_together = (('auth_backend', 'auth_backend_identifier'),) def save(self, *args, **kwargs): self.email = self.email.lower() diff --git a/src/pretix/control/templates/pretixcontrol/users/form.html b/src/pretix/control/templates/pretixcontrol/users/form.html index d466e0b448..658eeeb939 100644 --- a/src/pretix/control/templates/pretixcontrol/users/form.html +++ b/src/pretix/control/templates/pretixcontrol/users/form.html @@ -38,6 +38,14 @@ + {% if user.auth_backend_identifier %} +
+ +
+ +
+
+ {% endif %} {% bootstrap_field form.email layout='control' %} {% if form.new_pw %} {% bootstrap_field form.new_pw layout='control' %} diff --git a/src/pretix/control/views/users.py b/src/pretix/control/views/users.py index 33e065c6ee..20e1a676ba 100644 --- a/src/pretix/control/views/users.py +++ b/src/pretix/control/views/users.py @@ -162,10 +162,12 @@ class UserAnonymizeView(AdministratorPermissionRequiredMixin, RecentAuthenticati self.object = get_object_or_404(User, pk=self.kwargs.get("id")) self.object.log_action('pretix.user.anonymized', user=request.user) - self.object.email = "{}@disabled.pretix.eu".format(self.object.pk) + self.object.email = "{}.{}@disabled.pretix.eu".format(self.object.pk, self.object.auth_backend) self.object.fullname = "" self.object.is_active = False self.object.notifications_send = False + self.object.auth_backend = None + self.object.auth_backend_identifier = None self.object.save() for le in self.object.all_logentries.filter(action_type="pretix.user.settings.changed"): d = le.parsed_data