Skip to content

Commit

Permalink
Change unauthenticated search to not support pagination in REST API (m…
Browse files Browse the repository at this point in the history
…astodon#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
  • Loading branch information
Gargron authored and kadoshita committed Nov 19, 2022
1 parent 4c075d7 commit e55d581
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 9 deletions.
13 changes: 12 additions & 1 deletion app/controllers/api/v2/search_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ class Api::V2::SearchController < Api::BaseController
RESULTS_LIMIT = 50

before_action -> { authorize_if_got_token! :read, :'read:search' }
before_action :validate_search_params!

def index
@search = Search.new(search_results)
Expand All @@ -18,12 +19,22 @@ def index

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

Expand Down
5 changes: 5 additions & 0 deletions app/services/account_search_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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@/, '')
Expand Down Expand Up @@ -135,6 +138,8 @@ def following_ids
end

def limit_for_non_exact_results
return 0 if @account.nil? && query.size < MIN_QUERY_LENGTH

if exact_match?
limit - 1
else
Expand Down
62 changes: 54 additions & 8 deletions spec/controllers/api/v2/search_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,64 @@
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) {}

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 } }

expect(response).to have_http_status(200)
it 'returns http unauthorized' do
expect(response).to have_http_status(401)
end
end
end
end
end
end

0 comments on commit e55d581

Please sign in to comment.