From 1ae508bf2faae016b88d15e273b0dc01de4fd7af Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 26 Oct 2022 12:10:02 +0200 Subject: [PATCH] Change unauthenticated search to not support pagination in REST API (#19326) - Only exact search matches for queries with < 5 characters - Do not support queries with `offset` (pagination) - Return HTTP 401 on truthy `resolve` instead of overriding to false --- app/controllers/api/v2/search_controller.rb | 13 +++- app/services/account_search_service.rb | 5 ++ .../api/v2/search_controller_spec.rb | 62 ++++++++++++++++--- 3 files changed, 71 insertions(+), 9 deletions(-) diff --git a/app/controllers/api/v2/search_controller.rb b/app/controllers/api/v2/search_controller.rb index e384ecbaf..4d20aeb10 100644 --- a/app/controllers/api/v2/search_controller.rb +++ b/app/controllers/api/v2/search_controller.rb @@ -6,6 +6,7 @@ class Api::V2::SearchController < Api::BaseController RESULTS_LIMIT = 20 before_action -> { authorize_if_got_token! :read, :'read:search' } + before_action :validate_search_params! def index @search = Search.new(search_results) @@ -18,12 +19,22 @@ class Api::V2::SearchController < Api::BaseController private + def validate_search_params! + params.require(:q) + + return if user_signed_in? + + return render json: { error: 'Search queries pagination is not supported without authentication' }, status: 401 if params[:offset].present? + + render json: { error: 'Search queries that resolve remote resources are not supported without authentication' }, status: 401 if truthy_param?(:resolve) + end + def search_results SearchService.new.call( params[:q], current_account, limit_param(RESULTS_LIMIT), - search_params.merge(resolve: user_signed_in? ? truthy_param?(:resolve) : false, exclude_unreviewed: truthy_param?(:exclude_unreviewed)) + search_params.merge(resolve: truthy_param?(:resolve), exclude_unreviewed: truthy_param?(:exclude_unreviewed)) ) end diff --git a/app/services/account_search_service.rb b/app/services/account_search_service.rb index 4dcae20eb..35b2e05f5 100644 --- a/app/services/account_search_service.rb +++ b/app/services/account_search_service.rb @@ -3,6 +3,9 @@ class AccountSearchService < BaseService attr_reader :query, :limit, :offset, :options, :account + # Min. number of characters to look for non-exact matches + MIN_QUERY_LENGTH = 5 + def call(query, account = nil, options = {}) @acct_hint = query&.start_with?('@') @query = query&.strip&.gsub(/\A@/, '') @@ -135,6 +138,8 @@ class AccountSearchService < BaseService end def limit_for_non_exact_results + return 0 if @account.nil? && query.size < MIN_QUERY_LENGTH + if exact_match? limit - 1 else diff --git a/spec/controllers/api/v2/search_controller_spec.rb b/spec/controllers/api/v2/search_controller_spec.rb index fa20e1e51..d417ea58c 100644 --- a/spec/controllers/api/v2/search_controller_spec.rb +++ b/spec/controllers/api/v2/search_controller_spec.rb @@ -5,18 +5,64 @@ require 'rails_helper' RSpec.describe Api::V2::SearchController, type: :controller do render_views - let(:user) { Fabricate(:user) } - let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:search') } + context 'with token' do + let(:user) { Fabricate(:user) } + let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:search') } - before do - allow(controller).to receive(:doorkeeper_token) { token } + before do + allow(controller).to receive(:doorkeeper_token) { token } + end + + describe 'GET #index' do + before do + get :index, params: { q: 'test' } + end + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end end - describe 'GET #index' do - it 'returns http success' do - get :index, params: { q: 'test' } + context 'without token' do + describe 'GET #index' do + let(:search_params) {} - expect(response).to have_http_status(200) + before do + get :index, params: search_params + end + + context 'with a `q` shorter than 5 characters' do + let(:search_params) { { q: 'test' } } + + it 'returns http success' do + expect(response).to have_http_status(200) + end + end + + context 'with a `q` equal to or longer than 5 characters' do + let(:search_params) { { q: 'test1' } } + + it 'returns http success' do + expect(response).to have_http_status(200) + end + + context 'with truthy `resolve`' do + let(:search_params) { { q: 'test1', resolve: '1' } } + + it 'returns http unauthorized' do + expect(response).to have_http_status(401) + end + end + + context 'with `offset`' do + let(:search_params) { { q: 'test1', offset: 1 } } + + it 'returns http unauthorized' do + expect(response).to have_http_status(401) + end + end + end end end end