Refactor `find_for_git_client` method to not use assignment in conditionals and syntax fixes.

4 files changed, 37 insertions, 27 deletions

diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 59395abf401..a5b4031c30f 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -95,8 +95,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
   end
 
   def render_missing_personal_token
-    render plain: "HTTP Basic: Access denied\n"\
-                  "You have 2FA enabled, please use a personal access token for Git over HTTP.\n"\
+    render plain: "HTTP Basic: Access denied\n" \
+                  "You have 2FA enabled, please use a personal access token for Git over HTTP.\n" \
                   "You can generate one at #{profile_personal_access_tokens_url}",
            status: 401
   end
diff --git a/app/views/profiles/personal_access_tokens/index.html.haml b/app/views/profiles/personal_access_tokens/index.html.haml
index d2964c065d1..05a2ea67aa2 100644
--- a/app/views/profiles/personal_access_tokens/index.html.haml
+++ b/app/views/profiles/personal_access_tokens/index.html.haml
@@ -8,8 +8,8 @@
     %p
       You can generate a personal access token for each application you use that needs access to the GitLab API.
     %p
-      You can also use personal access tokens to authenticate against Git over HTTP. Use them specially when you
-      have Two-Factor Authentication (2FA) enabled.
+      You can also use personal access tokens to authenticate against Git over HTTP.
+      They are the only accepted password when you have Two-Factor Authentication (2FA) enabled.
 
   .col-lg-9
 
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index 538e001ec35..e60ce21388e 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -10,17 +10,8 @@ module Gitlab
 
         if valid_ci_request?(login, password, project)
           result.type = :ci
-        elsif result.user = find_with_user_password(login, password)
-          if result.user.two_factor_enabled?
-            result.user = nil
-            result.type = :missing_personal_token
-          else
-            result.type = :gitlab_or_ldap
-          end
-        elsif result.user = oauth_access_token_check(login, password)
-          result.type = :oauth
-        elsif result.user = personal_access_token_check(login, password)
-          result.type = :personal_token
+        else
+          result.user, result.type = populate_result(login, password)
         end
 
         success = result.user.present? || [:ci, :missing_personal_token].include?(result.type)
@@ -87,15 +78,36 @@ module Gitlab
       def oauth_access_token_check(login, password)
         if login == "oauth2" && password.present?
           token = Doorkeeper::AccessToken.by_token(password)
-          token && token.accessible? && User.find_by(id: token.resource_owner_id)
+          if token && token.accessible?
+            user = User.find_by(id: token.resource_owner_id)
+            return user, :oauth
+          end
         end
       end
 
       def personal_access_token_check(login, password)
         if login && password
           user = User.find_by_personal_access_token(password)
-          user if user && user.username == login
+          validation = User.by_login(login)
+          return user, :personal_token if user == validation
+        end
+      end
+
+      def user_with_password_for_git(login, password)
+        user = find_with_user_password(login, password)
+        return user, :gitlab_or_ldap if user
+      end
+
+      def populate_result(login, password)
+        user, type =
+          user_with_password_for_git(login, password) || oauth_access_token_check(login, password) || personal_access_token_check(login, password)
+
+        if user && user.two_factor_enabled? && type == :gitlab_or_ldap
+          user = nil
+          type = :missing_personal_token
         end
+
+        [user, type]
       end
     end
   end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 37c2586bbe2..afaf4b7cefb 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -199,21 +199,23 @@ describe 'Git HTTP requests', lib: true do
             end
 
             context 'when user has 2FA enabled' do
+              let(:user) { create(:user, :two_factor) }
+              let(:access_token) { create(:personal_access_token, user: user) }
+
               before do
-                @user = create(:user, :two_factor)
-                project.team << [@user, :master]
+                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|
+                  download("#{project.path_with_namespace}.git", user: user.username, password: user.password) do |response|
                     expect(response).to have_http_status(401)
                     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|
+                  upload("#{project.path_with_namespace}.git", user: user.username, password: user.password) do |response|
                     expect(response).to have_http_status(401)
                     expect(response.body).to include('You have 2FA enabled, please use a personal access token for Git over HTTP')
                   end
@@ -221,18 +223,14 @@ describe 'Git HTTP requests', lib: true do
               end
 
               context 'when username and personal access token are provided' do
-                before do
-                  @token = create(:personal_access_token, user: @user)
-                end
-
                 it 'allows clones' do
-                  download("#{project.path_with_namespace}.git", user: @user.username, password: @token.token) do |response|
+                  download("#{project.path_with_namespace}.git", user: user.username, password: access_token.token) do |response|
                     expect(response).to have_http_status(200)
                   end
                 end
 
                 it 'allows pushes' do
-                  upload("#{project.path_with_namespace}.git", user: @user.username, password: @token.token) do |response|
+                  upload("#{project.path_with_namespace}.git", user: user.username, password: access_token.token) do |response|
                     expect(response).to have_http_status(200)
                   end
                 end