Allow unauthenticated public snippet API calls

To be consistent with other areas of the product, we
are allowing REST API calls for public snippets to be made
without user authentication

changelogs/unreleased/vij-public-snippet-access.yml

+---
+title: Allow unauthenticated users access to public Personal Snippets via the REST API
+merge_request: 44135
+author:
+type: fixed

lib/api/snippets.rb

@@ -5,8 +5,6 @@ module API
   class Snippets < Grape::API::Instance
     include PaginationParams
 
-    before { authenticate! }
-
     resource :snippets do
       helpers Helpers::SnippetsHelpers
       helpers do
@@ -23,7 +21,7 @@ module API
         end
       end
 
-      desc 'Get a snippets list for authenticated user' do
+      desc 'Get a snippets list for an authenticated user' do
         detail 'This feature was introduced in GitLab 8.15.'
         success Entities::Snippet
       end
@@ -31,6 +29,8 @@ module API
         use :pagination
       end
       get do
+        authenticate!
+
         present paginate(snippets_for_current_user), with: Entities::Snippet, current_user: current_user
       end
 
@@ -42,6 +42,8 @@ module API
         use :pagination
       end
       get 'public' do
+        authenticate!
+
         present paginate(public_snippets), with: Entities::PersonalSnippet, current_user: current_user
       end
 
@@ -74,6 +76,8 @@ module API
         use :create_file_params
       end
       post do
+        authenticate!
+
         authorize! :create_snippet
 
         attrs = process_create_params(declared_params(include_missing: false))
@@ -109,6 +113,8 @@ module API
         use :minimum_update_params
       end
       put ':id' do
+        authenticate!
+
         snippet = snippets_for_current_user.find_by_id(params.delete(:id))
         break not_found!('Snippet') unless snippet
 
@@ -139,6 +145,8 @@ module API
         requires :id, type: Integer, desc: 'The ID of a snippet'
       end
       delete ':id' do
+        authenticate!
+
         snippet = snippets_for_current_user.find_by_id(params.delete(:id))
         break not_found!('Snippet') unless snippet
 

spec/requests/api/project_snippets_spec.rb

@@ -432,6 +432,14 @@ RSpec.describe API::ProjectSnippets do
   describe 'GET /projects/:project_id/snippets/:id/files/:ref/:file_path/raw' do
     let_it_be(:snippet) { create(:project_snippet, :repository, author: admin, project: project) }
 
+    context 'with no user' do
+      it 'requires authentication' do
+        get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/files/master/%2Egitattributes/raw", nil)
+
+        expect(response).to have_gitlab_http_status(:unauthorized)
+      end
+    end
+
     it_behaves_like 'raw snippet files' do
       let(:api_path) { "/projects/#{snippet.project.id}/snippets/#{snippet_id}/files/#{ref}/#{file_path}/raw" }
     end

spec/requests/api/snippets_spec.rb

@@ -5,14 +5,15 @@ require 'spec_helper'
 RSpec.describe API::Snippets do
   include SnippetHelpers
 
+  let_it_be(:admin)      { create(:user, :admin) }
   let_it_be(:user)       { create(:user) }
+  let_it_be(:other_user) { create(:user) }
+  let_it_be(:public_snippet)   { create(:personal_snippet, :repository, :public, author: user) }
+  let_it_be(:private_snippet)  { create(:personal_snippet, :repository, :private, author: user) }
+  let_it_be(:internal_snippet) { create(:personal_snippet, :repository, :internal, author: user) }
 
   describe 'GET /snippets/' do
-    it 'returns snippets available' do
-      public_snippet = create(:personal_snippet, :repository, :public, author: user)
-      private_snippet = create(:personal_snippet, :repository, :private, author: user)
-      internal_snippet = create(:personal_snippet, :repository, :internal, author: user)
-
+    it 'returns snippets available for user' do
       get api("/snippets/", user)
 
       expect(response).to have_gitlab_http_status(:ok)
@@ -29,9 +30,7 @@ RSpec.describe API::Snippets do
     end
 
     it 'hides private snippets from regular user' do
-      create(:personal_snippet, :private)
-
-      get api("/snippets/", user)
+      get api("/snippets/", other_user)
 
       expect(response).to have_gitlab_http_status(:ok)
       expect(response).to include_pagination_headers
@@ -39,9 +38,7 @@ RSpec.describe API::Snippets do
       expect(json_response.size).to eq(0)
     end
 
-    it 'returns 404 for non-authenticated' do
-      create(:personal_snippet, :internal)
-
+    it 'returns 401 for non-authenticated' do
       get api("/snippets/")
 
       expect(response).to have_gitlab_http_status(:unauthorized)
@@ -62,10 +59,6 @@ RSpec.describe API::Snippets do
   end
 
   describe 'GET /snippets/public' do
-    let_it_be(:other_user)               { create(:user) }
-    let_it_be(:public_snippet)           { create(:personal_snippet, :repository, :public, author: user) }
-    let_it_be(:private_snippet)          { create(:personal_snippet, :repository, :private, author: user) }
-    let_it_be(:internal_snippet)         { create(:personal_snippet, :repository, :internal, author: user) }
     let_it_be(:public_snippet_other)     { create(:personal_snippet, :repository, :public, author: other_user) }
     let_it_be(:private_snippet_other)    { create(:personal_snippet, :repository, :private, author: other_user) }
     let_it_be(:internal_snippet_other)   { create(:personal_snippet, :repository, :internal, author: other_user) }
@@ -73,8 +66,10 @@ RSpec.describe API::Snippets do
     let_it_be(:private_snippet_project)  { create(:project_snippet, :repository, :private, author: user) }
     let_it_be(:internal_snippet_project) { create(:project_snippet, :repository, :internal, author: user) }
 
-    it 'returns all snippets with public visibility from all users' do
-      get api("/snippets/public", user)
+    let(:path) { "/snippets/public" }
+
+    it 'returns only public snippets from all users when authenticated' do
+      get api(path, user)
 
       aggregate_failures do
         expect(response).to have_gitlab_http_status(:ok)
@@ -90,20 +85,23 @@ RSpec.describe API::Snippets do
         expect(json_response[1]['files'].first).to eq snippet_blob_file(public_snippet.blobs.first)
       end
     end
-  end
-
-  describe 'GET /snippets/:id/raw' do
-    let_it_be(:author) { create(:user) }
-    let_it_be(:snippet) { create(:personal_snippet, :repository, :private, author: author) }
 
     it 'requires authentication' do
-      get api("/snippets/#{snippet.id}", nil)
+      get api(path, nil)
 
       expect(response).to have_gitlab_http_status(:unauthorized)
     end
+  end
+
+  describe 'GET /snippets/:id/raw' do
+    let(:snippet) { private_snippet }
+
+    it_behaves_like 'snippet access with different users' do
+      let(:path) { "/snippets/#{snippet.id}/raw" }
+    end
 
     it 'returns raw text' do
-      get api("/snippets/#{snippet.id}/raw", author)
+      get api("/snippets/#{snippet.id}/raw", user)
 
       expect(response).to have_gitlab_http_status(:ok)
       expect(response.media_type).to eq 'text/plain'
@@ -113,28 +111,14 @@ RSpec.describe API::Snippets do
     it 'returns 404 for invalid snippet id' do
       snippet.destroy!
 
-      get api("/snippets/#{snippet.id}/raw", author)
-
-      expect(response).to have_gitlab_http_status(:not_found)
-      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(:not_found)
-    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(:ok)
+      expect(json_response['message']).to eq('404 Snippet Not Found')
     end
 
     it_behaves_like 'snippet blob content' do
-      let_it_be(:snippet_with_empty_repo) { create(:personal_snippet, :empty_repo, :private, author: author) }
+      let_it_be(:snippet_with_empty_repo) { create(:personal_snippet, :empty_repo, :private, author: user) }
 
       subject { get api("/snippets/#{snippet.id}/raw", snippet.author) }
     end
@@ -149,33 +133,11 @@ RSpec.describe API::Snippets do
   end
 
   describe 'GET /snippets/:id' do
-    let_it_be(:admin) { create(:user, :admin) }
-    let_it_be(:author) { create(:user) }
-    let_it_be(:private_snippet) { create(:personal_snippet, :repository, :private, author: author) }
-    let_it_be(:internal_snippet) { create(:personal_snippet, :repository, :internal, author: author) }
-    let(:snippet) { private_snippet }
-
-    subject { get api("/snippets/#{snippet.id}", user) }
-
-    it 'hides private snippets from an ordinary user' do
-      subject
-
-      expect(response).to have_gitlab_http_status(:not_found)
-    end
-
-    context 'without a user' do
-      let(:user) { nil }
-
-      it 'requires authentication' do
-        subject
+    let(:snippet_id) { private_snippet.id }
 
-        expect(response).to have_gitlab_http_status(:unauthorized)
-      end
-    end
+    subject { get api("/snippets/#{snippet_id}", user) }
 
     context 'with the author' do
-      let(:user) { author }
-
       it 'returns snippet json' do
         subject
 
@@ -191,18 +153,10 @@ RSpec.describe API::Snippets do
       end
     end
 
-    context 'with an admin' do
-      let(:user) { admin }
-
-      it 'shows private snippets to an admin' do
-        subject
-
-        expect(response).to have_gitlab_http_status(:ok)
-      end
-
-      it 'returns 404 for invalid snippet id' do
-        private_snippet.destroy!
+    context 'with a non-existent snippet ID' do
+      let(:snippet_id) { 0 }
 
+      it 'returns 404' do
         subject
 
         expect(response).to have_gitlab_http_status(:not_found)
@@ -210,19 +164,11 @@ RSpec.describe API::Snippets do
       end
     end
 
-    context 'with an internal snippet' do
-      let(:snippet) { internal_snippet }
-
-      it 'shows internal snippets to an ordinary user' do
-        subject
-
-        expect(response).to have_gitlab_http_status(:ok)
-      end
+    it_behaves_like 'snippet access with different users' do
+      let(:path) { "/snippets/#{snippet.id}" }
     end
 
-    it_behaves_like 'snippet_multiple_files feature disabled' do
-      let(:user) { author }
-    end
+    it_behaves_like 'snippet_multiple_files feature disabled'
   end
 
   describe 'POST /snippets/' do

spec/support/helpers/snippet_helpers.rb

@@ -8,7 +8,7 @@ module SnippetHelpers
   def snippet_blob_file(blob)
     {
       "path" => blob.path,
-      "raw_url" => gitlab_raw_snippet_blob_url(blob.container, blob.path)
+      "raw_url" => gitlab_raw_snippet_blob_url(blob.container, blob.path, host: 'localhost')
     }
   end
 end

spec/support/shared_examples/requests/api/snippets_shared_examples.rb

@@ -7,14 +7,6 @@ RSpec.shared_examples 'raw snippet files' do
   let(:file_path)  { '%2Egitattributes' }
   let(:ref)        { 'master' }
 
-  context 'with no user' do
-    it 'requires authentication' do
-      get api(api_path)
-
-      expect(response).to have_gitlab_http_status(:unauthorized)
-    end
-  end
-
   shared_examples 'not found' do
     it 'returns 404' do
       get api(api_path, user)
@@ -216,3 +208,58 @@ RSpec.shared_examples 'invalid snippet updates' do
     expect(json_response['error']).to eq 'title is empty'
   end
 end
+
+RSpec.shared_examples 'snippet access with different users' do
+  using RSpec::Parameterized::TableSyntax
+
+  where(:requester, :visibility, :status) do
+    :admin   | :public   | :ok
+    :admin   | :private  | :ok
+    :admin   | :internal | :ok
+    :author  | :public   | :ok
+    :author  | :private  | :ok
+    :author  | :internal | :ok
+    :other   | :public   | :ok
+    :other   | :private  | :not_found
+    :other   | :internal | :ok
+    nil      | :public   | :ok
+    nil      | :private  | :not_found
+    nil      | :internal | :not_found
+  end
+
+  with_them do
+    let(:snippet) { snippet_for(visibility) }
+
+    it 'returns the correct response' do
+      request_user = user_for(requester)
+
+      get api(path, request_user)
+
+      expect(response).to have_gitlab_http_status(status)
+    end
+  end
+
+  def user_for(user_type)
+    case user_type
+    when :author
+      user
+    when :other
+      other_user
+    when :admin
+      admin
+    else
+      nil
+    end
+  end
+
+  def snippet_for(snippet_type)
+    case snippet_type
+    when :private
+      private_snippet
+    when :internal
+      internal_snippet
+    when :public
+      public_snippet
+    end
+  end
+end