: Difference between revisions

E2057. Time travel Not Allowed..!!! Restrict TAs’ ability to change their own grade + limit file-size upload

This page provides a description of the Expertiza based OSS project.

Team Information

• Palash Gupta - pgupta25@ncsu.edu
• Sneha Kumar - skumar32@ncsu.edu
• Yen-An Jou - yjou@ncsu.edu

Deployment

You can review the deployed link here:

1. Expertiza working solution

Credentials for accessing the student who is also a TA are:

• Username: student12
• Password: testabcd

Issue 1412 - Problem Statement

If a person is listed as a TA in one course and as a student in another course, then if they navigate to the "Your scores" page of one of the assignments in which they are participating as a student, they can see a TA's view of that page - effectively allowing them to assign their own grade!

The below screenshot shows the TA view for the course he is added as the TA:

As evident from the screenshot, the user, "student003" is assigned as a TA for CSC502.

The user, "student003" is also a student in the course, CSC501:

The issue here is that this user, "student003" who is a TA in one course is able to alter the grades for his assignments in other courses he is taking in the semester:

Solutions

Once TA clicks on Assignment > view scores, they will no longer be able to see the form to add/edit the grade and comment for the course in which they are participating as a student.

Files modified: view_team.html

We are rendering the TA view(to grade and comment) only if the TA ID has an entry in the ta_mapping table. This ensures that the TA will be able modify the grades for courses for which they are assigned as TA.

• Get the course ID for the course which the student is currently viewing.
• Get the user ID, which will be the teacher ID as well
• Using these two fields we are restricting the access for the student to modify the grades.

Only for the courses for which a user is a TA, he will be able to see 'TA Grade-Comment:' section under Assignment > view scores

The green-highlighted lines indicate the changes.

Below is the screenshot which indicates the TA view for which the user is registered as a student:

Code Addition

• view_team.html.erb

  @assign_id = Assignment.find_by(id: @participant.parent_id).course_id;
  @teacher_id = @participant.user_id;  

 <% if TaMapping.where(ta_id: @teacher_id, course_id: @assign_id).exists? %>
   <%= form_tag 'save_grade_and_comment_for_submission' do %>
     <%= hidden_field_tag :participant_id, params[:id] %>
     <%= number_field_tag 'grade_for_submission', @team.try(:grade_for_submission) ,min: 0, max: 100, maxlength: 3, size: 3, class: "form-control width-150", placeholder: 'Grade' %>
     <%= text_area_tag 'comment_for_submission', @team.try(:comment_for_submission), size: '75x10', placeholder: 'Comment', class: "form-control width-500" %>
     <%= submit_tag 'Save' ,class: "btn btn-default" %>
   <% end %>
 <% end %>

Test Plan

The test plan is set to have an automated testing to ensure that the application is still working after we added our code. We are using RSpec for automated testing and show you the steps we did to test the UI. In the subsections below you can view the code snippets added to ensure that the TA is not able to change their own grade.

Edge Case Covered

In order to ensure that the TA is blocked from changing his own grade for the courses he/she is a student, we made the grade assigning section hidden.

RSpec

The test case assumes the role of a TA who is also a student in one of the courses. The student is allowed to complete one of the assignments in the course and when the student views the "View scores" page, he is prevented from updating the grades for the assignment.

• grades_controller_spec.rb

 describe '#view_team' do
   render_views
   context 'when view_team page is viewed by a student who is also a TA for another course' do
     it 'renders grades#view_team page' do
       allow(participant).to receive(:team).and_return(team)
       allow(AssignmentQuestionnaire).to receive(:find_by).with(assignment_id: 1, questionnaire_id: 1).and_return(assignment_questionnaire)
       allow(AssignmentQuestionnaire).to receive(:where).with(any_args).and_return([assignment_questionnaire])
       allow(assignment).to receive(:late_policy_id).and_return(false)
       allow(assignment).to receive(:calculate_penalty).and_return(false)
       allow(assignment).to receive(:compute_total_score).with(any_args).and_return(100)
       allow(review_questionnaire).to receive(:get_assessments_round_for).with(participant, 1).and_return([review_response])
       allow(Answer).to receive(:compute_scores).with([review_response], [question]).and_return(max: 95, min: 88, avg: 90)
       params = {id: 1}
       allow(TaMapping).to receive(:exists?).with(ta_id: 1, course_id: 1).and_return(true)
       stub_current_user(ta, ta.role.name, ta.role)
       get :view_team, params
       expect(response.body).not_to have_content "TA"
     end
   end
 end

Issue 1351 - Problem Statement

A student can upload files with their submission. In some cases, students upload long videos that might not be necessary for the submission.

What’s wrong with it: As there is no restriction on the files being uploaded, this is a security issue in Expertiza. Large files should be restricted. A student may also upload malware into the system affecting expertiza

Drawbacks and Solutions

• Problem 1: No file size restriction while uploading files.
• Solution:

  file_size_limit = 5
    
  # check file size
  if !check_content_size(file, file_size_limit)
    flash[:error] = "File size must smaller than #{file_size_limit}MB"
    redirect_to action: 'edit', id: participant.id
    return
  end

  # Verify the size of uploaded file is under specific value.
  # @param file [Object] uploaded file
  # @param size [Integer] maximum size(MB)
  # @return [Boolean] the result of verification
  def check_content_size(file, size)
    return !(file.size > size*1024*1024)
  end

• Problem 2: No file type restriction while uploading files. The existing method get_file_type only checks the extension of the file name. It is easy for users to bypass the validation.

  def get_file_type file_name
    base = File.basename(file_name)
    return base.split(".")[base.split(".").size - 1] if base.split(".").size > 1
  end

• Solution: We use MimeMagic to detect the mime type of a file by its content.

  file_content = file.read

  # check file type
  if !check_content_type_integrity(file_content)
    flash[:error] = 'File type error'
    redirect_to action: 'edit', id: participant.id
    return
  end

  # Verify the integrity of uploaded files.
  # @param file_content [Object] the content of uploaded file
  # @return [Boolean] the result of verification
  def check_content_type_integrity(file_content)
    limited_types = ['application/pdf', 'image/png', 'image/jpeg', 'application/zip', 'application/x-tar', 'application/x-7z-compressed', 'application/vnd.oasis.opendocument.text', 'application/vnd.openxmlformats-officedocument.wordprocessingml.document']
    mime = MimeMagic.by_magic(file_content)
    return limited_types.include? mime.to_s
  end

RSpec test

We modify the original tests and add the following tests to ensure the validation works.

  it "should not submit large file" do
    signup_topic
    # upload file
    file_path = Rails.root + "spec/features/assignment_submission_files/invalid_assignment_file.jpg"
    attach_file('uploaded_file', file_path)
    click_on 'Upload file'
    expect(page).to have_content "File size must smaller than"
  end

  it "should not submit invalid file" do
    signup_topic
    # upload file
    file_path = Rails.root + "spec/features/assignment_submission_files/invalid_assignment_file.txt"
    attach_file('uploaded_file', file_path)
    click_on 'Upload file'
    expect(page).to have_content "File type error"
  end

References

1. Expertiza on GitHub
2. E2057 Pull Request
3. E2057 Repository Fork