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

Support the stable API endpoint for MSC3283: new settings in /capabilities endpoint #11933

Merged
merged 4 commits into from
Feb 11, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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/11933.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support the stable API endpoint for [MSC3283](https://github.com/matrix-org/matrix-doc/pull/3283): new settings in `/capabilities` endpoint.
3 changes: 0 additions & 3 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,9 +39,6 @@ def read_config(self, config: JsonDict, **kwargs):
# MSC3244 (room version capabilities)
self.msc3244_enabled: bool = experimental.get("msc3244_enabled", True)

# MSC3283 (set displayname, avatar_url and change 3pid capabilities)
self.msc3283_enabled: bool = experimental.get("msc3283_enabled", False)

# MSC3266 (room summary api)
self.msc3266_enabled: bool = experimental.get("msc3266_enabled", False)

Expand Down
23 changes: 11 additions & 12 deletions synapse/rest/client/capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.
import logging
from http import HTTPStatus
from typing import TYPE_CHECKING, Tuple

from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, MSC3244_CAPABILITIES
Expand Down Expand Up @@ -54,6 +55,15 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
},
},
"m.change_password": {"enabled": change_password},
"m.set_displayname": {
"enabled": self.config.registration.enable_set_displayname
},
"m.set_avatar_url": {
"enabled": self.config.registration.enable_set_avatar_url
},
"m.3pid_changes": {
"enabled": self.config.registration.enable_3pid_changes
},
}
}

Expand All @@ -62,21 +72,10 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
"org.matrix.msc3244.room_capabilities"
] = MSC3244_CAPABILITIES

if self.config.experimental.msc3283_enabled:
response["capabilities"]["org.matrix.msc3283.set_displayname"] = {
"enabled": self.config.registration.enable_set_displayname
}
response["capabilities"]["org.matrix.msc3283.set_avatar_url"] = {
"enabled": self.config.registration.enable_set_avatar_url
}
response["capabilities"]["org.matrix.msc3283.3pid_changes"] = {
"enabled": self.config.registration.enable_3pid_changes
}

Copy link
Member

Choose a reason for hiding this comment

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

Do we want to keep adding these for a version or two to give a small window of backwards compat? Not sure if we care enough to bother?

If we do keep it we should add an issue and add it to the next month milestone for us to remember to rip this out

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want to keep adding these for a version or two to give a small window of backwards compat? Not sure if we care enough to bother?

I have no preferences here. Is there a standard procedure for Synapse? This kind of change is surely not the first, is it?

Copy link
Member

Choose a reason for hiding this comment

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

Have asked in #synapse-dev:matrix.org, will see if anyone has opinions.

Copy link
Member

Choose a reason for hiding this comment

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

The conclusion we came to is that we should leave this in for one release, and then rip it out. If you could make the change that would be grand, and I can file an issue and put in the right place in our tracker to make sure it doesn't get forgotten 🙂

if self.config.experimental.msc3440_enabled:
response["capabilities"]["io.element.thread"] = {"enabled": True}

return 200, response
return HTTPStatus.OK, response


def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
Expand Down
69 changes: 21 additions & 48 deletions tests/rest/client/test_capabilities.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@
# 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.
from http import HTTPStatus

import synapse.rest.admin
from synapse.api.room_versions import KNOWN_ROOM_VERSIONS
from synapse.rest.client import capabilities, login
Expand All @@ -28,7 +30,7 @@ class CapabilitiesTestCase(unittest.HomeserverTestCase):
]

def make_homeserver(self, reactor, clock):
self.url = b"/_matrix/client/r0/capabilities"
self.url = b"/capabilities"
hs = self.setup_test_homeserver()
self.config = hs.config
self.auth_handler = hs.get_auth_handler()
Expand Down Expand Up @@ -96,80 +98,51 @@ def test_get_change_password_capabilities_password_disabled(self):
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`."""
def test_get_change_users_attributes_capabilities(self):
"""Test that server returns capabilities by 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.assertEqual(channel.code, HTTPStatus.OK)
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({"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.assertTrue(capabilities["m.set_displayname"]["enabled"])
self.assertTrue(capabilities["m.set_avatar_url"]["enabled"])
self.assertTrue(capabilities["m.3pid_changes"]["enabled"])

self.assertEqual(channel.code, 200)
self.assertTrue(capabilities["m.change_password"]["enabled"])
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(
{
"enable_set_displayname": False,
"experimental_features": {"msc3283_enabled": True},
}
)
@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)

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(
{
"enable_set_avatar_url": False,
"experimental_features": {"msc3283_enabled": True},
}
)
self.assertEqual(channel.code, HTTPStatus.OK)
self.assertFalse(capabilities["m.set_displayname"]["enabled"])

@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)

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(
{
"enable_3pid_changes": False,
"experimental_features": {"msc3283_enabled": True},
}
)
def test_change_3pid_capabilities_3pid_disabled(self):
self.assertEqual(channel.code, HTTPStatus.OK)
self.assertFalse(capabilities["m.set_avatar_url"]["enabled"])

@override_config({"enable_3pid_changes": False})
def test_get_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)

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"])
self.assertEqual(channel.code, HTTPStatus.OK)
self.assertFalse(capabilities["m.3pid_changes"]["enabled"])

@override_config({"experimental_features": {"msc3244_enabled": False}})
def test_get_does_not_include_msc3244_fields_when_disabled(self):
Expand Down