♻️(backend) defend user provisioning against race condition

Move user provisioning logic out of the external token viewset into a
dedicated service to keep the viewset lightweight and easier to
maintain.

While extracting the logic, refactor user object handling to improve
robustness and make the provisioning workflow easier to reason about.

Defend against race conditions when concurrent requests attempt to
provision the same user. Rely on the existing database constraints to
guarantee uniqueness and gracefully handle integrity errors raised by
concurrent creations.
This commit is contained in:
lebaudantoine
2026-06-02 16:20:40 +02:00
committed by aleb_the_flash
parent e25aa6ce05
commit 3554b2eb53
3 changed files with 212 additions and 35 deletions
+14 -35
View File
@@ -4,7 +4,7 @@ from logging import getLogger
from django.conf import settings
from django.contrib.auth.hashers import check_password
from django.core.exceptions import SuspiciousOperation, ValidationError
from django.core.exceptions import ValidationError
from django.core.validators import validate_email
from lasuite.oidc_resource_server.authentication import ResourceServerAuthentication
@@ -23,6 +23,11 @@ from core import api, models
from core.api.feature_flag import FeatureFlag
from core.services.jwt_token import JwtTokenService
from ..services.provisional_user_service import (
ProvisionalUserCreationDisabledError,
ProvisionalUserIntegrityError,
ProvisionalUserService,
)
from . import authentication, permissions, serializers
logger = getLogger(__name__)
@@ -94,40 +99,14 @@ class ApplicationViewSet(viewsets.ViewSet):
)
try:
user = models.User.objects.get(email__iexact=email)
except models.User.DoesNotExist as e:
if (
settings.APPLICATION_ALLOW_USER_CREATION
and settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION
and not settings.OIDC_USER_SUB_FIELD_IMMUTABLE
):
# Create a provisional user without `sub`, identified by email only.
#
# This relies on Django LaSuite implicitly updating the `sub` field on the
# user's first successful OIDC authentication. If this stops working,
# check for behavior changes in Django LaSuite.
#
# `OIDC_USER_SUB_FIELD_IMMUTABLE` comes from Django LaSuite and prevents `sub`
# updates. We override its default value to allow setting `sub` for
# provisional users.
user = models.User(
sub=None,
email=email,
)
user.set_unusable_password()
user.save()
logger.info(
"Provisional user created via application: user_id=%s, email=%s, client_id=%s",
user.id,
email,
application.client_id,
)
else:
raise drf_exceptions.NotFound("User not found.") from e
except models.User.MultipleObjectsReturned as e:
raise SuspiciousOperation(
"Multiple user accounts share a common email."
) from e
user, _ = ProvisionalUserService().get_or_create(email, client_id)
except ProvisionalUserCreationDisabledError as not_found_error:
raise drf_exceptions.NotFound("User not found.") from not_found_error
except ProvisionalUserIntegrityError:
return drf_response.Response(
{"error": "Failed to create or retrieve provisional user."},
status=drf_status.HTTP_409_CONFLICT,
)
scope = " ".join(application.scopes or [])
@@ -0,0 +1,110 @@
"""Service for provisional user creation."""
import logging
from django.conf import settings
from django.core.exceptions import SuspiciousOperation, ValidationError
from django.db import IntegrityError
from core import models
logger = logging.getLogger(__name__)
class ProvisionalUserError(Exception):
"""Base exception for provisional user service errors."""
class ProvisionalUserCreationDisabledError(ProvisionalUserError):
"""Raised when provisional user creation is disabled by configuration."""
class ProvisionalUserIntegrityError(ProvisionalUserError):
"""Raised when a provisional user cannot be created or retrieved after a race condition."""
class ProvisionalUserService:
"""Handles creation and retrieval of provisional users.
A provisional user is created without a `sub`, identified by email only.
The `sub` is set on first successful OIDC authentication via Django LaSuite.
"""
def __init__(self):
"""Initialize the service."""
# `OIDC_USER_SUB_FIELD_IMMUTABLE` comes from Django LaSuite and prevents `sub`
# updates. We override its default value to allow setting `sub` for
# provisional users.
self._is_creation_enabled = (
settings.APPLICATION_ALLOW_USER_CREATION
and settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION
and not settings.OIDC_USER_SUB_FIELD_IMMUTABLE
)
def _get_by_email(self, email: str) -> models.User | None:
"""Return the user with this email, or None if not found."""
try:
return models.User.objects.get(email__iexact=email)
except models.User.DoesNotExist:
return None
except models.User.MultipleObjectsReturned as e:
raise SuspiciousOperation(
"Multiple user accounts share a common email."
) from e
def get_or_create(
self, email: str, client_id: str
) -> tuple[models.User | None, bool]:
"""Get or create a provisional user identified by email.
Args:
email: The email address to identify the user.
client_id: The application client_id, used for audit logging only.
Returns:
A (user, created) tuple — mirrors get_or_create conventions.
Raises:
ProvisionalUserError: If creation and retrieval both fail.
"""
user = self._get_by_email(email)
if user:
return user, False
if not self._is_creation_enabled:
raise ProvisionalUserCreationDisabledError(
"Provisional user creation is disabled by configuration."
)
# Create a provisional user without `sub`, identified by email only.
# This relies on Django LaSuite implicitly updating the `sub` field on the
# user's first successful OIDC authentication. If this stops working,
# check for behavior changes in Django LaSuite.
try:
user = models.User(sub=None, email=email)
user.set_unusable_password()
user.save()
logger.info(
"Provisional user created via application: user_id=%s, email=%s, client_id=%s",
user.id,
email,
client_id,
)
return user, True
except (IntegrityError, ValidationError) as e:
logger.warning(
"Race condition on provisional user creation, fetching existing: "
"email=%s, client_id=%s",
email,
client_id,
)
user = self._get_by_email(email)
if user:
return user, False
raise ProvisionalUserIntegrityError(
"Failed to create or retrieve provisional user."
) from e
@@ -4,6 +4,8 @@ Tests for external API /token endpoint
# pylint: disable=W0621
from unittest import mock
import jwt
import pytest
from freezegun import freeze_time
@@ -15,6 +17,7 @@ from core.factories import (
UserFactory,
)
from core.models import ApplicationScope, User
from core.services import provisional_user_service
pytestmark = pytest.mark.django_db
@@ -436,3 +439,88 @@ def test_api_applications_token_existing_user(settings):
"delegated": True,
"scope": "rooms:list rooms:create",
}
@mock.patch.object(provisional_user_service.ProvisionalUserService, "_get_by_email")
def test_api_applications_token_new_user_race_condition(mock_get_by_email, settings):
"""Should handle race condition where two concurrent requests create the same user."""
settings.APPLICATION_ALLOW_USER_CREATION = True
settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION = True
settings.OIDC_USER_SUB_FIELD_IMMUTABLE = False
application = ApplicationFactory(
is_active=True, scopes=[ApplicationScope.ROOMS_LIST]
)
plain_secret = "test-secret-123"
application.client_secret = plain_secret
application.save()
email = "john.doe@example.com"
# First call: lie and say user doesn't exist, simulating the race window
# Second call (recovery path): return the real user
existing_user = UserFactory(sub=None, email=email)
mock_get_by_email.side_effect = [None, existing_user]
client = APIClient()
response = client.post(
"/external-api/v1.0/application/token/",
{
"client_id": application.client_id,
"client_secret": plain_secret,
"grant_type": "client_credentials",
"scope": email,
},
format="json",
)
assert response.status_code == 200
assert mock_get_by_email.call_count == 2
token = response.data["access_token"]
payload = jwt.decode(
token,
settings.APPLICATION_JWT_SECRET_KEY,
algorithms=[settings.APPLICATION_JWT_ALG],
issuer=settings.APPLICATION_JWT_ISSUER,
audience=settings.APPLICATION_JWT_AUDIENCE,
)
assert payload["user_id"] == str(existing_user.id)
assert User.objects.filter(email=email).count() == 1
@mock.patch.object(
provisional_user_service.ProvisionalUserService,
"get_or_create",
side_effect=provisional_user_service.ProvisionalUserIntegrityError,
)
def test_api_applications_token_new_user_race_condition_unrecoverable(
mock_get_or_create, settings
):
"""Should return 500 when ProvisionalUserIntegrityError is raised."""
settings.APPLICATION_ALLOW_USER_CREATION = True
settings.OIDC_FALLBACK_TO_EMAIL_FOR_IDENTIFICATION = True
settings.OIDC_USER_SUB_FIELD_IMMUTABLE = False
application = ApplicationFactory(
is_active=True, scopes=[ApplicationScope.ROOMS_LIST]
)
plain_secret = "test-secret-123"
application.client_secret = plain_secret
application.save()
client = APIClient()
response = client.post(
"/external-api/v1.0/application/token/",
{
"client_id": application.client_id,
"client_secret": plain_secret,
"grant_type": "client_credentials",
"scope": "john.doe@example.com",
},
format="json",
)
assert response.status_code == 409
assert mock_get_or_create.call_count == 1