Reduce perceived complexity of capability computation
What does this MR do?
Fixes #216879 (closed)
This merge request This changes the capability computation from code (a very large if statement) to a transformation of static data.
The data we transform is a map from ability
to requirements
:
{
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.
This transformation is much simpler code, with a nearly branchless control-flow
(we do have a branch that tests if the capability
list is empty, since all
on an empty list is always true).
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
Documentation (if required) -
Code review guidelines -
Merge request performance guidelines -
Style guides -
Database guides -
Separation of EE specific content
Availability and Testing
This is a refactoring MR, and very fortunately we have extensive tests for the functionality that this code supports (a visibility check on events).
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process.
Security
This MR is a refactoring of authorization methods. It is not, however, intended to change the result of that authorization in any way. Therefore is should have a security review.
-
Label as security and @ mention @gitlab-com/gl-security/appsec
-
The MR includes necessary changes to maintain consistency between UI, API, email, or other methods -
Security reports checked/validated by a reviewer from the AppSec team