Refs #2465 -- Introduce unique identifiers for authentication backends (#2466)

This commit is contained in:
Raphael Michel
2022-02-23 10:16:35 +01:00
committed by GitHub
parent 2bf5a0ce8a
commit d7acd2b6bf
6 changed files with 221 additions and 10 deletions
+61 -7
View File
@@ -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 <config>`.
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 <config>`.
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)
+6 -1
View File
@@ -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
@@ -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')},
),
]
+121 -1
View File
@@ -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()
@@ -38,6 +38,14 @@
<input name="text" value="{{ backend }}" class="form-control" disabled>
</div>
</div>
{% if user.auth_backend_identifier %}
<div class="form-group">
<label class="col-md-3 control-label">{% trans "External identifier" %}</label>
<div class="col-md-9">
<input name="text" value="{{ user.auth_backend_identifier }}" class="form-control" disabled>
</div>
</div>
{% endif %}
{% bootstrap_field form.email layout='control' %}
{% if form.new_pw %}
{% bootstrap_field form.new_pw layout='control' %}
+3 -1
View File
@@ -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