user_groups: Audit UserGroup group based setting changes.

This add audit log entries when any group based setting of a user group
is updated. We store both the old and new values in extra_data, along
with the name of that setting. Entries populated during user group creation
are hardcoded to track "can_mention_group".

Potentially we can adjust "set_defaults_for_group_settings" so that it
populates realm audit logs with it, but that is out of scope for this change.

We use an atomic transaction so that the audit logs are committed
together with the updates.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>

zerver/actions/user_groups.py

@@ -67,7 +67,21 @@ def create_user_group_in_database(
             event_type=RealmAuditLog.USER_GROUP_CREATED,
             event_time=creation_time,
             modified_user_group=user_group,
-        )
+        ),
+        RealmAuditLog(
+            realm=realm,
+            acting_user=acting_user,
+            event_type=RealmAuditLog.USER_GROUP_GROUP_BASED_SETTING_CHANGED,
+            event_time=creation_time,
+            modified_user_group=user_group,
+            extra_data=orjson.dumps(
+                {
+                    RealmAuditLog.OLD_VALUE: None,
+                    RealmAuditLog.NEW_VALUE: user_group.can_mention_group.id,
+                    "property": "can_mention_group",
+                }
+            ).decode(),
+        ),
     ] + [
         RealmAuditLog(
             realm=realm,
@@ -403,6 +417,7 @@ def check_delete_user_group(
     do_send_delete_user_group_event(user_profile.realm, user_group_id, user_profile.realm.id)
 
 
+@transaction.atomic(savepoint=False)
 def do_change_user_group_permission_setting(
     user_group: UserGroup,
     setting_name: str,
@@ -410,11 +425,24 @@ def do_change_user_group_permission_setting(
     *,
     acting_user: Optional[UserProfile],
 ) -> None:
+    old_value = getattr(user_group, setting_name)
     setattr(user_group, setting_name, setting_value_group)
     user_group.save()
+    RealmAuditLog.objects.create(
+        realm=user_group.realm,
+        acting_user=acting_user,
+        event_type=RealmAuditLog.USER_GROUP_GROUP_BASED_SETTING_CHANGED,
+        event_time=timezone_now(),
+        modified_user_group=user_group,
+        extra_data=orjson.dumps(
+            {
+                RealmAuditLog.OLD_VALUE: old_value.id,
+                RealmAuditLog.NEW_VALUE: setting_value_group.id,
+                "property": setting_name,
+            }
+        ).decode(),
+    )
 
-    # RealmAuditLog changes are being done in a separate PR and will be
-    # added here once that is merged.
     setting_id_name = setting_name + "_id"
     event_data_dict: Dict[str, Union[str, int]] = {setting_id_name: setting_value_group.id}
     do_send_user_group_update_event(user_group, event_data_dict)

zerver/lib/user_groups.py

@@ -345,6 +345,22 @@ def create_system_user_groups_for_realm(realm: Realm) -> Dict[int, UserGroup]:
     for group in system_user_groups_list:
         user_group = set_defaults_for_group_settings(group, {}, system_groups_name_dict)
         groups_with_updated_settings.append(group)
+        realmauditlog_objects.append(
+            RealmAuditLog(
+                realm=realm,
+                acting_user=None,
+                event_type=RealmAuditLog.USER_GROUP_GROUP_BASED_SETTING_CHANGED,
+                event_time=creation_time,
+                modified_user_group=user_group,
+                extra_data=orjson.dumps(
+                    {
+                        RealmAuditLog.OLD_VALUE: None,
+                        RealmAuditLog.NEW_VALUE: user_group.can_mention_group.id,
+                        "property": "can_mention_group",
+                    }
+                ).decode(),
+            )
+        )
     UserGroup.objects.bulk_update(groups_with_updated_settings, ["can_mention_group"])
 
     subgroup_objects: List[GroupGroupMembership] = []

zerver/models.py

@@ -4581,6 +4581,7 @@ class AbstractRealmAuditLog(models.Model):
     # 709 to 719 reserved for membership changes
     USER_GROUP_NAME_CHANGED = 720
     USER_GROUP_DESCRIPTION_CHANGED = 721
+    USER_GROUP_GROUP_BASED_SETTING_CHANGED = 722
 
     # The following values are only for RemoteZulipServerAuditLog
     # Values should be exactly 10000 greater than the corresponding

zerver/tests/test_audit_log.py

@@ -49,6 +49,7 @@ from zerver.actions.user_groups import (
     add_subgroups_to_user_group,
     bulk_add_members_to_user_group,
     check_add_user_group,
+    do_change_user_group_permission_setting,
     do_update_user_group_description,
     do_update_user_group_name,
     remove_members_from_user_group,
@@ -1135,12 +1136,42 @@ class TestRealmAuditLog(ZulipTestCase):
             self.assertEqual(supergroup_id, expected_supergroup_id)
             self.assertEqual(subgroup_id, expected_subgroup_id)
 
+        audit_log_entries = sorted(
+            RealmAuditLog.objects.filter(
+                realm=realm,
+                event_type=RealmAuditLog.USER_GROUP_GROUP_BASED_SETTING_CHANGED,
+                event_time__gte=now,
+                acting_user=None,
+            ).values_list("modified_user_group_id", "extra_data")
+        )
+        nobody_group = UserGroup.objects.get(name=UserGroup.NOBODY_GROUP_NAME, realm=realm)
+        for (user_group_id, extra_data), expected_user_group_id in zip(
+            audit_log_entries, logged_system_group_ids
+        ):
+            self.assertEqual(user_group_id, expected_user_group_id)
+            self.assertDictEqual(
+                orjson.loads(assert_is_not_none(extra_data)),
+                {
+                    RealmAuditLog.OLD_VALUE: None,
+                    RealmAuditLog.NEW_VALUE: nobody_group.id,
+                    "property": "can_mention_group",
+                },
+            )
+
     def test_user_group_creation(self) -> None:
         hamlet = self.example_user("hamlet")
         cordelia = self.example_user("cordelia")
         now = timezone_now()
+        public_group = UserGroup.objects.get(
+            name=UserGroup.EVERYONE_ON_INTERNET_GROUP_NAME, realm=hamlet.realm
+        )
         user_group = check_add_user_group(
-            hamlet.realm, "empty", [hamlet, cordelia], acting_user=hamlet, description="lorem"
+            hamlet.realm,
+            "empty",
+            [hamlet, cordelia],
+            acting_user=hamlet,
+            description="lorem",
+            group_settings_map={"can_mention_group": public_group},
         )
 
         audit_log_entries = RealmAuditLog.objects.filter(
@@ -1163,6 +1194,27 @@ class TestRealmAuditLog(ZulipTestCase):
         self.assertEqual(audit_log_entries[0].modified_user, hamlet)
         self.assertEqual(audit_log_entries[1].modified_user, cordelia)
 
+        audit_log_entries = RealmAuditLog.objects.filter(
+            acting_user=hamlet,
+            realm=hamlet.realm,
+            event_time__gte=now,
+            event_type=RealmAuditLog.USER_GROUP_GROUP_BASED_SETTING_CHANGED,
+        )
+        self.assert_length(audit_log_entries, len(UserGroup.GROUP_PERMISSION_SETTINGS))
+        self.assertListEqual(
+            [
+                orjson.loads(assert_is_not_none(audit_log.extra_data))
+                for audit_log in audit_log_entries
+            ],
+            [
+                {
+                    RealmAuditLog.OLD_VALUE: None,
+                    RealmAuditLog.NEW_VALUE: public_group.id,
+                    "property": "can_mention_group",
+                }
+            ],
+        )
+
     def test_change_user_group_memberships(self) -> None:
         hamlet = self.example_user("hamlet")
         cordelia = self.example_user("cordelia")
@@ -1294,3 +1346,26 @@ class TestRealmAuditLog(ZulipTestCase):
                 RealmAuditLog.NEW_VALUE: "Foo",
             },
         )
+
+        old_group = user_group.can_mention_group
+        new_group = UserGroup.objects.get(
+            name=UserGroup.EVERYONE_ON_INTERNET_GROUP_NAME, realm=user_group.realm
+        )
+        self.assertNotEqual(old_group.id, new_group.id)
+        do_change_user_group_permission_setting(
+            user_group, "can_mention_group", new_group, acting_user=None
+        )
+        audit_log_entries = RealmAuditLog.objects.filter(
+            event_type=RealmAuditLog.USER_GROUP_GROUP_BASED_SETTING_CHANGED,
+            event_time__gte=now,
+        )
+        self.assert_length(audit_log_entries, 1)
+        self.assertIsNone(audit_log_entries[0].acting_user)
+        self.assertDictEqual(
+            orjson.loads(assert_is_not_none(audit_log_entries[0].extra_data)),
+            {
+                RealmAuditLog.OLD_VALUE: old_group.id,
+                RealmAuditLog.NEW_VALUE: new_group.id,
+                "property": "can_mention_group",
+            },
+        )

zerver/tests/test_slack_importer.py

@@ -1337,6 +1337,7 @@ class SlackImporter(ZulipTestCase):
                 RealmAuditLog.USER_GROUP_CREATED,
                 RealmAuditLog.USER_GROUP_DIRECT_SUBGROUP_MEMBERSHIP_ADDED,
                 RealmAuditLog.USER_GROUP_DIRECT_SUPERGROUP_MEMBERSHIP_ADDED,
+                RealmAuditLog.USER_GROUP_GROUP_BASED_SETTING_CHANGED,
             },
         )