Merge branch '21650-only-active-users-can-be-members' into 'master'

Exclude some pending or inactivated rows in Member scopes

An unapproved request or not-yet-accepted invite should not give access rights. Neither should a blocked user be considered a member of anything.

One visible outcome of this behaviour is that owners and masters of a group or project may be blocked, yet still receive notification emails for access requests.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/21650

See merge request !1994

Signed-off-by: Rémy Coutable <remy@rymai.me>

1 files changed, 50 insertions, 4 deletions

diff --git a/spec/models/member_spec.rb b/spec/models/member_spec.rb
index e9134a3d283..fdd7480b2c2 100644
--- a/spec/models/member_spec.rb
+++ b/spec/models/member_spec.rb
@@ -57,7 +57,7 @@ describe Member, models: true do
 
   describe 'Scopes & finders' do
     before do
-      project = create(:project)
+      project = create(:empty_project)
       group = create(:group)
       @owner_user = create(:user).tap { |u| group.add_owner(u) }
       @owner = group.members.find_by(user_id: @owner_user.id)
@@ -65,11 +65,30 @@ describe Member, models: true do
       @master_user = create(:user).tap { |u| project.team << [u, :master] }
       @master = project.members.find_by(user_id: @master_user.id)
 
-      ProjectMember.add_user(project.members, 'toto1@example.com', Gitlab::Access::DEVELOPER, @master_user)
+      @blocked_user = create(:user).tap do |u|
+        project.team << [u, :master]
+        project.team << [u, :developer]
+
+        u.block!
+      end
+      @blocked_master = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::MASTER)
+      @blocked_developer = project.members.find_by(user_id: @blocked_user.id, access_level: Gitlab::Access::DEVELOPER)
+
+      Member.add_user(
+        project.members,
+        'toto1@example.com',
+        Gitlab::Access::DEVELOPER,
+        current_user: @master_user
+      )
       @invited_member = project.members.invite.find_by_invite_email('toto1@example.com')
 
-      accepted_invite_user = build(:user)
-      ProjectMember.add_user(project.members, 'toto2@example.com', Gitlab::Access::DEVELOPER, @master_user)
+      accepted_invite_user = build(:user, state: :active)
+      Member.add_user(
+        project.members,
+        'toto2@example.com',
+        Gitlab::Access::DEVELOPER,
+        current_user: @master_user
+      )
       @accepted_invite_member = project.members.invite.find_by_invite_email('toto2@example.com').tap { |u| u.accept_invite!(accepted_invite_user) }
 
       requested_user = create(:user).tap { |u| project.request_access(u) }
@@ -119,6 +138,19 @@ describe Member, models: true do
       it { expect(described_class.non_pending).to include @accepted_request_member }
     end
 
+    describe '.developers' do
+      subject { described_class.developers.to_a }
+
+      it { is_expected.not_to include @owner }
+      it { is_expected.not_to include @master }
+      it { is_expected.to include @invited_member }
+      it { is_expected.to include @accepted_invite_member }
+      it { is_expected.not_to include @requested_member }
+      it { is_expected.to include @accepted_request_member }
+      it { is_expected.not_to include @blocked_master }
+      it { is_expected.not_to include @blocked_developer }
+    end
+
     describe '.owners_and_masters' do
       it { expect(described_class.owners_and_masters).to include @owner }
       it { expect(described_class.owners_and_masters).to include @master }
@@ -126,6 +158,20 @@ describe Member, models: true do
       it { expect(described_class.owners_and_masters).not_to include @accepted_invite_member }
       it { expect(described_class.owners_and_masters).not_to include @requested_member }
       it { expect(described_class.owners_and_masters).not_to include @accepted_request_member }
+      it { expect(described_class.owners_and_masters).not_to include @blocked_master }
+    end
+
+    describe '.has_access' do
+      subject { described_class.has_access.to_a }
+
+      it { is_expected.to include @owner }
+      it { is_expected.to include @master }
+      it { is_expected.to include @invited_member }
+      it { is_expected.to include @accepted_invite_member }
+      it { is_expected.not_to include @requested_member }
+      it { is_expected.to include @accepted_request_member }
+      it { is_expected.not_to include @blocked_master }
+      it { is_expected.not_to include @blocked_developer }
     end
   end