From 8d3fcc975076ba5397797587ebb453fcfdfe3a28 Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 20 Dec 2023 10:51:00 +0100 Subject: [PATCH 1/4] Require valid token to view series activities --- app/controllers/activities_controller.rb | 1 + app/controllers/series_controller.rb | 4 +--- app/policies/series_policy.rb | 5 +++++ 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index fe192e76f0..eba6d52d9c 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -61,6 +61,7 @@ def index @activities = if params[:series_id] @series = Series.find(params[:series_id]) authorize @series, :show? + raise Pundit::NotAuthorizedError unless SeriesPolicy.new(current_user, @series).valid_token?(params[:token]) policy(@series).overview? ? @series.activities : [] else policy_scope(Activity).order_by_popularity(:DESC) diff --git a/app/controllers/series_controller.rb b/app/controllers/series_controller.rb index f9a66e8ca8..81481e4d56 100644 --- a/app/controllers/series_controller.rb +++ b/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 SeriesPolicy.new(current_user, @series).valid_token?(params[:token]) end def send_zip(zip) diff --git a/app/policies/series_policy.rb b/app/policies/series_policy.rb index 376a8a2799..b4e1bec58c 100644 --- a/app/policies/series_policy.rb +++ b/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? From e2eed7cdc7a694aa6e955f917abeff4655afd27f Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 20 Dec 2023 10:57:59 +0100 Subject: [PATCH 2/4] Add tests --- .../controllers/activities_controller_test.rb | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/controllers/activities_controller_test.rb b/test/controllers/activities_controller_test.rb index d6630565c8..e9f9cb1874 100644 --- a/test/controllers/activities_controller_test.rb +++ b/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 From cf6f391771120a8b6b66276fd0da2795f253663e Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 20 Dec 2023 10:58:44 +0100 Subject: [PATCH 3/4] Fix linting --- app/controllers/activities_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index eba6d52d9c..d1a3d4b958 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -62,6 +62,7 @@ def index @series = Series.find(params[:series_id]) authorize @series, :show? raise Pundit::NotAuthorizedError unless SeriesPolicy.new(current_user, @series).valid_token?(params[:token]) + policy(@series).overview? ? @series.activities : [] else policy_scope(Activity).order_by_popularity(:DESC) From 6487fc5cd27c163bccbf4871a30ab6923083f91f Mon Sep 17 00:00:00 2001 From: jorg-vr Date: Wed, 20 Dec 2023 11:42:14 +0100 Subject: [PATCH 4/4] Apply feedback --- app/controllers/activities_controller.rb | 2 +- app/controllers/series_controller.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/activities_controller.rb b/app/controllers/activities_controller.rb index d1a3d4b958..de3b639af2 100644 --- a/app/controllers/activities_controller.rb +++ b/app/controllers/activities_controller.rb @@ -61,7 +61,7 @@ def index @activities = if params[:series_id] @series = Series.find(params[:series_id]) authorize @series, :show? - raise Pundit::NotAuthorizedError unless SeriesPolicy.new(current_user, @series).valid_token?(params[:token]) + raise Pundit::NotAuthorizedError unless policy(@series).valid_token?(params[:token]) policy(@series).overview? ? @series.activities : [] else diff --git a/app/controllers/series_controller.rb b/app/controllers/series_controller.rb index 81481e4d56..a90e1d743f 100644 --- a/app/controllers/series_controller.rb +++ b/app/controllers/series_controller.rb @@ -283,7 +283,7 @@ def set_series end def check_token - raise Pundit::NotAuthorizedError unless SeriesPolicy.new(current_user, @series).valid_token?(params[:token]) + raise Pundit::NotAuthorizedError unless policy(@series).valid_token?(params[:token]) end def send_zip(zip)