audit_log: Log acting_user in do_change_user_role.

matt- · Jul 7, 2020 · 87aaa84 · 87aaa84
1 parent 01f12b9
commit 87aaa84
Show file tree

Hide file tree

Showing 6 changed files with 16 additions and 15 deletions.
diff --git a/analytics/management/commands/populate_analytics_db.py b/analytics/management/commands/populate_analytics_db.py
@@ -64,7 +64,7 @@ def handle(self, *args: Any, **options: Any) -> None:
             shylock = create_user('[email protected]', 'Shylock', realm,
                                   full_name='Shylock', short_name='shylock',
                                   role=UserProfile.ROLE_REALM_ADMINISTRATOR)
-        do_change_user_role(shylock, UserProfile.ROLE_REALM_ADMINISTRATOR)
+        do_change_user_role(shylock, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None)
         stream = Stream.objects.create(
             name='all', realm=realm, date_created=installation_time)
         recipient = Recipient.objects.create(type_id=stream.id, type=Recipient.STREAM)

diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py
@@ -3484,12 +3484,12 @@ def do_change_default_all_public_streams(user_profile: UserProfile, value: bool,
                                  )),
                    bot_owner_user_ids(user_profile))
 
-def do_change_user_role(user_profile: UserProfile, value: int) -> None:
+def do_change_user_role(user_profile: UserProfile, value: int, acting_user: Optional[UserProfile]=None) -> None:
     old_value = user_profile.role
     user_profile.role = value
     user_profile.save(update_fields=["role"])
     RealmAuditLog.objects.create(
-        realm=user_profile.realm, modified_user=user_profile,
+        realm=user_profile.realm, modified_user=user_profile, acting_user=acting_user,
         event_type=RealmAuditLog.USER_ROLE_CHANGED, event_time=timezone_now(),
         extra_data=ujson.dumps({
             RealmAuditLog.OLD_VALUE: old_value,

diff --git a/zerver/management/commands/knight.py b/zerver/management/commands/knight.py
@@ -50,7 +50,7 @@ def handle(self, *args: Any, **options: Any) -> None:
                     if options['permission'] == "api_super_user":
                         do_change_is_api_super_user(user, True)
                     elif options['permission'] == "administer":
-                        do_change_user_role(user, UserProfile.ROLE_REALM_ADMINISTRATOR)
+                        do_change_user_role(user, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None)
                     print("Done!")
                 else:
                     print("Would have granted {} {} rights for {}".format(
@@ -62,7 +62,7 @@ def handle(self, *args: Any, **options: Any) -> None:
                     if options['permission'] == "api_super_user":
                         do_change_is_api_super_user(user, False)
                     elif options['permission'] == "administer":
-                        do_change_user_role(user, UserProfile.ROLE_MEMBER)
+                        do_change_user_role(user, UserProfile.ROLE_MEMBER, acting_user=None)
                     print("Done!")
                 else:
                     print("Would have removed {}'s {} rights on {}".format(email, options['permission'],

diff --git a/zerver/tests/test_audit_log.py b/zerver/tests/test_audit_log.py
@@ -66,17 +66,18 @@ def test_change_role(self) -> None:
         realm = get_realm('zulip')
         now = timezone_now()
         user_profile = self.example_user("hamlet")
-        do_change_user_role(user_profile, UserProfile.ROLE_REALM_ADMINISTRATOR)
-        do_change_user_role(user_profile, UserProfile.ROLE_MEMBER)
-        do_change_user_role(user_profile, UserProfile.ROLE_GUEST)
-        do_change_user_role(user_profile, UserProfile.ROLE_MEMBER)
-        do_change_user_role(user_profile, UserProfile.ROLE_REALM_OWNER)
-        do_change_user_role(user_profile, UserProfile.ROLE_MEMBER)
+        acting_user = self.example_user('iago')
+        do_change_user_role(user_profile, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=acting_user)
+        do_change_user_role(user_profile, UserProfile.ROLE_MEMBER, acting_user=acting_user)
+        do_change_user_role(user_profile, UserProfile.ROLE_GUEST, acting_user=acting_user)
+        do_change_user_role(user_profile, UserProfile.ROLE_MEMBER, acting_user=acting_user)
+        do_change_user_role(user_profile, UserProfile.ROLE_REALM_OWNER, acting_user=acting_user)
+        do_change_user_role(user_profile, UserProfile.ROLE_MEMBER, acting_user=acting_user)
         old_values_seen = set()
         new_values_seen = set()
         for event in RealmAuditLog.objects.filter(
                 event_type=RealmAuditLog.USER_ROLE_CHANGED,
-                realm=realm, modified_user=user_profile,
+                realm=realm, modified_user=user_profile, acting_user=acting_user,
                 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])

diff --git a/zerver/views/users.py b/zerver/views/users.py
@@ -151,7 +151,7 @@ def update_user_backend(
             return json_error(_('The owner permission cannot be removed from the only organization owner.'))
         if UserProfile.ROLE_REALM_OWNER in [role, target.role] and not user_profile.is_realm_owner:
             raise OrganizationOwnerRequired()
-        do_change_user_role(target, role)
+        do_change_user_role(target, role, acting_user=user_profile)
 
     if (full_name is not None and target.full_name != full_name and
             full_name.strip() != ""):

diff --git a/zilencer/management/commands/populate_db.py b/zilencer/management/commands/populate_db.py
@@ -301,12 +301,12 @@ def handle(self, **options: Any) -> None:
             create_users(zulip_realm, names, tos_version=settings.TOS_VERSION)
 
             iago = get_user_by_delivery_email("[email protected]", zulip_realm)
-            do_change_user_role(iago, UserProfile.ROLE_REALM_ADMINISTRATOR)
+            do_change_user_role(iago, UserProfile.ROLE_REALM_ADMINISTRATOR, acting_user=None)
             iago.is_staff = True
             iago.save(update_fields=['is_staff'])
 
             desdemona = get_user_by_delivery_email("[email protected]", zulip_realm)
-            do_change_user_role(desdemona, UserProfile.ROLE_REALM_OWNER)
+            do_change_user_role(desdemona, UserProfile.ROLE_REALM_OWNER, acting_user=None)
 
             guest_user = get_user_by_delivery_email("[email protected]", zulip_realm)
             guest_user.role = UserProfile.ROLE_GUEST