From b84eb3434d0493cd594eade68d344a9675d72b8a Mon Sep 17 00:00:00 2001
From: Lin Jen-Shin <godfat@godfat.org>
Date: Wed, 19 Jul 2017 16:42:47 +0800
Subject: [PATCH] Try to merge permission checks into one
---
app/services/ci/create_pipeline_service.rb | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index 8b689968895..f331f86e622 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -19,18 +19,20 @@ module Ci
return error('Pipeline is disabled')
end
- unless trigger_request || can?(current_user, :create_pipeline, project)
- return error('Insufficient permissions to create a new pipeline')
+ triggering_user = current_user || trigger_request.trigger.owner
+
+ unless allowed_to_trigger_pipeline?(triggering_user)
+ if can?(triggering_user, :create_pipeline, project)
+ return error("Insufficient permissions for protected ref '#{ref}'")
+ else
+ return error('Insufficient permissions to create a new pipeline')
+ end
end
unless branch? || tag?
return error('Reference not found')
end
- unless triggering_user_allowed_for_ref?(trigger_request)
- return error("Insufficient permissions for protected ref '#{ref}'")
- end
-
unless commit
return error('Commit not found')
end
@@ -74,9 +76,7 @@ module Ci
pipeline.tap(&:process!)
end
- def triggering_user_allowed_for_ref?(trigger_request)
- triggering_user = current_user || trigger_request.trigger.owner
-
+ def allowed_to_trigger_pipeline?(triggering_user)
if triggering_user
allowed_to_create?(triggering_user)
else # legacy triggers don't have a corresponding user
@@ -87,7 +87,7 @@ module Ci
def allowed_to_create?(triggering_user)
access = Gitlab::UserAccess.new(triggering_user, project: project)
- Ability.allowed?(triggering_user, :create_pipeline, project) &&
+ can?(triggering_user, :create_pipeline, project) &&
if branch?
access.can_update_branch?(ref)
elsif tag?
--
GitLab