Commit c144db29 authored by Patricio Cano's avatar Patricio Cano

Better authentication handling, syntax fixes and better actor handling for LFS Tokens

parent 85152f02
...@@ -4,8 +4,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -4,8 +4,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController
include ActionController::HttpAuthentication::Basic include ActionController::HttpAuthentication::Basic
include KerberosSpnegoHelper include KerberosSpnegoHelper
class MissingPersonalTokenError < StandardError; end
attr_reader :user attr_reader :user
# Git clients will not know what authenticity token to send along # Git clients will not know what authenticity token to send along
...@@ -40,10 +38,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -40,10 +38,8 @@ class Projects::GitHttpClientController < Projects::ApplicationController
send_challenges send_challenges
render plain: "HTTP Basic: Access denied\n", status: 401 render plain: "HTTP Basic: Access denied\n", status: 401
rescue Gitlab::Auth::MissingPersonalTokenError
rescue MissingPersonalTokenError
render_missing_personal_token render_missing_personal_token
return
end end
def basic_auth_provided? def basic_auth_provided?
...@@ -117,17 +113,20 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -117,17 +113,20 @@ class Projects::GitHttpClientController < Projects::ApplicationController
def handle_authentication(login, password) def handle_authentication(login, password)
auth_result = Gitlab::Auth.find_for_git_client(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 && download_request? case auth_result.type
@ci = true when :ci
elsif auth_result.type == :oauth && !download_request? @ci = true if download_request?
# Not allowed when :oauth
elsif auth_result.type == :missing_personal_token @user = auth_result.user if download_request?
raise MissingPersonalTokenError when :lfs_deploy_token
elsif auth_result.type == :lfs_deploy_token && download_request? if download_request?
@lfs_deploy_key = true @lfs_deploy_key = true
@user = auth_result.user
end
when :lfs_token, :personal_token, :gitlab_or_ldap
@user = auth_result.user @user = auth_result.user
else else
@user = auth_result.user # Not allowed
end end
end end
......
...@@ -27,7 +27,7 @@ module LfsHelper ...@@ -27,7 +27,7 @@ module LfsHelper
return true if project.public? || ci? || lfs_deploy_key? return true if project.public? || ci? || lfs_deploy_key?
(user && user.can?(:download_code, project)) user && user.can?(:download_code, project)
end end
def lfs_upload_access? def lfs_upload_access?
......
...@@ -78,14 +78,7 @@ module API ...@@ -78,14 +78,7 @@ module API
status 200 status 200
key = Key.find(params[:key_id]) key = Key.find(params[:key_id])
user = key.user token_handler = Gitlab::LfsToken.new(key)
token_handler =
if user
Gitlab::LfsToken.new(user)
else
Gitlab::LfsToken.new(key)
end
{ {
username: token_handler.actor_name, username: token_handler.actor_name,
......
...@@ -2,21 +2,13 @@ module Gitlab ...@@ -2,21 +2,13 @@ module Gitlab
module Auth module Auth
Result = Struct.new(:user, :type) Result = Struct.new(:user, :type)
class MissingPersonalTokenError < StandardError; end
class << self class << self
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?
result = Result.new populate_result(login, password, project, ip)
if valid_ci_request?(login, password, project)
result.type = :ci
else
result = populate_result(login, password)
end
success = result.user.present? || [:ci, :missing_personal_token].include?(result.type)
rate_limit!(ip, success: success, login: login)
result
end end
def find_with_user_password(login, password) def find_with_user_password(login, password)
...@@ -75,21 +67,26 @@ module Gitlab ...@@ -75,21 +67,26 @@ module Gitlab
end end
end end
def populate_result(login, password) def populate_result(login, password, project, ip)
result = result = Result.new(nil, :ci) if valid_ci_request?(login, password, project)
result ||=
user_with_password_for_git(login, password) || user_with_password_for_git(login, password) ||
oauth_access_token_check(login, password) || oauth_access_token_check(login, password) ||
lfs_token_check(login, password) || lfs_token_check(login, password) ||
personal_access_token_check(login, password) personal_access_token_check(login, password)
if result if result && result.type != :ci
result.type = nil unless result.user result.type = nil unless result.user
if result.user && result.type == :gitlab_or_ldap && result.user.two_factor_enabled? if result.user && result.type == :gitlab_or_ldap && result.user.two_factor_enabled?
result.type = :missing_personal_token raise Gitlab::Auth::MissingPersonalTokenError
end end
end end
success = result ? result.user.present? || [:ci].include?(result.type) : false
rate_limit!(ip, success: success, login: login)
result || Result.new result || Result.new
end end
...@@ -118,15 +115,17 @@ module Gitlab ...@@ -118,15 +115,17 @@ module Gitlab
def lfs_token_check(login, password) def lfs_token_check(login, password)
actor = actor =
if login.start_with?('lfs-deploy-key') if login =~ /\Alfs-deploy-key-\d+\Z/
DeployKey.find(login.sub('lfs-deploy-key-', '')) /\d+\Z/.match(login) do |id|
DeployKey.find(id[0])
end
else else
User.by_login(login) User.by_login(login)
end end
token_handler = Gitlab::LfsToken.new(actor) token_handler = Gitlab::LfsToken.new(actor)
Result.new(actor, token_handler.type) if actor && token_handler.value == password Result.new(actor, token_handler.type) if actor && Devise.secure_compare(token_handler.value, password)
end end
end end
end end
......
...@@ -2,15 +2,18 @@ module Gitlab ...@@ -2,15 +2,18 @@ module Gitlab
class LfsToken class LfsToken
attr_accessor :actor attr_accessor :actor
TOKEN_LENGTH = 50
EXPIRY_TIME = 1800
def initialize(actor) def initialize(actor)
@actor = actor set_actor(actor)
end end
def generate def generate
token = Devise.friendly_token(50) token = Devise.friendly_token(TOKEN_LENGTH)
Gitlab::Redis.with do |redis| Gitlab::Redis.with do |redis|
redis.set(redis_key, token, ex: 600) redis.set(redis_key, token, ex: EXPIRY_TIME)
end end
token token
...@@ -35,5 +38,17 @@ module Gitlab ...@@ -35,5 +38,17 @@ module Gitlab
def redis_key def redis_key
"gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor "gitlab:lfs_token:#{actor.class.name.underscore}_#{actor.id}" if actor
end end
def set_actor(actor)
@actor =
case actor
when DeployKey, User
actor
when Key
actor.user
else
#
end
end
end end
end end
...@@ -111,7 +111,7 @@ describe API::API, api: true do ...@@ -111,7 +111,7 @@ describe API::API, api: true do
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
expect(json_response['username']).to eq(user.username) expect(json_response['username']).to eq(user.username)
expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(user).value) expect(json_response['lfs_token']).to eq(Gitlab::LfsToken.new(key).value)
expect(json_response['repository_http_path']).to eq(project.http_url_to_repo) expect(json_response['repository_http_path']).to eq(project.http_url_to_repo)
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