api: translate absolute paths to full urls with the canvas host
This fixes image links to /equation_images/X, among other things. As part of this, I refactored the attachment.rb secure setting to be a domain.yml (HostUrl) setting that can be used app-wide to determine whether to use http or https when the code doesn't have access to a Request. Fixes #8784 I also started down the road of having notification emails/sms/etc use https links instead of http, but there is still work to do there, refs #9190 test plan: Use the rich text editor to post to a discussion or any other rich text field that can be retrieved via the api, and include an equation using the equation editor. Then retrieve that post through the api, and verify that the url to the equation image includes the canvas hostname and protocol (http://canvas.example.com/equation_images/X instead of just /equation_images/X) Change-Id: Iac28bf99d2d3b33c17d5b3eb128aa6d8488570fe Reviewed-on: https://gerrit.instructure.com/11867 Tested-by: Jenkins <jenkins@instructure.com> Reviewed-by: Simon Williams <simon@instructure.com>
This commit is contained in:
parent
ddbcc6ac02
commit
70639150ba
|
@ -132,9 +132,8 @@ class AccountAuthorizationConfig < ActiveRecord::Base
|
|||
domains = HostUrl.context_hosts(account, current_host)
|
||||
|
||||
settings = Onelogin::Saml::Settings.new
|
||||
protocol = Rails.env.development? ? 'http' : 'https'
|
||||
settings.sp_slo_url = "#{protocol}://#{domains.first}/saml_logout"
|
||||
settings.assertion_consumer_service_url = domains.map { |domain| "#{protocol}://#{domain}/saml_consume" }
|
||||
settings.sp_slo_url = "#{HostUrl.protocol}://#{domains.first}/saml_logout"
|
||||
settings.assertion_consumer_service_url = domains.map { |domain| "#{HostUrl.protocol}://#{domain}/saml_consume" }
|
||||
settings.tech_contact_name = app_config[:tech_contact_name] || 'Webmaster'
|
||||
settings.tech_contact_email = app_config[:tech_contact_email] || ''
|
||||
|
||||
|
|
|
@ -600,8 +600,6 @@ class Attachment < ActiveRecord::Base
|
|||
# Return existing value, even if nil, as long as it's defined
|
||||
@file_store_config ||= YAML.load_file(RAILS_ROOT + "/config/file_store.yml")[RAILS_ENV] rescue nil
|
||||
@file_store_config ||= { 'storage' => 'local' }
|
||||
# default the secure setting to true only in production
|
||||
@file_store_config['secure'] = Rails.env.production? unless @file_store_config.has_key?('secure')
|
||||
@file_store_config['path_prefix'] ||= @file_store_config['path'] || 'tmp/files'
|
||||
if RAILS_ENV == "test"
|
||||
# yes, a rescue nil; the problem is that in an automated test environment, this may be
|
||||
|
@ -642,8 +640,7 @@ class Attachment < ActiveRecord::Base
|
|||
def authenticated_s3_url(*args)
|
||||
return root_attachment.authenticated_s3_url(*args) if root_attachment
|
||||
protocol = args[0].is_a?(Hash) && args[0][:protocol]
|
||||
protocol ||= self.class.file_store_config['secure'] ? "https://" : "http://"
|
||||
protocol ||= "//"
|
||||
protocol ||= "#{HostUrl.protocol}://"
|
||||
"#{protocol}#{HostUrl.context_host(context)}/#{context_type.underscore.pluralize}/#{context_id}/files/#{id}/download?verifier=#{uuid}"
|
||||
end
|
||||
|
||||
|
|
|
@ -41,6 +41,7 @@ class Message < ActiveRecord::Base
|
|||
validates_length_of :transmission_errors, :maximum => maximum_text_length, :allow_nil => true, :allow_blank => true
|
||||
|
||||
def polymorphic_url_with_context_host(record_or_hash_or_array, options = {})
|
||||
options[:protocol] = HostUrl.protocol
|
||||
if record_or_hash_or_array.is_a? Array
|
||||
options[:host] = HostUrl.context_host(record_or_hash_or_array.first)
|
||||
else
|
||||
|
|
|
@ -6,4 +6,7 @@ development:
|
|||
|
||||
production:
|
||||
domain: "canvas.example.com"
|
||||
# whether this instance of canvas is served over ssl (https) or not
|
||||
# defaults to true for production, false for test/development
|
||||
ssl: true
|
||||
# files_domain: "canvasfiles.example.com"
|
||||
|
|
|
@ -1,6 +1,7 @@
|
|||
development:
|
||||
storage: local
|
||||
secure: false # defaults to true in production, false elsewhere
|
||||
# the secure option in this file is deprecated, use the ssl option in domain.yml instead
|
||||
# secure: false
|
||||
path_prefix: tmp/files
|
||||
|
||||
test:
|
||||
|
|
34
lib/api.rb
34
lib/api.rb
|
@ -201,6 +201,9 @@ module Api
|
|||
# See User.submissions_for_given_assignments and SubmissionsApiController#for_students
|
||||
mattr_accessor :assignment_ids_for_students_api
|
||||
|
||||
# a hash of allowed html attributes that represent urls, like { 'a' => ['href'], 'img' => ['src'] }
|
||||
UrlAttributes = Instructure::SanitizeField::SANITIZE[:protocols].inject({}) { |h,(k,v)| h[k] = v.keys; h }
|
||||
|
||||
def api_user_content(html, context = @context, user = @current_user)
|
||||
return html if html.blank?
|
||||
|
||||
|
@ -208,15 +211,17 @@ module Api
|
|||
# figure out what host is appropriate
|
||||
if self.is_a?(ApplicationController)
|
||||
host = request.host_with_port
|
||||
protocol = request.ssl? ? 'https' : 'http'
|
||||
else
|
||||
host = HostUrl.context_host(context, @account_domain.try(:host))
|
||||
protocol = HostUrl.protocol
|
||||
end
|
||||
|
||||
rewriter = UserContent::HtmlRewriter.new(context, user)
|
||||
rewriter.set_handler('files') do |match|
|
||||
obj = match.obj_class.find_by_id(match.obj_id)
|
||||
next unless obj && rewriter.user_can_view_content?(obj)
|
||||
file_download_url(obj.id, :verifier => obj.uuid, :download => '1', :host => host)
|
||||
file_download_url(obj.id, :verifier => obj.uuid, :download => '1', :host => host, :protocol => protocol)
|
||||
end
|
||||
html = rewriter.translate_content(html)
|
||||
|
||||
|
@ -233,7 +238,7 @@ module Api
|
|||
node['data-media_comment_type'] = 'audio'
|
||||
else
|
||||
node = Nokogiri::XML::Node.new('video', doc)
|
||||
thumbnail = media_object_thumbnail_url(media_id, :width => 550, :height => 448, :type => 3, :host => host)
|
||||
thumbnail = media_object_thumbnail_url(media_id, :width => 550, :height => 448, :type => 3, :host => host, :protocol => protocol)
|
||||
node['poster'] = thumbnail
|
||||
node['data-media_comment_type'] = 'video'
|
||||
end
|
||||
|
@ -241,12 +246,35 @@ module Api
|
|||
node['preload'] = 'none'
|
||||
node['class'] = 'instructure_inline_media_comment'
|
||||
node['data-media_comment_id'] = media_id
|
||||
media_redirect = polymorphic_url([context, :media_download], :entryId => media_id, :type => 'mp4', :redirect => '1', :host => host)
|
||||
media_redirect = polymorphic_url([context, :media_download], :entryId => media_id, :type => 'mp4', :redirect => '1', :host => host, :protocol => protocol)
|
||||
node['controls'] = 'controls'
|
||||
node['src'] = media_redirect
|
||||
anchor.replace(node)
|
||||
end
|
||||
|
||||
# rewrite any html attributes that are urls but just absolute paths, to
|
||||
# have the canvas domain prepended to make them a full url
|
||||
#
|
||||
# relative urls and invalid urls are currently ignored
|
||||
UrlAttributes.each do |tag, attributes|
|
||||
doc.css(tag).each do |element|
|
||||
attributes.each do |attribute|
|
||||
url_str = element[attribute]
|
||||
begin
|
||||
url = URI.parse(url_str)
|
||||
# if the url_str is "//example.com/a", the parsed url will have a host set
|
||||
# otherwise if it starts with a slash, it's a path that needs to be
|
||||
# made absolute with the canvas hostname prepended
|
||||
if !url.host && url_str[0] == '/'[0]
|
||||
element[attribute] = "#{protocol}://#{host}#{url_str}"
|
||||
end
|
||||
rescue URI::InvalidURIError => e
|
||||
# leave it as is
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
return doc.to_s
|
||||
end
|
||||
|
||||
|
|
|
@ -23,19 +23,37 @@ class HostUrl
|
|||
@@default_host = nil
|
||||
@@file_host = nil
|
||||
@@domain_config = nil
|
||||
@@protocol = nil
|
||||
|
||||
def reset_cache!
|
||||
@@default_host = @@file_host = @@domain_config = nil
|
||||
@@default_host = @@file_host = @@domain_config = @@protocol = nil
|
||||
end
|
||||
|
||||
def domain_config
|
||||
if !@@domain_config
|
||||
@@domain_config = File.exist?("#{RAILS_ROOT}/config/domain.yml") && YAML.load_file("#{RAILS_ROOT}/config/domain.yml")[RAILS_ENV].try(:with_indifferent_access)
|
||||
@@domain_config = Setting.from_config("domain")
|
||||
@@domain_config ||= {}
|
||||
end
|
||||
@@domain_config
|
||||
end
|
||||
|
||||
# returns "http" or "https" depending on whether this instance of canvas runs over ssl
|
||||
def protocol
|
||||
if !@@protocol
|
||||
if domain_config.key?('ssl')
|
||||
is_secure = domain_config['ssl']
|
||||
elsif Attachment.file_store_config.key?('secure')
|
||||
is_secure = Attachment.file_store_config['secure']
|
||||
else
|
||||
is_secure = Rails.env.production?
|
||||
end
|
||||
|
||||
@@protocol = is_secure ? "https" : "http"
|
||||
end
|
||||
|
||||
@@protocol
|
||||
end
|
||||
|
||||
def context_host(context=nil, current_host=nil)
|
||||
default_host
|
||||
end
|
||||
|
|
|
@ -117,7 +117,35 @@ describe UserContent, :type => :integration do
|
|||
:format => 'json', :course_id => @course.id.to_s, :id => @assignment.id.to_s })
|
||||
|
||||
doc = Nokogiri::HTML::DocumentFragment.parse(json['description'])
|
||||
doc.at_css('img')['src'].should == "/courses/#{@course.id}/files/#{@attachment.id}/preview"
|
||||
doc.at_css('img')['src'].should == "http://www.example.com/courses/#{@course.id}/files/#{@attachment.id}/preview"
|
||||
end
|
||||
|
||||
it "should prepend the hostname to all absolute-path links" do
|
||||
course_with_teacher(:active_all => true)
|
||||
@assignment = @course.assignments.create!(:title => "first assignment", :description => <<-HTML)
|
||||
<p>
|
||||
Hello, students.<br>
|
||||
<img src='/equation_images/1234'>
|
||||
<a href='/help'>click for teh help</a>
|
||||
<a href='//example.com/quiz'>a quiz</a>
|
||||
<a href='http://example.com/test1'>moar</a>
|
||||
<a href='invalid url'>broke</a>
|
||||
</p>
|
||||
HTML
|
||||
|
||||
json = api_call(:get,
|
||||
"/api/v1/courses/#{@course.id}/assignments/#{@assignment.id}",
|
||||
{ :controller => 'assignments_api', :action => 'show',
|
||||
:format => 'json', :course_id => @course.id.to_s, :id => @assignment.id.to_s })
|
||||
|
||||
doc = Nokogiri::HTML::DocumentFragment.parse(json['description'])
|
||||
doc.at_css('img')['src'].should == "http://www.example.com/equation_images/1234"
|
||||
doc.css('a').map { |e| e['href'] }.should == [
|
||||
"http://www.example.com/help",
|
||||
"//example.com/quiz",
|
||||
"http://example.com/test1",
|
||||
"invalid%20url",
|
||||
]
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -0,0 +1,45 @@
|
|||
#
|
||||
# Copyright (C) 2012 Instructure, Inc.
|
||||
#
|
||||
# This file is part of Canvas.
|
||||
#
|
||||
# Canvas is free software: you can redistribute it and/or modify it under
|
||||
# the terms of the GNU Affero General Public License as published by the Free
|
||||
# Software Foundation, version 3 of the License.
|
||||
#
|
||||
# Canvas is distributed in the hope that it will be useful, but WITHOUT ANY
|
||||
# WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR
|
||||
# A PARTICULAR PURPOSE. See the GNU Affero General Public License for more
|
||||
# details.
|
||||
#
|
||||
# You should have received a copy of the GNU Affero General Public License along
|
||||
# with this program. If not, see <http://www.gnu.org/licenses/>.
|
||||
#
|
||||
|
||||
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
|
||||
|
||||
describe 'HostUrl' do
|
||||
describe "protocol" do
|
||||
it "should return https if domain config says ssl" do
|
||||
Setting.expects(:from_config).with("domain").returns({})
|
||||
Attachment.stubs(:file_store_config).returns({})
|
||||
HostUrl.protocol.should == 'http'
|
||||
HostUrl.reset_cache!
|
||||
Setting.expects(:from_config).with("domain").returns('ssl' => true)
|
||||
HostUrl.protocol.should == 'https'
|
||||
end
|
||||
|
||||
it "should return https if file store config says secure" do
|
||||
Setting.stubs(:from_config).with("domain").returns({})
|
||||
Attachment.stubs(:file_store_config).returns('secure' => true)
|
||||
HostUrl.protocol.should == 'https'
|
||||
end
|
||||
|
||||
it "should return https for production" do
|
||||
HostUrl.protocol.should == 'http'
|
||||
HostUrl.reset_cache!
|
||||
Rails.env.expects(:production?).returns(true)
|
||||
HostUrl.protocol.should == 'https'
|
||||
end
|
||||
end
|
||||
end
|
|
@ -22,6 +22,6 @@ require File.expand_path(File.dirname(__FILE__) + '/messages_helper')
|
|||
describe 'account_user_notification.email' do
|
||||
it "should render" do
|
||||
@object = AccountUser.create(:account => account_model)
|
||||
generate_message(:account_user_notification, :email, @object)
|
||||
msg = generate_message(:account_user_notification, :email, @object)
|
||||
end
|
||||
end
|
||||
|
|
|
@ -17,13 +17,20 @@
|
|||
#
|
||||
|
||||
require File.expand_path(File.dirname(__FILE__) + '/../spec_helper.rb')
|
||||
require File.expand_path(File.dirname(__FILE__) + '/../messages/messages_helper')
|
||||
|
||||
describe Message do
|
||||
|
||||
it "should create a new instance given valid attributes" do
|
||||
message_model
|
||||
describe "parse!" do
|
||||
it "should use https when the domain is configured as ssl" do
|
||||
pending("switch messages to use url writing, rather than hard-coded strings")
|
||||
HostUrl.stubs(:protocol).returns("https")
|
||||
@au = AccountUser.create(:account => account_model)
|
||||
msg = generate_message(:account_user_notification, :email, @au)
|
||||
msg.body.should match(%r{https://www.example.com})
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
context "named scopes" do
|
||||
it "should be able to get messages in any state" do
|
||||
m1 = message_model(:workflow_state => 'bounced', :user => user)
|
||||
|
|
|
@ -140,7 +140,7 @@ module Instructure #:nodoc:
|
|||
],
|
||||
:transformers => lambda { |env| Instructure::SanitizeField.sanitize_style(env) if env[:node]['style'] }
|
||||
}
|
||||
|
||||
|
||||
module ClassMethods
|
||||
|
||||
def sanitize_field(*args)
|
||||
|
|
Loading…
Reference in New Issue