diff --git a/CHANGELOG b/CHANGELOG index b97116af73ca4a429f5e9cbf1bff5604ec132521..e2fa16e4eee8843359961f1ff41ef963ebdc09af 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -30,6 +30,10 @@ v 8.10.0 (unreleased) - Allow [ci skip] to be in any case and allow [skip ci]. !4785 (simon_w) - Add basic system information like memory and disk usage to the admin panel +v 8.9.5 (unreleased) + - Fix assigning shared runners as admins + - Show "locked" label for locked runners on runners admin + v 8.9.4 - Fix privilege escalation issue with OAuth external users. - Ensure references to private repos aren't shown to logged-out users. diff --git a/app/controllers/admin/runner_projects_controller.rb b/app/controllers/admin/runner_projects_controller.rb index bf20c5305a7b9d821cd1d11491cdf206043e5477..bc65dcc33d3c582a097f4d17d69a6cbf4e45f5c6 100644 --- a/app/controllers/admin/runner_projects_controller.rb +++ b/app/controllers/admin/runner_projects_controller.rb @@ -4,8 +4,6 @@ class Admin::RunnerProjectsController < Admin::ApplicationController def create @runner = Ci::Runner.find(params[:runner_project][:runner_id]) - return head(403) if @runner.is_shared? || @runner.locked? - runner_project = @runner.assign_to(@project, current_user) if runner_project.persisted? diff --git a/app/controllers/projects/runner_projects_controller.rb b/app/controllers/projects/runner_projects_controller.rb index dc1a18f8d4209aa393520e0d59e12ef02af139f3..8267b14941dff11bf55e96e456b30710cbf05545 100644 --- a/app/controllers/projects/runner_projects_controller.rb +++ b/app/controllers/projects/runner_projects_controller.rb @@ -6,8 +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.locked? - return head(403) unless current_user.ci_authorized_runners.include?(@runner) + return head(403) unless can?(current_user, :assign_runner, @runner) path = runners_path(project) runner_project = @runner.assign_to(project, current_user) diff --git a/app/models/ability.rb b/app/models/ability.rb index f5950879ccb5ce3637c12d228d61bbbab2b11190..ba1f2ae40752c3558b68471bbdb9759192d68e59 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -1,5 +1,6 @@ class Ability class << self + # rubocop: disable Metrics/CyclomaticComplexity def allowed(user, subject) return anonymous_abilities(user, subject) if user.nil? return [] unless user.is_a?(User) @@ -19,6 +20,7 @@ class Ability when ProjectMember then project_member_abilities(user, subject) when User then user_abilities when ExternalIssue, Deployment, Environment then project_abilities(user, subject.project) + when Ci::Runner then runner_abilities(user, subject) else [] end.concat(global_abilities(user)) end @@ -512,6 +514,18 @@ class Ability rules end + def runner_abilities(user, runner) + if user.is_admin? + [:assign_runner] + elsif runner.is_shared? || runner.locked? + [] + elsif user.ci_authorized_runners.include?(runner) + [:assign_runner] + else + [] + end + end + def user_abilities [:read_user] end diff --git a/app/views/admin/runners/_runner.html.haml b/app/views/admin/runners/_runner.html.haml index 36b21eefdee8ddc25286fc242595c0fec1dcba42..64893b38c58deef0c7f0f8c28cabc1c346173e2c 100644 --- a/app/views/admin/runners/_runner.html.haml +++ b/app/views/admin/runners/_runner.html.haml @@ -4,6 +4,8 @@ %span.label.label-success shared - else %span.label.label-info specific + - if runner.locked? + %span.label.label-warning locked - unless runner.active? %span.label.label-danger paused diff --git a/app/views/admin/runners/index.html.haml b/app/views/admin/runners/index.html.haml index 114bea92fc25a87ec0d23d842307f41e2dac48f7..a53876d67573d3e767473aacb1635be1292d304a 100644 --- a/app/views/admin/runners/index.html.haml +++ b/app/views/admin/runners/index.html.haml @@ -39,6 +39,9 @@ %li %span.label.label-info specific \- run builds from assigned projects + %li + %span.label.label-warning locked + \- runner cannot be assigned to other projects %li %span.label.label-danger paused \- runner will not receive any new builds diff --git a/spec/features/admin/admin_runners_spec.rb b/spec/features/admin/admin_runners_spec.rb index 2d297776cb021a87aeea909c7a889d39159717f2..2f82fafc13aaaf0a62fd79afb1f3d10943c3869e 100644 --- a/spec/features/admin/admin_runners_spec.rb +++ b/spec/features/admin/admin_runners_spec.rb @@ -62,19 +62,45 @@ describe "Admin Runners" do end describe 'enable/create' do - before do - @project1.runners << runner - visit admin_runner_path(runner) + shared_examples 'assignable runner' do + it 'enables a runner for a project' do + within '.unassigned-projects' do + click_on 'Enable' + end + + assigned_project = page.find('.assigned-projects') + + expect(assigned_project).to have_content(@project2.path) + end end - it 'enables specific runner for project' do - within '.unassigned-projects' do - click_on 'Enable' + context 'with specific runner' do + before do + @project1.runners << runner + visit admin_runner_path(runner) end - assigned_project = page.find('.assigned-projects') + it_behaves_like 'assignable runner' + end + + context 'with locked runner' do + before do + runner.update(locked: true) + @project1.runners << runner + visit admin_runner_path(runner) + end + + it_behaves_like 'assignable runner' + end + + context 'with shared runner' do + before do + @project1.destroy + runner.update(is_shared: true) + visit admin_runner_path(runner) + end - expect(assigned_project).to have_content(@project2.path) + it_behaves_like 'assignable runner' end end