Skip to content

Commit 2bdb4f2

Browse files
committed
restrict "recent messages" on old roster user page to instructors
it's leaky and slow and old and hopefully won't be missed also don't do all the calculations when profiles is enabled test plan: * make sure Profiles are not enabled on the root account * create a course with a discussion topic * post to the topic as a student * as a different student, visit the poster's course roster user page (i.e. click on their name on the course roster) * the "Recent Messages" should not be shown there (and possibly leak hidden discussion posts _gasp_) closes #CNVS-24111 Change-Id: Ifd954f32478796bbcdebe075cea84cb95d03a368 Reviewed-on: https://gerrit.instructure.com/89563 Tested-by: Jenkins Reviewed-by: Jeremy Stanley <jeremy@instructure.com> QA-Review: Deepeeca Soundarrajan <dsoundarrajan@instructure.com> Product-Review: Chris Ward <cward@instructure.com>
1 parent 0964e95 commit 2bdb4f2

3 files changed

Lines changed: 17 additions & 11 deletions

File tree

app/controllers/context_controller.rb

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -329,16 +329,7 @@ def roster_user
329329
return
330330
end
331331

332-
@topics = @context.discussion_topics.active.reject{|a| a.locked_for?(@current_user, :check_policies => true) }
333-
@entries = []
334-
@topics.each do |topic|
335-
@entries << topic if topic.user_id == @user.id
336-
@entries.concat topic.discussion_entries.active.where(user_id: @user)
337-
end
338-
@entries = @entries.sort_by {|e| e.created_at }
339332
@enrollments = @context.enrollments.for_user(@user) rescue []
340-
@messages = @entries
341-
@messages = @messages.select{|m| m.grants_right?(@current_user, session, :read) }.sort_by{|e| e.created_at }.reverse
342333

343334
if @domain_root_account.enable_profiles?
344335
@user_data = profile_data(
@@ -350,6 +341,19 @@ def roster_user
350341
render :new_roster_user
351342
return false
352343
end
344+
345+
if @user.grants_right?(@current_user, session, :read_profile)
346+
# self and instructors
347+
@topics = @context.discussion_topics.active.reject{|a| a.locked_for?(@current_user, :check_policies => true) }
348+
@messages = []
349+
@topics.each do |topic|
350+
@messages << topic if topic.user_id == @user.id
351+
end
352+
@messages += DiscussionEntry.active.where(:discussion_topic_id => @topics, :user_id => @user).to_a
353+
354+
@messages = @messages.select{|m| m.grants_right?(@current_user, session, :read) }.sort_by{|e| e.created_at }.reverse
355+
end
356+
353357
true
354358
end
355359
end

app/views/context/roster_user.html.erb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -176,12 +176,14 @@
176176
</div>
177177
<% end %>
178178

179+
<% if @messages %>
179180
<h2><%= t('headings.recent_messages', %{Recent Messages}) %></h2>
180181
<%= t('no_messages', "No Messages") if @messages.empty? %>
181182
<% @messages[0,10].each do |message| %>
182183
<% if message.is_a?(DiscussionEntry) %>
183184
<%= render :partial => 'discussion_topics/entry', :object => message, :locals => {:out_of_context => true, :show_context => true, :assignment_visible_to_user => message.discussion_topic.visible_for?(@current_user)} %>
184185
<% end %>
185186
<% end %>
187+
<% end %>
186188

187189
<% js_bundle 'legacy/context_roster_user' %>

spec/controllers/context_controller_spec.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@
9393
end
9494

9595
it "should assign variables" do
96-
user_session(@student)
96+
user_session(@teacher)
9797
@enrollment = @course.enroll_student(user(:active_all => true))
9898
@enrollment.accept!
9999
@student = @enrollment.user
@@ -103,7 +103,7 @@
103103
expect(assigns[:user]).not_to be_nil
104104
expect(assigns[:user]).to eql(@student)
105105
expect(assigns[:topics]).not_to be_nil
106-
expect(assigns[:entries]).not_to be_nil
106+
expect(assigns[:messages]).not_to be_nil
107107
end
108108

109109
describe 'across shards' do

0 commit comments

Comments
 (0)