diff --git a/CHANGELOG b/CHANGELOG
index 3adbb0d892731e73ccef18868265f1ebd1185a28..98638b8eb28ce969053ed180232d02abf5e7f228 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -68,6 +68,7 @@ v 8.11.0 (unreleased)
- Upgrade Grape from 0.13.0 to 0.15.0. !4601
- Trigram indexes for the "ci_runners" table have been removed to speed up UPDATE queries
- Fix devise deprecation warnings.
+ - Check for 2FA when using Git over HTTP and only allow PersonalAccessTokens as password in that case !5764
- Update version_sorter and use new interface for faster tag sorting
- Optimize checking if a user has read access to a list of issues !5370
- Store all DB secrets in secrets.yml, under descriptive names !5274
diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb
index 7c21bd181dc04d579ac8798e6cbee14d49380ef2..a5b4031c30f8e2e09551570881cff71acd9d282a 100644
--- a/app/controllers/projects/git_http_client_controller.rb
+++ b/app/controllers/projects/git_http_client_controller.rb
@@ -27,6 +27,9 @@ class Projects::GitHttpClientController < Projects::ApplicationController
@ci = true
elsif auth_result.type == :oauth && !download_request?
# Not allowed
+ elsif auth_result.type == :missing_personal_token
+ render_missing_personal_token
+ return # Render above denied access, nothing left to do
else
@user = auth_result.user
end
@@ -91,6 +94,13 @@ class Projects::GitHttpClientController < Projects::ApplicationController
[nil, nil]
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" \
+ "You can generate one at #{profile_personal_access_tokens_url}",
+ status: 401
+ end
+
def repository
_, suffix = project_id_with_suffix
if suffix == '.wiki.git'
diff --git a/app/views/profiles/personal_access_tokens/index.html.haml b/app/views/profiles/personal_access_tokens/index.html.haml
index 71ac367830d7a913e1abd36c4a3b3830e330d6ef..05a2ea67aa2189c8327fe45aa409c21f70ced497 100644
--- a/app/views/profiles/personal_access_tokens/index.html.haml
+++ b/app/views/profiles/personal_access_tokens/index.html.haml
@@ -7,6 +7,10 @@
= page_title
%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.
+ They are the only accepted password when you have Two-Factor Authentication (2FA) enabled.
+
.col-lg-9
- if flash[:personal_access_token]
diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb
index db1704af75ebb26077ce156ea508f78520f8b423..91f0270818a3f2568dbcb2edcf9a810f74ab4f33 100644
--- a/lib/gitlab/auth.rb
+++ b/lib/gitlab/auth.rb
@@ -10,13 +10,12 @@ module Gitlab
if valid_ci_request?(login, password, project)
result.type = :ci
- elsif result.user = find_with_user_password(login, password)
- result.type = :gitlab_or_ldap
- elsif result.user = oauth_access_token_check(login, password)
- result.type = :oauth
+ else
+ result = populate_result(login, password)
end
- rate_limit!(ip, success: !!result.user || (result.type == :ci), login: login)
+ success = result.user.present? || [:ci, :missing_personal_token].include?(result.type)
+ rate_limit!(ip, success: success, login: login)
result
end
@@ -76,10 +75,43 @@ module Gitlab
end
end
+ def populate_result(login, password)
+ result =
+ user_with_password_for_git(login, password) ||
+ oauth_access_token_check(login, password) ||
+ personal_access_token_check(login, password)
+
+ if result
+ result.type = nil unless result.user
+
+ if result.user && result.user.two_factor_enabled? && result.type == :gitlab_or_ldap
+ result.type = :missing_personal_token
+ end
+ end
+
+ result || Result.new
+ end
+
+ def user_with_password_for_git(login, password)
+ user = find_with_user_password(login, password)
+ Result.new(user, :gitlab_or_ldap) if user
+ end
+
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)
+ Result.new(user, :oauth)
+ end
+ end
+ end
+
+ def personal_access_token_check(login, password)
+ if login && password
+ user = User.find_by_personal_access_token(password)
+ validation = User.by_login(login)
+ Result.new(user, :personal_token) if user == validation
end
end
end
diff --git a/spec/requests/git_http_spec.rb b/spec/requests/git_http_spec.rb
index 8537c252b589fbdf99bd23c3a27a5ef09865e96f..afaf4b7cefbe0578daa2097e7475dddcfc495f86 100644
--- a/spec/requests/git_http_spec.rb
+++ b/spec/requests/git_http_spec.rb
@@ -198,6 +198,45 @@ describe 'Git HTTP requests', lib: true do
end
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
+ 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|
+ 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|
+ 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
+ end
+
+ context 'when username and personal access token are provided' do
+ it 'allows clones' do
+ 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: access_token.token) do |response|
+ expect(response).to have_http_status(200)
+ end
+ end
+ end
+ end
+
context "when blank password attempts follow a valid login" do
def attempt_login(include_password)
password = include_password ? user.password : ""