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.

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

changelogs/unreleased/41888-access-personal-snippets-by-api.yml

+---
+title: Allow all snippets to be accessed by API
+merge_request: 25772
+author:
+type: added

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

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