: Difference between revisions
No edit summary |
|||
(20 intermediate revisions by 3 users not shown) | |||
Line 5: | Line 5: | ||
__TOC__ | __TOC__ | ||
===Team Information=== | |||
* Palash Gupta - pgupta25@ncsu.edu | |||
* Sneha Kumar - skumar32@ncsu.edu | |||
* Yen-An Jou - yjou@ncsu.edu | |||
===Problem Statement=== | ===Deployment=== | ||
You can review the deployed link here: | |||
#[http://152.7.99.140:8080/ 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! | 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: | '''The below screenshot shows the TA view for the course he is added as the TA:''' | ||
[[File:E2057_TA_view.png|850px]] | [[File:E2057_TA_view.png|850px]] | ||
Line 16: | Line 29: | ||
As evident from the screenshot, the user, "student003" is assigned as a TA for CSC502. | 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 user, "student003" is also a student in the course, CSC501:''' | |||
[[File:E2057_view_student.png|850px]] | [[File:E2057_view_student.png|850px]] | ||
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: | |||
'''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:''' | |||
[[File:E2057_edit_grade.png|850px]] | [[File:E2057_edit_grade.png|850px]] | ||
Line 31: | Line 46: | ||
Files modified: view_team.html | 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. | 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 course ID for the course which the student is currently viewing. | ||
* Get the user ID, which will be teacher ID as well | * 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. | * 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 | 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 | ||
[[File:E2057_view_team.png|850px]] | [[File:E2057_view_team.png|850px]] | ||
The green-highlighted lines indicate the changes. | 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: | |||
[[File:E2057_student_view_fix.png|850px]] | [[File:E2057_student_view_fix.png|850px]] | ||
=====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''': | |||
<pre> | |||
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 | |||
</pre> | |||
<pre> | |||
# 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 | |||
</pre> | |||
* '''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. | |||
<pre> | |||
def get_file_type file_name | |||
base = File.basename(file_name) | |||
return base.split(".")[base.split(".").size - 1] if base.split(".").size > 1 | |||
end | |||
</pre> | |||
* '''Solution''': We use MimeMagic to detect the mime type of a file by its content. | |||
<pre> | |||
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 | |||
</pre> | |||
<pre> | |||
# 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 | |||
</pre> | |||
====Test Plan==== | |||
Since the existing tests didn't cover the type and size validation while submitting the files. Our plan was to write the validation tests to cover these cases using RSpec. | |||
=====Edge Case Covered===== | |||
In order to ensure the functionality of validation, we carefully developed our code and used MimeMagic to detect the MIME type of submission. It prevents someone from uploading a fake file. | |||
=====RSpec test===== | |||
We modify the original tests and add the following tests to ensure the validation works. | |||
<pre> | |||
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 | |||
</pre> | |||
<pre> | |||
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 | |||
</pre> | |||
===References=== | |||
#[https://github.com/expertiza/expertiza Expertiza on GitHub] | |||
#[https://github.com/expertiza/expertiza/pull/1793 E2057 Pull Request] | |||
#[https://github.com/palash03/expertiza E2057 Repository Fork] |
Latest revision as of 21:27, 5 November 2020
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:
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
Test Plan
Since the existing tests didn't cover the type and size validation while submitting the files. Our plan was to write the validation tests to cover these cases using RSpec.
Edge Case Covered
In order to ensure the functionality of validation, we carefully developed our code and used MimeMagic to detect the MIME type of submission. It prevents someone from uploading a fake file.
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