Merge branch 'security-fix-grafana-token-leaked-in-plain-to-other-maintainers' into 'master'

Mask grafana token with encryption

See merge request gitlab-org/security/gitlab!53

app/graphql/types/grafana_integration_type.rb

@@ -10,14 +10,19 @@ module Types
       description: 'Internal ID of the Grafana integration'
     field :grafana_url, GraphQL::STRING_TYPE, null: false,
       description: 'Url for the Grafana host for the Grafana integration'
-    field :token, GraphQL::STRING_TYPE, null: false,
-      description: 'API token for the Grafana integration'
     field :enabled, GraphQL::BOOLEAN_TYPE, null: false,
       description: 'Indicates whether Grafana integration is enabled'
-
     field :created_at, Types::TimeType, null: false,
           description: 'Timestamp of the issue\'s creation'
     field :updated_at, Types::TimeType, null: false,
           description: 'Timestamp of the issue\'s last activity'
+
+    field :token, GraphQL::STRING_TYPE, null: false,
+          deprecation_reason: 'Plain text token has been masked for security reasons',
+          description: 'API token for the Grafana integration. Field is permanently masked.'
+
+    def token
+      object.masked_token
+    end
   end
 end

app/helpers/projects_helper.rb

@@ -368,8 +368,8 @@ module ProjectsHelper
     @project.grafana_integration&.grafana_url
   end
 
-  def grafana_integration_token
-    @project.grafana_integration&.token
+  def grafana_integration_masked_token
+    @project.grafana_integration&.masked_token
   end
 
   def grafana_integration_enabled?

app/models/grafana_integration.rb

@@ -8,11 +8,13 @@ class GrafanaIntegration < ApplicationRecord
     algorithm: 'aes-256-gcm',
     key:       Settings.attr_encrypted_db_key_base_32
 
+  before_validation :check_token_changes
+
   validates :grafana_url,
             length: { maximum: 1024 },
             addressable_url: { enforce_sanitization: true, ascii_only: true }
 
-  validates :token, :project, presence: true
+  validates :encrypted_token, :project, presence: true
 
   validates :enabled, inclusion: { in: [true, false] }
 
@@ -23,4 +25,28 @@ class GrafanaIntegration < ApplicationRecord
 
     @client ||= ::Grafana::Client.new(api_url: grafana_url.chomp('/'), token: token)
   end
+
+  def masked_token
+    mask(encrypted_token)
+  end
+
+  def masked_token_was
+    mask(encrypted_token_was)
+  end
+
+  private
+
+  def token
+    decrypt(:token, encrypted_token)
+  end
+
+  def check_token_changes
+    return unless [encrypted_token_was, masked_token_was].include?(token)
+
+    clear_attribute_changes [:token, :encrypted_token, :encrypted_token_iv]
+  end
+
+  def mask(token)
+    token&.squish&.gsub(/./, '*')
+  end
 end

app/views/projects/settings/operations/_grafana_integration.html.haml

 .js-grafana-integration{ data: { operations_settings_endpoint: project_settings_operations_path(@project),
-  grafana_integration: { url: grafana_integration_url, token: grafana_integration_token, enabled: grafana_integration_enabled?.to_s } } }
+  grafana_integration: { url: grafana_integration_url, token: grafana_integration_masked_token, enabled: grafana_integration_enabled?.to_s } } }

changelogs/unreleased/security-fix-grafana-token-leaked-in-plain-to-other-maintainers.yml

+---
+title: Prevent gafana integration token from being displayed as a plain text to other project maintainers, by only displaying a masked version of it. GraphQL api deprecate token field in GrafanaIntegration type.
+merge_request:
+author:
+type: security

doc/api/graphql/reference/gitlab_schema.graphql

@@ -2658,9 +2658,9 @@ type GrafanaIntegration {
   id: ID!
 
   """
-  API token for the Grafana integration
+  API token for the Grafana integration. Field is permanently masked.
   """
-  token: String!
+  token: String! @deprecated(reason: "Plain text token has been masked for security reasons")
 
   """
   Timestamp of the issue's last activity

doc/api/graphql/reference/gitlab_schema.json

@@ -17366,7 +17366,7 @@
             },
             {
               "name": "token",
-              "description": "API token for the Grafana integration",
+              "description": "API token for the Grafana integration. Field is permanently masked.",
               "args": [
 
               ],
@@ -17379,8 +17379,8 @@
                   "ofType": null
                 }
               },
-              "isDeprecated": false,
-              "deprecationReason": null
+              "isDeprecated": true,
+              "deprecationReason": "Plain text token has been masked for security reasons"
             },
             {
               "name": "updatedAt",

doc/api/graphql/reference/index.md

@@ -403,7 +403,7 @@ Autogenerated return type of EpicTreeReorder
 | `enabled` | Boolean! | Indicates whether Grafana integration is enabled |
 | `grafanaUrl` | String! | Url for the Grafana host for the Grafana integration |
 | `id` | ID! | Internal ID of the Grafana integration |
-| `token` | String! | API token for the Grafana integration |
+| `token` | String! | API token for the Grafana integration. Field is permanently masked. |
 | `updatedAt` | Time! | Timestamp of the issue's last activity |
 
 ## Group

spec/helpers/projects_helper_spec.rb

@@ -935,14 +935,14 @@ describe ProjectsHelper do
       helper.instance_variable_set(:@project, project)
     end
 
-    subject { helper.grafana_integration_token }
+    subject { helper.grafana_integration_masked_token }
 
     it { is_expected.to eq(nil) }
 
     context 'grafana integration exists' do
       let!(:grafana_integration) { create(:grafana_integration, project: project) }
 
-      it { is_expected.to eq(grafana_integration.token) }
+      it { is_expected.to eq(grafana_integration.masked_token) }
     end
   end
 

spec/models/grafana_integration_spec.rb

@@ -9,7 +9,7 @@ describe GrafanaIntegration do
 
   describe 'validations' do
     it { is_expected.to validate_presence_of(:project) }
-    it { is_expected.to validate_presence_of(:token) }
+    it { is_expected.to validate_presence_of(:encrypted_token) }
 
     it 'disallows invalid urls for grafana_url' do
       unsafe_url = %{https://replaceme.com/'><script>alert(document.cookie)</script>}
@@ -66,4 +66,24 @@ describe GrafanaIntegration do
       end
     end
   end
+
+  describe 'attribute encryption' do
+    subject(:grafana_integration) { create(:grafana_integration, token: 'super-secret') }
+
+    context 'token' do
+      it 'encrypts original value into encrypted_token attribute' do
+        expect(grafana_integration.encrypted_token).not_to be_nil
+      end
+
+      it 'locks access to raw value in private method', :aggregate_failures do
+        expect { grafana_integration.token }.to raise_error(NoMethodError, /private method .token. called/)
+        expect(grafana_integration.send(:token)).to eql('super-secret')
+      end
+
+      it 'prevents overriding token value with its encrypted or masked version', :aggregate_failures do
+        expect { grafana_integration.update(token: grafana_integration.encrypted_token) }.not_to change { grafana_integration.reload.send(:token) }
+        expect { grafana_integration.update(token: grafana_integration.masked_token) }.not_to change { grafana_integration.reload.send(:token) }
+      end
+    end
+  end
 end

spec/requests/api/graphql/project/grafana_integration_spec.rb

@@ -45,7 +45,7 @@ describe 'Getting Grafana Integration' do
 
       it_behaves_like 'a working graphql query'
 
-      it { expect(integration_data['token']).to eql grafana_integration.token }
+      it { expect(integration_data['token']).to eql grafana_integration.masked_token }
       it { expect(integration_data['grafanaUrl']).to eql grafana_integration.grafana_url }
 
       it do

spec/services/projects/operations/update_service_spec.rb

@@ -210,7 +210,7 @@ describe Projects::Operations::UpdateService do
           integration = project.reload.grafana_integration
 
           expect(integration.grafana_url).to eq(expected_attrs[:grafana_url])
-          expect(integration.token).to eq(expected_attrs[:token])
+          expect(integration.send(:token)).to eq(expected_attrs[:token])
         end
       end
 
@@ -226,7 +226,7 @@ describe Projects::Operations::UpdateService do
           integration = project.reload.grafana_integration
 
           expect(integration.grafana_url).to eq(expected_attrs[:grafana_url])
-          expect(integration.token).to eq(expected_attrs[:token])
+          expect(integration.send(:token)).to eq(expected_attrs[:token])
         end
 
         context 'with all grafana attributes blank in params' do