Speed up Group security access specs

This is the Group equivalent of 13ad9a74

spec/features/security/group/internal_access_spec.rb

@@ -3,25 +3,12 @@ require 'rails_helper'
 describe 'Internal Group access', feature: true do
   include AccessMatchers
 
-  let(:group) { create(:group, :internal) }
+  let(:group)   { create(:group, :internal) }
   let(:project) { create(:project, :internal, group: group) }
-
-  let(:owner)     { create(:user) }
-  let(:master)    { create(:user) }
-  let(:developer) { create(:user) }
-  let(:reporter)  { create(:user) }
-  let(:guest)     { create(:user) }
-
-  let(:project_guest) { create(:user) }
-
-  before do
-    group.add_owner(owner)
-    group.add_master(master)
-    group.add_developer(developer)
-    group.add_reporter(reporter)
-    group.add_guest(guest)
-
-    project.team << [project_guest, :guest]
+  let(:project_guest) do
+    create(:user) do |user|
+      project.add_guest(user)
+    end
   end
 
   describe "Group should be internal" do
@@ -34,75 +21,75 @@ describe 'Internal Group access', feature: true do
   describe 'GET /groups/:path' do
     subject { group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_allowed_for :user }
-    it { is_expected.to be_denied_for :external }
-    it { is_expected.to be_denied_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_allowed_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
   end
 
   describe 'GET /groups/:path/issues' do
     subject { issues_group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_allowed_for :user }
-    it { is_expected.to be_denied_for :external }
-    it { is_expected.to be_denied_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_allowed_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
   end
 
   describe 'GET /groups/:path/merge_requests' do
     subject { merge_requests_group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_allowed_for :user }
-    it { is_expected.to be_denied_for :external }
-    it { is_expected.to be_denied_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_allowed_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
   end
 
   describe 'GET /groups/:path/group_members' do
     subject { group_group_members_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_allowed_for :user }
-    it { is_expected.to be_denied_for :external }
-    it { is_expected.to be_denied_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_allowed_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
   end
 
   describe 'GET /groups/:path/edit' do
     subject { edit_group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_denied_for master }
-    it { is_expected.to be_denied_for developer }
-    it { is_expected.to be_denied_for reporter }
-    it { is_expected.to be_denied_for guest }
-    it { is_expected.to be_denied_for project_guest }
-    it { is_expected.to be_denied_for :user }
-    it { is_expected.to be_denied_for :visitor }
-    it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_denied_for(:master).of(group) }
+    it { is_expected.to be_denied_for(:developer).of(group) }
+    it { is_expected.to be_denied_for(:reporter).of(group) }
+    it { is_expected.to be_denied_for(:guest).of(group) }
+    it { is_expected.to be_denied_for(project_guest) }
+    it { is_expected.to be_denied_for(:user) }
+    it { is_expected.to be_denied_for(:visitor) }
+    it { is_expected.to be_denied_for(:external) }
   end
 end

spec/features/security/group/private_access_spec.rb

@@ -3,25 +3,12 @@ require 'rails_helper'
 describe 'Private Group access', feature: true do
   include AccessMatchers
 
-  let(:group) { create(:group, :private) }
+  let(:group)   { create(:group, :private) }
   let(:project) { create(:project, :private, group: group) }
-
-  let(:owner)     { create(:user) }
-  let(:master)    { create(:user) }
-  let(:developer) { create(:user) }
-  let(:reporter)  { create(:user) }
-  let(:guest)     { create(:user) }
-
-  let(:project_guest) { create(:user) }
-
-  before do
-    group.add_owner(owner)
-    group.add_master(master)
-    group.add_developer(developer)
-    group.add_reporter(reporter)
-    group.add_guest(guest)
-
-    project.team << [project_guest, :guest]
+  let(:project_guest) do
+    create(:user) do |user|
+      project.add_guest(user)
+    end
   end
 
   describe "Group should be private" do
@@ -34,75 +21,75 @@ describe 'Private Group access', feature: true do
   describe 'GET /groups/:path' do
     subject { group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_denied_for :user }
-    it { is_expected.to be_denied_for :external }
-    it { is_expected.to be_denied_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_denied_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
   end
 
   describe 'GET /groups/:path/issues' do
     subject { issues_group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_denied_for :user }
-    it { is_expected.to be_denied_for :external }
-    it { is_expected.to be_denied_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_denied_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
   end
 
   describe 'GET /groups/:path/merge_requests' do
     subject { merge_requests_group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_denied_for :user }
-    it { is_expected.to be_denied_for :external }
-    it { is_expected.to be_denied_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_denied_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
   end
 
   describe 'GET /groups/:path/group_members' do
     subject { group_group_members_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_denied_for :user }
-    it { is_expected.to be_denied_for :external }
-    it { is_expected.to be_denied_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_denied_for(:user) }
+    it { is_expected.to be_denied_for(:external) }
+    it { is_expected.to be_denied_for(:visitor) }
   end
 
   describe 'GET /groups/:path/edit' do
     subject { edit_group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_denied_for master }
-    it { is_expected.to be_denied_for developer }
-    it { is_expected.to be_denied_for reporter }
-    it { is_expected.to be_denied_for guest }
-    it { is_expected.to be_denied_for project_guest }
-    it { is_expected.to be_denied_for :user }
-    it { is_expected.to be_denied_for :visitor }
-    it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_denied_for(:master).of(group) }
+    it { is_expected.to be_denied_for(:developer).of(group) }
+    it { is_expected.to be_denied_for(:reporter).of(group) }
+    it { is_expected.to be_denied_for(:guest).of(group) }
+    it { is_expected.to be_denied_for(project_guest) }
+    it { is_expected.to be_denied_for(:user) }
+    it { is_expected.to be_denied_for(:visitor) }
+    it { is_expected.to be_denied_for(:external) }
   end
 end

spec/features/security/group/public_access_spec.rb

@@ -3,25 +3,12 @@ require 'rails_helper'
 describe 'Public Group access', feature: true do
   include AccessMatchers
 
-  let(:group) { create(:group, :public) }
+  let(:group)   { create(:group, :public) }
   let(:project) { create(:project, :public, group: group) }
-
-  let(:owner)     { create(:user) }
-  let(:master)    { create(:user) }
-  let(:developer) { create(:user) }
-  let(:reporter)  { create(:user) }
-  let(:guest)     { create(:user) }
-
-  let(:project_guest) { create(:user) }
-
-  before do
-    group.add_owner(owner)
-    group.add_master(master)
-    group.add_developer(developer)
-    group.add_reporter(reporter)
-    group.add_guest(guest)
-    
-    project.team << [project_guest, :guest]
+  let(:project_guest) do
+    create(:user) do |user|
+      project.add_guest(user)
+    end
   end
 
   describe "Group should be public" do
@@ -34,75 +21,75 @@ describe 'Public Group access', feature: true do
   describe 'GET /groups/:path' do
     subject { group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_allowed_for :user }
-    it { is_expected.to be_allowed_for :external }
-    it { is_expected.to be_allowed_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_allowed_for(:user) }
+    it { is_expected.to be_allowed_for(:external) }
+    it { is_expected.to be_allowed_for(:visitor) }
   end
 
   describe 'GET /groups/:path/issues' do
     subject { issues_group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_allowed_for :user }
-    it { is_expected.to be_allowed_for :external }
-    it { is_expected.to be_allowed_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_allowed_for(:user) }
+    it { is_expected.to be_allowed_for(:external) }
+    it { is_expected.to be_allowed_for(:visitor) }
   end
 
   describe 'GET /groups/:path/merge_requests' do
     subject { merge_requests_group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_allowed_for :user }
-    it { is_expected.to be_allowed_for :external }
-    it { is_expected.to be_allowed_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_allowed_for(:user) }
+    it { is_expected.to be_allowed_for(:external) }
+    it { is_expected.to be_allowed_for(:visitor) }
   end
 
   describe 'GET /groups/:path/group_members' do
     subject { group_group_members_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_allowed_for master }
-    it { is_expected.to be_allowed_for developer }
-    it { is_expected.to be_allowed_for reporter }
-    it { is_expected.to be_allowed_for guest }
-    it { is_expected.to be_allowed_for project_guest }
-    it { is_expected.to be_allowed_for :user }
-    it { is_expected.to be_allowed_for :external }
-    it { is_expected.to be_allowed_for :visitor }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_allowed_for(:master).of(group) }
+    it { is_expected.to be_allowed_for(:developer).of(group) }
+    it { is_expected.to be_allowed_for(:reporter).of(group) }
+    it { is_expected.to be_allowed_for(:guest).of(group) }
+    it { is_expected.to be_allowed_for(project_guest) }
+    it { is_expected.to be_allowed_for(:user) }
+    it { is_expected.to be_allowed_for(:external) }
+    it { is_expected.to be_allowed_for(:visitor) }
   end
 
   describe 'GET /groups/:path/edit' do
     subject { edit_group_path(group) }
 
-    it { is_expected.to be_allowed_for :admin }
-    it { is_expected.to be_allowed_for owner }
-    it { is_expected.to be_denied_for master }
-    it { is_expected.to be_denied_for developer }
-    it { is_expected.to be_denied_for reporter }
-    it { is_expected.to be_denied_for guest }
-    it { is_expected.to be_denied_for project_guest }
-    it { is_expected.to be_denied_for :user }
-    it { is_expected.to be_denied_for :visitor }
-    it { is_expected.to be_denied_for :external }
+    it { is_expected.to be_allowed_for(:admin) }
+    it { is_expected.to be_allowed_for(:owner).of(group) }
+    it { is_expected.to be_denied_for(:master).of(group) }
+    it { is_expected.to be_denied_for(:developer).of(group) }
+    it { is_expected.to be_denied_for(:reporter).of(group) }
+    it { is_expected.to be_denied_for(:guest).of(group) }
+    it { is_expected.to be_denied_for(project_guest) }
+    it { is_expected.to be_denied_for(:user) }
+    it { is_expected.to be_denied_for(:visitor) }
+    it { is_expected.to be_denied_for(:external) }
   end
 end

spec/support/matchers/access_matchers.rb

@@ -7,7 +7,7 @@ module AccessMatchers
   extend RSpec::Matchers::DSL
   include Warden::Test::Helpers
 
-  def emulate_user(user, project = nil)
+  def emulate_user(user, membership = nil)
     case user
     when :user
       login_as(create(:user))
@@ -19,16 +19,17 @@ module AccessMatchers
       login_as(create(:user, external: true))
     when User
       login_as(user)
-    when :owner
-      raise ArgumentError, "cannot emulate owner without project" unless project
-
-      login_as(project.owner)
-    when *Gitlab::Access.sym_options.keys
-      raise ArgumentError, "cannot emulate user #{user} without project" unless project
+    when *Gitlab::Access.sym_options_with_owner.keys
+      raise ArgumentError, "cannot emulate #{user} without membership parent" unless membership
 
       role = user
-      user = create(:user)
-      project.public_send(:"add_#{role}", user)
+
+      if role == :owner && membership.owner
+        user = membership.owner
+      else
+        user = create(:user)
+        membership.public_send(:"add_#{role}", user)
+      end
 
       login_as(user)
     else
@@ -47,14 +48,14 @@ module AccessMatchers
 
   matcher :be_allowed_for do |user|
     match do |url|
-      emulate_user(user, @project)
+      emulate_user(user, @membership)
       visit(url)
 
       status_code != 404 && current_path != new_user_session_path
     end
 
-    chain :of do |project|
-      @project = project
+    chain :of do |membership|
+      @membership = membership
     end
 
     description { description_for(user, 'allowed') }
@@ -62,14 +63,14 @@ module AccessMatchers
 
   matcher :be_denied_for do |user|
     match do |url|
-      emulate_user(user, @project)
+      emulate_user(user, @membership)
       visit(url)
 
       status_code == 404 || current_path == new_user_session_path
     end
 
-    chain :of do |project|
-      @project = project
+    chain :of do |membership|
+      @membership = membership
     end
 
     description { description_for(user, 'denied') }