Merge branch '323487-add-referrer-property-to-jira-links-inside-gitlab' into 'master'

Add Atlassian referrer to Jira links inside GitLab

See merge request gitlab-org/gitlab!63900

app/models/integrations/jira.rb

@@ -12,6 +12,9 @@ module Integrations
     PROJECTS_PER_PAGE = 50
     JIRA_CLOUD_HOST = '.atlassian.net'
 
+    ATLASSIAN_REFERRER_GITLAB_COM = { atlOrigin: 'eyJpIjoiY2QyZTJiZDRkNGZhNGZlMWI3NzRkNTBmZmVlNzNiZTkiLCJwIjoianN3LWdpdGxhYi1pbnQifQ' }.freeze
+    ATLASSIAN_REFERRER_SELF_MANAGED = { atlOrigin: 'eyJpIjoiYjM0MTA4MzUyYTYxNDVkY2IwMzVjOGQ3ZWQ3NzMwM2QiLCJwIjoianN3LWdpdGxhYlNNLWludCJ9' }.freeze
+
     validates :url, public_url: true, presence: true, if: :activated?
     validates :api_url, public_url: true, allow_blank: true
     validates :username, presence: true, if: :activated?
@@ -38,8 +41,6 @@ module Integrations
       all_details: 2
     }
 
-    alias_method :project_url, :url
-
     # When these are false GitLab does not create cross reference
     # comments on Jira except when an issue gets transitioned.
     def self.supported_events
@@ -82,8 +83,8 @@ module Integrations
       {
         username: username&.strip,
         password: password,
-        site: URI.join(url, '/').to_s, # Intended to find the root
-        context_path: url.path,
+        site: URI.join(url, '/').to_s.delete_suffix('/'), # Intended to find the root
+        context_path: (url.path.presence || '/').delete_suffix('/'),
         auth_type: :basic,
         read_timeout: 120,
         use_cookies: true,
@@ -153,12 +154,37 @@ module Integrations
       ]
     end
 
+    def web_url(path = nil, **params)
+      return unless url.present?
+
+      if Gitlab.com?
+        params.merge!(ATLASSIAN_REFERRER_GITLAB_COM) unless Gitlab.staging?
+      else
+        params.merge!(ATLASSIAN_REFERRER_SELF_MANAGED) unless Gitlab.dev_or_test_env?
+      end
+
+      url = Addressable::URI.parse(self.url)
+      url.path = url.path.delete_suffix('/')
+      url.path << "/#{path.delete_prefix('/').delete_suffix('/')}" if path.present?
+      url.query_values = (url.query_values || {}).merge(params)
+      url.query_values = nil if url.query_values.empty?
+
+      url.to_s
+    end
+
+    override :project_url
+    def project_url
+      web_url
+    end
+
+    override :issues_url
     def issues_url
-      "#{url}/browse/:id"
+      web_url('browse/:id')
     end
 
+    override :new_issue_url
     def new_issue_url
-      "#{url}/secure/CreateIssue!default.jspa"
+      web_url('secure/CreateIssue!default.jspa')
     end
 
     alias_method :original_url, :url

ee/app/models/ee/integrations/jira.rb

@@ -38,7 +38,11 @@ module EE
       def new_issue_url_with_predefined_fields(summary, description)
         escaped_summary = CGI.escape(summary)
         escaped_description = CGI.escape(description)
-        "#{url}/secure/CreateIssueDetails!init.jspa?pid=#{jira_project_id}&issuetype=#{vulnerabilities_issuetype}&summary=#{escaped_summary}&description=#{escaped_description}"[0..MAX_URL_LENGTH]
+
+        # Put summary and description at the end of the URL in case we need to trim it
+        web_url('secure/CreateIssueDetails!init.jspa', pid: jira_project_id, issuetype: vulnerabilities_issuetype)
+          .concat("&summary=#{escaped_summary}&description=#{escaped_description}")
+          .slice(0..MAX_URL_LENGTH)
       end
 
       def create_issue(summary, description, current_user)

ee/app/serializers/integrations/jira_serializers/issue_entity.rb

@@ -55,7 +55,7 @@ module Integrations
       end
 
       expose :web_url do |jira_issue|
-        "#{base_web_url}/browse/#{jira_issue.key}"
+        project.jira_service.issue_url(jira_issue.key)
       end
 
       expose :gitlab_web_url do |jira_issue|
@@ -87,16 +87,12 @@ module Integrations
         # accountId is only available on Jira Cloud.
         # https://community.atlassian.com/t5/Jira-Questions/How-to-find-account-id-on-jira-on-premise/qaq-p/1168652
         if user['accountId'].present?
-          "#{base_web_url}/people/#{user['accountId']}"
+          project.jira_service.web_url("people/#{user['accountId']}")
         else
-          "#{base_web_url}/secure/ViewProfile.jspa?name=#{user['name']}"
+          project.jira_service.web_url('secure/ViewProfile.jspa', name: user['name'])
         end
       end
 
-      def base_web_url
-        @base_web_url ||= project.jira_service.url
-      end
-
       def project
         @project ||= options[:project]
       end

ee/lib/banzai/filter/jira_private_image_link_filter.rb

@@ -14,7 +14,7 @@ module Banzai
         doc.xpath(XPATH).each do |img|
           next unless img['src'].start_with?(PRIVATE_IMAGE_PATH)
 
-          img_link = "#{project.jira_service.url}#{img['src']}"
+          img_link = project.jira_service.web_url(img['src'])
           link = "<a class=\"#{CSS_WITH_ATTACHMENT_ICON}\" href=\"#{img_link}\">#{img_link}</a>"
 
           img.replace(link)

ee/spec/features/integrations/jira/jira_issues_list_spec.rb

@@ -19,7 +19,7 @@ RSpec.describe 'Jira issues list' do
       stub_licensed_features(jira_issues_integration: false)
     end
 
-    it 'renders "Create new issue" button' do
+    it 'does not render "Create new issue" button' do
       visit project_integrations_jira_issues_path(project)
 
       expect(page).to have_gitlab_http_status(:not_found)
@@ -30,6 +30,21 @@ RSpec.describe 'Jira issues list' do
   it 'renders "Create new issue" button', :js do
     visit project_integrations_jira_issues_path(project)
 
-    expect(page).to have_link('Create new issue in Jira')
+    expect(page).to have_link('Create new issue in Jira', href: "#{jira_integration.url}/secure/CreateIssue!default.jspa")
+  end
+
+  context 'on gitlab.com' do
+    before do
+      allow(Gitlab).to receive(:com?).and_return(true)
+    end
+
+    it 'includes the Atlassian referrer in Jira links', :js do
+      visit project_integrations_jira_issues_path(project)
+
+      referrer = Integrations::Jira::ATLASSIAN_REFERRER_GITLAB_COM.to_query
+
+      expect(page).to have_link('Open Jira', href: "#{jira_integration.url}?#{referrer}")
+      expect(page).to have_link('Create new issue in Jira', href: "#{jira_integration.url}/secure/CreateIssue!default.jspa?#{referrer}"), count: 2
+    end
   end
 end

ee/spec/lib/banzai/filter/jira_private_image_link_filter_spec.rb

@@ -11,12 +11,18 @@ RSpec.describe Banzai::Filter::JiraPrivateImageLinkFilter do
 
   context 'with a Jira private image' do
     let(:img_link) { '/secure/attachment/10017/10017_jira-logo.jpg' }
+    let(:doc) { filter("<img src=\"#{img_link}\">", context) }
 
     it 'replaces the Jira private images with the link to the image' do
-      doc = filter("<img src=\"#{img_link}\">", context)
-
       expect(doc.to_html).to eq("<a class=\"with-attachment-icon\" href=\"#{jira_service.url}#{img_link}\">#{jira_service.url}#{img_link}</a>")
     end
+
+    it 'includes the Atlassian referrer on gitlab.com' do
+      allow(Gitlab).to receive(:com?).and_return(true)
+      referrer = Integrations::Jira::ATLASSIAN_REFERRER_GITLAB_COM.to_query
+
+      expect(doc.to_html).to include("?#{referrer}")
+    end
   end
 
   context 'with other image' do

ee/spec/models/ee/integrations/jira_spec.rb

@@ -366,7 +366,7 @@ RSpec.describe Integrations::Jira do
       allow(jira_service).to receive(:vulnerabilities_issuetype).and_return('10001')
     end
 
-    let(:expected_new_issue_url) { "#{jira_service.url}/secure/CreateIssueDetails!init.jspa?pid=11223&issuetype=10001&summary=Special+Summary%21%3F&description=%2AID%2A%3A+2%0A_Issue_%3A+%21" }
+    let(:expected_new_issue_url) { "#{jira_service.url}/secure/CreateIssueDetails!init.jspa?issuetype=10001&pid=11223&summary=Special+Summary%21%3F&description=%2AID%2A%3A+2%0A_Issue_%3A+%21" }
 
     subject(:new_issue_url) { jira_service.new_issue_url_with_predefined_fields("Special Summary!?", "*ID*: 2\n_Issue_: !") }
 

ee/spec/serializers/integrations/jira_serializers/issue_detail_entity_spec.rb

@@ -122,9 +122,9 @@ RSpec.describe Integrations::JiraSerializers::IssueDetailEntity do
 
   context 'with Jira Server configuration' do
     it 'returns the Jira Server profile URL' do
-      expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter@reporter.com')
-      expect(subject[:assignees].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=assignee@assignee.com')
-      expect(subject[:comments].first[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=comment@author.com')
+      expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter%40reporter.com')
+      expect(subject[:assignees].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=assignee%40assignee.com')
+      expect(subject[:comments].first[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=comment%40author.com')
     end
 
     context 'with only url' do
@@ -134,7 +134,7 @@ RSpec.describe Integrations::JiraSerializers::IssueDetailEntity do
       end
 
       it 'returns URLs with the web url' do
-        expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter@reporter.com')
+        expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter%40reporter.com')
         expect(subject[:web_url]).to eq('http://jira.com/browse/GL-5')
       end
     end

ee/spec/serializers/integrations/jira_serializers/issue_entity_spec.rb

@@ -78,8 +78,17 @@ RSpec.describe Integrations::JiraSerializers::IssueEntity do
 
   context 'with Jira Server configuration' do
     it 'returns the Jira Server profile URL' do
-      expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter@reporter.com')
-      expect(subject[:assignees].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=assignee@assignee.com')
+      expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter%40reporter.com')
+      expect(subject[:assignees].first).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=assignee%40assignee.com')
+    end
+
+    it 'includes the Atlassian referrer on gitlab.com' do
+      allow(Gitlab).to receive(:com?).and_return(true)
+      referrer = Integrations::Jira::ATLASSIAN_REFERRER_GITLAB_COM.to_query
+
+      expect(subject[:web_url]).to eq("http://jira.com/browse/GL-5?#{referrer}")
+      expect(subject[:author]).to include(web_url: "http://jira.com/secure/ViewProfile.jspa?#{referrer}&name=reporter%40reporter.com")
+      expect(subject[:assignees].first).to include(web_url: "http://jira.com/secure/ViewProfile.jspa?#{referrer}&name=assignee%40assignee.com")
     end
 
     context 'with only url' do
@@ -89,7 +98,7 @@ RSpec.describe Integrations::JiraSerializers::IssueEntity do
       end
 
       it 'returns URLs with the web url' do
-        expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter@reporter.com')
+        expect(subject[:author]).to include(web_url: 'http://jira.com/secure/ViewProfile.jspa?name=reporter%40reporter.com')
         expect(subject[:web_url]).to eq('http://jira.com/browse/GL-5')
       end
     end
@@ -105,6 +114,15 @@ RSpec.describe Integrations::JiraSerializers::IssueEntity do
       expect(subject[:author]).to include(web_url: 'http://jira.com/people/12345')
       expect(subject[:assignees].first).to include(web_url: 'http://jira.com/people/67890')
     end
+
+    it 'includes the Atlassian referrer on gitlab.com' do
+      allow(Gitlab).to receive(:com?).and_return(true)
+      referrer = Integrations::Jira::ATLASSIAN_REFERRER_GITLAB_COM.to_query
+
+      expect(subject[:web_url]).to eq("http://jira.com/browse/GL-5?#{referrer}")
+      expect(subject[:author]).to include(web_url: "http://jira.com/people/12345?#{referrer}")
+      expect(subject[:assignees].first).to include(web_url: "http://jira.com/people/67890?#{referrer}")
+    end
   end
 
   context 'without assignee' do

spec/models/integrations/jira_spec.rb

@@ -35,20 +35,42 @@ RSpec.describe Integrations::Jira do
         username: 'username',
         password: 'test',
         jira_issue_transition_id: 24,
-        url: 'http://jira.test.com/path/'
+        url: 'http://jira.test.com:1234/path/'
       }
     end
 
-    let(:service) { described_class.create!(options) }
+    let(:integration) { described_class.create!(options) }
 
     it 'sets the URL properly' do
       # jira-ruby gem parses the URI and handles trailing slashes fine:
       # https://github.com/sumoheavy/jira-ruby/blob/v1.7.0/lib/jira/http_client.rb#L62
-      expect(service.options[:site]).to eq('http://jira.test.com/')
+      expect(integration.options[:site]).to eq('http://jira.test.com:1234')
     end
 
     it 'leaves out trailing slashes in context' do
-      expect(service.options[:context_path]).to eq('/path')
+      expect(integration.options[:context_path]).to eq('/path')
+    end
+
+    context 'URL without a path' do
+      before do
+        integration.url = 'http://jira.test.com/'
+      end
+
+      it 'leaves out trailing slashes in context' do
+        expect(integration.options[:site]).to eq('http://jira.test.com')
+        expect(integration.options[:context_path]).to eq('')
+      end
+    end
+
+    context 'URL with query string parameters' do
+      before do
+        integration.url << '?nosso&foo=bar'
+      end
+
+      it 'removes query string parameters' do
+        expect(integration.options[:site]).to eq('http://jira.test.com:1234')
+        expect(integration.options[:context_path]).to eq('/path')
+      end
     end
 
     context 'username with trailing whitespaces' do
@@ -57,13 +79,13 @@ RSpec.describe Integrations::Jira do
       end
 
       it 'leaves out trailing whitespaces in username' do
-        expect(service.options[:username]).to eq('username')
+        expect(integration.options[:username]).to eq('username')
       end
     end
 
     it 'provides additional cookies to allow basic auth with oracle webgate' do
-      expect(service.options[:use_cookies]).to eq(true)
-      expect(service.options[:additional_cookies]).to eq(['OBBasicAuth=fromDialog'])
+      expect(integration.options[:use_cookies]).to eq(true)
+      expect(integration.options[:additional_cookies]).to eq(['OBBasicAuth=fromDialog'])
     end
 
     context 'using api URL' do
@@ -72,7 +94,7 @@ RSpec.describe Integrations::Jira do
       end
 
       it 'leaves out trailing slashes in context' do
-        expect(service.options[:context_path]).to eq('/api_path')
+        expect(integration.options[:context_path]).to eq('/api_path')
       end
     end
   end
@@ -990,17 +1012,51 @@ RSpec.describe Integrations::Jira do
   end
 
   context 'generating external URLs' do
-    let(:service) { described_class.new(url: 'http://jira.test.com/path/') }
+    let(:integration) { described_class.new(url: 'http://jira.test.com/path/') }
+
+    describe '#web_url' do
+      it 'handles paths, slashes, and query string' do
+        expect(integration.web_url).to eq(integration.url)
+        expect(integration.web_url('subpath/')).to eq('http://jira.test.com/path/subpath')
+        expect(integration.web_url('/subpath/')).to eq('http://jira.test.com/path/subpath')
+        expect(integration.web_url('subpath', foo: :bar)).to eq('http://jira.test.com/path/subpath?foo=bar')
+      end
+
+      it 'preserves existing query string' do
+        integration.url = 'http://jira.test.com/path/?nosso&foo=bar%20bar'
+
+        expect(integration.web_url).to eq("http://jira.test.com/path?foo=bar%20bar&nosso")
+        expect(integration.web_url('subpath/')).to eq('http://jira.test.com/path/subpath?foo=bar%20bar&nosso')
+        expect(integration.web_url('/subpath/')).to eq('http://jira.test.com/path/subpath?foo=bar%20bar&nosso')
+        expect(integration.web_url('subpath', bar: 'baz baz')).to eq('http://jira.test.com/path/subpath?bar=baz%20baz&foo=bar%20bar&nosso')
+      end
+
+      it 'includes Atlassian referrer for gitlab.com' do
+        allow(Gitlab).to receive(:com?).and_return(true)
+
+        expect(integration.web_url).to eq("http://jira.test.com/path?#{described_class::ATLASSIAN_REFERRER_GITLAB_COM.to_query}")
+
+        allow(Gitlab).to receive(:staging?).and_return(true)
+
+        expect(integration.web_url).to eq(integration.url)
+      end
+
+      it 'includes Atlassian referrer for self-managed' do
+        allow(Gitlab).to receive(:dev_or_test_env?).and_return(false)
+
+        expect(integration.web_url).to eq("http://jira.test.com/path?#{described_class::ATLASSIAN_REFERRER_SELF_MANAGED.to_query}")
+      end
+    end
 
     describe '#issues_url' do
-      it 'handles trailing slashes' do
-        expect(service.issues_url).to eq('http://jira.test.com/path/browse/:id')
+      it 'returns the correct URL' do
+        expect(integration.issues_url).to eq('http://jira.test.com/path/browse/:id')
       end
     end
 
     describe '#new_issue_url' do
-      it 'handles trailing slashes' do
-        expect(service.new_issue_url).to eq('http://jira.test.com/path/secure/CreateIssue!default.jspa')
+      it 'returns the correct URL' do
+        expect(integration.new_issue_url).to eq('http://jira.test.com/path/secure/CreateIssue!default.jspa')
       end
     end
   end