From de24075ea5960bd7c6290c05496915e8f0ca23f2 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Thu, 8 Sep 2016 12:32:20 -0500 Subject: [PATCH] Further refactoring of authentication code, and code style fixes. --- .../projects/git_http_client_controller.rb | 20 ++++--- lib/gitlab/auth.rb | 53 ++++++++++--------- lib/gitlab/lfs_token.rb | 22 ++++---- 3 files changed, 48 insertions(+), 47 deletions(-) diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index 4be580e759e..f5a07608bf8 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -4,7 +4,7 @@ class Projects::GitHttpClientController < Projects::ApplicationController include ActionController::HttpAuthentication::Basic include KerberosSpnegoHelper - attr_reader :user + attr_reader :user, :actor # Git clients will not know what authenticity token to send along skip_before_action :verify_authenticity_token @@ -24,13 +24,13 @@ class Projects::GitHttpClientController < Projects::ApplicationController handle_basic_authentication(login, password) - if ci? || user + if ci? || actor return # Allow access end elsif allow_kerberos_spnego_auth? && spnego_provided? - @user = find_kerberos_user + @actor = find_kerberos_user - if user + if actor send_final_spnego_response return # Allow access end @@ -110,6 +110,10 @@ class Projects::GitHttpClientController < Projects::ApplicationController @ci.present? end + def user + @actor + end + def handle_basic_authentication(login, password) auth_result = Gitlab::Auth.find_for_git_client(login, password, project: project, ip: request.ip) @@ -117,21 +121,21 @@ class Projects::GitHttpClientController < Projects::ApplicationController when :ci @ci = true if download_request? when :oauth - @user = auth_result.user if download_request? + @actor = auth_result.actor if download_request? when :lfs_deploy_token if download_request? @lfs_deploy_key = true - @user = auth_result.user + @actor = auth_result.actor end when :lfs_token, :personal_token, :gitlab_or_ldap - @user = auth_result.user + @actor = auth_result.actor else # Not allowed end end def lfs_deploy_key? - @lfs_deploy_key.present? && (user && user.projects.include?(project)) + @lfs_deploy_key.present? && actor && actor.projects.include?(project) end def verify_workhorse_api! diff --git a/lib/gitlab/auth.rb b/lib/gitlab/auth.rb index f4e6ebb7bc7..391b8f2f5de 100644 --- a/lib/gitlab/auth.rb +++ b/lib/gitlab/auth.rb @@ -1,6 +1,6 @@ module Gitlab module Auth - Result = Struct.new(:user, :type) + Result = Struct.new(:actor, :type) class MissingPersonalTokenError < StandardError; end @@ -49,6 +49,24 @@ module Gitlab private + def populate_result(login, password, project, ip) + result = + ci_request_check(login, password, project) || + user_with_password_for_git(login, password) || + oauth_access_token_check(login, password) || + lfs_token_check(login, password) || + personal_access_token_check(login, password) + + if result && result.type != :ci + result.type = nil unless result.actor + end + + success = result ? result.actor.present? || result.type == :ci : false + rate_limit!(ip, success: success, login: login) + + result || Result.new + end + def valid_ci_request?(login, password, project) matched_login = /(?^[a-zA-Z]*-ci)-token$/.match(login) @@ -67,31 +85,14 @@ module Gitlab end end - def populate_result(login, password, project, ip) - result = Result.new(nil, :ci) if valid_ci_request?(login, password, project) - - result ||= - user_with_password_for_git(login, password) || - oauth_access_token_check(login, password) || - lfs_token_check(login, password) || - personal_access_token_check(login, password) - - if result && result.type != :ci - result.type = nil unless result.user - - if result.user && result.type == :gitlab_or_ldap && result.user.two_factor_enabled? - raise Gitlab::Auth::MissingPersonalTokenError - end - end - - success = result ? result.user.present? || [:ci].include?(result.type) : false - rate_limit!(ip, success: success, login: login) - - result || Result.new + def ci_request_check(login, password, project) + Result.new(nil, :ci) if valid_ci_request?(login, password, project) end def user_with_password_for_git(login, password) user = find_with_user_password(login, password) + raise Gitlab::Auth::MissingPersonalTokenError if user && user.two_factor_enabled? + Result.new(user, :gitlab_or_ldap) if user end @@ -114,11 +115,11 @@ module Gitlab end def lfs_token_check(login, password) + deploy_key_matches = login.match(/\Alfs\+deploy-key-(\d+)\z/) + actor = - if login =~ /\Alfs\+deploy-key-\d+\Z/ - /\d+\Z/.match(login) do |id| - DeployKey.find(id[0]) - end + if deploy_key_matches + DeployKey.find(deploy_key_matches[1]) else User.by_login(login) end diff --git a/lib/gitlab/lfs_token.rb b/lib/gitlab/lfs_token.rb index edf4dffc4c0..224e4516074 100644 --- a/lib/gitlab/lfs_token.rb +++ b/lib/gitlab/lfs_token.rb @@ -6,7 +6,15 @@ module Gitlab EXPIRY_TIME = 1800 def initialize(actor) - set_actor(actor) + @actor = + case actor + when DeployKey, User + actor + when Key + actor.user + else + # + end end def generate @@ -38,17 +46,5 @@ module Gitlab def redis_key "gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor end - - def set_actor(actor) - @actor = - case actor - when DeployKey, User - actor - when Key - actor.user - else - # - end - end end end