diff --git a/db/migrate/20240131123824_add_admin_cicd_variables_to_member_roles.rb b/db/migrate/20240131123824_add_admin_cicd_variables_to_member_roles.rb
new file mode 100644
index 0000000000000000000000000000000000000000..b4daa35e91a9735649cdcfdcb3a0fd338c7b5a3a
--- /dev/null
+++ b/db/migrate/20240131123824_add_admin_cicd_variables_to_member_roles.rb
@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class AddAdminCicdVariablesToMemberRoles < Gitlab::Database::Migration[2.2]
+ milestone '16.9'
+
+ enable_lock_retries!
+
+ def up
+ add_column :member_roles, :admin_cicd_variables, :boolean, default: false, null: false
+ end
+
+ def down
+ remove_column :member_roles, :admin_cicd_variables
+ end
+end
diff --git a/db/schema_migrations/20240131123824 b/db/schema_migrations/20240131123824
new file mode 100644
index 0000000000000000000000000000000000000000..2f0ca9bd23f6b34efea162d10f213c90d65b0267
--- /dev/null
+++ b/db/schema_migrations/20240131123824
@@ -0,0 +1 @@
+0720a5a4ff3738f1ce591ff7ada103b94ed584614757521d448b5cdb1fc983be
\ No newline at end of file
diff --git a/db/structure.sql b/db/structure.sql
index 2defc153e855e73f6a26c7cda2fab90ef541aef4..08175f3c1009deaa393cff3cde45b76769685607 100644
--- a/db/structure.sql
+++ b/db/structure.sql
@@ -19180,6 +19180,7 @@ CREATE TABLE member_roles (
manage_group_access_tokens boolean DEFAULT false NOT NULL,
remove_project boolean DEFAULT false NOT NULL,
admin_terraform_state boolean DEFAULT false NOT NULL,
+ admin_cicd_variables boolean DEFAULT false NOT NULL,
CONSTRAINT check_4364846f58 CHECK ((char_length(description) <= 255)),
CONSTRAINT check_9907916995 CHECK ((char_length(name) <= 255))
);
diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md
index 9a2b02d6edeb45be2183638526dde1f2c93159c6..a1e6e97680253857f4ea1267d1787275cfacde46 100644
--- a/doc/api/graphql/reference/index.md
+++ b/doc/api/graphql/reference/index.md
@@ -31526,6 +31526,7 @@ Member role permission.
| Value | Description |
| ----- | ----------- |
+| `ADMIN_CICD_VARIABLES` | Allows to admin CI/CD variables. |
| `ADMIN_GROUP_MEMBER` | Allows admin of group members. |
| `ADMIN_MERGE_REQUEST` | Allows approval of merge requests. |
| `ADMIN_TERRAFORM_STATE` | Allows to admin terraform state. |
diff --git a/ee/app/graphql/ee/types/query_type.rb b/ee/app/graphql/ee/types/query_type.rb
index 79f477f95d1cca7a3bcfcf6a345174fad0c464e0..a0c4825c4f84d051677fa0bc9cddc06fe96520b4 100644
--- a/ee/app/graphql/ee/types/query_type.rb
+++ b/ee/app/graphql/ee/types/query_type.rb
@@ -191,7 +191,7 @@ def ci_minutes_usage(namespace_id: nil, date: nil)
end
def member_role_permissions
- MemberRole.all_customizable_permissions.keys
+ MemberRole.all_customizable_permissions.keys.filter { |perm| ::MemberRole.permission_enabled?(perm) }
end
private
diff --git a/ee/app/models/auth/member_role_ability_loader.rb b/ee/app/models/auth/member_role_ability_loader.rb
index d91560bf2704bdd5b99227c14f9b39285ab4d6d6..f6802b99479ffb651a9556ea8b6bd6f7393c87f8 100644
--- a/ee/app/models/auth/member_role_ability_loader.rb
+++ b/ee/app/models/auth/member_role_ability_loader.rb
@@ -10,6 +10,7 @@ def initialize(user:, resource:, ability:)
def has_ability?
return false unless user.is_a?(User)
+ return false unless permission_enabled?
roles = if resource.is_a?(::Project)
preloaded_member_roles_for_project[resource.id]
@@ -24,6 +25,10 @@ def has_ability?
attr_reader :user, :resource, :ability
+ def permission_enabled?
+ ::MemberRole.permission_enabled?(ability)
+ end
+
def preloaded_member_roles_for_project
::Preloaders::UserMemberRolesInProjectsPreloader.new(
projects: [resource],
diff --git a/ee/app/models/members/member_role.rb b/ee/app/models/members/member_role.rb
index 091764e00ac1f98895459e0bb192afddde2a5a65..9727b47172029bd7e9998b1481b88d3ef197f76d 100644
--- a/ee/app/models/members/member_role.rb
+++ b/ee/app/models/members/member_role.rb
@@ -84,10 +84,18 @@ def all_customizable_group_permissions
def customizable_permissions_exempt_from_consuming_seat
MemberRole.all_customizable_permissions.select { |_k, v| v[:skip_seat_consumption] }.keys
end
+
+ def permission_enabled?(permission)
+ return true unless ::Feature::Definition.get("custom_ability_#{permission}")
+
+ ::Feature.enabled?("custom_ability_#{permission}")
+ end
end
def enabled_permissions
- MemberRole.all_customizable_permissions.keys.filter { |perm| attributes[perm.to_s] }
+ MemberRole.all_customizable_permissions.keys.filter do |permission|
+ attributes[permission.to_s] && self.class.permission_enabled?(permission)
+ end
end
private
diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb
index 48b1e7b24c7d6d3c60e993dc8bb7907c689e0afa..748798f9164b1dba2b46d829e76ea7d514222625 100644
--- a/ee/app/policies/ee/group_policy.rb
+++ b/ee/app/policies/ee/group_policy.rb
@@ -243,6 +243,15 @@ module GroupPolicy
).has_ability?
end
+ desc 'Custom role on group that enables admin CI/CD variables'
+ condition(:role_enables_admin_cicd_variables) do
+ ::Auth::MemberRoleAbilityLoader.new(
+ user: @user,
+ resource: @subject,
+ ability: :admin_cicd_variables
+ ).has_ability?
+ end
+
rule { owner & unique_project_download_limit_enabled }.policy do
enable :ban_group_member
end
@@ -548,6 +557,10 @@ module GroupPolicy
enable :admin_member_role
end
+ rule { custom_roles_allowed & role_enables_admin_cicd_variables }.policy do
+ enable :admin_cicd_variables
+ end
+
rule { can?(:read_group_security_dashboard) }.policy do
enable :create_vulnerability_export
enable :read_security_resource
diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb
index 2300ef5b7d83edf3f1ba304359dbb4bebef26cff..59884346607a31dbd2246039c9f7a8067a5b0158 100644
--- a/ee/app/policies/ee/project_policy.rb
+++ b/ee/app/policies/ee/project_policy.rb
@@ -299,6 +299,15 @@ module ProjectPolicy
).has_ability?
end
+ desc 'Custom role on project that enables admin CI/CD variables'
+ condition(:role_enables_admin_cicd_variables) do
+ ::Auth::MemberRoleAbilityLoader.new(
+ user: @user,
+ resource: @subject,
+ ability: :admin_cicd_variables
+ ).has_ability?
+ end
+
condition(:developer_access_to_admin_vulnerability) do
::Feature.disabled?(:disable_developer_access_to_admin_vulnerability, subject&.root_namespace) &&
can?(:developer_access)
@@ -365,6 +374,10 @@ module ProjectPolicy
(custom_roles_allowed? && merge_requests_is_a_private_feature? && role_enables_admin_merge_request?))
end
+ rule { custom_roles_allowed & role_enables_admin_cicd_variables }.policy do
+ enable :admin_cicd_variables
+ end
+
condition(:ci_cancellation_maintainers_only, scope: :subject) do
project.ci_cancellation_restriction.maintainers_only_allowed?
end
diff --git a/ee/config/custom_abilities/admin_cicd_variables.yml b/ee/config/custom_abilities/admin_cicd_variables.yml
new file mode 100644
index 0000000000000000000000000000000000000000..0c64af12c5f8c88f4f755c2320f7c6a398ab9352
--- /dev/null
+++ b/ee/config/custom_abilities/admin_cicd_variables.yml
@@ -0,0 +1,11 @@
+---
+name: admin_cicd_variables
+description: Allows to admin CI/CD variables.
+introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/437947
+introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143369
+feature_category: secrets_management
+milestone: '16.9'
+group_ability: true
+project_ability: true
+requirements: []
+available_from_access_level:
diff --git a/ee/config/feature_flags/wip/custom_ability_admin_cicd_variables.yml b/ee/config/feature_flags/wip/custom_ability_admin_cicd_variables.yml
new file mode 100644
index 0000000000000000000000000000000000000000..211ecb778e3dd40bfca879318f219015745a9271
--- /dev/null
+++ b/ee/config/feature_flags/wip/custom_ability_admin_cicd_variables.yml
@@ -0,0 +1,9 @@
+---
+name: custom_ability_admin_cicd_variables
+feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/437947
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/143369
+rollout_issue_url:
+milestone: "16.9"
+group: group::authorization
+type: wip
+default_enabled: false
diff --git a/ee/spec/models/auth/member_role_ability_loader_spec.rb b/ee/spec/models/auth/member_role_ability_loader_spec.rb
index ce759d31832c60dba544aea0fb570ff12eaf42b0..b466461e71ee55534506c1067323ec4f659a35dc 100644
--- a/ee/spec/models/auth/member_role_ability_loader_spec.rb
+++ b/ee/spec/models/auth/member_role_ability_loader_spec.rb
@@ -89,6 +89,20 @@
).has_ability?).to be false
end
end
+
+ context 'when the permission is disabled' do
+ before do
+ allow(::MemberRole).to receive(:permission_enabled?).with(:read_vulnerability).and_return(false)
+ end
+
+ it 'returns false' do
+ expect(described_class.new(
+ user: user,
+ resource: group,
+ ability: :read_vulnerability
+ ).has_ability?).to be false
+ end
+ end
end
context 'when custom role is for a project' do
diff --git a/ee/spec/models/ee/user_spec.rb b/ee/spec/models/ee/user_spec.rb
index 72a4f531b15d6f1e99a06178055cecac64fad35d..4eaed875aa6a5ce2b143d0dd7c9a3893778c668c 100644
--- a/ee/spec/models/ee/user_spec.rb
+++ b/ee/spec/models/ee/user_spec.rb
@@ -1263,7 +1263,8 @@
WHERE "members"."user_id" = "users"."id"
AND \(members.access_level > 10
OR "members"."access_level" = 10
- AND \(admin_group_member = true
+ AND \(admin_cicd_variables = true
+ OR admin_group_member = true
OR admin_merge_request = true
OR admin_terraform_state = true
OR admin_vulnerability = true
diff --git a/ee/spec/models/members/member_role_spec.rb b/ee/spec/models/members/member_role_spec.rb
index 5368231fba45a274f661612dc9aa2aeaa4199457..4780239065ff4662fd41b4077a37777708654b51 100644
--- a/ee/spec/models/members/member_role_spec.rb
+++ b/ee/spec/models/members/member_role_spec.rb
@@ -3,6 +3,8 @@
require 'spec_helper'
RSpec.describe ::MemberRole, feature_category: :system_access do
+ using RSpec::Parameterized::TableSyntax
+
describe 'associations' do
it { is_expected.to belong_to(:namespace) }
it { is_expected.to have_many(:members) }
@@ -268,6 +270,29 @@
end
end
+ describe '.permission_enabled?' do
+ let(:ability) { :my_custom_ability }
+
+ subject { described_class.permission_enabled?(ability) }
+
+ where(:flag_exists, :flag_enabled, :expected_result) do
+ true | false | false
+ true | true | true
+ false | true | true
+ end
+
+ with_them do
+ before do
+ if flag_exists
+ stub_feature_flag_definition("custom_ability_#{ability}")
+ stub_feature_flags("custom_ability_#{ability}" => flag_enabled)
+ end
+ end
+
+ it { is_expected.to eq(expected_result) }
+ end
+ end
+
describe 'covering all permissions columns' do
it 'has all attributes listed in the member_roles table' do
expect(described_class.attribute_names.map(&:to_sym))
@@ -277,11 +302,22 @@
end
describe '#enabled_permissions' do
- it 'returns the list of enabled abilities' do
- member_role = build_stubbed(:member_role, read_code: true, read_vulnerability: true, read_dependency: false)
+ let(:member_role) { build_stubbed(:member_role, read_code: true, read_vulnerability: true, read_dependency: false) }
+ it 'returns the list of enabled abilities' do
expect(member_role.enabled_permissions).to match_array([:read_code, :read_vulnerability])
end
+
+ context 'when a permission is behind a disabled feature flag' do
+ before do
+ stub_feature_flag_definition(:custom_ability_read_vulnerability)
+ stub_feature_flags(custom_ability_read_vulnerability: false)
+ end
+
+ it 'does not include the ability' do
+ expect(member_role.enabled_permissions).not_to include(:read_vulnerability)
+ end
+ end
end
shared_examples 'ability with the correct `available_from_access_level` attribute' do |policy_class|
diff --git a/ee/spec/policies/group_policy_spec.rb b/ee/spec/policies/group_policy_spec.rb
index d3bf86019919868a698580d0ce9c65994a2a74ae..9ae75c364ee1fe36fc96fe4a2e14514222328661 100644
--- a/ee/spec/policies/group_policy_spec.rb
+++ b/ee/spec/policies/group_policy_spec.rb
@@ -3277,6 +3277,13 @@ def create_member_role(member, abilities = member_role_abilities)
it { is_expected.to be_disallowed(*allowed_abilities) }
end
end
+
+ context 'for a custom role with the `admin_cicd_variables` ability' do
+ let(:member_role_abilities) { { admin_cicd_variables: true } }
+ let(:allowed_abilities) { [:admin_cicd_variables] }
+
+ 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 5dcd6a93383f3193bd2173002e3b047c4ab29b61..c9f22c73cfcdc6c6b437eb818c0a2eba95534eb4 100644
--- a/ee/spec/policies/project_policy_spec.rb
+++ b/ee/spec/policies/project_policy_spec.rb
@@ -2776,6 +2776,13 @@ def create_member_role(member, abilities = member_role_abilities)
it { is_expected.to be_disallowed(:read_dependency) }
it { is_expected.to be_allowed(:read_code) }
end
+
+ context 'for a custom role with the `admin_cicd_variables` ability' do
+ let(:member_role_abilities) { { admin_cicd_variables: true } }
+ let(:allowed_abilities) { [:admin_cicd_variables] }
+
+ it_behaves_like 'custom roles abilities'
+ end
end
describe 'permissions for suggested reviewers bot', :saas do
diff --git a/ee/spec/requests/api/member_roles_spec.rb b/ee/spec/requests/api/member_roles_spec.rb
index f4affed98450526be3f6c0903ed7f7254056a688..b93047ff302dd7ecb07110fee072ad2ecc8cd94d 100644
--- a/ee/spec/requests/api/member_roles_spec.rb
+++ b/ee/spec/requests/api/member_roles_spec.rb
@@ -110,6 +110,7 @@
"manage_project_access_tokens" => false,
"archive_project" => false,
"remove_project" => false,
+ "admin_cicd_variables" => false,
"group_id" => group_id
},
{
@@ -128,6 +129,7 @@
"manage_project_access_tokens" => false,
"archive_project" => false,
"remove_project" => false,
+ "admin_cicd_variables" => false,
"group_id" => group_id
}
]