From 0d37084cf26cb5405fba7c8a3a43537c3b7e0881 Mon Sep 17 00:00:00 2001 From: Kev Kloss Date: Wed, 19 Jun 2024 17:03:20 +0200 Subject: [PATCH 1/6] Add read_runners custom ability Changelog: changed EE: true --- .../json_schemas/member_role_permissions.json | 3 +++ doc/api/graphql/reference/index.md | 1 + doc/api/member_roles.md | 7 +++++++ doc/user/custom_roles/abilities.md | 1 + ee/app/policies/ee/ci/runner_policy.rb | 6 ++++++ ee/app/policies/ee/group_policy.rb | 4 ++++ ee/app/policies/ee/project_policy.rb | 4 ++++ ee/config/custom_abilities/read_runners.yml | 11 +++++++++++ .../controllers/groups/runners_controller_spec.rb | 15 +++++++++++++++ ee/spec/policies/group_policy_spec.rb | 7 +++++++ ee/spec/policies/project_policy_spec.rb | 7 +++++++ 11 files changed, 66 insertions(+) create mode 100644 ee/config/custom_abilities/read_runners.yml diff --git a/app/validators/json_schemas/member_role_permissions.json b/app/validators/json_schemas/member_role_permissions.json index 4a1f170d767290..86abb3ce9ceac8 100644 --- a/app/validators/json_schemas/member_role_permissions.json +++ b/app/validators/json_schemas/member_role_permissions.json @@ -69,6 +69,9 @@ }, "remove_project": { "type": "boolean" + }, + "read_runners": { + "type": "boolean" } } } diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index c1dfcccdbaf850..1605e138f86df0 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -34736,6 +34736,7 @@ Member role permission. | `READ_CODE` | Allows read-only access to the source code in the user interface. Does not allow users to edit or download repository archives, clone or pull repositories, view source code in an IDE, or view merge requests for private projects. You can download individual files because read-only access inherently grants the ability to make a local copy of the file. | | `READ_CRM_CONTACT` | Read CRM contact. | | `READ_DEPENDENCY` | Allows read-only access to the dependencies and licenses. | +| `READ_RUNNERS` | Allows read-only access to group or project runners, including the runner fleet dashboard. | | `READ_VULNERABILITY` | Read vulnerability reports and security dashboards. | | `REMOVE_GROUP` | Ability to delete or restore a group. This ability does not allow deleting top level groups. Review the Retention period settings to prevent accidental deletion. | | `REMOVE_PROJECT` | Allows deletion of projects. | diff --git a/doc/api/member_roles.md b/doc/api/member_roles.md index 7a7a15837c54fc..075b699ff06aee 100644 --- a/doc/api/member_roles.md +++ b/doc/api/member_roles.md @@ -77,6 +77,7 @@ Example response: "manage_project_access_tokens": false, "manage_security_policy_link": false, "read_code": true, + "read_runners": false, "read_dependency": false, "read_vulnerability": false, "remove_group": false, @@ -115,6 +116,7 @@ Supported attributes: | `manage_project_access_tokens` | boolean | no | Permission to manage project access tokens. | | `manage_security_policy_link` | boolean | no | Permission to link security policy projects. | | `read_code` | boolean | no | Permission to read project code. | +| `read_runners` | boolean | no | Permission to view project runners. | | `read_dependency` | boolean | no | Permission to read project dependencies. | | `read_vulnerability` | boolean | no | Permission to read project vulnerabilities. | | `remove_group` | boolean | no | Permission to delete or restore a group. | @@ -152,6 +154,7 @@ Example response: "manage_project_access_tokens": false, "manage_security_policy_link": false, "read_code": true, + "read_runners": false, "read_dependency": false, "read_vulnerability": false, "remove_group": false, @@ -236,6 +239,7 @@ Example response: "manage_project_access_tokens": false, "manage_security_policy_link": false, "read_code": true, + "read_runners": false, "read_dependency": false, "read_vulnerability": false, "remove_group": false, @@ -262,6 +266,7 @@ Example response: "manage_project_access_tokens": false, "manage_security_policy_link": false, "read_code": true, + "read_runners": false, "read_dependency": true, "read_vulnerability": true, "remove_group": false, @@ -300,6 +305,7 @@ Parameters: | `manage_project_access_tokens` | boolean | no | Permission to manage project access tokens. | | `manage_security_policy_link` | boolean | no | Permission to link security policy projects. | | `read_code` | boolean | no | Permission to read project code. | +| `read_runners` | boolean | no | Permission to view project runners. | | `read_dependency` | boolean | no | Permission to read project dependencies. | | `read_vulnerability` | boolean | no | Permission to read project vulnerabilities. | | `remove_group` | boolean | no | Permission to delete or restore a group. | @@ -335,6 +341,7 @@ Example response: "manage_project_access_tokens": false, "manage_security_policy_link": false, "read_code": true, + "read_runners": false, "read_dependency": false, "read_vulnerability": false, "remove_group": false, diff --git a/doc/user/custom_roles/abilities.md b/doc/user/custom_roles/abilities.md index 57fb1eae4bcaff..21157a17d1ddc5 100644 --- a/doc/user/custom_roles/abilities.md +++ b/doc/user/custom_roles/abilities.md @@ -67,6 +67,7 @@ These requirements are documented in the `Required permission` column in the fol | Name | Required permission | Description | Introduced in | Feature flag | Enabled in | |:-----|:------------|:------------------|:---------|:--------------|:---------| | [`admin_runners`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/151825) | | Create, view, edit, and delete group or project Runners. Includes configuring Runner settings. | GitLab [17.1](https://gitlab.com/gitlab-org/gitlab/-/issues/442851) | `custom_ability_admin_runners` | | +| [`read_runners`](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156798) | | Allows read-only access to group or project runners, including the runner fleet dashboard. | GitLab [17.2](https://gitlab.com/gitlab-org/gitlab/-/issues/468202) | | | ## Secrets management diff --git a/ee/app/policies/ee/ci/runner_policy.rb b/ee/app/policies/ee/ci/runner_policy.rb index 11ef5431bb6337..92b11ffa9dfd2f 100644 --- a/ee/app/policies/ee/ci/runner_policy.rb +++ b/ee/app/policies/ee/ci/runner_policy.rb @@ -15,12 +15,18 @@ module RunnerPolicy ::Authz::CustomAbility.allowed?(@user, :admin_runners, @subject) end + condition(:custom_role_enables_read_runners) do + ::Authz::CustomAbility.allowed?(@user, :read_runners, @subject) + end + rule { custom_role_enables_admin_runners }.policy do enable :assign_runner enable :read_runner enable :update_runner enable :delete_runner end + + rule { custom_role_enables_read_runners }.enable(:read_runner) end end end diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index 482a0357e9a1f4..0459f9f22caf0f 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -566,6 +566,10 @@ module GroupPolicy enable :admin_integrations end + rule { custom_role_enables_read_runners }.policy do + enable :read_group_runners + end + rule { can?(:admin_group) | can?(:admin_compliance_framework) | can?(:manage_deploy_tokens) | can?(:manage_merge_request_settings) }.policy do enable :view_edit_page end diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 0a0fe731638101..3f5b23f88b56dc 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -304,6 +304,10 @@ module ProjectPolicy enable :create_runner end + rule { custom_role_enables_read_runners }.policy do + enable :read_project_runners + end + condition(:ci_cancellation_maintainers_only, scope: :subject) do project.ci_cancellation_restriction.maintainers_only_allowed? end diff --git a/ee/config/custom_abilities/read_runners.yml b/ee/config/custom_abilities/read_runners.yml new file mode 100644 index 00000000000000..3e3efdfda83118 --- /dev/null +++ b/ee/config/custom_abilities/read_runners.yml @@ -0,0 +1,11 @@ +--- +name: read_runners +description: Allows read-only access to group or project runners, + including the runner fleet dashboard. +introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/468202 +introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/156798 +feature_category: runner +milestone: '17.2' +group_ability: true +project_ability: true +available_from_access_level: 40 diff --git a/ee/spec/controllers/groups/runners_controller_spec.rb b/ee/spec/controllers/groups/runners_controller_spec.rb index 6227defb1ce0ed..5a505fd1098db4 100644 --- a/ee/spec/controllers/groups/runners_controller_spec.rb +++ b/ee/spec/controllers/groups/runners_controller_spec.rb @@ -95,6 +95,21 @@ expect(response).to have_gitlab_http_status(:not_found) end + + context 'with read_runners ability' do + let(:user) { create(:user) } + + before do + role = create(:member_role, :developer, read_runners: true, namespace: group) + create(:group_member, user: user, group: group, member_role: role) + end + + it 'returns the dashboard' do + make_request + + expect(response).to have_gitlab_http_status(:ok) + end + end end context 'when feature is not available' do diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb index 5139397ea02540..b3f269f0080795 100644 --- a/ee/spec/policies/group_policy_spec.rb +++ b/ee/spec/policies/group_policy_spec.rb @@ -3767,6 +3767,13 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'custom roles abilities' end + + context 'for a member role with read_runners true' do + let(:member_role_abilities) { { read_runners: true } } + let(:allowed_abilities) { [:read_group_runners] } + + it_behaves_like 'custom roles abilities' + end end context 'for :read_limit_alert' do diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index f7b571c0c17498..2b14bd6c8b6038 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -3005,6 +3005,13 @@ def create_member_role(member, abilities = member_role_abilities) it_behaves_like 'custom roles abilities' end + + context 'for a custom role with the `read_runners` ability' do + let(:member_role_abilities) { { read_runners: true } } + let(:allowed_abilities) { [:read_project_runners] } + + it_behaves_like 'custom roles abilities' + end end describe 'permissions for suggested reviewers bot', :saas do -- GitLab From 119d3a77a43729acd193c8f24dc2b722606f4642 Mon Sep 17 00:00:00 2001 From: Kev Kloss Date: Thu, 20 Jun 2024 18:43:06 +0200 Subject: [PATCH 2/6] Move read_runners group test to separate spec --- .../groups/runners_controller_spec.rb | 15 ----- .../custom_roles/read_runners/request_spec.rb | 66 +++++++++++++++++++ 2 files changed, 66 insertions(+), 15 deletions(-) create mode 100644 ee/spec/requests/custom_roles/read_runners/request_spec.rb diff --git a/ee/spec/controllers/groups/runners_controller_spec.rb b/ee/spec/controllers/groups/runners_controller_spec.rb index 5a505fd1098db4..6227defb1ce0ed 100644 --- a/ee/spec/controllers/groups/runners_controller_spec.rb +++ b/ee/spec/controllers/groups/runners_controller_spec.rb @@ -95,21 +95,6 @@ expect(response).to have_gitlab_http_status(:not_found) end - - context 'with read_runners ability' do - let(:user) { create(:user) } - - before do - role = create(:member_role, :developer, read_runners: true, namespace: group) - create(:group_member, user: user, group: group, member_role: role) - end - - it 'returns the dashboard' do - make_request - - expect(response).to have_gitlab_http_status(:ok) - end - end end context 'when feature is not available' do diff --git a/ee/spec/requests/custom_roles/read_runners/request_spec.rb b/ee/spec/requests/custom_roles/read_runners/request_spec.rb new file mode 100644 index 00000000000000..aaf52a07b0f70e --- /dev/null +++ b/ee/spec/requests/custom_roles/read_runners/request_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require "spec_helper" + +RSpec.describe "User with read_runners custom role", feature_category: :runner do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :in_group) } + let_it_be_with_reload(:group) { project.group } + let_it_be(:role) { create(:member_role, :guest, :read_runners, namespace: project.group) } + + before do + stub_licensed_features(custom_roles: true) + end + + describe Groups::RunnersController do + let_it_be(:membership) { create(:group_member, :guest, member_role: role, user: user, source: group) } + + let_it_be(:runner) do + create(:ci_runner, :group, groups: [group], registration_type: :authenticated_user) + end + + before do + sign_in(user) + end + + it "#index" do + get group_runners_path(group) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "#show" do + get group_runners_path(group, runner) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "#new" do + get new_group_runner_path(group) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "#register" do + get register_group_runner_path(group, runner) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "#edit" do + get edit_group_runner_path(group, runner) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "#update" do + put group_runner_path(group, runner), params: { + runner: { + description: "example" + } + } + + expect(response).to have_gitlab_http_status(:not_found) + end + end +end -- GitLab From 4a0112afbcb1234f0ffaa47e8c9e8bd7852f999d Mon Sep 17 00:00:00 2001 From: Kev Kloss Date: Thu, 20 Jun 2024 18:44:58 +0200 Subject: [PATCH 3/6] Remove available_from_access_level from read_runners --- ee/config/custom_abilities/read_runners.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/ee/config/custom_abilities/read_runners.yml b/ee/config/custom_abilities/read_runners.yml index 3e3efdfda83118..a5fed618b5c58d 100644 --- a/ee/config/custom_abilities/read_runners.yml +++ b/ee/config/custom_abilities/read_runners.yml @@ -8,4 +8,3 @@ feature_category: runner milestone: '17.2' group_ability: true project_ability: true -available_from_access_level: 40 -- GitLab From d23abdbbf1f1b0f16060a9671912b33b0a5088a8 Mon Sep 17 00:00:00 2001 From: Kev Kloss Date: Thu, 20 Jun 2024 18:57:11 +0200 Subject: [PATCH 4/6] Allow viewing project runners with read_runners ability --- .../projects/runners_controller.rb | 3 +- app/policies/project_policy.rb | 1 + ee/app/policies/ee/project_policy.rb | 3 + ee/spec/policies/project_policy_spec.rb | 5 +- .../custom_roles/read_runners/request_spec.rb | 58 +++++++++++++++++++ 5 files changed, 67 insertions(+), 3 deletions(-) diff --git a/app/controllers/projects/runners_controller.rb b/app/controllers/projects/runners_controller.rb index bbdd27a7184eeb..3fd27f078fc3bb 100644 --- a/app/controllers/projects/runners_controller.rb +++ b/app/controllers/projects/runners_controller.rb @@ -1,7 +1,8 @@ # frozen_string_literal: true class Projects::RunnersController < Projects::ApplicationController - before_action :authorize_admin_runner! + before_action :authorize_read_runner! + before_action :authorize_admin_runner!, except: [:index, :show] before_action :authorize_create_runner!, only: [:new, :register] before_action :runner, only: [:edit, :update, :destroy, :pause, :resume, :show, :register] diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index 2c829bce3ee1e8..91ccdfe8f5dd2d 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -622,6 +622,7 @@ class ProjectPolicy < BasePolicy end rule { can?(:admin_build) }.enable :manage_trigger + rule { can?(:admin_runner) }.enable :read_runner rule { public_project & metrics_dashboard_allowed }.policy do enable :metrics_dashboard diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 3f5b23f88b56dc..efedf1ceaa2baf 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -304,8 +304,11 @@ module ProjectPolicy enable :create_runner end + rule { can?(:admin_runner) }.enable :read_runner + rule { custom_role_enables_read_runners }.policy do enable :read_project_runners + enable :read_runner end condition(:ci_cancellation_maintainers_only, scope: :subject) do diff --git a/ee/spec/policies/project_policy_spec.rb b/ee/spec/policies/project_policy_spec.rb index 2b14bd6c8b6038..edc6aa07cf0629 100644 --- a/ee/spec/policies/project_policy_spec.rb +++ b/ee/spec/policies/project_policy_spec.rb @@ -2778,7 +2778,8 @@ def create_member_role(member, abilities = member_role_abilities) let(:allowed_abilities) do [ :admin_runner, - :create_runner + :create_runner, + :read_runner ] end @@ -3008,7 +3009,7 @@ def create_member_role(member, abilities = member_role_abilities) context 'for a custom role with the `read_runners` ability' do let(:member_role_abilities) { { read_runners: true } } - let(:allowed_abilities) { [:read_project_runners] } + let(:allowed_abilities) { [:read_project_runners, :read_runner] } it_behaves_like 'custom roles abilities' end diff --git a/ee/spec/requests/custom_roles/read_runners/request_spec.rb b/ee/spec/requests/custom_roles/read_runners/request_spec.rb index aaf52a07b0f70e..c51804baef694f 100644 --- a/ee/spec/requests/custom_roles/read_runners/request_spec.rb +++ b/ee/spec/requests/custom_roles/read_runners/request_spec.rb @@ -63,4 +63,62 @@ expect(response).to have_gitlab_http_status(:not_found) end end + + describe Projects::RunnersController do + let_it_be(:membership) { create(:project_member, :guest, member_role: role, user: user, source: project) } + + before do + sign_in(user) + end + + it "#index" do + get project_runners_path(project) + + expect(response).to redirect_to(project_settings_ci_cd_path(project, anchor: 'js-runners-settings')) + end + + it "#new" do + get new_project_runner_path(project) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "#update" do + runner = create(:ci_runner, :project, active: true, projects: [project]) + + patch project_runner_path(project, runner), params: { runner: { description: "hello world" } } + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "#toggle_shared_runners" do + post toggle_shared_runners_project_runners_path(project) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "#register" do + runner = create(:ci_runner, :project, projects: [project], registration_type: :authenticated_user) + + get register_namespace_project_runner_path(group, project, runner) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "#destroy" do + runner = create(:ci_runner, :project, projects: [project]) + + delete project_runner_path(project, runner) + + expect(response).to have_gitlab_http_status(:not_found) + end + + it "#pause" do + runner = create(:ci_runner, :project, active: true, projects: [project]) + + post pause_project_runner_path(project, runner) + + expect(response).to have_gitlab_http_status(:not_found) + end + end end -- GitLab From 267971bbcbb0c04f9d42e0216d5a624dc1fa233a Mon Sep 17 00:00:00 2001 From: Kev Kloss Date: Thu, 20 Jun 2024 19:05:45 +0200 Subject: [PATCH 5/6] Test read_runners in runner_policy_spec --- ee/spec/policies/ee/ci/runner_policy_spec.rb | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/ee/spec/policies/ee/ci/runner_policy_spec.rb b/ee/spec/policies/ee/ci/runner_policy_spec.rb index 68619f12116f0d..b7e9e761c40c32 100644 --- a/ee/spec/policies/ee/ci/runner_policy_spec.rb +++ b/ee/spec/policies/ee/ci/runner_policy_spec.rb @@ -60,8 +60,9 @@ stub_licensed_features(custom_roles: true) end - where(:custom_permission, :abilities) do - :admin_runners | [:assign_runner, :read_runner, :update_runner, :delete_runner] + where(:custom_permission, :feature_flag, :abilities) do + :admin_runners | true | [:assign_runner, :read_runner, :update_runner, :delete_runner] + :read_runners | false | [:read_runner] end with_them do @@ -82,7 +83,10 @@ stub_feature_flags("custom_ability_#{custom_permission}": false) end - it { expect_disallowed(*abilities) } + it do + skip "feature flag 'custom_ability_#{custom_permission}' does not exists" unless feature_flag + expect_disallowed(*abilities) + end end context "with the custom roles feature disabled" do -- GitLab From 795fd9eb8d3271076774d283dcb8f2d01df83113 Mon Sep 17 00:00:00 2001 From: Kev Kloss Date: Mon, 24 Jun 2024 15:40:12 +0000 Subject: [PATCH 6/6] Fix typo --- ee/spec/policies/ee/ci/runner_policy_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ee/spec/policies/ee/ci/runner_policy_spec.rb b/ee/spec/policies/ee/ci/runner_policy_spec.rb index b7e9e761c40c32..be09c07bf0cd0a 100644 --- a/ee/spec/policies/ee/ci/runner_policy_spec.rb +++ b/ee/spec/policies/ee/ci/runner_policy_spec.rb @@ -84,7 +84,7 @@ end it do - skip "feature flag 'custom_ability_#{custom_permission}' does not exists" unless feature_flag + skip "feature flag 'custom_ability_#{custom_permission}' does not exist" unless feature_flag expect_disallowed(*abilities) end end -- GitLab