Merge branch '4269-public-repositories-api' into 'master'

Allow Repositories API GET endpoints to be requested anonymously

Closes #4269

See merge request !8148

changelogs/unreleased/4269-public-repositories-api.yml

+---
+title: Allow Repositories API GET endpoints to be requested anonymously
+merge_request: 8148
+author:

doc/api/repositories.md

@@ -2,7 +2,8 @@
 
 ## List repository tree
 
-Get a list of repository files and directories in a project.
+Get a list of repository files and directories in a project. This endpoint can
+be accessed without authentication if the repository is publicly accessible.
 
 ```
 GET /projects/:id/repository/tree
@@ -71,7 +72,8 @@ Parameters:
 
 ## Raw file content
 
-Get the raw file contents for a file by commit SHA and path.
+Get the raw file contents for a file by commit SHA and path. This endpoint can
+be accessed without authentication if the repository is publicly accessible.
 
 ```
 GET /projects/:id/repository/blobs/:sha
@@ -85,7 +87,8 @@ Parameters:
 
 ## Raw blob content
 
-Get the raw file contents for a blob by blob SHA.
+Get the raw file contents for a blob by blob SHA. This endpoint can be accessed
+without authentication if the repository is publicly accessible.
 
 ```
 GET /projects/:id/repository/raw_blobs/:sha
@@ -98,7 +101,8 @@ Parameters:
 
 ## Get file archive
 
-Get an archive of the repository
+Get an archive of the repository. This endpoint can be accessed without
+authentication if the repository is publicly accessible.
 
 ```
 GET /projects/:id/repository/archive
@@ -111,6 +115,9 @@ Parameters:
 
 ## Compare branches, tags or commits
 
+This endpoint can be accessed without authentication if the repository is
+publicly accessible.
+
 ```
 GET /projects/:id/repository/compare
 ```
@@ -163,7 +170,8 @@ Response:
 
 ## Contributors
 
-Get repository contributors list
+Get repository contributors list. This endpoint can be accessed without
+authentication if the repository is publicly accessible.
 
 ```
 GET /projects/:id/repository/contributors

lib/api/repositories.rb

@@ -2,7 +2,6 @@ require 'mime/types'
 
 module API
   class Repositories < Grape::API
-    before { authenticate! }
     before { authorize! :download_code, user_project }
 
     params do
@@ -79,8 +78,6 @@ module API
         optional :format, type: String, desc: 'The archive format'
       end
       get ':id/repository/archive', requirements: { format: Gitlab::Regex.archive_formats_regex } do
-        authorize! :download_code, user_project
-
         begin
           send_git_archive user_project.repository, ref: params[:sha], format: params[:format]
         rescue
@@ -96,7 +93,6 @@ module API
         requires :to, type: String, desc: 'The commit, branch name, or tag name to stop comparison'
       end
       get ':id/repository/compare' do
-        authorize! :download_code, user_project
         compare = Gitlab::Git::Compare.new(user_project.repository.raw_repository, params[:from], params[:to])
         present compare, with: Entities::Compare
       end
@@ -105,8 +101,6 @@ module API
         success Entities::Contributor
       end
       get ':id/repository/contributors' do
-        authorize! :download_code, user_project
-
         begin
           present user_project.repository.contributors,
                   with: Entities::Contributor

spec/requests/api/repositories_spec.rb

@@ -7,52 +7,58 @@ describe API::Repositories, api: true  do
   include WorkhorseHelpers
 
   let(:user) { create(:user) }
-  let(:user2) { create(:user) }
+  let(:guest) { create(:user).tap { |u| create(:project_member, :guest, user: u, project: project) } }
   let!(:project) { create(:project, creator_id: user.id) }
   let!(:master) { create(:project_member, :master, user: user, project: project) }
-  let!(:guest) { create(:project_member, :guest, user: user2, project: project) }
+
+  shared_context 'disabled repository' do
+    before do
+      project.project_feature.update_attributes!(
+        repository_access_level: ProjectFeature::DISABLED,
+        merge_requests_access_level: ProjectFeature::DISABLED,
+        builds_access_level: ProjectFeature::DISABLED
+      )
+      expect(project.feature_available?(:repository, current_user)).to be false
+    end
+  end
 
   describe "GET /projects/:id/repository/tree" do
-    context "authorized user" do
-      before { project.team << [user2, :reporter] }
+    let(:route) { "/projects/#{project.id}/repository/tree" }
 
-      it "returns project commits" do
-        get api("/projects/#{project.id}/repository/tree", user)
+    shared_examples_for 'repository tree' do
+      it 'returns the repository tree' do
+        get api(route, current_user)
 
         expect(response).to have_http_status(200)
 
+        first_commit = json_response.first
+
         expect(json_response).to be_an Array
-        expect(json_response.first['name']).to eq('bar')
-        expect(json_response.first['type']).to eq('tree')
-        expect(json_response.first['mode']).to eq('040000')
+        expect(first_commit['name']).to eq('bar')
+        expect(first_commit['type']).to eq('tree')
+        expect(first_commit['mode']).to eq('040000')
       end
 
-      it 'returns a 404 for unknown ref' do
-        get api("/projects/#{project.id}/repository/tree?ref_name=foo", user)
-        expect(response).to have_http_status(404)
-
-        expect(json_response).to be_an Object
-        json_response['message'] == '404 Tree Not Found'
+      context 'when ref does not exist' do
+        it_behaves_like '404 response' do
+          let(:request) { get api("#{route}?ref_name=foo", current_user) }
+          let(:message) { '404 Tree Not Found' }
         end
       end
 
-    context "unauthorized user" do
-      it "does not return project commits" do
-        get api("/projects/#{project.id}/repository/tree")
-        expect(response).to have_http_status(401)
-      end
+      context 'when repository is disabled' do
+        include_context 'disabled repository'
+
+        it_behaves_like '403 response' do
+          let(:request) { get api(route, current_user) }
         end
       end
 
-  describe 'GET /projects/:id/repository/tree?recursive=1' do
-    context 'authorized user' do
-      before { project.team << [user2, :reporter] }
-
-      it 'should return recursive project paths tree' do
-        get api("/projects/#{project.id}/repository/tree?recursive=1", user)
+      context 'with recursive=1' do
+        it 'returns recursive project paths tree' do
+          get api("#{route}?recursive=1", current_user)
 
           expect(response.status).to eq(200)
-
           expect(json_response).to be_an Array
           expect(json_response[4]['name']).to eq('html')
           expect(json_response[4]['path']).to eq('files/html')
@@ -60,118 +66,272 @@ describe API::Repositories, api: true  do
           expect(json_response[4]['mode']).to eq('040000')
         end
 
-      it 'returns a 404 for unknown ref' do
-        get api("/projects/#{project.id}/repository/tree?ref_name=foo&recursive=1", user)
-        expect(response).to have_http_status(404)
+        context 'when repository is disabled' do
+          include_context 'disabled repository'
 
-        expect(json_response).to be_an Object
-        json_response['message'] == '404 Tree Not Found'
+          it_behaves_like '403 response' do
+            let(:request) { get api(route, current_user) }
           end
         end
 
-    context "unauthorized user" do
-      it "does not return project commits" do
-        get api("/projects/#{project.id}/repository/tree?recursive=1")
-        expect(response).to have_http_status(401)
+        context 'when ref does not exist' do
+          it_behaves_like '404 response' do
+            let(:request) { get api("#{route}?recursive=1&ref_name=foo", current_user) }
+            let(:message) { '404 Tree Not Found' }
+          end
         end
       end
     end
 
-  describe "GET /projects/:id/repository/blobs/:sha" do
-    it "gets the raw file contents" do
-      get api("/projects/#{project.id}/repository/blobs/master?filepath=README.md", user)
-      expect(response).to have_http_status(200)
+    context 'when unauthenticated', 'and project is public' do
+      it_behaves_like 'repository tree' do
+        let(:project) { create(:project, :public) }
+        let(:current_user) { nil }
+      end
     end
 
-    it "returns 404 for invalid branch_name" do
-      get api("/projects/#{project.id}/repository/blobs/invalid_branch_name?filepath=README.md", user)
-      expect(response).to have_http_status(404)
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get api(route) }
+        let(:message) { '404 Project Not Found' }
+      end
     end
 
-    it "returns 404 for invalid file" do
-      get api("/projects/#{project.id}/repository/blobs/master?filepath=README.invalid", user)
-      expect(response).to have_http_status(404)
+    context 'when authenticated', 'as a developer' do
+      it_behaves_like 'repository tree' do
+        let(:current_user) { user }
+      end
     end
 
-    it "returns a 400 error if filepath is missing" do
-      get api("/projects/#{project.id}/repository/blobs/master", user)
-      expect(response).to have_http_status(400)
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, guest) }
+      end
     end
   end
 
-  describe "GET /projects/:id/repository/commits/:sha/blob" do
-    it "gets the raw file contents" do
-      get api("/projects/#{project.id}/repository/commits/master/blob?filepath=README.md", user)
+  {
+    'blobs/:sha' => 'blobs/master',
+    'commits/:sha/blob' => 'commits/master/blob'
+  }.each do |desc_path, example_path|
+    describe "GET /projects/:id/repository/#{desc_path}" do
+      let(:route) { "/projects/#{project.id}/repository/#{example_path}?filepath=README.md" }
+
+      shared_examples_for 'repository blob' do
+        it 'returns the repository blob' do
+          get api(route, current_user)
+
           expect(response).to have_http_status(200)
         end
+
+        context 'when sha does not exist' do
+          it_behaves_like '404 response' do
+            let(:request) { get api(route.sub('master', 'invalid_branch_name'), current_user) }
+            let(:message) { '404 Commit Not Found' }
+          end
+        end
+
+        context 'when filepath does not exist' do
+          it_behaves_like '404 response' do
+            let(:request) { get api(route.sub('README.md', 'README.invalid'), current_user) }
+            let(:message) { '404 File Not Found' }
+          end
+        end
+
+        context 'when no filepath is given' do
+          it_behaves_like '400 response' do
+            let(:request) { get api(route.sub('?filepath=README.md', ''), current_user) }
+          end
+        end
+
+        context 'when repository is disabled' do
+          include_context 'disabled repository'
+
+          it_behaves_like '403 response' do
+            let(:request) { get api(route, current_user) }
+          end
+        end
+      end
+
+      context 'when unauthenticated', 'and project is public' do
+        it_behaves_like 'repository blob' do
+          let(:project) { create(:project, :public) }
+          let(:current_user) { nil }
+        end
+      end
+
+      context 'when unauthenticated', 'and project is private' do
+        it_behaves_like '404 response' do
+          let(:request) { get api(route) }
+          let(:message) { '404 Project Not Found' }
+        end
+      end
+
+      context 'when authenticated', 'as a developer' do
+        it_behaves_like 'repository blob' do
+          let(:current_user) { user }
+        end
+      end
+
+      context 'when authenticated', 'as a guest' do
+        it_behaves_like '403 response' do
+          let(:request) { get api(route, guest) }
+        end
+      end
+    end
   end
 
   describe "GET /projects/:id/repository/raw_blobs/:sha" do
-    it "gets the raw file contents" do
-      get api("/projects/#{project.id}/repository/raw_blobs/#{sample_blob.oid}", user)
+    let(:route) { "/projects/#{project.id}/repository/raw_blobs/#{sample_blob.oid}" }
+
+    shared_examples_for 'repository raw blob' do
+      it 'returns the repository raw blob' do
+        get api(route, current_user)
+
         expect(response).to have_http_status(200)
       end
 
-    it 'returns a 404 for unknown blob' do
-      get api("/projects/#{project.id}/repository/raw_blobs/123456", user)
-      expect(response).to have_http_status(404)
+      context 'when sha does not exist' do
+        it_behaves_like '404 response' do
+          let(:request) { get api(route.sub(sample_blob.oid, '123456'), current_user) }
+          let(:message) { '404 Blob Not Found' }
+        end
+      end
+
+      context 'when repository is disabled' do
+        include_context 'disabled repository'
 
-      expect(json_response).to be_an Object
-      json_response['message'] == '404 Blob Not Found'
+        it_behaves_like '403 response' do
+          let(:request) { get api(route, current_user) }
+        end
+      end
+    end
+
+    context 'when unauthenticated', 'and project is public' do
+      it_behaves_like 'repository raw blob' do
+        let(:project) { create(:project, :public) }
+        let(:current_user) { nil }
+      end
+    end
+
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get api(route) }
+        let(:message) { '404 Project Not Found' }
+      end
+    end
+
+    context 'when authenticated', 'as a developer' do
+      it_behaves_like 'repository raw blob' do
+        let(:current_user) { user }
+      end
+    end
+
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, guest) }
+      end
     end
   end
 
   describe "GET /projects/:id/repository/archive(.:format)?:sha" do
-    it "gets the archive" do
-      get api("/projects/#{project.id}/repository/archive", user)
-      repo_name = project.repository.name.gsub("\.git", "")
+    let(:route) { "/projects/#{project.id}/repository/archive" }
+
+    shared_examples_for 'repository archive' do
+      it 'returns the repository archive' do
+        get api(route, current_user)
+
         expect(response).to have_http_status(200)
+
+        repo_name = project.repository.name.gsub("\.git", "")
         type, params = workhorse_send_data
+
         expect(type).to eq('git-archive')
         expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.gz/)
       end
 
-    it "gets the archive.zip" do
+      it 'returns the repository archive archive.zip' do
         get api("/projects/#{project.id}/repository/archive.zip", user)
-      repo_name = project.repository.name.gsub("\.git", "")
+
         expect(response).to have_http_status(200)
+
+        repo_name = project.repository.name.gsub("\.git", "")
         type, params = workhorse_send_data
+
         expect(type).to eq('git-archive')
         expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.zip/)
       end
 
-    it "gets the archive.tar.bz2" do
+      it 'returns the repository archive archive.tar.bz2' do
         get api("/projects/#{project.id}/repository/archive.tar.bz2", user)
-      repo_name = project.repository.name.gsub("\.git", "")
+
         expect(response).to have_http_status(200)
+
+        repo_name = project.repository.name.gsub("\.git", "")
         type, params = workhorse_send_data
+
         expect(type).to eq('git-archive')
         expect(params['ArchivePath']).to match(/#{repo_name}\-[^\.]+\.tar.bz2/)
       end
 
-    it "returns 404 for invalid sha" do
-      get api("/projects/#{project.id}/repository/archive/?sha=xxx", user)
-      expect(response).to have_http_status(404)
+      context 'when sha does not exist' do
+        it_behaves_like '404 response' do
+          let(:request) { get api("#{route}?sha=xxx", current_user) }
+          let(:message) { '404 File Not Found' }
+        end
+      end
+    end
+
+    context 'when unauthenticated', 'and project is public' do
+      it_behaves_like 'repository archive' do
+        let(:project) { create(:project, :public) }
+        let(:current_user) { nil }
+      end
+    end
+
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get api(route) }
+        let(:message) { '404 Project Not Found' }
+      end
+    end
+
+    context 'when authenticated', 'as a developer' do
+      it_behaves_like 'repository archive' do
+        let(:current_user) { user }
+      end
+    end
+
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, guest) }
+      end
     end
   end
 
   describe 'GET /projects/:id/repository/compare' do
+    let(:route) { "/projects/#{project.id}/repository/compare" }
+
+    shared_examples_for 'repository compare' do
       it "compares branches" do
-      get api("/projects/#{project.id}/repository/compare", user), from: 'master', to: 'feature'
+        get api(route, current_user), from: 'master', to: 'feature'
+
         expect(response).to have_http_status(200)
         expect(json_response['commits']).to be_present
         expect(json_response['diffs']).to be_present
       end
 
       it "compares tags" do
-      get api("/projects/#{project.id}/repository/compare", user), from: 'v1.0.0', to: 'v1.1.0'
+        get api(route, current_user), from: 'v1.0.0', to: 'v1.1.0'
+
         expect(response).to have_http_status(200)
         expect(json_response['commits']).to be_present
         expect(json_response['diffs']).to be_present
       end
 
       it "compares commits" do
-      get api("/projects/#{project.id}/repository/compare", user), from: sample_commit.id, to: sample_commit.parent_id
+        get api(route, current_user), from: sample_commit.id, to: sample_commit.parent_id
+
         expect(response).to have_http_status(200)
         expect(json_response['commits']).to be_empty
         expect(json_response['diffs']).to be_empty
@@ -179,14 +339,16 @@ describe API::Repositories, api: true  do
       end
 
       it "compares commits in reverse order" do
-      get api("/projects/#{project.id}/repository/compare", user), from: sample_commit.parent_id, to: sample_commit.id
+        get api(route, current_user), from: sample_commit.parent_id, to: sample_commit.id
+
         expect(response).to have_http_status(200)
         expect(json_response['commits']).to be_present
         expect(json_response['diffs']).to be_present
       end
 
       it "compares same refs" do
-      get api("/projects/#{project.id}/repository/compare", user), from: 'master', to: 'master'
+        get api(route, current_user), from: 'master', to: 'master'
+
         expect(response).to have_http_status(200)
         expect(json_response['commits']).to be_empty
         expect(json_response['diffs']).to be_empty
@@ -194,17 +356,77 @@ describe API::Repositories, api: true  do
       end
     end
 
+    context 'when unauthenticated', 'and project is public' do
+      it_behaves_like 'repository compare' do
+        let(:project) { create(:project, :public) }
+        let(:current_user) { nil }
+      end
+    end
+
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get api(route) }
+        let(:message) { '404 Project Not Found' }
+      end
+    end
+
+    context 'when authenticated', 'as a developer' do
+      it_behaves_like 'repository compare' do
+        let(:current_user) { user }
+      end
+    end
+
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, guest) }
+      end
+    end
+  end
+
   describe 'GET /projects/:id/repository/contributors' do
+    let(:route) { "/projects/#{project.id}/repository/contributors" }
+
+    shared_examples_for 'repository contributors' do
       it 'returns valid data' do
-      get api("/projects/#{project.id}/repository/contributors", user)
+        get api(route, current_user)
+
         expect(response).to have_http_status(200)
         expect(json_response).to be_an Array
-      contributor = json_response.first
-      expect(contributor['email']).to eq('tiagonbotelho@hotmail.com')
-      expect(contributor['name']).to eq('tiagonbotelho')
-      expect(contributor['commits']).to eq(1)
-      expect(contributor['additions']).to eq(0)
-      expect(contributor['deletions']).to eq(0)
+
+        first_contributor = json_response.first
+
+        expect(first_contributor['email']).to eq('tiagonbotelho@hotmail.com')
+        expect(first_contributor['name']).to eq('tiagonbotelho')
+        expect(first_contributor['commits']).to eq(1)
+        expect(first_contributor['additions']).to eq(0)
+        expect(first_contributor['deletions']).to eq(0)
+      end
+    end
+
+    context 'when unauthenticated', 'and project is public' do
+      it_behaves_like 'repository contributors' do
+        let(:project) { create(:project, :public) }
+        let(:current_user) { nil }
+      end
+    end
+
+    context 'when unauthenticated', 'and project is private' do
+      it_behaves_like '404 response' do
+        let(:request) { get api(route) }
+        let(:message) { '404 Project Not Found' }
+      end
+    end
+
+    context 'when authenticated', 'as a developer' do
+      it_behaves_like 'repository contributors' do
+        let(:current_user) { user }
+      end
+    end
+
+    context 'when authenticated', 'as a guest' do
+      it_behaves_like '403 response' do
+        let(:request) { get api(route, guest) }
+      end
     end
   end
 end

spec/support/api/status_shared_examples.rb

+# Specs for status checking.
+#
+# Requires an API request:
+#   let(:request) { get api("/projects/#{project.id}/repository/branches", user) }
+shared_examples_for '400 response' do
+  before do
+    # Fires the request
+    request
+  end
+
+  it 'returns 400' do
+    expect(response).to have_http_status(400)
+  end
+end
+
+shared_examples_for '403 response' do
+  before do
+    # Fires the request
+    request
+  end
+
+  it 'returns 403' do
+    expect(response).to have_http_status(403)
+  end
+end
+
+shared_examples_for '404 response' do
+  let(:message) { nil }
+  before do
+    # Fires the request
+    request
+  end
+
+  it 'returns 404' do
+    expect(response).to have_http_status(404)
+    expect(json_response).to be_an Object
+
+    if message.present?
+      expect(json_response['message']).to eq(message)
+    end
+  end
+end