Check anonymous search access in API endpoints

- Verify user authenication for each API
endpoint when searching epics, issues or
merge requests if feature flag
disable_anonymous_search is turned on

Changelog: changed
EE: true

app/graphql/resolvers/concerns/issue_resolver_arguments.rb

@@ -4,6 +4,7 @@ module IssueResolverArguments
   extend ActiveSupport::Concern
 
   prepended do
+    include SearchArguments
     include LooksAhead
 
     argument :iid, GraphQL::Types::String,
@@ -49,9 +50,6 @@ module IssueResolverArguments
     argument :closed_after, Types::TimeType,
              required: false,
              description: 'Issues closed after this date.'
-    argument :search, GraphQL::Types::String,
-             required: false,
-             description: 'Search query for issue title or description.'
     argument :types, [Types::IssueTypeEnum],
              as: :issue_types,
              description: 'Filter issues by the given issue types.',
@@ -91,6 +89,7 @@ module IssueResolverArguments
     params_not_mutually_exclusive(args, mutually_exclusive_assignee_username_args)
     params_not_mutually_exclusive(args, mutually_exclusive_milestone_args)
     params_not_mutually_exclusive(args.fetch(:not, {}), mutually_exclusive_milestone_args)
+    validate_anonymous_search_access! if args[:search].present?
 
     super
   end

app/graphql/resolvers/concerns/search_arguments.rb

+# frozen_string_literal: true
+
+module SearchArguments
+  extend ActiveSupport::Concern
+
+  included do
+    argument :search, GraphQL::Types::String,
+             required: false,
+             description: 'Search query for title or description.'
+  end
+
+  def validate_anonymous_search_access!
+    return if current_user.present? || Feature.disabled?(:disable_anonymous_search, type: :ops)
+
+    raise ::Gitlab::Graphql::Errors::ArgumentError,
+      "User must be authenticated to include the `search` argument."
+  end
+end

doc/api/graphql/reference/index.md

@@ -8072,7 +8072,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="boardepicancestorsmilestonetitle"></a>`milestoneTitle` | [`String`](#string) | Filter epics by milestone title, computed from epic's issues. |
 | <a id="boardepicancestorsmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. |
 | <a id="boardepicancestorsnot"></a>`not` | [`NegatedEpicFilterInput`](#negatedepicfilterinput) | Negated epic arguments. |
-| <a id="boardepicancestorssearch"></a>`search` | [`String`](#string) | Search query for epic title or description. |
+| <a id="boardepicancestorssearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="boardepicancestorssort"></a>`sort` | [`EpicSort`](#epicsort) | List epics by sort order. |
 | <a id="boardepicancestorsstartdate"></a>`startDate` **{warning-solid}** | [`Time`](#time) | **Deprecated** in 13.5. Use timeframe.start. |
 | <a id="boardepicancestorsstate"></a>`state` | [`EpicState`](#epicstate) | Filter epics by state. |
@@ -8105,7 +8105,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="boardepicchildrenmilestonetitle"></a>`milestoneTitle` | [`String`](#string) | Filter epics by milestone title, computed from epic's issues. |
 | <a id="boardepicchildrenmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. |
 | <a id="boardepicchildrennot"></a>`not` | [`NegatedEpicFilterInput`](#negatedepicfilterinput) | Negated epic arguments. |
-| <a id="boardepicchildrensearch"></a>`search` | [`String`](#string) | Search query for epic title or description. |
+| <a id="boardepicchildrensearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="boardepicchildrensort"></a>`sort` | [`EpicSort`](#epicsort) | List epics by sort order. |
 | <a id="boardepicchildrenstartdate"></a>`startDate` **{warning-solid}** | [`Time`](#time) | **Deprecated** in 13.5. Use timeframe.start. |
 | <a id="boardepicchildrenstate"></a>`state` | [`EpicState`](#epicstate) | Filter epics by state. |
@@ -9477,7 +9477,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="epicancestorsmilestonetitle"></a>`milestoneTitle` | [`String`](#string) | Filter epics by milestone title, computed from epic's issues. |
 | <a id="epicancestorsmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. |
 | <a id="epicancestorsnot"></a>`not` | [`NegatedEpicFilterInput`](#negatedepicfilterinput) | Negated epic arguments. |
-| <a id="epicancestorssearch"></a>`search` | [`String`](#string) | Search query for epic title or description. |
+| <a id="epicancestorssearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="epicancestorssort"></a>`sort` | [`EpicSort`](#epicsort) | List epics by sort order. |
 | <a id="epicancestorsstartdate"></a>`startDate` **{warning-solid}** | [`Time`](#time) | **Deprecated** in 13.5. Use timeframe.start. |
 | <a id="epicancestorsstate"></a>`state` | [`EpicState`](#epicstate) | Filter epics by state. |
@@ -9510,7 +9510,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="epicchildrenmilestonetitle"></a>`milestoneTitle` | [`String`](#string) | Filter epics by milestone title, computed from epic's issues. |
 | <a id="epicchildrenmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. |
 | <a id="epicchildrennot"></a>`not` | [`NegatedEpicFilterInput`](#negatedepicfilterinput) | Negated epic arguments. |
-| <a id="epicchildrensearch"></a>`search` | [`String`](#string) | Search query for epic title or description. |
+| <a id="epicchildrensearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="epicchildrensort"></a>`sort` | [`EpicSort`](#epicsort) | List epics by sort order. |
 | <a id="epicchildrenstartdate"></a>`startDate` **{warning-solid}** | [`Time`](#time) | **Deprecated** in 13.5. Use timeframe.start. |
 | <a id="epicchildrenstate"></a>`state` | [`EpicState`](#epicstate) | Filter epics by state. |
@@ -10181,7 +10181,7 @@ Returns [`Epic`](#epic).
 | <a id="groupepicmilestonetitle"></a>`milestoneTitle` | [`String`](#string) | Filter epics by milestone title, computed from epic's issues. |
 | <a id="groupepicmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. |
 | <a id="groupepicnot"></a>`not` | [`NegatedEpicFilterInput`](#negatedepicfilterinput) | Negated epic arguments. |
-| <a id="groupepicsearch"></a>`search` | [`String`](#string) | Search query for epic title or description. |
+| <a id="groupepicsearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="groupepicsort"></a>`sort` | [`EpicSort`](#epicsort) | List epics by sort order. |
 | <a id="groupepicstartdate"></a>`startDate` **{warning-solid}** | [`Time`](#time) | **Deprecated** in 13.5. Use timeframe.start. |
 | <a id="groupepicstate"></a>`state` | [`EpicState`](#epicstate) | Filter epics by state. |
@@ -10226,7 +10226,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="groupepicsmilestonetitle"></a>`milestoneTitle` | [`String`](#string) | Filter epics by milestone title, computed from epic's issues. |
 | <a id="groupepicsmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. |
 | <a id="groupepicsnot"></a>`not` | [`NegatedEpicFilterInput`](#negatedepicfilterinput) | Negated epic arguments. |
-| <a id="groupepicssearch"></a>`search` | [`String`](#string) | Search query for epic title or description. |
+| <a id="groupepicssearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="groupepicssort"></a>`sort` | [`EpicSort`](#epicsort) | List epics by sort order. |
 | <a id="groupepicsstartdate"></a>`startDate` **{warning-solid}** | [`Time`](#time) | **Deprecated** in 13.5. Use timeframe.start. |
 | <a id="groupepicsstate"></a>`state` | [`EpicState`](#epicstate) | Filter epics by state. |
@@ -10282,7 +10282,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="groupissuesmilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. |
 | <a id="groupissuesmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. Wildcard values "NONE" and "ANY" are supported. |
 | <a id="groupissuesnot"></a>`not` | [`NegatedIssueFilterInput`](#negatedissuefilterinput) | Negated arguments. |
-| <a id="groupissuessearch"></a>`search` | [`String`](#string) | Search query for issue title or description. |
+| <a id="groupissuessearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="groupissuessort"></a>`sort` | [`IssueSort`](#issuesort) | Sort issues by this criteria. |
 | <a id="groupissuesstate"></a>`state` | [`IssuableState`](#issuablestate) | Current state of this issue. |
 | <a id="groupissuestypes"></a>`types` | [`[IssueType!]`](#issuetype) | Filter issues by the given issue types. |
@@ -12714,7 +12714,7 @@ Returns [`Issue`](#issue).
 | <a id="projectissuemilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. |
 | <a id="projectissuemyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. Wildcard values "NONE" and "ANY" are supported. |
 | <a id="projectissuenot"></a>`not` | [`NegatedIssueFilterInput`](#negatedissuefilterinput) | Negated arguments. |
-| <a id="projectissuesearch"></a>`search` | [`String`](#string) | Search query for issue title or description. |
+| <a id="projectissuesearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="projectissuesort"></a>`sort` | [`IssueSort`](#issuesort) | Sort issues by this criteria. |
 | <a id="projectissuestate"></a>`state` | [`IssuableState`](#issuablestate) | Current state of this issue. |
 | <a id="projectissuetypes"></a>`types` | [`[IssueType!]`](#issuetype) | Filter issues by the given issue types. |
@@ -12747,7 +12747,7 @@ Returns [`IssueStatusCountsType`](#issuestatuscountstype).
 | <a id="projectissuestatuscountsmilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. |
 | <a id="projectissuestatuscountsmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. Wildcard values "NONE" and "ANY" are supported. |
 | <a id="projectissuestatuscountsnot"></a>`not` | [`NegatedIssueFilterInput`](#negatedissuefilterinput) | Negated arguments. |
-| <a id="projectissuestatuscountssearch"></a>`search` | [`String`](#string) | Search query for issue title or description. |
+| <a id="projectissuestatuscountssearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="projectissuestatuscountstypes"></a>`types` | [`[IssueType!]`](#issuetype) | Filter issues by the given issue types. |
 | <a id="projectissuestatuscountsupdatedafter"></a>`updatedAfter` | [`Time`](#time) | Issues updated after this date. |
 | <a id="projectissuestatuscountsupdatedbefore"></a>`updatedBefore` | [`Time`](#time) | Issues updated before this date. |
@@ -12784,7 +12784,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
 | <a id="projectissuesmilestonewildcardid"></a>`milestoneWildcardId` | [`MilestoneWildcardId`](#milestonewildcardid) | Filter issues by milestone ID wildcard. |
 | <a id="projectissuesmyreactionemoji"></a>`myReactionEmoji` | [`String`](#string) | Filter by reaction emoji applied by the current user. Wildcard values "NONE" and "ANY" are supported. |
 | <a id="projectissuesnot"></a>`not` | [`NegatedIssueFilterInput`](#negatedissuefilterinput) | Negated arguments. |
-| <a id="projectissuessearch"></a>`search` | [`String`](#string) | Search query for issue title or description. |
+| <a id="projectissuessearch"></a>`search` | [`String`](#string) | Search query for title or description. |
 | <a id="projectissuessort"></a>`sort` | [`IssueSort`](#issuesort) | Sort issues by this criteria. |
 | <a id="projectissuesstate"></a>`state` | [`IssuableState`](#issuablestate) | Current state of this issue. |
 | <a id="projectissuestypes"></a>`types` | [`[IssueType!]`](#issuetype) | Filter issues by the given issue types. |

ee/app/graphql/resolvers/epics_resolver.rb

@@ -3,6 +3,7 @@
 module Resolvers
   class EpicsResolver < BaseResolver
     include TimeFrameArguments
+    include SearchArguments
     include LooksAhead
     include SetsMaxPageSize
 
@@ -18,10 +19,6 @@ module Resolvers
              required: false,
              description: 'Filter epics by state.'
 
-    argument :search, GraphQL::Types::String,
-             required: false,
-             description: 'Search query for epic title or description.'
-
     argument :in, [Types::IssuableSearchableFieldEnum],
              required: false,
              description: 'Specify the fields to perform the search in. Defaults to `[TITLE, DESCRIPTION]`. Requires the `search` argument.'
@@ -74,6 +71,7 @@ module Resolvers
       validate_timeframe_params!(args)
       validate_starts_with_iid!(args)
       validate_search_in_params!(args)
+      validate_anonymous_search_access! if args[:search].present?
 
       super(**args)
     end

ee/lib/api/epics.rb

@@ -44,6 +44,7 @@ module API
       end
       [':id/epics', ':id/-/epics'].each do |path|
         get path do
+          validate_anonymous_search_access! if declared_params[:search].present?
           epics = paginate(find_epics(finder_params: { group_id: user_group.id })).with_api_entity_associations
 
           # issuable_metadata has to be set because `Entities::Epic` doesn't inherit from `Entities::IssuableEntity`

ee/spec/graphql/resolvers/epics_resolver_spec.rb

@@ -144,6 +144,35 @@ RSpec.describe Resolvers::EpicsResolver do
             expect(epics).to match_array([epic2, epic3, epic4])
           end
         end
+
+        context 'with anonymous user' do
+          let_it_be(:current_user) { nil }
+
+          context 'with disable_anonymous_search enabled' do
+            before do
+              stub_feature_flags(disable_anonymous_search: true)
+            end
+
+            it 'returns an error' do
+              error_message = "User must be authenticated to include the `search` argument."
+
+              expect { resolve_epics(search: 'created') }
+                .to raise_error(Gitlab::Graphql::Errors::ArgumentError, error_message)
+            end
+          end
+
+          context 'with disable_anonymous_search disabled' do
+            before do
+              stub_feature_flags(disable_anonymous_search: false)
+            end
+
+            it 'filters epics by search term' do
+              epics = resolve_epics(search: 'created')
+
+              expect(epics).to match_array([epic1, epic2])
+            end
+          end
+        end
       end
 
       context 'with author_username' do

ee/spec/requests/api/epics_spec.rb

@@ -199,18 +199,6 @@ RSpec.describe API::Epics do
         expect_paginated_array_response([epic.id])
       end
 
-      it 'returns epics matching given search string for title' do
-        get api(url), params: { search: epic2.title }
-
-        expect_paginated_array_response([epic2.id])
-      end
-
-      it 'returns epics matching given search string for description' do
-        get api(url), params: { search: epic2.description }
-
-        expect_paginated_array_response([epic2.id])
-      end
-
       it 'returns epics matching given status' do
         get api(url, user), params: { state: :opened }
 
@@ -379,6 +367,25 @@ RSpec.describe API::Epics do
         expect_paginated_array_response(epic.id)
       end
 
+      context 'with search param' do
+        it 'returns issues matching given search string for title' do
+          get api(url, user), params: { search: epic2.title }
+
+          expect_paginated_array_response(epic2.id)
+        end
+
+        it 'returns issues matching given search string for description' do
+          get api(url, user), params: { search: epic2.description }
+
+          expect_paginated_array_response(epic2.id)
+        end
+
+        it_behaves_like 'issuable anonymous search' do
+          let(:issuable) { epic2 }
+          let(:result) { issuable.id }
+        end
+      end
+
       describe "#to_reference" do
         it 'exposes reference path' do
           get api(url)

lib/api/helpers.rb

@@ -624,6 +624,12 @@ module API
       {}
     end
 
+    def validate_anonymous_search_access!
+      return if current_user.present? || Feature.disabled?(:disable_anonymous_search, type: :ops)
+
+      unprocessable_entity!('User must be authenticated to use search')
+    end
+
     private
 
     # rubocop:disable Gitlab/ModuleWithInstanceVariables

lib/api/issues.rb

@@ -131,6 +131,7 @@ module API
       end
       get do
         authenticate! unless params[:scope] == 'all'
+        validate_anonymous_search_access! if params[:search].present?
         issues = paginate(find_issues)
 
         options = {
@@ -169,6 +170,7 @@ module API
         optional :non_archived, type: Boolean, desc: 'Return issues from non archived projects', default: true
       end
       get ":id/issues" do
+        validate_anonymous_search_access! if declared_params[:search].present?
         issues = paginate(find_issues(group_id: user_group.id, include_subgroups: true))
 
         options = {
@@ -204,6 +206,7 @@ module API
         use :issues_params
       end
       get ":id/issues" do
+        validate_anonymous_search_access! if declared_params[:search].present?
         issues = paginate(find_issues(project_id: user_project.id))
 
         options = {

lib/api/merge_requests.rb

@@ -136,6 +136,7 @@ module API
       end
       get feature_category: :code_review do
         authenticate! unless params[:scope] == 'all'
+        validate_anonymous_search_access! if params[:search].present?
         merge_requests = find_merge_requests
 
         present merge_requests, serializer_options_for(merge_requests)
@@ -155,6 +156,7 @@ module API
         default: true
       end
       get ":id/merge_requests", feature_category: :code_review do
+        validate_anonymous_search_access! if declared_params[:search].present?
         merge_requests = find_merge_requests(group_id: user_group.id, include_subgroups: true)
 
         present merge_requests, serializer_options_for(merge_requests).merge(group: user_group)
@@ -195,6 +197,7 @@ module API
       end
       get ":id/merge_requests", feature_category: :code_review do
         authorize! :read_merge_request, user_project
+        validate_anonymous_search_access! if declared_params[:search].present?
 
         merge_requests = find_merge_requests(project_id: user_project.id)
 

spec/graphql/resolvers/issues_resolver_spec.rb

@@ -236,6 +236,36 @@ RSpec.describe Resolvers::IssuesResolver do
 
           resolve_issues(search: 'foo')
         end
+
+        context 'with anonymous user' do
+          let_it_be(:public_project) { create(:project, :public) }
+          let_it_be(:public_issue) { create(:issue, project: public_project, title: 'Test issue') }
+
+          context 'with disable_anonymous_search enabled' do
+            before do
+              stub_feature_flags(disable_anonymous_search: true)
+            end
+
+            it 'returns an error' do
+              error_message = "User must be authenticated to include the `search` argument."
+
+              expect { resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil }) }
+                .to raise_error(Gitlab::Graphql::Errors::ArgumentError, error_message)
+            end
+          end
+
+          context 'with disable_anonymous_search disabled' do
+            before do
+              stub_feature_flags(disable_anonymous_search: false)
+            end
+
+            it 'returns correct issues' do
+              expect(
+                resolve(described_class, obj: public_project, args: { search: 'test' }, ctx: { current_user: nil })
+              ).to contain_exactly(public_issue)
+            end
+          end
+        end
       end
 
       describe 'filters by negated params' do

spec/requests/api/issues/issues_spec.rb

@@ -138,6 +138,12 @@ RSpec.describe API::Issues do
         expect(json_response).to be_an Array
       end
 
+      it_behaves_like 'issuable anonymous search' do
+        let(:url) { '/issues' }
+        let(:issuable) { issue }
+        let(:result) { issuable.id }
+      end
+
       it 'returns authentication error without any scope' do
         get api('/issues')
 

spec/requests/api/merge_requests_spec.rb

@@ -49,6 +49,12 @@ RSpec.describe API::MergeRequests do
 
         expect_successful_response_with_paginated_array
       end
+
+      it_behaves_like 'issuable anonymous search' do
+        let(:url) { endpoint_path }
+        let(:issuable) { merge_request }
+        let(:result) { [merge_request_merged.id, merge_request_locked.id, merge_request_closed.id, merge_request.id] }
+      end
     end
 
     context 'when authenticated' do
@@ -613,6 +619,12 @@ RSpec.describe API::MergeRequests do
         )
       end
 
+      it_behaves_like 'issuable anonymous search' do
+        let(:url) { '/merge_requests' }
+        let(:issuable) { merge_request }
+        let(:result) { [merge_request_merged.id, merge_request_locked.id, merge_request_closed.id, merge_request.id] }
+      end
+
       it "returns authentication error without any scope" do
         get api("/merge_requests")
 

spec/support/shared_examples/requests/api/issuable_search_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'issuable anonymous search' do
+  context 'with anonymous user' do
+    context 'with disable_anonymous_search disabled' do
+      before do
+        stub_feature_flags(disable_anonymous_search: false)
+      end
+
+      it 'returns issuables matching given search string for title' do
+        get api(url), params: { scope: 'all', search: issuable.title }
+
+        expect_paginated_array_response(result)
+      end
+
+      it 'returns issuables matching given search string for description' do
+        get api(url), params: { scope: 'all', search: issuable.description }
+
+        expect_paginated_array_response(result)
+      end
+    end
+
+    context 'with disable_anonymous_search enabled' do
+      before do
+        stub_feature_flags(disable_anonymous_search: true)
+      end
+
+      it "returns 422 error" do
+        get api(url), params: { scope: 'all', search: issuable.title }
+
+        expect(response).to have_gitlab_http_status(:unprocessable_entity)
+        expect(json_response['message']).to eq('User must be authenticated to use search')
+      end
+    end
+  end
+end