From ef2228c890b44963c65f086eb1246c27ef43d256 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Feb 2019 13:55:58 +0000 Subject: Basic sentry integration --- synapse/app/_base.py | 22 ++++++++++++++++++++++ synapse/config/metrics.py | 8 ++++++++ synapse/python_dependencies.py | 1 + 3 files changed, 31 insertions(+) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 5b0ca312e2..d681ff5245 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -25,10 +25,12 @@ from daemonize import Daemonize from twisted.internet import error, reactor from twisted.protocols.tls import TLSMemoryBIOFactory +import synapse from synapse.app import check_bind_error from synapse.crypto import context_factory from synapse.util import PreserveLoggingContext from synapse.util.rlimit import change_resource_limit +from synapse.util.versionstring import get_version_string logger = logging.getLogger(__name__) @@ -266,9 +268,29 @@ def start(hs, listeners=None): # It is now safe to start your Synapse. hs.start_listening(listeners) hs.get_datastore().start_profiling() + + setup_sentry_io(hs) except Exception: traceback.print_exc(file=sys.stderr) reactor = hs.get_reactor() if reactor.running: reactor.stop() sys.exit(1) + + +def setup_sentry_io(hs): + if not hs.config.sentry_enabled: + return + + import sentry_sdk + sentry_sdk.init( + dsn=hs.config.sentry_dsn, + release=get_version_string(synapse), + ) + with sentry_sdk.configure_scope() as scope: + scope.set_tag("matrix_server_name", hs.config.server_name) + + app = hs.config.worker_app if hs.config.worker_app else "synapse.app.homeserver" + name = hs.config.worker_name if hs.config.worker_name else "master" + scope.set_tag("worker_app", app) + scope.set_tag("worker_name", name) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 718c43ae03..f312010a9b 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -23,12 +23,20 @@ class MetricsConfig(Config): self.metrics_port = config.get("metrics_port") self.metrics_bind_host = config.get("metrics_bind_host", "127.0.0.1") + self.sentry_enabled = "sentry" in config + if self.sentry_enabled: + self.sentry_dsn = config["sentry"]["dsn"] + def default_config(self, report_stats=None, **kwargs): res = """\ ## Metrics ### # Enable collection and rendering of performance metrics enable_metrics: False + + # Enable sentry.io integration + #sentry: + # dsn: "..." """ if report_stats is None: diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index 5d087ee26b..444fc2a979 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -86,6 +86,7 @@ CONDITIONAL_REQUIREMENTS = { "saml2": ["pysaml2>=4.5.0"], "url_preview": ["lxml>=3.5.0"], "test": ["mock>=2.0", "parameterized"], + "sentry": ["sentry-sdk>=0.7.2"], } -- cgit 1.5.1 From 6a8f902edbb1b003ae4a5f1c78d9c09fa4e87dfc Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Feb 2019 16:01:41 +0000 Subject: Raise an appropriate error message if sentry_sdk missing --- synapse/config/metrics.py | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index f312010a9b..7bab8b0b2b 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -13,7 +13,16 @@ # See the License for the specific language governing permissions and # limitations under the License. -from ._base import Config +from ._base import Config, ConfigError + +MISSING_SENTRY = ( + """Missing sentry_sdk library. This is required for enable sentry.io + integration. + + Install by running: + pip install sentry_sdk + """ +) class MetricsConfig(Config): @@ -25,6 +34,11 @@ class MetricsConfig(Config): self.sentry_enabled = "sentry" in config if self.sentry_enabled: + try: + import sentry_sdk # noqa F401 + except ImportError: + raise ConfigError(MISSING_SENTRY) + self.sentry_dsn = config["sentry"]["dsn"] def default_config(self, report_stats=None, **kwargs): -- cgit 1.5.1 From 93f7d2df3e8bb44e4f9fb6c5ce3fc23a86c30c1a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Feb 2019 16:03:40 +0000 Subject: Comments --- synapse/app/_base.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index d681ff5245..482f0e22ea 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -279,6 +279,12 @@ def start(hs, listeners=None): def setup_sentry_io(hs): + """Enable sentry.io integration, if enabled in configuration + + Args: + hs (synapse.server.HomeServer) + """ + if not hs.config.sentry_enabled: return @@ -287,6 +293,8 @@ def setup_sentry_io(hs): dsn=hs.config.sentry_dsn, release=get_version_string(synapse), ) + + # We set some default tags that give some context to this instance with sentry_sdk.configure_scope() as scope: scope.set_tag("matrix_server_name", hs.config.server_name) -- cgit 1.5.1 From dc7078905600cc3945d3eb1dcf008dd7a196d24a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 12 Feb 2019 16:07:43 +0000 Subject: Newsfile --- changelog.d/4632.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4632.feature diff --git a/changelog.d/4632.feature b/changelog.d/4632.feature new file mode 100644 index 0000000000..0bdeb3738a --- /dev/null +++ b/changelog.d/4632.feature @@ -0,0 +1 @@ +Add basic optional sentry.io integration -- cgit 1.5.1 From 6cb415b63fd58ab253f8519f725a581a0e15044e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 13 Feb 2019 16:14:37 +0000 Subject: Fixup comments and add warning --- changelog.d/4632.feature | 2 +- synapse/app/_base.py | 6 +++--- synapse/config/metrics.py | 9 +++++++-- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/changelog.d/4632.feature b/changelog.d/4632.feature index 0bdeb3738a..d053ab5a25 100644 --- a/changelog.d/4632.feature +++ b/changelog.d/4632.feature @@ -1 +1 @@ -Add basic optional sentry.io integration +Add basic optional sentry integration diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 482f0e22ea..0284151d0f 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -269,7 +269,7 @@ def start(hs, listeners=None): hs.start_listening(listeners) hs.get_datastore().start_profiling() - setup_sentry_io(hs) + setup_sentry(hs) except Exception: traceback.print_exc(file=sys.stderr) reactor = hs.get_reactor() @@ -278,8 +278,8 @@ def start(hs, listeners=None): sys.exit(1) -def setup_sentry_io(hs): - """Enable sentry.io integration, if enabled in configuration +def setup_sentry(hs): + """Enable sentry integration, if enabled in configuration Args: hs (synapse.server.HomeServer) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 7bab8b0b2b..185b895a24 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -16,7 +16,7 @@ from ._base import Config, ConfigError MISSING_SENTRY = ( - """Missing sentry_sdk library. This is required for enable sentry.io + """Missing sentry_sdk library. This is required for enable sentry integration. Install by running: @@ -48,7 +48,12 @@ class MetricsConfig(Config): # Enable collection and rendering of performance metrics enable_metrics: False - # Enable sentry.io integration + # Enable sentry integration + # NOTE: While attempts are made to ensure that the logs don't contain + # any sensitive information, this cannot be guaranteed. By enabling + # this option the sentry server may therefore receive sensitive + # information, and it in turn may then diseminate sensitive information + # through insecure notification channels if so configured. #sentry: # dsn: "..." """ -- cgit 1.5.1 From 7fc1196a362fc56d11e2652ee5c9699b1198cf40 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 14 Feb 2019 13:58:52 +0000 Subject: Correctly handle RequestSendFailed exceptions This mainly reduces the number of exceptions we log. --- synapse/crypto/keyring.py | 4 ++-- synapse/groups/attestations.py | 7 ++++++- synapse/handlers/device.py | 4 ++-- synapse/handlers/groups_local.py | 12 +++++++++--- 4 files changed, 19 insertions(+), 8 deletions(-) diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index 3a96980bed..cce40fdd2d 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -35,7 +35,7 @@ from unpaddedbase64 import decode_base64 from twisted.internet import defer -from synapse.api.errors import Codes, SynapseError +from synapse.api.errors import Codes, RequestSendFailed, SynapseError from synapse.util import logcontext, unwrapFirstError from synapse.util.logcontext import ( LoggingContext, @@ -656,7 +656,7 @@ def _handle_key_deferred(verify_request): try: with PreserveLoggingContext(): _, key_id, verify_key = yield verify_request.deferred - except IOError as e: + except (IOError, RequestSendFailed) as e: logger.warn( "Got IOError when downloading keys for %s: %s %s", server_name, type(e).__name__, str(e), diff --git a/synapse/groups/attestations.py b/synapse/groups/attestations.py index b04f4234ca..786149be65 100644 --- a/synapse/groups/attestations.py +++ b/synapse/groups/attestations.py @@ -42,7 +42,7 @@ from signedjson.sign import sign_json from twisted.internet import defer -from synapse.api.errors import SynapseError +from synapse.api.errors import RequestSendFailed, SynapseError from synapse.metrics.background_process_metrics import run_as_background_process from synapse.types import get_domain_from_id from synapse.util.logcontext import run_in_background @@ -191,6 +191,11 @@ class GroupAttestionRenewer(object): yield self.store.update_attestation_renewal( group_id, user_id, attestation ) + except RequestSendFailed as e: + logger.warning( + "Failed to renew attestation of %r in %r: %s", + user_id, group_id, e, + ) except Exception: logger.exception("Error renewing attestation of %r in %r", user_id, group_id) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 8955cde4ed..6eddb10e0d 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -20,7 +20,7 @@ from twisted.internet import defer from synapse.api import errors from synapse.api.constants import EventTypes -from synapse.api.errors import FederationDeniedError +from synapse.api.errors import FederationDeniedError, RequestSendFailed from synapse.types import RoomStreamToken, get_domain_from_id from synapse.util import stringutils from synapse.util.async_helpers import Linearizer @@ -504,7 +504,7 @@ class DeviceListEduUpdater(object): origin = get_domain_from_id(user_id) try: result = yield self.federation.query_user_devices(origin, user_id) - except NotRetryingDestination: + except (NotRetryingDestination, RequestSendFailed): # TODO: Remember that we are now out of sync and try again # later logger.warn( diff --git a/synapse/handlers/groups_local.py b/synapse/handlers/groups_local.py index 173315af6c..02c508acec 100644 --- a/synapse/handlers/groups_local.py +++ b/synapse/handlers/groups_local.py @@ -20,7 +20,7 @@ from six import iteritems from twisted.internet import defer -from synapse.api.errors import HttpResponseException, SynapseError +from synapse.api.errors import HttpResponseException, RequestSendFailed, SynapseError from synapse.types import get_domain_from_id logger = logging.getLogger(__name__) @@ -46,13 +46,19 @@ def _create_rerouter(func_name): # when the remote end responds with things like 403 Not # In Group, we can communicate that to the client instead # of a 500. - def h(failure): + def http_response_errback(failure): failure.trap(HttpResponseException) e = failure.value if e.code == 403: raise e.to_synapse_error() return failure - d.addErrback(h) + + def request_failed_errback(failure): + failure.trap(RequestSendFailed) + raise SynapseError(502, "Failed to contact group server") + + d.addErrback(http_response_errback) + d.addErrback(request_failed_errback) return d return f -- cgit 1.5.1 From 0927adb012396fef8a91d595fc07b7f2e0a06272 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 14 Feb 2019 14:02:04 +0000 Subject: Newsfile --- changelog.d/4643.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4643.misc diff --git a/changelog.d/4643.misc b/changelog.d/4643.misc new file mode 100644 index 0000000000..556cdd2240 --- /dev/null +++ b/changelog.d/4643.misc @@ -0,0 +1 @@ +Reduce number of exceptions we log -- cgit 1.5.1 From 1895d14e12b901ed4928950e6cc3b1e2e6fd89cd Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 15 Feb 2019 12:05:08 +0000 Subject: Support .well-known delegation when issuing certificates through ACME --- changelog.d/4652.feature | 1 + synapse/handlers/acme.py | 27 +++++++++++++++++++++++---- 2 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 changelog.d/4652.feature diff --git a/changelog.d/4652.feature b/changelog.d/4652.feature new file mode 100644 index 0000000000..48d9bb08a0 --- /dev/null +++ b/changelog.d/4652.feature @@ -0,0 +1 @@ +Support .well-known delegation when issuing certificates through ACME diff --git a/synapse/handlers/acme.py b/synapse/handlers/acme.py index dd0b217965..9d1b1a1c29 100644 --- a/synapse/handlers/acme.py +++ b/synapse/handlers/acme.py @@ -25,8 +25,11 @@ from twisted.python.filepath import FilePath from twisted.python.url import URL from twisted.web import server, static from twisted.web.resource import Resource +from twisted.web.client import URI from synapse.app import check_bind_error +from synapse.crypto.context_factory import ClientTLSOptionsFactory +from synapse.http.federation.matrix_federation_agent import MatrixFederationAgent logger = logging.getLogger(__name__) @@ -123,15 +126,31 @@ class AcmeHandler(object): @defer.inlineCallbacks def provision_certificate(self): - logger.warning("Reprovisioning %s", self.hs.hostname) + # Retrieve .well-known if it's in use. We do so through the federation + # agent, because that's where the .well-known logic lives. + agent = MatrixFederationAgent( + tls_client_options_factory=ClientTLSOptionsFactory(None), + reactor=self.reactor, + ) + delegated = yield agent._get_well_known(bytes(self.hs.hostname,"ascii")) + + # If .well-known is in use, use the delegated hostname instead of the + # homeserver's server_name. + if delegated: + cert_name = delegated.decode("ascii") + logger.info(".well-known is in use, provisionning %s instead of %s", cert_name, self.hs.hostname) + else: + cert_name = self.hs.hostname + + logger.warning("Reprovisioning %s", cert_name) try: - yield self._issuer.issue_cert(self.hs.hostname) + yield self._issuer.issue_cert(cert_name) except Exception: logger.exception("Fail!") raise - logger.warning("Reprovisioned %s, saving.", self.hs.hostname) - cert_chain = self._store.certs[self.hs.hostname] + logger.warning("Reprovisioned %s, saving.", cert_name) + cert_chain = self._store.certs[cert_name] try: with open(self.hs.config.tls_private_key_file, "wb") as private_key_file: -- cgit 1.5.1 From af8a2f679b38d0e3594e172b3b4f7a7c4468193e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 15 Feb 2019 12:27:43 +0000 Subject: Remove unused import --- synapse/handlers/acme.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/handlers/acme.py b/synapse/handlers/acme.py index 9d1b1a1c29..93a6a36e6a 100644 --- a/synapse/handlers/acme.py +++ b/synapse/handlers/acme.py @@ -25,7 +25,6 @@ from twisted.python.filepath import FilePath from twisted.python.url import URL from twisted.web import server, static from twisted.web.resource import Resource -from twisted.web.client import URI from synapse.app import check_bind_error from synapse.crypto.context_factory import ClientTLSOptionsFactory -- cgit 1.5.1 From f86b695cbd6a39492946fcddbfcc241ff836e767 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 15 Feb 2019 12:29:34 +0000 Subject: Various cosmetics to make TravisCI happy --- synapse/handlers/acme.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/acme.py b/synapse/handlers/acme.py index 93a6a36e6a..a56a9cd287 100644 --- a/synapse/handlers/acme.py +++ b/synapse/handlers/acme.py @@ -131,13 +131,16 @@ class AcmeHandler(object): tls_client_options_factory=ClientTLSOptionsFactory(None), reactor=self.reactor, ) - delegated = yield agent._get_well_known(bytes(self.hs.hostname,"ascii")) + delegated = yield agent._get_well_known(bytes(self.hs.hostname, "ascii")) # If .well-known is in use, use the delegated hostname instead of the # homeserver's server_name. if delegated: cert_name = delegated.decode("ascii") - logger.info(".well-known is in use, provisionning %s instead of %s", cert_name, self.hs.hostname) + logger.info( + ".well-known is in use, provisionning %s instead of %s", + cert_name, self.hs.hostname, + ) else: cert_name = self.hs.hostname -- cgit 1.5.1 From da95867d30bf5abeac2c70707927f8733d30d881 Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 15 Feb 2019 22:21:18 -0700 Subject: Changelog --- changelog.d/4657.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4657.misc diff --git a/changelog.d/4657.misc b/changelog.d/4657.misc new file mode 100644 index 0000000000..8872765819 --- /dev/null +++ b/changelog.d/4657.misc @@ -0,0 +1 @@ +Fix various spelling mistakes. -- cgit 1.5.1 From 68d2869c8dd2358270d3eb45f06658ceea59b907 Mon Sep 17 00:00:00 2001 From: "Juuso \"Linda\" Lapinlampi" Date: Wed, 13 Feb 2019 12:09:57 +0000 Subject: config: Remove a repeated word from a logger warning The warning for missing macaroon_secret_key was "missing missing". --- synapse/config/key.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/key.py b/synapse/config/key.py index dce4b19a2d..499ffd4e06 100644 --- a/synapse/config/key.py +++ b/synapse/config/key.py @@ -56,7 +56,7 @@ class KeyConfig(Config): if not self.macaroon_secret_key: # Unfortunately, there are people out there that don't have this # set. Lets just be "nice" and derive one from their secret key. - logger.warn("Config is missing missing macaroon_secret_key") + logger.warn("Config is missing macaroon_secret_key") seed = bytes(self.signing_key[0]) self.macaroon_secret_key = hashlib.sha256(seed).digest() -- cgit 1.5.1 From 6575df647da9d946dae0a41881d27b1569fff9c5 Mon Sep 17 00:00:00 2001 From: "Juuso \"Linda\" Lapinlampi" Date: Fri, 8 Feb 2019 21:26:16 +0000 Subject: UPGRADE.rst: Fix a typo in "Upgrading Synapse" section See: https://en.wiktionary.org/wiki/successful --- UPGRADE.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UPGRADE.rst b/UPGRADE.rst index d869b7111b..228222d534 100644 --- a/UPGRADE.rst +++ b/UPGRADE.rst @@ -39,7 +39,7 @@ instructions that may be required are listed later in this document. ./synctl restart -To check whether your update was sucessful, you can check the Server header +To check whether your update was successful, you can check the Server header returned by the Client-Server API: .. code:: bash -- cgit 1.5.1 From 6d02a13d81f7d99cb92081631b188398eea0c4d7 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Mon, 18 Feb 2019 11:36:34 +0000 Subject: Typo in info log Co-Authored-By: babolivier --- synapse/handlers/acme.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/acme.py b/synapse/handlers/acme.py index a56a9cd287..ca5b7257d3 100644 --- a/synapse/handlers/acme.py +++ b/synapse/handlers/acme.py @@ -138,7 +138,7 @@ class AcmeHandler(object): if delegated: cert_name = delegated.decode("ascii") logger.info( - ".well-known is in use, provisionning %s instead of %s", + ".well-known is in use, provisioning %s instead of %s", cert_name, self.hs.hostname, ) else: -- cgit 1.5.1 From 5b68e12fd842272833d065dab952ec66a6f3f9e6 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Mon, 18 Feb 2019 11:36:44 +0000 Subject: Typo in changelog Co-Authored-By: babolivier --- changelog.d/4652.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/4652.feature b/changelog.d/4652.feature index 48d9bb08a0..ebe6880b21 100644 --- a/changelog.d/4652.feature +++ b/changelog.d/4652.feature @@ -1 +1 @@ -Support .well-known delegation when issuing certificates through ACME +Support .well-known delegation when issuing certificates through ACME. -- cgit 1.5.1 From eb2b8523ae1ddd38bf1dd19ee37e44e7f4a3ee68 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 12:12:57 +0000 Subject: Split out registration to worker This allows registration to be handled by a worker, though the actual write to the database still happens on master. Note: due to the in-memory session map all registration requests must be handled by the same worker. --- synapse/app/client_reader.py | 2 + synapse/handlers/register.py | 63 ++++++++- synapse/replication/http/__init__.py | 4 +- synapse/replication/http/login.py | 85 ++++++++++++ synapse/replication/http/register.py | 91 ++++++++++++ synapse/rest/client/v2_alpha/register.py | 73 ++++++---- synapse/storage/registration.py | 230 +++++++++++++++---------------- 7 files changed, 401 insertions(+), 147 deletions(-) create mode 100644 synapse/replication/http/login.py create mode 100644 synapse/replication/http/register.py diff --git a/synapse/app/client_reader.py b/synapse/app/client_reader.py index a9d2147022..9250b6c239 100644 --- a/synapse/app/client_reader.py +++ b/synapse/app/client_reader.py @@ -47,6 +47,7 @@ from synapse.rest.client.v1.room import ( RoomMemberListRestServlet, RoomStateRestServlet, ) +from synapse.rest.client.v2_alpha.register import RegisterRestServlet from synapse.server import HomeServer from synapse.storage.engines import create_engine from synapse.util.httpresourcetree import create_resource_tree @@ -92,6 +93,7 @@ class ClientReaderServer(HomeServer): JoinedRoomMemberListRestServlet(self).register(resource) RoomStateRestServlet(self).register(resource) RoomEventContextServlet(self).register(resource) + RegisterRestServlet(self).register(resource) resources.update({ "/_matrix/client/r0": resource, diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 21c17c59a0..8ea557a003 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -27,6 +27,7 @@ from synapse.api.errors import ( SynapseError, ) from synapse.http.client import CaptchaServerHttpClient +from synapse.replication.http.register import ReplicationRegisterServlet from synapse.types import RoomAlias, RoomID, UserID, create_requester from synapse.util.async_helpers import Linearizer from synapse.util.threepids import check_3pid_allowed @@ -61,6 +62,9 @@ class RegistrationHandler(BaseHandler): ) self._server_notices_mxid = hs.config.server_notices_mxid + if hs.config.worker_app: + self._register_client = ReplicationRegisterServlet.make_client(hs) + @defer.inlineCallbacks def check_username(self, localpart, guest_access_token=None, assigned_user_id=None): @@ -185,7 +189,7 @@ class RegistrationHandler(BaseHandler): token = None if generate_token: token = self.macaroon_gen.generate_access_token(user_id) - yield self.store.register( + yield self._register_with_store( user_id=user_id, token=token, password_hash=password_hash, @@ -217,7 +221,7 @@ class RegistrationHandler(BaseHandler): if default_display_name is None: default_display_name = localpart try: - yield self.store.register( + yield self._register_with_store( user_id=user_id, token=token, password_hash=password_hash, @@ -316,7 +320,7 @@ class RegistrationHandler(BaseHandler): user_id, allowed_appservice=service ) - yield self.store.register( + yield self._register_with_store( user_id=user_id, password_hash="", appservice_id=service_id, @@ -494,7 +498,7 @@ class RegistrationHandler(BaseHandler): token = self.macaroon_gen.generate_access_token(user_id) if need_register: - yield self.store.register( + yield self._register_with_store( user_id=user_id, token=token, password_hash=password_hash, @@ -573,3 +577,54 @@ class RegistrationHandler(BaseHandler): action="join", ratelimit=False, ) + + def _register_with_store(self, user_id, token=None, password_hash=None, + was_guest=False, make_guest=False, appservice_id=None, + create_profile_with_displayname=None, admin=False, + user_type=None): + """Register user in the datastore. + + Args: + user_id (str): The desired user ID to register. + token (str): The desired access token to use for this user. If this + is not None, the given access token is associated with the user + id. + password_hash (str|None): Optional. The password hash for this user. + was_guest (bool): Optional. Whether this is a guest account being + upgraded to a non-guest account. + make_guest (boolean): True if the the new user should be guest, + false to add a regular user account. + appservice_id (str|None): The ID of the appservice registering the user. + create_profile_with_displayname (unicode|None): Optionally create a + profile for the user, setting their displayname to the given value + admin (boolean): is an admin user? + user_type (str|None): type of user. One of the values from + api.constants.UserTypes, or None for a normal user. + + Returns: + Deferred + """ + if self.hs.config.worker_app: + return self._register_client( + user_id=user_id, + token=token, + password_hash=password_hash, + was_guest=was_guest, + make_guest=make_guest, + appservice_id=appservice_id, + create_profile_with_displayname=create_profile_with_displayname, + admin=admin, + user_type=user_type, + ) + else: + return self.store.register( + user_id=user_id, + token=token, + password_hash=password_hash, + was_guest=was_guest, + make_guest=make_guest, + appservice_id=appservice_id, + create_profile_with_displayname=create_profile_with_displayname, + admin=admin, + user_type=user_type, + ) diff --git a/synapse/replication/http/__init__.py b/synapse/replication/http/__init__.py index 19f214281e..81b85352b1 100644 --- a/synapse/replication/http/__init__.py +++ b/synapse/replication/http/__init__.py @@ -14,7 +14,7 @@ # limitations under the License. from synapse.http.server import JsonResource -from synapse.replication.http import federation, membership, send_event +from synapse.replication.http import federation, login, membership, register, send_event REPLICATION_PREFIX = "/_synapse/replication" @@ -28,3 +28,5 @@ class ReplicationRestResource(JsonResource): send_event.register_servlets(hs, self) membership.register_servlets(hs, self) federation.register_servlets(hs, self) + login.register_servlets(hs, self) + register.register_servlets(hs, self) diff --git a/synapse/replication/http/login.py b/synapse/replication/http/login.py new file mode 100644 index 0000000000..797f6aabd1 --- /dev/null +++ b/synapse/replication/http/login.py @@ -0,0 +1,85 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 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.http.servlet import parse_json_object_from_request +from synapse.replication.http._base import ReplicationEndpoint + +logger = logging.getLogger(__name__) + + +class RegisterDeviceReplicationServlet(ReplicationEndpoint): + """Ensure a device is registered, generating a new access token for the + device. + + Used during registration and login. + """ + + NAME = "device_check_registered" + PATH_ARGS = ("user_id",) + + def __init__(self, hs): + super(RegisterDeviceReplicationServlet, self).__init__(hs) + self.auth_handler = hs.get_auth_handler() + self.device_handler = hs.get_device_handler() + self.macaroon_gen = hs.get_macaroon_generator() + + @staticmethod + def _serialize_payload(user_id, device_id, initial_display_name, is_guest): + """ + Args: + device_id (str|None): Device ID to use, if None a new one is + generated. + initial_display_name (str|None) + is_guest (bool) + """ + return { + "device_id": device_id, + "initial_display_name": initial_display_name, + "is_guest": is_guest, + } + + @defer.inlineCallbacks + def _handle_request(self, request, user_id): + content = parse_json_object_from_request(request) + + device_id = content["device_id"] + initial_display_name = content["initial_display_name"] + is_guest = content["is_guest"] + + device_id = yield self.device_handler.check_device_registered( + user_id, device_id, initial_display_name, + ) + + if is_guest: + access_token = self.macaroon_gen.generate_access_token( + user_id, ["guest = true"] + ) + else: + access_token = yield self.auth_handler.get_access_token_for_user_id( + user_id, device_id=device_id, + ) + + defer.returnValue((200, { + "device_id": device_id, + "access_token": access_token, + })) + + +def register_servlets(hs, http_server): + RegisterDeviceReplicationServlet(hs).register(http_server) diff --git a/synapse/replication/http/register.py b/synapse/replication/http/register.py new file mode 100644 index 0000000000..bdaf37396c --- /dev/null +++ b/synapse/replication/http/register.py @@ -0,0 +1,91 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 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.http.servlet import parse_json_object_from_request +from synapse.replication.http._base import ReplicationEndpoint + +logger = logging.getLogger(__name__) + + +class ReplicationRegisterServlet(ReplicationEndpoint): + """Register a new user + """ + + NAME = "register_user" + PATH_ARGS = ("user_id",) + + def __init__(self, hs): + super(ReplicationRegisterServlet, self).__init__(hs) + self.store = hs.get_datastore() + + @staticmethod + def _serialize_payload( + user_id, token, password_hash, was_guest, make_guest, appservice_id, + create_profile_with_displayname, admin, user_type, + ): + """ + Args: + user_id (str): The desired user ID to register. + token (str): The desired access token to use for this user. If this + is not None, the given access token is associated with the user + id. + password_hash (str|None): Optional. The password hash for this user. + was_guest (bool): Optional. Whether this is a guest account being + upgraded to a non-guest account. + make_guest (boolean): True if the the new user should be guest, + false to add a regular user account. + appservice_id (str|None): The ID of the appservice registering the user. + create_profile_with_displayname (unicode|None): Optionally create a + profile for the user, setting their displayname to the given value + admin (boolean): is an admin user? + user_type (str|None): type of user. One of the values from + api.constants.UserTypes, or None for a normal user. + """ + return { + "token": token, + "password_hash": password_hash, + "was_guest": was_guest, + "make_guest": make_guest, + "appservice_id": appservice_id, + "create_profile_with_displayname": create_profile_with_displayname, + "admin": admin, + "user_type": user_type, + } + + @defer.inlineCallbacks + def _handle_request(self, request, user_id): + content = parse_json_object_from_request(request) + + yield self.store.register( + user_id=user_id, + token=content["token"], + password_hash=content["password_hash"], + was_guest=content["was_guest"], + make_guest=content["make_guest"], + appservice_id=content["appservice_id"], + create_profile_with_displayname=content["create_profile_with_displayname"], + admin=content["admin"], + user_type=content["user_type"], + ) + + defer.returnValue((200, {})) + + +def register_servlets(hs, http_server): + ReplicationRegisterServlet(hs).register(http_server) diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index 7f812b8209..d78da50787 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -33,6 +33,7 @@ from synapse.http.servlet import ( parse_json_object_from_request, parse_string, ) +from synapse.replication.http.login import RegisterDeviceReplicationServlet from synapse.util.msisdn import phone_number_to_msisdn from synapse.util.ratelimitutils import FederationRateLimiter from synapse.util.threepids import check_3pid_allowed @@ -190,9 +191,15 @@ class RegisterRestServlet(RestServlet): self.registration_handler = hs.get_handlers().registration_handler self.identity_handler = hs.get_handlers().identity_handler self.room_member_handler = hs.get_room_member_handler() - self.device_handler = hs.get_device_handler() self.macaroon_gen = hs.get_macaroon_generator() + if self.hs.config.worker_app: + self._register_device_client = ( + RegisterDeviceReplicationServlet.make_client(hs) + ) + else: + self.device_handler = hs.get_device_handler() + @interactive_auth_handler @defer.inlineCallbacks def on_POST(self, request): @@ -633,12 +640,10 @@ class RegisterRestServlet(RestServlet): "home_server": self.hs.hostname, } if not params.get("inhibit_login", False): - device_id = yield self._register_device(user_id, params) - - access_token = ( - yield self.auth_handler.get_access_token_for_user_id( - user_id, device_id=device_id, - ) + device_id = params.get("device_id") + initial_display_name = params.get("initial_device_display_name") + device_id, access_token = yield self._register_device( + user_id, device_id, initial_display_name, is_guest=False, ) result.update({ @@ -647,25 +652,42 @@ class RegisterRestServlet(RestServlet): }) defer.returnValue(result) - def _register_device(self, user_id, params): - """Register a device for a user. - - This is called after the user's credentials have been validated, but - before the access token has been issued. + @defer.inlineCallbacks + def _register_device(self, user_id, device_id, initial_display_name, + is_guest): + """Register a device for a user and generate an access token. Args: - (str) user_id: full canonical @user:id - (object) params: registration parameters, from which we pull - device_id and initial_device_name + user_id (str): full canonical @user:id + device_id (str|None): The device ID to check, or None to generate + a new one. + initial_display_name (str|None): An optional display name for the + device. + is_guest (bool): Whether this is a guest account Returns: - defer.Deferred: (str) device_id + defer.Deferred[(str, str)]: Tuple of device ID and access token """ - # register the user's device - device_id = params.get("device_id") - initial_display_name = params.get("initial_device_display_name") - return self.device_handler.check_device_registered( - user_id, device_id, initial_display_name - ) + if self.hs.config.worker_app: + r = yield self._register_device_client( + user_id=user_id, + device_id=device_id, + initial_display_name=initial_display_name, + is_guest=is_guest, + ) + defer.returnValue((r["device_id"], r["access_token"])) + else: + device_id = yield self.device_handler.check_device_registered( + user_id, device_id, initial_display_name + ) + if is_guest: + access_token = self.macaroon_gen.generate_access_token( + user_id, ["guest = true"] + ) + else: + access_token = yield self.auth_handler.get_access_token_for_user_id( + user_id, device_id=device_id, + ) + defer.returnValue((device_id, access_token)) @defer.inlineCallbacks def _do_guest_registration(self, params): @@ -680,13 +702,10 @@ class RegisterRestServlet(RestServlet): # we have nowhere to store it. device_id = synapse.api.auth.GUEST_DEVICE_ID initial_display_name = params.get("initial_device_display_name") - yield self.device_handler.check_device_registered( - user_id, device_id, initial_display_name + device_id, access_token = yield self._register_device( + user_id, device_id, initial_display_name, is_guest=True, ) - access_token = self.macaroon_gen.generate_access_token( - user_id, ["guest = true"] - ) defer.returnValue((200, { "user_id": user_id, "device_id": device_id, diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index c9e11c3135..3bc5def48e 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -139,6 +139,121 @@ class RegistrationWorkerStore(SQLBaseStore): ) return True if res == UserTypes.SUPPORT else False + def get_users_by_id_case_insensitive(self, user_id): + """Gets users that match user_id case insensitively. + Returns a mapping of user_id -> password_hash. + """ + def f(txn): + sql = ( + "SELECT name, password_hash FROM users" + " WHERE lower(name) = lower(?)" + ) + txn.execute(sql, (user_id,)) + return dict(txn) + + return self.runInteraction("get_users_by_id_case_insensitive", f) + + @defer.inlineCallbacks + def count_all_users(self): + """Counts all users registered on the homeserver.""" + def _count_users(txn): + txn.execute("SELECT COUNT(*) AS users FROM users") + rows = self.cursor_to_dict(txn) + if rows: + return rows[0]["users"] + return 0 + + ret = yield self.runInteraction("count_users", _count_users) + defer.returnValue(ret) + + def count_daily_user_type(self): + """ + Counts 1) native non guest users + 2) native guests users + 3) bridged users + who registered on the homeserver in the past 24 hours + """ + def _count_daily_user_type(txn): + yesterday = int(self._clock.time()) - (60 * 60 * 24) + + sql = """ + SELECT user_type, COALESCE(count(*), 0) AS count FROM ( + SELECT + CASE + WHEN is_guest=0 AND appservice_id IS NULL THEN 'native' + WHEN is_guest=1 AND appservice_id IS NULL THEN 'guest' + WHEN is_guest=0 AND appservice_id IS NOT NULL THEN 'bridged' + END AS user_type + FROM users + WHERE creation_ts > ? + ) AS t GROUP BY user_type + """ + results = {'native': 0, 'guest': 0, 'bridged': 0} + txn.execute(sql, (yesterday,)) + for row in txn: + results[row[0]] = row[1] + return results + return self.runInteraction("count_daily_user_type", _count_daily_user_type) + + @defer.inlineCallbacks + def count_nonbridged_users(self): + def _count_users(txn): + txn.execute(""" + SELECT COALESCE(COUNT(*), 0) FROM users + WHERE appservice_id IS NULL + """) + count, = txn.fetchone() + return count + + ret = yield self.runInteraction("count_users", _count_users) + defer.returnValue(ret) + + @defer.inlineCallbacks + def find_next_generated_user_id_localpart(self): + """ + Gets the localpart of the next generated user ID. + + Generated user IDs are integers, and we aim for them to be as small as + we can. Unfortunately, it's possible some of them are already taken by + existing users, and there may be gaps in the already taken range. This + function returns the start of the first allocatable gap. This is to + avoid the case of ID 10000000 being pre-allocated, so us wasting the + first (and shortest) many generated user IDs. + """ + def _find_next_generated_user_id(txn): + txn.execute("SELECT name FROM users") + + regex = re.compile(r"^@(\d+):") + + found = set() + + for user_id, in txn: + match = regex.search(user_id) + if match: + found.add(int(match.group(1))) + for i in range(len(found) + 1): + if i not in found: + return i + + defer.returnValue((yield self.runInteraction( + "find_next_generated_user_id", + _find_next_generated_user_id + ))) + + @defer.inlineCallbacks + def get_3pid_guest_access_token(self, medium, address): + ret = yield self._simple_select_one( + "threepid_guest_access_tokens", + { + "medium": medium, + "address": address + }, + ["guest_access_token"], True, 'get_3pid_guest_access_token' + ) + if ret: + defer.returnValue(ret["guest_access_token"]) + defer.returnValue(None) + class RegistrationStore(RegistrationWorkerStore, background_updates.BackgroundUpdateStore): @@ -326,20 +441,6 @@ class RegistrationStore(RegistrationWorkerStore, ) txn.call_after(self.is_guest.invalidate, (user_id,)) - def get_users_by_id_case_insensitive(self, user_id): - """Gets users that match user_id case insensitively. - Returns a mapping of user_id -> password_hash. - """ - def f(txn): - sql = ( - "SELECT name, password_hash FROM users" - " WHERE lower(name) = lower(?)" - ) - txn.execute(sql, (user_id,)) - return dict(txn) - - return self.runInteraction("get_users_by_id_case_insensitive", f) - def user_set_password_hash(self, user_id, password_hash): """ NB. This does *not* evict any cache because the one use for this @@ -564,107 +665,6 @@ class RegistrationStore(RegistrationWorkerStore, desc="user_delete_threepids", ) - @defer.inlineCallbacks - def count_all_users(self): - """Counts all users registered on the homeserver.""" - def _count_users(txn): - txn.execute("SELECT COUNT(*) AS users FROM users") - rows = self.cursor_to_dict(txn) - if rows: - return rows[0]["users"] - return 0 - - ret = yield self.runInteraction("count_users", _count_users) - defer.returnValue(ret) - - def count_daily_user_type(self): - """ - Counts 1) native non guest users - 2) native guests users - 3) bridged users - who registered on the homeserver in the past 24 hours - """ - def _count_daily_user_type(txn): - yesterday = int(self._clock.time()) - (60 * 60 * 24) - - sql = """ - SELECT user_type, COALESCE(count(*), 0) AS count FROM ( - SELECT - CASE - WHEN is_guest=0 AND appservice_id IS NULL THEN 'native' - WHEN is_guest=1 AND appservice_id IS NULL THEN 'guest' - WHEN is_guest=0 AND appservice_id IS NOT NULL THEN 'bridged' - END AS user_type - FROM users - WHERE creation_ts > ? - ) AS t GROUP BY user_type - """ - results = {'native': 0, 'guest': 0, 'bridged': 0} - txn.execute(sql, (yesterday,)) - for row in txn: - results[row[0]] = row[1] - return results - return self.runInteraction("count_daily_user_type", _count_daily_user_type) - - @defer.inlineCallbacks - def count_nonbridged_users(self): - def _count_users(txn): - txn.execute(""" - SELECT COALESCE(COUNT(*), 0) FROM users - WHERE appservice_id IS NULL - """) - count, = txn.fetchone() - return count - - ret = yield self.runInteraction("count_users", _count_users) - defer.returnValue(ret) - - @defer.inlineCallbacks - def find_next_generated_user_id_localpart(self): - """ - Gets the localpart of the next generated user ID. - - Generated user IDs are integers, and we aim for them to be as small as - we can. Unfortunately, it's possible some of them are already taken by - existing users, and there may be gaps in the already taken range. This - function returns the start of the first allocatable gap. This is to - avoid the case of ID 10000000 being pre-allocated, so us wasting the - first (and shortest) many generated user IDs. - """ - def _find_next_generated_user_id(txn): - txn.execute("SELECT name FROM users") - - regex = re.compile(r"^@(\d+):") - - found = set() - - for user_id, in txn: - match = regex.search(user_id) - if match: - found.add(int(match.group(1))) - for i in range(len(found) + 1): - if i not in found: - return i - - defer.returnValue((yield self.runInteraction( - "find_next_generated_user_id", - _find_next_generated_user_id - ))) - - @defer.inlineCallbacks - def get_3pid_guest_access_token(self, medium, address): - ret = yield self._simple_select_one( - "threepid_guest_access_tokens", - { - "medium": medium, - "address": address - }, - ["guest_access_token"], True, 'get_3pid_guest_access_token' - ) - if ret: - defer.returnValue(ret["guest_access_token"]) - defer.returnValue(None) - @defer.inlineCallbacks def save_or_get_3pid_guest_access_token( self, medium, address, access_token, inviter_user_id -- cgit 1.5.1 From 91c8a7f9f4a3d598c98e0288541da3703605d7f9 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 12:15:27 +0000 Subject: Newsfile --- changelog.d/4666.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4666.feature diff --git a/changelog.d/4666.feature b/changelog.d/4666.feature new file mode 100644 index 0000000000..ebae3d703c --- /dev/null +++ b/changelog.d/4666.feature @@ -0,0 +1 @@ +Allow registration to be handled by a worker isntance. -- cgit 1.5.1 From 41c3f21c3b105375f75ad307a98b1481552a09d6 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 13:43:16 +0000 Subject: Fix unit tests --- tests/rest/client/v2_alpha/test_register.py | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 753d5c3e80..18080ebfd6 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -32,7 +32,18 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): self.identity_handler = Mock() self.login_handler = Mock() self.device_handler = Mock() - self.device_handler.check_device_registered = Mock(return_value="FAKE") + + def check_device_registered(user_id, device_id, initial_display_name): + # Just echo back the given device ID, or return a new "FAKE" device + # ID + if device_id: + return device_id + else: + return "FAKE" + + self.device_handler.check_device_registered = Mock( + side_effect=check_device_registered, + ) self.datastore = Mock(return_value=Mock()) self.datastore.get_current_state_deltas = Mock(return_value=[]) @@ -106,14 +117,12 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): user_id = "@kermit:muppet" token = "kermits_access_token" device_id = "frogfone" - request_data = json.dumps( - {"username": "kermit", "password": "monkey", "device_id": device_id} - ) + params = {"username": "kermit", "password": "monkey", "device_id": device_id} + request_data = json.dumps(params) self.registration_handler.check_username = Mock(return_value=True) - self.auth_result = (None, {"username": "kermit", "password": "monkey"}, None) + self.auth_result = (None, params, None) self.registration_handler.register = Mock(return_value=(user_id, None)) self.auth_handler.get_access_token_for_user_id = Mock(return_value=token) - self.device_handler.check_device_registered = Mock(return_value=device_id) request, channel = self.make_request(b"POST", self.url, request_data) self.render(request) -- cgit 1.5.1 From e83a1906436016df60b153cebab014d983231f79 Mon Sep 17 00:00:00 2001 From: Will Hunt Date: Mon, 18 Feb 2019 13:46:13 +0000 Subject: Update changelog.d/4666.feature Co-Authored-By: erikjohnston --- changelog.d/4666.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/4666.feature b/changelog.d/4666.feature index ebae3d703c..421060f9f9 100644 --- a/changelog.d/4666.feature +++ b/changelog.d/4666.feature @@ -1 +1 @@ -Allow registration to be handled by a worker isntance. +Allow registration to be handled by a worker instance. -- cgit 1.5.1 From dc5efc92a82dbf96a7d4d054b0c842b9788bea2a Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 13:52:49 +0000 Subject: Fixup --- synapse/config/metrics.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index 185b895a24..fc72e32d20 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -16,7 +16,7 @@ from ._base import Config, ConfigError MISSING_SENTRY = ( - """Missing sentry_sdk library. This is required for enable sentry + """Missing sentry_sdk library. This is required to enable sentry integration. Install by running: -- cgit 1.5.1 From 9caab0c364e4c79c76cc0816a598ad6174da364b Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Thu, 14 Feb 2019 13:43:50 +0000 Subject: Transfer bans on room upgrade --- synapse/handlers/room.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index f9af1f0046..924880d522 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -285,6 +285,7 @@ class RoomCreationHandler(BaseHandler): (EventTypes.RoomAvatar, ""), (EventTypes.Encryption, ""), (EventTypes.ServerACL, ""), + (EventTypes.Member, None), ) old_room_state_ids = yield self.store.get_filtered_current_state_ids( @@ -296,6 +297,19 @@ class RoomCreationHandler(BaseHandler): for k, old_event_id in iteritems(old_room_state_ids): old_event = old_room_state_events.get(old_event_id) if old_event: + + # Only transfer ban membership events + if ("membership" in old_event.content and + old_event.content["membership"] == "ban"): + yield self.room_member_handler.update_membership( + requester, + UserID.from_string(old_event['state_key']), + room_id, + "ban", + ratelimit=False, + content=old_event.content, + ) + initial_state[k] = old_event.content yield self._send_events_for_new_room( -- cgit 1.5.1 From 7033b05cada17a67057643608977236066f65d6a Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 18 Feb 2019 13:40:17 +0000 Subject: Add changelog --- changelog.d/4642.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4642.bugfix diff --git a/changelog.d/4642.bugfix b/changelog.d/4642.bugfix new file mode 100644 index 0000000000..bfbf95bcbb --- /dev/null +++ b/changelog.d/4642.bugfix @@ -0,0 +1 @@ +Transfer bans on room upgrade. \ No newline at end of file -- cgit 1.5.1 From 915421065b6738c78779d40994cb2dd33d618b9b Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 18 Feb 2019 14:02:09 +0000 Subject: Membership events are done later --- synapse/handlers/room.py | 32 ++++++++++++++++++++------------ 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 924880d522..2d24c115b6 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -294,21 +294,14 @@ class RoomCreationHandler(BaseHandler): # map from event_id to BaseEvent old_room_state_events = yield self.store.get_events(old_room_state_ids.values()) + member_events = [] for k, old_event_id in iteritems(old_room_state_ids): old_event = old_room_state_events.get(old_event_id) if old_event: - - # Only transfer ban membership events - if ("membership" in old_event.content and - old_event.content["membership"] == "ban"): - yield self.room_member_handler.update_membership( - requester, - UserID.from_string(old_event['state_key']), - room_id, - "ban", - ratelimit=False, - content=old_event.content, - ) + # Do membership events later + if ("membership" in old_event.content): + member_events.append(old_event) + continue initial_state[k] = old_event.content @@ -325,6 +318,21 @@ class RoomCreationHandler(BaseHandler): creation_content=creation_content, ) + # Transfer membership events + for old_event in member_events: + # Only transfer ban events + logger.info("Event type: " + str(old_event.content)) + if ("membership" in old_event.content and + old_event.content["membership"] == "ban"): + yield self.room_member_handler.update_membership( + requester, + UserID.from_string(old_event['state_key']), + new_room_id, + "ban", + ratelimit=False, + content=old_event.content, + ) + # XXX invites/joins # XXX 3pid invites -- cgit 1.5.1 From 32e54b472a6c6c12d0b92ac46c8733c300589f19 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 14:08:13 +0000 Subject: Fix kicking guest users in worker mode When guest_access changes from allowed to forbidden all local guest users should be kicked from the room. This did not happen when revocation was received from federation on a worker. Presumably broken in #4141 --- synapse/app/federation_reader.py | 2 ++ synapse/handlers/_base.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/app/federation_reader.py b/synapse/app/federation_reader.py index 6ee2b76dcd..b116c17669 100644 --- a/synapse/app/federation_reader.py +++ b/synapse/app/federation_reader.py @@ -40,6 +40,7 @@ from synapse.replication.slave.storage.profile import SlavedProfileStore from synapse.replication.slave.storage.push_rule import SlavedPushRuleStore from synapse.replication.slave.storage.pushers import SlavedPusherStore from synapse.replication.slave.storage.receipts import SlavedReceiptsStore +from synapse.replication.slave.storage.registration import SlavedRegistrationStore from synapse.replication.slave.storage.room import RoomStore from synapse.replication.slave.storage.transactions import SlavedTransactionStore from synapse.replication.tcp.client import ReplicationClientHandler @@ -62,6 +63,7 @@ class FederationReaderSlavedStore( SlavedReceiptsStore, SlavedEventStore, SlavedKeyStore, + SlavedRegistrationStore, RoomStore, DirectoryStore, SlavedTransactionStore, diff --git a/synapse/handlers/_base.py b/synapse/handlers/_base.py index 704181d2d3..594754cfd8 100644 --- a/synapse/handlers/_base.py +++ b/synapse/handlers/_base.py @@ -167,4 +167,4 @@ class BaseHandler(object): ratelimit=False, ) except Exception as e: - logger.warn("Error kicking guest user: %s" % (e,)) + logger.exception("Error kicking guest user: %s" % (e,)) -- cgit 1.5.1 From e07cc31cb800c6ec99a23960f78a2e7f968c255b Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 14:52:48 +0000 Subject: Correctly handle HttpResponseException --- synapse/handlers/device.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 6eddb10e0d..d9d65347bc 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -504,13 +504,13 @@ class DeviceListEduUpdater(object): origin = get_domain_from_id(user_id) try: result = yield self.federation.query_user_devices(origin, user_id) - except (NotRetryingDestination, RequestSendFailed): + except ( + NotRetryingDestination, RequestSendFailed, HttpResponseException, + ): # TODO: Remember that we are now out of sync and try again # later logger.warn( - "Failed to handle device list update for %s," - " we're not retrying the remote", - user_id, + "Failed to handle device list update for %s", user_id, ) # We abort on exceptions rather than accepting the update # as otherwise synapse will 'forget' that its device list -- cgit 1.5.1 From 2f16857ca97818052040ed6f8a0376885ef075bf Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 14:54:02 +0000 Subject: Newsfile --- changelog.d/4668.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4668.misc diff --git a/changelog.d/4668.misc b/changelog.d/4668.misc new file mode 100644 index 0000000000..556cdd2240 --- /dev/null +++ b/changelog.d/4668.misc @@ -0,0 +1 @@ +Reduce number of exceptions we log -- cgit 1.5.1 From e85aabb030580c39d7d5b5c780b32c7549b225e2 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 14:19:12 +0000 Subject: Newsfile --- changelog.d/4667.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4667.bugfix diff --git a/changelog.d/4667.bugfix b/changelog.d/4667.bugfix new file mode 100644 index 0000000000..33ad00c137 --- /dev/null +++ b/changelog.d/4667.bugfix @@ -0,0 +1 @@ +Fix kicking guest users on guest access revocation in worker mode. -- cgit 1.5.1 From fe725f7e452363e60cb0daf8ef55af603adaa0df Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 15:11:04 +0000 Subject: Cleanup top level request exception logging Firstly, we always logged that the request was being handled via `JsonResource._async_render`, so we change that to use the servlet name we add to the request. Secondly, we pass the exception information to the logger rather than formatting it manually. This makes it consistent with other exception logging, allwoing logging hooks and formatters to access the exception information. --- synapse/http/server.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/http/server.py b/synapse/http/server.py index 6a427d96a6..6c67a25a11 100644 --- a/synapse/http/server.py +++ b/synapse/http/server.py @@ -106,10 +106,10 @@ def wrap_json_request_handler(h): # trace. f = failure.Failure() logger.error( - "Failed handle request via %r: %r: %s", - h, + "Failed handle request via %r: %r", + request.request_metrics.name, request, - f.getTraceback().rstrip(), + exc_info=(f.type, f.value, f.getTracebackObject()), ) # Only respond with an error response if we haven't already started # writing, otherwise lets just kill the connection -- cgit 1.5.1 From 12ae64ce0d23f76dd19a27f3a754f37a82253496 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 15:23:10 +0000 Subject: Newsfile --- changelog.d/4669.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4669.misc diff --git a/changelog.d/4669.misc b/changelog.d/4669.misc new file mode 100644 index 0000000000..00a1a940ae --- /dev/null +++ b/changelog.d/4669.misc @@ -0,0 +1 @@ +Cleanup request exception logging -- cgit 1.5.1 From 94960cef037cac1361f42a4ab709b0693ba903ae Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 15:24:13 +0000 Subject: pep8 --- synapse/handlers/device.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index d9d65347bc..c708c35d4d 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -20,7 +20,11 @@ from twisted.internet import defer from synapse.api import errors from synapse.api.constants import EventTypes -from synapse.api.errors import FederationDeniedError, RequestSendFailed +from synapse.api.errors import ( + FederationDeniedError, + HttpResponseException, + RequestSendFailed, +) from synapse.types import RoomStreamToken, get_domain_from_id from synapse.util import stringutils from synapse.util.async_helpers import Linearizer -- cgit 1.5.1 From 8b9ae6d3a665e861072ee92bba38dd44a13d9f54 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 15:25:44 +0000 Subject: Update docs --- docs/workers.rst | 6 ++++++ synapse/rest/client/v2_alpha/register.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/workers.rst b/docs/workers.rst index dd3a84ba0d..6ce7d88c11 100644 --- a/docs/workers.rst +++ b/docs/workers.rst @@ -223,6 +223,12 @@ following regular expressions:: ^/_matrix/client/(api/v1|r0|unstable)/rooms/.*/members$ ^/_matrix/client/(api/v1|r0|unstable)/rooms/.*/state$ +Additionally, the following REST endpoints can be handled, but all requests must +be routed to the same instance:: + + ^/_matrix/client/(api/v1|r0|unstable)/register$ + + ``synapse.app.user_dir`` ~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index d78da50787..c52280c50c 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -665,7 +665,7 @@ class RegisterRestServlet(RestServlet): device. is_guest (bool): Whether this is a guest account Returns: - defer.Deferred[(str, str)]: Tuple of device ID and access token + defer.Deferred[tuple[str, str]]: Tuple of device ID and access token """ if self.hs.config.worker_app: r = yield self._register_device_client( -- cgit 1.5.1 From 45bb55c6de8b50fdd00893a6ef86623d2f34b864 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 18 Feb 2019 15:46:23 +0000 Subject: Use a configuration parameter to give the domain to generate a certificate for --- synapse/config/tls.py | 7 +++++++ synapse/handlers/acme.py | 29 ++++------------------------- 2 files changed, 11 insertions(+), 25 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 5fb3486db1..a3a5ece681 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -42,6 +42,7 @@ class TlsConfig(Config): self.acme_port = acme_config.get("port", 80) self.acme_bind_addresses = acme_config.get("bind_addresses", ['::', '0.0.0.0']) self.acme_reprovision_threshold = acme_config.get("reprovision_threshold", 30) + self.acme_domain = acme_config.get("domain", config.get("server_name")) self.tls_certificate_file = self.abspath(config.get("tls_certificate_path")) self.tls_private_key_file = self.abspath(config.get("tls_private_key_path")) @@ -229,6 +230,12 @@ class TlsConfig(Config): # # reprovision_threshold: 30 + # What domain the certificate should be for. Only useful if + # delegation via a /.well-known/matrix/server file is being used. + # Defaults to the server_name configuration parameter. + # + # domain: matrix.example.com + # List of allowed TLS fingerprints for this server to publish along # with the signing keys for this server. Other matrix servers that # make HTTPS requests to this server will check that the TLS diff --git a/synapse/handlers/acme.py b/synapse/handlers/acme.py index ca5b7257d3..f8a786a4da 100644 --- a/synapse/handlers/acme.py +++ b/synapse/handlers/acme.py @@ -27,8 +27,6 @@ from twisted.web import server, static from twisted.web.resource import Resource from synapse.app import check_bind_error -from synapse.crypto.context_factory import ClientTLSOptionsFactory -from synapse.http.federation.matrix_federation_agent import MatrixFederationAgent logger = logging.getLogger(__name__) @@ -125,34 +123,15 @@ class AcmeHandler(object): @defer.inlineCallbacks def provision_certificate(self): - # Retrieve .well-known if it's in use. We do so through the federation - # agent, because that's where the .well-known logic lives. - agent = MatrixFederationAgent( - tls_client_options_factory=ClientTLSOptionsFactory(None), - reactor=self.reactor, - ) - delegated = yield agent._get_well_known(bytes(self.hs.hostname, "ascii")) - - # If .well-known is in use, use the delegated hostname instead of the - # homeserver's server_name. - if delegated: - cert_name = delegated.decode("ascii") - logger.info( - ".well-known is in use, provisioning %s instead of %s", - cert_name, self.hs.hostname, - ) - else: - cert_name = self.hs.hostname - - logger.warning("Reprovisioning %s", cert_name) + logger.warning("Reprovisioning %s", self.hs.config.acme_domain) try: - yield self._issuer.issue_cert(cert_name) + yield self._issuer.issue_cert(self.hs.config.acme_domain) except Exception: logger.exception("Fail!") raise - logger.warning("Reprovisioned %s, saving.", cert_name) - cert_chain = self._store.certs[cert_name] + logger.warning("Reprovisioned %s, saving.", self.hs.config.acme_domain) + cert_chain = self._store.certs[self.hs.config.acme_domain] try: with open(self.hs.config.tls_private_key_file, "wb") as private_key_file: -- cgit 1.5.1 From af691e415c3247b912137227a06a68d4c4356586 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 16:49:38 +0000 Subject: Move register_device into handler --- synapse/handlers/register.py | 51 ++++++++++++++-- synapse/replication/http/login.py | 17 +----- synapse/rest/client/v1/login.py | 59 +++++++----------- synapse/rest/client/v2_alpha/register.py | 49 +-------------- tests/rest/client/v2_alpha/test_register.py | 93 +++++++---------------------- 5 files changed, 97 insertions(+), 172 deletions(-) diff --git a/synapse/handlers/register.py b/synapse/handlers/register.py index 8ea557a003..f92ab4d525 100644 --- a/synapse/handlers/register.py +++ b/synapse/handlers/register.py @@ -27,6 +27,7 @@ from synapse.api.errors import ( SynapseError, ) from synapse.http.client import CaptchaServerHttpClient +from synapse.replication.http.login import RegisterDeviceReplicationServlet from synapse.replication.http.register import ReplicationRegisterServlet from synapse.types import RoomAlias, RoomID, UserID, create_requester from synapse.util.async_helpers import Linearizer @@ -64,6 +65,11 @@ class RegistrationHandler(BaseHandler): if hs.config.worker_app: self._register_client = ReplicationRegisterServlet.make_client(hs) + self._register_device_client = ( + RegisterDeviceReplicationServlet.make_client(hs) + ) + else: + self.device_handler = hs.get_device_handler() @defer.inlineCallbacks def check_username(self, localpart, guest_access_token=None, @@ -159,7 +165,7 @@ class RegistrationHandler(BaseHandler): yield self.auth.check_auth_blocking(threepid=threepid) password_hash = None if password: - password_hash = yield self.auth_handler().hash(password) + password_hash = yield self._auth_handler.hash(password) if localpart: yield self.check_username(localpart, guest_access_token=guest_access_token) @@ -516,9 +522,6 @@ class RegistrationHandler(BaseHandler): defer.returnValue((user_id, token)) - def auth_handler(self): - return self.hs.get_auth_handler() - @defer.inlineCallbacks def get_or_register_3pid_guest(self, medium, address, inviter_user_id): """Get a guest access token for a 3PID, creating a guest account if @@ -628,3 +631,43 @@ class RegistrationHandler(BaseHandler): admin=admin, user_type=user_type, ) + + @defer.inlineCallbacks + def register_device(self, user_id, device_id, initial_display_name, + is_guest=False): + """Register a device for a user and generate an access token. + + Args: + user_id (str): full canonical @user:id + device_id (str|None): The device ID to check, or None to generate + a new one. + initial_display_name (str|None): An optional display name for the + device. + is_guest (bool): Whether this is a guest account + + Returns: + defer.Deferred[tuple[str, str]]: Tuple of device ID and access token + """ + + if self.hs.config.worker_app: + r = yield self._register_device_client( + user_id=user_id, + device_id=device_id, + initial_display_name=initial_display_name, + is_guest=is_guest, + ) + defer.returnValue((r["device_id"], r["access_token"])) + else: + device_id = yield self.device_handler.check_device_registered( + user_id, device_id, initial_display_name + ) + if is_guest: + access_token = self.macaroon_gen.generate_access_token( + user_id, ["guest = true"] + ) + else: + access_token = yield self._auth_handler.get_access_token_for_user_id( + user_id, device_id=device_id, + ) + + defer.returnValue((device_id, access_token)) diff --git a/synapse/replication/http/login.py b/synapse/replication/http/login.py index 797f6aabd1..1590eca317 100644 --- a/synapse/replication/http/login.py +++ b/synapse/replication/http/login.py @@ -35,9 +35,7 @@ class RegisterDeviceReplicationServlet(ReplicationEndpoint): def __init__(self, hs): super(RegisterDeviceReplicationServlet, self).__init__(hs) - self.auth_handler = hs.get_auth_handler() - self.device_handler = hs.get_device_handler() - self.macaroon_gen = hs.get_macaroon_generator() + self.registration_handler = hs.get_handlers().registration_handler @staticmethod def _serialize_payload(user_id, device_id, initial_display_name, is_guest): @@ -62,19 +60,10 @@ class RegisterDeviceReplicationServlet(ReplicationEndpoint): initial_display_name = content["initial_display_name"] is_guest = content["is_guest"] - device_id = yield self.device_handler.check_device_registered( - user_id, device_id, initial_display_name, + device_id, access_token = yield self.registration_handler.register_device( + user_id, device_id, initial_display_name, is_guest, ) - if is_guest: - access_token = self.macaroon_gen.generate_access_token( - user_id, ["guest = true"] - ) - else: - access_token = yield self.auth_handler.get_access_token_for_user_id( - user_id, device_id=device_id, - ) - defer.returnValue((200, { "device_id": device_id, "access_token": access_token, diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 942e4d3816..4a5775083f 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -94,7 +94,7 @@ class LoginRestServlet(ClientV1RestServlet): self.jwt_algorithm = hs.config.jwt_algorithm self.cas_enabled = hs.config.cas_enabled self.auth_handler = self.hs.get_auth_handler() - self.device_handler = self.hs.get_device_handler() + self.registration_handler = hs.get_handlers().registration_handler self.handlers = hs.get_handlers() self._well_known_builder = WellKnownBuilder(hs) @@ -220,11 +220,10 @@ class LoginRestServlet(ClientV1RestServlet): login_submission, ) - device_id = yield self._register_device( - canonical_user_id, login_submission, - ) - access_token = yield auth_handler.get_access_token_for_user_id( - canonical_user_id, device_id, + device_id = login_submission.get("device_id") + initial_display_name = login_submission.get("initial_device_display_name") + device_id, access_token = yield self.registration_handler.register_device( + canonical_user_id, device_id, initial_display_name, ) result = { @@ -246,10 +245,13 @@ class LoginRestServlet(ClientV1RestServlet): user_id = ( yield auth_handler.validate_short_term_login_token_and_get_user_id(token) ) - device_id = yield self._register_device(user_id, login_submission) - access_token = yield auth_handler.get_access_token_for_user_id( - user_id, device_id, + + device_id = login_submission.get("device_id") + initial_display_name = login_submission.get("initial_device_display_name") + device_id, access_token = yield self.registration_handler.register_device( + user_id, device_id, initial_display_name, ) + result = { "user_id": user_id, # may have changed "access_token": access_token, @@ -286,11 +288,10 @@ class LoginRestServlet(ClientV1RestServlet): auth_handler = self.auth_handler registered_user_id = yield auth_handler.check_user_exists(user_id) if registered_user_id: - device_id = yield self._register_device( - registered_user_id, login_submission - ) - access_token = yield auth_handler.get_access_token_for_user_id( - registered_user_id, device_id, + device_id = login_submission.get("device_id") + initial_display_name = login_submission.get("initial_device_display_name") + device_id, access_token = yield self.registration_handler.register_device( + registered_user_id, device_id, initial_display_name, ) result = { @@ -299,12 +300,16 @@ class LoginRestServlet(ClientV1RestServlet): "home_server": self.hs.hostname, } else: - # TODO: we should probably check that the register isn't going - # to fonx/change our user_id before registering the device - device_id = yield self._register_device(user_id, login_submission) user_id, access_token = ( yield self.handlers.registration_handler.register(localpart=user) ) + + device_id = login_submission.get("device_id") + initial_display_name = login_submission.get("initial_device_display_name") + device_id, access_token = yield self.registration_handler.register_device( + registered_user_id, device_id, initial_display_name, + ) + result = { "user_id": user_id, # may have changed "access_token": access_token, @@ -313,26 +318,6 @@ class LoginRestServlet(ClientV1RestServlet): defer.returnValue(result) - def _register_device(self, user_id, login_submission): - """Register a device for a user. - - This is called after the user's credentials have been validated, but - before the access token has been issued. - - Args: - (str) user_id: full canonical @user:id - (object) login_submission: dictionary supplied to /login call, from - which we pull device_id and initial_device_name - Returns: - defer.Deferred: (str) device_id - """ - device_id = login_submission.get("device_id") - initial_display_name = login_submission.get( - "initial_device_display_name") - return self.device_handler.check_device_registered( - user_id, device_id, initial_display_name - ) - class CasRedirectServlet(RestServlet): PATTERNS = client_path_patterns("/login/(cas|sso)/redirect") diff --git a/synapse/rest/client/v2_alpha/register.py b/synapse/rest/client/v2_alpha/register.py index c52280c50c..c1cdb8f9c8 100644 --- a/synapse/rest/client/v2_alpha/register.py +++ b/synapse/rest/client/v2_alpha/register.py @@ -33,7 +33,6 @@ from synapse.http.servlet import ( parse_json_object_from_request, parse_string, ) -from synapse.replication.http.login import RegisterDeviceReplicationServlet from synapse.util.msisdn import phone_number_to_msisdn from synapse.util.ratelimitutils import FederationRateLimiter from synapse.util.threepids import check_3pid_allowed @@ -193,13 +192,6 @@ class RegisterRestServlet(RestServlet): self.room_member_handler = hs.get_room_member_handler() self.macaroon_gen = hs.get_macaroon_generator() - if self.hs.config.worker_app: - self._register_device_client = ( - RegisterDeviceReplicationServlet.make_client(hs) - ) - else: - self.device_handler = hs.get_device_handler() - @interactive_auth_handler @defer.inlineCallbacks def on_POST(self, request): @@ -642,7 +634,7 @@ class RegisterRestServlet(RestServlet): if not params.get("inhibit_login", False): device_id = params.get("device_id") initial_display_name = params.get("initial_device_display_name") - device_id, access_token = yield self._register_device( + device_id, access_token = yield self.registration_handler.register_device( user_id, device_id, initial_display_name, is_guest=False, ) @@ -652,43 +644,6 @@ class RegisterRestServlet(RestServlet): }) defer.returnValue(result) - @defer.inlineCallbacks - def _register_device(self, user_id, device_id, initial_display_name, - is_guest): - """Register a device for a user and generate an access token. - - Args: - user_id (str): full canonical @user:id - device_id (str|None): The device ID to check, or None to generate - a new one. - initial_display_name (str|None): An optional display name for the - device. - is_guest (bool): Whether this is a guest account - Returns: - defer.Deferred[tuple[str, str]]: Tuple of device ID and access token - """ - if self.hs.config.worker_app: - r = yield self._register_device_client( - user_id=user_id, - device_id=device_id, - initial_display_name=initial_display_name, - is_guest=is_guest, - ) - defer.returnValue((r["device_id"], r["access_token"])) - else: - device_id = yield self.device_handler.check_device_registered( - user_id, device_id, initial_display_name - ) - if is_guest: - access_token = self.macaroon_gen.generate_access_token( - user_id, ["guest = true"] - ) - else: - access_token = yield self.auth_handler.get_access_token_for_user_id( - user_id, device_id=device_id, - ) - defer.returnValue((device_id, access_token)) - @defer.inlineCallbacks def _do_guest_registration(self, params): if not self.hs.config.allow_guest_access: @@ -702,7 +657,7 @@ class RegisterRestServlet(RestServlet): # we have nowhere to store it. device_id = synapse.api.auth.GUEST_DEVICE_ID initial_display_name = params.get("initial_device_display_name") - device_id, access_token = yield self._register_device( + device_id, access_token = yield self.registration_handler.register_device( user_id, device_id, initial_display_name, is_guest=True, ) diff --git a/tests/rest/client/v2_alpha/test_register.py b/tests/rest/client/v2_alpha/test_register.py index 18080ebfd6..906b348d3e 100644 --- a/tests/rest/client/v2_alpha/test_register.py +++ b/tests/rest/client/v2_alpha/test_register.py @@ -1,10 +1,7 @@ import json -from mock import Mock - -from twisted.python import failure - -from synapse.api.errors import InteractiveAuthIncompleteError +from synapse.api.constants import LoginType +from synapse.appservice import ApplicationService from synapse.rest.client.v2_alpha.register import register_servlets from tests import unittest @@ -18,61 +15,28 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): self.url = b"/_matrix/client/r0/register" - self.appservice = None - self.auth = Mock( - get_appservice_by_req=Mock(side_effect=lambda x: self.appservice) - ) - - self.auth_result = failure.Failure(InteractiveAuthIncompleteError(None)) - self.auth_handler = Mock( - check_auth=Mock(side_effect=lambda x, y, z: self.auth_result), - get_session_data=Mock(return_value=None), - ) - self.registration_handler = Mock() - self.identity_handler = Mock() - self.login_handler = Mock() - self.device_handler = Mock() - - def check_device_registered(user_id, device_id, initial_display_name): - # Just echo back the given device ID, or return a new "FAKE" device - # ID - if device_id: - return device_id - else: - return "FAKE" - - self.device_handler.check_device_registered = Mock( - side_effect=check_device_registered, - ) - - self.datastore = Mock(return_value=Mock()) - self.datastore.get_current_state_deltas = Mock(return_value=[]) - - # do the dance to hook it up to the hs global - self.handlers = Mock( - registration_handler=self.registration_handler, - identity_handler=self.identity_handler, - login_handler=self.login_handler, - ) self.hs = self.setup_test_homeserver() - self.hs.get_auth = Mock(return_value=self.auth) - self.hs.get_handlers = Mock(return_value=self.handlers) - self.hs.get_auth_handler = Mock(return_value=self.auth_handler) - self.hs.get_device_handler = Mock(return_value=self.device_handler) - self.hs.get_datastore = Mock(return_value=self.datastore) self.hs.config.enable_registration = True self.hs.config.registrations_require_3pid = [] self.hs.config.auto_join_rooms = [] + self.hs.config.enable_registration_captcha = False return self.hs def test_POST_appservice_registration_valid(self): - user_id = "@kermit:muppet" - token = "kermits_access_token" - self.appservice = {"id": "1234"} - self.registration_handler.appservice_register = Mock(return_value=user_id) - self.auth_handler.get_access_token_for_user_id = Mock(return_value=token) - request_data = json.dumps({"username": "kermit"}) + user_id = "@as_user_kermit:test" + as_token = "i_am_an_app_service" + + appservice = ApplicationService( + as_token, self.hs.config.hostname, + id="1234", + namespaces={ + "users": [{"regex": r"@as_user.*", "exclusive": True}], + }, + ) + + self.hs.get_datastore().services_cache.append(appservice) + request_data = json.dumps({"username": "as_user_kermit"}) request, channel = self.make_request( b"POST", self.url + b"?access_token=i_am_an_app_service", request_data @@ -82,7 +46,6 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): self.assertEquals(channel.result["code"], b"200", channel.result) det_data = { "user_id": user_id, - "access_token": token, "home_server": self.hs.hostname, } self.assertDictContainsSubset(det_data, channel.json_body) @@ -114,37 +77,30 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): self.assertEquals(channel.json_body["error"], "Invalid username") def test_POST_user_valid(self): - user_id = "@kermit:muppet" - token = "kermits_access_token" + user_id = "@kermit:test" device_id = "frogfone" - params = {"username": "kermit", "password": "monkey", "device_id": device_id} + params = { + "username": "kermit", + "password": "monkey", + "device_id": device_id, + "auth": {"type": LoginType.DUMMY}, + } request_data = json.dumps(params) - self.registration_handler.check_username = Mock(return_value=True) - self.auth_result = (None, params, None) - self.registration_handler.register = Mock(return_value=(user_id, None)) - self.auth_handler.get_access_token_for_user_id = Mock(return_value=token) - request, channel = self.make_request(b"POST", self.url, request_data) self.render(request) det_data = { "user_id": user_id, - "access_token": token, "home_server": self.hs.hostname, "device_id": device_id, } self.assertEquals(channel.result["code"], b"200", channel.result) self.assertDictContainsSubset(det_data, channel.json_body) - self.auth_handler.get_login_tuple_for_user_id( - user_id, device_id=device_id, initial_device_display_name=None - ) def test_POST_disabled_registration(self): self.hs.config.enable_registration = False request_data = json.dumps({"username": "kermit", "password": "monkey"}) - self.registration_handler.check_username = Mock(return_value=True) self.auth_result = (None, {"username": "kermit", "password": "monkey"}, None) - self.registration_handler.register = Mock(return_value=("@user:id", "t")) request, channel = self.make_request(b"POST", self.url, request_data) self.render(request) @@ -153,16 +109,13 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase): self.assertEquals(channel.json_body["error"], "Registration has been disabled") def test_POST_guest_registration(self): - user_id = "a@b" self.hs.config.macaroon_secret_key = "test" self.hs.config.allow_guest_access = True - self.registration_handler.register = Mock(return_value=(user_id, None)) request, channel = self.make_request(b"POST", self.url + b"?kind=guest", b"{}") self.render(request) det_data = { - "user_id": user_id, "home_server": self.hs.hostname, "device_id": "guest_device", } -- cgit 1.5.1 From d328a93b51d22039e361178a5fcf952d9735cd3f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 16:53:56 +0000 Subject: Fixup error handling and message --- synapse/config/metrics.py | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/synapse/config/metrics.py b/synapse/config/metrics.py index fc72e32d20..35f1074765 100644 --- a/synapse/config/metrics.py +++ b/synapse/config/metrics.py @@ -16,11 +16,8 @@ from ._base import Config, ConfigError MISSING_SENTRY = ( - """Missing sentry_sdk library. This is required to enable sentry + """Missing sentry-sdk library. This is required to enable sentry integration. - - Install by running: - pip install sentry_sdk """ ) @@ -39,7 +36,11 @@ class MetricsConfig(Config): except ImportError: raise ConfigError(MISSING_SENTRY) - self.sentry_dsn = config["sentry"]["dsn"] + self.sentry_dsn = config["sentry"].get("dsn") + if not self.sentry_dsn: + raise ConfigError( + "sentry.dsn field is required when sentry integration is enabled", + ) def default_config(self, report_stats=None, **kwargs): res = """\ -- cgit 1.5.1 From 4cc4400b4deabed113548b296656d0e6e404dbde Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 17:19:01 +0000 Subject: Split /login into client_reader --- synapse/app/client_reader.py | 2 + synapse/storage/registration.py | 82 ++++++++++++++++++++--------------------- 2 files changed, 43 insertions(+), 41 deletions(-) diff --git a/synapse/app/client_reader.py b/synapse/app/client_reader.py index 9250b6c239..043b48f8f3 100644 --- a/synapse/app/client_reader.py +++ b/synapse/app/client_reader.py @@ -40,6 +40,7 @@ from synapse.replication.slave.storage.registration import SlavedRegistrationSto from synapse.replication.slave.storage.room import RoomStore from synapse.replication.slave.storage.transactions import SlavedTransactionStore from synapse.replication.tcp.client import ReplicationClientHandler +from synapse.rest.client.v1.login import LoginRestServlet from synapse.rest.client.v1.room import ( JoinedRoomMemberListRestServlet, PublicRoomListRestServlet, @@ -94,6 +95,7 @@ class ClientReaderServer(HomeServer): RoomStateRestServlet(self).register(resource) RoomEventContextServlet(self).register(resource) RegisterRestServlet(self).register(resource) + LoginRestServlet(self).register(resource) resources.update({ "/_matrix/client/r0": resource, diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py index 3bc5def48e..9b9572890b 100644 --- a/synapse/storage/registration.py +++ b/synapse/storage/registration.py @@ -254,6 +254,47 @@ class RegistrationWorkerStore(SQLBaseStore): defer.returnValue(ret["guest_access_token"]) defer.returnValue(None) + @defer.inlineCallbacks + def get_user_id_by_threepid(self, medium, address): + """Returns user id from threepid + + Args: + medium (str): threepid medium e.g. email + address (str): threepid address e.g. me@example.com + + Returns: + Deferred[str|None]: user id or None if no user id/threepid mapping exists + """ + user_id = yield self.runInteraction( + "get_user_id_by_threepid", self.get_user_id_by_threepid_txn, + medium, address + ) + defer.returnValue(user_id) + + def get_user_id_by_threepid_txn(self, txn, medium, address): + """Returns user id from threepid + + Args: + txn (cursor): + medium (str): threepid medium e.g. email + address (str): threepid address e.g. me@example.com + + Returns: + str|None: user id or None if no user id/threepid mapping exists + """ + ret = self._simple_select_one_txn( + txn, + "user_threepids", + { + "medium": medium, + "address": address + }, + ['user_id'], True + ) + if ret: + return ret['user_id'] + return None + class RegistrationStore(RegistrationWorkerStore, background_updates.BackgroundUpdateStore): @@ -613,47 +654,6 @@ class RegistrationStore(RegistrationWorkerStore, ) defer.returnValue(ret) - @defer.inlineCallbacks - def get_user_id_by_threepid(self, medium, address): - """Returns user id from threepid - - Args: - medium (str): threepid medium e.g. email - address (str): threepid address e.g. me@example.com - - Returns: - Deferred[str|None]: user id or None if no user id/threepid mapping exists - """ - user_id = yield self.runInteraction( - "get_user_id_by_threepid", self.get_user_id_by_threepid_txn, - medium, address - ) - defer.returnValue(user_id) - - def get_user_id_by_threepid_txn(self, txn, medium, address): - """Returns user id from threepid - - Args: - txn (cursor): - medium (str): threepid medium e.g. email - address (str): threepid address e.g. me@example.com - - Returns: - str|None: user id or None if no user id/threepid mapping exists - """ - ret = self._simple_select_one_txn( - txn, - "user_threepids", - { - "medium": medium, - "address": address - }, - ['user_id'], True - ) - if ret: - return ret['user_id'] - return None - def user_delete_threepid(self, user_id, medium, address): return self._simple_delete( "user_threepids", -- cgit 1.5.1 From 128902d60a79b3fc36be084413e742ecf9300c82 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 17:21:51 +0000 Subject: Update worker docs --- changelog.d/4666.feature | 2 +- docs/workers.rst | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/changelog.d/4666.feature b/changelog.d/4666.feature index 421060f9f9..b3a3915eb0 100644 --- a/changelog.d/4666.feature +++ b/changelog.d/4666.feature @@ -1 +1 @@ -Allow registration to be handled by a worker instance. +Allow registration and login to be handled by a worker instance. diff --git a/docs/workers.rst b/docs/workers.rst index 6ce7d88c11..3ba5879f76 100644 --- a/docs/workers.rst +++ b/docs/workers.rst @@ -222,11 +222,12 @@ following regular expressions:: ^/_matrix/client/(api/v1|r0|unstable)/rooms/.*/context/.*$ ^/_matrix/client/(api/v1|r0|unstable)/rooms/.*/members$ ^/_matrix/client/(api/v1|r0|unstable)/rooms/.*/state$ + ^/_matrix/client/(api/v1|r0|unstable)/login$ Additionally, the following REST endpoints can be handled, but all requests must be routed to the same instance:: - ^/_matrix/client/(api/v1|r0|unstable)/register$ + ^/_matrix/client/(r0|unstable)/register$ ``synapse.app.user_dir`` -- cgit 1.5.1 From f3ab0b2390b000e86c9b86ce1b156b28c4049858 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 17:22:01 +0000 Subject: Newsfile --- changelog.d/4670.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4670.feature diff --git a/changelog.d/4670.feature b/changelog.d/4670.feature new file mode 100644 index 0000000000..b3a3915eb0 --- /dev/null +++ b/changelog.d/4670.feature @@ -0,0 +1 @@ +Allow registration and login to be handled by a worker instance. -- cgit 1.5.1 From f8b9ca53cedced14c5687581d981b5473bb7054d Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 18 Feb 2019 16:56:34 +0000 Subject: Move member event processing and changelog fix --- changelog.d/4642.bugfix | 1 - changelog.d/4642.feature | 1 + synapse/handlers/room.py | 16 +++++++--------- 3 files changed, 8 insertions(+), 10 deletions(-) delete mode 100644 changelog.d/4642.bugfix create mode 100644 changelog.d/4642.feature diff --git a/changelog.d/4642.bugfix b/changelog.d/4642.bugfix deleted file mode 100644 index bfbf95bcbb..0000000000 --- a/changelog.d/4642.bugfix +++ /dev/null @@ -1 +0,0 @@ -Transfer bans on room upgrade. \ No newline at end of file diff --git a/changelog.d/4642.feature b/changelog.d/4642.feature new file mode 100644 index 0000000000..bfbf95bcbb --- /dev/null +++ b/changelog.d/4642.feature @@ -0,0 +1 @@ +Transfer bans on room upgrade. \ No newline at end of file diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 2d24c115b6..0676e7f626 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -285,7 +285,6 @@ class RoomCreationHandler(BaseHandler): (EventTypes.RoomAvatar, ""), (EventTypes.Encryption, ""), (EventTypes.ServerACL, ""), - (EventTypes.Member, None), ) old_room_state_ids = yield self.store.get_filtered_current_state_ids( @@ -294,15 +293,9 @@ class RoomCreationHandler(BaseHandler): # map from event_id to BaseEvent old_room_state_events = yield self.store.get_events(old_room_state_ids.values()) - member_events = [] for k, old_event_id in iteritems(old_room_state_ids): old_event = old_room_state_events.get(old_event_id) if old_event: - # Do membership events later - if ("membership" in old_event.content): - member_events.append(old_event) - continue - initial_state[k] = old_event.content yield self._send_events_for_new_room( @@ -319,9 +312,14 @@ class RoomCreationHandler(BaseHandler): ) # Transfer membership events - for old_event in member_events: + old_room_member_state_ids = yield self.store.get_filtered_current_state_ids( + old_room_id, StateFilter.from_types([(EventTypes.Member, None)]), + ) + + # map from event_id to BaseEvent + old_room_member_state_events = yield self.store.get_events(old_room_member_state_ids.values()) + for k, old_event in iteritems(old_room_member_state_events): # Only transfer ban events - logger.info("Event type: " + str(old_event.content)) if ("membership" in old_event.content and old_event.content["membership"] == "ban"): yield self.room_member_handler.update_membership( -- cgit 1.5.1 From a9b5ea6fc1e26ff791118b67af01fdad8e9c68c8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 17:53:31 +0000 Subject: Batch cache invalidation over replication Currently whenever the current state changes in a room invalidate a lot of caches, which cause *a lot* of traffic over replication. Instead, lets batch up all those invalidations and send a single poke down the replication streams. Hopefully this will reduce load on the master process by substantially reducing traffic. --- synapse/replication/slave/storage/_base.py | 19 ++++++---- synapse/storage/_base.py | 57 +++++++++++++++++++++++++++++- synapse/storage/events.py | 25 +------------ 3 files changed, 69 insertions(+), 32 deletions(-) diff --git a/synapse/replication/slave/storage/_base.py b/synapse/replication/slave/storage/_base.py index 2d81d49e9a..1353a32d00 100644 --- a/synapse/replication/slave/storage/_base.py +++ b/synapse/replication/slave/storage/_base.py @@ -17,7 +17,7 @@ import logging import six -from synapse.storage._base import SQLBaseStore +from synapse.storage._base import _CURRENT_STATE_CACHE_NAME, SQLBaseStore from synapse.storage.engines import PostgresEngine from ._slaved_id_tracker import SlavedIdTracker @@ -54,12 +54,17 @@ class BaseSlavedStore(SQLBaseStore): if stream_name == "caches": self._cache_id_gen.advance(token) for row in rows: - try: - getattr(self, row.cache_func).invalidate(tuple(row.keys)) - except AttributeError: - # We probably haven't pulled in the cache in this worker, - # which is fine. - pass + if row.cache_func == _CURRENT_STATE_CACHE_NAME: + room_id = row.keys[0] + members_changed = set(row.keys[1:]) + self._invalidate_state_caches(room_id, members_changed) + else: + try: + getattr(self, row.cache_func).invalidate(tuple(row.keys)) + except AttributeError: + # We probably haven't pulled in the cache in this worker, + # which is fine. + pass def _invalidate_cache_and_stream(self, txn, cache_func, keys): txn.call_after(cache_func.invalidate, keys) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index e124161845..f7c6d714ab 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -28,6 +28,7 @@ from twisted.internet import defer from synapse.api.errors import StoreError from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage.engines import PostgresEngine, Sqlite3Engine +from synapse.types import get_domain_from_id from synapse.util.caches.descriptors import Cache from synapse.util.logcontext import LoggingContext, PreserveLoggingContext from synapse.util.stringutils import exception_to_unicode @@ -64,6 +65,10 @@ UNIQUE_INDEX_BACKGROUND_UPDATES = { "event_search": "event_search_event_id_idx", } +# This is a special cache name we use to batch multiple invalidations of caches +# based on the current state when notifying workers over replication. +_CURRENT_STATE_CACHE_NAME = "cs_cache_fake" + class LoggingTransaction(object): """An object that almost-transparently proxies for the 'txn' object @@ -1184,6 +1189,56 @@ class SQLBaseStore(object): be invalidated. """ txn.call_after(cache_func.invalidate, keys) + self._send_invalidation_to_replication(txn, cache_func.__name__, keys) + + def _invalidate_state_caches_and_stream(self, txn, room_id, members_changed): + """Special case invalidation of caches based on current state. + + We special case this so that we can batch the cache invalidations into a + single replication poke. + + Args: + txn + room_id (str): Room were state changed + members_changed (set[str]): The user_ids of members that have changed + """ + txn.call_after(self._invalidate_state_caches, room_id, members_changed) + + keys = [room_id] + keys.extend(members_changed) + self._send_invalidation_to_replication( + txn, _CURRENT_STATE_CACHE_NAME, keys, + ) + + def _invalidate_state_caches(self, room_id, members_changed): + """Invalidates caches that are based on the current state, but does + not stream invalidations down replication. + + Args: + room_id (str): Room were state changed + members_changed (set[str]): The user_ids of members that have changed + """ + for member in members_changed: + self.get_rooms_for_user_with_stream_ordering.invalidate((member,)) + + for host in set(get_domain_from_id(u) for u in members_changed): + self.is_host_joined.invalidate((room_id, host)) + self.was_host_joined.invalidate((room_id, host)) + + self.get_users_in_room.invalidate((room_id,)) + self.get_room_summary.invalidate((room_id,)) + self.get_current_state_ids.invalidate((room_id,)) + + def _send_invalidation_to_replication(self, txn, cache_name, keys): + """Notifies replication that given cache has been invalidated. + + Note that this does *not* invalidate the cache locally. + + Args: + txn + cache_name (str) + keys (list[str]) + """ if isinstance(self.database_engine, PostgresEngine): # get_next() returns a context manager which is designed to wrap @@ -1201,7 +1256,7 @@ class SQLBaseStore(object): table="cache_invalidation_stream", values={ "stream_id": stream_id, - "cache_func": cache_func.__name__, + "cache_func": cache_name, "keys": list(keys), "invalidation_ts": self.clock.time_msec(), } diff --git a/synapse/storage/events.py b/synapse/storage/events.py index 81b250480d..06db9e56e6 100644 --- a/synapse/storage/events.py +++ b/synapse/storage/events.py @@ -979,30 +979,7 @@ class EventsStore(StateGroupWorkerStore, EventFederationStore, EventsWorkerStore if ev_type == EventTypes.Member ) - for member in members_changed: - self._invalidate_cache_and_stream( - txn, self.get_rooms_for_user_with_stream_ordering, (member,) - ) - - for host in set(get_domain_from_id(u) for u in members_changed): - self._invalidate_cache_and_stream( - txn, self.is_host_joined, (room_id, host) - ) - self._invalidate_cache_and_stream( - txn, self.was_host_joined, (room_id, host) - ) - - self._invalidate_cache_and_stream( - txn, self.get_users_in_room, (room_id,) - ) - - self._invalidate_cache_and_stream( - txn, self.get_room_summary, (room_id,) - ) - - self._invalidate_cache_and_stream( - txn, self.get_current_state_ids, (room_id,) - ) + self._invalidate_state_caches_and_stream(txn, room_id, members_changed) def _update_forward_extremities_txn(self, txn, new_forward_extremities, max_stream_order): -- cgit 1.5.1 From 92e6fb5c89eace7aedca0cd73900d4aa44129af6 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 18 Feb 2019 17:58:17 +0000 Subject: Newsfile --- changelog.d/4671.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4671.misc diff --git a/changelog.d/4671.misc b/changelog.d/4671.misc new file mode 100644 index 0000000000..4dc18378e7 --- /dev/null +++ b/changelog.d/4671.misc @@ -0,0 +1 @@ +Improve replication performance by reducing cache invalidation traffic. -- cgit 1.5.1 From 34ac75ce2c576f32a58fb7070fd52279cba70515 Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Mon, 18 Feb 2019 18:23:37 +0000 Subject: lint --- synapse/handlers/room.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/room.py b/synapse/handlers/room.py index 0676e7f626..67b15697fd 100644 --- a/synapse/handlers/room.py +++ b/synapse/handlers/room.py @@ -317,7 +317,9 @@ class RoomCreationHandler(BaseHandler): ) # map from event_id to BaseEvent - old_room_member_state_events = yield self.store.get_events(old_room_member_state_ids.values()) + old_room_member_state_events = yield self.store.get_events( + old_room_member_state_ids.values(), + ) for k, old_event in iteritems(old_room_member_state_events): # Only transfer ban events if ("membership" in old_event.content and -- cgit 1.5.1 From 561eebe170d02047e92141fa04b70313beb2ac0b Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Tue, 19 Feb 2019 16:18:05 +1100 Subject: fix to use makeContext so that we don't need to rebuild the certificateoptions each time --- synapse/crypto/context_factory.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 85f2848fb1..49cbc7098f 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -1,4 +1,5 @@ # Copyright 2014-2016 OpenMarket Ltd +# Copyright 2019 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. @@ -11,6 +12,7 @@ # 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 zope.interface import implementer @@ -105,9 +107,7 @@ class ClientTLSOptions(object): self._hostnameBytes = _idnaBytes(hostname) self._sendSNI = True - ctx.set_info_callback( - _tolerateErrors(self._identityVerifyingInfoCallback) - ) + ctx.set_info_callback(_tolerateErrors(self._identityVerifyingInfoCallback)) def clientConnectionForTLS(self, tlsProtocol): context = self._ctx @@ -128,10 +128,8 @@ class ClientTLSOptionsFactory(object): def __init__(self, config): # We don't use config options yet - pass + self._options = CertificateOptions(verify=False) def get_options(self, host): - return ClientTLSOptions( - host, - CertificateOptions(verify=False).getContext() - ) + # Use _makeContext so that we get a fresh OpenSSL CTX each time. + return ClientTLSOptions(host, self._options._makeContext()) -- cgit 1.5.1 From 2b2466f78bafe1d7083d32f1495f20746159b470 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Tue, 19 Feb 2019 16:18:48 +1100 Subject: changelog --- changelog.d/4674.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4674.misc diff --git a/changelog.d/4674.misc b/changelog.d/4674.misc new file mode 100644 index 0000000000..84630bb201 --- /dev/null +++ b/changelog.d/4674.misc @@ -0,0 +1 @@ +Reduce the overhead of creating outbound federation connections over TLS by caching the TLS client options. -- cgit 1.5.1 From 0869f01e74b356e2f5f617a61aa6e0b6b2c08f79 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 19 Feb 2019 09:52:03 +0000 Subject: Test against Postgres 9.5 as well as 9.4 Postgres 9.5 is the first to support UPSERTs, so we should really run against it as well as 9.4. --- .travis.yml | 46 ++++++++++++++++++++++++++++++++-------------- changelog.d/4676.misc | 1 + 2 files changed, 33 insertions(+), 14 deletions(-) create mode 100644 changelog.d/4676.misc diff --git a/.travis.yml b/.travis.yml index 3cab77ce4d..f6c91c2621 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,9 +12,6 @@ cache: # - $HOME/.cache/pip/wheels -addons: - postgresql: "9.4" - # don't clone the whole repo history, one commit will do git: depth: 1 @@ -25,6 +22,7 @@ branches: - master - develop - /^release-v/ + - rav/pg95 # When running the tox environments that call Twisted Trial, we can pass the -j # flag to run the tests concurrently. We set this to 2 for CPU bound tests @@ -32,36 +30,53 @@ branches: matrix: fast_finish: true include: - - python: 2.7 - env: TOX_ENV=packaging - - - python: 3.6 - env: TOX_ENV="pep8,check_isort" + - name: "pep8" + python: 3.6 + env: TOX_ENV="pep8,check_isort,packaging" - - python: 2.7 + - name: "py2.7 / sqlite" + python: 2.7 env: TOX_ENV=py27,codecov TRIAL_FLAGS="-j 2" - - python: 2.7 + - name: "py2.7 / sqlite / olddeps" + python: 2.7 env: TOX_ENV=py27-old TRIAL_FLAGS="-j 2" - - python: 2.7 + - name: "py2.7 / postgres9.5" + python: 2.7 + addons: + postgresql: "9.5" env: TOX_ENV=py27-postgres,codecov TRIAL_FLAGS="-j 4" services: - postgresql - - python: 3.5 + - name: "py3.5 / sqlite" + python: 3.5 env: TOX_ENV=py35,codecov TRIAL_FLAGS="-j 2" - - python: 3.6 + - name: "py3.6 / sqlite" + python: 3.6 env: TOX_ENV=py36,codecov TRIAL_FLAGS="-j 2" - - python: 3.6 + - name: "py3.6 / postgres9.4" + python: 3.6 + addons: + postgresql: "9.4" + env: TOX_ENV=py36-postgres TRIAL_FLAGS="-j 4" + services: + - postgresql + + - name: "py3.6 / postgres9.5" + python: 3.6 + addons: + postgresql: "9.5" env: TOX_ENV=py36-postgres,codecov TRIAL_FLAGS="-j 4" services: - postgresql - # we only need to check for the newsfragment if it's a PR build if: type = pull_request + name: "check-newsfragment" python: 3.6 env: TOX_ENV=check-newsfragment script: @@ -70,6 +85,9 @@ matrix: - tox -e $TOX_ENV install: + # this just logs the postgres version we will be testing against (if any) + - psql -At -U postgres -c 'select version();' + - pip install tox # if we don't have python3.6 in this environment, travis unhelpfully gives us diff --git a/changelog.d/4676.misc b/changelog.d/4676.misc new file mode 100644 index 0000000000..a250558e69 --- /dev/null +++ b/changelog.d/4676.misc @@ -0,0 +1 @@ +Test against Postgres 9.5 as well as 9.4 -- cgit 1.5.1 From 107aeb6915927a4278290dd3bdc5e01497a0dce0 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 19 Feb 2019 10:18:48 +0000 Subject: misc->feature --- changelog.d/4674.feature | 1 + changelog.d/4674.misc | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) create mode 100644 changelog.d/4674.feature delete mode 100644 changelog.d/4674.misc diff --git a/changelog.d/4674.feature b/changelog.d/4674.feature new file mode 100644 index 0000000000..84630bb201 --- /dev/null +++ b/changelog.d/4674.feature @@ -0,0 +1 @@ +Reduce the overhead of creating outbound federation connections over TLS by caching the TLS client options. diff --git a/changelog.d/4674.misc b/changelog.d/4674.misc deleted file mode 100644 index 84630bb201..0000000000 --- a/changelog.d/4674.misc +++ /dev/null @@ -1 +0,0 @@ -Reduce the overhead of creating outbound federation connections over TLS by caching the TLS client options. -- cgit 1.5.1 From a8626901cd384f263c8ae578466f95f0c3cceb95 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 19 Feb 2019 10:54:33 +0000 Subject: Fetch ACME domain into an instance member --- synapse/handlers/acme.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/handlers/acme.py b/synapse/handlers/acme.py index f8a786a4da..813777bf18 100644 --- a/synapse/handlers/acme.py +++ b/synapse/handlers/acme.py @@ -56,6 +56,7 @@ class AcmeHandler(object): def __init__(self, hs): self.hs = hs self.reactor = hs.get_reactor() + self._acme_domain = hs.config.acme_domain @defer.inlineCallbacks def start_listening(self): @@ -123,15 +124,15 @@ class AcmeHandler(object): @defer.inlineCallbacks def provision_certificate(self): - logger.warning("Reprovisioning %s", self.hs.config.acme_domain) + logger.warning("Reprovisioning %s", self._acme_domain) try: - yield self._issuer.issue_cert(self.hs.config.acme_domain) + yield self._issuer.issue_cert(self._acme_domain) except Exception: logger.exception("Fail!") raise - logger.warning("Reprovisioned %s, saving.", self.hs.config.acme_domain) - cert_chain = self._store.certs[self.hs.config.acme_domain] + logger.warning("Reprovisioned %s, saving.", self._acme_domain) + cert_chain = self._store.certs[self._acme_domain] try: with open(self.hs.config.tls_private_key_file, "wb") as private_key_file: -- cgit 1.5.1 From 5a707a2f9a82ed67f5339ff2c6898790341ce20f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 19 Feb 2019 10:59:26 +0000 Subject: Improve config documentation --- synapse/config/tls.py | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index a3a5ece681..38425bb056 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -230,9 +230,17 @@ class TlsConfig(Config): # # reprovision_threshold: 30 - # What domain the certificate should be for. Only useful if - # delegation via a /.well-known/matrix/server file is being used. - # Defaults to the server_name configuration parameter. + # The domain that the certificate should be for. Normally this + # should be the same as your Matrix domain (i.e., 'server_name'), but, + # by putting a file at 'https:///.well-known/matrix/server', + # you can delegate incoming traffic to another server. If you do that, + # you should give the target of the delegation here. + # + # For example: if your 'server_name' is 'example.com', but + # 'https://example.com/.well-known/matrix/server' delegates to + # 'matrix.example.com', you should put 'matrix.example.com' here. + # + # If not set, defaults to your 'server_name'. # # domain: matrix.example.com -- cgit 1.5.1 From bc8fa1509d3885d111a2ef98e12e9ce66a19a3a8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 19 Feb 2019 11:24:59 +0000 Subject: Documentation --- docs/tcp_replication.rst | 21 ++++++++++++++++++++- synapse/storage/_base.py | 8 ++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/docs/tcp_replication.rst b/docs/tcp_replication.rst index 62225ba6f4..852f1113a3 100644 --- a/docs/tcp_replication.rst +++ b/docs/tcp_replication.rst @@ -137,7 +137,6 @@ for each stream so that on reconneciton it can start streaming from the correct place. Note: not all RDATA have valid tokens due to batching. See ``RdataCommand`` for more details. - Example ~~~~~~~ @@ -221,3 +220,23 @@ SYNC (S, C) See ``synapse/replication/tcp/commands.py`` for a detailed description and the format of each command. + + +Cache Invalidation Stream +~~~~~~~~~~~~~~~~~~~~~~~~~ + +The cache invalidation stream is used to inform workers when they need to +invalidate any of their caches in the data store. This is done by streaming all +cache invalidations done on master down to the workers, assuming that any caches +on the workers also exist on the master. + +Each individual cache invalidation results in a row being sent down replication, +which includes the cache name (the name of the function) and they key to +invalidate. For example:: + + > RDATA caches 550953771 ["get_user_by_id", ["@bob:example.com"], 1550574873251] + +However, there are times when a number of caches need to be invalidated at the +same time with the same key. To reduce traffic we batch those invalidations into +a single poke by defining a special cache name that workers understand to mean +to expand to invalidate the correct caches. diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index f7c6d714ab..1c8d3f0026 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -1199,8 +1199,8 @@ class SQLBaseStore(object): Args: txn - room_id (str): Room were state changed - members_changed (set[str]): The user_ids of members that have changed + room_id (str): Room where state changed + members_changed (Iterable[str]): The user_ids of members that have changed """ txn.call_after(self._invalidate_state_caches, room_id, members_changed) @@ -1215,7 +1215,7 @@ class SQLBaseStore(object): not stream invalidations down replication. Args: - room_id (str): Room were state changed + room_id (str): Room where state changed members_changed (set[str]): The user_ids of members that have changed """ for member in members_changed: @@ -1237,7 +1237,7 @@ class SQLBaseStore(object): Args: txn cache_name (str) - keys (list[str]) + keys (iterable[str]) """ if isinstance(self.database_engine, PostgresEngine): -- cgit 1.5.1 From 1bb35e3a83146a55bf7d8a18d38aa0d59f1289d5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 19 Feb 2019 11:34:40 +0000 Subject: Use itertools --- synapse/storage/_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 1c8d3f0026..9db594bc42 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -12,6 +12,7 @@ # 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 itertools import logging import sys import threading @@ -1204,8 +1205,7 @@ class SQLBaseStore(object): """ txn.call_after(self._invalidate_state_caches, room_id, members_changed) - keys = [room_id] - keys.extend(members_changed) + keys = itertools.chain([room_id], members_changed) self._send_invalidation_to_replication( txn, _CURRENT_STATE_CACHE_NAME, keys, ) -- cgit 1.5.1 From 62175a20e51b4ce71b9e7a18755a42e259bd2ff8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 19 Feb 2019 11:38:40 +0000 Subject: Docs --- docs/tcp_replication.rst | 5 +++++ synapse/storage/_base.py | 5 +++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docs/tcp_replication.rst b/docs/tcp_replication.rst index 852f1113a3..73436cea62 100644 --- a/docs/tcp_replication.rst +++ b/docs/tcp_replication.rst @@ -240,3 +240,8 @@ However, there are times when a number of caches need to be invalidated at the same time with the same key. To reduce traffic we batch those invalidations into a single poke by defining a special cache name that workers understand to mean to expand to invalidate the correct caches. + +Currently the special cache names are declared in ``synapse/storage/_base.py`` +and are: + +1. ``cs_cache_fake`` ─ invalidates caches that depend on the current state diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 9db594bc42..f1a5366b95 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -1201,7 +1201,7 @@ class SQLBaseStore(object): Args: txn room_id (str): Room where state changed - members_changed (Iterable[str]): The user_ids of members that have changed + members_changed (iterable[str]): The user_ids of members that have changed """ txn.call_after(self._invalidate_state_caches, room_id, members_changed) @@ -1216,7 +1216,8 @@ class SQLBaseStore(object): Args: room_id (str): Room where state changed - members_changed (set[str]): The user_ids of members that have changed + members_changed (iterable[str]): The user_ids of members that have + changed """ for member in members_changed: self.get_rooms_for_user_with_stream_ordering.invalidate((member,)) -- cgit 1.5.1