From 84027e073fce0183acc9ff68fbc655d092abe2dc Mon Sep 17 00:00:00 2001 From: david-yz-liu Date: Fri, 2 Aug 2024 09:49:48 -0400 Subject: [PATCH] Add setting to enable filtering of new course creation from LTI launch --- Changelog.md | 1 + app/controllers/lti_deployments_controller.rb | 9 ++++++++ app/lib/lti_config.rb | 2 ++ app/views/shared/http_status.html.erb | 2 +- config/dummy_lti_config.rb | 7 ++++++ config/initializers/config.rb | 6 +++++ config/initializers/lti_config.rb | 7 ++++++ config/settings/development.yml | 6 +++-- config/settings/test.yml | 4 ++-- .../lti_deployments_controller_spec.rb | 23 +++++++++++++++++-- 10 files changed, 60 insertions(+), 7 deletions(-) create mode 100644 app/lib/lti_config.rb create mode 100644 config/dummy_lti_config.rb create mode 100644 config/initializers/lti_config.rb diff --git a/Changelog.md b/Changelog.md index d6650cc626..c7a3eb3017 100644 --- a/Changelog.md +++ b/Changelog.md @@ -22,6 +22,7 @@ - Added validations to the `TextAnnotation` model to ensure `line_start` and `line_end` are >= 1, and `column_start` and `column_end` are >= 0. (#7081) - Calculate and display the exact future time for the next token generation to help students plan their test runs more effectively. (#7127) - Set the default date for "Tokens available on" to the current time (#7173). +- Add setting to enable filtering of new course creation from LTI launch (#7151) ### 🐛 Bug fixes diff --git a/app/controllers/lti_deployments_controller.rb b/app/controllers/lti_deployments_controller.rb index dd3c4c03e4..5e033e8df0 100644 --- a/app/controllers/lti_deployments_controller.rb +++ b/app/controllers/lti_deployments_controller.rb @@ -180,6 +180,15 @@ def check_host end def create_course + if LtiConfig.respond_to?(:allowed_to_create_course?) && !LtiConfig.allowed_to_create_course?(record) + render 'shared/http_status', + locals: { code: '422', + message: format(Settings.lti.unpermitted_new_course_message, + course_name: record.lms_course_name) }, + status: :unprocessable_entity, layout: false + return + end + new_course = Course.find_or_initialize_by(name: params['name']) unless new_course.new_record? flash_message(:error, I18n.t('lti.course_exists')) diff --git a/app/lib/lti_config.rb b/app/lib/lti_config.rb new file mode 100644 index 0000000000..2fb1625fb5 --- /dev/null +++ b/app/lib/lti_config.rb @@ -0,0 +1,2 @@ +module LtiConfig +end diff --git a/app/views/shared/http_status.html.erb b/app/views/shared/http_status.html.erb index 81019758b7..94fe72a039 100644 --- a/app/views/shared/http_status.html.erb +++ b/app/views/shared/http_status.html.erb @@ -13,7 +13,7 @@

<%= code + ' ' + HttpStatusHelper::ERROR_CODE[code] %>

-

Uh oh! <%= message %>

+

<%= message %>

diff --git a/config/dummy_lti_config.rb b/config/dummy_lti_config.rb new file mode 100644 index 0000000000..7a836be33a --- /dev/null +++ b/config/dummy_lti_config.rb @@ -0,0 +1,7 @@ +module LtiConfig + # Implement LtiConfig.allowed_to_create_course? to set a filter for LTI deployments that are + # permitted to trigger course creation for MarkUs. + def self.allowed_to_create_course?(lti_deployment) + lti_deployment.lms_course_name.start_with? 'csc' + end +end diff --git a/config/initializers/config.rb b/config/initializers/config.rb index f0b5b69bcb..0fb7acc8f9 100644 --- a/config/initializers/config.rb +++ b/config/initializers/config.rb @@ -188,6 +188,12 @@ optional(:lti).filled(:string) optional(:repos).filled(:string) end + optional(:lti).hash do + optional(:course_filter_file).filled(:string) + optional(:domains).array(:str?) + required(:token_endpoint).filled(:string) + optional(:unpermitted_new_course_message).filled(:string) + end end end end diff --git a/config/initializers/lti_config.rb b/config/initializers/lti_config.rb new file mode 100644 index 0000000000..c562f9d7e1 --- /dev/null +++ b/config/initializers/lti_config.rb @@ -0,0 +1,7 @@ +if Settings.lti.course_filter_file.present? + # config.to_prepare is used so that the block is executed on reloads. + # See https://guides.rubyonrails.org/autoloading_and_reloading_constants.html#autoload-on-boot-and-on-each-reload + Rails.application.config.to_prepare do + load Settings.lti.course_filter_file + end +end diff --git a/config/settings/development.yml b/config/settings/development.yml index 64d6953c2b..dee3d11c08 100644 --- a/config/settings/development.yml +++ b/config/settings/development.yml @@ -41,5 +41,7 @@ autotest: # The settings below are for an experimental feature that is not available # in production yet. Please disregard for now. lti: - domains: <%= %w[host.docker.internal] %> - token_endpoint: "http://host.docker.internal:3100/login/oauth2/token" + course_filter_file: <%= "#{::Rails.root}/config/dummy_lti_config.rb" %> + domains: <%= %w[host.docker.internal localhost] %> + token_endpoint: "http://host.docker.internal:80/login/oauth2/token" + unpermitted_new_course_message: 'You are not permitted to create a new MarkUs course for %{course_name}. Please contact your system administrator.' diff --git a/config/settings/test.yml b/config/settings/test.yml index e3bdcf720e..c2fb4f99f5 100644 --- a/config/settings/test.yml +++ b/config/settings/test.yml @@ -31,8 +31,8 @@ logging: autotest: max_batch_size: 10 -# The settings below are for an experimental feature that is not available -# in production yet. Please disregard for now. lti: + course_filter_file: <%= "#{::Rails.root}/config/dummy_lti_config.rb" %> domains: <%= %w[test.host] %> token_endpoint: "http://test.host.com/login/oauth2/token" + unpermitted_new_course_message: 'You are not permitted to create a new MarkUs course for %{course_name}. Please contact your system administrator.' diff --git a/spec/controllers/lti_deployments_controller_spec.rb b/spec/controllers/lti_deployments_controller_spec.rb index 3608509404..1e71bc6a5e 100644 --- a/spec/controllers/lti_deployments_controller_spec.rb +++ b/spec/controllers/lti_deployments_controller_spec.rb @@ -62,8 +62,8 @@ end end - describe '#new_course' do - let!(:lti_deployment) { create(:lti_deployment) } + 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' } } context 'as an instructor' do @@ -120,6 +120,25 @@ expect(response).to have_http_status(:found) 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') } + + before do + session[:lti_deployment_id] = lti_deployment.id + end + + it 'does not create a new course' do + post_as instructor, :create_course, params: course_params + expect(Course.count).to eq(1) + end + + it 'responds with an error message' do + post_as instructor, :create_course, params: course_params + expect(response).to have_http_status(:unprocessable_entity) + end + end end describe '#public_jwk' do