user_groups: Audit UserGroup memberships changes.

This also add audit log entries during user creation and role change,
because we modify system group memberships there.

Signed-off-by: Zixuan James Li <p359101898@gmail.com>
This commit is contained in:
Zixuan James Li
2022-11-20 22:26:34 -08:00
committed by Tim Abbott
parent 63f5936207
commit 44781ddfa9
7 changed files with 168 additions and 4 deletions

View File

@@ -467,6 +467,14 @@ def do_create_user(
system_user_group = get_system_user_group_for_user(user_profile)
UserGroupMembership.objects.create(user_profile=user_profile, user_group=system_user_group)
RealmAuditLog.objects.create(
realm=user_profile.realm,
modified_user=user_profile,
modified_user_group=system_user_group,
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_ADDED,
event_time=event_time,
acting_user=acting_user,
)
if user_profile.role == UserProfile.ROLE_MEMBER and not user_profile.is_provisional_member:
full_members_system_group = UserGroup.objects.get(
@@ -477,6 +485,14 @@ def do_create_user(
UserGroupMembership.objects.create(
user_profile=user_profile, user_group=full_members_system_group
)
RealmAuditLog.objects.create(
realm=user_profile.realm,
modified_user=user_profile,
modified_user_group=full_members_system_group,
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_ADDED,
event_time=event_time,
acting_user=acting_user,
)
# Note that for bots, the caller will send an additional event
# with bot-specific info like services.

View File

@@ -240,6 +240,18 @@ def bulk_add_members_to_user_group(
for user_id in user_profile_ids
]
UserGroupMembership.objects.bulk_create(memberships)
now = timezone_now()
RealmAuditLog.objects.bulk_create(
RealmAuditLog(
realm=user_group.realm,
modified_user_id=user_id,
modified_user_group=user_group,
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_ADDED,
event_time=now,
acting_user=acting_user,
)
for user_id in user_profile_ids
)
do_send_user_group_members_update_event("add_members", user_group, user_profile_ids)
@@ -251,6 +263,18 @@ def remove_members_from_user_group(
UserGroupMembership.objects.filter(
user_group_id=user_group.id, user_profile_id__in=user_profile_ids
).delete()
now = timezone_now()
RealmAuditLog.objects.bulk_create(
RealmAuditLog(
realm=user_group.realm,
modified_user_id=user_id,
modified_user_group=user_group,
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_REMOVED,
event_time=now,
acting_user=acting_user,
)
for user_id in user_profile_ids
)
do_send_user_group_members_update_event("remove_members", user_group, user_profile_ids)

View File

@@ -345,7 +345,28 @@ def do_change_user_role(
).delete()
system_group = get_system_user_group_for_user(user_profile)
now = timezone_now()
UserGroupMembership.objects.create(user_profile=user_profile, user_group=system_group)
RealmAuditLog.objects.bulk_create(
[
RealmAuditLog(
realm=user_profile.realm,
modified_user=user_profile,
modified_user_group=old_system_group,
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_REMOVED,
event_time=now,
acting_user=acting_user,
),
RealmAuditLog(
realm=user_profile.realm,
modified_user=user_profile,
modified_user_group=system_group,
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_ADDED,
event_time=now,
acting_user=acting_user,
),
]
)
do_send_user_group_members_update_event("remove_members", old_system_group, [user_profile.id])

View File

@@ -313,6 +313,8 @@ def create_system_user_groups_for_realm(realm: Realm) -> Dict[int, UserGroup]:
can_mention_group_id=initial_group_setting_value,
)
# Order of this list here is important to create correct GroupGroupMembership objects
# Note that because we do not create user memberships here, no audit log entries for
# user memberships are populated either.
system_user_groups_list = [
nobody_system_group,
role_system_groups_dict[UserProfile.ROLE_REALM_OWNER],

View File

@@ -45,7 +45,11 @@ from zerver.actions.streams import (
do_deactivate_stream,
do_rename_stream,
)
from zerver.actions.user_groups import check_add_user_group
from zerver.actions.user_groups import (
bulk_add_members_to_user_group,
check_add_user_group,
remove_members_from_user_group,
)
from zerver.actions.user_settings import (
do_change_avatar_fields,
do_change_password,
@@ -104,7 +108,7 @@ class TestRealmAuditLog(ZulipTestCase):
do_activate_mirror_dummy_user(user, acting_user=user)
do_deactivate_user(user, acting_user=user)
do_reactivate_user(user, acting_user=user)
self.assertEqual(RealmAuditLog.objects.filter(event_time__gte=now).count(), 6)
self.assertEqual(RealmAuditLog.objects.filter(event_time__gte=now).count(), 8)
event_types = list(
RealmAuditLog.objects.filter(
realm=realm,
@@ -121,12 +125,15 @@ class TestRealmAuditLog(ZulipTestCase):
event_types,
[
RealmAuditLog.USER_CREATED,
RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_ADDED,
RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_ADDED,
RealmAuditLog.USER_DEACTIVATED,
RealmAuditLog.USER_ACTIVATED,
RealmAuditLog.USER_DEACTIVATED,
RealmAuditLog.USER_REACTIVATED,
],
)
modified_user_group_names = []
for event in RealmAuditLog.objects.filter(
realm=realm,
acting_user=user,
@@ -135,10 +142,22 @@ class TestRealmAuditLog(ZulipTestCase):
event_time__gte=now,
event_time__lte=now + timedelta(minutes=60),
):
if event.event_type == RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_ADDED:
self.assertIsNone(event.extra_data)
modified_user_group_names.append(assert_is_not_none(event.modified_user_group).name)
continue
extra_data = orjson.loads(assert_is_not_none(event.extra_data))
self.check_role_count_schema(extra_data[RealmAuditLog.ROLE_COUNT])
self.assertNotIn(RealmAuditLog.OLD_VALUE, extra_data)
self.assertListEqual(
modified_user_group_names,
[
UserGroup.MEMBERS_GROUP_NAME,
UserGroup.FULL_MEMBERS_GROUP_NAME,
],
)
def test_change_role(self) -> None:
realm = get_realm("zulip")
now = timezone_now()
@@ -182,6 +201,59 @@ class TestRealmAuditLog(ZulipTestCase):
)
self.assertEqual(old_values_seen, new_values_seen)
expected_system_user_group_names = [
UserGroup.ADMINISTRATORS_GROUP_NAME,
UserGroup.MEMBERS_GROUP_NAME,
UserGroup.FULL_MEMBERS_GROUP_NAME,
UserGroup.EVERYONE_GROUP_NAME,
UserGroup.MEMBERS_GROUP_NAME,
UserGroup.FULL_MEMBERS_GROUP_NAME,
UserGroup.OWNERS_GROUP_NAME,
UserGroup.MEMBERS_GROUP_NAME,
UserGroup.FULL_MEMBERS_GROUP_NAME,
UserGroup.MODERATORS_GROUP_NAME,
]
user_group_modified_names = (
RealmAuditLog.objects.filter(
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_ADDED,
realm=realm,
modified_user=user_profile,
acting_user=acting_user,
event_time__gte=now,
event_time__lte=now + timedelta(minutes=60),
)
.order_by("event_time")
.values_list("modified_user_group__name", flat=True)
)
self.assertListEqual(
list(user_group_modified_names),
[
*expected_system_user_group_names,
UserGroup.MEMBERS_GROUP_NAME,
UserGroup.FULL_MEMBERS_GROUP_NAME,
],
)
user_group_modified_names = (
RealmAuditLog.objects.filter(
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_REMOVED,
realm=realm,
modified_user=user_profile,
acting_user=acting_user,
event_time__gte=now,
event_time__lte=now + timedelta(minutes=60),
)
.order_by("event_time")
.values_list("modified_user_group__name", flat=True)
)
self.assertListEqual(
list(user_group_modified_names),
[
UserGroup.MEMBERS_GROUP_NAME,
UserGroup.FULL_MEMBERS_GROUP_NAME,
*expected_system_user_group_names,
],
)
def test_change_password(self) -> None:
now = timezone_now()
user = self.example_user("hamlet")
@@ -1045,3 +1117,32 @@ class TestRealmAuditLog(ZulipTestCase):
self.assert_length(audit_log_entries, 2)
self.assertEqual(audit_log_entries[0].modified_user, hamlet)
self.assertEqual(audit_log_entries[1].modified_user, cordelia)
def test_change_user_group_memberships(self) -> None:
hamlet = self.example_user("hamlet")
cordelia = self.example_user("cordelia")
now = timezone_now()
user_group = check_add_user_group(hamlet.realm, "foo", [], acting_user=None)
bulk_add_members_to_user_group(user_group, [hamlet.id, cordelia.id], acting_user=hamlet)
audit_log_entries = RealmAuditLog.objects.filter(
acting_user=hamlet,
realm=hamlet.realm,
modified_user_group=user_group,
event_time__gte=now,
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_ADDED,
)
self.assert_length(audit_log_entries, 2)
self.assertEqual(audit_log_entries[0].modified_user, hamlet)
self.assertEqual(audit_log_entries[1].modified_user, cordelia)
remove_members_from_user_group(user_group, [hamlet.id], acting_user=hamlet)
audit_log_entries = RealmAuditLog.objects.filter(
acting_user=hamlet,
realm=hamlet.realm,
modified_user_group=user_group,
event_time__gte=now,
event_type=RealmAuditLog.USER_GROUP_DIRECT_USER_MEMBERSHIP_REMOVED,
)
self.assert_length(audit_log_entries, 1)
self.assertEqual(audit_log_entries[0].modified_user, hamlet)

View File

@@ -939,7 +939,7 @@ class LoginTest(ZulipTestCase):
ContentType.objects.clear_cache()
# Ensure the number of queries we make is not O(streams)
with self.assert_database_query_count(102), cache_tries_captured() as cache_tries:
with self.assert_database_query_count(104), cache_tries_captured() as cache_tries:
with self.captureOnCommitCallbacks(execute=True):
self.register(self.nonreg_email("test"), "test")

View File

@@ -786,7 +786,7 @@ class QueryCountTest(ZulipTestCase):
prereg_user = PreregistrationUser.objects.get(email="fred@zulip.com")
with self.assert_database_query_count(91):
with self.assert_database_query_count(93):
with cache_tries_captured() as cache_tries:
with self.capture_send_event_calls(expected_num_events=11) as events:
fred = do_create_user(