Merge branch 'zj-fix-label-creation-non-members' into 'security'

Fix label creation non members

Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/23416

See merge request !2006

3 files changed, 26 insertions, 17 deletions

diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 7bae055b241..b17553211d2 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -697,6 +697,14 @@ describe API::API, api: true  do
         expect(Time.parse(json_response['created_at'])).to be_like_time(creation_time)
       end
     end
+
+    context 'the user can only read the issue' do
+      it 'cannot create new labels' do
+        expect do
+          post api("/projects/#{project.id}/issues", non_member), title: 'new issue', labels: 'label, label2'
+        end.not_to change { project.labels.count }
+      end
+    end
   end
 
   describe 'POST /projects/:id/issues with spam filtering' do
@@ -839,8 +847,8 @@ describe API::API, api: true  do
     end
 
     it 'removes all labels' do
-      put api("/projects/#{project.id}/issues/#{issue.id}", user),
-          labels: ''
+      put api("/projects/#{project.id}/issues/#{issue.id}", user), labels: ''
+
       expect(response).to have_http_status(200)
       expect(json_response['labels']).to eq([])
     end
@@ -892,8 +900,8 @@ describe API::API, api: true  do
         update_time = 2.weeks.ago
         put api("/projects/#{project.id}/issues/#{issue.id}", user),
           labels: 'label3', state_event: 'close', updated_at: update_time
-        expect(response).to have_http_status(200)
 
+        expect(response).to have_http_status(200)
         expect(json_response['labels']).to include 'label3'
         expect(Time.parse(json_response['updated_at'])).to be_like_time(update_time)
       end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 37fcb2bc3a9..3ecf3eea5f5 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -402,14 +402,6 @@ describe API::API, api: true  do
     end
   end
 
-  describe "PUT /projects/:id/merge_requests/:merge_request_id to close MR" do
-    it "returns merge_request" do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close"
-      expect(response).to have_http_status(200)
-      expect(json_response['state']).to eq('closed')
-    end
-  end
-
   describe "PUT /projects/:id/merge_requests/:merge_request_id/merge" do
     let(:pipeline) { create(:ci_pipeline_without_jobs) }
 
@@ -486,6 +478,15 @@ describe API::API, api: true  do
   end
 
   describe "PUT /projects/:id/merge_requests/:merge_request_id" do
+    context "to close a MR" do
+      it "returns merge_request" do
+        put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), state_event: "close"
+
+        expect(response).to have_http_status(200)
+        expect(json_response['state']).to eq('closed')
+      end
+    end
+
     it "updates title and returns merge_request" do
       put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user), title: "New title"
       expect(response).to have_http_status(200)
@@ -511,10 +512,10 @@ describe API::API, api: true  do
     end
 
     it 'allows special label names' do
-      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}",
-              user),
-          title: 'new issue',
-          labels: 'label, label?, label&foo, ?, &'
+      put api("/projects/#{project.id}/merge_requests/#{merge_request.id}", user),
+        title: 'new issue',
+        labels: 'label, label?, label&foo, ?, &'
+
       expect(response.status).to eq(200)
       expect(json_response['labels']).to include 'label'
       expect(json_response['labels']).to include 'label?'
@@ -543,7 +544,7 @@ describe API::API, api: true  do
 
     it "returns 404 if note is attached to non existent merge request" do
       post api("/projects/#{project.id}/merge_requests/404/comments", user),
-           note: 'My comment'
+        note: 'My comment'
       expect(response).to have_http_status(404)
     end
   end
diff --git a/spec/services/labels/transfer_service_spec.rb b/spec/services/labels/transfer_service_spec.rb
index ddf3527dc0f..13654a0881c 100644
--- a/spec/services/labels/transfer_service_spec.rb
+++ b/spec/services/labels/transfer_service_spec.rb
@@ -2,7 +2,7 @@ require 'spec_helper'
 
 describe Labels::TransferService, services: true do
   describe '#execute' do
-    let(:user)    { create(:user) }
+    let(:user)    { create(:admin) }
     let(:group_1) { create(:group) }
     let(:group_2) { create(:group) }
     let(:group_3) { create(:group) }