summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--GITLAB_SHELL_VERSION2
-rw-r--r--lib/api/internal.rb2
-rw-r--r--lib/gitlab/backend/grack_auth.rb2
-rw-r--r--lib/gitlab/git_access.rb51
-rw-r--r--lib/gitlab/git_access_status.rb15
-rw-r--r--lib/gitlab/git_access_wiki.rb8
-rw-r--r--spec/lib/gitlab/git_access_spec.rb24
-rw-r--r--spec/lib/gitlab/git_access_wiki_spec.rb4
-rw-r--r--spec/requests/api/internal_spec.rb16
9 files changed, 76 insertions, 48 deletions
diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION
index ccbccc3dc62..276cbf9e285 100644
--- a/GITLAB_SHELL_VERSION
+++ b/GITLAB_SHELL_VERSION
@@ -1 +1 @@
-2.2.0
+2.3.0
diff --git a/lib/api/internal.rb b/lib/api/internal.rb
index ebf2296097d..1648834f034 100644
--- a/lib/api/internal.rb
+++ b/lib/api/internal.rb
@@ -43,7 +43,7 @@ module API
return false unless actor
- access.allowed?(
+ access.check(
actor,
params[:action],
project,
diff --git a/lib/gitlab/backend/grack_auth.rb b/lib/gitlab/backend/grack_auth.rb
index df1461a45c9..762639414e0 100644
--- a/lib/gitlab/backend/grack_auth.rb
+++ b/lib/gitlab/backend/grack_auth.rb
@@ -80,7 +80,7 @@ module Grack
case git_cmd
when *Gitlab::GitAccess::DOWNLOAD_COMMANDS
if user
- Gitlab::GitAccess.new.download_allowed?(user, project)
+ Gitlab::GitAccess.new.download_access_check(user, project).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 129881060d5..3452240dad8 100644
--- a/lib/gitlab/git_access.rb
+++ b/lib/gitlab/git_access.rb
@@ -5,61 +5,60 @@ module Gitlab
attr_reader :params, :project, :git_cmd, :user
- def allowed?(actor, cmd, project, changes = nil)
+ def check(actor, cmd, project, changes = nil)
case cmd
when *DOWNLOAD_COMMANDS
if actor.is_a? User
- download_allowed?(actor, project)
+ download_access_check(actor, project)
elsif actor.is_a? DeployKey
actor.projects.include?(project)
elsif actor.is_a? Key
- download_allowed?(actor.user, project)
+ download_access_check(actor.user, project)
else
raise 'Wrong actor'
end
when *PUSH_COMMANDS
if actor.is_a? User
- push_allowed?(actor, project, changes)
+ push_access_check(actor, project, changes)
elsif actor.is_a? DeployKey
- # Deploy key not allowed to push
- return false
+ return build_status_object(false, "Deploy key not allowed to push")
elsif actor.is_a? Key
- push_allowed?(actor.user, project, changes)
+ push_access_check(actor.user, project, changes)
else
raise 'Wrong actor'
end
else
- false
+ return build_status_object(false, "Wrong command")
end
end
- def download_allowed?(user, project)
- if user && user_allowed?(user)
- user.can?(:download_code, project)
+ def download_access_check(user, project)
+ if user && user_allowed?(user) && user.can?(:download_code, project)
+ build_status_object(true)
else
- false
+ build_status_object(false, "You don't have access")
end
end
- def push_allowed?(user, project, changes)
- return false unless user && user_allowed?(user)
- return true if changes.blank?
+ def push_access_check(user, project, changes)
+ return build_status_object(false, "You don't have access") unless user && user_allowed?(user)
+ return build_status_object(true) if changes.blank?
changes = changes.lines if changes.kind_of?(String)
# Iterate over all changes to find if user allowed all of them to be applied
changes.each do |change|
- unless change_allowed?(user, project, change)
+ status = change_access_check(user, project, change)
+ unless status.allowed?
# If user does not have access to make at least one change - cancel all push
- return false
+ return status
end
end
- # If user has access to make all changes
- true
+ return build_status_object(true)
end
- def change_allowed?(user, project, change)
+ def change_access_check(user, project, change)
oldrev, newrev, ref = change.split(' ')
action = if project.protected_branch?(branch_name(ref))
@@ -79,7 +78,11 @@ module Gitlab
:push_code
end
- user.can?(action, project)
+ if user.can?(action, project)
+ build_status_object(true)
+ else
+ build_status_object(false, "You don't have permission")
+ end
end
def forced_push?(project, oldrev, newrev)
@@ -116,5 +119,11 @@ module Gitlab
nil
end
end
+
+ protected
+
+ def build_status_object(status, message = '')
+ GitAccessStatus.new(status, message)
+ end
end
end
diff --git a/lib/gitlab/git_access_status.rb b/lib/gitlab/git_access_status.rb
new file mode 100644
index 00000000000..3d451ecebee
--- /dev/null
+++ b/lib/gitlab/git_access_status.rb
@@ -0,0 +1,15 @@
+module Gitlab
+ class GitAccessStatus
+ attr_accessor :status, :message
+ alias_method :allowed?, :status
+
+ def initialize(status, message = '')
+ @status = status
+ @message = message
+ end
+
+ def to_json
+ {status: @status, message: @message}.to_json
+ end
+ end
+end \ No newline at end of file
diff --git a/lib/gitlab/git_access_wiki.rb b/lib/gitlab/git_access_wiki.rb
index 9f0eb3be20f..f7d1428deb2 100644
--- a/lib/gitlab/git_access_wiki.rb
+++ b/lib/gitlab/git_access_wiki.rb
@@ -1,7 +1,11 @@
module Gitlab
class GitAccessWiki < GitAccess
- def change_allowed?(user, project, change)
- user.can?(:write_wiki, project)
+ def change_allowed_check(user, project, change)
+ if user.can?(:write_wiki, project)
+ build_status_object(true)
+ else
+ build_status_object(false, "You don't have access")
+ end
end
end
end
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index fe0a6bbdabb..1addba55787 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -5,14 +5,14 @@ describe Gitlab::GitAccess do
let(:project) { create(:project) }
let(:user) { create(:user) }
- describe 'download_allowed?' do
+ describe 'download_access_check' do
describe 'master permissions' do
before { project.team << [user, :master] }
context 'pull code' do
- subject { access.download_allowed?(user, project) }
+ subject { access.download_access_check(user, project) }
- it { should be_true }
+ it { subject.allowed?.should be_true }
end
end
@@ -20,9 +20,9 @@ describe Gitlab::GitAccess do
before { project.team << [user, :guest] }
context 'pull code' do
- subject { access.download_allowed?(user, project) }
+ subject { access.download_access_check(user, project) }
- it { should be_false }
+ it { subject.allowed?.should be_false }
end
end
@@ -33,22 +33,22 @@ describe Gitlab::GitAccess do
end
context 'pull code' do
- subject { access.download_allowed?(user, project) }
+ subject { access.download_access_check(user, project) }
- it { should be_false }
+ it { subject.allowed?.should be_false }
end
end
describe 'without acccess to project' do
context 'pull code' do
- subject { access.download_allowed?(user, project) }
+ subject { access.download_access_check(user, project) }
- it { should be_false }
+ it { subject.allowed?.should be_false }
end
end
end
- describe 'push_allowed?' do
+ describe 'push_access_check' do
def protect_feature_branch
create(:protected_branch, name: 'feature', project: project)
end
@@ -117,9 +117,9 @@ describe Gitlab::GitAccess do
permissions_matrix[role].each do |action, allowed|
context action do
- subject { access.push_allowed?(user, project, changes[action]) }
+ subject { access.push_access_check(user, project, changes[action]) }
- it { should allowed ? be_true : be_false }
+ it { subject.allowed?.should allowed ? be_true : be_false }
end
end
end
diff --git a/spec/lib/gitlab/git_access_wiki_spec.rb b/spec/lib/gitlab/git_access_wiki_spec.rb
index ed5785b31e6..d8d19fd50f0 100644
--- a/spec/lib/gitlab/git_access_wiki_spec.rb
+++ b/spec/lib/gitlab/git_access_wiki_spec.rb
@@ -11,9 +11,9 @@ describe Gitlab::GitAccessWiki do
project.team << [user, :developer]
end
- subject { access.push_allowed?(user, project, changes) }
+ subject { access.push_access_check(user, project, changes) }
- it { should be_true }
+ it { subject.should be_true }
end
def changes
diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb
index 677b1494041..53b7808d4c3 100644
--- a/spec/requests/api/internal_spec.rb
+++ b/spec/requests/api/internal_spec.rb
@@ -37,7 +37,7 @@ describe API::API, api: true do
pull(key, project)
response.status.should == 200
- response.body.should == 'true'
+ JSON.parse(response.body)["status"].should be_true
end
end
@@ -46,7 +46,7 @@ describe API::API, api: true do
push(key, project)
response.status.should == 200
- response.body.should == 'true'
+ JSON.parse(response.body)["status"].should be_true
end
end
end
@@ -61,7 +61,7 @@ describe API::API, api: true do
pull(key, project)
response.status.should == 200
- response.body.should == 'false'
+ JSON.parse(response.body)["status"].should be_false
end
end
@@ -70,7 +70,7 @@ describe API::API, api: true do
push(key, project)
response.status.should == 200
- response.body.should == 'false'
+ JSON.parse(response.body)["status"].should be_false
end
end
end
@@ -87,7 +87,7 @@ describe API::API, api: true do
pull(key, personal_project)
response.status.should == 200
- response.body.should == 'false'
+ JSON.parse(response.body)["status"].should be_false
end
end
@@ -96,7 +96,7 @@ describe API::API, api: true do
push(key, personal_project)
response.status.should == 200
- response.body.should == 'false'
+ JSON.parse(response.body)["status"].should be_false
end
end
end
@@ -114,7 +114,7 @@ describe API::API, api: true do
pull(key, project)
response.status.should == 200
- response.body.should == 'true'
+ JSON.parse(response.body)["status"].should be_true
end
end
@@ -123,7 +123,7 @@ describe API::API, api: true do
push(key, project)
response.status.should == 200
- response.body.should == 'false'
+ JSON.parse(response.body)["status"].should be_false
end
end
end