Skip to content

Commit

Permalink
Sanitize LTI deployment course names when creating new courses
Browse files Browse the repository at this point in the history
  • Loading branch information
david-yz-liu committed Aug 9, 2024
1 parent 07012b3 commit d8c6046
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 3 deletions.
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
- Fixed grader view rendering when a pre-defined annotation's content is blank (#7147)
- Fixed AJAX requests in grader view, which were missing CSRF token headers (#7174)
- Use jQuery `.find` method in `ModalMarkus` to guard against potential XSS attack (#7176)
- Sanitize LTI deployment course names when creating new courses (#7151)

### 🔧 Internal changes

Expand Down
3 changes: 2 additions & 1 deletion app/controllers/lti_deployments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,8 @@ def create_course
return
end

new_course = Course.find_or_initialize_by(name: params['name'])
name = params['name'].gsub(/[^a-zA-Z0-9\-_]/, '-') # Sanitize name to comply with Course name validation
new_course = Course.find_or_initialize_by(name: name)
unless new_course.new_record?
flash_message(:error, I18n.t('lti.course_exists'))
redirect_to choose_course_lti_deployment_path
Expand Down
19 changes: 17 additions & 2 deletions spec/controllers/lti_deployments_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,9 @@

describe '#create_course' do
let!(:lti_deployment) { create(:lti_deployment, lms_course_name: 'csc108') }
let(:course_params) { { id: lti_deployment.id, display_name: 'Introduction to Computer Science', name: 'csc108' } }
let(:course_params) do
{ id: lti_deployment.id, display_name: 'Introduction to Computer Science', name: lti_deployment.lms_course_name }
end

context 'as an instructor' do
before do
Expand Down Expand Up @@ -106,7 +108,7 @@

context 'when a course already exists' do
before do
create(:course, display_name: 'Introduction to Computer Science', name: 'csc108')
create(:course, display_name: 'Introduction to Computer Science', name: lti_deployment.lms_course_name)
session[:lti_deployment_id] = lti_deployment.id
end

Expand All @@ -121,6 +123,19 @@
end
end

context 'when the LTI deployment course name has invalid characters' do
let!(:lti_deployment) { create(:lti_deployment, lms_course_name: 'csc108 fall 3000!') }

before do
session[:lti_deployment_id] = lti_deployment.id
end

it 'creates a new course with a sanitized name' do
post_as instructor, :create_course, params: course_params
expect(Course.exists?(name: 'csc108-fall-3000-')).not_to be_nil
end
end

context 'when the course is rejected by the filter' do
# NOTE: the default filter in config/dummy_lti_config.rb only accepts course names starting with 'csc'
let!(:lti_deployment) { create(:lti_deployment, lms_course_name: 'sta130') }
Expand Down

0 comments on commit d8c6046

Please # to comment.