summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/helpers/external_wiki_helper.rb12
-rw-r--r--app/helpers/projects_helper.rb13
-rw-r--r--app/policies/project_policy.rb2
-rw-r--r--app/views/layouts/nav/sidebar/_project.html.haml21
-rw-r--r--app/views/projects/blob/viewers/_readme.html.haml2
-rw-r--r--app/views/projects/wikis/pages.html.haml2
-rw-r--r--app/views/projects/wikis/show.html.haml2
-rw-r--r--changelogs/unreleased/security-fix-wiki-access-rights-with-external-wiki-enabled.yml5
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/helpers/projects_helper_spec.rb20
-rw-r--r--spec/models/ability_spec.rb1
-rw-r--r--spec/models/project_services/external_wiki_service_spec.rb21
-rw-r--r--spec/policies/project_policy_spec.rb24
-rw-r--r--spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb54
14 files changed, 131 insertions, 51 deletions
diff --git a/app/helpers/external_wiki_helper.rb b/app/helpers/external_wiki_helper.rb
deleted file mode 100644
index e36d63b2946..00000000000
--- a/app/helpers/external_wiki_helper.rb
+++ /dev/null
@@ -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
diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb
index eceee054ede..b0d3d509f5d 100644
--- a/app/helpers/projects_helper.rb
+++ b/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,
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index d90bb647018..95ae85ed60c 100644
--- a/app/policies/project_policy.rb
+++ b/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
diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml
index 207c08ee5bb..dd7833647b7 100644
--- a/app/views/layouts/nav/sidebar/_project.html.haml
+++ b/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
diff --git a/app/views/projects/blob/viewers/_readme.html.haml b/app/views/projects/blob/viewers/_readme.html.haml
index d8492abc638..c2329a7aa66 100644
--- a/app/views/projects/blob/viewers/_readme.html.haml
+++ b/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)
diff --git a/app/views/projects/wikis/pages.html.haml b/app/views/projects/wikis/pages.html.haml
index aeef64fd7eb..94267b6e0cf 100644
--- a/app/views/projects/wikis/pages.html.haml
+++ b/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")
diff --git a/app/views/projects/wikis/show.html.haml b/app/views/projects/wikis/show.html.haml
index 4d5fd55364c..8b348bb4e4f 100644
--- a/app/views/projects/wikis/show.html.haml
+++ b/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" }
diff --git a/changelogs/unreleased/security-fix-wiki-access-rights-with-external-wiki-enabled.yml b/changelogs/unreleased/security-fix-wiki-access-rights-with-external-wiki-enabled.yml
new file mode 100644
index 00000000000..d5f20b87a90
--- /dev/null
+++ b/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
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 5b794ae587b..bb98fc06ed6 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3151,6 +3151,9 @@ msgstr ""
msgid "External URL"
msgstr ""
+msgid "External Wiki"
+msgstr ""
+
msgid "Facebook"
msgstr ""
diff --git a/spec/helpers/projects_helper_spec.rb b/spec/helpers/projects_helper_spec.rb
index 88b5d87f087..c2dd666f9df 100644
--- a/spec/helpers/projects_helper_spec.rb
+++ b/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
diff --git a/spec/models/ability_spec.rb b/spec/models/ability_spec.rb
index 199f49d0bf2..eee80e9bad7 100644
--- a/spec/models/ability_spec.rb
+++ b/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)
diff --git a/spec/models/project_services/external_wiki_service_spec.rb b/spec/models/project_services/external_wiki_service_spec.rb
index 25e6ce7e804..62fd97b038b 100644
--- a/spec/models/project_services/external_wiki_service_spec.rb
+++ b/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
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index c96e35b872c..f9f8ef18948 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/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
diff --git a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb b/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb
index 2852aa380b2..d9f05e5f94f 100644
--- a/spec/views/layouts/nav/sidebar/_project.html.haml_spec.rb
+++ b/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
View Lorry Controller Status