allow non-http developer key redirect URIs

fixes CNVS-30780

Change-Id: I0413e5c8f099bfe63d0bde235e5bef4c30189b5f
Reviewed-on: https://gerrit.instructure.com/88452
Tested-by: Jenkins
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Cody Cutrer <cody@instructure.com>
QA-Review: Cody Cutrer <cody@instructure.com>
This commit is contained in:
Cody Cutrer 2016-08-22 16:05:03 -06:00
parent ef4149106a
commit 42e8ff44eb
4 changed files with 14 additions and 7 deletions

View File

@ -35,7 +35,7 @@ class DeveloperKey < ActiveRecord::Base
before_save :nullify_empty_icon_url before_save :nullify_empty_icon_url
after_save :clear_cache after_save :clear_cache
validates_as_url :redirect_uri validates_as_url :redirect_uri, allowed_schemes: nil
validate :validate_redirect_uris validate :validate_redirect_uris
scope :nondeleted, -> { where("workflow_state<>'deleted'") } scope :nondeleted, -> { where("workflow_state<>'deleted'") }
@ -57,7 +57,7 @@ class DeveloperKey < ActiveRecord::Base
def validate_redirect_uris def validate_redirect_uris
uris = redirect_uris.map do |value| uris = redirect_uris.map do |value|
value, _ = CanvasHttp.validate_url(value) value, _ = CanvasHttp.validate_url(value, allowed_schemes: nil)
value value
end end

View File

@ -48,7 +48,7 @@ module CanvasHttp
loop do loop do
raise(TooManyRedirectsError) if redirect_limit <= 0 raise(TooManyRedirectsError) if redirect_limit <= 0
_, uri = CanvasHttp.validate_url(url_str, last_host, last_scheme) # uses the last host and scheme for relative redirects _, uri = CanvasHttp.validate_url(url_str, host: last_host, scheme: last_scheme) # uses the last host and scheme for relative redirects
http = CanvasHttp.connection_for_uri(uri) http = CanvasHttp.connection_for_uri(uri)
multipart_query = nil multipart_query = nil
@ -93,7 +93,7 @@ module CanvasHttp
end end
# returns [normalized_url_string, URI] if valid, raises otherwise # returns [normalized_url_string, URI] if valid, raises otherwise
def self.validate_url(value, host=nil, scheme=nil) def self.validate_url(value, host: nil, scheme: nil, allowed_schemes: %w{http https})
value = value.strip value = value.strip
raise ArgumentError if value.empty? raise ArgumentError if value.empty?
uri = URI.parse(value) uri = URI.parse(value)
@ -108,7 +108,7 @@ module CanvasHttp
end end
uri = URI.parse(value) # it's still a URI::Generic uri = URI.parse(value) # it's still a URI::Generic
end end
raise ArgumentError unless %w(http https).include?(uri.scheme.downcase) raise ArgumentError if !allowed_schemes.nil? && !allowed_schemes.include?(uri.scheme.downcase)
raise(RelativeUriError) if uri.host.nil? || uri.host.strip.empty? raise(RelativeUriError) if uri.host.nil? || uri.host.strip.empty?
return value, uri return value, uri

View File

@ -21,10 +21,10 @@ module CustomValidations
module ClassMethods module ClassMethods
def validates_as_url(*fields) def validates_as_url(*fields, allowed_schemes: %w{http https})
validates_each(fields, :allow_nil => true) do |record, attr, value| validates_each(fields, :allow_nil => true) do |record, attr, value|
begin begin
value, uri = CanvasHttp.validate_url(value) value, uri = CanvasHttp.validate_url(value, allowed_schemes: allowed_schemes)
record.send("#{attr}=", value) record.send("#{attr}=", value)
rescue URI::Error, ArgumentError rescue URI::Error, ArgumentError

View File

@ -48,6 +48,13 @@ describe DeveloperKey do
end end
end end
it "allows non-http redirect URIs" do
key = DeveloperKey.new
key.redirect_uri = 'tealpass://somewhere.edu/authentication'
key.redirect_uris = ['tealpass://somewhere.edu/authentication']
expect(key).to be_valid
end
describe "#redirect_domain_matches?" do describe "#redirect_domain_matches?" do
it "should match domains exactly, and sub-domains" do it "should match domains exactly, and sub-domains" do
key = DeveloperKey.create!(:redirect_uri => "http://example.com/a/b") key = DeveloperKey.create!(:redirect_uri => "http://example.com/a/b")