From c03ae6201f5480c179acb26ba06e3824a2cb7aad Mon Sep 17 00:00:00 2001 From: Alex Kalderimis Date: Tue, 23 Jul 2019 22:40:23 +0000 Subject: [PATCH] Allowing all users to view history This removes the create_wiki permission check from the history controller, allowing show and history to have the same level of permissions. Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/29528 --- app/controllers/projects/wikis_controller.rb | 2 +- .../allow-all-users-to-see-history.yml | 4 + .../projects/wikis_controller_spec.rb | 41 ++++++ spec/policies/project_policy_spec.rb | 120 ++++++++++++++++++ 4 files changed, 166 insertions(+), 1 deletion(-) create mode 100644 changelogs/unreleased/allow-all-users-to-see-history.yml diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index b0998d7f3be..d1914c35bd3 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -6,7 +6,7 @@ class Projects::WikisController < Projects::ApplicationController include Gitlab::Utils::StrongMemoize before_action :authorize_read_wiki! - before_action :authorize_create_wiki!, only: [:edit, :create, :history] + before_action :authorize_create_wiki!, only: [:edit, :create] before_action :authorize_admin_wiki!, only: :destroy before_action :load_project_wiki before_action :load_page, only: [:show, :edit, :update, :history, :destroy] diff --git a/changelogs/unreleased/allow-all-users-to-see-history.yml b/changelogs/unreleased/allow-all-users-to-see-history.yml new file mode 100644 index 00000000000..7423fa079cc --- /dev/null +++ b/changelogs/unreleased/allow-all-users-to-see-history.yml @@ -0,0 +1,4 @@ +--- +title: Align access permissions for wiki history to those of wiki pages +merge_request: 30470 +type: fixed diff --git a/spec/controllers/projects/wikis_controller_spec.rb b/spec/controllers/projects/wikis_controller_spec.rb index a7e5a79b51d..fbca1d5740f 100644 --- a/spec/controllers/projects/wikis_controller_spec.rb +++ b/spec/controllers/projects/wikis_controller_spec.rb @@ -31,6 +31,47 @@ describe Projects::WikisController do end end + describe 'GET #history' do + before do + allow(controller) + .to receive(:can?) + .with(any_args) + .and_call_original + + # The :create_wiki permission is irrelevant to reading history. + expect(controller) + .not_to receive(:can?) + .with(anything, :create_wiki, any_args) + + allow(controller) + .to receive(:can?) + .with(anything, :read_wiki, any_args) + .and_return(allow_read_wiki) + end + + shared_examples 'fetching history' do |expected_status| + before do + get :history, params: { namespace_id: project.namespace, project_id: project, id: wiki_title } + end + + it "returns status #{expected_status}" do + expect(response).to have_http_status(expected_status) + end + end + + it_behaves_like 'fetching history', :ok do + let(:allow_read_wiki) { true } + + it 'assigns @page_versions' do + expect(assigns(:page_versions)).to be_present + end + end + + it_behaves_like 'fetching history', :not_found do + let(:allow_read_wiki) { false } + end + end + describe 'GET #show' do render_views diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index fd82150c12a..8fd54e0bf1d 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -126,6 +126,126 @@ describe ProjectPolicy do end end end + + describe 'read_wiki' do + subject { described_class.new(user, project) } + + member_roles = %i[guest developer] + stranger_roles = %i[anonymous non_member] + + user_roles = stranger_roles + member_roles + + # When a user is anonymous, their `current_user == nil` + let(:user) { create(:user) unless user_role == :anonymous } + + before do + project.visibility = project_visibility + project.project_feature.update_attribute(:wiki_access_level, wiki_access_level) + project.add_user(user, user_role) if member_roles.include?(user_role) + end + + title = ->(project_visibility, wiki_access_level, user_role) do + [ + "project is #{Gitlab::VisibilityLevel.level_name project_visibility}", + "wiki is #{ProjectFeature.str_from_access_level wiki_access_level}", + "user is #{user_role}" + ].join(', ') + end + + describe 'Situations where :read_wiki is always false' do + where(case_names: title, + project_visibility: Gitlab::VisibilityLevel.options.values, + wiki_access_level: [ProjectFeature::DISABLED], + user_role: user_roles) + + with_them do + it { is_expected.to be_disallowed(:read_wiki) } + end + end + + describe 'Situations where :read_wiki is always true' do + where(case_names: title, + project_visibility: [Gitlab::VisibilityLevel::PUBLIC], + wiki_access_level: [ProjectFeature::ENABLED], + user_role: user_roles) + + with_them do + it { is_expected.to be_allowed(:read_wiki) } + end + end + + describe 'Situations where :read_wiki requires project membership' do + context 'the wiki is private, and the user is a member' do + where(case_names: title, + project_visibility: [Gitlab::VisibilityLevel::PUBLIC, + Gitlab::VisibilityLevel::INTERNAL], + wiki_access_level: [ProjectFeature::PRIVATE], + user_role: member_roles) + + with_them do + it { is_expected.to be_allowed(:read_wiki) } + end + end + + context 'the wiki is private, and the user is not member' do + where(case_names: title, + project_visibility: [Gitlab::VisibilityLevel::PUBLIC, + Gitlab::VisibilityLevel::INTERNAL], + wiki_access_level: [ProjectFeature::PRIVATE], + user_role: stranger_roles) + + with_them do + it { is_expected.to be_disallowed(:read_wiki) } + end + end + + context 'the wiki is enabled, and the user is a member' do + where(case_names: title, + project_visibility: [Gitlab::VisibilityLevel::PRIVATE], + wiki_access_level: [ProjectFeature::ENABLED], + user_role: member_roles) + + with_them do + it { is_expected.to be_allowed(:read_wiki) } + end + end + + context 'the wiki is enabled, and the user is not a member' do + where(case_names: title, + project_visibility: [Gitlab::VisibilityLevel::PRIVATE], + wiki_access_level: [ProjectFeature::ENABLED], + user_role: stranger_roles) + + with_them do + it { is_expected.to be_disallowed(:read_wiki) } + end + end + end + + describe 'Situations where :read_wiki prohibits anonymous access' do + context 'the user is not anonymous' do + where(case_names: title, + project_visibility: [Gitlab::VisibilityLevel::INTERNAL], + wiki_access_level: [ProjectFeature::ENABLED, ProjectFeature::PUBLIC], + user_role: user_roles.reject { |u| u == :anonymous }) + + with_them do + it { is_expected.to be_allowed(:read_wiki) } + end + end + + context 'the user is not anonymous' do + where(case_names: title, + project_visibility: [Gitlab::VisibilityLevel::INTERNAL], + wiki_access_level: [ProjectFeature::ENABLED, ProjectFeature::PUBLIC], + user_role: %i[anonymous]) + + with_them do + it { is_expected.to be_disallowed(:read_wiki) } + end + end + end + end end context 'issues feature' do