From 078dbf9a4e5c76f1faeaf025a2d2f149e4681285 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Sat, 3 Nov 2018 02:16:11 +1100 Subject: [PATCH 01/10] tests --- tests/rest/client/test_consent.py | 101 ++++++++++++++++++++++++++++++ tests/server.py | 8 ++- tests/unittest.py | 10 ++- 3 files changed, 114 insertions(+), 5 deletions(-) create mode 100644 tests/rest/client/test_consent.py diff --git a/tests/rest/client/test_consent.py b/tests/rest/client/test_consent.py new file mode 100644 index 000000000000..be4f21f31928 --- /dev/null +++ b/tests/rest/client/test_consent.py @@ -0,0 +1,101 @@ +# -*- coding: utf-8 -*- +# Copyright 2018 New Vector +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +import pkg_resources + +from synapse.api.urls import ConsentURIBuilder +from synapse.rest.client.v1 import admin, login, room +from synapse.rest.consent import consent_resource + +from tests import unittest +from tests.server import render + + +class RoomSearchTestCase(unittest.HomeserverTestCase): + servlets = [ + admin.register_servlets, + room.register_servlets, + login.register_servlets, + ] + user_id = True + hijack_auth = False + + def make_homeserver(self, reactor, clock): + + config = self.default_config() + config.user_consent_version = "1" + config.public_baseurl = "" + config.form_secret = "123abc" + + # Make some temporary templates... + temp_consent_path = self.mktemp() + os.mkdir(temp_consent_path) + os.mkdir(os.path.join(temp_consent_path, 'en')) + config.user_consent_template_dir = os.path.abspath(temp_consent_path) + + with open(os.path.join(temp_consent_path, "en/1.html"), 'w') as f: + f.write("{{version}}") + + with open(os.path.join(temp_consent_path, "en/success.html"), 'w') as f: + f.write("yay!") + + hs = self.setup_test_homeserver(config=config) + return hs + + def test_accept_consent(self): + """ + A user can use the consent form to accept the terms. + """ + uri_builder = ConsentURIBuilder(self.hs.config) + store = self.hs.get_datastore() + resource = consent_resource.ConsentResource(self.hs) + + # Register a user + user_id = self.register_user("user", "pass") + access_token = self.login("user", "pass") + + # They haven't consented, so they'll have a consent version of None + user_data = self.get_success(store.get_user_by_id(user_id)) + self.assertIs(user_data["consent_version"], None) + + # Fetch the consent page, to get the consent version + consent_uri = ( + uri_builder.build_user_consent_uri(user_id).replace("_matrix/", "") + + "&u=user" + ) + request, channel = self.make_request( + "GET", consent_uri, access_token=access_token, shorthand=False + ) + render(request, resource, self.reactor) + self.assertEqual(channel.code, 200) + + # Get the version from the body + version = channel.result["body"] + + # POST to the consent page, saying we've agreed + request, channel = self.make_request( + "POST", + consent_uri + "&v=" + version, + access_token=access_token, + shorthand=False, + ) + render(request, resource, self.reactor) + self.assertEqual(channel.code, 200) + + # Now we've consented! + user_data = self.get_success(store.get_user_by_id(user_id)) + self.assertEqual(user_data["consent_version"], "1") diff --git a/tests/server.py b/tests/server.py index cc6dbe04ac58..86b2a4cccb06 100644 --- a/tests/server.py +++ b/tests/server.py @@ -104,7 +104,9 @@ def info(self, *args, **kwargs): return FakeLogger() -def make_request(method, path, content=b"", access_token=None, request=SynapseRequest): +def make_request( + method, path, content=b"", access_token=None, request=SynapseRequest, shorthand=True +): """ Make a web request using the given method and path, feed it the content, and return the Request and the Channel underneath. @@ -115,8 +117,8 @@ def make_request(method, path, content=b"", access_token=None, request=SynapseRe if not isinstance(path, bytes): path = path.encode('ascii') - # Decorate it to be the full path - if not path.startswith(b"/_matrix"): + # Decorate it to be the full path, if we're using shorthand + if shorthand and not path.startswith(b"/_matrix"): path = b"/_matrix/client/r0/" + path path = path.replace(b"//", b"/") diff --git a/tests/unittest.py b/tests/unittest.py index 4d40bdb6a553..5939d9b678f1 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -258,7 +258,13 @@ def prepare(self, reactor, clock, homeserver): """ def make_request( - self, method, path, content=b"", access_token=None, request=SynapseRequest + self, + method, + path, + content=b"", + access_token=None, + request=SynapseRequest, + shorthand=True, ): """ Create a SynapseRequest at the path using the method and containing the @@ -277,7 +283,7 @@ def make_request( if isinstance(content, dict): content = json.dumps(content).encode('utf8') - return make_request(method, path, content, access_token, request) + return make_request(method, path, content, access_token, request, shorthand) def render(self, request): """ From 442f22f3cd7c864cb19a637f69acd7f1f54869ca Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Sat, 3 Nov 2018 02:16:15 +1100 Subject: [PATCH 02/10] fix --- synapse/rest/consent/consent_resource.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/consent/consent_resource.py b/synapse/rest/consent/consent_resource.py index 89b82b059117..c85e84b46502 100644 --- a/synapse/rest/consent/consent_resource.py +++ b/synapse/rest/consent/consent_resource.py @@ -227,7 +227,7 @@ def _check_hash(self, userid, userhmac): key=self._hmac_secret, msg=userid.encode('utf-8'), digestmod=sha256, - ).hexdigest() + ).hexdigest().encode('ascii') if not compare_digest(want_mac, userhmac): raise SynapseError(http_client.FORBIDDEN, "HMAC incorrect") From d85fe463d6cc808f607281a0e61e10ebed4e785e Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Sat, 3 Nov 2018 02:17:04 +1100 Subject: [PATCH 03/10] bugfix --- changelog.d/4140.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4140.bugfix diff --git a/changelog.d/4140.bugfix b/changelog.d/4140.bugfix new file mode 100644 index 000000000000..c7e0ee229dc3 --- /dev/null +++ b/changelog.d/4140.bugfix @@ -0,0 +1 @@ +Generating the user consent URI no longer fails on Python 3. From 46234d46b124c1a332642ca70ba6025b7322a147 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Sat, 3 Nov 2018 02:36:13 +1100 Subject: [PATCH 04/10] fix py3 --- tests/rest/client/test_consent.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/rest/client/test_consent.py b/tests/rest/client/test_consent.py index be4f21f31928..3f374a6309e4 100644 --- a/tests/rest/client/test_consent.py +++ b/tests/rest/client/test_consent.py @@ -15,8 +15,6 @@ import os -import pkg_resources - from synapse.api.urls import ConsentURIBuilder from synapse.rest.client.v1 import admin, login, room from synapse.rest.consent import consent_resource @@ -84,7 +82,7 @@ def test_accept_consent(self): self.assertEqual(channel.code, 200) # Get the version from the body - version = channel.result["body"] + version = channel.result["body"].decode('ascii') # POST to the consent page, saying we've agreed request, channel = self.make_request( From 16696dfdf64ad74817f56d8915df26c9e1d6f71e Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Sat, 3 Nov 2018 02:40:20 +1100 Subject: [PATCH 05/10] fix name --- tests/rest/client/test_consent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/rest/client/test_consent.py b/tests/rest/client/test_consent.py index 3f374a6309e4..5c498c4d85ff 100644 --- a/tests/rest/client/test_consent.py +++ b/tests/rest/client/test_consent.py @@ -23,7 +23,7 @@ from tests.server import render -class RoomSearchTestCase(unittest.HomeserverTestCase): +class ConsentResourceTestCase(unittest.HomeserverTestCase): servlets = [ admin.register_servlets, room.register_servlets, From a976e5a6c7a0cc92bd29f77dce4f1f07e7ccd332 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Sat, 3 Nov 2018 02:54:52 +1100 Subject: [PATCH 06/10] skip jinja use on old installs --- tests/rest/client/test_consent.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/rest/client/test_consent.py b/tests/rest/client/test_consent.py index 5c498c4d85ff..34784b4b0185 100644 --- a/tests/rest/client/test_consent.py +++ b/tests/rest/client/test_consent.py @@ -22,8 +22,14 @@ from tests import unittest from tests.server import render +try: + from synapse.push.mailer import load_jinja2_templates +except Exception: + load_jinja2_templates = None + class ConsentResourceTestCase(unittest.HomeserverTestCase): + skip = "No Jinja installed" if not load_jinja2_templates else None servlets = [ admin.register_servlets, room.register_servlets, From 4319e8ce259cbf5d900e23551e059c76d092afba Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Tue, 6 Nov 2018 00:28:34 +1100 Subject: [PATCH 07/10] some docs --- tests/server.py | 12 ++++++++++++ tests/unittest.py | 2 ++ 2 files changed, 14 insertions(+) diff --git a/tests/server.py b/tests/server.py index 86b2a4cccb06..f63f33c94f7b 100644 --- a/tests/server.py +++ b/tests/server.py @@ -110,6 +110,18 @@ def make_request( """ Make a web request using the given method and path, feed it the content, and return the Request and the Channel underneath. + + Args: + method (bytes/unicode): The HTTP request method ("verb"). + path (bytes/unicode): The HTTP path, suitably URL encoded (e.g. + escaped UTF-8 & spaces and such). + content (bytes or dict): The body of the request. JSON-encoded, if + a dict. + shorthand: Whether to try and be helpful and prefix the given URL + with the usual REST API path, if it doesn't contain it. + + Returns: + A synapse.http.site.SynapseRequest. """ if not isinstance(method, bytes): method = method.encode('ascii') diff --git a/tests/unittest.py b/tests/unittest.py index 5939d9b678f1..5e35c943d7b4 100644 --- a/tests/unittest.py +++ b/tests/unittest.py @@ -276,6 +276,8 @@ def make_request( escaped UTF-8 & spaces and such). content (bytes or dict): The body of the request. JSON-encoded, if a dict. + shorthand: Whether to try and be helpful and prefix the given URL + with the usual REST API path, if it doesn't contain it. Returns: A synapse.http.site.SynapseRequest. From 646bb76eaffd1d614189d4de45fd97a445ecd2c1 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Tue, 6 Nov 2018 00:30:03 +1100 Subject: [PATCH 08/10] api level --- tests/rest/client/test_consent.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/tests/rest/client/test_consent.py b/tests/rest/client/test_consent.py index 34784b4b0185..ebc151f94504 100644 --- a/tests/rest/client/test_consent.py +++ b/tests/rest/client/test_consent.py @@ -100,6 +100,15 @@ def test_accept_consent(self): render(request, resource, self.reactor) self.assertEqual(channel.code, 200) - # Now we've consented! - user_data = self.get_success(store.get_user_by_id(user_id)) - self.assertEqual(user_data["consent_version"], "1") + # Fetch the consent page, to get the consent version -- it should have + # changed + request, channel = self.make_request( + "GET", consent_uri, access_token=access_token, shorthand=False + ) + render(request, resource, self.reactor) + self.assertEqual(channel.code, 200) + + # Get the version from the body, and check that it's the version we + # agreed to + version = channel.result["body"].decode('ascii') + self.assertEqual(version, "1") From 446a3256dda05bdcd708343234032d363d2568ed Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Tue, 6 Nov 2018 00:53:06 +1100 Subject: [PATCH 09/10] no datastore usage --- tests/rest/client/test_consent.py | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/tests/rest/client/test_consent.py b/tests/rest/client/test_consent.py index ebc151f94504..61e7f95056ea 100644 --- a/tests/rest/client/test_consent.py +++ b/tests/rest/client/test_consent.py @@ -52,7 +52,7 @@ def make_homeserver(self, reactor, clock): config.user_consent_template_dir = os.path.abspath(temp_consent_path) with open(os.path.join(temp_consent_path, "en/1.html"), 'w') as f: - f.write("{{version}}") + f.write("{{version}},{{has_consented}}") with open(os.path.join(temp_consent_path, "en/success.html"), 'w') as f: f.write("yay!") @@ -72,10 +72,6 @@ def test_accept_consent(self): user_id = self.register_user("user", "pass") access_token = self.login("user", "pass") - # They haven't consented, so they'll have a consent version of None - user_data = self.get_success(store.get_user_by_id(user_id)) - self.assertIs(user_data["consent_version"], None) - # Fetch the consent page, to get the consent version consent_uri = ( uri_builder.build_user_consent_uri(user_id).replace("_matrix/", "") @@ -87,8 +83,9 @@ def test_accept_consent(self): render(request, resource, self.reactor) self.assertEqual(channel.code, 200) - # Get the version from the body - version = channel.result["body"].decode('ascii') + # Get the version from the body, and whether we've consented + version, consented = channel.result["body"].decode('ascii').split(",") + self.assertEqual(consented, "False") # POST to the consent page, saying we've agreed request, channel = self.make_request( @@ -109,6 +106,7 @@ def test_accept_consent(self): self.assertEqual(channel.code, 200) # Get the version from the body, and check that it's the version we - # agreed to - version = channel.result["body"].decode('ascii') + # agreed to, and that we've consented to it. + version, consented = channel.result["body"].decode('ascii').split(",") + self.assertEqual(consented, "True") self.assertEqual(version, "1") From 35d8097d19dbf0bfe450aa16d25ae505dfd41f35 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Tue, 6 Nov 2018 01:07:49 +1100 Subject: [PATCH 10/10] no datastore usage, pt 2 --- tests/rest/client/test_consent.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/rest/client/test_consent.py b/tests/rest/client/test_consent.py index 61e7f95056ea..df3f1cde6ed7 100644 --- a/tests/rest/client/test_consent.py +++ b/tests/rest/client/test_consent.py @@ -65,7 +65,6 @@ def test_accept_consent(self): A user can use the consent form to accept the terms. """ uri_builder = ConsentURIBuilder(self.hs.config) - store = self.hs.get_datastore() resource = consent_resource.ConsentResource(self.hs) # Register a user