From 7735ef86f0714a5b2a4cb4db8ec0471654563885 Mon Sep 17 00:00:00 2001 From: Patricio Cano Date: Mon, 20 Jun 2016 20:40:56 -0500 Subject: [PATCH] Only allow Git Access on the allowed protocol --- .../projects/git_http_controller.rb | 2 +- app/helpers/application_settings_helper.rb | 4 ++-- app/helpers/button_helper.rb | 8 ++++---- app/models/application_setting.rb | 3 ++- lib/api/internal.rb | 7 +++++-- lib/gitlab/git/hook.rb | 3 ++- lib/gitlab/git_access.rb | 19 +++++++++++++++++-- lib/gitlab/protocol_access.rb | 13 +++++++++++++ 8 files changed, 46 insertions(+), 13 deletions(-) create mode 100644 lib/gitlab/protocol_access.rb diff --git a/app/controllers/projects/git_http_controller.rb b/app/controllers/projects/git_http_controller.rb index 62c3fa8de53..79a7e61e3fe 100644 --- a/app/controllers/projects/git_http_controller.rb +++ b/app/controllers/projects/git_http_controller.rb @@ -162,7 +162,7 @@ class Projects::GitHttpController < Projects::ApplicationController return false unless Gitlab.config.gitlab_shell.upload_pack if user - Gitlab::GitAccess.new(user, project).download_access_check.allowed? + Gitlab::GitAccess.new(user, project, 'http').download_access_check.allowed? else ci? || project.public? end diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index 4cbb7c54cb7..19403388dc6 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -47,9 +47,9 @@ module ApplicationSettingsHelper def enabled_project_tooltip(project, protocol) case protocol when 'ssh' - sanitize_clone_button(ssh_clone_button(project)) + sanitize_clone_button(ssh_clone_button(project, 'bottom')) else - sanitize_clone_button(http_clone_button(project)) + sanitize_clone_button(http_clone_button(project, 'bottom')) end end diff --git a/app/helpers/button_helper.rb b/app/helpers/button_helper.rb index 9051a493b9b..a64e96eaec9 100644 --- a/app/helpers/button_helper.rb +++ b/app/helpers/button_helper.rb @@ -40,7 +40,7 @@ module ButtonHelper type: :button end - def http_clone_button(project) + def http_clone_button(project, placement = 'right') klass = 'http-selector' klass << ' has-tooltip' if current_user.try(:require_password?) @@ -51,13 +51,13 @@ module ButtonHelper href: project.http_url_to_repo, data: { html: true, - placement: 'right', + placement: placement, container: 'body', title: "Set a password on your account
to pull or push via #{protocol}" } end - def ssh_clone_button(project) + def ssh_clone_button(project, placement = 'right') klass = 'ssh-selector' klass << ' has-tooltip' if current_user.try(:require_ssh_key?) @@ -66,7 +66,7 @@ module ButtonHelper href: project.ssh_url_to_repo, data: { html: true, - placement: 'right', + placement: placement, container: 'body', title: 'Add an SSH key to your profile
to pull or push via SSH.' } diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index 7d0114fc549..314e69fa8b6 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -59,7 +59,8 @@ class ApplicationSetting < ActiveRecord::Base presence: true, inclusion: { in: ->(_object) { Gitlab.config.repositories.storages.keys } } - validates_inclusion_of :enabled_git_access_protocols, in: %w(ssh http), allow_blank: true, allow_nil: true + validates :enabled_git_access_protocols, + inclusion: { in: %w(ssh http), allow_blank: true, allow_nil: true } validates_each :restricted_visibility_levels do |record, attr, value| unless value.nil? diff --git a/lib/api/internal.rb b/lib/api/internal.rb index b32503e8516..d5dfba5e0cc 100644 --- a/lib/api/internal.rb +++ b/lib/api/internal.rb @@ -13,6 +13,7 @@ module API # action - git action (git-upload-pack or git-receive-pack) # ref - branch name # forced_push - forced_push + # protocol - Git access protocol being used, e.g. HTTP or SSH # helpers do @@ -46,11 +47,13 @@ module API User.find_by(id: params[:user_id]) end + protocol = params[:protocol] + access = if wiki? - Gitlab::GitAccessWiki.new(actor, project) + Gitlab::GitAccessWiki.new(actor, project, protocol) else - Gitlab::GitAccess.new(actor, project) + Gitlab::GitAccess.new(actor, project, protocol) end access_status = access.check(params[:action], params[:changes]) diff --git a/lib/gitlab/git/hook.rb b/lib/gitlab/git/hook.rb index 420c6883c45..0b61c8bf332 100644 --- a/lib/gitlab/git/hook.rb +++ b/lib/gitlab/git/hook.rb @@ -34,7 +34,8 @@ module Gitlab vars = { 'GL_ID' => gl_id, - 'PWD' => repo_path + 'PWD' => repo_path, + 'PROTOCOL' => 'web' } options = { diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb index d2a0e316cbe..7aec650d1a1 100644 --- a/lib/gitlab/git_access.rb +++ b/lib/gitlab/git_access.rb @@ -3,11 +3,12 @@ module Gitlab DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive } PUSH_COMMANDS = %w{ git-receive-pack } - attr_reader :actor, :project + attr_reader :actor, :project, :protocol - def initialize(actor, project) + def initialize(actor, project, protocol = nil) @actor = actor @project = project + @protocol = protocol end def user @@ -49,6 +50,8 @@ module Gitlab end def check(cmd, changes = nil) + return build_status_object(false, "Git access over #{protocol.upcase} is not allowed") unless protocol_allowed? + unless actor return build_status_object(false, "No user or key was provided.") end @@ -72,6 +75,8 @@ module Gitlab end def download_access_check + return build_status_object(false, "Git access over #{protocol.upcase} is not allowed") unless protocol_allowed? + if user user_download_access_check elsif deploy_key @@ -82,6 +87,8 @@ module Gitlab end def push_access_check(changes) + return build_status_object(false, "Git access over #{protocol.upcase} is not allowed") unless protocol_allowed? + if user user_push_access_check(changes) elsif deploy_key @@ -92,6 +99,8 @@ module Gitlab end def user_download_access_check + return build_status_object(false, "Git access over #{protocol.upcase} is not allowed") unless protocol_allowed? + unless user.can?(:download_code, project) return build_status_object(false, "You are not allowed to download code from this project.") end @@ -100,6 +109,8 @@ module Gitlab end def user_push_access_check(changes) + return build_status_object(false, "Git access over #{protocol.upcase} is not allowed") unless protocol_allowed? + if changes.blank? return build_status_object(true) end @@ -188,6 +199,10 @@ module Gitlab Gitlab::UserAccess.allowed?(user) end + def protocol_allowed? + protocol ? Gitlab::ProtocolAccess.allowed?(protocol) : true + end + def branch_name(ref) ref = ref.to_s if Gitlab::Git.branch_ref?(ref) diff --git a/lib/gitlab/protocol_access.rb b/lib/gitlab/protocol_access.rb new file mode 100644 index 00000000000..0498a72d4cf --- /dev/null +++ b/lib/gitlab/protocol_access.rb @@ -0,0 +1,13 @@ +module Gitlab + module ProtocolAccess + def self.allowed?(protocol) + if protocol.to_s == 'web' + true + elsif !current_application_settings.enabled_git_access_protocols.present? + true + else + protocol.to_s == current_application_settings.enabled_git_access_protocols + end + end + end +end