Skip to content
Merged
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/19033.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Stabilized support for [MSC4326](https://github.com/matrix-org/matrix-spec-proposals/pull/4326): Device masquerading for appservices. Contributed by @tulir @ Beeper.
22 changes: 10 additions & 12 deletions synapse/api/auth/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,12 +302,9 @@ async def get_appservice_user(
(the user_id URI parameter allows an application service to masquerade
any applicable user in its namespace)
- what device the application service should be treated as controlling
(the device_id[^1] URI parameter allows an application service to masquerade
(the device_id URI parameter allows an application service to masquerade
as any device that exists for the relevant user)

[^1] Unstable and provided by MSC3202.
Must use `org.matrix.msc3202.device_id` in place of `device_id` for now.

Returns:
the application service `Requester` of that request

Expand All @@ -319,7 +316,8 @@ async def get_appservice_user(
- The returned device ID, if present, has been checked to be a valid device ID
for the returned user ID.
"""
DEVICE_ID_ARG_NAME = b"org.matrix.msc3202.device_id"
# TODO: We can drop unstable support after 2026-01-01 (couple months after stable support)
UNSTABLE_DEVICE_ID_ARG_NAME = b"org.matrix.msc3202.device_id"

app_service = self.store.get_app_service_by_token(access_token)
if app_service is None:
Expand All @@ -341,13 +339,11 @@ async def get_appservice_user(
else:
effective_user_id = app_service.sender

effective_device_id: Optional[str] = None

if (
self.hs.config.experimental.msc3202_device_masquerading_enabled
and DEVICE_ID_ARG_NAME in request.args
):
effective_device_id = request.args[DEVICE_ID_ARG_NAME][0].decode("utf8")
effective_device_id_args = request.args.get(
b"device_id", request.args.get(UNSTABLE_DEVICE_ID_ARG_NAME)
)
if effective_device_id_args:
effective_device_id = effective_device_id_args[0].decode("utf8")
# We only just set this so it can't be None!
Copy link
Contributor

Choose a reason for hiding this comment

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

This is pre-existing but I don't understand this comment. As far as I can tell, this is user-provided input

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think .decode("utf8") should never return None (invalid unicode would throw an error) and the list is probably also guaranteed to only contain bytes, so the assertion should never fail

Copy link
Contributor

Choose a reason for hiding this comment

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

I see. It makes sense that it can't be None.

Perhaps this was done to appease mypy but if I reveal_type(effective_device_id) before/after the assertion, even the types don't show it as being possible to be None as you explained.

synapse/api/auth/base.py:351: note: Revealed type is "Union[builtins.str, Any]"
synapse/api/auth/base.py:354: note: Revealed type is "Union[builtins.str, Any]"

I guess we can remove that check altogether.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave it for now since the diff no longer touches that line at all

assert effective_device_id is not None
device_opt = await self.store.get_device(
Expand All @@ -359,6 +355,8 @@ async def get_appservice_user(
f"Application service trying to use a device that doesn't exist ('{effective_device_id}' for {effective_user_id})",
Codes.UNKNOWN_DEVICE,
)
else:
effective_device_id = None

return create_requester(
effective_user_id, app_service=app_service, device_id=effective_device_id
Expand Down
5 changes: 0 additions & 5 deletions synapse/config/experimental.py
Original file line number Diff line number Diff line change
Expand Up @@ -412,11 +412,6 @@ def read_config(
"msc2409_to_device_messages_enabled", False
)

# The portion of MSC3202 which is related to device masquerading.
self.msc3202_device_masquerading_enabled: bool = experimental.get(
"msc3202_device_masquerading", False
)

# The portion of MSC3202 related to transaction extensions:
# sending device list changes, one-time key counts and fallback key
# usage to application services.
Expand Down
7 changes: 2 additions & 5 deletions tests/api/test_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
from synapse.util.clock import Clock

from tests import unittest
from tests.unittest import override_config
from tests.utils import mock_getRawHeaders


Expand Down Expand Up @@ -237,7 +236,6 @@ def test_get_user_by_req_appservice_valid_token_bad_user_id(self) -> None:
request.requestHeaders.getRawHeaders = mock_getRawHeaders()
self.get_failure(self.auth.get_user_by_req(request), AuthError)

@override_config({"experimental_features": {"msc3202_device_masquerading": True}})
def test_get_user_by_req_appservice_valid_token_valid_device_id(self) -> None:
"""
Tests that when an application service passes the device_id URL parameter
Expand All @@ -264,15 +262,14 @@ def test_get_user_by_req_appservice_valid_token_valid_device_id(self) -> None:
request.getClientAddress.return_value.host = "127.0.0.1"
request.args[b"access_token"] = [self.test_token]
request.args[b"user_id"] = [masquerading_user_id]
request.args[b"org.matrix.msc3202.device_id"] = [masquerading_device_id]
request.args[b"device_id"] = [masquerading_device_id]
request.requestHeaders.getRawHeaders = mock_getRawHeaders()
requester = self.get_success(self.auth.get_user_by_req(request))
self.assertEqual(
requester.user.to_string(), masquerading_user_id.decode("utf8")
)
self.assertEqual(requester.device_id, masquerading_device_id.decode("utf8"))

@override_config({"experimental_features": {"msc3202_device_masquerading": True}})
def test_get_user_by_req_appservice_valid_token_invalid_device_id(self) -> None:
"""
Tests that when an application service passes the device_id URL parameter
Expand All @@ -299,7 +296,7 @@ def test_get_user_by_req_appservice_valid_token_invalid_device_id(self) -> None:
request.getClientAddress.return_value.host = "127.0.0.1"
request.args[b"access_token"] = [self.test_token]
request.args[b"user_id"] = [masquerading_user_id]
request.args[b"org.matrix.msc3202.device_id"] = [masquerading_device_id]
request.args[b"device_id"] = [masquerading_device_id]
request.requestHeaders.getRawHeaders = mock_getRawHeaders()

failure = self.get_failure(self.auth.get_user_by_req(request), AuthError)
Expand Down
Loading