Merge branch 'master' into as-add-email-to-user-renderer

Author: artursmirnov

lib/travis/api/app/endpoint/authorization.rb

@@ -104,7 +104,7 @@ class Authorization < Endpoint
       #
       # * **redirect_uri**: URI to redirect to after handshake.
       get '/handshake/?:provider?' do
-        method = Travis::Features.enabled_for_all?(:vcs_login) ? :vcs_handshake : :handshake
+        method = org? ? :handshake : :vcs_handshake
         params[:provider] ||= 'github'
 
         send(method) do |user, token, redirect_uri|

lib/travis/api/v3/models/request.rb

@@ -4,7 +4,7 @@ class RequestConfigs < Struct.new(:data)
       def raw_configs
         Array(data[:raw_configs]).map do |attrs|
           raw_config = RequestRawConfig.new(config: attrs[:config])
-          RequestRawConfiguration.new(source: attrs[:source], raw_config: raw_config)
+          RequestRawConfiguration.new(source: attrs[:source], merge_mode: attrs[:mode], raw_config: raw_config)
         end
       end
 

lib/travis/api/v3/renderer/request_raw_configuration.rb

@@ -1,6 +1,6 @@
 module Travis::API::V3
   class Renderer::RequestRawConfiguration < ModelRenderer
-    representation(:minimal, :config, :source)
+    representation(:minimal, :config, :source, :merge_mode)
     representation(:standard, *representations[:minimal])
 
     def config

lib/travis/api/v3/services/requests/create.rb

@@ -5,7 +5,7 @@ class Services::Requests::Create < Service
     private_constant :TIME_FRAME, :LIMIT
 
     result_type :request
-    params "request", "user", :merge_mode, :config, :message, :branch, :sha, :token
+    params "request", "user", :merge_mode, :config, :configs, :message, :branch, :sha, :token
 
     def run
       repository = check_login_and_find(:repository)

spec/unit/endpoint/authorization_spec.rb

@@ -40,6 +40,13 @@
   end
 
   describe "GET /auth/handshake" do
+    before do
+      ENV['TRAVIS_SITE'] = 'org'
+    end
+    after do
+      ENV['TRAVIS_SITE'] = nil
+    end
+    
     describe 'evil hackers messing with the state' do
       it 'does not succeed if state cookie mismatches' do
         Travis.redis.sadd('github:states', 'github-state')
@@ -50,7 +57,7 @@
       end
     end
 
-    describe 'evil hackers messing with redirection' do
+    describe 'On org, evil hackers messing with redirection' do
       before do
         WebMock.stub_request(:post, "https://foobar.com/access_token_path")
           .to_return(status: 200, body: 'access_token=token&token_type=bearer')
@@ -109,6 +116,66 @@
       end
     end
 
+    describe 'On com and enterprise, evil hackers messing with redirection' do
+      before do
+        WebMock.stub_request(:post, "https://foobar.com/access_token_path")
+          .to_return(status: 200, body: 'access_token=token&token_type=bearer')
+
+        WebMock.stub_request(:get, "https://api.github.com/user?per_page=100")
+          .to_return(
+            status: 200,
+            body: JSON.dump(name: 'Piotr Sarnacki', login: 'drogus', gravatar_id: '123', id: 456, foo: 'bar'), headers: {'X-OAuth-Scopes' => 'repo, user, new_scope'}
+          )
+
+        cookie_jar['travis.state-github'] = state
+        Travis.redis.sadd('github:states', state)
+        ENV['TRAVIS_SITE'] = nil
+      end
+
+      after do
+        Travis.redis.srem('github:states', state)
+      end
+
+      context 'when redirect uri is correct' do
+        let(:state) { 'github-state:::https://travis-ci.com/?any=params' }
+
+        it 'it does allow redirect' do
+          response = get "/auth/handshake/github?code=1234&state=#{URI.encode(state)}"
+          expect(response.status).to eq(200)
+        end
+      end
+
+      context 'when redirect uri is not allowed' do
+        let(:state) { 'github-state:::https://dark-corner-of-web.com/' }
+
+        it 'does not allow redirect' do
+          response = get "/auth/handshake/github?code=1234&state=#{URI.encode(state)}"
+          expect(response.status).to eq(401)
+          expect(response.body).to eq("target URI not allowed")
+        end
+      end
+
+      context 'when script tag is injected into redirect uri' do
+        let(:state) { 'github-state:::https://travis-ci.com/<sCrIpt' }
+
+        it 'does not allow redirect' do
+          response = get "/auth/handshake/github?code=1234&state=#{URI.encode(state)}"
+          expect(response.status).to eq(401)
+          expect(response.body).to eq("target URI not allowed")
+        end
+      end
+
+      context 'when onerror tag is injected into redirect uri' do
+        let(:state) { 'github-state:::https://travis-ci.com/<img% src="" onerror="badcode()"' }
+
+        it 'does not allow redirect' do
+          response = get "/auth/handshake/github?code=1234&state=#{URI.encode(state)}"
+          expect(response.status).to eq(401)
+          expect(response.body).to eq("target URI not allowed")
+        end
+      end
+    end
+
     describe 'with insufficient oauth permissions' do
       before do
         Travis.redis.sadd('github:states', 'github-state')

spec/v3/service_index_spec.rb

@@ -77,6 +77,7 @@ module Routes
               "accepted_params" => %w(
                 request.merge_mode
                 request.config
+                request.configs
                 request.message
                 request.branch
                 request.sha

spec/v3/services/request/find_spec.rb

@@ -40,9 +40,9 @@
     let(:one) { request.raw_configs.build(key: '123', config: 'language: ruby') }
     let(:two) { request.raw_configs.build(key: '234', config: 'rvm: 2.5.1') }
 
-    before { request.raw_configurations.create!(raw_config: one, source: '.travis.yml') }
-    before { request.raw_configurations.create!(raw_config: two, source: 'other.yml') }
-    before { request.raw_configurations.create!(raw_config: one, source: '.travis.yml') } # accidental duplicate
+    before { request.raw_configurations.create!(raw_config: one, source: '.travis.yml', merge_mode: 'one') }
+    before { request.raw_configurations.create!(raw_config: two, source: 'other.yml', merge_mode: 'two') }
+    before { request.raw_configurations.create!(raw_config: one, source: '.travis.yml', merge_mode: 'one') } # accidental duplicate
 
     before { get("/v3/repo/#{repo.id}/request/#{request.id}?include=request.raw_configs") }
 
@@ -55,13 +55,15 @@
             '@type' => 'request_raw_configuration',
             '@representation' => 'standard',
             'config' => 'language: ruby',
-            'source' => '.travis.yml'
+            'source' => '.travis.yml',
+            'merge_mode' => 'one',
           },
           {
             '@type' => 'request_raw_configuration',
             '@representation' => 'standard',
             'config' => 'rvm: 2.5.1',
-            'source' => 'other.yml'
+            'source' => 'other.yml',
+            'merge_mode' => 'two',
           }
         ]
       )

spec/v3/services/request/preview_spec.rb

@@ -8,7 +8,7 @@
 
   let(:configs) do
     {
-      raw_configs: [source: 'travis-ci/travis-yml:.travis.yml@ref', config: 'script: true'],
+      raw_configs: [source: 'travis-ci/travis-yml:.travis.yml@ref', config: 'script: true', mode: 'replace'],
       config: { script: ['true'] },
       matrix: [script: ['true']],
       messages: [type: :type, level: :info, key: :key, code: :code, args: { one: 'one' }, src: '.travis.yml', line: 1],
@@ -53,6 +53,7 @@ def parse(str)
           '@type': 'request_raw_configuration',
           '@representation': 'minimal',
           source: 'travis-ci/travis-yml:.travis.yml@ref',
+          merge_mode: 'replace',
           config: 'script: true'
         ],
         request_config: {