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

to_device messages aimed at a local device are not removed after that device is deleted #9346

Closed
anoadragon453 opened this issue Feb 8, 2021 · 1 comment · Fixed by #10969
Labels
S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@anoadragon453
Copy link
Member

When a user's device is deleted - either manually, when they log out or when their account is deactivated - to_device messages aimed at that device are not deleted, and end up sitting in the database forever.

async def delete_device(self, user_id: str, device_id: str) -> None:
""" Delete the given device
Args:
user_id: The user to delete the device from.
device_id: The device to delete.
"""
try:
await self.store.delete_device(user_id, device_id)
except errors.StoreError as e:
if e.code == 404:
# no match
set_tag("error", True)
log_kv(
{"reason": "User doesn't have device id.", "device_id": device_id}
)
pass
else:
raise
await self._auth_handler.delete_access_tokens_for_user(
user_id, device_id=device_id
)
await self.store.delete_e2e_keys_by_device(user_id=user_id, device_id=device_id)
await self.notify_device_update(user_id, [device_id])
@trace
async def delete_all_devices_for_user(
self, user_id: str, except_device_id: Optional[str] = None
) -> None:
"""Delete all of the user's devices
Args:
user_id: The user to remove all devices from
except_device_id: optional device id which should not be deleted
"""
device_map = await self.store.get_devices_by_user(user_id)
device_ids = list(device_map)
if except_device_id is not None:
device_ids = [d for d in device_ids if d != except_device_id]
await self.delete_devices(user_id, device_ids)
async def delete_devices(self, user_id: str, device_ids: List[str]) -> None:
""" Delete several devices
Args:
user_id: The user to delete devices from.
device_ids: The list of device IDs to delete
"""
try:
await self.store.delete_devices(user_id, device_ids)
except errors.StoreError as e:
if e.code == 404:
# no match
set_tag("error", True)
set_tag("reason", "User doesn't have that device id.")
pass
else:
raise
# Delete access tokens and e2e keys for each device. Not optimised as it is not
# considered as part of a critical path.
for device_id in device_ids:
await self._auth_handler.delete_access_tokens_for_user(
user_id, device_id=device_id
)
await self.store.delete_e2e_keys_by_device(
user_id=user_id, device_id=device_id
)
await self.notify_device_update(user_id, device_ids)

There's no point in keeping them around, and should get rid of them when a device goes pop.

Related: #3599

@anoadragon453 anoadragon453 added S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. labels Feb 8, 2021
@anoadragon453 anoadragon453 changed the title We don't delete to_device messages aimed at a local device once that device is deleted to_device messages aimed at a local device are not removed after that device is deleted Feb 8, 2021
@anoadragon453
Copy link
Member Author

After deleting these manually I found that ~48% of the device_inbox table was taken up by messages to devices that no longer existed.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant