Skip to content
Snippets Groups Projects
Commit 0e896ffe authored by Jacob Vosmaer (GitLab)'s avatar Jacob Vosmaer (GitLab)
Browse files

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.
parent cfc99bbd
No related branches found
No related tags found
No related merge requests found
Loading
@@ -42,7 +42,7 @@ class JwtController < ApplicationController
Loading
@@ -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
Loading
Loading
Loading
@@ -43,7 +43,7 @@ class Projects::GitHttpController < Projects::ApplicationController
Loading
@@ -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
Loading
Loading
Loading
@@ -12,7 +12,7 @@ Doorkeeper.configure do
Loading
@@ -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.
Loading
Loading
Loading
@@ -11,7 +11,7 @@ module API
Loading
@@ -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
Loading
Loading
Loading
@@ -3,14 +3,14 @@ module Gitlab
Loading
@@ -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
Loading
@@ -20,7 +20,7 @@ module Gitlab
Loading
@@ -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.
Loading
Loading
Loading
@@ -95,7 +95,7 @@ module Grack
Loading
@@ -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)
Loading
Loading
Loading
@@ -41,7 +41,7 @@ describe Gitlab::Auth, lib: true do
Loading
@@ -41,7 +41,7 @@ describe Gitlab::Auth, lib: true do
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,
Loading
@@ -52,25 +52,25 @@ describe Gitlab::Auth, lib: true do
Loading
@@ -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
Loading
@@ -81,13 +81,13 @@ describe Gitlab::Auth, lib: true do
Loading
@@ -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
Loading
Loading
Loading
@@ -44,7 +44,7 @@ describe JwtController do
Loading
@@ -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 }
   
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment