diff --git a/app/controllers/ci/application_controller.rb b/app/controllers/ci/application_controller.rb
index c420b59c3a20708d3000b00d24c69c9c5f6263db..59c77653509a1adc93fffb93710427372b3769e7 100644
--- a/app/controllers/ci/application_controller.rb
+++ b/app/controllers/ci/application_controller.rb
@@ -13,7 +13,7 @@ module Ci
end
def authorize_manage_builds!
- unless can?(current_user, :manage_builds, project)
+ unless can?(current_user, :update_build, project)
return page_404
end
end
diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb
index f159a6d6dc68270194450328fac3320a9733f0e3..cfea12665163db0f29e8e24e36b4812f10df0642 100644
--- a/app/controllers/projects/artifacts_controller.rb
+++ b/app/controllers/projects/artifacts_controller.rb
@@ -1,6 +1,6 @@
class Projects::ArtifactsController < Projects::ApplicationController
layout 'project'
- before_action :authorize_read_build_artifacts!
+ before_action :authorize_read_build!
def download
unless artifacts_file.file_storage?
@@ -43,14 +43,4 @@ class Projects::ArtifactsController < Projects::ApplicationController
def artifacts_file
@artifacts_file ||= build.artifacts_file
end
-
- def authorize_read_build_artifacts!
- unless can?(current_user, :read_build_artifacts, @project)
- if current_user.nil?
- return authenticate_user!
- else
- return render_404
- end
- end
- end
end
diff --git a/app/controllers/projects/builds_controller.rb b/app/controllers/projects/builds_controller.rb
index 92d9699fe849b38267da9eca7c57464c0c9b8f85..9e89296e71df4ca779ca1ff21f315b92ff01744a 100644
--- a/app/controllers/projects/builds_controller.rb
+++ b/app/controllers/projects/builds_controller.rb
@@ -1,7 +1,8 @@
class Projects::BuildsController < Projects::ApplicationController
before_action :build, except: [:index, :cancel_all]
- before_action :authorize_manage_builds!, except: [:index, :show, :status]
+ before_action :authorize_read_build!, except: [:cancel, :cancel_all, :retry]
+ before_action :authorize_update_build!, except: [:index, :show, :status]
layout "project"
@@ -69,10 +70,4 @@ class Projects::BuildsController < Projects::ApplicationController
def build_path(build)
namespace_project_build_path(build.project.namespace, build.project, build)
end
-
- def authorize_manage_builds!
- unless can?(current_user, :manage_builds, project)
- return render_404
- end
- end
end
diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb
index f5a169e5aa984847cc9435914d7745bdc5a88079..2bf367d2a25aa96d614421b2c8922d9bd3469aaa 100644
--- a/app/controllers/projects/commit_controller.rb
+++ b/app/controllers/projects/commit_controller.rb
@@ -4,10 +4,10 @@
class Projects::CommitController < Projects::ApplicationController
# Authorize
before_action :require_non_empty_project
- before_action :authorize_download_code!, except: [:cancel_builds]
- before_action :authorize_manage_builds!, only: [:cancel_builds]
+ before_action :authorize_download_code!, except: [:cancel_builds, :retry_builds]
+ before_action :authorize_update_build!, only: [:cancel_builds, :retry_builds]
+ before_action :authorize_read_commit_status!, only: [:builds]
before_action :commit
- before_action :authorize_manage_builds!, only: [:cancel_builds, :retry_builds]
before_action :define_show_vars, only: [:show, :builds]
def show
@@ -77,10 +77,4 @@ class Projects::CommitController < Projects::ApplicationController
@statuses = ci_commit.statuses if ci_commit
end
-
- def authorize_manage_builds!
- unless can?(current_user, :manage_builds, project)
- return render_404
- end
- end
end
diff --git a/app/controllers/projects/runner_projects_controller.rb b/app/controllers/projects/runner_projects_controller.rb
index e2785caa2fb0301c9a6b839d0af4a9857972ab75..bedeb4a295cf7d6e83e27d5b74af87c8f2fdeff0 100644
--- a/app/controllers/projects/runner_projects_controller.rb
+++ b/app/controllers/projects/runner_projects_controller.rb
@@ -1,5 +1,5 @@
class Projects::RunnerProjectsController < Projects::ApplicationController
- before_action :authorize_admin_project!
+ before_action :authorize_admin_build!
layout 'project_settings'
diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb
index 4993b2648a55981a5bbe0523c4c4e766983162b7..0dd2d6a99bec9cc2ee7ce842f57c3eb4a6d81d00 100644
--- a/app/controllers/projects/runners_controller.rb
+++ b/app/controllers/projects/runners_controller.rb
@@ -1,6 +1,6 @@
class Projects::RunnersController < Projects::ApplicationController
+ before_action :authorize_admin_build!
before_action :set_runner, only: [:edit, :update, :destroy, :pause, :resume, :show]
- before_action :authorize_admin_project!
layout 'project_settings'
diff --git a/app/controllers/projects/triggers_controller.rb b/app/controllers/projects/triggers_controller.rb
index 30adfad1daa9f7fad7f97988d2182273a1411c0a..92359745cec883d6d9405340a5d9f571243529c8 100644
--- a/app/controllers/projects/triggers_controller.rb
+++ b/app/controllers/projects/triggers_controller.rb
@@ -1,5 +1,5 @@
class Projects::TriggersController < Projects::ApplicationController
- before_action :authorize_admin_project!
+ before_action :authorize_admin_build!
layout 'project_settings'
diff --git a/app/controllers/projects/variables_controller.rb b/app/controllers/projects/variables_controller.rb
index 10efafea9db71657ba77d15e7e9cfe40e0d4d1d9..002346545780385f108dd1b6247b3310080df493 100644
--- a/app/controllers/projects/variables_controller.rb
+++ b/app/controllers/projects/variables_controller.rb
@@ -1,5 +1,5 @@
class Projects::VariablesController < Projects::ApplicationController
- before_action :authorize_admin_project!
+ before_action :authorize_admin_build!
layout 'project_settings'
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index 8c8b355028c62e7bf7483d39166e05cb97c04382..4543eff0d63fde674546c7d7ca7f75a812f38192 100644
--- a/app/helpers/projects_helper.rb
+++ b/app/helpers/projects_helper.rb
@@ -126,7 +126,7 @@ module ProjectsHelper
nav_tabs << :merge_requests
end
- if project.builds_enabled? && can?(current_user, :read_build, project)
+ if can?(current_user, :read_build, project)
nav_tabs << :builds
end
diff --git a/app/models/ability.rb b/app/models/ability.rb
index ab59a3506a29943cc2dc2644037e7fdcc8adf530..e58e7a40273206e1878f1349cc50455af41136f3 100644
--- a/app/models/ability.rb
+++ b/app/models/ability.rb
@@ -52,10 +52,15 @@ class Ability
:read_project_member,
:read_merge_request,
:read_note,
+ :read_commit_status,
:read_build,
:download_code
]
+ if project.restrict_builds?
+ rules -= :read_build
+ end
+
rules - project_disabled_features_rules(project)
else
[]
@@ -113,6 +118,10 @@ class Ability
if project.public? || project.internal?
rules.push(*public_project_rules)
+
+ if team.guest?(user) && project.restrict_builds?
+ rules -= named_abilities('build')
+ end
end
if project.owner == user || user.admin?
@@ -134,7 +143,9 @@ class Ability
def public_project_rules
@public_project_rules ||= project_guest_rules + [
:download_code,
- :fork_project
+ :fork_project,
+ :read_commit_status,
+ :read_build,
]
end
@@ -149,7 +160,7 @@ class Ability
:read_project_member,
:read_merge_request,
:read_note,
- :read_build,
+ :read_commit_status,
:create_project,
:create_issue,
:create_note
@@ -158,24 +169,25 @@ class Ability
def project_report_rules
@project_report_rules ||= project_guest_rules + [
- :create_commit_status,
- :read_commit_statuses,
- :read_build_artifacts,
:download_code,
:fork_project,
:create_project_snippet,
:update_issue,
:admin_issue,
- :admin_label
+ :admin_label,
+ :read_build,
]
end
def project_dev_rules
@project_dev_rules ||= project_report_rules + [
:admin_merge_request,
+ :create_commit_status,
+ :update_commit_status,
+ :create_build,
+ :update_build,
:create_merge_request,
:create_wiki,
- :manage_builds,
:push_code
]
end
@@ -201,7 +213,9 @@ class Ability
:admin_merge_request,
:admin_note,
:admin_wiki,
- :admin_project
+ :admin_project,
+ :admin_commit_status,
+ :admin_build
]
end
@@ -240,6 +254,10 @@ class Ability
rules += named_abilities('wiki')
end
+ unless project.builds_enabled
+ rules += named_abilities('build')
+ end
+
rules
end
diff --git a/app/views/admin/builds/_build.html.haml b/app/views/admin/builds/_build.html.haml
index c395bd908c334a01d8f8ca65c121cb798d4cf044..bd7fb0b36f421cfffa102b32152a7649ebaf9f9e 100644
--- a/app/views/admin/builds/_build.html.haml
+++ b/app/views/admin/builds/_build.html.haml
@@ -4,7 +4,7 @@
= ci_status_with_icon(build.status)
%td.build-link
- - if build.target_url
+ - if can?(current_user, :read_build, project) && build.target_url
= link_to build.target_url do
%strong Build ##{build.id}
- else
@@ -60,10 +60,10 @@
%td
.pull-right
- - if current_user && can?(current_user, :read_build_artifacts, project) && build.artifacts?
+ - if can?(current_user, :read_build, project) && build.artifacts?
= link_to build.artifacts_download_url, title: 'Download artifacts' do
%i.fa.fa-download
- - if current_user && can?(current_user, :manage_builds, build.project)
+ - if current_user && can?(current_user, :update_build, build.project)
- if build.active?
- if build.cancel_url
= link_to build.cancel_url, method: :post, title: 'Cancel' do
diff --git a/app/views/projects/builds/index.html.haml b/app/views/projects/builds/index.html.haml
index bbb6944a65adedadcfe756dbeb9bed8ad504f567..2a2a4745bed9a3818f44d610efcbfe4f1b00bb04 100644
--- a/app/views/projects/builds/index.html.haml
+++ b/app/views/projects/builds/index.html.haml
@@ -3,7 +3,7 @@
.project-issuable-filter
.controls
- - if can?(current_user, :manage_builds, @project)
+ - if can?(current_user, :update_build, @project)
.pull-left.hidden-xs
- if @all_builds.running_or_pending.any?
= link_to 'Cancel running', cancel_all_namespace_project_builds_path(@project.namespace, @project),
diff --git a/app/views/projects/builds/show.html.haml b/app/views/projects/builds/show.html.haml
index ba1fdc6f0e7416ea88419de2784facb9baf96e73..c43d6c3b427c725913a4f431d3a086462d8499cf 100644
--- a/app/views/projects/builds/show.html.haml
+++ b/app/views/projects/builds/show.html.haml
@@ -89,8 +89,7 @@
Test coverage
%h1 #{@build.coverage}%
- - if current_user && can?(current_user, :read_build_artifacts, @project) && @build.artifacts?
-
+ - if can?(current_user, :read_build, @project) && @build.artifacts?
.build-widget.artifacts
%h4.title Build artifacts
.center
@@ -102,7 +101,7 @@
.build-widget
%h4.title
Build ##{@build.id}
- - if current_user && can?(current_user, :manage_builds, @project)
+ - if current_user && can?(current_user, :update_build, @project)
.pull-right
- if @build.cancel_url
= link_to "Cancel", @build.cancel_url, class: 'btn btn-sm btn-danger', method: :post
diff --git a/app/views/projects/commit/_builds.html.haml b/app/views/projects/commit/_builds.html.haml
index 329aaa0bb8b77bfd4ee9dbb76aceccd872db2776..befad27666c320067f017b9df5d4e086bc70ce86 100644
--- a/app/views/projects/commit/_builds.html.haml
+++ b/app/views/projects/commit/_builds.html.haml
@@ -1,6 +1,6 @@
.gray-content-block.middle-block
.pull-right
- - if can?(current_user, :manage_builds, @ci_commit.project)
+ - if can?(current_user, :update_build, @ci_commit.project)
- if @ci_commit.builds.latest.failed.any?(&:retryable?)
= link_to "Retry failed", retry_builds_namespace_project_commit_path(@ci_commit.project.namespace, @ci_commit.project, @ci_commit.sha), class: 'btn btn-grouped btn-primary', method: :post
diff --git a/app/views/projects/commit_statuses/_commit_status.html.haml b/app/views/projects/commit_statuses/_commit_status.html.haml
index 2e3c956ddc44b7aa8665cd4dd19ee02a87c6c3c3..fba4405cb7def3e657438a2d3915598e83fb6d0a 100644
--- a/app/views/projects/commit_statuses/_commit_status.html.haml
+++ b/app/views/projects/commit_statuses/_commit_status.html.haml
@@ -8,7 +8,7 @@
= ci_status_with_icon(commit_status.status)
%td.commit_status-link
- - if commit_status.target_url
+ - if can?(current_user, :read_build, commit_status.project) && commit_status.target_url
= link_to commit_status.target_url do
%strong ##{commit_status.id}
- else
@@ -66,10 +66,10 @@
%td
.pull-right
- - if current_user && can?(current_user, :read_build_artifacts, commit_status.project) && commit_status.artifacts_download_url
+ - if can?(current_user, :read_build, commit_status.project) && commit_status.artifacts_download_url
= link_to commit_status.artifacts_download_url, title: 'Download artifacts' do
%i.fa.fa-download
- - if current_user && can?(current_user, :manage_builds, commit_status.project)
+ - if can?(current_user, :update_build, commit_status.project)
- if commit_status.active?
- if commit_status.cancel_url
= link_to commit_status.cancel_url, method: :post, title: 'Cancel' do
diff --git a/lib/api/builds.rb b/lib/api/builds.rb
index d293f988165d8d0d7725290ca74b6b0ee479ec14..a8bd3842ce441bada1ba840871e4ce9471a66715 100644
--- a/lib/api/builds.rb
+++ b/lib/api/builds.rb
@@ -13,11 +13,12 @@ module API
# Example Request:
# GET /projects/:id/builds
get ':id/builds' do
+
builds = user_project.builds.order('id DESC')
builds = filter_builds(builds, params[:scope])
present paginate(builds), with: Entities::Build,
- user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
+ user_can_download_artifacts: can?(current_user, :read_build, user_project)
end
# Get builds for a specific commit of a project
@@ -30,6 +31,8 @@ module API
# Example Request:
# GET /projects/:id/repository/commits/:sha/builds
get ':id/repository/commits/:sha/builds' do
+ authorize_read_builds!
+
commit = user_project.ci_commits.find_by_sha(params[:sha])
return not_found! unless commit
@@ -37,7 +40,7 @@ module API
builds = filter_builds(builds, params[:scope])
present paginate(builds), with: Entities::Build,
- user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
+ user_can_download_artifacts: can?(current_user, :read_build, user_project)
end
# Get a specific build of a project
@@ -48,11 +51,13 @@ module API
# Example Request:
# GET /projects/:id/builds/:build_id
get ':id/builds/:build_id' do
+ authorize_read_builds!
+
build = get_build(params[:build_id])
return not_found!(build) unless build
present build, with: Entities::Build,
- user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
+ user_can_download_artifacts: can?(current_user, :read_build, user_project)
end
# Get a trace of a specific build of a project
@@ -67,6 +72,8 @@ module API
# is saved in the DB instead of file). But before that, we need to consider how to replace the value of
# `runners_token` with some mask (like `xxxxxx`) when sending trace file directly by workhorse.
get ':id/builds/:build_id/trace' do
+ authorize_read_builds!
+
build = get_build(params[:build_id])
return not_found!(build) unless build
@@ -86,7 +93,7 @@ module API
# example request:
# post /projects/:id/build/:build_id/cancel
post ':id/builds/:build_id/cancel' do
- authorize_manage_builds!
+ authorize_update_builds!
build = get_build(params[:build_id])
return not_found!(build) unless build
@@ -94,7 +101,7 @@ module API
build.cancel
present build, with: Entities::Build,
- user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
+ user_can_download_artifacts: can?(current_user, :read_build, user_project)
end
# Retry a specific build of a project
@@ -105,7 +112,7 @@ module API
# example request:
# post /projects/:id/build/:build_id/retry
post ':id/builds/:build_id/retry' do
- authorize_manage_builds!
+ authorize_update_builds!
build = get_build(params[:build_id])
return forbidden!('Build is not retryable') unless build && build.retryable?
@@ -113,7 +120,7 @@ module API
build = Ci::Build.retry(build)
present build, with: Entities::Build,
- user_can_download_artifacts: can?(current_user, :download_build_artifacts, user_project)
+ user_can_download_artifacts: can?(current_user, :read_build, user_project)
end
end
@@ -141,8 +148,12 @@ module API
builds.where(status: available_statuses && scope)
end
- def authorize_manage_builds!
- authorize! :manage_builds, user_project
+ def authorize_read_builds!
+ authorize! :read_build, user_project
+ end
+
+ def authorize_update_builds!
+ authorize! :update_build, user_project
end
end
end
diff --git a/lib/api/commit_statuses.rb b/lib/api/commit_statuses.rb
index 1162271f5fcd506bb35acf0d3710ac45c90e000b..9422d438d21e191678c1903114c7ef02762dec63 100644
--- a/lib/api/commit_statuses.rb
+++ b/lib/api/commit_statuses.rb
@@ -18,7 +18,7 @@ module API
# Examples:
# GET /projects/:id/repository/commits/:sha/statuses
get ':id/repository/commits/:sha/statuses' do
- authorize! :read_commit_statuses, user_project
+ authorize! :read_commit_status, user_project
sha = params[:sha]
ci_commit = user_project.ci_commit(sha)
not_found! 'Commit' unless ci_commit
diff --git a/lib/api/triggers.rb b/lib/api/triggers.rb
index 5e4964f446cde3a9ab447fac7eec92f26948d190..d1d07394e92847bae0e4c7ee8efd4f4914e8ee4a 100644
--- a/lib/api/triggers.rb
+++ b/lib/api/triggers.rb
@@ -54,7 +54,7 @@ module API
# GET /projects/:id/triggers
get ':id/triggers' do
authenticate!
- authorize_admin_project
+ authorize! :admin_build, user_project
triggers = user_project.triggers.includes(:trigger_requests)
triggers = paginate(triggers)
@@ -71,7 +71,7 @@ module API
# GET /projects/:id/triggers/:token
get ':id/triggers/:token' do
authenticate!
- authorize_admin_project
+ authorize! :admin_build, user_project
trigger = user_project.triggers.find_by(token: params[:token].to_s)
return not_found!('Trigger') unless trigger
@@ -87,7 +87,7 @@ module API
# POST /projects/:id/triggers
post ':id/triggers' do
authenticate!
- authorize_admin_project
+ authorize! :admin_build, user_project
trigger = user_project.triggers.create
@@ -103,7 +103,7 @@ module API
# DELETE /projects/:id/triggers/:token
delete ':id/triggers/:token' do
authenticate!
- authorize_admin_project
+ authorize! :admin_build, user_project
trigger = user_project.triggers.find_by(token: params[:token].to_s)
return not_found!('Trigger') unless trigger
diff --git a/lib/api/variables.rb b/lib/api/variables.rb
index d9a055f6c927ad00b1e1843d613ff1391fc76ff3..f6495071a11b96d0f64b81f3874fea59f79533c7 100644
--- a/lib/api/variables.rb
+++ b/lib/api/variables.rb
@@ -2,7 +2,7 @@ module API
# Projects variables API
class Variables < Grape::API
before { authenticate! }
- before { authorize_admin_project }
+ before { authorize! :admin_build, user_project }
resource :projects do
# Get project variables
diff --git a/spec/requests/api/builds_spec.rb b/spec/requests/api/builds_spec.rb
index 8c9f5a382b7004db977e2b8edfa33e1c10c92a49..6c07802db8b32d041af9871ab2c5f7d298f54738 100644
--- a/spec/requests/api/builds_spec.rb
+++ b/spec/requests/api/builds_spec.rb
@@ -113,7 +113,7 @@ describe API::API, api: true do
describe 'POST /projects/:id/builds/:build_id/cancel' do
context 'authorized user' do
- context 'user with :manage_builds persmission' do
+ context 'user with :update_build persmission' do
it 'should cancel running or pending build' do
post api("/projects/#{project.id}/builds/#{build.id}/cancel", user)
@@ -122,7 +122,7 @@ describe API::API, api: true do
end
end
- context 'user without :manage_builds permission' do
+ context 'user without :update_build permission' do
it 'should not cancel build' do
post api("/projects/#{project.id}/builds/#{build.id}/cancel", user2)
@@ -142,7 +142,7 @@ describe API::API, api: true do
describe 'POST /projects/:id/builds/:build_id/retry' do
context 'authorized user' do
- context 'user with :manage_builds persmission' do
+ context 'user with :update_build persmission' do
it 'should retry non-running build' do
post api("/projects/#{project.id}/builds/#{build_canceled.id}/retry", user)
@@ -152,7 +152,7 @@ describe API::API, api: true do
end
end
- context 'user without :manage_builds permission' do
+ context 'user without :update_build permission' do
it 'should not retry build' do
post api("/projects/#{project.id}/builds/#{build_canceled.id}/retry", user2)