Fix a bug that deactivated users appear in the directory (#8933)

Fixes a bug that deactivated users appear in the directory when their profile information was updated.

To change profile information of deactivated users is neccesary for example you will remove displayname or avatar.
But they should not appear in directory. They are deactivated.



Co-authored-by: Erik Johnston <erikj@jki.re>
This commit is contained in:
Dirk Klimpel 2020-12-17 13:05:39 +01:00 committed by GitHub
parent 06006058d7
commit c07022303e
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 95 additions and 4 deletions

1
changelog.d/8933.bugfix Normal file
View file

@ -0,0 +1 @@
Fix a bug where deactivated users appeared in the user directory when their profile information was updated.

View file

@ -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
)

View file

@ -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(

View file

@ -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.