Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Add experimental support for MSC3202: allowing application services to masquerade as specific devices. #11538

Merged
merged 18 commits into from
Dec 15, 2021
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
a39ccfc
Expand return type of get_appservice_user_id to allow returning a dev…
reivilibre Dec 8, 2021
be8814f
Expand get_user_by_req to support handling a device ID
reivilibre Dec 8, 2021
7ea5022
Remove superfluous lines
reivilibre Dec 8, 2021
9551a3e
Remove early return because we need more logic here
reivilibre Dec 8, 2021
86ef692
Add get_device_opt which returns None instead of raising if it doesn'…
reivilibre Dec 8, 2021
d3b0be5
Allow masquerading as a device by specifying the device_id URI parameter
reivilibre Dec 8, 2021
8a078ce
Newsfile
reivilibre Dec 8, 2021
cc2bbcd
Switch to the 400 M_EXCLUSIVE error code for non-existent device IDs
reivilibre Dec 9, 2021
7e39806
Add a pair of tests for the ?device_id parameter for AS device masque…
reivilibre Dec 9, 2021
ae968ea
Add an experimental flag to control device masquerading
reivilibre Dec 9, 2021
11e2192
Update tests to enable experimental features
reivilibre Dec 9, 2021
63042ac
Use get_device (fixing in upstream develop)
reivilibre Dec 13, 2021
2becd52
TEMPORARY Revert "Use get_device (fixing in upstream develop)"
reivilibre Dec 13, 2021
405f3f9
Fix comment
reivilibre Dec 13, 2021
075a2b7
Merge branch 'develop' into rei/as_device_masquerading_msc3202
reivilibre Dec 13, 2021
15cb2f0
Use get_device
reivilibre Dec 13, 2021
24e3fad
Merge branch 'develop' into rei/as_device_masquerading_msc3202
reivilibre Dec 15, 2021
f35234a
Fix up: mock get_device in lieu of get_device_opt (since changes from…
reivilibre Dec 15, 2021
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
67 changes: 51 additions & 16 deletions synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,24 +155,34 @@ async def get_user_by_req(

access_token = self.get_access_token_from_request(request)

user_id, app_service = await self._get_appservice_user_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(
user_id=user_id,
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
)
turt2live marked this conversation as resolved.
Show resolved Hide resolved

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
Expand Down Expand Up @@ -274,33 +284,58 @@ async def validate_appservice_can_control_user_id(
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

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
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, app_service
return effective_user_id, None, app_service

async def get_user_by_access_token(
self,
Expand Down
22 changes: 22 additions & 0 deletions synapse/storage/databases/main/devices.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,8 @@ async def get_device(self, user_id: str, device_id: str) -> Dict[str, Any]:
A dict containing the device information
Raises:
StoreError: if the device is not found
See also:
`get_device_opt` which returns None instead if the device is not found
"""
return await self.db_pool.simple_select_one(
table="devices",
Expand All @@ -120,6 +122,26 @@ async def get_device(self, user_id: str, device_id: str) -> Dict[str, Any]:
desc="get_device",
)

async def get_device_opt(
Copy link
Member

Choose a reason for hiding this comment

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

I think it'd be nicer if we just switched get_device to return an Optional, and then change up the callers to look for None instead of a StoreError.

That would also fix up callers that seem to currently 500 if a device isn't found(!)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Span out to #11565 .

self, user_id: str, device_id: str
) -> Optional[Dict[str, Any]]:
"""Retrieve a device. Only returns devices that are not marked as
hidden.

Args:
user_id: The ID of the user which owns the device
device_id: The ID of the device to retrieve
Returns:
A dict containing the device information, or None if the device does not exist.
"""
return await self.db_pool.simple_select_one(
table="devices",
keyvalues={"user_id": user_id, "device_id": device_id, "hidden": False},
retcols=("user_id", "device_id", "display_name"),
desc="get_device",
allow_none=True,
)

async def get_devices_by_user(self, user_id: str) -> Dict[str, Dict[str, str]]:
"""Retrieve all of a user's registered devices. Only returns devices
that are not marked as hidden.
Expand Down