From 893b1eb1d3290a662a01188d2055798778bc442a Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 22 May 2017 19:51:09 +0300 Subject: [PATCH 1/4] Fix: Wiki is not searchable with Guest permissions --- app/services/search_service.rb | 2 +- app/views/search/_category.html.haml | 77 +++++++++++++++------------- spec/services/search_service_spec.rb | 9 ++++ 3 files changed, 52 insertions(+), 36 deletions(-) diff --git a/app/services/search_service.rb b/app/services/search_service.rb index 22736c71725..1d4d03a8b7d 100644 --- a/app/services/search_service.rb +++ b/app/services/search_service.rb @@ -12,7 +12,7 @@ class SearchService @project = if params[:project_id].present? the_project = Project.find_by(id: params[:project_id]) - can?(current_user, :download_code, the_project) ? the_project : nil + can?(current_user, :read_project, the_project) ? the_project : nil else nil end diff --git a/app/views/search/_category.html.haml b/app/views/search/_category.html.haml index 059a0d1ac78..7ec4aa9998f 100644 --- a/app/views/search/_category.html.haml +++ b/app/views/search/_category.html.haml @@ -3,41 +3,48 @@ .fade-right= icon('angle-right') %ul.nav-links.search-filter.scrolling-tabs - if @project - %li{ class: active_when(@scope == 'blobs') } - = link_to search_filter_path(scope: 'blobs') do - Code - %span.badge - = @search_results.blobs_count - %li{ class: active_when(@scope == 'issues') } - = link_to search_filter_path(scope: 'issues') do - Issues - %span.badge - = @search_results.issues_count - %li{ class: active_when(@scope == 'merge_requests') } - = link_to search_filter_path(scope: 'merge_requests') do - Merge requests - %span.badge - = @search_results.merge_requests_count - %li{ class: active_when(@scope == 'milestones') } - = link_to search_filter_path(scope: 'milestones') do - Milestones - %span.badge - = @search_results.milestones_count - %li{ class: active_when(@scope == 'notes') } - = link_to search_filter_path(scope: 'notes') do - Comments - %span.badge - = @search_results.notes_count - %li{ class: active_when(@scope == 'wiki_blobs') } - = link_to search_filter_path(scope: 'wiki_blobs') do - Wiki - %span.badge - = @search_results.wiki_blobs_count - %li{ class: active_when(@scope == 'commits') } - = link_to search_filter_path(scope: 'commits') do - Commits - %span.badge - = @search_results.commits_count + - if can?(current_user, :download_code, @project) + %li{ class: active_when(@scope == 'blobs') } + = link_to search_filter_path(scope: 'blobs') do + Code + %span.badge + = @search_results.blobs_count + - if can?(current_user, :read_issue, @project) + %li{ class: active_when(@scope == 'issues') } + = link_to search_filter_path(scope: 'issues') do + Issues + %span.badge + = @search_results.issues_count + - if can?(current_user, :read_merge_request, @project) + %li{ class: active_when(@scope == 'merge_requests') } + = link_to search_filter_path(scope: 'merge_requests') do + Merge requests + %span.badge + = @search_results.merge_requests_count + - if can?(current_user, :read_milestone, @project) + %li{ class: active_when(@scope == 'milestones') } + = link_to search_filter_path(scope: 'milestones') do + Milestones + %span.badge + = @search_results.milestones_count + - if can?(current_user, :read_merge_request, @project) || can?(current_user, :read_issue, @project) + %li{ class: active_when(@scope == 'notes') } + = link_to search_filter_path(scope: 'notes') do + Comments + %span.badge + = @search_results.notes_count + - if can?(current_user, :read_wiki, @project) + %li{ class: active_when(@scope == 'wiki_blobs') } + = link_to search_filter_path(scope: 'wiki_blobs') do + Wiki + %span.badge + = @search_results.wiki_blobs_count + - if can?(current_user, :download_code, @project) + %li{ class: active_when(@scope == 'commits') } + = link_to search_filter_path(scope: 'commits') do + Commits + %span.badge + = @search_results.commits_count - elsif @show_snippets %li{ class: active_when(@scope == 'snippet_blobs') } diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb index 2112f1cf9ea..694124a8be3 100644 --- a/spec/services/search_service_spec.rb +++ b/spec/services/search_service_spec.rb @@ -26,6 +26,15 @@ describe SearchService, services: true do expect(project).to eq accessible_project end + + it 'returns the project for guests' do + search_project = create :empty_project + search_project.team << [user, :guest] + + project = SearchService.new(user, project_id: search_project.id).project + + expect(project).to eq search_project + end end context 'when the project is not accessible' do From 7487d06c4ba7f186fddc11aa39ab66e8cb4ccc14 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 22 May 2017 19:51:18 +0300 Subject: [PATCH 2/4] update changelog --- .../30917-wiki-is-not-searchable-with-guest-permissions.yml | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelogs/unreleased/30917-wiki-is-not-searchable-with-guest-permissions.yml diff --git a/changelogs/unreleased/30917-wiki-is-not-searchable-with-guest-permissions.yml b/changelogs/unreleased/30917-wiki-is-not-searchable-with-guest-permissions.yml new file mode 100644 index 00000000000..c9bd2dc465e --- /dev/null +++ b/changelogs/unreleased/30917-wiki-is-not-searchable-with-guest-permissions.yml @@ -0,0 +1,4 @@ +--- +title: 'Fix: Wiki is not searchable with Guest permissions' +merge_request: +author: From 1a424a9bc9cf1fbabd70ac2384978ae94674e6d7 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Tue, 23 May 2017 13:40:18 +0300 Subject: [PATCH 3/4] Explicitly test that guest is able to search through the wiki --- app/helpers/search_helper.rb | 6 ++++++ app/views/search/_category.html.haml | 2 +- spec/lib/gitlab/project_search_results_spec.rb | 4 ++-- spec/services/search_service_spec.rb | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 9c46035057f..9021525784d 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -33,6 +33,12 @@ module SearchHelper def parse_search_result(result) Gitlab::ProjectSearchResults.parse_search_result(result) end + + def show_notes_tab? + [:read_merge_request, :download_code, :read_issue, :read_project_snippet].all? do |ability| + can?(current_user, :read_merge_request, @project) + end + end private diff --git a/app/views/search/_category.html.haml b/app/views/search/_category.html.haml index 7ec4aa9998f..df73fb173a8 100644 --- a/app/views/search/_category.html.haml +++ b/app/views/search/_category.html.haml @@ -27,7 +27,7 @@ Milestones %span.badge = @search_results.milestones_count - - if can?(current_user, :read_merge_request, @project) || can?(current_user, :read_issue, @project) + - if show_notes_tab? %li{ class: active_when(@scope == 'notes') } = link_to search_filter_path(scope: 'notes') do Comments diff --git a/spec/lib/gitlab/project_search_results_spec.rb b/spec/lib/gitlab/project_search_results_spec.rb index 1b8690ba613..3d22784909d 100644 --- a/spec/lib/gitlab/project_search_results_spec.rb +++ b/spec/lib/gitlab/project_search_results_spec.rb @@ -123,8 +123,8 @@ describe Gitlab::ProjectSearchResults, lib: true do context 'when wiki is internal' do let(:project) { create(:project, :public, :wiki_private) } - it 'finds wiki blobs for members' do - project.add_reporter(user) + it 'finds wiki blobs for guest' do + project.add_guest(user) is_expected.not_to be_empty end diff --git a/spec/services/search_service_spec.rb b/spec/services/search_service_spec.rb index 694124a8be3..5cf989105d0 100644 --- a/spec/services/search_service_spec.rb +++ b/spec/services/search_service_spec.rb @@ -29,7 +29,7 @@ describe SearchService, services: true do it 'returns the project for guests' do search_project = create :empty_project - search_project.team << [user, :guest] + search_project.add_guest(user) project = SearchService.new(user, project_id: search_project.id).project From be9ffbafbba1b105bba3102cf7b5d93296478223 Mon Sep 17 00:00:00 2001 From: Valery Sizov Date: Mon, 29 May 2017 16:52:02 +0300 Subject: [PATCH 4/4] Create a separate helper to check if we show particular tab on a search page --- app/helpers/projects_helper.rb | 33 +++++++++++++++++++++------- app/helpers/search_helper.rb | 6 ----- app/views/search/_category.html.haml | 14 ++++++------ 3 files changed, 32 insertions(+), 21 deletions(-) diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index 98bbcfaaba5..835473430c8 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -85,6 +85,12 @@ module ProjectsHelper @nav_tabs ||= get_project_nav_tabs(@project, current_user) end + def project_search_tabs?(tab) + abilities = Array(search_tab_ability_map[tab]) + + abilities.any? { |ability| can?(current_user, ability, @project) } + end + def project_nav_tab?(name) project_nav_tabs.include? name end @@ -203,7 +209,17 @@ module ProjectsHelper nav_tabs << :container_registry end - tab_ability_map = { + tab_ability_map.each do |tab, ability| + if can?(current_user, ability, project) + nav_tabs << tab + end + end + + nav_tabs.flatten + end + + def tab_ability_map + { environments: :read_environment, milestones: :read_milestone, pipelines: :read_pipeline, @@ -215,14 +231,15 @@ module ProjectsHelper team: :read_project_member, wiki: :read_wiki } + end - tab_ability_map.each do |tab, ability| - if can?(current_user, ability, project) - nav_tabs << tab - end - end - - nav_tabs.flatten + def search_tab_ability_map + @search_tab_ability_map ||= tab_ability_map.merge( + blobs: :download_code, + commits: :download_code, + merge_requests: :read_merge_request, + notes: [:read_merge_request, :download_code, :read_issue, :read_project_snippet] + ) end def project_lfs_status(project) diff --git a/app/helpers/search_helper.rb b/app/helpers/search_helper.rb index 9021525784d..9c46035057f 100644 --- a/app/helpers/search_helper.rb +++ b/app/helpers/search_helper.rb @@ -33,12 +33,6 @@ module SearchHelper def parse_search_result(result) Gitlab::ProjectSearchResults.parse_search_result(result) end - - def show_notes_tab? - [:read_merge_request, :download_code, :read_issue, :read_project_snippet].all? do |ability| - can?(current_user, :read_merge_request, @project) - end - end private diff --git a/app/views/search/_category.html.haml b/app/views/search/_category.html.haml index df73fb173a8..314d8e9cb25 100644 --- a/app/views/search/_category.html.haml +++ b/app/views/search/_category.html.haml @@ -3,43 +3,43 @@ .fade-right= icon('angle-right') %ul.nav-links.search-filter.scrolling-tabs - if @project - - if can?(current_user, :download_code, @project) + - if project_search_tabs?(:blobs) %li{ class: active_when(@scope == 'blobs') } = link_to search_filter_path(scope: 'blobs') do Code %span.badge = @search_results.blobs_count - - if can?(current_user, :read_issue, @project) + - if project_search_tabs?(:issues) %li{ class: active_when(@scope == 'issues') } = link_to search_filter_path(scope: 'issues') do Issues %span.badge = @search_results.issues_count - - if can?(current_user, :read_merge_request, @project) + - if project_search_tabs?(:merge_requests) %li{ class: active_when(@scope == 'merge_requests') } = link_to search_filter_path(scope: 'merge_requests') do Merge requests %span.badge = @search_results.merge_requests_count - - if can?(current_user, :read_milestone, @project) + - if project_search_tabs?(:milestones) %li{ class: active_when(@scope == 'milestones') } = link_to search_filter_path(scope: 'milestones') do Milestones %span.badge = @search_results.milestones_count - - if show_notes_tab? + - if project_search_tabs?(:notes) %li{ class: active_when(@scope == 'notes') } = link_to search_filter_path(scope: 'notes') do Comments %span.badge = @search_results.notes_count - - if can?(current_user, :read_wiki, @project) + - if project_search_tabs?(:wiki) %li{ class: active_when(@scope == 'wiki_blobs') } = link_to search_filter_path(scope: 'wiki_blobs') do Wiki %span.badge = @search_results.wiki_blobs_count - - if can?(current_user, :download_code, @project) + - if project_search_tabs?(:commits) %li{ class: active_when(@scope == 'commits') } = link_to search_filter_path(scope: 'commits') do Commits