Commit 6c0c3d9f authored by Timothy Andrew's avatar Timothy Andrew

Implement review comments from maintainer (@DouweM)

1. `add_column_with_default` needs a `down` block

2. Add specs for the auditor user to `spec/features/security`. This directory
   contains a series of feature specs to test the access various user roles have
   to various project/admin pages, which is the perfect place to test auditor
   user access.

3. Other minor changes (views, typos)
parent ac299a0e
......@@ -13,9 +13,7 @@
= image_tag avatar_icon(current_user), alt: current_user.to_reference, class: 'avatar s40'
.timeline-content.timeline-content-form
= render "projects/notes/form", view: diff_view
- elsif current_user.present? && current_user.auditor?
-# Display nothing
- else
- elsif !current_user
.disabled-comment.text-center
.disabled-comment-text.inline
Please
......
......@@ -8,7 +8,11 @@ class AddColumnAuditorToUsers < ActiveRecord::Migration
disable_ddl_transaction!
def change
def up
add_column_with_default :users, :auditor, :boolean, default: false, allow_null: false
end
def down
remove_column :users, :auditor
end
end
# Auditor Users
>**Note:** [Introduced][998] in GitLab 8.16.
>**Note:** [Introduced][998] in GitLab 8.17.
With Gitlab Enterprise Edition Premium, you can create *auditor* users, who
are given read-only access to all projects, groups, and other resources on the
......
......@@ -9,6 +9,7 @@ describe "Admin::Projects", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
it { is_expected.to be_denied_for :auditor }
end
describe "GET /admin/users" do
......@@ -17,6 +18,7 @@ describe "Admin::Projects", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
it { is_expected.to be_denied_for :auditor }
end
describe "GET /admin/hooks" do
......@@ -25,5 +27,6 @@ describe "Admin::Projects", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_denied_for :user }
it { is_expected.to be_denied_for :visitor }
it { is_expected.to be_denied_for :auditor }
end
end
......@@ -8,6 +8,7 @@ describe "Dashboard access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
......@@ -16,6 +17,7 @@ describe "Dashboard access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
......@@ -24,6 +26,7 @@ describe "Dashboard access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
......@@ -32,6 +35,7 @@ describe "Dashboard access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
......@@ -40,6 +44,7 @@ describe "Dashboard access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_allowed_for :visitor }
end
......@@ -53,6 +58,7 @@ describe "Dashboard access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
end
......@@ -60,12 +66,14 @@ describe "Dashboard access", feature: true do
describe "GET /projects/new" do
it { expect(new_project_path).to be_allowed_for :admin }
it { expect(new_project_path).to be_allowed_for :user }
it { expect(new_project_path).to be_allowed_for :auditor }
it { expect(new_project_path).to be_denied_for :visitor }
end
describe "GET /groups/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_allowed_for :auditor }
it { expect(new_group_path).to be_denied_for :visitor }
end
......@@ -74,6 +82,7 @@ describe "Dashboard access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
end
......@@ -22,6 +22,7 @@ describe 'Internal Group access', feature: true do
subject { group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -37,6 +38,7 @@ describe 'Internal Group access', feature: true do
subject { issues_group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -52,6 +54,7 @@ describe 'Internal Group access', feature: true do
subject { merge_requests_group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -67,6 +70,7 @@ describe 'Internal Group access', feature: true do
subject { group_group_members_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -82,6 +86,7 @@ describe 'Internal Group access', feature: true do
subject { edit_group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_denied_for(:auditor) }
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) }
......
......@@ -22,6 +22,7 @@ describe 'Private Group access', feature: true do
subject { group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -37,6 +38,7 @@ describe 'Private Group access', feature: true do
subject { issues_group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -52,6 +54,7 @@ describe 'Private Group access', feature: true do
subject { merge_requests_group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -67,6 +70,7 @@ describe 'Private Group access', feature: true do
subject { group_group_members_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -82,6 +86,7 @@ describe 'Private Group access', feature: true do
subject { edit_group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_denied_for(:auditor) }
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) }
......
......@@ -22,6 +22,7 @@ describe 'Public Group access', feature: true do
subject { group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -37,6 +38,7 @@ describe 'Public Group access', feature: true do
subject { issues_group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -52,6 +54,7 @@ describe 'Public Group access', feature: true do
subject { merge_requests_group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -67,6 +70,7 @@ describe 'Public Group access', feature: true do
subject { group_group_members_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
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) }
......@@ -82,6 +86,7 @@ describe 'Public Group access', feature: true do
subject { edit_group_path(group) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_denied_for(:auditor) }
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) }
......
......@@ -8,6 +8,7 @@ describe "Profile access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
......@@ -16,6 +17,7 @@ describe "Profile access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
......@@ -24,6 +26,7 @@ describe "Profile access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
......@@ -32,6 +35,7 @@ describe "Profile access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
......@@ -40,6 +44,7 @@ describe "Profile access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
......@@ -48,6 +53,7 @@ describe "Profile access", feature: true do
it { is_expected.to be_allowed_for :admin }
it { is_expected.to be_allowed_for :user }
it { is_expected.to be_allowed_for :auditor }
it { is_expected.to be_denied_for :visitor }
end
end
......@@ -12,6 +12,7 @@ describe "Internal Project Snippets Access", feature: true do
subject { namespace_project_snippets_path(project.namespace, project) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -26,6 +27,7 @@ describe "Internal Project Snippets Access", feature: true do
subject { new_namespace_project_snippet_path(project.namespace, project) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_denied_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -41,6 +43,7 @@ describe "Internal Project Snippets Access", feature: true do
subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -55,6 +58,7 @@ describe "Internal Project Snippets Access", feature: true do
subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -71,6 +75,7 @@ describe "Internal Project Snippets Access", feature: true do
subject { raw_namespace_project_snippet_path(project.namespace, project, internal_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -85,6 +90,7 @@ describe "Internal Project Snippets Access", feature: true do
subject { raw_namespace_project_snippet_path(project.namespace, project, private_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......
......@@ -11,6 +11,7 @@ describe "Private Project Snippets Access", feature: true do
subject { namespace_project_snippets_path(project.namespace, project) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -25,6 +26,7 @@ describe "Private Project Snippets Access", feature: true do
subject { new_namespace_project_snippet_path(project.namespace, project) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_denied_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -39,6 +41,7 @@ describe "Private Project Snippets Access", feature: true do
subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -53,6 +56,7 @@ describe "Private Project Snippets Access", feature: true do
subject { raw_namespace_project_snippet_path(project.namespace, project, private_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......
......@@ -13,6 +13,7 @@ describe "Public Project Snippets Access", feature: true do
subject { namespace_project_snippets_path(project.namespace, project) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -27,6 +28,7 @@ describe "Public Project Snippets Access", feature: true do
subject { new_namespace_project_snippet_path(project.namespace, project) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_denied_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -42,6 +44,7 @@ describe "Public Project Snippets Access", feature: true do
subject { namespace_project_snippet_path(project.namespace, project, public_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -56,6 +59,7 @@ describe "Public Project Snippets Access", feature: true do
subject { namespace_project_snippet_path(project.namespace, project, internal_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -70,6 +74,7 @@ describe "Public Project Snippets Access", feature: true do
subject { namespace_project_snippet_path(project.namespace, project, private_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -86,6 +91,7 @@ describe "Public Project Snippets Access", feature: true do
subject { raw_namespace_project_snippet_path(project.namespace, project, public_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -100,6 +106,7 @@ describe "Public Project Snippets Access", feature: true do
subject { raw_namespace_project_snippet_path(project.namespace, project, internal_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......@@ -114,6 +121,7 @@ describe "Public Project Snippets Access", feature: true do
subject { raw_namespace_project_snippet_path(project.namespace, project, private_snippet) }
it { is_expected.to be_allowed_for(:admin) }
it { is_expected.to be_allowed_for(:auditor) }
it { is_expected.to be_allowed_for(:owner).of(project) }
it { is_expected.to be_allowed_for(:master).of(project) }
it { is_expected.to be_allowed_for(:developer).of(project) }
......
......@@ -15,6 +15,8 @@ module AccessMatchers
logout
when :admin
login_as(create(:admin))
when :auditor
login_as(create(:user, :auditor))
when :external
login_as(create(:user, external: true))
when User
......
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