Commit 8c1a01e0 authored by Lin Jen-Shin's avatar Lin Jen-Shin

We never check user privilege if it's a deploy key

parent 24893322
...@@ -501,10 +501,6 @@ class User < ActiveRecord::Base ...@@ -501,10 +501,6 @@ class User < ActiveRecord::Base
several_namespaces? || admin several_namespaces? || admin
end end
def has_access_to?(project)
can?(:read_project, project)
end
def can?(action, subject) def can?(action, subject)
Ability.allowed?(self, action, subject) Ability.allowed?(self, action, subject)
end end
......
module Gitlab module Gitlab
module Checks module Checks
class ChangeAccess class ChangeAccess
attr_reader :user_access, :project attr_reader :user_access, :project, :skip_authorization
def initialize(change, user_access:, project:) def initialize(
change, user_access:, project:, skip_authorization: false)
@oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref) @oldrev, @newrev, @ref = change.values_at(:oldrev, :newrev, :ref)
@branch_name = Gitlab::Git.branch_name(@ref) @branch_name = Gitlab::Git.branch_name(@ref)
@user_access = user_access @user_access = user_access
@project = project @project = project
@skip_authorization = skip_authorization
end end
def exec def exec
...@@ -23,6 +25,7 @@ module Gitlab ...@@ -23,6 +25,7 @@ module Gitlab
protected protected
def protected_branch_checks def protected_branch_checks
return if skip_authorization
return unless @branch_name return unless @branch_name
return unless project.protected_branch?(@branch_name) return unless project.protected_branch?(@branch_name)
...@@ -48,6 +51,8 @@ module Gitlab ...@@ -48,6 +51,8 @@ module Gitlab
end end
def tag_checks def tag_checks
return if skip_authorization
tag_ref = Gitlab::Git.tag_name(@ref) tag_ref = Gitlab::Git.tag_name(@ref)
if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project) if tag_ref && protected_tag?(tag_ref) && user_access.cannot_do_action?(:admin_project)
...@@ -56,6 +61,8 @@ module Gitlab ...@@ -56,6 +61,8 @@ module Gitlab
end end
def push_checks def push_checks
return if skip_authorization
if user_access.cannot_do_action?(:push_code) if user_access.cannot_do_action?(:push_code)
"You are not allowed to push code to this project." "You are not allowed to push code to this project."
end end
......
...@@ -27,7 +27,7 @@ module Gitlab ...@@ -27,7 +27,7 @@ module Gitlab
def check(cmd, changes) def check(cmd, changes)
check_protocol! check_protocol!
check_active_user! check_active_user! unless deploy_key?
check_project_accessibility! check_project_accessibility!
check_command_existence!(cmd) check_command_existence!(cmd)
...@@ -44,9 +44,13 @@ module Gitlab ...@@ -44,9 +44,13 @@ module Gitlab
end end
def download_access_check def download_access_check
if user if deploy_key
true
elsif user
user_download_access_check user_download_access_check
elsif deploy_key.nil? && !Guest.can?(:download_code, project) elsif Guest.can?(:download_code, project)
true
else
raise UnauthorizedError, ERROR_MESSAGES[:download] raise UnauthorizedError, ERROR_MESSAGES[:download]
end end
end end
...@@ -148,7 +152,10 @@ module Gitlab ...@@ -148,7 +152,10 @@ module Gitlab
def check_single_change_access(change) def check_single_change_access(change)
Checks::ChangeAccess.new( Checks::ChangeAccess.new(
change, user_access: user_access, project: project).exec change,
user_access: user_access,
project: project,
skip_authorization: deploy_key?).exec
end end
def matching_merge_request?(newrev, branch_name) def matching_merge_request?(newrev, branch_name)
...@@ -156,17 +163,19 @@ module Gitlab ...@@ -156,17 +163,19 @@ module Gitlab
end end
def deploy_key def deploy_key
actor if actor.is_a?(DeployKey) actor if deploy_key?
end
def deploy_key?
actor.is_a?(DeployKey)
end end
def can_read_project? def can_read_project?
if deploy_key if deploy_key
project.public? || deploy_key.has_access_to?(project) deploy_key.has_access_to?(project)
elsif user elsif user
user_access.can_read_project? user.can?(:read_project, project)
else end || Guest.can?(:read_project, project)
Guest.can?(:read_project, project)
end
end end
protected protected
......
...@@ -115,10 +115,6 @@ describe Gitlab::GitAccess, lib: true do ...@@ -115,10 +115,6 @@ describe Gitlab::GitAccess, lib: true do
let(:key) { create(:deploy_key, user: user) } let(:key) { create(:deploy_key, user: user) }
let(:actor) { key } let(:actor) { key }
before do
project.team << [user, :master]
end
context 'pull code' do context 'pull code' do
context 'when project is authorized' do context 'when project is authorized' do
before { key.projects << project } before { key.projects << project }
...@@ -387,16 +383,6 @@ describe Gitlab::GitAccess, lib: true do ...@@ -387,16 +383,6 @@ describe Gitlab::GitAccess, lib: true do
end end
end end
describe 'full authentication abilities' do
let(:authentication_abilities) { full_authentication_abilities }
it_behaves_like 'pushing code', :to do
def authorize
project.team << [user, :developer]
end
end
end
describe 'build authentication abilities' do describe 'build authentication abilities' do
let(:authentication_abilities) { build_authentication_abilities } let(:authentication_abilities) { build_authentication_abilities }
...@@ -411,10 +397,6 @@ describe Gitlab::GitAccess, lib: true do ...@@ -411,10 +397,6 @@ describe Gitlab::GitAccess, lib: true do
let(:key) { create(:deploy_key, user: user, can_push: can_push) } let(:key) { create(:deploy_key, user: user, can_push: can_push) }
let(:actor) { key } let(:actor) { key }
before do
project.team << [user, :master]
end
context 'when deploy_key can push' do context 'when deploy_key can push' do
let(:can_push) { true } let(:can_push) { true }
......
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