From 2b664b048eac5f37421ab958e26efed9c158c8d0 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 3 Jun 2019 18:16:11 +0200 Subject: [PATCH 01/23] User: Add function to get AP ids from nicknames. --- lib/pleroma/user.ex | 8 ++++++++ test/user_test.exs | 12 ++++++++++++ 2 files changed, 20 insertions(+) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 474cd8c1a..dc534b05c 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -1441,4 +1441,12 @@ defmodule Pleroma.User do update_and_set_cache(cng) end end + + def get_ap_ids_by_nicknames(nicknames) do + from(u in User, + where: u.nickname in ^nicknames, + select: u.ap_id + ) + |> Repo.all() + end end diff --git a/test/user_test.exs b/test/user_test.exs index 019f2b56d..d7473ef43 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -1266,4 +1266,16 @@ defmodule Pleroma.UserTest do assert user.info.keys == "xxx" end end + + describe "get_ap_ids_by_nicknames" do + test "it returns a list of AP ids for a given set of nicknames" do + user = insert(:user) + user_two = insert(:user) + + ap_ids = User.get_ap_ids_by_nicknames([user.nickname, user_two.nickname, "nonexistent"]) + assert length(ap_ids) == 2 + assert user.ap_id in ap_ids + assert user_two.ap_id in ap_ids + end + end end From 80d4d83aaccf49ddc2a594448851585bf80443bb Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 3 Jun 2019 18:17:08 +0200 Subject: [PATCH 02/23] CommonAPI: Add explicit addressing. --- lib/pleroma/web/common_api/common_api.ex | 4 +++- lib/pleroma/web/common_api/utils.ex | 24 +++++++++++++----------- test/web/common_api/common_api_test.exs | 19 +++++++++++++++++++ 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 5212d5ce5..f5f62eee3 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -201,8 +201,10 @@ defmodule Pleroma.Web.CommonAPI do data, visibility ), + mentioned_users <- for({_, mentioned_user} <- mentions, do: mentioned_user.ap_id), + addressed_users <- get_addressed_users(mentioned_users, data["to"]), {poll, poll_emoji} <- make_poll_data(data), - {to, cc} <- to_for_user_and_mentions(user, mentions, in_reply_to, visibility), + {to, cc} <- get_to_and_cc(user, addressed_users, in_reply_to, visibility), context <- make_context(in_reply_to), cw <- data["spoiler_text"] || "", sensitive <- data["sensitive"] || Enum.member?(tags, {"#nsfw", "nsfw"}), diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index f35ed36ab..6d82c0bd2 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -61,9 +61,9 @@ defmodule Pleroma.Web.CommonAPI.Utils do end) end - def to_for_user_and_mentions(user, mentions, inReplyTo, "public") do - mentioned_users = Enum.map(mentions, fn {_, %{ap_id: ap_id}} -> ap_id end) - + @spec get_to_and_cc(User.t(), list(String.t()), Activity.t() | nil, String.t()) :: + {list(String.t()), list(String.t())} + def get_to_and_cc(user, mentioned_users, inReplyTo, "public") do to = ["https://www.w3.org/ns/activitystreams#Public" | mentioned_users] cc = [user.follower_address] @@ -74,9 +74,7 @@ defmodule Pleroma.Web.CommonAPI.Utils do end end - def to_for_user_and_mentions(user, mentions, inReplyTo, "unlisted") do - mentioned_users = Enum.map(mentions, fn {_, %{ap_id: ap_id}} -> ap_id end) - + def get_to_and_cc(user, mentioned_users, inReplyTo, "unlisted") do to = [user.follower_address | mentioned_users] cc = ["https://www.w3.org/ns/activitystreams#Public"] @@ -87,14 +85,12 @@ defmodule Pleroma.Web.CommonAPI.Utils do end end - def to_for_user_and_mentions(user, mentions, inReplyTo, "private") do - {to, cc} = to_for_user_and_mentions(user, mentions, inReplyTo, "direct") + def get_to_and_cc(user, mentioned_users, inReplyTo, "private") do + {to, cc} = get_to_and_cc(user, mentioned_users, inReplyTo, "direct") {[user.follower_address | to], cc} end - def to_for_user_and_mentions(_user, mentions, inReplyTo, "direct") do - mentioned_users = Enum.map(mentions, fn {_, %{ap_id: ap_id}} -> ap_id end) - + def get_to_and_cc(_user, mentioned_users, inReplyTo, "direct") do if inReplyTo do {Enum.uniq([inReplyTo.data["actor"] | mentioned_users]), []} else @@ -102,6 +98,12 @@ defmodule Pleroma.Web.CommonAPI.Utils do end end + def get_addressed_users(_, to) when is_list(to) do + User.get_ap_ids_by_nicknames(to) + end + + def get_addressed_users(mentioned_users, _), do: mentioned_users + def make_poll_data(%{"poll" => %{"options" => options, "expires_in" => expires_in}} = data) when is_list(options) do %{max_expiration: max_expiration, min_expiration: min_expiration} = diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 696060fb1..26efe6140 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -56,6 +56,25 @@ defmodule Pleroma.Web.CommonAPITest do end describe "posting" do + test "it supports explicit addressing" do + user = insert(:user) + user_two = insert(:user) + user_three = insert(:user) + user_four = insert(:user) + + {:ok, activity} = + CommonAPI.post(user, %{ + "status" => + "Hey, I think @#{user_three.nickname} is ugly. @#{user_four.nickname} is alright though.", + "to" => [user_two.nickname, user_four.nickname, "nonexistent"] + }) + + assert user.ap_id in activity.recipients + assert user_two.ap_id in activity.recipients + assert user_four.ap_id in activity.recipients + refute user_three.ap_id in activity.recipients + end + test "it filters out obviously bad tags when accepting a post as HTML" do user = insert(:user) From 804b6a4d8f2e7db128f66bb0e38b8b1baf782302 Mon Sep 17 00:00:00 2001 From: lain Date: Mon, 3 Jun 2019 19:08:38 +0200 Subject: [PATCH 03/23] CommonAPI.Utils: Add tests. --- test/web/common_api/common_api_utils_test.exs | 133 ++++++++++++++++++++++++++ 1 file changed, 133 insertions(+) diff --git a/test/web/common_api/common_api_utils_test.exs b/test/web/common_api/common_api_utils_test.exs index ab4c62b35..b3a334d50 100644 --- a/test/web/common_api/common_api_utils_test.exs +++ b/test/web/common_api/common_api_utils_test.exs @@ -5,10 +5,15 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do alias Pleroma.Builders.UserBuilder alias Pleroma.Object + alias Pleroma.Web.CommonAPI alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.Endpoint use Pleroma.DataCase + import Pleroma.Factory + + @public_address "https://www.w3.org/ns/activitystreams#Public" + test "it adds attachment links to a given text and attachment set" do name = "Sakura%20Mana%20%E2%80%93%20Turned%20on%20by%20a%20Senior%20OL%20with%20a%20Temptating%20Tight%20Skirt-s%20Full%20Hipline%20and%20Panty%20Shot-%20Beautiful%20Thick%20Thighs-%20and%20Erotic%20Ass-%20-2015-%20--%20Oppaitime%208-28-2017%206-50-33%20PM.png" @@ -214,4 +219,132 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do assert Utils.date_to_asctime(nil) == expected end end + + describe "get_to_and_cc" do + test "for public posts, not a reply" do + user = insert(:user) + mentioned_user = insert(:user) + mentions = [mentioned_user.ap_id] + + {to, cc} = Utils.get_to_and_cc(user, mentions, nil, "public") + + assert length(to) == 2 + assert length(cc) == 1 + + assert @public_address in to + assert mentioned_user.ap_id in to + assert user.follower_address in cc + end + + test "for public posts, a reply" do + user = insert(:user) + mentioned_user = insert(:user) + third_user = insert(:user) + {:ok, activity} = CommonAPI.post(third_user, %{"status" => "uguu"}) + mentions = [mentioned_user.ap_id] + + {to, cc} = Utils.get_to_and_cc(user, mentions, activity, "public") + + assert length(to) == 3 + assert length(cc) == 1 + + assert @public_address in to + assert mentioned_user.ap_id in to + assert third_user.ap_id in to + assert user.follower_address in cc + end + + test "for unlisted posts, not a reply" do + user = insert(:user) + mentioned_user = insert(:user) + mentions = [mentioned_user.ap_id] + + {to, cc} = Utils.get_to_and_cc(user, mentions, nil, "unlisted") + + assert length(to) == 2 + assert length(cc) == 1 + + assert @public_address in cc + assert mentioned_user.ap_id in to + assert user.follower_address in to + end + + test "for unlisted posts, a reply" do + user = insert(:user) + mentioned_user = insert(:user) + third_user = insert(:user) + {:ok, activity} = CommonAPI.post(third_user, %{"status" => "uguu"}) + mentions = [mentioned_user.ap_id] + + {to, cc} = Utils.get_to_and_cc(user, mentions, activity, "unlisted") + + assert length(to) == 3 + assert length(cc) == 1 + + assert @public_address in cc + assert mentioned_user.ap_id in to + assert third_user.ap_id in to + assert user.follower_address in to + end + + test "for private posts, not a reply" do + user = insert(:user) + mentioned_user = insert(:user) + mentions = [mentioned_user.ap_id] + + {to, cc} = Utils.get_to_and_cc(user, mentions, nil, "private") + + assert length(to) == 2 + assert length(cc) == 0 + + assert mentioned_user.ap_id in to + assert user.follower_address in to + end + + test "for private posts, a reply" do + user = insert(:user) + mentioned_user = insert(:user) + third_user = insert(:user) + {:ok, activity} = CommonAPI.post(third_user, %{"status" => "uguu"}) + mentions = [mentioned_user.ap_id] + + {to, cc} = Utils.get_to_and_cc(user, mentions, activity, "private") + + assert length(to) == 3 + assert length(cc) == 0 + + assert mentioned_user.ap_id in to + assert third_user.ap_id in to + assert user.follower_address in to + end + + test "for direct posts, not a reply" do + user = insert(:user) + mentioned_user = insert(:user) + mentions = [mentioned_user.ap_id] + + {to, cc} = Utils.get_to_and_cc(user, mentions, nil, "direct") + + assert length(to) == 1 + assert length(cc) == 0 + + assert mentioned_user.ap_id in to + end + + test "for direct posts, a reply" do + user = insert(:user) + mentioned_user = insert(:user) + third_user = insert(:user) + {:ok, activity} = CommonAPI.post(third_user, %{"status" => "uguu"}) + mentions = [mentioned_user.ap_id] + + {to, cc} = Utils.get_to_and_cc(user, mentions, activity, "direct") + + assert length(to) == 2 + assert length(cc) == 0 + + assert mentioned_user.ap_id in to + assert third_user.ap_id in to + end + end end From 25198d48f7e799ff350c8c7c57518b6ee49e6f8d Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 4 Jun 2019 10:49:57 +0200 Subject: [PATCH 04/23] Docs: Add Explicit addressing to Readme and changelog. --- CHANGELOG.md | 1 + docs/api/differences_in_mastoapi_responses.md | 1 + 2 files changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2144bbe28..5b7191211 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## [unreleased] ### Added +- Explicit addressing option for posting. - Optional SSH access mode. (Needs `erlang-ssh` package on some distributions). - [MongooseIM](https://github.com/esl/MongooseIM) http authentication support. - LDAP authentication diff --git a/docs/api/differences_in_mastoapi_responses.md b/docs/api/differences_in_mastoapi_responses.md index 36b47608e..e8629e9ef 100644 --- a/docs/api/differences_in_mastoapi_responses.md +++ b/docs/api/differences_in_mastoapi_responses.md @@ -69,6 +69,7 @@ Additional parameters can be added to the JSON body/Form data: - `preview`: boolean, if set to `true` the post won't be actually posted, but the status entitiy would still be rendered back. This could be useful for previewing rich text/custom emoji, for example. - `content_type`: string, contain the MIME type of the status, it is transformed into HTML by the backend. You can get the list of the supported MIME types with the nodeinfo endpoint. +- `to`: A list of nicknames (like `lain@soykaf.club` or `lain` on the local server) that will be used to determine who is going to be addressed by this post. Using this will disable the implicit addressing by mentioned names in the `status` body, only the people in the `to` list will be addressed. The normal rules for for post visibility are not affected by this and will still apply. ## PATCH `/api/v1/update_credentials` From 18c35d84fd732a6d55c9d3174fcbb837e098ea0a Mon Sep 17 00:00:00 2001 From: lain Date: Tue, 4 Jun 2019 17:10:54 +0200 Subject: [PATCH 05/23] NodeInfo: Add explicit addressing to nodeinfo. --- lib/pleroma/web/nodeinfo/nodeinfo_controller.ex | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex b/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex index 57f5b61bb..32be430b7 100644 --- a/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex +++ b/lib/pleroma/web/nodeinfo/nodeinfo_controller.ex @@ -98,6 +98,7 @@ defmodule Pleroma.Web.Nodeinfo.NodeinfoController do "mastodon_api", "mastodon_api_streaming", "polls", + "pleroma_explicit_addressing", if Config.get([:media_proxy, :enabled]) do "media_proxy" end, From 93397fce3de54985bde3c3f260660a63157077be Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Fri, 31 May 2019 16:22:13 +0700 Subject: [PATCH 06/23] Contain search for unauthenticated users --- lib/pleroma/activity.ex | 2 + lib/pleroma/activity/search.ex | 75 ++++++++++++++++++++++ .../web/mastodon_api/mastodon_api_controller.ex | 61 +----------------- test/activity_test.exs | 38 +++++++++++ 4 files changed, 117 insertions(+), 59 deletions(-) create mode 100644 lib/pleroma/activity/search.ex diff --git a/lib/pleroma/activity.ex b/lib/pleroma/activity.ex index 99589590c..6db41fe6e 100644 --- a/lib/pleroma/activity.ex +++ b/lib/pleroma/activity.ex @@ -343,4 +343,6 @@ defmodule Pleroma.Activity do ) ) end + + defdelegate search(user, query), to: Pleroma.Activity.Search end diff --git a/lib/pleroma/activity/search.ex b/lib/pleroma/activity/search.ex new file mode 100644 index 000000000..f2fdfffe1 --- /dev/null +++ b/lib/pleroma/activity/search.ex @@ -0,0 +1,75 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Activity.Search do + alias Pleroma.Activity + alias Pleroma.Object.Fetcher + alias Pleroma.Repo + alias Pleroma.User + alias Pleroma.Web.ActivityPub.Visibility + + import Ecto.Query + + def search(user, search_query) do + index_type = if Pleroma.Config.get([:database, :rum_enabled]), do: :rum, else: :gin + + Activity + |> Activity.with_preloaded_object() + |> Activity.restrict_deactivated_users() + |> restrict_public() + |> query_with(index_type, search_query) + |> maybe_restrict_local(user) + |> Repo.all() + |> maybe_fetch(user, search_query) + end + + defp restrict_public(q) do + from([a, o] in q, + where: fragment("?->>'type' = 'Create'", a.data), + where: "https://www.w3.org/ns/activitystreams#Public" in a.recipients, + limit: 40 + ) + end + + defp query_with(q, :gin, search_query) do + from([a, o] in q, + where: + fragment( + "to_tsvector('english', ?->>'content') @@ plainto_tsquery('english', ?)", + o.data, + ^search_query + ), + order_by: [desc: :id] + ) + end + + defp query_with(q, :rum, search_query) do + from([a, o] in q, + where: + fragment( + "? @@ plainto_tsquery('english', ?)", + o.fts_content, + ^search_query + ), + order_by: [fragment("? <=> now()::date", o.inserted_at)] + ) + end + + # users can search everything + defp maybe_restrict_local(q, %User{}), do: q + + # unauthenticated users can only search local activities + defp maybe_restrict_local(q, _), do: where(q, local: true) + + defp maybe_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"]), + true <- Visibility.visible_for_user?(activity, user) do + activities ++ [activity] + else + _ -> activities + end + end +end diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index d825555c6..92cd77f62 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -14,7 +14,6 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do alias Pleroma.HTTP alias Pleroma.Notification alias Pleroma.Object - alias Pleroma.Object.Fetcher alias Pleroma.Pagination alias Pleroma.Repo alias Pleroma.ScheduledActivity @@ -1125,64 +1124,9 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do end end - def status_search_query_with_gin(q, query) do - from([a, o] in q, - where: - fragment( - "to_tsvector('english', ?->>'content') @@ plainto_tsquery('english', ?)", - o.data, - ^query - ), - order_by: [desc: :id] - ) - end - - def status_search_query_with_rum(q, query) do - from([a, o] in q, - where: - fragment( - "? @@ plainto_tsquery('english', ?)", - o.fts_content, - ^query - ), - order_by: [fragment("? <=> now()::date", o.inserted_at)] - ) - end - - def status_search(user, query) do - fetched = - if Regex.match?(~r/https?:/, query) do - with {:ok, object} <- Fetcher.fetch_object_from_id(query), - %Activity{} = activity <- Activity.get_create_by_object_ap_id(object.data["id"]), - true <- Visibility.visible_for_user?(activity, user) do - [activity] - else - _e -> [] - end - end || [] - - q = - from([a, o] in Activity.with_preloaded_object(Activity), - where: fragment("?->>'type' = 'Create'", a.data), - where: "https://www.w3.org/ns/activitystreams#Public" in a.recipients, - limit: 40 - ) - - q = - if Pleroma.Config.get([:database, :rum_enabled]) do - status_search_query_with_rum(q, query) - else - status_search_query_with_gin(q, query) - end - - Repo.all(q) ++ fetched - end - def search2(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do accounts = User.search(query, resolve: params["resolve"] == "true", for_user: user) - - statuses = status_search(user, query) - + statuses = Activity.search(user, query) tags_path = Web.base_url() <> "/tag/" tags = @@ -1205,8 +1149,7 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do def search(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do accounts = User.search(query, resolve: params["resolve"] == "true", for_user: user) - - statuses = status_search(user, query) + statuses = Activity.search(user, query) tags = query diff --git a/test/activity_test.exs b/test/activity_test.exs index 15c95502a..5260ebb9e 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -99,4 +99,42 @@ defmodule Pleroma.ActivityTest do assert Activity.get_bookmark(queried_activity, user) == bookmark end end + + describe "search" do + setup do + Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end) + + user = insert(:user) + + params = %{ + "@context" => "https://www.w3.org/ns/activitystreams", + "actor" => "http://mastodon.example.org/users/admin", + "type" => "Create", + "id" => "http://mastodon.example.org/users/admin/activities/1", + "object" => %{ + "type" => "Note", + "content" => "find me!", + "id" => "http://mastodon.example.org/users/admin/objects/1", + "attributedTo" => "http://mastodon.example.org/users/admin" + }, + "to" => ["https://www.w3.org/ns/activitystreams#Public"] + } + + {:ok, local_activity} = Pleroma.Web.CommonAPI.post(user, %{"status" => "find me!"}) + {:ok, remote_activity} = Pleroma.Web.Federator.incoming_ap_doc(params) + %{local_activity: local_activity, remote_activity: remote_activity, user: user} + end + + test "find local and remote statuses for authenticated users", %{ + local_activity: local_activity, + remote_activity: remote_activity, + user: user + } do + assert [^remote_activity, ^local_activity] = Activity.search(user, "find me") + end + + test "find only local statuses for unauthenticated users", %{local_activity: local_activity} do + assert [^local_activity] = Activity.search(nil, "find me") + end + end end From 94b9e9d844ad47c198817e732a54ad286caa346a Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Fri, 31 May 2019 16:37:33 +0700 Subject: [PATCH 07/23] Update benchmark mix task --- lib/mix/tasks/benchmark.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/mix/tasks/benchmark.ex b/lib/mix/tasks/benchmark.ex index 0fbb4dbb1..e4b1a638a 100644 --- a/lib/mix/tasks/benchmark.ex +++ b/lib/mix/tasks/benchmark.ex @@ -7,7 +7,7 @@ defmodule Mix.Tasks.Pleroma.Benchmark do Benchee.run(%{ "search" => fn -> - Pleroma.Web.MastodonAPI.MastodonAPIController.status_search(nil, "cofe") + Pleroma.Activity.search(nil, "cofe") end }) end From 17a6f81f7b2ecd42ea003dc5d81258d6bd04bfa5 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Fri, 31 May 2019 17:11:45 +0700 Subject: [PATCH 08/23] Fix tests with enabled RUM --- test/activity_test.exs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/test/activity_test.exs b/test/activity_test.exs index 5260ebb9e..1814e313d 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -130,7 +130,9 @@ defmodule Pleroma.ActivityTest do remote_activity: remote_activity, user: user } do - assert [^remote_activity, ^local_activity] = Activity.search(user, "find me") + activities = Enum.sort_by(Activity.search(user, "find me"), & &1.id) + + assert [^local_activity, ^remote_activity] = activities end test "find only local statuses for unauthenticated users", %{local_activity: local_activity} do From 17004a0f1a227d1ba8400fcdd5d351de83b09673 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 3 Jun 2019 18:57:24 +0700 Subject: [PATCH 09/23] Create index on `activities.local` --- .../migrations/20190603115238_add_index_on_activities_local.exs | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 priv/repo/migrations/20190603115238_add_index_on_activities_local.exs diff --git a/priv/repo/migrations/20190603115238_add_index_on_activities_local.exs b/priv/repo/migrations/20190603115238_add_index_on_activities_local.exs new file mode 100644 index 000000000..89daa9705 --- /dev/null +++ b/priv/repo/migrations/20190603115238_add_index_on_activities_local.exs @@ -0,0 +1,7 @@ +defmodule Pleroma.Repo.Migrations.AddIndexOnActivitiesLocal do + use Ecto.Migration + + def change do + create(index("activities", [:local])) + end +end From 5b04f07a1ebe6763270b406aa6638336cab04a31 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Wed, 5 Jun 2019 16:34:14 +0700 Subject: [PATCH 10/23] Limit search for unauthenticated users to local users only --- lib/pleroma/user.ex | 117 +---------------- lib/pleroma/user/search.ex | 145 +++++++++++++++++++++ test/user_test.exs | 30 ++++- .../mastodon_api/mastodon_api_controller_test.exs | 3 + 4 files changed, 178 insertions(+), 117 deletions(-) create mode 100644 lib/pleroma/user/search.ex diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index dc534b05c..498428269 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -735,121 +735,6 @@ defmodule Pleroma.User do |> Repo.all() end - def search(query, resolve \\ false, for_user \\ nil) do - # Strip the beginning @ off if there is a query - query = String.trim_leading(query, "@") - - if resolve, do: get_or_fetch(query) - - {:ok, results} = - Repo.transaction(fn -> - Ecto.Adapters.SQL.query(Repo, "select set_limit(0.25)", []) - Repo.all(search_query(query, for_user)) - end) - - results - end - - def search_query(query, for_user) do - fts_subquery = fts_search_subquery(query) - trigram_subquery = trigram_search_subquery(query) - union_query = from(s in trigram_subquery, union_all: ^fts_subquery) - distinct_query = from(s in subquery(union_query), order_by: s.search_type, distinct: s.id) - - from(s in subquery(boost_search_rank_query(distinct_query, for_user)), - order_by: [desc: s.search_rank], - limit: 40 - ) - end - - defp boost_search_rank_query(query, nil), do: query - - defp boost_search_rank_query(query, for_user) do - friends_ids = get_friends_ids(for_user) - followers_ids = get_followers_ids(for_user) - - from(u in subquery(query), - select_merge: %{ - search_rank: - fragment( - """ - CASE WHEN (?) THEN (?) * 1.3 - WHEN (?) THEN (?) * 1.2 - WHEN (?) THEN (?) * 1.1 - ELSE (?) END - """, - u.id in ^friends_ids and u.id in ^followers_ids, - u.search_rank, - u.id in ^friends_ids, - u.search_rank, - u.id in ^followers_ids, - u.search_rank, - u.search_rank - ) - } - ) - end - - defp fts_search_subquery(term, query \\ User) do - processed_query = - term - |> String.replace(~r/\W+/, " ") - |> String.trim() - |> String.split() - |> Enum.map(&(&1 <> ":*")) - |> Enum.join(" | ") - - from( - u in query, - select_merge: %{ - search_type: ^0, - search_rank: - fragment( - """ - ts_rank_cd( - setweight(to_tsvector('simple', regexp_replace(?, '\\W', ' ', 'g')), 'A') || - setweight(to_tsvector('simple', regexp_replace(coalesce(?, ''), '\\W', ' ', 'g')), 'B'), - to_tsquery('simple', ?), - 32 - ) - """, - u.nickname, - u.name, - ^processed_query - ) - }, - where: - fragment( - """ - (setweight(to_tsvector('simple', regexp_replace(?, '\\W', ' ', 'g')), 'A') || - setweight(to_tsvector('simple', regexp_replace(coalesce(?, ''), '\\W', ' ', 'g')), 'B')) @@ to_tsquery('simple', ?) - """, - u.nickname, - u.name, - ^processed_query - ) - ) - |> restrict_deactivated() - end - - defp trigram_search_subquery(term) do - from( - u in User, - select_merge: %{ - # ^1 gives 'Postgrex expected a binary, got 1' for some weird reason - search_type: fragment("?", 1), - search_rank: - fragment( - "similarity(?, trim(? || ' ' || coalesce(?, '')))", - ^term, - u.nickname, - u.name - ) - }, - where: fragment("trim(? || ' ' || coalesce(?, '')) % ?", u.nickname, u.name, ^term) - ) - |> restrict_deactivated() - end def mute(muter, %User{ap_id: ap_id}) do info_cng = @@ -1449,4 +1334,6 @@ defmodule Pleroma.User do ) |> Repo.all() end + + defdelegate search(query, opts \\ []), to: User.Search end diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex new file mode 100644 index 000000000..d5b2eaa9f --- /dev/null +++ b/lib/pleroma/user/search.ex @@ -0,0 +1,145 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2019 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.User.Search do + alias Pleroma.Repo + alias Pleroma.User + import Ecto.Query + + def search(query, opts \\ []) do + resolve = Keyword.get(opts, :resolve, false) + for_user = Keyword.get(opts, :for_user) + + # Strip the beginning @ off if there is a query + query = String.trim_leading(query, "@") + + if match?(%User{}, for_user) and resolve, do: User.get_or_fetch(query) + + {:ok, results} = + Repo.transaction(fn -> + Ecto.Adapters.SQL.query(Repo, "select set_limit(0.25)", []) + + query + |> search_query(for_user) + |> Repo.all() + end) + + results + end + + defp search_query(query, for_user) do + query + |> union_query() + |> distinct_query() + |> boost_search_rank_query(for_user) + |> subquery() + |> order_by(desc: :search_rank) + |> limit(20) + |> maybe_restrict_local(for_user) + end + + defp union_query(query) do + fts_subquery = fts_search_subquery(query) + trigram_subquery = trigram_search_subquery(query) + + from(s in trigram_subquery, union_all: ^fts_subquery) + end + + defp distinct_query(q) do + from(s in subquery(q), order_by: s.search_type, distinct: s.id) + end + + # unauthenticated users can only search local activities + defp maybe_restrict_local(q, %User{}), do: q + defp maybe_restrict_local(q, _), do: where(q, [u], u.local == true) + + defp boost_search_rank_query(query, nil), do: query + + defp boost_search_rank_query(query, for_user) do + friends_ids = User.get_friends_ids(for_user) + followers_ids = User.get_followers_ids(for_user) + + from(u in subquery(query), + select_merge: %{ + search_rank: + fragment( + """ + CASE WHEN (?) THEN (?) * 1.3 + WHEN (?) THEN (?) * 1.2 + WHEN (?) THEN (?) * 1.1 + ELSE (?) END + """, + u.id in ^friends_ids and u.id in ^followers_ids, + u.search_rank, + u.id in ^friends_ids, + u.search_rank, + u.id in ^followers_ids, + u.search_rank, + u.search_rank + ) + } + ) + end + + defp fts_search_subquery(term, query \\ User) do + processed_query = + term + |> String.replace(~r/\W+/, " ") + |> String.trim() + |> String.split() + |> Enum.map(&(&1 <> ":*")) + |> Enum.join(" | ") + + from( + u in query, + select_merge: %{ + search_type: ^0, + search_rank: + fragment( + """ + ts_rank_cd( + setweight(to_tsvector('simple', regexp_replace(?, '\\W', ' ', 'g')), 'A') || + setweight(to_tsvector('simple', regexp_replace(coalesce(?, ''), '\\W', ' ', 'g')), 'B'), + to_tsquery('simple', ?), + 32 + ) + """, + u.nickname, + u.name, + ^processed_query + ) + }, + where: + fragment( + """ + (setweight(to_tsvector('simple', regexp_replace(?, '\\W', ' ', 'g')), 'A') || + setweight(to_tsvector('simple', regexp_replace(coalesce(?, ''), '\\W', ' ', 'g')), 'B')) @@ to_tsquery('simple', ?) + """, + u.nickname, + u.name, + ^processed_query + ) + ) + |> User.restrict_deactivated() + end + + defp trigram_search_subquery(term) do + from( + u in User, + select_merge: %{ + # ^1 gives 'Postgrex expected a binary, got 1' for some weird reason + search_type: fragment("?", 1), + search_rank: + fragment( + "similarity(?, trim(? || ' ' || coalesce(?, '')))", + ^term, + u.nickname, + u.name + ) + }, + where: fragment("trim(? || ' ' || coalesce(?, '')) % ?", u.nickname, u.name, ^term) + ) + |> User.restrict_deactivated() + end +end diff --git a/test/user_test.exs b/test/user_test.exs index d7473ef43..1a82aa6f7 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -1055,7 +1055,7 @@ defmodule Pleroma.UserTest do u3 = insert(:user, %{name: "ebn", nickname: "lain@mastodon.social"}) u4 = insert(:user, %{nickname: "lain@pleroma.soykaf.com"}) - assert [u4.id, u3.id, u1.id] == Enum.map(User.search("lain@ple"), & &1.id) + assert [u4.id, u3.id, u1.id] == Enum.map(User.search("lain@ple", for_user: u1), & &1.id) end test "finds users, handling misspelled requests" do @@ -1077,6 +1077,28 @@ defmodule Pleroma.UserTest do Enum.map(User.search("doe", resolve: false, for_user: u1), & &1.id) == [] end + test "find local and remote statuses for authenticated users" do + u1 = insert(:user, %{name: "lain"}) + u2 = insert(:user, %{name: "ebn", nickname: "lain@mastodon.social", local: false}) + u3 = insert(:user, %{nickname: "lain@pleroma.soykaf.com", local: false}) + + results = + "lain" + |> User.search(for_user: u1) + |> Enum.map(& &1.id) + |> Enum.sort() + + assert [u1.id, u2.id, u3.id] == results + end + + test "find only local statuses for unauthenticated users" do + %{id: id} = insert(:user, %{name: "lain"}) + insert(:user, %{name: "ebn", nickname: "lain@mastodon.social", local: false}) + insert(:user, %{nickname: "lain@pleroma.soykaf.com", local: false}) + + assert [%{id: ^id}] = User.search("lain") + end + test "finds a user whose name is nil" do _user = insert(:user, %{name: "notamatch", nickname: "testuser@pleroma.amplifie.red"}) user_two = insert(:user, %{name: nil, nickname: "lain@pleroma.soykaf.com"}) @@ -1097,7 +1119,11 @@ defmodule Pleroma.UserTest do end test "works with URIs" do - results = User.search("http://mastodon.example.org/users/admin", resolve: true) + user = insert(:user) + + results = + User.search("http://mastodon.example.org/users/admin", resolve: true, for_user: user) + result = results |> List.first() user = User.get_cached_by_ap_id("http://mastodon.example.org/users/admin") diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index 8679a083d..51c1cdfac 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -2173,8 +2173,11 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIControllerTest do end test "search fetches remote accounts", %{conn: conn} do + user = insert(:user) + conn = conn + |> assign(:user, user) |> get("/api/v1/search", %{"q" => "shp@social.heldscal.la", "resolve" => "true"}) assert results = json_response(conn, 200) From 1cb245c9825febb0b83cfc361f78d132cb5d05a8 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Wed, 5 Jun 2019 16:55:17 +0700 Subject: [PATCH 11/23] Fix formatting --- lib/pleroma/user.ex | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 498428269..cae2c14e3 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -735,7 +735,6 @@ defmodule Pleroma.User do |> Repo.all() end - def mute(muter, %User{ap_id: ap_id}) do info_cng = muter.info From 3eefb274f45e57ad855246cb930a6a094eeffe0e Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 5 Jun 2019 13:02:13 +0300 Subject: [PATCH 12/23] OAuth consumer: tests fix, comments, Keycloak config notes. --- config/test.exs | 2 ++ docs/config.md | 20 +++++++++++++++++++- lib/pleroma/web/auth/pleroma_authenticator.ex | 11 ++++++++++- lib/pleroma/web/oauth/oauth_controller.ex | 6 +++++- 4 files changed, 36 insertions(+), 3 deletions(-) diff --git a/config/test.exs b/config/test.exs index 41cddb9bd..7861b9598 100644 --- a/config/test.exs +++ b/config/test.exs @@ -17,6 +17,8 @@ config :pleroma, Pleroma.Captcha, # Print only warnings and errors during test config :logger, level: :warn +config :pleroma, :auth, oauth_consumer_strategies: [] + config :pleroma, Pleroma.Upload, filters: [], link_name: false config :pleroma, Pleroma.Uploaders.Local, uploads: "test/uploads" diff --git a/docs/config.md b/docs/config.md index f4a1868fd..93ede6464 100644 --- a/docs/config.md +++ b/docs/config.md @@ -514,7 +514,7 @@ Authentication / authorization settings. * `auth_template`: authentication form template. By default it's `show.html` which corresponds to `lib/pleroma/web/templates/o_auth/o_auth/show.html.eex`. * `oauth_consumer_template`: OAuth consumer mode authentication form template. By default it's `consumer.html` which corresponds to `lib/pleroma/web/templates/o_auth/o_auth/consumer.html.eex`. -* `oauth_consumer_strategies`: the list of enabled OAuth consumer strategies; by default it's set by OAUTH_CONSUMER_STRATEGIES environment variable. Each entry in this space-delimited string should be of format `` or `:` (e.g. `twitter` or `keycloak:ueberauth_keycloak_strategy` in case dependency is named differently than `ueberauth_`). +* `oauth_consumer_strategies`: the list of enabled OAuth consumer strategies; by default it's set by `OAUTH_CONSUMER_STRATEGIES` environment variable. Each entry in this space-delimited string should be of format `` or `:` (e.g. `twitter` or `keycloak:ueberauth_keycloak_strategy` in case dependency is named differently than `ueberauth_`). ## OAuth consumer mode @@ -567,6 +567,24 @@ config :ueberauth, Ueberauth, providers: [ microsoft: {Ueberauth.Strategy.Microsoft, [callback_params: []]} ] + +# Keycloak +# Note: make sure to add `keycloak:ueberauth_keycloak_strategy` entry to `OAUTH_CONSUMER_STRATEGIES` environment variable +keycloak_url = "https://publicly-reachable-keycloak-instance.org:8080" + +config :ueberauth, Ueberauth.Strategy.Keycloak.OAuth, + client_id: System.get_env("KEYCLOAK_CLIENT_ID"), + client_secret: System.get_env("KEYCLOAK_CLIENT_SECRET"), + site: keycloak_url, + authorize_url: "#{keycloak_url}/auth/realms/master/protocol/openid-connect/auth", + token_url: "#{keycloak_url}/auth/realms/master/protocol/openid-connect/token", + userinfo_url: "#{keycloak_url}/auth/realms/master/protocol/openid-connect/userinfo", + token_method: :post + +config :ueberauth, Ueberauth, + providers: [ + keycloak: {Ueberauth.Strategy.Keycloak, [uid_field: :email]} + ] ``` ## OAuth 2.0 provider - :oauth2 diff --git a/lib/pleroma/web/auth/pleroma_authenticator.ex b/lib/pleroma/web/auth/pleroma_authenticator.ex index c4a6fce08..a9164ad98 100644 --- a/lib/pleroma/web/auth/pleroma_authenticator.ex +++ b/lib/pleroma/web/auth/pleroma_authenticator.ex @@ -24,6 +24,14 @@ defmodule Pleroma.Web.Auth.PleromaAuthenticator do end end + @doc """ + Gets or creates Pleroma.Registration record from Ueberauth assigns. + Note: some strategies (like `keycloak`) might need extra configuration to fill `uid` from callback response — + see [`docs/config.md`](docs/config.md). + """ + def get_registration(%Plug.Conn{assigns: %{ueberauth_auth: %{uid: nil}}}), + do: {:error, :missing_uid} + def get_registration(%Plug.Conn{ assigns: %{ueberauth_auth: %{provider: provider, uid: uid} = auth} }) do @@ -51,9 +59,10 @@ defmodule Pleroma.Web.Auth.PleromaAuthenticator do def get_registration(%Plug.Conn{} = _conn), do: {:error, :missing_credentials} + @doc "Creates Pleroma.User record basing on params and Pleroma.Registration record." def create_from_registration( %Plug.Conn{params: %{"authorization" => registration_attrs}}, - registration + %Registration{} = registration ) do nickname = value([registration_attrs["nickname"], Registration.nickname(registration)]) email = value([registration_attrs["email"], Registration.email(registration)]) diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex index ae2b80d95..79d803295 100644 --- a/lib/pleroma/web/oauth/oauth_controller.ex +++ b/lib/pleroma/web/oauth/oauth_controller.ex @@ -17,6 +17,8 @@ defmodule Pleroma.Web.OAuth.OAuthController do alias Pleroma.Web.OAuth.Token.Strategy.Revoke, as: RevokeToken alias Pleroma.Web.OAuth.Scopes + require Logger + if Pleroma.Config.oauth_consumer_enabled?(), do: plug(Ueberauth) plug(:fetch_session) @@ -318,7 +320,9 @@ defmodule Pleroma.Web.OAuth.OAuthController do |> registration_details(%{"authorization" => registration_params}) end else - _ -> + error -> + Logger.debug(inspect(["OAUTH_ERROR", error, conn.assigns])) + conn |> put_flash(:error, "Failed to set up user account.") |> redirect(external: redirect_uri(conn, params["redirect_uri"])) From 8b9a0dd4a7e60f610e3aa1db92e62bc0fbe54521 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 5 Jun 2019 12:06:45 +0200 Subject: [PATCH 13/23] User: Don't error out when following a user that's already followed. This leads to a few situations where it is impossible to follow a user. --- lib/pleroma/user.ex | 4 +- .../transmogrifier/follow_handling_test.exs | 115 +++++++++++++++++++++ test/web/activity_pub/transmogrifier_test.exs | 54 ---------- test/web/twitter_api/twitter_api_test.exs | 6 +- 4 files changed, 119 insertions(+), 60 deletions(-) create mode 100644 test/web/activity_pub/transmogrifier/follow_handling_test.exs diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index dc534b05c..48b9f1d7d 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -370,8 +370,8 @@ defmodule Pleroma.User do ap_followers = followed.follower_address cond do - following?(follower, followed) or info.deactivated -> - {:error, "Could not follow user: #{followed.nickname} is already on your list."} + info.deactivated -> + {:error, "Could not follow user: You are deactivatedt."} deny_follow_blocked and blocks?(followed, follower) -> {:error, "Could not follow user: #{followed.nickname} blocked you."} diff --git a/test/web/activity_pub/transmogrifier/follow_handling_test.exs b/test/web/activity_pub/transmogrifier/follow_handling_test.exs new file mode 100644 index 000000000..9f89e876b --- /dev/null +++ b/test/web/activity_pub/transmogrifier/follow_handling_test.exs @@ -0,0 +1,115 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2018 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ActivityPub.Transmogrifier.FollowHandlingTest do + use Pleroma.DataCase + alias Pleroma.Activity + alias Pleroma.Repo + alias Pleroma.User + alias Pleroma.Web.ActivityPub.Transmogrifier + alias Pleroma.Web.ActivityPub.Utils + + import Pleroma.Factory + import Ecto.Query + + setup_all do + Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end) + :ok + end + + describe "handle_incoming" do + test "it works for incoming follow requests" do + user = insert(:user) + + data = + File.read!("test/fixtures/mastodon-follow-activity.json") + |> Poison.decode!() + |> Map.put("object", user.ap_id) + + {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) + + assert data["actor"] == "http://mastodon.example.org/users/admin" + assert data["type"] == "Follow" + assert data["id"] == "http://mastodon.example.org/users/admin#follows/2" + assert User.following?(User.get_cached_by_ap_id(data["actor"]), user) + end + + test "it works for follow requests when you are already followed, creating a new accept activity" do + # This is important because the remote might have the wrong idea about the current follow status. + # This can lead to instance A thinking that x@A is followed by y@B, but B thinks they are not. In + # this case, the follow can never go through again because it will never get an Accept. + user = insert(:user) + + data = + File.read!("test/fixtures/mastodon-follow-activity.json") + |> Poison.decode!() + |> Map.put("object", user.ap_id) + + {:ok, %Activity{local: false}} = Transmogrifier.handle_incoming(data) + + accepts = + from( + a in Activity, + where: fragment("?->>'type' = ?", a.data, "Accept") + ) + |> Repo.all() + + assert length(accepts) == 1 + + data = + File.read!("test/fixtures/mastodon-follow-activity.json") + |> Poison.decode!() + |> Map.put("id", String.replace(data["id"], "2", "3")) + |> Map.put("object", user.ap_id) + + {:ok, %Activity{local: false}} = Transmogrifier.handle_incoming(data) + + accepts = + from( + a in Activity, + where: fragment("?->>'type' = ?", a.data, "Accept") + ) + |> Repo.all() + + assert length(accepts) == 2 + end + + test "it rejects incoming follow requests from blocked users when deny_follow_blocked is enabled" do + Pleroma.Config.put([:user, :deny_follow_blocked], true) + + user = insert(:user) + {:ok, target} = User.get_or_fetch("http://mastodon.example.org/users/admin") + + {:ok, user} = User.block(user, target) + + data = + File.read!("test/fixtures/mastodon-follow-activity.json") + |> Poison.decode!() + |> Map.put("object", user.ap_id) + + {:ok, %Activity{data: %{"id" => id}}} = Transmogrifier.handle_incoming(data) + + %Activity{} = activity = Activity.get_by_ap_id(id) + + assert activity.data["state"] == "reject" + end + + test "it works for incoming follow requests from hubzilla" do + user = insert(:user) + + data = + File.read!("test/fixtures/hubzilla-follow-activity.json") + |> Poison.decode!() + |> Map.put("object", user.ap_id) + |> Utils.normalize_params() + + {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) + + assert data["actor"] == "https://hubzilla.example.org/channel/kaniini" + assert data["type"] == "Follow" + assert data["id"] == "https://hubzilla.example.org/channel/kaniini#follows/2" + assert User.following?(User.get_cached_by_ap_id(data["actor"]), user) + end + end +end diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 89c8f79c9..28971ae45 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -11,7 +11,6 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Transmogrifier - alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.OStatus alias Pleroma.Web.Websub.WebsubClientSubscription @@ -248,59 +247,6 @@ defmodule Pleroma.Web.ActivityPub.TransmogrifierTest do assert object_data["cc"] == to end - test "it works for incoming follow requests" do - user = insert(:user) - - data = - File.read!("test/fixtures/mastodon-follow-activity.json") - |> Poison.decode!() - |> Map.put("object", user.ap_id) - - {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) - - assert data["actor"] == "http://mastodon.example.org/users/admin" - assert data["type"] == "Follow" - assert data["id"] == "http://mastodon.example.org/users/admin#follows/2" - assert User.following?(User.get_cached_by_ap_id(data["actor"]), user) - end - - test "it rejects incoming follow requests from blocked users when deny_follow_blocked is enabled" do - Pleroma.Config.put([:user, :deny_follow_blocked], true) - - user = insert(:user) - {:ok, target} = User.get_or_fetch("http://mastodon.example.org/users/admin") - - {:ok, user} = User.block(user, target) - - data = - File.read!("test/fixtures/mastodon-follow-activity.json") - |> Poison.decode!() - |> Map.put("object", user.ap_id) - - {:ok, %Activity{data: %{"id" => id}}} = Transmogrifier.handle_incoming(data) - - %Activity{} = activity = Activity.get_by_ap_id(id) - - assert activity.data["state"] == "reject" - end - - test "it works for incoming follow requests from hubzilla" do - user = insert(:user) - - data = - File.read!("test/fixtures/hubzilla-follow-activity.json") - |> Poison.decode!() - |> Map.put("object", user.ap_id) - |> Utils.normalize_params() - - {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) - - assert data["actor"] == "https://hubzilla.example.org/channel/kaniini" - assert data["type"] == "Follow" - assert data["id"] == "https://hubzilla.example.org/channel/kaniini#follows/2" - assert User.following?(User.get_cached_by_ap_id(data["actor"]), user) - end - test "it works for incoming likes" do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => "hello"}) diff --git a/test/web/twitter_api/twitter_api_test.exs b/test/web/twitter_api/twitter_api_test.exs index d601c8f1f..475531a09 100644 --- a/test/web/twitter_api/twitter_api_test.exs +++ b/test/web/twitter_api/twitter_api_test.exs @@ -116,8 +116,7 @@ defmodule Pleroma.Web.TwitterAPI.TwitterAPITest do {:ok, user, followed, _activity} = TwitterAPI.follow(user, %{"user_id" => followed.id}) assert User.ap_followers(followed) in user.following - {:error, msg} = TwitterAPI.follow(user, %{"user_id" => followed.id}) - assert msg == "Could not follow user: #{followed.nickname} is already on your list." + {:ok, _, _, _} = TwitterAPI.follow(user, %{"user_id" => followed.id}) end test "Follow another user using screen_name" do @@ -132,8 +131,7 @@ defmodule Pleroma.Web.TwitterAPI.TwitterAPITest do followed = User.get_cached_by_ap_id(followed.ap_id) assert followed.info.follower_count == 1 - {:error, msg} = TwitterAPI.follow(user, %{"screen_name" => followed.nickname}) - assert msg == "Could not follow user: #{followed.nickname} is already on your list." + {:ok, _, _, _} = TwitterAPI.follow(user, %{"screen_name" => followed.nickname}) end test "Unfollow another user using user_id" do From 024dfdc39c80e3a0c9f625f31101fc6aa896e93d Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 5 Jun 2019 12:45:28 +0200 Subject: [PATCH 14/23] Typo + Linting. --- lib/pleroma/user.ex | 2 +- test/web/activity_pub/transmogrifier/follow_handling_test.exs | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index 48b9f1d7d..d873d7623 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -371,7 +371,7 @@ defmodule Pleroma.User do cond do info.deactivated -> - {:error, "Could not follow user: You are deactivatedt."} + {:error, "Could not follow user: You are deactivated."} deny_follow_blocked and blocks?(followed, follower) -> {:error, "Could not follow user: #{followed.nickname} blocked you."} diff --git a/test/web/activity_pub/transmogrifier/follow_handling_test.exs b/test/web/activity_pub/transmogrifier/follow_handling_test.exs index 9f89e876b..5ddf6cd52 100644 --- a/test/web/activity_pub/transmogrifier/follow_handling_test.exs +++ b/test/web/activity_pub/transmogrifier/follow_handling_test.exs @@ -36,9 +36,10 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.FollowHandlingTest do end test "it works for follow requests when you are already followed, creating a new accept activity" do - # This is important because the remote might have the wrong idea about the current follow status. - # This can lead to instance A thinking that x@A is followed by y@B, but B thinks they are not. In - # this case, the follow can never go through again because it will never get an Accept. + # This is important because the remote might have the wrong idea about the + # current follow status. This can lead to instance A thinking that x@A is + # followed by y@B, but B thinks they are not. In this case, the follow can + # never go through again because it will never get an Accept. user = insert(:user) data = From 3115b64cfe220c1db61c71fc4cef51bdf167b9ab Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 5 Jun 2019 14:10:46 +0200 Subject: [PATCH 15/23] Transmogrifier: Add tests for incoming follows to locked accounts. --- lib/pleroma/web/activity_pub/transmogrifier.ex | 7 +++--- .../transmogrifier/follow_handling_test.exs | 29 +++++++++++++++++++++- 2 files changed, 32 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 66fa7c0b3..8aa44ec09 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -458,10 +458,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do {:ok, %User{} = follower} <- User.get_or_fetch_by_ap_id(follower), {:ok, activity} <- ActivityPub.follow(follower, followed, id, false) do with deny_follow_blocked <- Pleroma.Config.get([:user, :deny_follow_blocked]), - {:user_blocked, false} <- + {_, false} <- {:user_blocked, User.blocks?(followed, follower) && deny_follow_blocked}, - {:user_locked, false} <- {:user_locked, User.locked?(followed)}, - {:follow, {:ok, follower}} <- {:follow, User.follow(follower, followed)} do + {_, false} <- {:user_locked, User.locked?(followed)}, + {_, {:ok, follower}} <- {:follow, User.follow(follower, followed)}, + {_, {:ok, _}} <- {:follow_state_update, Utils.update_follow_state(activity, "accept")} do ActivityPub.accept(%{ to: [follower.ap_id], actor: followed, diff --git a/test/web/activity_pub/transmogrifier/follow_handling_test.exs b/test/web/activity_pub/transmogrifier/follow_handling_test.exs index 5ddf6cd52..857d65564 100644 --- a/test/web/activity_pub/transmogrifier/follow_handling_test.exs +++ b/test/web/activity_pub/transmogrifier/follow_handling_test.exs @@ -27,14 +27,41 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.FollowHandlingTest do |> Poison.decode!() |> Map.put("object", user.ap_id) - {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) + {:ok, %Activity{data: data, local: false} = activity} = Transmogrifier.handle_incoming(data) assert data["actor"] == "http://mastodon.example.org/users/admin" assert data["type"] == "Follow" assert data["id"] == "http://mastodon.example.org/users/admin#follows/2" + + activity = Repo.get(Activity, activity.id) + assert activity.data["state"] == "accept" assert User.following?(User.get_cached_by_ap_id(data["actor"]), user) end + test "with locked accounts, it does not create a follow or an accept" do + user = insert(:user, info: %{locked: true}) + + data = + File.read!("test/fixtures/mastodon-follow-activity.json") + |> Poison.decode!() + |> Map.put("object", user.ap_id) + + {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) + + assert data["state"] == "pending" + + refute User.following?(User.get_cached_by_ap_id(data["actor"]), user) + + accepts = + from( + a in Activity, + where: fragment("?->>'type' = ?", a.data, "Accept") + ) + |> Repo.all() + + assert length(accepts) == 0 + end + test "it works for follow requests when you are already followed, creating a new accept activity" do # This is important because the remote might have the wrong idea about the # current follow status. This can lead to instance A thinking that x@A is From 076c9ae40e1b944839472d5d337d32335590ab0c Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 5 Jun 2019 14:24:31 +0200 Subject: [PATCH 16/23] User: Remove superfluous `maybe_follow`. --- lib/pleroma/user.ex | 8 -------- lib/pleroma/web/activity_pub/transmogrifier.ex | 21 ++++++++------------- lib/pleroma/web/common_api/common_api.ex | 2 +- test/user_test.exs | 2 +- 4 files changed, 10 insertions(+), 23 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index d873d7623..3d8fa0edb 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -324,14 +324,6 @@ defmodule Pleroma.User do end end - def maybe_follow(%User{} = follower, %User{info: _info} = followed) do - if not following?(follower, followed) do - follow(follower, followed) - else - {:ok, follower} - end - end - @doc "A mass follow for local users. Respects blocks in both directions but does not create activities." @spec follow_all(User.t(), list(User.t())) :: {atom(), User.t()} def follow_all(follower, followeds) do diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index 8aa44ec09..e8df112b9 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -509,19 +509,14 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do {:ok, follow_activity} <- get_follow_activity(follow_object, followed), {:ok, follow_activity} <- Utils.update_follow_state(follow_activity, "accept"), %User{local: true} = follower <- User.get_cached_by_ap_id(follow_activity.data["actor"]), - {:ok, activity} <- - ActivityPub.accept(%{ - to: follow_activity.data["to"], - type: "Accept", - actor: followed, - object: follow_activity.data["id"], - local: false - }) do - if not User.following?(follower, followed) do - {:ok, _follower} = User.follow(follower, followed) - end - - {:ok, activity} + {:ok, _follower} = User.follow(follower, followed) do + ActivityPub.accept(%{ + to: follow_activity.data["to"], + type: "Accept", + actor: followed, + object: follow_activity.data["id"], + local: false + }) else _e -> :error end diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 2b3ae3de5..71e166128 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -35,7 +35,7 @@ defmodule Pleroma.Web.CommonAPI do end def accept_follow_request(follower, followed) do - with {:ok, follower} <- User.maybe_follow(follower, followed), + with {:ok, follower} <- User.follow(follower, followed), %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed), {:ok, follow_activity} <- Utils.update_follow_state(follow_activity, "accept"), {:ok, _activity} <- diff --git a/test/user_test.exs b/test/user_test.exs index d7473ef43..466ddbf74 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -75,7 +75,7 @@ defmodule Pleroma.UserTest do Pleroma.Web.TwitterAPI.TwitterAPI.follow(pending_follower, %{"user_id" => locked.id}) Pleroma.Web.TwitterAPI.TwitterAPI.follow(pending_follower, %{"user_id" => locked.id}) Pleroma.Web.TwitterAPI.TwitterAPI.follow(accepted_follower, %{"user_id" => locked.id}) - User.maybe_follow(accepted_follower, locked) + User.follow(accepted_follower, locked) assert {:ok, [activity]} = User.get_follow_requests(locked) assert activity From 827a51e777a917c0a0f949c95d33192fd60c1b60 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 5 Jun 2019 15:43:54 +0200 Subject: [PATCH 17/23] CommonAPI: Add test for accept_follow_request. --- test/web/common_api/common_api_test.exs | 43 +++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 26efe6140..7ff23b63d 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -7,6 +7,7 @@ defmodule Pleroma.Web.CommonAPITest do alias Pleroma.Activity alias Pleroma.Object alias Pleroma.User + alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.CommonAPI import Pleroma.Factory @@ -339,4 +340,46 @@ defmodule Pleroma.Web.CommonAPITest do assert User.showing_reblogs?(muter, muted) == true end end + + describe "accept_follow_request/2" do + test "after acceptance, it sets all existing pending follow request states to 'accept'" do + user = insert(:user, info: %{locked: true}) + follower = insert(:user) + follower_two = insert(:user) + + {:ok, follow_activity} = ActivityPub.follow(follower, user) + {:ok, follow_activity_two} = ActivityPub.follow(follower, user) + {:ok, follow_activity_three} = ActivityPub.follow(follower_two, user) + + assert follow_activity.data["state"] == "pending" + assert follow_activity_two.data["state"] == "pending" + assert follow_activity_three.data["state"] == "pending" + + {:ok, _follower} = CommonAPI.accept_follow_request(follower, user) + + assert Repo.get(Activity, follow_activity.id).data["state"] == "accept" + assert Repo.get(Activity, follow_activity_two.id).data["state"] == "accept" + assert Repo.get(Activity, follow_activity_three.id).data["state"] == "pending" + end + + test "after rejection, it sets all existing pending follow request states to 'reject'" do + user = insert(:user, info: %{locked: true}) + follower = insert(:user) + follower_two = insert(:user) + + {:ok, follow_activity} = ActivityPub.follow(follower, user) + {:ok, follow_activity_two} = ActivityPub.follow(follower, user) + {:ok, follow_activity_three} = ActivityPub.follow(follower_two, user) + + assert follow_activity.data["state"] == "pending" + assert follow_activity_two.data["state"] == "pending" + assert follow_activity_three.data["state"] == "pending" + + {:ok, _follower} = CommonAPI.reject_follow_request(follower, user) + + assert Repo.get(Activity, follow_activity.id).data["state"] == "reject" + assert Repo.get(Activity, follow_activity_two.id).data["state"] == "reject" + assert Repo.get(Activity, follow_activity_three.id).data["state"] == "pending" + end + end end From ad19bfc7feecddd5a0f049ece436c1415335efa3 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 5 Jun 2019 16:43:35 +0200 Subject: [PATCH 18/23] Utils: Split update_follow_state and update_follow_state_for_all. --- lib/pleroma/web/activity_pub/utils.ex | 4 +-- test/web/activity_pub/utils_test.exs | 48 +++++++++++++++++++++++++++++++++++ 2 files changed, 50 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/activity_pub/utils.ex b/lib/pleroma/web/activity_pub/utils.ex index faae7e747..10ff572a2 100644 --- a/lib/pleroma/web/activity_pub/utils.ex +++ b/lib/pleroma/web/activity_pub/utils.ex @@ -376,8 +376,8 @@ defmodule Pleroma.Web.ActivityPub.Utils do @doc """ Updates a follow activity's state (for locked accounts). """ - def update_follow_state( - %Activity{data: %{"actor" => actor, "object" => object, "state" => "pending"}} = activity, + def update_follow_state_for_all( + %Activity{data: %{"actor" => actor, "object" => object}} = activity, state ) do try do diff --git a/test/web/activity_pub/utils_test.exs b/test/web/activity_pub/utils_test.exs index de741c64b..932d5f5e7 100644 --- a/test/web/activity_pub/utils_test.exs +++ b/test/web/activity_pub/utils_test.exs @@ -2,6 +2,7 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do use Pleroma.DataCase alias Pleroma.Activity alias Pleroma.Object + alias Pleroma.Repo alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Utils @@ -247,4 +248,51 @@ defmodule Pleroma.Web.ActivityPub.UtilsTest do assert fetched_vote.id == vote.id end end + + describe "update_follow_state_for_all/2" do + test "updates the state of all Follow activities with the same actor and object" do + user = insert(:user, info: %{locked: true}) + follower = insert(:user) + + {:ok, follow_activity} = ActivityPub.follow(follower, user) + {:ok, follow_activity_two} = ActivityPub.follow(follower, user) + + data = + follow_activity_two.data + |> Map.put("state", "accept") + + cng = Ecto.Changeset.change(follow_activity_two, data: data) + + {:ok, follow_activity_two} = Repo.update(cng) + + {:ok, follow_activity_two} = + Utils.update_follow_state_for_all(follow_activity_two, "accept") + + assert Repo.get(Activity, follow_activity.id).data["state"] == "accept" + assert Repo.get(Activity, follow_activity_two.id).data["state"] == "accept" + end + end + + describe "update_follow_state/2" do + test "updates the state of the given follow activity" do + user = insert(:user, info: %{locked: true}) + follower = insert(:user) + + {:ok, follow_activity} = ActivityPub.follow(follower, user) + {:ok, follow_activity_two} = ActivityPub.follow(follower, user) + + data = + follow_activity_two.data + |> Map.put("state", "accept") + + cng = Ecto.Changeset.change(follow_activity_two, data: data) + + {:ok, follow_activity_two} = Repo.update(cng) + + {:ok, follow_activity_two} = Utils.update_follow_state(follow_activity_two, "reject") + + assert Repo.get(Activity, follow_activity.id).data["state"] == "pending" + assert Repo.get(Activity, follow_activity_two.id).data["state"] == "reject" + end + end end From e1370ba1316734f61d8326200df1cb0789a4686f Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 5 Jun 2019 16:51:28 +0200 Subject: [PATCH 19/23] Utils: Use update_follow_state_for_all when appropriate. --- CHANGELOG.md | 1 + lib/pleroma/web/activity_pub/transmogrifier.ex | 11 ++++++----- lib/pleroma/web/common_api/common_api.ex | 4 ++-- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d1d87238c..376df2e57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -90,6 +90,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Respond with a 404 Not implemented JSON error message when requested API is not implemented ### Fixed +- Follow requests don't get 'stuck' anymore. - Added an FTS index on objects. Running `vacuum analyze` and setting a larger `work_mem` is recommended. - Followers counter not being updated when a follower is blocked - Deactivated users being able to request an access token diff --git a/lib/pleroma/web/activity_pub/transmogrifier.ex b/lib/pleroma/web/activity_pub/transmogrifier.ex index e8df112b9..ff031a16e 100644 --- a/lib/pleroma/web/activity_pub/transmogrifier.ex +++ b/lib/pleroma/web/activity_pub/transmogrifier.ex @@ -462,7 +462,8 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do {:user_blocked, User.blocks?(followed, follower) && deny_follow_blocked}, {_, false} <- {:user_locked, User.locked?(followed)}, {_, {:ok, follower}} <- {:follow, User.follow(follower, followed)}, - {_, {:ok, _}} <- {:follow_state_update, Utils.update_follow_state(activity, "accept")} do + {_, {:ok, _}} <- + {:follow_state_update, Utils.update_follow_state_for_all(activity, "accept")} do ActivityPub.accept(%{ to: [follower.ap_id], actor: followed, @@ -471,7 +472,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do }) else {:user_blocked, true} -> - {:ok, _} = Utils.update_follow_state(activity, "reject") + {:ok, _} = Utils.update_follow_state_for_all(activity, "reject") ActivityPub.reject(%{ to: [follower.ap_id], @@ -481,7 +482,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do }) {:follow, {:error, _}} -> - {:ok, _} = Utils.update_follow_state(activity, "reject") + {:ok, _} = Utils.update_follow_state_for_all(activity, "reject") ActivityPub.reject(%{ to: [follower.ap_id], @@ -507,7 +508,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do with actor <- Containment.get_actor(data), {:ok, %User{} = followed} <- User.get_or_fetch_by_ap_id(actor), {:ok, follow_activity} <- get_follow_activity(follow_object, followed), - {:ok, follow_activity} <- Utils.update_follow_state(follow_activity, "accept"), + {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "accept"), %User{local: true} = follower <- User.get_cached_by_ap_id(follow_activity.data["actor"]), {:ok, _follower} = User.follow(follower, followed) do ActivityPub.accept(%{ @@ -528,7 +529,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier do with actor <- Containment.get_actor(data), {:ok, %User{} = followed} <- User.get_or_fetch_by_ap_id(actor), {:ok, follow_activity} <- get_follow_activity(follow_object, followed), - {:ok, follow_activity} <- Utils.update_follow_state(follow_activity, "reject"), + {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "reject"), %User{local: true} = follower <- User.get_cached_by_ap_id(follow_activity.data["actor"]), {:ok, activity} <- ActivityPub.reject(%{ diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index 71e166128..f5193512e 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -37,7 +37,7 @@ defmodule Pleroma.Web.CommonAPI do def accept_follow_request(follower, followed) do with {:ok, follower} <- User.follow(follower, followed), %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed), - {:ok, follow_activity} <- Utils.update_follow_state(follow_activity, "accept"), + {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "accept"), {:ok, _activity} <- ActivityPub.accept(%{ to: [follower.ap_id], @@ -51,7 +51,7 @@ defmodule Pleroma.Web.CommonAPI do def reject_follow_request(follower, followed) do with %Activity{} = follow_activity <- Utils.fetch_latest_follow(follower, followed), - {:ok, follow_activity} <- Utils.update_follow_state(follow_activity, "reject"), + {:ok, follow_activity} <- Utils.update_follow_state_for_all(follow_activity, "reject"), {:ok, _activity} <- ActivityPub.reject(%{ to: [follower.ap_id], From f82382de22c860c4a67a69e579e2d1fd2b186a87 Mon Sep 17 00:00:00 2001 From: Egor Date: Thu, 6 Jun 2019 12:17:49 +0000 Subject: [PATCH 20/23] [#943] Make the unauthenticated users limitation optional --- CHANGELOG.md | 1 + config/config.exs | 3 ++- docs/config.md | 9 +++++---- lib/pleroma/activity/search.ex | 8 +++++++- lib/pleroma/user/search.ex | 25 +++++++++++++++++++++++-- test/activity_test.exs | 14 ++++++++++++++ test/user_test.exs | 22 ++++++++++++++++++++-- 7 files changed, 72 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 376df2e57..cf2232b09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Configuration: `notify_email` option - Configuration: Media proxy `whitelist` option - Configuration: `report_uri` option +- Configuration: `limit_unauthenticated_to_local_content` option - Pleroma API: User subscriptions - Pleroma API: Healthcheck endpoint - Pleroma API: `/api/v1/pleroma/mascot` per-user frontend mascot configuration endpoints diff --git a/config/config.exs b/config/config.exs index a3f33cfbb..4e2b1703b 100644 --- a/config/config.exs +++ b/config/config.exs @@ -244,7 +244,8 @@ config :pleroma, :instance, safe_dm_mentions: false, healthcheck: false, remote_post_retention_days: 90, - skip_thread_containment: false + skip_thread_containment: false, + limit_unauthenticated_to_local_content: true config :pleroma, :app_account_creation, enabled: true, max_requests: 25, interval: 1800 diff --git a/docs/config.md b/docs/config.md index 93ede6464..c61a5d8a3 100644 --- a/docs/config.md +++ b/docs/config.md @@ -108,10 +108,11 @@ config :pleroma, Pleroma.Emails.Mailer, * `welcome_message`: A message that will be send to a newly registered users as a direct message. * `welcome_user_nickname`: The nickname of the local user that sends the welcome message. * `max_report_comment_size`: The maximum size of the report comment (Default: `1000`) -* `safe_dm_mentions`: If set to true, only mentions at the beginning of a post will be used to address people in direct messages. This is to prevent accidental mentioning of people when talking about them (e.g. "@friend hey i really don't like @enemy"). (Default: `false`) -* `healthcheck`: if set to true, system data will be shown on ``/api/pleroma/healthcheck``. -* `remote_post_retention_days`: the default amount of days to retain remote posts when pruning the database -* `skip_thread_containment`: Skip filter out broken threads. the default is `false`. +* `safe_dm_mentions`: If set to true, only mentions at the beginning of a post will be used to address people in direct messages. This is to prevent accidental mentioning of people when talking about them (e.g. "@friend hey i really don't like @enemy"). Default: `false`. +* `healthcheck`: If set to true, system data will be shown on ``/api/pleroma/healthcheck``. +* `remote_post_retention_days`: The default amount of days to retain remote posts when pruning the database. +* `skip_thread_containment`: Skip filter out broken threads. The default is `false`. +* `limit_unauthenticated_to_local_content`: Limit unauthenticated users to search for local statutes and users only. The default is `true`. ## :app_account_creation REST API for creating an account settings diff --git a/lib/pleroma/activity/search.ex b/lib/pleroma/activity/search.ex index f2fdfffe1..9ccedcd13 100644 --- a/lib/pleroma/activity/search.ex +++ b/lib/pleroma/activity/search.ex @@ -60,7 +60,13 @@ defmodule Pleroma.Activity.Search do defp maybe_restrict_local(q, %User{}), do: q # unauthenticated users can only search local activities - defp maybe_restrict_local(q, _), do: where(q, local: true) + defp maybe_restrict_local(q, _) do + if Pleroma.Config.get([:instance, :limit_unauthenticated_to_local_content], true) do + where(q, local: true) + else + q + end + end defp maybe_fetch(activities, user, search_query) do with true <- Regex.match?(~r/https?:/, search_query), diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index d5b2eaa9f..e74143cd0 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -14,7 +14,7 @@ defmodule Pleroma.User.Search do # Strip the beginning @ off if there is a query query = String.trim_leading(query, "@") - if match?(%User{}, for_user) and resolve, do: User.get_or_fetch(query) + maybe_resolve(resolve, for_user, query) {:ok, results} = Repo.transaction(fn -> @@ -28,6 +28,16 @@ defmodule Pleroma.User.Search do results end + defp maybe_resolve(true, %User{}, query) do + User.get_or_fetch(query) + end + + defp maybe_resolve(true, _, query) do + unless restrict_local?(), do: User.get_or_fetch(query) + end + + defp maybe_resolve(_, _, _), do: :noop + defp search_query(query, for_user) do query |> union_query() @@ -39,6 +49,10 @@ defmodule Pleroma.User.Search do |> maybe_restrict_local(for_user) end + defp restrict_local? do + Pleroma.Config.get([:instance, :limit_unauthenticated_to_local_content], true) + end + defp union_query(query) do fts_subquery = fts_search_subquery(query) trigram_subquery = trigram_search_subquery(query) @@ -52,7 +66,14 @@ defmodule Pleroma.User.Search do # unauthenticated users can only search local activities defp maybe_restrict_local(q, %User{}), do: q - defp maybe_restrict_local(q, _), do: where(q, [u], u.local == true) + + defp maybe_restrict_local(q, _) do + if restrict_local?() do + where(q, [u], u.local == true) + else + q + end + end defp boost_search_rank_query(query, nil), do: query diff --git a/test/activity_test.exs b/test/activity_test.exs index 1814e313d..e56e39096 100644 --- a/test/activity_test.exs +++ b/test/activity_test.exs @@ -138,5 +138,19 @@ defmodule Pleroma.ActivityTest do test "find only local statuses for unauthenticated users", %{local_activity: local_activity} do assert [^local_activity] = Activity.search(nil, "find me") end + + test "find all statuses for unauthenticated users when `limit_unauthenticated_to_local_content` is `false`", + %{ + local_activity: local_activity, + remote_activity: remote_activity + } do + Pleroma.Config.put([:instance, :limit_unauthenticated_to_local_content], false) + + activities = Enum.sort_by(Activity.search(nil, "find me"), & &1.id) + + assert [^local_activity, ^remote_activity] = activities + + Pleroma.Config.put([:instance, :limit_unauthenticated_to_local_content], true) + end end end diff --git a/test/user_test.exs b/test/user_test.exs index 108883ba3..8dd672173 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -1077,7 +1077,7 @@ defmodule Pleroma.UserTest do Enum.map(User.search("doe", resolve: false, for_user: u1), & &1.id) == [] end - test "find local and remote statuses for authenticated users" do + test "find local and remote users for authenticated users" do u1 = insert(:user, %{name: "lain"}) u2 = insert(:user, %{name: "ebn", nickname: "lain@mastodon.social", local: false}) u3 = insert(:user, %{nickname: "lain@pleroma.soykaf.com", local: false}) @@ -1091,7 +1091,7 @@ defmodule Pleroma.UserTest do assert [u1.id, u2.id, u3.id] == results end - test "find only local statuses for unauthenticated users" do + test "find only local users for unauthenticated users" do %{id: id} = insert(:user, %{name: "lain"}) insert(:user, %{name: "ebn", nickname: "lain@mastodon.social", local: false}) insert(:user, %{nickname: "lain@pleroma.soykaf.com", local: false}) @@ -1099,6 +1099,24 @@ defmodule Pleroma.UserTest do assert [%{id: ^id}] = User.search("lain") end + test "find all users for unauthenticated users when `limit_unauthenticated_to_local_content` is `false`" do + Pleroma.Config.put([:instance, :limit_unauthenticated_to_local_content], false) + + u1 = insert(:user, %{name: "lain"}) + u2 = insert(:user, %{name: "ebn", nickname: "lain@mastodon.social", local: false}) + u3 = insert(:user, %{nickname: "lain@pleroma.soykaf.com", local: false}) + + results = + "lain" + |> User.search() + |> Enum.map(& &1.id) + |> Enum.sort() + + assert [u1.id, u2.id, u3.id] == results + + Pleroma.Config.put([:instance, :limit_unauthenticated_to_local_content], true) + end + test "finds a user whose name is nil" do _user = insert(:user, %{name: "notamatch", nickname: "testuser@pleroma.amplifie.red"}) user_two = insert(:user, %{name: nil, nickname: "lain@pleroma.soykaf.com"}) From 9ae8f012a516495387aed07a73c46f91df0ad59d Mon Sep 17 00:00:00 2001 From: rinpatch Date: Thu, 6 Jun 2019 16:36:56 +0300 Subject: [PATCH 21/23] Switch to manual Supervisor child specifications instead of Supervisor.Spec Supervisor.Spec is deprecated and causes warnings on Elixir master, see https://hexdocs.pm/elixir/Supervisor.Spec.html --- lib/pleroma/application.ex | 205 ++++++++++++++++++++++++++------------------- 1 file changed, 120 insertions(+), 85 deletions(-) diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index 76df3945e..69a8a5761 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -4,7 +4,6 @@ defmodule Pleroma.Application do use Application - import Supervisor.Spec @name Mix.Project.config()[:name] @version Mix.Project.config()[:version] @@ -31,96 +30,127 @@ defmodule Pleroma.Application do children = [ # Start the Ecto repository - supervisor(Pleroma.Repo, []), - worker(Pleroma.Emoji, []), - worker(Pleroma.Captcha, []), - worker( - Cachex, - [ - :used_captcha_cache, - [ - ttl_interval: :timer.seconds(Pleroma.Config.get!([Pleroma.Captcha, :seconds_valid])) - ] - ], - id: :cachex_used_captcha_cache - ), - worker( - Cachex, - [ - :user_cache, - [ - default_ttl: 25_000, - ttl_interval: 1000, - limit: 2500 - ] - ], - id: :cachex_user - ), - worker( - Cachex, - [ - :object_cache, - [ - default_ttl: 25_000, - ttl_interval: 1000, - limit: 2500 - ] - ], - id: :cachex_object - ), - worker( - Cachex, - [ - :rich_media_cache, - [ - default_ttl: :timer.minutes(120), - limit: 5000 - ] - ], - id: :cachex_rich_media - ), - worker( - Cachex, - [ - :scrubber_cache, - [ - limit: 2500 - ] - ], - id: :cachex_scrubber - ), - worker( - Cachex, - [ - :idempotency_cache, - [ - expiration: - expiration( - default: :timer.seconds(6 * 60 * 60), - interval: :timer.seconds(60) - ), - limit: 2500 - ] - ], - id: :cachex_idem - ), - worker(Pleroma.FlakeId, []), - worker(Pleroma.ScheduledActivityWorker, []) + %{id: Pleroma.Repo, start: {Pleroma.Repo, :start_link, []}, type: :supervisor}, + %{id: Pleroma.Emoji, start: {Pleroma.Emoji, :start_link, []}}, + %{id: Pleroma.Captcha, start: {Pleroma.Captcha, :start_link, []}}, + %{ + id: :cachex_used_captcha_cache, + start: + {Cachex, :start_link, + [ + :used_captcha_cache, + [ + ttl_interval: + :timer.seconds(Pleroma.Config.get!([Pleroma.Captcha, :seconds_valid])) + ] + ]} + }, + %{ + id: :cachex_user, + start: + {Cachex, :start_link, + [ + :user_cache, + [ + default_ttl: 25_000, + ttl_interval: 1000, + limit: 2500 + ] + ]} + }, + %{ + id: :cachex_object, + start: + {Cachex, :start_link, + [ + :object_cache, + [ + default_ttl: 25_000, + ttl_interval: 1000, + limit: 2500 + ] + ]} + }, + %{ + id: :cachex_rich_media, + start: + {Cachex, :start_link, + [ + :rich_media_cache, + [ + default_ttl: :timer.minutes(120), + limit: 5000 + ] + ]} + }, + %{ + id: :cachex_scrubber, + start: + {Cachex, :start_link, + [ + :scrubber_cache, + [ + limit: 2500 + ] + ]} + }, + %{ + id: :cachex_idem, + start: + {Cachex, :start_link, + [ + :idempotency_cache, + [ + expiration: + expiration( + default: :timer.seconds(6 * 60 * 60), + interval: :timer.seconds(60) + ), + limit: 2500 + ] + ]} + }, + %{id: Pleroma.FlakeId, start: {Pleroma.FlakeId, :start_link, []}}, + %{ + id: Pleroma.ScheduledActivityWorker, + start: {Pleroma.ScheduledActivityWorker, :start_link, []} + } ] ++ hackney_pool_children() ++ [ - worker(Pleroma.Web.Federator.RetryQueue, []), - worker(Pleroma.Web.OAuth.Token.CleanWorker, []), - worker(Pleroma.Stats, []), - worker(Task, [&Pleroma.Web.Push.init/0], restart: :temporary, id: :web_push_init), - worker(Task, [&Pleroma.Web.Federator.init/0], restart: :temporary, id: :federator_init) + %{ + id: Pleroma.Web.Federator.RetryQueue, + start: {Pleroma.Web.Federator.RetryQueue, :start_link, []} + }, + %{ + id: Pleroma.Web.OAuth.Token.CleanWorker, + start: {Pleroma.Web.OAuth.Token.CleanWorker, :start_link, []} + }, + %{ + id: Pleroma.Stats, + start: {Pleroma.Stats, :start_link, []} + }, + %{ + id: :web_push_init, + start: {Task, :start_link, [&Pleroma.Web.Push.init/0]}, + restart: :temporary + }, + %{ + id: :federator_init, + start: {Task, :start_link, [&Pleroma.Web.Federator.init/0]}, + restart: :temporary + } ] ++ streamer_child() ++ chat_child() ++ [ # Start the endpoint when the application starts - supervisor(Pleroma.Web.Endpoint, []), - worker(Pleroma.Gopher.Server, []) + %{ + id: Pleroma.Web.Endpoint, + start: {Pleroma.Web.Endpoint, :start_link, []}, + type: :supervisor + }, + %{id: Pleroma.Gopher.Server, start: {Pleroma.Gopher.Server, :start_link, []}} ] # See http://elixir-lang.org/docs/stable/elixir/Supervisor.html @@ -169,12 +199,17 @@ defmodule Pleroma.Application do defp chat_child, do: [] else defp streamer_child do - [worker(Pleroma.Web.Streamer, [])] + [%{id: Pleroma.Web.Streamer, start: {Pleroma.Web.Streamer, :start_link, []}}] end defp chat_child do if Pleroma.Config.get([:chat, :enabled]) do - [worker(Pleroma.Web.ChatChannel.ChatChannelState, [])] + [ + %{ + id: Pleroma.Web.ChatChannel.ChatChannelState, + start: {Pleroma.Web.ChatChannel.ChatChannelState, :start_link, []} + } + ] else [] end From 7e9f7ea0c21d51c5fedf89ea1feb503b77c78bba Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 6 Jun 2019 16:16:22 +0200 Subject: [PATCH 22/23] MastodonAPI: Add test for user search. --- test/web/mastodon_api/mastodon_api_controller_test.exs | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index 51c1cdfac..33c8e209a 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -2185,6 +2185,15 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIControllerTest do assert account["acct"] == "shp@social.heldscal.la" end + test "search doesn't fetch remote accounts if resolve is false", %{conn: conn} do + conn = + conn + |> get("/api/v1/search", %{"q" => "shp@social.heldscal.la", "resolve" => "false"}) + + assert results = json_response(conn, 200) + assert [] == results["accounts"] + end + test "returns the favorites of a user", %{conn: conn} do user = insert(:user) other_user = insert(:user) From cb2bcee842f07358db4d37de1b9a32c9d69d487c Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 6 Jun 2019 16:18:27 +0200 Subject: [PATCH 23/23] User Search: Boost friends more strongly. --- lib/pleroma/user/search.ex | 4 ++-- test/tasks/user_test.exs | 21 +++++++++++++++++++++ 2 files changed, 23 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/user/search.ex b/lib/pleroma/user/search.ex index e74143cd0..add6a0bbf 100644 --- a/lib/pleroma/user/search.ex +++ b/lib/pleroma/user/search.ex @@ -86,8 +86,8 @@ defmodule Pleroma.User.Search do search_rank: fragment( """ - CASE WHEN (?) THEN (?) * 1.3 - WHEN (?) THEN (?) * 1.2 + CASE WHEN (?) THEN 0.5 + (?) * 1.3 + WHEN (?) THEN 0.5 + (?) * 1.2 WHEN (?) THEN (?) * 1.1 ELSE (?) END """, diff --git a/test/tasks/user_test.exs b/test/tasks/user_test.exs index 260ce0d95..6fd7c7113 100644 --- a/test/tasks/user_test.exs +++ b/test/tasks/user_test.exs @@ -366,4 +366,25 @@ defmodule Mix.Tasks.Pleroma.UserTest do refute user.info.confirmation_token end end + + describe "search" do + test "it returns users matching" do + user = insert(:user) + moon = insert(:user, nickname: "moon", name: "fediverse expert moon") + moot = insert(:user, nickname: "moot") + kawen = insert(:user, nickname: "kawen", name: "fediverse expert moon") + + {:ok, user} = User.follow(user, kawen) + + assert [moon.id, kawen.id] == User.Search.search("moon") |> Enum.map(& &1.id) + res = User.search("moo") |> Enum.map(& &1.id) + assert moon.id in res + assert moot.id in res + assert kawen.id in res + assert [moon.id, kawen.id] == User.Search.search("moon fediverse") |> Enum.map(& &1.id) + + assert [kawen.id, moon.id] == + User.Search.search("moon fediverse", for_user: user) |> Enum.map(& &1.id) + end + end end