Commit afef3853 authored by Shinya Maeda's avatar Shinya Maeda

Add doc. Fix spec. Add erase_build in protected_ref rule

parent cb5e35d5
...@@ -4,7 +4,7 @@ class Projects::JobsController < Projects::ApplicationController ...@@ -4,7 +4,7 @@ class Projects::JobsController < Projects::ApplicationController
before_action :authorize_read_build!, before_action :authorize_read_build!,
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, :erase]
before_action :authorize_erase_build!, only: [:erase] before_action :authorize_erase_build!, only: [:erase]
layout 'project' layout 'project'
......
...@@ -192,7 +192,7 @@ module Ci ...@@ -192,7 +192,7 @@ module Ci
project.build_timeout project.build_timeout
end end
def owned_by?(current_user) def triggered_by?(current_user)
user == current_user user == current_user
end end
......
...@@ -10,11 +10,15 @@ module Ci ...@@ -10,11 +10,15 @@ module Ci
end end
end end
condition(:owner_of_build) do condition(:owner_of_job) do
can?(:developer_access) && @subject.owned_by?(@user) can?(:developer_access) && @subject.triggered_by?(@user)
end end
rule { protected_ref }.prevent :update_build rule { protected_ref }.policy do
rule { can?(:master_access) | owner_of_build }.enable :erase_build prevent :update_build
prevent :erase_build
end
rule { can?(:master_access) | owner_of_job }.enable :erase_build
end end
end end
...@@ -197,6 +197,7 @@ instance and project. In addition, all admins can use the admin interface under ...@@ -197,6 +197,7 @@ instance and project. In addition, all admins can use the admin interface under
|---------------------------------------|-----------------|-------------|----------|--------| |---------------------------------------|-----------------|-------------|----------|--------|
| See commits and jobs | ✓ | ✓ | ✓ | ✓ | | See commits and jobs | ✓ | ✓ | ✓ | ✓ |
| Retry or cancel job | | ✓ | ✓ | ✓ | | Retry or cancel job | | ✓ | ✓ | ✓ |
| Erase job artifacts and trace | | ✓ [^7] | ✓ | ✓ |
| Remove project | | | ✓ | ✓ | | Remove project | | | ✓ | ✓ |
| Create project | | | ✓ | ✓ | | Create project | | | ✓ | ✓ |
| Change project configuration | | | ✓ | ✓ | | Change project configuration | | | ✓ | ✓ |
...@@ -261,5 +262,6 @@ only. ...@@ -261,5 +262,6 @@ only.
[^4]: Not allowed for Guest, Reporter, Developer, Master, or Owner [^4]: Not allowed for Guest, Reporter, Developer, Master, or Owner
[^5]: Only if user is not external one. [^5]: Only if user is not external one.
[^6]: Only if user is a member of the project. [^6]: Only if user is a member of the project.
[^7]: Only if the build was triggered by the user
[ce-18994]: https://gitlab.com/gitlab-org/gitlab-ce/issues/18994 [ce-18994]: https://gitlab.com/gitlab-org/gitlab-ce/issues/18994
[new-mod]: project/new_ci_build_permissions_model.md [new-mod]: project/new_ci_build_permissions_model.md
...@@ -136,7 +136,6 @@ module API ...@@ -136,7 +136,6 @@ 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) authorize!(:erase_build, build)
return forbidden!('Job is not erasable!') unless build.erasable? return forbidden!('Job is not erasable!') unless build.erasable?
......
...@@ -169,7 +169,6 @@ module API ...@@ -169,7 +169,6 @@ 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) authorize!(:erase_build, build)
return forbidden!('Build is not erasable!') unless build.erasable? return forbidden!('Build is not erasable!') unless build.erasable?
......
...@@ -372,12 +372,14 @@ describe Projects::JobsController do ...@@ -372,12 +372,14 @@ describe Projects::JobsController do
describe 'POST erase' do describe 'POST erase' do
before do before do
project.add_developer(user) project.team << [user, role]
sign_in(user) sign_in(user)
post_erase post_erase
end end
let(:role) { :master }
context 'when job is erasable' do context 'when job is erasable' do
let(:job) { create(:ci_build, :erasable, :trace, pipeline: pipeline) } let(:job) { create(:ci_build, :erasable, :trace, pipeline: pipeline) }
...@@ -404,6 +406,27 @@ describe Projects::JobsController do ...@@ -404,6 +406,27 @@ describe Projects::JobsController do
end end
end end
context 'when user is developer' do
let(:role) { :developer }
let(:job) { create(:ci_build, :erasable, :trace, pipeline: pipeline, user: triggered_by) }
context 'when triggered by same user' do
let(:triggered_by) { user }
it 'has successful status' do
expect(response).to have_gitlab_http_status(:found)
end
end
context 'when triggered by different user' do
let(:triggered_by) { create(:user) }
it 'does not have successful status' do
expect(response).to have_gitlab_http_status(:not_found)
end
end
end
def post_erase def post_erase
post :erase, namespace_id: project.namespace, post :erase, namespace_id: project.namespace,
project_id: project, project_id: project,
......
...@@ -270,8 +270,8 @@ describe Ci::Build do ...@@ -270,8 +270,8 @@ describe Ci::Build do
end end
end end
describe '#owned_by?' do describe '#triggered_by?' do
subject { build.owned_by?(user) } subject { build.triggered_by?(user) }
context 'when user is owner' do context 'when user is owner' do
let(:build) { create(:ci_build, pipeline: pipeline, user: user) } let(:build) { create(:ci_build, pipeline: pipeline, user: user) }
......
...@@ -152,39 +152,57 @@ describe Ci::BuildPolicy do ...@@ -152,39 +152,57 @@ describe Ci::BuildPolicy do
end end
describe 'rules for erase build' do describe 'rules for erase build' do
let(:project) { create(:project, :repository) } let(:project) { create(:project) }
let(:build) { create(:ci_build, pipeline: pipeline, user: owner) } let(:build) { create(:ci_build, pipeline: pipeline, ref: 'some-ref', user: owner) }
context 'when developer created a build' do context 'when a developer erases a build' do
before do before do
project.add_developer(user) project.add_developer(user)
end end
context 'when the build was created by the user' do context 'when developers can push to the branch' do
let(:owner) { user } before do
create(:protected_branch, :developers_can_merge,
name: build.ref, project: project)
end
it { expect(policy).to be_allowed :erase_build } context 'when the build was created by the developer' do
let(:owner) { user }
it { expect(policy).to be_allowed :erase_build }
end
context 'when the build was created by the other' do
let(:owner) { create(:user) }
it { expect(policy).to be_disallowed :erase_build }
end
end end
context 'when the build was created by others' do context 'when no one can push or merge to the branch' do
let(:owner) { create(:user) } let(:owner) { user }
before do
create(:protected_branch, :no_one_can_push,
name: build.ref, project: project)
end
it { expect(policy).to be_disallowed :erase_build } it { expect(policy).to be_disallowed :erase_build }
end end
end end
context 'when master erases a build' do context 'when a master erases a build' do
before do before do
project.add_master(user) project.add_master(user)
end end
context 'when the build was created by the user' do context 'when the build was created by the master' do
let(:owner) { user } let(:owner) { user }
it { expect(policy).to be_allowed :erase_build } it { expect(policy).to be_allowed :erase_build }
end end
context 'when the build was created by others' do context 'when the build was created by the other' do
let(:owner) { create(:user) } let(:owner) { create(:user) }
it { expect(policy).to be_allowed :erase_build } it { expect(policy).to be_allowed :erase_build }
......
...@@ -491,6 +491,8 @@ describe API::Jobs do ...@@ -491,6 +491,8 @@ describe API::Jobs do
describe 'POST /projects/:id/jobs/:job_id/erase' do describe 'POST /projects/:id/jobs/:job_id/erase' do
before do before do
project.add_master(user)
post api("/projects/#{project.id}/jobs/#{job.id}/erase", user) post api("/projects/#{project.id}/jobs/#{job.id}/erase", user)
end end
......
...@@ -408,6 +408,8 @@ describe API::V3::Builds do ...@@ -408,6 +408,8 @@ describe API::V3::Builds do
describe 'POST /projects/:id/builds/:build_id/erase' do describe 'POST /projects/:id/builds/:build_id/erase' do
before do before do
project.add_master(user)
post v3_api("/projects/#{project.id}/builds/#{build.id}/erase", user) post v3_api("/projects/#{project.id}/builds/#{build.id}/erase", user)
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