Merge branch 'fj-secret-snippet-migrations' into 'master'

Added migrations for secret snippets

See merge request gitlab-org/gitlab!19939

app/models/project_snippet.rb

@@ -4,4 +4,5 @@ class ProjectSnippet < Snippet
   belongs_to :project
 
   validates :project, presence: true
+  validates :secret, inclusion: { in: [false] }
 end

app/models/snippet.rb

@@ -51,8 +51,8 @@ class Snippet < ApplicationRecord
   # Scopes
   scope :are_internal, -> { where(visibility_level: Snippet::INTERNAL) }
   scope :are_private, -> { where(visibility_level: Snippet::PRIVATE) }
-  scope :are_public, -> { where(visibility_level: Snippet::PUBLIC) }
-  scope :public_and_internal, -> { where(visibility_level: [Snippet::PUBLIC, Snippet::INTERNAL]) }
+  scope :are_public, -> { public_only }
+  scope :are_secret, -> { public_only.where(secret: true) }
   scope :fresh, -> { order("created_at DESC") }
   scope :inc_author, -> { includes(:author) }
   scope :inc_relations_for_view, -> { includes(author: :status) }
@@ -63,6 +63,11 @@ class Snippet < ApplicationRecord
   attr_spammable :title, spam_title: true
   attr_spammable :content, spam_description: true
 
+  attr_encrypted :secret_token,
+    key:       Settings.attr_encrypted_db_key_base_truncated,
+    mode:      :per_attribute_iv,
+    algorithm: 'aes-256-cbc'
+
   def self.with_optional_visibility(value = nil)
     if value
       where(visibility_level: value)
@@ -112,11 +117,8 @@ class Snippet < ApplicationRecord
   end
 
   def self.visible_to_or_authored_by(user)
-    where(
-      'snippets.visibility_level IN (?) OR snippets.author_id = ?',
-      Gitlab::VisibilityLevel.levels_for_user(user),
-      user.id
-    )
+    query = where(visibility_level: Gitlab::VisibilityLevel.levels_for_user(user))
+    query.or(where(author_id: user.id))
   end
 
   def self.reference_prefix
@@ -222,6 +224,19 @@ class Snippet < ApplicationRecord
     model_name.singular
   end
 
+  def valid_secret_token?(token)
+    return false unless token && secret_token
+
+    ActiveSupport::SecurityUtils.secure_compare(token.to_s, secret_token.to_s)
+  end
+
+  def as_json(options = {})
+    options[:except] = Array.wrap(options[:except])
+    options[:except] << :secret_token
+
+    super
+  end
+
   class << self
     # Searches for snippets with a matching title or file name.
     #

app/views/snippets/_snippets_scope_menu.html.haml

@@ -9,7 +9,7 @@
         - if include_private
           = subject.snippets.count
         - else
-          = subject.snippets.public_and_internal.count
+          = subject.snippets.public_and_internal_only.count
 
   - if include_private
     %li{ class: active_when(params[:scope] == "are_private") }

changelogs/unreleased/fj-secret-snippet-migrations.yml

+---
+title: Add migrations for secret snippets
+merge_request: 19939
+author:
+type: added

db/migrate/20191025092748_add_secret_token_to_snippet.rb

+# frozen_string_literal: true
+
+class AddSecretTokenToSnippet < ActiveRecord::Migration[5.2]
+  DOWNTIME = false
+
+  def change
+    add_column :snippets, :encrypted_secret_token, :string, limit: 255
+    add_column :snippets, :encrypted_secret_token_iv, :string, limit: 255
+  end
+end

db/migrate/20191105155113_add_secret_to_snippet.rb

+# frozen_string_literal: true
+
+class AddSecretToSnippet < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    unless column_exists?(:snippets, :secret)
+      add_column_with_default :snippets, :secret, :boolean, default: false
+    end
+
+    add_concurrent_index :snippets, [:visibility_level, :secret]
+    remove_concurrent_index :snippets, :visibility_level
+  end
+
+  def down
+    add_concurrent_index :snippets, :visibility_level
+    remove_concurrent_index :snippets, [:visibility_level, :secret]
+
+    if column_exists?(:snippets, :secret)
+      remove_column :snippets, :secret
+    end
+  end
+end

db/schema.rb

@@ -3555,13 +3555,16 @@ ActiveRecord::Schema.define(version: 2019_11_15_091425) do
     t.integer "cached_markdown_version"
     t.text "description"
     t.text "description_html"
+    t.string "encrypted_secret_token", limit: 255
+    t.string "encrypted_secret_token_iv", limit: 255
+    t.boolean "secret", default: false, null: false
     t.index ["author_id"], name: "index_snippets_on_author_id"
     t.index ["content"], name: "index_snippets_on_content_trigram", opclass: :gin_trgm_ops, using: :gin
     t.index ["file_name"], name: "index_snippets_on_file_name_trigram", opclass: :gin_trgm_ops, using: :gin
     t.index ["project_id", "visibility_level"], name: "index_snippets_on_project_id_and_visibility_level"
     t.index ["title"], name: "index_snippets_on_title_trigram", opclass: :gin_trgm_ops, using: :gin
     t.index ["updated_at"], name: "index_snippets_on_updated_at"
-    t.index ["visibility_level"], name: "index_snippets_on_visibility_level"
+    t.index ["visibility_level", "secret"], name: "index_snippets_on_visibility_level_and_secret"
   end
 
   create_table "software_license_policies", id: :serial, force: :cascade do |t|

lib/gitlab/import_export/import_export.yml

@@ -163,6 +163,9 @@ excluded_attributes:
     - :identifier
   snippets:
     - :expired_at
+    - :secret
+    - :encrypted_secret_token
+    - :encrypted_secret_token_iv
   merge_request_diff:
     - :external_diff
     - :stored_externally

spec/factories/snippets.rb

@@ -7,6 +7,7 @@ FactoryBot.define do
     content { generate(:title) }
     description { generate(:title) }
     file_name { generate(:filename) }
+    secret { false }
 
     trait :public do
       visibility_level { Snippet::PUBLIC }
@@ -27,5 +28,9 @@ FactoryBot.define do
   end
 
   factory :personal_snippet, parent: :snippet, class: :PersonalSnippet do
+    trait :secret do
+      visibility_level { Snippet::PUBLIC }
+      secret { true }
+    end
   end
 end

spec/models/project_snippet_spec.rb

@@ -9,6 +9,7 @@ describe ProjectSnippet do
 
   describe "Validation" do
     it { is_expected.to validate_presence_of(:project) }
+    it { is_expected.to validate_inclusion_of(:secret).in_array([false]) }
   end
 
   describe '#embeddable?' do

spec/models/snippet_spec.rb

@@ -451,4 +451,20 @@ describe Snippet do
       expect(blob.data).to eq(snippet.content)
     end
   end
+
+  describe '#to_json' do
+    let(:snippet) { build(:snippet) }
+
+    it 'excludes secret_token from generated json' do
+      expect(JSON.parse(to_json).keys).not_to include("secret_token")
+    end
+
+    it 'does not override existing exclude option value' do
+      expect(JSON.parse(to_json(except: [:id])).keys).not_to include("secret_token", "id")
+    end
+
+    def to_json(params = {})
+      snippet.to_json(params)
+    end
+  end
 end