Merge branch 'token-revocation' into 'master'

Add a service for token revocation

See merge request gitlab-org/gitlab!46356

changelogs/unreleased/token-revocation.yml

+---
+title: Add a service for token revocation
+merge_request: 46356
+author:
+type: added

db/migrate/20201109180311_add_secret_detection_revocation_token_types_application_settings.rb

+# frozen_string_literal: true
+
+class AddSecretDetectionRevocationTokenTypesApplicationSettings < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def up
+    add_column :application_settings, :secret_detection_revocation_token_types_url, :text, null: true # rubocop:disable Migration/AddLimitToTextColumns
+  end
+
+  def down
+    remove_column :application_settings, :secret_detection_revocation_token_types_url
+  end
+end

db/migrate/20201109184023_add_text_limit_to_secret_detection_revocation_token_types_application_settings.rb

+# frozen_string_literal: true
+
+class AddTextLimitToSecretDetectionRevocationTokenTypesApplicationSettings < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_text_limit :application_settings, :secret_detection_revocation_token_types_url, 255
+  end
+
+  def down
+    remove_text_limit :application_settings, :secret_detection_revocation_token_types_url
+  end
+end

db/schema_migrations/20201109180311

+49143d2a7dd0a53c051151b0cdc93745a0fa1b01e6d54bb663e147c2064d9290
\ No newline at end of file

db/schema_migrations/20201109184023

+698bcedf387fc01fbb7f1899f0f7660ba86a197fa72cf71d998cc90e3d1da9f3
\ No newline at end of file

db/structure.sql

@@ -9344,6 +9344,7 @@ CREATE TABLE application_settings (
     new_user_signups_cap integer,
     encrypted_cloud_license_auth_token text,
     encrypted_cloud_license_auth_token_iv text,
+    secret_detection_revocation_token_types_url text,
     CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
     CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)),
     CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)),
@@ -9351,6 +9352,7 @@ CREATE TABLE application_settings (
     CONSTRAINT check_85a39b68ff CHECK ((char_length(encrypted_ci_jwt_signing_key_iv) <= 255)),
     CONSTRAINT check_9a719834eb CHECK ((char_length(secret_detection_token_revocation_url) <= 255)),
     CONSTRAINT check_9c6c447a13 CHECK ((char_length(maintenance_mode_message) <= 255)),
+    CONSTRAINT check_a5704163cc CHECK ((char_length(secret_detection_revocation_token_types_url) <= 255)),
     CONSTRAINT check_d03919528d CHECK ((char_length(container_registry_vendor) <= 255)),
     CONSTRAINT check_d820146492 CHECK ((char_length(spam_check_endpoint_url) <= 255)),
     CONSTRAINT check_e5aba18f02 CHECK ((char_length(container_registry_version) <= 255)),

ee/app/helpers/ee/application_settings_helper.rb

@@ -61,6 +61,7 @@ module EE
         :secret_detection_token_revocation_enabled,
         :secret_detection_token_revocation_url,
         :secret_detection_token_revocation_token,
+        :secret_detection_revocation_token_types_url,
         :shared_runners_minutes,
         :slack_app_enabled,
         :slack_app_id,

ee/app/models/ee/application_setting.rb

@@ -60,6 +60,18 @@ module EE
                 presence: { message: "can't be blank when indexing is enabled" },
                 if: ->(setting) { setting.elasticsearch_indexing? }
 
+      validates :secret_detection_revocation_token_types_url,
+                presence: { message: "can't be blank when secret detection token revocation is enabled" },
+                if: ->(setting) { setting.secret_detection_token_revocation_enabled? }
+
+      validates :secret_detection_token_revocation_url,
+                presence: { message: "can't be blank when secret detection token revocation is enabled" },
+                if: ->(setting) { setting.secret_detection_token_revocation_enabled? }
+
+      validates :secret_detection_token_revocation_token,
+                presence: { message: "can't be blank when secret detection token revocation is enabled" },
+                if: ->(setting) { setting.secret_detection_token_revocation_enabled? }
+
       validate :check_elasticsearch_url_scheme, if: :elasticsearch_url_changed?
 
       validates :elasticsearch_aws_region,
@@ -150,6 +162,7 @@ module EE
           secret_detection_token_revocation_enabled: false,
           secret_detection_token_revocation_url: nil,
           secret_detection_token_revocation_token: nil,
+          secret_detection_revocation_token_types_url: nil,
           slack_app_enabled: false,
           slack_app_id: nil,
           slack_app_secret: nil,

ee/app/services/security/token_revocation_service.rb

+# frozen_string_literal: true
+
+module Security
+  # Service for alerting revocation service of leaked security tokens
+  #
+  class TokenRevocationService < ::BaseService
+    RevocationFailedError = Class.new(StandardError)
+
+    def initialize(revocable_keys:)
+      @revocable_keys = revocable_keys
+    end
+
+    def execute
+      return error('Token revocation is disabled') unless token_revocation_enabled?
+
+      response = revoke_tokens
+      response.success? ? success : error('Failed to revoke tokens')
+    rescue RevocationFailedError => exception
+      error(exception.message)
+    rescue StandardError => exception
+      log_token_revocation_error(exception)
+      error(exception.message)
+    end
+
+    private
+
+    def token_revocation_enabled?
+      ::Gitlab::CurrentSettings.secret_detection_token_revocation_enabled?
+    end
+
+    def revoke_tokens
+      raise RevocationFailedError, 'Missing revocation tokens data' if missing_token_data?
+
+      ::Gitlab::HTTP.post(
+        token_revocation_url,
+        body: message,
+        headers: {
+         'Content-Type' => 'application/json',
+         'X-Token' => revocation_api_token
+        }
+      )
+    end
+
+    def missing_token_data?
+      token_revocation_url.blank? || token_types_url.blank? || revocation_api_token.blank?
+    end
+
+    def log_token_revocation_error(error)
+      log_error(
+        error: error.class.name,
+        message: error.message,
+        source: "#{__FILE__}:#{__LINE__}",
+        backtrace: error.backtrace
+      )
+    end
+
+    def message
+      response = ::Gitlab::HTTP.get(
+        token_types_url,
+        headers: {
+          'Content-Type' => 'application/json',
+          'X-Token' => revocation_api_token
+        }
+      )
+      raise RevocationFailedError, 'Failed to get revocation token types' unless response.success?
+
+      token_types = ::Gitlab::Json.parse(response.body)['types']
+      raise RevocationFailedError, 'No token type is available' if token_types.blank?
+
+      @revocable_keys.filter! { |key| token_types.include?(key[:type]) }
+      raise RevocationFailedError, 'No revocable key is present' if @revocable_keys.blank?
+
+      @revocable_keys.to_json
+    end
+
+    def token_types_url
+      ::Gitlab::CurrentSettings.secret_detection_revocation_token_types_url
+    end
+
+    def token_revocation_url
+      ::Gitlab::CurrentSettings.secret_detection_token_revocation_url
+    end
+
+    def revocation_api_token
+      ::Gitlab::CurrentSettings.secret_detection_token_revocation_token
+    end
+  end
+end

ee/lib/ee/api/helpers/settings_helpers.rb

@@ -33,6 +33,7 @@ module EE
             optional :secret_detection_token_revocation_enabled, type: ::Grape::API::Boolean, desc: 'Enable Secret Detection Token Revocation'
             given secret_detection_token_revocation_enabled: ->(val) { val } do
               requires :secret_detection_token_revocation_url, type: String, desc: 'The configured Secret Detection Token Revocation instance URL'
+              requires :secret_detection_revocation_token_types_url, type: String, desc: 'The configured Secret Detection Revocation Token Types instance URL'
             end
 
             optional :email_additional_text, type: String, desc: 'Additional text added to the bottom of every email for legal/auditing/compliance reasons'

ee/spec/models/application_setting_spec.rb

@@ -102,6 +102,19 @@ RSpec.describe ApplicationSetting do
       it { is_expected.not_to allow_value("a" * (subject.email_additional_text_character_limit + 1)).for(:email_additional_text) }
     end
 
+    describe 'when secret detection token revocation is enabled' do
+      before do
+        stub_application_setting(secret_detection_token_revocation_enabled: true)
+      end
+
+      it { is_expected.to allow_value("http://test.com").for(:secret_detection_token_revocation_url) }
+      it { is_expected.to allow_value("AKVD34#$%56").for(:secret_detection_token_revocation_token) }
+      it { is_expected.to allow_value("http://test.com").for(:secret_detection_revocation_token_types_url) }
+      it { is_expected.not_to allow_value(nil).for(:secret_detection_token_revocation_url) }
+      it { is_expected.not_to allow_value(nil).for(:secret_detection_token_revocation_token) }
+      it { is_expected.not_to allow_value(nil).for(:secret_detection_revocation_token_types_url) }
+    end
+
     context 'when validating allowed_ips' do
       where(:allowed_ips, :is_valid) do
         "192.1.1.1"                   | true

ee/spec/requests/api/settings_spec.rb

@@ -70,16 +70,24 @@ RSpec.describe API::Settings, 'EE Settings' do
 
     context 'secret_detection_token_revocation_enabled is true' do
       context 'secret_detection_token_revocation_url value is present' do
+        let(:revocation_url) { 'https://example.com/secret_detection_token_revocation' }
+        let(:revocation_token_types_url) { 'https://example.com/secret_detection_revocation_token_types' }
+        let(:revocation_token) { 'AKDD345$%^^' }
+
         it 'updates secret_detection_token_revocation_url' do
           put api('/application/settings', admin),
             params: {
               secret_detection_token_revocation_enabled: true,
-              secret_detection_token_revocation_url: 'https://example.com/secret_detection_token_revocation'
+              secret_detection_token_revocation_url: revocation_url,
+              secret_detection_token_revocation_token: revocation_token,
+              secret_detection_revocation_token_types_url: revocation_token_types_url
             }
 
           expect(response).to have_gitlab_http_status(:ok)
           expect(json_response['secret_detection_token_revocation_enabled']).to be(true)
-          expect(json_response['secret_detection_token_revocation_url']).to eq('https://example.com/secret_detection_token_revocation')
+          expect(json_response['secret_detection_token_revocation_url']).to eq(revocation_url)
+          expect(json_response['secret_detection_revocation_token_types_url']).to eq(revocation_token_types_url)
+          expect(json_response['secret_detection_token_revocation_token']).to eq(revocation_token)
         end
       end
 
@@ -88,7 +96,7 @@ RSpec.describe API::Settings, 'EE Settings' do
           put api('/application/settings', admin), params: { secret_detection_token_revocation_enabled: true }
 
           expect(response).to have_gitlab_http_status(:bad_request)
-          expect(json_response['error']).to include('secret_detection_token_revocation_url is missing')
+          expect(json_response['error']).to include('secret_detection_token_revocation_url is missing, secret_detection_revocation_token_types_url is missing')
         end
       end
     end

ee/spec/services/security/token_revocation_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Security::TokenRevocationService, '#execute' do
+  let_it_be(:revocation_token_types_url) { 'https://myhost.com/api/v1/token_types' }
+  let_it_be(:token_revocation_url) { 'https://myhost.com/api/v1/revoke' }
+
+  let_it_be(:revocable_keys) do
+    [{
+      'type': 'aws_key_id',
+      'token': 'AKIASOMEAWSACCESSKEY',
+      'location': 'https://mywebsite.com/some-repo/blob/abcdefghijklmnop/compromisedfile.java'
+     },
+     {
+        'type': 'aws_secret',
+        'token': 'some_aws_secret_key_some_aws_secret_key_',
+        'location': 'https://mywebsite.com/some-repo/blob/abcdefghijklmnop/compromisedfile.java'
+     },
+     {
+        'type': 'aws_secret',
+        'token': 'another_aws_secret_key_another_secret_key',
+        'location': 'https://mywebsite.com/some-repo/blob/abcdefghijklmnop/compromisedfile.java'
+     }]
+  end
+
+  let_it_be(:revocable_token_types) do
+    { 'types': %w(aws_key_id aws_secret gcp_key_id gcp_secret) }
+  end
+
+  subject { described_class.new(revocable_keys: revocable_keys).execute }
+
+  before do
+    stub_application_setting(secret_detection_revocation_token_types_url: revocation_token_types_url)
+    stub_application_setting(secret_detection_token_revocation_token: 'token1')
+    stub_application_setting(secret_detection_token_revocation_url: token_revocation_url)
+  end
+
+  context 'when revocation token API returns a response with failure' do
+    before do
+      stub_application_setting(secret_detection_token_revocation_enabled: true)
+      stub_revoke_token_api_with_failure
+      stub_revocation_token_types_api_with_success
+    end
+
+    it 'returns error' do
+      expect(subject[:status]).to be(:error)
+      expect(subject[:message]).to eql('Failed to revoke tokens')
+    end
+  end
+
+  context 'when revocation token API returns invalid token types' do
+    before do
+      stub_application_setting(secret_detection_token_revocation_enabled: true)
+      stub_invalid_token_types_api_with_success
+    end
+
+    specify { expect(subject).to eql({ message: 'No token type is available', status: :error }) }
+  end
+
+  context 'when revocation service is disabled' do
+    specify { expect(subject).to eql({ message: 'Token revocation is disabled', status: :error }) }
+  end
+
+  context 'when revocation service is enabled' do
+    before do
+      stub_application_setting(secret_detection_token_revocation_enabled: true)
+      stub_revoke_token_api_with_success
+    end
+
+    context 'with a list of valid token types' do
+      before do
+        stub_revocation_token_types_api_with_success
+      end
+
+      context 'when there is a list of tokens to be revoked' do
+        specify { expect(subject[:status]).to be(:success) }
+      end
+
+      context 'when token_revocation_url is missing' do
+        before do
+          allow_next_instance_of(described_class) do |token_revocation_service|
+            allow(token_revocation_service).to receive(:token_revocation_url) { nil }
+          end
+        end
+
+        specify { expect(subject).to eql({ message: 'Missing revocation tokens data', status: :error }) }
+      end
+
+      context 'when token_types_url is missing' do
+        before do
+          allow_next_instance_of(described_class) do |token_revocation_service|
+            allow(token_revocation_service).to receive(:token_types_url) { nil }
+          end
+        end
+
+        specify { expect(subject).to eql({ message: 'Missing revocation tokens data', status: :error }) }
+      end
+
+      context 'when revocation_api_token is missing' do
+        before do
+          allow_next_instance_of(described_class) do |token_revocation_service|
+            allow(token_revocation_service).to receive(:revocation_api_token) { nil }
+          end
+        end
+
+        specify { expect(subject).to eql({ message: 'Missing revocation tokens data', status: :error }) }
+      end
+
+      context 'when there is no token to be revoked' do
+        let_it_be(:revocable_token_types) do
+          { 'types': %w() }
+        end
+
+        specify { expect(subject).to eql({ message: 'No token type is available', status: :error }) }
+      end
+    end
+
+    context 'when revocation token types API returns an unsuccessful response' do
+      before do
+        stub_revocation_token_types_api_with_failure
+      end
+
+      specify { expect(subject).to eql({ message: 'Failed to get revocation token types', status: :error }) }
+    end
+  end
+
+  def stub_revoke_token_api_with_success
+    stub_request(:post, token_revocation_url)
+      .with(body: revocable_keys.to_json)
+      .to_return(
+        status: 200,
+        headers: { 'Content-Type' => 'application/json' },
+        body: {}.to_json
+      )
+  end
+
+  def stub_revoke_token_api_with_failure
+    stub_request(:post, token_revocation_url)
+      .with(body: revocable_keys.to_json)
+      .to_return(
+        status: 400,
+        headers: { 'Content-Type' => 'application/json' },
+        body: {}.to_json
+      )
+  end
+
+  def stub_revocation_token_types_api_with_success
+    stub_request(:get, revocation_token_types_url)
+      .to_return(
+        status: 200,
+        headers: { 'Content-Type' => 'application/json' },
+        body: revocable_token_types.to_json
+      )
+  end
+
+  def stub_invalid_token_types_api_with_success
+    stub_request(:get, revocation_token_types_url)
+      .to_return(
+        status: 200,
+        headers: { 'Content-Type' => 'application/json' },
+        body: {}.to_json
+      )
+  end
+
+  def stub_revocation_token_types_api_with_failure
+    stub_request(:get, revocation_token_types_url)
+      .to_return(
+        status: 400,
+        headers: { 'Content-Type' => 'application/json' },
+        body: {}.to_json
+      )
+  end
+end

spec/requests/api/settings_spec.rb

@@ -23,6 +23,7 @@ RSpec.describe API::Settings, 'Settings' do
       expect(json_response['sourcegraph_enabled']).to be_falsey
       expect(json_response['sourcegraph_url']).to be_nil
       expect(json_response['secret_detection_token_revocation_url']).to be_nil
+      expect(json_response['secret_detection_revocation_token_types_url']).to be_nil
       expect(json_response['sourcegraph_public_only']).to be_truthy
       expect(json_response['default_project_visibility']).to be_a String
       expect(json_response['default_snippet_visibility']).to be_a String