added request validation for urls with and without ports

.gitignore

@@ -11,3 +11,4 @@ Gemfile.lock
 bin
 docs/_build
 *.bak
+*.iml

lib/twilio-ruby/security/request_validator.rb

@@ -25,16 +25,25 @@ def initialize(auth_token = nil)
       #
       # @return [Boolean] whether or not the computed signature matches the signature parameter
       def validate(url, params, signature)
+        parsed_url = URI(url)
+        url_with_port = add_port(parsed_url)
+        url_without_port = remove_port(parsed_url)
+
+        valid_body = true # default succeed, since body not always provided
         params_hash = body_or_hash(params)
-        if params_hash.is_a? Enumerable
-          expected = build_signature_for(url, params_hash)
-          secure_compare(expected, signature)
-        else
-          expected_signature = build_signature_for(url, {})
-          body_hash = URI.decode_www_form(URI(url).query).to_h['bodySHA256']
-          expected_hash = build_hash_for(params)
-          secure_compare(expected_signature, signature) && secure_compare(expected_hash, body_hash)
+        unless params_hash.is_a? Enumerable
+          body_hash = URI.decode_www_form(parsed_url.query).to_h['bodySHA256']
+          params_hash = build_hash_for(params)
+          valid_body = !(params_hash.nil? || body_hash.nil?) && secure_compare(params_hash, body_hash)
+          params_hash = {}
         end
+
+        # Check signature of the url with and without port numbers
+        # since signature generation on the back end is inconsistent
+        valid_signature_with_port = secure_compare(build_signature_for(url_with_port, params_hash), signature)
+        valid_signature_without_port = secure_compare(build_signature_for(url_without_port, params_hash), signature)
+
+        valid_body && (valid_signature_with_port || valid_signature_without_port)
       end
 
       ##
@@ -92,6 +101,50 @@ def body_or_hash(params_or_body)
           params_or_body
         end
       end
+
+      ##
+      # Adds the standard port to the url if it doesn't already have one
+      #
+      # @param [URI] parsed_url The parsed request url
+      #
+      # @return [String] The URL with a port number
+      def add_port(parsed_url)
+        if parsed_url.port.nil? || parsed_url.port == parsed_url.default_port
+          build_url_with_port_for(parsed_url)
+        else
+          parsed_url.to_s
+        end
+      end
+
+      ##
+      # Removes the port from the url
+      #
+      # @param [URI] parsed_url The parsed request url
+      #
+      # @return [String] The URL without a port number
+      def remove_port(parsed_url)
+        parsed_url.port = nil
+        parsed_url.to_s
+      end
+
+      ##
+      # Builds the url from its component pieces, with the standard port
+      #
+      # @param [URI] parsed_url The parsed request url
+      #
+      # @return [String] The URL with the standard port number
+      def build_url_with_port_for(parsed_url)
+        url = ''
+
+        url += parsed_url.scheme ? "#{parsed_url.scheme}://" : ''
+        url += parsed_url.userinfo ? "#{parsed_url.userinfo}@" : ''
+        url += parsed_url.host ? "#{parsed_url.host}:#{parsed_url.port}" : ''
+        url += parsed_url.path
+        url += parsed_url.query ? "?#{parsed_url.query}" : ''
+        url += parsed_url.fragment ? "##{parsed_url.fragment}" : ''
+
+        url
+      end
     end
   end
 end

spec/security/request_validator_spec.rb

@@ -80,5 +80,46 @@
     it 'should fail validation with body but no signature' do
       expect(validator.validate(url, body, default_signature)).to eq(false)
     end
+
+    it 'should validate https urls with ports by stripping them' do
+      url_with_port = url.sub('.com', '.com:1234')
+      expect(validator.validate(url_with_port, params, default_signature)).to eq(true)
+    end
+
+    it 'should validate http urls with ports' do
+      http_url_with_port = url.sub('.com', '.com:1234')
+      http_url_with_port = http_url_with_port.sub('https', 'http')
+      signature = 'Zmvh+3yNM1Phv2jhDCwEM3q5ebU=' # hash of http url with port 1234
+      expect(validator.validate(http_url_with_port, params, signature)).to eq(true)
+    end
+
+    it 'should validate https urls without ports by adding standard port 443' do
+      signature = 'kvajT1Ptam85bY51eRf/AJRuM3w=' # hash of https url with port 443
+      expect(validator.validate(url, params, signature)).to eq(true)
+    end
+
+    it 'should validate http urls without ports by adding standard port 80' do
+      http_url = url.sub('https', 'http')
+      signature = '0ZXoZLH/DfblKGATFgpif+LLRf4=' # hash of http url with port 80
+      expect(validator.validate(http_url, params, signature)).to eq(true)
+    end
+
+    it 'should validate urls with credentials' do
+      url_with_creds = 'https://user:pass@mycompany.com/myapp.php?foo=1&bar=2'
+      signature = 'CukzLTc1tT5dXEDIHm/tKBanW10=' # expected hash of the url
+      expect(validator.validate(url_with_creds, params, signature)).to eq(true)
+    end
+
+    it 'should validate urls with just username' do
+      url_with_creds = 'https://user@mycompany.com/myapp.php?foo=1&bar=2'
+      signature = '2YRLlVAflCqxaNicjMpJcSTgzSs=' # expected hash of the url
+      expect(validator.validate(url_with_creds, params, signature)).to eq(true)
+    end
+
+    it 'should validate urls with credentials by adding port' do
+      url_with_creds = 'https://user:pass@mycompany.com/myapp.php?foo=1&bar=2'
+      signature = 'ZQFR1PTIZXF2MXB8ZnKCvnnA+rI=' # expected hash of the url with port 443
+      expect(validator.validate(url_with_creds, params, signature)).to eq(true)
+    end
   end
 end