Fixed bug when external wiki is enabled

When the external wiki is enabled, the internal wiki link is replaced
by the external wiki url. But the internal wiki is still accessible.
In this change the external wiki will have its own tab in the sidebar
and only if the services are disabled the tab (and access rights)
will not be displayed.

app/helpers/external_wiki_helper.rb

@@ -1,12 +0,0 @@
-# frozen_string_literal: true
-
-module ExternalWikiHelper
-  def get_project_wiki_path(project)
-    external_wiki_service = project.external_wiki
-    if external_wiki_service
-      external_wiki_service.properties['external_wiki_url']
-    else
-      project_wiki_path(project, :home)
-    end
-  end
-end

app/helpers/projects_helper.rb

@@ -313,19 +313,24 @@ module ProjectsHelper
       nav_tabs << :operations
     end
 
-    if project.external_issue_tracker
-      nav_tabs << :external_issue_tracker
-    end
-
     tab_ability_map.each do |tab, ability|
       if can?(current_user, ability, project)
         nav_tabs << tab
       end
     end
 
+    nav_tabs << external_nav_tabs(project)
+
     nav_tabs.flatten
   end
 
+  def external_nav_tabs(project)
+    [].tap do |tabs|
+      tabs << :external_issue_tracker if project.external_issue_tracker
+      tabs << :external_wiki if project.has_external_wiki?
+    end
+  end
+
   def tab_ability_map
     {
       environments:     :read_environment,

app/policies/project_policy.rb

@@ -314,7 +314,7 @@ class ProjectPolicy < BasePolicy
     prevent(*create_read_update_admin_destroy(:project_snippet))
   end
 
-  rule { wiki_disabled & ~has_external_wiki }.policy do
+  rule { wiki_disabled }.policy do
     prevent(*create_read_update_admin_destroy(:wiki))
     prevent(:download_wiki_code)
   end

app/views/layouts/nav/sidebar/_project.html.haml

@@ -281,19 +281,34 @@
                 %strong.fly-out-top-item-name
                   = _('Registry')
 
-      - if project_nav_tab? :wiki
+      - if project_nav_tab?(:wiki)
+        - wiki_url = project_wiki_path(@project, :home)
         = nav_link(controller: :wikis) do
-          = link_to get_project_wiki_path(@project), class: 'shortcuts-wiki qa-wiki-link' do
+          = link_to wiki_url, class: 'shortcuts-wiki qa-wiki-link' do
             .nav-icon-container
               = sprite_icon('book')
             %span.nav-item-name
               = _('Wiki')
           %ul.sidebar-sub-level-items.is-fly-out-only
             = nav_link(controller: :wikis, html_options: { class: "fly-out-top-item" } ) do
-              = link_to get_project_wiki_path(@project) do
+              = link_to wiki_url do
                 %strong.fly-out-top-item-name
                   = _('Wiki')
 
+      - if project_nav_tab?(:external_wiki)
+        - external_wiki_url = @project.external_wiki.external_wiki_url
+        = nav_link do
+          = link_to external_wiki_url, class: 'shortcuts-external_wiki' do
+            .nav-icon-container
+              = sprite_icon('issue-external')
+            %span.nav-item-name
+              = _('External Wiki')
+          %ul.sidebar-sub-level-items.is-fly-out-only
+            = nav_link(html_options: { class: "fly-out-top-item" } ) do
+              = link_to external_wiki_url do
+                %strong.fly-out-top-item-name
+                  = _('External Wiki')
+
       - if project_nav_tab? :snippets
         = nav_link(controller: :snippets) do
           = link_to project_snippets_path(@project), class: 'shortcuts-snippets' do

app/views/projects/blob/viewers/_readme.html.haml

@@ -1,4 +1,4 @@
 = icon('info-circle fw')
 = succeed '.' do
   To learn more about this project, read
-  = link_to "the wiki", get_project_wiki_path(viewer.project)
+  = link_to "the wiki", project_wiki_path(viewer.project, :home)

app/views/projects/wikis/pages.html.haml

@@ -1,5 +1,5 @@
 - @no_container = true
-- add_to_breadcrumbs "Wiki", get_project_wiki_path(@project)
+- add_to_breadcrumbs "Wiki", project_wiki_path(@project, :home)
 - breadcrumb_title s_("Wiki|Pages")
 - page_title s_("Wiki|Pages"), _("Wiki")
 

app/views/projects/wikis/show.html.haml

@@ -2,7 +2,7 @@
 - breadcrumb_title @page.human_title
 - wiki_breadcrumb_dropdown_links(@page.slug)
 - page_title @page.human_title, _("Wiki")
-- add_to_breadcrumbs _("Wiki"), get_project_wiki_path(@project)
+- add_to_breadcrumbs _("Wiki"), project_wiki_path(@project, :home)
 
 .wiki-page-header.has-sidebar-toggle
   %button.btn.btn-default.sidebar-toggle.js-sidebar-wiki-toggle{ role: "button", type: "button" }

changelogs/unreleased/security-fix-wiki-access-rights-with-external-wiki-enabled.yml

@@ -0,0 +1,5 @@
+---
+title: Fix wiki access rights when external wiki is enabled
+merge_request:
+author:
+type: security

locale/gitlab.pot

@@ -3151,6 +3151,9 @@ msgstr ""
 msgid "External URL"
 msgstr ""
 
+msgid "External Wiki"
+msgstr ""
+
 msgid "Facebook"
 msgstr ""
 

spec/helpers/projects_helper_spec.rb

@@ -358,6 +358,26 @@ describe ProjectsHelper do
         is_expected.not_to include(:pipelines)
       end
     end
+
+    context 'when project has external wiki' do
+      before do
+        allow(project).to receive(:has_external_wiki?).and_return(true)
+      end
+
+      it 'includes external wiki tab' do
+        is_expected.to include(:external_wiki)
+      end
+    end
+
+    context 'when project does not have external wiki' do
+      before do
+        allow(project).to receive(:has_external_wiki?).and_return(false)
+      end
+
+      it 'does not include external wiki tab' do
+        is_expected.not_to include(:external_wiki)
+      end
+    end
   end
 
   describe '#show_projects' do

spec/models/ability_spec.rb

@@ -298,7 +298,6 @@ describe Ability do
 
     context 'wiki named abilities' do
       it 'disables wiki abilities if the project has no wiki' do
-        expect(project).to receive(:has_external_wiki?).and_return(false)
         expect(subject).not_to be_allowed(:read_wiki)
         expect(subject).not_to be_allowed(:create_wiki)
         expect(subject).not_to be_allowed(:update_wiki)

spec/models/project_services/external_wiki_service_spec.rb

@@ -1,7 +1,6 @@
 require 'spec_helper'
 
 describe ExternalWikiService do
-  include ExternalWikiHelper
   describe "Associations" do
     it { is_expected.to belong_to :project }
     it { is_expected.to have_one :service_hook }
@@ -25,24 +24,4 @@ describe ExternalWikiService do
       it { is_expected.not_to validate_presence_of(:external_wiki_url) }
     end
   end
-
-  describe 'External wiki' do
-    let(:project) { create(:project) }
-
-    context 'when it is active' do
-      before do
-        properties = { 'external_wiki_url' => 'https://gitlab.com' }
-        @service = project.create_external_wiki_service(active: true, properties: properties)
-      end
-
-      after do
-        @service.destroy!
-      end
-
-      it 'replaces the wiki url' do
-        wiki_path = get_project_wiki_path(project)
-        expect(wiki_path).to match('https://gitlab.com')
-      end
-    end
-  end
 end

spec/policies/project_policy_spec.rb

@@ -102,15 +102,27 @@ describe ProjectPolicy do
     expect(Ability).not_to be_allowed(user, :read_issue, project)
   end
 
-  context 'when the feature is disabled' do
+  context 'wiki feature' do
+    let(:permissions) { %i(read_wiki create_wiki update_wiki admin_wiki download_wiki_code) }
+
     subject { described_class.new(owner, project) }
 
-    before do
-      project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED)
-    end
+    context 'when the feature is disabled' do
+      before do
+        project.project_feature.update_attribute(:wiki_access_level, ProjectFeature::DISABLED)
+      end
 
-    it 'does not include the wiki permissions' do
-      expect_disallowed :read_wiki, :create_wiki, :update_wiki, :admin_wiki, :download_wiki_code
+      it 'does not include the wiki permissions' do
+        expect_disallowed(*permissions)
+      end
+
+      context 'when there is an external wiki' do
+        it 'does not include the wiki permissions' do
+          allow(project).to receive(:has_external_wiki?).and_return(true)
+
+          expect_disallowed(*permissions)
+        end
+      end
     end
   end
 

spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb

@@ -57,4 +57,58 @@ describe 'layouts/nav/sidebar/_project' do
       expect(rendered).to have_link('Releases', href: project_releases_path(project))
     end
   end
+
+  describe 'wiki entry tab' do
+    let(:can_read_wiki) { true }
+
+    before do
+      allow(view).to receive(:can?).with(nil, :read_wiki, project).and_return(can_read_wiki)
+    end
+
+    describe 'when wiki is enabled' do
+      it 'shows the wiki tab with the wiki internal link' do
+        render
+
+        expect(rendered).to have_link('Wiki', href: project_wiki_path(project, :home))
+      end
+    end
+
+    describe 'when wiki is disabled' do
+      let(:can_read_wiki) { false }
+
+      it 'does not show the wiki tab' do
+        render
+
+        expect(rendered).not_to have_link('Wiki', href: project_wiki_path(project, :home))
+      end
+    end
+  end
+
+  describe 'external wiki entry tab' do
+    let(:properties) { { 'external_wiki_url' => 'https://gitlab.com' } }
+    let(:service_status) { true }
+
+    before do
+      project.create_external_wiki_service(active: service_status, properties: properties)
+      project.reload
+    end
+
+    context 'when it is active' do
+      it 'shows the external wiki tab with the external wiki service link' do
+        render
+
+        expect(rendered).to have_link('External Wiki', href: properties['external_wiki_url'])
+      end
+    end
+
+    context 'when it is disabled' do
+      let(:service_status) { false }
+
+      it 'does not show the external wiki tab' do
+        render
+
+        expect(rendered).not_to have_link('External Wiki', href: project_wiki_path(project, :home))
+      end
+    end
+  end
 end