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

@@ -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
 

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,

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.

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,

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') {

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!"),

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"

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

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),
+        ),
+    ]

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,

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",

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"))

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

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