Commit 1cbd5c66 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'gitlab-auth-method-names' into 'master'

Improve Gitlab::Auth method names

Auth.find was a very generic name for a very specific method.
Auth.find_in_gitlab_or_ldap was inaccurate in GitLab EE where it also
looks in Kerberos.


See merge request !4589
parents 04e40776 f73cf3e9
...@@ -42,7 +42,7 @@ class JwtController < ApplicationController ...@@ -42,7 +42,7 @@ class JwtController < ApplicationController
end end
def authenticate_user(login, password) def authenticate_user(login, password)
user = Gitlab::Auth.find_in_gitlab_or_ldap(login, password) user = Gitlab::Auth.find_with_user_password(login, password)
Gitlab::Auth.rate_limit!(request.ip, success: user.present?, login: login) Gitlab::Auth.rate_limit!(request.ip, success: user.present?, login: login)
user user
end end
......
...@@ -43,7 +43,7 @@ class Projects::GitHttpController < Projects::ApplicationController ...@@ -43,7 +43,7 @@ class Projects::GitHttpController < Projects::ApplicationController
return if project && project.public? && upload_pack? return if project && project.public? && upload_pack?
authenticate_or_request_with_http_basic do |login, password| authenticate_or_request_with_http_basic do |login, password|
auth_result = Gitlab::Auth.find(login, password, project: project, ip: request.ip) auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip)
if auth_result.type == :ci && upload_pack? if auth_result.type == :ci && upload_pack?
@ci = true @ci = true
......
...@@ -12,7 +12,7 @@ Doorkeeper.configure do ...@@ -12,7 +12,7 @@ Doorkeeper.configure do
end end
resource_owner_from_credentials do |routes| resource_owner_from_credentials do |routes|
Gitlab::Auth.find_in_gitlab_or_ldap(params[:username], params[:password]) Gitlab::Auth.find_with_user_password(params[:username], params[:password])
end end
# If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below. # If you want to restrict access to the web interface for adding oauth authorized applications, you need to declare the block below.
......
...@@ -11,7 +11,7 @@ module API ...@@ -11,7 +11,7 @@ module API
# Example Request: # Example Request:
# POST /session # POST /session
post "/session" do post "/session" do
user = Gitlab::Auth.find_in_gitlab_or_ldap(params[:email] || params[:login], params[:password]) user = Gitlab::Auth.find_with_user_password(params[:email] || params[:login], params[:password])
return unauthorized! unless user return unauthorized! unless user
present user, with: Entities::UserLogin present user, with: Entities::UserLogin
......
...@@ -3,14 +3,14 @@ module Gitlab ...@@ -3,14 +3,14 @@ module Gitlab
Result = Struct.new(:user, :type) Result = Struct.new(:user, :type)
class << self class << self
def find(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?
result = Result.new result = Result.new
if valid_ci_request?(login, password, project) if valid_ci_request?(login, password, project)
result.type = :ci result.type = :ci
elsif result.user = find_in_gitlab_or_ldap(login, password) elsif result.user = find_with_user_password(login, password)
result.type = :gitlab_or_ldap result.type = :gitlab_or_ldap
elsif result.user = oauth_access_token_check(login, password) elsif result.user = oauth_access_token_check(login, password)
result.type = :oauth result.type = :oauth
...@@ -20,7 +20,7 @@ module Gitlab ...@@ -20,7 +20,7 @@ module Gitlab
result result
end end
def find_in_gitlab_or_ldap(login, password) def find_with_user_password(login, password)
user = User.by_login(login) user = User.by_login(login)
# If no user is found, or it's an LDAP server, try LDAP. # If no user is found, or it's an LDAP server, try LDAP.
......
...@@ -95,7 +95,7 @@ module Grack ...@@ -95,7 +95,7 @@ module Grack
end end
def authenticate_user(login, password) def authenticate_user(login, password)
user = Gitlab::Auth.find_in_gitlab_or_ldap(login, password) user = Gitlab::Auth.find_with_user_password(login, password)
unless user unless user
user = oauth_access_token_check(login, password) user = oauth_access_token_check(login, password)
......
...@@ -3,7 +3,7 @@ require 'spec_helper' ...@@ -3,7 +3,7 @@ require 'spec_helper'
describe Gitlab::Auth, lib: true do describe Gitlab::Auth, lib: true do
let(:gl_auth) { described_class } let(:gl_auth) { described_class }
describe 'find' do describe 'find_for_git_client' do
it 'recognizes CI' do it 'recognizes CI' do
token = '123' token = '123'
project = create(:empty_project) project = create(:empty_project)
...@@ -11,7 +11,7 @@ describe Gitlab::Auth, lib: true do ...@@ -11,7 +11,7 @@ describe Gitlab::Auth, lib: true do
ip = 'ip' ip = 'ip'
expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'gitlab-ci-token') expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: 'gitlab-ci-token')
expect(gl_auth.find('gitlab-ci-token', token, project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, :ci)) expect(gl_auth.find_for_git_client('gitlab-ci-token', token, project: project, ip: ip)).to eq(Gitlab::Auth::Result.new(nil, :ci))
end end
it 'recognizes master passwords' do it 'recognizes master passwords' do
...@@ -19,7 +19,7 @@ describe Gitlab::Auth, lib: true do ...@@ -19,7 +19,7 @@ describe Gitlab::Auth, lib: true do
ip = 'ip' ip = 'ip'
expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username) expect(gl_auth).to receive(:rate_limit!).with(ip, success: true, login: user.username)
expect(gl_auth.find(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :gitlab_or_ldap)) expect(gl_auth.find_for_git_client(user.username, 'password', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :gitlab_or_ldap))
end end
it 'recognizes OAuth tokens' do it 'recognizes OAuth tokens' do
...@@ -29,7 +29,7 @@ describe Gitlab::Auth, lib: true do ...@@ -29,7 +29,7 @@ describe Gitlab::Auth, lib: true do
ip = 'ip' ip = 'ip'
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("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :oauth)) expect(gl_auth.find_for_git_client("oauth2", token.token, project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new(user, :oauth))
end end
it 'returns double nil for invalid credentials' do it 'returns double nil for invalid credentials' do
...@@ -37,11 +37,11 @@ describe Gitlab::Auth, lib: true do ...@@ -37,11 +37,11 @@ describe Gitlab::Auth, lib: true do
ip = 'ip' ip = 'ip'
expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: login) expect(gl_auth).to receive(:rate_limit!).with(ip, success: false, login: login)
expect(gl_auth.find(login, 'bar', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new) expect(gl_auth.find_for_git_client(login, 'bar', project: nil, ip: ip)).to eq(Gitlab::Auth::Result.new)
end end
end end
describe 'find_in_gitlab_or_ldap' do describe 'find_with_user_password' do
let!(:user) do let!(:user) do
create(:user, create(:user,
username: username, username: username,
...@@ -52,25 +52,25 @@ describe Gitlab::Auth, lib: true do ...@@ -52,25 +52,25 @@ describe Gitlab::Auth, lib: true do
let(:password) { 'my-secret' } let(:password) { 'my-secret' }
it "should find user by valid login/password" do it "should find user by valid login/password" do
expect( gl_auth.find_in_gitlab_or_ldap(username, password) ).to eql user expect( gl_auth.find_with_user_password(username, password) ).to eql user
end end
it 'should find user by valid email/password with case-insensitive email' do it 'should find user by valid email/password with case-insensitive email' do
expect(gl_auth.find_in_gitlab_or_ldap(user.email.upcase, password)).to eql user expect(gl_auth.find_with_user_password(user.email.upcase, password)).to eql user
end end
it 'should find user by valid username/password with case-insensitive username' do it 'should find user by valid username/password with case-insensitive username' do
expect(gl_auth.find_in_gitlab_or_ldap(username.upcase, password)).to eql user expect(gl_auth.find_with_user_password(username.upcase, password)).to eql user
end end
it "should not find user with invalid password" do it "should not find user with invalid password" do
password = 'wrong' password = 'wrong'
expect( gl_auth.find_in_gitlab_or_ldap(username, password) ).not_to eql user expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
end end
it "should not find user with invalid login" do it "should not find user with invalid login" do
user = 'wrong' user = 'wrong'
expect( gl_auth.find_in_gitlab_or_ldap(username, password) ).not_to eql user expect( gl_auth.find_with_user_password(username, password) ).not_to eql user
end end
context "with ldap enabled" do context "with ldap enabled" do
...@@ -81,13 +81,13 @@ describe Gitlab::Auth, lib: true do ...@@ -81,13 +81,13 @@ describe Gitlab::Auth, lib: true do
it "tries to autheticate with db before ldap" do it "tries to autheticate with db before ldap" do
expect(Gitlab::LDAP::Authentication).not_to receive(:login) expect(Gitlab::LDAP::Authentication).not_to receive(:login)
gl_auth.find_in_gitlab_or_ldap(username, password) gl_auth.find_with_user_password(username, password)
end end
it "uses ldap as fallback to for authentication" do it "uses ldap as fallback to for authentication" do
expect(Gitlab::LDAP::Authentication).to receive(:login) expect(Gitlab::LDAP::Authentication).to receive(:login)
gl_auth.find_in_gitlab_or_ldap('ldap_user', 'password') gl_auth.find_with_user_password('ldap_user', 'password')
end end
end end
end end
......
...@@ -44,7 +44,7 @@ describe JwtController do ...@@ -44,7 +44,7 @@ describe JwtController do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:headers) { { authorization: credentials('user', 'password') } } let(:headers) { { authorization: credentials('user', 'password') } }
before { expect(Gitlab::Auth).to receive(:find_in_gitlab_or_ldap).with('user', 'password').and_return(user) } before { expect(Gitlab::Auth).to receive(:find_with_user_password).with('user', 'password').and_return(user) }
subject! { get '/jwt/auth', parameters, headers } subject! { get '/jwt/auth', parameters, headers }
......
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