From 7298ed7c5145ee11cf8a8d866562170c3161c63c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 21 Nov 2017 10:50:23 +0000 Subject: Clean up dependency list remove those that aren't used at all, and replace the ones that don't have builders with simple getters rather than dynamically-generated methods. --- synapse/server.py | 31 +++++++++++++++---------------- 1 file changed, 15 insertions(+), 16 deletions(-) (limited to 'synapse/server.py') diff --git a/synapse/server.py b/synapse/server.py index 10e3e9a4f1..4746cc7b6c 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -90,17 +90,12 @@ class HomeServer(object): """ DEPENDENCIES = [ - 'config', - 'clock', 'http_client', 'db_pool', - 'persistence_service', 'replication_layer', - 'datastore', 'handlers', 'v1auth', 'auth', - 'rest_servlet_factory', 'state_handler', 'presence_handler', 'sync_handler', @@ -118,18 +113,7 @@ class HomeServer(object): 'device_message_handler', 'profile_handler', 'notifier', - 'distributor', - 'client_resource', - 'resource_for_federation', - 'resource_for_static_content', - 'resource_for_web_client', - 'resource_for_content_repo', - 'resource_for_server_key', - 'resource_for_server_key_v2', - 'resource_for_media_repository', - 'resource_for_metrics', 'event_sources', - 'ratelimiter', 'keyring', 'pusherpool', 'event_builder_factory', @@ -183,6 +167,21 @@ class HomeServer(object): def is_mine_id(self, string): return string.split(":", 1)[1] == self.hostname + def get_clock(self): + return self.clock + + def get_datastore(self): + return self.datastore + + def get_config(self): + return self.config + + def get_distributor(self): + return self.distributor + + def get_ratelimiter(self): + return self.ratelimiter + def build_replication_layer(self): return initialize_http_replication(self) -- cgit 1.4.1 From e1fd4751de8e96907ea97afaf91525e68ce22227 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 21 Nov 2017 11:08:08 +0000 Subject: Build MediaRepositoryResource as a homeserver dependency This avoids the scenario where we have four different PreviewUrlResources configured on a single app, each of which have their own caches and cache clearing jobs. --- synapse/app/homeserver.py | 3 +-- synapse/app/media_repository.py | 3 +-- synapse/server.py | 11 ++++++++++- synapse/server.pyi | 7 +++++++ 4 files changed, 19 insertions(+), 5 deletions(-) (limited to 'synapse/server.py') diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 9e26146338..4b6164baa2 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -43,7 +43,6 @@ from synapse.rest import ClientRestResource from synapse.rest.key.v1.server_key_resource import LocalKey from synapse.rest.key.v2 import KeyApiV2Resource from synapse.rest.media.v0.content_repository import ContentRepoResource -from synapse.rest.media.v1.media_repository import MediaRepositoryResource from synapse.server import HomeServer from synapse.storage import are_all_users_on_domain from synapse.storage.engines import IncorrectDatabaseSetup, create_engine @@ -195,7 +194,7 @@ class SynapseHomeServer(HomeServer): }) if name in ["media", "federation", "client"]: - media_repo = MediaRepositoryResource(self) + media_repo = self.get_media_repository_resource() resources.update({ MEDIA_PREFIX: media_repo, LEGACY_MEDIA_PREFIX: media_repo, diff --git a/synapse/app/media_repository.py b/synapse/app/media_repository.py index 36c18bdbcb..f54beeb15d 100644 --- a/synapse/app/media_repository.py +++ b/synapse/app/media_repository.py @@ -35,7 +35,6 @@ from synapse.replication.slave.storage.registration import SlavedRegistrationSto from synapse.replication.slave.storage.transactions import TransactionStore from synapse.replication.tcp.client import ReplicationClientHandler from synapse.rest.media.v0.content_repository import ContentRepoResource -from synapse.rest.media.v1.media_repository import MediaRepositoryResource from synapse.server import HomeServer from synapse.storage.engines import create_engine from synapse.storage.media_repository import MediaRepositoryStore @@ -89,7 +88,7 @@ class MediaRepositoryServer(HomeServer): if name == "metrics": resources[METRICS_PREFIX] = MetricsResource(self) elif name == "media": - media_repo = MediaRepositoryResource(self) + media_repo = self.get_media_repository_resource() resources.update({ MEDIA_PREFIX: media_repo, LEGACY_MEDIA_PREFIX: media_repo, diff --git a/synapse/server.py b/synapse/server.py index 4746cc7b6c..853f4647b7 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -60,7 +60,10 @@ from synapse.http.matrixfederationclient import MatrixFederationHttpClient from synapse.notifier import Notifier from synapse.push.action_generator import ActionGenerator from synapse.push.pusherpool import PusherPool -from synapse.rest.media.v1.media_repository import MediaRepository +from synapse.rest.media.v1.media_repository import ( + MediaRepository, + MediaRepositoryResource, +) from synapse.state import StateHandler from synapse.storage import DataStore from synapse.streams.events import EventSources @@ -121,6 +124,7 @@ class HomeServer(object): 'http_client_context_factory', 'simple_http_client', 'media_repository', + 'media_repository_resource', 'federation_transport_client', 'federation_sender', 'receipts_handler', @@ -293,6 +297,11 @@ class HomeServer(object): **self.db_config.get("args", {}) ) + def build_media_repository_resource(self): + # build the media repo resource. This indirects through the HomeServer + # to ensure that we only have a single instance of + return MediaRepositoryResource(self) + def build_media_repository(self): return MediaRepository(self) diff --git a/synapse/server.pyi b/synapse/server.pyi index e8c0386b7f..3064a497eb 100644 --- a/synapse/server.pyi +++ b/synapse/server.pyi @@ -5,6 +5,7 @@ import synapse.handlers import synapse.handlers.auth import synapse.handlers.device import synapse.handlers.e2e_keys +import synapse.rest.media.v1.media_repository import synapse.storage import synapse.state @@ -35,3 +36,9 @@ class HomeServer(object): def get_federation_transport_client(self) -> synapse.federation.transport.client.TransportLayerClient: pass + + def get_media_repository_resource(self) -> synapse.rest.media.v1.media_repository.MediaRepositoryResource: + pass + + def get_media_repository(self) -> synapse.rest.media.v1.media_repository.MediaRepository: + pass -- cgit 1.4.1 From 7ca5c682338e073060050f4ff78a1ab83530f9f2 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 29 Nov 2017 11:48:43 +0000 Subject: Move deactivate_account into its own handler Non-functional refactoring to move deactivate_account. This means that we'll be able to properly deactivate devices and access tokens without introducing a dependency loop. --- synapse/handlers/auth.py | 16 ------------ synapse/handlers/deactivate_account.py | 44 +++++++++++++++++++++++++++++++++ synapse/rest/client/v1/admin.py | 4 +-- synapse/rest/client/v2_alpha/account.py | 7 +++--- synapse/server.py | 5 ++++ synapse/server.pyi | 7 +++++- 6 files changed, 61 insertions(+), 22 deletions(-) create mode 100644 synapse/handlers/deactivate_account.py (limited to 'synapse/server.py') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 0ba66bc947..cfcb4ea2a0 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -665,22 +665,6 @@ class AuthHandler(BaseHandler): user_id, except_token_id=except_access_token_id, ) - @defer.inlineCallbacks - def deactivate_account(self, user_id): - """Deactivate a user's account - - Args: - user_id (str): ID of user to be deactivated - - Returns: - Deferred - """ - # FIXME: Theoretically there is a race here wherein user resets - # password using threepid. - yield self.delete_access_tokens_for_user(user_id) - yield self.store.user_delete_threepids(user_id) - yield self.store.user_set_password_hash(user_id, None) - @defer.inlineCallbacks def delete_access_token(self, access_token): """Invalidate a single access token diff --git a/synapse/handlers/deactivate_account.py b/synapse/handlers/deactivate_account.py new file mode 100644 index 0000000000..70f02c9b76 --- /dev/null +++ b/synapse/handlers/deactivate_account.py @@ -0,0 +1,44 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 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. +from twisted.internet import defer + +from ._base import BaseHandler + +import logging + +logger = logging.getLogger(__name__) + + +class DeactivateAccountHandler(BaseHandler): + """Handler which deals with deactivating user accounts.""" + def __init__(self, hs): + super(DeactivateAccountHandler, self).__init__(hs) + self._auth_handler = hs.get_auth_handler() + + @defer.inlineCallbacks + def deactivate_account(self, user_id): + """Deactivate a user's account + + Args: + user_id (str): ID of user to be deactivated + + Returns: + Deferred + """ + # FIXME: Theoretically there is a race here wherein user resets + # password using threepid. + yield self._auth_handler.delete_access_tokens_for_user(user_id) + yield self.store.user_delete_threepids(user_id) + yield self.store.user_set_password_hash(user_id, None) diff --git a/synapse/rest/client/v1/admin.py b/synapse/rest/client/v1/admin.py index 1197158fdc..a67e22790b 100644 --- a/synapse/rest/client/v1/admin.py +++ b/synapse/rest/client/v1/admin.py @@ -137,8 +137,8 @@ class DeactivateAccountRestServlet(ClientV1RestServlet): PATTERNS = client_path_patterns("/admin/deactivate/(?P[^/]*)") def __init__(self, hs): - self._auth_handler = hs.get_auth_handler() super(DeactivateAccountRestServlet, self).__init__(hs) + self._deactivate_account_handler = hs.get_deactivate_account_handler() @defer.inlineCallbacks def on_POST(self, request, target_user_id): @@ -149,7 +149,7 @@ class DeactivateAccountRestServlet(ClientV1RestServlet): if not is_admin: raise AuthError(403, "You are not a server admin") - yield self._auth_handler.deactivate_account(target_user_id) + yield self._deactivate_account_handler.deactivate_account(target_user_id) defer.returnValue((200, {})) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 726e0a2826..6202e8849d 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -161,10 +161,11 @@ class DeactivateAccountRestServlet(RestServlet): PATTERNS = client_v2_patterns("/account/deactivate$") def __init__(self, hs): + super(DeactivateAccountRestServlet, self).__init__() self.hs = hs self.auth = hs.get_auth() self.auth_handler = hs.get_auth_handler() - super(DeactivateAccountRestServlet, self).__init__() + self._deactivate_account_handler = hs.get_deactivate_account_handler() @defer.inlineCallbacks def on_POST(self, request): @@ -179,7 +180,7 @@ class DeactivateAccountRestServlet(RestServlet): # allow ASes to dectivate their own users if requester and requester.app_service: - yield self.auth_handler.deactivate_account( + yield self._deactivate_account_handler.deactivate_account( requester.user.to_string() ) defer.returnValue((200, {})) @@ -206,7 +207,7 @@ class DeactivateAccountRestServlet(RestServlet): logger.error("Auth succeeded but no known type!", result.keys()) raise SynapseError(500, "", Codes.UNKNOWN) - yield self.auth_handler.deactivate_account(user_id) + yield self._deactivate_account_handler.deactivate_account(user_id) defer.returnValue((200, {})) diff --git a/synapse/server.py b/synapse/server.py index 853f4647b7..fea19e68d1 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -39,6 +39,7 @@ from synapse.federation.transaction_queue import TransactionQueue from synapse.handlers import Handlers from synapse.handlers.appservice import ApplicationServicesHandler from synapse.handlers.auth import AuthHandler, MacaroonGeneartor +from synapse.handlers.deactivate_account import DeactivateAccountHandler from synapse.handlers.devicemessage import DeviceMessageHandler from synapse.handlers.device import DeviceHandler from synapse.handlers.e2e_keys import E2eKeysHandler @@ -115,6 +116,7 @@ class HomeServer(object): 'application_service_handler', 'device_message_handler', 'profile_handler', + 'deactivate_account_handler', 'notifier', 'event_sources', 'keyring', @@ -268,6 +270,9 @@ class HomeServer(object): def build_profile_handler(self): return ProfileHandler(self) + def build_deactivate_account_handler(self): + return DeactivateAccountHandler(self) + def build_event_sources(self): return EventSources(self) diff --git a/synapse/server.pyi b/synapse/server.pyi index 3064a497eb..e1d0a71fd4 100644 --- a/synapse/server.pyi +++ b/synapse/server.pyi @@ -3,11 +3,13 @@ import synapse.federation.transaction_queue import synapse.federation.transport.client import synapse.handlers import synapse.handlers.auth +import synapse.handlers.deactivate_account import synapse.handlers.device import synapse.handlers.e2e_keys import synapse.rest.media.v1.media_repository -import synapse.storage import synapse.state +import synapse.storage + class HomeServer(object): def get_auth(self) -> synapse.api.auth.Auth: @@ -31,6 +33,9 @@ class HomeServer(object): def get_state_handler(self) -> synapse.state.StateHandler: pass + def get_deactivate_account_handler(self) -> synapse.handlers.deactivate_account.DeactivateAccountHandler: + pass + def get_federation_sender(self) -> synapse.federation.transaction_queue.TransactionQueue: pass -- cgit 1.4.1 From ae31f8ce4507e8c68e5c1aea3363789dbd8ca999 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 29 Nov 2017 14:10:46 +0000 Subject: Move set_password into its own handler Non-functional refactoring to move set_password. This means that we'll be able to properly deactivate devices and access tokens without introducing a dependency loop. --- synapse/handlers/auth.py | 16 ------------ synapse/handlers/set_password.py | 45 +++++++++++++++++++++++++++++++++ synapse/rest/client/v1/admin.py | 4 +-- synapse/rest/client/v2_alpha/account.py | 3 ++- synapse/server.py | 5 ++++ synapse/server.pyi | 4 +++ 6 files changed, 58 insertions(+), 19 deletions(-) create mode 100644 synapse/handlers/set_password.py (limited to 'synapse/server.py') diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index cfcb4ea2a0..2f30f183ce 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -649,22 +649,6 @@ class AuthHandler(BaseHandler): except Exception: raise AuthError(403, "Invalid token", errcode=Codes.FORBIDDEN) - @defer.inlineCallbacks - def set_password(self, user_id, newpassword, requester=None): - password_hash = self.hash(newpassword) - - except_access_token_id = requester.access_token_id if requester else None - - try: - yield self.store.user_set_password_hash(user_id, password_hash) - except StoreError as e: - if e.code == 404: - raise SynapseError(404, "Unknown user", Codes.NOT_FOUND) - raise e - yield self.delete_access_tokens_for_user( - user_id, except_token_id=except_access_token_id, - ) - @defer.inlineCallbacks def delete_access_token(self, access_token): """Invalidate a single access token diff --git a/synapse/handlers/set_password.py b/synapse/handlers/set_password.py new file mode 100644 index 0000000000..c0d83f229c --- /dev/null +++ b/synapse/handlers/set_password.py @@ -0,0 +1,45 @@ +# -*- coding: utf-8 -*- +# Copyright 2017 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. +import logging + +from twisted.internet import defer + +from synapse.api.errors import Codes, StoreError, SynapseError +from ._base import BaseHandler + +logger = logging.getLogger(__name__) + + +class SetPasswordHandler(BaseHandler): + """Handler which deals with changing user account passwords""" + def __init__(self, hs): + super(SetPasswordHandler, self).__init__(hs) + self._auth_handler = hs.get_auth_handler() + + @defer.inlineCallbacks + def set_password(self, user_id, newpassword, requester=None): + password_hash = self._auth_handler.hash(newpassword) + + except_access_token_id = requester.access_token_id if requester else None + + try: + yield self.store.user_set_password_hash(user_id, password_hash) + except StoreError as e: + if e.code == 404: + raise SynapseError(404, "Unknown user", Codes.NOT_FOUND) + raise e + yield self._auth_handler.delete_access_tokens_for_user( + user_id, except_token_id=except_access_token_id, + ) diff --git a/synapse/rest/client/v1/admin.py b/synapse/rest/client/v1/admin.py index a67e22790b..5022808ea9 100644 --- a/synapse/rest/client/v1/admin.py +++ b/synapse/rest/client/v1/admin.py @@ -309,7 +309,7 @@ class ResetPasswordRestServlet(ClientV1RestServlet): super(ResetPasswordRestServlet, self).__init__(hs) self.hs = hs self.auth = hs.get_auth() - self.auth_handler = hs.get_auth_handler() + self._set_password_handler = hs.get_set_password_handler() @defer.inlineCallbacks def on_POST(self, request, target_user_id): @@ -330,7 +330,7 @@ class ResetPasswordRestServlet(ClientV1RestServlet): logger.info("new_password: %r", new_password) - yield self.auth_handler.set_password( + yield self._set_password_handler.set_password( target_user_id, new_password, requester ) defer.returnValue((200, {})) diff --git a/synapse/rest/client/v2_alpha/account.py b/synapse/rest/client/v2_alpha/account.py index 6202e8849d..c26ce63bcf 100644 --- a/synapse/rest/client/v2_alpha/account.py +++ b/synapse/rest/client/v2_alpha/account.py @@ -98,6 +98,7 @@ class PasswordRestServlet(RestServlet): self.auth = hs.get_auth() self.auth_handler = hs.get_auth_handler() self.datastore = self.hs.get_datastore() + self._set_password_handler = hs.get_set_password_handler() @defer.inlineCallbacks def on_POST(self, request): @@ -147,7 +148,7 @@ class PasswordRestServlet(RestServlet): raise SynapseError(400, "", Codes.MISSING_PARAM) new_password = params['new_password'] - yield self.auth_handler.set_password( + yield self._set_password_handler.set_password( user_id, new_password, requester ) diff --git a/synapse/server.py b/synapse/server.py index fea19e68d1..18c72d21a8 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -45,6 +45,7 @@ from synapse.handlers.device import DeviceHandler from synapse.handlers.e2e_keys import E2eKeysHandler from synapse.handlers.presence import PresenceHandler from synapse.handlers.room_list import RoomListHandler +from synapse.handlers.set_password import SetPasswordHandler from synapse.handlers.sync import SyncHandler from synapse.handlers.typing import TypingHandler from synapse.handlers.events import EventHandler, EventStreamHandler @@ -117,6 +118,7 @@ class HomeServer(object): 'device_message_handler', 'profile_handler', 'deactivate_account_handler', + 'set_password_handler', 'notifier', 'event_sources', 'keyring', @@ -273,6 +275,9 @@ class HomeServer(object): def build_deactivate_account_handler(self): return DeactivateAccountHandler(self) + def build_set_password_handler(self): + return SetPasswordHandler(self) + def build_event_sources(self): return EventSources(self) diff --git a/synapse/server.pyi b/synapse/server.pyi index e1d0a71fd4..41416ef252 100644 --- a/synapse/server.pyi +++ b/synapse/server.pyi @@ -6,6 +6,7 @@ import synapse.handlers.auth import synapse.handlers.deactivate_account import synapse.handlers.device import synapse.handlers.e2e_keys +import synapse.handlers.set_password import synapse.rest.media.v1.media_repository import synapse.state import synapse.storage @@ -36,6 +37,9 @@ class HomeServer(object): def get_deactivate_account_handler(self) -> synapse.handlers.deactivate_account.DeactivateAccountHandler: pass + def get_set_password_handler(self) -> synapse.handlers.set_password.SetPasswordHandler: + pass + def get_federation_sender(self) -> synapse.federation.transaction_queue.TransactionQueue: pass -- cgit 1.4.1