diff --git a/CHANGELOG.md b/CHANGELOG.md index 1ce759d..a63d301 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ and this project adheres to - 🧑‍💻(docker) split frontend to another file #924 +### Fixed + +- 🐛(webhook) handle user on different home server than room server + ## [1.17.0] - 2025-06-11 ### Added diff --git a/src/backend/core/tests/fixtures/matrix.py b/src/backend/core/tests/fixtures/matrix.py index 9ce5f4f..03c451d 100644 --- a/src/backend/core/tests/fixtures/matrix.py +++ b/src/backend/core/tests/fixtures/matrix.py @@ -3,6 +3,54 @@ from rest_framework import status +# SEARCH +def mock_search_empty(): + """Mock response when no Matrix user has been found through search.""" + return { + "message": {"limited": "false", "results": []}, + "status_code": status.HTTP_200_OK, + } + + +def mock_search_successful(user): + """Mock response when exactly one user has been found through search.""" + return { + "message": { + "limited": "false", + "results": [ + { + "user_id": f"@{user.email.replace('@', '-')}:user_server.com", + "display_name": f"@{user.name} [Fake]", + "avatar_url": "null", + }, + ], + }, + "status_code": status.HTTP_200_OK, + } + + +def mock_search_successful_multiple(user): + """Mock response when more than one user has been found through search.""" + return { + "message": { + "limited": "false", + "results": [ + { + "user_id": f"@{user.email.replace('@', '-')}:user_server1.com", + "display_name": f"@{user.name} [Fake]", + "avatar_url": "null", + }, + { + "user_id": f"@{user.email.replace('@', '-')}:user_server2.com", + "display_name": f"@{user.name} [Other Fake]", + "avatar_url": "null", + }, + ], + }, + "status_code": status.HTTP_200_OK, + } + + # JOIN ROOMS def mock_join_room_successful(room_id): """Mock response when succesfully joining room. Same response if already in room.""" @@ -39,7 +87,7 @@ def mock_invite_user_already_in_room(user): return { "message": { "errcode": "M_FORBIDDEN", - "error": f"{user.email.replace('@', ':')} is already in the room.", + "error": f"{user.email.replace('@', '-')}:home_server.fr is already in the room.", }, "status_code": status.HTTP_403_FORBIDDEN, } @@ -56,7 +104,7 @@ def mock_kick_user_forbidden(user): return { "message": { "errcode": "M_FORBIDDEN", - "error": f"You cannot kick user @{user.email.replace('@', ':')}.", + "error": f"You cannot kick user @{user.email.replace('@', '-')}.", }, "status_code": status.HTTP_403_FORBIDDEN, } diff --git a/src/backend/core/tests/team_accesses/test_api_team_accesses_create.py b/src/backend/core/tests/team_accesses/test_api_team_accesses_create.py index 27fefdb..c25240b 100644 --- a/src/backend/core/tests/team_accesses/test_api_team_accesses_create.py +++ b/src/backend/core/tests/team_accesses/test_api_team_accesses_create.py @@ -247,7 +247,7 @@ def test_api_team_accesses_create__with_matrix_webhook(): team = factories.TeamFactory(users=[(user, "owner")]) webhook = factories.TeamWebhookFactory( team=team, - url="https://server.fr/#/room/room_id:home_server.fr", + url="https://server.fr/#/room/room_id:room_server.fr", secret="some-secret-you-should-not-store-on-a-postit", protocol=enums.WebhookProtocolChoices.MATRIX, ) @@ -264,6 +264,11 @@ def test_api_team_accesses_create__with_matrix_webhook(): status=matrix.mock_join_room_successful("room_id")["status_code"], content_type="application/json", ) + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful(other_user)["message"]), + status=matrix.mock_search_successful(user)["status_code"], + ) responses.post( re.compile(r".*/invite"), body=str(matrix.mock_invite_successful()["message"]), @@ -281,22 +286,23 @@ def test_api_team_accesses_create__with_matrix_webhook(): ) assert response.status_code == status.HTTP_201_CREATED - assert len(responses.calls) == 2 + assert len(responses.calls) == 3 assert ( responses.calls[0].request.url - == "https://home_server.fr/_matrix/client/v3/rooms/room_id:home_server.fr/join" + == "https://room_server.fr/_matrix/client/v3/rooms/room_id:room_server.fr/join" ) # Payload sent to matrix server assert webhook.secret in responses.calls[0].request.headers["Authorization"] - assert json.loads(responses.calls[1].request.body) == { - "user_id": f"@{other_user.email.replace('@', ':')}", + assert json.loads(responses.calls[2].request.body) == { + "user_id": f"@{other_user.email.replace('@', '-')}:user_server.com", "reason": f"User added to team {webhook.team} on People", } assert models.TeamAccess.objects.filter(user=other_user, team=team).exists() +@responses.activate def test_api_team_accesses_create__multiple_webhooks_success(caplog): """ When the team has multiple webhooks, creating a team access should fire all the expected calls. @@ -312,7 +318,7 @@ def test_api_team_accesses_create__multiple_webhooks_success(caplog): ) webhook_matrix = factories.TeamWebhookFactory( team=team, - url="https://www.webhookserver.fr/#/room/room_id:home_server/", + url="https://www.webhookserver.fr/#/room/room_id:home_server.fr/", protocol=enums.WebhookProtocolChoices.MATRIX, secret="yo", ) @@ -322,39 +328,40 @@ def test_api_team_accesses_create__multiple_webhooks_success(caplog): client = APIClient() client.force_login(user) - with responses.RequestsMock() as rsps: - # Ensure successful response by scim provider using "responses": - rsps.add( - rsps.PATCH, - re.compile(r".*/Groups/.*"), - body="{}", - status=200, - content_type="application/json", - ) - rsps.add( - rsps.POST, - re.compile(r".*/join"), - body=str(matrix.mock_join_room_successful), - status=status.HTTP_200_OK, - content_type="application/json", - ) - rsps.add( - rsps.POST, - re.compile(r".*/invite"), - body=str(matrix.mock_invite_successful()["message"]), - status=matrix.mock_invite_successful()["status_code"], - content_type="application/json", - ) + # Ensure successful response by scim provider using "responses": + responses.patch( + re.compile(r".*/Groups/.*"), + body="{}", + status=status.HTTP_200_OK, + content_type="application/json", + ) + responses.post( + re.compile(r".*/join"), + body=str(matrix.mock_join_room_successful("room_id")["message"]), + status=status.HTTP_200_OK, + content_type="application/json", + ) + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful(user)["message"]), + status=matrix.mock_search_successful(user)["status_code"], + ) + responses.post( + re.compile(r".*/invite"), + body=str(matrix.mock_invite_successful()["message"]), + status=matrix.mock_invite_successful()["status_code"], + content_type="application/json", + ) - response = client.post( - f"/api/v1.0/teams/{team.id!s}/accesses/", - { - "user": str(other_user.id), - "role": role, - }, - format="json", - ) - assert response.status_code == 201 + response = client.post( + f"/api/v1.0/teams/{team.id!s}/accesses/", + { + "user": str(other_user.id), + "role": role, + }, + format="json", + ) + assert response.status_code == 201 # Logger log_messages = [msg.message for msg in caplog.records] diff --git a/src/backend/core/tests/utils/test_webhooks_matrix_client.py b/src/backend/core/tests/utils/test_webhooks_matrix_client.py index e29797c..d4b8aac 100644 --- a/src/backend/core/tests/utils/test_webhooks_matrix_client.py +++ b/src/backend/core/tests/utils/test_webhooks_matrix_client.py @@ -13,11 +13,63 @@ from rest_framework import status from core import factories from core.enums import WebhookProtocolChoices from core.tests.fixtures import matrix +from core.utils.matrix import MatrixAPIClient from core.utils.webhooks import webhooks_synchronizer pytestmark = pytest.mark.django_db +## SEARCH +@responses.activate +def test_matrix_webhook__search_user_unknown(caplog): + """When searching for a user in the Matrix federation but cannot find any, + we invite a (future ?) user using user's email and room's server.""" + caplog.set_level(logging.INFO) + + user = factories.UserFactory() + webhook = factories.TeamWebhookFactory( + protocol=WebhookProtocolChoices.MATRIX, + url="https://www.matrix.org/#/room/room_id:room_server.au", + secret="secret-access-token", + ) + + client = MatrixAPIClient() + # Mock successful responses + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_empty()["message"]), + status=status.HTTP_200_OK, + content_type="application/json", + ) + response = client.get_user_id(user=user, webhook=webhook) + assert response == f"@{user.email.replace('@', '-')}:room_server.au" + + +@responses.activate +def test_matrix_webhook__search_multiple_ids(caplog): + """When searching for a user in Matrix federation, + if user directory returns multiple ids, invite the first one.""" + caplog.set_level(logging.INFO) + + user = factories.UserFactory() + webhook = factories.TeamWebhookFactory( + protocol=WebhookProtocolChoices.MATRIX, + url="https://www.matrix.org/#/room/room_id:room_server.au", + secret="secret-access-token", + ) + + client = MatrixAPIClient() + # Mock successful responses + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful_multiple(user)["message"]), + status=status.HTTP_200_OK, + content_type="application/json", + ) + response = client.get_user_id(user=user, webhook=webhook) + assert response == f"@{user.email.replace('@', '-')}:user_server1.com" + + ## INVITE @responses.activate def test_matrix_webhook__invite_user_to_room_forbidden(caplog): @@ -38,6 +90,11 @@ def test_matrix_webhook__invite_user_to_room_forbidden(caplog): body=str(matrix.mock_join_room_successful), status=status.HTTP_200_OK, ) + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful(user)["message"]), + status=matrix.mock_search_successful(user)["status_code"], + ) responses.post( re.compile(r".*/invite"), body=str(error["message"]), @@ -64,6 +121,11 @@ def test_matrix_webhook__invite_user_to_room_already_in_room(caplog): body=str(matrix.mock_join_room_successful("room_id")["message"]), status=matrix.mock_join_room_successful("room_id")["status_code"], ) + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful(user)["message"]), + status=matrix.mock_search_successful(user)["status_code"], + ) responses.post( re.compile(r".*/invite"), body=str(matrix.mock_invite_user_already_in_room(user)["message"]), @@ -101,6 +163,11 @@ def test_matrix_webhook__invite_user_to_room_success(caplog): body=str(matrix.mock_join_room_successful("room_id")["message"]), status=matrix.mock_join_room_successful("room_id")["status_code"], ) + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful(user)["message"]), + status=matrix.mock_search_successful(user)["status_code"], + ) responses.post( re.compile(r".*/invite"), body=str(matrix.mock_invite_successful()["message"]), @@ -113,8 +180,8 @@ def test_matrix_webhook__invite_user_to_room_success(caplog): assert webhook.secret in headers["Authorization"] # Check payloads sent to Matrix API - assert json.loads(responses.calls[1].request.body) == { - "user_id": f"@{user.email.replace('@', ':')}", + assert json.loads(responses.calls[2].request.body) == { + "user_id": f"@{user.email.replace('@', '-')}:user_server.com", "reason": f"User added to team {webhook.team} on People", } @@ -147,6 +214,11 @@ def test_matrix_webhook__override_secret_for_tchap(): body=str(matrix.mock_join_room_successful("room_id")["message"]), status=matrix.mock_join_room_successful("room_id")["status_code"], ) + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful(user)["message"]), + status=matrix.mock_search_successful(user)["status_code"], + ) responses.post( re.compile(r".*/invite"), body=str(matrix.mock_invite_successful()["message"]), @@ -178,6 +250,11 @@ def test_matrix_webhook__kick_user_from_room_not_in_room(caplog): body=str(matrix.mock_join_room_successful), status=status.HTTP_200_OK, ) + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful(user)["message"]), + status=matrix.mock_search_successful(user)["status_code"], + ) responses.post( re.compile(r".*/kick"), body=str(matrix.mock_kick_user_not_in_room()["message"]), @@ -205,7 +282,7 @@ def test_matrix_webhook__kick_user_from_room_success(caplog): user = factories.UserFactory() webhook = factories.TeamWebhookFactory( protocol=WebhookProtocolChoices.MATRIX, - url="https://www.matrix.org/#/room/room_id:home_server", + url="https://www.matrix.org/#/room/room_id:room_server", secret="secret-access-token", ) @@ -214,6 +291,11 @@ def test_matrix_webhook__kick_user_from_room_success(caplog): body=str(matrix.mock_join_room_successful), status=status.HTTP_200_OK, ) + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful(user)["message"]), + status=matrix.mock_search_successful(user)["status_code"], + ) responses.post( re.compile(r".*/kick"), body=str(matrix.mock_kick_successful), @@ -222,8 +304,8 @@ def test_matrix_webhook__kick_user_from_room_success(caplog): webhooks_synchronizer.remove_user_from_group(team=webhook.team, user=user) # Check payloads sent to Matrix API - assert json.loads(responses.calls[1].request.body) == { - "user_id": f"@{user.email.replace('@', ':')}", + assert json.loads(responses.calls[2].request.body) == { + "user_id": f"@{user.email.replace('@', '-')}:user_server.com", "reason": f"User removed from team {webhook.team} on People", } @@ -258,6 +340,11 @@ def test_matrix_webhook__kick_user_from_room_forbidden(caplog): body=str(matrix.mock_join_room_successful), status=status.HTTP_200_OK, ) + responses.post( + re.compile(r".*/search"), + body=json.dumps(matrix.mock_search_successful(user)["message"]), + status=matrix.mock_search_successful(user)["status_code"], + ) responses.post( re.compile(r".*/kick"), body=str(error["message"]), diff --git a/src/backend/core/utils/matrix.py b/src/backend/core/utils/matrix.py index 976127d..4cacd30 100644 --- a/src/backend/core/utils/matrix.py +++ b/src/backend/core/utils/matrix.py @@ -28,7 +28,7 @@ session.mount("https://", adapter) class MatrixAPIClient: """A client to interact with Matrix API""" - + def get_headers(self, webhook): """Build header dict from webhook object.""" headers = {"Content-Type": "application/json"} @@ -49,12 +49,27 @@ class MatrixAPIClient: base_url = f"matrix.{base_url}" return f"https://{base_url}/_matrix/client/v3/rooms/{room_id}" - def get_user_id(self, user): + def get_user_id(self, user, webhook): """Returns user id from email.""" if user.email is None: raise ValueError("You must first set an email for the user.") - return f"@{user.email.replace('@', ':')}" + if settings.MATRIX_BASE_HOME_SERVER: + home_server = settings.MATRIX_BASE_HOME_SERVER + search = session.post( + f"{home_server}/_matrix/client/v3/user_directory/search", + json={"search_term": f"@{user.email.replace('@', '-')}"}, + headers=self.get_headers(webhook), + verify=True, + timeout=3, + ) + results = search.json()["results"] + if len(results) > 0: + return results[0]["user_id"] + + # try and invite unknown user using room home server + room_home_server = webhook.url.split(":")[2] + return f"@{user.email.replace('@', '-')}:{room_home_server}" def join_room(self, webhook): """Accept invitation to the room. As of today, it is a mandatory step @@ -76,8 +91,11 @@ class MatrixAPIClient: webhook.url, ) return join_response, False + logger.info( + "Succesfully joined room", + ) - user_id = self.get_user_id(user) + user_id = self.get_user_id(user, webhook) response = session.post( f"{self._get_room_url(webhook.url)}/invite", json={ @@ -110,7 +128,7 @@ class MatrixAPIClient: ) return join_response, False - user_id = self.get_user_id(user) + user_id = self.get_user_id(user, webhook) response = session.post( f"{self._get_room_url(webhook.url)}/kick", json={ diff --git a/src/backend/core/utils/webhooks.py b/src/backend/core/utils/webhooks.py index f0f06a1..556831e 100644 --- a/src/backend/core/utils/webhooks.py +++ b/src/backend/core/utils/webhooks.py @@ -32,7 +32,6 @@ class WebhookClient: response, webhook_succeeded = self._get_response_and_status( name, webhook, user ) - if response is not None: extra = {"response": response.content} # pylint: disable=no-member diff --git a/src/backend/people/settings.py b/src/backend/people/settings.py index 36267fb..5890cea 100755 --- a/src/backend/people/settings.py +++ b/src/backend/people/settings.py @@ -601,6 +601,11 @@ class Base(Configuration): environ_name="DNS_PROVISIONING_API_CREDENTIALS", environ_prefix=None, ) + MATRIX_BASE_HOME_SERVER = values.Value( + default="https://matrix.agent.dinum.tchap.gouv.fr", + environ_name="MATRIX_BASE_HOME_SERVER", + environ_prefix=None, + ) MATRIX_BOT_ACCESS_TOKEN = values.Value( default=None, environ_name="MATRIX_BOT_ACCESS_TOKEN",