From d79de5eca4450caee514089ee2df3ffd43f27d14 Mon Sep 17 00:00:00 2001 From: Benjamin Dauvergne Date: Mon, 12 Aug 2019 11:27:15 +0200 Subject: [PATCH] api: prevent password change on get_or_create (#34950) --- src/authentic2/api_mixins.py | 2 ++ src/authentic2/api_views.py | 21 +++++++++++++-------- tests/test_api.py | 7 +++++++ 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/authentic2/api_mixins.py b/src/authentic2/api_mixins.py index 74c6740c..4248b41c 100644 --- a/src/authentic2/api_mixins.py +++ b/src/authentic2/api_mixins.py @@ -47,6 +47,7 @@ class GetOrCreateModelSerializer(object): defaults[key] = value with transaction.atomic(): instance, created = self.Meta.model._default_manager.get_or_create(**kwargs) + instance._a2_created = created if many_to_many and created: self.update(instance, many_to_many) return instance @@ -78,6 +79,7 @@ class GetOrCreateModelSerializer(object): defaults[key] = value with transaction.atomic(): instance, created = self.Meta.model._default_manager.get_or_create(**kwargs) + instance._a2_created = created if many_to_many or not created: self.update(instance, validated_data) return instance diff --git a/src/authentic2/api_views.py b/src/authentic2/api_views.py index 913fa71a..287e85eb 100644 --- a/src/authentic2/api_views.py +++ b/src/authentic2/api_views.py @@ -339,9 +339,7 @@ class BaseUserSerializer(api_mixins.GetOrCreateModelSerializer, send_registration_email = serializers.BooleanField(write_only=True, required=False, default=False) send_registration_email_next_url = serializers.URLField(write_only=True, required=False) - password = serializers.CharField(max_length=128, - default=CreateOnlyDefault(utils.generate_password), - required=False) + password = serializers.CharField(max_length=128, required=False) force_password_reset = serializers.BooleanField(write_only=True, required=False, default=False) def __init__(self, *args, **kwargs): @@ -394,8 +392,12 @@ class BaseUserSerializer(api_mixins.GetOrCreateModelSerializer, attributes = validated_data.pop('attributes', {}) is_verified = validated_data.pop('is_verified', {}) + password = validated_data.pop('password', None) self.check_perm('custom_user.add_user', validated_data.get('ou')) instance = super(BaseUserSerializer, self).create(validated_data) + # prevent update on a get_or_create + if not getattr(instance, '_a2_created', True): + return instance for key, value in attributes.items(): if is_verified.get(key): setattr(instance.verified_attributes, key, value) @@ -405,9 +407,11 @@ class BaseUserSerializer(api_mixins.GetOrCreateModelSerializer, instance.verified_attributes.first_name = instance.first_name if is_verified.get('last_name'): instance.verified_attributes.last_name = instance.last_name - if 'password' in validated_data: - instance.set_password(validated_data['password']) - instance.save() + if password is not None: + instance.set_password(password) + else: + instance.set_unusable_password() + instance.save() if force_password_reset: PasswordReset.objects.get_or_create(user=instance) if send_registration_email and validated_data.get('email'): @@ -433,6 +437,7 @@ class BaseUserSerializer(api_mixins.GetOrCreateModelSerializer, validated_data.pop('send_registration_email_next_url', None) attributes = validated_data.pop('attributes', {}) is_verified = validated_data.pop('is_verified', {}) + password = validated_data.pop('password', None) # Double check: to move an user from one ou into another you must be administrator of both self.check_perm('custom_user.change_user', instance.ou) if 'ou' in validated_data: @@ -456,8 +461,8 @@ class BaseUserSerializer(api_mixins.GetOrCreateModelSerializer, instance.verified_attributes.first_name = instance.first_name if is_verified.get('last_name'): instance.verified_attributes.last_name = instance.last_name - if 'password' in validated_data: - instance.set_password(validated_data['password']) + if password is not None: + instance.set_password(password) instance.save() if force_password_reset: PasswordReset.objects.get_or_create(user=instance) diff --git a/tests/test_api.py b/tests/test_api.py index 86694a72..5748fbc8 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -1163,17 +1163,21 @@ def test_api_users_get_or_create(settings, app, admin): id = resp.json['id'] assert User.objects.get(id=id).first_name == 'John' assert User.objects.get(id=id).last_name == 'Doe' + password = User.objects.get(id=id).password resp = app.post_json('/api/users/?get_or_create=email', params=payload, status=201) assert id == resp.json['id'] assert User.objects.get(id=id).first_name == 'John' assert User.objects.get(id=id).last_name == 'Doe' + assert User.objects.get(id=id).password == password + paylaod = {} payload['first_name'] = 'Jane' resp = app.post_json('/api/users/?update_or_create=email', params=payload, status=201) assert id == resp.json['id'] assert User.objects.get(id=id).first_name == 'Jane' assert User.objects.get(id=id).last_name == 'Doe' + assert User.objects.get(id=id).password == password def test_api_users_get_or_create_email_is_unique(settings, app, admin): @@ -1214,16 +1218,19 @@ def test_api_users_get_or_create_multi_key(settings, app, admin): id = resp.json['id'] assert User.objects.get(id=id).first_name == 'John' assert User.objects.get(id=id).last_name == 'Doe' + password = User.objects.get(id=id).password resp = app.post_json('/api/users/?get_or_create=first_name&get_or_create=last_name', params=payload, status=201) assert id == resp.json['id'] assert User.objects.get(id=id).first_name == 'John' assert User.objects.get(id=id).last_name == 'Doe' + assert User.objects.get(id=id).password == password payload['email'] = 'john.doe@example2.net' resp = app.post_json('/api/users/?update_or_create=first_name&update_or_create=last_name', params=payload, status=201) assert id == resp.json['id'] assert User.objects.get(id=id).email == 'john.doe@example2.net' + assert User.objects.get(id=id).password == password def test_api_roles_get_or_create(settings, ou1, app, admin): -- 2.23.0.rc1