kaslo/synapse
1
0
Fork 0
Code Wiki Activity

Merge commit 'c9c1c9d82' into anoa/dinsic_release_1_31_0

Browse Source
This commit is contained in:
Andrew Morgan
2021-04-20 16:59:59 +01:00
parent de2918f0cb c9c1c9d82f
commit ed6742cb30
14 changed files with 255 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/8931.feature Normal file
View File

@@ -0,0 +1 @@
Make search statement in List Room and List User Admin API case-insensitive.

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.

1
changelog.d/8959.bugfix Normal file
View File

@@ -0,0 +1 @@
Fix a bug causing common English words to not be considered for a user directory search.

1
changelog.d/8964.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.

9
docs/admin_api/user_admin_api.rst
View File

@@ -30,7 +30,12 @@ It returns a JSON body like the following:
],
"avatar_url": "<avatar_url>",
"admin": false,
"deactivated": false
"deactivated": false,
"password_hash": "$2b$12$p9B4GkqYdRTPGD",
"creation_ts": 1560432506,
"appservice_id": null,
"consent_server_notice_sent": null,
"consent_version": null
}
URL parameters:
@@ -139,7 +144,6 @@ A JSON body is returned with the following shape:
"users": [
{
"name": "<user_id1>",
"password_hash": "<password_hash1>",
"is_guest": 0,
"admin": 0,
"user_type": null,
@@ -148,7 +152,6 @@ A JSON body is returned with the following shape:
"avatar_url": null
}, {
"name": "<user_id2>",
"password_hash": "<password_hash2>",
"is_guest": 0,
"admin": 1,
"user_type": null,

8
synapse/handlers/user_directory.py
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
)

7
synapse/storage/databases/main/__init__.py
View File

@@ -339,12 +339,13 @@ class DataStore(
filters = []
args = [self.hs.config.server_name]
# `name` is in database already in lower case
if name:
filters.append("(name LIKE ? OR displayname LIKE ?)")
args.extend(["@%" + name + "%:%", "%" + name + "%"])
filters.append("(name LIKE ? OR LOWER(displayname) LIKE ?)")
args.extend(["@%" + name.lower() + "%:%", "%" + name.lower() + "%"])
elif user_id:
filters.append("name LIKE ?")
args.extend(["%" + user_id + "%"])
args.extend(["%" + user_id.lower() + "%"])
if not guests:
filters.append("is_guest = 0")

4
synapse/storage/databases/main/room.py
View File

@@ -399,14 +399,14 @@ class RoomWorkerStore(SQLBaseStore):
# Filter room names by a string
where_statement = ""
if search_term:
where_statement = "WHERE state.name LIKE ?"
where_statement = "WHERE LOWER(state.name) LIKE ?"
# Our postgres db driver converts ? -> %s in SQL strings as that's the
# placeholder for postgres.
# HOWEVER, if you put a % into your SQL then everything goes wibbly.
# To get around this, we're going to surround search_term with %'s
# before giving it to the database in python instead
search_term = "%" + search_term + "%"
search_term = "%" + search_term.lower() + "%"
# Set ordering
if RoomSortOrder(order_by) == RoomSortOrder.SIZE:

24
synapse/storage/databases/main/user_directory.py
View File

@@ -396,9 +396,9 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
sql = """
INSERT INTO user_directory_search(user_id, vector)
VALUES (?,
setweight(to_tsvector('english', ?), 'A')
|| setweight(to_tsvector('english', ?), 'D')
|| setweight(to_tsvector('english', COALESCE(?, '')), 'B')
setweight(to_tsvector('simple', ?), 'A')
|| setweight(to_tsvector('simple', ?), 'D')
|| setweight(to_tsvector('simple', COALESCE(?, '')), 'B')
) ON CONFLICT (user_id) DO UPDATE SET vector=EXCLUDED.vector
"""
txn.execute(
@@ -418,9 +418,9 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
sql = """
INSERT INTO user_directory_search(user_id, vector)
VALUES (?,
setweight(to_tsvector('english', ?), 'A')
|| setweight(to_tsvector('english', ?), 'D')
|| setweight(to_tsvector('english', COALESCE(?, '')), 'B')
setweight(to_tsvector('simple', ?), 'A')
|| setweight(to_tsvector('simple', ?), 'D')
|| setweight(to_tsvector('simple', COALESCE(?, '')), 'B')
)
"""
txn.execute(
@@ -435,9 +435,9 @@ class UserDirectoryBackgroundUpdateStore(StateDeltasStore):
elif new_entry is False:
sql = """
UPDATE user_directory_search
SET vector = setweight(to_tsvector('english', ?), 'A')
|| setweight(to_tsvector('english', ?), 'D')
|| setweight(to_tsvector('english', COALESCE(?, '')), 'B')
SET vector = setweight(to_tsvector('simple', ?), 'A')
|| setweight(to_tsvector('simple', ?), 'D')
|| setweight(to_tsvector('simple', COALESCE(?, '')), 'B')
WHERE user_id = ?
"""
txn.execute(
@@ -784,7 +784,7 @@ class UserDirectoryStore(UserDirectoryBackgroundUpdateStore):
INNER JOIN user_directory AS d USING (user_id)
WHERE
%(where_clause)s
AND vector @@ to_tsquery('english', ?)
AND vector @@ to_tsquery('simple', ?)
ORDER BY
(CASE WHEN d.user_id IS NOT NULL THEN 4.0 ELSE 1.0 END)
* (CASE WHEN display_name IS NOT NULL THEN 1.2 ELSE 1.0 END)
@@ -793,13 +793,13 @@ class UserDirectoryStore(UserDirectoryBackgroundUpdateStore):
3 * ts_rank_cd(
'{0.1, 0.1, 0.9, 1.0}',
vector,
to_tsquery('english', ?),
to_tsquery('simple', ?),
8
)
+ ts_rank_cd(
'{0.1, 0.1, 0.9, 1.0}',
vector,
to_tsquery('english', ?),
to_tsquery('simple', ?),
8
)
)

40
tests/handlers/test_user_directory.py
View File

@@ -57,6 +57,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)
@@ -66,13 +70,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(

7
tests/rest/admin/test_room.py
View File

@@ -1050,6 +1050,13 @@ class RoomTestCase(unittest.HomeserverTestCase):
_search_test(room_id_2, "else")
_search_test(room_id_2, "se")
# Test case insensitive
_search_test(room_id_1, "SOMETHING")
_search_test(room_id_1, "THING")
_search_test(room_id_2, "ELSE")
_search_test(room_id_2, "SE")
_search_test(None, "foo")
_search_test(None, "bar")
_search_test(None, "", expected_http_code=400)

149
tests/rest/admin/test_user.py
View File

@@ -18,6 +18,7 @@ import hmac
import json
import urllib.parse
from binascii import unhexlify
from typing import Optional
from mock import Mock
@@ -466,8 +467,12 @@ class UsersListTestCase(unittest.HomeserverTestCase):
self.admin_user = self.register_user("admin", "pass", admin=True)
self.admin_user_tok = self.login("admin", "pass")
self.register_user("user1", "pass1", admin=False)
self.register_user("user2", "pass2", admin=False)
self.user1 = self.register_user(
"user1", "pass1", admin=False, displayname="Name 1"
)
self.user2 = self.register_user(
"user2", "pass2", admin=False, displayname="Name 2"
)
def test_no_auth(self):
"""
@@ -476,7 +481,18 @@ class UsersListTestCase(unittest.HomeserverTestCase):
channel = self.make_request("GET", self.url, b"{}")
self.assertEqual(401, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual("M_MISSING_TOKEN", channel.json_body["errcode"])
self.assertEqual(Codes.MISSING_TOKEN, channel.json_body["errcode"])
def test_requester_is_no_admin(self):
"""
If the user is not a server admin, an error is returned.
"""
other_user_token = self.login("user1", "pass1")
channel = self.make_request("GET", self.url, access_token=other_user_token)
self.assertEqual(403, int(channel.result["code"]), msg=channel.result["body"])
self.assertEqual(Codes.FORBIDDEN, channel.json_body["errcode"])
def test_all_users(self):
"""
@@ -493,6 +509,83 @@ class UsersListTestCase(unittest.HomeserverTestCase):
self.assertEqual(3, len(channel.json_body["users"]))
self.assertEqual(3, channel.json_body["total"])
# Check that all fields are available
for u in channel.json_body["users"]:
self.assertIn("name", u)
self.assertIn("is_guest", u)
self.assertIn("admin", u)
self.assertIn("user_type", u)
self.assertIn("deactivated", u)
self.assertIn("displayname", u)
self.assertIn("avatar_url", u)
def test_search_term(self):
"""Test that searching for a users works correctly"""
def _search_test(
expected_user_id: Optional[str],
search_term: str,
search_field: Optional[str] = "name",
expected_http_code: Optional[int] = 200,
):
"""Search for a user and check that the returned user's id is a match
Args:
expected_user_id: The user_id expected to be returned by the API. Set
to None to expect zero results for the search
search_term: The term to search for user names with
search_field: Field which is to request: `name` or `user_id`
expected_http_code: The expected http code for the request
"""
url = self.url + "?%s=%s" % (search_field, search_term,)
channel = self.make_request(
"GET", url.encode("ascii"), access_token=self.admin_user_tok,
)
self.assertEqual(expected_http_code, channel.code, msg=channel.json_body)
if expected_http_code != 200:
return
# Check that users were returned
self.assertTrue("users" in channel.json_body)
users = channel.json_body["users"]
# Check that the expected number of users were returned
expected_user_count = 1 if expected_user_id else 0
self.assertEqual(len(users), expected_user_count)
self.assertEqual(channel.json_body["total"], expected_user_count)
if expected_user_id:
# Check that the first returned user id is correct
u = users[0]
self.assertEqual(expected_user_id, u["name"])
# Perform search tests
_search_test(self.user1, "er1")
_search_test(self.user1, "me 1")
_search_test(self.user2, "er2")
_search_test(self.user2, "me 2")
_search_test(self.user1, "er1", "user_id")
_search_test(self.user2, "er2", "user_id")
# Test case insensitive
_search_test(self.user1, "ER1")
_search_test(self.user1, "NAME 1")
_search_test(self.user2, "ER2")
_search_test(self.user2, "NAME 2")
_search_test(self.user1, "ER1", "user_id")
_search_test(self.user2, "ER2", "user_id")
_search_test(None, "foo")
_search_test(None, "bar")
_search_test(None, "foo", "user_id")
_search_test(None, "bar", "user_id")
class UserRestTestCase(unittest.HomeserverTestCase):
@@ -508,7 +601,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
@@ -917,6 +1010,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})
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"})
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.

7
tests/storage/test_main.py
View File

@@ -48,3 +48,10 @@ class DataStoreTestCase(unittest.TestCase):
self.assertEquals(1, total)
self.assertEquals(self.displayname, users.pop()["displayname"])
users, total = yield defer.ensureDeferred(
self.store.get_users_paginate(0, 10, name="BC", guests=False)
)
self.assertEquals(1, total)
self.assertEquals(self.displayname, users.pop()["displayname"])

23
tests/storage/test_user_directory.py
View File

@@ -21,6 +21,8 @@ from tests.utils import setup_test_homeserver
ALICE = "@alice:a"
BOB = "@bob:b"
BOBBY = "@bobby:a"
# The localpart isn't 'Bela' on purpose so we can test looking up display names.
BELA = "@somenickname:a"
class UserDirectoryStoreTestCase(unittest.TestCase):
@@ -40,6 +42,9 @@ class UserDirectoryStoreTestCase(unittest.TestCase):
yield defer.ensureDeferred(
self.store.update_profile_in_user_dir(BOBBY, "bobby", None)
)
yield defer.ensureDeferred(
self.store.update_profile_in_user_dir(BELA, "Bela", None)
)
yield defer.ensureDeferred(
self.store.add_users_in_public_rooms("!room:id", (ALICE, BOB))
)
@@ -72,3 +77,21 @@ class UserDirectoryStoreTestCase(unittest.TestCase):
)
finally:
self.hs.config.user_directory_search_all_users = False
@defer.inlineCallbacks
def test_search_user_dir_stop_words(self):
"""Tests that a user can look up another user by searching for the start if its
display name even if that name happens to be a common English word that would
usually be ignored in full text searches.
"""
self.hs.config.user_directory_search_all_users = True
try:
r = yield defer.ensureDeferred(self.store.search_user_dir(ALICE, "be", 10))
self.assertFalse(r["limited"])
self.assertEqual(1, len(r["results"]))
self.assertDictEqual(
r["results"][0],
{"user_id": BELA, "display_name": "Bela", "avatar_url": None},
)
finally:
self.hs.config.user_directory_search_all_users = False