Merge branch '976-confidential-issue' into 'master'

Exclude blocked users from potential MR approvers

This is a very minor security issue (we could even argue it's not a real security issue but just a bug).

Closes https://gitlab.com/gitlab-org/gitlab-ee/issues/976

See merge request !503

CHANGELOG-EE

 Please view this file on the master branch, on stable branches it's out of date.
 v 8.12.0 (Unreleased)
 
+v 8.11.6
+  - Exclude blocked users from potential MR approvers
+
 v 8.11.5
   - API: Restore backward-compatibility for POST /projects/:id/members when membership is locked
 

app/models/merge_request.rb

@@ -677,7 +677,10 @@ class MergeRequest < ActiveRecord::Base
       wheres << "id IN (#{project.group.members.where(has_access).select(:user_id).to_sql})"
     end
 
-    User.where("(#{wheres.join(' OR ')}) AND id NOT IN (#{approvals.select(:user_id).to_sql}) AND id != #{author.id}").count
+    User.
+      active.
+      where("(#{wheres.join(' OR ')}) AND id NOT IN (#{approvals.select(:user_id).to_sql}) AND id != #{author.id}").
+      count
   end
 
   # Users in the list of approvers who have not already approved this MR.

spec/models/merge_request_spec.rb

@@ -336,6 +336,15 @@ describe MergeRequest, models: true do
       end.to change { merge_request.number_of_potential_approvers }.by(1)
     end
 
+    it "excludes blocked users" do
+      developer = create(:user)
+      blocked_developer = create(:user).tap { |u| u.block! }
+      project.team << [developer, :developer]
+      project.team << [blocked_developer, :developer]
+
+      expect(merge_request.number_of_potential_approvers).to eq(1)
+    end
+
     context "when the project is part of a group" do
       let(:group) { create(:group) }
       before { project.update_attributes(group: group) }
@@ -346,6 +355,8 @@ describe MergeRequest, models: true do
           group.add_reporter(create(:user))
           group.add_developer(create(:user))
           group.add_master(create(:user))
+          blocked_developer = create(:user).tap { |u| u.block! }
+          group.add_developer(blocked_developer)
         end.to change { merge_request.number_of_potential_approvers }.by(2)
       end
     end