Merge branch 'vij-public-project-snippets' into 'master'

Allow unauthenticated public project snippet API access

See merge request gitlab-org/gitlab!44446

changelogs/unreleased/vij-public-project-snippets.yml

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

lib/api/project_snippets.rb

@@ -4,7 +4,6 @@ module API
   class ProjectSnippets < Grape::API::Instance
     include PaginationParams
 
-    before { authenticate! }
     before { check_snippets_enabled }
 
     params do
@@ -37,6 +36,8 @@ module API
         use :pagination
       end
       get ":id/snippets" do
+        authenticate!
+
         present paginate(snippets_for_current_user), with: Entities::ProjectSnippet, current_user: current_user
       end
 
@@ -48,6 +49,9 @@ module API
       end
       get ":id/snippets/:snippet_id" do
         snippet = snippets_for_current_user.find(params[:snippet_id])
+
+        not_found!('Snippet') unless snippet
+
         present snippet, with: Entities::ProjectSnippet, current_user: current_user
       end
 
@@ -63,6 +67,8 @@ module API
         use :create_file_params
       end
       post ":id/snippets" do
+        authenticate!
+
         authorize! :create_snippet, user_project
 
         snippet_params = process_create_params(declared_params(include_missing: false))
@@ -97,6 +103,8 @@ module API
       end
       # rubocop: disable CodeReuse/ActiveRecord
       put ":id/snippets/:snippet_id" do
+        authenticate!
+
         snippet = snippets_for_current_user.find_by(id: params.delete(:snippet_id))
         not_found!('Snippet') unless snippet
 
@@ -125,6 +133,8 @@ module API
       end
       # rubocop: disable CodeReuse/ActiveRecord
       delete ":id/snippets/:snippet_id" do
+        authenticate!
+
         snippet = snippets_for_current_user.find_by(id: params[:snippet_id])
         not_found!('Snippet') unless snippet
 

spec/requests/api/project_snippets_spec.rb

@@ -16,8 +16,8 @@ RSpec.describe API::ProjectSnippets do
   end
 
   describe "GET /projects/:project_id/snippets/:id/user_agent_detail" do
-    let(:snippet) { create(:project_snippet, :public, project: project) }
-    let!(:user_agent_detail) { create(:user_agent_detail, subject: snippet) }
+    let_it_be(:snippet) { create(:project_snippet, :public, project: project) }
+    let_it_be(:user_agent_detail) { create(:user_agent_detail, subject: snippet) }
 
     it 'exposes known attributes' do
       get api("/projects/#{project.id}/snippets/#{snippet.id}/user_agent_detail", admin)
@@ -86,8 +86,8 @@ RSpec.describe API::ProjectSnippets do
   end
 
   describe 'GET /projects/:project_id/snippets/:id' do
-    let_it_be(:user) { create(:user) }
     let_it_be(:snippet) { create(:project_snippet, :public, :repository, project: project) }
+    let_it_be(:private_snippet) { create(:project_snippet, :private, project: project) }
 
     it 'returns snippet json' do
       get api("/projects/#{project.id}/snippets/#{snippet.id}", user)
@@ -116,6 +116,10 @@ RSpec.describe API::ProjectSnippets do
         let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/123", user) }
       end
     end
+
+    it_behaves_like 'project snippet access levels' do
+      let(:path) { "/projects/#{snippet.project.id}/snippets/#{snippet.id}" }
+    end
   end
 
   describe 'POST /projects/:project_id/snippets/' do
@@ -396,7 +400,8 @@ RSpec.describe API::ProjectSnippets do
   end
 
   describe 'GET /projects/:project_id/snippets/:id/raw' do
-    let_it_be(:snippet) { create(:project_snippet, :repository, author: admin, project: project) }
+    let_it_be(:snippet) { create(:project_snippet, :repository, :public, author: admin, project: project) }
+    let_it_be(:private_snippet) { create(:project_snippet, :repository, :private, author: admin, project: project) }
 
     it 'returns raw text' do
       get api("/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw", admin)
@@ -412,6 +417,10 @@ RSpec.describe API::ProjectSnippets do
       expect(json_response['message']).to eq('404 Snippet Not Found')
     end
 
+    it_behaves_like 'project snippet access levels' do
+      let(:path) { "/projects/#{snippet.project.id}/snippets/#{snippet.id}/raw" }
+    end
+
     context 'with snippets disabled' do
       it_behaves_like '403 response' do
         let(:request) { get api("/projects/#{project_no_snippets.id}/snippets/123/raw", admin) }
@@ -428,16 +437,12 @@ 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
+
+    it_behaves_like 'project snippet access levels' do
+      let(:path) { "/projects/#{snippet.project.id}/snippets/#{snippet.id}/files/master/%2Egitattributes/raw" }
+    end
   end
 end

spec/requests/api/snippets_spec.rb

@@ -130,6 +130,10 @@ RSpec.describe API::Snippets do
     it_behaves_like 'raw snippet files' do
       let(:api_path) { "/snippets/#{snippet_id}/files/#{ref}/#{file_path}/raw" }
     end
+
+    it_behaves_like 'snippet access with different users' do
+      let(:path) { "/snippets/#{snippet.id}/files/master/%2Egitattributes/raw" }
+    end
   end
 
   describe 'GET /snippets/:id' do

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

@@ -7,7 +7,9 @@ RSpec.shared_examples 'raw snippet files' do
   let(:file_path)  { '%2Egitattributes' }
   let(:ref)        { 'master' }
 
-  shared_examples 'not found' do
+  context 'with an invalid snippet ID' do
+    let(:snippet_id) { 'invalid' }
+
     it 'returns 404' do
       get api(api_path, user)
 
@@ -16,18 +18,6 @@ RSpec.shared_examples 'raw snippet files' do
     end
   end
 
-  context 'when not authorized' do
-    let(:user) { unauthorized_user }
-
-    it_behaves_like 'not found'
-  end
-
-  context 'with an invalid snippet ID' do
-    let(:snippet_id) { 'invalid' }
-
-    it_behaves_like 'not found'
-  end
-
   context 'with valid params' do
     it 'returns the raw file info' do
       expect(Gitlab::Workhorse).to receive(:send_git_blob).and_call_original
@@ -263,3 +253,76 @@ RSpec.shared_examples 'snippet access with different users' do
     end
   end
 end
+
+RSpec.shared_examples 'expected response status' do
+  it 'returns the correct response' do
+    get api(path, user)
+
+    expect(response).to have_gitlab_http_status(status)
+  end
+end
+
+RSpec.shared_examples 'unauthenticated project snippet access' do
+  using RSpec::Parameterized::TableSyntax
+
+  let(:user) { nil }
+
+  where(:project_visibility, :snippet_visibility, :status) do
+    :public   | :public   | :ok
+    :public   | :private  | :not_found
+    :public   | :internal | :not_found
+    :internal | :public   | :not_found
+    :private  | :public   | :not_found
+  end
+
+  with_them do
+    it_behaves_like 'expected response status'
+  end
+end
+
+RSpec.shared_examples 'non-member project snippet access' do
+  using RSpec::Parameterized::TableSyntax
+
+  where(:project_visibility, :snippet_visibility, :status) do
+    :public   | :public   | :ok
+    :public   | :internal | :ok
+    :internal | :public   | :ok
+    :public   | :private  | :not_found
+    :private  | :public   | :not_found
+  end
+
+  with_them do
+    it_behaves_like 'expected response status'
+  end
+end
+
+RSpec.shared_examples 'member project snippet access' do
+  using RSpec::Parameterized::TableSyntax
+
+  before do
+    project.add_guest(user)
+  end
+
+  where(:project_visibility, :snippet_visibility, :status) do
+    :public   | :public   | :ok
+    :public   | :internal | :ok
+    :internal | :public   | :ok
+    :public   | :private  | :ok
+    :private  | :public   | :ok
+  end
+
+  with_them do
+    it_behaves_like 'expected response status'
+  end
+end
+
+RSpec.shared_examples 'project snippet access levels' do
+  let(:project) { create(:project, project_visibility) }
+  let(:snippet) { create(:project_snippet, :repository, snippet_visibility, project: project) }
+
+  it_behaves_like 'unauthenticated project snippet access'
+
+  it_behaves_like 'non-member project snippet access'
+
+  it_behaves_like 'member project snippet access'
+end