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

4 files changed, 166 insertions, 1 deletions

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