Allow all personal snippets to be accessed by API

Previously, you could only access personal snippets in the API if you
had authored them. The documentation doesn't state that this is the
case, and it's quite surprising.

4 files changed, 83 insertions, 25 deletions

diff --git a/app/finders/snippets_finder.rb b/app/finders/snippets_finder.rb
index d3774746cb8..bf29f15642d 100644
--- a/app/finders/snippets_finder.rb
+++ b/app/finders/snippets_finder.rb
@@ -69,6 +69,8 @@ class SnippetsFinder < UnionFinder
     base.with_optional_visibility(visibility_from_scope).fresh
   end
 
+  private
+
   # Produces a query that retrieves snippets from multiple projects.
   #
   # The resulting query will, depending on the user's permissions, include the
diff --git a/changelogs/unreleased/41888-access-personal-snippets-by-api.yml b/changelogs/unreleased/41888-access-personal-snippets-by-api.yml
new file mode 100644
index 00000000000..3561a01ec5f
--- /dev/null
+++ b/changelogs/unreleased/41888-access-personal-snippets-by-api.yml
@@ -0,0 +1,5 @@
+---
+title: Allow all snippets to be accessed by API
+merge_request: 25772
+author:
+type: added
diff --git a/lib/api/snippets.rb b/lib/api/snippets.rb
index 326d55afd0e..f8b37b33348 100644
--- a/lib/api/snippets.rb
+++ b/lib/api/snippets.rb
@@ -16,6 +16,10 @@ module API
         def public_snippets
           SnippetsFinder.new(current_user, scope: :are_public).execute
         end
+
+        def snippets
+          SnippetsFinder.new(current_user).execute
+        end
       end
 
       desc 'Get a snippets list for authenticated user' do
@@ -48,7 +52,10 @@ module API
         requires :id, type: Integer, desc: 'The ID of a snippet'
       end
       get ':id' do
-        snippet = snippets_for_current_user.find(params[:id])
+        snippet = snippets.find_by_id(params[:id])
+
+        break not_found!('Snippet') unless snippet
+
         present snippet, with: Entities::PersonalSnippet
       end
 
@@ -94,9 +101,8 @@ module API
                               desc: 'The visibility of the snippet'
         at_least_one_of :title, :file_name, :content, :visibility
       end
-      # rubocop: disable CodeReuse/ActiveRecord
       put ':id' do
-        snippet = snippets_for_current_user.find_by(id: params.delete(:id))
+        snippet = snippets_for_current_user.find_by_id(params.delete(:id))
         break not_found!('Snippet') unless snippet
 
         authorize! :update_personal_snippet, snippet
@@ -113,7 +119,6 @@ module API
           render_validation_error!(snippet)
         end
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       desc 'Remove snippet' do
         detail 'This feature was introduced in GitLab 8.15.'
@@ -122,16 +127,14 @@ module API
       params do
         requires :id, type: Integer, desc: 'The ID of a snippet'
       end
-      # rubocop: disable CodeReuse/ActiveRecord
       delete ':id' do
-        snippet = snippets_for_current_user.find_by(id: params.delete(:id))
+        snippet = snippets_for_current_user.find_by_id(params.delete(:id))
         break not_found!('Snippet') unless snippet
 
         authorize! :destroy_personal_snippet, snippet
 
         destroy_conditionally!(snippet)
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       desc 'Get a raw snippet' do
         detail 'This feature was introduced in GitLab 8.15.'
@@ -139,9 +142,8 @@ module API
       params do
         requires :id, type: Integer, desc: 'The ID of a snippet'
       end
-      # rubocop: disable CodeReuse/ActiveRecord
       get ":id/raw" do
-        snippet = snippets_for_current_user.find_by(id: params.delete(:id))
+        snippet = snippets.find_by_id(params.delete(:id))
         break not_found!('Snippet') unless snippet
 
         env['api.format'] = :txt
@@ -149,7 +151,6 @@ module API
         header['Content-Disposition'] = 'attachment'
         present snippet.content
       end
-      # rubocop: enable CodeReuse/ActiveRecord
 
       desc 'Get the user agent details for a snippet' do
         success Entities::UserAgentDetail
@@ -157,17 +158,15 @@ module API
       params do
         requires :id, type: Integer, desc: 'The ID of a snippet'
       end
-      # rubocop: disable CodeReuse/ActiveRecord
       get ":id/user_agent_detail" do
         authenticated_as_admin!
 
-        snippet = Snippet.find_by!(id: params[:id])
+        snippet = Snippet.find_by_id!(params[:id])
 
         break not_found!('UserAgentDetail') unless snippet.user_agent_detail
 
         present snippet.user_agent_detail, with: Entities::UserAgentDetail
       end
-      # rubocop: enable CodeReuse/ActiveRecord
     end
   end
 end
diff --git a/spec/requests/api/snippets_spec.rb b/spec/requests/api/snippets_spec.rb
index 7c8512f7589..d600076e9fb 100644
--- a/spec/requests/api/snippets_spec.rb
+++ b/spec/requests/api/snippets_spec.rb
@@ -84,10 +84,17 @@ describe API::Snippets do
   end
 
   describe 'GET /snippets/:id/raw' do
-    let(:snippet) { create(:personal_snippet, author: user) }
+    set(:author) { create(:user) }
+    set(:snippet) { create(:personal_snippet, :private, author: author) }
+
+    it 'requires authentication' do
+      get api("/snippets/#{snippet.id}", nil)
+
+      expect(response).to have_gitlab_http_status(401)
+    end
 
     it 'returns raw text' do
-      get api("/snippets/#{snippet.id}/raw", user)
+      get api("/snippets/#{snippet.id}/raw", author)
 
       expect(response).to have_gitlab_http_status(200)
       expect(response.content_type).to eq 'text/plain'
@@ -95,38 +102,83 @@ describe API::Snippets do
     end
 
     it 'forces attachment content disposition' do
-      get api("/snippets/#{snippet.id}/raw", user)
+      get api("/snippets/#{snippet.id}/raw", author)
 
       expect(headers['Content-Disposition']).to match(/^attachment/)
     end
 
     it 'returns 404 for invalid snippet id' do
-      get api("/snippets/1234/raw", user)
+      snippet.destroy
+
+      get api("/snippets/#{snippet.id}/raw", author)
 
       expect(response).to have_gitlab_http_status(404)
       expect(json_response['message']).to eq('404 Snippet Not Found')
     end
+
+    it 'hides private snippets from ordinary users' do
+      get api("/snippets/#{snippet.id}/raw", user)
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    it 'shows internal snippets to ordinary users' do
+      internal_snippet = create(:personal_snippet, :internal, author: author)
+
+      get api("/snippets/#{internal_snippet.id}/raw", user)
+
+      expect(response).to have_gitlab_http_status(200)
+    end
   end
 
   describe 'GET /snippets/:id' do
-    let(:snippet) { create(:personal_snippet, author: user) }
+    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) }
+
+    it 'requires authentication' do
+      get api("/snippets/#{private_snippet.id}", nil)
+
+      expect(response).to have_gitlab_http_status(401)
+    end
 
     it 'returns snippet json' do
-      get api("/snippets/#{snippet.id}", user)
+      get api("/snippets/#{private_snippet.id}", author)
 
       expect(response).to have_gitlab_http_status(200)
 
-      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['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
+      get api("/snippets/#{private_snippet.id}", admin)
+
+      expect(response).to have_gitlab_http_status(200)
+    end
+
+    it 'hides private snippets from an ordinary user' do
+      get api("/snippets/#{private_snippet.id}", user)
+
+      expect(response).to have_gitlab_http_status(404)
+    end
+
+    it 'shows internal snippets to an ordinary user' do
+      get api("/snippets/#{internal_snippet.id}", user)
+
+      expect(response).to have_gitlab_http_status(200)
     end
 
     it 'returns 404 for invalid snippet id' do
-      get api("/snippets/1234", user)
+      private_snippet.destroy
+
+      get api("/snippets/#{private_snippet.id}", admin)
 
       expect(response).to have_gitlab_http_status(404)
-      expect(json_response['message']).to eq('404 Not found')
+      expect(json_response['message']).to eq('404 Snippet Not Found')
     end
   end