diff options
| -rw-r--r-- | changelog.d/4141.feature | 1 | ||||
| -rw-r--r-- | synapse/api/auth.py | 5 | ||||
| -rw-r--r-- | synapse/api/constants.py | 6 | ||||
| -rw-r--r-- | synapse/handlers/register.py | 12 | ||||
| -rw-r--r-- | synapse/handlers/user_directory.py | 52 | ||||
| -rw-r--r-- | synapse/storage/monthly_active_users.py | 3 | ||||
| -rw-r--r-- | synapse/storage/prepare_database.py | 2 | ||||
| -rw-r--r-- | synapse/storage/registration.py | 23 | ||||
| -rw-r--r-- | synapse/storage/schema/delta/53/add_user_type_to_users.sql | 20 | ||||
| -rw-r--r-- | tests/api/test_auth.py | 2 | ||||
| -rw-r--r-- | tests/handlers/test_register.py | 24 | ||||
| -rw-r--r-- | tests/handlers/test_user_directory.py | 91 | ||||
| -rw-r--r-- | tests/storage/test_monthly_active_users.py | 20 | ||||
| -rw-r--r-- | tests/storage/test_registration.py | 22 | ||||
| -rw-r--r-- | tests/utils.py | 1 |
15 files changed, 246 insertions, 38 deletions
diff --git a/changelog.d/4141.feature b/changelog.d/4141.feature new file mode 100644 index 0000000000..34ec4115a7 --- /dev/null +++ b/changelog.d/4141.feature @@ -0,0 +1 @@ +Special case a support user for use in verifying user behaviour of a given server, the support user does not appear in user directory or monthly active user counts. diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 34382e4e3c..f3a9aeb934 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -791,9 +791,10 @@ class Auth(object): threepid should never be set at the same time. """ - # Never fail an auth check for the server notices users + # Never fail an auth check for the server notices users or support user # This can be a problem where event creation is prohibited due to blocking - if user_id == self.hs.config.server_notices_mxid: + is_support = yield self.store.is_support_user(user_id) + if user_id == self.hs.config.server_notices_mxid or is_support: return if self.hs.config.hs_disabled: diff --git a/synapse/api/constants.py b/synapse/api/constants.py index f20e0fcf0b..a9fb6e7362 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -119,3 +119,9 @@ KNOWN_ROOM_VERSIONS = { ServerNoticeMsgType = "m.server_notice" ServerNoticeLimitReached = "m.server_notice.usage_limit_reached" + + +# Allows for user type specific behaviour, if we'd had a crystal ball would +# probably have included admin and guest, normal users are type None +class UserTypes(object): + SUPPORT = "support" diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index d2beb275cf..ceab38f18c 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -221,12 +221,18 @@ class RegistrationHandler(BaseHandler): # auto-join the user to any rooms we're supposed to dump them into fake_requester = create_requester(user_id) - # try to create the room if we're the first user on the server + # try to create the room if we're the first real user on the server. Note + # that an auto generated support user is not a real user and never be + # the user to create the room should_auto_create_rooms = False - if self.hs.config.autocreate_auto_join_rooms: + is_support = yield self.store.is_support_user(user_id) + # There is an edge case where the first user is the support user, then + # the room is never created, though this seems unlikely and + # recoverable from given the support user being involved in the first + # place. + if (self.hs.config.autocreate_auto_join_rooms and not is_support): count = yield self.store.count_all_users() should_auto_create_rooms = count == 1 - for r in self.hs.config.auto_join_rooms: try: if should_auto_create_rooms: diff --git a/synapse/handlers/user_directory.py b/synapse/handlers/user_directory.py index f11b430126..fbfaea8aeb 100644 --- a/synapse/handlers/user_directory.py +++ b/synapse/handlers/user_directory.py @@ -125,9 +125,11 @@ class UserDirectoryHandler(object): """ # FIXME(#3714): We should probably do this in the same worker as all # the other changes. - yield self.store.update_profile_in_user_dir( - user_id, profile.display_name, profile.avatar_url, None, - ) + is_support = yield self.store.is_support_user(user_id) + if not is_support: + yield self.store.update_profile_in_user_dir( + user_id, profile.display_name, profile.avatar_url, None, + ) @defer.inlineCallbacks def handle_user_deactivated(self, user_id): @@ -135,8 +137,10 @@ class UserDirectoryHandler(object): """ # FIXME(#3714): We should probably do this in the same worker as all # the other changes. - yield self.store.remove_from_user_dir(user_id) - yield self.store.remove_from_user_in_public_room(user_id) + is_support = yield self.store.is_support_user(user_id) + if not is_support: + yield self.store.remove_from_user_dir(user_id) + yield self.store.remove_from_user_in_public_room(user_id) @defer.inlineCallbacks def _unsafe_process(self): @@ -329,13 +333,6 @@ class UserDirectoryHandler(object): public_value=Membership.JOIN, ) - if change is None: - # Handle any profile changes - yield self._handle_profile_change( - state_key, room_id, prev_event_id, event_id, - ) - continue - if not change: # Need to check if the server left the room entirely, if so # we might need to remove all the users in that room @@ -349,21 +346,32 @@ class UserDirectoryHandler(object): # need to remove those users or not user_ids = yield self.store.get_users_in_dir_due_to_room(room_id) for user_id in user_ids: - yield self._handle_remove_user(room_id, user_id) + is_support = yield self.store.is_support_user(state_key) + if not is_support: + yield self._handle_remove_user(room_id, user_id) return else: logger.debug("Server is still in room: %r", room_id) - if change: # The user joined - event = yield self.store.get_event(event_id, allow_none=True) - profile = ProfileInfo( - avatar_url=event.content.get("avatar_url"), - display_name=event.content.get("displayname"), - ) + is_support = yield self.store.is_support_user(state_key) + if not is_support: + if change is None: + # Handle any profile changes + yield self._handle_profile_change( + state_key, room_id, prev_event_id, event_id, + ) + continue + + if change: # The user joined + event = yield self.store.get_event(event_id, allow_none=True) + profile = ProfileInfo( + avatar_url=event.content.get("avatar_url"), + display_name=event.content.get("displayname"), + ) - yield self._handle_new_user(room_id, state_key, profile) - else: # The user left - yield self._handle_remove_user(room_id, state_key) + yield self._handle_new_user(room_id, state_key, profile) + else: # The user left + yield self._handle_remove_user(room_id, state_key) else: logger.debug("Ignoring irrelevant type: %r", typ) diff --git a/synapse/storage/monthly_active_users.py b/synapse/storage/monthly_active_users.py index 479e01ddc1..6e162a5fcd 100644 --- a/synapse/storage/monthly_active_users.py +++ b/synapse/storage/monthly_active_users.py @@ -208,6 +208,9 @@ class MonthlyActiveUsersStore(SQLBaseStore): bool: True if a new entry was created, False if an existing one was updated. """ + # Support user never to be included in MAU stats + if user_id == self.hs.config.support_user_id: + return # Am consciously deciding to lock the table on the basis that is ought # never be a big table and alternative approaches (batching multiple # upserts into a single txn) introduced a lot of extra complexity. diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index d5d2f89a77..fa36daac52 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -25,7 +25,7 @@ logger = logging.getLogger(__name__) # Remember to update this number every time a change is made to database # schema files, so the users will be informed on server restarts. -SCHEMA_VERSION = 52 +SCHEMA_VERSION = 53 dir_path = os.path.abspath(os.path.dirname(__file__)) diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 80d76bf9d7..2c6a417c79 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -19,6 +19,7 @@ from six.moves import range from twisted.internet import defer +from synapse.api.constants import UserTypes from synapse.api.errors import Codes, StoreError from synapse.storage import background_updates from synapse.storage._base import SQLBaseStore @@ -167,7 +168,7 @@ class RegistrationStore(RegistrationWorkerStore, def register(self, user_id, token=None, password_hash=None, was_guest=False, make_guest=False, appservice_id=None, - create_profile_with_localpart=None, admin=False): + create_profile_with_localpart=None, admin=False, user_type=None): """Attempts to register an account. Args: @@ -183,6 +184,8 @@ class RegistrationStore(RegistrationWorkerStore, appservice_id (str): The ID of the appservice registering the user. create_profile_with_localpart (str): Optionally create a profile for the given localpart. + admin (boolean): is an admin user? + user_type (synapse.api.constants import UserTypes): type of user Raises: StoreError if the user_id could not be registered. """ @@ -196,7 +199,8 @@ class RegistrationStore(RegistrationWorkerStore, make_guest, appservice_id, create_profile_with_localpart, - admin + admin, + user_type ) def _register( @@ -210,6 +214,7 @@ class RegistrationStore(RegistrationWorkerStore, appservice_id, create_profile_with_localpart, admin, + user_type, ): now = int(self.clock.time()) @@ -244,6 +249,7 @@ class RegistrationStore(RegistrationWorkerStore, "is_guest": 1 if make_guest else 0, "appservice_id": appservice_id, "admin": 1 if admin else 0, + "user_type": user_type } ) else: @@ -257,6 +263,7 @@ class RegistrationStore(RegistrationWorkerStore, "is_guest": 1 if make_guest else 0, "appservice_id": appservice_id, "admin": 1 if admin else 0, + "user_type": user_type, } ) except self.database_engine.module.IntegrityError: @@ -450,6 +457,18 @@ class RegistrationStore(RegistrationWorkerStore, defer.returnValue(res if res else False) + @cachedInlineCallbacks() + def is_support_user(self, user_id): + res = yield self._simple_select_one_onecol( + table="users", + keyvalues={"name": user_id}, + retcol="user_type", + allow_none=True, + desc="is_support_user", + ) + + defer.returnValue(True if res == UserTypes.SUPPORT else False) + @defer.inlineCallbacks def user_add_threepid(self, user_id, medium, address, validated_at, added_at): yield self._simple_upsert("user_threepids", { diff --git a/synapse/storage/schema/delta/53/add_user_type_to_users.sql b/synapse/storage/schema/delta/53/add_user_type_to_users.sql new file mode 100644 index 0000000000..9044e0152c --- /dev/null +++ b/synapse/storage/schema/delta/53/add_user_type_to_users.sql @@ -0,0 +1,20 @@ +/* Copyright 2018 New Vector Ltd + * + * 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. + */ + +/* record whether we have sent a server notice about consenting to the + * privacy policy. Specifically records the version of the policy we sent + * a message about. + */ +ALTER TABLE users ADD COLUMN user_type TEXT DEFAULT NULL; diff --git a/tests/api/test_auth.py b/tests/api/test_auth.py index 379e9c4ab1..69dc40428b 100644 --- a/tests/api/test_auth.py +++ b/tests/api/test_auth.py @@ -50,6 +50,8 @@ class AuthTestCase(unittest.TestCase): # this is overridden for the appservice tests self.store.get_app_service_by_token = Mock(return_value=None) + self.store.is_support_user = Mock(return_value=defer.succeed(False)) + @defer.inlineCallbacks def test_get_user_by_req_user_valid_token(self): user_info = {"name": self.test_user, "token_id": "ditto", "device_id": "device"} diff --git a/tests/handlers/test_register.py b/tests/handlers/test_register.py index 3e9a190727..dc297c030b 100644 --- a/tests/handlers/test_register.py +++ b/tests/handlers/test_register.py @@ -17,7 +17,7 @@ from mock import Mock from twisted.internet import defer -from synapse.api.errors import ResourceLimitError +from synapse.api.errors import ResourceLimitError, SynapseError from synapse.handlers.register import RegistrationHandler from synapse.types import RoomAlias, UserID, create_requester @@ -43,10 +43,6 @@ class RegistrationTestCase(unittest.TestCase): self.addCleanup, expire_access_token=True, ) - self.macaroon_generator = Mock( - generate_access_token=Mock(return_value='secret') - ) - self.hs.get_macaroon_generator = Mock(return_value=self.macaroon_generator) self.handler = self.hs.get_handlers().registration_handler self.store = self.hs.get_datastore() self.hs.config.max_mau_value = 50 @@ -64,7 +60,7 @@ class RegistrationTestCase(unittest.TestCase): requester, frank.localpart, "Frankie" ) self.assertEquals(result_user_id, user_id) - self.assertEquals(result_token, 'secret') + self.assertTrue(result_token is not None) @defer.inlineCallbacks def test_if_user_exists(self): @@ -82,7 +78,7 @@ class RegistrationTestCase(unittest.TestCase): requester, local_part, None ) self.assertEquals(result_user_id, user_id) - self.assertEquals(result_token, 'secret') + self.assertTrue(result_token is not None) @defer.inlineCallbacks def test_mau_limits_when_disabled(self): @@ -184,3 +180,17 @@ class RegistrationTestCase(unittest.TestCase): res = yield self.handler.register(localpart='jeff') rooms = yield self.store.get_rooms_for_user(res[0]) self.assertEqual(len(rooms), 0) + + @defer.inlineCallbacks + def test_auto_create_auto_join_rooms_when_support_user_exists(self): + room_alias_str = "#room:test" + self.hs.config.auto_join_rooms = [room_alias_str] + + self.store.is_support_user = Mock(return_value=True) + res = yield self.handler.register(localpart='support') + rooms = yield self.store.get_rooms_for_user(res[0]) + self.assertEqual(len(rooms), 0) + directory_handler = self.hs.get_handlers().directory_handler + room_alias = RoomAlias.from_string(room_alias_str) + with self.assertRaises(SynapseError): + yield directory_handler.get_association(room_alias) diff --git a/tests/handlers/test_user_directory.py b/tests/handlers/test_user_directory.py new file mode 100644 index 0000000000..11f2bae698 --- /dev/null +++ b/tests/handlers/test_user_directory.py @@ -0,0 +1,91 @@ +# -*- coding: utf-8 -*- +# Copyright 2018 New Vector +# +# 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. +from mock import Mock + +from twisted.internet import defer + +from synapse.api.constants import UserTypes +from synapse.handlers.user_directory import UserDirectoryHandler +from synapse.storage.roommember import ProfileInfo + +from tests import unittest +from tests.utils import setup_test_homeserver + + +class UserDirectoryHandlers(object): + def __init__(self, hs): + self.user_directory_handler = UserDirectoryHandler(hs) + + +class UserDirectoryTestCase(unittest.TestCase): + """ Tests the UserDirectoryHandler. """ + + @defer.inlineCallbacks + def setUp(self): + hs = yield setup_test_homeserver(self.addCleanup) + self.store = hs.get_datastore() + hs.handlers = UserDirectoryHandlers(hs) + + self.handler = hs.get_handlers().user_directory_handler + + @defer.inlineCallbacks + def test_handle_local_profile_change_with_support_user(self): + support_user_id = "@support:test" + yield self.store.register( + user_id=support_user_id, + token="123", + password_hash=None, + user_type=UserTypes.SUPPORT + ) + + yield self.handler.handle_local_profile_change(support_user_id, None) + profile = yield self.store.get_user_in_directory(support_user_id) + self.assertTrue(profile is None) + display_name = 'display_name' + + profile_info = ProfileInfo( + avatar_url='avatar_url', + display_name=display_name, + ) + regular_user_id = '@regular:test' + yield self.handler.handle_local_profile_change(regular_user_id, profile_info) + profile = yield self.store.get_user_in_directory(regular_user_id) + self.assertTrue(profile['display_name'] == display_name) + + @defer.inlineCallbacks + def test_handle_user_deactivated_support_user(self): + s_user_id = "@support:test" + self.store.register( + user_id=s_user_id, + token="123", + password_hash=None, + user_type=UserTypes.SUPPORT + ) + + self.store.remove_from_user_dir = Mock() + self.store.remove_from_user_in_public_room = Mock() + yield self.handler.handle_user_deactivated(s_user_id) + self.store.remove_from_user_dir.not_called() + self.store.remove_from_user_in_public_room.not_called() + + @defer.inlineCallbacks + def test_handle_user_deactivated_regular_user(self): + r_user_id = "@regular:test" + self.store.register(user_id=r_user_id, token="123", password_hash=None) + self.store.remove_from_user_dir = Mock() + self.store.remove_from_user_in_public_room = Mock() + yield self.handler.handle_user_deactivated(r_user_id) + self.store.remove_from_user_dir.called_once_with(r_user_id) + self.store.remove_from_user_in_public_room.assert_called_once_with(r_user_id) diff --git a/tests/storage/test_monthly_active_users.py b/tests/storage/test_monthly_active_users.py index 8664bc3d54..38fadf32bd 100644 --- a/tests/storage/test_monthly_active_users.py +++ b/tests/storage/test_monthly_active_users.py @@ -16,6 +16,8 @@ from mock import Mock from twisted.internet import defer +from synapse.api.constants import UserTypes + from tests.unittest import HomeserverTestCase FORTY_DAYS = 40 * 24 * 60 * 60 @@ -28,6 +30,7 @@ class MonthlyActiveUsersTestCase(HomeserverTestCase): self.store = hs.get_datastore() hs.config.limit_usage_by_mau = True hs.config.max_mau_value = 50 + # Advance the clock a bit reactor.advance(FORTY_DAYS) @@ -221,6 +224,23 @@ class MonthlyActiveUsersTestCase(HomeserverTestCase): count = self.store.get_registered_reserved_users_count() self.assertEquals(self.get_success(count), len(threepids)) + def test_support_user_not_add_to_mau_limits(self): + count = self.store.get_monthly_active_count() + self.pump() + self.assertEqual(self.get_success(count), 0) + support_user_id = '@support:test' + self.store.register( + user_id=support_user_id, + token="123", + password_hash=None, + user_type=UserTypes.SUPPORT + ) + + self.store.upsert_monthly_active_user(support_user_id) + count = self.store.get_monthly_active_count() + self.pump() + self.assertEqual(self.get_success(count), 0) + def test_track_monthly_users_without_cap(self): self.hs.config.limit_usage_by_mau = False self.hs.config.mau_stats_only = True diff --git a/tests/storage/test_registration.py b/tests/storage/test_registration.py index 3dfb7b903a..421713cb32 100644 --- a/tests/storage/test_registration.py +++ b/tests/storage/test_registration.py @@ -16,6 +16,8 @@ from twisted.internet import defer +from synapse.api.constants import UserTypes + from tests import unittest from tests.utils import setup_test_homeserver @@ -99,6 +101,26 @@ class RegistrationStoreTestCase(unittest.TestCase): user = yield self.store.get_user_by_access_token(self.tokens[0]) self.assertIsNone(user, "access token was not deleted without device_id") + @defer.inlineCallbacks + def test_is_support_user(self): + TEST_USER = "@test:test" + SUPPORT_USER = "@support:test" + + res = yield self.store.is_support_user(None) + self.assertFalse(res) + yield self.store.register(user_id=TEST_USER, token="123", password_hash=None) + res = yield self.store.is_support_user(TEST_USER) + self.assertFalse(res) + + self.store.register( + user_id=SUPPORT_USER, + token="456", + password_hash=None, + user_type=UserTypes.SUPPORT + ) + res = yield self.store.is_support_user(SUPPORT_USER) + self.assertTrue(res) + class TokenGenerator: def __init__(self): diff --git a/tests/utils.py b/tests/utils.py index 52ab762010..7771da242e 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -139,7 +139,6 @@ def default_config(name): config.admin_contact = None config.rc_messages_per_second = 10000 config.rc_message_burst_count = 10000 - config.use_frozen_dicts = False # we need a sane default_room_version, otherwise attempts to create rooms will |