Fix registering a device on an account with lots of devices (#15348)

Fixes up #15183
This commit is contained in:
Erik Johnston 2023-03-29 14:37:06 +01:00 committed by GitHub
parent 5350b5d04d
commit f0d8f66eaa
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 56 additions and 3 deletions

1
changelog.d/15348.misc Normal file
View File

@ -0,0 +1 @@
Prune user's old devices on login if they have too many.

View File

@ -946,6 +946,8 @@ class RegistrationHandler:
if not device_ids: if not device_ids:
return return
logger.info("Pruning %d stale devices for %s", len(device_ids), user_id)
# Now spawn a background loop that deletes said devices. # Now spawn a background loop that deletes said devices.
async def _prune_too_many_devices_loop() -> None: async def _prune_too_many_devices_loop() -> None:
if user_id in self._currently_pruning_devices_for_users: if user_id in self._currently_pruning_devices_for_users:

View File

@ -1638,19 +1638,22 @@ class DeviceBackgroundUpdateStore(SQLBaseStore):
""" """
rows = await self.db_pool.execute( rows = await self.db_pool.execute(
"check_too_many_devices_for_user_last_seen", None, sql, (user_id,) "check_too_many_devices_for_user_last_seen",
None,
sql,
user_id,
) )
if rows: if rows:
max_last_seen = max(rows[0][0], max_last_seen) max_last_seen = max(rows[0][0], max_last_seen)
# Fetch the devices to delete. # Fetch the devices to delete.
sql = """ sql = """
SELECT DISTINCT device_id FROM devices SELECT device_id FROM devices
LEFT JOIN e2e_device_keys_json USING (user_id, device_id) LEFT JOIN e2e_device_keys_json USING (user_id, device_id)
WHERE WHERE
user_id = ? user_id = ?
AND NOT hidden AND NOT hidden
AND last_seen < ? AND last_seen <= ?
AND key_json IS NULL AND key_json IS NULL
ORDER BY last_seen ORDER BY last_seen
""" """

View File

@ -794,6 +794,53 @@ class RegisterRestServletTestCase(unittest.HomeserverTestCase):
ApprovalNoticeMedium.NONE, channel.json_body["approval_notice_medium"] ApprovalNoticeMedium.NONE, channel.json_body["approval_notice_medium"]
) )
def test_check_stale_devices_get_pruned(self) -> None:
"""Check that if a user has some stale devices we log them out when they
log in a new device."""
# Register some devices, but not too many that we go over the threshold
# where we prune more aggressively.
user_id = self.register_user("user", "pass")
for _ in range(0, 50):
self.login(user_id, "pass")
store = self.hs.get_datastores().main
res = self.get_success(store.get_devices_by_user(user_id))
self.assertEqual(len(res), 50)
# Advance time so that the above devices are considered "old".
self.reactor.advance(30 * 24 * 60 * 60 * 1000)
self.login(user_id, "pass")
self.reactor.pump([60] * 10) # Ensure background job runs
# We expect all old devices to have been logged out
res = self.get_success(store.get_devices_by_user(user_id))
self.assertEqual(len(res), 1)
def test_check_recent_devices_get_pruned(self) -> None:
"""Check that if a user has many devices we log out the last oldest
ones.
Note: this is similar to above, except if we lots of devices we prune
devices even if they're not old.
"""
# Register a lot of devices in a short amount of time
user_id = self.register_user("user", "pass")
for _ in range(0, 100):
self.login(user_id, "pass")
self.reactor.advance(100)
store = self.hs.get_datastores().main
# We keep up to 50 devices that have been used in the last week, plus
# the device that was last logged in.
res = self.get_success(store.get_devices_by_user(user_id))
self.assertEqual(len(res), 51)
class AccountValidityTestCase(unittest.HomeserverTestCase): class AccountValidityTestCase(unittest.HomeserverTestCase):
servlets = [ servlets = [