Skip to content

G
gitlab-ce
Commit 2e034dbe authored by Rémy Coutable's avatar Rémy Coutable
Browse files
Options

Merge branch 'rs-group-security-spec-speed' into 'master' 

Speed up Group security access specs

This is the Group equivalent of 13ad9a74

See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/7779

Part of https://gitlab.com/gitlab-org/gitlab-ce/issues/24899

See merge request !7813
parents 7e5fa10b f7351b04
Show whitespace changes
Inline Side-by-side
Showing with 181 additions and 219 deletions
spec/features/security/group/internal_access_spec.rb
View file @ 2e034dbe
...@@ -5,23 +5,10 @@ describe 'Internal Group access', feature: true do ...@@ -5,23 +5,10 @@ describe 'Internal Group access', feature: true do
let(:group) { create(:group, :internal) } let(:group) { create(:group, :internal) }
let(:project) { create(:project, :internal, group: group) } let(:project) { create(:project, :internal, group: group) }
let(:project_guest) do
let(:owner) { create(:user) } create(:user) do |user|
let(:master) { create(:user) } project.add_guest(user)
let(:developer) { create(:user) } end
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]
end end
describe "Group should be internal" do describe "Group should be internal" do
...@@ -34,75 +21,75 @@ describe 'Internal Group access', feature: true do ...@@ -34,75 +21,75 @@ describe 'Internal Group access', feature: true do
describe 'GET /groups/:path' do describe 'GET /groups/:path' do
subject { group_path(group) } subject { group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for(:user) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
end end
describe 'GET /groups/:path/issues' do describe 'GET /groups/:path/issues' do
subject { issues_group_path(group) } subject { issues_group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for(:user) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
end end
describe 'GET /groups/:path/merge_requests' do describe 'GET /groups/:path/merge_requests' do
subject { merge_requests_group_path(group) } subject { merge_requests_group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for(:user) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
end end
describe 'GET /groups/:path/group_members' do describe 'GET /groups/:path/group_members' do
subject { group_group_members_path(group) } subject { group_group_members_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for(:user) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
end end
describe 'GET /groups/:path/edit' do describe 'GET /groups/:path/edit' do
subject { edit_group_path(group) } subject { edit_group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_denied_for master } it { is_expected.to be_denied_for(:master).of(group) }
it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for(:developer).of(group) }
it { is_expected.to be_denied_for reporter } it { is_expected.to be_denied_for(:reporter).of(group) }
it { is_expected.to be_denied_for guest } 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(project_guest) }
it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
end end
end end
spec/features/security/group/private_access_spec.rb
View file @ 2e034dbe
...@@ -5,23 +5,10 @@ describe 'Private Group access', feature: true do ...@@ -5,23 +5,10 @@ describe 'Private Group access', feature: true do
let(:group) { create(:group, :private) } let(:group) { create(:group, :private) }
let(:project) { create(:project, :private, group: group) } let(:project) { create(:project, :private, group: group) }
let(:project_guest) do
let(:owner) { create(:user) } create(:user) do |user|
let(:master) { create(:user) } project.add_guest(user)
let(:developer) { create(:user) } end
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]
end end
describe "Group should be private" do describe "Group should be private" do
...@@ -34,75 +21,75 @@ describe 'Private Group access', feature: true do ...@@ -34,75 +21,75 @@ describe 'Private Group access', feature: true do
describe 'GET /groups/:path' do describe 'GET /groups/:path' do
subject { group_path(group) } subject { group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
end end
describe 'GET /groups/:path/issues' do describe 'GET /groups/:path/issues' do
subject { issues_group_path(group) } subject { issues_group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
end end
describe 'GET /groups/:path/merge_requests' do describe 'GET /groups/:path/merge_requests' do
subject { merge_requests_group_path(group) } subject { merge_requests_group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
end end
describe 'GET /groups/:path/group_members' do describe 'GET /groups/:path/group_members' do
subject { group_group_members_path(group) } subject { group_group_members_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
end end
describe 'GET /groups/:path/edit' do describe 'GET /groups/:path/edit' do
subject { edit_group_path(group) } subject { edit_group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_denied_for master } it { is_expected.to be_denied_for(:master).of(group) }
it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for(:developer).of(group) }
it { is_expected.to be_denied_for reporter } it { is_expected.to be_denied_for(:reporter).of(group) }
it { is_expected.to be_denied_for guest } 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(project_guest) }
it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
end end
end end
spec/features/security/group/public_access_spec.rb
View file @ 2e034dbe
...@@ -5,23 +5,10 @@ describe 'Public Group access', feature: true do ...@@ -5,23 +5,10 @@ describe 'Public Group access', feature: true do
let(:group) { create(:group, :public) } let(:group) { create(:group, :public) }
let(:project) { create(:project, :public, group: group) } let(:project) { create(:project, :public, group: group) }
let(:project_guest) do
let(:owner) { create(:user) } create(:user) do |user|
let(:master) { create(:user) } project.add_guest(user)
let(:developer) { create(:user) } end
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]
end end
describe "Group should be public" do describe "Group should be public" do
...@@ -34,75 +21,75 @@ describe 'Public Group access', feature: true do ...@@ -34,75 +21,75 @@ describe 'Public Group access', feature: true do
describe 'GET /groups/:path' do describe 'GET /groups/:path' do
subject { group_path(group) } subject { group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for(:user) }
it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for(:external) }
it { is_expected.to be_allowed_for :visitor } it { is_expected.to be_allowed_for(:visitor) }
end end
describe 'GET /groups/:path/issues' do describe 'GET /groups/:path/issues' do
subject { issues_group_path(group) } subject { issues_group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for(:user) }
it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for(:external) }
it { is_expected.to be_allowed_for :visitor } it { is_expected.to be_allowed_for(:visitor) }
end end
describe 'GET /groups/:path/merge_requests' do describe 'GET /groups/:path/merge_requests' do
subject { merge_requests_group_path(group) } subject { merge_requests_group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for(:user) }
it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for(:external) }
it { is_expected.to be_allowed_for :visitor } it { is_expected.to be_allowed_for(:visitor) }
end end
describe 'GET /groups/:path/group_members' do describe 'GET /groups/:path/group_members' do
subject { group_group_members_path(group) } subject { group_group_members_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_allowed_for master } it { is_expected.to be_allowed_for(:master).of(group) }
it { is_expected.to be_allowed_for developer } it { is_expected.to be_allowed_for(:developer).of(group) }
it { is_expected.to be_allowed_for reporter } it { is_expected.to be_allowed_for(:reporter).of(group) }
it { is_expected.to be_allowed_for guest } 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(project_guest) }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for(:user) }
it { is_expected.to be_allowed_for :external } it { is_expected.to be_allowed_for(:external) }
it { is_expected.to be_allowed_for :visitor } it { is_expected.to be_allowed_for(:visitor) }
end end
describe 'GET /groups/:path/edit' do describe 'GET /groups/:path/edit' do
subject { edit_group_path(group) } subject { edit_group_path(group) }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for owner } it { is_expected.to be_allowed_for(:owner).of(group) }
it { is_expected.to be_denied_for master } it { is_expected.to be_denied_for(:master).of(group) }
it { is_expected.to be_denied_for developer } it { is_expected.to be_denied_for(:developer).of(group) }
it { is_expected.to be_denied_for reporter } it { is_expected.to be_denied_for(:reporter).of(group) }
it { is_expected.to be_denied_for guest } 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(project_guest) }
it { is_expected.to be_denied_for :user } it { is_expected.to be_denied_for(:user) }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for(:visitor) }
it { is_expected.to be_denied_for :external } it { is_expected.to be_denied_for(:external) }
end end
end end
spec/support/matchers/access_matchers.rb
View file @ 2e034dbe
...@@ -7,7 +7,7 @@ module AccessMatchers ...@@ -7,7 +7,7 @@ module AccessMatchers
extend RSpec::Matchers::DSL extend RSpec::Matchers::DSL
include Warden::Test::Helpers include Warden::Test::Helpers
def emulate_user(user, project = nil) def emulate_user(user, membership = nil)
case user case user
when :user when :user
login_as(create(:user)) login_as(create(:user))
...@@ -19,16 +19,17 @@ module AccessMatchers ...@@ -19,16 +19,17 @@ module AccessMatchers
login_as(create(:user, external: true)) login_as(create(:user, external: true))
when User when User
login_as(user) login_as(user)
when :owner when *Gitlab::Access.sym_options_with_owner.keys
raise ArgumentError, "cannot emulate owner without project" unless project raise ArgumentError, "cannot emulate #{user} without membership parent" unless membership
login_as(project.owner)
when *Gitlab::Access.sym_options.keys
raise ArgumentError, "cannot emulate user #{user} without project" unless project
role = user role = user
if role == :owner && membership.owner
user = membership.owner
else
user = create(:user) user = create(:user)
project.public_send(:"add_#{role}", user) membership.public_send(:"add_#{role}", user)
end
login_as(user) login_as(user)
else else
...@@ -47,14 +48,14 @@ module AccessMatchers ...@@ -47,14 +48,14 @@ module AccessMatchers
matcher :be_allowed_for do |user| matcher :be_allowed_for do |user|
match do |url| match do |url|
emulate_user(user, @project) emulate_user(user, @membership)
visit(url) visit(url)
status_code != 404 && current_path != new_user_session_path status_code != 404 && current_path != new_user_session_path
end end
chain :of do |project| chain :of do |membership|
@project = project @membership = membership
end end
description { description_for(user, 'allowed') } description { description_for(user, 'allowed') }
...@@ -62,14 +63,14 @@ module AccessMatchers ...@@ -62,14 +63,14 @@ module AccessMatchers
matcher :be_denied_for do |user| matcher :be_denied_for do |user|
match do |url| match do |url|
emulate_user(user, @project) emulate_user(user, @membership)
visit(url) visit(url)
status_code == 404 || current_path == new_user_session_path status_code == 404 || current_path == new_user_session_path
end end
chain :of do |project| chain :of do |membership|
@project = project @membership = membership
end end
description { description_for(user, 'denied') } description { description_for(user, 'denied') }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7