Fix permissions in the project labels API

Merge branch 'security-357963-prevent-editing-group-labels-14-10' into '14-10-stable-ee'

See merge request gitlab-org/security/gitlab!2534

Changelog: security

lib/api/helpers/label_helpers.rb

@@ -82,8 +82,14 @@ module API
         params.delete(:label_id)
         params.delete(:name)
 
-        label = ::Labels::UpdateService.new(declared_params(include_missing: false)).execute(label)
-        render_validation_error!(label) unless label.valid?
+        update_params = declared_params(include_missing: false)
+
+        if update_params.present?
+          authorize! :admin_label, label
+
+          label = ::Labels::UpdateService.new(update_params).execute(label)
+          render_validation_error!(label) unless label.valid?
+        end
 
         if parent.is_a?(Project) && update_priority
           if priority.nil?
@@ -97,10 +103,10 @@ module API
       end
 
       def delete_label(parent)
-        authorize! :admin_label, parent
-
         label = find_label(parent, params_id_or_title, include_ancestor_groups: false)
 
+        authorize! :admin_label, label
+
         destroy_conditionally!(label)
       end
 

spec/requests/api/labels_spec.rb

@@ -483,6 +483,29 @@ RSpec.describe API::Labels do
       let(:request) { api("/projects/#{project.id}/labels", user) }
       let(:params) { { name: valid_label_title_1 } }
     end
+
+    context 'with group label' do
+      let_it_be(:group) { create(:group) }
+      let_it_be(:group_label) { create(:group_label, title: valid_group_label_title_1, group: group) }
+
+      before do
+        project.update!(group: group)
+      end
+
+      it 'returns 401 if user does not have access' do
+        delete api("/projects/#{project.id}/labels/#{group_label.id}", user)
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+
+      it 'returns 204 if user has access' do
+        group.add_developer(user)
+
+        delete api("/projects/#{project.id}/labels/#{group_label.id}", user)
+
+        expect(response).to have_gitlab_http_status(:no_content)
+      end
+    end
   end
 
   describe 'PUT /projects/:id/labels' do
@@ -537,6 +560,44 @@ RSpec.describe API::Labels do
 
       expect(response).to have_gitlab_http_status(:bad_request)
     end
+
+    context 'with group label' do
+      let_it_be(:group) { create(:group) }
+      let_it_be(:group_label) { create(:group_label, title: valid_group_label_title_1, group: group) }
+
+      before do
+        project.update!(group: group)
+      end
+
+      it 'allows updating of group label priority' do
+        put api("/projects/#{project.id}/labels/#{group_label.id}", user), params: { priority: 5 }
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['priority']).to eq(5)
+      end
+
+      it 'returns 401 when updating other fields' do
+        put api("/projects/#{project.id}/labels/#{group_label.id}", user), params: {
+          priority: 5,
+          new_name: 'new label name'
+        }
+
+        expect(response).to have_gitlab_http_status(:forbidden)
+      end
+
+      it 'returns 200 when user has access to the group label' do
+        group.add_developer(user)
+
+        put api("/projects/#{project.id}/labels/#{group_label.id}", user), params: {
+          priority: 5,
+          new_name: 'new label name'
+        }
+
+        expect(response).to have_gitlab_http_status(:ok)
+        expect(json_response['priority']).to eq(5)
+        expect(json_response['name']).to eq('new label name')
+      end
+    end
   end
 
   describe 'PUT /projects/:id/labels/promote' do