Merge branch 'security-redact-links-11-4' into 'security-11-4'

[11.4] Redact unsubscribe links in issuable texts

See merge request gitlab/gitlabhq!2565

app/models/concerns/issuable.rb

@@ -9,6 +9,7 @@
 module Issuable
   extend ActiveSupport::Concern
   include Gitlab::SQL::Pattern
+  include Redactable
   include CacheMarkdownField
   include Participable
   include Mentionable
@@ -32,6 +33,8 @@ module Issuable
     cache_markdown_field :title, pipeline: :single_line
     cache_markdown_field :description, issuable_state_filter_enabled: true
 
+    redact_field :description
+
     belongs_to :author, class_name: "User"
     belongs_to :updated_by, class_name: "User"
     belongs_to :last_edited_by, class_name: 'User'

app/models/concerns/redactable.rb

+# frozen_string_literal: true
+
+# This module searches and redacts sensitive information in
+# redactable fields. Currently only unsubscribe link is redacted.
+# Add following lines into your model:
+#
+#     include Redactable
+#     redact_field :foo
+#
+module Redactable
+  extend ActiveSupport::Concern
+
+  UNSUBSCRIBE_PATTERN = %r{/sent_notifications/\h{32}/unsubscribe}
+
+  class_methods do
+    def redact_field(field)
+      before_validation do
+        redact_field!(field) if attribute_changed?(field)
+      end
+    end
+  end
+
+  private
+
+  def redact_field!(field)
+    text = public_send(field) # rubocop:disable GitlabSecurity/PublicSend
+    return unless text.present?
+
+    redacted = text.gsub(UNSUBSCRIBE_PATTERN, '/sent_notifications/REDACTED/unsubscribe')
+
+    public_send("#{field}=", redacted) # rubocop:disable GitlabSecurity/PublicSend
+  end
+end

app/models/note.rb

@@ -10,6 +10,7 @@ class Note < ActiveRecord::Base
   include Awardable
   include Importable
   include FasterCacheKeys
+  include Redactable
   include CacheMarkdownField
   include AfterCommitQueue
   include ResolvableNote
@@ -33,6 +34,8 @@ def values
 
   cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true
 
+  redact_field :note
+
   # Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with notes.
   # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10392/diffs#note_28719102
   alias_attribute :last_edited_at, :updated_at

app/models/snippet.rb

@@ -2,6 +2,7 @@
 
 class Snippet < ActiveRecord::Base
   include Gitlab::VisibilityLevel
+  include Redactable
   include CacheMarkdownField
   include Noteable
   include Participable
@@ -18,6 +19,8 @@ class Snippet < ActiveRecord::Base
   cache_markdown_field :description
   cache_markdown_field :content
 
+  redact_field :description
+
   # Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with snippets.
   # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10392/diffs#note_28719102
   alias_attribute :last_edited_at, :updated_at

changelogs/unreleased/redact-links-dev.yml

+---
+title: Redact personal tokens in unsubscribe links.
+merge_request:
+author:
+type: security

db/post_migrate/20181014121030_enqueue_redact_links.rb

+# frozen_string_literal: true
+
+class EnqueueRedactLinks < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  BATCH_SIZE = 1000
+  DELAY_INTERVAL = 5.minutes.to_i
+  MIGRATION = 'RedactLinks'
+
+  disable_ddl_transaction!
+
+  class Note < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'notes'
+    self.inheritance_column = :_type_disabled
+  end
+
+  class Issue < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'issues'
+    self.inheritance_column = :_type_disabled
+  end
+
+  class MergeRequest < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'merge_requests'
+    self.inheritance_column = :_type_disabled
+  end
+
+  class Snippet < ActiveRecord::Base
+    include EachBatch
+
+    self.table_name = 'snippets'
+    self.inheritance_column = :_type_disabled
+  end
+
+  def up
+    disable_statement_timeout do
+      schedule_migration(Note, 'note')
+      schedule_migration(Issue, 'description')
+      schedule_migration(MergeRequest, 'description')
+      schedule_migration(Snippet, 'description')
+    end
+  end
+
+  def down
+    # nothing to do
+  end
+
+  private
+
+  def schedule_migration(model, field)
+    link_pattern = "%/sent_notifications/" + ("_" * 32) + "/unsubscribe%"
+
+    model.where("#{field} like ?", link_pattern).each_batch(of: BATCH_SIZE) do |batch, index|
+      start_id, stop_id = batch.pluck('MIN(id)', 'MAX(id)').first
+
+      BackgroundMigrationWorker.perform_in(index * DELAY_INTERVAL, MIGRATION, [model.name.demodulize, field, start_id, stop_id])
+    end
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20181002172433) do
+ActiveRecord::Schema.define(version: 20181014121030) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"

lib/gitlab/background_migration/redact_links.rb

+# frozen_string_literal: true
+# rubocop:disable Style/Documentation
+
+module Gitlab
+  module BackgroundMigration
+    class RedactLinks
+      module Redactable
+        extend ActiveSupport::Concern
+
+        def redact_field!(field)
+          self[field].gsub!(%r{/sent_notifications/\h{32}/unsubscribe}, '/sent_notifications/REDACTED/unsubscribe')
+
+          if self.changed?
+            self.update_columns(field => self[field],
+                                "#{field}_html" => nil)
+          end
+        end
+      end
+
+      class Note < ActiveRecord::Base
+        include EachBatch
+        include Redactable
+
+        self.table_name = 'notes'
+        self.inheritance_column = :_type_disabled
+      end
+
+      class Issue < ActiveRecord::Base
+        include EachBatch
+        include Redactable
+
+        self.table_name = 'issues'
+        self.inheritance_column = :_type_disabled
+      end
+
+      class MergeRequest < ActiveRecord::Base
+        include EachBatch
+        include Redactable
+
+        self.table_name = 'merge_requests'
+        self.inheritance_column = :_type_disabled
+      end
+
+      class Snippet < ActiveRecord::Base
+        include EachBatch
+        include Redactable
+
+        self.table_name = 'snippets'
+        self.inheritance_column = :_type_disabled
+      end
+
+      def perform(model_name, field, start_id, stop_id)
+        link_pattern = "%/sent_notifications/" + ("_" * 32) + "/unsubscribe%"
+        model = "Gitlab::BackgroundMigration::RedactLinks::#{model_name}".constantize
+
+        model.where("#{field} like ?", link_pattern).where(id: start_id..stop_id).each do |resource|
+          resource.redact_field!(field)
+        end
+      end
+    end
+  end
+end

spec/lib/gitlab/background_migration/redact_links_spec.rb

+require 'spec_helper'
+
+describe Gitlab::BackgroundMigration::RedactLinks, :migration, schema: 20181014121030 do
+  let(:namespaces) { table(:namespaces) }
+  let(:projects) { table(:projects) }
+  let(:issues) { table(:issues) }
+  let(:notes) { table(:notes) }
+  let(:snippets) { table(:snippets) }
+  let(:users) { table(:users) }
+  let(:merge_requests) { table(:merge_requests) }
+  let(:namespace) { namespaces.create(name: 'gitlab', path: 'gitlab-org') }
+  let(:project) { projects.create(namespace_id: namespace.id, name: 'foo') }
+  let(:user) { users.create!(email: 'test@example.com', projects_limit: 100, username: 'test') }
+
+  def create_merge_request(id, params)
+    params.merge!(id: id,
+                  target_project_id: project.id,
+                  target_branch: 'master',
+                  source_project_id: project.id,
+                  source_branch: 'mr name',
+                  title: "mr name#{id}")
+
+    merge_requests.create(params)
+  end
+
+  def create_issue(id, params)
+    params.merge!(id: id, title: "issue#{id}", project_id: project.id)
+
+    issues.create(params)
+  end
+
+  def create_note(id, params)
+    params[:id] = id
+
+    notes.create(params)
+  end
+
+  def create_snippet(id, params)
+    params.merge!(id: id, author_id: user.id)
+
+    snippets.create(params)
+  end
+
+  def create_resource(model, id, params)
+    send("create_#{model.name.underscore}", id, params)
+  end
+
+  shared_examples_for 'redactable resource' do
+    it 'updates only matching texts' do
+      matching_text = 'some text /sent_notifications/00000000000000000000000000000000/unsubscribe more text'
+      redacted_text = 'some text /sent_notifications/REDACTED/unsubscribe more text'
+      create_resource(model, 1, { field => matching_text })
+      create_resource(model, 2, { field => 'not matching text' })
+      create_resource(model, 3, { field => matching_text })
+      create_resource(model, 4, { field => redacted_text })
+      create_resource(model, 5, { field => matching_text })
+
+      expected = { field => 'some text /sent_notifications/REDACTED/unsubscribe more text',
+                   "#{field}_html" => nil }
+      expect_any_instance_of("Gitlab::BackgroundMigration::RedactLinks::#{model}".constantize).to receive(:update_columns).with(expected).and_call_original
+
+      subject.perform(model, field, 2, 4)
+
+      expect(model.where(field => matching_text).pluck(:id)).to eq [1, 5]
+      expect(model.find(3).reload[field]).to eq redacted_text
+    end
+  end
+
+  context 'resource is Issue' do
+    it_behaves_like 'redactable resource' do
+      let(:model) { Issue }
+      let(:field) { :description }
+    end
+  end
+
+  context 'resource is Merge Request' do
+    it_behaves_like 'redactable resource' do
+      let(:model) { MergeRequest }
+      let(:field) { :description }
+    end
+  end
+
+  context 'resource is Note' do
+    it_behaves_like 'redactable resource' do
+      let(:model) { Note }
+      let(:field) { :note }
+    end
+  end
+
+  context 'resource is Snippet' do
+    it_behaves_like 'redactable resource' do
+      let(:model) { Snippet }
+      let(:field) { :description }
+    end
+  end
+end

spec/migrations/enqueue_redact_links_spec.rb

+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20181014121030_enqueue_redact_links.rb')
+
+describe EnqueueRedactLinks, :migration, :sidekiq do
+  let(:merge_requests) { table(:merge_requests) }
+  let(:issues) { table(:issues) }
+  let(:notes) { table(:notes) }
+  let(:projects) { table(:projects) }
+  let(:namespaces) { table(:namespaces) }
+  let(:snippets) { table(:snippets) }
+  let(:users) { table(:users) }
+  let(:user) { users.create!(email: 'test@example.com', projects_limit: 100, username: 'test') }
+
+  before do
+    stub_const("#{described_class.name}::BATCH_SIZE", 1)
+
+    text = 'some text /sent_notifications/00000000000000000000000000000000/unsubscribe more text'
+    group = namespaces.create!(name: 'gitlab', path: 'gitlab')
+    project = projects.create!(namespace_id: group.id)
+
+    merge_requests.create!(id: 1, target_project_id: project.id, source_project_id: project.id, target_branch: 'feature', source_branch: 'master', description: text)
+    issues.create!(id: 1, description: text)
+    notes.create!(id: 1, note: text)
+    notes.create!(id: 2, note: text)
+    snippets.create!(id: 1, description: text, author_id: user.id)
+  end
+
+  it 'correctly schedules background migrations' do
+    Sidekiq::Testing.fake! do
+      Timecop.freeze do
+        migrate!
+
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, "Note", "note", 1, 1)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(10.minutes, "Note", "note", 2, 2)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, "Issue", "description", 1, 1)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, "MergeRequest", "description", 1, 1)
+        expect(described_class::MIGRATION).to be_scheduled_delayed_migration(5.minutes, "Snippet", "description", 1, 1)
+        expect(BackgroundMigrationWorker.jobs.size).to eq 5
+      end
+    end
+  end
+end

spec/models/concerns/redactable_spec.rb

+require 'spec_helper'
+
+describe Redactable do
+  shared_examples 'model with redactable field' do
+    it 'redacts unsubscribe token' do
+      model[field] = 'some text /sent_notifications/00000000000000000000000000000000/unsubscribe more text'
+
+      model.save!
+
+      expect(model[field]).to eq 'some text /sent_notifications/REDACTED/unsubscribe more text'
+    end
+
+    it 'ignores not hexadecimal tokens' do
+      text = 'some text /sent_notifications/token/unsubscribe more text'
+      model[field] = text
+
+      model.save!
+
+      expect(model[field]).to eq text
+    end
+
+    it 'ignores not matching texts' do
+      text = 'some text /sent_notifications/.*/unsubscribe more text'
+      model[field] = text
+
+      model.save!
+
+      expect(model[field]).to eq text
+    end
+
+    it 'redacts the field when saving the model before creating markdown cache' do
+      model[field] = 'some text /sent_notifications/00000000000000000000000000000000/unsubscribe more text'
+
+      model.save!
+
+      expected = 'some text /sent_notifications/REDACTED/unsubscribe more text'
+      expect(model[field]).to eq expected
+      expect(model["#{field}_html"]).to eq "<p dir=\"auto\">#{expected}</p>"
+    end
+  end
+
+  context 'when model is an issue' do
+    it_behaves_like 'model with redactable field' do
+      let(:model) { create(:issue) }
+      let(:field) { :description }
+    end
+  end
+
+  context 'when model is a merge request' do
+    it_behaves_like 'model with redactable field' do
+      let(:model) { create(:merge_request) }
+      let(:field) { :description }
+    end
+  end
+
+  context 'when model is a note' do
+    it_behaves_like 'model with redactable field' do
+      let(:model) { create(:note) }
+      let(:field) { :note }
+    end
+  end
+
+  context 'when model is a snippet' do
+    it_behaves_like 'model with redactable field' do
+      let(:model) { create(:snippet) }
+      let(:field) { :description }
+    end
+  end
+end