Merge branch 'if-require_otp_for_git_access' into 'master'

Allow authenticating with password+OTP for git+HTTP access

See merge request gitlab-org/gitlab!43474

app/controllers/jwt_controller.rb

@@ -37,7 +37,7 @@ class JwtController < ApplicationController
         render_unauthorized
       end
     end
-  rescue Gitlab::Auth::MissingPersonalAccessTokenError
+  rescue Gitlab::Auth::Missing2FAError
     render_missing_personal_access_token
   end
 
@@ -46,7 +46,8 @@ class JwtController < ApplicationController
       errors: [
         { code: 'UNAUTHORIZED',
           message: _('HTTP Basic: Access denied\n' \
-                   'You must use a personal access token with \'api\' scope for Git over HTTP.\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 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,8 +60,10 @@ module Repositories
 
       send_challenges
       render plain: "HTTP Basic: Access denied\n", status: :unauthorized
-    rescue Gitlab::Auth::MissingPersonalAccessTokenError
+    rescue Gitlab::Auth::Missing2FAError
       render_missing_personal_access_token
+    rescue Gitlab::Auth::InvalidOTPError
+      render_invalid_otp
     end
 
     def basic_auth_provided?
@@ -97,9 +99,16 @@ module Repositories
 
     def render_missing_personal_access_token
       render plain: "HTTP Basic: Access denied\n" \
-                    "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
+                    "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",
+             status: :unauthorized
     end
 
     def repository

doc/security/two_factor_authentication.md

@@ -149,3 +149,14 @@ 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

@@ -355,10 +355,7 @@ applications and U2F / WebAuthn devices.
 
 ## Personal access tokens
 
-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.
+Please refer to the [Personal Access Tokens documentation](../personal_access_tokens.md).
 
 ## Recovery options
 

doc/user/profile/personal_access_tokens.md

@@ -14,7 +14,9 @@ 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 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.
+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.
 
 Personal access tokens expire on the date you define, at midnight UTC.
 

lib/gitlab/auth.rb

@@ -2,7 +2,8 @@
 
 module Gitlab
   module Auth
-    MissingPersonalAccessTokenError = Class.new(StandardError)
+    Missing2FAError = Class.new(StandardError)
+    InvalidOTPError = Class.new(StandardError)
     IpBlacklisted = Class.new(StandardError)
 
     # Scopes used for GitLab API access
@@ -52,6 +53,7 @@ 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
 
@@ -62,7 +64,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::MissingPersonalAccessTokenError
+        raise Gitlab::Auth::Missing2FAError
       end
 
       # Find and return a user if the provided password is valid for various
@@ -167,11 +169,26 @@ 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::MissingPersonalAccessTokenError if user.two_factor_enabled?
+        raise Gitlab::Auth::Missing2FAError if user.two_factor_enabled?
 
         Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities)
       end

locale/gitlab.pot

@@ -13901,7 +13901,7 @@ msgstr ""
 msgid "Guideline"
 msgstr ""
 
-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}"
+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}"
 msgstr ""
 
 msgid "Hashed Storage must be enabled to use Geo"

spec/lib/gitlab/auth_spec.rb

@@ -133,7 +133,8 @@ 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!).twice.and_call_original
+          expect(Gitlab::Auth::UniqueIpsLimiter).to(
+            receive(:limit_user!).exactly(3).times.and_call_original)
 
           gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip')
         end
@@ -383,6 +384,28 @@ 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(
@@ -428,7 +451,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::MissingPersonalAccessTokenError)
+      expect { gl_auth.find_for_git_client('foo', 'bar', project: nil, ip: 'ip') }.to raise_error(Gitlab::Auth::Missing2FAError)
     end
 
     context 'while using deploy tokens' do

spec/requests/git_http_spec.rb

@@ -571,14 +571,49 @@ 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('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP')
+                      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
 
                   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('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP')
+                      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
                     end
                   end
                 end
@@ -640,14 +675,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('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP')
+                    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
 
                 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('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP')
+                    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
 
@@ -661,7 +696,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('You must use a personal access token with \'read_repository\' or \'write_repository\' scope for Git over HTTP')
+                      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")
                     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('You must use a personal access token with \'api\' scope for Git over HTTP')
+              expect(response.body).to include('or 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('You must use a personal access token with \'api\' scope for Git over HTTP')
+            expect(response.body).not_to include('or 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('You must use a personal access token with \'api\' scope for Git over HTTP')
+            expect(response.body).to include('or use a personal access token with \'api\' scope for Git over HTTP')
           end
         end
       end