Skip to content

G
gitlab-ce
Commit 54673124 authored by Douwe Maan's avatar Douwe Maan
Browse files
Options

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

Speed up security feature specs

Before: `rspec spec/features/security/  0.12s user 0.04s system 0% cpu 3:38.00 total`

After: `rspec spec/features/security/  0.12s user 0.04s system 0% cpu 1:40.58 total`

The majority of the speed improvements is from two things:

1. Instead of using our standard `login_as` helper in the matchers, we take advantage
   of the `Warden::Test::Helpers` version of the method which bypasses the login form
   and logs the user in directly. We were essentially testing that filling out the login
   form works hundreds of times.
2. There were many tests that verified if a user of a certain access level
   (master, owner, reporter, guest) had access to a resource. Unfortunately we were
   creating every type of user for each test even though a test was only verifying one of
   them at a time. Now the tests only create the one user role they're testing.

See merge request !1023
parents 39dc39e3 b42422a7
Expand all Hide whitespace changes
Inline Side-by-side
Showing with 371 additions and 425 deletions
spec/features/admin/security_spec.rb → spec/features/security/admin_access_spec.rb
View file @ 54673124
require 'spec_helper' require 'spec_helper'
describe "Admin::Projects", feature: true do describe "Admin::Projects", feature: true do
include AccessMatchers
describe "GET /admin/projects" do describe "GET /admin/projects" do
subject { admin_namespaces_projects_path } subject { admin_namespaces_projects_path }
......
spec/features/security/dashboard_access_spec.rb
View file @ 54673124
require 'spec_helper' require 'spec_helper'
describe "Dashboard access", feature: true do describe "Dashboard access", feature: true do
include AccessMatchers
describe "GET /dashboard" do describe "GET /dashboard" do
subject { dashboard_path } subject { dashboard_path }
......
spec/features/security/group/group_access_spec.rb deleted 100644 → 0
View file @ 39dc39e3
require 'spec_helper'
describe "Group access", feature: true do
describe "GET /projects/new" do
it { expect(new_group_path).to be_allowed_for :admin }
it { expect(new_group_path).to be_allowed_for :user }
it { expect(new_group_path).to be_denied_for :visitor }
end
describe "Group" do
let(:group) { create(:group) }
let(:owner) { create(:owner) }
let(:master) { create(:user) }
let(:reporter) { create(:user) }
let(:guest) { create(:user) }
let(:nonmember) { create(:user) }
before do
group.add_user(owner, Gitlab::Access::OWNER)
group.add_user(master, Gitlab::Access::MASTER)
group.add_user(reporter, Gitlab::Access::REPORTER)
group.add_user(guest, Gitlab::Access::GUEST)
end
describe "GET /groups/:path" do
subject { group_path(group) }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_denied_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_denied_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_denied_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_denied_for :user }
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 owner }
it { is_expected.to be_denied_for master }
it { is_expected.to be_denied_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
end
describe "GET /groups/:path/projects" do
subject { projects_group_path(group) }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_denied_for master }
it { is_expected.to be_denied_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
end
end
end
spec/features/security/group/internal_group_access_spec.rb deleted 100644 → 0
View file @ 39dc39e3
require 'spec_helper'
describe "Group with internal project access", feature: true do
describe "Group" do
let(:group) { create(:group) }
let(:owner) { create(:owner) }
let(:master) { create(:user) }
let(:reporter) { create(:user) }
let(:guest) { create(:user) }
let(:nonmember) { create(:user) }
before do
group.add_user(owner, Gitlab::Access::OWNER)
group.add_user(master, Gitlab::Access::MASTER)
group.add_user(reporter, Gitlab::Access::REPORTER)
group.add_user(guest, Gitlab::Access::GUEST)
create(:project, :internal, group: group)
end
describe "GET /groups/:path" do
subject { group_path(group) }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_denied_for master }
it { is_expected.to be_denied_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
end
end
end
spec/features/security/group/mixed_group_access_spec.rb deleted 100644 → 0
View file @ 39dc39e3
require 'spec_helper'
describe "Group access", feature: true do
describe "Group" do
let(:group) { create(:group) }
let(:owner) { create(:owner) }
let(:master) { create(:user) }
let(:reporter) { create(:user) }
let(:guest) { create(:user) }
let(:nonmember) { create(:user) }
before do
group.add_user(owner, Gitlab::Access::OWNER)
group.add_user(master, Gitlab::Access::MASTER)
group.add_user(reporter, Gitlab::Access::REPORTER)
group.add_user(guest, Gitlab::Access::GUEST)
create(:project, :internal, path: "internal_project", group: group)
create(:project, :public, path: "public_project", group: group)
end
describe "GET /groups/:path" do
subject { group_path(group) }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_denied_for master }
it { is_expected.to be_denied_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
end
end
end
spec/features/security/group/public_group_access_spec.rb deleted 100644 → 0
View file @ 39dc39e3
require 'spec_helper'
describe "Group with public project access", feature: true do
describe "Group" do
let(:group) { create(:group) }
let(:owner) { create(:owner) }
let(:master) { create(:user) }
let(:reporter) { create(:user) }
let(:guest) { create(:user) }
let(:nonmember) { create(:user) }
before do
group.add_user(owner, Gitlab::Access::OWNER)
group.add_user(master, Gitlab::Access::MASTER)
group.add_user(reporter, Gitlab::Access::REPORTER)
group.add_user(guest, Gitlab::Access::GUEST)
create(:project, :public, group: group)
end
describe "GET /groups/:path" do
subject { group_path(group) }
it { is_expected.to be_allowed_for owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_allowed_for master }
it { is_expected.to be_allowed_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for guest }
it { is_expected.to be_allowed_for :user }
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 owner }
it { is_expected.to be_denied_for master }
it { is_expected.to be_denied_for reporter }
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for guest }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
end
end
end
spec/features/security/group_access_spec.rb 0 → 100644
View file @ 54673124
This diff is collapsed.
spec/features/security/profile_access_spec.rb
View file @ 54673124
require 'spec_helper' require 'spec_helper'
describe "Profile access", feature: true do describe "Profile access", feature: true do
before do include AccessMatchers
@u1 = create(:user)
end
describe "GET /login" do
it { expect(new_user_session_path).not_to be_not_found_for :visitor }
end
describe "GET /profile/keys" do describe "GET /profile/keys" do
subject { profile_keys_path } subject { profile_keys_path }
it { is_expected.to be_allowed_for @u1 }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :user }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for :visitor }
...@@ -21,7 +14,6 @@ describe "Profile access", feature: true do ...@@ -21,7 +14,6 @@ describe "Profile access", feature: true do
describe "GET /profile" do describe "GET /profile" do
subject { profile_path } subject { profile_path }
it { is_expected.to be_allowed_for @u1 }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :user }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for :visitor }
...@@ -30,7 +22,6 @@ describe "Profile access", feature: true do ...@@ -30,7 +22,6 @@ describe "Profile access", feature: true do
describe "GET /profile/account" do describe "GET /profile/account" do
subject { profile_account_path } subject { profile_account_path }
it { is_expected.to be_allowed_for @u1 }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :user }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for :visitor }
...@@ -39,7 +30,6 @@ describe "Profile access", feature: true do ...@@ -39,7 +30,6 @@ describe "Profile access", feature: true do
describe "GET /profile/preferences" do describe "GET /profile/preferences" do
subject { profile_preferences_path } subject { profile_preferences_path }
it { is_expected.to be_allowed_for @u1 }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :user }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for :visitor }
...@@ -48,7 +38,6 @@ describe "Profile access", feature: true do ...@@ -48,7 +38,6 @@ describe "Profile access", feature: true do
describe "GET /profile/audit_log" do describe "GET /profile/audit_log" do
subject { audit_log_profile_path } subject { audit_log_profile_path }
it { is_expected.to be_allowed_for @u1 }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :user }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for :visitor }
...@@ -57,7 +46,6 @@ describe "Profile access", feature: true do ...@@ -57,7 +46,6 @@ describe "Profile access", feature: true do
describe "GET /profile/notifications" do describe "GET /profile/notifications" do
subject { profile_notifications_path } subject { profile_notifications_path }
it { is_expected.to be_allowed_for @u1 }
it { is_expected.to be_allowed_for :admin } it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user } it { is_expected.to be_allowed_for :user }
it { is_expected.to be_denied_for :visitor } it { is_expected.to be_denied_for :visitor }
......
spec/features/security/project/internal_access_spec.rb
View file @ 54673124
require 'spec_helper' require 'spec_helper'
describe "Internal Project Access", feature: true do describe "Internal Project Access", feature: true do
include AccessMatchers
let(:project) { create(:project, :internal) } let(:project) { create(:project, :internal) }
let(:master) { create(:user) } let(:master) { create(:user) }
......
spec/features/security/project/private_access_spec.rb
View file @ 54673124
require 'spec_helper' require 'spec_helper'
describe "Private Project Access", feature: true do describe "Private Project Access", feature: true do
include AccessMatchers
let(:project) { create(:project) } let(:project) { create(:project) }
let(:master) { create(:user) } let(:master) { create(:user) }
......
spec/features/security/project/public_access_spec.rb
View file @ 54673124
require 'spec_helper' require 'spec_helper'
describe "Public Project Access", feature: true do describe "Public Project Access", feature: true do
include AccessMatchers
let(:project) { create(:project) } let(:project) { create(:project) }
let(:master) { create(:user) } let(:master) { create(:user) }
...@@ -17,7 +19,6 @@ describe "Public Project Access", feature: true do ...@@ -17,7 +19,6 @@ describe "Public Project Access", feature: true do
# readonly # readonly
project.team << [reporter, :reporter] project.team << [reporter, :reporter]
end end
describe "Project should be public" do describe "Project should be public" do
......
spec/support/matchers.rb deleted 100644 → 0
View file @ 39dc39e3
RSpec::Matchers.define :be_valid_commit do
match do |actual|
actual &&
actual.id == ValidCommit::ID &&
actual.message == ValidCommit::MESSAGE &&
actual.author_name == ValidCommit::AUTHOR_FULL_NAME
end
end
def emulate_user(user)
user = case user
when :user then create(:user)
when :visitor then nil
when :admin then create(:admin)
else user
end
login_with(user) if user
end
RSpec::Matchers.define :be_allowed_for do |user|
match do |url|
emulate_user(user)
visit url
status_code != 404 && current_path != new_user_session_path
end
end
RSpec::Matchers.define :be_denied_for do |user|
match do |url|
emulate_user(user)
visit url
status_code == 404 || current_path == new_user_session_path
end
end
RSpec::Matchers.define :be_not_found_for do |user|
match do |url|
emulate_user(user)
visit url
status_code == 404
end
end
RSpec::Matchers.define :include_module do |expected|
match do
described_class.included_modules.include?(expected)
end
description do
"includes the #{expected} module"
end
failure_message do
"expected #{described_class} to include the #{expected} module"
end
end
# Extend shoulda-matchers
module Shoulda::Matchers::ActiveModel
class ValidateLengthOfMatcher
# Shortcut for is_at_least and is_at_most
def is_within(range)
is_at_least(range.min) && is_at_most(range.max)
end
end
end
spec/support/matchers/access_matchers.rb 0 → 100644
View file @ 54673124
# AccessMatchers
#
# The custom matchers contained in this module are used to test a user's access
# to a URL by emulating a specific user or type of user account, visiting the
# URL, and then checking the response status code and resulting path.
module AccessMatchers
extend RSpec::Matchers::DSL
include Warden::Test::Helpers
def emulate_user(user)
case user
when :user
login_as(create(:user))
when :visitor
logout
when :admin
login_as(create(:admin))
when User
login_as(user)
else
raise ArgumentError, "cannot emulate user #{user}"
end
end
def description_for(user, type)
if user.kind_of?(User)
# User#inspect displays too much information for RSpec's description
# messages
"be #{type} for supplied User"
else
"be #{type} for #{user}"
end
end
matcher :be_allowed_for do |user|
match do |url|
emulate_user(user)
visit url
status_code != 404 && current_path != new_user_session_path
end
description { description_for(user, 'allowed') }
end
matcher :be_denied_for do |user|
match do |url|
emulate_user(user)
visit url
status_code == 404 || current_path == new_user_session_path
end
description { description_for(user, 'denied') }
end
end
spec/support/matchers/include_module.rb 0 → 100644
View file @ 54673124
RSpec::Matchers.define :include_module do |expected|
match do
described_class.included_modules.include?(expected)
end
description do
"includes the #{expected} module"
end
failure_message do
"expected #{described_class} to include the #{expected} module"
end
end
spec/support/matchers/is_within.rb 0 → 100644
View file @ 54673124
# Extend shoulda-matchers
module Shoulda::Matchers::ActiveModel
class ValidateLengthOfMatcher
# Shortcut for is_at_least and is_at_most
def is_within(range)
is_at_least(range.min) && is_at_most(range.max)
end
end
end
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