Allow external redirect for render host and LTI

We only use the `launch_presentation_return_url` provided to the @provider, in order to prevent using an open redirect.
This commit is contained in:
Sebastian Serth
2022-12-05 22:13:59 +01:00
parent b7a3fd4586
commit 9977e1614b
3 changed files with 7 additions and 6 deletions

View File

@ -110,7 +110,7 @@ class ApplicationController < ActionController::Base
redirect_to :root, alert: message redirect_to :root, alert: message
# Redirect to main domain if the request originated from our render_host # Redirect to main domain if the request originated from our render_host
elsif request.path == '/' && request.host == RENDER_HOST elsif request.path == '/' && request.host == RENDER_HOST
redirect_to Rails.application.config.action_mailer.default_url_options redirect_to Rails.application.config.action_mailer.default_url_options, allow_other_host: true
else else
redirect_back fallback_location: :root, allow_other_host: false, alert: message redirect_back fallback_location: :root, allow_other_host: false, alert: message
end end

View File

@ -124,10 +124,10 @@ module Lti
private :require_valid_oauth_signature private :require_valid_oauth_signature
def return_to_consumer(options = {}) def return_to_consumer(options = {})
consumer_return_url = @provider.try(:launch_presentation_return_url) || params[:launch_presentation_return_url] consumer_return_url = @provider.try(:launch_presentation_return_url)
if consumer_return_url if consumer_return_url
consumer_return_url += "?#{options.to_query}" if options.present? consumer_return_url += "?#{options.to_query}" if options.present?
redirect_to(consumer_return_url) redirect_to(consumer_return_url, allow_other_host: true)
else else
flash[:danger] = options[:lti_errormsg] flash[:danger] = options[:lti_errormsg]
flash[:info] = options[:lti_msg] flash[:info] = options[:lti_msg]

View File

@ -63,17 +63,18 @@ describe Lti do
describe '#return_to_consumer' do describe '#return_to_consumer' do
context 'with a return URL' do context 'with a return URL' do
let(:consumer_return_url) { 'https://example.org' } let(:consumer_return_url) { 'https://example.org' }
let(:provider) { instance_double(IMS::LTI::ToolProvider, launch_presentation_return_url: consumer_return_url) }
before { allow(controller).to receive(:params).and_return(launch_presentation_return_url: consumer_return_url) } before { controller.instance_variable_set(:@provider, provider) }
it 'redirects to the tool consumer' do it 'redirects to the tool consumer' do
expect(controller).to receive(:redirect_to).with(consumer_return_url) expect(controller).to receive(:redirect_to).with(consumer_return_url, allow_other_host: true)
controller.send(:return_to_consumer) controller.send(:return_to_consumer)
end end
it 'passes messages to the consumer' do it 'passes messages to the consumer' do
message = I18n.t('sessions.oauth.failure') message = I18n.t('sessions.oauth.failure')
expect(controller).to receive(:redirect_to).with("#{consumer_return_url}?lti_errorlog=#{CGI.escape(message)}") expect(controller).to receive(:redirect_to).with("#{consumer_return_url}?lti_errorlog=#{CGI.escape(message)}", allow_other_host: true)
controller.send(:return_to_consumer, lti_errorlog: message) controller.send(:return_to_consumer, lti_errorlog: message)
end end
end end