fix(search): fetch remote statuses only when resolving is allowed
Status search fetched any http(s) query from its remote server, for anonymous users, on every result page and with resolve=false, so anyone could make the server fetch URLs. As in Mastodon and in Pleroma's user search, fetch only with resolve=true, on the first page, and for anonymous users only if [:instance, :limit_to_local_content] is false. Based on Akkoma ca9e33ed2 and 17efec060. Co-authored-by: Oneric <oneric@oneric.stub>
This commit is contained in:
parent
3e9dc477c9
commit
db43aec782
6 changed files with 61 additions and 14 deletions
1
changelog.d/search-remote-fetch.security
Normal file
1
changelog.d/search-remote-fetch.security
Normal file
|
|
@ -0,0 +1 @@
|
|||
Status search only fetches remote URLs with resolve=true, on the first page, and for anonymous users only if limit_to_local_content allows it
|
||||
|
|
@ -45,7 +45,7 @@ defmodule Pleroma.Search.DatabaseSearch do
|
|||
end
|
||||
|
||||
# Outside the query transaction, as it may fetch over the network.
|
||||
maybe_fetch(activities, user, search_query)
|
||||
maybe_fetch(activities, user, search_query, options)
|
||||
end
|
||||
|
||||
@doc """
|
||||
|
|
@ -160,7 +160,27 @@ defmodule Pleroma.Search.DatabaseSearch do
|
|||
|
||||
defp restrict_local(q), do: where(q, local: true)
|
||||
|
||||
def maybe_fetch(activities, user, search_query) do
|
||||
@doc """
|
||||
Adds the status a URL query points to, fetching it if needed. As in Mastodon,
|
||||
this only happens with `resolve: true` and on the first page, and for
|
||||
anonymous users only if `[:instance, :limit_to_local_content]` is `false`.
|
||||
"""
|
||||
def maybe_fetch(activities, user, search_query, options) do
|
||||
if fetch_allowed?(user, options),
|
||||
do: fetch(activities, user, search_query),
|
||||
else: activities
|
||||
end
|
||||
|
||||
defp fetch_allowed?(user, options) do
|
||||
options[:resolve] == true and (options[:offset] || 0) == 0 and
|
||||
case Config.get([:instance, :limit_to_local_content], :unauthenticated) do
|
||||
:all -> false
|
||||
:unauthenticated -> match?(%User{}, user)
|
||||
false -> true
|
||||
end
|
||||
end
|
||||
|
||||
defp fetch(activities, user, search_query) do
|
||||
with true <- Regex.match?(~r/https?:/, search_query),
|
||||
{:ok, object} <- Fetcher.fetch_object_from_id(search_query),
|
||||
%Activity{} = activity <- Activity.get_create_by_object_ap_id(object.data["id"]),
|
||||
|
|
|
|||
|
|
@ -111,11 +111,11 @@ defmodule Pleroma.Search.Meilisearch do
|
|||
|> maybe_restrict_local(user)
|
||||
|> maybe_restrict_author(author)
|
||||
|> maybe_restrict_blocked(user)
|
||||
|> maybe_fetch(user, query)
|
||||
|> maybe_fetch(user, query, options)
|
||||
|> order_by([object: obj], desc: obj.data["published"])
|
||||
|> Pleroma.Repo.all()
|
||||
rescue
|
||||
_ -> maybe_fetch([], user, query)
|
||||
_ -> maybe_fetch([], user, query, options)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ defmodule Pleroma.Search.ParadeDB do
|
|||
|
||||
import Pleroma.Search.DatabaseSearch,
|
||||
only: [
|
||||
maybe_fetch: 3,
|
||||
maybe_fetch: 4,
|
||||
maybe_restrict_author: 2,
|
||||
maybe_restrict_blocked: 2,
|
||||
maybe_restrict_local: 2
|
||||
|
|
@ -149,19 +149,19 @@ defmodule Pleroma.Search.ParadeDB do
|
|||
|
||||
case search_ids(query, limit, offset, author) do
|
||||
{:ok, []} ->
|
||||
maybe_fetch([], user, query)
|
||||
maybe_fetch([], user, query, options)
|
||||
|
||||
{:ok, ids} ->
|
||||
ids
|
||||
|> fetch_activities(user, query, author)
|
||||
|> fetch_activities(user, query, author, options)
|
||||
|
||||
{:error, error} ->
|
||||
Logger.error("ParadeDB search failed: #{inspect(error)}")
|
||||
maybe_fetch([], user, query)
|
||||
maybe_fetch([], user, query, options)
|
||||
end
|
||||
end
|
||||
|
||||
defp fetch_activities(ids, user, query, author) do
|
||||
defp fetch_activities(ids, user, query, author, options) do
|
||||
dumped_ids =
|
||||
ids
|
||||
|> Enum.map(&dump_activity_id/1)
|
||||
|
|
@ -183,7 +183,7 @@ defmodule Pleroma.Search.ParadeDB do
|
|||
)
|
||||
)
|
||||
|> Pleroma.Repo.all()
|
||||
|> maybe_fetch(user, query)
|
||||
|> maybe_fetch(user, query, options)
|
||||
end
|
||||
|
||||
defp maybe_restrict_public(query, %Pleroma.User{}) do
|
||||
|
|
|
|||
|
|
@ -10,7 +10,7 @@ defmodule Pleroma.Search.QdrantSearch do
|
|||
alias __MODULE__.OpenAIClient
|
||||
alias __MODULE__.QdrantClient
|
||||
|
||||
import Pleroma.Search.DatabaseSearch, only: [maybe_fetch: 3]
|
||||
import Pleroma.Search.DatabaseSearch, only: [maybe_fetch: 4]
|
||||
|
||||
@impl true
|
||||
def create_index do
|
||||
|
|
@ -131,7 +131,7 @@ defmodule Pleroma.Search.QdrantSearch do
|
|||
|> Activity.restrict_deactivated_users()
|
||||
|> Ecto.Query.order_by([a], fragment("array_position(?, ?)", ^ids, a.id))
|
||||
|> Pleroma.Repo.all()
|
||||
|> maybe_fetch(user, original_query)
|
||||
|> maybe_fetch(user, original_query, options)
|
||||
else
|
||||
_ ->
|
||||
[]
|
||||
|
|
|
|||
|
|
@ -328,7 +328,9 @@ defmodule Pleroma.Web.MastodonAPI.SearchControllerTest do
|
|||
assert status["id"] == to_string(activity.id)
|
||||
end
|
||||
|
||||
test "search fetches remote statuses and prefers them over other results", %{conn: conn} do
|
||||
test "search fetches remote statuses and prefers them over other results" do
|
||||
%{conn: conn} = oauth_access(["read:search"])
|
||||
|
||||
{:ok, %{id: activity_id}} =
|
||||
CommonAPI.post(insert(:user), %{
|
||||
status: "check out http://mastodon.example.org/@admin/99541947525187367"
|
||||
|
|
@ -336,7 +338,9 @@ defmodule Pleroma.Web.MastodonAPI.SearchControllerTest do
|
|||
|
||||
%{"url" => result_url, "id" => result_id} =
|
||||
conn
|
||||
|> get("/api/v1/search?q=http://mastodon.example.org/@admin/99541947525187367")
|
||||
|> get(
|
||||
"/api/v1/search?resolve=true&q=http://mastodon.example.org/@admin/99541947525187367"
|
||||
)
|
||||
|> json_response_and_validate_schema(200)
|
||||
|> Map.get("statuses")
|
||||
|> List.first()
|
||||
|
|
@ -345,6 +349,28 @@ defmodule Pleroma.Web.MastodonAPI.SearchControllerTest do
|
|||
assert match?(^result_url, "http://mastodon.example.org/@admin/99541947525187367")
|
||||
end
|
||||
|
||||
test "search fetches remote statuses only with resolve, when logged in, on the first page",
|
||||
%{conn: anonymous} do
|
||||
%{conn: conn} = oauth_access(["read:search"])
|
||||
url = "http://mastodon.example.org/@admin/99541947525187367"
|
||||
|
||||
for {conn, params} <- [
|
||||
{conn, ""},
|
||||
{conn, "&resolve=false"},
|
||||
{conn, "&resolve=true&offset=5"},
|
||||
{anonymous, "&resolve=true"}
|
||||
] do
|
||||
assert %{"statuses" => []} =
|
||||
conn
|
||||
|> get("/api/v1/search?q=#{url}#{params}")
|
||||
|> json_response_and_validate_schema(200)
|
||||
end
|
||||
|
||||
refute Pleroma.Object.get_by_ap_id(
|
||||
"http://mastodon.example.org/users/admin/statuses/99541947525187367"
|
||||
)
|
||||
end
|
||||
|
||||
test "search doesn't show statuses that it shouldn't", %{conn: conn} do
|
||||
{:ok, activity} =
|
||||
CommonAPI.post(insert(:user), %{
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue