From e0c9b0fdffa1d00bc4dd0251c9e4e96c21f97af7 Mon Sep 17 00:00:00 2001 From: arpit551 Date: Tue, 30 Jun 2020 03:01:25 +0530 Subject: [PATCH] audit_log: Log RealmAuditLog when Stream is created. Added new Event Type in AbstractRealmAuditLog STREAM_CREATED. Since we finally create streams in create_stream_if_needed function in zerver/lib/streams.py so logged realm_audit there. Passed acting_user when create_stream_if_needed or ensure_stream function is called. Added tests in test_audit_log. --- zerver/lib/actions.py | 12 +++++++----- zerver/lib/streams.py | 19 +++++++++++++++---- .../commands/add_users_to_streams.py | 2 +- .../commands/create_default_stream_groups.py | 2 +- zerver/management/commands/create_stream.py | 2 +- .../commands/generate_multiuse_invite_link.py | 2 +- zerver/models.py | 2 ++ zerver/tests/test_audit_log.py | 10 ++++++++++ zerver/tests/test_subs.py | 8 ++++---- .../commands/add_mock_conversation.py | 2 +- 10 files changed, 43 insertions(+), 18 deletions(-) diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py index 5a65e9f586..676f426585 100644 --- a/zerver/lib/actions.py +++ b/zerver/lib/actions.py @@ -1909,10 +1909,12 @@ def check_send_typing_notification(sender: UserProfile, def ensure_stream(realm: Realm, stream_name: str, invite_only: bool=False, - stream_description: str="") -> Stream: + stream_description: str="", + acting_user: Optional[UserProfile]=None) -> Stream: return create_stream_if_needed(realm, stream_name, invite_only=invite_only, - stream_description=stream_description)[0] + stream_description=stream_description, + acting_user=acting_user)[0] def get_recipient_from_user_profiles(recipient_profiles: Sequence[UserProfile], forwarded_mirror_message: bool, @@ -2404,7 +2406,7 @@ def _internal_prep_message(realm: Realm, if addressee.is_stream(): stream_name = addressee.stream_name() if stream_name is not None: - ensure_stream(realm, stream_name) + ensure_stream(realm, stream_name, acting_user=sender) try: return check_message(sender, get_client("Internal"), addressee, @@ -3675,7 +3677,7 @@ def do_create_realm(string_id: str, name: str, # Create stream once Realm object has been saved notifications_stream = ensure_stream( realm, Realm.DEFAULT_NOTIFICATION_STREAM_NAME, - stream_description="Everyone is added to this stream by default. Welcome! :octopus:") + stream_description="Everyone is added to this stream by default. Welcome! :octopus:", acting_user=None) realm.notifications_stream = notifications_stream # With the current initial streams situation, the only public @@ -3684,7 +3686,7 @@ def do_create_realm(string_id: str, name: str, signup_notifications_stream = ensure_stream( realm, Realm.INITIAL_PRIVATE_STREAM_NAME, invite_only=True, - stream_description="A private stream for core team members.") + stream_description="A private stream for core team members.", acting_user=None) realm.signup_notifications_stream = signup_notifications_stream realm.save(update_fields=['notifications_stream', 'signup_notifications_stream']) diff --git a/zerver/lib/streams.py b/zerver/lib/streams.py index 24cf22d055..5307097738 100644 --- a/zerver/lib/streams.py +++ b/zerver/lib/streams.py @@ -2,6 +2,7 @@ from typing import Any, Iterable, List, Mapping, Optional, Set, Tuple, Union from django.conf import settings from django.db.models.query import QuerySet +from django.utils.timezone import now as timezone_now from django.utils.translation import ugettext as _ from zerver.lib.markdown import markdown_convert @@ -9,6 +10,7 @@ from zerver.lib.request import JsonableError from zerver.models import ( DefaultStreamGroup, Realm, + RealmAuditLog, Recipient, Stream, Subscription, @@ -59,7 +61,8 @@ def create_stream_if_needed(realm: Realm, stream_post_policy: int=Stream.STREAM_POST_POLICY_EVERYONE, history_public_to_subscribers: Optional[bool]=None, stream_description: str="", - message_retention_days: Optional[int]=None) -> Tuple[Stream, bool]: + message_retention_days: Optional[int]=None, + acting_user: Optional[UserProfile]=None) -> Tuple[Stream, bool]: history_public_to_subscribers = get_default_value_for_history_public_to_subscribers( realm, invite_only, history_public_to_subscribers) @@ -90,10 +93,16 @@ def create_stream_if_needed(realm: Realm, realm_admin_ids = [user.id for user in stream.realm.get_admin_users_and_bots()] send_stream_creation_event(stream, realm_admin_ids) + + event_time = timezone_now() + RealmAuditLog.objects.create(realm=realm, acting_user=acting_user, + modified_stream=stream, event_type=RealmAuditLog.STREAM_CREATED, + event_time=event_time) return stream, created def create_streams_if_needed(realm: Realm, - stream_dicts: List[Mapping[str, Any]]) -> Tuple[List[Stream], List[Stream]]: + stream_dicts: List[Mapping[str, Any]], + acting_user: Optional[UserProfile]=None) -> Tuple[List[Stream], List[Stream]]: """Note that stream_dict["name"] is assumed to already be stripped of whitespace""" added_streams: List[Stream] = [] @@ -106,7 +115,8 @@ def create_streams_if_needed(realm: Realm, stream_post_policy=stream_dict.get("stream_post_policy", Stream.STREAM_POST_POLICY_EVERYONE), history_public_to_subscribers=stream_dict.get("history_public_to_subscribers"), stream_description=stream_dict.get("description", ""), - message_retention_days=stream_dict.get("message_retention_days", None) + message_retention_days=stream_dict.get("message_retention_days", None), + acting_user=acting_user ) if created: @@ -460,7 +470,8 @@ def list_to_streams(streams_raw: Iterable[Mapping[str, Any]], # paranoid approach, since often on Zulip two people will discuss # creating a new stream, and both people eagerly do it.) created_streams, dup_streams = create_streams_if_needed(realm=user_profile.realm, - stream_dicts=missing_stream_dicts) + stream_dicts=missing_stream_dicts, + acting_user=user_profile) existing_streams += dup_streams return existing_streams, created_streams diff --git a/zerver/management/commands/add_users_to_streams.py b/zerver/management/commands/add_users_to_streams.py index 04ecf73303..2ab536a3c0 100644 --- a/zerver/management/commands/add_users_to_streams.py +++ b/zerver/management/commands/add_users_to_streams.py @@ -29,7 +29,7 @@ class Command(ZulipBaseCommand): for stream_name in set(stream_names): for user_profile in user_profiles: - stream = ensure_stream(realm, stream_name) + stream = ensure_stream(realm, stream_name, acting_user=None) _ignore, already_subscribed = bulk_add_subscriptions([stream], [user_profile]) was_there_already = user_profile.id in {tup[0].id for tup in already_subscribed} print("{} {} to {}".format( diff --git a/zerver/management/commands/create_default_stream_groups.py b/zerver/management/commands/create_default_stream_groups.py index 6f13a7eaea..4b622cc715 100644 --- a/zerver/management/commands/create_default_stream_groups.py +++ b/zerver/management/commands/create_default_stream_groups.py @@ -46,7 +46,7 @@ Create default stream groups which the users can choose during sign up. streams = [] stream_names = {stream.strip() for stream in options["streams"].split(",")} for stream_name in set(stream_names): - stream = ensure_stream(realm, stream_name) + stream = ensure_stream(realm, stream_name, acting_user=None) streams.append(stream) try: diff --git a/zerver/management/commands/create_stream.py b/zerver/management/commands/create_stream.py index 7bec5f8934..d9dc6517eb 100644 --- a/zerver/management/commands/create_stream.py +++ b/zerver/management/commands/create_stream.py @@ -21,4 +21,4 @@ the command.""" assert realm is not None # Should be ensured by parser stream_name = options['stream_name'] - create_stream_if_needed(realm, stream_name) + create_stream_if_needed(realm, stream_name, acting_user=None) diff --git a/zerver/management/commands/generate_multiuse_invite_link.py b/zerver/management/commands/generate_multiuse_invite_link.py index 6ee0839471..0a8e216ad8 100644 --- a/zerver/management/commands/generate_multiuse_invite_link.py +++ b/zerver/management/commands/generate_multiuse_invite_link.py @@ -34,7 +34,7 @@ class Command(ZulipBaseCommand): if options["streams"]: stream_names = {stream.strip() for stream in options["streams"].split(",")} for stream_name in set(stream_names): - stream = ensure_stream(realm, stream_name) + stream = ensure_stream(realm, stream_name, acting_user=None) streams.append(stream) referred_by = self.get_user(options['referred_by'], realm) diff --git a/zerver/models.py b/zerver/models.py index e414f13c4e..43fed32556 100644 --- a/zerver/models.py +++ b/zerver/models.py @@ -2743,6 +2743,8 @@ class AbstractRealmAuditLog(models.Model): CUSTOMER_PLAN_CREATED = 502 CUSTOMER_SWITCHED_FROM_MONTHLY_TO_ANNUAL_PLAN = 503 + STREAM_CREATED = 601 + event_type: int = models.PositiveSmallIntegerField() # event_types synced from on-prem installations to Zulip Cloud when diff --git a/zerver/tests/test_audit_log.py b/zerver/tests/test_audit_log.py index 22335a9bcf..808fa72bd5 100644 --- a/zerver/tests/test_audit_log.py +++ b/zerver/tests/test_audit_log.py @@ -24,6 +24,7 @@ from zerver.lib.actions import ( do_regenerate_api_key, get_streams_traffic, ) +from zerver.lib.streams import create_stream_if_needed from zerver.lib.test_classes import ZulipTestCase from zerver.models import RealmAuditLog, UserProfile, get_client, get_realm @@ -212,3 +213,12 @@ class TestRealmAuditLog(ZulipTestCase): log_entry = RealmAuditLog.objects.get(realm=realm, event_type=RealmAuditLog.REALM_REACTIVATED) extra_data = ujson.loads(log_entry.extra_data) self.check_role_count_schema(extra_data[RealmAuditLog.ROLE_COUNT]) + + def test_create_stream_if_needed(self) -> None: + now = timezone_now() + realm = get_realm('zulip') + user = self.example_user('hamlet') + stream = create_stream_if_needed(realm, "test", invite_only=False, stream_description="Test Description", acting_user=user)[0] + self.assertEqual(RealmAuditLog.objects.filter(realm=realm, event_type=RealmAuditLog.STREAM_CREATED, + event_time__gte=now, acting_user=user, + modified_stream=stream).count(), 1) diff --git a/zerver/tests/test_subs.py b/zerver/tests/test_subs.py index d98f20751b..3570d939f5 100644 --- a/zerver/tests/test_subs.py +++ b/zerver/tests/test_subs.py @@ -2660,7 +2660,7 @@ class SubscriptionAPITest(ZulipTestCase): streams_to_sub, dict(principals=ujson.dumps([user1.id, user2.id])), ) - self.assert_length(queries, 39) + self.assert_length(queries, 40) self.assert_length(events, 7) for ev in [x for x in events if x['event']['type'] not in ('message', 'stream')]: @@ -3445,7 +3445,7 @@ class SubscriptionAPITest(ZulipTestCase): [new_streams[0]], dict(principals=ujson.dumps([user1.id, user2.id])), ) - self.assert_length(queries, 39) + self.assert_length(queries, 40) # Test creating private stream. with queries_captured() as queries: @@ -3455,7 +3455,7 @@ class SubscriptionAPITest(ZulipTestCase): dict(principals=ujson.dumps([user1.id, user2.id])), invite_only=True, ) - self.assert_length(queries, 39) + self.assert_length(queries, 40) # Test creating a public stream with announce when realm has a notification stream. notifications_stream = get_stream(self.streams[0], self.test_realm) @@ -3470,7 +3470,7 @@ class SubscriptionAPITest(ZulipTestCase): principals=ujson.dumps([user1.id, user2.id]), ), ) - self.assert_length(queries, 51) + self.assert_length(queries, 52) class GetStreamsTest(ZulipTestCase): def test_streams_api_for_bot_owners(self) -> None: diff --git a/zilencer/management/commands/add_mock_conversation.py b/zilencer/management/commands/add_mock_conversation.py index 452dc23eb9..e9dd515218 100644 --- a/zilencer/management/commands/add_mock_conversation.py +++ b/zilencer/management/commands/add_mock_conversation.py @@ -42,7 +42,7 @@ From image editing program: def add_message_formatting_conversation(self) -> None: realm = get_realm('zulip') - stream = ensure_stream(realm, 'zulip features') + stream = ensure_stream(realm, 'zulip features', acting_user=None) UserProfile.objects.filter(email__contains='stage').delete() starr = do_create_user('1@stage.example.com', 'password', realm, 'Ada Starr', '', acting_user=None)