Commit 9b58b8e3 authored by Shinya Maeda's avatar Shinya Maeda

Do not allow jobs to be erased

parent d4ceec9d
...@@ -5,6 +5,7 @@ class Projects::JobsController < Projects::ApplicationController ...@@ -5,6 +5,7 @@ class Projects::JobsController < Projects::ApplicationController
only: [:index, :show, :status, :raw, :trace] only: [:index, :show, :status, :raw, :trace]
before_action :authorize_update_build!, before_action :authorize_update_build!,
except: [:index, :show, :status, :raw, :trace, :cancel_all] except: [:index, :show, :status, :raw, :trace, :cancel_all]
before_action :authorize_erase_build!, only: [:erase]
layout 'project' layout 'project'
...@@ -131,6 +132,10 @@ class Projects::JobsController < Projects::ApplicationController ...@@ -131,6 +132,10 @@ class Projects::JobsController < Projects::ApplicationController
return access_denied! unless can?(current_user, :update_build, build) return access_denied! unless can?(current_user, :update_build, build)
end end
def authorize_erase_build!
return access_denied! unless can?(current_user, :erase_build, build)
end
def build def build
@build ||= project.builds.find(params[:id]) @build ||= project.builds.find(params[:id])
.present(current_user: current_user) .present(current_user: current_user)
......
...@@ -192,6 +192,10 @@ module Ci ...@@ -192,6 +192,10 @@ module Ci
project.build_timeout project.build_timeout
end end
def owned_by?(current_user)
user == current_user
end
# A slugified version of the build ref, suitable for inclusion in URLs and # A slugified version of the build ref, suitable for inclusion in URLs and
# domain names. Rules: # domain names. Rules:
# #
......
...@@ -10,6 +10,11 @@ module Ci ...@@ -10,6 +10,11 @@ module Ci
end end
end end
condition(:owner_of_build) do
can?(:developer_access) && @subject.owned_by?(@user)
end
rule { protected_ref }.prevent :update_build rule { protected_ref }.prevent :update_build
rule { can?(:master_access) | owner_of_build }.enable :erase_build
end end
end end
...@@ -6,7 +6,7 @@ class BuildDetailsEntity < JobEntity ...@@ -6,7 +6,7 @@ class BuildDetailsEntity < JobEntity
expose :pipeline, using: PipelineEntity expose :pipeline, using: PipelineEntity
expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity expose :erased_by, if: -> (*) { build.erased? }, using: UserEntity
expose :erase_path, if: -> (*) { build.erasable? && can?(current_user, :update_build, project) } do |build| expose :erase_path, if: -> (*) { build.erasable? && can?(current_user, :erase_build, build) } do |build|
erase_project_job_path(project, build) erase_project_job_path(project, build)
end end
......
...@@ -71,7 +71,7 @@ ...@@ -71,7 +71,7 @@
class: 'js-raw-link-controller has-tooltip controllers-buttons' do class: 'js-raw-link-controller has-tooltip controllers-buttons' do
= icon('file-text-o') = icon('file-text-o')
- if can?(current_user, :update_build, @project) && @build.erasable? - if @build.erasable? && can?(current_user, :erase_build, @build)
= link_to erase_project_job_path(@project, @build), = link_to erase_project_job_path(@project, @build),
method: :post, method: :post,
data: { confirm: 'Are you sure you want to erase this build?', placement: 'top', container: 'body' }, data: { confirm: 'Are you sure you want to erase this build?', placement: 'top', container: 'body' },
......
...@@ -136,7 +136,7 @@ module API ...@@ -136,7 +136,7 @@ module API
authorize_update_builds! authorize_update_builds!
build = find_build!(params[:job_id]) build = find_build!(params[:job_id])
authorize!(:update_build, build) authorize!(:erase_build, build)
return forbidden!('Job is not erasable!') unless build.erasable? return forbidden!('Job is not erasable!') unless build.erasable?
build.erase(erased_by: current_user) build.erase(erased_by: current_user)
......
...@@ -169,7 +169,7 @@ module API ...@@ -169,7 +169,7 @@ module API
authorize_update_builds! authorize_update_builds!
build = get_build!(params[:build_id]) build = get_build!(params[:build_id])
authorize!(:update_build, build) authorize!(:erase_build, build)
return forbidden!('Build is not erasable!') unless build.erasable? return forbidden!('Build is not erasable!') unless build.erasable?
build.erase(erased_by: current_user) build.erase(erased_by: current_user)
......
...@@ -150,5 +150,47 @@ describe Ci::BuildPolicy do ...@@ -150,5 +150,47 @@ describe Ci::BuildPolicy do
end end
end end
end end
# TODO: Finish spec
describe 'rules for erase build' do
let(:project) { create(:project, :repository) }
let(:another_user) { create(:user) }
context 'when developer created a build' do
before do
project.add_developer(user)
end
context 'when the build was created by the user' do
let(:build) { create(:ci_build, user: user) }
it { expect(policy).to be_allowed :erase_build }
end
context 'when the build was created by others' do
let(:build) { create(:ci_build, user: another_user) }
it { expect(policy).to be_disallowed :erase_build }
end
end
context 'when master erases a build' do
before do
project.add_master(user)
end
context 'when the build was created by the user' do
let(:build) { create(:ci_build, user: user) }
it { expect(policy).to be_allowed :erase_build }
end
context 'when the build was created by others' do
let(:build) { create(:ci_build, user: another_user) }
it { expect(policy).to be_allowed :erase_build }
end
end
end
end end
end end
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