From 22cd202e9d9b92607d51dc8d5aea029a8e521e74 Mon Sep 17 00:00:00 2001
From: Sebastian Serth <Sebastian.Serth@hpi.de>
Date: Sun, 4 Sep 2022 11:42:36 +0200
Subject: [PATCH] Refactor reject_illegal_file_attributes check

* Improve readability of method
* Add a new check for the author of a submission
---
 app/controllers/concerns/file_parameters.rb |  8 +++++++-
 spec/concerns/file_parameters_spec.rb       | 17 +++++++++++++++--
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/app/controllers/concerns/file_parameters.rb b/app/controllers/concerns/file_parameters.rb
index dfe4b06b..5c4f48f8 100644
--- a/app/controllers/concerns/file_parameters.rb
+++ b/app/controllers/concerns/file_parameters.rb
@@ -5,8 +5,14 @@ module FileParameters
     if exercise && params
       params.reject do |_, file_attributes|
         file = CodeOcean::File.find_by(id: file_attributes[:file_id])
+        next true if file.nil? || file.hidden || file.read_only
         # avoid that public files from other contexts can be created
-        file.nil? || file.hidden || file.read_only || (file.context_type == 'Exercise' && file.context_id != exercise.id) || (file.context_type == 'CommunitySolution' && controller_name != 'community_solutions')
+        # `next` is similar to an early return and will proceed with the next iteration of the loop
+        next true if file.context_type == 'Exercise' && file.context_id != exercise.id
+        next true if file.context_type == 'Submission' && file.context.user != current_user
+        next true if file.context_type == 'CommunitySolution' && controller_name != 'community_solutions'
+
+        false
       end
     else
       []
diff --git a/spec/concerns/file_parameters_spec.rb b/spec/concerns/file_parameters_spec.rb
index c7470689..1a4bc595 100644
--- a/spec/concerns/file_parameters_spec.rb
+++ b/spec/concerns/file_parameters_spec.rb
@@ -25,6 +25,8 @@ describe FileParameters do
 
       it 'new file' do
         submission = create(:submission, exercise: hello_world, id: 1337)
+        controller.instance_variable_set(:@current_user, submission.user)
+
         new_file = create(:file, context: submission)
         expect(file_accepted?(new_file)).to be true
       end
@@ -42,16 +44,27 @@ describe FileParameters do
         expect(file_accepted?(hidden_file)).to be false
       end
 
-      it 'read only file' do
+      it 'read-only file' do
         read_only_file = create(:file, context: hello_world, read_only: true)
         expect(file_accepted?(read_only_file)).to be false
       end
 
-      it 'non existent file' do
+      it 'non-existent file' do
         # Ensure to use an invalid id for the file.
         non_existent_file = build(:file, context: hello_world, id: -1)
         expect(file_accepted?(non_existent_file)).to be false
       end
+
+      it 'file of another submission' do
+        learner1 = create(:learner)
+        learner2 = create(:learner)
+        submission_learner1 = create(:submission, exercise: hello_world, user: learner1)
+        _submission_learner2 = create(:submission, exercise: hello_world, user: learner2)
+
+        controller.instance_variable_set(:@current_user, learner2)
+        other_submissions_file = create(:file, context: submission_learner1)
+        expect(file_accepted?(other_submissions_file)).to be false
+      end
     end
   end
 end