diff --git a/ee/app/models/authz/custom_ability.rb b/ee/app/models/authz/custom_ability.rb index b69241f0f4dd51393b01e5c4ce21d3a3d7951008..0d2fe6e1be4ac2a02ad1ef84aec46aa3f0dce4ad 100644 --- a/ee/app/models/authz/custom_ability.rb +++ b/ee/app/models/authz/custom_ability.rb @@ -18,7 +18,7 @@ def allowed?(user, resource) class << self def allowed?(user, ability, resource) - new(Gitlab::CustomRoles::Definition.all[ability.to_sym]) + new(Gitlab::CustomRoles::Definition.all[ability&.to_sym]) .allowed?(user, resource) end end @@ -27,9 +27,19 @@ def allowed?(user, ability, resource) attr_reader :attributes + def enabled?(attribute, default: false) + attributes.fetch(attribute, default) + end + + def disabled?(attribute) + !enabled?(attribute) + end + def enabled_for?(user, resource) return false if attributes.blank? return false unless user.is_a?(User) + return false if resource.is_a?(::Group) && disabled?(:group_ability) + return false if resource.is_a?(::Project) && disabled?(:project_ability) return false unless ::MemberRole.permission_enabled?(name, user) custom_roles_enabled?(resource) diff --git a/ee/spec/models/authz/custom_ability_spec.rb b/ee/spec/models/authz/custom_ability_spec.rb index 0ffc8ee8aa50615003353d10321a382d8bb509a8..95c802b999384088b46817ade6f7ee0b4ea94bf8 100644 --- a/ee/spec/models/authz/custom_ability_spec.rb +++ b/ee/spec/models/authz/custom_ability_spec.rb @@ -18,49 +18,67 @@ let_it_be(:group_runner) { create(:ci_runner, :group, groups: [group]) } where(:source, :ability, :resource, :expected) do - nil | :admin_vulnerability | ref(:group) | false - nil | :admin_vulnerability | ref(:project) | false - nil | :read_code | ref(:group) | false - nil | :read_code | ref(:project) | false - nil | :read_dependency | ref(:group) | false - nil | :read_dependency | ref(:group_runner) | false - nil | :read_dependency | ref(:project) | false - nil | :read_dependency | ref(:project_runner) | false - nil | :read_vulnerability | ref(:group) | false - nil | :read_vulnerability | ref(:project) | false - ref(:child_group) | :admin_vulnerability | ref(:child_group) | true - ref(:child_group) | :admin_vulnerability | ref(:child_project) | true - ref(:child_group) | :admin_vulnerability | ref(:group) | false - ref(:child_group) | :admin_vulnerability | ref(:project) | false - ref(:child_group) | :admin_vulnerability | ref(:root_group) | false - ref(:group) | :admin_vulnerability | ref(:group) | true - ref(:group) | :admin_vulnerability | ref(:project) | true - ref(:group) | :read_code | ref(:project) | true - ref(:group) | :read_code | ref(:project) | true - ref(:group) | :read_dependency | ref(:group) | true - ref(:group) | :read_dependency | ref(:group_runner) | true - ref(:group) | :read_dependency | ref(:project) | true - ref(:group) | :read_dependency | ref(:project_runner) | true - ref(:group) | :read_vulnerability | ref(:group) | true - ref(:group) | :read_vulnerability | ref(:project) | true - ref(:project) | :admin_vulnerability | ref(:group) | false - ref(:project) | :admin_vulnerability | ref(:project) | true - ref(:project) | :read_code | ref(:project) | true - ref(:project) | :read_code | ref(:project) | true - ref(:project) | :read_dependency | ref(:group) | false - ref(:project) | :read_dependency | ref(:group_runner) | false - ref(:project) | :read_dependency | ref(:project) | true - ref(:project) | :read_dependency | ref(:project_runner) | true - ref(:project) | :read_vulnerability | ref(:group) | false - ref(:project) | :read_vulnerability | ref(:project) | true - ref(:root_group) | :admin_vulnerability | ref(:child_group) | true - ref(:root_group) | :admin_vulnerability | ref(:group) | true - ref(:root_group) | :admin_vulnerability | ref(:project) | true - ref(:root_group) | :admin_vulnerability | "unknown" | false + Gitlab::CustomRoles::Definition.all.each do |(name, attrs)| # rubocop:disable Rails/FindEach -- this is not a rails model + nil | name | ref(:root_group) | false + nil | name | ref(:group) | false + nil | name | ref(:child_group) | false + nil | name | ref(:project) | false + nil | name | ref(:child_project) | false + nil | name | ref(:group_runner) | false + nil | name | ref(:project_runner) | false + nil | name | "unknown" | false + + ref(:root_group) | name | ref(:root_group) | attrs.fetch(:group_ability, true) + ref(:root_group) | name | ref(:group) | attrs.fetch(:group_ability, true) + ref(:root_group) | name | ref(:child_group) | attrs.fetch(:group_ability, true) + ref(:root_group) | name | ref(:project) | attrs.fetch(:project_ability, true) + ref(:root_group) | name | ref(:child_project) | attrs.fetch(:project_ability, true) + ref(:root_group) | name | ref(:group_runner) | attrs.fetch(:group_ability, true) + ref(:root_group) | name | ref(:project_runner) | attrs.fetch(:project_ability, true) + ref(:root_group) | name | "unknown" | false + + ref(:group) | name | ref(:root_group) | false + ref(:group) | name | ref(:group) | attrs.fetch(:group_ability, true) + ref(:group) | name | ref(:child_group) | attrs.fetch(:group_ability, true) + ref(:group) | name | ref(:project) | attrs.fetch(:project_ability, true) + ref(:group) | name | ref(:child_project) | attrs.fetch(:project_ability, true) + ref(:group) | name | ref(:group_runner) | attrs.fetch(:group_ability, true) + ref(:group) | name | ref(:project_runner) | attrs.fetch(:project_ability, true) + ref(:group) | name | "unknown" | false + + ref(:child_group) | name | ref(:root_group) | false + ref(:child_group) | name | ref(:group) | false + ref(:child_group) | name | ref(:child_group) | attrs.fetch(:group_ability, true) + ref(:child_group) | name | ref(:project) | false + ref(:child_group) | name | ref(:child_project) | attrs.fetch(:project_ability, true) + ref(:child_group) | name | ref(:group_runner) | false + ref(:child_group) | name | ref(:project_runner) | false + ref(:child_group) | name | "unknown" | false + + ref(:project) | name | ref(:root_group) | false + ref(:project) | name | ref(:group) | false + ref(:project) | name | ref(:child_group) | false + ref(:project) | name | ref(:project) | attrs.fetch(:project_ability, true) + ref(:project) | name | ref(:child_project) | false + ref(:project) | name | ref(:group_runner) | false + ref(:project) | name | ref(:project_runner) | attrs.fetch(:project_ability, true) + ref(:project) | name | "unknown" | false + + ref(:child_project) | name | ref(:root_group) | false + ref(:child_project) | name | ref(:group) | false + ref(:child_project) | name | ref(:child_group) | false + ref(:child_project) | name | ref(:project) | false + ref(:child_project) | name | ref(:child_project) | attrs.fetch(:project_ability, true) + ref(:child_project) | name | ref(:group_runner) | false + ref(:child_project) | name | ref(:project_runner) | false + ref(:child_project) | name | "unknown" | false + end + + nil | nil | nil | false end with_them do - let!(:role) { create(:member_role, :guest, ability, namespace: root_group) } + let!(:role) { create(:member_role, :guest, ability, namespace: root_group) if ability } let!(:membership_type) { source.is_a?(Project) ? :project_member : :group_member } let!(:membership) { create(membership_type, :guest, member_role: role, user: user, source: source) if source } @@ -74,6 +92,10 @@ it { is_expected.not_to be_allowed(user, ability, resource) } end + context 'with a nil user' do + it { is_expected.not_to be_allowed(nil, ability, resource) } + end + context 'with `custom_roles` disabled' do before do stub_licensed_features(custom_roles: false)