require context read access for discussion topics

test plan:
* create a course with a discussion topic
* keep it unpublished but add a student
* as a student, should not be able to
 view the discussion topic through a direct link

closes #CNVS-6384

Change-Id: I0c0bc66e377d870de6c97555e25e82a8226264f4
Reviewed-on: https://gerrit.instructure.com/48278
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
QA-Review: Adam Stone <astone@instructure.com>
Product-Review: James Williams  <jamesw@instructure.com>

Author: maneframe

app/controllers/application_controller.rb

@@ -420,6 +420,10 @@ def require_context
     return @context != nil
   end
 
+  def require_context_and_read_access
+    return require_context && authorized_action(@context, @current_user, :read)
+  end
+
   helper_method :clean_return_to
 
   MAX_ACCOUNT_LINEAGE_TO_SHOW_IN_CRUMBS = 3

app/controllers/discussion_entries_controller.rb

@@ -18,7 +18,7 @@
 
 # @API Discussion Topics
 class DiscussionEntriesController < ApplicationController
-  before_filter :require_context, :except => :public_feed
+  before_filter :require_context_and_read_access, :except => :public_feed
 
   def show
     @entry = @context.discussion_entries.find(params[:id]).tap{|e| e.current_user = @current_user}
@@ -152,7 +152,7 @@ def public_feed
       render :template => "shared/unauthorized_feed", :layout => "layouts/application", :status => :bad_request, :formats => [:html] # :template => "shared/unauthorized_feed", :status => :bad_request
       return
     end
-    if authorized_action(@topic, @current_user, :read)
+    if authorized_action(@context, @current_user, :read) && authorized_action(@topic, @current_user, :read)
       @all_discussion_entries = @topic.discussion_entries.active
       @discussion_entries = @all_discussion_entries
       if request.format == :rss && !@topic.podcast_has_student_posts

app/controllers/discussion_topics_api_controller.rb

@@ -23,7 +23,7 @@ class DiscussionTopicsApiController < ApplicationController
   include Api::V1::DiscussionTopics
   include Api::V1::User
 
-  before_filter :require_context
+  before_filter :require_context_and_read_access
   before_filter :require_topic
   before_filter :require_initial_post, except: [:add_entry, :mark_topic_read,
                                                 :mark_topic_unread, :show,

app/controllers/discussion_topics_controller.rb

@@ -217,7 +217,7 @@
 #     }
 #
 class DiscussionTopicsController < ApplicationController
-  before_filter :require_context, :except => :public_feed
+  before_filter :require_context_and_read_access, :except => :public_feed
 
   include Api::V1::DiscussionTopics
   include Api::V1::Assignment
@@ -694,6 +694,7 @@ def destroy
 
   def public_feed
     return unless get_feed_context
+
     feed = Atom::Feed.new do |f|
       f.title = t :discussion_feed_title, "%{title} Discussion Feed", :title => @context.name
       f.links << Atom::Link.new(:href => polymorphic_url([@context, :discussion_topics]), :rel => 'self')

spec/apis/v1/discussion_topics_api_spec.rb

@@ -431,6 +431,13 @@ def blank_fallback
         expect(json).to eq @response_json.merge("subscribed" => @topic.subscribed?(@user))
       end
 
+      it "should require course to be published for students" do
+        @course.claim
+        json = api_call(:get, "/api/v1/courses/#{@course.id}/discussion_topics/#{@topic.id}",
+                        {:controller => 'discussion_topics_api', :action => 'show', :format => 'json', :course_id => @course.id.to_s, :topic_id => @topic.id.to_s},
+                        {}, :expected_status => 401)
+      end
+
       it "should properly translate a video media comment in the discussion topic's message" do
         @topic.update_attributes(
           message: '<p><a id="media_comment_m-spHRwKY5ATHvPQAMKdZV_g" class="instructure_inline_media_comment video_comment" href="/media_objects/m-spHRwKY5ATHvPQAMKdZV_g">this is a media comment</a></p>'
@@ -1701,6 +1708,7 @@ def create_graded_discussion_for_da(assignment_opts={})
       course_with_teacher
       create_topic(@course, :title => "topic", :message => "topic")
       course_with_observer_logged_in(:course => @course)
+      @course.offer
       json = api_call(:get, "/api/v1/courses/#{@course.id}/discussion_topics.json",
                       { :controller => 'discussion_topics', :action => 'index', :format => 'json',
                         :course_id => @course.id.to_s })

spec/controllers/discussion_entries_controller_spec.rb

@@ -42,6 +42,13 @@ def topic_with_media_reply
       get 'show', :course_id => @course.id, :id => @entry.id
       assert_unauthorized
     end
+
+    it "should require course to be published for students" do
+      user_session(@student)
+      @course.claim
+      get 'show', :course_id => @course.id, :id => @entry.id
+      assert_unauthorized
+    end
 
     it "should assign variables" do
       user_session(@student)

spec/controllers/discussion_topics_controller_spec.rb

@@ -51,6 +51,13 @@ def topic_entry
       get 'index', :course_id => @course.id
       assert_unauthorized
     end
+
+    it "should require the course to be published for students" do
+      @course.claim
+      user_session(@student)
+      get 'index', :course_id => @course.id
+      assert_unauthorized
+    end
   end
 
   describe "GET 'show'" do
@@ -60,6 +67,14 @@ def topic_entry
       assert_unauthorized
     end
 
+    it "should require the course to be published for students" do
+      course_topic
+      @course.claim
+      user_session(@student)
+      get 'show', :course_id => @course.id, :id => @topic.id
+      assert_unauthorized
+    end
+
     it "should work for announcements in a public course" do
       @course.update_attribute(:is_public, true)
       @announcement = @course.announcements.create!(

spec/integration/context_module_spec.rb

@@ -20,7 +20,7 @@
 
 describe ContextModule do
   def course_module
-    course_with_student_logged_in
+    course_with_student_logged_in(:active_all => true)
     @module = @course.context_modules.create!(:name => "some module")
   end
 

spec/selenium/announcements_teacher_spec.rb

@@ -201,7 +201,7 @@
     end
 
     it "should remove delayed_post_at when unchecking delay_posting" do
-      topic = announcement_model(:title => @topic_title, :user => @user, :delayed_post_at => 10.days.ago)
+      topic = @course.announcements.create!(:title => @topic_title, :user => @user, :delayed_post_at => 10.days.ago, :message => "message")
       get "/courses/#{@course.id}/announcements/#{topic.id}"
       expect_new_page_load { f(".edit-btn").click }