Merge branch 'security-48259-private-snippet' into 'master'

[master] Prevent private snippet from being embeddable

See merge request gitlab/gitlabhq!2692

3 files changed, 96 insertions, 0 deletions

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 d2a56518f65..d762531da7e 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 7a7272ccb60..664dc3fa145 100644
--- a/spec/models/snippet_spec.rb
+++ b/spec/models/snippet_spec.rb
@@ -423,4 +423,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