Block private snippets from being embeddable

6 files changed, 82 insertions, 7 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 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