Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix reactivated users not being added to the user directory #10782

Merged
1 change: 1 addition & 0 deletions changelog.d/10762.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug where reactivated users would be missing from the user directory.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably just remove this one (but also be sure to remove it from the other branch).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure I follow you here (given that towncrier coalesces identical fragments). Is there a specific gain by tidying this up, or is it just nicer to have?

(Happy to clean up, but just anxious that it's further git-churn)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't seem useful is all, it would end up pointing people to two identical PRs, which seems confusing.

I don't feel strongly either way, however.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, looking at the review page properly, I see what you mean: this PR adds two changelog files!!! (I thought you were referring to the two different files on two different PRs)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 please remove the duplicate newsfile!

1 change: 1 addition & 0 deletions changelog.d/10782.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a bug where reactivated users would be missing from the user directory.
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
12 changes: 6 additions & 6 deletions synapse/handlers/deactivate_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -255,16 +255,16 @@ async def activate_account(self, user_id: str) -> None:
Args:
user_id: ID of user to be re-activated
"""
# Add the user to the directory, if necessary.
user = UserID.from_string(user_id)
if self.hs.config.user_directory_search_all_users:
profile = await self.store.get_profileinfo(user.localpart)
await self.user_directory_handler.handle_local_profile_change(
user_id, profile
)

# Ensure the user is not marked as erased.
await self.store.mark_user_not_erased(user_id)

# Mark the user as active.
await self.store.set_user_deactivated_status(user_id, False)

# Add the user to the directory, if necessary. Note that
# this must be done after the user is re-activated, because
# deactivated users are excluded from the user directory.
profile = await self.store.get_profileinfo(user.localpart)
await self.user_directory_handler.handle_local_profile_change(user_id, profile)
45 changes: 45 additions & 0 deletions tests/handlers/test_user_directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
from synapse.api.room_versions import RoomVersion, RoomVersions
from synapse.rest.client import login, room, user_directory
from synapse.storage.roommember import ProfileInfo
from synapse.types import create_requester

from tests import unittest
from tests.unittest import override_config
Expand Down Expand Up @@ -130,6 +131,50 @@ def test_handle_user_deactivated_regular_user(self):
self.get_success(self.handler.handle_user_deactivated(r_user_id))
self.store.remove_from_user_dir.called_once_with(r_user_id)

def test_reactivation_makes_regular_user_searchable(self):
user = self.register_user("regular", "pass")
password_hash = self.get_success(
self.store.db_pool.simple_select_one_onecol(
"users",
{"name": user},
"password_hash",
)
)
user_token = self.login(user, "pass")
admin_user = self.register_user("admin", "pass", admin=True)

# Ensure the regular user is publicly visible and searchable.
self.helper.create_room_as(user, is_public=True, tok=user_token)
s = self.get_success(self.handler.search_users(admin_user, user, 10))
self.assertEqual(len(s["results"]), 1)
self.assertEqual(s["results"][0]["user_id"], user)

# Deactivate the user and check they're not searchable.
deactivate_handler = self.hs._deactivate_account_handler
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved
self.get_success(
deactivate_handler.deactivate_account(
user, erase_data=False, requester=create_requester(admin_user)
)
)
s = self.get_success(self.handler.search_users(admin_user, user, 10))
self.assertEqual(s["results"], [])

# Reactivate the user
self.get_success(deactivate_handler.activate_account(user))
# Hackily reset password by restoring the old pw hash.
self.get_success(
self.hs.get_set_password_handler().set_password(
user, password_hash, logout_devices=False
)
)
user_token = self.login(user, "pass")
self.helper.create_room_as(user, is_public=True, tok=user_token)
DMRobertson marked this conversation as resolved.
Show resolved Hide resolved

# Check they're searchable.
s = self.get_success(self.handler.search_users(admin_user, user, 10))
self.assertEqual(len(s["results"]), 1)
self.assertEqual(s["results"][0]["user_id"], user)

def test_private_room(self):
"""
A user can be searched for only by people that are either in a public
Expand Down