Merge branch 'master' of dev.gitlab.org:gitlab/gitlabhq into backup-archive-permissions

24 files changed, 469 insertions, 461 deletions

diff --git a/spec/controllers/users_controller_spec.rb b/spec/controllers/users_controller_spec.rb
index d47a37914df..9f89101d7f7 100644
--- a/spec/controllers/users_controller_spec.rb
+++ b/spec/controllers/users_controller_spec.rb
@@ -1,25 +1,38 @@
 require 'spec_helper'
 
 describe UsersController do
-  let(:user)    { create(:user, username: 'user1', name: 'User 1', email: 'user1@gitlab.com') }
-
-  before do
-    sign_in(user)
-  end
+  let(:user) { create(:user) }
 
   describe 'GET #show' do
-    render_views
+    it 'is case-insensitive' do
+      user = create(:user, username: 'CamelCaseUser')
+      sign_in(user)
+
+      get :show, username: user.username.downcase
 
-    it 'renders the show template' do
-      get :show, username: user.username
-      expect(response.status).to eq(200)
-      expect(response).to render_template('show')
+      expect(response).to be_success
+    end
+
+    context 'with rendered views' do
+      render_views
+
+      it 'renders the show template' do
+        sign_in(user)
+
+        get :show, username: user.username
+
+        expect(response).to be_success
+        expect(response).to render_template('show')
+      end
     end
   end
 
   describe 'GET #calendar' do
     it 'renders calendar' do
+      sign_in(user)
+
       get :calendar, username: user.username
+
       expect(response).to render_template('calendar')
     end
   end
@@ -30,6 +43,8 @@ describe UsersController do
 
     before do
       allow_any_instance_of(User).to receive(:contributed_projects_ids).and_return([project.id])
+
+      sign_in(user)
       project.team << [user, :developer]
     end
 
diff --git a/spec/features/admin/security_spec.rb b/spec/features/security/admin_access_spec.rb
index 175fa9d4647..fe8cd7b7602 100644
--- a/spec/features/admin/security_spec.rb
+++ b/spec/features/security/admin_access_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe "Admin::Projects", feature: true  do
+  include AccessMatchers
+
   describe "GET /admin/projects" do
     subject { admin_namespaces_projects_path }
 
diff --git a/spec/features/security/dashboard_access_spec.rb b/spec/features/security/dashboard_access_spec.rb
index 67238e3ab76..c38cddbb904 100644
--- a/spec/features/security/dashboard_access_spec.rb
+++ b/spec/features/security/dashboard_access_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe "Dashboard access", feature: true  do
+  include AccessMatchers
+
   describe "GET /dashboard" do
     subject { dashboard_path }
 
diff --git a/spec/features/security/group/group_access_spec.rb b/spec/features/security/group/group_access_spec.rb
deleted file mode 100644
index 63793149459..00000000000
--- a/spec/features/security/group/group_access_spec.rb
+++ /dev/null
@@ -1,98 +0,0 @@
-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
diff --git a/spec/features/security/group/internal_group_access_spec.rb b/spec/features/security/group/internal_group_access_spec.rb
deleted file mode 100644
index d17a7412e43..00000000000
--- a/spec/features/security/group/internal_group_access_spec.rb
+++ /dev/null
@@ -1,82 +0,0 @@
-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
diff --git a/spec/features/security/group/mixed_group_access_spec.rb b/spec/features/security/group/mixed_group_access_spec.rb
deleted file mode 100644
index b3db7b5dea4..00000000000
--- a/spec/features/security/group/mixed_group_access_spec.rb
+++ /dev/null
@@ -1,83 +0,0 @@
-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
diff --git a/spec/features/security/group/public_group_access_spec.rb b/spec/features/security/group/public_group_access_spec.rb
deleted file mode 100644
index c16f0c0d1e1..00000000000
--- a/spec/features/security/group/public_group_access_spec.rb
+++ /dev/null
@@ -1,82 +0,0 @@
-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
diff --git a/spec/features/security/group_access_spec.rb b/spec/features/security/group_access_spec.rb
new file mode 100644
index 00000000000..8ce15388605
--- /dev/null
+++ b/spec/features/security/group_access_spec.rb
@@ -0,0 +1,284 @@
+require 'rails_helper'
+
+describe 'Group access', feature: true do
+  include AccessMatchers
+
+  def group
+    @group ||= create(:group)
+  end
+
+  def create_project(access_level)
+    if access_level == :mixed
+      create(:empty_project, :public, group: group)
+      create(:empty_project, :internal, group: group)
+    else
+      create(:empty_project, access_level, group: group)
+    end
+  end
+
+  def group_member(access_level, group = group)
+    level = Object.const_get("Gitlab::Access::#{access_level.upcase}")
+
+    create(:user).tap do |user|
+      group.add_user(user, level)
+    end
+  end
+
+  describe 'GET /groups/new' do
+    subject { new_group_path }
+
+    it { is_expected.to be_allowed_for :admin }
+    it { is_expected.to be_allowed_for :user }
+    it { is_expected.to be_denied_for :visitor }
+  end
+
+  describe 'GET /groups/:path' do
+    subject { group_path(group) }
+
+    context 'with public projects' do
+      let!(:project) { create_project(:public) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
+    end
+
+    context 'with mixed projects' do
+      let!(:project) { create_project(:mixed) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
+    end
+
+    context 'with internal projects' do
+      let!(:project) { create_project(:internal) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+
+    context 'with no projects' do
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_denied_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+  end
+
+  describe 'GET /groups/:path/issues' do
+    subject { issues_group_path(group) }
+
+    context 'with public projects' do
+      let!(:project) { create_project(:public) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
+    end
+
+    context 'with mixed projects' do
+      let!(:project) { create_project(:mixed) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
+    end
+
+    context 'with internal projects' do
+      let!(:project) { create_project(:internal) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+
+    context 'with no projects' do
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_denied_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+  end
+
+  describe 'GET /groups/:path/merge_requests' do
+    subject { merge_requests_group_path(group) }
+
+    context 'with public projects' do
+      let!(:project) { create_project(:public) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
+    end
+
+    context 'with mixed projects' do
+      let!(:project) { create_project(:mixed) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
+    end
+
+    context 'with internal projects' do
+      let!(:project) { create_project(:internal) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+
+    context 'with no projects' do
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_denied_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+  end
+
+  describe 'GET /groups/:path/group_members' do
+    subject { group_group_members_path(group) }
+
+    context 'with public projects' do
+      let!(:project) { create_project(:public) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
+    end
+
+    context 'with mixed projects' do
+      let!(:project) { create_project(:mixed) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_allowed_for :visitor }
+    end
+
+    context 'with internal projects' do
+      let!(:project) { create_project(:internal) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_allowed_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+
+    context 'with no projects' do
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_allowed_for group_member(:master) }
+      it { is_expected.to be_allowed_for group_member(:reporter) }
+      it { is_expected.to be_allowed_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_denied_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+  end
+
+  describe 'GET /groups/:path/edit' do
+    subject { edit_group_path(group) }
+
+    context 'with public projects' do
+      let!(:project) { create_project(:public) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_denied_for group_member(:master) }
+      it { is_expected.to be_denied_for group_member(:reporter) }
+      it { is_expected.to be_denied_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_denied_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+
+    context 'with mixed projects' do
+      let!(:project) { create_project(:mixed) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_denied_for group_member(:master) }
+      it { is_expected.to be_denied_for group_member(:reporter) }
+      it { is_expected.to be_denied_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_denied_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+
+    context 'with internal projects' do
+      let!(:project) { create_project(:internal) }
+
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_denied_for group_member(:master) }
+      it { is_expected.to be_denied_for group_member(:reporter) }
+      it { is_expected.to be_denied_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_denied_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+
+    context 'with no projects' do
+      it { is_expected.to be_allowed_for group_member(:owner) }
+      it { is_expected.to be_denied_for group_member(:master) }
+      it { is_expected.to be_denied_for group_member(:reporter) }
+      it { is_expected.to be_denied_for group_member(:guest) }
+      it { is_expected.to be_allowed_for :admin }
+      it { is_expected.to be_denied_for :user }
+      it { is_expected.to be_denied_for :visitor }
+    end
+  end
+end
diff --git a/spec/features/security/profile_access_spec.rb b/spec/features/security/profile_access_spec.rb
index bcabc2d53ac..c19678ab381 100644
--- a/spec/features/security/profile_access_spec.rb
+++ b/spec/features/security/profile_access_spec.rb
@@ -1,18 +1,11 @@
 require 'spec_helper'
 
 describe "Profile access", feature: true  do
-  before do
-    @u1 = create(:user)
-  end
-
-  describe "GET /login" do
-    it { expect(new_user_session_path).not_to be_not_found_for :visitor }
-  end
+  include AccessMatchers
 
   describe "GET /profile/keys" do
     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 :user }
     it { is_expected.to be_denied_for :visitor }
@@ -21,7 +14,6 @@ describe "Profile access", feature: true  do
   describe "GET /profile" do
     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 :user }
     it { is_expected.to be_denied_for :visitor }
@@ -30,7 +22,6 @@ describe "Profile access", feature: true  do
   describe "GET /profile/account" do
     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 :user }
     it { is_expected.to be_denied_for :visitor }
@@ -39,7 +30,6 @@ describe "Profile access", feature: true  do
   describe "GET /profile/preferences" do
     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 :user }
     it { is_expected.to be_denied_for :visitor }
@@ -48,7 +38,6 @@ describe "Profile access", feature: true  do
   describe "GET /profile/audit_log" do
     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 :user }
     it { is_expected.to be_denied_for :visitor }
@@ -57,7 +46,6 @@ describe "Profile access", feature: true  do
   describe "GET /profile/notifications" do
     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 :user }
     it { is_expected.to be_denied_for :visitor }
diff --git a/spec/features/security/project/internal_access_spec.rb b/spec/features/security/project/internal_access_spec.rb
index 4649e58cb1a..57563add74c 100644
--- a/spec/features/security/project/internal_access_spec.rb
+++ b/spec/features/security/project/internal_access_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe "Internal Project Access", feature: true  do
+  include AccessMatchers
+
   let(:project) { create(:project, :internal) }
 
   let(:master) { create(:user) }
diff --git a/spec/features/security/project/private_access_spec.rb b/spec/features/security/project/private_access_spec.rb
index 2866bf0355b..a1e111c6cab 100644
--- a/spec/features/security/project/private_access_spec.rb
+++ b/spec/features/security/project/private_access_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe "Private Project Access", feature: true  do
+  include AccessMatchers
+
   let(:project) { create(:project) }
 
   let(:master)   { create(:user) }
diff --git a/spec/features/security/project/public_access_spec.rb b/spec/features/security/project/public_access_spec.rb
index 554c96bcdc5..655d2c8b7d9 100644
--- a/spec/features/security/project/public_access_spec.rb
+++ b/spec/features/security/project/public_access_spec.rb
@@ -1,6 +1,8 @@
 require 'spec_helper'
 
 describe "Public Project Access", feature: true  do
+  include AccessMatchers
+
   let(:project) { create(:project) }
 
   let(:master) { create(:user) }
@@ -17,7 +19,6 @@ describe "Public Project Access", feature: true  do
 
     # readonly
     project.team << [reporter, :reporter]
-
   end
 
   describe "Project should be public" do
diff --git a/spec/helpers/auth_helper_spec.rb b/spec/helpers/auth_helper_spec.rb
new file mode 100644
index 00000000000..e47a54fdac5
--- /dev/null
+++ b/spec/helpers/auth_helper_spec.rb
@@ -0,0 +1,20 @@
+require "spec_helper"
+
+describe AuthHelper do
+  describe "button_based_providers" do
+    it 'returns all enabled providers' do
+      allow(helper).to receive(:auth_providers) { [:twitter, :github] }
+      expect(helper.button_based_providers).to include(*[:twitter, :github])
+    end
+
+    it 'does not return ldap provider' do
+      allow(helper).to receive(:auth_providers) { [:twitter, :ldapmain] }
+      expect(helper.button_based_providers).to include(:twitter)
+    end
+
+    it 'returns empty array' do
+      allow(helper).to receive(:auth_providers) { [] }
+      expect(helper.button_based_providers).to eq([])
+    end
+  end
+end
diff --git a/spec/helpers/oauth_helper_spec.rb b/spec/helpers/oauth_helper_spec.rb
deleted file mode 100644
index 3ef35f35102..00000000000
--- a/spec/helpers/oauth_helper_spec.rb
+++ /dev/null
@@ -1,20 +0,0 @@
-require "spec_helper"
-
-describe OauthHelper do
-  describe "additional_providers" do
-    it 'returns all enabled providers' do
-      allow(helper).to receive(:enabled_oauth_providers) { [:twitter, :github] }
-      expect(helper.additional_providers).to include(*[:twitter, :github])
-    end
-
-    it 'does not return ldap provider' do
-      allow(helper).to receive(:enabled_oauth_providers) { [:twitter, :ldapmain] }
-      expect(helper.additional_providers).to include(:twitter)
-    end
-
-    it 'returns empty array' do
-      allow(helper).to receive(:enabled_oauth_providers) { [] }
-      expect(helper.additional_providers).to eq([])
-    end
-  end
-end
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index beb9b4e438e..99abb95d906 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/spec/helpers/projects_helper_spec.rb
@@ -22,7 +22,7 @@ describe ProjectsHelper do
 
     let(:user) { create(:user) }
 
-    it "returns false if there are no approipriate permissions" do
+    it "returns false if there are no appropriate permissions" do
       allow(helper).to receive(:can?) { false }
 
       expect(helper.can_change_visibility_level?(project, user)).to be_falsey
@@ -52,4 +52,22 @@ describe ProjectsHelper do
       end
     end
   end
+
+  describe "readme_cache_key" do
+    let(:project) { create(:project) }
+
+    before do
+      helper.instance_variable_set(:@project, project)
+    end
+
+    it "returns a valid cach key" do
+      expect(helper.send(:readme_cache_key)).to eq("#{project.id}-#{project.commit.id}-readme")
+    end
+
+    it "returns a valid cache key if HEAD does not exist" do
+      allow(project).to receive(:commit) { nil }
+
+      expect(helper.send(:readme_cache_key)).to eq("#{project.id}-nil-readme")
+    end
+  end
 end
diff --git a/spec/javascripts/merge_request_tabs_spec.js.coffee b/spec/javascripts/merge_request_tabs_spec.js.coffee
index 6cc96fb68a0..a0cfba455ea 100644
--- a/spec/javascripts/merge_request_tabs_spec.js.coffee
+++ b/spec/javascripts/merge_request_tabs_spec.js.coffee
@@ -51,6 +51,12 @@ describe 'MergeRequestTabs', ->
       expect(@subject('notes')).toBe('/foo/bar/merge_requests/1')
       expect(@subject('commits')).toBe('/foo/bar/merge_requests/1/commits')
 
+    it 'changes from diffs.html', ->
+      @class._location = stubLocation(pathname: '/foo/bar/merge_requests/1/diffs.html')
+
+      expect(@subject('notes')).toBe('/foo/bar/merge_requests/1')
+      expect(@subject('commits')).toBe('/foo/bar/merge_requests/1/commits')
+
     it 'changes from notes', ->
       @class._location = stubLocation(pathname: '/foo/bar/merge_requests/1')
 
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 16902317f10..76f16323e2f 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -442,6 +442,18 @@ describe User do
     end
   end
 
+  describe '.find_by_username!' do
+    it 'raises RecordNotFound' do
+      expect { described_class.find_by_username!('JohnDoe') }.
+        to raise_error(ActiveRecord::RecordNotFound)
+    end
+
+    it 'is case-insensitive' do
+      user = create(:user, username: 'JohnDoe')
+      expect(described_class.find_by_username!('JOHNDOE')).to eq user
+    end
+  end
+
   describe 'all_ssh_keys' do
     it { is_expected.to have_many(:keys).dependent(:destroy) }
 
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 7030c105b58..29db035b2de 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -8,6 +8,7 @@ describe API::API, api: true  do
   let!(:merge_request_closed) { create(:merge_request, state: "closed", author: user, assignee: user, source_project: project, target_project: project, title: "Closed test") }
   let!(:merge_request_merged) { create(:merge_request, state: "merged", author: user, assignee: user, source_project: project, target_project: project, title: "Merged test") }
   let!(:note) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "a comment on a MR") }
+  let!(:note2) { create(:note_on_merge_request, author: user, project: project, noteable: merge_request, note: "another comment on a MR") }
 
   before do
     project.team << [user, :reporters]
@@ -397,13 +398,14 @@ describe API::API, api: true  do
   end
 
   describe "GET :id/merge_request/:merge_request_id/comments" do
-    it "should return merge_request comments" do
+    it "should return merge_request comments ordered by created_at" do
       get api("/projects/#{project.id}/merge_request/#{merge_request.id}/comments", user)
       expect(response.status).to eq(200)
       expect(json_response).to be_an Array
-      expect(json_response.length).to eq(1)
+      expect(json_response.length).to eq(2)
       expect(json_response.first['note']).to eq("a comment on a MR")
       expect(json_response.first['author']['id']).to eq(user.id)
+      expect(json_response.last['note']).to eq("another comment on a MR")
     end
 
     it "should return a 404 error if merge_request_id not found" do
diff --git a/spec/services/projects/create_service_spec.rb b/spec/services/projects/create_service_spec.rb
index 97b206c9854..66cdfd5d758 100644
--- a/spec/services/projects/create_service_spec.rb
+++ b/spec/services/projects/create_service_spec.rb
@@ -4,13 +4,19 @@ describe Projects::CreateService do
   describe :create_by_user do
     before do
       @user = create :user
-      @admin = create :user, admin: true
       @opts = {
         name: "GitLab",
         namespace: @user.namespace
       }
     end
 
+    it 'creates services on Project creation' do
+      project = create_project(@user, @opts)
+      project.reload
+
+      expect(project.services).not_to be_empty
+    end
+
     context 'user namespace' do
       before do
         @project = create_project(@user, @opts)
@@ -75,7 +81,9 @@ describe Projects::CreateService do
       end
 
       it 'should allow a restricted visibility level for admins' do
-        project = create_project(@admin, @opts)
+        admin = create(:admin)
+        project = create_project(admin, @opts)
+
         expect(project.errors.any?).to be(false)
         expect(project.saved?).to be(true)
       end
diff --git a/spec/support/matchers.rb b/spec/support/matchers.rb
deleted file mode 100644
index a2f853e3e70..00000000000
--- a/spec/support/matchers.rb
+++ /dev/null
@@ -1,66 +0,0 @@
-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
diff --git a/spec/support/matchers/access_matchers.rb b/spec/support/matchers/access_matchers.rb
new file mode 100644
index 00000000000..558e8b1612f
--- /dev/null
+++ b/spec/support/matchers/access_matchers.rb
@@ -0,0 +1,54 @@
+# 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
diff --git a/spec/support/matchers/include_module.rb b/spec/support/matchers/include_module.rb
new file mode 100644
index 00000000000..0a78af1e90e
--- /dev/null
+++ b/spec/support/matchers/include_module.rb
@@ -0,0 +1,13 @@
+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
diff --git a/spec/support/matchers/is_within.rb b/spec/support/matchers/is_within.rb
new file mode 100644
index 00000000000..0c35fc7e899
--- /dev/null
+++ b/spec/support/matchers/is_within.rb
@@ -0,0 +1,9 @@
+# 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
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index ae0b95ea444..8dc687c3580 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -12,7 +12,8 @@ module TestEnv
     'fix'              => '12d65c8',
     'improve/awesome'  => '5937ac0',
     'markdown'         => '0ed8c6c',
-    'master'           => '5937ac0'
+    'master'           => '5937ac0',
+    "'test'"           => 'e56497b',
   }
 
   # gitlab-test-fork is a fork of gitlab-fork, but we don't necessarily