From 1b8f52d9206bdf19c0dde04505c4c0b1cf46cfbe Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin <godfat@godfat.org> Date: Tue, 14 Jun 2016 22:58:38 +0800 Subject: [PATCH] Avoid enabling locked runners. Give 403 in this case --- .../admin/runner_projects_controller.rb | 2 ++ .../projects/runner_projects_controller.rb | 1 + lib/api/runners.rb | 1 + spec/requests/api/runners_spec.rb | 16 ++++++++++++++-- 4 files changed, 18 insertions(+), 2 deletions(-) diff --git a/app/controllers/admin/runner_projects_controller.rb b/app/controllers/admin/runner_projects_controller.rb index d25619d94e0..29307aeab6d 100644 --- a/app/controllers/admin/runner_projects_controller.rb +++ b/app/controllers/admin/runner_projects_controller.rb @@ -9,6 +9,8 @@ class Admin::RunnerProjectsController < Admin::ApplicationController def create @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) redirect_to admin_runner_path(@runner) else diff --git a/app/controllers/projects/runner_projects_controller.rb b/app/controllers/projects/runner_projects_controller.rb index bedeb4a295c..4c013303269 100644 --- a/app/controllers/projects/runner_projects_controller.rb +++ b/app/controllers/projects/runner_projects_controller.rb @@ -6,6 +6,7 @@ class Projects::RunnerProjectsController < Projects::ApplicationController def create @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) path = runners_path(project) diff --git a/lib/api/runners.rb b/lib/api/runners.rb index 2d09b6193d9..3ae228d61d8 100644 --- a/lib/api/runners.rb +++ b/lib/api/runners.rb @@ -163,6 +163,7 @@ module API def authenticate_enable_runner!(runner) forbidden!("Runner is shared") if runner.is_shared? + forbidden!("Runner is locked") if runner.locked? return if current_user.is_admin? forbidden!("No access granted") unless user_can_access_runner?(runner) end diff --git a/spec/requests/api/runners_spec.rb b/spec/requests/api/runners_spec.rb index f4f9c417bbb..26dfa7bed05 100644 --- a/spec/requests/api/runners_spec.rb +++ b/spec/requests/api/runners_spec.rb @@ -362,11 +362,13 @@ describe API::Runners, api: true do describe 'POST /projects/:id/runners' do context 'authorized user' do - it 'should enable specific runner' do - specific_runner2 = create(:ci_runner).tap do |runner| + let(:specific_runner2) do + create(:ci_runner).tap do |runner| create(:ci_runner_project, runner: runner, project: project2) end + end + it 'should enable specific runner' do expect do post api("/projects/#{project.id}/runners", user), runner_id: specific_runner2.id end.to change{ project.runners.count }.by(+1) @@ -380,6 +382,16 @@ describe API::Runners, api: true do expect(response.status).to eq(201) 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 post api("/projects/#{project.id}/runners", user), runner_id: shared_runner.id -- 2.30.9