Find and remove duplicate policy checks
The problem
We have hundreds of policy checks in our DeclarativePolicy classes. Consolidating into fewer abilities will help make our permissions system more manageable. Many of these abilities are enabled/disabled using duplicate logic. Consolidating these abilities will help us to have fewer ability checks overall.
More context
DeclarativePolicy
tells us which policies are logically equivalent. For example:
GroupPolicy.ability_map.map
=> {
:create_metrics_dashboard_annotation=>[[:enable, #<Rule developer>]],
:delete_metrics_dashboard_annotation=>[[:enable, #<Rule developer>]],
:update_metrics_dashboard_annotation=>[[:enable, #<Rule developer>]]
}
The only rule that is every checked for these 3 abilities is whether the current user is a developer. So, we can safely merge these into one ability, perhaps manage_metrics_dashboard_annotation
. This change would occur both in the policy file itself and in any code that references these authorization checks.
This query shows that 92 out of the 192 abilities in GroupPolicy
and 178 of the 354 abilities in ProjectPolicy
are essentially duplicates (mapped to the same set of checks)
GroupPolicy.ability_map.map.keys.count
=> 192
# select any ability whose condition logic is equal to the condition logic of another ability
GroupPolicy.ability_map.map.group_by{|k,v| v}.select{|k,v| v.count > 1}.values.map do |value|
value.map { |v| v[0] }
end.flatten.count
=> 92
ProjectPolicy.ability_map.map.group_by{|k,v| v}.select{|k,v| v.count > 1}.values.map do |value|
value.map { |v| v[0] }
end.flatten.count
=> 178
ProjectPolicy.ability_map.map.keys.count
=> 354
Consolidating these abilities will allow us to cut the number of permissions checks in half.
After this consolidation, we may still want to consolidate even further, but this is the first step.
This issue represents how to plan for a refactor of this scale. The steps should be something like:
- Identify duplicates
- Track duplicates somehow (Likely Google doc -> GitLab isssue)
- Consolidate duplicates one at a time, using human judgment to decide which name to keep.