From f49cd0bed48190cfa2e076a4d59cc2a45f1d82f4 Mon Sep 17 00:00:00 2001 From: Karol Date: Wed, 18 Dec 2019 17:52:34 +0100 Subject: [PATCH] forbid users to import an exercise they do not have access to (previously a new one was created) --- app/controllers/exercises_controller.rb | 2 ++ app/errors/proforma/exercise_not_owned.rb | 5 +++++ app/services/proforma_service/import.rb | 8 ++++++-- spec/controllers/exercises_controller_spec.rb | 9 +++++++++ spec/services/proforma_service/import_spec.rb | 4 ++-- 5 files changed, 24 insertions(+), 4 deletions(-) create mode 100644 app/errors/proforma/exercise_not_owned.rb diff --git a/app/controllers/exercises_controller.rb b/app/controllers/exercises_controller.rb index 805c4268..b08cecf3 100644 --- a/app/controllers/exercises_controller.rb +++ b/app/controllers/exercises_controller.rb @@ -188,6 +188,8 @@ class ExercisesController < ApplicationController exercise.save! return render json: {}, status: 201 end + rescue Proforma::ExerciseNotOwned + render json: {}, status: 401 rescue Proforma::ProformaError render json: t('exercises.import_codeharbor.import_errors.invalid'), status: 400 rescue StandardError diff --git a/app/errors/proforma/exercise_not_owned.rb b/app/errors/proforma/exercise_not_owned.rb new file mode 100644 index 00000000..afe96364 --- /dev/null +++ b/app/errors/proforma/exercise_not_owned.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +module Proforma + class ExerciseNotOwned < StandardError; end +end diff --git a/app/services/proforma_service/import.rb b/app/services/proforma_service/import.rb index 9864b31e..9a2e6317 100644 --- a/app/services/proforma_service/import.rb +++ b/app/services/proforma_service/import.rb @@ -28,9 +28,13 @@ module ProformaService def base_exercise exercise = Exercise.find_by(uuid: @task.uuid) - return exercise if exercise && ExercisePolicy.new(@user, exercise).update? + if exercise + raise Proforma::ExerciseNotOwned unless ExercisePolicy.new(@user, exercise).update? - Exercise.new(uuid: @task.uuid, unpublished: true) + exercise + else + Exercise.new(uuid: @task.uuid, unpublished: true) + end end def import_multi diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index ae41de10..7a687e38 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -489,6 +489,15 @@ describe ExercisesController do end end + context 'when import fails with ExerciseNotOwned' do + before { allow(ProformaService::Import).to receive(:call).and_raise(Proforma::ExerciseNotOwned) } + + it 'responds with correct status code' do + post_request + expect(response).to have_http_status(:unauthorized) + end + end + context 'when import fails due to another error' do before { allow(ProformaService::Import).to receive(:call).and_raise(StandardError) } diff --git a/spec/services/proforma_service/import_spec.rb b/spec/services/proforma_service/import_spec.rb index 6f83c4bf..d674a68d 100644 --- a/spec/services/proforma_service/import_spec.rb +++ b/spec/services/proforma_service/import_spec.rb @@ -155,8 +155,8 @@ describe ProformaService::Import do context 'when another user imports the exercise' do let(:import_user) { FactoryBot.create(:teacher) } - it 'raises a validation error' do - expect { imported_exercise.save! } .to raise_error ActiveRecord::RecordInvalid + it 'raises a proforma error' do + expect { imported_exercise.save! } .to raise_error Proforma::ExerciseNotOwned end end end