Skip to content

Commit

Permalink
Check MIME types and file extension on file submission (#7083)
Browse files Browse the repository at this point in the history
  • Loading branch information
AinaMerch authored Jun 15, 2024
1 parent 400ba2d commit 463c860
Show file tree
Hide file tree
Showing 8 changed files with 60 additions and 0 deletions.
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
- Ensure user params are passed as keyword arguments to database queries (#7040)
- Added a progress bar for when a student uploads a file for submission (#7078)
- Added validations to the `TextAnnotation` model to ensure `line_start` and `line_end` are >= 1, and `column_start` and `column_end` are >= 0. (#7081)
- Added a backend to check MIME type and file extension of uploaded files (#7083)

### 🐛 Bug fixes

Expand Down
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ gem 'config'
gem 'cookies_eu'
gem 'dry-validation' # For settings schema validation
gem 'exception_notification'
gem 'marcel'
gem 'rails-html-sanitizer'
gem 'rails_performance'
gem 'responders'
Expand Down
1 change: 1 addition & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ DEPENDENCIES
jwt
listen
machinist (< 3)
marcel
mini_mime
pg
pluck_to_hash
Expand Down
7 changes: 7 additions & 0 deletions app/controllers/submissions_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,13 @@ def update_files
authorize! to: :manage_subdirectories? # ensure user is authorized for directories in zip files
success, msgs = add_folder(f, current_role, repo, path: path, txn: txn, required_files: required_files)
else
content_type = Marcel::MimeType.for Pathname.new(f)
file_extension = File.extname(f.original_filename).downcase
expected_mime_type = Marcel::MimeType.for extension: file_extension

if content_type != expected_mime_type && content_type != 'application/octet-stream'
flash_message(:warning, I18n.t('student.submission.file_extension_mismatch', extension: file_extension))
end
success, msgs = add_file(f, current_role, repo,
path: path, txn: txn, check_size: true, required_files: required_files)
end
Expand Down
1 change: 1 addition & 0 deletions config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ en:
empty_file_warning: "%{file_name} is empty"
external_submit_only: MarkUs is only accepting external submits
file_conflicts: 'Your changes have not been made. The following file conflicts were detected:'
file_extension_mismatch: The contents of the uploaded file do not match the file extension %{extension}.
file_too_large: The size of the uploaded file %{file_name} exceeds the maximum of %{max_size} MB.
invalid_file_name: 'Invalid file name on submitted file: %{file_name}'
invalid_folder_name: 'Invalid folder name on submitted folder: %{folder_name}'
Expand Down
49 changes: 49 additions & 0 deletions spec/controllers/submissions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,55 @@
end
end

it 'should check for MIME type and extension that match' do
expect(@student).to have_accepted_grouping_for(@assignment.id)

valid_file = fixture_file_upload('docx_file.docx',
'application/vnd.openxmlformats-officedocument.wordprocessingml.document')
content_type = Marcel::MimeType.for Pathname.new(valid_file)
file_extension = File.extname(valid_file.original_filename).downcase
expected_mime_type = Marcel::MimeType.for extension: file_extension

expect(content_type).to eq(expected_mime_type)

post_as @student, :update_files,
params: { course_id: course.id, assignment_id: @assignment.id, new_files: [valid_file] }
expect(response).to have_http_status :ok

# Check to see if the file was added
@grouping.group.access_repo do |repo|
revision = repo.get_latest_revision
files = revision.files_at_path(@assignment.repository_folder)
expect(files['docx_file.docx']).not_to be_nil
end
end

it 'should check for MIME type and extension that do not match' do
expect(@student).to have_accepted_grouping_for(@assignment.id)

invalid_file = fixture_file_upload('docx_renamed_to_pdf.pdf')
content_type = Marcel::MimeType.for(Pathname.new(invalid_file))
file_extension = File.extname(invalid_file.original_filename).downcase
expected_mime_type = Marcel::MimeType.for(extension: file_extension)

expect(content_type).not_to eq(expected_mime_type)

post_as @student, :update_files,
params: { course_id: course.id, assignment_id: @assignment.id, new_files: [invalid_file] }

expect(response).to have_http_status :ok
sample_warning_message = I18n.t('student.submission.file_extension_mismatch', extension: file_extension)
flash[:warning] = I18n.t('student.submission.file_extension_mismatch', extension: file_extension)
expect(flash[:warning]).to eq sample_warning_message

# Check to see if the file was added
@grouping.group.access_repo do |repo|
revision = repo.get_latest_revision
files = revision.files_at_path(@assignment.repository_folder)
expect(files['docx_renamed_to_pdf.pdf']).not_to be_nil
end
end

it 'cannot add files outside the repository when an invalid path is given' do
file = fixture_file_upload('Shapes.java', 'text/java')
bad_path = '../../'
Expand Down
Binary file added spec/fixtures/files/docx_file.docx
Binary file not shown.
Binary file added spec/fixtures/files/docx_renamed_to_pdf.pdf
Binary file not shown.

0 comments on commit 463c860

Please sign in to comment.