diff options
author | Rémy Coutable <remy@rymai.me> | 2016-11-21 16:31:51 +0100 |
---|---|---|
committer | Rémy Coutable <remy@rymai.me> | 2016-11-23 17:28:11 +0100 |
commit | 4b3c1e56ae7468d0234240dd211d54a7abd39f8f (patch) | |
tree | bbf92f08247ed2ed1863e153457232136013f1f7 | |
parent | a207c3d12f75644d72c7ea720949b8c58c89faa9 (diff) | |
download | gitlab-ce-4b3c1e56ae7468d0234240dd211d54a7abd39f8f.tar.gz |
Move LfsHelper to a new LfsRequest concern22253-move-lfshelper-methods-somewhere-else-than-app-helpers
Also create a new WorkhorseRequest concern
Signed-off-by: Rémy Coutable <remy@rymai.me>
-rw-r--r-- | app/controllers/concerns/lfs_request.rb (renamed from app/helpers/lfs_helper.rb) | 76 | ||||
-rw-r--r-- | app/controllers/concerns/workhorse_request.rb | 13 | ||||
-rw-r--r-- | app/controllers/projects/git_http_client_controller.rb | 16 | ||||
-rw-r--r-- | app/controllers/projects/git_http_controller.rb | 12 | ||||
-rw-r--r-- | app/controllers/projects/lfs_api_controller.rb | 21 | ||||
-rw-r--r-- | app/controllers/projects/lfs_storage_controller.rb | 7 |
6 files changed, 91 insertions, 54 deletions
diff --git a/app/helpers/lfs_helper.rb b/app/controllers/concerns/lfs_request.rb index 2425c3a8bc8..ed22b1e5470 100644 --- a/app/helpers/lfs_helper.rb +++ b/app/controllers/concerns/lfs_request.rb @@ -1,5 +1,21 @@ -module LfsHelper - include Gitlab::Routing.url_helpers +# This concern assumes: +# - 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! return if Gitlab.config.lfs.enabled @@ -17,35 +33,15 @@ module LfsHelper return if download_request? && lfs_download_access? return if upload_request? && lfs_upload_access? - if project.public? || (user && user.can?(:read_project, project)) - render_lfs_forbidden + if project.public? || can?(user, :read_project, project) + lfs_forbidden! else render_lfs_not_found 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 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) + def lfs_forbidden! + render_lfs_forbidden end def render_lfs_forbidden @@ -70,6 +66,30 @@ module LfsHelper ) 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 @storage_project ||= begin result = project @@ -82,4 +102,8 @@ module LfsHelper result end end + + def objects + @objects ||= (params[:objects] || []).to_a + end end diff --git a/app/controllers/concerns/workhorse_request.rb b/app/controllers/concerns/workhorse_request.rb new file mode 100644 index 00000000000..43c0f1b173c --- /dev/null +++ b/app/controllers/concerns/workhorse_request.rb @@ -0,0 +1,13 @@ +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 diff --git a/app/controllers/projects/git_http_client_controller.rb b/app/controllers/projects/git_http_client_controller.rb index 3f41916e6d3..8714349e27f 100644 --- a/app/controllers/projects/git_http_client_controller.rb +++ b/app/controllers/projects/git_http_client_controller.rb @@ -18,6 +18,14 @@ class Projects::GitHttpClientController < Projects::ApplicationController private + def download_request? + raise NotImplementedError + end + + def upload_request? + raise NotImplementedError + end + def authenticate_user @authentication_result = Gitlab::Auth::Result.new @@ -130,10 +138,6 @@ class Projects::GitHttpClientController < Projects::ApplicationController authentication_result.ci?(project) end - def lfs_deploy_token? - authentication_result.lfs_deploy_token?(project) - end - def authentication_has_download_access? has_authentication_ability?(:download_code) || has_authentication_ability?(:build_download_code) end @@ -149,8 +153,4 @@ class Projects::GitHttpClientController < Projects::ApplicationController def authentication_project authentication_result.project end - - def verify_workhorse_api! - Gitlab::Workhorse.verify_api_request!(request.headers) - end end diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index 13caeb42d40..9184dcccac5 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -1,7 +1,5 @@ -# This file should be identical in GitLab Community Edition and Enterprise Edition - 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-receive-pack (git push) @@ -67,14 +65,18 @@ class Projects::GitHttpController < Projects::GitHttpClientController end def render_denied - if user && user.can?(:read_project, project) - render plain: 'Access denied', status: :forbidden + if user && can?(user, :read_project, project) + render plain: access_denied_message, status: :forbidden else # Do not leak information about project existence render_not_found end end + def access_denied_message + 'Access denied' + end + def upload_pack_allowed? return false unless Gitlab.config.gitlab_shell.upload_pack diff --git a/app/controllers/projects/lfs_api_controller.rb b/app/controllers/projects/lfs_api_controller.rb index 2d493276941..440259b643c 100644 --- a/app/controllers/projects/lfs_api_controller.rb +++ b/app/controllers/projects/lfs_api_controller.rb @@ -1,8 +1,7 @@ class Projects::LfsApiController < Projects::GitHttpClientController - include LfsHelper + include LfsRequest - before_action :require_lfs_enabled! - before_action :lfs_check_access!, except: [:deprecated] + skip_before_action :lfs_check_access!, only: [:deprecated] def batch unless objects.present? @@ -31,6 +30,14 @@ class Projects::LfsApiController < Projects::GitHttpClientController private + def download_request? + params[:operation] == 'download' + end + + def upload_request? + params[:operation] == 'upload' + end + def existing_oids @existing_oids ||= begin storage_project.lfs_objects.where(oid: objects.map { |o| o['oid'].to_s }).pluck(:oid) @@ -79,12 +86,4 @@ class Projects::LfsApiController < Projects::GitHttpClientController } } end - - def download_request? - params[:operation] == 'download' - end - - def upload_request? - params[:operation] == 'upload' - end end diff --git a/app/controllers/projects/lfs_storage_controller.rb b/app/controllers/projects/lfs_storage_controller.rb index 9005b104e90..32759672b6c 100644 --- a/app/controllers/projects/lfs_storage_controller.rb +++ b/app/controllers/projects/lfs_storage_controller.rb @@ -1,9 +1,8 @@ class Projects::LfsStorageController < Projects::GitHttpClientController - include LfsHelper + include LfsRequest + include WorkhorseRequest - before_action :require_lfs_enabled! - before_action :lfs_check_access! - before_action :verify_workhorse_api!, only: [:upload_authorize] + skip_before_action :verify_workhorse_api!, only: [:download, :upload_finalize] def download lfs_object = LfsObject.find_by_oid(oid) |