From 6fabc26074f0f36151922d2431bd0dcc47c2f87a Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 16 Aug 2023 21:44:55 +0100 Subject: [PATCH 1/5] filter out unwanted user_agents --- synapse/storage/databases/main/metrics.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/metrics.py b/synapse/storage/databases/main/metrics.py index 595e22982e90..633488267626 100644 --- a/synapse/storage/databases/main/metrics.py +++ b/synapse/storage/databases/main/metrics.py @@ -411,7 +411,10 @@ def _generate_user_daily_visits(txn: LoggingTransaction) -> None: ON u.user_id = udv.user_id AND u.device_id=udv.device_id INNER JOIN users ON users.name=u.user_id WHERE ? <= last_seen AND last_seen < ? - AND udv.timestamp IS NULL AND users.is_guest=0 + AND udv.timestamp IS NULL + AND u.user_agent !='sync-v3-proxy-' + AND u.user_agent !='matrix-rust-sdk' + AND users.is_guest=0 AND users.appservice_id IS NULL GROUP BY u.user_id, u.device_id """ From e987a8a47ed6eeb2c37623a994f4f8d4856f671e Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Wed, 16 Aug 2023 21:52:42 +0100 Subject: [PATCH 2/5] ua bug --- changelog.d/16124.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/16124.bugfix diff --git a/changelog.d/16124.bugfix b/changelog.d/16124.bugfix new file mode 100644 index 000000000000..fb1d501a2fac --- /dev/null +++ b/changelog.d/16124.bugfix @@ -0,0 +1 @@ +Filter out user agent references to the sliding sync proxy and rust-sdk from the user_daily_visits table to ensure that Element X can be represented fully. From 548b64b7a63f3aff7b2a9bdfc52a01aab1fb3b24 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Sun, 20 Aug 2023 20:55:02 +0100 Subject: [PATCH 3/5] Filtered in the incorrect place. Revert "filter out unwanted user_agents" This reverts commit 6fabc26074f0f36151922d2431bd0dcc47c2f87a. --- synapse/storage/databases/main/metrics.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/synapse/storage/databases/main/metrics.py b/synapse/storage/databases/main/metrics.py index 633488267626..595e22982e90 100644 --- a/synapse/storage/databases/main/metrics.py +++ b/synapse/storage/databases/main/metrics.py @@ -411,10 +411,7 @@ def _generate_user_daily_visits(txn: LoggingTransaction) -> None: ON u.user_id = udv.user_id AND u.device_id=udv.device_id INNER JOIN users ON users.name=u.user_id WHERE ? <= last_seen AND last_seen < ? - AND udv.timestamp IS NULL - AND u.user_agent !='sync-v3-proxy-' - AND u.user_agent !='matrix-rust-sdk' - AND users.is_guest=0 + AND udv.timestamp IS NULL AND users.is_guest=0 AND users.appservice_id IS NULL GROUP BY u.user_id, u.device_id """ From d8324d650327040a15111bed7fda06d118c3b801 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Mon, 21 Aug 2023 21:42:43 +0100 Subject: [PATCH 4/5] Filter out proxy user agent from user_ips to prevent false reporting. --- synapse/storage/databases/main/client_ips.py | 5 ++ tests/storage/test_client_ips.py | 65 ++++++++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/synapse/storage/databases/main/client_ips.py b/synapse/storage/databases/main/client_ips.py index 0df160d2b089..6f205d52a8f8 100644 --- a/synapse/storage/databases/main/client_ips.py +++ b/synapse/storage/databases/main/client_ips.py @@ -579,6 +579,11 @@ async def insert_client_ip( device_id: Optional[str], now: Optional[int] = None, ) -> None: + # The sync proxy continuously triggers /sync even if the user is not + # present so should be excluded from user_ips entries. + if user_agent is "sync-v3-proxy-": + return + if not now: now = int(self._clock.time_msec()) key = (user_id, access_token, ip) diff --git a/tests/storage/test_client_ips.py b/tests/storage/test_client_ips.py index cd0079871ca2..209d68b40ba9 100644 --- a/tests/storage/test_client_ips.py +++ b/tests/storage/test_client_ips.py @@ -654,6 +654,71 @@ def test_old_user_ips_pruned(self) -> None: r, ) + def test_invalid_user_agents_are_ignored(self) -> None: + # First make sure we have completed all updates. + self.wait_for_background_updates() + + user_id1 = "@user1:id" + user_id2 = "@user2:id" + device_id1 = "MY_DEVICE1" + device_id2 = "MY_DEVICE2" + access_token1 = "access_token1" + access_token2 = "access_token2" + + # Insert a user IP 1 + self.get_success( + self.store.store_device( + user_id1, + device_id1, + "display name1", + ) + ) + # Insert a user IP 2 + self.get_success( + self.store.store_device( + user_id2, + device_id2, + "display name2", + ) + ) + + self.get_success( + self.store.insert_client_ip( + user_id1, access_token1, "ip", "sync-v3-proxy-", device_id1 + ) + ) + self.get_success( + self.store.insert_client_ip( + user_id2, access_token2, "ip", "user_agent", device_id2 + ) + ) + # Force persisting to disk + self.reactor.advance(200) + + # We should see that in the DB + result = self.get_success( + self.store.db_pool.simple_select_list( + table="user_ips", + keyvalues={}, + retcols=["access_token", "ip", "user_agent", "device_id", "last_seen"], + desc="get_user_ip_and_agents", + ) + ) + + # ensure user1 is filtered out + self.assertEqual( + result, + [ + { + "access_token": access_token2, + "ip": "ip", + "user_agent": "user_agent", + "device_id": device_id2, + "last_seen": 0, + } + ], + ) + class ClientIpAuthTestCase(unittest.HomeserverTestCase): servlets = [ From 39cd1b05cc3f9928b2829b2f76968aa11ca2a770 Mon Sep 17 00:00:00 2001 From: Neil Johnson Date: Tue, 22 Aug 2023 08:59:19 +0100 Subject: [PATCH 5/5] swap out is --- synapse/storage/databases/main/client_ips.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/client_ips.py b/synapse/storage/databases/main/client_ips.py index 6f205d52a8f8..d8d333e11d04 100644 --- a/synapse/storage/databases/main/client_ips.py +++ b/synapse/storage/databases/main/client_ips.py @@ -581,7 +581,7 @@ async def insert_client_ip( ) -> None: # The sync proxy continuously triggers /sync even if the user is not # present so should be excluded from user_ips entries. - if user_agent is "sync-v3-proxy-": + if user_agent == "sync-v3-proxy-": return if not now: