diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index b23cb460..e0c1fc58 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -22,6 +22,8 @@ class ExercisesController < ApplicationController skip_after_action :verify_authorized, only: %i[import_exercise import_uuid_check] skip_after_action :verify_policy_scoped, only: %i[import_exercise import_uuid_check], raise: false + rescue_from Pundit::NotAuthorizedError, with: :not_authorized_for_exercise + def authorize! authorize(@exercise || @exercises) end @@ -294,8 +296,6 @@ class ExercisesController < ApplicationController private :update_exercise_tips def implement - redirect_to(@exercise, alert: t('exercises.implement.unpublished')) if @exercise.unpublished? && current_user.role != 'admin' && current_user.role != 'teacher' # TODO: TESTESTEST - redirect_to(@exercise, alert: t('exercises.implement.no_files')) unless @exercise.files.visible.exists? user_solved_exercise = @exercise.solved_by?(current_user) count_interventions_today = UserExerciseIntervention.where(user: current_user).where('created_at >= ?', Time.zone.now.beginning_of_day).count @@ -434,6 +434,16 @@ class ExercisesController < ApplicationController authorize! end + def not_authorized_for_exercise(_exception) + if %w[implement working_times intervention search reload].include?(action_name) && (current_user.admin? || current_user.teacher?) + redirect_to(@exercise, alert: t('exercises.implement.unpublished')) if @exercise.unpublished? + redirect_to(@exercise, alert: t('exercises.implement.no_files')) unless @exercise.files.visible.exists? + else + render_not_authorized + end + end + private :not_authorized_for_exercise + def set_execution_environments @execution_environments = ExecutionEnvironment.all.order(:name) end diff --git a/app/policies/exercise_policy.rb b/app/policies/exercise_policy.rb index 8963d795..0452ce94 100644 --- a/app/policies/exercise_policy.rb +++ b/app/policies/exercise_policy.rb @@ -30,7 +30,11 @@ class ExercisePolicy < AdminOrAuthorPolicy end %i[implement? working_times? intervention? search? reload?].each do |action| - define_method(action) { everyone } + define_method(action) do + return no_one unless @record.files.visible.exists? + + admin? || teacher_in_study_group? || author? || (everyone && !@record.unpublished?) + end end def submit? diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index ad245b64..dab8b754 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -187,6 +187,17 @@ describe ExercisesController do expect_flash_message(:alert, :'exercises.implement.no_files') expect_redirect(:exercise) end + + context 'with other users accessing an unpublished exercise' do + let(:exercise) { create(:fibonacci, unpublished: true) } + let(:user) { create(:teacher) } + + before { perform_request.call } + + expect_assigns(exercise: :exercise) + expect_flash_message(:alert, :'exercises.implement.unpublished') + expect_redirect(:exercise) + end end describe 'GET #index' do @@ -223,6 +234,8 @@ describe ExercisesController do describe 'GET #reload' do context 'when being anyone' do + let(:exercise) { create(:fibonacci) } + before { get :reload, format: :json, params: {id: exercise.id} } expect_assigns(exercise: :exercise) diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index e890fa7c..3ccfe563 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -110,10 +110,54 @@ describe ExercisePolicy do end end - permissions :implement? do - it 'grants access to anyone' do - %i[admin external_user teacher].each do |factory_name| - expect(policy).to permit(build(factory_name), Exercise.new) + %i[implement? working_times? intervention? search? reload?].each do |action| + permissions(action) do + context 'when the exercise has no visible files' do + let(:exercise) { create(:dummy) } + + it 'does not grant access to anyone' do + %i[admin external_user teacher].each do |factory_name| + expect(policy).not_to permit(build(factory_name), exercise) + end + end + end + + context 'when the exercise has visible files' do + let(:exercise) { create(:fibonacci) } + + it 'grants access to anyone' do + %i[admin external_user teacher].each do |factory_name| + expect(policy).to permit(build(factory_name), exercise) + end + end + end + + context 'when the exercise is published' do + let(:exercise) { create(:fibonacci, unpublished: false) } + + it 'grants access to anyone' do + %i[admin external_user teacher].each do |factory_name| + expect(policy).to permit(build(factory_name), exercise) + end + end + end + + context 'when the exercise is unpublished' do + let(:exercise) { create(:fibonacci, unpublished: true) } + + it 'grants access to admins' do + expect(policy).to permit(build(:admin), exercise) + end + + it 'grants access to the author' do + expect(policy).to permit(exercise.author, exercise) + end + + it 'does not grant access to everyone' do + %i[external_user teacher].each do |factory_name| + expect(policy).not_to permit(build(factory_name), exercise) + end + end end end end