Commit 1b8f52d9 authored by Lin Jen-Shin's avatar Lin Jen-Shin

Avoid enabling locked runners. Give 403 in this case

parent cbd6ca69
...@@ -9,6 +9,8 @@ class Admin::RunnerProjectsController < Admin::ApplicationController ...@@ -9,6 +9,8 @@ class Admin::RunnerProjectsController < Admin::ApplicationController
def create def create
@runner = Ci::Runner.find(params[:runner_project][:runner_id]) @runner = Ci::Runner.find(params[:runner_project][:runner_id])
return head(403) if runner.is_shared? || runner.is_locked?
if @runner.assign_to(@project, current_user) if @runner.assign_to(@project, current_user)
redirect_to admin_runner_path(@runner) redirect_to admin_runner_path(@runner)
else else
......
...@@ -6,6 +6,7 @@ class Projects::RunnerProjectsController < Projects::ApplicationController ...@@ -6,6 +6,7 @@ class Projects::RunnerProjectsController < Projects::ApplicationController
def create def create
@runner = Ci::Runner.find(params[:runner_project][:runner_id]) @runner = Ci::Runner.find(params[:runner_project][:runner_id])
return head(403) if runner.is_shared? || runner.is_locked?
return head(403) unless current_user.ci_authorized_runners.include?(@runner) return head(403) unless current_user.ci_authorized_runners.include?(@runner)
path = runners_path(project) path = runners_path(project)
......
...@@ -163,6 +163,7 @@ module API ...@@ -163,6 +163,7 @@ module API
def authenticate_enable_runner!(runner) def authenticate_enable_runner!(runner)
forbidden!("Runner is shared") if runner.is_shared? forbidden!("Runner is shared") if runner.is_shared?
forbidden!("Runner is locked") if runner.locked?
return if current_user.is_admin? return if current_user.is_admin?
forbidden!("No access granted") unless user_can_access_runner?(runner) forbidden!("No access granted") unless user_can_access_runner?(runner)
end end
......
...@@ -362,11 +362,13 @@ describe API::Runners, api: true do ...@@ -362,11 +362,13 @@ describe API::Runners, api: true do
describe 'POST /projects/:id/runners' do describe 'POST /projects/:id/runners' do
context 'authorized user' do context 'authorized user' do
it 'should enable specific runner' do let(:specific_runner2) do
specific_runner2 = create(:ci_runner).tap do |runner| create(:ci_runner).tap do |runner|
create(:ci_runner_project, runner: runner, project: project2) create(:ci_runner_project, runner: runner, project: project2)
end end
end
it 'should enable specific runner' do
expect do expect do
post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id
end.to change{ project.runners.count }.by(+1) end.to change{ project.runners.count }.by(+1)
...@@ -380,6 +382,16 @@ describe API::Runners, api: true do ...@@ -380,6 +382,16 @@ describe API::Runners, api: true do
expect(response.status).to eq(201) expect(response.status).to eq(201)
end end
it 'should not enable locked runner' do
specific_runner2.update(locked: true)
expect do
post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id
end.to change{ project.runners.count }.by(0)
expect(response.status).to eq(403)
end
it 'should not enable shared runner' do it 'should not enable shared runner' do
post api("/projects/#{project.id}/runners", user), runner_id: shared_runner.id post api("/projects/#{project.id}/runners", user), runner_id: shared_runner.id
......
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