From fff301677a2e54b4ca2ff1f16c663252aaf9bb9b Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 21 Jul 2021 22:57:15 +0200 Subject: [PATCH 1/6] Support MSC3283: Expose `enable_set_displayname` in capabilities --- synapse/rest/client/v2_alpha/capabilities.py | 8 ++ .../rest/client/v2_alpha/test_capabilities.py | 84 +++++++++++-------- 2 files changed, 58 insertions(+), 34 deletions(-) diff --git a/synapse/rest/client/v2_alpha/capabilities.py b/synapse/rest/client/v2_alpha/capabilities.py index 88e3aac7974b..346ae60930f7 100644 --- a/synapse/rest/client/v2_alpha/capabilities.py +++ b/synapse/rest/client/v2_alpha/capabilities.py @@ -61,6 +61,14 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: "org.matrix.msc3244.room_capabilities" ] = MSC3244_CAPABILITIES + # Add this to response only if `enable_set_displayname` is set to `false` (not default) + # because it is experimental + # This needs an update after release of MSC3283. Then it will be always available. + if not self.config.enable_set_displayname: + response["capabilities"]["org.matrix.msc3283.set_displayname"] = { + "enabled": self.config.enable_set_displayname + } + return 200, response diff --git a/tests/rest/client/v2_alpha/test_capabilities.py b/tests/rest/client/v2_alpha/test_capabilities.py index f80f48a45577..86ff289f6633 100644 --- a/tests/rest/client/v2_alpha/test_capabilities.py +++ b/tests/rest/client/v2_alpha/test_capabilities.py @@ -31,19 +31,22 @@ class CapabilitiesTestCase(unittest.HomeserverTestCase): def make_homeserver(self, reactor, clock): self.url = b"/_matrix/client/r0/capabilities" hs = self.setup_test_homeserver() - self.store = hs.get_datastore() self.config = hs.config self.auth_handler = hs.get_auth_handler() return hs + def prepare(self, reactor, clock, hs): + self.localpart = "user" + self.password = "pass" + self.user = self.register_user(self.localpart, self.password) + def test_check_auth_required(self): channel = self.make_request("GET", self.url) self.assertEqual(channel.code, 401) def test_get_room_version_capabilities(self): - self.register_user("user", "pass") - access_token = self.login("user", "pass") + access_token = self.login(self.localpart, self.password) channel = self.make_request("GET", self.url, access_token=access_token) capabilities = channel.json_body["capabilities"] @@ -58,58 +61,60 @@ def test_get_room_version_capabilities(self): ) def test_get_change_password_capabilities_password_login(self): - localpart = "user" - password = "pass" - user = self.register_user(localpart, password) - access_token = self.login(user, password) - - channel = self.make_request("GET", self.url, access_token=access_token) - capabilities = channel.json_body["capabilities"] + access_token = self.login(self.localpart, self.password) - self.assertEqual(channel.code, 200) - self.assertTrue(capabilities["m.change_password"]["enabled"]) + self._test_capability("m.change_password", access_token, True) @override_config({"password_config": {"localdb_enabled": False}}) def test_get_change_password_capabilities_localdb_disabled(self): - localpart = "user" - password = "pass" - user = self.register_user(localpart, password) access_token = self.get_success( self.auth_handler.get_access_token_for_user_id( - user, device_id=None, valid_until_ms=None + self.user, device_id=None, valid_until_ms=None ) ) - channel = self.make_request("GET", self.url, access_token=access_token) - capabilities = channel.json_body["capabilities"] - - self.assertEqual(channel.code, 200) - self.assertFalse(capabilities["m.change_password"]["enabled"]) + self._test_capability("m.change_password", access_token, False) @override_config({"password_config": {"enabled": False}}) def test_get_change_password_capabilities_password_disabled(self): - localpart = "user" - password = "pass" - user = self.register_user(localpart, password) access_token = self.get_success( self.auth_handler.get_access_token_for_user_id( - user, device_id=None, valid_until_ms=None + self.user, device_id=None, valid_until_ms=None ) ) + self._test_capability("m.change_password", access_token, False) + + def test_get_change_users_attributes_capabilities(self): + """ + Test that per default server returns `m.change_password` + but not `org.matrix.msc3283.set_displayname`. + In feature we can add further capabilites. + If MSC3283 is in spec, the test must be updated to test that server reponds + with `m.enable_set_displayname` per default. + """ + access_token = self.login(self.localpart, self.password) + channel = self.make_request("GET", self.url, access_token=access_token) capabilities = channel.json_body["capabilities"] self.assertEqual(channel.code, 200) - self.assertFalse(capabilities["m.change_password"]["enabled"]) + self.assertTrue(capabilities["m.change_password"]["enabled"]) + self.assertNotIn("org.matrix.msc3283.set_displayname", capabilities) + + @override_config({"enable_set_displayname": False}) + def test_get_set_displayname_capabilities_displayname_disabled(self): + """ + Test if set displayname is disabled that the server responds it. + """ + access_token = self.login(self.localpart, self.password) + + self._test_capability("org.matrix.msc3283.set_displayname", access_token, False) def test_get_does_not_include_msc3244_fields_by_default(self): - localpart = "user" - password = "pass" - user = self.register_user(localpart, password) access_token = self.get_success( self.auth_handler.get_access_token_for_user_id( - user, device_id=None, valid_until_ms=None + self.user, device_id=None, valid_until_ms=None ) ) @@ -123,12 +128,9 @@ def test_get_does_not_include_msc3244_fields_by_default(self): @override_config({"experimental_features": {"msc3244_enabled": True}}) def test_get_does_include_msc3244_fields_when_enabled(self): - localpart = "user" - password = "pass" - user = self.register_user(localpart, password) access_token = self.get_success( self.auth_handler.get_access_token_for_user_id( - user, device_id=None, valid_until_ms=None + self.user, device_id=None, valid_until_ms=None ) ) @@ -148,3 +150,17 @@ def test_get_does_include_msc3244_fields_when_enabled(self): self.assertGreater(len(details["support"]), 0) for room_version in details["support"]: self.assertTrue(room_version in KNOWN_ROOM_VERSIONS, str(room_version)) + + def _test_capability(self, capability: str, access_token: str, expect_success=True): + """ + Requests the capabilities from server and check if the value is expected. + """ + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + + self.assertEqual(channel.code, 200) + + if expect_success: + self.assertTrue(capabilities[capability]["enabled"]) + else: + self.assertFalse(capabilities[capability]["enabled"]) From af0ef5ee82f1d5c9cf3ad4c9ceb4d39060e7aab4 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Wed, 21 Jul 2021 23:02:27 +0200 Subject: [PATCH 2/6] newsfile --- changelog.d/10452.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10452.feature diff --git a/changelog.d/10452.feature b/changelog.d/10452.feature new file mode 100644 index 000000000000..f332b383e370 --- /dev/null +++ b/changelog.d/10452.feature @@ -0,0 +1 @@ +Add support for [MSC3283](https://github.com/matrix-org/matrix-doc/pull/3283): Expose enable_set_displayname in capabilities. \ No newline at end of file From e747047aaaf6604897f4939b04148e826ac940ec Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 22 Jul 2021 19:49:05 +0200 Subject: [PATCH 3/6] add avatar url and 3pid --- synapse/rest/client/v2_alpha/capabilities.py | 8 ++++++++ .../rest/client/v2_alpha/test_capabilities.py | 20 +++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/synapse/rest/client/v2_alpha/capabilities.py b/synapse/rest/client/v2_alpha/capabilities.py index 346ae60930f7..b533fb765489 100644 --- a/synapse/rest/client/v2_alpha/capabilities.py +++ b/synapse/rest/client/v2_alpha/capabilities.py @@ -68,6 +68,14 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: response["capabilities"]["org.matrix.msc3283.set_displayname"] = { "enabled": self.config.enable_set_displayname } + if not self.config.enable_set_avatar_url: + response["capabilities"]["org.matrix.msc3283.set_avatar_url"] = { + "enabled": self.config.enable_set_avatar_url + } + if not self.config.enable_3pid_changes: + response["capabilities"]["org.matrix.msc3283.3pid_changes"] = { + "enabled": self.config.enable_3pid_changes + } return 200, response diff --git a/tests/rest/client/v2_alpha/test_capabilities.py b/tests/rest/client/v2_alpha/test_capabilities.py index 86ff289f6633..20104c3679a5 100644 --- a/tests/rest/client/v2_alpha/test_capabilities.py +++ b/tests/rest/client/v2_alpha/test_capabilities.py @@ -101,6 +101,8 @@ def test_get_change_users_attributes_capabilities(self): self.assertEqual(channel.code, 200) self.assertTrue(capabilities["m.change_password"]["enabled"]) self.assertNotIn("org.matrix.msc3283.set_displayname", capabilities) + self.assertNotIn("org.matrix.msc3283.set_avatar_url", capabilities) + self.assertNotIn("org.matrix.msc3283.3pid_changes", capabilities) @override_config({"enable_set_displayname": False}) def test_get_set_displayname_capabilities_displayname_disabled(self): @@ -111,6 +113,24 @@ def test_get_set_displayname_capabilities_displayname_disabled(self): self._test_capability("org.matrix.msc3283.set_displayname", access_token, False) + @override_config({"enable_set_avatar_url": False}) + def test_get_set_avatar_url_capabilities_avatar_url_disabled(self): + """ + Test if set avatar_url is disabled that the server responds it. + """ + access_token = self.login(self.localpart, self.password) + + self._test_capability("org.matrix.msc3283.set_avatar_url", access_token, False) + + @override_config({"enable_3pid_changes": False}) + def test_change_3pid_capabilities_3pid_disabled(self): + """ + Test if change 3pid is disabled that the server responds it. + """ + access_token = self.login(self.localpart, self.password) + + self._test_capability("org.matrix.msc3283.3pid_changes", access_token, False) + def test_get_does_not_include_msc3244_fields_by_default(self): access_token = self.get_success( self.auth_handler.get_access_token_for_user_id( From a684667f0132036e7ea2d24d69ecf65597a9ef89 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 29 Jul 2021 20:40:10 +0200 Subject: [PATCH 4/6] put it behind an `experimental` confi option --- synapse/config/experimental.py | 3 + synapse/rest/client/v2_alpha/capabilities.py | 7 +-- .../rest/client/v2_alpha/test_capabilities.py | 63 +++++++++++-------- 3 files changed, 42 insertions(+), 31 deletions(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 040c4504d8a6..1334ba822a40 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -35,3 +35,6 @@ def read_config(self, config: JsonDict, **kwargs): # MSC3244 (room version capabilities) self.msc3244_enabled: bool = experimental.get("msc3244_enabled", False) + + # MSC3283 (room version capabilities) + self.msc3283_enabled: bool = experimental.get("msc3283_enabled", False) diff --git a/synapse/rest/client/v2_alpha/capabilities.py b/synapse/rest/client/v2_alpha/capabilities.py index b533fb765489..093549512ebc 100644 --- a/synapse/rest/client/v2_alpha/capabilities.py +++ b/synapse/rest/client/v2_alpha/capabilities.py @@ -61,18 +61,13 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]: "org.matrix.msc3244.room_capabilities" ] = MSC3244_CAPABILITIES - # Add this to response only if `enable_set_displayname` is set to `false` (not default) - # because it is experimental - # This needs an update after release of MSC3283. Then it will be always available. - if not self.config.enable_set_displayname: + if self.config.experimental.msc3283_enabled: response["capabilities"]["org.matrix.msc3283.set_displayname"] = { "enabled": self.config.enable_set_displayname } - if not self.config.enable_set_avatar_url: response["capabilities"]["org.matrix.msc3283.set_avatar_url"] = { "enabled": self.config.enable_set_avatar_url } - if not self.config.enable_3pid_changes: response["capabilities"]["org.matrix.msc3283.3pid_changes"] = { "enabled": self.config.enable_3pid_changes } diff --git a/tests/rest/client/v2_alpha/test_capabilities.py b/tests/rest/client/v2_alpha/test_capabilities.py index 20104c3679a5..08a82df5c18d 100644 --- a/tests/rest/client/v2_alpha/test_capabilities.py +++ b/tests/rest/client/v2_alpha/test_capabilities.py @@ -85,14 +85,8 @@ def test_get_change_password_capabilities_password_disabled(self): self._test_capability("m.change_password", access_token, False) - def test_get_change_users_attributes_capabilities(self): - """ - Test that per default server returns `m.change_password` - but not `org.matrix.msc3283.set_displayname`. - In feature we can add further capabilites. - If MSC3283 is in spec, the test must be updated to test that server reponds - with `m.enable_set_displayname` per default. - """ + def test_get_change_users_attributes_capabilities_when_msc3283_disabled(self): + """Test that per default msc3283 is disabled server returns `m.change_password`.""" access_token = self.login(self.localpart, self.password) channel = self.make_request("GET", self.url, access_token=access_token) @@ -104,29 +98,52 @@ def test_get_change_users_attributes_capabilities(self): self.assertNotIn("org.matrix.msc3283.set_avatar_url", capabilities) self.assertNotIn("org.matrix.msc3283.3pid_changes", capabilities) - @override_config({"enable_set_displayname": False}) + @override_config({"experimental_features": {"msc3283_enabled": True}}) + def test_get_change_users_attributes_capabilities_when_msc3283_enabled(self): + """Test if msc3283 is enabled server returns capabilities.""" + access_token = self.login(self.localpart, self.password) + + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + + self.assertEqual(channel.code, 200) + self.assertTrue(capabilities["m.change_password"]["enabled"]) + self.assertTrue("org.matrix.msc3283.set_displayname", capabilities) + self.assertTrue("org.matrix.msc3283.set_avatar_url", capabilities) + self.assertTrue("org.matrix.msc3283.3pid_changes", capabilities) + + @override_config( + { + "enable_set_displayname": False, + "experimental_features": {"msc3283_enabled": True}, + } + ) def test_get_set_displayname_capabilities_displayname_disabled(self): - """ - Test if set displayname is disabled that the server responds it. - """ + """Test if set displayname is disabled that the server responds it.""" access_token = self.login(self.localpart, self.password) self._test_capability("org.matrix.msc3283.set_displayname", access_token, False) - @override_config({"enable_set_avatar_url": False}) + @override_config( + { + "enable_set_avatar_url": False, + "experimental_features": {"msc3283_enabled": True}, + } + ) def test_get_set_avatar_url_capabilities_avatar_url_disabled(self): - """ - Test if set avatar_url is disabled that the server responds it. - """ + """Test if set avatar_url is disabled that the server responds it.""" access_token = self.login(self.localpart, self.password) self._test_capability("org.matrix.msc3283.set_avatar_url", access_token, False) - @override_config({"enable_3pid_changes": False}) + @override_config( + { + "enable_3pid_changes": False, + "experimental_features": {"msc3283_enabled": True}, + } + ) def test_change_3pid_capabilities_3pid_disabled(self): - """ - Test if change 3pid is disabled that the server responds it. - """ + """Test if change 3pid is disabled that the server responds it.""" access_token = self.login(self.localpart, self.password) self._test_capability("org.matrix.msc3283.3pid_changes", access_token, False) @@ -179,8 +196,4 @@ def _test_capability(self, capability: str, access_token: str, expect_success=Tr capabilities = channel.json_body["capabilities"] self.assertEqual(channel.code, 200) - - if expect_success: - self.assertTrue(capabilities[capability]["enabled"]) - else: - self.assertFalse(capabilities[capability]["enabled"]) + self.assertEqual(capabilities[capability]["enabled"], expect_success) From e578d0370af8e6dcfc169517d20917b831705ba7 Mon Sep 17 00:00:00 2001 From: dklimpel <5740567+dklimpel@users.noreply.github.com> Date: Thu, 29 Jul 2021 21:15:37 +0200 Subject: [PATCH 5/6] reformat tests --- .../rest/client/v2_alpha/test_capabilities.py | 52 ++++++++++++------- 1 file changed, 33 insertions(+), 19 deletions(-) diff --git a/tests/rest/client/v2_alpha/test_capabilities.py b/tests/rest/client/v2_alpha/test_capabilities.py index 08a82df5c18d..31b081ee600e 100644 --- a/tests/rest/client/v2_alpha/test_capabilities.py +++ b/tests/rest/client/v2_alpha/test_capabilities.py @@ -63,7 +63,11 @@ def test_get_room_version_capabilities(self): def test_get_change_password_capabilities_password_login(self): access_token = self.login(self.localpart, self.password) - self._test_capability("m.change_password", access_token, True) + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + + self.assertEqual(channel.code, 200) + self.assertTrue(capabilities["m.change_password"]["enabled"]) @override_config({"password_config": {"localdb_enabled": False}}) def test_get_change_password_capabilities_localdb_disabled(self): @@ -73,7 +77,11 @@ def test_get_change_password_capabilities_localdb_disabled(self): ) ) - self._test_capability("m.change_password", access_token, False) + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + + self.assertEqual(channel.code, 200) + self.assertFalse(capabilities["m.change_password"]["enabled"]) @override_config({"password_config": {"enabled": False}}) def test_get_change_password_capabilities_password_disabled(self): @@ -83,7 +91,11 @@ def test_get_change_password_capabilities_password_disabled(self): ) ) - self._test_capability("m.change_password", access_token, False) + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + + self.assertEqual(channel.code, 200) + self.assertFalse(capabilities["m.change_password"]["enabled"]) def test_get_change_users_attributes_capabilities_when_msc3283_disabled(self): """Test that per default msc3283 is disabled server returns `m.change_password`.""" @@ -108,9 +120,9 @@ def test_get_change_users_attributes_capabilities_when_msc3283_enabled(self): self.assertEqual(channel.code, 200) self.assertTrue(capabilities["m.change_password"]["enabled"]) - self.assertTrue("org.matrix.msc3283.set_displayname", capabilities) - self.assertTrue("org.matrix.msc3283.set_avatar_url", capabilities) - self.assertTrue("org.matrix.msc3283.3pid_changes", capabilities) + self.assertTrue(capabilities["org.matrix.msc3283.set_displayname"]["enabled"]) + self.assertTrue(capabilities["org.matrix.msc3283.set_avatar_url"]["enabled"]) + self.assertTrue(capabilities["org.matrix.msc3283.3pid_changes"]["enabled"]) @override_config( { @@ -122,7 +134,11 @@ def test_get_set_displayname_capabilities_displayname_disabled(self): """Test if set displayname is disabled that the server responds it.""" access_token = self.login(self.localpart, self.password) - self._test_capability("org.matrix.msc3283.set_displayname", access_token, False) + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + + self.assertEqual(channel.code, 200) + self.assertFalse(capabilities["org.matrix.msc3283.set_displayname"]["enabled"]) @override_config( { @@ -134,7 +150,11 @@ def test_get_set_avatar_url_capabilities_avatar_url_disabled(self): """Test if set avatar_url is disabled that the server responds it.""" access_token = self.login(self.localpart, self.password) - self._test_capability("org.matrix.msc3283.set_avatar_url", access_token, False) + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + + self.assertEqual(channel.code, 200) + self.assertFalse(capabilities["org.matrix.msc3283.set_avatar_url"]["enabled"]) @override_config( { @@ -146,7 +166,11 @@ def test_change_3pid_capabilities_3pid_disabled(self): """Test if change 3pid is disabled that the server responds it.""" access_token = self.login(self.localpart, self.password) - self._test_capability("org.matrix.msc3283.3pid_changes", access_token, False) + channel = self.make_request("GET", self.url, access_token=access_token) + capabilities = channel.json_body["capabilities"] + + self.assertEqual(channel.code, 200) + self.assertFalse(capabilities["org.matrix.msc3283.3pid_changes"]["enabled"]) def test_get_does_not_include_msc3244_fields_by_default(self): access_token = self.get_success( @@ -187,13 +211,3 @@ def test_get_does_include_msc3244_fields_when_enabled(self): self.assertGreater(len(details["support"]), 0) for room_version in details["support"]: self.assertTrue(room_version in KNOWN_ROOM_VERSIONS, str(room_version)) - - def _test_capability(self, capability: str, access_token: str, expect_success=True): - """ - Requests the capabilities from server and check if the value is expected. - """ - channel = self.make_request("GET", self.url, access_token=access_token) - capabilities = channel.json_body["capabilities"] - - self.assertEqual(channel.code, 200) - self.assertEqual(capabilities[capability]["enabled"], expect_success) From 50e4f321057f3d762f404265dbcabdf6466285b0 Mon Sep 17 00:00:00 2001 From: Dirk Klimpel <5740567+dklimpel@users.noreply.github.com> Date: Tue, 10 Aug 2021 22:16:11 +0200 Subject: [PATCH 6/6] Update experimental.py --- synapse/config/experimental.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 1334ba822a40..fa6b465eb520 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -36,5 +36,5 @@ def read_config(self, config: JsonDict, **kwargs): # MSC3244 (room version capabilities) self.msc3244_enabled: bool = experimental.get("msc3244_enabled", False) - # MSC3283 (room version capabilities) + # MSC3283 (set displayname, avatar_url and change 3pid capabilities) self.msc3283_enabled: bool = experimental.get("msc3283_enabled", False)