diff --git a/app/controllers/projects/snippets_controller.rb b/app/controllers/projects/snippets_controller.rb index a44acb12bdf..255f1f3569a 100644 --- a/app/controllers/projects/snippets_controller.rb +++ b/app/controllers/projects/snippets_controller.rb @@ -75,7 +75,14 @@ class Projects::SnippetsController < Projects::ApplicationController format.json do render_blob_json(blob) end - format.js { render 'shared/snippets/show'} + + format.js do + if @snippet.embeddable? + render 'shared/snippets/show' + else + head :not_found + end + end end end diff --git a/app/controllers/snippets_controller.rb b/app/controllers/snippets_controller.rb index dd9bf17cf0c..8ea5450b4e8 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -80,7 +80,13 @@ class SnippetsController < ApplicationController render_blob_json(blob) end - format.js { render 'shared/snippets/show' } + format.js do + if @snippet.embeddable? + render 'shared/snippets/show' + else + head :not_found + end + end end end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index e623ee8161f..f9b23bbbf6c 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -176,11 +176,9 @@ class Snippet < ActiveRecord::Base end def embeddable? - if project_id? - Ability.allowed?(nil, :read_project_snippet, self) - else - Ability.allowed?(nil, :read_personal_snippet, self) - end + ability = project_id? ? :read_project_snippet : :read_personal_snippet + + Ability.allowed?(nil, ability, self) end def notes_with_associations diff --git a/changelogs/unreleased/security-48259-private-snippet.yml b/changelogs/unreleased/security-48259-private-snippet.yml new file mode 100644 index 00000000000..6cf1e5dc694 --- /dev/null +++ b/changelogs/unreleased/security-48259-private-snippet.yml @@ -0,0 +1,5 @@ +--- +title: Prevent private snippets from being embeddable +merge_request: +author: +type: security diff --git a/spec/controllers/projects/snippets_controller_spec.rb b/spec/controllers/projects/snippets_controller_spec.rb index 1a3fb4da15f..e4b78aff25d 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -379,6 +379,46 @@ describe Projects::SnippetsController do end end + describe "GET #show for embeddable content" do + let(:project_snippet) { create(:project_snippet, snippet_permission, project: project, author: user) } + + before do + sign_in(user) + + get :show, namespace_id: project.namespace, project_id: project, id: project_snippet.to_param, format: :js + end + + context 'when snippet is private' do + let(:snippet_permission) { :private } + + it 'responds with status 404' do + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when snippet is public' do + let(:snippet_permission) { :public } + + it 'responds with status 200' do + expect(assigns(:snippet)).to eq(project_snippet) + expect(response).to have_gitlab_http_status(200) + end + end + + context 'when the project is private' do + let(:project) { create(:project_empty_repo, :private) } + + context 'when snippet is public' do + let(:project_snippet) { create(:project_snippet, :public, project: project, author: user) } + + it 'responds with status 404' do + expect(assigns(:snippet)).to eq(project_snippet) + expect(response).to have_gitlab_http_status(404) + end + end + end + end + describe 'GET #raw' do let(:project_snippet) do create( diff --git a/spec/controllers/snippets_controller_spec.rb b/spec/controllers/snippets_controller_spec.rb index 01a5161f775..af44598bfe4 100644 --- a/spec/controllers/snippets_controller_spec.rb +++ b/spec/controllers/snippets_controller_spec.rb @@ -80,6 +80,12 @@ describe SnippetsController do expect(assigns(:snippet)).to eq(personal_snippet) expect(response).to have_gitlab_http_status(200) end + + it 'responds with status 404 when embeddable content is requested' do + get :show, id: personal_snippet.to_param, format: :js + + expect(response).to have_gitlab_http_status(404) + end end end @@ -106,6 +112,12 @@ describe SnippetsController do expect(assigns(:snippet)).to eq(personal_snippet) expect(response).to have_gitlab_http_status(200) end + + it 'responds with status 404 when embeddable content is requested' do + get :show, id: personal_snippet.to_param, format: :js + + expect(response).to have_gitlab_http_status(404) + end end context 'when not signed in' do @@ -131,6 +143,13 @@ describe SnippetsController do expect(assigns(:snippet)).to eq(personal_snippet) expect(response).to have_gitlab_http_status(200) end + + it 'responds with status 200 when embeddable content is requested' do + get :show, id: personal_snippet.to_param, format: :js + + expect(assigns(:snippet)).to eq(personal_snippet) + expect(response).to have_gitlab_http_status(200) + end end context 'when not signed in' do