Documentation incorrectly states that the room list always returns an empty list if disabled, while federation actually returns a 403 error

[MTRNord]

Description

https://github.com/matrix-org/synapse/blob/develop/docs/usage/configuration/config_documentation.md?plain=1#L3337-L3339 states that both internally and externally synapse will return an empty list on ALL queries.

However

synapse/synapse/federation/transport/client.py

Line 491 in c52abc1

"You are not allowed to view the public rooms list of %s"

(and testing it via fed) states that actually M_FORBIDDEN and a 403 error is returned.

Steps to reproduce

• Access a remote room list via fed where the remote server runs default config.

Homeserver

Not relevant. This is a docs issue.

Synapse Version

1.61.0

Installation Method

Docker (matrixdotorg/synapse)

Platform

Not relevant. This is a docs issue.

Relevant log output

Not relevant. This is a docs issue.

Anything else that would be useful to know?

No response

[MTRNord]

@deepbluev7 Mentioned also that this might actually be an implementation bug, as https://spec.matrix.org/v1.3/server-server-api/#public-room-directory states that the docs mention the correct thing and 403 is not a valid response from the federation endpoint on fed side. (It's also not a valid response on client side according to https://spec.matrix.org/v1.3/client-server-api/#get_matrixclientv3publicrooms )

[DMRobertson]

I think there are a few things going on here; I don't think your diagnosis is completely correct.

The wording in question is

### `enable_room_list_search`

Set to false to disable searching the public room list. When disabled
blocks searching local and remote room lists for local and remote
users by always returning an empty list for all queries. Defaults to true. 

Where does this config value get used? I see three relevant reads:

synapse/synapse/handlers/directory.py

Lines 453 to 457 in a164a46

	if visibility == "public" and not self.enable_room_list_search:
	# The room list has been disabled.
	raise AuthError(
	403, "This user is not permitted to publish rooms to the room list"
	)

synapse/synapse/handlers/room_list.py

Lines 86 to 87 in 888a29f

	if not self.enable_room_list_search:
	return {"chunk": [], "total_room_count_estimate": 0}

synapse/synapse/handlers/room_list.py

Lines 373 to 374 in 888a29f

	if not self.enable_room_list_search:
	return {"chunk": [], "total_room_count_estimate": 0}

So AFAICS the comment accurately describes the behaviour of calls to /publicRooms (the second two snippets).

The first snippet corresponds to /directory/list/room/<room ID> which is a different endpoint. (See https://spec.matrix.org/v1.3/client-server-api/#get_matrixclientv3directorylistroomroomid and https://spec.matrix.org/v1.3/client-server-api/#put_matrixclientv3directorylistroomroomid). On the PUT version the spec notes

Servers may choose to implement additional access control checks here, for instance that room visibility can only be changed by the room creator or a server administrator.

which sounds reasonable. I think one could argue that this wording makes the 403 return code okay. (An aside for @matrix-org/spec-core-team: it is never clear to me if a list of status codes in the spec is normative and/or exhaustive. I think in practice not, because servers can fall over and return 500 at any time.)

In passing, I note that synapse supports a DELETE /directory/list/room/<room ID> endpoint which appears to be unspecced.

synapse/synapse/rest/client/directory.py

Lines 157 to 166 in e24ff8e

	async def on_DELETE(
	self, request: SynapseRequest, room_id: str
	) -> Tuple[int, JsonDict]:
	requester = await self.auth.get_user_by_req(request)
	await self.directory_handler.edit_published_room_list(
	requester, room_id, "private"
	)
	return 200, {}

[DMRobertson]

TL;DR I don't think Synapse is doing anything obviously wrong (but I'm happy to discuss if you're not convinced).

We should update the docs to better communicate that:

• the empty list trick applies to /publicRooms calls
• disabling this option means that Synapse will refuse to modify the published room list
• the abstract public room list is a separate concept to the published room list

Ugh. The situation is a bit of a mess, isn't it?

[DMRobertson]

Hmm. On a re-read I'm now a bit confused.

However

synapse/synapse/federation/transport/client.py

Line 491 in c52abc1

"You are not allowed to view the public rooms list of %s"

In context, this snippet is propagating a 403 from the remote server forwards with another error message.

synapse/synapse/federation/transport/client.py

Lines 453 to 523 in d9f0922

	async def get_public_rooms(
	self,
	remote_server: str,
	limit: Optional[int] = None,
	since_token: Optional[str] = None,
	search_filter: Optional[Dict] = None,
	include_all_networks: bool = False,
	third_party_instance_id: Optional[str] = None,
	) -> JsonDict:
	"""Get the list of public rooms from a remote homeserver
	See synapse.federation.federation_client.FederationClient.get_public_rooms for
	more information.
	"""
	if search_filter:
	# this uses MSC2197 (Search Filtering over Federation)
	path = _create_v1_path("/publicRooms")
	data: Dict[str, Any] = {
	"include_all_networks": "true" if include_all_networks else "false"
	}
	if third_party_instance_id:
	data["third_party_instance_id"] = third_party_instance_id
	if limit:
	data["limit"] = limit
	if since_token:
	data["since"] = since_token
	data["filter"] = search_filter
	try:
	response = await self.client.post_json(
	destination=remote_server, path=path, data=data, ignore_backoff=True
	)
	except HttpResponseException as e:
	if e.code == 403:
	raise SynapseError(
	403,
	"You are not allowed to view the public rooms list of %s"
	% (remote_server,),
	errcode=Codes.FORBIDDEN,
	)
	raise
	else:
	path = _create_v1_path("/publicRooms")
	args: Dict[str, Union[str, Iterable[str]]] = {
	"include_all_networks": "true" if include_all_networks else "false"
	}
	if third_party_instance_id:
	args["third_party_instance_id"] = (third_party_instance_id,)
	if limit:
	args["limit"] = [str(limit)]
	if since_token:
	args["since"] = [since_token]
	try:
	response = await self.client.get_json(
	destination=remote_server, path=path, args=args, ignore_backoff=True
	)
	except HttpResponseException as e:
	if e.code == 403:
	raise SynapseError(
	403,
	"You are not allowed to view the public rooms list of %s"
	% (remote_server,),
	errcode=Codes.FORBIDDEN,
	)
	raise
	return response

But I can't immediately see where or why this happens.

Can you be more precise about how this came aboute?

• What HTTP request are you making? To which server?
• What's the value of enable_room_list_search on that server? What about on the remote server?

[MTRNord]

Hmm. On a re-read I'm now a bit confused.

However

synapse/synapse/federation/transport/client.py

Line 491 in c52abc1

"You are not allowed to view the public rooms list of %s"

In context, this snippet is propagating a 403 from the remote server forwards with another error message.

synapse/synapse/federation/transport/client.py

Lines 453 to 523 in d9f0922

	async def get_public_rooms(
	self,
	remote_server: str,
	limit: Optional[int] = None,
	since_token: Optional[str] = None,
	search_filter: Optional[Dict] = None,
	include_all_networks: bool = False,
	third_party_instance_id: Optional[str] = None,
	) -> JsonDict:
	"""Get the list of public rooms from a remote homeserver
	See synapse.federation.federation_client.FederationClient.get_public_rooms for
	more information.
	"""
	if search_filter:
	# this uses MSC2197 (Search Filtering over Federation)
	path = _create_v1_path("/publicRooms")
	data: Dict[str, Any] = {
	"include_all_networks": "true" if include_all_networks else "false"
	}
	if third_party_instance_id:
	data["third_party_instance_id"] = third_party_instance_id
	if limit:
	data["limit"] = limit
	if since_token:
	data["since"] = since_token
	data["filter"] = search_filter
	try:
	response = await self.client.post_json(
	destination=remote_server, path=path, data=data, ignore_backoff=True
	)
	except HttpResponseException as e:
	if e.code == 403:
	raise SynapseError(
	403,
	"You are not allowed to view the public rooms list of %s"
	% (remote_server,),
	errcode=Codes.FORBIDDEN,
	)
	raise
	else:
	path = _create_v1_path("/publicRooms")
	args: Dict[str, Union[str, Iterable[str]]] = {
	"include_all_networks": "true" if include_all_networks else "false"
	}
	if third_party_instance_id:
	args["third_party_instance_id"] = (third_party_instance_id,)
	if limit:
	args["limit"] = [str(limit)]
	if since_token:
	args["since"] = [since_token]
	try:
	response = await self.client.get_json(
	destination=remote_server, path=path, args=args, ignore_backoff=True
	)
	except HttpResponseException as e:
	if e.code == 403:
	raise SynapseError(
	403,
	"You are not allowed to view the public rooms list of %s"
	% (remote_server,),
	errcode=Codes.FORBIDDEN,
	)
	raise
	return response

But I can't immediately see where or why this happens.

Can you be more precise about how this came aboute?

• What HTTP request are you making? To which server?

I was simply adding a remote server to my element Web server list. So it should be the client side room list endpoint I am requesting and where I saw the error message from.

• What's the value of enable_room_list_search on that server? What about on the remote server?

The server runs the default docker image config of Synapse. Only modified fields are the required ones. On my own server it is the same.

So my view was client side. The note about the spec came from @deepbluev7 iirc. Either way it feels a little confusing in view of spec. I don't think the Feature is wrong I just find the documentation confusing on this. The fact that element Web just gives a non descriptive error message for this didn't help either. It took quite some time for me to figure out why the room list wasn't reachable on a fresh server.

[turt2live]

An aside for @matrix-org/spec-core-team: it is never clear to me if a list of status codes in the spec is normative and/or exhaustive. I think in practice not, because servers can fall over and return 500 at any time.

@DMRobertson please definitely open an issue for this.