Merge branch 'revert-c02edde6' into 'master'

Revert "Merge branch 'if-require_otp_for_git_access' into 'master'"

See merge request gitlab-org/gitlab!50041

app/controllers/jwt_controller.rb

@@ -37,7 +37,7 @@ class JwtController < ApplicationController
         render_unauthorized
       end
     end
-  rescue Gitlab::Auth::Missing2FAError
+  rescue Gitlab::Auth::MissingPersonalAccessTokenError
     render_missing_personal_access_token
   end
 
@@ -46,8 +46,7 @@ class JwtController < ApplicationController
       errors: [
         { code: 'UNAUTHORIZED',
           message: _('HTTP Basic: Access denied\n' \
-                   'You must append your OTP code after your password\n' \
-                   'or use a personal access token with \'api\' scope 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}') % { profile_personal_access_tokens_url: profile_personal_access_tokens_url } }
       ]
     }, status: :unauthorized

app/controllers/repositories/git_http_client_controller.rb

@@ -60,10 +60,8 @@ module Repositories
 
       send_challenges
       render plain: "HTTP Basic: Access denied\n", status: :unauthorized
-    rescue Gitlab::Auth::Missing2FAError
+    rescue Gitlab::Auth::MissingPersonalAccessTokenError
       render_missing_personal_access_token
-    rescue Gitlab::Auth::InvalidOTPError
-      render_invalid_otp
     end
 
     def basic_auth_provided?
@@ -99,15 +97,8 @@ module Repositories
 
     def render_missing_personal_access_token
       render plain: "HTTP Basic: Access denied\n" \
-                    "You must append your OTP code after your password (no spaces)\n" \
-                    "or use a personal access token (PAT) with a 'read_repository' or 'write_repository' scope for Git over HTTP.\n" \
-                    "You can generate a PAT at #{profile_personal_access_tokens_url}",
-             status: :unauthorized
-    end
-
-    def render_invalid_otp
-      render plain: "HTTP Basic: Access denied\n" \
-                    "Invalid OTP provided",
+                    "You must use a personal access token with 'read_repository' or 'write_repository' scope for Git over HTTP.\n" \
+                    "You can generate one at #{profile_personal_access_tokens_url}",
             status: :unauthorized
     end
 

doc/security/two_factor_authentication.md

@@ -149,14 +149,3 @@ To disable it:
 ```ruby
 Feature.disable(:two_factor_for_cli)
 ```
-
-## Two-factor Authentication (2FA) for Git over HTTP operations
-
-> - Introduced in [GitLab 13.7](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48943)
-
-When 2FA is enabled, users must either:
-
-- Use a [personal access token](../user/profile/personal_access_tokens.md) with
-  the `read_repository` or `write_repository` scope, in place of a password.
-- Append a [One-time password](../user/profile/account/two_factor_authentication.md#enabling-2fa),
-  directly to the end of the regular password (no spaces).

doc/user/profile/account/two_factor_authentication.md

@@ -358,7 +358,10 @@ applications and U2F / WebAuthn devices.
 
 ## Personal access tokens
 
-Please refer to the [Personal Access Tokens documentation](../personal_access_tokens.md).
+When 2FA is enabled, you can no longer use your normal account password to
+authenticate with Git over HTTPS on the command line or when using
+the [GitLab API](../../../api/README.md). You must use a
+[personal access token](../personal_access_tokens.md) instead.
 
 ## Recovery options
 

doc/user/profile/personal_access_tokens.md

@@ -14,9 +14,7 @@ info: To determine the technical writer assigned to the Stage/Group associated w
 
 If you're unable to use [OAuth2](../../api/oauth2.md), you can use a personal access token to authenticate with the [GitLab API](../../api/README.md#personalproject-access-tokens).
 
-You can also use [personal access tokens or an OTP](../../security/two_factor_authentication.md#two-factor-authentication-2fa-for-git-over-http-operations)
-with Git to authenticate over HTTP. When 2FA is enabled, you can no longer use
-your normal account password to authenticate with Git over HTTPS.
+You can also use personal access tokens with Git to authenticate over HTTP. Personal access tokens are required when [Two-Factor Authentication (2FA)](account/two_factor_authentication.md) is enabled. In both cases, you can authenticate with a token in place of your password.
 
 Personal access tokens expire on the date you define, at midnight UTC.
 

lib/gitlab/auth.rb

@@ -2,8 +2,7 @@
 
 module Gitlab
   module Auth
-    Missing2FAError = Class.new(StandardError)
-    InvalidOTPError = Class.new(StandardError)
+    MissingPersonalAccessTokenError = Class.new(StandardError)
     IpBlacklisted = Class.new(StandardError)
 
     # Scopes used for GitLab API access
@@ -53,7 +52,6 @@ module Gitlab
           oauth_access_token_check(login, password) ||
           personal_access_token_check(password, project) ||
           deploy_token_check(login, password, project) ||
-          user_with_password_and_otp_for_git(login, password) ||
           user_with_password_for_git(login, password) ||
           Gitlab::Auth::Result.new
 
@@ -64,7 +62,7 @@ module Gitlab
 
         # If sign-in is disabled and LDAP is not configured, recommend a
         # personal access token on failed auth attempts
-        raise Gitlab::Auth::Missing2FAError
+        raise Gitlab::Auth::MissingPersonalAccessTokenError
       end
 
       # Find and return a user if the provided password is valid for various
@@ -169,26 +167,11 @@ module Gitlab
         end
       end
 
-      def user_with_password_and_otp_for_git(login, password)
-        return unless password
-
-        password, otp_token = password[0..-7], password[-6..-1]
-
-        user = find_with_user_password(login, password)
-
-        return unless user&.otp_required_for_login?
-
-        otp_validation_result = ::Users::ValidateOtpService.new(user).execute(otp_token)
-        raise Gitlab::Auth::InvalidOTPError unless otp_validation_result[:status] == :success
-
-        Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
-      end
-
       def user_with_password_for_git(login, password)
         user = find_with_user_password(login, password)
         return unless user
 
-        raise Gitlab::Auth::Missing2FAError if user.two_factor_enabled?
+        raise Gitlab::Auth::MissingPersonalAccessTokenError if user.two_factor_enabled?
 
         Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
       end

locale/gitlab.pot

@@ -13997,7 +13997,7 @@ msgstr ""
 msgid "Guideline"
 msgstr ""
 
-msgid "HTTP Basic: Access denied\\nYou must append your OTP code after your password\\nor use a personal access token with 'api' scope for Git over HTTP.\\nYou can generate one at %{profile_personal_access_tokens_url}"
+msgid "HTTP Basic: Access denied\\nYou must use a personal access token with 'api' scope for Git over HTTP.\\nYou can generate one at %{profile_personal_access_tokens_url}"
 msgstr ""
 
 msgid "Hashed Storage must be enabled to use Geo"

spec/lib/gitlab/auth_spec.rb

@@ -133,8 +133,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
           expect_next_instance_of(Gitlab::Auth::IpRateLimiter) do |rate_limiter|
             expect(rate_limiter).to receive(:reset!)
           end
-          expect(Gitlab::Auth::UniqueIpsLimiter).to(
-            receive(:limit_user!).exactly(3).times.and_call_original)
+          expect(Gitlab::Auth::UniqueIpsLimiter).to receive(:limit_user!).twice.and_call_original
 
           gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')
         end
@@ -384,28 +383,6 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
       end
     end
 
-    context 'while using passwords with OTP' do
-      let_it_be(:user) { create(:user, :two_factor) }
-
-      context 'with valid OTP code' do
-        let(:password) { "#{user.password}#{user.current_otp}" }
-
-        it 'accepts password with OTP' do
-          expect(gl_auth.find_for_git_client(user.username, password, project: nil, ip: 'ip'))
-            .to(eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, described_class.full_authentication_abilities)))
-        end
-      end
-
-      context 'with invalid OTP code' do
-        let(:password) { "#{user.password}abcdef" }
-
-        it 'throws error' do
-          expect { gl_auth.find_for_git_client(user.username, password, project: nil, ip: 'ip') }
-            .to raise_error(Gitlab::Auth::InvalidOTPError)
-        end
-      end
-    end
-
     context 'while using regular user and password' do
       it 'fails for a blocked user' do
         user = create(
@@ -451,7 +428,7 @@ RSpec.describe Gitlab::Auth, :use_clean_rails_memory_store_caching do
     it 'throws an error suggesting user create a PAT when internal auth is disabled' do
       allow_any_instance_of(ApplicationSetting).to receive(:password_authentication_enabled_for_git?) { false }
 
-      expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::Missing2FAError)
+      expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::MissingPersonalAccessTokenError)
     end
 
     context 'while using deploy tokens' do

spec/requests/git_http_spec.rb

@@ -571,49 +571,14 @@ RSpec.describe 'Git HTTP requests' 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_gitlab_http_status(:unauthorized)
-                      expect(response.body).to include("or use a personal access token (PAT) with a 'read_repository' or 'write_repository' scope for Git over HTTP")
+                      expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP')
                     end
                   end
 
                   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_gitlab_http_status(:unauthorized)
-                      expect(response.body).to include("or use a personal access token (PAT) with a 'read_repository' or 'write_repository' scope for Git over HTTP")
-                    end
-                  end
-                end
-
-                context 'when username, password and OTP code are provided' do
-                  context 'with valid OTP code' do
-                    let(:password) { "#{user.password}#{user.current_otp}" }
-                    let(:env) { { user: user.username, password: password } }
-
-                    before do
-                      service = instance_double(::Users::ValidateOtpService)
-                      expect(::Users::ValidateOtpService).to receive(:new).twice.and_return(service)
-                      expect(service).to receive(:execute).with(user.current_otp).twice.and_return({ status: :success })
-                    end
-
-                    it_behaves_like 'pulls are allowed'
-                    it_behaves_like 'pushes are allowed'
-                  end
-
-                  context 'with invalid OTP code' do
-                    let(:password) { "#{user.password}abcdef" }
-                    let(:env) { { user: user.username, password: password } }
-
-                    it 'rejects the pull attempt' do
-                      download(path, **env) do |response|
-                        expect(response).to have_gitlab_http_status(:unauthorized)
-                        expect(response.body).to include('Invalid OTP provided')
-                      end
-                    end
-
-                    it 'rejects the push attempt' do
-                      upload(path, **env) do |response|
-                        expect(response).to have_gitlab_http_status(:unauthorized)
-                        expect(response.body).to include('Invalid OTP provided')
-                      end
+                      expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP')
                     end
                   end
                 end
@@ -675,14 +640,14 @@ RSpec.describe 'Git HTTP requests' do
                 it 'rejects pulls with personal access token error message' do
                   download(path, user: 'foo', password: 'bar') do |response|
                     expect(response).to have_gitlab_http_status(:unauthorized)
-                    expect(response.body).to include("or use a personal access token (PAT) with a 'read_repository' or 'write_repository' scope for Git over HTTP")
+                    expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' 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_gitlab_http_status(:unauthorized)
-                    expect(response.body).to include("or use a personal access token (PAT) with a 'read_repository' or 'write_repository' scope for Git over HTTP")
+                    expect(response.body).to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP')
                   end
                 end
 
@@ -696,7 +661,7 @@ RSpec.describe 'Git HTTP requests' do
                   it 'does not display the personal access token error message' do
                     upload(path, user: 'foo', password: 'bar') do |response|
                       expect(response).to have_gitlab_http_status(:unauthorized)
-                      expect(response.body).not_to include("or use a personal access token (PAT) with a 'read_repository' or 'write_repository' scope for Git over HTTP")
+                      expect(response.body).not_to include('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP')
                     end
                   end
                 end

spec/requests/jwt_controller_spec.rb

@@ -144,7 +144,7 @@ RSpec.describe JwtController do
           context 'without personal token' do
             it 'rejects the authorization attempt' do
               expect(response).to have_gitlab_http_status(:unauthorized)
-              expect(response.body).to include('or use a personal access token with \'api\' scope 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
 
@@ -175,7 +175,7 @@ RSpec.describe JwtController do
             get '/jwt/auth', params: parameters, headers: headers
 
             expect(response).to have_gitlab_http_status(:unauthorized)
-            expect(response.body).not_to include('or use a personal access token with \'api\' scope for Git over HTTP')
+            expect(response.body).not_to include('You must use a personal access token with \'api\' scope for Git over HTTP')
           end
         end
 
@@ -187,7 +187,7 @@ RSpec.describe JwtController do
             get '/jwt/auth', params: parameters, headers: headers
 
             expect(response).to have_gitlab_http_status(:unauthorized)
-            expect(response.body).to include('or use a personal access token with \'api\' scope 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