Merge branch 'use-atlassian-jwt-gem' into 'master'

Use atlassian-jwt-ruby gem

See merge request gitlab-org/gitlab-ee!15978

Gemfile

@@ -226,6 +226,7 @@ gem 'hipchat', '~> 1.5.0'
 
 # Jira integration
 gem 'jira-ruby', '~> 1.7'
+gem 'atlassian-jwt', '~> 0.2.0'
 
 # Flowdock integration
 gem 'flowdock', '~> 0.7'

Gemfile.lock

@@ -1058,6 +1058,7 @@ DEPENDENCIES
   asciidoctor (~> 2.0.10)
   asciidoctor-include-ext (~> 0.3.1)
   asciidoctor-plantuml (= 0.0.9)
+  atlassian-jwt (~> 0.2.0)
   attr_encrypted (~> 3.1.0)
   awesome_print
   aws-sdk

ee/app/controllers/jira_connect/application_controller.rb

@@ -26,7 +26,7 @@ class JiraConnect::ApplicationController < ApplicationController
     payload, _ = decode_auth_token!
 
     # Make sure `qsh` claim matches the current request
-    render_403 unless payload['qsh'] == Atlassian::Jwt.create_query_string_hash(request.method, request.url, base_uri: jira_connect_base_url)
+    render_403 unless payload['qsh'] == Atlassian::Jwt.create_query_string_hash(request.url, request.method, jira_connect_base_url)
   rescue
     render_403
   end

ee/lib/atlassian/jira_connect/client.rb

@@ -34,10 +34,10 @@ module Atlassian
 
       def jwt_token(http_method, uri)
         claims = Atlassian::Jwt.build_claims(
-          issuer: Atlassian::JiraConnect.app_key,
-          method: http_method,
-          uri: uri,
-          base_uri: @base_uri
+          Atlassian::JiraConnect.app_key,
+          uri,
+          http_method,
+          @base_uri
         )
 
         Atlassian::Jwt.encode(claims, @shared_secret)

ee/lib/atlassian/jwt.rb

-# frozen_string_literal: true
-
-require 'digest'
-
-# This is based on https://bitbucket.org/atlassian/atlassian-jwt-ruby
-# which is unmaintained and incompatible with later versions of jwt-ruby
-
-module Atlassian
-  module Jwt
-    class << self
-      def decode(token, secret, validate = true, options = {})
-        options = { algorithm: 'HS256' }.merge(options)
-        ::JWT.decode(token, secret, validate, options)
-      end
-
-      def encode(payload, secret, algorithm = 'HS256', header_fields = {})
-        ::JWT.encode(payload, secret, algorithm, header_fields)
-      end
-
-      def create_query_string_hash(http_method, uri, base_uri: '')
-        Digest::SHA256.hexdigest(
-          create_canonical_request(http_method, uri, base_uri)
-        )
-      end
-
-      def build_claims(issuer:, method:, uri:, base_uri: '', issued_at: nil, expires: nil, other_claims: {})
-        issued_at ||= Time.now.to_i
-        expires ||= issued_at + 60
-
-        qsh = create_query_string_hash(method, uri, base_uri: base_uri)
-
-        {
-          iss: issuer,
-          iat: issued_at,
-          exp: expires,
-          qsh: qsh
-        }.merge(other_claims)
-      end
-
-      private
-
-      def create_canonical_request(http_method, uri, base_uri)
-        uri = URI.parse(uri) unless uri.is_a?(URI)
-        base_uri = URI.parse(base_uri) unless base_uri.is_a?(URI)
-
-        [
-          http_method.upcase,
-          canonicalize_uri(uri, base_uri),
-          canonicalize_query_string(uri.query)
-        ].join(CANONICAL_QUERY_SEPARATOR)
-      end
-
-      def canonicalize_uri(uri, base_uri)
-        path = uri.path.sub(/^#{base_uri.path}/, '')
-        path = '/' if path.nil? || path.empty?
-        path = '/' + path unless path.start_with? '/'
-        path.chomp!('/') if path.length > 1
-        path.gsub(CANONICAL_QUERY_SEPARATOR, ESCAPED_CANONICAL_QUERY_SEPARATOR)
-      end
-
-      def canonicalize_query_string(query)
-        return '' if query.nil? || query.empty?
-
-        query = CGI.parse(query)
-        query.delete('jwt')
-
-        query.each do |k, v|
-          query[k] = v.map { |a| CGI.escape a }.join(',') if v.is_a?(Array)
-          query[k].gsub!('+', '%20') # Use %20, not CGI.escape default of "+"
-          query[k].gsub!('%7E', '~') # Unescape "~"
-        end
-
-        query = Hash[query.sort]
-        query.map { |k, v| "#{CGI.escape k}=#{v}" }.join('&')
-      end
-    end
-  end
-end

ee/spec/controllers/jira_connect/events_controller_spec.rb

@@ -65,7 +65,7 @@ describe JiraConnect::EventsController do
 
     describe '#uninstalled' do
       let!(:installation) { create(:jira_connect_installation) }
-      let(:qsh) { Atlassian::Jwt.create_query_string_hash('POST', '/events/uninstalled') }
+      let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/events/uninstalled', 'POST', 'https://gitlab.test') }
 
       before do
         request.headers['Authorization'] = "JWT #{auth_token}"

ee/spec/controllers/jira_connect/subscriptions_controller_spec.rb

@@ -56,7 +56,7 @@ describe JiraConnect::SubscriptionsController do
       end
 
       context 'with valid JWT' do
-        let(:qsh) { Atlassian::Jwt.create_query_string_hash('GET', '/subscriptions') }
+        let(:qsh) { Atlassian::Jwt.create_query_string_hash('https://gitlab.test/subscriptions', 'GET', 'https://gitlab.test') }
         let(:jwt) { Atlassian::Jwt.encode({ iss: installation.client_key, qsh: qsh }, installation.shared_secret) }
 
         it 'returns 200' do

ee/spec/lib/atlassian/jira_connect/client_spec.rb

@@ -15,9 +15,9 @@ describe Atlassian::JiraConnect::Client do
     it "calls the API with auth headers" do
       expected_jwt = Atlassian::Jwt.encode(
         Atlassian::Jwt.build_claims(
-          issuer: Atlassian::JiraConnect.app_key,
-          method: 'POST',
-          uri: '/rest/devinfo/0.10/bulk'
+          Atlassian::JiraConnect.app_key,
+          '/rest/devinfo/0.10/bulk',
+          'POST'
         ),
         'sample_secret'
       )

ee/spec/lib/atlassian/jwt_spec.rb

-# frozen_string_literal: true
-
-require 'fast_spec_helper'
-require 'rspec-parameterized'
-require 'timecop'
-
-describe Atlassian::Jwt do
-  describe '#create_query_string_hash' do
-    using RSpec::Parameterized::TableSyntax
-
-    let(:base_uri) { 'https://example.com/-/jira_connect' }
-
-    where(:path, :method, :expected_hash) do
-      '/events/uninstalled'  | 'POST' | '57d5306d4c520456ebb58ac802779232a941e583589354b8a31aa949cdd4c9ae'
-      '/events/uninstalled/' | 'post' | '57d5306d4c520456ebb58ac802779232a941e583589354b8a31aa949cdd4c9ae'
-      '/configuration'       | 'GET'  | 'be30d9dc39ca6a6543a0b05a253ed9aa36d282311af4cecad54b487dffa62769'
-      '/'                    | 'PUT'  | 'c88c7735138a8806c60f95f0d3e133d1d3d313e2a9d590abbb5f898dabad7b62'
-      ''                     | 'PUT'  | 'c88c7735138a8806c60f95f0d3e133d1d3d313e2a9d590abbb5f898dabad7b62'
-    end
-
-    with_them do
-      it 'generates correct hash with base URI' do
-        hash = subject.create_query_string_hash(method, base_uri + path, base_uri: base_uri)
-
-        expect(hash).to eq(expected_hash)
-      end
-
-      it 'generates correct hash with base URI already removed' do
-        hash = subject.create_query_string_hash(method, path)
-
-        expect(hash).to eq(expected_hash)
-      end
-    end
-  end
-
-  describe '#build_claims' do
-    let(:other_options) { {} }
-
-    subject { described_class.build_claims(issuer: 'gitlab', method: 'post', uri: '/rest/devinfo/0.10/bulk', **other_options) }
-
-    it 'sets the iss claim' do
-      expect(subject[:iss]).to eq('gitlab')
-    end
-
-    it 'sets qsh claim based on HTTP method and path' do
-      expect(subject[:qsh]).to eq(described_class.create_query_string_hash('post', '/rest/devinfo/0.10/bulk'))
-    end
-
-    describe 'iat claim' do
-      it 'sets default value to current time' do
-        Timecop.freeze do
-          expect(subject[:iat]).to eq(Time.now.to_i)
-        end
-      end
-
-      context do
-        let(:issued_time) { Time.now + 30.days }
-        let(:other_options) { { issued_at: issued_time.to_i } }
-
-        it 'allows overriding with option' do
-          expect(subject[:iat]).to eq(issued_time.to_i)
-        end
-      end
-    end
-
-    describe 'exp claim' do
-      it 'sets default value to 1 minute from now' do
-        Timecop.freeze do
-          expect(subject[:exp]).to eq(Time.now.to_i + 60)
-        end
-      end
-
-      context do
-        let(:expiry_time) { Time.now + 30.days }
-        let(:other_options) { { expires: expiry_time.to_i } }
-
-        it 'allows overriding with option' do
-          expect(subject[:exp]).to eq(expiry_time.to_i)
-        end
-      end
-    end
-
-    describe 'other claims' do
-      let(:other_options) { { other_claims: { some_claim: 'some_claim_value' } } }
-
-      it 'allows adding of additional claims' do
-        expect(subject[:some_claim]).to eq('some_claim_value')
-      end
-    end
-  end
-end