Merge branch 'security-mask-sentry-token-12-4-ce' into '12-4-stable'

Mask Sentry auth token

See merge request gitlab/gitlabhq!3504

6 files changed, 51 insertions, 4 deletions

diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb
index 8d08f0cda94..92d4ef85ecf 100644
--- a/app/services/error_tracking/list_projects_service.rb
+++ b/app/services/error_tracking/list_projects_service.rb
@@ -32,7 +32,7 @@ module ErrorTracking
           project_slug: 'proj'
         )
 
-        setting.token = params[:token]
+        setting.token = token(setting)
         setting.enabled = true
       end
     end
@@ -40,5 +40,12 @@ module ErrorTracking
     def can_read?
       can?(current_user, :read_sentry_issue, project)
     end
+
+    def token(setting)
+      # Use param token if not masked, otherwise use database token
+      return params[:token] unless /\A\*+\z/.match?(params[:token])
+
+      setting.token
+    end
   end
 end
diff --git a/app/services/projects/operations/update_service.rb b/app/services/projects/operations/update_service.rb
index 64519501ff4..0ca89664304 100644
--- a/app/services/projects/operations/update_service.rb
+++ b/app/services/projects/operations/update_service.rb
@@ -36,15 +36,17 @@ module Projects
           organization_slug: settings.dig(:project, :organization_slug)
         )
 
-        {
+        params = {
           error_tracking_setting_attributes: {
             api_url: api_url,
-            token: settings[:token],
             enabled: settings[:enabled],
             project_name: settings.dig(:project, :name),
             organization_name: settings.dig(:project, :organization_name)
           }
         }
+        params[:error_tracking_setting_attributes][:token] = settings[:token] unless /\A\*+\z/.match?(settings[:token]) # Don't update token if we receive masked value
+
+        params
       end
 
       def grafana_integration_params
diff --git a/app/views/projects/settings/operations/_error_tracking.html.haml b/app/views/projects/settings/operations/_error_tracking.html.haml
index 583fc08f375..589d3037eba 100644
--- a/app/views/projects/settings/operations/_error_tracking.html.haml
+++ b/app/views/projects/settings/operations/_error_tracking.html.haml
@@ -17,4 +17,4 @@
         project: error_tracking_setting_project_json,
         api_host: setting.api_host,
         enabled: setting.enabled.to_json,
-        token: setting.token } }
+        token: setting.token.present? ? '*' * 12 : nil } }
diff --git a/changelogs/unreleased/security-mask-sentry-token-ce.yml b/changelogs/unreleased/security-mask-sentry-token-ce.yml
new file mode 100644
index 00000000000..e9fe780a488
--- /dev/null
+++ b/changelogs/unreleased/security-mask-sentry-token-ce.yml
@@ -0,0 +1,4 @@
+---
+title: Mask sentry auth token in Error Tracking dashboard
+author:
+type: security
diff --git a/spec/services/error_tracking/list_projects_service_spec.rb b/spec/services/error_tracking/list_projects_service_spec.rb
index 730fccc599e..a272a604184 100644
--- a/spec/services/error_tracking/list_projects_service_spec.rb
+++ b/spec/services/error_tracking/list_projects_service_spec.rb
@@ -50,6 +50,19 @@ describe ErrorTracking::ListProjectsService do
         end
       end
 
+      context 'masked param token' do
+        let(:params) { ActionController::Parameters.new(token: "*********", api_host: new_api_host) }
+
+        before do
+          expect(error_tracking_setting).to receive(:list_sentry_projects)
+            .and_return({ projects: [] })
+        end
+
+        it 'uses database token' do
+          expect { subject.execute }.not_to change { error_tracking_setting.token }
+        end
+      end
+
       context 'sentry client raises exception' do
         context 'Sentry::Client::Error' do
           before do
diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb
index b2f9fd6df79..81d59a98b9b 100644
--- a/spec/services/projects/operations/update_service_spec.rb
+++ b/spec/services/projects/operations/update_service_spec.rb
@@ -145,6 +145,27 @@ describe Projects::Operations::UpdateService do
         end
       end
 
+      context 'with masked param token' do
+        let(:params) do
+          {
+            error_tracking_setting_attributes: {
+              enabled: false,
+              token: '*' * 8
+            }
+          }
+        end
+
+        before do
+          create(:project_error_tracking_setting, project: project, token: 'token')
+        end
+
+        it 'does not update token' do
+          expect(result[:status]).to eq(:success)
+
+          expect(project.error_tracking_setting.token).to eq('token')
+        end
+      end
+
       context 'with invalid parameters' do
         let(:params) { {} }