Merge branch 'explicit-requesters-scope' into 'master'

Exclude requesters from Project#members, Group#members and User#members

## What does this MR do?

It excludes requesters from the `Project#members`, `Group#members` and `User#members` associations, and adds new `Project#requesters` and `Group#requesters` associations.

## Are there points in the code the reviewer needs to double check?

No.

## Why was this MR needed?

Without this, if you call `project.members`, requesters are included in the results! This is at best misleading, and at worst can lead to security issues. By excluding requesters from the `#members` associations, we avoid introducing security inadvertently since you have to call the `#requesters` association explicitly to get requesters.

## What are the relevant issue numbers?

This is something I realized while fixing the security issue #19102.

## Does this MR meet the acceptance criteria?

- [x] I don't think this needs a CHANGELOG since this is an internal change
- Tests
  - [x] Added for this feature/bug
  - [ ] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !4946

1 files changed, 66 insertions, 0 deletions

diff --git a/spec/helpers/members_helper_spec.rb b/spec/helpers/members_helper_spec.rb
index f75fdb739f6..7b2155e9a4e 100644
--- a/spec/helpers/members_helper_spec.rb
+++ b/spec/helpers/members_helper_spec.rb
@@ -57,6 +57,72 @@ describe MembersHelper do
     end
   end
 
+  describe '#can_see_request_access_button?' do
+    let(:user) { create(:user) }
+    let(:group) { create(:group, :public) }
+    let(:project) { create(:project, :public, group: group) }
+
+    before do
+      allow(helper).to receive(:current_user).and_return(user)
+    end
+
+    context 'source is a group' do
+      context 'current_user is not a member' do
+        it 'returns true' do
+          expect(helper.can_see_request_access_button?(group)).to be_truthy
+        end
+      end
+
+      context 'current_user is a member' do
+        it 'returns false' do
+          group.add_owner(user)
+
+          expect(helper.can_see_request_access_button?(group)).to be_falsy
+        end
+      end
+
+      context 'current_user is a requester' do
+        it 'returns true' do
+          group.request_access(user)
+
+          expect(helper.can_see_request_access_button?(group)).to be_truthy
+        end
+      end
+    end
+
+    context 'source is a project' do
+      context 'current_user is not a member' do
+        it 'returns true' do
+          expect(helper.can_see_request_access_button?(project)).to be_truthy
+        end
+      end
+
+      context 'current_user is a group member' do
+        it 'returns false' do
+          group.add_owner(user)
+
+          expect(helper.can_see_request_access_button?(project)).to be_falsy
+        end
+      end
+
+      context 'current_user is a group requester' do
+        it 'returns false' do
+          group.request_access(user)
+
+          expect(helper.can_see_request_access_button?(project)).to be_falsy
+        end
+      end
+
+      context 'current_user is a member' do
+        it 'returns false' do
+          project.team << [user, :master]
+
+          expect(helper.can_see_request_access_button?(project)).to be_falsy
+        end
+      end
+    end
+  end
+
   describe '#remove_member_message' do
     let(:requester) { build(:user) }
     let(:project) { create(:project) }