Commit 8e160b09 authored by Alex Kalderimis's avatar Alex Kalderimis

Reduce perceived complexity of capability computation

This changes the capability computation from code (a very large if
statement) to transformation of static data.

The data we transform is a map from `ability` to `requirements`:

```ruby
{
  ability_needed: [:test_one?, :test_two?],
  other_ability: [:test_three?]
}
```

If the object passes _any_ of the requirement tests, then it must pass
the policy ability check for all of the matched abilities. If the tests
are inductive then this becomes a one-to-one reverse mapping, but it
need not be, so this formulation can actually express stricter policy
tests than the current if statement can (i.e. we can assert that some
event needs multiple abilities).

This change expresses (like a DSL) the rules as symbols that must be
implemented as methods on the receiver, so we need to `send` the
requirements to the receiving event.
parent 8fc733e6
...@@ -149,7 +149,9 @@ class Event < ApplicationRecord ...@@ -149,7 +149,9 @@ class Event < ApplicationRecord
def visible_to_user?(user = nil) def visible_to_user?(user = nil)
return false unless capability.present? return false unless capability.present?
Ability.allowed?(user, capability, permission_object) capability.all? do |rule|
Ability.allowed?(user, rule, permission_object)
end
end end
def resource_parent def resource_parent
...@@ -361,34 +363,30 @@ class Event < ApplicationRecord ...@@ -361,34 +363,30 @@ class Event < ApplicationRecord
protected protected
# rubocop:disable Metrics/CyclomaticComplexity
# rubocop:disable Metrics/PerceivedComplexity
#
# TODO Refactor this method so we no longer need to disable the above cops
# https://gitlab.com/gitlab-org/gitlab/-/issues/216879.
def capability def capability
@capability ||= begin @capability ||= begin
if push_action? || commit_note? capabilities.flat_map do |ability, syms|
:download_code if syms.any? { |sym| send(sym) } # rubocop: disable GitlabSecurity/PublicSend
elsif membership_changed? || created_project_action? [ability]
:read_project else
elsif issue? || issue_note? []
:read_issue end
elsif merge_request? || merge_request_note? end
:read_merge_request end
elsif personal_snippet_note? || project_snippet_note? end
:read_snippet
elsif milestone? def capabilities
:read_milestone {
elsif wiki_page? download_code: %i[push_action? commit_note?],
:read_wiki read_project: %i[membership_changed? created_project_action?],
elsif design_note? || design? read_issue: %i[issue? issue_note?],
:read_design read_merge_request: %i[merge_request? merge_request_note?],
end read_snippet: %i[personal_snippet_note? project_snippet_note?],
end read_milestone: %i[milestone?],
end read_wiki: %i[wiki_page?],
# rubocop:enable Metrics/CyclomaticComplexity read_design: %i[design_note? design?]
# rubocop:enable Metrics/PerceivedComplexity }
end
private private
......
...@@ -15,15 +15,9 @@ module EE ...@@ -15,15 +15,9 @@ module EE
scope :epics, -> { where(target_type: 'Epic') } scope :epics, -> { where(target_type: 'Epic') }
end end
override :capability override :capabilities
def capability def capabilities
@capability ||= begin super.merge(read_epic: %i[epic? epic_note?])
if epic? || epic_note?
:read_epic
else
super
end
end
end end
override :action_name override :action_name
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment