Commit 2f86860a authored by Patricio Cano's avatar Patricio Cano

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

parent 5f5d8a8e
...@@ -95,8 +95,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -95,8 +95,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
end end
def render_missing_personal_token def render_missing_personal_token
render plain: "HTTP Basic: Access denied\n"\ render plain: "HTTP Basic: Access denied\n" \
"You have 2FA enabled, please use a personal access token for Git over HTTP.\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}", "You can generate one at #{profile_personal_access_tokens_url}",
status: 401 status: 401
end end
......
...@@ -8,8 +8,8 @@ ...@@ -8,8 +8,8 @@
%p %p
You can generate a personal access token for each application you use that needs access to the GitLab API. You can generate a personal access token for each application you use that needs access to the GitLab API.
%p %p
You can also use personal access tokens to authenticate against Git over HTTP. Use them specially when you You can also use personal access tokens to authenticate against Git over HTTP.
have Two-Factor Authentication (2FA) enabled. They are the only accepted password when you have Two-Factor Authentication (2FA) enabled.
.col-lg-9 .col-lg-9
......
...@@ -10,17 +10,8 @@ module Gitlab ...@@ -10,17 +10,8 @@ module Gitlab
if valid_ci_request?(login, password, project) if valid_ci_request?(login, password, project)
result.type = :ci result.type = :ci
elsif result.user = find_with_user_password(login, password) else
if result.user.two_factor_enabled? result.user, result.type = populate_result(login, password)
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
end end
success = result.user.present? || [:ci, :missing_personal_token].include?(result.type) success = result.user.present? || [:ci, :missing_personal_token].include?(result.type)
...@@ -87,15 +78,36 @@ module Gitlab ...@@ -87,15 +78,36 @@ module Gitlab
def oauth_access_token_check(login, password) def oauth_access_token_check(login, password)
if login == "oauth2" && password.present? if login == "oauth2" && password.present?
token = Doorkeeper::AccessToken.by_token(password) 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
end end
def personal_access_token_check(login, password) def personal_access_token_check(login, password)
if login && password if login && password
user = User.find_by_personal_access_token(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 end
[user, type]
end end
end end
end end
......
...@@ -199,21 +199,23 @@ describe 'Git HTTP requests', lib: true do ...@@ -199,21 +199,23 @@ describe 'Git HTTP requests', lib: true do
end end
context 'when user has 2FA enabled' do context 'when user has 2FA enabled' do
let(:user) { create(:user, :two_factor) }
let(:access_token) { create(:personal_access_token, user: user) }
before do before do
@user = create(:user, :two_factor) project.team << [user, :master]
project.team << [@user, :master]
end end
context 'when username and password are provided' do context 'when username and password are provided' do
it 'rejects the clone attempt' 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).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 have 2FA enabled, please use a personal access token for Git over HTTP')
end end
end end
it 'rejects the push attempt' do 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).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 have 2FA enabled, please use a personal access token for Git over HTTP')
end end
...@@ -221,18 +223,14 @@ describe 'Git HTTP requests', lib: true do ...@@ -221,18 +223,14 @@ describe 'Git HTTP requests', lib: true do
end end
context 'when username and personal access token are provided' do context 'when username and personal access token are provided' do
before do
@token = create(:personal_access_token, user: @user)
end
it 'allows clones' do 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) expect(response).to have_http_status(200)
end end
end end
it 'allows pushes' do 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) expect(response).to have_http_status(200)
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment