diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb index e732c1bd86fe62b54450191a5da13b012f26b16e..ffb08e10f1f353a84de868d483af2ed1a37a8384 100644 --- a/app/models/project_snippet.rb +++ b/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 diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 4010a3e216722006b0832a08f960c0797c103b2a..51ab94e6f4ab8385229612449faced1e170a10b9 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -51,8 +51,8 @@ def content_html_invalidated? # 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 @@ def content_html_invalidated? 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 @@ def self.for_project_with_user(project, user = nil) 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 @@ def to_ability_name 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. # diff --git a/app/views/snippets/_snippets_scope_menu.html.haml b/app/views/snippets/_snippets_scope_menu.html.haml index c312226dd6c9878acd5e7b0d16e77408fea00140..cb59b11ca2b34fbb6cbd827b7c9d5713d90c2800 100644 --- a/app/views/snippets/_snippets_scope_menu.html.haml +++ b/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") } diff --git a/changelogs/unreleased/fj-secret-snippet-migrations.yml b/changelogs/unreleased/fj-secret-snippet-migrations.yml new file mode 100644 index 0000000000000000000000000000000000000000..67eefcbfd351891ebac803af1bdd0c99c9d5e5f4 --- /dev/null +++ b/changelogs/unreleased/fj-secret-snippet-migrations.yml @@ -0,0 +1,5 @@ +--- +title: Add migrations for secret snippets +merge_request: 19939 +author: +type: added diff --git a/db/migrate/20191025092748_add_secret_token_to_snippet.rb b/db/migrate/20191025092748_add_secret_token_to_snippet.rb new file mode 100644 index 0000000000000000000000000000000000000000..0649f58d23e5b41cb4bfbb35f8072742e77195f4 --- /dev/null +++ b/db/migrate/20191025092748_add_secret_token_to_snippet.rb @@ -0,0 +1,10 @@ +# 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 diff --git a/db/migrate/20191105155113_add_secret_to_snippet.rb b/db/migrate/20191105155113_add_secret_to_snippet.rb new file mode 100644 index 0000000000000000000000000000000000000000..ae514d484942f997a0342c96b783e1c2ef253683 --- /dev/null +++ b/db/migrate/20191105155113_add_secret_to_snippet.rb @@ -0,0 +1,27 @@ +# 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 diff --git a/db/schema.rb b/db/schema.rb index fd36599e1f9462df61a5de03d824b0f9ca4322b0..6d7b99a9f9fb453ce71a8e8f52327ccc8ca41a14 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -3555,13 +3555,16 @@ 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| diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 1aafe5804c00c4a7a6a28e68c8fd2779abec8526..d491f45b0da035e2d8825a3ef3a9173e9f92cc2f 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/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 diff --git a/spec/factories/snippets.rb b/spec/factories/snippets.rb index ede071ae70c3bd243169ae0a0c18d480583c7eb3..5990ed7ffb0a3bcb799bf72bb650469d9410c3de 100644 --- a/spec/factories/snippets.rb +++ b/spec/factories/snippets.rb @@ -7,6 +7,7 @@ content { generate(:title) } description { generate(:title) } file_name { generate(:filename) } + secret { false } trait :public do visibility_level { Snippet::PUBLIC } @@ -27,5 +28,9 @@ end factory :personal_snippet, parent: :snippet, class: :PersonalSnippet do + trait :secret do + visibility_level { Snippet::PUBLIC } + secret { true } + end end end diff --git a/spec/models/project_snippet_spec.rb b/spec/models/project_snippet_spec.rb index 46025507cb5367d4e538523a96c0c9364f6e4377..903671afb135bda0f955b7b89c396008c127ace7 100644 --- a/spec/models/project_snippet_spec.rb +++ b/spec/models/project_snippet_spec.rb @@ -9,6 +9,7 @@ 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 diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index e4cc89318409f00fade104e5c7915cb88349bbe6..cfc0703af23ab9e4dd63f4214bcb18c32349d60c 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -451,4 +451,20 @@ 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