diff --git a/ee/app/models/authz/custom_ability/definition.rb b/ee/app/models/authz/custom_ability/definition.rb
index db2dec610209b960fdccbce0fdc90e5cb441ccca..2b146e57f78abd2c8757c613095f2b7aecc26d42 100644
--- a/ee/app/models/authz/custom_ability/definition.rb
+++ b/ee/app/models/authz/custom_ability/definition.rb
@@ -27,7 +27,9 @@ def project_ability_enabled?
end
def admin_ability_enabled?
- attributes.fetch(:admin_ability, false)
+ return false if group_ability_enabled? || project_ability_enabled?
+
+ exists?
end
private
diff --git a/ee/app/models/members/member_role.rb b/ee/app/models/members/member_role.rb
index 3dd9d8e7bf1f6c55f5c7a76ef8b7fbda07e5d8a4..a8d7a3257c05b151dfec219c19bc14501e910ce6 100644
--- a/ee/app/models/members/member_role.rb
+++ b/ee/app/models/members/member_role.rb
@@ -93,23 +93,23 @@ def all_customizable_permissions
end
def all_customizable_standard_permissions
- MemberRole.all_customizable_permissions.select { |_k, v| !v[:admin_ability] }
+ Gitlab::CustomRoles::Definition.standard
end
def all_customizable_admin_permissions
- MemberRole.all_customizable_permissions.select { |_k, v| v[:admin_ability] }
+ Gitlab::CustomRoles::Definition.admin
end
def all_customizable_project_permissions
- MemberRole.all_customizable_permissions.select { |_k, v| v[:project_ability] }.keys
+ MemberRole.all_customizable_standard_permissions.select { |_k, v| v[:project_ability] }.keys
end
def all_customizable_group_permissions
- MemberRole.all_customizable_permissions.select { |_k, v| v[:group_ability] }.keys
+ MemberRole.all_customizable_standard_permissions.select { |_k, v| v[:group_ability] }.keys
end
def all_customizable_admin_permission_keys
- MemberRole.all_customizable_permissions.select { |_k, v| v[:admin_ability] }.keys
+ Gitlab::CustomRoles::Definition.admin.keys
end
def customizable_permissions_exempt_from_consuming_seat
diff --git a/ee/config/custom_abilities/read_admin_cicd.yml b/ee/config/custom_abilities/admin/read_admin_cicd.yml
similarity index 78%
rename from ee/config/custom_abilities/read_admin_cicd.yml
rename to ee/config/custom_abilities/admin/read_admin_cicd.yml
index cc8a510be64d98ba487459cd6be402ccf9e716f8..f50c707a41337237e56c59ae2c5e2fe4a981f835 100644
--- a/ee/config/custom_abilities/read_admin_cicd.yml
+++ b/ee/config/custom_abilities/admin/read_admin_cicd.yml
@@ -6,7 +6,3 @@ introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/507960
introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/177233
feature_category: admin
milestone: '17.9'
-group_ability: false
-project_ability: false
-admin_ability: true
-requirements: []
diff --git a/ee/config/custom_abilities/read_admin_dashboard.yml b/ee/config/custom_abilities/admin/read_admin_dashboard.yml
similarity index 79%
rename from ee/config/custom_abilities/read_admin_dashboard.yml
rename to ee/config/custom_abilities/admin/read_admin_dashboard.yml
index c267b2ce6590cbfbbdd910ad1cb0e3c249ae92a5..43a6c9c433960a505ae0e2712c4798325c5c5b68 100644
--- a/ee/config/custom_abilities/read_admin_dashboard.yml
+++ b/ee/config/custom_abilities/admin/read_admin_dashboard.yml
@@ -6,7 +6,3 @@ introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/501549
introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/171581
feature_category: admin
milestone: '17.6'
-group_ability: false
-project_ability: false
-admin_ability: true
-requirements: []
diff --git a/ee/config/custom_abilities/read_admin_monitoring.yml b/ee/config/custom_abilities/admin/read_admin_monitoring.yml
similarity index 83%
rename from ee/config/custom_abilities/read_admin_monitoring.yml
rename to ee/config/custom_abilities/admin/read_admin_monitoring.yml
index f981229e153d3f083a161d36e03e726f66e82a65..b8b4bfbdef5ee772c12b968fb51b1a9cc2c7a243 100644
--- a/ee/config/custom_abilities/read_admin_monitoring.yml
+++ b/ee/config/custom_abilities/admin/read_admin_monitoring.yml
@@ -6,7 +6,3 @@ introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/507959
introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/179439
feature_category: admin
milestone: '17.9'
-group_ability: false
-project_ability: false
-admin_ability: true
-requirements: []
diff --git a/ee/config/custom_abilities/read_admin_subscription.yml b/ee/config/custom_abilities/admin/read_admin_subscription.yml
similarity index 79%
rename from ee/config/custom_abilities/read_admin_subscription.yml
rename to ee/config/custom_abilities/admin/read_admin_subscription.yml
index 84384cf778c746bbab0f7022bd5afff9dfa30b6e..263a3dbcd7e3d4c5836155f875771db072cf7fe4 100644
--- a/ee/config/custom_abilities/read_admin_subscription.yml
+++ b/ee/config/custom_abilities/admin/read_admin_subscription.yml
@@ -6,7 +6,3 @@ introduced_by_issue: https://gitlab.com/gitlab-org/gitlab/-/issues/507961
introduced_by_mr: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/178230
feature_category: admin
milestone: '17.9'
-admin_ability: true
-group_ability: false
-project_ability: false
-requirements: []
diff --git a/ee/config/custom_abilities/admin/type_schema.json b/ee/config/custom_abilities/admin/type_schema.json
new file mode 100644
index 0000000000000000000000000000000000000000..88ea91f3c2829baa06f7d34f286707aada09e739
--- /dev/null
+++ b/ee/config/custom_abilities/admin/type_schema.json
@@ -0,0 +1,55 @@
+{
+ "$schema": "https://json-schema.org/draft/2020-12/schema",
+ "$id": "https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/config/custom_abilities/type_schema.json",
+ "type": "object",
+ "additionalProperties": false,
+ "properties": {
+ "title": {
+ "type": "string",
+ "description": "Human readable title for the custom ability"
+ },
+ "name": {
+ "type": "string",
+ "description": "Unique name for the custom ability"
+ },
+ "description": {
+ "type": "string",
+ "description": "Human readable description of this custom ability"
+ },
+ "introduced_by_issue": {
+ "type": "string",
+ "format": "uri",
+ "description": "URL to GitLab issue that added this custom ability",
+ "qt-uri-protocols": [
+ "https"
+ ]
+ },
+ "introduced_by_mr": {
+ "type": "string",
+ "format": "uri",
+ "description": "URL to GitLab merge request that added this custom ability",
+ "qt-uri-protocols": [
+ "https"
+ ]
+ },
+ "feature_category": {
+ "type": "string",
+ "description": "The feature category of this this custom ability. For example, vulnerability_management"
+ },
+ "milestone": {
+ "type": "string",
+ "description": "Milestone that introduced this custom ability. For example, 15.8",
+ "pattern": "\\A[0-9]+\\.[0-9]+\\z"
+ }
+ },
+ "required": [
+ "description",
+ "feature_category",
+ "introduced_by_issue",
+ "introduced_by_mr",
+ "milestone",
+ "name",
+ "title"
+ ],
+ "title": "GitLabCustomPermission"
+}
diff --git a/ee/config/custom_abilities/type_schema.json b/ee/config/custom_abilities/type_schema.json
index bc101704ba9c95b1cb870ca13bca2359689264a4..3428732cfb490fc3f776d2f71087aaccfa8b2f15 100644
--- a/ee/config/custom_abilities/type_schema.json
+++ b/ee/config/custom_abilities/type_schema.json
@@ -10,11 +10,11 @@
},
"name": {
"type": "string",
- "description": "Unique identifying name for this custom ability"
+ "description": "Unique name for the custom ability"
},
"description": {
"type": "string",
- "description": "A human-readable description of this custom ability"
+ "description": "Human readable description of this custom ability"
},
"introduced_by_issue": {
"type": "string",
@@ -39,11 +39,7 @@
"milestone": {
"type": "string",
"description": "Milestone that introduced this custom ability. For example, 15.8",
- "pattern": "^[0-9]+\\.[0-9]+$"
- },
- "admin_ability": {
- "type": "boolean",
- "description": "Indicate whether this ability is checked at the admin level."
+ "pattern": "\\A[0-9]+\\.[0-9]+\\z"
},
"group_ability": {
"type": "boolean",
@@ -154,11 +150,6 @@
"enum": [
false
]
- },
- "admin_ability": {
- "enum": [
- false
- ]
}
}
},
diff --git a/ee/lib/gitlab/custom_roles/definition.rb b/ee/lib/gitlab/custom_roles/definition.rb
index 9adc9d6758df059d66ef8267e9207831a91737af..314fdf43eeb9762082add89f9bd8a6654bc03fbe 100644
--- a/ee/lib/gitlab/custom_roles/definition.rb
+++ b/ee/lib/gitlab/custom_roles/definition.rb
@@ -9,26 +9,43 @@ class << self
attr_accessor :definitions
def all
- load_abilities! if @definitions.nil?
+ standard.merge(admin)
+ end
+
+ def admin
+ @admin_definitions ||= load_definitions(admin_path)
+ end
- @definitions
+ def standard
+ @standard_definitions ||= load_definitions(standard_path)
end
def load_abilities!
- @definitions = {}
+ @standard_definitions = load_definitions(standard_path)
+ @admin_definitions = load_definitions(admin_path)
+ end
+
+ private
+
+ def standard_path
+ Rails.root.join("ee/config/custom_abilities/*.yml")
+ end
+
+ def admin_path
+ Rails.root.join("ee/config/custom_abilities/admin/*.yml")
+ end
+
+ def load_definitions(path)
+ definitions = {}
Dir.glob(path).each do |file|
definition = load_from_file(file)
- name = definition[:name].to_sym
- @definitions[name] = definition
+ name = definition[:name].to_sym
+ definitions[name] = definition
end
- end
-
- private
- def path
- Rails.root.join("ee/config/custom_abilities/*.yml")
+ definitions
end
def load_from_file(path)
diff --git a/ee/spec/factories/member_roles.rb b/ee/spec/factories/member_roles.rb
index 6b3b0f06271631e1b7e24c23e58a1bd21749d4cc..87b0840ef5281439c2204a36f5ba7f77684d232d 100644
--- a/ee/spec/factories/member_roles.rb
+++ b/ee/spec/factories/member_roles.rb
@@ -12,18 +12,22 @@
trait(role) { base_access_level { value } }
end
- Gitlab::CustomRoles::Definition.all.each_value do |attributes|
+ Gitlab::CustomRoles::Definition.standard.each_value do |attributes|
trait attributes[:name].to_sym do
send(attributes[:name].to_sym) { true }
attributes.fetch(:requirements, []).each do |requirement|
send(requirement.to_sym) { true }
end
+ end
+ end
- if attributes[:admin_ability]
- namespace { nil }
- base_access_level { nil }
- end
+ Gitlab::CustomRoles::Definition.admin.each_value do |attributes|
+ trait attributes[:name].to_sym do
+ send(attributes[:name].to_sym) { true }
+
+ namespace { nil }
+ base_access_level { nil }
end
end
diff --git a/ee/spec/lib/gitlab/custom_roles/definition_spec.rb b/ee/spec/lib/gitlab/custom_roles/definition_spec.rb
index c9b9311ac92c2d65d0bb84dce19eeed50d61cf08..7f8b1c67efc2d56b05767cbd597da1f80d4aee1f 100644
--- a/ee/spec/lib/gitlab/custom_roles/definition_spec.rb
+++ b/ee/spec/lib/gitlab/custom_roles/definition_spec.rb
@@ -3,60 +3,106 @@
require 'spec_helper'
RSpec.describe Gitlab::CustomRoles::Definition, feature_category: :permissions do
- yaml_files = Dir.glob(Rails.root.join("ee/config/custom_abilities/*.yml"))
+ standard_yaml_files = Dir.glob(Rails.root.join("ee/config/custom_abilities/*.yml"))
+ admin_yaml_files = Dir.glob(Rails.root.join("ee/config/custom_abilities/admin/*.yml"))
+
+ def defined_abilities(files)
+ files.map { |file| File.basename(file, '.yml').to_sym }
+ end
+
+ let_it_be(:defined_standard_abilities) { defined_abilities(standard_yaml_files) }
+ let_it_be(:defined_admin_abilities) { defined_abilities(admin_yaml_files) }
+ let_it_be(:all_defined_abilities) { defined_standard_abilities + defined_admin_abilities }
describe '.all' do
subject(:abilities) { described_class.all }
- let_it_be(:defined_abilities) do
- yaml_files.map do |file|
- File.basename(file, '.yml').to_sym
- end
+ it 'returns the defined abilities' do
+ expect(abilities.keys).to match_array(all_defined_abilities)
end
- context 'when initialized' do
- it 'does not reload the abilities from the yaml files' do
- expect(described_class).not_to receive(:load_abilities!)
-
- abilities
- end
+ it 'does not have overlapping role names' do
+ admin_abilities = described_class.admin.keys
+ standard_abilities = described_class.standard.keys
- it 'returns the defined abilities' do
- expect(abilities.keys).to match_array(defined_abilities)
- end
+ expect(standard_abilities & admin_abilities).to be_empty
end
context 'when not initialized' do
before do
- described_class.instance_variable_set(:@definitions, nil)
+ described_class.instance_variable_set(:@standard_definitions, nil)
+ described_class.instance_variable_set(:@admin_definitions, nil)
end
it 'reloads the abilities from the yaml files' do
- expect(described_class).to receive(:load_abilities!)
+ expect(described_class).to receive(:load_definitions)
+ .with(described_class.send(:standard_path)).and_call_original
+ expect(described_class).to receive(:load_definitions)
+ .with(described_class.send(:admin_path)).and_call_original
abilities
end
it 'returns the defined abilities' do
- expect(abilities.keys).to match_array(defined_abilities)
+ expect(abilities.keys).to match_array(all_defined_abilities)
end
end
end
+ describe '.admin' do
+ subject(:abilities) { described_class.admin }
+
+ it 'returns the defined abilities' do
+ expect(abilities.keys).to match_array(defined_admin_abilities)
+ end
+ end
+
+ describe '.standard' do
+ subject(:abilities) { described_class.standard }
+
+ it 'returns the defined abilities' do
+ expect(abilities.keys).to match_array(defined_standard_abilities)
+ end
+ end
+
+ describe '.load_abilities!' do
+ before do
+ described_class.instance_variable_set(:@standard_definitions, { old: 'ability' })
+ described_class.instance_variable_set(:@admin_definitions, { old: 'ability' })
+ end
+
+ it 'returns the defined abilities' do
+ expect { described_class.load_abilities! }.to change { described_class.admin.keys }
+ .from(%i[old]).to(defined_admin_abilities).and change { described_class.standard.keys }
+ .from(%i[old]).to(defined_standard_abilities)
+ end
+ end
+
describe 'validations' do
- def validate(data)
+ let(:validator) { JSONSchemer.schema(Pathname.new(Rails.root.join(validator_path))) }
+
+ def validate(ability_file)
+ data = YAML.load_file(ability_file)
validator.validate(data).pluck('error')
end
- let_it_be(:validator) do
- JSONSchemer.schema(Pathname.new(Rails.root.join('ee/config/custom_abilities/type_schema.json')))
+ describe 'for standard abilities' do
+ let(:validator_path) { 'ee/config/custom_abilities/type_schema.json' }
+
+ standard_yaml_files.each do |ability_file|
+ it "validates #{ability_file}" do
+ expect(validate(ability_file)).to be_empty
+ end
+ end
end
- yaml_files.each do |ability_file|
- it "validates #{ability_file}" do
- data = YAML.load_file(ability_file)
+ describe 'for admin abilities' do
+ let(:validator_path) { 'ee/config/custom_abilities/admin/type_schema.json' }
- expect(validate(data)).to be_empty
+ admin_yaml_files.each do |ability_file|
+ it "validates #{ability_file}" do
+ expect(validate(ability_file)).to be_empty
+ end
end
end
end
diff --git a/ee/spec/models/authz/custom_ability/definition_spec.rb b/ee/spec/models/authz/custom_ability/definition_spec.rb
index f81ef16473c0458ab2b0c82cfedfd48595d5dfcc..0954336609efbafbeee240655348b8c0029dda94 100644
--- a/ee/spec/models/authz/custom_ability/definition_spec.rb
+++ b/ee/spec/models/authz/custom_ability/definition_spec.rb
@@ -14,8 +14,7 @@
admin_group_member: {
name: 'admin_group_member',
group_ability: group_ability,
- project_ability: project_ability,
- admin_ability: admin_ability
+ project_ability: project_ability
}
}
end
@@ -91,12 +90,21 @@
end
describe '#admin_ability_enabled?' do
+ let(:group_ability) { false }
+ let(:project_ability) { false }
+
subject { definition.admin_ability_enabled? }
it { is_expected.to be_truthy }
- context 'when admin ability is restricted' do
- let(:admin_ability) { false }
+ context 'when it is a group ability' do
+ let(:group_ability) { true }
+
+ it { is_expected.to be_falsey }
+ end
+
+ context 'when it is a project ability' do
+ let(:project_ability) { true }
it { is_expected.to be_falsey }
end
diff --git a/ee/spec/requests/api/graphql/member_role/permissions_list_spec.rb b/ee/spec/requests/api/graphql/member_role/permissions_list_spec.rb
index 0d2f4a33c0fdd34db2f93ab6f7bf6279bdb6036f..c06f98c8d18b52d1f95b83b667861efb4cf15063 100644
--- a/ee/spec/requests/api/graphql/member_role/permissions_list_spec.rb
+++ b/ee/spec/requests/api/graphql/member_role/permissions_list_spec.rb
@@ -22,6 +22,33 @@
QUERY
end
+ let(:mock_permissions) do
+ {
+ admin_ability_one: {
+ title: 'Admin something',
+ description: 'Allows admin access to do something.',
+ project_ability: true,
+ available_from_access_level: 50,
+ enabled_for_project_access_levels: [50]
+ },
+ admin_ability_two: {
+ title: 'Admin something else',
+ description: 'Allows admin access to do something else.',
+ requirements: [:read_ability_two],
+ group_ability: true,
+ enabled_for_group_access_levels: [40, 50]
+ },
+ read_ability_two: {
+ title: 'Read something else',
+ description: 'Allows read access to do something else.',
+ group_ability: true,
+ project_ability: true,
+ enabled_for_group_access_levels: [20, 30, 40, 50],
+ enabled_for_project_access_levels: [20, 30, 40, 50]
+ }
+ }
+ end
+
let(:query) do
graphql_query_for('memberRolePermissions', fields)
end
@@ -44,31 +71,9 @@ def reset_enum!
end
before do
- allow(MemberRole).to receive(:all_customizable_permissions).and_return(
- {
- admin_ability_one: {
- title: 'Admin something',
- description: 'Allows admin access to do something.',
- project_ability: true,
- available_from_access_level: 50,
- enabled_for_project_access_levels: [50]
- },
- admin_ability_two: {
- title: 'Admin something else',
- description: 'Allows admin access to do something else.',
- requirements: [:read_ability_two],
- group_ability: true,
- enabled_for_group_access_levels: [40, 50]
- },
- read_ability_two: {
- title: 'Read something else',
- description: 'Allows read access to do something else.',
- group_ability: true,
- project_ability: true,
- enabled_for_group_access_levels: [20, 30, 40, 50],
- enabled_for_project_access_levels: [20, 30, 40, 50]
- }
- }
+ allow(MemberRole).to receive_messages(
+ all_customizable_permissions: mock_permissions,
+ all_customizable_standard_permissions: mock_permissions
)
redefine_enum!
@@ -84,7 +89,7 @@ def reset_enum!
it_behaves_like 'a working graphql query'
- it 'returns all customizable ablities' do
+ it 'returns all customizable ablities', :unlimited_max_formatted_output_length do
expected_result = [
{ 'availableFor' => ['project'], 'description' => 'Allows admin access to do something.',
'name' => 'Admin something', 'requirements' => nil, 'value' => 'ADMIN_ABILITY_ONE',
diff --git a/tooling/custom_roles/docs/templates/custom_abilities.md.erb b/tooling/custom_roles/docs/templates/custom_abilities.md.erb
index 1f33eb970855744ebd5791fae83f87bb2d3e5397..c5775e6eb6ca02f9ec523c17999b958b4e948148 100644
--- a/tooling/custom_roles/docs/templates/custom_abilities.md.erb
+++ b/tooling/custom_roles/docs/templates/custom_abilities.md.erb
@@ -21,7 +21,7 @@
<% end %>
<% def scope(ability) %>
<% scopes = [] %>
-<% scopes << 'Instance' if ability[:admin_ability]%>
+<% scopes << 'Instance' unless ability[:group_ability] || ability[:project_ability]%>
<% scopes << 'Group' if ability[:group_ability]%>
<% scopes << 'Project' if ability[:project_ability]%>
<% return scopes.join(',<br> ') %>