From a39ccfc769c30f53ee200059d54e79f42d980705 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 8 Dec 2021 14:47:43 +0000 Subject: Expand return type of get_appservice_user_id to allow returning a device ID to masquerade as --- synapse/api/auth.py | 44 ++++++++++++++++++++++++++++++++++++-------- 1 file changed, 36 insertions(+), 8 deletions(-) (limited to 'synapse/api') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 44883c6663..1a245c8b5c 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -155,7 +155,9 @@ class Auth: access_token = self.get_access_token_from_request(request) - user_id, app_service = await self._get_appservice_user_id(request) + user_id, _, app_service = await self._get_appservice_user_id_and_device_id( + request + ) if user_id and app_service: if ip_addr and self._track_appservice_user_ips: await self.store.insert_client_ip( @@ -274,33 +276,59 @@ class Auth: 403, "Application service has not registered this user (%s)" % user_id ) - async def _get_appservice_user_id( + async def _get_appservice_user_id_and_device_id( self, request: Request - ) -> Tuple[Optional[str], Optional[ApplicationService]]: + ) -> Tuple[Optional[str], Optional[str], Optional[ApplicationService]]: + """ + Given a request, reads the request parameters to determine: + - whether it's an application service that's making this request + - what user the application service should be treated as controlling + (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 + 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: + 3-tuple of + (user ID?, device ID?, application service?) + + Postconditions: + - If an application service is returned, so is a user ID + - A user ID is never returned without an application service + - A device ID is never returned without a user ID or an application service + - The returned application service, if present, is permitted to control the + returned user ID. + - The returned device ID, if present, has been checked to be a valid device ID + for the returned user ID. + """ app_service = self.store.get_app_service_by_token( self.get_access_token_from_request(request) ) if app_service is None: - return None, None + return None, None, None if app_service.ip_range_whitelist: ip_address = IPAddress(request.getClientIP()) if ip_address not in app_service.ip_range_whitelist: - return None, None + return None, None, None # This will always be set by the time Twisted calls us. assert request.args is not None if b"user_id" not in request.args: - return app_service.sender, app_service + return app_service.sender, None, app_service user_id = request.args[b"user_id"][0].decode("utf8") await self.validate_appservice_can_control_user_id(app_service, user_id) if app_service.sender == user_id: - return app_service.sender, app_service + return app_service.sender, None, app_service - return user_id, app_service + return user_id, None, app_service async def get_user_by_access_token( self, -- cgit 1.5.1 From be8814fcaa2b354a2563fc7fbcf93d98cdc03216 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 8 Dec 2021 14:57:12 +0000 Subject: Expand get_user_by_req to support handling a device ID --- synapse/api/auth.py | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) (limited to 'synapse/api') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 1a245c8b5c..26d6007a04 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -155,9 +155,11 @@ class Auth: access_token = self.get_access_token_from_request(request) - user_id, _, app_service = await self._get_appservice_user_id_and_device_id( - request - ) + ( + user_id, + device_id, + app_service, + ) = await self._get_appservice_user_id_and_device_id(request) if user_id and app_service: if ip_addr and self._track_appservice_user_ips: await self.store.insert_client_ip( @@ -165,16 +167,22 @@ class Auth: access_token=access_token, ip=ip_addr, user_agent=user_agent, - device_id="dummy-device", # stubbed + device_id="dummy-device" + if device_id is None + else device_id, # stubbed ) - requester = create_requester(user_id, app_service=app_service) + requester = create_requester( + user_id, app_service=app_service, device_id=device_id + ) request.requester = user_id if user_id in self._force_tracing_for_users: opentracing.force_tracing() opentracing.set_tag("authenticated_entity", user_id) opentracing.set_tag("user_id", user_id) + if device_id is not None: + opentracing.set_tag("device_id", device_id) opentracing.set_tag("appservice_id", app_service.id) return requester -- cgit 1.5.1 From 7ea5022be8b7891dd8a01dc85ab31501bc2c212c Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 8 Dec 2021 15:00:13 +0000 Subject: Remove superfluous lines --- synapse/api/auth.py | 2 -- 1 file changed, 2 deletions(-) (limited to 'synapse/api') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 26d6007a04..5f7e9163c6 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -333,8 +333,6 @@ class Auth: user_id = request.args[b"user_id"][0].decode("utf8") await self.validate_appservice_can_control_user_id(app_service, user_id) - if app_service.sender == user_id: - return app_service.sender, None, app_service return user_id, None, app_service -- cgit 1.5.1 From 9551a3ed678e729062194e615074500f44b73ba1 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 8 Dec 2021 15:01:12 +0000 Subject: Remove early return because we need more logic here --- synapse/api/auth.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) (limited to 'synapse/api') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 5f7e9163c6..d34d9f8abe 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -327,14 +327,15 @@ class Auth: # This will always be set by the time Twisted calls us. assert request.args is not None - if b"user_id" not in request.args: - return app_service.sender, None, app_service - - user_id = request.args[b"user_id"][0].decode("utf8") - await self.validate_appservice_can_control_user_id(app_service, user_id) - + if b"user_id" in request.args: + effective_user_id = request.args[b"user_id"][0].decode("utf8") + await self.validate_appservice_can_control_user_id( + app_service, effective_user_id + ) + else: + effective_user_id = app_service.sender - return user_id, None, app_service + return effective_user_id, None, app_service async def get_user_by_access_token( self, -- cgit 1.5.1 From d3b0be57f961f5860d71e3e89daf93b493d40bd7 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Wed, 8 Dec 2021 15:12:32 +0000 Subject: Allow masquerading as a device by specifying the device_id URI parameter --- synapse/api/auth.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 'synapse/api') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index d34d9f8abe..65369afd13 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -313,6 +313,8 @@ class Auth: - 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" + app_service = self.store.get_app_service_by_token( self.get_access_token_from_request(request) ) @@ -335,7 +337,22 @@ class Auth: else: effective_user_id = app_service.sender - return effective_user_id, None, app_service + effective_device_id: Optional[str] = None + + if DEVICE_ID_ARG_NAME in request.args: + effective_device_id = request.args[DEVICE_ID_ARG_NAME][0].decode("utf8") + # We only just set this so it can't be None! + assert effective_device_id is not None + device_opt = await self.store.get_device_opt( + effective_user_id, effective_device_id + ) + if device_opt is None: + raise AuthError( + 403, + f"Application service trying to use a device that doesn't exist ('{effective_device_id}' for {effective_user_id})", + ) + + return effective_user_id, effective_device_id, app_service async def get_user_by_access_token( self, -- cgit 1.5.1 From cc2bbcd4dce1f53dc057390cc740e25ba5e7fe64 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 9 Dec 2021 12:29:08 +0000 Subject: Switch to the 400 M_EXCLUSIVE error code for non-existent device IDs This is as a result of a discussion on the MSC --- synapse/api/auth.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'synapse/api') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 65369afd13..fd48735e1c 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -347,9 +347,12 @@ class Auth: effective_user_id, effective_device_id ) if device_opt is None: + # For now, use 400 M_EXCLUSIVE if the device doesn't exist. + # This is an open thread of discussion on MSC3202 as of 2021-12-09. raise AuthError( - 403, + 400, f"Application service trying to use a device that doesn't exist ('{effective_device_id}' for {effective_user_id})", + Codes.EXCLUSIVE, ) return effective_user_id, effective_device_id, app_service -- cgit 1.5.1 From ae968eaa936513455b77768c92dd22ef0bf847a7 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Thu, 9 Dec 2021 13:10:18 +0000 Subject: Add an experimental flag to control device masquerading --- synapse/api/auth.py | 5 ++++- synapse/config/experimental.py | 5 +++++ 2 files changed, 9 insertions(+), 1 deletion(-) (limited to 'synapse/api') diff --git a/synapse/api/auth.py b/synapse/api/auth.py index fd48735e1c..0bb4b77f84 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -339,7 +339,10 @@ class Auth: effective_device_id: Optional[str] = None - if DEVICE_ID_ARG_NAME in request.args: + 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") # We only just set this so it can't be None! assert effective_device_id is not None diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index d78a15097c..678c78d565 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -49,3 +49,8 @@ class ExperimentalConfig(Config): # MSC3030 (Jump to date API endpoint) self.msc3030_enabled: bool = experimental.get("msc3030_enabled", False) + + # The portion of MSC3202 which is related to device masquerading. + self.msc3202_device_masquerading_enabled: bool = experimental.get( + "msc3202_device_masquerading", False + ) -- cgit 1.5.1