Skip to content

Commit

Permalink
Add setting to enable filtering of new course creation from LTI launch
Browse files Browse the repository at this point in the history
  • Loading branch information
david-yz-liu committed Aug 9, 2024
1 parent 079d434 commit 84027e0
Show file tree
Hide file tree
Showing 10 changed files with 60 additions and 7 deletions.
1 change: 1 addition & 0 deletions Changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 9 additions & 0 deletions app/controllers/lti_deployments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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'))
Expand Down
2 changes: 2 additions & 0 deletions app/lib/lti_config.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
module LtiConfig
end
2 changes: 1 addition & 1 deletion app/views/shared/http_status.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<div id='wrapper'>
<section id='content'>
<h1><%= code + ' ' + HttpStatusHelper::ERROR_CODE[code] %></h1>
<p>Uh oh! <%= message %></p>
<p><%= message %></p>
</section>
</div>
</body>
Expand Down
7 changes: 7 additions & 0 deletions config/dummy_lti_config.rb
Original file line number Diff line number Diff line change
@@ -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
6 changes: 6 additions & 0 deletions config/initializers/config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 7 additions & 0 deletions config/initializers/lti_config.rb
Original file line number Diff line number Diff line change
@@ -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
6 changes: 4 additions & 2 deletions config/settings/development.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/#/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/#/oauth2/token"
unpermitted_new_course_message: 'You are not permitted to create a new MarkUs course for %{course_name}. Please contact your system administrator.'
4 changes: 2 additions & 2 deletions config/settings/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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/#/oauth2/token"
unpermitted_new_course_message: 'You are not permitted to create a new MarkUs course for %{course_name}. Please contact your system administrator.'
23 changes: 21 additions & 2 deletions spec/controllers/lti_deployments_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 84027e0

Please # to comment.