From 146dfa6f0b7b6e81f6420ffff5e824ed3179d7bd Mon Sep 17 00:00:00 2001
From: Durga Akhil Mundroy <akhil.m.durga2@gmail.com>
Date: Sun, 16 Jul 2017 09:00:44 +0000
Subject: [PATCH] org-permissions: Add allow_edit_history organiztion setting.

This new setting controls whether or not users are allowed to see the
edit history in a Zulip organization.  It controls access through 2
key mechanisms:

* For long-ago edited messages, get_messages removes the edit history
  content from messages it sends to clients.

* For newly edited messages, clients are responsible for checking the
  setting and not saving the edit history data.  Since the webapp was
  the only client displaying it before this change, this just required
  some changes in message_events.js.

Significantly modified by tabbott to fix some logic bugs and add a
test.
---
 docs/security-model.md                        |  5 +--
 static/js/admin.js                            |  1 +
 static/js/message_events.js                   | 33 ++++++++++--------
 static/js/popovers.js                         |  2 +-
 static/js/server_events_dispatch.js           |  2 ++
 static/js/settings_org.js                     |  5 +++
 .../organization-permissions-admin.handlebars | 13 +++++++
 zerver/lib/actions.py                         |  8 +++++
 .../0091_realm_allow_edit_history.py          | 20 +++++++++++
 zerver/models.py                              |  2 ++
 zerver/tests/test_home.py                     |  1 +
 zerver/tests/test_messages.py                 | 34 +++++++++++++++++++
 zerver/views/messages.py                      |  6 ++++
 zerver/views/realm.py                         |  3 +-
 14 files changed, 116 insertions(+), 19 deletions(-)
 create mode 100644 zerver/migrations/0091_realm_allow_edit_history.py

diff --git a/docs/security-model.md b/docs/security-model.md
index 0fe3fcd5e8..354fd3ccc1 100644
--- a/docs/security-model.md
+++ b/docs/security-model.md
@@ -123,8 +123,9 @@ your organization.
   content (e.g. a password) shared unintentionally. Other users may
   have seen and saved the content of the original message, or have an
   integration (e.g. push notifications) forwarding all messages they
-  receive to another service. Zulip also stores and sends to clients
-  the content of every historical version of a message.
+  receive to another service.  Zulip stores the edit history of
+  messages, but it may or may not be available to clients, depending
+  on an organization-level setting.
 
 ## Users and Bots
 
diff --git a/static/js/admin.js b/static/js/admin.js
index 403630f992..d083d8f0ff 100644
--- a/static/js/admin.js
+++ b/static/js/admin.js
@@ -43,6 +43,7 @@ function _setup_page() {
         realm_message_content_edit_limit_minutes:
             Math.ceil(page_params.realm_message_content_edit_limit_seconds / 60),
         realm_message_retention_days: page_params.realm_message_retention_days,
+        realm_allow_edit_history: page_params.realm_allow_edit_history,
         language_list: page_params.language_list,
         realm_default_language: page_params.realm_default_language,
         realm_waiting_period_threshold: page_params.realm_waiting_period_threshold,
diff --git a/static/js/message_events.js b/static/js/message_events.js
index 3a1be769d5..48eb54903a 100644
--- a/static/js/message_events.js
+++ b/static/js/message_events.js
@@ -198,22 +198,25 @@ exports.update_messages = function update_messages(events) {
         }
 
         if (event.orig_content !== undefined) {
-            // Most correctly, we should do this for topic edits as
-            // well; but we don't use the data except for content
-            // edits anyway.
-            var edit_history_entry = {
-                edited_by: event.edited_by,
-                prev_content: event.orig_content,
-                prev_rendered_content: event.orig_rendered_content,
-                prev_rendered_content_version: event.prev_rendered_content_version,
-                timestamp: event.edit_timestamp,
-            };
-            // Add message's edit_history in message dict
-            // For messages that are edited, edit_history needs to be added to message in frontend.
-            if (msg.edit_history === undefined) {
-                msg.edit_history = [];
+            if (page_params.realm_allow_edit_history) {
+                // Most correctly, we should do this for topic edits as
+                // well; but we don't use the data except for content
+                // edits anyway.
+                var edit_history_entry = {
+                    edited_by: event.edited_by,
+                    prev_content: event.orig_content,
+                    prev_rendered_content: event.orig_rendered_content,
+                    prev_rendered_content_version: event.prev_rendered_content_version,
+                    timestamp: event.edit_timestamp,
+                };
+                // Add message's edit_history in message dict
+                // For messages that are edited, edit_history needs to
+                // be added to message in frontend.
+                if (msg.edit_history === undefined) {
+                    msg.edit_history = [];
+                }
+                msg.edit_history = [edit_history_entry].concat(msg.edit_history);
             }
-            msg.edit_history = [edit_history_entry].concat(msg.edit_history);
             message_content_edited = true;
 
             // Update raw_content, so that editing a few times in a row is fast.
diff --git a/static/js/popovers.js b/static/js/popovers.js
index a2c3419288..8e0caae458 100644
--- a/static/js/popovers.js
+++ b/static/js/popovers.js
@@ -161,7 +161,7 @@ exports.toggle_actions_popover = function (element, id) {
 
         var should_display_edit_history_option = _.any(message.edit_history, function (entry) {
             return entry.prev_content !== undefined;
-        });
+        }) && page_params.realm_allow_edit_history;
         var should_display_delete_option = page_params.is_admin;
         var args = {
             message: message,
diff --git a/static/js/server_events_dispatch.js b/static/js/server_events_dispatch.js
index 2d7d1ba41a..1ad7dce8bd 100644
--- a/static/js/server_events_dispatch.js
+++ b/static/js/server_events_dispatch.js
@@ -76,6 +76,8 @@ exports.dispatch_normal_event = function dispatch_normal_event(event) {
         } else if (event.op === 'update' && event.property === 'add_emoji_by_admins_only') {
             page_params.realm_add_emoji_by_admins_only = event.value;
             settings_emoji.update_custom_emoji_ui();
+        } else if (event.op === 'update' && event.property === 'allow_edit_history') {
+            page_params.realm_allow_edit_history = event.value;
         } else if (event.op === 'update' && event.property === 'restricted_to_domain') {
             page_params.realm_restricted_to_domain = event.value;
         } else if (event.op === 'update' && event.property === 'message_retention_days') {
diff --git a/static/js/settings_org.js b/static/js/settings_org.js
index bf09ce9c67..d3f1a0a572 100644
--- a/static/js/settings_org.js
+++ b/static/js/settings_org.js
@@ -206,6 +206,11 @@ function _set_up() {
                 checked_msg: i18n.t("Only administrators may now add new emoji!"),
                 unchecked_msg: i18n.t("Any user may now add new emoji!"),
             },
+            allow_edit_history: {
+                type: 'bool',
+                checked_msg: i18n.t("Users can view message edit history!"),
+                unchecked_msg: i18n.t("Users can no longer view message edit history!"),
+            },
             create_stream_by_admins_only: {
                 type: 'bool',
                 checked_msg: i18n.t("Only administrators may now create new streams!"),
diff --git a/static/templates/settings/organization-permissions-admin.handlebars b/static/templates/settings/organization-permissions-admin.handlebars
index aecd6d08dd..d06d10eaf3 100644
--- a/static/templates/settings/organization-permissions-admin.handlebars
+++ b/static/templates/settings/organization-permissions-admin.handlebars
@@ -9,6 +9,7 @@
         <div class="alert" id="admin-realm-inline-url-embed-preview-status"></div>
         <div class="alert" id="admin-realm-create-stream-by-admins-only-status"></div>
         <div class="alert" id="admin-realm-add-emoji-by-admins-only-status"></div>
+        <div class="alert" id="admin-realm-allow-edit-history-status"></div>
         <div class="alert" id="admin-realm-message-editing-status"></div>
         <div class="alert" id="admin-realm-name-changes-disabled-status"></div>
         <div class="alert" id="admin-realm-email-changes-disabled-status"></div>
@@ -155,6 +156,18 @@
                   {{#unless realm_allow_message_editing}}disabled="disabled"{{/unless}} />
             </div>
 
+            <div class="input-group">
+                <label class="checkbox">
+                    <input type="checkbox" id="id_realm_allow_edit_history" name="realm_allow_edit_history"
+                           {{#if realm_allow_edit_history}}checked="checked"{{/if}} />
+                    <span></span>
+                </label>
+                <label for="id_realm_allow_edit_history" id="id_realm_allow_edit_history_label" class="inline-block"
+                    title="{{t 'If checked, users will be able view message edit history.'}}">
+                    {{t "Users can view message edit history" }}
+                </label>
+            </div>
+
             {{#if false}}
             <div class="input-group">
                 <label for="realm_message_retention_days"
diff --git a/zerver/lib/actions.py b/zerver/lib/actions.py
index 820765c408..ce22442882 100644
--- a/zerver/lib/actions.py
+++ b/zerver/lib/actions.py
@@ -2765,6 +2765,14 @@ def do_update_message(user_profile, message, subject, propagate_mode, content, r
             # Don't highlight message edit diffs on prod
             rendered_content = highlight_html_differences(first_rendered_content, rendered_content)
 
+        # One could imagine checking realm.allow_edit_history here and
+        # modifying the events based on that setting, but doing so
+        # doesn't really make sense.  We need to send the edit event
+        # to clients regardless, and a client already had access to
+        # the original/pre-edit content of the message anyway.  That
+        # setting must be enforced on the client side, and making a
+        # change here simply complicates the logic for clients parsing
+        # edit history events.
         event['orig_content'] = message.content
         event['orig_rendered_content'] = message.rendered_content
         edit_history_event["prev_content"] = message.content
diff --git a/zerver/migrations/0091_realm_allow_edit_history.py b/zerver/migrations/0091_realm_allow_edit_history.py
new file mode 100644
index 0000000000..fb5ec62806
--- /dev/null
+++ b/zerver/migrations/0091_realm_allow_edit_history.py
@@ -0,0 +1,20 @@
+# -*- coding: utf-8 -*-
+# Generated by Django 1.11.2 on 2017-07-16 08:57
+from __future__ import unicode_literals
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('zerver', '0090_userprofile_high_contrast_mode'),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name='realm',
+            name='allow_edit_history',
+            field=models.BooleanField(default=True),
+        ),
+    ]
diff --git a/zerver/models.py b/zerver/models.py
index 26e3493a5c..775feecae1 100644
--- a/zerver/models.py
+++ b/zerver/models.py
@@ -131,6 +131,7 @@ class Realm(ModelReprMixin, models.Model):
     DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS = 600  # if changed, also change in admin.js
     message_content_edit_limit_seconds = models.IntegerField(default=DEFAULT_MESSAGE_CONTENT_EDIT_LIMIT_SECONDS)  # type: int
     message_retention_days = models.IntegerField(null=True)  # type: Optional[int]
+    allow_edit_history = models.BooleanField(default=True)  # type: bool
 
     # Valid org_types are {CORPORATE, COMMUNITY}
     CORPORATE = 1
@@ -148,6 +149,7 @@ class Realm(ModelReprMixin, models.Model):
     # Define the types of the various automatically managed properties
     property_types = dict(
         add_emoji_by_admins_only=bool,
+        allow_edit_history=bool,
         create_stream_by_admins_only=bool,
         default_language=Text,
         description=Text,
diff --git a/zerver/tests/test_home.py b/zerver/tests/test_home.py
index 90c40c22a3..261c6e3632 100644
--- a/zerver/tests/test_home.py
+++ b/zerver/tests/test_home.py
@@ -98,6 +98,7 @@ class HomeTest(ZulipTestCase):
             "prompt_for_invites",
             "queue_id",
             "realm_add_emoji_by_admins_only",
+            "realm_allow_edit_history",
             "realm_allow_message_editing",
             "realm_authentication_methods",
             "realm_bot_domain",
diff --git a/zerver/tests/test_messages.py b/zerver/tests/test_messages.py
index ca81e62ccb..c6f3d72d50 100644
--- a/zerver/tests/test_messages.py
+++ b/zerver/tests/test_messages.py
@@ -39,6 +39,7 @@ from zerver.models import (
 from zerver.lib.actions import (
     check_message,
     check_send_message,
+    do_set_realm_property,
     extract_recipients,
     do_create_user,
     get_client,
@@ -1229,6 +1230,39 @@ class EditMessageTest(ZulipTestCase):
         content = Message.objects.filter(id=msg_id).values_list('content', flat = True)[0]
         self.assertEqual(content, "(deleted)")
 
+    def test_edit_message_history_disabled(self):
+        # type: () -> None
+        user_profile = self.example_user("hamlet")
+        do_set_realm_property(user_profile.realm, "allow_edit_history", False)
+        self.login(self.example_email("hamlet"))
+
+        # Single-line edit
+        msg_id_1 = self.send_message(self.example_email("hamlet"),
+                                     "Denmark",
+                                     Recipient.STREAM,
+                                     subject="editing",
+                                     content="content before edit")
+
+        new_content_1 = 'content after edit'
+        result_1 = self.client_patch("/json/messages/" + str(msg_id_1), {
+            'message_id': msg_id_1, 'content': new_content_1
+        })
+        self.assert_json_success(result_1)
+
+        result = self.client_get(
+            "/json/messages/" + str(msg_id_1) + "/history")
+        self.assert_json_error(result, "Message edit history is disabled in this organization")
+
+        # Now verify that if we fetch the message directly, there's no
+        # edit history data attached.
+        messages_result = self.client_get("/json/messages",
+                                          {"anchor": msg_id_1, "num_before": 0, "num_after": 10})
+        self.assert_json_success(messages_result)
+        json_messages = ujson.loads(
+            messages_result.content.decode('utf-8'))
+        for msg in json_messages['messages']:
+            self.assertNotIn("edit_history", msg)
+
     def test_edit_message_history(self):
         # type: () -> None
         self.login(self.example_email("hamlet"))
diff --git a/zerver/views/messages.py b/zerver/views/messages.py
index 96931f7f54..4124a7c5af 100644
--- a/zerver/views/messages.py
+++ b/zerver/views/messages.py
@@ -776,6 +776,10 @@ def get_messages_backend(request, user_profile,
         msg_dict = message_dicts[message_id]
         msg_dict.update({"flags": user_message_flags[message_id]})
         msg_dict.update(search_fields.get(message_id, {}))
+        # Make sure that we never send message edit history to clients
+        # in realms with allow_edit_history disabled.
+        if "edit_history" in msg_dict and not user_profile.realm.allow_edit_history:
+            del msg_dict["edit_history"]
         message_list.append(msg_dict)
 
     statsd.incr('loaded_old_messages', len(message_list))
@@ -1027,6 +1031,8 @@ def fill_edit_history_entries(message_history, message):
 def get_message_edit_history(request, user_profile,
                              message_id=REQ(converter=to_non_negative_int)):
     # type: (HttpRequest, UserProfile, int) -> HttpResponse
+    if not user_profile.realm.allow_edit_history:
+        return json_error(_("Message edit history is disabled in this organization"))
     message, ignored_user_message = access_message(user_profile, message_id)
 
     # Extract the message edit history from the message
diff --git a/zerver/views/realm.py b/zerver/views/realm.py
index 3ee18f5fae..8944ec8014 100644
--- a/zerver/views/realm.py
+++ b/zerver/views/realm.py
@@ -35,12 +35,13 @@ def update_realm(request, user_profile, name=REQ(validator=check_string, default
                  allow_message_editing=REQ(validator=check_bool, default=None),
                  mandatory_topics=REQ(validator=check_bool, default=None),
                  message_content_edit_limit_seconds=REQ(converter=to_non_negative_int, default=None),
+                 allow_edit_history=REQ(validator=check_bool, default=None),
                  default_language=REQ(validator=check_string, default=None),
                  waiting_period_threshold=REQ(converter=to_non_negative_int, default=None),
                  authentication_methods=REQ(validator=check_dict([]), default=None),
                  notifications_stream_id=REQ(validator=check_int, default=None),
                  message_retention_days=REQ(converter=to_not_negative_int_or_none, default=None)):
-    # type: (HttpRequest, UserProfile, Optional[str], Optional[str], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[int], Optional[str], Optional[int], Optional[dict], Optional[int], Optional[int]) -> HttpResponse
+    # type: (HttpRequest, UserProfile, Optional[str], Optional[str], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[bool], Optional[int], Optional[bool], Optional[str], Optional[int], Optional[dict], Optional[int], Optional[int]) -> HttpResponse
     realm = user_profile.realm
 
     # Additional validation/error checking beyond types go here, so