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
This commit is contained in:
Eugen Rochko 2022-10-26 12:10:02 +02:00 committed by GitHub
parent 8f07381856
commit 1ae508bf2f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 71 additions and 9 deletions

View file

@ -6,6 +6,7 @@ class Api::V2::SearchController < Api::BaseController
RESULTS_LIMIT = 20 RESULTS_LIMIT = 20
before_action -> { authorize_if_got_token! :read, :'read:search' } before_action -> { authorize_if_got_token! :read, :'read:search' }
before_action :validate_search_params!
def index def index
@search = Search.new(search_results) @search = Search.new(search_results)
@ -18,12 +19,22 @@ class Api::V2::SearchController < Api::BaseController
private 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 def search_results
SearchService.new.call( SearchService.new.call(
params[:q], params[:q],
current_account, current_account,
limit_param(RESULTS_LIMIT), 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 end

View file

@ -3,6 +3,9 @@
class AccountSearchService < BaseService class AccountSearchService < BaseService
attr_reader :query, :limit, :offset, :options, :account 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 = {}) def call(query, account = nil, options = {})
@acct_hint = query&.start_with?('@') @acct_hint = query&.start_with?('@')
@query = query&.strip&.gsub(/\A@/, '') @query = query&.strip&.gsub(/\A@/, '')
@ -135,6 +138,8 @@ class AccountSearchService < BaseService
end end
def limit_for_non_exact_results def limit_for_non_exact_results
return 0 if @account.nil? && query.size < MIN_QUERY_LENGTH
if exact_match? if exact_match?
limit - 1 limit - 1
else else

View file

@ -5,6 +5,7 @@ require 'rails_helper'
RSpec.describe Api::V2::SearchController, type: :controller do RSpec.describe Api::V2::SearchController, type: :controller do
render_views render_views
context 'with token' do
let(:user) { Fabricate(:user) } let(:user) { Fabricate(:user) }
let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:search') } let(:token) { Fabricate(:accessible_access_token, resource_owner_id: user.id, scopes: 'read:search') }
@ -13,10 +14,55 @@ RSpec.describe Api::V2::SearchController, type: :controller do
end end
describe 'GET #index' do describe 'GET #index' do
it 'returns http success' do before do
get :index, params: { q: 'test' } get :index, params: { q: 'test' }
end
it 'returns http success' do
expect(response).to have_http_status(200) expect(response).to have_http_status(200)
end end
end end
end
context 'without token' do
describe 'GET #index' do
let(:search_params) {}
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 end