From fcad3e716ad8dc60bd3d94e5b2e0aa18af4c9376 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 19 Nov 2020 18:08:22 +0300 Subject: [PATCH 1/2] [#2301] Quick fix: users with is_discoverable == false (default!) are included in search results. --- lib/pleroma/user/search.ex | 8 +++++--- test/pleroma/user_search_test.exs | 5 +++-- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index 2dab67211..b54111090 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -85,7 +85,7 @@ defp search_query(query_string, for_user, following, top_user_ids) do |> base_query(following) |> filter_blocked_user(for_user) |> filter_invisible_users() - |> filter_discoverable_users() + |> filter_non_discoverable_users() |> filter_internal_users() |> filter_blocked_domains(for_user) |> fts_search(query_string) @@ -163,8 +163,10 @@ defp filter_invisible_users(query) do from(q in query, where: q.invisible == false) end - defp filter_discoverable_users(query) do - from(q in query, where: q.is_discoverable == true) + defp filter_non_discoverable_users(query) do + # Note: commented out — can't do it with users being non-discoverable by default + # from(q in query, where: q.is_discoverable == true) + query end defp filter_internal_users(query) do diff --git a/test/pleroma/user_search_test.exs b/test/pleroma/user_search_test.exs index 31d787ffa..d5ab5a003 100644 --- a/test/pleroma/user_search_test.exs +++ b/test/pleroma/user_search_test.exs @@ -65,12 +65,13 @@ test "excludes invisible users from results" do assert found_user.id == user.id end - test "excludes users when discoverable is false" do + # NOTE: as long as users are non-discoverable by default, we can't filter out most users: #2301 + test "does NOT exclude non-discoverable users from results (as long as it's the default)" do insert(:user, %{nickname: "john 3000", is_discoverable: false}) insert(:user, %{nickname: "john 3001"}) users = User.search("john") - assert Enum.count(users) == 1 + assert Enum.count(users) == 2 end test "excludes service actors from results" do From e164c37139c4365d7d46a2a990b364ad26dfdbf7 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 19 Nov 2020 19:30:02 +0300 Subject: [PATCH 2/2] [#2301] Proper handling of `User.is_discoverable`: users appear in in-service search but are hidden from external services like search bots. --- CHANGELOG.md | 4 +- docs/API/admin_api.md | 2 +- docs/API/differences_in_mastoapi_responses.md | 4 +- lib/pleroma/user/search.ex | 7 --- .../web/activity_pub/views/user_view.ex | 1 + .../api_spec/operations/account_operation.ex | 2 +- lib/pleroma/web/api_spec/schemas/account.ex | 2 +- .../controllers/account_controller.ex | 2 + .../metadata/providers/restrict_indexing.ex | 2 +- test/pleroma/user_search_test.exs | 4 +- test/pleroma/web/admin_api/search_test.exs | 1 + .../providers/restrict_indexing_test.exs | 2 +- test/pleroma/web/metadata_test.exs | 49 ------------------- 13 files changed, 16 insertions(+), 66 deletions(-) delete mode 100644 test/pleroma/web/metadata_test.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 8658d5440..6caed1123 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed +- Users with `is_discoverable` field set to false (default value) will appear in in-service search results but be hidden from external services (search bots etc.). +
API Changes - Mastodon API: Current user is now included in conversation if it's the only participant. @@ -70,7 +72,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Renamed `:await_up_timeout` in `:connections_pool` namespace to `:connect_timeout`, old name is deprecated. - Renamed `:timeout` in `pools` namespace to `:recv_timeout`, old name is deprecated. - The `discoverable` field in the `User` struct will now add a NOINDEX metatag to profile pages when false. -- Users with the `discoverable` field set to false will not show up in searches. +- Users with the `is_discoverable` field set to false will not show up in searches ([bug](https://git.pleroma.social/pleroma/pleroma/-/issues/2301)). - Minimum lifetime for ephmeral activities changed to 10 minutes and made configurable (`:min_lifetime` option). - Introduced optional dependencies on `ffmpeg`, `ImageMagick`, `exiftool` software packages. Please refer to `docs/installation/optional/media_graphics_packages.md`. -
diff --git a/docs/API/admin_api.md b/docs/API/admin_api.md index 19ac6a65f..266f8cef8 100644 --- a/docs/API/admin_api.md +++ b/docs/API/admin_api.md @@ -554,7 +554,7 @@ Response: * `show_role` * `skip_thread_containment` * `fields` - * `discoverable` + * `is_discoverable` * `actor_type` * Responses: diff --git a/docs/API/differences_in_mastoapi_responses.md b/docs/API/differences_in_mastoapi_responses.md index 843496482..6b0ad85d1 100644 --- a/docs/API/differences_in_mastoapi_responses.md +++ b/docs/API/differences_in_mastoapi_responses.md @@ -84,7 +84,7 @@ Has these additional fields under the `pleroma` object: - `show_role`: boolean, nullable, true when the user wants his role (e.g admin, moderator) to be shown - `no_rich_text` - boolean, nullable, true when html tags are stripped from all statuses requested from the API -- `discoverable`: boolean, true when the user allows discovery of the account in search results and other services. +- `discoverable`: boolean, true when the user allows external services (search bots) etc. to index / list the account (regardless of this setting, user will still appear in regular search results) - `actor_type`: string, the type of this account. ## Conversations @@ -207,7 +207,7 @@ Additional parameters can be added to the JSON body/Form data: - `skip_thread_containment` - if true, skip filtering out broken threads - `allow_following_move` - if true, allows automatically follow moved following accounts - `pleroma_background_image` - sets the background image of the user. Can be set to "" (an empty string) to reset. -- `discoverable` - if true, discovery of this account in search results and other services is allowed. +- `discoverable` - if true, external services (search bots) etc. are allowed to index / list the account (regardless of this setting, user will still appear in regular search results). - `actor_type` - the type of this account. - `accepts_chat_messages` - if false, this account will reject all chat messages. diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index b54111090..f1761ef03 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -85,7 +85,6 @@ defp search_query(query_string, for_user, following, top_user_ids) do |> base_query(following) |> filter_blocked_user(for_user) |> filter_invisible_users() - |> filter_non_discoverable_users() |> filter_internal_users() |> filter_blocked_domains(for_user) |> fts_search(query_string) @@ -163,12 +162,6 @@ defp filter_invisible_users(query) do from(q in query, where: q.invisible == false) end - defp filter_non_discoverable_users(query) do - # Note: commented out — can't do it with users being non-discoverable by default - # from(q in query, where: q.is_discoverable == true) - query - end - defp filter_internal_users(query) do from(q in query, where: q.actor_type != "Application") end diff --git a/lib/pleroma/web/activity_pub/views/user_view.ex b/lib/pleroma/web/activity_pub/views/user_view.ex index 4dc45cde3..93c9f436c 100644 --- a/lib/pleroma/web/activity_pub/views/user_view.ex +++ b/lib/pleroma/web/activity_pub/views/user_view.ex @@ -110,6 +110,7 @@ def render("user.json", %{user: user}) do "endpoints" => endpoints, "attachment" => fields, "tag" => emoji_tags, + # Note: key name is indeed "discoverable" (not an error) "discoverable" => user.is_discoverable, "capabilities" => capabilities } diff --git a/lib/pleroma/web/api_spec/operations/account_operation.ex b/lib/pleroma/web/api_spec/operations/account_operation.ex index 05595bc2a..280100c3d 100644 --- a/lib/pleroma/web/api_spec/operations/account_operation.ex +++ b/lib/pleroma/web/api_spec/operations/account_operation.ex @@ -624,7 +624,7 @@ defp update_credentials_request do allOf: [BooleanLike], nullable: true, description: - "Discovery of this account in search results and other services is allowed." + "Discovery (listing, indexing) of this account by external services (search bots etc.) is allowed." }, actor_type: ActorType }, diff --git a/lib/pleroma/web/api_spec/schemas/account.ex b/lib/pleroma/web/api_spec/schemas/account.ex index ca79f0747..684f6fc92 100644 --- a/lib/pleroma/web/api_spec/schemas/account.ex +++ b/lib/pleroma/web/api_spec/schemas/account.ex @@ -127,7 +127,7 @@ defmodule Pleroma.Web.ApiSpec.Schemas.Account do discoverable: %Schema{ type: :boolean, description: - "whether the user allows discovery of the account in search results and other services." + "whether the user allows indexing / listing of the account by external services (search engines etc.)." }, no_rich_text: %Schema{ type: :boolean, diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex index 7ed4603a4..7011b7eb1 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -208,7 +208,9 @@ def update_credentials(%{assigns: %{user: user}, body_params: params} = conn, _p if bot, do: {:ok, "Service"}, else: {:ok, "Person"} end) |> Maps.put_if_present(:actor_type, params[:actor_type]) + # Note: param name is indeed :locked (not an error) |> Maps.put_if_present(:is_locked, params[:locked]) + # Note: param name is indeed :discoverable (not an error) |> Maps.put_if_present(:is_discoverable, params[:discoverable]) # What happens here: diff --git a/lib/pleroma/web/metadata/providers/restrict_indexing.ex b/lib/pleroma/web/metadata/providers/restrict_indexing.ex index 900c2434d..a08a04b4a 100644 --- a/lib/pleroma/web/metadata/providers/restrict_indexing.ex +++ b/lib/pleroma/web/metadata/providers/restrict_indexing.ex @@ -6,7 +6,7 @@ defmodule Pleroma.Web.Metadata.Providers.RestrictIndexing do @behaviour Pleroma.Web.Metadata.Providers.Provider @moduledoc """ - Restricts indexing of remote users. + Restricts indexing of remote and/or non-discoverable users. """ @impl true diff --git a/test/pleroma/user_search_test.exs b/test/pleroma/user_search_test.exs index d5ab5a003..de1df2e9c 100644 --- a/test/pleroma/user_search_test.exs +++ b/test/pleroma/user_search_test.exs @@ -65,8 +65,8 @@ test "excludes invisible users from results" do assert found_user.id == user.id end - # NOTE: as long as users are non-discoverable by default, we can't filter out most users: #2301 - test "does NOT exclude non-discoverable users from results (as long as it's the default)" do + # Note: as in Mastodon, `is_discoverable` doesn't anyhow relate to user searchability + test "includes non-discoverable users in results" do insert(:user, %{nickname: "john 3000", is_discoverable: false}) insert(:user, %{nickname: "john 3001"}) diff --git a/test/pleroma/web/admin_api/search_test.exs b/test/pleroma/web/admin_api/search_test.exs index 92a116c65..9bc58640c 100644 --- a/test/pleroma/web/admin_api/search_test.exs +++ b/test/pleroma/web/admin_api/search_test.exs @@ -203,6 +203,7 @@ test "it returns unconfirmed user" do assert count == 1 end + # Note: as in Mastodon, `is_discoverable` doesn't anyhow relate to user searchability test "it returns non-discoverable users" do insert(:user) insert(:user, is_discoverable: false) diff --git a/test/pleroma/web/metadata/providers/restrict_indexing_test.exs b/test/pleroma/web/metadata/providers/restrict_indexing_test.exs index 282d132c8..52399fdc8 100644 --- a/test/pleroma/web/metadata/providers/restrict_indexing_test.exs +++ b/test/pleroma/web/metadata/providers/restrict_indexing_test.exs @@ -18,7 +18,7 @@ test "for local user" do }) == [] end - test "for local user when discoverable is false" do + test "for local user when `is_discoverable` is false" do assert Pleroma.Web.Metadata.Providers.RestrictIndexing.build_tags(%{ user: %Pleroma.User{local: true, is_discoverable: false} }) == [{:meta, [name: "robots", content: "noindex, noarchive"], []}] diff --git a/test/pleroma/web/metadata_test.exs b/test/pleroma/web/metadata_test.exs deleted file mode 100644 index 8fb946540..000000000 --- a/test/pleroma/web/metadata_test.exs +++ /dev/null @@ -1,49 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Web.MetadataTest do - use Pleroma.DataCase, async: true - - import Pleroma.Factory - - describe "restrict indexing remote users" do - test "for remote user" do - user = insert(:user, local: false) - - assert Pleroma.Web.Metadata.build_tags(%{user: user}) =~ - "" - end - - test "for local user" do - user = insert(:user, is_discoverable: false) - - assert Pleroma.Web.Metadata.build_tags(%{user: user}) =~ - "" - end - - test "for local user set to discoverable" do - user = insert(:user, is_discoverable: true) - - refute Pleroma.Web.Metadata.build_tags(%{user: user}) =~ - "" - end - end - - describe "no metadata for private instances" do - test "for local user set to discoverable" do - clear_config([:instance, :public], false) - user = insert(:user, bio: "This is my secret fedi account bio", is_discoverable: true) - - assert "" = Pleroma.Web.Metadata.build_tags(%{user: user}) - end - - test "search exclusion metadata is included" do - clear_config([:instance, :public], false) - user = insert(:user, bio: "This is my secret fedi account bio", is_discoverable: false) - - assert ~s() == - Pleroma.Web.Metadata.build_tags(%{user: user}) - end - end -end