From 166819f388e5ff612a9d3502b29f540b7351511e Mon Sep 17 00:00:00 2001 From: Rich Wareham <rjw57@cam.ac.uk> Date: Thu, 26 Apr 2018 14:19:11 +0100 Subject: [PATCH] authentication: remove race condition in creating users Previously we emulated a get_or_create user behaviour by attempting to get the user and, if that failed, create it. This introduced a race where another worker could create a user between these checks. (We observed this in production when an initial log in causes a flurry of requests to lookupproxy.) This was a result of Django's autocommit mode since the user fetch and subsequent user creation were separate transactions allowing for database change between them. The race-free approach is to *always* attempt to create the user and only if that fails do we fetch the user. These are still run as separate transactions but the user object will always be created by the first worker to talk to the DB and the rest will see an integrity error causing them to fetch the new user object. --- lookupapi/authentication.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lookupapi/authentication.py b/lookupapi/authentication.py index d5f9378..e952862 100644 --- a/lookupapi/authentication.py +++ b/lookupapi/authentication.py @@ -4,9 +4,9 @@ OAuth2 authentication for Django REST Framework views. """ import datetime import logging +import django.db from django.conf import settings from django.contrib.auth import get_user_model -from django.core.exceptions import ObjectDoesNotExist from rest_framework.authentication import BaseAuthentication from requests.adapters import HTTPAdapter from requests_oauthlib import OAuth2Session @@ -83,10 +83,12 @@ class OAuth2TokenAuthentication(BaseAuthentication): # This is not quite the same as the default get_or_create() behaviour because we make # use of the create_user() helper here. This ensures the user is created and that # set_unusable_password() is also called on it. + # + # See https://stackoverflow.com/questions/7511391/ try: - user = get_user_model().objects.get(username=subject) - except ObjectDoesNotExist: user = get_user_model().objects.create_user(username=subject) + except django.db.IntegrityError: + user = get_user_model().objects.get(username=subject) else: user = None -- GitLab