Skip to content

Commit 0287b77

Browse files
Famedly release/v1.145.0_2 (#235)
# Famedly Synapse Release v1.145.0_2 ## Famedly additions for v1.145.0_2 - fix: invalidate access token cache on device deletion When an access token had a refresh token associated to it in the database, deleting this refresh token (for example when deleting the device using it) would cascade delete the access token, which wouldn't be returned by the sql query that was supposed to delete it on its own, and an empty array was passed to the cache invalidation function.
2 parents eff77b7 + 332cc86 commit 0287b77

File tree

3 files changed

+115
-10
lines changed

3 files changed

+115
-10
lines changed

CHANGES.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ Ubuntu 25.04 (Plucky Puffin) will be end of life on Jan 17, 2026. Synapse will s
1010

1111
The "Updates to locked dependencies" section has been removed from the changelog due to lack of use and the maintenance burden. ([\#19254](https://github.com/element-hq/synapse/issues/19254))
1212

13+
### Famedly additions for v1.145.0_2
14+
- fix: invalidate access token cache on device deletion (FrenchGithubUser)
1315

1416
### Famedly additions for v1.145.0_1
1517
- fix: typo in changelog (FrenchGithubUser)

synapse/storage/databases/main/registration.py

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2569,14 +2569,9 @@ async def user_delete_access_tokens_for_devices(
25692569
def user_delete_access_tokens_for_devices_txn(
25702570
txn: LoggingTransaction, batch_device_ids: StrCollection
25712571
) -> list[tuple[str, int, str | None]]:
2572-
self.db_pool.simple_delete_many_txn(
2573-
txn,
2574-
table="refresh_tokens",
2575-
keyvalues={"user_id": user_id},
2576-
column="device_id",
2577-
values=batch_device_ids,
2578-
)
2579-
2572+
# Delete access tokens first, before refresh tokens.
2573+
# This ensures we can capture the deleted access tokens for cache invalidation
2574+
# before any CASCADE deletes occur.
25802575
clause, args = make_in_list_sql_clause(
25812576
txn.database_engine, "device_id", batch_device_ids
25822577
)
@@ -2595,6 +2590,14 @@ def user_delete_access_tokens_for_devices_txn(
25952590
self.get_user_by_access_token,
25962591
[(t[0],) for t in tokens_and_devices],
25972592
)
2593+
self.db_pool.simple_delete_many_txn(
2594+
txn,
2595+
table="refresh_tokens",
2596+
keyvalues={"user_id": user_id},
2597+
column="device_id",
2598+
values=batch_device_ids,
2599+
)
2600+
25982601
return tokens_and_devices
25992602

26002603
results = []

tests/rest/client/test_auth.py

Lines changed: 102 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,16 @@
3030
from synapse.api.constants import ApprovalNoticeMedium, LoginType
3131
from synapse.api.errors import Codes, SynapseError
3232
from synapse.handlers.ui_auth.checkers import UserInteractiveAuthChecker
33-
from synapse.rest.client import account, auth, devices, login, logout, register
33+
from synapse.rest.client import (
34+
account,
35+
auth,
36+
devices,
37+
login,
38+
logout,
39+
read_marker,
40+
register,
41+
room,
42+
)
3443
from synapse.rest.synapse.client import build_synapse_client_resource_tree
3544
from synapse.server import HomeServer
3645
from synapse.storage.database import LoggingTransaction
@@ -625,10 +634,13 @@ class RefreshAuthTests(unittest.HomeserverTestCase):
625634
servlets = [
626635
auth.register_servlets,
627636
account.register_servlets,
637+
devices.register_servlets,
628638
login.register_servlets,
629639
logout.register_servlets,
630-
synapse.rest.admin.register_servlets_for_client_rest_resource,
640+
read_marker.register_servlets,
631641
register.register_servlets,
642+
room.register_servlets,
643+
synapse.rest.admin.register_servlets_for_client_rest_resource,
632644
]
633645
hijack_auth = False
634646

@@ -1394,6 +1406,94 @@ def _txn(txn: LoggingTransaction) -> int:
13941406
self.assertEqual(_table_length("access_tokens"), 0)
13951407
self.assertEqual(_table_length("refresh_tokens"), 0)
13961408

1409+
def test_token_invalid_after_refresh_token_issued_and_device_removed(self) -> None:
1410+
"""
1411+
Test that sending a read marker (dummy action) fails after the device (which had a refresh token) is removed by another device.
1412+
The removal of a refresh token cascade deletes the associated access token in the db, which can make cache invalidation fail, if not handled properly.
1413+
This test will catch such behavior if it ever happens again.
1414+
1. User logs in with device1
1415+
2. User logs in with device2 and requests a refresh token
1416+
3. Device2 sends a read marker (should work)
1417+
4. Device1 removes device2
1418+
5. Device2 tries to send a read marker (should fail)
1419+
"""
1420+
# Login with device1
1421+
device1_tok = self.login("test", self.user_pass, device_id="device1")
1422+
1423+
# Login with device2 and request a refresh token
1424+
login_response = self.make_request(
1425+
"POST",
1426+
"/_matrix/client/v3/login",
1427+
{
1428+
"type": "m.login.password",
1429+
"user": "test",
1430+
"password": self.user_pass,
1431+
"device_id": "device2",
1432+
"refresh_token": True,
1433+
},
1434+
)
1435+
self.assertEqual(login_response.code, HTTPStatus.OK, login_response.result)
1436+
device2_tok = login_response.json_body["access_token"]
1437+
device2_id = login_response.json_body["device_id"]
1438+
self.assertEqual(device2_id, "device2")
1439+
1440+
# Create a room and send a message
1441+
room_id = self.helper.create_room_as(self.user, tok=device1_tok)
1442+
event_id = self.helper.send(
1443+
room_id=room_id, body="test message", tok=device1_tok
1444+
)["event_id"]
1445+
1446+
# Device2 sends a read marker (should work)
1447+
channel = self.make_request(
1448+
"POST",
1449+
f"/rooms/{room_id}/read_markers",
1450+
content={
1451+
"m.fully_read": event_id,
1452+
},
1453+
access_token=device2_tok,
1454+
)
1455+
self.assertEqual(channel.code, HTTPStatus.OK, channel.result)
1456+
1457+
# Device1 removes device2
1458+
# First, attempt to delete device2
1459+
delete_channel = self.make_request(
1460+
"DELETE",
1461+
f"devices/{device2_id}",
1462+
access_token=device1_tok,
1463+
)
1464+
self.assertEqual(
1465+
delete_channel.code, HTTPStatus.UNAUTHORIZED, delete_channel.result
1466+
)
1467+
session = delete_channel.json_body["session"]
1468+
1469+
# Complete the UI auth flow
1470+
delete_channel = self.make_request(
1471+
"DELETE",
1472+
f"devices/{device2_id}",
1473+
content={
1474+
"auth": {
1475+
"type": "m.login.password",
1476+
"user": "test",
1477+
"password": self.user_pass,
1478+
"session": session,
1479+
},
1480+
},
1481+
access_token=device1_tok,
1482+
)
1483+
self.assertEqual(delete_channel.code, HTTPStatus.OK, delete_channel.result)
1484+
1485+
# Device2 tries to send a read marker (should fail)
1486+
channel = self.make_request(
1487+
"POST",
1488+
f"/rooms/{room_id}/read_markers",
1489+
content={
1490+
"m.fully_read": event_id,
1491+
},
1492+
access_token=device2_tok,
1493+
)
1494+
self.assertEqual(channel.code, HTTPStatus.UNAUTHORIZED, channel.result)
1495+
self.assertEqual(channel.json_body["errcode"], Codes.UNKNOWN_TOKEN)
1496+
13971497

13981498
def oidc_config(
13991499
id: str, with_localpart_template: bool, **kwargs: Any

0 commit comments

Comments
 (0)