API merge pipelines and merge trains settings can be set without a license
Problem
This code is buggy because the intent is to only allow users to update merge_pipelines_enabled and merge_trains_enabled if they have the licensed feature and they can admin the project.
Instead we don't do the licensed feature verification if they have permissions to admin project(which they always have on this endpoint):
def verify_merge_pipelines_attrs!(project, attrs)
return if can?(current_user, :admin_project, project)
attrs.delete(:merge_pipelines_enabled) unless project.feature_available?(:merge_pipelines)
attrs.delete(:merge_trains_enabled) unless project.feature_available?(:merge_trains)
end
It also lacks code coverage for the scenario where no licenses exists.
Proposal
The admin check is already on the endpoint and shouldn't be here because it means we don't perform the other checks if the user HAS permission. So we can remove it:
def verify_merge_pipelines_attrs!(project, attrs)
attrs.delete(:merge_pipelines_enabled) unless project.feature_available?(:merge_pipelines)
attrs.delete(:merge_trains_enabled) unless project.feature_available?(:merge_trains)
end
We can also add:
attrs.delete(:merge_trains_skip_train_allowed) unless project.feature_available?(:merge_trains)
Add code coverage for the license checks.
Edited by 🤖 GitLab Bot 🤖