Merge branch 'pat-msg-on-auth-failure' into 'master'

Prompt user to create personal access token for Git over HTTP

See merge request !11986

app/controllers/jwt_controller.rb

@@ -39,7 +39,7 @@ class JwtController < ApplicationController
       errors: [
         { code: 'UNAUTHORIZED',
           message: "HTTP Basic: Access denied\n" \
-                   "You have 2FA enabled, please use a personal access token for Git over HTTP.\n" \
+                   "You must use a personal access token with 'api' scope for Git over HTTP.\n" \
                    "You can generate one at #{profile_personal_access_tokens_url}" }
       ]
     }, status: 401

app/controllers/projects/git_http_client_controller.rb

@@ -104,7 +104,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController
 
   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" \
+                  "You must use a personal access token with 'api' scope for Git over HTTP.\n" \
                   "You can generate one at #{profile_personal_access_tokens_url}",
            status: 401
   end

changelogs/unreleased/pat-msg-on-auth-failure.yml

+---
+title: Instruct user to use personal access token for Git over HTTP
+merge_request: 11986
+author: Robin Bobbitt

lib/gitlab/auth.rb

@@ -37,7 +37,11 @@ module Gitlab
         rate_limit!(ip, success: result.success?, login: login)
         Gitlab::Auth::UniqueIpsLimiter.limit_user!(result.actor)
 
-        result
+        return result if result.success? || current_application_settings.signin_enabled? || Gitlab::LDAP::Config.enabled?
+
+        # If sign-in is disabled and LDAP is not configured, recommend a
+        # personal access token on failed auth attempts
+        raise Gitlab::Auth::MissingPersonalTokenError
       end
 
       def find_with_user_password(login, password)

spec/lib/gitlab/auth_spec.rb

@@ -204,6 +204,12 @@ describe Gitlab::Auth, lib: true do
       expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: login)
       expect(gl_auth.find_for_git_client(login, 'bar', project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new)
     end
+
+    it 'throws an error suggesting user create a PAT when internal auth is disabled' do
+      allow_any_instance_of(ApplicationSetting).to receive(:signin_enabled?) { false }
+
+      expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::MissingPersonalTokenError)
+    end
   end
 
   describe 'find_with_user_password' do

spec/requests/git_http_spec.rb

@@ -418,17 +418,17 @@ describe 'Git HTTP requests', lib: true do
                 end
 
                 context 'when username and password are provided' do
-                  it 'rejects pulls with 2FA error message' do
+                  it 'rejects pulls with personal access token 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')
+                      expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP')
                     end
                   end
 
-                  it 'rejects the push attempt' do
+                  it 'rejects the push attempt with personal access token error message' do
                     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')
+                      expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP')
                     end
                   end
                 end
@@ -441,6 +441,41 @@ describe 'Git HTTP requests', lib: true do
                 end
               end
 
+              context 'when internal auth is disabled' do
+                before do
+                  allow_any_instance_of(ApplicationSetting).to receive(:signin_enabled?) { false }
+                end
+
+                it 'rejects pulls with personal access token error message' do
+                  download(path, user: 'foo', password: 'bar') do |response|
+                    expect(response).to have_http_status(:unauthorized)
+                    expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP')
+                  end
+                end
+
+                it 'rejects pushes with personal access token error message' do
+                  upload(path, user: 'foo', password: 'bar') do |response|
+                    expect(response).to have_http_status(:unauthorized)
+                    expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP')
+                  end
+                end
+
+                context 'when LDAP is configured' do
+                  before do
+                    allow(Gitlab::LDAP::Config).to receive(:enabled?).and_return(true)
+                    allow_any_instance_of(Gitlab::LDAP::Authentication).
+                      to receive(:login).and_return(nil)
+                  end
+
+                  it 'does not display the personal access token error message' do
+                    upload(path, user: 'foo', password: 'bar') do |response|
+                      expect(response).to have_http_status(:unauthorized)
+                      expect(response.body).not_to include('You must use a personal access token with \'api\' scope for Git over HTTP')
+                    end
+                  end
+                end
+              end
+
               context "when blank password attempts follow a valid login" do
                 def attempt_login(include_password)
                   password = include_password ? user.password : ""

spec/requests/jwt_controller_spec.rb

@@ -70,7 +70,7 @@ describe JwtController do
         context 'without personal token' do
           it 'rejects the authorization attempt' do
             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')
+            expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP')
           end
         end
 
@@ -88,9 +88,24 @@ describe JwtController do
     context 'using invalid login' do
       let(:headers) { { authorization: credentials('invalid', 'password') } }
 
-      subject! { get '/jwt/auth', parameters, headers }
+      context 'when internal auth is enabled' do
+        it 'rejects the authorization attempt' do
+          get '/jwt/auth', parameters, headers
 
-      it { expect(response).to have_http_status(401) }
+          expect(response).to have_http_status(401)
+          expect(response.body).not_to include('You must use a personal access token with \'api\' scope for Git over HTTP')
+        end
+      end
+
+      context 'when internal auth is disabled' do
+        it 'rejects the authorization attempt with personal access token message' do
+          allow_any_instance_of(ApplicationSetting).to receive(:signin_enabled?) { false }
+          get '/jwt/auth', parameters, headers
+
+          expect(response).to have_http_status(401)
+          expect(response.body).to include('You must use a personal access token with \'api\' scope for Git over HTTP')
+        end
+      end
     end
   end