From 96f5f1f8d7ee769568a433abb95c595666b6e286 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Fri, 26 Apr 2024 09:31:41 +0200 Subject: [PATCH] Enforce valid lis_outcome_service_url Recently, a new institution joined CodeOcean and used a relative URL. This won't work, so that we are rejecting non-absolute URLs by now. --- app/controllers/concerns/lti.rb | 10 ++++++++++ app/controllers/sessions_controller.rb | 3 ++- config/locales/de.yml | 1 + config/locales/en.yml | 1 + spec/controllers/sessions_controller_spec.rb | 10 +++++++++- 5 files changed, 23 insertions(+), 2 deletions(-) diff --git a/app/controllers/concerns/lti.rb b/app/controllers/concerns/lti.rb index f9d1af77..afc33b2f 100644 --- a/app/controllers/concerns/lti.rb +++ b/app/controllers/concerns/lti.rb @@ -85,6 +85,16 @@ module Lti private :require_valid_consumer_key + def require_valid_lis_outcome_service_url + # We want to check that any URL given is absolute, but none URL is fine, too. + return unless params[:lis_outcome_service_url] + + url = URI.parse(params[:lis_outcome_service_url]) + refuse_lti_launch(message: t('sessions.oauth.invalid_lis_outcome_service_url')) unless url.absolute? + end + + private :require_valid_lis_outcome_service_url + def require_valid_exercise_token proxy_exercise = ProxyExercise.find_by(token: params[:custom_token]) @exercise = if proxy_exercise.nil? diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 3b2323c7..e3d0d325 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -4,7 +4,8 @@ class SessionsController < ApplicationController include Lti %i[require_oauth_parameters require_valid_consumer_key require_valid_oauth_signature require_unique_oauth_nonce - set_current_user require_valid_exercise_token set_study_group_membership set_embedding_options].each do |method_name| + require_valid_lis_outcome_service_url set_current_user require_valid_exercise_token set_study_group_membership + set_embedding_options].each do |method_name| before_action(method_name, only: :create_through_lti) end diff --git a/config/locales/de.yml b/config/locales/de.yml index 6e9e41b4..f5f1fac6 100644 --- a/config/locales/de.yml +++ b/config/locales/de.yml @@ -903,6 +903,7 @@ de: failure: Leider ist ein Fehler aufgetreten. invalid_consumer: Ungültiger OAuth-Key. invalid_exercise_token: Ungültiges Aufgaben-Token. + invalid_lis_outcome_service_url: Ungültige URL für den LTI-Ergebnisdienst. invalid_signature: Ungültige OAuth-Signatur. missing_parameters: OAuth-Parameter fehlen. used_nonce: Die Nonce wurde bereits verwendet. diff --git a/config/locales/en.yml b/config/locales/en.yml index 07a045f8..fd902ca6 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -903,6 +903,7 @@ en: failure: 'Sorry, something went wrong.' invalid_consumer: Invalid OAuth key. invalid_exercise_token: Invalid exercise token. + invalid_lis_outcome_service_url: Invalid LTI outcome service URL. invalid_signature: Invalid OAuth signature. missing_parameters: Missing OAuth parameters. used_nonce: Nonce has already been used. diff --git a/spec/controllers/sessions_controller_spec.rb b/spec/controllers/sessions_controller_spec.rb index 5a02a830..f4afc75e 100644 --- a/spec/controllers/sessions_controller_spec.rb +++ b/spec/controllers/sessions_controller_spec.rb @@ -65,6 +65,14 @@ RSpec.describe SessionsController do end end + context 'without a valid absolute LIS Outcome service URL' do + it 'refuses the LTI launch' do + allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) + expect(controller).to receive(:refuse_lti_launch).with(message: I18n.t('sessions.oauth.invalid_lis_outcome_service_url')).and_call_original + post :create_through_lti, params: {oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex, lis_outcome_service_url: '/relative/url'} + end + end + context 'without a valid exercise token' do it 'refuses the LTI launch' do allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) @@ -75,7 +83,7 @@ RSpec.describe SessionsController do context 'with valid launch parameters' do let(:locale) { :de } - let(:perform_request) { post :create_through_lti, params: {custom_locale: locale, custom_token: exercise.token, oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex, user_id: user.external_id} } + let(:perform_request) { post :create_through_lti, params: {custom_locale: locale, custom_token: exercise.token, oauth_consumer_key: consumer.oauth_key, oauth_nonce: nonce, oauth_signature: SecureRandom.hex, user_id: user.external_id, lis_outcome_service_url: 'https://example.org/'} } let(:user) { create(:external_user, consumer:) } before { allow_any_instance_of(IMS::LTI::ToolProvider).to receive(:valid_request?).and_return(true) }