From 4c0991caa6a47cb84ecf7aa22763751dc753fb56 Mon Sep 17 00:00:00 2001 From: Mikel Larreategi Date: Thu, 21 Nov 2024 00:21:35 +0100 Subject: [PATCH] additional tests (#1840) * additional tests * changelog * return a 400 Bad request when trying to change the username to an existing one --- news/1840.internal | 2 + src/plone/restapi/services/users/update.py | 27 +++- .../restapi/tests/test_services_users.py | 136 ++++++++++++++++++ 3 files changed, 163 insertions(+), 2 deletions(-) create mode 100644 news/1840.internal diff --git a/news/1840.internal b/news/1840.internal new file mode 100644 index 000000000..468f80444 --- /dev/null +++ b/news/1840.internal @@ -0,0 +1,2 @@ +Additional tests to login name changes +[erral] diff --git a/src/plone/restapi/services/users/update.py b/src/plone/restapi/services/users/update.py index 3d7d3b724..e2fdb1960 100644 --- a/src/plone/restapi/services/users/update.py +++ b/src/plone/restapi/services/users/update.py @@ -107,7 +107,20 @@ def reply(self): if security.use_email_as_login and "email" in user_settings_to_update: value = user_settings_to_update["email"] pas = getToolByName(self.context, "acl_users") - pas.updateLoginName(user.getId(), value) + + try: + pas.updateLoginName(user.getId(), value) + except ValueError: + return self._error( + 400, + "Bad request", + _( + "Cannot update login name of user to '${new_email}'.", + mapping={ + "new_email": value, + }, + ), + ) roles = user_settings_to_update.get("roles", {}) if roles: @@ -149,7 +162,17 @@ def reply(self): if security.use_email_as_login and "email" in user_settings_to_update: value = user_settings_to_update["email"] - set_own_login_name(user, value) + try: + set_own_login_name(user, value) + except ValueError: + return self._error( + 400, + "Bad request", + _( + "Cannot update login name of user to '${new_email}'.", + mapping={"new_email": value}, + ), + ) else: if self._is_anonymous: diff --git a/src/plone/restapi/tests/test_services_users.py b/src/plone/restapi/tests/test_services_users.py index 3d8032076..2352f62af 100644 --- a/src/plone/restapi/tests/test_services_users.py +++ b/src/plone/restapi/tests/test_services_users.py @@ -1663,3 +1663,139 @@ def test_user_changes_email_when_login_with_email_and_uuid_userids(self): }, ) self.assertTrue(new_login_with_new_email_response.ok) + + def test_manager_changes_email_to_existing_when_login_with_email(self): + """test that when login with email is enabled and a manager tries to change a user's email + to a previously existing one + """ + # enable use_email_as_login + security_settings = getAdapter(self.portal, ISecuritySchema) + security_settings.use_email_as_login = True + transaction.commit() + + # Create user 1 + response = self.api_session.post( + "/@users", + json={ + "email": "howard.zinn@example.com", + "password": TEST_USER_PASSWORD, + }, + ) + self.assertTrue(response.ok) + userid = response.json()["id"] + + # Create user 2 + response = self.api_session.post( + "/@users", + json={ + "email": "second@example.com", + "password": TEST_USER_PASSWORD, + }, + ) + self.assertTrue(response.ok) + + transaction.commit() + + # Log in + anon_response = self.anon_api_session.post( + "/@login", + json={ + "login": "howard.zinn@example.com", + "password": TEST_USER_PASSWORD, + }, + ) + self.assertTrue(anon_response.ok) + + # try to change the email to an existing one, it should fail + email_change_response = self.api_session.patch( + f"/@users/{userid}", + json={ + "email": "second@example.com", + }, + ) + self.assertFalse(email_change_response.ok) + self.assertEqual(email_change_response.status_code, 400) + email_change_response_json = email_change_response.json() + self.assertEqual( + email_change_response_json.get("error", {}).get("message"), + "Cannot update login name of user to 'second@example.com'.", + ) + + # Email was not changed, so log in with the old one + new_login_with_old_email_response = self.anon_api_session.post( + "/@login", + json={ + "login": "howard.zinn@example.com", + "password": TEST_USER_PASSWORD, + }, + ) + self.assertTrue(new_login_with_old_email_response.ok) + + def test_user_changes_email_to_existing_one_when_login_with_email(self): + """test that when login with email is enabled and the user changes their email + they can log in with the new email + """ + # enable use_email_as_login + security_settings = getAdapter(self.portal, ISecuritySchema) + security_settings.use_email_as_login = True + transaction.commit() + + # Create user 1 + response = self.api_session.post( + "/@users", + json={ + "email": "howard.zinn@example.com", + "password": TEST_USER_PASSWORD, + }, + ) + self.assertTrue(response.ok) + userid = response.json()["id"] + + # Create user 2 + response = self.api_session.post( + "/@users", + json={ + "email": "second@example.com", + "password": TEST_USER_PASSWORD, + }, + ) + self.assertTrue(response.ok) + transaction.commit() + + # log in with email + anon_response = self.anon_api_session.post( + "/@login", + json={ + "login": "howard.zinn@example.com", + "password": TEST_USER_PASSWORD, + }, + ) + self.assertTrue(anon_response.ok) + auth_token = anon_response.json().get("token") + + user_api_session = RelativeSession(self.portal_url, test=self) + user_api_session.headers.update({"Accept": "application/json"}) + user_api_session.headers.update({"Authorization": f"Bearer {auth_token}"}) + + # try to change e-mail to an existing one, it should fail + email_change_response = user_api_session.patch( + f"/@users/{userid}", + json={"email": "second@example.com"}, + ) + + self.assertEqual(email_change_response.status_code, 400) + email_change_response_json = email_change_response.json() + self.assertEqual( + email_change_response_json.get("error", {}).get("message"), + "Cannot update login name of user to 'second@example.com'.", + ) + + # email was not changed, so log in with the old one + new_login_with_old_email_response = self.anon_api_session.post( + "/@login", + json={ + "login": "howard.zinn@example.com", + "password": TEST_USER_PASSWORD, + }, + ) + self.assertTrue(new_login_with_old_email_response.ok)