diff options
| -rw-r--r-- | changelog.d/8933.bugfix | 1 | ||||
| -rw-r--r-- | synapse/handlers/user_directory.py | 8 | ||||
| -rw-r--r-- | tests/handlers/test_user_directory.py | 40 | ||||
| -rw-r--r-- | tests/rest/admin/test_user.py | 50 |
4 files changed, 95 insertions, 4 deletions
diff --git a/changelog.d/8933.bugfix b/changelog.d/8933.bugfix new file mode 100644 index 0000000000..295933d6cd --- /dev/null +++ b/changelog.d/8933.bugfix @@ -0,0 +1 @@ +Fix a bug where deactivated users appeared in the user directory when their profile information was updated. diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index 3d80371f06..7c4eeaaa5e 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -113,9 +113,13 @@ class UserDirectoryHandler(StateDeltasHandler): """ # FIXME(#3714): We should probably do this in the same worker as all # the other changes. - is_support = await self.store.is_support_user(user_id) + # Support users are for diagnostics and should not appear in the user directory. - if not is_support: + is_support = await self.store.is_support_user(user_id) + # When change profile information of deactivated user it should not appear in the user directory. + is_deactivated = await self.store.get_user_deactivated_status(user_id) + + if not (is_support or is_deactivated): await self.store.update_profile_in_user_dir( user_id, profile.display_name, profile.avatar_url ) diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py index 1260721dbf..9c886d671a 100644 --- a/tests/handlers/test_user_directory.py +++ b/tests/handlers/test_user_directory.py @@ -54,6 +54,10 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): user_id=support_user_id, password_hash=None, user_type=UserTypes.SUPPORT ) ) + regular_user_id = "@regular:test" + self.get_success( + self.store.register_user(user_id=regular_user_id, password_hash=None) + ) self.get_success( self.handler.handle_local_profile_change(support_user_id, None) @@ -63,13 +67,47 @@ class UserDirectoryTestCase(unittest.HomeserverTestCase): display_name = "display_name" profile_info = ProfileInfo(avatar_url="avatar_url", display_name=display_name) - regular_user_id = "@regular:test" self.get_success( self.handler.handle_local_profile_change(regular_user_id, profile_info) ) profile = self.get_success(self.store.get_user_in_directory(regular_user_id)) self.assertTrue(profile["display_name"] == display_name) + def test_handle_local_profile_change_with_deactivated_user(self): + # create user + r_user_id = "@regular:test" + self.get_success( + self.store.register_user(user_id=r_user_id, password_hash=None) + ) + + # update profile + display_name = "Regular User" + profile_info = ProfileInfo(avatar_url="avatar_url", display_name=display_name) + self.get_success( + self.handler.handle_local_profile_change(r_user_id, profile_info) + ) + + # profile is in directory + profile = self.get_success(self.store.get_user_in_directory(r_user_id)) + self.assertTrue(profile["display_name"] == display_name) + + # deactivate user + self.get_success(self.store.set_user_deactivated_status(r_user_id, True)) + self.get_success(self.handler.handle_user_deactivated(r_user_id)) + + # profile is not in directory + profile = self.get_success(self.store.get_user_in_directory(r_user_id)) + self.assertTrue(profile is None) + + # update profile after deactivation + self.get_success( + self.handler.handle_local_profile_change(r_user_id, profile_info) + ) + + # profile is furthermore not in directory + profile = self.get_success(self.store.get_user_in_directory(r_user_id)) + self.assertTrue(profile is None) + def test_handle_user_deactivated_support_user(self): s_user_id = "@support:test" self.get_success( diff --git a/tests/rest/admin/test_user.py b/tests/rest/admin/test_user.py index 4f379a5e55..9d6ef02511 100644 --- a/tests/rest/admin/test_user.py +++ b/tests/rest/admin/test_user.py @@ -603,7 +603,7 @@ class UserRestTestCase(unittest.HomeserverTestCase): self.admin_user = self.register_user("admin", "pass", admin=True) self.admin_user_tok = self.login("admin", "pass") - self.other_user = self.register_user("user", "pass") + self.other_user = self.register_user("user", "pass", displayname="User") self.other_user_token = self.login("user", "pass") self.url_other_user = "/_synapse/admin/v2/users/%s" % urllib.parse.quote( self.other_user @@ -1012,6 +1012,54 @@ class UserRestTestCase(unittest.HomeserverTestCase): self.assertEqual("@user:test", channel.json_body["name"]) self.assertEqual(True, channel.json_body["deactivated"]) + @override_config({"user_directory": {"enabled": True, "search_all_users": True}}) + def test_change_name_deactivate_user_user_directory(self): + """ + Test change profile information of a deactivated user and + check that it does not appear in user directory + """ + + # is in user directory + profile = self.get_success(self.store.get_user_in_directory(self.other_user)) + self.assertTrue(profile["display_name"] == "User") + + # Deactivate user + body = json.dumps({"deactivated": True}) + + request, channel = self.make_request( + "PUT", + self.url_other_user, + access_token=self.admin_user_tok, + content=body.encode(encoding="utf_8"), + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("@user:test", channel.json_body["name"]) + self.assertEqual(True, channel.json_body["deactivated"]) + + # is not in user directory + profile = self.get_success(self.store.get_user_in_directory(self.other_user)) + self.assertTrue(profile is None) + + # Set new displayname user + body = json.dumps({"displayname": "Foobar"}) + + request, channel = self.make_request( + "PUT", + self.url_other_user, + access_token=self.admin_user_tok, + content=body.encode(encoding="utf_8"), + ) + + self.assertEqual(200, int(channel.result["code"]), msg=channel.result["body"]) + self.assertEqual("@user:test", channel.json_body["name"]) + self.assertEqual(True, channel.json_body["deactivated"]) + self.assertEqual("Foobar", channel.json_body["displayname"]) + + # is not in user directory + profile = self.get_success(self.store.get_user_in_directory(self.other_user)) + self.assertTrue(profile is None) + def test_reactivate_user(self): """ Test reactivating another user. |