summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/controllers/projects/merge_requests_controller.rb2
-rw-r--r--app/helpers/branches_helper.rb2
-rw-r--r--app/helpers/tree_helper.rb2
-rw-r--r--app/services/files/create_service.rb2
-rw-r--r--app/services/files/delete_service.rb2
-rw-r--r--app/services/files/update_service.rb2
-rw-r--r--lib/api/internal.rb36
-rw-r--r--lib/api/merge_requests.rb3
-rw-r--r--lib/gitlab/backend/grack_auth.rb2
-rw-r--r--lib/gitlab/git_access.rb128
-rw-r--r--lib/gitlab/git_access_wiki.rb2
-rw-r--r--spec/lib/gitlab/git_access_spec.rb38
-rw-r--r--spec/lib/gitlab/git_access_wiki_spec.rb4
13 files changed, 132 insertions, 93 deletions
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index e9b7d7e0083..47ce8467358 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -257,7 +257,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
end
def allowed_to_push_code?(project, branch)
- ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, branch)
+ ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(branch)
end
def merge_request_params
diff --git a/app/helpers/branches_helper.rb b/app/helpers/branches_helper.rb
index 4a5edf6d101..d6eaa7d57bc 100644
--- a/app/helpers/branches_helper.rb
+++ b/app/helpers/branches_helper.rb
@@ -12,6 +12,6 @@ module BranchesHelper
def can_push_branch?(project, branch_name)
return false unless project.repository.branch_names.include?(branch_name)
- ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, branch_name)
+ ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(branch_name)
end
end
diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb
index b6fb7a8aa5a..bf6726574ec 100644
--- a/app/helpers/tree_helper.rb
+++ b/app/helpers/tree_helper.rb
@@ -56,7 +56,7 @@ module TreeHelper
ref ||= @ref
return false unless project.repository.branch_names.include?(ref)
- ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
+ ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref)
end
def tree_breadcrumbs(tree, max_links = 2)
diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb
index de5322e990a..eeafefc25af 100644
--- a/app/services/files/create_service.rb
+++ b/app/services/files/create_service.rb
@@ -3,7 +3,7 @@ require_relative "base_service"
module Files
class CreateService < BaseService
def execute
- allowed = Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
+ allowed = Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref)
unless allowed
return error("You are not allowed to create file in this branch")
diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb
index 8e73c2e2727..1497a0f883b 100644
--- a/app/services/files/delete_service.rb
+++ b/app/services/files/delete_service.rb
@@ -3,7 +3,7 @@ require_relative "base_service"
module Files
class DeleteService < BaseService
def execute
- allowed = ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
+ allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref)
unless allowed
return error("You are not allowed to push into this branch")
diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb
index 328cf3a4b06..0724d3ae634 100644
--- a/app/services/files/update_service.rb
+++ b/app/services/files/update_service.rb
@@ -3,7 +3,7 @@ require_relative "base_service"
module Files
class UpdateService < BaseService
def execute
- allowed = ::Gitlab::GitAccess.can_push_to_branch?(current_user, project, ref)
+ allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref)
unless allowed
return error("You are not allowed to push into this branch")
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index 753d0fcbd98..30bade74cf6 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -17,39 +17,37 @@ module API
post "/allowed" do
status 200
- actor = if params[:key_id]
- Key.find_by(id: params[:key_id])
- elsif params[:user_id]
- User.find_by(id: params[:user_id])
- end
+ actor =
+ if params[:key_id]
+ Key.find_by(id: params[:key_id])
+ elsif params[:user_id]
+ User.find_by(id: params[:user_id])
+ end
unless actor
return Gitlab::GitAccessStatus.new(false, 'No such user or key')
end
project_path = params[:project]
-
+
# Check for *.wiki repositories.
# Strip out the .wiki from the pathname before finding the
# project. This applies the correct project permissions to
# the wiki repository as well.
- access =
- if project_path.end_with?('.wiki')
- project_path.chomp!('.wiki')
- Gitlab::GitAccessWiki.new
- else
- Gitlab::GitAccess.new
- end
+ wiki = project_path.end_with?('.wiki')
+ project_path.chomp!('.wiki') if wiki
project = Project.find_with_namespace(project_path)
if project
- status = access.check(
- actor,
- params[:action],
- project,
- params[:changes]
- )
+ access =
+ if wiki
+ Gitlab::GitAccessWiki.new(actor, project)
+ else
+ Gitlab::GitAccess.new(actor, project)
+ end
+
+ status = access.check(params[:action], params[:changes])
end
if project && status && status.allowed?
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index 25b7857f4b1..f3765f5ab03 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -178,7 +178,8 @@ module API
put ":id/merge_request/:merge_request_id/merge" do
merge_request = user_project.merge_requests.find(params[:merge_request_id])
- allowed = ::Gitlab::GitAccess.can_push_to_branch?(current_user, user_project, merge_request.target_branch)
+ allowed = ::Gitlab::GitAccess.new(current_user, user_project).
+ can_push_to_branch?(merge_request.target_branch)
if allowed
if merge_request.unchecked?
diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb
index ee877e099b1..b051ae4c842 100644
--- a/lib/gitlab/backend/grack_auth.rb
+++ b/lib/gitlab/backend/grack_auth.rb
@@ -112,7 +112,7 @@ module Grack
case git_cmd
when *Gitlab::GitAccess::DOWNLOAD_COMMANDS
if user
- Gitlab::GitAccess.new.download_access_check(user, project).allowed?
+ Gitlab::GitAccess.new(user, project).download_access_check.allowed?
elsif project.public?
# Allow clone/fetch for public projects
true
diff --git a/lib/gitlab/git_access.rb b/lib/gitlab/git_access.rb
index cb69e4b13d3..d6e609e2c44 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -3,9 +3,32 @@ module Gitlab
DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }
PUSH_COMMANDS = %w{ git-receive-pack }
- attr_reader :params, :project, :git_cmd, :user
+ attr_reader :actor, :project
- def self.can_push_to_branch?(user, project, ref)
+ def initialize(actor, project)
+ @actor = actor
+ @project = project
+ end
+
+ def user
+ return @user if defined?(@user)
+
+ @user =
+ case actor
+ when User
+ actor
+ when DeployKey
+ nil
+ when Key
+ actor.user
+ end
+ end
+
+ def deploy_key
+ actor if actor.is_a?(DeployKey)
+ end
+
+ def can_push_to_branch?(ref)
return false unless user
if project.protected_branch?(ref) &&
@@ -16,51 +39,65 @@ module Gitlab
end
end
- def check(actor, cmd, project, changes = nil)
+ def can_read_project?
+ if user
+ user.can?(:read_project, project)
+ elsif deploy_key
+ deploy_key.projects.include?(project)
+ else
+ false
+ end
+ end
+
+ def check(cmd, changes = nil)
case cmd
when *DOWNLOAD_COMMANDS
- download_access_check(actor, project)
+ download_access_check
when *PUSH_COMMANDS
- if actor.is_a? User
- push_access_check(actor, project, changes)
- elsif actor.is_a? DeployKey
- return build_status_object(false, "Deploy key not allowed to push")
- elsif actor.is_a? Key
- push_access_check(actor.user, project, changes)
- else
- raise 'Wrong actor'
- end
+ push_access_check(changes)
else
- return build_status_object(false, "Wrong command")
+ build_status_object(false, "Wrong command")
end
end
- def download_access_check(actor, project)
- if actor.is_a?(User)
- user_download_access_check(actor, project)
- elsif actor.is_a?(DeployKey)
- if actor.projects.include?(project)
- build_status_object(true)
- else
- build_status_object(false, "Deploy key not allowed to access this project")
- end
- elsif actor.is_a? Key
- user_download_access_check(actor.user, project)
+ def download_access_check
+ if user
+ user_download_access_check
+ elsif deploy_key
+ deploy_key_download_access_check
else
raise 'Wrong actor'
end
end
- def user_download_access_check(user, project)
- if user && user_allowed?(user) && user.can?(:download_code, project)
+ def push_access_check(changes)
+ if user
+ user_push_access_check(changes)
+ elsif deploy_key
+ build_status_object(false, "Deploy key not allowed to push")
+ else
+ raise 'Wrong actor'
+ end
+ end
+
+ def user_download_access_check
+ if user && user_allowed? && user.can?(:download_code, project)
build_status_object(true)
else
build_status_object(false, "You don't have access")
end
end
- def push_access_check(user, project, changes)
- unless user && user_allowed?(user)
+ def deploy_key_download_access_check
+ if can_read_project?
+ build_status_object(true)
+ else
+ build_status_object(false, "Deploy key not allowed to access this project")
+ end
+ end
+
+ def user_push_access_check(changes)
+ unless user && user_allowed?
return build_status_object(false, "You don't have access")
end
@@ -76,27 +113,28 @@ module Gitlab
# Iterate over all changes to find if user allowed all of them to be applied
changes.map(&:strip).reject(&:blank?).each do |change|
- status = change_access_check(user, project, change)
+ status = change_access_check(change)
unless status.allowed?
# If user does not have access to make at least one change - cancel all push
return status
end
end
- return build_status_object(true)
+ build_status_object(true)
end
- def change_access_check(user, project, change)
+ def change_access_check(change)
oldrev, newrev, ref = change.split(' ')
- action = if project.protected_branch?(branch_name(ref))
- protected_branch_action(project, oldrev, newrev, branch_name(ref))
- elsif protected_tag?(project, tag_name(ref))
- # Prevent any changes to existing git tag unless user has permissions
- :admin_project
- else
- :push_code
- end
+ action =
+ if project.protected_branch?(branch_name(ref))
+ protected_branch_action(oldrev, newrev, branch_name(ref))
+ elsif protected_tag?(tag_name(ref))
+ # Prevent any changes to existing git tag unless user has permissions
+ :admin_project
+ else
+ :push_code
+ end
if user.can?(action, project)
build_status_object(true)
@@ -105,15 +143,15 @@ module Gitlab
end
end
- def forced_push?(project, oldrev, newrev)
+ def forced_push?(oldrev, newrev)
Gitlab::ForcePushCheck.force_push?(project, oldrev, newrev)
end
private
- def protected_branch_action(project, oldrev, newrev, branch_name)
+ def protected_branch_action(oldrev, newrev, branch_name)
# we dont allow force push to protected branch
- if forced_push?(project, oldrev, newrev)
+ if forced_push?(oldrev, newrev)
:force_push_code_to_protected_branches
elsif Gitlab::Git.blank_ref?(newrev)
# and we dont allow remove of protected branch
@@ -125,11 +163,11 @@ module Gitlab
end
end
- def protected_tag?(project, tag_name)
+ def protected_tag?(tag_name)
project.repository.tag_names.include?(tag_name)
end
- def user_allowed?(user)
+ def user_allowed?
Gitlab::UserAccess.allowed?(user)
end
diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb
index a2177c8d548..73d99b96202 100644
--- a/lib/gitlab/git_access_wiki.rb
+++ b/lib/gitlab/git_access_wiki.rb
@@ -1,6 +1,6 @@
module Gitlab
class GitAccessWiki < GitAccess
- def change_access_check(user, project, change)
+ def change_access_check(change)
if user.can?(:write_wiki, project)
build_status_object(true)
else
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 666398eedd4..39be9d64644 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -1,25 +1,26 @@
require 'spec_helper'
describe Gitlab::GitAccess do
- let(:access) { Gitlab::GitAccess.new }
+ let(:access) { Gitlab::GitAccess.new(actor, project) }
let(:project) { create(:project) }
let(:user) { create(:user) }
+ let(:actor) { user }
describe 'can_push_to_branch?' do
describe 'push to none protected branch' do
it "returns true if user is a master" do
project.team << [user, :master]
- expect(Gitlab::GitAccess.can_push_to_branch?(user, project, "random_branch")).to be_truthy
+ expect(access.can_push_to_branch?("random_branch")).to be_truthy
end
it "returns true if user is a developer" do
project.team << [user, :developer]
- expect(Gitlab::GitAccess.can_push_to_branch?(user, project, "random_branch")).to be_truthy
+ expect(access.can_push_to_branch?("random_branch")).to be_truthy
end
it "returns false if user is a reporter" do
project.team << [user, :reporter]
- expect(Gitlab::GitAccess.can_push_to_branch?(user, project, "random_branch")).to be_falsey
+ expect(access.can_push_to_branch?("random_branch")).to be_falsey
end
end
@@ -30,17 +31,17 @@ describe Gitlab::GitAccess do
it "returns true if user is a master" do
project.team << [user, :master]
- expect(Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name)).to be_truthy
+ expect(access.can_push_to_branch?(@branch.name)).to be_truthy
end
it "returns false if user is a developer" do
project.team << [user, :developer]
- expect(Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name)).to be_falsey
+ expect(access.can_push_to_branch?(@branch.name)).to be_falsey
end
it "returns false if user is a reporter" do
project.team << [user, :reporter]
- expect(Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name)).to be_falsey
+ expect(access.can_push_to_branch?(@branch.name)).to be_falsey
end
end
@@ -51,17 +52,17 @@ describe Gitlab::GitAccess do
it "returns true if user is a master" do
project.team << [user, :master]
- expect(Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name)).to be_truthy
+ expect(access.can_push_to_branch?(@branch.name)).to be_truthy
end
it "returns true if user is a developer" do
project.team << [user, :developer]
- expect(Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name)).to be_truthy
+ expect(access.can_push_to_branch?(@branch.name)).to be_truthy
end
it "returns false if user is a reporter" do
project.team << [user, :reporter]
- expect(Gitlab::GitAccess.can_push_to_branch?(user, project, @branch.name)).to be_falsey
+ expect(access.can_push_to_branch?(@branch.name)).to be_falsey
end
end
@@ -72,7 +73,7 @@ describe Gitlab::GitAccess do
before { project.team << [user, :master] }
context 'pull code' do
- subject { access.download_access_check(user, project) }
+ subject { access.download_access_check }
it { expect(subject.allowed?).to be_truthy }
end
@@ -82,7 +83,7 @@ describe Gitlab::GitAccess do
before { project.team << [user, :guest] }
context 'pull code' do
- subject { access.download_access_check(user, project) }
+ subject { access.download_access_check }
it { expect(subject.allowed?).to be_falsey }
end
@@ -95,7 +96,7 @@ describe Gitlab::GitAccess do
end
context 'pull code' do
- subject { access.download_access_check(user, project) }
+ subject { access.download_access_check }
it { expect(subject.allowed?).to be_falsey }
end
@@ -103,7 +104,7 @@ describe Gitlab::GitAccess do
describe 'without acccess to project' do
context 'pull code' do
- subject { access.download_access_check(user, project) }
+ subject { access.download_access_check }
it { expect(subject.allowed?).to be_falsey }
end
@@ -111,17 +112,18 @@ describe Gitlab::GitAccess do
describe 'deploy key permissions' do
let(:key) { create(:deploy_key) }
+ let(:actor) { key }
context 'pull code' do
context 'allowed' do
before { key.projects << project }
- subject { access.download_access_check(key, project) }
+ subject { access.download_access_check }
it { expect(subject.allowed?).to be_truthy }
end
context 'denied' do
- subject { access.download_access_check(key, project) }
+ subject { access.download_access_check }
it { expect(subject.allowed?).to be_falsey }
end
@@ -205,7 +207,7 @@ describe Gitlab::GitAccess do
permissions_matrix[role].each do |action, allowed|
context action do
- subject { access.push_access_check(user, project, changes[action]) }
+ subject { access.push_access_check(changes[action]) }
it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
end
@@ -221,7 +223,7 @@ describe Gitlab::GitAccess do
updated_permissions_matrix[role].each do |action, allowed|
context action do
- subject { access.push_access_check(user, project, changes[action]) }
+ subject { access.push_access_check(changes[action]) }
it { expect(subject.allowed?).to allowed ? be_truthy : be_falsey }
end
diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb
index c31c6764091..4cb91094cb3 100644
--- a/spec/lib/gitlab/git_access_wiki_spec.rb
+++ b/spec/lib/gitlab/git_access_wiki_spec.rb
@@ -1,7 +1,7 @@
require 'spec_helper'
describe Gitlab::GitAccessWiki do
- let(:access) { Gitlab::GitAccessWiki.new }
+ let(:access) { Gitlab::GitAccessWiki.new(user, project) }
let(:project) { create(:project) }
let(:user) { create(:user) }
@@ -11,7 +11,7 @@ describe Gitlab::GitAccessWiki do
project.team << [user, :developer]
end
- subject { access.push_access_check(user, project, changes) }
+ subject { access.push_access_check(changes) }
it { expect(subject.allowed?).to be_truthy }
end
View Lorry Controller Status