Add support for knocking to workers

[dklimpel]

Fixes: #12806

Code was implemented with #6739, bot not used/activated.

Part of knocking only is:

• ^/_matrix/client/(api/v1|r0|v3|unstable)/knock/

and not:

• ^/_matrix/client/(api/v1|r0|v3|unstable)/rooms/.*/(join|invite|leave|ban|unban|kick)$

synapse/synapse/rest/client/room.py

Lines 928 to 932 in 8219525

	def register(self, http_server: HttpServer) -> None:
	# /rooms/$roomid/[invite|join|leave]
	PATTERNS = (
	"/rooms/(?P<room_id>[^/]*)/"
	"(?P<membership_action>join|invite|leave|ban|unban|kick)"

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Pull request includes a sign off
• Code style is correct
  (run the linters)

Signed-off-by: Dirk Klimpel 5740567+dklimpel@users.noreply.github.com

[clokep]

@dklimpel Looks like all the knocking tests are failing when workers are used, so this sounds like it might not be safe after all?

[dklimpel]

Current main issue:

event_creator1 | 2023-02-22 11:55:37,717 - synapse.http.server - 123 - ERROR - POST-28 - Failed handle request via 'KnockRoomAliasServlet': <SynapseRequest at 0x7fb9bad4d490 method='POST' uri='/_matrix/client/v3/knock/%21jrbrudPMkOwghwHXsW:hs1?server_name=hs1' clientproto='HTTP/1.0' site='18013'>
  Traceback (most recent call last):
    File "/usr/local/lib/python3.11/site-packages/synapse/http/server.py", line 306, in _async_render_wrapper
      callback_return = await self._async_render(request)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/http/server.py", line 516, in _async_render
      callback_return = await raw_callback_return
                        ^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/rest/client/knock.py", line 84, in on_POST
      await self.room_member_handler.update_membership(
    File "/usr/local/lib/python3.11/site-packages/synapse/handlers/room_member.py", line 613, in update_membership
      result = await self.update_membership_locked(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/handlers/room_member.py", line 1072, in update_membership_locked
      return await self.remote_knock(
             ^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/handlers/room_member_worker.py", line 125, in remote_knock
      ret = await self._remote_knock_client(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/replication/http/_base.py", line 237, in send_request
      data = await cls._serialize_payload(**kwargs)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  TypeError: ReplicationRemoteKnockRestServlet._serialize_payload() got an unexpected keyword argument 'user'

synapse/synapse/handlers/room_member_worker.py

Lines 114 to 131 in 6cddf24

	async def remote_knock(
	self,
	remote_room_hosts: List[str],
	room_id: str,
	user: UserID,
	content: dict,
	) -> Tuple[str, int]:
	"""Sends a knock to a room.
	Implements RoomMemberHandler.remote_knock
	"""
	ret = await self._remote_knock_client(
	remote_room_hosts=remote_room_hosts,
	room_id=room_id,
	user=user,
	content=content,
	)
	return ret["event_id"], ret["stream_id"]

[dklimpel]

current status

  event_creator1 | 2023-02-24 13:12:17,333 - synapse.http.server - 123 - ERROR - POST-9 - Failed handle request via 'KnockRoomAliasServlet': <SynapseRequest at 0x7f1f68b36490 method='POST' uri='/_matrix/client/v3/knock/%21xPTqglkCzOYcCiwtuD:hs1?server_name=hs1' clientproto='HTTP/1.0' site='18013'>
  Traceback (most recent call last):
    File "/usr/local/lib/python3.11/site-packages/synapse/http/server.py", line 306, in _async_render_wrapper
      callback_return = await self._async_render(request)
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/http/server.py", line 516, in _async_render
      callback_return = await raw_callback_return
                        ^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/rest/client/knock.py", line 84, in on_POST
      await self.room_member_handler.update_membership(
    File "/usr/local/lib/python3.11/site-packages/synapse/handlers/room_member.py", line 614, in update_membership
      result = await self.update_membership_locked(
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/handlers/room_member.py", line 1073, in update_membership_locked
      return await self.remote_knock(
             ^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/handlers/room_member_worker.py", line 126, in remote_knock
      ret = await self._remote_knock_client(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/site-packages/synapse/replication/http/_base.py", line 257, in send_request
      url_args = [
                 ^
    File "/usr/local/lib/python3.11/site-packages/synapse/replication/http/_base.py", line 258, in <listcomp>
      urllib.parse.quote(kwargs[name], safe="") for name in cls.PATH_ARGS
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/urllib/parse.py", line 867, in quote
      return quote_from_bytes(string, safe)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    File "/usr/local/lib/python3.11/urllib/parse.py", line 897, in quote_from_bytes
      raise TypeError("quote_from_bytes() expected bytes")
  TypeError: quote_from_bytes() expected bytes

[DMRobertson]

The arguments you pass to _remote_knock_client need to have the same names as the arguments to _serialise_payload

synapse/synapse/replication/http/membership.py

Lines 122 to 142 in 9187fd9

	@staticmethod
	async def _serialize_payload( # type: ignore[override]
	requester: Requester,
	room_id: str,
	user_id: str,
	remote_room_hosts: List[str],
	content: JsonDict,
	) -> JsonDict:
	"""
	Args:
	requester: The user making the request, according to the access token.
	room_id: The ID of the room to knock on.
	user_id: The ID of the knocking user.
	remote_room_hosts: Servers to try and send the knock via.
	content: The event content to use for the knock event.
	"""
	return {
	"requester": requester.serialize(),
	"remote_room_hosts": remote_room_hosts,
	"content": content,
	}

and any arguments that appear in the URL instead of the request body need to have matching names in PATH_ARGS:

synapse/synapse/replication/http/membership.py

Line 113 in 9187fd9

PATH_ARGS = ("room_id", "user_id")

[DMRobertson]

Oh ignore me: I think that's already the case as of your PR. Hmm.

[dklimpel]

Thank your for the hints. It will help me.

[dklimpel]

Is there a hint how to fix ist?

2023-02-27T09:47:51.8325487Z ##[group]�[0;31m❌ TestKnocking/A_user_that_has_already_knocked_is_allowed_to_knock_again_on_the_same_room#01�[0;37m (10ms)�[0m
2023-02-27T09:47:51.8325973Z     knocking_test.go:359: MatchResponse got status 404 want 200 - http://127.0.0.1:33246/_matrix/client/v3/knock/%21TBuAgdkMCuKOqIesve:hs1?server_name=hs1 => {"errcode":"M_UNRECOGNIZED","error":"Unrecognized request"}

[DMRobertson]

{"errcode":"M_UNRECOGNIZED","error":"Unrecognized request"}

I would guess that in the complement-with-workers config we're missing a listener/resource on some kind of worker.

There might be some clues in the top-level logs: https://github.com/matrix-org/synapse/actions/runs/4280704294/jobs/7452884787#step:6:5536

[dklimpel]

There is not test that call ReplicationRemoteRescindKnockRestServlet (/_synapse/replication/remote_rescind_knock/:event_id) at the moment.

[clokep]

Sorry for the delay here, thanks for fixing this! A real shame we had all the code and just never hooked it up. 😢

[clokep]

There is not test that call ReplicationRemoteRescindKnockRestServlet (/_synapse/replication/remote_rescind_knock/:event_id) at the moment.

That's slightly concerning -- did you try it manually?

[dklimpel]

Unfortunately not.
I've only found that all tests pass, even if the servlet is commented out.