diff options
author | John Jarvis <jarv@gitlab.com> | 2018-12-27 14:47:47 +0100 |
---|---|---|
committer | John Jarvis <jarv@gitlab.com> | 2018-12-27 14:47:47 +0100 |
commit | 39c502fd650eb5955f73a8ed88a886c3485f7428 (patch) | |
tree | 5bef4d4f6bb1a2ff1e18ed78830c5f1b3e4e7eea | |
parent | 7703a04b53ea1d9a3e141de68dac765fd4d1a46a (diff) | |
parent | bceb82d8e1a8b6762f386b51d1c07fa731255fa1 (diff) | |
download | gitlab-ce-39c502fd650eb5955f73a8ed88a886c3485f7428.tar.gz |
Merge remote-tracking branch 'origin/security-48259-private-snippet-11-4' into security-11-4
-rw-r--r-- | app/controllers/projects/snippets_controller.rb | 9 | ||||
-rw-r--r-- | app/controllers/snippets_controller.rb | 8 | ||||
-rw-r--r-- | app/helpers/snippets_helper.rb | 8 | ||||
-rw-r--r-- | app/models/snippet.rb | 6 | ||||
-rw-r--r-- | app/views/shared/snippets/_header.html.haml | 2 | ||||
-rw-r--r-- | changelogs/unreleased/security-48259-private-snippet.yml | 5 | ||||
-rw-r--r-- | spec/controllers/projects/snippets_controller_spec.rb | 40 | ||||
-rw-r--r-- | spec/controllers/snippets_controller_spec.rb | 19 | ||||
-rw-r--r-- | spec/models/snippet_spec.rb | 37 |
9 files changed, 123 insertions, 11 deletions
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 694c3a59e2b..9fe1f9a2bde 100644 --- a/app/controllers/snippets_controller.rb +++ b/app/controllers/snippets_controller.rb @@ -84,7 +84,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/helpers/snippets_helper.rb b/app/helpers/snippets_helper.rb index c7d31f3469d..a20c47ed91a 100644 --- a/app/helpers/snippets_helper.rb +++ b/app/helpers/snippets_helper.rb @@ -130,12 +130,4 @@ module SnippetsHelper link_to external_snippet_icon('download'), download_url, class: 'btn', target: '_blank', title: 'Download', rel: 'noopener noreferrer' end - - def public_snippet? - if @snippet.project_id? - can?(nil, :read_project_snippet, @snippet) - else - can?(nil, :read_personal_snippet, @snippet) - end - end end diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 1c5846b4023..964a66efe62 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -140,6 +140,12 @@ class Snippet < ActiveRecord::Base :visibility_level end + def embeddable? + ability = project_id? ? :read_project_snippet : :read_personal_snippet + + Ability.allowed?(nil, ability, self) + end + def notes_with_associations notes.includes(:author) end diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index 10bfc30492a..a43296aa806 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -30,7 +30,7 @@ - if @snippet.updated_at != @snippet.created_at = edited_time_ago_with_tooltip(@snippet, placement: 'bottom', html_class: 'snippet-edited-ago', exclude_author: true) - - if public_snippet? + - if @snippet.embeddable? .embed-snippet .input-group .input-group-prepend 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 9c383bd7628..70bf182cdee 100644 --- a/spec/controllers/projects/snippets_controller_spec.rb +++ b/spec/controllers/projects/snippets_controller_spec.rb @@ -371,6 +371,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 9effe47ab05..f3d6493786e 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 diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index e09d89d235d..da61a4075ed 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -212,4 +212,41 @@ describe Snippet do expect(blob.data).to eq(snippet.content) end end + + describe '#embeddable?' do + context 'project snippet' do + [ + { project: :public, snippet: :public, embeddable: true }, + { project: :internal, snippet: :public, embeddable: false }, + { project: :private, snippet: :public, embeddable: false }, + { project: :public, snippet: :internal, embeddable: false }, + { project: :internal, snippet: :internal, embeddable: false }, + { project: :private, snippet: :internal, embeddable: false }, + { project: :public, snippet: :private, embeddable: false }, + { project: :internal, snippet: :private, embeddable: false }, + { project: :private, snippet: :private, embeddable: false } + ].each do |combination| + it 'only returns true when both project and snippet are public' do + project = create(:project, combination[:project]) + snippet = create(:project_snippet, combination[:snippet], project: project) + + expect(snippet.embeddable?).to eq(combination[:embeddable]) + end + end + end + + context 'personal snippet' do + [ + { snippet: :public, embeddable: true }, + { snippet: :internal, embeddable: false }, + { snippet: :private, embeddable: false } + ].each do |combination| + it 'only returns true when snippet is public' do + snippet = create(:personal_snippet, combination[:snippet]) + + expect(snippet.embeddable?).to eq(combination[:embeddable]) + end + end + end + end end |