From ff8a053d5ddf154cd52c3e21ac24619dbbee0dc7 Mon Sep 17 00:00:00 2001
From: Michael Kozono <mkozono@gmail.com>
Date: Mon, 15 May 2017 16:13:36 -0700
Subject: Fix Git over HTTP spec
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* The spec has 7 failures at this point
* Specify rendered error messages
* Render the GitAccess message rather than “Access denied”
* Render the Not Found message provided by GitAccess, instead of a custom one
* Expect GitAccess to check the config for whether Git-over-HTTP pull or push is disabled, rather than doing it in the controller
* Add more thorough testing for authentication
* Dried up a lot of tests
* Fixed some broken tests
---
 spec/requests/git_http_spec.rb | 616 ++++++++++++++++++++++++-----------------
 1 file changed, 364 insertions(+), 252 deletions(-)

(limited to 'spec/requests/git_http_spec.rb')

diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 6ca3ef18fe6..080e2f12cd7 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -5,76 +5,217 @@ describe 'Git HTTP requests', lib: true do
   include WorkhorseHelpers
   include UserActivitiesHelpers
 
-  it "gives WWW-Authenticate hints" do
-    clone_get('doesnt/exist.git')
+  shared_examples 'pulls require Basic HTTP Authentication' do
+    context "when no credentials are provided" do
+      it "responds to downloads with status 401 Unauthorized (no project existence information leak)" do
+        download(path) do |response|
+          expect(response).to have_http_status(:unauthorized)
+          expect(response.header['WWW-Authenticate']).to start_with('Basic ')
+        end
+      end
+    end
 
-    expect(response.header['WWW-Authenticate']).to start_with('Basic ')
-  end
+    context "when only username is provided" do
+      it "responds to downloads with status 401 Unauthorized" do
+        download(path, user: user.username) do |response|
+          expect(response).to have_http_status(:unauthorized)
+          expect(response.header['WWW-Authenticate']).to start_with('Basic ')
+        end
+      end
+    end
 
-  describe "User with no identities" do
-    let(:user)    { create(:user) }
-    let(:project) { create(:project, :repository, path: 'project.git-project') }
+    context "when username and password are provided" do
+      context "when authentication fails" do
+        it "responds to downloads with status 401 Unauthorized" do
+          download(path, user: user.username, password: "wrong-password") do |response|
+            expect(response).to have_http_status(:unauthorized)
+            expect(response.header['WWW-Authenticate']).to start_with('Basic ')
+          end
+        end
+      end
 
-    context "when the project doesn't exist" do
-      context "when no authentication is provided" do
-        it "responds with status 401 (no project existence information leak)" do
-          download('doesnt/exist.git') do |response|
-            expect(response).to have_http_status(401)
+      context "when authentication succeeds" do
+        it "does not respond to downloads with status 401 Unauthorized" do
+          download(path, user: user.username, password: user.password) do |response|
+            expect(response).not_to have_http_status(:unauthorized)
+            expect(response.header['WWW-Authenticate']).to be_nil
           end
         end
       end
+    end
+  end
 
-      context "when username and password are provided" do
-        context "when authentication fails" do
-          it "responds with status 401" do
-            download('doesnt/exist.git', user: user.username, password: "nope") do |response|
-              expect(response).to have_http_status(401)
-            end
+  shared_examples 'pushes require Basic HTTP Authentication' do
+    context "when no credentials are provided" do
+      it "responds to uploads with status 401 Unauthorized (no project existence information leak)" do
+        upload(path) do |response|
+          expect(response).to have_http_status(:unauthorized)
+          expect(response.header['WWW-Authenticate']).to start_with('Basic ')
+        end
+      end
+    end
+
+    context "when only username is provided" do
+      it "responds to uploads with status 401 Unauthorized" do
+        upload(path, user: user.username) do |response|
+          expect(response).to have_http_status(:unauthorized)
+          expect(response.header['WWW-Authenticate']).to start_with('Basic ')
+        end
+      end
+    end
+
+    context "when username and password are provided" do
+      context "when authentication fails" do
+        it "responds to uploads with status 401 Unauthorized" do
+          upload(path, user: user.username, password: "wrong-password") do |response|
+            expect(response).to have_http_status(:unauthorized)
+            expect(response.header['WWW-Authenticate']).to start_with('Basic ')
           end
         end
+      end
 
-        context "when authentication succeeds" do
-          it "responds with status 404" do
-            download('/doesnt/exist.git', user: user.username, password: user.password) do |response|
-              expect(response).to have_http_status(404)
-            end
+      context "when authentication succeeds" do
+        it "does not respond to uploads with status 401 Unauthorized" do
+          upload(path, user: user.username, password: user.password) do |response|
+            expect(response).not_to have_http_status(:unauthorized)
+            expect(response.header['WWW-Authenticate']).to be_nil
           end
         end
       end
     end
+  end
+
+  shared_examples_for 'pulls are allowed' do
+    it do
+      download(path, env) do |response|
+        expect(response).to have_http_status(:ok)
+        expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+      end
+    end
+  end
+
+  shared_examples_for 'pushes are allowed' do
+    it do
+      upload(path, env) do |response|
+        expect(response).to have_http_status(:ok)
+        expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+      end
+    end
+  end
+
+  describe "User with no identities" do
+    let(:user) { create(:user) }
 
-    context "when the Wiki for a project exists" do
-      it "responds with the right project" do
-        wiki = ProjectWiki.new(project)
-        project.update_attribute(:visibility_level, Project::PUBLIC)
+    context "when the project doesn't exist" do
+      let(:path) { 'doesnt/exist.git' }
 
-        download("/#{wiki.repository.path_with_namespace}.git") do |response|
-          json_body = ActiveSupport::JSON.decode(response.body)
+      it_behaves_like 'pulls require Basic HTTP Authentication'
+      it_behaves_like 'pushes require Basic HTTP Authentication'
 
-          expect(response).to have_http_status(200)
-          expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace)
-          expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+      context 'when authenticated' do
+        it 'rejects downloads and uploads with 404 Not Found' do
+          download_or_upload(path, user: user.username, password: user.password) do |response|
+            expect(response).to have_http_status(:not_found)
+          end
         end
       end
+    end
+
+    context "when requesting the Wiki" do
+      let(:wiki) { ProjectWiki.new(project) }
+      let(:path) { "/#{wiki.repository.path_with_namespace}.git" }
+
+      context "when the project is public" do
+        let(:project) { create(:project, :repository, :public, :wiki_enabled) }
+
+        it_behaves_like 'pushes require Basic HTTP Authentication'
 
-      context 'but the repo is disabled' do
-        let(:project) { create(:project, :repository_disabled, :wiki_enabled) }
-        let(:wiki) { ProjectWiki.new(project) }
-        let(:path) { "/#{wiki.repository.path_with_namespace}.git" }
+        context 'when unauthenticated' do
+          let(:env) { {} }
 
-        before do
-          project.team << [user, :developer]
+          it_behaves_like 'pulls are allowed'
+
+          it "responds to pulls with the wiki's repo" do
+            download(path) do |response|
+              json_body = ActiveSupport::JSON.decode(response.body)
+
+              expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace)
+            end
+          end
         end
 
-        it 'allows clones' do
-          download(path, user: user.username, password: user.password) do |response|
-            expect(response).to have_http_status(200)
+        context 'when authenticated' do
+          let(:env) { { user: user.username, password: user.password } }
+
+          context 'and as a developer on the team' do
+            before do
+              project.team << [user, :developer]
+            end
+
+            context 'but the repo is disabled' do
+              let(:project) { create(:project, :repository, :public, :repository_disabled, :wiki_enabled) }
+
+              it_behaves_like 'pulls are allowed'
+              it_behaves_like 'pushes are allowed'
+            end
+          end
+
+          context 'and not on the team' do
+            it_behaves_like 'pulls are allowed'
+
+            it 'rejects pushes with 403 Forbidden' do
+              upload(path, env) do |response|
+                expect(response).to have_http_status(:forbidden)
+                expect(response.body).to eq(git_access_wiki_error(:write_to_wiki))
+              end
+            end
           end
         end
+      end
 
-        it 'allows pushes' do
-          upload(path, user: user.username, password: user.password) do |response|
-            expect(response).to have_http_status(200)
+      context "when the project is private" do
+        let(:project) { create(:project, :repository, :private, :wiki_enabled) }
+
+        it_behaves_like 'pulls require Basic HTTP Authentication'
+        it_behaves_like 'pushes require Basic HTTP Authentication'
+
+        context 'when authenticated' do
+          context 'and as a developer on the team' do
+            before do
+              project.team << [user, :developer]
+            end
+
+            context 'but the repo is disabled' do
+              let(:project) { create(:project, :repository, :private, :repository_disabled, :wiki_enabled) }
+
+              it 'allows clones' do
+                download(path, user: user.username, password: user.password) do |response|
+                  expect(response).to have_http_status(:ok)
+                end
+              end
+
+              it 'pushes are allowed' do
+                upload(path, user: user.username, password: user.password) do |response|
+                  expect(response).to have_http_status(:ok)
+                end
+              end
+            end
+          end
+
+          context 'and not on the team' do
+            it 'rejects clones with 404 Not Found' do
+              download(path, user: user.username, password: user.password) do |response|
+                expect(response).to have_http_status(:not_found)
+                expect(response.body).to eq(git_access_error(:project_not_found))
+              end
+            end
+
+            it 'rejects pushes with 404 Not Found' do
+              upload(path, user: user.username, password: user.password) do |response|
+                expect(response).to have_http_status(:not_found)
+                expect(response.body).to eq(git_access_error(:project_not_found))
+              end
+            end
           end
         end
       end
@@ -84,49 +225,60 @@ describe 'Git HTTP requests', lib: true do
       let(:path) { "#{project.path_with_namespace}.git" }
 
       context "when the project is public" do
-        before do
-          project.update_attribute(:visibility_level, Project::PUBLIC)
-        end
+        let(:project) { create(:project, :repository, :public) }
 
-        it "downloads get status 200" do
-          download(path, {}) do |response|
-            expect(response).to have_http_status(200)
-            expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
-          end
-        end
+        it_behaves_like 'pushes require Basic HTTP Authentication'
 
-        it "uploads get status 401" do
-          upload(path, {}) do |response|
-            expect(response).to have_http_status(401)
-          end
+        context 'when not authenticated' do
+          let(:env) { {} }
+
+          it_behaves_like 'pulls are allowed'
         end
 
-        context "with correct credentials" do
+        context "when authenticated" do
           let(:env) { { user: user.username, password: user.password } }
 
-          it "uploads get status 403" do
-            upload(path, env) do |response|
-              expect(response).to have_http_status(403)
+          context 'as a developer on the team' do
+            before do
+              project.team << [user, :developer]
             end
-          end
 
-          context 'but git-receive-pack is disabled' do
-            it "responds with status 404" do
-              allow(Gitlab.config.gitlab_shell).to receive(:receive_pack).and_return(false)
+            it_behaves_like 'pulls are allowed'
+            it_behaves_like 'pushes are allowed'
 
-              upload(path, env) do |response|
-                expect(response).to have_http_status(403)
+            context 'but git-receive-pack over HTTP is disabled in config' do
+              before do
+                allow(Gitlab.config.gitlab_shell).to receive(:receive_pack).and_return(false)
+              end
+
+              it 'rejects pushes with 403 Forbidden' do
+                upload(path, env) do |response|
+                  expect(response).to have_http_status(:forbidden)
+                  expect(response.body).to eq(git_access_error(:receive_pack_disabled_in_config))
+                end
+              end
+            end
+
+            context 'but git-upload-pack over HTTP is disabled in config' do
+              it "rejects pushes with 403 Forbidden" do
+                allow(Gitlab.config.gitlab_shell).to receive(:upload_pack).and_return(false)
+
+                download(path, env) do |response|
+                  expect(response).to have_http_status(:forbidden)
+                  expect(response.body).to eq(git_access_error(:upload_pack_disabled_in_config))
+                end
               end
             end
           end
-        end
 
-        context 'but git-upload-pack is disabled' do
-          it "responds with status 404" do
-            allow(Gitlab.config.gitlab_shell).to receive(:upload_pack).and_return(false)
+          context 'and not a member of the team' do
+            it_behaves_like 'pulls are allowed'
 
-            download(path, {}) do |response|
-              expect(response).to have_http_status(404)
+            it 'rejects pushes with 403 Forbidden' do
+              upload(path, env) do |response|
+                expect(response).to have_http_status(:forbidden)
+                expect(response.body).to eq(change_access_error(:push_code))
+              end
             end
           end
         end
@@ -141,66 +293,41 @@ describe 'Git HTTP requests', lib: true do
 
         context 'when the repo is public' do
           context 'but the repo is disabled' do
-            it 'does not allow to clone the repo' do
-              project = create(:project, :public, :repository_disabled)
+            let(:project) { create(:project, :public, :repository, :repository_disabled) }
+            let(:path) { "#{project.path_with_namespace}.git" }
+            let(:env) { {} }
 
-              download("#{project.path_with_namespace}.git", {}) do |response|
-                expect(response).to have_http_status(:unauthorized)
-              end
-            end
+            it_behaves_like 'pulls require Basic HTTP Authentication'
+            it_behaves_like 'pushes require Basic HTTP Authentication'
           end
 
           context 'but the repo is enabled' do
-            it 'allows to clone the repo' do
-              project = create(:project, :public, :repository_enabled)
+            let(:project) { create(:project, :public, :repository, :repository_enabled) }
+            let(:path) { "#{project.path_with_namespace}.git" }
+            let(:env) { {} }
 
-              download("#{project.path_with_namespace}.git", {}) do |response|
-                expect(response).to have_http_status(:ok)
-              end
-            end
+            it_behaves_like 'pulls are allowed'
           end
 
           context 'but only project members are allowed' do
-            it 'does not allow to clone the repo' do
-              project = create(:project, :public, :repository_private)
+            let(:project) { create(:project, :public, :repository, :repository_private) }
 
-              download("#{project.path_with_namespace}.git", {}) do |response|
-                expect(response).to have_http_status(:unauthorized)
-              end
-            end
+            it_behaves_like 'pulls require Basic HTTP Authentication'
+            it_behaves_like 'pushes require Basic HTTP Authentication'
           end
         end
       end
 
       context "when the project is private" do
-        before do
-          project.update_attribute(:visibility_level, Project::PRIVATE)
-        end
+        let(:project) { create(:project, :repository, :private) }
 
-        context "when no authentication is provided" do
-          it "responds with status 401 to downloads" do
-            download(path, {}) do |response|
-              expect(response).to have_http_status(401)
-            end
-          end
-
-          it "responds with status 401 to uploads" do
-            upload(path, {}) do |response|
-              expect(response).to have_http_status(401)
-            end
-          end
-        end
+        it_behaves_like 'pulls require Basic HTTP Authentication'
+        it_behaves_like 'pushes require Basic HTTP Authentication'
 
         context "when username and password are provided" do
           let(:env) { { user: user.username, password: 'nope' } }
 
           context "when authentication fails" do
-            it "responds with status 401" do
-              download(path, env) do |response|
-                expect(response).to have_http_status(401)
-              end
-            end
-
             context "when the user is IP banned" do
               it "responds with status 401" do
                 expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true)
@@ -208,7 +335,7 @@ describe 'Git HTTP requests', lib: true do
 
                 clone_get(path, env)
 
-                expect(response).to have_http_status(401)
+                expect(response).to have_http_status(:unauthorized)
               end
             end
           end
@@ -222,37 +349,39 @@ describe 'Git HTTP requests', lib: true do
               end
 
               context "when the user is blocked" do
-                it "responds with status 401" do
+                it "rejects pulls with 401 Unauthorized" do
                   user.block
                   project.team << [user, :master]
 
                   download(path, env) do |response|
-                    expect(response).to have_http_status(401)
+                    expect(response).to have_http_status(:unauthorized)
                   end
                 end
 
-                it "responds with status 401 for unknown projects (no project existence information leak)" do
+                it "rejects pulls with 401 Unauthorized for unknown projects (no project existence information leak)" do
                   user.block
 
                   download('doesnt/exist.git', env) do |response|
-                    expect(response).to have_http_status(401)
+                    expect(response).to have_http_status(:unauthorized)
                   end
                 end
               end
 
               context "when the user isn't blocked" do
-                it "downloads get status 200" do
-                  expect(Rack::Attack::Allow2Ban).to receive(:reset)
-
-                  clone_get(path, env)
+                it "resets the IP in Rack Attack on download" do
+                  expect(Rack::Attack::Allow2Ban).to receive(:reset).twice
 
-                  expect(response).to have_http_status(200)
-                  expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+                  download(path, env) do
+                    expect(response).to have_http_status(:ok)
+                    expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
+                  end
                 end
 
-                it "uploads get status 200" do
-                  upload(path, env) do |response|
-                    expect(response).to have_http_status(200)
+                it "resets the IP in Rack Attack on upload" do
+                  expect(Rack::Attack::Allow2Ban).to receive(:reset).twice
+
+                  upload(path, env) do
+                    expect(response).to have_http_status(:ok)
                     expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
                   end
                 end
@@ -272,56 +401,43 @@ describe 'Git HTTP requests', lib: true do
                   @token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api")
                 end
 
-                it "downloads get status 200" do
-                  clone_get "#{project.path_with_namespace}.git", user: 'oauth2', password: @token.token
-
-                  expect(response).to have_http_status(200)
-                  expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
-                end
-
-                it "uploads get status 200" do
-                  push_get "#{project.path_with_namespace}.git", user: 'oauth2', password: @token.token
+                let(:path) { "#{project.path_with_namespace}.git" }
+                let(:env) { { user: 'oauth2', password: @token.token } }
 
-                  expect(response).to have_http_status(200)
-                end
+                it_behaves_like 'pulls are allowed'
+                it_behaves_like 'pushes are allowed'
               end
 
               context 'when user has 2FA enabled' do
                 let(:user) { create(:user, :two_factor) }
                 let(:access_token) { create(:personal_access_token, user: user) }
+                let(:path) { "#{project.path_with_namespace}.git" }
 
                 before do
                   project.team << [user, :master]
                 end
 
                 context 'when username and password are provided' do
-                  it 'rejects the clone attempt' do
-                    download("#{project.path_with_namespace}.git", user: user.username, password: user.password) do |response|
-                      expect(response).to have_http_status(401)
+                  it 'rejects pulls with 2FA error message' do
+                    download(path, user: user.username, password: user.password) do |response|
+                      expect(response).to have_http_status(:unauthorized)
                       expect(response.body).to include('You have 2FA enabled, please use a personal access token for Git over HTTP')
                     end
                   end
 
                   it 'rejects the push attempt' do
-                    upload("#{project.path_with_namespace}.git", user: user.username, password: user.password) do |response|
-                      expect(response).to have_http_status(401)
+                    upload(path, user: user.username, password: user.password) do |response|
+                      expect(response).to have_http_status(:unauthorized)
                       expect(response.body).to include('You have 2FA enabled, please use a personal access token for Git over HTTP')
                     end
                   end
                 end
 
                 context 'when username and personal access token are provided' do
-                  it 'allows clones' do
-                    download("#{project.path_with_namespace}.git", user: user.username, password: access_token.token) do |response|
-                      expect(response).to have_http_status(200)
-                    end
-                  end
+                  let(:env) { { user: user.username, password: access_token.token } }
 
-                  it 'allows pushes' do
-                    upload("#{project.path_with_namespace}.git", user: user.username, password: access_token.token) do |response|
-                      expect(response).to have_http_status(200)
-                    end
-                  end
+                  it_behaves_like 'pulls are allowed'
+                  it_behaves_like 'pushes are allowed'
                 end
               end
 
@@ -357,15 +473,15 @@ describe 'Git HTTP requests', lib: true do
             end
 
             context "when the user doesn't have access to the project" do
-              it "downloads get status 404" do
+              it "pulls get status 404" do
                 download(path, user: user.username, password: user.password) do |response|
-                  expect(response).to have_http_status(404)
+                  expect(response).to have_http_status(:not_found)
                 end
               end
 
               it "uploads get status 404" do
                 upload(path, user: user.username, password: user.password) do |response|
-                  expect(response).to have_http_status(404)
+                  expect(response).to have_http_status(:not_found)
                 end
               end
             end
@@ -378,23 +494,24 @@ describe 'Git HTTP requests', lib: true do
           let(:other_project) { create(:empty_project) }
 
           context 'when build created by system is authenticated' do
-            it "downloads get status 200" do
-              clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+            let(:path) { "#{project.path_with_namespace}.git" }
+            let(:env) { { user: 'gitlab-ci-token', password: build.token } }
 
-              expect(response).to have_http_status(200)
-              expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
-            end
+            it_behaves_like 'pulls are allowed'
 
-            it "uploads get status 401 (no project existence information leak)" do
-              push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+            # TODO Verify this is desired behavior
+            it "rejects pushes with 401 Unauthorized (no project existence information leak)" do
+              push_get(path, env)
 
-              expect(response).to have_http_status(401)
+              expect(response).to have_http_status(:unauthorized)
             end
 
-            it "downloads from other project get status 404" do
-              clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
+            # TODO Verify this is desired behavior. Should be 403 Forbidden?
+            it "rejects pulls for other project with 404 Not Found" do
+              clone_get("#{other_project.path_with_namespace}.git", env)
 
-              expect(response).to have_http_status(404)
+              expect(response).to have_http_status(:not_found)
+              expect(response.body).to eq('TODO: What should this be?')
             end
           end
 
@@ -412,7 +529,7 @@ describe 'Git HTTP requests', lib: true do
                 clone_get "#{project.path_with_namespace}.git",
                   user: 'gitlab-ci-token', password: build.token
 
-                expect(response).to have_http_status(200)
+                expect(response).to have_http_status(:ok)
                 expect(response.content_type.to_s).to eq(Gitlab::Workhorse::INTERNAL_API_CONTENT_TYPE)
               end
 
@@ -423,13 +540,13 @@ describe 'Git HTTP requests', lib: true do
                 clone_get "#{project.path_with_namespace}.git",
                   user: 'gitlab-ci-token', password: build.token
 
-                expect(response).to have_http_status(403)
+                expect(response).to have_http_status(:forbidden)
               end
 
               it 'uploads get status 403' do
                 push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
 
-                expect(response).to have_http_status(401)
+                expect(response).to have_http_status(:unauthorized)
               end
             end
 
@@ -441,7 +558,7 @@ describe 'Git HTTP requests', lib: true do
               it 'downloads from other project get status 403' do
                 clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
 
-                expect(response).to have_http_status(403)
+                expect(response).to have_http_status(:forbidden)
               end
             end
 
@@ -453,91 +570,93 @@ describe 'Git HTTP requests', lib: true do
               it 'downloads from other project get status 404' do
                 clone_get "#{other_project.path_with_namespace}.git", user: 'gitlab-ci-token', password: build.token
 
-                expect(response).to have_http_status(404)
+                expect(response).to have_http_status(:not_found)
               end
             end
           end
         end
       end
-    end
 
-    context "when the project path doesn't end in .git" do
-      context "GET info/refs" do
-        let(:path) { "/#{project.path_with_namespace}/info/refs" }
+      context "when the project path doesn't end in .git" do
+        let(:project) { create(:project, :repository, :public, path: 'project.git-project') }
+
+        context "GET info/refs" do
+          let(:path) { "/#{project.path_with_namespace}/info/refs" }
 
-        context "when no params are added" do
-          before { get path }
+          context "when no params are added" do
+            before { get path }
 
-          it "redirects to the .git suffix version" do
-            expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs")
+            it "redirects to the .git suffix version" do
+              expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs")
+            end
           end
-        end
 
-        context "when the upload-pack service is requested" do
-          let(:params) { { service: 'git-upload-pack' } }
-          before { get path, params }
+          context "when the upload-pack service is requested" do
+            let(:params) { { service: 'git-upload-pack' } }
+            before { get path, params }
 
-          it "redirects to the .git suffix version" do
-            expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}")
+            it "redirects to the .git suffix version" do
+              expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}")
+            end
           end
-        end
 
-        context "when the receive-pack service is requested" do
-          let(:params) { { service: 'git-receive-pack' } }
-          before { get path, params }
+          context "when the receive-pack service is requested" do
+            let(:params) { { service: 'git-receive-pack' } }
+            before { get path, params }
 
-          it "redirects to the .git suffix version" do
-            expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}")
+            it "redirects to the .git suffix version" do
+              expect(response).to redirect_to("/#{project.path_with_namespace}.git/info/refs?service=#{params[:service]}")
+            end
           end
-        end
 
-        context "when the params are anything else" do
-          let(:params) { { service: 'git-implode-pack' } }
-          before { get path, params }
+          context "when the params are anything else" do
+            let(:params) { { service: 'git-implode-pack' } }
+            before { get path, params }
 
-          it "redirects to the sign-in page" do
-            expect(response).to redirect_to(new_user_session_path)
+            it "redirects to the sign-in page" do
+              expect(response).to redirect_to(new_user_session_path)
+            end
           end
         end
-      end
 
-      context "POST git-upload-pack" do
-        it "fails to find a route" do
-          expect { clone_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError)
+        context "POST git-upload-pack" do
+          it "fails to find a route" do
+            expect { clone_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError)
+          end
         end
-      end
 
-      context "POST git-receive-pack" do
-        it "failes to find a route" do
-          expect { push_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError)
+        context "POST git-receive-pack" do
+          it "failes to find a route" do
+            expect { push_post(project.path_with_namespace) }.to raise_error(ActionController::RoutingError)
+          end
         end
       end
-    end
 
-    context "retrieving an info/refs file" do
-      before { project.update_attribute(:visibility_level, Project::PUBLIC) }
+      context "retrieving an info/refs file" do
+        let(:project) { create(:project, :repository, :public) }
+
+        context "when the file exists" do
+          before do
+            # Provide a dummy file in its place
+            allow_any_instance_of(Repository).to receive(:blob_at).and_call_original
+            allow_any_instance_of(Repository).to receive(:blob_at).with('b83d6e391c22777fca1ed3012fce84f633d7fed0', 'info/refs') do
+              Gitlab::Git::Blob.find(project.repository, 'master', 'bar/branch-test.txt')
+            end
 
-      context "when the file exists" do
-        before do
-          # Provide a dummy file in its place
-          allow_any_instance_of(Repository).to receive(:blob_at).and_call_original
-          allow_any_instance_of(Repository).to receive(:blob_at).with('b83d6e391c22777fca1ed3012fce84f633d7fed0', 'info/refs') do
-            Gitlab::Git::Blob.find(project.repository, 'master', 'bar/branch-test.txt')
+            get "/#{project.path_with_namespace}/blob/master/info/refs"
           end
 
-          get "/#{project.path_with_namespace}/blob/master/info/refs"
+          it "returns the file" do
+            expect(response).to have_http_status(:ok)
+          end
         end
 
-        it "returns the file" do
-          expect(response).to have_http_status(200)
-        end
-      end
+        context "when the file does not exist" do
+          before { get "/#{project.path_with_namespace}/blob/master/info/refs" }
 
-      context "when the file does not exist" do
-        before { get "/#{project.path_with_namespace}/blob/master/info/refs" }
-
-        it "returns not found" do
-          expect(response).to have_http_status(404)
+          it "returns not found" do
+            expect(response).to have_http_status(:not_found)
+          end
         end
       end
     end
@@ -546,6 +665,7 @@ describe 'Git HTTP requests', lib: true do
   describe "User with LDAP identity" do
     let(:user) { create(:omniauth_user, extern_uid: dn) }
     let(:dn) { 'uid=john,ou=people,dc=example,dc=com' }
+    let(:path) { 'doesnt/exist.git' }
 
     before do
       allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
@@ -553,44 +673,36 @@ describe 'Git HTTP requests', lib: true do
       allow(Gitlab::LDAP::Authentication).to receive(:login).with(user.username, user.password).and_return(user)
     end
 
-    context "when authentication fails" do
-      context "when no authentication is provided" do
-        it "responds with status 401" do
-          download('doesnt/exist.git') do |response|
-            expect(response).to have_http_status(401)
-          end
-        end
-      end
-
-      context "when username and invalid password are provided" do
-        it "responds with status 401" do
-          download('doesnt/exist.git', user: user.username, password: "nope") do |response|
-            expect(response).to have_http_status(401)
-          end
-        end
-      end
-    end
+    it_behaves_like 'pulls require Basic HTTP Authentication'
+    it_behaves_like 'pushes require Basic HTTP Authentication'
 
     context "when authentication succeeds" do
       context "when the project doesn't exist" do
-        it "responds with status 404" do
-          download('/doesnt/exist.git', user: user.username, password: user.password) do |response|
-            expect(response).to have_http_status(404)
+        it "responds with status 404 Not Found" do
+          download(path, user: user.username, password: user.password) do |response|
+            expect(response).to have_http_status(:not_found)
           end
         end
       end
 
       context "when the project exists" do
-        let(:project) { create(:project, path: 'project.git-project') }
+        let(:project) { create(:project, :repository) }
+        let(:path) { "#{project.full_path}.git" }
+        let(:env) { { user: user.username, password: user.password } }
 
-        before do
-          project.team << [user, :master]
-        end
+        context 'and the user is on the team' do
+          before do
+            project.team << [user, :master]
+          end
 
-        it "responds with status 200" do
-          clone_get(path, user: user.username, password: user.password) do |response|
-            expect(response).to have_http_status(200)
+          it "responds with status 200" do
+            clone_get(path, env) do |response|
+              expect(response).to have_http_status(200)
+            end
           end
+
+          it_behaves_like 'pulls are allowed'
+          it_behaves_like 'pushes are allowed'
         end
       end
     end
-- 
cgit v1.2.1