From bcfbb774dc82eeadcb44c39ed40399b2e87f14ce Mon Sep 17 00:00:00 2001
From: Art Lukyanchyk <artiom.lukyanchyk@hs-hannover.de>
Date: Sun, 29 Aug 2021 20:44:49 +0200
Subject: [PATCH] Prevent personal data leaks when IDP provides duplicate
usernames
---
ssoauth/auth_utils.py | 42 +++++++++++++++++-------------------------
1 file changed, 17 insertions(+), 25 deletions(-)
diff --git a/ssoauth/auth_utils.py b/ssoauth/auth_utils.py
index 4ce6bd6..f8323bc 100644
--- a/ssoauth/auth_utils.py
+++ b/ssoauth/auth_utils.py
@@ -1,4 +1,4 @@
-from uuid import UUID
+from uuid import UUID, uuid4
from django.db import transaction
from django.contrib.auth import get_user_model
from django.contrib.auth.models import Group
@@ -31,15 +31,25 @@ def get_user(uuid=None, username=None):
def get_or_create_user(uuid, username):
"""
Returns a user.
- Is able to process many weird cases like changed username or re-created user in DS.
+ Should be able to process a bunch of weird cases like changed username or duplicate usernames.
If it dies with an exception, with 90% probability the data state is very bad.
"""
- def get_user_by_uuid(uuid, username):
+ def free_up_username(username):
+ try:
+ other_user = get_user(username=username)
+ except get_user_model().DoesNotExist:
+ return
+ new_username = "{0}_OLD_{1}".format(other_user.username, uuid4())
+ logger.warning("Found another user with username {old_username}. Renaming {old_username} to {new_username}".format(old_username=other_user.username, new_username=new_username))
+ other_user.username = new_username
+ other_user.save()
+
+ def get_existing_user(uuid, username):
try:
user = get_user(uuid=uuid)
if user.username != username:
- # have to do it because users might be renamed
+ free_up_username(username)
logger.warning("Username has changed. Renaming locally from {0} to {1}.".format(user.username, username))
user.username = username
user.save()
@@ -47,23 +57,9 @@ def get_or_create_user(uuid, username):
except models.UserMapping.DoesNotExist:
return None
- def get_user_by_username(uuid, username):
- try:
- user = get_user(username=username)
- except get_user_model().DoesNotExist:
- return None
- try:
- meta = models.UserMapping.objects.get(user=user)
- logger.warning("User UUID changed. Assuming re-created in DS. Automagically fixing...")
- meta.uuid = uuid
- meta.save()
- except models.UserMapping.DoesNotExist:
- logger.error("User {username} lost their meta. Auto-fixing and hoping for the best.".format(**locals()))
- models.UserMapping.objects.create(user=user, uuid=uuid)
- return user
-
def create_user(uuid, username):
_validate_username(username)
+ free_up_username(username)
user = get_user_model().objects.create(username=username, is_staff=False)
user.set_unusable_password()
user.save()
@@ -77,12 +73,8 @@ def get_or_create_user(uuid, username):
assert isinstance(uuid, UUID) and isinstance(username, str)
username = username.lower()
# get or create
- user = get_user_by_uuid(uuid, username) # best case scenario
- if not user:
- user = get_user_by_username(uuid, username) # worst case scenario
- if not user:
- user = create_user(uuid, username) # create if not present
- # sanity check
+ user = get_existing_user(uuid, username) or create_user(uuid, username)
+ # just in case, ensure the user object complies with the security rules
cleanup_direct_permissions(user)
return user
--
GitLab