From 436b68a5819e01b8f08f1d645c8987505e109c4a Mon Sep 17 00:00:00 2001 From: Jan Cernik <66427541+jancernik@users.noreply.github.com> Date: Wed, 24 May 2023 09:05:20 -0300 Subject: [PATCH] FIX: Do not preview chat channels to read-only users (#21700) We want to simplify this case as it contains a lot of rabbit holes. --- .../chat/api/channels_controller.rb | 2 +- plugins/chat/lib/chat/channel_fetcher.rb | 2 +- .../spec/requests/chat_controller_spec.rb | 4 +-- plugins/chat/spec/system/jit_messages_spec.rb | 29 ------------------- .../chat/spec/system/visit_channel_spec.rb | 13 ++------- 5 files changed, 7 insertions(+), 43 deletions(-) diff --git a/plugins/chat/app/controllers/chat/api/channels_controller.rb b/plugins/chat/app/controllers/chat/api/channels_controller.rb index 1911e587e9e..1f73b610293 100644 --- a/plugins/chat/app/controllers/chat/api/channels_controller.rb +++ b/plugins/chat/app/controllers/chat/api/channels_controller.rb @@ -121,7 +121,7 @@ class Chat::Api::ChannelsController < Chat::ApiController @channel ||= begin channel = Chat::Channel.find(params.require(:channel_id)) - guardian.ensure_can_preview_chat_channel!(channel) + guardian.ensure_can_join_chat_channel!(channel) channel end end diff --git a/plugins/chat/lib/chat/channel_fetcher.rb b/plugins/chat/lib/chat/channel_fetcher.rb index 5a36cf599b7..4f86b7e9662 100644 --- a/plugins/chat/lib/chat/channel_fetcher.rb +++ b/plugins/chat/lib/chat/channel_fetcher.rb @@ -230,7 +230,7 @@ module Chat end raise Discourse::NotFound if chat_channel.blank? - raise Discourse::InvalidAccess if !guardian.can_preview_chat_channel?(chat_channel) + raise Discourse::InvalidAccess if !guardian.can_join_chat_channel?(chat_channel) chat_channel end end diff --git a/plugins/chat/spec/requests/chat_controller_spec.rb b/plugins/chat/spec/requests/chat_controller_spec.rb index 887b4b09409..4959c78c361 100644 --- a/plugins/chat/spec/requests/chat_controller_spec.rb +++ b/plugins/chat/spec/requests/chat_controller_spec.rb @@ -1125,7 +1125,7 @@ RSpec.describe Chat::ChatController do channel = Fabricate(:category_channel, chatable: Fabricate(:category)) message = Fabricate(:chat_message, chat_channel: channel) - Guardian.any_instance.expects(:can_preview_chat_channel?).with(channel) + Guardian.any_instance.expects(:can_join_chat_channel?).with(channel) sign_in(Fabricate(:user)) get "/chat/message/#{message.id}.json" @@ -1141,7 +1141,7 @@ RSpec.describe Chat::ChatController do before { sign_in(user) } it "ensures message's channel can be seen" do - Guardian.any_instance.expects(:can_preview_chat_channel?).with(channel) + Guardian.any_instance.expects(:can_join_chat_channel?).with(channel) get "/chat/lookup/#{message.id}.json", params: { chat_channel_id: channel.id } end diff --git a/plugins/chat/spec/system/jit_messages_spec.rb b/plugins/chat/spec/system/jit_messages_spec.rb index 1b1c1b4aabd..1fa423b765e 100644 --- a/plugins/chat/spec/system/jit_messages_spec.rb +++ b/plugins/chat/spec/system/jit_messages_spec.rb @@ -50,35 +50,6 @@ RSpec.describe "JIT messages", type: :system, js: true do ) end end - - context "when user can’t access a non read_restrictd channel" do - let!(:everyone) { Group.find(Group::AUTO_GROUPS[:everyone]) } - fab!(:category) { Fabricate(:category) } - fab!(:readonly_channel) { Fabricate(:category_channel, chatable: category) } - - before do - Fabricate( - :category_group, - category: category, - group: everyone, - permission_type: CategoryGroup.permission_types[:readonly], - ) - everyone.add(other_user) - readonly_channel.add(current_user) - end - - it "displays a mention warning" do - Jobs.run_immediately! - - chat.visit_channel(readonly_channel) - channel.send_message("hi @#{other_user.username}") - - expect(page).to have_content( - I18n.t("js.chat.mention_warning.cannot_see", username: other_user.username), - wait: 5, - ) - end - end end context "when category channel permission is readonly for everyone" do diff --git a/plugins/chat/spec/system/visit_channel_spec.rb b/plugins/chat/spec/system/visit_channel_spec.rb index 924c43bd5ab..6d91008b352 100644 --- a/plugins/chat/spec/system/visit_channel_spec.rb +++ b/plugins/chat/spec/system/visit_channel_spec.rb @@ -110,17 +110,10 @@ RSpec.describe "Visit channel", type: :system, js: true do ) end - it "doesn't allow user to join it" do - chat.visit_channel(readonly_category_channel_1) + it "shows an error" do + chat.visit_channel(inaccessible_dm_channel_1) - expect(page).not_to have_content(I18n.t("js.chat.channel_settings.join_channel")) - end - - it "shows a preview of the channel" do - chat.visit_channel(readonly_category_channel_1) - - expect(page).to have_content(readonly_category_channel_1.name) - expect(chat).to have_message(message_1) + expect(page).to have_content(I18n.t("invalid_access")) end end