Implements admin API to lock an user (MSC3939) (#15870)

This commit is contained in:
Mathieu Velten 2023-08-10 11:10:55 +02:00 committed by GitHub
parent 0328b56468
commit dac97642e4
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
19 changed files with 262 additions and 11 deletions

View file

@ -0,0 +1 @@
Implements an admin API to lock an user without deactivating them. Based on [MSC3939](https://github.com/matrix-org/matrix-spec-proposals/pull/3939).

View file

@ -146,6 +146,7 @@ Body parameters:
- `admin` - **bool**, optional, defaults to `false`. Whether the user is a homeserver administrator,
granting them access to the Admin API, among other things.
- `deactivated` - **bool**, optional. If unspecified, deactivation state will be left unchanged.
- `locked` - **bool**, optional. If unspecified, locked state will be left unchanged.
Note: the `password` field must also be set if both of the following are true:
- `deactivated` is set to `false` and the user was previously deactivated (you are reactivating this user)

View file

@ -3631,6 +3631,7 @@ This option has the following sub-options:
* `prefer_local_users`: Defines whether to prefer local users in search query results.
If set to true, local users are more likely to appear above remote users when searching the
user directory. Defaults to false.
* `show_locked_users`: Defines whether to show locked users in search query results. Defaults to false.
Example configuration:
```yaml
@ -3638,6 +3639,7 @@ user_directory:
enabled: false
search_all_users: true
prefer_local_users: true
show_locked_users: true
```
---
### `user_consent`

View file

@ -123,7 +123,7 @@ BOOLEAN_COLUMNS = {
"redactions": ["have_censored"],
"room_stats_state": ["is_federatable"],
"rooms": ["is_public", "has_auth_chain_index"],
"users": ["shadow_banned", "approved"],
"users": ["shadow_banned", "approved", "locked"],
"un_partial_stated_event_stream": ["rejection_status_changed"],
"users_who_share_rooms": ["share_private"],
"per_user_experimental_features": ["enabled"],

View file

@ -60,6 +60,7 @@ class Auth(Protocol):
request: SynapseRequest,
allow_guest: bool = False,
allow_expired: bool = False,
allow_locked: bool = False,
) -> Requester:
"""Get a registered user's ID.

View file

@ -58,6 +58,7 @@ class InternalAuth(BaseAuth):
request: SynapseRequest,
allow_guest: bool = False,
allow_expired: bool = False,
allow_locked: bool = False,
) -> Requester:
"""Get a registered user's ID.
@ -79,7 +80,7 @@ class InternalAuth(BaseAuth):
parent_span = active_span()
with start_active_span("get_user_by_req"):
requester = await self._wrapped_get_user_by_req(
request, allow_guest, allow_expired
request, allow_guest, allow_expired, allow_locked
)
if parent_span:
@ -107,6 +108,7 @@ class InternalAuth(BaseAuth):
request: SynapseRequest,
allow_guest: bool,
allow_expired: bool,
allow_locked: bool,
) -> Requester:
"""Helper for get_user_by_req
@ -126,6 +128,17 @@ class InternalAuth(BaseAuth):
access_token, allow_expired=allow_expired
)
# Deny the request if the user account is locked.
if not allow_locked and await self.store.get_user_locked_status(
requester.user.to_string()
):
raise AuthError(
401,
"User account has been locked",
errcode=Codes.USER_LOCKED,
additional_fields={"soft_logout": True},
)
# Deny the request if the user account has expired.
# This check is only done for regular users, not appservice ones.
if not allow_expired:

View file

@ -27,6 +27,7 @@ from twisted.web.http_headers import Headers
from synapse.api.auth.base import BaseAuth
from synapse.api.errors import (
AuthError,
Codes,
HttpResponseException,
InvalidClientTokenError,
OAuthInsufficientScopeError,
@ -196,6 +197,7 @@ class MSC3861DelegatedAuth(BaseAuth):
request: SynapseRequest,
allow_guest: bool = False,
allow_expired: bool = False,
allow_locked: bool = False,
) -> Requester:
access_token = self.get_access_token_from_request(request)
@ -205,6 +207,17 @@ class MSC3861DelegatedAuth(BaseAuth):
# so that we don't provision the user if they don't have enough permission:
requester = await self.get_user_by_access_token(access_token, allow_expired)
# Deny the request if the user account is locked.
if not allow_locked and await self.store.get_user_locked_status(
requester.user.to_string()
):
raise AuthError(
401,
"User account has been locked",
errcode=Codes.USER_LOCKED,
additional_fields={"soft_logout": True},
)
if not allow_guest and requester.is_guest:
raise OAuthInsufficientScopeError([SCOPE_MATRIX_API])

View file

@ -80,6 +80,8 @@ class Codes(str, Enum):
WEAK_PASSWORD = "M_WEAK_PASSWORD"
INVALID_SIGNATURE = "M_INVALID_SIGNATURE"
USER_DEACTIVATED = "M_USER_DEACTIVATED"
# USER_LOCKED = "M_USER_LOCKED"
USER_LOCKED = "ORG_MATRIX_MSC3939_USER_LOCKED"
# Part of MSC3848
# https://github.com/matrix-org/matrix-spec-proposals/pull/3848

View file

@ -35,3 +35,4 @@ class UserDirectoryConfig(Config):
self.user_directory_search_prefer_local_users = user_directory_config.get(
"prefer_local_users", False
)
self.show_locked_users = user_directory_config.get("show_locked_users", False)

View file

@ -67,6 +67,7 @@ class AdminHandler:
"name",
"admin",
"deactivated",
"locked",
"shadow_banned",
"creation_ts",
"appservice_id",

View file

@ -94,6 +94,7 @@ class UserDirectoryHandler(StateDeltasHandler):
self.is_mine_id = hs.is_mine_id
self.update_user_directory = hs.config.worker.should_update_user_directory
self.search_all_users = hs.config.userdirectory.user_directory_search_all_users
self.show_locked_users = hs.config.userdirectory.show_locked_users
self._spam_checker_module_callbacks = hs.get_module_api_callbacks().spam_checker
self._hs = hs
@ -144,7 +145,9 @@ class UserDirectoryHandler(StateDeltasHandler):
]
}
"""
results = await self.store.search_user_dir(user_id, search_term, limit)
results = await self.store.search_user_dir(
user_id, search_term, limit, self.show_locked_users
)
# Remove any spammy users from the results.
non_spammy_users = []

View file

@ -280,6 +280,17 @@ class UserRestServletV2(RestServlet):
HTTPStatus.BAD_REQUEST, "'deactivated' parameter is not of type boolean"
)
lock = body.get("locked", False)
if not isinstance(lock, bool):
raise SynapseError(
HTTPStatus.BAD_REQUEST, "'locked' parameter is not of type boolean"
)
if deactivate and lock:
raise SynapseError(
HTTPStatus.BAD_REQUEST, "An user can't be deactivated and locked"
)
approved: Optional[bool] = None
if "approved" in body and self._msc3866_enabled:
approved = body["approved"]
@ -397,6 +408,12 @@ class UserRestServletV2(RestServlet):
target_user.to_string()
)
if "locked" in body:
if lock and not user["locked"]:
await self.store.set_user_locked_status(user_id, True)
elif not lock and user["locked"]:
await self.store.set_user_locked_status(user_id, False)
if "user_type" in body:
await self.store.set_user_type(target_user, user_type)

View file

@ -40,7 +40,9 @@ class LogoutRestServlet(RestServlet):
self._device_handler = handler
async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request, allow_expired=True)
requester = await self.auth.get_user_by_req(
request, allow_expired=True, allow_locked=True
)
if requester.device_id is None:
# The access token wasn't associated with a device.
@ -67,7 +69,9 @@ class LogoutAllRestServlet(RestServlet):
self._device_handler = handler
async def on_POST(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
requester = await self.auth.get_user_by_req(request, allow_expired=True)
requester = await self.auth.get_user_by_req(
request, allow_expired=True, allow_locked=True
)
user_id = requester.user.to_string()
# first delete all of the user's devices

View file

@ -205,7 +205,8 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore):
name, password_hash, is_guest, admin, consent_version, consent_ts,
consent_server_notice_sent, appservice_id, creation_ts, user_type,
deactivated, COALESCE(shadow_banned, FALSE) AS shadow_banned,
COALESCE(approved, TRUE) AS approved
COALESCE(approved, TRUE) AS approved,
COALESCE(locked, FALSE) AS locked
FROM users
WHERE name = ?
""",
@ -230,10 +231,15 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore):
# want to make sure we're returning the right type of data.
# Note: when adding a column name to this list, be wary of NULLable columns,
# since NULL values will be turned into False.
boolean_columns = ["admin", "deactivated", "shadow_banned", "approved"]
boolean_columns = [
"admin",
"deactivated",
"shadow_banned",
"approved",
"locked",
]
for column in boolean_columns:
if not isinstance(row[column], bool):
row[column] = bool(row[column])
row[column] = bool(row[column])
return row
@ -1116,6 +1122,27 @@ class RegistrationWorkerStore(CacheInvalidationWorkerStore):
# Convert the integer into a boolean.
return res == 1
@cached()
async def get_user_locked_status(self, user_id: str) -> bool:
"""Retrieve the value for the `locked` property for the provided user.
Args:
user_id: The ID of the user to retrieve the status for.
Returns:
True if the user was locked, false if the user is still active.
"""
res = await self.db_pool.simple_select_one_onecol(
table="users",
keyvalues={"name": user_id},
retcol="locked",
desc="get_user_locked_status",
)
# Convert the potential integer into a boolean.
return bool(res)
async def get_threepid_validation_session(
self,
medium: Optional[str],
@ -2111,6 +2138,33 @@ class RegistrationBackgroundUpdateStore(RegistrationWorkerStore):
self._invalidate_cache_and_stream(txn, self.get_user_by_id, (user_id,))
txn.call_after(self.is_guest.invalidate, (user_id,))
async def set_user_locked_status(self, user_id: str, locked: bool) -> None:
"""Set the `locked` property for the provided user to the provided value.
Args:
user_id: The ID of the user to set the status for.
locked: The value to set for `locked`.
"""
await self.db_pool.runInteraction(
"set_user_locked_status",
self.set_user_locked_status_txn,
user_id,
locked,
)
def set_user_locked_status_txn(
self, txn: LoggingTransaction, user_id: str, locked: bool
) -> None:
self.db_pool.simple_update_one_txn(
txn=txn,
table="users",
keyvalues={"name": user_id},
updatevalues={"locked": locked},
)
self._invalidate_cache_and_stream(txn, self.get_user_locked_status, (user_id,))
self._invalidate_cache_and_stream(txn, self.get_user_by_id, (user_id,))
def update_user_approval_status_txn(
self, txn: LoggingTransaction, user_id: str, approved: bool
) -> None:

View file

@ -995,7 +995,11 @@ class UserDirectoryStore(UserDirectoryBackgroundUpdateStore):
)
async def search_user_dir(
self, user_id: str, search_term: str, limit: int
self,
user_id: str,
search_term: str,
limit: int,
show_locked_users: bool = False,
) -> SearchResult:
"""Searches for users in directory
@ -1029,6 +1033,9 @@ class UserDirectoryStore(UserDirectoryBackgroundUpdateStore):
)
"""
if not show_locked_users:
where_clause += " AND (u.locked IS NULL OR u.locked = FALSE)"
# We allow manipulating the ranking algorithm by injecting statements
# based on config options.
additional_ordering_statements = []
@ -1060,6 +1067,7 @@ class UserDirectoryStore(UserDirectoryBackgroundUpdateStore):
SELECT d.user_id AS user_id, display_name, avatar_url
FROM matching_users as t
INNER JOIN user_directory AS d USING (user_id)
LEFT JOIN users AS u ON t.user_id = u.name
WHERE
%(where_clause)s
ORDER BY
@ -1115,6 +1123,7 @@ class UserDirectoryStore(UserDirectoryBackgroundUpdateStore):
SELECT d.user_id AS user_id, display_name, avatar_url
FROM user_directory_search as t
INNER JOIN user_directory AS d USING (user_id)
LEFT JOIN users AS u ON t.user_id = u.name
WHERE
%(where_clause)s
AND value MATCH ?

View file

@ -0,0 +1,16 @@
/* Copyright 2023 The Matrix.org Foundation C.I.C.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
ALTER TABLE users ADD locked BOOLEAN DEFAULT FALSE NOT NULL;

View file

@ -69,6 +69,7 @@ class AuthTestCase(unittest.HomeserverTestCase):
)
self.store.get_user_by_access_token = simple_async_mock(user_info)
self.store.mark_access_token_as_used = simple_async_mock(None)
self.store.get_user_locked_status = simple_async_mock(False)
request = Mock(args={})
request.args[b"access_token"] = [self.test_token]
@ -293,6 +294,7 @@ class AuthTestCase(unittest.HomeserverTestCase):
)
self.store.insert_client_ip = simple_async_mock(None)
self.store.mark_access_token_as_used = simple_async_mock(None)
self.store.get_user_locked_status = simple_async_mock(False)
request = Mock(args={})
request.getClientAddress.return_value.host = "127.0.0.1"
request.args[b"access_token"] = [self.test_token]
@ -311,6 +313,7 @@ class AuthTestCase(unittest.HomeserverTestCase):
token_used=True,
)
)
self.store.get_user_locked_status = simple_async_mock(False)
self.store.insert_client_ip = simple_async_mock(None)
self.store.mark_access_token_as_used = simple_async_mock(None)
request = Mock(args={})

View file

@ -29,7 +29,16 @@ from synapse.api.constants import ApprovalNoticeMedium, LoginType, UserTypes
from synapse.api.errors import Codes, HttpResponseException, ResourceLimitError
from synapse.api.room_versions import RoomVersions
from synapse.media.filepath import MediaFilePaths
from synapse.rest.client import devices, login, logout, profile, register, room, sync
from synapse.rest.client import (
devices,
login,
logout,
profile,
register,
room,
sync,
user_directory,
)
from synapse.server import HomeServer
from synapse.types import JsonDict, UserID, create_requester
from synapse.util import Clock
@ -1477,6 +1486,7 @@ class UserRestTestCase(unittest.HomeserverTestCase):
login.register_servlets,
sync.register_servlets,
register.register_servlets,
user_directory.register_servlets,
]
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
@ -2464,6 +2474,105 @@ class UserRestTestCase(unittest.HomeserverTestCase):
# This key was removed intentionally. Ensure it is not accidentally re-included.
self.assertNotIn("password_hash", channel.json_body)
def test_locked_user(self) -> None:
# User can sync
channel = self.make_request(
"GET",
"/_matrix/client/v3/sync",
access_token=self.other_user_token,
)
self.assertEqual(200, channel.code, msg=channel.json_body)
# Lock user
channel = self.make_request(
"PUT",
self.url_other_user,
access_token=self.admin_user_tok,
content={"locked": True},
)
# User is not authorized to sync anymore
channel = self.make_request(
"GET",
"/_matrix/client/v3/sync",
access_token=self.other_user_token,
)
self.assertEqual(401, channel.code, msg=channel.json_body)
self.assertEqual(Codes.USER_LOCKED, channel.json_body["errcode"])
self.assertTrue(channel.json_body["soft_logout"])
@override_config({"user_directory": {"enabled": True, "search_all_users": True}})
def test_locked_user_not_in_user_dir(self) -> None:
# User is available in the user dir
channel = self.make_request(
"POST",
"/_matrix/client/v3/user_directory/search",
{"search_term": self.other_user},
access_token=self.admin_user_tok,
)
self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertIn("results", channel.json_body)
self.assertEqual(1, len(channel.json_body["results"]))
# Lock user
channel = self.make_request(
"PUT",
self.url_other_user,
access_token=self.admin_user_tok,
content={"locked": True},
)
# User is not available anymore in the user dir
channel = self.make_request(
"POST",
"/_matrix/client/v3/user_directory/search",
{"search_term": self.other_user},
access_token=self.admin_user_tok,
)
self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertIn("results", channel.json_body)
self.assertEqual(0, len(channel.json_body["results"]))
@override_config(
{
"user_directory": {
"enabled": True,
"search_all_users": True,
"show_locked_users": True,
}
}
)
def test_locked_user_in_user_dir_with_show_locked_users_option(self) -> None:
# User is available in the user dir
channel = self.make_request(
"POST",
"/_matrix/client/v3/user_directory/search",
{"search_term": self.other_user},
access_token=self.admin_user_tok,
)
self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertIn("results", channel.json_body)
self.assertEqual(1, len(channel.json_body["results"]))
# Lock user
channel = self.make_request(
"PUT",
self.url_other_user,
access_token=self.admin_user_tok,
content={"locked": True},
)
# User is still available in the user dir
channel = self.make_request(
"POST",
"/_matrix/client/v3/user_directory/search",
{"search_term": self.other_user},
access_token=self.admin_user_tok,
)
self.assertEqual(200, channel.code, msg=channel.json_body)
self.assertIn("results", channel.json_body)
self.assertEqual(1, len(channel.json_body["results"]))
@override_config({"user_directory": {"enabled": True, "search_all_users": True}})
def test_change_name_deactivate_user_user_directory(self) -> None:
"""

View file

@ -48,6 +48,7 @@ class RegistrationStoreTestCase(HomeserverTestCase):
"creation_ts": 0,
"user_type": None,
"deactivated": 0,
"locked": 0,
"shadow_banned": 0,
"approved": 1,
},