Merge branch 'security-remove-post-in-jira-oauth' into 'master'

[master] Remove HTTP POST in JIRA oauth endpoint

Closes #364

See merge request gitlab/gitlab-ee!775

ee/app/controllers/oauth/jira/authorizations_controller.rb

@@ -29,13 +29,18 @@ class Oauth::Jira::AuthorizationsController < ApplicationController
 
   # 3. Rewire and adjust access_token request accordingly.
   def access_token
-    auth_params = params
-                    .slice(:code, :client_id, :client_secret)
-                    .merge(grant_type: 'authorization_code', redirect_uri: oauth_jira_callback_url)
+    # We have to modify request.parameters because Doorkeeper::Server reads params from there
+    request.parameters[:redirect_uri] = oauth_jira_callback_url
+
+    begin
+      strategy = Doorkeeper::Server.new(self).token_request('authorization_code')
+      auth_response = strategy.authorize.body
+    rescue Doorkeeper::Errors::DoorkeeperError
+      auth_response = {}
+    end
 
-    auth_response = Gitlab::HTTP.post(oauth_token_url, body: auth_params, allow_local_requests: true)
     token_type, scope, token = auth_response['token_type'], auth_response['scope'], auth_response['access_token']
 
-    render text: "access_token=#{token}&scope=#{scope}&token_type=#{token_type}"
+    render body: "access_token=#{token}&scope=#{scope}&token_type=#{token_type}"
   end
 end

ee/changelogs/unreleased/security-remove-post-in-jira-oauth.yml

+---
+title: Remove HTTP POST in JIRA OAuth access_token endpoint
+merge_request:
+author:
+type: security

ee/spec/controllers/oauth/jira/authorizations_controller_spec.rb

@@ -30,15 +30,9 @@ describe Oauth::Jira::AuthorizationsController do
   end
 
   describe 'POST access_token' do
-    it 'send post call to oauth_token_url with correct params' do
-      expected_auth_params = { 'code' => 'code-123',
-                               'client_id' => 'client-123',
-                               'client_secret' => 'secret-123',
-                               'grant_type' => 'authorization_code',
-                               'redirect_uri' => 'http://test.host/login/oauth/callback' }
-
-      expect(Gitlab::HTTP).to receive(:post).with(oauth_token_url, allow_local_requests: true, body: ActionController::Parameters.new(expected_auth_params)) do
-        { 'access_token' => 'fake-123', 'scope' => 'foo', 'token_type' => 'bar' }
+    it 'returns oauth params in a format JIRA expects' do
+      expect_any_instance_of(Doorkeeper::Request::AuthorizationCode).to receive(:authorize) do
+        double(body: { 'access_token' => 'fake-123', 'scope' => 'foo', 'token_type' => 'bar' })
       end
 
       post :access_token, params: { code: 'code-123', client_id: 'client-123', client_secret: 'secret-123' }

ee/spec/requests/jira_authorizations_spec.rb

+require 'spec_helper'
+
+describe 'JIRA authorization requests' do
+  let(:user) { create :user }
+  let(:application) { create :oauth_application, scopes: 'api' }
+  let(:redirect_uri) { oauth_jira_callback_url(host: "http://www.example.com") }
+
+  def generate_access_grant
+    create :oauth_access_grant, application: application, resource_owner_id: user.id, redirect_uri: redirect_uri
+  end
+
+  describe 'POST access_token' do
+    it 'should return values similar to a POST to /oauth/token' do
+      post_data = {
+        client_id: application.uid,
+        client_secret: application.secret
+      }
+
+      post '/oauth/token', params: post_data.merge({
+        code: generate_access_grant.token,
+        grant_type: 'authorization_code',
+        redirect_uri: redirect_uri
+      })
+      oauth_response = json_response
+
+      post '/login/oauth/access_token', params: post_data.merge({
+        code: generate_access_grant.token
+      })
+      jira_response = response.body
+
+      access_token, scope, token_type = oauth_response.values_at('access_token', 'scope', 'token_type')
+      expect(jira_response).to eq("access_token=#{access_token}&scope=#{scope}&token_type=#{token_type}")
+    end
+  end
+end