Commit 29414ab0 authored by Drew Blessing's avatar Drew Blessing

Reduce hits to LDAP on Git HTTP auth by reordering auth mechanisms

We accept half a dozen different authentication mechanisms for
Git over HTTP. Fairly high in the list we were checking user
password, which would also query LDAP. In the case of LFS,
OAuth tokens or personal access tokens, we were unnecessarily
hitting LDAP when the authentication will not succeed. This
was causing some LDAP/AD systems to lock the account. Now,
user password authentication is the last mechanism tried since
it's the most expensive.
parent b78d06b7
---
title: Reduce hits to LDAP on Git HTTP auth by reordering auth mechanisms
merge_request: 8752
author:
...@@ -10,13 +10,16 @@ module Gitlab ...@@ -10,13 +10,16 @@ module Gitlab
def find_for_git_client(login, password, project:, ip:) def find_for_git_client(login, password, project:, ip:)
raise "Must provide an IP for rate limiting" if ip.nil? raise "Must provide an IP for rate limiting" if ip.nil?
# `user_with_password_for_git` should be the last check
# because it's the most expensive, especially when LDAP
# is enabled.
result = result =
service_request_check(login, password, project) || service_request_check(login, password, project) ||
build_access_token_check(login, password) || build_access_token_check(login, password) ||
user_with_password_for_git(login, password) ||
oauth_access_token_check(login, password) ||
lfs_token_check(login, password) || lfs_token_check(login, password) ||
oauth_access_token_check(login, password) ||
personal_access_token_check(login, password) || personal_access_token_check(login, password) ||
user_with_password_for_git(login, password) ||
Gitlab::Auth::Result.new Gitlab::Auth::Result.new
rate_limit!(ip, success: result.success?, login: login) rate_limit!(ip, success: result.success?, login: login)
...@@ -143,7 +146,9 @@ module Gitlab ...@@ -143,7 +146,9 @@ module Gitlab
read_authentication_abilities read_authentication_abilities
end end
Result.new(actor, nil, token_handler.type, authentication_abilities) if Devise.secure_compare(token_handler.token, password) if Devise.secure_compare(token_handler.token, password)
Gitlab::Auth::Result.new(actor, nil, token_handler.type, authentication_abilities)
end
end end
def build_access_token_check(login, password) def build_access_token_check(login, password)
......
...@@ -58,6 +58,7 @@ describe Gitlab::Auth, lib: true do ...@@ -58,6 +58,7 @@ describe Gitlab::Auth, lib: true 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, full_authentication_abilities)) 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, full_authentication_abilities))
end end
context 'while using LFS authenticate' do
it 'recognizes user lfs tokens' do it 'recognizes user lfs tokens' do
user = create(:user) user = create(:user)
token = Gitlab::LfsToken.new(user).token token = Gitlab::LfsToken.new(user).token
...@@ -74,42 +75,85 @@ describe Gitlab::Auth, lib: true do ...@@ -74,42 +75,85 @@ describe Gitlab::Auth, lib: true do
expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities)) expect(gl_auth.find_for_git_client("lfs+deploy-key-#{key.id}", token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(key, nil, :lfs_deploy_token, read_authentication_abilities))
end end
context "while using OAuth tokens as passwords" do it 'does not try password auth before oauth' do
it 'succeeds for OAuth tokens with the `api` scope' do
user = create(:user) user = create(:user)
application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user) token = Gitlab::LfsToken.new(user).token
token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "api")
expect(gl_auth).not_to receive(:find_with_user_password)
gl_auth.find_for_git_client(user.username, token, project: nil, ip: 'ip')
end
end
context 'while using OAuth tokens as passwords' do
let(:user) { create(:user) }
let(:token_w_api_scope) { Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'api') }
let(:application) { Doorkeeper::Application.create!(name: 'MyApp', redirect_uri: 'https://app.com', owner: user) }
it 'succeeds for OAuth tokens with the `api` scope' do
expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'oauth2') expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: 'oauth2')
expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, read_authentication_abilities)) expect(gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :oauth, read_authentication_abilities))
end end
it 'fails for OAuth tokens with other scopes' do it 'fails for OAuth tokens with other scopes' do
user = create(:user) token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: 'read_user')
application = Doorkeeper::Application.create!(name: "MyApp", redirect_uri: "https://app.com", owner: user)
token = Doorkeeper::AccessToken.create!(application_id: application.id, resource_owner_id: user.id, scopes: "read_user")
expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'oauth2') expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: 'oauth2')
expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil)) expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
end end
it 'does not try password auth before oauth' do
expect(gl_auth).not_to receive(:find_with_user_password)
gl_auth.find_for_git_client("oauth2", token_w_api_scope.token, project: nil, ip: 'ip')
end
end end
context "while using personal access tokens as passwords" do context 'while using personal access tokens as passwords' do
it 'succeeds for personal access tokens with the `api` scope' do let(:user) { create(:user) }
user = create(:user) let(:token_w_api_scope) { create(:personal_access_token, user: user, scopes: ['api']) }
personal_access_token = create(:personal_access_token, user: user, scopes: ['api'])
it 'succeeds for personal access tokens with the `api` scope' do
expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: user.email) expect(gl_auth).to receive(:rate_limit!).with('ip', success: true, login: user.email)
expect(gl_auth.find_for_git_client(user.email, personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :personal_token, full_authentication_abilities)) expect(gl_auth.find_for_git_client(user.email, token_w_api_scope.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(user, nil, :personal_token, full_authentication_abilities))
end end
it 'fails for personal access tokens with other scopes' do it 'fails for personal access tokens with other scopes' do
user = create(:user)
personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user']) personal_access_token = create(:personal_access_token, user: user, scopes: ['read_user'])
expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: user.email) expect(gl_auth).to receive(:rate_limit!).with('ip', success: false, login: user.email)
expect(gl_auth.find_for_git_client(user.email, personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil)) expect(gl_auth.find_for_git_client(user.email, personal_access_token.token, project: nil, ip: 'ip')).to eq(Gitlab::Auth::Result.new(nil, nil))
end end
it 'does not try password auth before personal access tokens' do
expect(gl_auth).not_to receive(:find_with_user_password)
gl_auth.find_for_git_client(user.email, token_w_api_scope.token, project: nil, ip: 'ip')
end
end
context 'while using regular user and password' do
it 'falls through lfs authentication' do
user = create(
:user,
username: 'normal_user',
password: 'my-secret',
)
expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip'))
.to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities))
end
it 'falls through oauth authentication when the username is oauth2' do
user = create(
:user,
username: 'oauth2',
password: 'my-secret',
)
expect(gl_auth.find_for_git_client(user.username, user.password, project: nil, ip: 'ip'))
.to eq(Gitlab::Auth::Result.new(user, nil, :gitlab_or_ldap, full_authentication_abilities))
end
end end
it 'returns double nil for invalid credentials' do it 'returns double nil for invalid credentials' do
......
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