diff --git a/app/assets/javascripts/notes.js b/app/assets/javascripts/notes.js index fcd5b391b38e8fc0e0e6b114dcaf811c0f144a1e..dffe99859f9f28296c968749257fd01f27b12824 100644 --- a/app/assets/javascripts/notes.js +++ b/app/assets/javascripts/notes.js @@ -298,7 +298,7 @@ export default class Notes { this.refreshing = true; axios - .get(`${this.notes_url}?html=true`, { + .get(this.urlWithHtmlParam(this.notes_url), { headers: { 'X-Last-Fetched-At': this.last_fetched_at, }, @@ -1230,7 +1230,7 @@ export default class Notes { $editForm .find('form') - .attr('action', `${postUrl}?html=true`) + .attr('action', this.urlWithHtmlParam(postUrl)) .attr('data-remote', 'true'); $editForm.find('.js-form-target-id').val(targetId); $editForm.find('.js-form-target-type').val(targetType); @@ -1667,7 +1667,7 @@ export default class Notes { // Make request to submit comment on server return axios - .post(`${formAction}?html=true`, formData) + .post(this.urlWithHtmlParam(formAction), formData) .then(res => { const note = res.data; @@ -1818,7 +1818,7 @@ export default class Notes { // Make request to update comment on server axios - .post(`${formAction}?html=true`, formData) + .post(this.urlWithHtmlParam(formAction), formData) .then(({ data }) => { // Submission successful! render final note element this.updateNote(data, $editingNote); @@ -1835,6 +1835,20 @@ export default class Notes { return $closeBtn.text($closeBtn.data('originalText')); } + + urlWithHtmlParam(url) { + const urlHash = new URL(url, this.noteable_url); + let search; + + if (urlHash.search) { + search = `${urlHash.search}&html=true`; + } else { + search = `?html=true`; + } + + urlHash.search = search; + return urlHash.toString(); + } } window.Notes = Notes; diff --git a/app/assets/javascripts/pages/snippets/form.js b/app/assets/javascripts/pages/snippets/form.js index 8859557e62de055a4a0994f49216e069b963ca55..b24d8354c84da2ed492cf612ebcc336ef3b255d4 100644 --- a/app/assets/javascripts/pages/snippets/form.js +++ b/app/assets/javascripts/pages/snippets/form.js @@ -14,4 +14,18 @@ export default () => { snippets: false, }); new ZenMode(); // eslint-disable-line no-new + + const secretElement = $('.snippet-form input.snippet_secret'); + + $( + '.snippet-form .visibility-level-setting input[data-track-property="visibility_level_secret"]', + ).click(() => { + secretElement.val('true'); + }); + + $( + '.snippet-form .visibility-level-setting input[data-track-property!="visibility_level_secret"]', + ).click(() => { + secretElement.val('false'); + }); }; diff --git a/app/controllers/snippets/notes_controller.rb b/app/controllers/snippets/notes_controller.rb index 551b37cb3d35e9170d6f26a5ca817a606236c1fc..710968081ae2aaf1067f12358684dec04251c516 100644 --- a/app/controllers/snippets/notes_controller.rb +++ b/app/controllers/snippets/notes_controller.rb @@ -5,6 +5,7 @@ class Snippets::NotesController < ApplicationController include ToggleAwardEmoji skip_before_action :authenticate_user!, only: [:index] + before_action :authorize_secret_snippet! before_action :authorize_read_snippet!, only: [:show, :index] before_action :authorize_create_note!, only: [:create] @@ -39,4 +40,11 @@ def authorize_read_snippet! def authorize_create_note! access_denied! unless can?(current_user, :create_note, noteable) end + + def authorize_secret_snippet! + return unless Feature.enabled?(:secret_snippets, current_user) + return if can?(current_user, :admin_personal_snippet, snippet) + + render_404 if snippet.secret? && !snippet.valid_secret_token?(params[:token]) + end end diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index 54774df5e761cc12c11a0fdc50b661008f6d84d2..c2538f9e7f31c0add1fb741fd036d1eee1d13c2d 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -13,7 +13,9 @@ class SnippetsController < ApplicationController skip_before_action :verify_authenticity_token, if: -> { action_name == 'show' && js_request? } - before_action :snippet, only: [:show, :edit, :destroy, :update, :raw] + before_action :snippet, only: [:show, :edit, :destroy, :update, :raw, :toggle_award_emoji] + + before_action :authorize_secret_snippet!, only: [:show, :raw, :toggle_award_emoji] before_action :authorize_create_snippet!, only: [:new, :create] before_action :authorize_read_snippet!, only: [:show, :raw] @@ -119,13 +121,7 @@ def spammable_path end def authorize_read_snippet! - return if can?(current_user, :read_personal_snippet, @snippet) - - if current_user - render_404 - else - authenticate_user! - end + return access_error unless can?(current_user, :read_personal_snippet, @snippet) end def authorize_update_snippet! @@ -140,8 +136,18 @@ def authorize_create_snippet! return render_404 unless can?(current_user, :create_personal_snippet) end + def authorize_secret_snippet! + return if Feature.disabled?(:secret_snippets, current_user) + return if can?(current_user, :admin_personal_snippet, @snippet) + return access_error if snippet&.secret? && !snippet&.valid_secret_token?(params[:token]) + end + + def access_error + current_user ? render_404 : authenticate_user! + end + def snippet_params - params.require(:personal_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description) + params.require(:personal_snippet).permit(:title, :content, :file_name, :private, :visibility_level, :description, :secret) end def move_temporary_files diff --git a/app/controllers/uploads_controller.rb b/app/controllers/uploads_controller.rb index 67d3364847077e23c5ee5bf5201f40e7e0b7cd79..9aec00077ce0ed9f461d5856a284f5c5fc00c1a9 100644 --- a/app/controllers/uploads_controller.rb +++ b/app/controllers/uploads_controller.rb @@ -65,6 +65,10 @@ def authorize_create_access! case model when User can?(current_user, :update_user, model) + when PersonalSnippet + if can?(current_user, :create_note, model) + authorize_secret_snippet + end else can?(current_user, :create_note, model) end @@ -106,4 +110,11 @@ def upload_mount_satisfied? upload_model_class.uploader_options.has_key?(upload_mount) end + + def authorize_secret_snippet + return true if Feature.disabled?(:secret_snippets, current_user) || !model.secret? + return true if can?(current_user, :admin_personal_snippet, model) + + model.valid_secret_token?(params[:token]) + end end diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb index 5819f279eaa37b89e69a065f1fc6d19d57de2df5..aab02c5e5c4b2c1c030848fc711ce79e4729a928 100644 --- a/app/finders/snippets_finder.rb +++ b/app/finders/snippets_finder.rb @@ -67,7 +67,7 @@ def execute items = init_collection items = by_ids(items) - items.with_optional_visibility(visibility_from_scope).fresh + items.with_optional_visibility(*visibility_from_scope).fresh end private @@ -89,7 +89,10 @@ def init_collection # We only show personal snippets here because this page is meant for # discovery, and project snippets are of limited interest here. def snippets_for_explore - Snippet.public_to_user(current_user).only_personal_snippets + Snippet + .without_secret + .public_to_user(current_user) + .only_personal_snippets end # Produces a query that retrieves snippets from multiple projects. @@ -159,9 +162,10 @@ def snippets_for_author if author == current_user # If the current user is also the author of all snippets, then we can - # include private snippets. + # include private and secret snippets. base else + base = base.without_secret unless current_user&.admin? base.public_to_user(current_user) end end @@ -169,13 +173,15 @@ def snippets_for_author def visibility_from_scope case scope.to_s when 'are_private' - Snippet::PRIVATE + [Snippet::PRIVATE, false] + when 'are_secret' + [Snippet::PUBLIC, true] when 'are_internal' - Snippet::INTERNAL + [Snippet::INTERNAL, false] when 'are_public' - Snippet::PUBLIC + [Snippet::PUBLIC, false] else - nil + [nil, false] end end diff --git a/app/helpers/award_emoji_helper.rb b/app/helpers/award_emoji_helper.rb index 13df53a751bfe8cb2cfe9d2611a73d7564522fd2..82fe0d599b51d149e5a3a0dd4d7c9d7f5ceb3bae 100644 --- a/app/helpers/award_emoji_helper.rb +++ b/app/helpers/award_emoji_helper.rb @@ -2,6 +2,7 @@ module AwardEmojiHelper def toggle_award_url(awardable) + return gitlab_toggle_award_emoji_snippet_path(awardable) if awardable.is_a?(PersonalSnippet) return url_for([:toggle_award_emoji, awardable]) unless @project || awardable.is_a?(Note) if awardable.is_a?(Note) diff --git a/app/helpers/gitlab_routing_helper.rb b/app/helpers/gitlab_routing_helper.rb index 78c41257404338bb9109640f5a859941e12611ee..c491ffc909af8dfdb9cb9f8397d3c95cf1bf5a7a 100644 --- a/app/helpers/gitlab_routing_helper.rb +++ b/app/helpers/gitlab_routing_helper.rb @@ -282,6 +282,10 @@ def snippet_query_params(snippet, *args) {} end + if Feature.enabled?(:secret_snippets, @current_user) && snippet.secret? + opts[:token] = snippet.secret_token + end + args << opts end end diff --git a/app/helpers/snippets_helper.rb b/app/helpers/snippets_helper.rb index 1c7690f30d27169da671e51b5e5d57cf1d84968b..eff591bd6ed70eba3619276c0ca28bf7d450d0d1 100644 --- a/app/helpers/snippets_helper.rb +++ b/app/helpers/snippets_helper.rb @@ -5,7 +5,7 @@ def snippets_upload_path(snippet, user) return unless user if snippet&.persisted? - upload_path('personal_snippet', id: snippet.id) + upload_path('personal_snippet', id: snippet.id, token: snippet.secret_token) else upload_path('user', id: user.id) end @@ -137,6 +137,8 @@ def snippet_badge(snippet) def snippet_badge_attributes(snippet) if snippet.private? ['fa-lock', _('private')] + elsif snippet.secret? + ['fa-user-secret', _('secret')] end end @@ -160,4 +162,30 @@ def embedded_snippet_download_button title: 'Download', rel: 'noopener noreferrer') end + + def snippet_visibility_level_icon(resource) + return icon('user-secret') if secret_snippet_or_visibility_level?(resource) + + level = resource.is_a?(Snippet) ? resource.visibility_level : resource + + visibility_level_icon(level, fw: false) + end + + def snippet_visibility_level_label(resource) + return s_('Snippets|Secret') if secret_snippet_or_visibility_level?(resource) + + level = resource.is_a?(Snippet) ? resource.visibility_level : resource + + visibility_level_label(level) + end + + private + + def secret_snippet_or_visibility_level?(resource) + resource.is_a?(Snippet) ? resource.secret? : snippet_visibility_secret?(resource) + end + + def snippet_visibility_secret?(level) + level == Snippet::VISIBILITY_SECRET + end end diff --git a/app/helpers/visibility_level_helper.rb b/app/helpers/visibility_level_helper.rb index a36de5dc548289f8a04df1e30e1a6e1b8f33899e..5e0bd53f2d44a2d7472aebb2ffa7ce7c11159779 100644 --- a/app/helpers/visibility_level_helper.rb +++ b/app/helpers/visibility_level_helper.rb @@ -61,7 +61,13 @@ def snippet_visibility_level_description(level, snippet = nil) when Gitlab::VisibilityLevel::INTERNAL _("The snippet is visible to any logged in user.") when Gitlab::VisibilityLevel::PUBLIC - _("The snippet can be accessed without any authentication.") + if snippet&.secret? + _("The snippet can be accessed without any authentication, but is not searchable.") + else + _("The snippet can be accessed without any authentication.") + end + when Snippet::VISIBILITY_SECRET + _("The snippet can be accessed without any authentication, but is not searchable.") end end diff --git a/app/models/personal_snippet.rb b/app/models/personal_snippet.rb index 1b5be8698b152f85ea96ace18ed1a40f8e47b9d5..6c5dbc66083919ad336ae791671dad48a7cfd37e 100644 --- a/app/models/personal_snippet.rb +++ b/app/models/personal_snippet.rb @@ -2,4 +2,31 @@ class PersonalSnippet < Snippet include WithUploads + + before_save :update_secret_snippet_data + + def searchable? + !secret? + end + + def self.visibility_level_values(user) + Gitlab::VisibilityLevel.values.tap do |values| + values << VISIBILITY_SECRET if Feature.enabled?(:secret_snippets, user) + end + end + + private + + # Allow secret to be true only for public snippets + def snippet_can_be_secret? + public? + end + + def update_secret_snippet_data + self.secret = !!self.secret && snippet_can_be_secret? + # If the snippet is secret and secret_token is empty we create a new one + # If the snippet is secret and secret_token is not empty we we leave the existing one + # If the snippet is not secret we reset the token to nil + self.secret_token = self.secret ? (self.secret_token || SecureRandom.hex) : nil + end end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 92746d28f05d78b0bdbe20920db59e9ad7346a30..fcb540420d73d7c583640a945fcd637cb6065e5b 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -16,6 +16,8 @@ class Snippet < ApplicationRecord include FromUnion extend ::Gitlab::Utils::Override + VISIBILITY_SECRET = 'secret'.freeze + cache_markdown_field :title, pipeline: :single_line cache_markdown_field :description cache_markdown_field :content @@ -64,8 +66,10 @@ def content_html_invalidated? # Scopes scope :are_internal, -> { where(visibility_level: Snippet::INTERNAL) } scope :are_private, -> { where(visibility_level: Snippet::PRIVATE) } - scope :are_public, -> { public_only } scope :are_secret, -> { public_only.where(secret: true) } + scope :are_public, -> { public_only.without_secret } + scope :without_secret, -> { where(secret: false) } + scope :fresh, -> { order("created_at DESC") } scope :inc_author, -> { includes(:author) } scope :inc_relations_for_view, -> { includes(author: :status) } @@ -83,9 +87,9 @@ def content_html_invalidated? mode: :per_attribute_iv, algorithm: 'aes-256-cbc' - def self.with_optional_visibility(value = nil) - if value - where(visibility_level: value) + def self.with_optional_visibility(visibility_level = nil, secret = false) + if visibility_level + where(visibility_level: visibility_level, secret: secret) else all end @@ -133,6 +137,8 @@ def self.for_project_with_user(project, user = nil) def self.visible_to_or_authored_by(user) query = where(visibility_level: Gitlab::VisibilityLevel.levels_for_user(user)) + query = query.without_secret unless user.admin? + query.or(where(author_id: user.id)) end @@ -279,6 +285,12 @@ def parent_class ::Project end end + + protected + + def snippet_can_be_secret? + false + end end Snippet.prepend_if_ee('EE::Snippet') diff --git a/app/views/dashboard/snippets/index.html.haml b/app/views/dashboard/snippets/index.html.haml index 2caa8e0cac4b6a60ba8c37fb16468527672094bf..db6bb160c6b29c1071fb2404189a460b7da2599b 100644 --- a/app/views/dashboard/snippets/index.html.haml +++ b/app/views/dashboard/snippets/index.html.haml @@ -4,7 +4,7 @@ = render 'dashboard/snippets_head' - if current_user.snippets.exists? - = render partial: 'snippets/snippets_scope_menu', locals: { include_private: true } + = render partial: 'snippets/snippets_scope_menu', locals: { include_private: true, include_secret: true } - if current_user.snippets.exists? = render partial: 'shared/snippets/list', locals: { link_project: true } diff --git a/app/views/projects/snippets/index.html.haml b/app/views/projects/snippets/index.html.haml index 7682d01a5a1e76a1f9e4109a4353360d1f73ce9f..a0ceb88ee7d86079f9409b3d935b243f0bdbe0a6 100644 --- a/app/views/projects/snippets/index.html.haml +++ b/app/views/projects/snippets/index.html.haml @@ -4,7 +4,7 @@ - if current_user .top-area - include_private = @project.team.member?(current_user) || current_user.admin? - = render partial: 'snippets/snippets_scope_menu', locals: { subject: @project, include_private: include_private } + = render partial: 'snippets/snippets_scope_menu', locals: { subject: @project, include_private: include_private, include_secret: false } - if can?(current_user, :create_project_snippet, @project) .nav-controls diff --git a/app/views/shared/_visibility_radio_option.html.haml b/app/views/shared/_visibility_radio_option.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..1cfc7652bf1d7d8141a72257bf1961a1cd44ce6e --- /dev/null +++ b/app/views/shared/_visibility_radio_option.html.haml @@ -0,0 +1,7 @@ += form.radio_button model_method, level, checked: (selected_level == level), class: 'form-check-input', data: { track_label: "blank_project", track_event: "activate_form_input", track_property: "#{model_method}_#{level}", track_value: "", qa_selector: "#{visibility_level_label(level).downcase}_radio" } += form.label "#{model_method}_#{level}", class: 'form-check-label' do + = visibility_level_icon(level) + .option-title + = visibility_level_label(level) + .option-description + = visibility_level_description(level, form_model) diff --git a/app/views/shared/_visibility_radios.html.haml b/app/views/shared/_visibility_radios.html.haml index 80532c9187bd5589c2fa4a72bd4d4471a927ba12..19724eb46c1ba1ea534923e5977a8695caadbfc2 100644 --- a/app/views/shared/_visibility_radios.html.haml +++ b/app/views/shared/_visibility_radios.html.haml @@ -1,16 +1,15 @@ -- Gitlab::VisibilityLevel.values.each do |level| +- Gitlab::VisibilityLevel.values_for(form_model, current_user).each do |level| - disallowed = disallowed_visibility_level?(form_model, level) - restricted = restricted_visibility_levels.include?(level) - next if disallowed || restricted + - params = { form: form, form_model: form_model, model_method: model_method, selected_level: selected_level, level: level } + .form-check - = form.radio_button model_method, level, checked: (selected_level == level), class: 'form-check-input', data: { track_label: "blank_project", track_event: "activate_form_input", track_property: "#{model_method}_#{level}", track_value: "", qa_selector: "#{visibility_level_label(level).downcase}_radio" } - = form.label "#{model_method}_#{level}", class: 'form-check-label' do - = visibility_level_icon(level) - .option-title - = visibility_level_label(level) - .option-description - = visibility_level_description(level, form_model) + - if Feature.enabled?(:secret_snippets, current_user) && form_model.is_a?(PersonalSnippet) + = render 'snippets/visibility_radio_option', params + - else + = render 'shared/visibility_radio_option', params .text-muted - if all_visibility_levels_restricted? diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml index 73401029da43af2295fa8e3994d96402387b468b..91fca29cfa4f4038af8b7795b30ae7f0a87b1e94 100644 --- a/app/views/shared/snippets/_form.html.haml +++ b/app/views/shared/snippets/_form.html.haml @@ -6,6 +6,9 @@ html: { class: "snippet-form js-requires-input js-quick-submit common-note-form" } do |f| = form_errors(@snippet) + - if @snippet.is_a?(PersonalSnippet) && Feature.enabled?(:secret_snippets, current_user) + = f.hidden_field :secret, class: 'snippet_secret' + .form-group.row .col-sm-2.col-form-label = f.label :title diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index 1243bdab6ddacd4548f51fd664fbde1c6692cc51..02e389c24468eda54b313ef110d42dc26166fbe0 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -2,8 +2,8 @@ .detail-page-header-body .snippet-box.qa-snippet-box.has-tooltip.inline.append-right-5{ title: snippet_visibility_level_description(@snippet.visibility_level, @snippet), data: { container: "body" } } %span.sr-only - = visibility_level_label(@snippet.visibility_level) - = visibility_level_icon(@snippet.visibility_level, fw: false) + = snippet_visibility_level_label(@snippet) + = snippet_visibility_level_icon(@snippet) %span.creator Authored = time_ago_with_tooltip(@snippet.created_at, placement: 'bottom', html_class: 'snippet_updated_ago') diff --git a/app/views/shared/snippets/_snippet.html.haml b/app/views/shared/snippets/_snippet.html.haml index f38e30b0c550d742c14acf67e4fe0bf7bef5b112..264c0068e54f60cdb917a94e89f85d16ce0ba765 100644 --- a/app/views/shared/snippets/_snippet.html.haml +++ b/app/views/shared/snippets/_snippet.html.haml @@ -19,8 +19,8 @@ = notes_count %li %span.sr-only - = visibility_level_label(snippet.visibility_level) - = visibility_level_icon(snippet.visibility_level, fw: false) + = snippet_visibility_level_label(snippet) + = snippet_visibility_level_icon(snippet) .snippet-info #{snippet.to_reference} · diff --git a/app/views/snippets/_snippets_scope_menu.html.haml b/app/views/snippets/_snippets_scope_menu.html.haml index cb59b11ca2b34fbb6cbd827b7c9d5713d90c2800..8e17b6299533395b44beb03ea808196a79581ca7 100644 --- a/app/views/snippets/_snippets_scope_menu.html.haml +++ b/app/views/snippets/_snippets_scope_menu.html.haml @@ -24,6 +24,13 @@ %span.badge.badge-pill = subject.snippets.are_internal.count + - if Feature.enabled?(:secret_snippets, current_user) && include_secret + %li{ class: active_when(params[:scope] == "are_secret") } + = link_to subject_snippets_path(subject, scope: 'are_secret') do + = s_('Secret') + %span.badge.badge-pill + = subject.snippets.are_secret.count + %li{ class: active_when(params[:scope] == "are_public") } = link_to subject_snippets_path(subject, scope: 'are_public') do = _("Public") diff --git a/app/views/snippets/_visibility_radio_option.html.haml b/app/views/snippets/_visibility_radio_option.html.haml new file mode 100644 index 0000000000000000000000000000000000000000..16ff1718b47a48854746e305726c499a017b5891 --- /dev/null +++ b/app/views/snippets/_visibility_radio_option.html.haml @@ -0,0 +1,12 @@ +- model_method = :visibility_level +- is_secret = snippet_visibility_secret?(level) && form_model.secret? +- is_visibility_level = form_model.visibility_level == level +- value = snippet_visibility_secret?(level) ? Snippet::PUBLIC : level + += form.radio_button model_method, value, checked: (is_secret || is_visibility_level), id: "#{form_model.class.underscore}_#{model_method}_#{level}", class: 'form-check-input', data: { track_label: "blank_project", track_event: "activate_form_input", track_property: "#{model_method}_#{level}", track_value: "", qa_selector: "#{snippet_visibility_level_label(level).downcase}_radio" } += form.label "#{model_method}_#{level}", class: 'form-check-label' do + = snippet_visibility_level_icon(level) + .option-title + = snippet_visibility_level_label(level) + .option-description + = snippet_visibility_level_description(level) diff --git a/changelogs/unreleased/13235-secret-snippets.yml b/changelogs/unreleased/13235-secret-snippets.yml new file mode 100644 index 0000000000000000000000000000000000000000..e2635abe6977daba43cdac117b544d8ad9b4e31c --- /dev/null +++ b/changelogs/unreleased/13235-secret-snippets.yml @@ -0,0 +1,5 @@ +--- +title: Support Secret Snippets +merge_request: 24042 +author: +type: changed diff --git a/ee/app/finders/ee/snippets_finder.rb b/ee/app/finders/ee/snippets_finder.rb index 2f1904ca155e6659b8a598c5e63245523242c0c7..c012ba9141b34d6ba45b38f8ddc2842c8c22b4f7 100644 --- a/ee/app/finders/ee/snippets_finder.rb +++ b/ee/app/finders/ee/snippets_finder.rb @@ -43,7 +43,7 @@ def restricted_personal_snippets elsif current_user current_user.snippets else - ::Snippet.public_to_user + ::Snippet.public_to_user.without_secret end.only_personal_snippets end end diff --git a/ee/spec/finders/snippets_finder_spec.rb b/ee/spec/finders/snippets_finder_spec.rb index e37650eb027fcda937509ab7af976f2d2a178ea0..255b149aae9216d12ff2518f5afbd8801103ce35 100644 --- a/ee/spec/finders/snippets_finder_spec.rb +++ b/ee/spec/finders/snippets_finder_spec.rb @@ -32,9 +32,11 @@ let_it_be(:private_personal_snippet) { create(:personal_snippet, :private, author: user) } let_it_be(:internal_personal_snippet) { create(:personal_snippet, :internal, author: user) } let_it_be(:public_personal_snippet) { create(:personal_snippet, :public, author: user) } + let_it_be(:secret_personal_snippet) { create(:personal_snippet, :secret, author: user) } let_it_be(:other_private_personal_snippet) { create(:personal_snippet, :private, author: other_user) } let_it_be(:other_internal_personal_snippet) { create(:personal_snippet, :internal, author: other_user) } let_it_be(:other_public_personal_snippet) { create(:personal_snippet, :public, author: other_user) } + let_it_be(:other_secret_personal_snippet) { create(:personal_snippet, :secret, author: other_user) } let_it_be(:other_private_project_snippet) { create(:project_snippet, :private, project: other_project, author: other_user) } let_it_be(:other_internal_project_snippet) { create(:project_snippet, :internal, project: other_project, author: other_user) } let_it_be(:other_public_project_snippet) { create(:project_snippet, :public, project: other_project, author: other_user) } @@ -52,7 +54,7 @@ context 'when user is not a member of any project' do it 'returns only user personal snippets' do - expect(subject).to match_array([public_personal_snippet, internal_personal_snippet, private_personal_snippet]) + expect(subject).to match_array([public_personal_snippet, internal_personal_snippet, private_personal_snippet, secret_personal_snippet]) end end @@ -66,6 +68,7 @@ public_personal_snippet, internal_personal_snippet, private_personal_snippet, + secret_personal_snippet, public_project_snippet, internal_project_snippet, private_project_snippet @@ -83,6 +86,7 @@ public_personal_snippet, internal_personal_snippet, private_personal_snippet, + secret_personal_snippet, public_project_snippet, internal_project_snippet, private_project_snippet, @@ -102,7 +106,7 @@ end it 'returns only user personal snippets' do - expect(subject).to contain_exactly(public_personal_snippet, internal_personal_snippet, private_personal_snippet) + expect(subject).to contain_exactly(public_personal_snippet, internal_personal_snippet, private_personal_snippet, secret_personal_snippet) end end end @@ -117,6 +121,7 @@ public_personal_snippet, internal_personal_snippet, private_personal_snippet, + secret_personal_snippet, public_project_snippet, internal_project_snippet, private_project_snippet diff --git a/ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb b/ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb index 21fa027b68173cd8ceab59a2e29a822776c40b8e..6be16cf09ecaaeaa88cc6181034d555802328a6b 100644 --- a/ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb +++ b/ee/spec/lib/ee/gitlab/snippet_search_results_spec.rb @@ -2,8 +2,11 @@ require 'spec_helper' describe Gitlab::SnippetSearchResults do - let!(:snippet) { create(:snippet, content: 'foo', file_name: 'foo') } - let(:user) { snippet.author } + let_it_be(:user) { create(:user) } + let_it_be(:snippet) { create(:snippet, author: user, content: 'foo', file_name: 'foo') } + let_it_be(:secret_snippet) { create(:personal_snippet, :secret, author: user, content: 'foo', file_name: 'foo') } + let_it_be(:other_secret_snippet) { create(:personal_snippet, :secret, content: 'foo', file_name: 'foo') } + let(:com_value) { true } let(:flag_enabled) { true } @@ -20,6 +23,10 @@ subject end + + it 'returns the amount of matched snippet titles' do + expect(subject.count).to eq(1) + end end context 'when not in Gitlab.com' do diff --git a/ee/spec/lib/gitlab/elastic/snippet_search_results_spec.rb b/ee/spec/lib/gitlab/elastic/snippet_search_results_spec.rb index 0d8ee03ccc8bbdacab498ddd0eae0752af93a1b6..e9b14d312d5c7b1e7f7840244b16758725809a76 100644 --- a/ee/spec/lib/gitlab/elastic/snippet_search_results_spec.rb +++ b/ee/spec/lib/gitlab/elastic/snippet_search_results_spec.rb @@ -87,4 +87,13 @@ expect(described_class.new(nil, 'xyz', []).snippet_blobs_count).to eq(0) end end + + context 'when the snippet is secret' do + let(:snippet) { create(:personal_snippet, :secret, content: 'foo', file_name: 'foo') } + + it 'returns nothing' do + expect(results.snippet_titles_count).to eq(0) + expect(results.snippet_blobs_count).to eq(0) + end + end end diff --git a/lib/gitlab/snippet_search_results.rb b/lib/gitlab/snippet_search_results.rb index e955ccd35daed9affbaf2c352714ed8876350228..28eea6335de4f1acec9759e9e6361fc8ac133675 100644 --- a/lib/gitlab/snippet_search_results.rb +++ b/lib/gitlab/snippet_search_results.rb @@ -47,6 +47,7 @@ def limited_snippet_blobs_count def snippets SnippetsFinder.new(current_user, finder_params) .execute + .without_secret .includes(:author) .reorder(updated_at: :desc) end diff --git a/lib/gitlab/visibility_level.rb b/lib/gitlab/visibility_level.rb index a1d462ea9f56f2add6e062886809c92c5a3ef481..c5d9912a5d9c14ae51dcca121dbf251dc4c9e4c5 100644 --- a/lib/gitlab/visibility_level.rb +++ b/lib/gitlab/visibility_level.rb @@ -113,6 +113,14 @@ def level_value(level) def string_level(level) string_options.key(level) end + + def values_for(form_model, user = nil) + if form_model.class.respond_to?(:visibility_level_values) + form_model.class.visibility_level_values(user) + else + values + end + end end def visibility_level_decreased? diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 5c417aa1f2980c6702a98f819dd63560f96940d4..77496141e96b97bb9c7b9f21a633d733bfdb2b96 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -16354,6 +16354,9 @@ msgstr "" msgid "SnippetsEmptyState|They can be either public or private." msgstr "" +msgid "Snippets|Secret" +msgstr "" + msgid "Snowplow" msgstr "" @@ -17714,6 +17717,9 @@ msgstr "" msgid "The schedule time must be in the future!" msgstr "" +msgid "The snippet can be accessed without any authentication, but is not searchable." +msgstr "" + msgid "The snippet can be accessed without any authentication." msgstr "" @@ -21717,6 +21723,9 @@ msgstr "" msgid "score" msgstr "" +msgid "secret" +msgstr "" + msgid "security Reports|There was an error creating the merge request" msgstr "" diff --git a/spec/controllers/snippets/notes_controller_spec.rb b/spec/controllers/snippets/notes_controller_spec.rb index fd4b95ce226c9b58bc31a21cb648cf577e966205..37f38ef726cbb2522176050d8e247c507f23f402 100644 --- a/spec/controllers/snippets/notes_controller_spec.rb +++ b/spec/controllers/snippets/notes_controller_spec.rb @@ -8,36 +8,102 @@ let(:private_snippet) { create(:personal_snippet, :private) } let(:internal_snippet) { create(:personal_snippet, :internal) } let(:public_snippet) { create(:personal_snippet, :public) } + let(:secret_snippet) { create(:personal_snippet, :secret) } let(:note_on_private) { create(:note_on_personal_snippet, noteable: private_snippet) } let(:note_on_internal) { create(:note_on_personal_snippet, noteable: internal_snippet) } let(:note_on_public) { create(:note_on_personal_snippet, noteable: public_snippet) } + let(:note_on_secret) { create(:note_on_personal_snippet, noteable: secret_snippet) } describe 'GET index' do + let(:snippet_params) { { snippet_id: snippet } } + let(:params) { snippet_params} + + subject { get :index, params: params } + context 'when a snippet is public' do + let(:snippet) { public_snippet } + before do note_on_public - get :index, params: { snippet_id: public_snippet } + subject end - it "returns status 200" do + it 'returns status 200' do expect(response).to have_gitlab_http_status(200) end - it "returns not empty array of notes" do + it 'returns not empty array of notes' do expect(json_response["notes"].empty?).to be_falsey end end + context 'when a snippet is secret' do + let(:snippet) { secret_snippet } + let(:flag_value) { true } + + before do + note_on_secret + + stub_feature_flags(secret_snippets: flag_value) + + subject + end + + context 'when token is not present' do + it 'returns status 404' do + expect(response).to have_gitlab_http_status(404) + end + + context 'when secret_snippets flag is disabled' do + let(:flag_value) { false } + + it 'returns status 200' do + expect(response).to have_gitlab_http_status(200) + end + end + end + + context 'when token is invalid' do + let(:params) { snippet_params.merge(token: 'foo') } + + it 'returns status 404' do + expect(response).to have_gitlab_http_status(404) + end + + context 'when secret_snippets flag is disabled' do + let(:flag_value) { false } + + it 'returns status 200' do + expect(response).to have_gitlab_http_status(200) + end + end + end + + context 'when token is present' do + let(:params) { snippet_params.merge(token: snippet.secret_token) } + + it 'returns status 200' do + expect(response).to have_gitlab_http_status(200) + end + + it 'returns not empty array of notes' do + expect(json_response['notes'].empty?).to be_falsey + end + end + end + context 'when a snippet is internal' do + let(:snippet) { internal_snippet } + before do note_on_internal end context 'when user not logged in' do - it "returns status 404" do - get :index, params: { snippet_id: internal_snippet } + it 'returns status 404' do + subject expect(response).to have_gitlab_http_status(404) end @@ -48,8 +114,8 @@ sign_in(user) end - it "returns status 200" do - get :index, params: { snippet_id: internal_snippet } + it 'returns status 200' do + subject expect(response).to have_gitlab_http_status(200) end @@ -57,13 +123,15 @@ end context 'when a snippet is private' do + let(:snippet) { private_snippet } + before do note_on_private end context 'when user not logged in' do - it "returns status 404" do - get :index, params: { snippet_id: private_snippet } + it 'returns status 404' do + subject expect(response).to have_gitlab_http_status(404) end @@ -74,8 +142,8 @@ sign_in(user) end - it "returns status 404" do - get :index, params: { snippet_id: private_snippet } + it 'returns status 404' do + subject expect(response).to have_gitlab_http_status(404) end @@ -85,17 +153,17 @@ before do note_on_private - sign_in(private_snippet.author) + sign_in(snippet.author) end - it "returns status 200" do - get :index, params: { snippet_id: private_snippet } + it 'returns status 200' do + subject expect(response).to have_gitlab_http_status(200) end - it "returns 1 note" do - get :index, params: { snippet_id: private_snippet } + it 'returns 1 note' do + subject expect(json_response['notes'].count).to eq(1) end @@ -103,6 +171,8 @@ end context 'dont show non visible notes' do + let(:snippet) { public_snippet } + before do note_on_public @@ -111,8 +181,8 @@ expect_any_instance_of(Note).to receive(:cross_reference_not_visible_for?).and_return(true) end - it "does not return any note" do - get :index, params: { snippet_id: public_snippet } + it 'does not return any note' do + subject expect(json_response['notes'].count).to eq(0) end @@ -120,114 +190,93 @@ end describe 'POST create' do - context 'when a snippet is public' do - let(:request_params) do - { - note: attributes_for(:note_on_personal_snippet, noteable: public_snippet), - snippet_id: public_snippet.id - } - end + let(:snippet_params) do + { + note: attributes_for(:note_on_personal_snippet, noteable: snippet), + snippet_id: snippet.id + } + end + let(:request_params) { snippet_params } - before do - sign_in user - end + subject { post :create, params: request_params } + + before do + sign_in user + end + shared_examples 'creates the note' do it 'returns status 302' do - post :create, params: request_params + subject expect(response).to have_gitlab_http_status(302) end it 'creates the note' do - expect { post :create, params: request_params }.to change { Note.count }.by(1) + expect { subject }.to change { Note.count }.by(1) end end - context 'when a snippet is internal' do - let(:request_params) do - { - note: attributes_for(:note_on_personal_snippet, noteable: internal_snippet), - snippet_id: internal_snippet.id - } - end + shared_examples 'does not create the note' do + it 'returns status 404' do + subject - before do - sign_in user + expect(response).to have_gitlab_http_status(404) end - it 'returns status 302' do - post :create, params: request_params - - expect(response).to have_gitlab_http_status(302) + it 'does not create the note' do + expect { subject }.not_to change { Note.count } end + end - it 'creates the note' do - expect { post :create, params: request_params }.to change { Note.count }.by(1) - end + context 'when a snippet is public' do + let(:snippet) { public_snippet } + + it_behaves_like 'creates the note' end - context 'when a snippet is private' do - let(:request_params) do - { - note: attributes_for(:note_on_personal_snippet, noteable: private_snippet), - snippet_id: private_snippet.id - } + context 'when a snippet is secret' do + let(:snippet) { secret_snippet } + + context 'when token is not present' do + it_behaves_like 'does not create the note' end - before do - sign_in user + context 'when token is not valid' do + let(:request_params) { snippet_params.merge(token: 'foo') } + + it_behaves_like 'does not create the note' end - context 'when user is not the author' do - before do - sign_in(user) - end + context 'when token is valid' do + let(:request_params) { snippet_params.merge(token: snippet.secret_token) } - it 'returns status 404' do - post :create, params: request_params + it_behaves_like 'creates the note' + end + end - expect(response).to have_gitlab_http_status(404) - end + context 'when a snippet is internal' do + let(:snippet) { internal_snippet } - it 'does not create the note' do - expect { post :create, params: request_params }.not_to change { Note.count } - end + it_behaves_like 'creates the note' + end - context 'when user sends a snippet_id for a public snippet' do - let(:request_params) do - { - note: attributes_for(:note_on_personal_snippet, noteable: private_snippet), - snippet_id: public_snippet.id - } - end + context 'when a snippet is private' do + let(:snippet) { private_snippet } - it 'returns status 302' do - post :create, params: request_params + context 'when user is not the author' do + it_behaves_like 'does not create the note' - expect(response).to have_gitlab_http_status(302) - end + context 'when user sends a snippet_id for a public snippet' do + let(:request_params) { snippet_params.merge(snippet_id: public_snippet.id) } - it 'creates the note on the public snippet' do - expect { post :create, params: request_params }.to change { Note.count }.by(1) - expect(Note.last.noteable).to eq public_snippet - end + it_behaves_like 'creates the note' end end context 'when user is the author' do - before do - sign_in(private_snippet.author) - end - - it 'returns status 302' do - post :create, params: request_params + let(:user) { private_snippet.author } - expect(response).to have_gitlab_http_status(302) - end - - it 'creates the note' do - expect { post :create, params: request_params }.to change { Note.count }.by(1) - end + it_behaves_like 'creates the note' end end end @@ -287,27 +336,58 @@ end describe 'POST toggle_award_emoji' do - let(:note) { create(:note_on_personal_snippet, noteable: public_snippet) } + let(:snippet) { public_snippet } + let(:note) { create(:note_on_personal_snippet, noteable: snippet) } let(:emoji_name) { 'thumbsup'} + let(:snippet_params) { { snippet_id: snippet, id: note.id, name: emoji_name } } + let(:params) { snippet_params } before do sign_in(user) end - subject { post(:toggle_award_emoji, params: { snippet_id: public_snippet, id: note.id, name: emoji_name }) } + subject { post(:toggle_award_emoji, params: params) } + + shared_examples 'toggles/removes award emoji' do + it 'toggles the award emoji' do + expect { subject }.to change { note.award_emoji.count }.by(1) + + expect(response).to have_gitlab_http_status(200) + end + + it 'removes the already awarded emoji when it exists' do + create(:award_emoji, awardable: note, name: emoji_name, user: user) - it "toggles the award emoji" do - expect { subject }.to change { note.award_emoji.count }.by(1) + expect { subject }.to change { AwardEmoji.count }.by(-1) - expect(response).to have_gitlab_http_status(200) + expect(response).to have_gitlab_http_status(200) + end end - it "removes the already awarded emoji when it exists" do - create(:award_emoji, awardable: note, name: emoji_name, user: user) + it_behaves_like 'toggles/removes award emoji' + + context 'when snippet is secret' do + let(:snippet) { secret_snippet } + + context 'when token is not present' do + it 'returns status 404' do + expect(subject).to have_gitlab_http_status(404) + end + end + + context 'when token is invalid' do + let(:params) { snippet_params.merge(token: 'foo') } - expect { subject }.to change { AwardEmoji.count }.by(-1) + it 'returns status 404' do + expect(subject).to have_gitlab_http_status(404) + end + end + + context 'when token is valid' do + let(:params) { snippet_params.merge(token: snippet.secret_token) } - expect(response).to have_gitlab_http_status(200) + it_behaves_like 'toggles/removes award emoji' + end end end end diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 510db4374c05d40097a15ffadea44b723f565b4f..14c1519d8a83e45d3bb5f9ead3e15dfc9b2471f7 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -5,9 +5,79 @@ describe SnippetsController do let(:user) { create(:user) } - describe 'GET #index' do - let(:user) { create(:user) } + shared_examples 'allow read access to secret snippets' do + let_it_be(:secret_snippet) { create(:personal_snippet, :secret) } + let(:author) { secret_snippet.author } + let(:snippet_params) { { id: secret_snippet.to_param } } + let(:secret_token) { secret_snippet.secret_token } + let(:params) { snippet_params } + + before do + sign_in(user) if user + + subject + end + + context 'when not signed in' do + let(:user) { nil } + + it 'redirects to the sign in page' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'when user is not the author' do + context 'when the token is not present' do + it 'responds with status 404' do + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the token is not valid' do + let(:params) { snippet_params.merge(token: 'foo') } + + it 'responds with status 404' do + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the token is valid' do + let(:params) { snippet_params.merge(token: secret_token) } + + it 'responds with status 200' do + expect(response).to have_gitlab_http_status(200) + end + end + end + + context 'when user is the author' do + let(:user) { author } + + context 'when the token is not present' do + it 'responds with status 200' do + expect(response).to have_gitlab_http_status(200) + end + end + context 'when the token is not valid' do + let(:params) { snippet_params.merge(token: 'foo') } + + it 'responds with status 200' do + expect(response).to have_gitlab_http_status(200) + end + end + + context 'when the token is valid' do + let(:params) { snippet_params.merge(token: secret_token) } + + it 'responds with status 200' do + expect(response).to have_gitlab_http_status(200) + end + end + end + end + + describe 'GET #index' do context 'when username parameter is present' do it_behaves_like 'paginated collection' do let(:collection) { Snippet.all } @@ -75,6 +145,10 @@ end describe 'GET #show' do + it_behaves_like 'allow read access to secret snippets' do + subject { get :show, params: params } + end + context 'when the personal snippet is private' do let(:personal_snippet) { create(:personal_snippet, :private, author: user) } @@ -332,12 +406,59 @@ def create_snippet(snippet_params = {}, additional_params = {}) end end + describe 'GET #edit' do + context 'when the snippet is secret' do + let_it_be(:snippet) { create :personal_snippet, :secret } + let(:user) { snippet.author } + let(:params) { { id: snippet.id, token: snippet.secret_token } } + + subject { get :edit, params: params } + + before do + sign_in(user) if user + + subject + end + + context 'when the user is not signed in' do + let(:user) { nil } + + it 'redirects to the sign in page' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'when the user not the author' do + let(:user) { create(:user) } + + it 'responds with 404' do + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the user is the author' do + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + + context 'without the token param' do + let(:params) { { id: snippet.id } } + + it 'responds with 200' do + expect(response).to have_gitlab_http_status(200) + end + end + end + end + end + describe 'PUT #update' do let(:project) { create :project } - let(:snippet) { create :personal_snippet, author: user, project: project, visibility_level: visibility_level } + let(:snippet) { create :personal_snippet, visibility_level } + let(:user) { snippet.author } def update_snippet(snippet_params = {}, additional_params = {}) - sign_in(user) + sign_in(user) if user put :update, params: { id: snippet.id, @@ -355,7 +476,7 @@ def update_snippet(snippet_params = {}, additional_params = {}) end context 'when the snippet is private' do - let(:visibility_level) { Snippet::PRIVATE } + let(:visibility_level) { :private } it 'updates the snippet' do expect { update_snippet(title: 'Foo') } @@ -364,7 +485,7 @@ def update_snippet(snippet_params = {}, additional_params = {}) end context 'when a private snippet is made public' do - let(:visibility_level) { Snippet::PRIVATE } + let(:visibility_level) { :private } it 'rejects the snippet' do expect { update_snippet(title: 'Foo', visibility_level: Snippet::PUBLIC) } @@ -409,7 +530,7 @@ def update_snippet(snippet_params = {}, additional_params = {}) end context 'when the snippet is public' do - let(:visibility_level) { Snippet::PUBLIC } + let(:visibility_level) { :public } it 'rejects the shippet' do expect { update_snippet(title: 'Foo') } @@ -453,6 +574,39 @@ def update_snippet(snippet_params = {}, additional_params = {}) end end end + + context 'when the snippet is secret' do + let(:visibility_level) { :secret } + + subject { update_snippet({ title: 'Foo' }, { token: snippet.secret_token }) } + + before do + subject + end + + context 'when the user is not authenticated' do + let(:user) { nil } + + it 'redirects to the sign in page' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'when the user is not the author' do + let(:user) { create(:user) } + + it 'responds with 404' do + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the user is the author' do + it 'responds with 302' do + expect(response).to have_gitlab_http_status(302) + expect(snippet.title).to eq 'Foo' + end + end + end end describe 'POST #mark_as_spam' do @@ -481,6 +635,10 @@ def mark_as_spam end describe "GET #raw" do + it_behaves_like 'allow read access to secret snippets' do + subject { get :raw, params: params } + end + context 'when the personal snippet is private' do let(:personal_snippet) { create(:personal_snippet, :private, author: user) } @@ -625,32 +783,58 @@ def mark_as_spam end context 'award emoji on snippets' do - let(:personal_snippet) { create(:personal_snippet, :public, author: user) } - let(:another_user) { create(:user) } + let(:personal_snippet) { create(:personal_snippet, visibility, author: user) } + let(:snippet_params) { { id: personal_snippet.to_param, name: "thumbsup" } } + let(:params) { snippet_params } + let_it_be(:another_user) { create(:user) } before do sign_in(another_user) end - describe 'POST #toggle_award_emoji' do - it "toggles the award emoji" do + shared_examples 'handles award emoji' do + it 'toggles the award emoji' do expect do - post(:toggle_award_emoji, params: { id: personal_snippet.to_param, name: "thumbsup" }) + toggle_award_emoji(params) end.to change { personal_snippet.award_emoji.count }.from(0).to(1) expect(response.status).to eq(200) end - it "removes the already awarded emoji" do - post(:toggle_award_emoji, params: { id: personal_snippet.to_param, name: "thumbsup" }) + it 'removes the already awarded emoji' do + toggle_award_emoji(params) expect do - post(:toggle_award_emoji, params: { id: personal_snippet.to_param, name: "thumbsup" }) + toggle_award_emoji(params) end.to change { personal_snippet.award_emoji.count }.from(1).to(0) expect(response.status).to eq(200) end end + + describe 'POST #toggle_award_emoji' do + it_behaves_like 'handles award emoji' do + let(:visibility) { :public } + end + + context 'when snippet is secret' do + let(:visibility) { :secret } + + it_behaves_like 'handles award emoji' do + let(:params) { snippet_params.merge(token: personal_snippet.secret_token) } + end + + it 'does not toggle the award emoji when token is not present' do + toggle_award_emoji(params) + + expect(response.status).to eq(404) + end + end + end + + def toggle_award_emoji(params = {}) + post(:toggle_award_emoji, params: params) + end end describe 'POST #preview_markdown' do @@ -664,4 +848,50 @@ def mark_as_spam expect(json_response.keys).to match_array(%w(body references)) end end + + describe "DELETE #destroy" do + context 'when the snippet is secret' do + let_it_be(:snippet) { create :personal_snippet, :secret } + let(:user) { snippet.author } + let(:params) { { id: snippet.id, token: snippet.secret_token } } + + subject { delete :destroy, params: params } + + before do + sign_in(user) if user + + subject + end + + context 'when the user is not signed in' do + let(:user) { nil } + + it 'redirects to the sign in page' do + expect(response).to redirect_to(new_user_session_path) + end + end + + context 'when the user not the author' do + let(:user) { create(:user) } + + it 'responds with 404' do + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the user is the author' do + it 'responds with 302' do + expect(response).to have_gitlab_http_status(302) + end + + context 'without the token param' do + let(:params) { { id: snippet.id } } + + it 'responds with 200' do + expect(response).to have_gitlab_http_status(302) + end + end + end + end + end end diff --git a/spec/controllers/uploads_controller_spec.rb b/spec/controllers/uploads_controller_spec.rb index f35babc1b56361025552e4d1252cb2106ebff5f8..6bb7c9c5bd85d61488ec394587f69389f770ce8a 100644 --- a/spec/controllers/uploads_controller_spec.rb +++ b/spec/controllers/uploads_controller_spec.rb @@ -25,11 +25,62 @@ let!(:user) { create(:user, avatar: fixture_file_upload("spec/fixtures/dk.png", "image/png")) } describe 'POST #authorize' do + let(:uploader_class) { PersonalFileUploader } + let(:params) do + { model: 'personal_snippet', id: model.id } + end + it_behaves_like 'handle uploads authorize' do - let(:uploader_class) { PersonalFileUploader } let(:model) { create(:personal_snippet, :public) } - let(:params) do - { model: 'personal_snippet', id: model.id } + end + + context 'when snippet is secret' do + let(:model) { create(:personal_snippet, :secret) } + + context 'when the user can admin the snippet' do + it_behaves_like 'handle uploads authorize' + end + + context 'when the user cannot admin the snippet' do + before do + sign_in(user) + end + + context 'when the token is not present' do + it 'returns 404 status' do + expect(post_authorize.status).to eq(404) + end + end + + context 'when the token is not valid' do + let(:params) do + { model: 'personal_snippet', id: model.id, token: 'foo' } + end + + it 'returns 404 status' do + expect(post_authorize.status).to eq(404) + end + end + + context 'when the token is valid' do + let(:params) do + { model: 'personal_snippet', id: model.id, token: model.secret_token } + end + + before do + post_authorize + end + + it 'responds with status 200' do + expect(response.status).to eq 200 + end + + it 'uses the gitlab-workhorse content type' do + post_authorize + + expect(response.headers["Content-Type"]).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE) + end + end end end end @@ -44,7 +95,7 @@ context 'when a user does not have permissions to upload a file' do it "returns 401 when the user is not logged in" do - post :create, params: { model: model, id: snippet.id }, format: :json + create_request expect(response).to have_gitlab_http_status(401) end @@ -53,10 +104,20 @@ private_snippet = create(:personal_snippet, :private) sign_in(user) - post :create, params: { model: model, id: private_snippet.id }, format: :json + create_request(id: private_snippet.id) expect(response).to have_gitlab_http_status(404) end + + context 'when the snippet is secret' do + let(:snippet) { create(:personal_snippet, :secret) } + + it "returns 401 when the user is not logged in" do + create_request + + expect(response).to have_gitlab_http_status(401) + end + end end context 'when a user is logged in' do @@ -65,29 +126,29 @@ end it "returns an error without file" do - post :create, params: { model: model, id: snippet.id }, format: :json + create_request expect(response).to have_gitlab_http_status(422) end it "returns an error with invalid model" do - expect { post :create, params: { model: 'invalid', id: snippet.id }, format: :json } + expect { create_request(model: 'invalid') } .to raise_error(ActionController::UrlGenerationError) end it "returns 404 status when object not found" do - post :create, params: { model: model, id: 9999 }, format: :json + create_request(id: 9999) expect(response).to have_gitlab_http_status(404) end - context 'with valid image' do + shared_examples 'sucessfully uploads the file' do before do - post :create, params: { model: 'personal_snippet', id: snippet.id, file: jpg }, format: :json + request end it 'returns a content with original filename, new link, and correct type.' do - expect(response.body).to match '\"alt\":\"rails_sample\"' + expect(response.body).to match "\"alt\":\"#{file_name}\"" expect(response.body).to match "\"url\":\"/uploads" end @@ -101,26 +162,82 @@ end end - context 'with valid non-image file' do - before do - post :create, params: { model: 'personal_snippet', id: snippet.id, file: txt }, format: :json + context 'with valid image' do + it_behaves_like 'sucessfully uploads the file' do + let(:request) { create_request(file: jpg)} + let(:file_name) { 'rails_sample' } end - it 'returns a content with original filename, new link, and correct type.' do - expect(response.body).to match '\"alt\":\"doc_sample.txt\"' - expect(response.body).to match "\"url\":\"/uploads" + context 'when the snippet is secret' do + let(:snippet) { create(:personal_snippet, :secret) } + + context 'when the token is not present' do + it 'returns 404' do + create_request(file: jpg) + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the token is not valid' do + it 'returns 404' do + create_request(file: jpg, token: 'foo') + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the token is valid' do + it_behaves_like 'sucessfully uploads the file' do + let(:request) { create_request(file: jpg, token: snippet.secret_token)} + let(:file_name) { 'rails_sample' } + end + end end + end - it 'creates a corresponding Upload record' do - upload = Upload.last + context 'with valid non-image file' do + it_behaves_like 'sucessfully uploads the file' do + let(:request) { create_request(file: txt)} + let(:file_name) { 'doc_sample.txt' } + end - aggregate_failures do - expect(upload).to exist - expect(upload.model).to eq snippet + context 'when the snippet is secret' do + let(:snippet) { create(:personal_snippet, :secret) } + + context 'when the token is not present' do + it 'returns 404' do + create_request(file: txt) + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the token is not valid' do + it 'returns 404' do + create_request(file: txt, token: 'foo') + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when the token is valid' do + it_behaves_like 'sucessfully uploads the file' do + let(:request) { create_request(file: txt, token: snippet.secret_token)} + let(:file_name) { 'doc_sample.txt' } + end end end end end + + def create_request(model: 'personal_snippet', id: snippet.id, file: nil, token: nil) + params = { model: model, id: id } + params[:file] = file if file + params[:token] = token if token + + post :create, params: params, format: :json + end end context 'user uploads' do @@ -651,6 +768,6 @@ def post_authorize(verified: true) request.headers.merge!(workhorse_internal_api_request_header) if verified - post :authorize, params: { model: 'personal_snippet', id: model.id }, format: :json + post :authorize, params: params, format: :json end end diff --git a/spec/features/dashboard/snippets_spec.rb b/spec/features/dashboard/snippets_spec.rb index 4fb01995cb03de4e194e422e37e8804f5e48b47d..eef753549132d71bcf2158be4f10ce51874689fd 100644 --- a/spec/features/dashboard/snippets_spec.rb +++ b/spec/features/dashboard/snippets_spec.rb @@ -37,22 +37,31 @@ create(:personal_snippet, :public, author: user), create(:personal_snippet, :internal, author: user), create(:personal_snippet, :private, author: user), + create(:personal_snippet, :secret, author: user), create(:personal_snippet, :public) ] end + let(:flag_value) { true } before do + stub_feature_flags(secret_snippets: flag_value) + sign_in(user) visit dashboard_snippets_path end + it 'contains 5 snippet sections' do + expect(page).to have_selector('.snippet-scope-menu li', count: 5) + end + it 'contains all snippets of logged user' do - expect(page).to have_selector('.snippet-row', count: 3) + expect(page).to have_selector('.snippet-row', count: 4) expect(page).to have_content(snippets[0].title) expect(page).to have_content(snippets[1].title) expect(page).to have_content(snippets[2].title) + expect(page).to have_content(snippets[3].title) end it 'contains all private snippets of logged user when clicking on private' do @@ -75,5 +84,20 @@ expect(page).to have_selector('.snippet-row', count: 1) expect(page).to have_content(snippets[0].title) end + + it 'contains all secret snippets of logged user when clicking on secret' do + click_link('Secret') + + expect(page).to have_selector('.snippet-row', count: 1) + expect(page).to have_content(snippets[3].title) + end + + context 'when secret_snippets feature flag is disabled' do + let(:flag_value) { false } + + it 'does not contain secret snippets section' do + expect(page).to have_selector('.snippet-scope-menu li', count: 4) + end + end end end diff --git a/spec/features/projects/snippets/user_updates_snippet_spec.rb b/spec/features/projects/snippets/user_updates_snippet_spec.rb index c7ff4f89fd613178fbfd0f52200aca126b169ef4..29af0caf98095118d92559ada74bd71b8215804b 100644 --- a/spec/features/projects/snippets/user_updates_snippet_spec.rb +++ b/spec/features/projects/snippets/user_updates_snippet_spec.rb @@ -24,4 +24,12 @@ expect(page).to have_content('Snippet new title') end + + it 'does not include secret hidden field' do + page.within('.detail-page-header') do + first(:link, 'Edit').click + end + + expect(page).not_to have_css('input.snippet_secret', visible: false) + end end diff --git a/spec/features/projects/snippets/user_views_snippets_spec.rb b/spec/features/projects/snippets/user_views_snippets_spec.rb index 5739c9510a8b06cf9b940af00657cf3abaf9c832..1936a0e2414712fea94c95736db9ca881b3fb389 100644 --- a/spec/features/projects/snippets/user_views_snippets_spec.rb +++ b/spec/features/projects/snippets/user_views_snippets_spec.rb @@ -31,4 +31,8 @@ expect(page).to have_link(project_snippet.title, href: project_snippet_path(project, project_snippet)) expect(page).not_to have_content(snippet.title) end + + it 'contains 4 snippet sections' do + expect(page).to have_selector('.snippet-scope-menu li', count: 4) + end end diff --git a/spec/features/snippets/secret_snippets_spec.rb b/spec/features/snippets/secret_snippets_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..14cdbaeae9414261ccced1d3dcb4e22fc01d2a91 --- /dev/null +++ b/spec/features/snippets/secret_snippets_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Secret Snippets', :js do + let_it_be(:snippet) { create(:personal_snippet, :secret) } + let_it_be(:user) { create(:user) } + let(:author) { snippet.author } + let(:token) { snippet.secret_token } + + before do + sign_in(user) if user + end + + shared_examples 'user cannot access' do + it 'secret snippet' do + visit snippet_path(snippet) + wait_for_requests + + expect(page).not_to have_content(snippet.content) + end + + it 'raw secret snippet' do + visit raw_snippet_path(snippet) + + expect(page).not_to have_content(snippet.content) + end + end + + shared_examples 'user can access' do + it 'secret snippet' do + visit snippet_path(snippet, token: token) + wait_for_requests + + expect(page).to have_content(snippet.content) + end + + it 'raw secret snippet' do + visit raw_snippet_path(snippet, token: token) + + expect(page).to have_content(snippet.content) + end + end + + context 'when user is unauthenticated' do + let(:user) { nil } + + context 'without the snippet token' do + it_behaves_like 'user cannot access' + end + + context 'with the snippet token' do + it_behaves_like 'user can access' + end + end + + context 'when user is authenticated' do + context 'without the snippet token' do + it_behaves_like 'user cannot access' + end + + context 'with the snippet token' do + it_behaves_like 'user can access' + end + end + + context 'when user is the author' do + let(:user) { author } + + context 'without the snippet token' do + let(:token) { nil } + + it_behaves_like 'user can access' + end + + context 'with the snippet token' do + it_behaves_like 'user can access' + end + end + + context 'when user is an admin uthor' do + let(:user) { create(:admin) } + + context 'without the snippet token' do + let(:token) { nil } + + it_behaves_like 'user can access' + end + + context 'with the snippet token' do + it_behaves_like 'user can access' + end + end +end diff --git a/spec/features/snippets/user_edits_snippet_spec.rb b/spec/features/snippets/user_edits_snippet_spec.rb index 1d26660a4f627f850400a470df5e5993b9cb70e0..338b97f47b9f69f97821853484a05244e443bcdc 100644 --- a/spec/features/snippets/user_edits_snippet_spec.rb +++ b/spec/features/snippets/user_edits_snippet_spec.rb @@ -58,4 +58,13 @@ expect(page).to have_no_xpath("//i[@class='fa fa-lock']") expect(page).to have_xpath("//i[@class='fa fa-globe']") end + + it 'updates the snippet to make it secret' do + choose 'Secret' + + click_button 'Save changes' + wait_for_requests + + expect(page).to have_xpath("//i[@class='fa fa-user-secret']") + end end diff --git a/spec/features/snippets/user_snippets_spec.rb b/spec/features/snippets/user_snippets_spec.rb index d9faea55b29b618f94a99cf65fb0b84a441eb72c..9a90c1558d7a9df0d597f6e72f897a71ac8bcf1c 100644 --- a/spec/features/snippets/user_snippets_spec.rb +++ b/spec/features/snippets/user_snippets_spec.rb @@ -3,10 +3,11 @@ require 'spec_helper' describe 'User Snippets' do - let(:author) { create(:user) } - let!(:public_snippet) { create(:personal_snippet, :public, author: author, title: "This is a public snippet") } - let!(:internal_snippet) { create(:personal_snippet, :internal, author: author, title: "This is an internal snippet") } - let!(:private_snippet) { create(:personal_snippet, :private, author: author, title: "This is a private snippet") } + let_it_be(:author) { create(:user) } + let_it_be(:public_snippet) { create(:personal_snippet, :public, author: author, title: "This is a public snippet") } + let_it_be(:internal_snippet) { create(:personal_snippet, :internal, author: author, title: "This is an internal snippet") } + let_it_be(:private_snippet) { create(:personal_snippet, :private, author: author, title: "This is a private snippet") } + let_it_be(:secret_snippet) { create(:personal_snippet, :secret, author: author, title: "This is a secret snippet") } before do sign_in author @@ -17,6 +18,7 @@ expect(page).to have_link(public_snippet.title, href: snippet_path(public_snippet)) expect(page).to have_link(internal_snippet.title, href: snippet_path(internal_snippet)) expect(page).to have_link(private_snippet.title, href: snippet_path(private_snippet)) + expect(page).to have_link(secret_snippet.title, href: snippet_path(secret_snippet, token: secret_snippet.secret_token)) end it 'View my public snippets' do @@ -27,6 +29,7 @@ expect(page).to have_content(public_snippet.title) expect(page).not_to have_content(internal_snippet.title) expect(page).not_to have_content(private_snippet.title) + expect(page).not_to have_content(secret_snippet.title) end it 'View my internal snippets' do @@ -37,6 +40,7 @@ expect(page).not_to have_content(public_snippet.title) expect(page).to have_content(internal_snippet.title) expect(page).not_to have_content(private_snippet.title) + expect(page).not_to have_content(secret_snippet.title) end it 'View my private snippets' do @@ -47,5 +51,17 @@ expect(page).not_to have_content(public_snippet.title) expect(page).not_to have_content(internal_snippet.title) expect(page).to have_content(private_snippet.title) + expect(page).not_to have_content(secret_snippet.title) + end + + it 'View my secret snippets' do + page.within('.snippet-scope-menu') do + click_link "Secret" + end + + expect(page).not_to have_content(public_snippet.title) + expect(page).not_to have_content(internal_snippet.title) + expect(page).not_to have_content(private_snippet.title) + expect(page).to have_link(secret_snippet.title, href: snippet_path(secret_snippet, token: secret_snippet.secret_token)) end end diff --git a/spec/finders/snippets_finder_spec.rb b/spec/finders/snippets_finder_spec.rb index 8f83cb777094b0a0fe93b8047f5f68c86e47bc12..23a27cb78b047f2a586dff1667b9520e728b21f2 100644 --- a/spec/finders/snippets_finder_spec.rb +++ b/spec/finders/snippets_finder_spec.rb @@ -18,6 +18,7 @@ describe '#execute' do let_it_be(:user) { create(:user) } + let_it_be(:other_user) { create(:user) } let_it_be(:admin) { create(:admin) } let_it_be(:group) { create(:group, :public) } let_it_be(:project) { create(:project, :public, group: group) } @@ -25,6 +26,8 @@ let_it_be(:private_personal_snippet) { create(:personal_snippet, :private, author: user) } let_it_be(:internal_personal_snippet) { create(:personal_snippet, :internal, author: user) } let_it_be(:public_personal_snippet) { create(:personal_snippet, :public, author: user) } + let_it_be(:secret_personal_snippet) { create(:personal_snippet, :secret, author: user) } + let_it_be(:other_secret_personal_snippet) { create(:personal_snippet, :secret) } let_it_be(:private_project_snippet) { create(:project_snippet, :private, project: project) } let_it_be(:internal_project_snippet) { create(:project_snippet, :internal, project: project) } @@ -32,84 +35,113 @@ context 'filter by scope' do it "returns all snippets for 'all' scope" do - snippets = described_class.new(user, scope: :all).execute - - expect(snippets).to contain_exactly( + expect(find_snippets(:all)).to contain_exactly( private_personal_snippet, internal_personal_snippet, public_personal_snippet, - internal_project_snippet, public_project_snippet + internal_project_snippet, public_project_snippet, secret_personal_snippet ) end it "returns all snippets for 'are_private' scope" do - snippets = described_class.new(user, scope: :are_private).execute - - expect(snippets).to contain_exactly(private_personal_snippet) + expect(find_snippets(:are_private)).to contain_exactly(private_personal_snippet) end it "returns all snippets for 'are_internal' scope" do - snippets = described_class.new(user, scope: :are_internal).execute - - expect(snippets).to contain_exactly(internal_personal_snippet, internal_project_snippet) + expect(find_snippets(:are_internal)).to contain_exactly(internal_personal_snippet, internal_project_snippet) end it "returns all snippets for 'are_public' scope" do - snippets = described_class.new(user, scope: :are_public).execute + expect(find_snippets(:are_public)).to contain_exactly(public_personal_snippet, public_project_snippet) + end - expect(snippets).to contain_exactly(public_personal_snippet, public_project_snippet) + it "returns all snippets for 'are_secret' scope" do + expect(find_snippets(:are_secret)).to contain_exactly(secret_personal_snippet) end - end - context 'filter by author' do - context 'when the author is a User object' do - it 'returns all public and internal snippets' do - snippets = described_class.new(create(:user), author: user).execute + context 'when the user it not the author' do + let(:user) { other_user } - expect(snippets).to contain_exactly(internal_personal_snippet, public_personal_snippet) + it "returns all snippets for 'all' scope except secret snippet" do + expect(find_snippets(:all)).to contain_exactly( + internal_personal_snippet, public_personal_snippet, + internal_project_snippet, public_project_snippet + ) end end - context 'when the author is the User id' do - it 'returns all public and internal snippets' do - snippets = described_class.new(create(:user), author: user.id).execute + context 'when the user is an admin' do + let(:user) { admin } - expect(snippets).to contain_exactly(internal_personal_snippet, public_personal_snippet) + it "returns all snippets for 'all' scope" do + expect(find_snippets(:all)).to contain_exactly( + private_personal_snippet, internal_personal_snippet, public_personal_snippet, + internal_project_snippet, public_project_snippet, secret_personal_snippet, + other_secret_personal_snippet, private_project_snippet + ) end - end - it 'returns internal snippets' do - snippets = described_class.new(user, author: user, scope: :are_internal).execute + it "returns all snippets for 'are_private' scope" do + expect(find_snippets(:are_private)).to contain_exactly(private_personal_snippet, private_project_snippet) + end - expect(snippets).to contain_exactly(internal_personal_snippet) - end + it "returns all snippets for 'are_internal' scope" do + expect(find_snippets(:are_internal)).to contain_exactly(internal_personal_snippet, internal_project_snippet) + end - it 'returns private snippets' do - snippets = described_class.new(user, author: user, scope: :are_private).execute + it "returns all snippets for 'are_public' scope" do + expect(find_snippets(:are_public)).to contain_exactly(public_personal_snippet, public_project_snippet) + end - expect(snippets).to contain_exactly(private_personal_snippet) + it "returns all snippets for 'are_secret' scope" do + expect(find_snippets(:are_secret)).to contain_exactly(secret_personal_snippet, other_secret_personal_snippet) + end end - it 'returns public snippets' do - snippets = described_class.new(user, author: user, scope: :are_public).execute - - expect(snippets).to contain_exactly(public_personal_snippet) + def find_snippets(scope) + described_class.new(user, scope: scope).execute end + end + + context 'filter by author' do + let(:author) { user } + + shared_examples 'scope filters with author' do + it 'returns internal snippets' do + expect(find_snippets(scope: :are_internal)).to contain_exactly(internal_personal_snippet) + end + + it 'returns private snippets' do + expect(find_snippets(scope: :are_private)).to contain_exactly(private_personal_snippet) + end - it 'returns all snippets' do - snippets = described_class.new(user, author: user).execute + it 'returns public snippets' do + expect(find_snippets(scope: :are_public)).to contain_exactly(public_personal_snippet) + end + + it 'returns secret snippets' do + expect(find_snippets(scope: :are_secret)).to contain_exactly(secret_personal_snippet) + end - expect(snippets).to contain_exactly(private_personal_snippet, internal_personal_snippet, public_personal_snippet) + it 'returns all snippets' do + expect(find_snippets).to contain_exactly(private_personal_snippet, internal_personal_snippet, public_personal_snippet, secret_personal_snippet) + end end - it 'returns only public snippets if unauthenticated user' do - snippets = described_class.new(nil, author: user).execute + it_behaves_like 'scope filters with author' do + let(:search_user) { user } + end - expect(snippets).to contain_exactly(public_personal_snippet) + context 'when the user is an admin' do + it_behaves_like 'scope filters with author' do + let(:search_user) { admin } + end end - it 'returns all snippets for an admin' do - snippets = described_class.new(admin, author: user).execute + context 'when the search user is different from author' do + let(:search_user) { other_user } - expect(snippets).to contain_exactly(private_personal_snippet, internal_personal_snippet, public_personal_snippet) + it 'returns all public and internal snippets' do + expect(find_snippets).to contain_exactly(internal_personal_snippet, public_personal_snippet) + end end context 'when author is not valid' do @@ -121,6 +153,10 @@ expect(finder.execute).to be_empty end end + + def find_snippets(scope: nil) + described_class.new(search_user, author: author, scope: scope).execute + end end context 'filter by project' do @@ -141,15 +177,11 @@ end it 'returns public and internal snippets for non project members' do - snippets = described_class.new(user, project: project).execute - - expect(snippets).to contain_exactly(internal_project_snippet, public_project_snippet) + expect(find_snippets(user)).to contain_exactly(internal_project_snippet, public_project_snippet) end it 'returns public snippets for non project members' do - snippets = described_class.new(user, project: project, scope: :are_public).execute - - expect(snippets).to contain_exactly(public_project_snippet) + expect(find_snippets(user, scope: :are_public)).to contain_exactly(public_project_snippet) end it 'returns internal snippets for non project members' do @@ -159,31 +191,23 @@ end it 'does not return private snippets for non project members' do - snippets = described_class.new(user, project: project, scope: :are_private).execute - - expect(snippets).to be_empty + expect(find_snippets(user, scope: :are_private)).to be_empty end it 'returns all snippets for project members' do project.add_developer(user) - snippets = described_class.new(user, project: project).execute - - expect(snippets).to contain_exactly(private_project_snippet, internal_project_snippet, public_project_snippet) + expect(find_snippets(user)).to contain_exactly(private_project_snippet, internal_project_snippet, public_project_snippet) end it 'returns private snippets for project members' do project.add_developer(user) - snippets = described_class.new(user, project: project, scope: :are_private).execute - - expect(snippets).to contain_exactly(private_project_snippet) + expect(find_snippets(user, scope: :are_private)).to contain_exactly(private_project_snippet) end it 'returns all snippets for an admin' do - snippets = described_class.new(admin, project: project).execute - - expect(snippets).to contain_exactly(private_project_snippet, internal_project_snippet, public_project_snippet) + expect(find_snippets(admin)).to contain_exactly(private_project_snippet, internal_project_snippet, public_project_snippet) end context 'filter by author' do @@ -215,6 +239,10 @@ expect(finder.execute).to be_empty end end + + def find_snippets(search_user, author: nil, scope: nil) + described_class.new(search_user, author: author, scope: scope, project: project).execute + end end context 'filter by snippet type' do @@ -224,7 +252,9 @@ expect(snippets).to contain_exactly(private_personal_snippet, internal_personal_snippet, - public_personal_snippet) + public_personal_snippet, + secret_personal_snippet, + other_secret_personal_snippet) end end @@ -252,26 +282,24 @@ context 'explore snippets' do it 'returns only public personal snippets for unauthenticated users' do - snippets = described_class.new(nil, explore: true).execute - - expect(snippets).to contain_exactly(public_personal_snippet) + expect(find_snippets(nil)).to contain_exactly(public_personal_snippet) end it 'also returns internal personal snippets for authenticated users' do - snippets = described_class.new(user, explore: true).execute - - expect(snippets).to contain_exactly( + expect(find_snippets(user)).to contain_exactly( internal_personal_snippet, public_personal_snippet ) end it 'returns all personal snippets for admins' do - snippets = described_class.new(admin, explore: true).execute - - expect(snippets).to contain_exactly( + expect(find_snippets(admin)).to contain_exactly( private_personal_snippet, internal_personal_snippet, public_personal_snippet ) end + + def find_snippets(search_user) + described_class.new(search_user, explore: true).execute + end end context 'when the user cannot read cross project' do @@ -281,7 +309,9 @@ end it 'returns only personal snippets when the user cannot read cross project' do - expect(described_class.new(user).execute).to contain_exactly(private_personal_snippet, internal_personal_snippet, public_personal_snippet) + expect(described_class.new(user).execute).to contain_exactly( + private_personal_snippet, internal_personal_snippet, public_personal_snippet, secret_personal_snippet + ) end end end @@ -293,6 +323,8 @@ let(:project) { create(:project) } let!(:snippet) { create(:project_snippet, :public, project: project) } + subject { described_class.new(user, project: project).execute } + before do project.add_maintainer(user) end @@ -305,17 +337,13 @@ it 'includes the result if the external service allows access' do external_service_allow_access(user, project) - results = described_class.new(user, project: project).execute - - expect(results).to contain_exactly(snippet) + expect(subject).to contain_exactly(snippet) end it 'does not include any results if the external service denies access' do external_service_deny_access(user, project) - results = described_class.new(user, project: project).execute - - expect(results).to be_empty + expect(subject).to be_empty end end end diff --git a/spec/helpers/award_emoji_helper_spec.rb b/spec/helpers/award_emoji_helper_spec.rb index 2ad6b68a34ce024020b72a498a0b64253a620d29..3e50cc6367e2cdddc2d9d04f74c8b6de73bd1cf2 100644 --- a/spec/helpers/award_emoji_helper_spec.rb +++ b/spec/helpers/award_emoji_helper_spec.rb @@ -18,6 +18,16 @@ expect(subject).to eq(expected_url) end + + context 'when snippet is secret' do + let(:snippet) { create(:personal_snippet, :secret) } + + it 'returns correct url' do + expected_url = "/snippets/#{note.noteable.id}/notes/#{note.id}/toggle_award_emoji?token=#{snippet.secret_token}" + + expect(subject).to eq(expected_url) + end + end end context 'note on project item' do @@ -42,6 +52,26 @@ expect(subject).to eq(expected_url) end + + context 'when snippet is secret' do + let(:snippet) { create(:personal_snippet, :secret) } + + context 'when feature flag secret_snippets is enabled' do + it 'returns correct url' do + expected_url = "/snippets/#{snippet.id}/toggle_award_emoji?token=#{snippet.secret_token}" + + expect(subject).to eq(expected_url) + end + end + + context 'when feature flag secret_snippets is disabled' do + it 'does not include token' do + stub_feature_flags(secret_snippets: false) + + expect(subject).to eq "/snippets/#{snippet.id}/toggle_award_emoji" + end + end + end end context 'merge request' do diff --git a/spec/helpers/gitlab_routing_helper_spec.rb b/spec/helpers/gitlab_routing_helper_spec.rb index e76ebcb5637dfc89b341371fefeb23a45ba1d0d4..542becf9bf740439406fc833372a816fdf5ec85b 100644 --- a/spec/helpers/gitlab_routing_helper_spec.rb +++ b/spec/helpers/gitlab_routing_helper_spec.rb @@ -116,13 +116,27 @@ context 'snippets' do let_it_be(:personal_snippet) { create(:personal_snippet) } let_it_be(:project_snippet) { create(:project_snippet) } + let_it_be(:secret_snippet) { create(:personal_snippet, :secret) } let_it_be(:note) { create(:note_on_personal_snippet, noteable: personal_snippet) } + let_it_be(:secret_note) { create(:note_on_personal_snippet, noteable: secret_snippet) } describe '#gitlab_snippet_path' do it 'returns the personal snippet path' do expect(gitlab_snippet_path(personal_snippet)).to eq("/snippets/#{personal_snippet.id}") end + context 'with secret snippet' do + it 'returns the path with token' do + expect(gitlab_snippet_path(secret_snippet)).to eq("/snippets/#{secret_snippet.id}?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_snippet_path(secret_snippet)).to eq("/snippets/#{secret_snippet.id}") + end + end + it 'returns the project snippet path' do expect(gitlab_snippet_path(project_snippet)).to eq("/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}") end @@ -133,6 +147,18 @@ expect(gitlab_snippet_url(personal_snippet)).to eq("http://test.host/snippets/#{personal_snippet.id}") end + context 'with secret snippet' do + it 'returns the url with token' do + expect(gitlab_snippet_url(secret_snippet)).to eq("http://test.host/snippets/#{secret_snippet.id}?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_snippet_url(secret_snippet)).to eq("http://test.host/snippets/#{secret_snippet.id}") + end + end + it 'returns the project snippet url' do expect(gitlab_snippet_url(project_snippet)).to eq("http://test.host/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}") end @@ -143,6 +169,18 @@ expect(gitlab_raw_snippet_path(personal_snippet)).to eq("/snippets/#{personal_snippet.id}/raw") end + context 'with secret snippet' do + it 'returns the raw path with token' do + expect(gitlab_raw_snippet_path(secret_snippet)).to eq("/snippets/#{secret_snippet.id}/raw?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_raw_snippet_path(secret_snippet)).to eq("/snippets/#{secret_snippet.id}/raw") + end + end + it 'returns the raw project snippet path' do expect(gitlab_raw_snippet_path(project_snippet)).to eq("/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}/raw") end @@ -153,6 +191,18 @@ expect(gitlab_raw_snippet_url(personal_snippet)).to eq("http://test.host/snippets/#{personal_snippet.id}/raw") end + context 'with secret snippet' do + it 'returns the raw url with token' do + expect(gitlab_raw_snippet_url(secret_snippet)).to eq("http://test.host/snippets/#{secret_snippet.id}/raw?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_raw_snippet_url(secret_snippet)).to eq("http://test.host/snippets/#{secret_snippet.id}/raw") + end + end + it 'returns the raw project snippet url' do expect(gitlab_raw_snippet_url(project_snippet)).to eq("http://test.host/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}/raw") end @@ -162,48 +212,144 @@ it 'returns the notes path for the personal snippet' do expect(gitlab_snippet_notes_path(personal_snippet)).to eq("/snippets/#{personal_snippet.id}/notes") end + + context 'with secret snippet' do + it 'returns the notes path with token' do + expect(gitlab_snippet_notes_path(secret_snippet)).to eq("/snippets/#{secret_snippet.id}/notes?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_snippet_notes_path(secret_snippet)).to eq("/snippets/#{secret_snippet.id}/notes") + end + end end describe '#gitlab_snippet_notes_url' do it 'returns the notes url for the personal snippet' do expect(gitlab_snippet_notes_url(personal_snippet)).to eq("http://test.host/snippets/#{personal_snippet.id}/notes") end + + context 'with secret snippet' do + it 'returns the notes url with token' do + expect(gitlab_snippet_notes_url(secret_snippet)).to eq("http://test.host/snippets/#{secret_snippet.id}/notes?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_snippet_notes_url(secret_snippet)).to eq("http://test.host/snippets/#{secret_snippet.id}/notes") + end + end end describe '#gitlab_snippet_note_path' do it 'returns the note path for the personal snippet' do expect(gitlab_snippet_note_path(personal_snippet, note)).to eq("/snippets/#{personal_snippet.id}/notes/#{note.id}") end + + context 'with secret snippet' do + it 'returns the note path with token' do + expect(gitlab_snippet_note_path(secret_snippet, secret_note)).to eq("/snippets/#{secret_snippet.id}/notes/#{secret_note.id}?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_snippet_note_path(secret_snippet, secret_note)).to eq("/snippets/#{secret_snippet.id}/notes/#{secret_note.id}") + end + end end describe '#gitlab_snippet_note_url' do it 'returns the note url for the personal snippet' do expect(gitlab_snippet_note_url(personal_snippet, note)).to eq("http://test.host/snippets/#{personal_snippet.id}/notes/#{note.id}") end + + context 'with secret snippet' do + it 'returns the note url with token' do + expect(gitlab_snippet_note_url(secret_snippet, secret_note)).to eq("http://test.host/snippets/#{secret_snippet.id}/notes/#{secret_note.id}?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_snippet_note_url(secret_snippet, secret_note)).to eq("http://test.host/snippets/#{secret_snippet.id}/notes/#{secret_note.id}") + end + end end describe '#gitlab_toggle_award_emoji_snippet_note_path' do it 'returns the note award emoji path for the personal snippet' do expect(gitlab_toggle_award_emoji_snippet_note_path(personal_snippet, note)).to eq("/snippets/#{personal_snippet.id}/notes/#{note.id}/toggle_award_emoji") end + + context 'with secret snippet' do + it 'returns the note award emoji path with token' do + expect(gitlab_toggle_award_emoji_snippet_note_path(secret_snippet, secret_note)).to eq("/snippets/#{secret_snippet.id}/notes/#{secret_note.id}/toggle_award_emoji?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_toggle_award_emoji_snippet_note_path(secret_snippet, secret_note)).to eq("/snippets/#{secret_snippet.id}/notes/#{secret_note.id}/toggle_award_emoji") + end + end end describe '#gitlab_toggle_award_emoji_snippet_note_url' do it 'returns the note award emoji url for the personal snippet' do expect(gitlab_toggle_award_emoji_snippet_note_url(personal_snippet, note)).to eq("http://test.host/snippets/#{personal_snippet.id}/notes/#{note.id}/toggle_award_emoji") end + + context 'with secret snippet' do + it 'returns the note award emoji url with token' do + expect(gitlab_toggle_award_emoji_snippet_note_url(secret_snippet, secret_note)).to eq("http://test.host/snippets/#{secret_snippet.id}/notes/#{secret_note.id}/toggle_award_emoji?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_toggle_award_emoji_snippet_note_url(secret_snippet, secret_note)).to eq("http://test.host/snippets/#{secret_snippet.id}/notes/#{secret_note.id}/toggle_award_emoji") + end + end end describe '#gitlab_toggle_award_emoji_snippet_path' do it 'returns the award emoji path for the personal snippet' do expect(gitlab_toggle_award_emoji_snippet_path(personal_snippet)).to eq("/snippets/#{personal_snippet.id}/toggle_award_emoji") end + + context 'with secret snippet' do + it 'returns the award emoji path with token' do + expect(gitlab_toggle_award_emoji_snippet_path(secret_snippet)).to eq("/snippets/#{secret_snippet.id}/toggle_award_emoji?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_toggle_award_emoji_snippet_path(secret_snippet)).to eq("/snippets/#{secret_snippet.id}/toggle_award_emoji") + end + end end describe '#gitlab_toggle_award_emoji_snippet_url' do it 'returns the award url for the personal snippet' do expect(gitlab_toggle_award_emoji_snippet_url(personal_snippet)).to eq("http://test.host/snippets/#{personal_snippet.id}/toggle_award_emoji") end + + context 'with secret snippet' do + it 'returns the award emoji url with token' do + expect(gitlab_toggle_award_emoji_snippet_url(secret_snippet)).to eq("http://test.host/snippets/#{secret_snippet.id}/toggle_award_emoji?token=#{secret_snippet.secret_token}") + end + + it 'does not return token when feature flag is disabled' do + stub_feature_flags(secret_snippets: false) + + expect(gitlab_toggle_award_emoji_snippet_url(secret_snippet)).to eq("http://test.host/snippets/#{secret_snippet.id}/toggle_award_emoji") + end + end end end end diff --git a/spec/helpers/snippets_helper_spec.rb b/spec/helpers/snippets_helper_spec.rb index 6fdf4f5cfb4dd47bdbc91d541efb2c5e81f5d1e0..0db237d57490e41d30715253bae31aced1d7bcef 100644 --- a/spec/helpers/snippets_helper_spec.rb +++ b/spec/helpers/snippets_helper_spec.rb @@ -3,11 +3,12 @@ require 'spec_helper' describe SnippetsHelper do - include Gitlab::Routing include IconsHelper let_it_be(:public_personal_snippet) { create(:personal_snippet, :public) } + let_it_be(:secret_snippet) { create(:personal_snippet, :secret) } let_it_be(:public_project_snippet) { create(:project_snippet, :public) } + let(:current_user) { nil } describe '#embedded_raw_snippet_button' do subject { embedded_raw_snippet_button.to_s } @@ -17,6 +18,12 @@ expect(subject).to eq(download_link("http://test.host/snippets/#{@snippet.id}/raw")) end + it 'returns raw button of embedded snippets for secret personal snippets' do + @snippet = create(:personal_snippet, :secret) + + expect(subject).to eq(download_link("http://test.host/snippets/#{@snippet.id}/raw?token=#{@snippet.secret_token}")) + end + it 'returns view raw button of embedded snippets for project snippets' do @snippet = create(:project_snippet, :public) @@ -34,17 +41,23 @@ def download_link(url) it 'returns download button of embedded snippets for personal snippets' do @snippet = create(:personal_snippet, :public) - expect(subject).to eq(download_link("http://test.host/snippets/#{@snippet.id}/raw")) + expect(subject).to eq(download_link("http://test.host/snippets/#{@snippet.id}/raw?inline=false")) + end + + it 'returns download button of embedded snippets for secret personal snippets' do + @snippet = create(:personal_snippet, :secret) + + expect(subject).to eq(download_link("http://test.host/snippets/#{@snippet.id}/raw?inline=false&token=#{@snippet.secret_token}")) end it 'returns download button of embedded snippets for project snippets' do @snippet = create(:project_snippet, :public) - expect(subject).to eq(download_link("http://test.host/#{@snippet.project.path_with_namespace}/snippets/#{@snippet.id}/raw")) + expect(subject).to eq(download_link("http://test.host/#{@snippet.project.path_with_namespace}/snippets/#{@snippet.id}/raw?inline=false")) end def download_link(url) - "#{external_snippet_icon('download')}" + "#{external_snippet_icon('download')}" end end @@ -56,7 +69,15 @@ def download_link(url) context 'public' do it 'returns a script tag with the snippet full url' do - expect(subject).to eq(script_embed("http://test.host/snippets/#{snippet.id}")) + expect(subject).to eq(script_embed("http://test.host/snippets/#{snippet.id}.js")) + end + end + + context 'secret snippets' do + let(:snippet) { secret_snippet } + + it 'returns a script tag with the snippet full url' do + expect(subject).to eq(script_embed("http://test.host/snippets/#{snippet.id}.js?token=#{snippet.secret_token}")) end end end @@ -65,12 +86,12 @@ def download_link(url) let(:snippet) { public_project_snippet } it 'returns a script tag with the snippet full url' do - expect(subject).to eq(script_embed("http://test.host/#{snippet.project.path_with_namespace}/snippets/#{snippet.id}")) + expect(subject).to eq(script_embed("http://test.host/#{snippet.project.path_with_namespace}/snippets/#{snippet.id}.js")) end end def script_embed(url) - "" + "" end end @@ -80,8 +101,18 @@ def script_embed(url) context 'with personal snippet' do let(:snippet) { public_personal_snippet } - it 'returns the download button' do - expect(subject).to eq(download_link("/snippets/#{snippet.id}/raw")) + context 'public' do + it 'returns the download button' do + expect(subject).to eq(download_link("/snippets/#{snippet.id}/raw?inline=false")) + end + end + + context 'secret' do + let(:snippet) { secret_snippet } + + it 'returns the download button' do + expect(subject).to eq(download_link("/snippets/#{snippet.id}/raw?inline=false&token=#{snippet.secret_token}")) + end end end @@ -89,12 +120,12 @@ def script_embed(url) let(:snippet) { public_project_snippet } it 'returns the download button' do - expect(subject).to eq(download_link("/#{snippet.project.path_with_namespace}/snippets/#{snippet.id}/raw")) + expect(subject).to eq(download_link("/#{snippet.project.path_with_namespace}/snippets/#{snippet.id}/raw?inline=false")) end end def download_link(url) - "" + "" end end @@ -126,6 +157,14 @@ def download_link(url) expect(subject).to be_nil end end + + context 'when snippet is secret' do + let(:visibility) { :secret } + + it 'returns the snippet badge' do + expect(subject).to eq " secret" + end + end end describe '#snippet_embed_input' do diff --git a/spec/helpers/visibility_level_helper_spec.rb b/spec/helpers/visibility_level_helper_spec.rb index 1a176cfe965d279014ec944320e77253b40ce3e2..b983cdb4a9acac71a69595e1dd971137bbaa6c51 100644 --- a/spec/helpers/visibility_level_helper_spec.rb +++ b/spec/helpers/visibility_level_helper_spec.rb @@ -69,6 +69,8 @@ end describe "#snippet_visibility_level_description" do + let(:secret_snippet) { build(:personal_snippet, :secret) } + it 'describes visibility only for me' do expect(snippet_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE, personal_snippet)) .to eq "The snippet is visible only to me." @@ -79,6 +81,16 @@ .to eq "The snippet is visible only to project members." end + it 'describes visibility for secret snippets' do + expect(snippet_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, secret_snippet)) + .to eq "The snippet can be accessed without any authentication, but is not searchable." + end + + it 'describes visibility for public snippets' do + expect(snippet_visibility_level_description(Gitlab::VisibilityLevel::PUBLIC, personal_snippet)) + .to eq "The snippet can be accessed without any authentication." + end + it 'defaults to personal snippet' do expect(snippet_visibility_level_description(Gitlab::VisibilityLevel::PRIVATE)) .to eq "The snippet is visible only to me." @@ -229,4 +241,24 @@ it { is_expected.to eq(expected) } end end + + describe '.visibility_level_label' do + context 'PRIVATE' do + it 'returns Private' do + expect(visibility_level_label(Gitlab::VisibilityLevel::PRIVATE)).to eq('Private') + end + end + + context 'INTERNAL' do + it 'returns Internal' do + expect(visibility_level_label(Gitlab::VisibilityLevel::INTERNAL)).to eq('Internal') + end + end + + context 'PUBLIC' do + it 'returns Public' do + expect(visibility_level_label(Gitlab::VisibilityLevel::PUBLIC)).to eq('Public') + end + end + end end diff --git a/spec/lib/gitlab/snippet_search_results_spec.rb b/spec/lib/gitlab/snippet_search_results_spec.rb index 47f26fdebe259d4760172d14bebc8a6888ddaafb..a3bc1a6b56951301c368885c8fe62e6788426ea0 100644 --- a/spec/lib/gitlab/snippet_search_results_spec.rb +++ b/spec/lib/gitlab/snippet_search_results_spec.rb @@ -5,18 +5,22 @@ describe Gitlab::SnippetSearchResults do include SearchHelpers - let!(:snippet) { create(:snippet, content: 'foo', file_name: 'foo') } - let(:results) { described_class.new(snippet.author, 'foo') } + let_it_be(:user) { create(:user) } + let_it_be(:snippet) { create(:snippet, author: user, content: 'foo', file_name: 'foo') } + let_it_be(:secret_snippet) { create(:personal_snippet, :secret, author: user, content: 'foo', file_name: 'foo') } + let_it_be(:other_secret_snippet) { create(:personal_snippet, :secret, content: 'foo', file_name: 'foo') } + + subject { described_class.new(user, 'foo') } describe '#snippet_titles_count' do it 'returns the amount of matched snippet titles' do - expect(results.limited_snippet_titles_count).to eq(1) + expect(subject.limited_snippet_titles_count).to eq(1) end end describe '#snippet_blobs_count' do it 'returns the amount of matched snippet blobs' do - expect(results.limited_snippet_blobs_count).to eq(1) + expect(subject.limited_snippet_blobs_count).to eq(1) end end @@ -32,8 +36,8 @@ with_them do it 'returns the expected formatted count' do - expect(results).to receive(count_method).and_return(1234) if count_method - expect(results.formatted_count(scope)).to eq(expected) + expect(subject).to receive(count_method).and_return(1234) if count_method + expect(subject.formatted_count(scope)).to eq(expected) end end end diff --git a/spec/models/personal_snippet_spec.rb b/spec/models/personal_snippet_spec.rb index 276c8e22731b255c2ff7c6c2aca0c2f9dbb649dd..6a4a26858b3b209c8b7caef857d4d2f510b12372 100644 --- a/spec/models/personal_snippet_spec.rb +++ b/spec/models/personal_snippet_spec.rb @@ -3,6 +3,67 @@ require 'spec_helper' describe PersonalSnippet do + describe '#update_secret_snippet_data' do + let!(:snippet) { create(:personal_snippet, :public, secret: secret) } + let(:secret) { false } + + context 'when secret is false' do + it 'does not update the secret_token' do + expect(snippet.secret_token).to be_nil + end + end + + context 'when secret is true' do + let(:secret) { true } + + it 'assigns a random hex value' do + expect(snippet.secret_token).not_to be_nil + end + + it 'does not overwrite existing secret_token' do + expect do + snippet.update(title: 'foobar') + end.not_to change { snippet.secret_token } + end + + context 'when the secret flag is disabled' do + it 'sets the secret_token to nil' do + expect do + snippet.update(secret: false) + end.to change { snippet.secret_token }.to(nil) + end + end + + context 'when the visibility_level changes to any other level' do + it 'sets the secret_token to nil' do + expect do + snippet.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end.to change { snippet.secret_token }.to(nil) + end + + it 'disables the secret flag' do + expect do + snippet.update(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + end.to change { snippet.secret }.to(false) + end + end + end + end + + describe '#snippet_can_be_secret?' do + [ + { visibility: :public, result: true }, + { visibility: :private, result: false }, + { visibility: :internal, result: false } + ].each do |combination| + it 'returns true when snippet is public' do + snippet = build(:personal_snippet, combination[:visibility]) + + expect(snippet.send(:snippet_can_be_secret?)).to eq(combination[:result]) + end + end + end + describe '#embeddable?' do [ { snippet: :public, embeddable: true }, @@ -10,10 +71,31 @@ { snippet: :private, embeddable: false } ].each do |combination| it 'returns true when snippet is public' do - snippet = build(:personal_snippet, combination[:snippet]) + snippet = create(:personal_snippet, combination[:snippet]) expect(snippet.embeddable?).to eq(combination[:embeddable]) end end end + + describe '.visibility_level_values' do + it 'includes secret visibility' do + expect(described_class.visibility_level_values(nil)) + .to contain_exactly(Gitlab::VisibilityLevel::PRIVATE, + Gitlab::VisibilityLevel::INTERNAL, + Gitlab::VisibilityLevel::PUBLIC, + described_class::VISIBILITY_SECRET) + end + + context 'when secret_snippets flag is disabled' do + it 'includes secret visibility' do + stub_feature_flags(secret_snippets: false) + + expect(described_class.visibility_level_values(nil)) + .to contain_exactly(Gitlab::VisibilityLevel::PRIVATE, + Gitlab::VisibilityLevel::INTERNAL, + Gitlab::VisibilityLevel::PUBLIC) + end + end + end end diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb index 29ef040ee2ef36e37499953fac2ebc40267c5765..673b9919549466238252d4214042a269d43a716b 100644 --- a/spec/requests/api/snippets_spec.rb +++ b/spec/requests/api/snippets_spec.rb @@ -10,6 +10,7 @@ public_snippet = create(:personal_snippet, :public, author: user) private_snippet = create(:personal_snippet, :private, author: user) internal_snippet = create(:personal_snippet, :internal, author: user) + secret_snippet = create(:personal_snippet, :secret, author: user) get api("/snippets/", user) @@ -19,7 +20,8 @@ expect(json_response.map { |snippet| snippet['id']} ).to contain_exactly( public_snippet.id, internal_snippet.id, - private_snippet.id) + private_snippet.id, + secret_snippet.id) expect(json_response.last).to have_key('web_url') expect(json_response.last).to have_key('raw_url') expect(json_response.last).to have_key('visibility') @@ -27,6 +29,7 @@ it 'hides private snippets from regular user' do create(:personal_snippet, :private) + create(:personal_snippet, :secret) get api("/snippets/", user) @@ -38,6 +41,7 @@ it 'returns 404 for non-authenticated' do create(:personal_snippet, :internal) + create(:personal_snippet, :secret) get api("/snippets/") @@ -63,6 +67,7 @@ let!(:public_snippet) { create(:personal_snippet, :public, author: user) } let!(:private_snippet) { create(:personal_snippet, :private, author: user) } let!(:internal_snippet) { create(:personal_snippet, :internal, author: user) } + let!(:secret_snippet) { create(:personal_snippet, :secret, author: user) } let!(:public_snippet_other) { create(:personal_snippet, :public, author: other_user) } let!(:private_snippet_other) { create(:personal_snippet, :private, author: other_user) } let!(:internal_snippet_other) { create(:personal_snippet, :internal, author: other_user) } @@ -137,10 +142,67 @@ end describe 'GET /snippets/:id' do - set(:admin) { create(:user, :admin) } - set(:author) { create(:user) } - set(:private_snippet) { create(:personal_snippet, :private, author: author) } - set(:internal_snippet) { create(:personal_snippet, :internal, author: author) } + let_it_be(:admin) { create(:user, :admin) } + let_it_be(:author) { create(:user) } + let_it_be(:public_snippet) { create(:personal_snippet, :public, author: author) } + let_it_be(:private_snippet) { create(:personal_snippet, :private, author: author) } + let_it_be(:internal_snippet) { create(:personal_snippet, :internal, author: author) } + let_it_be(:secret_snippet) { create(:personal_snippet, :secret, author: author) } + + context 'attributes' do + subject { get api("/snippets/#{snippet.id}", author) } + + before do + subject + end + + shared_examples 'returns basic attributes' do + it do + expect(json_response['title']).to eq(snippet.title) + expect(json_response['description']).to eq(snippet.description) + expect(json_response['file_name']).to eq(snippet.file_name) + expect(json_response['visibility']).to eq(snippet.visibility) + expect(json_response['raw_url']).to eq(snippet.raw_url) + expect(json_response['web_url']).to eq(snippet.web_url) + end + end + + shared_examples 'does not include secret_token attribute' do + it do + expect(json_response).not_to include('secret_token') + end + end + + context 'public snippet' do + let(:snippet) { public_snippet } + + it_behaves_like 'does not include secret_token attribute' + end + + context 'private snippet' do + let(:snippet) { private_snippet } + + it_behaves_like 'does not include secret_token attribute' + end + + context 'internal snippet' do + let(:snippet) { internal_snippet } + + it_behaves_like 'does not include secret_token attribute' + end + + context 'secret snippet' do + let(:snippet) { secret_snippet } + + it 'web_url includes token' do + expect(json_response['web_url']).to include("?token=#{snippet.secret_token}") + end + + it 'raw_url includes token' do + expect(json_response['raw_url']).to include("?token=#{snippet.secret_token}") + end + end + end it 'requires authentication' do get api("/snippets/#{private_snippet.id}", nil) @@ -152,11 +214,6 @@ get api("/snippets/#{private_snippet.id}", author) expect(response).to have_gitlab_http_status(200) - - expect(json_response['title']).to eq(private_snippet.title) - expect(json_response['description']).to eq(private_snippet.description) - expect(json_response['file_name']).to eq(private_snippet.file_name) - expect(json_response['visibility']).to eq(private_snippet.visibility) end it 'shows private snippets to an admin' do diff --git a/spec/services/notes/build_service_spec.rb b/spec/services/notes/build_service_spec.rb index 984658cbd190b9c056055e435c92c73108c2005e..65b967093326e75fe2fbd1c1dc240d822cd2574b 100644 --- a/spec/services/notes/build_service_spec.rb +++ b/spec/services/notes/build_service_spec.rb @@ -68,9 +68,9 @@ def reply(note, user = nil) let(:snippet_author) { create(:user) } - context 'when a snippet is public' do - it 'creates a reply note' do - snippet = create(:personal_snippet, :public) + shared_examples 'creates a reply note' do + it do + snippet = create(:personal_snippet, visibility) note = create(:discussion_note_on_personal_snippet, noteable: snippet) new_note = reply(note) @@ -80,6 +80,18 @@ def reply(note, user = nil) end end + context 'when a snippet is public' do + it_behaves_like 'creates a reply note' do + let(:visibility) { :public } + end + end + + context 'when a snippet is secret' do + it_behaves_like 'creates a reply note' do + let(:visibility) { :secret } + end + end + context 'when a snippet is private' do let(:snippet) { create(:personal_snippet, :private, author: snippet_author) } let(:note) { create(:discussion_note_on_personal_snippet, noteable: snippet) } diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 79ea8f48a589a8d7ed8e0c2fbf7e234320f77d08..4881804dd5121bcba5e7c01171d7f6830a0ed423 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -334,16 +334,26 @@ { note: 'comment', noteable_type: 'Snippet', noteable_id: snippet.id } end - it 'returns a valid note' do - expect(subject).to be_valid - end + shared_examples 'creates valid content' do + it 'returns a valid note' do + expect(subject).to be_valid + end - it 'returns a persisted note' do - expect(subject).to be_persisted + it 'returns a persisted note' do + expect(subject).to be_persisted + end + + it 'note has valid content' do + expect(subject.note).to eq(params[:note]) + end end - it 'note has valid content' do - expect(subject.note).to eq(params[:note]) + it_behaves_like 'creates valid content' + + context 'when snippet is secret' do + let(:snippet) { create(:personal_snippet, :secret) } + + it_behaves_like 'creates valid content' end end diff --git a/spec/views/snippets/edit.html.haml_spec.rb b/spec/views/snippets/edit.html.haml_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..2d1fc68dcb5c2dfcf5a6e1d730f8b46250ab1a17 --- /dev/null +++ b/spec/views/snippets/edit.html.haml_spec.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'snippets/edit' do + let_it_be(:snippet) { create(:personal_snippet) } + let(:user) { snippet.author } + let(:flag_value) { true } + + before do + stub_feature_flags(secret_snippets: flag_value) + + assign(:snippet, snippet) + + allow(controller).to receive(:current_user).and_return(user) + + render + end + + it 'shows secret visibility radio' do + expect(rendered).to have_css('input#personal_snippet_visibility_level_secret') + end + + it 'shows secret visibility icon' do + expect(rendered).to have_css('label[for="personal_snippet_visibility_level_secret"] i.fa-user-secret') + end + + it 'shows secret visibility description' do + expect(rendered).to have_content('The snippet can be accessed without any authentication, but is not searchable') + end + + it 'shows secret hidden input' do + expect(rendered).to have_css('input.snippet_secret', visible: false) + end + + context 'when secret_snippets feature flag is disabled' do + let(:flag_value) { false } + + it 'does not show secret visibility radio' do + expect(rendered).not_to have_css('input#personal_snippet_visibility_level_secret') + end + + it 'does not show secret visibility icon' do + expect(rendered).not_to have_css('label[for="personal_snippet_visibility_level_secret"] i.fa-user-secret') + end + + it 'does not show secret visibility description' do + expect(rendered).not_to have_content('The snippet can be accessed without any authentication, but is not searchable') + end + + it 'does not show secret hidden input' do + expect(rendered).not_to have_css('input.snippet_secret', visible: false) + end + end +end diff --git a/spec/views/snippets/show.html.haml_spec.rb b/spec/views/snippets/show.html.haml_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..454025b02e695f3654af2f03f2ecc3a824450c12 --- /dev/null +++ b/spec/views/snippets/show.html.haml_spec.rb @@ -0,0 +1,41 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'snippets/show' do + let(:user) { snippet.author } + + before do + stub_feature_flags(snippets_vue: false) + + # Since the show view requires the user status loaded + # we can't just assign the `snippet` + assign(:snippet, PersonalSnippet.find_by(id: snippet.id)) + assign(:note, Note.new(noteable: snippet)) + assign(:noteable, snippet) + assign(:discussions, snippet.discussions) + + controller.params[:controller] = 'snippets' + controller.params[:action] = 'show' + controller.params[:id] = snippet.id + allow(controller).to receive(:current_user).and_return(user) + + render + end + + context 'when snippet is secret' do + let_it_be(:snippet) { create(:personal_snippet, :secret) } + + it 'shows secret description tooltip' do + expect(rendered).to have_css('.detail-page-header-body .snippet-box[title="The snippet can be accessed without any authentication, but is not searchable."]') + end + + it 'shows secret icon' do + expect(rendered).to have_css('.snippet-box i.fa-user-secret') + end + + it 'shows secret icon' do + expect(rendered).to have_content('Secret') + end + end +end