Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Support MSC3283: Expose set_displayname, set_avatar_url and 3pid_changes in capabilities #10452

Merged
merged 8 commits into from
Aug 19, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10452.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for [MSC3283](https://github.com/matrix-org/matrix-doc/pull/3283): Expose enable_set_displayname in capabilities.
16 changes: 16 additions & 0 deletions synapse/rest/client/v2_alpha/capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,22 @@ 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
}
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
}
richvdh marked this conversation as resolved.
Show resolved Hide resolved

richvdh marked this conversation as resolved.
Show resolved Hide resolved
return 200, response


Expand Down
104 changes: 70 additions & 34 deletions tests/rest/client/v2_alpha/test_capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand All @@ -58,58 +61,80 @@ 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)
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._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)
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):
"""
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})
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):
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
)
)

Expand All @@ -123,12 +148,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
)
)

Expand All @@ -148,3 +170,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"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if expect_success:
self.assertTrue(capabilities[capability]["enabled"])
else:
self.assertFalse(capabilities[capability]["enabled"])
self.assertEqual(capabilities[capability]["enabled"], expect_success)

... though to be honest, I would be inclined to have it return capabilities and leave the self.assertTrue(capabilities["m.change_password"]["enabled"]) or similar in the caller.