Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Removal: Remove support for experimental msc3886 #17638

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all 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/17638.removal
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove support for closed MSC3886.
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
Remove support for closed MSC3886.
Remove support for closed [MSC3886](https://github.com/matrix-org/matrix-spec-proposals/pull/3886).

5 changes: 0 additions & 5 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -363,11 +363,6 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
# MSC3874: Filtering /messages with rel_types / not_rel_types.
self.msc3874_enabled: bool = experimental.get("msc3874_enabled", False)

# MSC3886: Simple client rendezvous capability
self.msc3886_endpoint: Optional[str] = experimental.get(
"msc3886_endpoint", None
)

# MSC3890: Remotely silence local notifications
# Note: This option requires "experimental_features.msc3391_enabled" to be
# set to "true", in order to communicate account data deletions to clients.
Expand Down
4 changes: 0 additions & 4 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,6 @@ class HttpListenerConfig:
additional_resources: Dict[str, dict] = attr.Factory(dict)
tag: Optional[str] = None
request_id_header: Optional[str] = None
# If true, the listener will return CORS response headers compatible with MSC3886:
# https://github.com/matrix-org/matrix-spec-proposals/pull/3886
experimental_cors_msc3886: bool = False


@attr.s(slots=True, frozen=True, auto_attribs=True)
Expand Down Expand Up @@ -999,7 +996,6 @@ def parse_listener_def(num: int, listener: Any) -> ListenerConfig:
additional_resources=listener.get("additional_resources", {}),
tag=listener.get("tag"),
request_id_header=listener.get("request_id_header"),
experimental_cors_msc3886=listener.get("experimental_cors_msc3886", False),
)

if socket_path:
Expand Down
9 changes: 0 additions & 9 deletions synapse/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -921,15 +921,6 @@ def set_cors_headers(request: "SynapseRequest") -> None:
b"Access-Control-Expose-Headers",
b"Synapse-Trace-Id, Server, ETag",
)
elif request.experimental_cors_msc3886:
request.setHeader(
b"Access-Control-Allow-Headers",
b"X-Requested-With, Content-Type, Authorization, Date, If-Match, If-None-Match",
)
request.setHeader(
b"Access-Control-Expose-Headers",
b"ETag, Location, X-Max-Bytes",
)
else:
request.setHeader(
b"Access-Control-Allow-Headers",
Expand Down
5 changes: 0 additions & 5 deletions synapse/http/site.py
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,6 @@ def __init__(
self.reactor = site.reactor
self._channel = channel # this is used by the tests
self.start_time = 0.0
self.experimental_cors_msc3886 = site.experimental_cors_msc3886

# The requester, if authenticated. For federation requests this is the
# server name, for client requests this is the Requester object.
Expand Down Expand Up @@ -666,10 +665,6 @@ def __init__(

request_id_header = config.http_options.request_id_header

self.experimental_cors_msc3886: bool = (
config.http_options.experimental_cors_msc3886
)

def request_factory(channel: HTTPChannel, queued: bool) -> Request:
return request_class(
channel,
Expand Down
48 changes: 0 additions & 48 deletions synapse/rest/client/rendezvous.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,51 +34,6 @@
logger = logging.getLogger(__name__)


# n.b [MSC3886](https://github.com/matrix-org/matrix-spec-proposals/pull/3886) has now been closed.
# However, we want to keep this implementation around for some time.
# TODO: define an end-of-life date for this implementation.
class MSC3886RendezvousServlet(RestServlet):
"""
This is a placeholder implementation of [MSC3886](https://github.com/matrix-org/matrix-spec-proposals/pull/3886)
simple client rendezvous capability that is used by the "Sign in with QR" functionality.

This implementation only serves as a 307 redirect to a configured server rather than being a full implementation.

A module that implements the full functionality is available at: https://pypi.org/project/matrix-http-rendezvous-synapse/.

Request:

POST /rendezvous HTTP/1.1
Content-Type: ...

...

Response:

HTTP/1.1 307
Location: <configured endpoint>
"""

PATTERNS = client_patterns(
"/org.matrix.msc3886/rendezvous$", releases=[], v1=False, unstable=True
)

def __init__(self, hs: "HomeServer"):
super().__init__()
redirection_target: Optional[str] = hs.config.experimental.msc3886_endpoint
assert (
redirection_target is not None
), "Servlet is only registered if there is a redirection target"
self.endpoint = redirection_target.encode("utf-8")

async def on_POST(self, request: SynapseRequest) -> None:
respond_with_redirect(
request, self.endpoint, statusCode=TEMPORARY_REDIRECT, cors=True
)

# PUT, GET and DELETE are not implemented as they should be fulfilled by the redirect target.


class MSC4108DelegationRendezvousServlet(RestServlet):
PATTERNS = client_patterns(
"/org.matrix.msc4108/rendezvous$", releases=[], v1=False, unstable=True
Expand Down Expand Up @@ -114,9 +69,6 @@ def on_POST(self, request: SynapseRequest) -> None:


def register_servlets(hs: "HomeServer", http_server: HttpServer) -> None:
if hs.config.experimental.msc3886_endpoint is not None:
MSC3886RendezvousServlet(hs).register(http_server)

if hs.config.experimental.msc4108_enabled:
MSC4108RendezvousServlet(hs).register(http_server)

Expand Down
3 changes: 0 additions & 3 deletions synapse/rest/client/versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,6 @@ async def on_GET(self, request: SynapseRequest) -> Tuple[int, JsonDict]:
"org.matrix.msc3881": msc3881_enabled,
# Adds support for filtering /messages by event relation.
"org.matrix.msc3874": self.config.experimental.msc3874_enabled,
# Adds support for simple HTTP rendezvous as per MSC3886
"org.matrix.msc3886": self.config.experimental.msc3886_endpoint
is not None,
# Adds support for relation-based redactions as per MSC3912.
"org.matrix.msc3912": self.config.experimental.msc3912_enabled,
# Whether recursively provide relations is supported.
Expand Down
1 change: 0 additions & 1 deletion tests/logging/test_terse_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,6 @@ def test_with_request_context(self) -> None:
site.site_tag = "test-site"
site.server_version_string = "Server v1"
site.reactor = Mock()
site.experimental_cors_msc3886 = False
request = SynapseRequest(
cast(HTTPChannel, FakeChannel(site, self.reactor)), site
)
Expand Down
9 changes: 0 additions & 9 deletions tests/rest/client/test_rendezvous.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
from tests.unittest import override_config
from tests.utils import HAS_AUTHLIB

msc3886_endpoint = "/_matrix/client/unstable/org.matrix.msc3886/rendezvous"
msc4108_endpoint = "/_matrix/client/unstable/org.matrix.msc4108/rendezvous"


Expand All @@ -54,17 +53,9 @@ def create_resource_dict(self) -> Dict[str, Resource]:
}

def test_disabled(self) -> None:
channel = self.make_request("POST", msc3886_endpoint, {}, access_token=None)
self.assertEqual(channel.code, 404)
channel = self.make_request("POST", msc4108_endpoint, {}, access_token=None)
self.assertEqual(channel.code, 404)

@override_config({"experimental_features": {"msc3886_endpoint": "/asd"}})
def test_msc3886_redirect(self) -> None:
channel = self.make_request("POST", msc3886_endpoint, {}, access_token=None)
self.assertEqual(channel.code, 307)
self.assertEqual(channel.headers.getRawHeaders("Location"), ["/asd"])

@unittest.skip_unless(HAS_AUTHLIB, "requires authlib")
@override_config(
{
Expand Down
2 changes: 0 additions & 2 deletions tests/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,6 @@ def __init__(
self,
resource: IResource,
reactor: IReactorTime,
experimental_cors_msc3886: bool = False,
):
"""

Expand All @@ -350,7 +349,6 @@ def __init__(
"""
self._resource = resource
self.reactor = reactor
self.experimental_cors_msc3886 = experimental_cors_msc3886

def getResourceFor(self, request: Request) -> IResource:
return self._resource
Expand Down
41 changes: 1 addition & 40 deletions tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -233,9 +233,7 @@ def render(self, request: SynapseRequest) -> bytes:
self.resource = OptionsResource()
self.resource.putChild(b"res", DummyResource())

def _make_request(
self, method: bytes, path: bytes, experimental_cors_msc3886: bool = False
) -> FakeChannel:
def _make_request(self, method: bytes, path: bytes) -> FakeChannel:
"""Create a request from the method/path and return a channel with the response."""
# Create a site and query for the resource.
site = SynapseSite(
Expand All @@ -246,7 +244,6 @@ def _make_request(
{
"type": "http",
"port": 0,
"experimental_cors_msc3886": experimental_cors_msc3886,
},
),
self.resource,
Expand Down Expand Up @@ -283,32 +280,6 @@ def _check_cors_standard_headers(self, channel: FakeChannel) -> None:
[b"Synapse-Trace-Id, Server"],
)

def _check_cors_msc3886_headers(self, channel: FakeChannel) -> None:
# Ensure the correct CORS headers have been added
# as per https://github.com/matrix-org/matrix-spec-proposals/blob/hughns/simple-rendezvous-capability/proposals/3886-simple-rendezvous-capability.md#cors
self.assertEqual(
channel.headers.getRawHeaders(b"Access-Control-Allow-Origin"),
[b"*"],
"has correct CORS Origin header",
)
self.assertEqual(
channel.headers.getRawHeaders(b"Access-Control-Allow-Methods"),
[b"GET, HEAD, POST, PUT, DELETE, OPTIONS"], # HEAD isn't in the spec
"has correct CORS Methods header",
)
self.assertEqual(
channel.headers.getRawHeaders(b"Access-Control-Allow-Headers"),
[
b"X-Requested-With, Content-Type, Authorization, Date, If-Match, If-None-Match"
],
"has correct CORS Headers header",
)
self.assertEqual(
channel.headers.getRawHeaders(b"Access-Control-Expose-Headers"),
[b"ETag, Location, X-Max-Bytes"],
"has correct CORS Expose Headers header",
)

def test_unknown_options_request(self) -> None:
"""An OPTIONS requests to an unknown URL still returns 204 No Content."""
channel = self._make_request(b"OPTIONS", b"/foo/")
Expand All @@ -325,16 +296,6 @@ def test_known_options_request(self) -> None:

self._check_cors_standard_headers(channel)

def test_known_options_request_msc3886(self) -> None:
"""An OPTIONS requests to an known URL still returns 204 No Content."""
channel = self._make_request(
b"OPTIONS", b"/res/", experimental_cors_msc3886=True
)
self.assertEqual(channel.code, 204)
self.assertNotIn("body", channel.result)

self._check_cors_msc3886_headers(channel)

def test_unknown_request(self) -> None:
"""A non-OPTIONS request to an unknown URL should 404."""
channel = self._make_request(b"GET", b"/foo/")
Expand Down