From 4dcdbc4c73b68a6da23680502f291145cce0a65e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20Barrag=C3=A1n=20Merino?= Date: Fri, 11 Nov 2016 13:06:02 +0100 Subject: [PATCH] Fix #4756: PATCH and PUT can't change user email --- taiga/users/api.py | 6 ++-- tests/integration/test_users.py | 60 +++++++++++++++++++++++++++++---- 2 files changed, 56 insertions(+), 10 deletions(-) diff --git a/taiga/users/api.py b/taiga/users/api.py index b67882d6..b3e2e707 100644 --- a/taiga/users/api.py +++ b/taiga/users/api.py @@ -112,7 +112,7 @@ class UsersViewSet(ModelCrudViewSet): user = self.get_object() self.check_permissions(request, "update", user) - new_email = request.DATA.get('email', None) + new_email = request.DATA.pop('email', None) if new_email is not None: valid_new_email = True duplicated_email = models.User.objects.filter(email=new_email).exists() @@ -143,9 +143,7 @@ class UsersViewSet(ModelCrudViewSet): ) email.send() - ret = super().partial_update(request, *args, **kwargs) - - return ret + return super().partial_update(request, *args, **kwargs) def destroy(self, request, pk=None): user = self.get_object() diff --git a/tests/integration/test_users.py b/tests/integration/test_users.py index 3accebb7..cad6507c 100644 --- a/tests/integration/test_users.py +++ b/tests/integration/test_users.py @@ -47,6 +47,10 @@ import os pytestmark = pytest.mark.django_db +############################## +## Create user +############################## + def test_users_create_through_standard_api(client): user = f.UserFactory.create(is_superuser=True) @@ -62,6 +66,10 @@ def test_users_create_through_standard_api(client): assert response.status_code == 405 +############################## +## Change email +############################## + def test_update_user_with_same_email(client): user = f.UserFactory.create(email="same@email.com") url = reverse('users-detail', kwargs={"pk": user.pk}) @@ -73,6 +81,9 @@ def test_update_user_with_same_email(client): assert response.status_code == 400 assert response.data['_error_message'] == 'Duplicated email' + user.refresh_from_db() + assert user.email == "same@email.com" + def test_update_user_with_duplicated_email(client): f.UserFactory.create(email="one@email.com") @@ -86,6 +97,9 @@ def test_update_user_with_duplicated_email(client): assert response.status_code == 400 assert response.data['_error_message'] == 'Duplicated email' + user.refresh_from_db() + assert user.email == "two@email.com" + def test_update_user_with_invalid_email(client): user = f.UserFactory.create(email="my@email.com") @@ -98,6 +112,9 @@ def test_update_user_with_invalid_email(client): assert response.status_code == 400 assert response.data['_error_message'] == 'Not valid email' + user.refresh_from_db() + assert user.email == "my@email.com" + def test_update_user_with_unallowed_domain_email(client, settings): settings.USER_EMAIL_ALLOWED_DOMAINS = ['email.com'] @@ -111,6 +128,8 @@ def test_update_user_with_unallowed_domain_email(client, settings): assert response.status_code == 400 assert response.data['_error_message'] == 'Not valid email' + user.refresh_from_db() + assert user.email == "my@email.com" def test_update_user_with_allowed_domain_email(client, settings): settings.USER_EMAIL_ALLOWED_DOMAINS = ['email.com'] @@ -123,7 +142,9 @@ def test_update_user_with_allowed_domain_email(client, settings): response = client.patch(url, json.dumps(data), content_type="application/json") assert response.status_code == 200 - user = models.User.objects.get(pk=user.id) + + user.refresh_from_db() + assert user.email == "old@email.com" assert user.email_token is not None assert user.new_email == "new@email.com" @@ -137,13 +158,14 @@ def test_update_user_with_valid_email(client): response = client.patch(url, json.dumps(data), content_type="application/json") assert response.status_code == 200 - user = models.User.objects.get(pk=user.id) + user.refresh_from_db() + assert user.email == "old@email.com" assert user.email_token is not None assert user.new_email == "new@email.com" def test_validate_requested_email_change(client): - user = f.UserFactory.create(email_token="change_email_token", new_email="new@email.com") + user = f.UserFactory.create(email="old@email.com", email_token="change_email_token", new_email="new@email.com") url = reverse('users-change-email') data = {"email_token": "change_email_token"} @@ -151,24 +173,26 @@ def test_validate_requested_email_change(client): response = client.post(url, json.dumps(data), content_type="application/json") assert response.status_code == 204 - user = models.User.objects.get(pk=user.id) + user.refresh_from_db() assert user.email_token is None assert user.new_email is None assert user.email == "new@email.com" + def test_validate_requested_email_change_for_anonymous_user(client): - user = f.UserFactory.create(email_token="change_email_token", new_email="new@email.com") + user = f.UserFactory.create(email="old@email.com", email_token="change_email_token", new_email="new@email.com") url = reverse('users-change-email') data = {"email_token": "change_email_token"} response = client.post(url, json.dumps(data), content_type="application/json") assert response.status_code == 204 - user = models.User.objects.get(pk=user.id) + user.refresh_from_db() assert user.email_token is None assert user.new_email is None assert user.email == "new@email.com" + def test_validate_requested_email_change_without_token(client): user = f.UserFactory.create(email_token="change_email_token", new_email="new@email.com") url = reverse('users-change-email') @@ -190,6 +214,10 @@ def test_validate_requested_email_change_with_invalid_token(client): assert response.status_code == 400 +############################## +## Delete user +############################## + def test_delete_self_user(client): user = f.UserFactory.create() url = reverse('users-detail', kwargs={"pk": user.pk}) @@ -229,6 +257,10 @@ def test_delete_self_user_remove_membership_projects(client): assert project.memberships.all().count() == 0 +############################## +## Cancel account +############################## + def test_cancel_self_user_with_valid_token(client): user = f.UserFactory.create() url = reverse('users-cancel') @@ -252,6 +284,10 @@ def test_cancel_self_user_with_invalid_token(client): assert response.status_code == 400 +############################## +## Avatar +############################## + def test_change_avatar(client): url = reverse('users-change-avatar') @@ -346,6 +382,10 @@ def test_remove_avatar(client): assert not any(list(map(os.path.exists, original_photo_paths))) +############################## +## Contacts +############################## + def test_list_contacts_private_projects(client): project = f.ProjectFactory.create() user_1 = f.UserFactory.create() @@ -406,6 +446,10 @@ def test_list_contacts_public_projects(client): assert response_content[0]["id"] == user_2.id +############################## +## Mail permissions +############################## + def test_mail_permissions(client): user_1 = f.UserFactory.create(is_superuser=True) user_2 = f.UserFactory.create() @@ -445,6 +489,10 @@ def test_mail_permissions(client): assert "email" in response.data +############################## +## Watchers, Likes and Votes +############################## + def test_get_watched_list(): fav_user = f.UserFactory() viewer_user = f.UserFactory()