From f355b760242a723b8616c0f4c4d7f15f42f84737 Mon Sep 17 00:00:00 2001 From: Tim Abbott Date: Fri, 8 Dec 2023 17:10:05 -0800 Subject: [PATCH] zilencer: Only accept SYNCED_BILLING_EVENTS. I expect we would ignore extra events anyway, but this just makes it easier to reason about how the system works. --- zerver/tests/test_push_notifications.py | 21 +++++++++++++++++++++ zilencer/views.py | 25 +++++++++++++++++++++---- 2 files changed, 42 insertions(+), 4 deletions(-) diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 61a740a797..9f4008103d 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -1434,6 +1434,27 @@ class AnalyticsBouncerTest(BouncerTestCase): RealmCount.objects.all(), InstallationCount.objects.all(), RealmAuditLog.objects.all() ) + # This first post should fail because of excessive audit log event types. + result = self.uuid_post( + self.server_uuid, + "/api/v1/remotes/server/analytics", + { + "realm_counts": orjson.dumps(realm_count_data).decode(), + "installation_counts": orjson.dumps(installation_count_data).decode(), + "realmauditlog_rows": orjson.dumps(realmauditlog_data).decode(), + "realms": orjson.dumps([]).decode(), + }, + subdomain="", + ) + self.assert_json_error(result, "Invalid event type.") + + # Start again only using synced billing events. + realm_count_data, installation_count_data, realmauditlog_data = build_analytics_data( + RealmCount.objects.all(), + InstallationCount.objects.all(), + RealmAuditLog.objects.filter(event_type__in=RemoteRealmAuditLog.SYNCED_BILLING_EVENTS), + ) + # Send the data to the bouncer without any realms data. This should lead # to successful saving of the data, but with the remote_realm foreign key # set to NULL. diff --git a/zilencer/views.py b/zilencer/views.py index c81c7556db..d831662afa 100644 --- a/zilencer/views.py +++ b/zilencer/views.py @@ -523,7 +523,11 @@ def remote_server_notify_push( def validate_incoming_table_data( - server: RemoteZulipServer, model: Any, rows: List[Dict[str, Any]], is_count_stat: bool = False + server: RemoteZulipServer, + model: Any, + rows: List[Dict[str, Any]], + *, + is_count_stat: bool, ) -> None: last_id = get_last_id_from_server(server, model) for row in rows: @@ -532,6 +536,10 @@ def validate_incoming_table_data( or row["property"] in BOUNCER_ONLY_REMOTE_COUNT_STAT_PROPERTIES ): raise JsonableError(_("Invalid property {property}").format(property=row["property"])) + + if not is_count_stat and row["event_type"] not in RemoteRealmAuditLog.SYNCED_BILLING_EVENTS: + raise JsonableError(_("Invalid event type.")) + if row.get("id") is None: # This shouldn't be possible, as submitting data like this should be # prevented by our param validators. @@ -702,15 +710,24 @@ def remote_server_post_analytics( remote_server_version_updated = True validate_incoming_table_data( - server, RemoteRealmCount, [dict(count) for count in realm_counts], True + server, + RemoteRealmCount, + [dict(count) for count in realm_counts], + is_count_stat=True, ) validate_incoming_table_data( - server, RemoteInstallationCount, [dict(count) for count in installation_counts], True + server, + RemoteInstallationCount, + [dict(count) for count in installation_counts], + is_count_stat=True, ) if realmauditlog_rows is not None: validate_incoming_table_data( - server, RemoteRealmAuditLog, [dict(row) for row in realmauditlog_rows] + server, + RemoteRealmAuditLog, + [dict(row) for row in realmauditlog_rows], + is_count_stat=False, ) if realms is not None: