diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index cf435f28d3..4719e67cce 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -240,6 +240,19 @@ def bot_owner_user_ids(user_profile: UserProfile) -> Set[int]: def realm_user_count(realm: Realm) -> int: return UserProfile.objects.filter(realm=realm, is_active=True, is_bot=False).count() +def realm_user_count_by_role(realm: Realm) -> Dict[str, Any]: + human_counts = {UserProfile.ROLE_REALM_ADMINISTRATOR: 0, + UserProfile.ROLE_MEMBER: 0, + UserProfile.ROLE_GUEST: 0} + for value_dict in list(UserProfile.objects.filter( + realm=realm, is_bot=False, is_active=True).values('role').annotate(Count('role'))): + human_counts[value_dict['role']] = value_dict['role__count'] + bot_count = UserProfile.objects.filter(realm=realm, is_bot=True, is_active=True).count() + return { + RealmAuditLog.ROLE_COUNT_HUMANS: human_counts, + RealmAuditLog.ROLE_COUNT_BOTS: bot_count, + } + def send_signup_message(sender: UserProfile, admin_realm_signup_notifications_stream: str, user_profile: UserProfile, internal: bool=False, realm: Optional[Realm]=None) -> None: @@ -507,8 +520,12 @@ def do_create_user(email: str, password: Optional[str], realm: Realm, full_name: source_profile=source_profile) event_time = user_profile.date_joined - RealmAuditLog.objects.create(realm=user_profile.realm, modified_user=user_profile, - event_type=RealmAuditLog.USER_CREATED, event_time=event_time) + RealmAuditLog.objects.create( + realm=user_profile.realm, modified_user=user_profile, + event_type=RealmAuditLog.USER_CREATED, event_time=event_time, + extra_data=ujson.dumps({ + RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(user_profile.realm) + })) do_increment_logging_stat(user_profile.realm, COUNT_STATS['active_users_log:is_bot:day'], user_profile.is_bot, event_time) if settings.BILLING_ENABLED: @@ -534,8 +551,12 @@ def do_activate_user(user_profile: UserProfile) -> None: "is_mirror_dummy", "tos_version"]) event_time = user_profile.date_joined - RealmAuditLog.objects.create(realm=user_profile.realm, modified_user=user_profile, - event_type=RealmAuditLog.USER_ACTIVATED, event_time=event_time) + RealmAuditLog.objects.create( + realm=user_profile.realm, modified_user=user_profile, + event_type=RealmAuditLog.USER_ACTIVATED, event_time=event_time, + extra_data=ujson.dumps({ + RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(user_profile.realm) + })) do_increment_logging_stat(user_profile.realm, COUNT_STATS['active_users_log:is_bot:day'], user_profile.is_bot, event_time) if settings.BILLING_ENABLED: @@ -550,9 +571,12 @@ def do_reactivate_user(user_profile: UserProfile, acting_user: Optional[UserProf user_profile.save(update_fields=["is_active"]) event_time = timezone_now() - RealmAuditLog.objects.create(realm=user_profile.realm, modified_user=user_profile, - event_type=RealmAuditLog.USER_REACTIVATED, event_time=event_time, - acting_user=acting_user) + RealmAuditLog.objects.create( + realm=user_profile.realm, modified_user=user_profile, acting_user=acting_user, + event_type=RealmAuditLog.USER_REACTIVATED, event_time=event_time, + extra_data=ujson.dumps({ + RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(user_profile.realm) + })) do_increment_logging_stat(user_profile.realm, COUNT_STATS['active_users_log:is_bot:day'], user_profile.is_bot, event_time) if settings.BILLING_ENABLED: @@ -755,9 +779,12 @@ def do_deactivate_user(user_profile: UserProfile, clear_scheduled_emails([user_profile.id]) event_time = timezone_now() - RealmAuditLog.objects.create(realm=user_profile.realm, modified_user=user_profile, - acting_user=acting_user, - event_type=RealmAuditLog.USER_DEACTIVATED, event_time=event_time) + RealmAuditLog.objects.create( + realm=user_profile.realm, modified_user=user_profile, acting_user=acting_user, + event_type=RealmAuditLog.USER_DEACTIVATED, event_time=event_time, + extra_data=ujson.dumps({ + RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(user_profile.realm) + })) do_increment_logging_stat(user_profile.realm, COUNT_STATS['active_users_log:is_bot:day'], user_profile.is_bot, event_time, increment=-1) if settings.BILLING_ENABLED: @@ -3469,6 +3496,7 @@ def do_change_is_admin(user_profile: UserProfile, value: bool, # TODO: This function and do_change_is_guest should be merged into # a single do_change_user_role function in a future refactor. if permission == "administer": + old_value = user_profile.role if value: user_profile.role = UserProfile.ROLE_REALM_ADMINISTRATOR else: @@ -3481,6 +3509,14 @@ def do_change_is_admin(user_profile: UserProfile, value: bool, raise AssertionError("Invalid admin permission") if permission == 'administer': + RealmAuditLog.objects.create( + realm=user_profile.realm, modified_user=user_profile, + event_type=RealmAuditLog.USER_ROLE_CHANGED, event_time=timezone_now(), + extra_data=ujson.dumps({ + RealmAuditLog.OLD_VALUE: old_value, + RealmAuditLog.NEW_VALUE: UserProfile.ROLE_REALM_ADMINISTRATOR, + RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(user_profile.realm), + })) event = dict(type="realm_user", op="update", person=dict(email=user_profile.email, user_id=user_profile.id, @@ -3490,11 +3526,21 @@ def do_change_is_admin(user_profile: UserProfile, value: bool, def do_change_is_guest(user_profile: UserProfile, value: bool) -> None: # TODO: This function and do_change_is_admin should be merged into # a single do_change_user_role function in a future refactor. + old_value = user_profile.role if value: user_profile.role = UserProfile.ROLE_GUEST else: user_profile.role = UserProfile.ROLE_MEMBER user_profile.save(update_fields=["role"]) + + RealmAuditLog.objects.create( + realm=user_profile.realm, modified_user=user_profile, + event_type=RealmAuditLog.USER_ROLE_CHANGED, event_time=timezone_now(), + extra_data=ujson.dumps({ + RealmAuditLog.OLD_VALUE: old_value, + RealmAuditLog.NEW_VALUE: UserProfile.ROLE_GUEST, + RealmAuditLog.ROLE_COUNT: realm_user_count_by_role(user_profile.realm), + })) event = dict(type="realm_user", op="update", person=dict(email=user_profile.email, user_id=user_profile.id, diff --git a/zerver/models.py b/zerver/models.py index cd61fd18d8..90f1e10e1b 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -2571,6 +2571,14 @@ class RealmAuditLog(models.Model): # If True, event_time is an overestimate of the true time. Can be used # by migrations when introducing a new event_type. backfilled = models.BooleanField(default=False) # type: bool + + # Keys within extra_data, when extra_data is a json dict. Keys are strings because + # json keys must always be strings. + OLD_VALUE = '1' + NEW_VALUE = '2' + ROLE_COUNT = '10' + ROLE_COUNT_HUMANS = '11' + ROLE_COUNT_BOTS = '12' extra_data = models.TextField(null=True) # type: Optional[str] # USER_* event_types between 100 and 119 are synced from on-prem installations @@ -2583,6 +2591,7 @@ class RealmAuditLog(models.Model): USER_ACTIVATED = 102 USER_DEACTIVATED = 103 USER_REACTIVATED = 104 + USER_ROLE_CHANGED = 105 USER_SOFT_ACTIVATED = 120 USER_SOFT_DEACTIVATED = 121 diff --git a/zerver/tests/test_audit_log.py b/zerver/tests/test_audit_log.py index 45cb25611c..0bbaa41274 100644 --- a/zerver/tests/test_audit_log.py +++ b/zerver/tests/test_audit_log.py @@ -4,17 +4,28 @@ from zerver.lib.actions import do_create_user, do_deactivate_user, \ do_activate_user, do_reactivate_user, do_change_password, \ do_change_user_delivery_email, do_change_avatar_fields, do_change_bot_owner, \ do_regenerate_api_key, do_change_tos_version, \ - bulk_add_subscriptions, bulk_remove_subscriptions, get_streams_traffic + bulk_add_subscriptions, bulk_remove_subscriptions, get_streams_traffic, \ + do_change_is_admin, do_change_is_guest from zerver.lib.test_classes import ZulipTestCase -from zerver.models import RealmAuditLog, get_client, get_realm +from zerver.models import RealmAuditLog, get_client, get_realm, UserProfile from analytics.models import StreamCount from datetime import timedelta from django.contrib.auth.password_validation import validate_password +from typing import Any, Dict import ujson class TestRealmAuditLog(ZulipTestCase): + def check_role_count_schema(self, role_counts: Dict[str, Any]) -> None: + for key in [UserProfile.ROLE_REALM_ADMINISTRATOR, + UserProfile.ROLE_MEMBER, + UserProfile.ROLE_GUEST]: + # str(key) since json keys are always strings, and ujson.dumps will have converted + # the UserProfile.role values into strings + self.assertTrue(isinstance(role_counts[RealmAuditLog.ROLE_COUNT_HUMANS][str(key)], int)) + self.assertTrue(isinstance(role_counts[RealmAuditLog.ROLE_COUNT_BOTS], int)) + def test_user_activation(self) -> None: realm = get_realm('zulip') now = timezone_now() @@ -28,8 +39,32 @@ class TestRealmAuditLog(ZulipTestCase): realm=realm, acting_user=None, modified_user=user, modified_stream=None, event_time__gte=now, event_time__lte=now+timedelta(minutes=60)) .order_by('event_time').values_list('event_type', flat=True)) - self.assertEqual(event_types, [RealmAuditLog.USER_CREATED, RealmAuditLog.USER_DEACTIVATED, RealmAuditLog.USER_ACTIVATED, - RealmAuditLog.USER_DEACTIVATED, RealmAuditLog.USER_REACTIVATED]) + self.assertEqual(event_types, [RealmAuditLog.USER_CREATED, RealmAuditLog.USER_DEACTIVATED, + RealmAuditLog.USER_ACTIVATED, RealmAuditLog.USER_DEACTIVATED, + RealmAuditLog.USER_REACTIVATED]) + for event in RealmAuditLog.objects.filter( + realm=realm, acting_user=None, modified_user=user, modified_stream=None, + event_time__gte=now, event_time__lte=now+timedelta(minutes=60)): + extra_data = ujson.loads(event.extra_data) + self.check_role_count_schema(extra_data[RealmAuditLog.ROLE_COUNT]) + self.assertNotIn(RealmAuditLog.OLD_VALUE, extra_data) + + def test_change_role(self) -> None: + realm = get_realm('zulip') + now = timezone_now() + user_profile = self.example_user("hamlet") + do_change_is_admin(user_profile, True) + do_change_is_admin(user_profile, False) + do_change_is_guest(user_profile, True) + do_change_is_guest(user_profile, False) + for event in RealmAuditLog.objects.filter( + event_type=RealmAuditLog.USER_ROLE_CHANGED, + realm=realm, modified_user=user_profile, + event_time__gte=now, event_time__lte=now+timedelta(minutes=60)): + extra_data = ujson.loads(event.extra_data) + self.check_role_count_schema(extra_data[RealmAuditLog.ROLE_COUNT]) + self.assertIn(RealmAuditLog.OLD_VALUE, extra_data) + self.assertIn(RealmAuditLog.NEW_VALUE, extra_data) def test_change_password(self) -> None: now = timezone_now() diff --git a/zerver/tests/test_signup.py b/zerver/tests/test_signup.py index 269d0f3e85..7827d21e5d 100644 --- a/zerver/tests/test_signup.py +++ b/zerver/tests/test_signup.py @@ -496,7 +496,7 @@ class LoginTest(ZulipTestCase): with queries_captured() as queries: self.register(self.nonreg_email('test'), "test") # Ensure the number of queries we make is not O(streams) - self.assertEqual(len(queries), 78) + self.assertEqual(len(queries), 80) user_profile = self.nonreg_user('test') self.assert_logged_in_user_id(user_profile.id) self.assertFalse(user_profile.enable_stream_desktop_notifications)