diff --git a/plugins/chat/app/controllers/chat_controller.rb b/plugins/chat/app/controllers/chat_controller.rb index 9849ffc2909..31ad38e02a6 100644 --- a/plugins/chat/app/controllers/chat_controller.rb +++ b/plugins/chat/app/controllers/chat_controller.rb @@ -432,9 +432,10 @@ class Chat::ChatController < Chat::ChatBaseController def set_draft if params[:data].present? - ChatDraft.find_or_initialize_by(user: current_user, chat_channel_id: @chat_channel.id).update( - data: params[:data], - ) + ChatDraft.find_or_initialize_by( + user: current_user, + chat_channel_id: @chat_channel.id, + ).update!(data: params[:data]) else ChatDraft.where(user: current_user, chat_channel_id: @chat_channel.id).destroy_all end diff --git a/plugins/chat/app/models/chat_draft.rb b/plugins/chat/app/models/chat_draft.rb index 1d3781fa826..7dc1b7feeb0 100644 --- a/plugins/chat/app/models/chat_draft.rb +++ b/plugins/chat/app/models/chat_draft.rb @@ -3,6 +3,13 @@ class ChatDraft < ActiveRecord::Base belongs_to :user belongs_to :chat_channel + + validate :data_length + def data_length + if self.data && self.data.length > SiteSetting.max_chat_draft_length + self.errors.add(:base, I18n.t("chat.errors.draft_too_long")) + end + end end # == Schema Information diff --git a/plugins/chat/assets/javascripts/discourse/services/chat.js b/plugins/chat/assets/javascripts/discourse/services/chat.js index 0cb1c56ace3..931f480e438 100644 --- a/plugins/chat/assets/javascripts/discourse/services/chat.js +++ b/plugins/chat/assets/javascripts/discourse/services/chat.js @@ -373,11 +373,13 @@ export default class Chat extends Service { data.data = JSON.stringify(draft); } - ajax("/chat/drafts", { type: "POST", data, ignoreUnsent: false }) + ajax("/chat/drafts.json", { type: "POST", data, ignoreUnsent: false }) .then(() => { this.markNetworkAsReliable(); }) .catch((error) => { + // we ignore a draft which can't be saved because it's too big + // and only deal with network error for now if (!error.jqXHR?.responseJSON?.errors?.length) { this.markNetworkAsUnreliable(); } diff --git a/plugins/chat/config/locales/server.en.yml b/plugins/chat/config/locales/server.en.yml index f0c8abefb9d..372eda5e4a3 100644 --- a/plugins/chat/config/locales/server.en.yml +++ b/plugins/chat/config/locales/server.en.yml @@ -59,6 +59,7 @@ en: delete_channel_failed: "Delete channel failed, please try again." minimum_length_not_met: "Message is too short, must have a minimum of %{minimum} characters." message_too_long: "Message is too long, messages must be a maximum of %{maximum} characters." + draft_too_long: "Draft is too long." max_reactions_limit_reached: "New reactions are not allowed on this message." message_move_invalid_channel: "The source and destination channel must be public channels." message_move_no_messages_found: "No messages were found with the provided message IDs." diff --git a/plugins/chat/config/settings.yml b/plugins/chat/config/settings.yml index 072e9d97921..015477dc038 100644 --- a/plugins/chat/config/settings.yml +++ b/plugins/chat/config/settings.yml @@ -110,3 +110,6 @@ chat: default: 5 max: 10 min: 0 + max_chat_draft_length: + default: 50_000 + hidden: true diff --git a/plugins/chat/db/post_migrate/20230116090324_drop_chat_drafts_over_max_length.rb b/plugins/chat/db/post_migrate/20230116090324_drop_chat_drafts_over_max_length.rb new file mode 100644 index 00000000000..1d6913086f5 --- /dev/null +++ b/plugins/chat/db/post_migrate/20230116090324_drop_chat_drafts_over_max_length.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class DropChatDraftsOverMaxLength < ActiveRecord::Migration[7.0] + def up + if table_exists?(:chat_drafts) + # Delete abusive drafts + execute <<~SQL + DELETE FROM chat_drafts + WHERE LENGTH(data) > 50000 + SQL + end + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/plugins/chat/plugin.rb b/plugins/chat/plugin.rb index b3b5070a198..8a9127e51e6 100644 --- a/plugins/chat/plugin.rb +++ b/plugins/chat/plugin.rb @@ -455,6 +455,8 @@ after_initialize do add_to_serializer(:current_user, :chat_drafts) do ChatDraft .where(user_id: object.id) + .order(updated_at: :desc) + .limit(20) .pluck(:chat_channel_id, :data) .map { |row| { channel_id: row[0], data: row[1] } } end diff --git a/plugins/chat/spec/fabricators/chat_fabricator.rb b/plugins/chat/spec/fabricators/chat_fabricator.rb index e123c06bc54..a8d7f3e544e 100644 --- a/plugins/chat/spec/fabricators/chat_fabricator.rb +++ b/plugins/chat/spec/fabricators/chat_fabricator.rb @@ -126,3 +126,16 @@ Fabricator(:user_chat_channel_membership_for_dm, from: :user_chat_channel_member desktop_notification_level 2 mobile_notification_level 2 end + +Fabricator(:chat_draft) do + user + chat_channel + + transient :value, "chat draft message" + transient :uploads, [] + transient :reply_to_msg + + data do |attrs| + { value: attrs[:value], replyToMsg: attrs[:reply_to_msg], uploads: attrs[:uploads] }.to_json + end +end diff --git a/plugins/chat/spec/models/chat_draft_spec.rb b/plugins/chat/spec/models/chat_draft_spec.rb new file mode 100644 index 00000000000..27794bafaec --- /dev/null +++ b/plugins/chat/spec/models/chat_draft_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +RSpec.describe ChatDraft do + before { SiteSetting.max_chat_draft_length = 100 } + + it "errors when data.value is greater than `max_chat_draft_length`" do + draft = + described_class.create( + user_id: Fabricate(:user).id, + chat_channel_id: Fabricate(:chat_channel).id, + data: { value: "A" * (SiteSetting.max_chat_draft_length + 1) }.to_json, + ) + + expect(draft.errors.full_messages).to eq( + [I18n.t("chat.errors.draft_too_long", { maximum: SiteSetting.max_chat_draft_length })], + ) + end +end diff --git a/plugins/chat/spec/requests/chat_controller_spec.rb b/plugins/chat/spec/requests/chat_controller_spec.rb index 8ff5912df5b..c2eac575378 100644 --- a/plugins/chat/spec/requests/chat_controller_spec.rb +++ b/plugins/chat/spec/requests/chat_controller_spec.rb @@ -1285,6 +1285,19 @@ RSpec.describe Chat::ChatController do post "/chat/drafts.json", params: { chat_channel_id: dm_channel.id, data: "{}" } }.to change { ChatDraft.count }.by(1) end + + it "cannot create a too long chat draft" do + SiteSetting.max_chat_draft_length = 100 + + post "/chat/drafts.json", + params: { + chat_channel_id: chat_channel.id, + data: { value: "a" * (SiteSetting.max_chat_draft_length + 1) }.to_json, + } + + expect(response.status).to eq(422) + expect(response.parsed_body["errors"]).to eq([I18n.t("chat.errors.draft_too_long")]) + end end describe "#message_link" do diff --git a/plugins/chat/spec/serializer/core_ext/current_user_serializer_spec.rb b/plugins/chat/spec/serializer/core_ext/current_user_serializer_spec.rb new file mode 100644 index 00000000000..2e2fd20a0ef --- /dev/null +++ b/plugins/chat/spec/serializer/core_ext/current_user_serializer_spec.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +RSpec.describe CurrentUserSerializer do + fab!(:current_user) { Fabricate(:user) } + + let(:serializer) do + described_class.new(current_user, scope: Guardian.new(current_user), root: false) + end + + before do + SiteSetting.chat_enabled = true + SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:everyone] + current_user.user_option.update(chat_enabled: true) + end + + describe "#chat_drafts" do + context "when user can't chat" do + before { SiteSetting.chat_allowed_groups = Group::AUTO_GROUPS[:staff] } + + it "is not present" do + expect(serializer.as_json[:chat_drafts]).to be_blank + end + end + + it "is ordered by most recent drafts" do + Fabricate(:chat_draft, user: current_user, value: "second draft") + Fabricate(:chat_draft, user: current_user, value: "first draft") + + values = + serializer.as_json[:chat_drafts].map { |draft| MultiJson.load(draft[:data])["value"] } + expect(values).to eq(["first draft", "second draft"]) + end + + it "limits the numbers of drafts" do + 21.times { Fabricate(:chat_draft, user: current_user) } + + expect(serializer.as_json[:chat_drafts].length).to eq(20) + end + end +end