diff --git a/app/controllers/discourse_assign/assign_controller.rb b/app/controllers/discourse_assign/assign_controller.rb index 3b90bf7..41177cc 100644 --- a/app/controllers/discourse_assign/assign_controller.rb +++ b/app/controllers/discourse_assign/assign_controller.rb @@ -83,6 +83,8 @@ module DiscourseAssign end def assigned + raise Discourse::InvalidAccess unless current_user&.admin? + offset = (params[:offset] || 0).to_i limit = (params[:limit] || 100).to_i diff --git a/lib/pending_assigns_reminder.rb b/lib/pending_assigns_reminder.rb index 370a9d1..8a807c8 100644 --- a/lib/pending_assigns_reminder.rb +++ b/lib/pending_assigns_reminder.rb @@ -35,8 +35,11 @@ class PendingAssignsReminder end def assigned_topics(user, order:) + secure = Topic.listable_topics.secured(Guardian.new(user)).or(Topic.private_messages_for_user(user)) + Topic.joins(:_custom_fields).select(:slug, :id, :title, :fancy_title, 'topic_custom_fields.created_at AS assigned_at') .where('topic_custom_fields.name = ? AND topic_custom_fields.value = ?', TopicAssigner::ASSIGNED_TO_ID, user.id.to_s) + .merge(secure) .order("topic_custom_fields.created_at #{order}") .limit(3) end diff --git a/lib/topic_assigner.rb b/lib/topic_assigner.rb index a11ae5b..4183403 100644 --- a/lib/topic_assigner.rb +++ b/lib/topic_assigner.rb @@ -136,19 +136,7 @@ class ::TopicAssigner def can_be_assigned?(assign_to) return false unless allowed_user_ids.include?(assign_to.id) - return true if (!@topic.private_message? || assign_to.admin?) - - results = DB.query_single(<<~SQL - SELECT 1 - FROM topics - LEFT OUTER JOIN topic_allowed_users tau ON tau.topic_id = topics.id - LEFT OUTER JOIN topic_allowed_groups tag ON tag.topic_id = topics.id - LEFT OUTER JOIN group_users gu ON gu.group_id = tag.group_id - WHERE topics.id = #{@topic.id} AND (gu.user_id = #{assign_to.id} OR tau.user_id = #{assign_to.id}) - SQL - ) - - results.present? + Guardian.new(assign_to).can_see_topic?(@topic) end def assign(assign_to, silent: false) diff --git a/spec/lib/pending_assigns_reminder_spec.rb b/spec/lib/pending_assigns_reminder_spec.rb index 0418101..a67cf05 100644 --- a/spec/lib/pending_assigns_reminder_spec.rb +++ b/spec/lib/pending_assigns_reminder_spec.rb @@ -31,14 +31,19 @@ RSpec.describe PendingAssignsReminder do before do add_to_assign_allowed_group(user) + secure_category = Fabricate(:private_category, group: Fabricate(:group)) + @post1 = Fabricate(:post) @post2 = Fabricate(:post) @post2.topic.update_column(:fancy_title, nil) @post3 = Fabricate(:post) + @post4 = Fabricate(:post) TopicAssigner.new(@post1.topic, user).assign(user) TopicAssigner.new(@post2.topic, user).assign(user) TopicAssigner.new(@post3.topic, user).assign(user) + TopicAssigner.new(@post4.topic, user).assign(user) @post3.topic.trash! + @post4.topic.update(category: secure_category) end it 'creates a reminder for a particular user and sets the timestamp of the last reminder' do @@ -57,11 +62,13 @@ RSpec.describe PendingAssignsReminder do expect(topic.title).to eq(I18n.t( 'pending_assigns_reminder.title', - pending_assignments: 2 + pending_assignments: 3 )) expect(post.raw).to include(@post1.topic.fancy_title) expect(post.raw).to include(@post2.topic.fancy_title) + expect(post.raw).to_not include(@post3.topic.fancy_title) + expect(post.raw).to_not include(@post4.topic.fancy_title) expect( user.reload.custom_fields[described_class::REMINDED_AT].to_datetime diff --git a/spec/lib/topic_assigner_spec.rb b/spec/lib/topic_assigner_spec.rb index c492abb..bf72dd1 100644 --- a/spec/lib/topic_assigner_spec.rb +++ b/spec/lib/topic_assigner_spec.rb @@ -32,6 +32,8 @@ RSpec.describe TopicAssigner do context "assigning and unassigning" do let(:post) { Fabricate(:post) } let(:topic) { post.topic } + let(:secure_category) { Fabricate(:private_category, group: Fabricate(:group)) } + let(:secure_topic) { Fabricate(:post).topic.tap { |t| t.update(category: secure_category) } } let(:moderator) { Fabricate(:moderator, groups: [assign_allowed_group]) } let(:moderator2) { Fabricate(:moderator, groups: [assign_allowed_group]) } let(:assigner) { TopicAssigner.new(topic, moderator2) } @@ -189,13 +191,20 @@ RSpec.describe TopicAssigner do fab!(:admin) { Fabricate(:admin) } - it 'fails to assign when the assigned user cannot view the topic' do + it 'fails to assign when the assigned user cannot view the pm' do assign = TopicAssigner.new(pm, admin).assign(moderator) expect(assign[:success]).to eq(false) expect(assign[:reason]).to eq(:forbidden_assign_to) end + it 'fails to assign when the assigned user cannot view the topic' do + assign = TopicAssigner.new(secure_topic, admin).assign(moderator) + + expect(assign[:success]).to eq(false) + expect(assign[:reason]).to eq(:forbidden_assign_to) + end + it "assigns the PM to the moderator when it's included in the list of allowed users" do pm.allowed_users << moderator diff --git a/spec/requests/assign_controller_spec.rb b/spec/requests/assign_controller_spec.rb index 649f00d..5737f64 100644 --- a/spec/requests/assign_controller_spec.rb +++ b/spec/requests/assign_controller_spec.rb @@ -175,6 +175,25 @@ RSpec.describe DiscourseAssign::AssignController do get '/assign/assigned.json', params: { offset: 2 } expect(JSON.parse(response.body)['topics'].map { |t| t['id'] }).to match_array([post1.topic_id]) end + + context "with custom allowed groups" do + let(:custom_allowed_group) { Fabricate(:group, name: 'mygroup') } + let(:other_user) { Fabricate(:user, groups: [custom_allowed_group]) } + before do + SiteSetting.assign_allowed_on_groups += "|#{custom_allowed_group.id}" + end + + it 'works for admins' do + get '/assign/assigned.json' + expect(response.status).to eq(200) + end + + it 'does not work for other groups' do + sign_in(other_user) + get '/assign/assigned.json' + expect(response.status).to eq(403) + end + end end end