Merge branch 'git-http-controller' into 'master'

Dismantling Grack::Auth part 1: Git HTTP clients

Part of https://gitlab.com/gitlab-org/gitlab-ce/issues/14501

This does not completely get rid of Grack::Auth yet because Git LFS
support is 'behind' it and I would like to not make this MR bigger
than needed.

- changed tests to make HTTP requests instead of calling Rack apps
- added missing test cases for Git HTTP authentication
- moved Git HTTP requests into a 'normal' Rails controller

See merge request !3361

4 files changed, 362 insertions, 219 deletions

diff --git a/spec/lib/gitlab/auth_spec.rb b/spec/lib/gitlab/auth_spec.rb
index aad291c03cd..a814ad2a4e7 100644
--- a/spec/lib/gitlab/auth_spec.rb
+++ b/spec/lib/gitlab/auth_spec.rb
@@ -1,9 +1,47 @@
 require 'spec_helper'
 
 describe Gitlab::Auth, lib: true do
-  let(:gl_auth) { Gitlab::Auth.new }
+  let(:gl_auth) { described_class }
 
-  describe :find do
+  describe 'find' do
+    it 'recognizes CI' do
+      token = '123'
+      project = create(:empty_project)
+      project.update_attributes(runners_token: token, builds_enabled: true)
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'gitlab-ci-token')
+      expect(gl_auth.find('gitlab-ci-token', token, project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, :ci))
+    end
+
+    it 'recognizes master passwords' do
+      user = create(:user, password: 'password')
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
+      expect(gl_auth.find(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :gitlab_or_ldap))
+    end
+
+    it 'recognizes OAuth tokens' do
+      user = create(:user)
+      application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
+      token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id)
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'oauth2')
+      expect(gl_auth.find("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :oauth))
+    end
+
+    it 'returns double nil for invalid credentials' do
+      login = 'foo'
+      ip = 'ip'
+
+      expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: login)
+      expect(gl_auth.find(login, 'bar', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new)
+    end
+  end
+
+  describe 'find_in_gitlab_or_ldap' do
     let!(:user) do
       create(:user,
         username: username,
@@ -14,25 +52,25 @@ describe Gitlab::Auth, lib: true do
     let(:password) { 'my-secret' }
 
     it "should find user by valid login/password" do
-      expect( gl_auth.find(username, password) ).to eql user
+      expect( gl_auth.find_in_gitlab_or_ldap(username, password) ).to eql user
     end
 
     it 'should find user by valid email/password with case-insensitive email' do
-      expect(gl_auth.find(user.email.upcase, password)).to eql user
+      expect(gl_auth.find_in_gitlab_or_ldap(user.email.upcase, password)).to eql user
     end
 
     it 'should find user by valid username/password with case-insensitive username' do
-      expect(gl_auth.find(username.upcase, password)).to eql user
+      expect(gl_auth.find_in_gitlab_or_ldap(username.upcase, password)).to eql user
     end
 
     it "should not find user with invalid password" do
       password = 'wrong'
-      expect( gl_auth.find(username, password) ).not_to eql user
+      expect( gl_auth.find_in_gitlab_or_ldap(username, password) ).not_to eql user
     end
 
     it "should not find user with invalid login" do
       user = 'wrong'
-      expect( gl_auth.find(username, password) ).not_to eql user
+      expect( gl_auth.find_in_gitlab_or_ldap(username, password) ).not_to eql user
     end
 
     context "with ldap enabled" do
@@ -43,13 +81,13 @@ describe Gitlab::Auth, lib: true do
       it "tries to autheticate with db before ldap" do
         expect(Gitlab::LDAP::Authentication).not_to receive(:login)
 
-        gl_auth.find(username, password)
+        gl_auth.find_in_gitlab_or_ldap(username, password)
       end
 
       it "uses ldap as fallback to for authentication" do
         expect(Gitlab::LDAP::Authentication).to receive(:login)
 
-        gl_auth.find('ldap_user', 'password')
+        gl_auth.find_in_gitlab_or_ldap('ldap_user', 'password')
       end
     end
   end
diff --git a/spec/lib/gitlab/backend/grack_auth_spec.rb b/spec/lib/gitlab/backend/grack_auth_spec.rb
deleted file mode 100644
index cd26dca0998..00000000000
--- a/spec/lib/gitlab/backend/grack_auth_spec.rb
+++ /dev/null
@@ -1,209 +0,0 @@
-require "spec_helper"
-
-describe Grack::Auth, lib: true do
-  let(:user)    { create(:user) }
-  let(:project) { create(:project) }
-
-  let(:app)   { lambda { |env| [200, {}, "Success!"] } }
-  let!(:auth) { Grack::Auth.new(app) }
-  let(:env) do
-    {
-      'rack.input'     => '',
-      'REQUEST_METHOD' => 'GET',
-      'QUERY_STRING'   => 'service=git-upload-pack'
-    }
-  end
-  let(:status) { auth.call(env).first }
-
-  describe "#call" do
-    context "when the project doesn't exist" do
-      before do
-        env["PATH_INFO"] = "doesnt/exist.git"
-      end
-
-      context "when no authentication is provided" do
-        it "responds with status 401" do
-          expect(status).to eq(401)
-        end
-      end
-
-      context "when username and password are provided" do
-        context "when authentication fails" do
-          before do
-            env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope")
-          end
-
-          it "responds with status 401" do
-            expect(status).to eq(401)
-          end
-        end
-
-        context "when authentication succeeds" do
-          before do
-            env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
-          end
-
-          it "responds with status 404" do
-            expect(status).to eq(404)
-          end
-        end
-      end
-    end
-
-    context "when the Wiki for a project exists" do
-      before do
-        @wiki = ProjectWiki.new(project)
-        env["PATH_INFO"] = "#{@wiki.repository.path_with_namespace}.git/info/refs"
-        project.update_attribute(:visibility_level, Project::PUBLIC)
-      end
-
-      it "responds with the right project" do
-        response = auth.call(env)
-        json_body = ActiveSupport::JSON.decode(response[2][0])
-
-        expect(response.first).to eq(200)
-        expect(json_body['RepoPath']).to include(@wiki.repository.path_with_namespace)
-      end
-    end
-
-    context "when the project exists" do
-      before do
-        env["PATH_INFO"] = project.path_with_namespace + ".git"
-      end
-
-      context "when the project is public" do
-        before do
-          project.update_attribute(:visibility_level, Project::PUBLIC)
-        end
-
-        it "responds with status 200" do
-          expect(status).to eq(200)
-        end
-      end
-
-      context "when the project is private" do
-        before do
-          project.update_attribute(:visibility_level, Project::PRIVATE)
-        end
-
-        context "when no authentication is provided" do
-          it "responds with status 401" do
-            expect(status).to eq(401)
-          end
-        end
-
-        context "when username and password are provided" do
-          context "when authentication fails" do
-            before do
-              env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, "nope")
-            end
-
-            it "responds with status 401" do
-              expect(status).to eq(401)
-            end
-
-            context "when the user is IP banned" do
-              before do
-                expect(Rack::Attack::Allow2Ban).to receive(:filter).and_return(true)
-                allow_any_instance_of(Rack::Request).to receive(:ip).and_return('1.2.3.4')
-              end
-
-              it "responds with status 401" do
-                expect(status).to eq(401)
-              end
-            end
-          end
-
-          context "when authentication succeeds" do
-            before do
-              env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password)
-            end
-
-            context "when the user has access to the project" do
-              before do
-                project.team << [user, :master]
-              end
-
-              context "when the user is blocked" do
-                before do
-                  user.block
-                  project.team << [user, :master]
-                end
-
-                it "responds with status 404" do
-                  expect(status).to eq(404)
-                end
-              end
-
-              context "when the user isn't blocked" do
-                before do
-                  expect(Rack::Attack::Allow2Ban).to receive(:reset)
-                end
-
-                it "responds with status 200" do
-                  expect(status).to eq(200)
-                end
-              end
-
-              context "when blank password attempts follow a valid login" do
-                let(:options) { Gitlab.config.rack_attack.git_basic_auth }
-                let(:maxretry) { options[:maxretry] - 1 }
-                let(:ip) { '1.2.3.4' }
-
-                before do
-                  allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip)
-                  Rack::Attack::Allow2Ban.reset(ip, options)
-                end
-
-                after do
-                  Rack::Attack::Allow2Ban.reset(ip, options)
-                end
-
-                def attempt_login(include_password)
-                  password = include_password ? user.password : ""
-                  env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials(user.username, password)
-                  Grack::Auth.new(app)
-                  auth.call(env).first
-                end
-
-                it "repeated attempts followed by successful attempt" do
-                  maxretry.times.each do
-                    expect(attempt_login(false)).to eq(401)
-                  end
-
-                  expect(attempt_login(true)).to eq(200)
-                  expect(Rack::Attack::Allow2Ban.banned?(ip)).to be_falsey
-
-                  maxretry.times.each do
-                    expect(attempt_login(false)).to eq(401)
-                  end
-                end
-              end
-            end
-
-            context "when the user doesn't have access to the project" do
-              it "responds with status 404" do
-                expect(status).to eq(404)
-              end
-            end
-          end
-        end
-
-        context "when a gitlab ci token is provided" do
-          let(:token) { "123" }
-          let(:project) { FactoryGirl.create :empty_project }
-
-          before do
-            project.update_attributes(runners_token: token, builds_enabled: true)
-
-            env["HTTP_AUTHORIZATION"] = ActionController::HttpAuthentication::Basic.encode_credentials("gitlab-ci-token", token)
-          end
-
-          it "responds with status 200" do
-            expect(status).to eq(200)
-          end
-        end
-      end
-    end
-  end
-end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
new file mode 100644
index 00000000000..594a60a4340
--- /dev/null
+++ b/spec/requests/git_http_spec.rb
@@ -0,0 +1,314 @@
+require "spec_helper"
+
+describe 'Git HTTP requests', lib: true do
+  let(:user)    { create(:user) }
+  let(:project) { create(:project) }
+
+  it "gives WWW-Authenticate hints" do
+    clone_get('doesnt/exist.git')
+
+    expect(response.header['WWW-Authenticate']).to start_with('Basic ')
+  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.status).to eq(401)
+        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.status).to eq(401)
+          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.status).to eq(404)
+          end
+        end
+      end
+    end
+  end
+
+  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)
+
+      download("/#{wiki.repository.path_with_namespace}.git") do |response|
+        json_body = ActiveSupport::JSON.decode(response.body)
+
+        expect(response.status).to eq(200)
+        expect(json_body['RepoPath']).to include(wiki.repository.path_with_namespace)
+      end
+    end
+  end
+
+  context "when the project exists" 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
+
+      it "downloads get status 200" do
+        download(path, {}) do |response|
+          expect(response.status).to eq(200)
+        end
+      end
+
+      it "uploads get status 401" do
+        upload(path, {}) do |response|
+          expect(response.status).to eq(401)
+        end
+      end
+
+      context "with correct credentials" do
+        let(:env) { { user: user.username, password: user.password } }
+
+        it "uploads get status 200 (because Git hooks do the real check)" do
+          upload(path, env) do |response|
+            expect(response.status).to eq(200)
+          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)
+
+            upload(path, env) do |response|
+              expect(response.status).to eq(404)
+            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)
+
+          download(path, {}) do |response|
+            expect(response.status).to eq(404)
+          end
+        end
+      end
+    end
+
+    context "when the project is private" do
+      before do
+        project.update_attribute(:visibility_level, Project::PRIVATE)
+      end
+
+      context "when no authentication is provided" do
+        it "responds with status 401 to downloads" do
+          download(path, {}) do |response|
+            expect(response.status).to eq(401)
+          end
+        end
+
+        it "responds with status 401 to uploads" do
+          upload(path, {}) do |response|
+            expect(response.status).to eq(401)
+          end
+        end
+      end
+
+      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.status).to eq(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)
+              allow_any_instance_of(Rack::Request).to receive(:ip).and_return('1.2.3.4')
+
+              clone_get(path, env)
+
+              expect(response.status).to eq(401)
+            end
+          end
+        end
+
+        context "when authentication succeeds" do
+          let(:env) { { user: user.username, password: user.password } }
+
+          context "when the user has access to the project" do
+            before do
+              project.team << [user, :master]
+            end
+
+            context "when the user is blocked" do
+              it "responds with status 404" do
+                user.block
+                project.team << [user, :master]
+
+                download(path, env) do |response|
+                  expect(response.status).to eq(404)
+                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)
+
+                expect(response.status).to eq(200)
+              end
+
+              it "uploads get status 200" do
+                upload(path, env) do |response|
+                  expect(response.status).to eq(200)
+                end
+              end
+            end
+
+            context "when an oauth token is provided" do
+              before do
+                application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
+                @token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id)
+              end
+
+              it "downloads get status 200" do
+                clone_get "#{project.path_with_namespace}.git", user: 'oauth2', password: @token.token
+
+                expect(response.status).to eq(200)
+              end
+
+              it "uploads get status 401 (no project existence information leak)" do
+                push_get "#{project.path_with_namespace}.git", user: 'oauth2', password: @token.token
+
+                expect(response.status).to eq(401)
+              end
+            end
+
+            context "when blank password attempts follow a valid login" do
+              def attempt_login(include_password)
+                password = include_password ? user.password : ""
+                clone_get path, user: user.username, password: password
+                response.status
+              end
+
+              it "repeated attempts followed by successful attempt" do
+                options = Gitlab.config.rack_attack.git_basic_auth
+                maxretry = options[:maxretry] - 1
+                ip = '1.2.3.4'
+
+                allow_any_instance_of(Rack::Request).to receive(:ip).and_return(ip)
+                Rack::Attack::Allow2Ban.reset(ip, options)
+
+                maxretry.times.each do
+                  expect(attempt_login(false)).to eq(401)
+                end
+
+                expect(attempt_login(true)).to eq(200)
+                expect(Rack::Attack::Allow2Ban.banned?(ip)).to be_falsey
+
+                maxretry.times.each do
+                  expect(attempt_login(false)).to eq(401)
+                end
+
+                Rack::Attack::Allow2Ban.reset(ip, options)
+              end
+            end
+          end
+
+          context "when the user doesn't have access to the project" do
+            it "downloads get status 404" do
+              download(path, user: user.username, password: user.password) do |response|
+                expect(response.status).to eq(404)
+              end
+            end
+
+            it "uploads get status 200 (because Git hooks do the real check)" do
+              upload(path, user: user.username, password: user.password) do |response|
+                expect(response.status).to eq(200)
+              end
+            end
+          end
+        end
+      end
+
+      context "when a gitlab ci token is provided" do
+        let(:token) { 123 }
+        let(:project) { FactoryGirl.create :empty_project }
+
+        before do
+          project.update_attributes(runners_token: token, builds_enabled: true)
+        end
+
+        it "downloads get status 200" do
+          clone_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: token
+
+          expect(response.status).to eq(200)
+        end
+
+        it "uploads get status 401 (no project existence information leak)" do
+          push_get "#{project.path_with_namespace}.git", user: 'gitlab-ci-token', password: token
+
+          expect(response.status).to eq(401)
+        end
+      end
+    end
+  end
+
+  def clone_get(project, options={})
+    get "/#{project}/info/refs", { service: 'git-upload-pack' }, auth_env(*options.values_at(:user, :password))
+  end
+
+  def clone_post(project, options={})
+    post "/#{project}/git-upload-pack", {}, auth_env(*options.values_at(:user, :password))
+  end
+
+  def push_get(project, options={})
+    get "/#{project}/info/refs", { service: 'git-receive-pack' }, auth_env(*options.values_at(:user, :password))
+  end
+
+  def push_post(project, options={})
+    post "/#{project}/git-receive-pack", {}, auth_env(*options.values_at(:user, :password))
+  end
+
+  def download(project, user: nil, password: nil)
+    args = [project, { user: user, password: password }]
+
+    clone_get(*args)
+    yield response
+
+    clone_post(*args)
+    yield response
+  end
+
+  def upload(project, user: nil, password: nil)
+    args = [project, { user: user, password: password }]
+
+    push_get(*args)
+    yield response
+
+    push_post(*args)
+    yield response
+  end
+
+  def auth_env(user, password)
+    if user && password
+      { 'HTTP_AUTHORIZATION' => ActionController::HttpAuthentication::Basic.encode_credentials(user, password) }
+    else
+      {}
+    end
+  end
+end
diff --git a/spec/requests/jwt_controller_spec.rb b/spec/requests/jwt_controller_spec.rb
index d006ff195cf..c995993a853 100644
--- a/spec/requests/jwt_controller_spec.rb
+++ b/spec/requests/jwt_controller_spec.rb
@@ -44,7 +44,7 @@ describe JwtController do
       let(:user) { create(:user) }
       let(:headers) { { authorization: credentials('user', 'password') } }
 
-      before { expect_any_instance_of(Gitlab::Auth).to receive(:find).with('user', 'password').and_return(user) }
+      before { expect(Gitlab::Auth).to receive(:find_in_gitlab_or_ldap).with('user', 'password').and_return(user) }
 
       subject! { get '/jwt/auth', parameters, headers }