diff --git a/Gemfile b/Gemfile index 385f63e4cf8a17816e4d7595bbeb7e7c886d1bf9..0a8b40ec65925324e0da85a3316dd39d80e4c18a 100644 --- a/Gemfile +++ b/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' diff --git a/Gemfile.lock b/Gemfile.lock index 43e0da80a69daccea8a62d00ad69195a43e5787e..87e3581df3fe8e99332f9d82e0df1a00f99a9267 100644 --- a/Gemfile.lock +++ b/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 diff --git a/ee/app/controllers/jira_connect/application_controller.rb b/ee/app/controllers/jira_connect/application_controller.rb index 528cf5a569e2335a21fa35f890a014950d5dc663..20cfcbd57a0a4894419faf0cbdd7bbb620d26129 100644 --- a/ee/app/controllers/jira_connect/application_controller.rb +++ b/ee/app/controllers/jira_connect/application_controller.rb @@ -26,7 +26,7 @@ def verify_qsh_claim! 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 diff --git a/ee/lib/atlassian/jira_connect/client.rb b/ee/lib/atlassian/jira_connect/client.rb index 44f86df322eb70db233f18e965f346c049613e02..0b578c03782bd422648f7774c5fd0ee9c7f44d73 100644 --- a/ee/lib/atlassian/jira_connect/client.rb +++ b/ee/lib/atlassian/jira_connect/client.rb @@ -34,10 +34,10 @@ def store_dev_info(project:, commits: nil, branches: nil, merge_requests: nil) 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) diff --git a/ee/lib/atlassian/jwt.rb b/ee/lib/atlassian/jwt.rb deleted file mode 100644 index 0d63bd8b1be3b4e91d1fbae0d69478518627bccd..0000000000000000000000000000000000000000 --- a/ee/lib/atlassian/jwt.rb +++ /dev/null @@ -1,78 +0,0 @@ -# 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 diff --git a/ee/spec/controllers/jira_connect/events_controller_spec.rb b/ee/spec/controllers/jira_connect/events_controller_spec.rb index 8dc91373dcc42c45f338563052f80e566a8752e2..36941fae2a0df66efc105ad338e3455ccdf3fae4 100644 --- a/ee/spec/controllers/jira_connect/events_controller_spec.rb +++ b/ee/spec/controllers/jira_connect/events_controller_spec.rb @@ -65,7 +65,7 @@ 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}" diff --git a/ee/spec/controllers/jira_connect/subscriptions_controller_spec.rb b/ee/spec/controllers/jira_connect/subscriptions_controller_spec.rb index 8d938c6dcf73a0f4622a3ed7934de02d154ab3be..659b48bbc90b7d14c6c24381fa6de6f659bc62f8 100644 --- a/ee/spec/controllers/jira_connect/subscriptions_controller_spec.rb +++ b/ee/spec/controllers/jira_connect/subscriptions_controller_spec.rb @@ -56,7 +56,7 @@ 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 diff --git a/ee/spec/lib/atlassian/jira_connect/client_spec.rb b/ee/spec/lib/atlassian/jira_connect/client_spec.rb index 8431514347174135a993d744f7e6a7a865af50cb..5a82cbaf67cbac3fd228aaef0835d18689d65795 100644 --- a/ee/spec/lib/atlassian/jira_connect/client_spec.rb +++ b/ee/spec/lib/atlassian/jira_connect/client_spec.rb @@ -15,9 +15,9 @@ 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' ) diff --git a/ee/spec/lib/atlassian/jwt_spec.rb b/ee/spec/lib/atlassian/jwt_spec.rb deleted file mode 100644 index 43ae93d8f5134eacdb76d0a3ec872c11d177a899..0000000000000000000000000000000000000000 --- a/ee/spec/lib/atlassian/jwt_spec.rb +++ /dev/null @@ -1,91 +0,0 @@ -# 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