Commit 9add3fbb authored by Jacob Vosmaer's avatar Jacob Vosmaer

Some changes after review from Rémy and Valery

parent d3541da4
class Projects::GitHttpController < Projects::ApplicationController class Projects::GitHttpController < Projects::ApplicationController
skip_before_action :repository skip_before_action :repository
before_action :authenticate_user before_action :authenticate_user
before_action :project_found? before_action :ensure_project_found?
# We support two actions (git push and git pull) which use four # GET /foo/bar.git/info/refs?service=git-upload-pack (git pull)
# different HTTP requests: # GET /foo/bar.git/info/refs?service=git-receive-pack (git push)
# def info_refs
# - GET /foo/bar.git/info/refs?service=git-upload-pack (pull)
# - GET /foo/bar.git/info/refs?service=git-receive-pack (push)
# - POST /foo/bar.git/git-upload-pack (pull)
# - POST /foo/bar.git/git-receive-pack" (push)
#
# The Rails routes divide these four requests over three methods:
# info_refs, git_upload_pack, and git_receive_pack.
def git_rpc
if upload_pack? && upload_pack_allowed? if upload_pack? && upload_pack_allowed?
render_ok render_ok
elsif receive_pack? && receive_pack_allowed? elsif receive_pack? && receive_pack_allowed?
...@@ -24,8 +15,22 @@ class Projects::GitHttpController < Projects::ApplicationController ...@@ -24,8 +15,22 @@ class Projects::GitHttpController < Projects::ApplicationController
end end
end end
%i{info_refs git_receive_pack git_upload_pack}.each do |method| # POST /foo/bar.git/git-upload-pack (git pull)
alias_method method, :git_rpc def git_upload_pack
if upload_pack? && upload_pack_allowed?
render_ok
else
render_not_found
end
end
# POST /foo/bar.git/git-receive-pack" (git push)
def git_receive_pack
if receive_pack? && receive_pack_allowed?
render_ok
else
render_not_found
end
end end
private private
...@@ -34,7 +39,7 @@ class Projects::GitHttpController < Projects::ApplicationController ...@@ -34,7 +39,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|
return @ci = true if ci_request?(login, password) return @ci = true if valid_ci_request?(login, password)
@user = Gitlab::Auth.new.find(login, password) @user = Gitlab::Auth.new.find(login, password)
@user ||= oauth_access_token_check(login, password) @user ||= oauth_access_token_check(login, password)
...@@ -42,19 +47,21 @@ class Projects::GitHttpController < Projects::ApplicationController ...@@ -42,19 +47,21 @@ class Projects::GitHttpController < Projects::ApplicationController
end end
end end
def project_found? def ensure_project_found?
render_not_found if project.blank? render_not_found if project.blank?
end end
def ci_request?(login, password) def valid_ci_request?(login, password)
matched_login = /(?<s>^[a-zA-Z]*-ci)-token$/.match(login) matched_login = /(?<service>^[a-zA-Z]*-ci)-token$/.match(login)
if project && matched_login.present? && upload_pack? if project && matched_login.present? && upload_pack?
underscored_service = matched_login['s'].underscore underscored_service = matched_login['service'].underscore
if underscored_service == 'gitlab_ci' if underscored_service == 'gitlab_ci'
return project && project.valid_build_token?(password) return project && project.valid_build_token?(password)
elsif Service.available_services_names.include?(underscored_service) elsif Service.available_services_names.include?(underscored_service)
# We treat underscored_service as a trusted input because it is included
# in the Service.available_services_names whitelist.
service_method = "#{underscored_service}_service" service_method = "#{underscored_service}_service"
service = project.send(service_method) service = project.send(service_method)
...@@ -126,6 +133,7 @@ class Projects::GitHttpController < Projects::ApplicationController ...@@ -126,6 +133,7 @@ class Projects::GitHttpController < Projects::ApplicationController
return id.slice(0, id.length - suffix.length) if id.end_with?(suffix) return id.slice(0, id.length - suffix.length) if id.end_with?(suffix)
end end
# No valid id was found.
nil nil
end end
...@@ -140,14 +148,14 @@ class Projects::GitHttpController < Projects::ApplicationController ...@@ -140,14 +148,14 @@ class Projects::GitHttpController < Projects::ApplicationController
end end
def upload_pack? def upload_pack?
rpc == 'git-upload-pack' git_command == 'git-upload-pack'
end end
def receive_pack? def receive_pack?
rpc == 'git-receive-pack' git_command == 'git-receive-pack'
end end
def rpc def git_command
if action_name == 'info_refs' if action_name == 'info_refs'
params[:service] params[:service]
else else
...@@ -178,11 +186,8 @@ class Projects::GitHttpController < Projects::ApplicationController ...@@ -178,11 +186,8 @@ class Projects::GitHttpController < Projects::ApplicationController
true true
elsif user elsif user
Gitlab::GitAccess.new(user, project).download_access_check.allowed? Gitlab::GitAccess.new(user, project).download_access_check.allowed?
elsif project.public?
# Allow clone/fetch for public projects
true
else else
false project.public?
end end
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