From e6018bcc1af22747d5a41d5d703091ab32b999fd Mon Sep 17 00:00:00 2001 From: Michael Kaye <1917473+michaelkaye@users.noreply.github.com> Date: Thu, 4 Oct 2018 15:15:26 +0100 Subject: [PATCH 01/30] Use labels to tag builds with their SHA1 version. The additional sha1 tagged builds in docker hub are messy, so instead tag the build with the SHA1 version. --- .circleci/config.yml | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/.circleci/config.yml b/.circleci/config.yml index ec3848b048..6c0c577668 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -4,8 +4,8 @@ jobs: machine: true steps: - checkout - - run: docker build -f docker/Dockerfile -t matrixdotorg/synapse:${CIRCLE_TAG} . - - run: docker build -f docker/Dockerfile -t matrixdotorg/synapse:${CIRCLE_TAG}-py3 --build-arg PYTHON_VERSION=3.6 . + - run: docker build -f docker/Dockerfile --label gitsha1=${CIRCLE_SHA1} -t matrixdotorg/synapse:${CIRCLE_TAG} . + - run: docker build -f docker/Dockerfile --label gitsha1=${CIRCLE_SHA1} -t matrixdotorg/synapse:${CIRCLE_TAG}-py3 --build-arg PYTHON_VERSION=3.6 . - run: docker login --username $DOCKER_HUB_USERNAME --password $DOCKER_HUB_PASSWORD - run: docker push matrixdotorg/synapse:${CIRCLE_TAG} - run: docker push matrixdotorg/synapse:${CIRCLE_TAG}-py3 @@ -13,13 +13,9 @@ jobs: machine: true steps: - checkout - - run: docker build -f docker/Dockerfile -t matrixdotorg/synapse:${CIRCLE_SHA1} . - - run: docker build -f docker/Dockerfile -t matrixdotorg/synapse:${CIRCLE_SHA1}-py3 --build-arg PYTHON_VERSION=3.6 . + - run: docker build -f docker/Dockerfile --label gitsha1=${CIRCLE_SHA1} -t matrixdotorg/synapse:${CIRCLE_SHA1} . + - run: docker build -f docker/Dockerfile --label gitsha1=${CIRCLE_SHA1} -t matrixdotorg/synapse:${CIRCLE_SHA1}-py3 --build-arg PYTHON_VERSION=3.6 . - run: docker login --username $DOCKER_HUB_USERNAME --password $DOCKER_HUB_PASSWORD - - run: docker tag matrixdotorg/synapse:${CIRCLE_SHA1} matrixdotorg/synapse:latest - - run: docker tag matrixdotorg/synapse:${CIRCLE_SHA1}-py3 matrixdotorg/synapse:latest-py3 - - run: docker push matrixdotorg/synapse:${CIRCLE_SHA1} - - run: docker push matrixdotorg/synapse:${CIRCLE_SHA1}-py3 - run: docker push matrixdotorg/synapse:latest - run: docker push matrixdotorg/synapse:latest-py3 sytestpy2: From 455df4dda0d90e27d7fe5b7676c2ac9b60777ea0 Mon Sep 17 00:00:00 2001 From: axel simon Date: Tue, 20 Nov 2018 16:57:54 +0100 Subject: [PATCH 02/30] =?UTF-8?q?Replace=20mentions=20of=20Vector=20with?= =?UTF-8?q?=C2=A0Riot?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/vector-im/vector-web/issues/1977 --> https://github.com/vector-im/riot-web/issues/1977 And mention of Vector as a client replaced with Riot. --- README.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.rst b/README.rst index 9165db8319..077939a45d 100644 --- a/README.rst +++ b/README.rst @@ -333,7 +333,7 @@ content served to web browsers a matrix API from being able to attack webapps ho on the same domain. This is particularly true of sharing a matrix webclient and server on the same domain. -See https://github.com/vector-im/vector-web/issues/1977 and +See https://github.com/vector-im/riot-web/issues/1977 and https://developer.github.com/changes/2014-04-25-user-content-security for more details. @@ -827,7 +827,7 @@ Password reset ============== If a user has registered an email address to their account using an identity -server, they can request a password-reset token via clients such as Vector. +server, they can request a password-reset token via clients such as Riot. A manual password reset can be done via direct database access as follows. From 6c18cc4b506a81c2a92665a15c55a0313ca47db3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 20 Nov 2018 22:46:51 +0000 Subject: [PATCH 03/30] Ignore __pycache__ directories in schema delta dir Now that we use py3, compiled python ends up in __pycache__ rather than *.pyc. --- changelog.d/4214.misc | 1 + synapse/storage/prepare_database.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/4214.misc diff --git a/changelog.d/4214.misc b/changelog.d/4214.misc new file mode 100644 index 0000000000..b2f62060e3 --- /dev/null +++ b/changelog.d/4214.misc @@ -0,0 +1 @@ +Ignore __pycache__ directories in the database schema folder diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index bd740e1e45..d5d2f89a77 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -257,7 +257,7 @@ def _upgrade_existing_database(cur, current_version, applied_delta_files, module.run_create(cur, database_engine) if not is_empty: module.run_upgrade(cur, database_engine, config=config) - elif ext == ".pyc": + elif ext == ".pyc" or file_name == "__pycache__": # Sometimes .pyc files turn up anyway even though we've # disabled their generation; e.g. from distribution package # installers. Silently skip it From de8772a655e49fc57138d91e6bb184dadeac838a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 27 Nov 2018 03:00:33 +0100 Subject: [PATCH 04/30] Do a GC after each test to fix logcontext leaks (#4227) * Some words about garbage collections and logcontexts * Do a GC after each test to fix logcontext leaks This feels like an awful hack, but... * changelog --- changelog.d/4227.misc | 1 + docs/log_contexts.rst | 58 ++++++++++++++++++++++++++++++++++++++++++- tests/unittest.py | 15 +++++++++-- 3 files changed, 71 insertions(+), 3 deletions(-) create mode 100644 changelog.d/4227.misc diff --git a/changelog.d/4227.misc b/changelog.d/4227.misc new file mode 100644 index 0000000000..7ebd51b6a4 --- /dev/null +++ b/changelog.d/4227.misc @@ -0,0 +1 @@ +Garbage-collect after each unit test to fix logcontext leaks \ No newline at end of file diff --git a/docs/log_contexts.rst b/docs/log_contexts.rst index 82ac4f91e5..27cde11cf7 100644 --- a/docs/log_contexts.rst +++ b/docs/log_contexts.rst @@ -163,7 +163,7 @@ the logcontext was set, this will make things work out ok: provided It's all too easy to forget to ``yield``: for instance if we forgot that ``do_some_stuff`` returned a deferred, we might plough on regardless. This leads to a mess; it will probably work itself out eventually, but not before -a load of stuff has been logged against the wrong content. (Normally, other +a load of stuff has been logged against the wrong context. (Normally, other things will break, more obviously, if you forget to ``yield``, so this tends not to be a major problem in practice.) @@ -440,3 +440,59 @@ To conclude: I think this scheme would have worked equally well, with less danger of messing it up, and probably made some more esoteric code easier to write. But again — changing the conventions of the entire Synapse codebase is not a sensible option for the marginal improvement offered. + + +A note on garbage-collection of Deferred chains +----------------------------------------------- + +It turns out that our logcontext rules do not play nicely with Deferred +chains which get orphaned and garbage-collected. + +Imagine we have some code that looks like this: + +.. code:: python + + listener_queue = [] + + def on_something_interesting(): + for d in listener_queue: + d.callback("foo") + + @defer.inlineCallbacks + def await_something_interesting(): + new_deferred = defer.Deferred() + listener_queue.append(new_deferred) + + with PreserveLoggingContext(): + yield new_deferred + +Obviously, the idea here is that we have a bunch of things which are waiting +for an event. (It's just an example of the problem here, but a relatively +common one.) + +Now let's imagine two further things happen. First of all, whatever was +waiting for the interesting thing goes away. (Perhaps the request times out, +or something *even more* interesting happens.) + +Secondly, let's suppose that we decide that the interesting thing is never +going to happen, and we reset the listener queue: + +.. code:: python + + def reset_listener_queue(): + listener_queue.clear() + +So, both ends of the deferred chain have now dropped their references, and the +deferred chain is now orphaned, and will be garbage-collected at some point. +Note that ``await_something_interesting`` is a generator function, and when +Python garbage-collects generator functions, it gives them a chance to clean +up by making the ``yield`` raise a ``GeneratorExit`` exception. In our case, +that means that the ``__exit__`` handler of ``PreserveLoggingContext`` will +carefully restore the request context, but there is now nothing waiting for +its return, so the request context is never cleared. + +To reiterate, this problem only arises when *both* ends of a deferred chain +are dropped. Dropping the the reference to a deferred you're supposed to be +calling is probably bad practice, so this doesn't actually happen too much. +Unfortunately, when it does happen, it will lead to leaked logcontexts which +are incredibly hard to track down. diff --git a/tests/unittest.py b/tests/unittest.py index a9ce57da9a..2049187fd9 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -13,7 +13,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 gc import hashlib import hmac import logging @@ -31,7 +31,7 @@ from synapse.http.server import JsonResource from synapse.http.site import SynapseRequest from synapse.server import HomeServer from synapse.types import UserID, create_requester -from synapse.util.logcontext import LoggingContextFilter +from synapse.util.logcontext import LoggingContext, LoggingContextFilter from tests.server import get_clock, make_request, render, setup_test_homeserver from tests.utils import default_config @@ -115,6 +115,17 @@ class TestCase(unittest.TestCase): logging.getLogger().setLevel(level) return orig() + @around(self) + def tearDown(orig): + ret = orig() + + # force a GC to workaround problems with deferreds leaking logcontexts when + # they are GCed (see the logcontext docs) + gc.collect() + LoggingContext.set_current_context(LoggingContext.sentinel) + + return ret + def assertObjectHasAttributes(self, attrs, obj): """Asserts that the given object has each of the attributes given, and that the value of each matches according to assertEquals.""" From 80527b568d64e402e91a85710e72d99448b4e384 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 27 Nov 2018 03:01:04 +0100 Subject: [PATCH 05/30] Fix more logcontext leaks in tests (#4209) --- changelog.d/4209.misc | 1 + tests/rest/media/v1/test_media_storage.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changelog.d/4209.misc diff --git a/changelog.d/4209.misc b/changelog.d/4209.misc new file mode 100644 index 0000000000..efd1f4abd6 --- /dev/null +++ b/changelog.d/4209.misc @@ -0,0 +1 @@ +Fix logcontext leaks in EmailPusher and in tests \ No newline at end of file diff --git a/tests/rest/media/v1/test_media_storage.py b/tests/rest/media/v1/test_media_storage.py index fd131e3454..ad5e9a612f 100644 --- a/tests/rest/media/v1/test_media_storage.py +++ b/tests/rest/media/v1/test_media_storage.py @@ -30,6 +30,7 @@ from synapse.rest.media.v1._base import FileInfo from synapse.rest.media.v1.filepath import MediaFilePaths from synapse.rest.media.v1.media_storage import MediaStorage from synapse.rest.media.v1.storage_provider import FileStorageProviderBackend +from synapse.util.logcontext import make_deferred_yieldable from synapse.util.module_loader import load_module from tests import unittest @@ -113,7 +114,7 @@ class MediaRepoTests(unittest.HomeserverTestCase): d = Deferred() d.addCallback(write_to) self.fetches.append((d, destination, path, args)) - return d + return make_deferred_yieldable(d) client = Mock() client.get_file = get_file From a44c0a096fbdca979cb47a3e7308cfe52be8d684 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 27 Nov 2018 03:47:18 +0100 Subject: [PATCH 06/30] Check logcontexts before and after each test (#4190) * Add better diagnostics to flakey keyring test * fix interpolation fail * Check logcontexts before and after each test * update changelog * update changelog --- changelog.d/4190.misc | 1 + tests/crypto/test_keyring.py | 18 ++++++++++++------ tests/unittest.py | 11 +++++++++-- 3 files changed, 22 insertions(+), 8 deletions(-) create mode 100644 changelog.d/4190.misc diff --git a/changelog.d/4190.misc b/changelog.d/4190.misc new file mode 100644 index 0000000000..6700a5150d --- /dev/null +++ b/changelog.d/4190.misc @@ -0,0 +1 @@ +Add some diagnostics to the tests to detect logcontext problems diff --git a/tests/crypto/test_keyring.py b/tests/crypto/test_keyring.py index 8299dc72c8..d643bec887 100644 --- a/tests/crypto/test_keyring.py +++ b/tests/crypto/test_keyring.py @@ -63,6 +63,14 @@ class KeyringTestCase(unittest.TestCase): keys = self.mock_perspective_server.get_verify_keys() self.hs.config.perspectives = {self.mock_perspective_server.server_name: keys} + def assert_sentinel_context(self): + if LoggingContext.current_context() != LoggingContext.sentinel: + self.fail( + "Expected sentinel context but got %s" % ( + LoggingContext.current_context(), + ) + ) + def check_context(self, _, expected): self.assertEquals( getattr(LoggingContext.current_context(), "request", None), expected @@ -70,8 +78,6 @@ class KeyringTestCase(unittest.TestCase): @defer.inlineCallbacks def test_wait_for_previous_lookups(self): - sentinel_context = LoggingContext.current_context() - kr = keyring.Keyring(self.hs) lookup_1_deferred = defer.Deferred() @@ -99,8 +105,10 @@ class KeyringTestCase(unittest.TestCase): ["server1"], {"server1": lookup_2_deferred} ) self.assertFalse(wait_2_deferred.called) + # ... so we should have reset the LoggingContext. - self.assertIs(LoggingContext.current_context(), sentinel_context) + self.assert_sentinel_context() + wait_2_deferred.addBoth(self.check_context, "two") # let the first lookup complete (in the sentinel context) @@ -198,8 +206,6 @@ class KeyringTestCase(unittest.TestCase): json1 = {} signedjson.sign.sign_json(json1, "server9", key1) - sentinel_context = LoggingContext.current_context() - with LoggingContext("one") as context_one: context_one.request = "one" @@ -213,7 +219,7 @@ class KeyringTestCase(unittest.TestCase): defer = kr.verify_json_for_server("server9", json1) self.assertFalse(defer.called) - self.assertIs(LoggingContext.current_context(), sentinel_context) + self.assert_sentinel_context() yield defer self.assertIs(LoggingContext.current_context(), context_one) diff --git a/tests/unittest.py b/tests/unittest.py index 2049187fd9..a191cccc29 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -102,8 +102,16 @@ class TestCase(unittest.TestCase): # traceback when a unit test exits leaving things on the reactor. twisted.internet.base.DelayedCall.debug = True - old_level = logging.getLogger().level + # if we're not starting in the sentinel logcontext, then to be honest + # all future bets are off. + if LoggingContext.current_context() is not LoggingContext.sentinel: + self.fail( + "Test starting with non-sentinel logging context %s" % ( + LoggingContext.current_context(), + ) + ) + old_level = logging.getLogger().level if old_level != level: @around(self) @@ -118,7 +126,6 @@ class TestCase(unittest.TestCase): @around(self) def tearDown(orig): ret = orig() - # force a GC to workaround problems with deferreds leaking logcontexts when # they are GCed (see the logcontext docs) gc.collect() From 944d524f183177d4da0910a380f0659d15564823 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 27 Nov 2018 08:51:52 +0100 Subject: [PATCH 07/30] Support m.login.sso (#4220) * Clean up the CSS for the fallback login form I was finding this hard to work with, so simplify a bunch of things. Each flow is now a form inside a div of class login_flow. The login_flow class now has a fixed width, as that looks much better than each flow having a differnt width. * Support m.login.sso MSC1721 renames m.login.cas to m.login.sso. This implements the change (retaining support for m.login.cas for older clients). * changelog --- changelog.d/4220.feature | 1 + synapse/rest/client/v1/login.py | 13 ++++++--- synapse/static/client/login/index.html | 37 +++++++++++-------------- synapse/static/client/login/js/login.js | 32 +++++++++++---------- synapse/static/client/login/style.css | 19 ++++--------- 5 files changed, 50 insertions(+), 52 deletions(-) create mode 100644 changelog.d/4220.feature diff --git a/changelog.d/4220.feature b/changelog.d/4220.feature new file mode 100644 index 0000000000..e7a3e40483 --- /dev/null +++ b/changelog.d/4220.feature @@ -0,0 +1 @@ +Rename login type m.login.cas to m.login.sso diff --git a/synapse/rest/client/v1/login.py b/synapse/rest/client/v1/login.py index 0010699d31..f6b4a85e40 100644 --- a/synapse/rest/client/v1/login.py +++ b/synapse/rest/client/v1/login.py @@ -27,7 +27,7 @@ from twisted.web.client import PartialDownloadError from synapse.api.errors import Codes, LoginError, SynapseError from synapse.http.server import finish_request -from synapse.http.servlet import parse_json_object_from_request +from synapse.http.servlet import RestServlet, parse_json_object_from_request from synapse.types import UserID from synapse.util.msisdn import phone_number_to_msisdn @@ -83,6 +83,7 @@ class LoginRestServlet(ClientV1RestServlet): PATTERNS = client_path_patterns("/login$") SAML2_TYPE = "m.login.saml2" CAS_TYPE = "m.login.cas" + SSO_TYPE = "m.login.sso" TOKEN_TYPE = "m.login.token" JWT_TYPE = "m.login.jwt" @@ -105,6 +106,10 @@ class LoginRestServlet(ClientV1RestServlet): if self.saml2_enabled: flows.append({"type": LoginRestServlet.SAML2_TYPE}) if self.cas_enabled: + flows.append({"type": LoginRestServlet.SSO_TYPE}) + + # we advertise CAS for backwards compat, though MSC1721 renamed it + # to SSO. flows.append({"type": LoginRestServlet.CAS_TYPE}) # While its valid for us to advertise this login type generally, @@ -384,11 +389,11 @@ class SAML2RestServlet(ClientV1RestServlet): defer.returnValue((200, {"status": "not_authenticated"})) -class CasRedirectServlet(ClientV1RestServlet): - PATTERNS = client_path_patterns("/login/cas/redirect", releases=()) +class CasRedirectServlet(RestServlet): + PATTERNS = client_path_patterns("/login/(cas|sso)/redirect") def __init__(self, hs): - super(CasRedirectServlet, self).__init__(hs) + super(CasRedirectServlet, self).__init__() self.cas_server_url = hs.config.cas_server_url.encode('ascii') self.cas_service_url = hs.config.cas_service_url.encode('ascii') diff --git a/synapse/static/client/login/index.html b/synapse/static/client/login/index.html index 96c8723cab..bcb6bc6bb7 100644 --- a/synapse/static/client/login/index.html +++ b/synapse/static/client/login/index.html @@ -12,35 +12,30 @@

Log in with one of the following methods

-
-
-