From 29035d83059e78df2620497cc27c74983f64c5c6 Mon Sep 17 00:00:00 2001
From: Brett Walker <bwalker@gitlab.com>
Date: Sat, 11 Jul 2020 12:53:02 -0500
Subject: [PATCH] Handle jira service exceptions a little better
by not exposing any detailed information,
which might be a security issue
---
.../projects/integrations/jira/issues_finder.rb | 6 +++---
.../integrations/jira/issues_controller.rb | 17 +++++++++++++----
.../integrations/jira/issues_controller_spec.rb | 6 +++---
locale/gitlab.pot | 3 +++
.../integrations/jira/issues_finder_spec.rb | 6 +++---
5 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/app/finders/projects/integrations/jira/issues_finder.rb b/app/finders/projects/integrations/jira/issues_finder.rb
index 239ff4ba2b209..69d50d031a7c3 100644
--- a/app/finders/projects/integrations/jira/issues_finder.rb
+++ b/app/finders/projects/integrations/jira/issues_finder.rb
@@ -3,10 +3,10 @@
module Projects
module Integrations
module Jira
- IntegrationError = Class.new(StandardError)
- RequestError = Class.new(StandardError)
-
class IssuesFinder
+ IntegrationError = Class.new(StandardError)
+ RequestError = Class.new(StandardError)
+
attr_reader :issues, :total_count, :per_page
class << self
diff --git a/ee/app/controllers/projects/integrations/jira/issues_controller.rb b/ee/app/controllers/projects/integrations/jira/issues_controller.rb
index 448f2bb8129cb..8012fff0c1939 100644
--- a/ee/app/controllers/projects/integrations/jira/issues_controller.rb
+++ b/ee/app/controllers/projects/integrations/jira/issues_controller.rb
@@ -14,13 +14,14 @@ class IssuesController < Projects::ApplicationController
push_frontend_feature_flag(:jira_integration, project)
end
+ rescue_from ::Projects::Integrations::Jira::IssuesFinder::IntegrationError, with: :render_integration_error
+ rescue_from ::Projects::Integrations::Jira::IssuesFinder::RequestError, with: :render_request_error
+
def index
respond_to do |format|
format.html
format.json do
render json: issues_json
- rescue Projects::Integrations::Jira::IntegrationError, Projects::Integrations::Jira::RequestError => e
- render_bad_request(e)
end
end
end
@@ -72,8 +73,16 @@ def check_feature_enabled!
return render_404 unless Feature.enabled?(:jira_integration, project) && project.external_issue_tracker
end
- def render_bad_request(error)
- render json: { errors: [error.message] }, status: :bad_request
+ # Return the informational message to the user
+ def render_integration_error(exception)
+ render json: { errors: [exception.message] }, status: :bad_request
+ end
+
+ # Log the specific request error details and return generic message
+ def render_request_error(exception)
+ Gitlab::AppLogger.error(exception)
+
+ render json: { errors: [_('An error occurred while requesting data from the Jira service')] }, status: :bad_request
end
end
end
diff --git a/ee/spec/controllers/projects/integrations/jira/issues_controller_spec.rb b/ee/spec/controllers/projects/integrations/jira/issues_controller_spec.rb
index 542adf7a09754..4fb5e6f1d5859 100644
--- a/ee/spec/controllers/projects/integrations/jira/issues_controller_spec.rb
+++ b/ee/spec/controllers/projects/integrations/jira/issues_controller_spec.rb
@@ -66,7 +66,7 @@
it 'renders bad request for IntegrationError' do
expect_any_instance_of(Projects::Integrations::Jira::IssuesFinder).to receive(:execute)
- .and_raise(Projects::Integrations::Jira::IntegrationError, 'Integration error')
+ .and_raise(Projects::Integrations::Jira::IssuesFinder::IntegrationError, 'Integration error')
get :index, params: { namespace_id: project.namespace, project_id: project }, format: :json
@@ -76,12 +76,12 @@
it 'renders bad request for RequestError' do
expect_any_instance_of(Projects::Integrations::Jira::IssuesFinder).to receive(:execute)
- .and_raise(Projects::Integrations::Jira::RequestError, 'Request error')
+ .and_raise(Projects::Integrations::Jira::IssuesFinder::RequestError, 'Request error')
get :index, params: { namespace_id: project.namespace, project_id: project }, format: :json
expect(response).to have_gitlab_http_status(:bad_request)
- expect(json_response['errors']).to eq ['Request error']
+ expect(json_response['errors']).to eq ['An error occurred while requesting data from the Jira service']
end
it 'sets pagination headers' do
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 5b847b1e963d6..6091e59854b35 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -2660,6 +2660,9 @@ msgstr ""
msgid "An error occurred while reordering issues."
msgstr ""
+msgid "An error occurred while requesting data from the Jira service"
+msgstr ""
+
msgid "An error occurred while retrieving calendar activity"
msgstr ""
diff --git a/spec/finders/projects/integrations/jira/issues_finder_spec.rb b/spec/finders/projects/integrations/jira/issues_finder_spec.rb
index fd55585cc3bc5..939aed2208652 100644
--- a/spec/finders/projects/integrations/jira/issues_finder_spec.rb
+++ b/spec/finders/projects/integrations/jira/issues_finder_spec.rb
@@ -24,7 +24,7 @@
context 'when jira service integration does not have project_key' do
it 'raises error' do
- expect { subject }.to raise_error(Projects::Integrations::Jira::IntegrationError, 'Jira project key is not configured')
+ expect { subject }.to raise_error(Projects::Integrations::Jira::IssuesFinder::IntegrationError, 'Jira project key is not configured')
end
end
@@ -34,7 +34,7 @@
end
it 'raises error' do
- expect { subject }.to raise_error(Projects::Integrations::Jira::IntegrationError, 'Jira service not configured.')
+ expect { subject }.to raise_error(Projects::Integrations::Jira::IssuesFinder::IntegrationError, 'Jira service not configured.')
end
end
@@ -55,7 +55,7 @@
end
it 'raises error', :aggregate_failures do
- expect { subject }.to raise_error(Projects::Integrations::Jira::RequestError)
+ expect { subject }.to raise_error(Projects::Integrations::Jira::IssuesFinder::RequestError)
end
end
--
GitLab