Merge pull request #5249 from dodona-edu/fix/security-leak

Require valid token to view series activities

app/controllers/activities_controller.rb

@@ -61,6 +61,8 @@ def index
     @activities = if params[:series_id]
                     @series = Series.find(params[:series_id])
                     authorize @series, :show?
+                    raise Pundit::NotAuthorizedError unless policy(@series).valid_token?(params[:token])
+
                     policy(@series).overview? ? @series.activities : []
                   else
                     policy_scope(Activity).order_by_popularity(:DESC)

app/controllers/series_controller.rb

@@ -283,9 +283,7 @@ def set_series
   end
 
   def check_token
-    raise Pundit::NotAuthorizedError if @series.hidden? &&
-                                        !current_user&.course_admin?(@series.course) &&
-                                        @series.access_token != params[:token]
+    raise Pundit::NotAuthorizedError unless policy(@series).valid_token?(params[:token])
   end
 
   def send_zip(zip)

app/policies/series_policy.rb

@@ -129,6 +129,11 @@ def show_progress?
     record.instance_of?(Series) && (record.progress_enabled || course_admin?)
   end
 
+  def valid_token?(token)
+    !record.hidden? || user&.course_admin?(record.course) ||
+      record.access_token == token
+  end
+
   private
 
   def course_member?

test/controllers/activities_controller_test.rb

@@ -988,4 +988,46 @@ class ExerciseDescriptionTest < ActionDispatch::IntegrationTest
 
     assert_equal [other_exercise.id, exercise.id], activities_json.pluck('id')
   end
+
+  test 'activities for hidden series should only be shown with token' do
+    course = courses(:course1)
+    exercise = exercises(:python_exercise)
+    other_exercise = create :exercise
+    series = create :series, course: course, exercises: [exercise, other_exercise], visibility: :hidden
+
+    sign_in users(:student)
+
+    get series_activities_url(series, format: :json)
+
+    # should return 403
+    assert_response :forbidden
+
+    get series_activities_url(series, format: :json, token: series.access_token)
+
+    assert_response :success
+    activities_json = response.parsed_body
+
+    assert_equal [exercise.id, other_exercise.id], activities_json.pluck('id')
+  end
+
+  test 'admin should be able to see hidden series' do
+    course = courses(:course1)
+    exercise = exercises(:python_exercise)
+    other_exercise = create :exercise
+    series = create :series, course: course, exercises: [exercise, other_exercise], visibility: :hidden
+
+    sign_in users(:staff)
+
+    get series_activities_url(series, format: :json)
+
+    assert_response :forbidden
+
+    course.administrating_members << users(:staff)
+    get series_activities_url(series, format: :json)
+
+    assert_response :success
+    activities_json = response.parsed_body
+
+    assert_equal [exercise.id, other_exercise.id], activities_json.pluck('id')
+  end
 end