Redact unsubscribe links in issuable texts

It's possible that user pastes accidentally also unsubscribe link
which is included in footer of notification emails. This unsubscribe
link contains personal token which attacker then use to act as the
original user (e.g. for sending comments under his/her identity).

11 files changed, 382 insertions, 1 deletions

diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 2aa52bbaeea..a808f9ad4ad 100644
--- a/app/models/concerns/issuable.rb
+++ b/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'
diff --git a/app/models/concerns/redactable.rb b/app/models/concerns/redactable.rb
new file mode 100644
index 00000000000..5ad96d6cc46
--- /dev/null
+++ b/app/models/concerns/redactable.rb
@@ -0,0 +1,33 @@
+# 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
diff --git a/app/models/note.rb b/app/models/note.rb
index 1b595ef60b4..725c3d68c37 100644
--- a/app/models/note.rb
+++ b/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 @@ class Note < ActiveRecord::Base
 
   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
diff --git a/app/models/snippet.rb b/app/models/snippet.rb
index e9533ee7c77..1c5846b4023 100644
--- a/app/models/snippet.rb
+++ b/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
diff --git a/changelogs/unreleased/redact-links-dev.yml b/changelogs/unreleased/redact-links-dev.yml
new file mode 100644
index 00000000000..338e7965465
--- /dev/null
+++ b/changelogs/unreleased/redact-links-dev.yml
@@ -0,0 +1,5 @@
+---
+title: Redact personal tokens in unsubscribe links.
+merge_request:
+author:
+type: security
diff --git a/db/post_migrate/20181014121030_enqueue_redact_links.rb b/db/post_migrate/20181014121030_enqueue_redact_links.rb
new file mode 100644
index 00000000000..1ee4703c88a
--- /dev/null
+++ b/db/post_migrate/20181014121030_enqueue_redact_links.rb
@@ -0,0 +1,65 @@
+# 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
diff --git a/db/schema.rb b/db/schema.rb
index 31b71784ba6..50768d2693c 100644
--- a/db/schema.rb
+++ b/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"
diff --git a/lib/gitlab/background_migration/redact_links.rb b/lib/gitlab/background_migration/redact_links.rb
new file mode 100644
index 00000000000..f5d3bcdd517
--- /dev/null
+++ b/lib/gitlab/background_migration/redact_links.rb
@@ -0,0 +1,62 @@
+# 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
diff --git a/spec/lib/gitlab/background_migration/redact_links_spec.rb b/spec/lib/gitlab/background_migration/redact_links_spec.rb
new file mode 100644
index 00000000000..a40e68069cc
--- /dev/null
+++ b/spec/lib/gitlab/background_migration/redact_links_spec.rb
@@ -0,0 +1,96 @@
+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
diff --git a/spec/migrations/enqueue_redact_links_spec.rb b/spec/migrations/enqueue_redact_links_spec.rb
new file mode 100644
index 00000000000..a5da76977b7
--- /dev/null
+++ b/spec/migrations/enqueue_redact_links_spec.rb
@@ -0,0 +1,42 @@
+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
diff --git a/spec/models/concerns/redactable_spec.rb b/spec/models/concerns/redactable_spec.rb
new file mode 100644
index 00000000000..7d320edd492
--- /dev/null
+++ b/spec/models/concerns/redactable_spec.rb
@@ -0,0 +1,69 @@
+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