Commit 8fad76b6 authored by Robert Speicher's avatar Robert Speicher

Merge branch '22253-move-lfshelper-methods-somewhere-else-than-app-helpers' into 'master'

This moves methods from `LfsHelper` to a new `LfsRequest` concern and
introduces a new `WorkhorseRequest` concern.

Closes #22253

See merge request !7623
parents 3fc6eff5 4b3c1e56
module LfsHelper # This concern assumes:
include Gitlab::Routing.url_helpers # - a `#project` accessor
# - a `#user` accessor
# - a `#authentication_result` accessor
# - a `#can?(object, action, subject)` method
# - a `#ci?` method
# - a `#download_request?` method
# - a `#upload_request?` method
# - a `#has_authentication_ability?(ability)` method
module LfsRequest
extend ActiveSupport::Concern
included do
before_action :require_lfs_enabled!
before_action :lfs_check_access!
end
private
def require_lfs_enabled! def require_lfs_enabled!
return if Gitlab.config.lfs.enabled return if Gitlab.config.lfs.enabled
...@@ -17,35 +33,15 @@ module LfsHelper ...@@ -17,35 +33,15 @@ module LfsHelper
return if download_request? && lfs_download_access? return if download_request? && lfs_download_access?
return if upload_request? && lfs_upload_access? return if upload_request? && lfs_upload_access?
if project.public? || (user && user.can?(:read_project, project)) if project.public? || can?(user, :read_project, project)
render_lfs_forbidden lfs_forbidden!
else else
render_lfs_not_found render_lfs_not_found
end end
end end
def lfs_download_access? def lfs_forbidden!
return false unless project.lfs_enabled? render_lfs_forbidden
ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code?
end
def objects
@objects ||= (params[:objects] || []).to_a
end
def user_can_download_code?
has_authentication_ability?(:download_code) && can?(user, :download_code, project)
end
def build_can_download_code?
has_authentication_ability?(:build_download_code) && can?(user, :build_download_code, project)
end
def lfs_upload_access?
return false unless project.lfs_enabled?
has_authentication_ability?(:push_code) && can?(user, :push_code, project)
end end
def render_lfs_forbidden def render_lfs_forbidden
...@@ -70,6 +66,30 @@ module LfsHelper ...@@ -70,6 +66,30 @@ module LfsHelper
) )
end end
def lfs_download_access?
return false unless project.lfs_enabled?
ci? || lfs_deploy_token? || user_can_download_code? || build_can_download_code?
end
def lfs_upload_access?
return false unless project.lfs_enabled?
has_authentication_ability?(:push_code) && can?(user, :push_code, project)
end
def lfs_deploy_token?
authentication_result.lfs_deploy_token?(project)
end
def user_can_download_code?
has_authentication_ability?(:download_code) && can?(user, :download_code, project)
end
def build_can_download_code?
has_authentication_ability?(:build_download_code) && can?(user, :build_download_code, project)
end
def storage_project def storage_project
@storage_project ||= begin @storage_project ||= begin
result = project result = project
...@@ -82,4 +102,8 @@ module LfsHelper ...@@ -82,4 +102,8 @@ module LfsHelper
result result
end end
end end
def objects
@objects ||= (params[:objects] || []).to_a
end
end end
module WorkhorseRequest
extend ActiveSupport::Concern
included do
before_action :verify_workhorse_api!
end
private
def verify_workhorse_api!
Gitlab::Workhorse.verify_api_request!(request.headers)
end
end
...@@ -18,6 +18,14 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -18,6 +18,14 @@ class Projects::GitHttpClientController < Projects::ApplicationController
private private
def download_request?
raise NotImplementedError
end
def upload_request?
raise NotImplementedError
end
def authenticate_user def authenticate_user
@authentication_result = Gitlab::Auth::Result.new @authentication_result = Gitlab::Auth::Result.new
...@@ -130,10 +138,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -130,10 +138,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController
authentication_result.ci?(project) authentication_result.ci?(project)
end end
def lfs_deploy_token?
authentication_result.lfs_deploy_token?(project)
end
def authentication_has_download_access? def authentication_has_download_access?
has_authentication_ability?(:download_code) || has_authentication_ability?(:build_download_code) has_authentication_ability?(:download_code) || has_authentication_ability?(:build_download_code)
end end
...@@ -149,8 +153,4 @@ class Projects::GitHttpClientController < Projects::ApplicationController ...@@ -149,8 +153,4 @@ class Projects::GitHttpClientController < Projects::ApplicationController
def authentication_project def authentication_project
authentication_result.project authentication_result.project
end end
def verify_workhorse_api!
Gitlab::Workhorse.verify_api_request!(request.headers)
end
end end
# This file should be identical in GitLab Community Edition and Enterprise Edition
class Projects::GitHttpController < Projects::GitHttpClientController class Projects::GitHttpController < Projects::GitHttpClientController
before_action :verify_workhorse_api! include WorkhorseRequest
# GET /foo/bar.git/info/refs?service=git-upload-pack (git pull) # GET /foo/bar.git/info/refs?service=git-upload-pack (git pull)
# GET /foo/bar.git/info/refs?service=git-receive-pack (git push) # GET /foo/bar.git/info/refs?service=git-receive-pack (git push)
...@@ -67,14 +65,18 @@ class Projects::GitHttpController < Projects::GitHttpClientController ...@@ -67,14 +65,18 @@ class Projects::GitHttpController < Projects::GitHttpClientController
end end
def render_denied def render_denied
if user && user.can?(:read_project, project) if user && can?(user, :read_project, project)
render plain: 'Access denied', status: :forbidden render plain: access_denied_message, status: :forbidden
else else
# Do not leak information about project existence # Do not leak information about project existence
render_not_found render_not_found
end end
end end
def access_denied_message
'Access denied'
end
def upload_pack_allowed? def upload_pack_allowed?
return false unless Gitlab.config.gitlab_shell.upload_pack return false unless Gitlab.config.gitlab_shell.upload_pack
......
class Projects::LfsApiController < Projects::GitHttpClientController class Projects::LfsApiController < Projects::GitHttpClientController
include LfsHelper include LfsRequest
before_action :require_lfs_enabled! skip_before_action :lfs_check_access!, only: [:deprecated]
before_action :lfs_check_access!, except: [:deprecated]
def batch def batch
unless objects.present? unless objects.present?
...@@ -31,6 +30,14 @@ class Projects::LfsApiController < Projects::GitHttpClientController ...@@ -31,6 +30,14 @@ class Projects::LfsApiController < Projects::GitHttpClientController
private private
def download_request?
params[:operation] == 'download'
end
def upload_request?
params[:operation] == 'upload'
end
def existing_oids def existing_oids
@existing_oids ||= begin @existing_oids ||= begin
storage_project.lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid) storage_project.lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid)
...@@ -79,12 +86,4 @@ class Projects::LfsApiController < Projects::GitHttpClientController ...@@ -79,12 +86,4 @@ class Projects::LfsApiController < Projects::GitHttpClientController
} }
} }
end end
def download_request?
params[:operation] == 'download'
end
def upload_request?
params[:operation] == 'upload'
end
end end
class Projects::LfsStorageController < Projects::GitHttpClientController class Projects::LfsStorageController < Projects::GitHttpClientController
include LfsHelper include LfsRequest
include WorkhorseRequest
before_action :require_lfs_enabled! skip_before_action :verify_workhorse_api!, only: [:download, :upload_finalize]
before_action :lfs_check_access!
before_action :verify_workhorse_api!, only: [:upload_authorize]
def download def download
lfs_object = LfsObject.find_by_oid(oid) lfs_object = LfsObject.find_by_oid(oid)
......
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