Commit edab3aa9 authored by Dylan Griffith's avatar Dylan Griffith Committed by Sean McGivern

Revert "Revert "Merge branch...""

This reverts commit 5c234b63.
parent 3cae13f0
...@@ -15,6 +15,7 @@ class SearchController < ApplicationController ...@@ -15,6 +15,7 @@ class SearchController < ApplicationController
around_action :allow_gitaly_ref_name_caching around_action :allow_gitaly_ref_name_caching
before_action :block_anonymous_global_searches
skip_before_action :authenticate_user! skip_before_action :authenticate_user!
requires_cross_project_access if: -> do requires_cross_project_access if: -> do
search_term_present = params[:search].present? || params[:term].present? search_term_present = params[:search].present? || params[:term].present?
...@@ -128,6 +129,16 @@ class SearchController < ApplicationController ...@@ -128,6 +129,16 @@ class SearchController < ApplicationController
payload[:metadata]['meta.search.search'] = params[:search] payload[:metadata]['meta.search.search'] = params[:search]
payload[:metadata]['meta.search.scope'] = params[:scope] payload[:metadata]['meta.search.scope'] = params[:scope]
end end
def block_anonymous_global_searches
return if params[:project_id].present? || params[:group_id].present?
return if current_user
return unless ::Feature.enabled?(:block_anonymous_global_searches)
store_location_for(:user, request.fullpath)
redirect_to new_user_session_path, alert: _('You must be logged in to search across all of GitLab')
end
end end
SearchController.prepend_if_ee('EE::SearchController') SearchController.prepend_if_ee('EE::SearchController')
---
name: block_anonymous_global_searches
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41041
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/244276
group: group::global search
type: development
default_enabled: false
\ No newline at end of file
...@@ -244,7 +244,7 @@ module EE ...@@ -244,7 +244,7 @@ module EE
when Project when Project
elasticsearch_indexes_project?(scope) elasticsearch_indexes_project?(scope)
else else
false # Never use elasticsearch for the global scope when limiting is on ::Feature.enabled?(:advanced_global_search_for_limited_indexing)
end end
end end
......
---
name: advanced_global_search_for_limited_indexing
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/41041
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/244276
group: group::global search
type: development
default_enabled: false
\ No newline at end of file
...@@ -266,7 +266,19 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do ...@@ -266,7 +266,19 @@ RSpec.describe 'Global elastic search', :elastic, :sidekiq_inline do
end end
RSpec.describe 'Global elastic search redactions', :elastic do RSpec.describe 'Global elastic search redactions', :elastic do
it_behaves_like 'a redacted search results page' do context 'when block_anonymous_global_searches is disabled' do
let(:search_path) { explore_root_path } before do
stub_feature_flags(block_anonymous_global_searches: false)
end
it_behaves_like 'a redacted search results page' do
let(:search_path) { explore_root_path }
end
end
context 'when block_anonymous_global_searches is enabled' do
it_behaves_like 'a redacted search results page', include_anonymous: false do
let(:search_path) { explore_root_path }
end
end end
end end
...@@ -37,19 +37,31 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si ...@@ -37,19 +37,31 @@ RSpec.describe 'Snippet elastic search', :js, :elastic, :aggregate_failures, :si
context 'as anonymous user' do context 'as anonymous user' do
let(:current_user) { nil } let(:current_user) { nil }
it 'finds only public snippets' do context 'when block_anonymous_global_searches is enabled' do
within('.results') do it 'redirects to login page' do
expect(page).to have_content('public personal snippet') expect(page).to have_content('You must be logged in to search across all of GitLab')
expect(page).not_to have_content('public project snippet') end
end
expect(page).not_to have_content('internal personal snippet') context 'when block_anonymous_global_searches is disabled' do
expect(page).not_to have_content('internal project snippet') before(:context) do
stub_feature_flags(block_anonymous_global_searches: false)
end
expect(page).not_to have_content('authorized personal snippet') it 'finds only public snippets' do
expect(page).not_to have_content('authorized project snippet') within('.results') do
expect(page).to have_content('public personal snippet')
expect(page).not_to have_content('public project snippet')
expect(page).not_to have_content('private personal snippet') expect(page).not_to have_content('internal personal snippet')
expect(page).not_to have_content('private project snippet') expect(page).not_to have_content('internal project snippet')
expect(page).not_to have_content('authorized personal snippet')
expect(page).not_to have_content('authorized project snippet')
expect(page).not_to have_content('private personal snippet')
expect(page).not_to have_content('private project snippet')
end
end end
end end
end end
......
...@@ -407,8 +407,8 @@ RSpec.describe ApplicationSetting do ...@@ -407,8 +407,8 @@ RSpec.describe ApplicationSetting do
end end
describe '#search_using_elasticsearch?' do describe '#search_using_elasticsearch?' do
# Constructs a truth table with 16 entries to run the specs against # Constructs a truth table to run the specs against
where(indexing: [true, false], searching: [true, false], limiting: [true, false]) where(indexing: [true, false], searching: [true, false], limiting: [true, false], advanced_global_search_for_limited_indexing: [true, false])
with_them do with_them do
let_it_be(:included_project_container) { create(:elasticsearch_indexed_project) } let_it_be(:included_project_container) { create(:elasticsearch_indexed_project) }
...@@ -430,12 +430,14 @@ RSpec.describe ApplicationSetting do ...@@ -430,12 +430,14 @@ RSpec.describe ApplicationSetting do
elasticsearch_search: searching, elasticsearch_search: searching,
elasticsearch_limit_indexing: limiting elasticsearch_limit_indexing: limiting
) )
stub_feature_flags(advanced_global_search_for_limited_indexing: advanced_global_search_for_limited_indexing)
end end
context 'global scope' do context 'global scope' do
let(:scope) { nil } let(:scope) { nil }
it { is_expected.to eq(only_when_enabled_globally) } it { is_expected.to eq(indexing && searching && (!limiting || advanced_global_search_for_limited_indexing)) }
end end
context 'namespace (in scope)' do context 'namespace (in scope)' do
......
This diff is collapsed.
...@@ -177,15 +177,37 @@ RSpec.describe Search::GlobalService do ...@@ -177,15 +177,37 @@ RSpec.describe Search::GlobalService do
end end
end end
context 'when ES is not used' do context 'when elasticearch_search is disabled' do
before do before do
stub_ee_application_setting(elasticsearch_limit_indexing: true) stub_ee_application_setting(elasticsearch_search: false)
end end
it 'does not include ES-specific scopes' do it 'does not include ES-specific scopes' do
expect(described_class.new(user, {}).allowed_scopes).not_to include('commits') expect(described_class.new(user, {}).allowed_scopes).not_to include('commits')
end end
end end
context 'when elasticsearch_limit_indexing is enabled' do
before do
stub_ee_application_setting(elasticsearch_limit_indexing: true)
end
context 'when advanced_global_search_for_limited_indexing feature flag is disabled' do
before do
stub_feature_flags(advanced_global_search_for_limited_indexing: false)
end
it 'does not include ES-specific scopes' do
expect(described_class.new(user, {}).allowed_scopes).not_to include('commits')
end
end
context 'when advanced_global_search_for_limited_indexing feature flag is enabled' do
it 'includes ES-specific scopes' do
expect(described_class.new(user, {}).allowed_scopes).to include('commits')
end
end
end
end end
context 'confidential notes' do context 'confidential notes' do
......
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'a redacted search results page' do RSpec.shared_examples 'a redacted search results page' do |include_anonymous: true|
let(:public_group) { create(:group, :public) } let(:public_group) { create(:group, :public) }
let(:public_restricted_project) { create(:project, :repository, :public, :wiki_repo, namespace: public_group, name: 'The Project') } let(:public_restricted_project) { create(:project, :repository, :public, :wiki_repo, namespace: public_group, name: 'The Project') }
let(:issue_access_level) { ProjectFeature::PRIVATE } let(:issue_access_level) { ProjectFeature::PRIVATE }
...@@ -41,7 +41,7 @@ RSpec.shared_examples 'a redacted search results page' do ...@@ -41,7 +41,7 @@ RSpec.shared_examples 'a redacted search results page' do
end end
it_behaves_like 'redacted search results page assertions', true it_behaves_like 'redacted search results page assertions', true
it_behaves_like 'redacted search results page assertions', false it_behaves_like 'redacted search results page assertions', false if include_anonymous
end end
# Only intended to be used in the above shared examples to avoid duplication of # Only intended to be used in the above shared examples to avoid duplication of
......
...@@ -28853,6 +28853,9 @@ msgstr "" ...@@ -28853,6 +28853,9 @@ msgstr ""
msgid "You must accept our Terms of Service and privacy policy in order to register an account" msgid "You must accept our Terms of Service and privacy policy in order to register an account"
msgstr "" msgstr ""
msgid "You must be logged in to search across all of GitLab"
msgstr ""
msgid "You must disassociate %{domain} from all clusters it is attached to before deletion." msgid "You must disassociate %{domain} from all clusters it is attached to before deletion."
msgstr "" msgstr ""
......
...@@ -95,49 +95,77 @@ RSpec.describe SearchController do ...@@ -95,49 +95,77 @@ RSpec.describe SearchController do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
render_views render_views
it 'omits pipeline status from load' do context 'when block_anonymous_global_searches is disabled' do
project = create(:project, :public) before do
expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects) stub_feature_flags(block_anonymous_global_searches: false)
end
get :show, params: { scope: 'projects', search: project.name } it 'omits pipeline status from load' do
project = create(:project, :public)
expect(Gitlab::Cache::Ci::ProjectPipelineStatus).not_to receive(:load_in_batch_for_projects)
expect(assigns[:search_objects].first).to eq project get :show, params: { scope: 'projects', search: project.name }
end
context 'check search term length' do expect(assigns[:search_objects].first).to eq project
let(:search_queries) do
char_limit = SearchService::SEARCH_CHAR_LIMIT
term_limit = SearchService::SEARCH_TERM_LIMIT
{
chars_under_limit: ('a' * (char_limit - 1)),
chars_over_limit: ('a' * (char_limit + 1)),
terms_under_limit: ('abc ' * (term_limit - 1)),
terms_over_limit: ('abc ' * (term_limit + 1))
}
end end
where(:string_name, :expectation) do context 'check search term length' do
:chars_under_limit | :not_to_set_flash let(:search_queries) do
:chars_over_limit | :set_chars_flash char_limit = SearchService::SEARCH_CHAR_LIMIT
:terms_under_limit | :not_to_set_flash term_limit = SearchService::SEARCH_TERM_LIMIT
:terms_over_limit | :set_terms_flash {
end chars_under_limit: ('a' * (char_limit - 1)),
chars_over_limit: ('a' * (char_limit + 1)),
terms_under_limit: ('abc ' * (term_limit - 1)),
terms_over_limit: ('abc ' * (term_limit + 1))
}
end
with_them do where(:string_name, :expectation) do
it do :chars_under_limit | :not_to_set_flash
get :show, params: { scope: 'projects', search: search_queries[string_name] } :chars_over_limit | :set_chars_flash
:terms_under_limit | :not_to_set_flash
case expectation :terms_over_limit | :set_terms_flash
when :not_to_set_flash end
expect(controller).not_to set_flash[:alert]
when :set_chars_flash with_them do
expect(controller).to set_flash[:alert].to(/characters/) it do
when :set_terms_flash get :show, params: { scope: 'projects', search: search_queries[string_name] }
expect(controller).to set_flash[:alert].to(/terms/)
case expectation
when :not_to_set_flash
expect(controller).not_to set_flash[:alert]
when :set_chars_flash
expect(controller).to set_flash[:alert].to(/characters/)
when :set_terms_flash
expect(controller).to set_flash[:alert].to(/terms/)
end
end end
end end
end end
end end
context 'when block_anonymous_global_searches is enabled' do
context 'for unauthenticated user' do
before do
sign_out(user)
end
it 'redirects to login page' do
get :show, params: { scope: 'projects', search: '*' }
expect(response).to redirect_to new_user_session_path
end
end
context 'for authenticated user' do
it 'succeeds' do
get :show, params: { scope: 'projects', search: '*' }
expect(response).to have_gitlab_http_status(:ok)
end
end
end
end end
it 'finds issue comments' do it 'finds issue comments' do
......
...@@ -86,20 +86,33 @@ RSpec.describe 'User searches for issues', :js do ...@@ -86,20 +86,33 @@ RSpec.describe 'User searches for issues', :js do
end end
context 'when signed out' do context 'when signed out' do
let(:project) { create(:project, :public) } context 'when block_anonymous_global_searches is disabled' do
let(:project) { create(:project, :public) }
before do before do
visit(search_path) stub_feature_flags(block_anonymous_global_searches: false)
end visit(search_path)
end
include_examples 'top right search form' include_examples 'top right search form'
it 'finds an issue' do it 'finds an issue' do
search_for_issue(issue1.title) search_for_issue(issue1.title)
page.within('.results') do page.within('.results') do
expect(page).to have_link(issue1.title) expect(page).to have_link(issue1.title)
expect(page).not_to have_link(issue2.title) expect(page).not_to have_link(issue2.title)
end
end
end
context 'when block_anonymous_global_searches is enabled' do
before do
visit(search_path)
end
it 'is redirected to login page' do
expect(page).to have_content('You must be logged in to search across all of GitLab')
end end
end end
end end
......
...@@ -6,31 +6,44 @@ RSpec.describe 'User searches for projects' do ...@@ -6,31 +6,44 @@ RSpec.describe 'User searches for projects' do
let!(:project) { create(:project, :public, name: 'Shop') } let!(:project) { create(:project, :public, name: 'Shop') }
context 'when signed out' do context 'when signed out' do
include_examples 'top right search form' context 'when block_anonymous_global_searches is disabled' do
before do
stub_feature_flags(block_anonymous_global_searches: false)
end
it 'finds a project' do include_examples 'top right search form'
visit(search_path)
fill_in('dashboard_search', with: project.name[0..3]) it 'finds a project' do
click_button('Search') visit(search_path)
expect(page).to have_link(project.name) fill_in('dashboard_search', with: project.name[0..3])
end click_button('Search')
it 'preserves the group being searched in' do expect(page).to have_link(project.name)
visit(search_path(group_id: project.namespace.id)) end
submit_search('foo') it 'preserves the group being searched in' do
visit(search_path(group_id: project.namespace.id))
expect(find('#group_id', visible: false).value).to eq(project.namespace.id.to_s) submit_search('foo')
end
expect(find('#group_id', visible: false).value).to eq(project.namespace.id.to_s)
end
it 'preserves the project being searched in' do it 'preserves the project being searched in' do
visit(search_path(project_id: project.id)) visit(search_path(project_id: project.id))
submit_search('foo') submit_search('foo')
expect(find('#project_id', visible: false).value).to eq(project.id.to_s)
end
end
expect(find('#project_id', visible: false).value).to eq(project.id.to_s) context 'when block_anonymous_global_searches is enabled' do
it 'is redirected to login page' do
visit(search_path)
expect(page).to have_content('You must be logged in to search across all of GitLab')
end
end end
end end
end end
...@@ -7,10 +7,16 @@ RSpec.describe SearchController, '(JavaScript fixtures)', type: :controller do ...@@ -7,10 +7,16 @@ RSpec.describe SearchController, '(JavaScript fixtures)', type: :controller do
render_views render_views
let_it_be(:user) { create(:admin) }
before(:all) do before(:all) do
clean_frontend_fixtures('search/') clean_frontend_fixtures('search/')
end end
before do
sign_in(user)
end
it 'search/show.html' do it 'search/show.html' do
get :show get :show
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment