From 9b6c7843d669c80bd062214e598d0f5d9738de8e Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Tue, 3 Mar 2020 04:58:12 +0100 Subject: [PATCH 01/25] debian_based_*.md: Use erlang-nox metapackage --- docs/installation/debian_based_en.md | 10 +++------- docs/installation/debian_based_jp.md | 22 +++++++++------------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/docs/installation/debian_based_en.md b/docs/installation/debian_based_en.md index fe2dbb92d..a900ec61d 100644 --- a/docs/installation/debian_based_en.md +++ b/docs/installation/debian_based_en.md @@ -7,13 +7,9 @@ This guide will assume you are on Debian Stretch. This guide should also work wi * `postgresql` (9.6+, Ubuntu 16.04 comes with 9.5, you can get a newer version from [here](https://www.postgresql.org/download/linux/ubuntu/)) * `postgresql-contrib` (9.6+, same situtation as above) -* `elixir` (1.5+, [install from here, Debian and Ubuntu ship older versions](https://elixir-lang.org/install.html#unix-and-unix-like) or use [asdf](https://github.com/asdf-vm/asdf) as the pleroma user) +* `elixir` (1.8+, [install from here, Debian and Ubuntu ship older versions](https://elixir-lang.org/install.html#unix-and-unix-like) or use [asdf](https://github.com/asdf-vm/asdf) as the pleroma user) * `erlang-dev` -* `erlang-tools` -* `erlang-parsetools` -* `erlang-eldap`, if you want to enable ldap authenticator -* `erlang-ssh` -* `erlang-xmerl` +* `erlang-nox` * `git` * `build-essential` @@ -50,7 +46,7 @@ sudo dpkg -i /tmp/erlang-solutions_1.0_all.deb ```shell sudo apt update -sudo apt install elixir erlang-dev erlang-parsetools erlang-xmerl erlang-tools erlang-ssh +sudo apt install elixir erlang-dev erlang-nox ``` ### Install PleromaBE diff --git a/docs/installation/debian_based_jp.md b/docs/installation/debian_based_jp.md index 7aa0bcc24..a3c4621d8 100644 --- a/docs/installation/debian_based_jp.md +++ b/docs/installation/debian_based_jp.md @@ -10,21 +10,17 @@ ### 必要なソフトウェア - PostgreSQL 9.6以上 (Ubuntu16.04では9.5しか提供されていないので，[](https://www.postgresql.org/download/linux/ubuntu/)こちらから新しいバージョンを入手してください) -- postgresql-contrib 9.6以上 (同上) -- Elixir 1.5 以上 ([Debianのリポジトリからインストールしないこと！！！ ここからインストールすること！](https://elixir-lang.org/install.html#unix-and-unix-like)。または [asdf](https://github.com/asdf-vm/asdf) をpleromaユーザーでインストールしてください) - - erlang-dev -- erlang-tools -- erlang-parsetools -- erlang-eldap (LDAP認証を有効化するときのみ必要) -- erlang-ssh -- erlang-xmerl -- git -- build-essential +- `postgresql-contrib` 9.6以上 (同上) +- Elixir 1.8 以上 ([Debianのリポジトリからインストールしないこと！！！ ここからインストールすること！](https://elixir-lang.org/install.html#unix-and-unix-like)。または [asdf](https://github.com/asdf-vm/asdf) をpleromaユーザーでインストールしてください) +- `erlang-dev` +- `erlang-nox` +- `git` +- `build-essential` #### このガイドで利用している追加パッケージ -- nginx (おすすめです。他のリバースプロキシを使う場合は、参考となる設定をこのリポジトリから探してください) -- certbot (または何らかのLet's Encrypt向けACMEクライアント) +- `nginx` (おすすめです。他のリバースプロキシを使う場合は、参考となる設定をこのリポジトリから探してください) +- `certbot` (または何らかのLet's Encrypt向けACMEクライアント) ### システムを準備する @@ -51,7 +47,7 @@ sudo dpkg -i /tmp/erlang-solutions_1.0_all.deb * ElixirとErlangをインストールします、 ``` sudo apt update -sudo apt install elixir erlang-dev erlang-parsetools erlang-xmerl erlang-tools erlang-ssh +sudo apt install elixir erlang-dev erlang-nox ``` ### Pleroma BE (バックエンド) をインストールします From 9a3c74b244bce6097a8c6da99692bfc9973e1ec8 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Sun, 12 Apr 2020 20:26:35 -0500 Subject: [PATCH 02/25] Always accept deletions through SimplePolicy --- .../web/activity_pub/mrf/simple_policy.ex | 3 +++ .../activity_pub/mrf/simple_policy_test.exs | 23 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex index 4edc007fd..b23f263f5 100644 --- a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex @@ -148,6 +148,9 @@ defp check_banner_removal(%{host: actor_host} = _actor_info, %{"image" => _image defp check_banner_removal(_actor_info, object), do: {:ok, object} + @impl true + def filter(%{"type" => "Delete"} = object), do: {:ok, object} + @impl true def filter(%{"actor" => actor} = object) do actor_info = URI.parse(actor) diff --git a/test/web/activity_pub/mrf/simple_policy_test.exs b/test/web/activity_pub/mrf/simple_policy_test.exs index 91c24c2d9..eaa595706 100644 --- a/test/web/activity_pub/mrf/simple_policy_test.exs +++ b/test/web/activity_pub/mrf/simple_policy_test.exs @@ -258,6 +258,14 @@ test "actor has a matching host" do assert SimplePolicy.filter(remote_user) == {:reject, nil} end + + test "always accept deletions" do + Config.put([:mrf_simple, :reject], ["remote.instance"]) + + deletion_message = build_remote_deletion_message() + + assert SimplePolicy.filter(deletion_message) == {:ok, deletion_message} + end end describe "when :accept" do @@ -308,6 +316,14 @@ test "actor has a matching host" do assert SimplePolicy.filter(remote_user) == {:ok, remote_user} end + + test "always accept deletions" do + Config.put([:mrf_simple, :accept], ["non.matching.remote"]) + + deletion_message = build_remote_deletion_message() + + assert SimplePolicy.filter(deletion_message) == {:ok, deletion_message} + end end describe "when :avatar_removal" do @@ -408,4 +424,11 @@ defp build_remote_user do "type" => "Person" } end + + defp build_remote_deletion_message do + %{ + "type" => "Delete", + "actor" => "https://remote.instance/users/bob" + } + end end From c28aaf9d82a781508eba886bd455767a110d1b7c Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 13 Apr 2020 21:21:04 +0400 Subject: [PATCH 03/25] Add OpenAPI spec for CustomEmojiController --- .../operations/custom_emoji_operation.ex | 25 +++++++++++ .../web/api_spec/schemas/custom_emoji.ex | 30 +++++++++++++ .../schemas/custom_emojis_response.ex | 42 +++++++++++++++++++ .../controllers/custom_emoji_controller.ex | 4 ++ .../custom_emoji_controller_test.exs | 27 ++++++++++-- 5 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex create mode 100644 lib/pleroma/web/api_spec/schemas/custom_emoji.ex create mode 100644 lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex diff --git a/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex b/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex new file mode 100644 index 000000000..cf2215823 --- /dev/null +++ b/lib/pleroma/web/api_spec/operations/custom_emoji_operation.ex @@ -0,0 +1,25 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.CustomEmojiOperation do + alias OpenApiSpex.Operation + alias Pleroma.Web.ApiSpec.Schemas.CustomEmojisResponse + + def open_api_operation(action) do + operation = String.to_existing_atom("#{action}_operation") + apply(__MODULE__, operation, []) + end + + def index_operation do + %Operation{ + tags: ["custom_emojis"], + summary: "List custom custom emojis", + description: "Returns custom emojis that are available on the server.", + operationId: "CustomEmojiController.index", + responses: %{ + 200 => Operation.response("Custom Emojis", "application/json", CustomEmojisResponse) + } + } + end +end diff --git a/lib/pleroma/web/api_spec/schemas/custom_emoji.ex b/lib/pleroma/web/api_spec/schemas/custom_emoji.ex new file mode 100644 index 000000000..5531b2081 --- /dev/null +++ b/lib/pleroma/web/api_spec/schemas/custom_emoji.ex @@ -0,0 +1,30 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.Schemas.CustomEmoji do + alias OpenApiSpex.Schema + + require OpenApiSpex + + OpenApiSpex.schema(%{ + title: "CustomEmoji", + description: "Response schema for an CustomEmoji", + type: :object, + properties: %{ + shortcode: %Schema{type: :string}, + url: %Schema{type: :string}, + static_url: %Schema{type: :string}, + visible_in_picker: %Schema{type: :boolean}, + category: %Schema{type: :string}, + tags: %Schema{type: :array} + }, + example: %{ + "shortcode" => "aaaa", + "url" => "https://files.mastodon.social/custom_emojis/images/000/007/118/original/aaaa.png", + "static_url" => + "https://files.mastodon.social/custom_emojis/images/000/007/118/static/aaaa.png", + "visible_in_picker" => true + } + }) +end diff --git a/lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex b/lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex new file mode 100644 index 000000000..01582a63d --- /dev/null +++ b/lib/pleroma/web/api_spec/schemas/custom_emojis_response.ex @@ -0,0 +1,42 @@ +# Pleroma: A lightweight social networking server +# Copyright © 2017-2020 Pleroma Authors +# SPDX-License-Identifier: AGPL-3.0-only + +defmodule Pleroma.Web.ApiSpec.Schemas.CustomEmojisResponse do + alias Pleroma.Web.ApiSpec.Schemas.CustomEmoji + + require OpenApiSpex + + OpenApiSpex.schema(%{ + title: "CustomEmojisResponse", + description: "Response schema for custom emojis", + type: :array, + items: CustomEmoji, + example: [ + %{ + "category" => "Fun", + "shortcode" => "blank", + "static_url" => "https://lain.com/emoji/blank.png", + "tags" => ["Fun"], + "url" => "https://lain.com/emoji/blank.png", + "visible_in_picker" => true + }, + %{ + "category" => "Gif,Fun", + "shortcode" => "firefox", + "static_url" => "https://lain.com/emoji/Firefox.gif", + "tags" => ["Gif", "Fun"], + "url" => "https://lain.com/emoji/Firefox.gif", + "visible_in_picker" => true + }, + %{ + "category" => "pack:mixed", + "shortcode" => "sadcat", + "static_url" => "https://lain.com/emoji/mixed/sadcat.png", + "tags" => ["pack:mixed"], + "url" => "https://lain.com/emoji/mixed/sadcat.png", + "visible_in_picker" => true + } + ] + }) +end diff --git a/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex b/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex index d82de1db5..3bfebef8b 100644 --- a/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/custom_emoji_controller.ex @@ -5,6 +5,10 @@ defmodule Pleroma.Web.MastodonAPI.CustomEmojiController do use Pleroma.Web, :controller + plug(OpenApiSpex.Plug.CastAndValidate) + + defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.CustomEmojiOperation + def index(conn, _params) do render(conn, "index.json", custom_emojis: Pleroma.Emoji.get_all()) end diff --git a/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs b/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs index 6567a0667..0b2ffa470 100644 --- a/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs +++ b/test/web/mastodon_api/controllers/custom_emoji_controller_test.exs @@ -4,13 +4,18 @@ defmodule Pleroma.Web.MastodonAPI.CustomEmojiControllerTest do use Pleroma.Web.ConnCase, async: true + alias Pleroma.Web.ApiSpec + alias Pleroma.Web.ApiSpec.Schemas.CustomEmoji + alias Pleroma.Web.ApiSpec.Schemas.CustomEmojisResponse + import OpenApiSpex.TestAssertions test "with tags", %{conn: conn} do - [emoji | _body] = - conn - |> get("/api/v1/custom_emojis") - |> json_response(200) + assert resp = + conn + |> get("/api/v1/custom_emojis") + |> json_response(200) + assert [emoji | _body] = resp assert Map.has_key?(emoji, "shortcode") assert Map.has_key?(emoji, "static_url") assert Map.has_key?(emoji, "tags") @@ -18,5 +23,19 @@ test "with tags", %{conn: conn} do assert Map.has_key?(emoji, "category") assert Map.has_key?(emoji, "url") assert Map.has_key?(emoji, "visible_in_picker") + assert_schema(resp, "CustomEmojisResponse", ApiSpec.spec()) + assert_schema(emoji, "CustomEmoji", ApiSpec.spec()) + end + + test "CustomEmoji example matches schema" do + api_spec = ApiSpec.spec() + schema = CustomEmoji.schema() + assert_schema(schema.example, "CustomEmoji", api_spec) + end + + test "CustomEmojisResponse example matches schema" do + api_spec = ApiSpec.spec() + schema = CustomEmojisResponse.schema() + assert_schema(schema.example, "CustomEmojisResponse", api_spec) end end From 6cda360fea8a42168b5835ef903cf3bf89c8151a Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Thu, 16 Apr 2020 10:36:37 +0300 Subject: [PATCH 04/25] don't restart postgrex --- lib/pleroma/config/loader.ex | 2 +- lib/pleroma/config/transfer_task.ex | 20 +++++++++++--------- test/config/transfer_task_test.exs | 9 +++++++++ test/fixtures/config/temp.secret.exs | 2 ++ test/tasks/config_test.exs | 3 ++- 5 files changed, 25 insertions(+), 11 deletions(-) diff --git a/lib/pleroma/config/loader.ex b/lib/pleroma/config/loader.ex index 6ca6550bd..0f3ecf1ed 100644 --- a/lib/pleroma/config/loader.ex +++ b/lib/pleroma/config/loader.ex @@ -47,7 +47,7 @@ defp filter(configs) do @spec filter_group(atom(), keyword()) :: keyword() def filter_group(group, configs) do Enum.reject(configs[group], fn {key, _v} -> - key in @reject_keys or (group == :phoenix and key == :serve_endpoints) + key in @reject_keys or (group == :phoenix and key == :serve_endpoints) or group == :postgrex end) end end diff --git a/lib/pleroma/config/transfer_task.ex b/lib/pleroma/config/transfer_task.ex index f4722f99d..c02b70e96 100644 --- a/lib/pleroma/config/transfer_task.ex +++ b/lib/pleroma/config/transfer_task.ex @@ -46,14 +46,6 @@ def load_and_update_env(deleted_settings \\ [], restart_pleroma? \\ true) do with {_, true} <- {:configurable, Config.get(:configurable_from_database)} do # We need to restart applications for loaded settings take effect - # TODO: some problem with prometheus after restart! - reject_restart = - if restart_pleroma? do - [nil, :prometheus] - else - [:pleroma, nil, :prometheus] - end - {logger, other} = (Repo.all(ConfigDB) ++ deleted_settings) |> Enum.map(&transform_and_merge/1) @@ -65,10 +57,20 @@ def load_and_update_env(deleted_settings \\ [], restart_pleroma? \\ true) do started_applications = Application.started_applications() + # TODO: some problem with prometheus after restart! + reject = [nil, :prometheus, :postgrex] + + reject = + if restart_pleroma? do + reject + else + [:pleroma | reject] + end + other |> Enum.map(&update/1) |> Enum.uniq() - |> Enum.reject(&(&1 in reject_restart)) + |> Enum.reject(&(&1 in reject)) |> maybe_set_pleroma_last() |> Enum.each(&restart(started_applications, &1, Config.get(:env))) diff --git a/test/config/transfer_task_test.exs b/test/config/transfer_task_test.exs index 00db0b686..473899d1d 100644 --- a/test/config/transfer_task_test.exs +++ b/test/config/transfer_task_test.exs @@ -16,6 +16,7 @@ test "transfer config values from db to env" do refute Application.get_env(:pleroma, :test_key) refute Application.get_env(:idna, :test_key) refute Application.get_env(:quack, :test_key) + refute Application.get_env(:postgrex, :test_key) initial = Application.get_env(:logger, :level) ConfigDB.create(%{ @@ -36,6 +37,12 @@ test "transfer config values from db to env" do value: [:test_value1, :test_value2] }) + ConfigDB.create(%{ + group: ":postgrex", + key: ":test_key", + value: :value + }) + ConfigDB.create(%{group: ":logger", key: ":level", value: :debug}) TransferTask.start_link([]) @@ -44,11 +51,13 @@ test "transfer config values from db to env" do assert Application.get_env(:idna, :test_key) == [live: 15, com: 35] assert Application.get_env(:quack, :test_key) == [:test_value1, :test_value2] assert Application.get_env(:logger, :level) == :debug + assert Application.get_env(:postgrex, :test_key) == :value on_exit(fn -> Application.delete_env(:pleroma, :test_key) Application.delete_env(:idna, :test_key) Application.delete_env(:quack, :test_key) + Application.delete_env(:postgrex, :test_key) Application.put_env(:logger, :level, initial) end) end diff --git a/test/fixtures/config/temp.secret.exs b/test/fixtures/config/temp.secret.exs index f4686c101..dc950ca30 100644 --- a/test/fixtures/config/temp.secret.exs +++ b/test/fixtures/config/temp.secret.exs @@ -7,3 +7,5 @@ config :quack, level: :info config :pleroma, Pleroma.Repo, pool: Ecto.Adapters.SQL.Sandbox + +config :postgrex, :json_library, Poison diff --git a/test/tasks/config_test.exs b/test/tasks/config_test.exs index 3dee4f082..04bc947a9 100644 --- a/test/tasks/config_test.exs +++ b/test/tasks/config_test.exs @@ -38,7 +38,7 @@ test "error if file with custom settings doesn't exist" do on_exit(fn -> Application.put_env(:quack, :level, initial) end) end - test "settings are migrated to db" do + test "filtered settings are migrated to db" do assert Repo.all(ConfigDB) == [] Mix.Tasks.Pleroma.Config.migrate_to_db("test/fixtures/config/temp.secret.exs") @@ -47,6 +47,7 @@ test "settings are migrated to db" do config2 = ConfigDB.get_by_params(%{group: ":pleroma", key: ":second_setting"}) config3 = ConfigDB.get_by_params(%{group: ":quack", key: ":level"}) refute ConfigDB.get_by_params(%{group: ":pleroma", key: "Pleroma.Repo"}) + refute ConfigDB.get_by_params(%{group: ":postgrex", key: ":json_library"}) assert ConfigDB.from_binary(config1.value) == [key: "value", key2: [Repo]] assert ConfigDB.from_binary(config2.value) == [key: "value2", key2: ["Activity"]] From 258d8975797298b9eadddb48a8a2fcf3a9dbf211 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Mon, 20 Apr 2020 16:38:00 +0400 Subject: [PATCH 05/25] Cleanup and DRY the Router --- lib/pleroma/web/router.ex | 113 ++++++++++++++------------------------ 1 file changed, 40 insertions(+), 73 deletions(-) diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 7e5960949..153802a43 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -16,79 +16,60 @@ defmodule Pleroma.Web.Router do plug(Pleroma.Plugs.UserEnabledPlug) end - pipeline :api do - plug(:accepts, ["json"]) - plug(:fetch_session) + pipeline :authenticate do plug(Pleroma.Plugs.OAuthPlug) plug(Pleroma.Plugs.BasicAuthDecoderPlug) plug(Pleroma.Plugs.UserFetcherPlug) plug(Pleroma.Plugs.SessionAuthenticationPlug) plug(Pleroma.Plugs.LegacyAuthenticationPlug) plug(Pleroma.Plugs.AuthenticationPlug) + end + + pipeline :after_auth do plug(Pleroma.Plugs.UserEnabledPlug) plug(Pleroma.Plugs.SetUserSessionIdPlug) plug(Pleroma.Plugs.EnsureUserKeyPlug) - plug(Pleroma.Plugs.IdempotencyPlug) + end + + pipeline :base_api do + plug(:accepts, ["json"]) + plug(:fetch_session) + plug(:authenticate) plug(OpenApiSpex.Plug.PutApiSpec, module: Pleroma.Web.ApiSpec) end + pipeline :api do + plug(:base_api) + plug(:after_auth) + plug(Pleroma.Plugs.IdempotencyPlug) + end + pipeline :authenticated_api do - plug(:accepts, ["json"]) - plug(:fetch_session) + plug(:base_api) plug(Pleroma.Plugs.AuthExpectedPlug) - plug(Pleroma.Plugs.OAuthPlug) - plug(Pleroma.Plugs.BasicAuthDecoderPlug) - plug(Pleroma.Plugs.UserFetcherPlug) - plug(Pleroma.Plugs.SessionAuthenticationPlug) - plug(Pleroma.Plugs.LegacyAuthenticationPlug) - plug(Pleroma.Plugs.AuthenticationPlug) - plug(Pleroma.Plugs.UserEnabledPlug) - plug(Pleroma.Plugs.SetUserSessionIdPlug) + plug(:after_auth) plug(Pleroma.Plugs.EnsureAuthenticatedPlug) plug(Pleroma.Plugs.IdempotencyPlug) - plug(OpenApiSpex.Plug.PutApiSpec, module: Pleroma.Web.ApiSpec) end pipeline :admin_api do - plug(:accepts, ["json"]) - plug(:fetch_session) - plug(Pleroma.Plugs.OAuthPlug) - plug(Pleroma.Plugs.BasicAuthDecoderPlug) - plug(Pleroma.Plugs.UserFetcherPlug) - plug(Pleroma.Plugs.SessionAuthenticationPlug) - plug(Pleroma.Plugs.LegacyAuthenticationPlug) - plug(Pleroma.Plugs.AuthenticationPlug) + plug(:base_api) plug(Pleroma.Plugs.AdminSecretAuthenticationPlug) - plug(Pleroma.Plugs.UserEnabledPlug) - plug(Pleroma.Plugs.SetUserSessionIdPlug) + plug(:after_auth) plug(Pleroma.Plugs.EnsureAuthenticatedPlug) plug(Pleroma.Plugs.UserIsAdminPlug) plug(Pleroma.Plugs.IdempotencyPlug) - plug(OpenApiSpex.Plug.PutApiSpec, module: Pleroma.Web.ApiSpec) end pipeline :mastodon_html do - plug(:accepts, ["html"]) - plug(:fetch_session) - plug(Pleroma.Plugs.OAuthPlug) - plug(Pleroma.Plugs.BasicAuthDecoderPlug) - plug(Pleroma.Plugs.UserFetcherPlug) - plug(Pleroma.Plugs.SessionAuthenticationPlug) - plug(Pleroma.Plugs.LegacyAuthenticationPlug) - plug(Pleroma.Plugs.AuthenticationPlug) - plug(Pleroma.Plugs.UserEnabledPlug) - plug(Pleroma.Plugs.SetUserSessionIdPlug) - plug(Pleroma.Plugs.EnsureUserKeyPlug) + plug(:browser) + plug(:authenticate) + plug(:after_auth) end pipeline :pleroma_html do - plug(:accepts, ["html"]) - plug(:fetch_session) - plug(Pleroma.Plugs.OAuthPlug) - plug(Pleroma.Plugs.BasicAuthDecoderPlug) - plug(Pleroma.Plugs.UserFetcherPlug) - plug(Pleroma.Plugs.SessionAuthenticationPlug) - plug(Pleroma.Plugs.AuthenticationPlug) + plug(:browser) + plug(:authenticate) plug(Pleroma.Plugs.EnsureUserKeyPlug) end @@ -515,7 +496,7 @@ defmodule Pleroma.Web.Router do end scope "/api" do - pipe_through(:api) + pipe_through(:base_api) get("/openapi", OpenApiSpex.Plug.RenderSpec, []) end @@ -529,10 +510,6 @@ defmodule Pleroma.Web.Router do post("/qvitter/statuses/notifications/read", TwitterAPI.Controller, :notifications_read) end - pipeline :ap_service_actor do - plug(:accepts, ["activity+json", "json"]) - end - pipeline :ostatus do plug(:accepts, ["html", "xml", "rss", "atom", "activity+json", "json"]) plug(Pleroma.Plugs.StaticFEPlug) @@ -543,8 +520,7 @@ defmodule Pleroma.Web.Router do end scope "/", Pleroma.Web do - pipe_through(:ostatus) - pipe_through(:http_signature) + pipe_through([:ostatus, :http_signature]) get("/objects/:uuid", OStatus.OStatusController, :object) get("/activities/:uuid", OStatus.OStatusController, :activity) @@ -562,13 +538,6 @@ defmodule Pleroma.Web.Router do get("/mailer/unsubscribe/:token", Mailer.SubscriptionController, :unsubscribe) end - # Server to Server (S2S) AP interactions - pipeline :activitypub do - plug(:accepts, ["activity+json", "json"]) - plug(Pleroma.Web.Plugs.HTTPSignaturePlug) - plug(Pleroma.Web.Plugs.MappedSignatureToIdentityPlug) - end - scope "/", Pleroma.Web.ActivityPub do # XXX: not really ostatus pipe_through(:ostatus) @@ -576,19 +545,22 @@ defmodule Pleroma.Web.Router do get("/users/:nickname/outbox", ActivityPubController, :outbox) end + pipeline :ap_service_actor do + plug(:accepts, ["activity+json", "json"]) + end + + # Server to Server (S2S) AP interactions + pipeline :activitypub do + plug(:ap_service_actor) + plug(:http_signature) + end + # Client to Server (C2S) AP interactions pipeline :activitypub_client do - plug(:accepts, ["activity+json", "json"]) + plug(:ap_service_actor) plug(:fetch_session) - plug(Pleroma.Plugs.OAuthPlug) - plug(Pleroma.Plugs.BasicAuthDecoderPlug) - plug(Pleroma.Plugs.UserFetcherPlug) - plug(Pleroma.Plugs.SessionAuthenticationPlug) - plug(Pleroma.Plugs.LegacyAuthenticationPlug) - plug(Pleroma.Plugs.AuthenticationPlug) - plug(Pleroma.Plugs.UserEnabledPlug) - plug(Pleroma.Plugs.SetUserSessionIdPlug) - plug(Pleroma.Plugs.EnsureUserKeyPlug) + plug(:authenticate) + plug(:after_auth) end scope "/", Pleroma.Web.ActivityPub do @@ -660,12 +632,7 @@ defmodule Pleroma.Web.Router do get("/web/*path", MastoFEController, :index) end - pipeline :remote_media do - end - scope "/proxy/", Pleroma.Web.MediaProxy do - pipe_through(:remote_media) - get("/:sig/:url", MediaProxyController, :remote) get("/:sig/:url/:filename", MediaProxyController, :remote) end From b54c8813d632cb44c7deb207e91bd32f01f33794 Mon Sep 17 00:00:00 2001 From: Alex Gleason Date: Mon, 13 Apr 2020 13:48:32 -0500 Subject: [PATCH 06/25] Add :reject_deletes option to SimplePolicy --- CHANGELOG.md | 2 + config/config.exs | 3 +- config/description.exs | 10 ++- docs/configuration/mrf.md | 3 +- .../web/activity_pub/mrf/simple_policy.ex | 14 +++- .../activity_pub/mrf/simple_policy_test.exs | 79 +++++++++++++++---- 6 files changed, 89 insertions(+), 22 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 36897503a..cd2536f9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - NodeInfo: `pleroma_emoji_reactions` to the `features` list. - Configuration: `:restrict_unauthenticated` setting, restrict access for unauthenticated users to timelines (public and federate), user profiles and statuses. - New HTTP adapter [gun](https://github.com/ninenines/gun). Gun adapter requires minimum OTP version of 22.2 otherwise Pleroma won’t start. For hackney OTP update is not required. +- Added `:reject_deletes` group to SimplePolicy
API Changes - Mastodon API: Support for `include_types` in `/api/v1/notifications`. @@ -20,6 +21,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Support pagination in conversations API +- **Breaking**: SimplePolicy `:reject` and `:accept` allow deletions again ## [unreleased-patch] diff --git a/config/config.exs b/config/config.exs index 232a91bf1..9a6b93a37 100644 --- a/config/config.exs +++ b/config/config.exs @@ -334,7 +334,8 @@ reject: [], accept: [], avatar_removal: [], - banner_removal: [] + banner_removal: [], + reject_deletes: [] config :pleroma, :mrf_keyword, reject: [], diff --git a/config/description.exs b/config/description.exs index 642f1a3ce..9d8e3b93c 100644 --- a/config/description.exs +++ b/config/description.exs @@ -1317,13 +1317,13 @@ %{ key: :reject, type: {:list, :string}, - description: "List of instances to reject any activities from", + description: "List of instances to reject activities from (except deletes)", suggestions: ["example.com", "*.example.com"] }, %{ key: :accept, type: {:list, :string}, - description: "List of instances to accept any activities from", + description: "List of instances to only accept activities from (except deletes)", suggestions: ["example.com", "*.example.com"] }, %{ @@ -1343,6 +1343,12 @@ type: {:list, :string}, description: "List of instances to strip banners from", suggestions: ["example.com", "*.example.com"] + }, + %{ + key: :reject_deletes, + type: {:list, :string}, + description: "List of instances to reject deletions from", + suggestions: ["example.com", "*.example.com"] } ] }, diff --git a/docs/configuration/mrf.md b/docs/configuration/mrf.md index c3957c255..2eb9631bd 100644 --- a/docs/configuration/mrf.md +++ b/docs/configuration/mrf.md @@ -43,9 +43,10 @@ Once `SimplePolicy` is enabled, you can configure various groups in the `:mrf_si * `media_removal`: Servers in this group will have media stripped from incoming messages. * `media_nsfw`: Servers in this group will have the #nsfw tag and sensitive setting injected into incoming messages which contain media. -* `reject`: Servers in this group will have their messages rejected. +* `reject`: Servers in this group will have their messages (except deletions) rejected. * `federated_timeline_removal`: Servers in this group will have their messages unlisted from the public timelines by flipping the `to` and `cc` fields. * `report_removal`: Servers in this group will have their reports (flags) rejected. +* `reject_deletes`: Deletion requests will be rejected from these servers. Servers should be configured as lists. diff --git a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex index b23f263f5..b7dcb1b86 100644 --- a/lib/pleroma/web/activity_pub/mrf/simple_policy.ex +++ b/lib/pleroma/web/activity_pub/mrf/simple_policy.ex @@ -149,7 +149,19 @@ defp check_banner_removal(%{host: actor_host} = _actor_info, %{"image" => _image defp check_banner_removal(_actor_info, object), do: {:ok, object} @impl true - def filter(%{"type" => "Delete"} = object), do: {:ok, object} + def filter(%{"type" => "Delete", "actor" => actor} = object) do + %{host: actor_host} = URI.parse(actor) + + reject_deletes = + Pleroma.Config.get([:mrf_simple, :reject_deletes]) + |> MRF.subdomains_regex() + + if MRF.subdomain_match?(reject_deletes, actor_host) do + {:reject, nil} + else + {:ok, object} + end + end @impl true def filter(%{"actor" => actor} = object) do diff --git a/test/web/activity_pub/mrf/simple_policy_test.exs b/test/web/activity_pub/mrf/simple_policy_test.exs index eaa595706..b7b9bc6a2 100644 --- a/test/web/activity_pub/mrf/simple_policy_test.exs +++ b/test/web/activity_pub/mrf/simple_policy_test.exs @@ -17,7 +17,8 @@ defmodule Pleroma.Web.ActivityPub.MRF.SimplePolicyTest do reject: [], accept: [], avatar_removal: [], - banner_removal: [] + banner_removal: [], + reject_deletes: [] ) describe "when :media_removal" do @@ -258,14 +259,6 @@ test "actor has a matching host" do assert SimplePolicy.filter(remote_user) == {:reject, nil} end - - test "always accept deletions" do - Config.put([:mrf_simple, :reject], ["remote.instance"]) - - deletion_message = build_remote_deletion_message() - - assert SimplePolicy.filter(deletion_message) == {:ok, deletion_message} - end end describe "when :accept" do @@ -316,14 +309,6 @@ test "actor has a matching host" do assert SimplePolicy.filter(remote_user) == {:ok, remote_user} end - - test "always accept deletions" do - Config.put([:mrf_simple, :accept], ["non.matching.remote"]) - - deletion_message = build_remote_deletion_message() - - assert SimplePolicy.filter(deletion_message) == {:ok, deletion_message} - end end describe "when :avatar_removal" do @@ -398,6 +383,66 @@ test "match with wildcard domain" do end end + describe "when :reject_deletes is empty" do + setup do: Config.put([:mrf_simple, :reject_deletes], []) + + test "it accepts deletions even from rejected servers" do + Config.put([:mrf_simple, :reject], ["remote.instance"]) + + deletion_message = build_remote_deletion_message() + + assert SimplePolicy.filter(deletion_message) == {:ok, deletion_message} + end + + test "it accepts deletions even from non-whitelisted servers" do + Config.put([:mrf_simple, :accept], ["non.matching.remote"]) + + deletion_message = build_remote_deletion_message() + + assert SimplePolicy.filter(deletion_message) == {:ok, deletion_message} + end + end + + describe "when :reject_deletes is not empty but it doesn't have a matching host" do + setup do: Config.put([:mrf_simple, :reject_deletes], ["non.matching.remote"]) + + test "it accepts deletions even from rejected servers" do + Config.put([:mrf_simple, :reject], ["remote.instance"]) + + deletion_message = build_remote_deletion_message() + + assert SimplePolicy.filter(deletion_message) == {:ok, deletion_message} + end + + test "it accepts deletions even from non-whitelisted servers" do + Config.put([:mrf_simple, :accept], ["non.matching.remote"]) + + deletion_message = build_remote_deletion_message() + + assert SimplePolicy.filter(deletion_message) == {:ok, deletion_message} + end + end + + describe "when :reject_deletes has a matching host" do + setup do: Config.put([:mrf_simple, :reject_deletes], ["remote.instance"]) + + test "it rejects the deletion" do + deletion_message = build_remote_deletion_message() + + assert SimplePolicy.filter(deletion_message) == {:reject, nil} + end + end + + describe "when :reject_deletes match with wildcard domain" do + setup do: Config.put([:mrf_simple, :reject_deletes], ["*.remote.instance"]) + + test "it rejects the deletion" do + deletion_message = build_remote_deletion_message() + + assert SimplePolicy.filter(deletion_message) == {:reject, nil} + end + end + defp build_local_message do %{ "actor" => "#{Pleroma.Web.base_url()}/users/alice", From f719a5b23a9bec4ed94f36c07e24aa1413654bae Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 13:28:34 +0200 Subject: [PATCH 07/25] WebPush: Return proper values for jobs. --- lib/pleroma/web/push/impl.ex | 3 ++- test/web/push/impl_test.exs | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/pleroma/web/push/impl.ex b/lib/pleroma/web/push/impl.ex index f1740a6e0..a9f893f7b 100644 --- a/lib/pleroma/web/push/impl.ex +++ b/lib/pleroma/web/push/impl.ex @@ -55,11 +55,12 @@ def perform( |> Jason.encode!() |> push_message(build_sub(subscription), gcm_api_key, subscription) end + |> (&{:ok, &1}).() end def perform(_) do Logger.warn("Unknown notification type") - :error + {:error, :unknown_type} end @doc "Push message to web" diff --git a/test/web/push/impl_test.exs b/test/web/push/impl_test.exs index 9121d90e7..b2664bf28 100644 --- a/test/web/push/impl_test.exs +++ b/test/web/push/impl_test.exs @@ -63,12 +63,12 @@ test "performs sending notifications" do activity: activity ) - assert Impl.perform(notif) == [:ok, :ok] + assert Impl.perform(notif) == {:ok, [:ok, :ok]} end @tag capture_log: true test "returns error if notif does not match " do - assert Impl.perform(%{}) == :error + assert Impl.perform(%{}) == {:error, :unknown_type} end test "successful message sending" do From 923513b6417973f700a80ee969c6c92ed2c9faee Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 13:28:52 +0200 Subject: [PATCH 08/25] Federator: Return proper values for jobs --- lib/pleroma/web/federator/federator.ex | 13 +++++++++---- test/web/federator_test.exs | 7 +++++-- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/web/federator/federator.ex b/lib/pleroma/web/federator/federator.ex index fd904ef0a..f5803578d 100644 --- a/lib/pleroma/web/federator/federator.ex +++ b/lib/pleroma/web/federator/federator.ex @@ -72,19 +72,24 @@ def perform(:incoming_ap_doc, params) do # actor shouldn't be acting on objects outside their own AP server. with {:ok, _user} <- ap_enabled_actor(params["actor"]), nil <- Activity.normalize(params["id"]), - :ok <- Containment.contain_origin_from_id(params["actor"], params), + {_, :ok} <- + {:correct_origin?, Containment.contain_origin_from_id(params["actor"], params)}, {:ok, activity} <- Transmogrifier.handle_incoming(params) do {:ok, activity} else + {:correct_origin?, _} -> + Logger.debug("Origin containment failure for #{params["id"]}") + {:error, :origin_containment_failed} + %Activity{} -> Logger.debug("Already had #{params["id"]}") - :error + {:error, :already_present} - _e -> + e -> # Just drop those for now Logger.debug("Unhandled activity") Logger.debug(Jason.encode!(params, pretty: true)) - :error + {:error, e} end end diff --git a/test/web/federator_test.exs b/test/web/federator_test.exs index 59e53bb03..261518ef0 100644 --- a/test/web/federator_test.exs +++ b/test/web/federator_test.exs @@ -130,6 +130,9 @@ test "successfully processes incoming AP docs with correct origin" do assert {:ok, job} = Federator.incoming_ap_doc(params) assert {:ok, _activity} = ObanHelpers.perform(job) + + assert {:ok, job} = Federator.incoming_ap_doc(params) + assert {:error, :already_present} = ObanHelpers.perform(job) end test "rejects incoming AP docs with incorrect origin" do @@ -148,7 +151,7 @@ test "rejects incoming AP docs with incorrect origin" do } assert {:ok, job} = Federator.incoming_ap_doc(params) - assert :error = ObanHelpers.perform(job) + assert {:error, :origin_containment_failed} = ObanHelpers.perform(job) end test "it does not crash if MRF rejects the post" do @@ -164,7 +167,7 @@ test "it does not crash if MRF rejects the post" do |> Poison.decode!() assert {:ok, job} = Federator.incoming_ap_doc(params) - assert :error = ObanHelpers.perform(job) + assert {:error, _} = ObanHelpers.perform(job) end end end From 5102468d0f8067548ef233b5947da4bc517f5774 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 14:06:39 +0200 Subject: [PATCH 09/25] Polls: Persist and show voters' count --- lib/pleroma/object.ex | 5 ++++- lib/pleroma/web/activity_pub/activity_pub.ex | 5 +++-- lib/pleroma/web/mastodon_api/views/poll_view.ex | 7 +++++++ test/web/mastodon_api/views/poll_view_test.exs | 16 ++++++++++++++-- 4 files changed, 28 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex index 9574432f0..e678fd415 100644 --- a/lib/pleroma/object.ex +++ b/lib/pleroma/object.ex @@ -261,7 +261,7 @@ def decrease_replies_count(ap_id) do end end - def increase_vote_count(ap_id, name) do + def increase_vote_count(ap_id, name, actor) do with %Object{} = object <- Object.normalize(ap_id), "Question" <- object.data["type"] do multiple = Map.has_key?(object.data, "anyOf") @@ -276,12 +276,15 @@ def increase_vote_count(ap_id, name) do option end) + voters = [actor | object.data["voters"] || []] |> Enum.uniq() + data = if multiple do Map.put(object.data, "anyOf", options) else Map.put(object.data, "oneOf", options) end + |> Map.put("voters", voters) object |> Object.change(%{data: data}) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index eedea08a2..4a133498e 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -118,9 +118,10 @@ def decrease_replies_count_if_reply(_object), do: :noop def increase_poll_votes_if_vote(%{ "object" => %{"inReplyTo" => reply_ap_id, "name" => name}, - "type" => "Create" + "type" => "Create", + "actor" => actor }) do - Object.increase_vote_count(reply_ap_id, name) + Object.increase_vote_count(reply_ap_id, name, actor) end def increase_poll_votes_if_vote(_create_data), do: :noop diff --git a/lib/pleroma/web/mastodon_api/views/poll_view.ex b/lib/pleroma/web/mastodon_api/views/poll_view.ex index 40edbb213..59a5deb28 100644 --- a/lib/pleroma/web/mastodon_api/views/poll_view.ex +++ b/lib/pleroma/web/mastodon_api/views/poll_view.ex @@ -19,6 +19,7 @@ def render("show.json", %{object: object, multiple: multiple, options: options} expired: expired, multiple: multiple, votes_count: votes_count, + voters_count: (multiple || nil) && voters_count(object), options: options, voted: voted?(params), emojis: Pleroma.Web.MastodonAPI.StatusView.build_emojis(object.data["emoji"]) @@ -62,6 +63,12 @@ defp options_and_votes_count(options) do end) end + defp voters_count(%{data: %{"voters" => [_ | _] = voters}}) do + length(voters) + end + + defp voters_count(_), do: 0 + defp voted?(%{object: object} = opts) do if opts[:for] do existing_votes = Pleroma.Web.ActivityPub.Utils.get_existing_votes(opts[:for].ap_id, object) diff --git a/test/web/mastodon_api/views/poll_view_test.exs b/test/web/mastodon_api/views/poll_view_test.exs index 6211fa888..63b204387 100644 --- a/test/web/mastodon_api/views/poll_view_test.exs +++ b/test/web/mastodon_api/views/poll_view_test.exs @@ -43,7 +43,8 @@ test "renders a poll" do %{title: "why are you even asking?", votes_count: 0} ], voted: false, - votes_count: 0 + votes_count: 0, + voters_count: nil } result = PollView.render("show.json", %{object: object}) @@ -69,9 +70,20 @@ test "detects if it is multiple choice" do } }) + voter = insert(:user) + object = Object.normalize(activity) - assert %{multiple: true} = PollView.render("show.json", %{object: object}) + {:ok, _votes, object} = CommonAPI.vote(voter, object, [0, 1]) + + assert match?( + %{ + multiple: true, + voters_count: 1, + votes_count: 2 + }, + PollView.render("show.json", %{object: object}) + ) end test "detects emoji" do From 568f48435e7db4582c54a443b2d543cc004f7f9b Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 12:10:20 +0000 Subject: [PATCH 10/25] Apply suggestion to docs/installation/debian_based_en.md --- docs/installation/debian_based_en.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/installation/debian_based_en.md b/docs/installation/debian_based_en.md index a900ec61d..62d8733f7 100644 --- a/docs/installation/debian_based_en.md +++ b/docs/installation/debian_based_en.md @@ -7,7 +7,7 @@ This guide will assume you are on Debian Stretch. This guide should also work wi * `postgresql` (9.6+, Ubuntu 16.04 comes with 9.5, you can get a newer version from [here](https://www.postgresql.org/download/linux/ubuntu/)) * `postgresql-contrib` (9.6+, same situtation as above) -* `elixir` (1.8+, [install from here, Debian and Ubuntu ship older versions](https://elixir-lang.org/install.html#unix-and-unix-like) or use [asdf](https://github.com/asdf-vm/asdf) as the pleroma user) +* `elixir` (1.8+, Follow the guide to install from the Erlang Solutions repo or use [asdf](https://github.com/asdf-vm/asdf) as the pleroma user) * `erlang-dev` * `erlang-nox` * `git` From c10485db163d56acd7206980f91f0e51153ef36a Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 14:26:19 +0200 Subject: [PATCH 11/25] StatusController: Ignore nil scheduled_at parameters. --- .../web/mastodon_api/controllers/status_controller.ex | 3 ++- .../controllers/status_controller_test.exs | 11 +++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex index 397dd10e3..f6e4f7d66 100644 --- a/lib/pleroma/web/mastodon_api/controllers/status_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/status_controller.ex @@ -127,7 +127,8 @@ def index(%{assigns: %{user: user}} = conn, %{"ids" => ids} = params) do def create( %{assigns: %{user: user}} = conn, %{"status" => _, "scheduled_at" => scheduled_at} = params - ) do + ) + when not is_nil(scheduled_at) do params = Map.put(params, "in_reply_to_status_id", params["in_reply_to_id"]) with {:far_enough, true} <- {:far_enough, ScheduledActivity.far_enough?(scheduled_at)}, diff --git a/test/web/mastodon_api/controllers/status_controller_test.exs b/test/web/mastodon_api/controllers/status_controller_test.exs index 162f7b1b2..85068edd0 100644 --- a/test/web/mastodon_api/controllers/status_controller_test.exs +++ b/test/web/mastodon_api/controllers/status_controller_test.exs @@ -302,6 +302,17 @@ test "creates a scheduled activity", %{conn: conn} do assert [] == Repo.all(Activity) end + test "ignores nil values", %{conn: conn} do + conn = + post(conn, "/api/v1/statuses", %{ + "status" => "not scheduled", + "scheduled_at" => nil + }) + + assert result = json_response(conn, 200) + assert Activity.get_by_id(result["id"]) + end + test "creates a scheduled activity with a media attachment", %{user: user, conn: conn} do scheduled_at = NaiveDateTime.add(NaiveDateTime.utc_now(), :timer.minutes(120), :millisecond) From 5b3952619818d38f8fdba9a64b050ce3f24394ff Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 15:04:26 +0200 Subject: [PATCH 12/25] AccountController: Use code 400 for self-follow. --- .../controllers/account_controller.ex | 21 ++++++++++--------- .../controllers/account_controller_test.exs | 6 +++--- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex index e8e59ac66..5a92cebd8 100644 --- a/lib/pleroma/web/mastodon_api/controllers/account_controller.ex +++ b/lib/pleroma/web/mastodon_api/controllers/account_controller.ex @@ -293,7 +293,7 @@ def lists(%{assigns: %{user: user, account: account}} = conn, _params) do @doc "POST /api/v1/accounts/:id/follow" def follow(%{assigns: %{user: %{id: id}, account: %{id: id}}}, _params) do - {:error, :not_found} + {:error, "Can not follow yourself"} end def follow(%{assigns: %{user: follower, account: followed}} = conn, _params) do @@ -306,7 +306,7 @@ def follow(%{assigns: %{user: follower, account: followed}} = conn, _params) do @doc "POST /api/v1/accounts/:id/unfollow" def unfollow(%{assigns: %{user: %{id: id}, account: %{id: id}}}, _params) do - {:error, :not_found} + {:error, "Can not unfollow yourself"} end def unfollow(%{assigns: %{user: follower, account: followed}} = conn, _params) do @@ -356,14 +356,15 @@ def unblock(%{assigns: %{user: blocker, account: blocked}} = conn, _params) do end @doc "POST /api/v1/follows" - def follows(%{assigns: %{user: follower}} = conn, %{"uri" => uri}) do - with {_, %User{} = followed} <- {:followed, User.get_cached_by_nickname(uri)}, - {_, true} <- {:followed, follower.id != followed.id}, - {:ok, follower, followed, _} <- CommonAPI.follow(follower, followed) do - render(conn, "show.json", user: followed, for: follower) - else - {:followed, _} -> {:error, :not_found} - {:error, message} -> json_response(conn, :forbidden, %{error: message}) + def follows(conn, %{"uri" => uri}) do + case User.get_cached_by_nickname(uri) do + %User{} = user -> + conn + |> assign(:account, user) + |> follow(%{}) + + nil -> + {:error, :not_found} end end diff --git a/test/web/mastodon_api/controllers/account_controller_test.exs b/test/web/mastodon_api/controllers/account_controller_test.exs index 61c2697b2..8c428efee 100644 --- a/test/web/mastodon_api/controllers/account_controller_test.exs +++ b/test/web/mastodon_api/controllers/account_controller_test.exs @@ -681,17 +681,17 @@ test "following without reblogs" do test "following / unfollowing errors", %{user: user, conn: conn} do # self follow conn_res = post(conn, "/api/v1/accounts/#{user.id}/follow") - assert %{"error" => "Record not found"} = json_response(conn_res, 404) + assert %{"error" => "Can not follow yourself"} = json_response(conn_res, 400) # self unfollow user = User.get_cached_by_id(user.id) conn_res = post(conn, "/api/v1/accounts/#{user.id}/unfollow") - assert %{"error" => "Record not found"} = json_response(conn_res, 404) + assert %{"error" => "Can not unfollow yourself"} = json_response(conn_res, 400) # self follow via uri user = User.get_cached_by_id(user.id) conn_res = post(conn, "/api/v1/follows", %{"uri" => user.nickname}) - assert %{"error" => "Record not found"} = json_response(conn_res, 404) + assert %{"error" => "Can not follow yourself"} = json_response(conn_res, 400) # follow non existing user conn_res = post(conn, "/api/v1/accounts/doesntexist/follow") From 8b88e2a6e2b3a777ca99bf94676ab47f2d4cc0ea Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 15:31:37 +0200 Subject: [PATCH 13/25] Stats: Ignore internal users for user count. --- lib/pleroma/stats.ex | 19 ++++++++++++++----- test/{stat_test.exs => stats_test.exs} | 13 ++++++++++++- 2 files changed, 26 insertions(+), 6 deletions(-) rename test/{stat_test.exs => stats_test.exs} (84%) diff --git a/lib/pleroma/stats.ex b/lib/pleroma/stats.ex index 4446562ac..6763786a7 100644 --- a/lib/pleroma/stats.ex +++ b/lib/pleroma/stats.ex @@ -45,11 +45,11 @@ def get_peers do end def init(_args) do - {:ok, get_stat_data()} + {:ok, calculate_stat_data()} end def handle_call(:force_update, _from, _state) do - new_stats = get_stat_data() + new_stats = calculate_stat_data() {:reply, new_stats, new_stats} end @@ -58,12 +58,12 @@ def handle_call(:get_state, _from, state) do end def handle_cast(:run_update, _state) do - new_stats = get_stat_data() + new_stats = calculate_stat_data() {:noreply, new_stats} end - defp get_stat_data do + def calculate_stat_data do peers = from( u in User, @@ -77,7 +77,16 @@ defp get_stat_data do status_count = Repo.aggregate(User.Query.build(%{local: true}), :sum, :note_count) - user_count = Repo.aggregate(User.Query.build(%{local: true, active: true}), :count, :id) + users_query = + from(u in User, + where: u.deactivated != true, + where: u.local == true, + where: not is_nil(u.nickname), + where: fragment("? not like 'internal.%'", u.nickname), + where: fragment("? not like '%/relay'", u.ap_id) + ) + + user_count = Repo.aggregate(users_query, :count, :id) %{ peers: peers, diff --git a/test/stat_test.exs b/test/stats_test.exs similarity index 84% rename from test/stat_test.exs rename to test/stats_test.exs index bccc1c8d0..73c7c1495 100644 --- a/test/stat_test.exs +++ b/test/stats_test.exs @@ -2,11 +2,22 @@ # Copyright © 2017-2020 Pleroma Authors # SPDX-License-Identifier: AGPL-3.0-only -defmodule Pleroma.StateTest do +defmodule Pleroma.StatsTest do use Pleroma.DataCase import Pleroma.Factory alias Pleroma.Web.CommonAPI + describe "user count" do + test "it ignores internal users" do + _user = insert(:user, local: true) + _internal = insert(:user, local: true, nickname: nil) + _internal = insert(:user, local: true, nickname: "internal.dude") + _internal = Pleroma.Web.ActivityPub.Relay.get_actor() + + assert match?(%{stats: %{user_count: 1}}, Pleroma.Stats.calculate_stat_data()) + end + end + describe "status visibility count" do test "on new status" do user = insert(:user) From 452072ec95781214df262962b71c60b7d771b7b1 Mon Sep 17 00:00:00 2001 From: Karol Kosek Date: Wed, 22 Apr 2020 16:02:40 +0200 Subject: [PATCH 14/25] static_fe: Add microformats2 classes --- .../templates/static_fe/static_fe/_attachment.html.eex | 6 +++--- .../web/templates/static_fe/static_fe/_notice.html.eex | 10 +++++++--- .../templates/static_fe/static_fe/_user_card.html.eex | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/lib/pleroma/web/templates/static_fe/static_fe/_attachment.html.eex b/lib/pleroma/web/templates/static_fe/static_fe/_attachment.html.eex index 7e04e9550..4853e7f4b 100644 --- a/lib/pleroma/web/templates/static_fe/static_fe/_attachment.html.eex +++ b/lib/pleroma/web/templates/static_fe/static_fe/_attachment.html.eex @@ -1,8 +1,8 @@ <%= case @mediaType do %> <% "audio" -> %> - + <% "video" -> %> - + <% _ -> %> -<%= @name %> +<%= @name %> <% end %> diff --git a/lib/pleroma/web/templates/static_fe/static_fe/_notice.html.eex b/lib/pleroma/web/templates/static_fe/static_fe/_notice.html.eex index df5e5eedd..df0244795 100644 --- a/lib/pleroma/web/templates/static_fe/static_fe/_notice.html.eex +++ b/lib/pleroma/web/templates/static_fe/static_fe/_notice.html.eex @@ -1,12 +1,16 @@ -
id="selected" <% end %>> +
id="selected" <% end %>>

- <%= link format_date(@published), to: @link, class: "activity-link" %> + + +

<%= render("_user_card.html", %{user: @user}) %>
<%= if @title != "" do %>
open<% end %>> - <%= raw @title %> + <%= raw @title %>
<%= raw @content %>
<% else %> diff --git a/lib/pleroma/web/templates/static_fe/static_fe/_user_card.html.eex b/lib/pleroma/web/templates/static_fe/static_fe/_user_card.html.eex index 56f3a1524..977b894d3 100644 --- a/lib/pleroma/web/templates/static_fe/static_fe/_user_card.html.eex +++ b/lib/pleroma/web/templates/static_fe/static_fe/_user_card.html.eex @@ -1,10 +1,10 @@
- +
- <%= raw Formatter.emojify(@user.name, @user.emoji) %> + <%= raw Formatter.emojify(@user.name, @user.emoji) %> <%= @user.nickname %> From 7a3a88a13ef526fba18bb6aeadc93f5da934dc5b Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 17:21:13 +0200 Subject: [PATCH 15/25] Streamer: Stream boosts to the boosting user. --- lib/pleroma/user.ex | 4 +++- lib/pleroma/web/streamer/worker.ex | 18 --------------- test/user_test.exs | 12 ++++++++++ test/web/streamer/streamer_test.exs | 36 +++++++++++++++++++++++++++++ 4 files changed, 51 insertions(+), 19 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index bef4679cb..477237756 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -1180,7 +1180,9 @@ def get_users_from_set(ap_ids, local_only \\ true) do end @spec get_recipients_from_activity(Activity.t()) :: [User.t()] - def get_recipients_from_activity(%Activity{recipients: to}) do + def get_recipients_from_activity(%Activity{recipients: to, actor: actor}) do + to = [actor | to] + User.Query.build(%{recipients_from_activity: to, local: true, deactivated: false}) |> Repo.all() end diff --git a/lib/pleroma/web/streamer/worker.ex b/lib/pleroma/web/streamer/worker.ex index abfed21c8..f6160fa4d 100644 --- a/lib/pleroma/web/streamer/worker.ex +++ b/lib/pleroma/web/streamer/worker.ex @@ -158,24 +158,6 @@ defp should_send?(%User{} = user, %Notification{activity: activity}) do should_send?(user, activity) end - def push_to_socket(topics, topic, %Activity{data: %{"type" => "Announce"}} = item) do - Enum.each(topics[topic] || [], fn %StreamerSocket{ - transport_pid: transport_pid, - user: socket_user - } -> - # Get the current user so we have up-to-date blocks etc. - if socket_user do - user = User.get_cached_by_ap_id(socket_user.ap_id) - - if should_send?(user, item) do - send(transport_pid, {:text, StreamerView.render("update.json", item, user)}) - end - else - send(transport_pid, {:text, StreamerView.render("update.json", item)}) - end - end) - end - def push_to_socket(topics, topic, %Participation{} = participation) do Enum.each(topics[topic] || [], fn %StreamerSocket{transport_pid: transport_pid} -> send(transport_pid, {:text, StreamerView.render("conversation.json", participation)}) diff --git a/test/user_test.exs b/test/user_test.exs index 65e118d6d..cd4041673 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -987,6 +987,18 @@ test "it imports user blocks from list" do end describe "get_recipients_from_activity" do + test "works for announces" do + actor = insert(:user) + user = insert(:user, local: true) + + {:ok, activity} = CommonAPI.post(actor, %{"status" => "hello"}) + {:ok, announce, _} = CommonAPI.repeat(activity.id, user) + + recipients = User.get_recipients_from_activity(announce) + + assert user in recipients + end + test "get recipients" do actor = insert(:user) user = insert(:user, local: true) diff --git a/test/web/streamer/streamer_test.exs b/test/web/streamer/streamer_test.exs index eb082b79f..8b8d8af6c 100644 --- a/test/web/streamer/streamer_test.exs +++ b/test/web/streamer/streamer_test.exs @@ -28,6 +28,42 @@ defmodule Pleroma.Web.StreamerTest do {:ok, %{user: user, notify: notify}} end + test "it streams the user's post in the 'user' stream", %{user: user} do + task = + Task.async(fn -> + assert_receive {:text, _}, @streamer_timeout + end) + + Streamer.add_socket( + "user", + %{transport_pid: task.pid, assigns: %{user: user}} + ) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "hey"}) + + Streamer.stream("user", activity) + Task.await(task) + end + + test "it streams boosts of the user in the 'user' stream", %{user: user} do + task = + Task.async(fn -> + assert_receive {:text, _}, @streamer_timeout + end) + + Streamer.add_socket( + "user", + %{transport_pid: task.pid, assigns: %{user: user}} + ) + + other_user = insert(:user) + {:ok, activity} = CommonAPI.post(other_user, %{"status" => "hey"}) + {:ok, announce, _} = CommonAPI.repeat(activity.id, user) + + Streamer.stream("user", announce) + Task.await(task) + end + test "it sends notify to in the 'user' stream", %{user: user, notify: notify} do task = Task.async(fn -> From 88b82e5c3edae649f1caa45c6ef805828e4b8b1e Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Tue, 21 Apr 2020 20:05:25 +0400 Subject: [PATCH 16/25] Fix follow import --- .../controllers/util_controller.ex | 25 +++++++++---------- test/web/twitter_api/util_controller_test.exs | 24 ++++++++++++++++++ 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/lib/pleroma/web/twitter_api/controllers/util_controller.ex b/lib/pleroma/web/twitter_api/controllers/util_controller.ex index 537f9f778..824951d59 100644 --- a/lib/pleroma/web/twitter_api/controllers/util_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/util_controller.ex @@ -199,15 +199,16 @@ def follow_import(conn, %{"list" => %Plug.Upload{} = listfile}) do end def follow_import(%{assigns: %{user: follower}} = conn, %{"list" => list}) do - with lines <- String.split(list, "\n"), - followed_identifiers <- - Enum.map(lines, fn line -> - String.split(line, ",") |> List.first() - end) - |> List.delete("Account address") do - User.follow_import(follower, followed_identifiers) - json(conn, "job started") - end + followed_identifiers = + list + |> String.split("\n") + |> Enum.map(&(&1 |> String.split(",") |> List.first())) + |> List.delete("Account address") + |> Enum.map(&(&1 |> String.trim() |> String.trim_leading("@"))) + |> Enum.reject(&(&1 == "")) + + User.follow_import(follower, followed_identifiers) + json(conn, "job started") end def blocks_import(conn, %{"list" => %Plug.Upload{} = listfile}) do @@ -215,10 +216,8 @@ def blocks_import(conn, %{"list" => %Plug.Upload{} = listfile}) do end def blocks_import(%{assigns: %{user: blocker}} = conn, %{"list" => list}) do - with blocked_identifiers <- String.split(list) do - User.blocks_import(blocker, blocked_identifiers) - json(conn, "job started") - end + User.blocks_import(blocker, _blocked_identifiers = String.split(list)) + json(conn, "job started") end def change_password(%{assigns: %{user: user}} = conn, params) do diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index 30e54bebd..85aaab19b 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -95,6 +95,30 @@ test "requires 'follow' or 'write:follows' permissions" do end end end + + test "it imports with different nickname variations", %{conn: conn} do + [user2, user3, user4, user5, user6] = insert_list(5, :user) + + identifiers = + [ + user2.ap_id, + user3.nickname, + " ", + "@" <> user4.nickname, + user5.nickname <> "@localhost", + "@" <> user6.nickname <> "@localhost" + ] + |> Enum.join("\n") + + response = + conn + |> post("/api/pleroma/follow_import", %{"list" => identifiers}) + |> json_response(:ok) + + assert response == "job started" + assert [job_result] = ObanHelpers.perform_all() + assert job_result == [user2, user3, user4, user5, user6] + end end describe "POST /api/pleroma/blocks_import" do From e7771424a895572626ec36a98bf862952dc6ff07 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Wed, 22 Apr 2020 18:13:13 +0400 Subject: [PATCH 17/25] Fix blocks import --- .../controllers/util_controller.ex | 3 ++- test/web/twitter_api/util_controller_test.exs | 25 ++++++++++++++++++- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lib/pleroma/web/twitter_api/controllers/util_controller.ex b/lib/pleroma/web/twitter_api/controllers/util_controller.ex index 824951d59..d5d5ce08f 100644 --- a/lib/pleroma/web/twitter_api/controllers/util_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/util_controller.ex @@ -216,7 +216,8 @@ def blocks_import(conn, %{"list" => %Plug.Upload{} = listfile}) do end def blocks_import(%{assigns: %{user: blocker}} = conn, %{"list" => list}) do - User.blocks_import(blocker, _blocked_identifiers = String.split(list)) + blocked_identifiers = list |> String.split() |> Enum.map(&String.trim_leading(&1, "@")) + User.blocks_import(blocker, blocked_identifiers) json(conn, "job started") end diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index 85aaab19b..d835331ae 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -96,7 +96,7 @@ test "requires 'follow' or 'write:follows' permissions" do end end - test "it imports with different nickname variations", %{conn: conn} do + test "it imports follows with different nickname variations", %{conn: conn} do [user2, user3, user4, user5, user6] = insert_list(5, :user) identifiers = @@ -160,6 +160,29 @@ test "it imports blocks users from file", %{user: user1, conn: conn} do ) end end + + test "it imports blocks with different nickname variations", %{conn: conn} do + [user2, user3, user4, user5, user6] = insert_list(5, :user) + + identifiers = + [ + user2.ap_id, + user3.nickname, + "@" <> user4.nickname, + user5.nickname <> "@localhost", + "@" <> user6.nickname <> "@localhost" + ] + |> Enum.join(" ") + + response = + conn + |> post("/api/pleroma/blocks_import", %{"list" => identifiers}) + |> json_response(:ok) + + assert response == "job started" + assert [job_result] = ObanHelpers.perform_all() + assert job_result == [user2, user3, user4, user5, user6] + end end describe "PUT /api/pleroma/notification_settings" do From 6db52c3b3680efbdb56d53d84d5dec0f4b6e34f0 Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Wed, 22 Apr 2020 19:00:08 +0400 Subject: [PATCH 18/25] Fix Oban warning Warning example: [warn] Expected Elixir.Pleroma.Workers.BackgroundWorker.perform/2 to return :ok, {:ok, value}, or {:error, reason}. Instead received: [error: "not found @user@server.party", error: "not found "] The job will be considered a success. --- lib/pleroma/workers/background_worker.ex | 4 ++-- test/user_test.exs | 4 ++-- test/web/twitter_api/util_controller_test.exs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/pleroma/workers/background_worker.ex b/lib/pleroma/workers/background_worker.ex index 0f8ece2c4..57c3a9c3a 100644 --- a/lib/pleroma/workers/background_worker.ex +++ b/lib/pleroma/workers/background_worker.ex @@ -35,7 +35,7 @@ def perform( _job ) do blocker = User.get_cached_by_id(blocker_id) - User.perform(:blocks_import, blocker, blocked_identifiers) + {:ok, User.perform(:blocks_import, blocker, blocked_identifiers)} end def perform( @@ -47,7 +47,7 @@ def perform( _job ) do follower = User.get_cached_by_id(follower_id) - User.perform(:follow_import, follower, followed_identifiers) + {:ok, User.perform(:follow_import, follower, followed_identifiers)} end def perform(%{"op" => "media_proxy_preload", "message" => message}, _job) do diff --git a/test/user_test.exs b/test/user_test.exs index 65e118d6d..23e7cf6e3 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -756,8 +756,8 @@ test "it imports user followings from list" do ] {:ok, job} = User.follow_import(user1, identifiers) - result = ObanHelpers.perform(job) + assert {:ok, result} = ObanHelpers.perform(job) assert is_list(result) assert result == [user2, user3] end @@ -979,8 +979,8 @@ test "it imports user blocks from list" do ] {:ok, job} = User.blocks_import(user1, identifiers) - result = ObanHelpers.perform(job) + assert {:ok, result} = ObanHelpers.perform(job) assert is_list(result) assert result == [user2, user3] end diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index d835331ae..b701239a0 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -116,7 +116,7 @@ test "it imports follows with different nickname variations", %{conn: conn} do |> json_response(:ok) assert response == "job started" - assert [job_result] = ObanHelpers.perform_all() + assert [{:ok, job_result}] = ObanHelpers.perform_all() assert job_result == [user2, user3, user4, user5, user6] end end @@ -180,7 +180,7 @@ test "it imports blocks with different nickname variations", %{conn: conn} do |> json_response(:ok) assert response == "job started" - assert [job_result] = ObanHelpers.perform_all() + assert [{:ok, job_result}] = ObanHelpers.perform_all() assert job_result == [user2, user3, user4, user5, user6] end end From 771c1ad735eb06842278e823a29acb94fd9acafb Mon Sep 17 00:00:00 2001 From: Egor Kislitsyn Date: Wed, 22 Apr 2020 19:31:03 +0400 Subject: [PATCH 19/25] Update CHANGELOG --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2d8e7efc3..702c58180 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Fixed - Support pagination in conversations API - **Breaking**: SimplePolicy `:reject` and `:accept` allow deletions again +- Fix follower/blocks import when nicknames starts with @ ## [unreleased-patch] ### Fixed From e62173dfc8b739508345b7ab97477ae04fcdb457 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 18:40:53 +0200 Subject: [PATCH 20/25] SideEffects: Run in transaction. This fixes race conditions. --- lib/pleroma/web/activity_pub/side_effects.ex | 11 +++++--- test/web/common_api/common_api_test.exs | 27 ++++++++++++++++++++ 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index 6a8f1af96..a0f71fd88 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -15,12 +15,15 @@ def handle(object, meta \\ []) # - Add like to object # - Set up notification def handle(%{data: %{"type" => "Like"}} = object, meta) do - liked_object = Object.get_by_ap_id(object.data["object"]) - Utils.add_like_to_object(object, liked_object) + Pleroma.Repo.transaction(fn -> + liked_object = Object.get_by_ap_id(object.data["object"]) + Utils.add_like_to_object(object, liked_object) - Notification.create_notifications(object) + Notification.create_notifications(object) - {:ok, object, meta} + {:ok, object, meta} + end) + |> (fn {:ok, res} -> res end).() end # Nothing to do diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index e130736ec..68a29108a 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -21,6 +21,33 @@ defmodule Pleroma.Web.CommonAPITest do setup do: clear_config([:instance, :limit]) setup do: clear_config([:instance, :max_pinned_statuses]) + test "favoriting race condition" do + user = insert(:user) + users_serial = insert_list(10, :user) + users = insert_list(10, :user) + + {:ok, activity} = CommonAPI.post(user, %{"status" => "."}) + + users_serial + |> Enum.map(fn user -> + CommonAPI.favorite(user, activity.id) + end) + + object = Object.get_by_ap_id(activity.data["object"]) + assert object.data["like_count"] == 10 + + users + |> Enum.map(fn user -> + Task.async(fn -> + CommonAPI.favorite(user, activity.id) + end) + end) + |> Enum.map(&Task.await/1) + + object = Object.get_by_ap_id(activity.data["object"]) + assert object.data["like_count"] == 20 + end + test "when replying to a conversation / participation, it will set the correct context id even if no explicit reply_to is given" do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{"status" => ".", "visibility" => "direct"}) From f5bda09de648a6de3151c8614005ebc70447facb Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 19:02:22 +0200 Subject: [PATCH 21/25] Stats: Use `invisible` property for filtering. --- lib/pleroma/stats.ex | 3 +-- test/stats_test.exs | 1 - 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/pleroma/stats.ex b/lib/pleroma/stats.ex index 6763786a7..8d2809bbb 100644 --- a/lib/pleroma/stats.ex +++ b/lib/pleroma/stats.ex @@ -82,8 +82,7 @@ def calculate_stat_data do where: u.deactivated != true, where: u.local == true, where: not is_nil(u.nickname), - where: fragment("? not like 'internal.%'", u.nickname), - where: fragment("? not like '%/relay'", u.ap_id) + where: not u.invisible ) user_count = Repo.aggregate(users_query, :count, :id) diff --git a/test/stats_test.exs b/test/stats_test.exs index 73c7c1495..c1aeb2c7f 100644 --- a/test/stats_test.exs +++ b/test/stats_test.exs @@ -11,7 +11,6 @@ defmodule Pleroma.StatsTest do test "it ignores internal users" do _user = insert(:user, local: true) _internal = insert(:user, local: true, nickname: nil) - _internal = insert(:user, local: true, nickname: "internal.dude") _internal = Pleroma.Web.ActivityPub.Relay.get_actor() assert match?(%{stats: %{user_count: 1}}, Pleroma.Stats.calculate_stat_data()) From 1bcbdc7a9f5df35f42d1ab331bc4b6785e180284 Mon Sep 17 00:00:00 2001 From: lain Date: Wed, 22 Apr 2020 21:21:21 +0200 Subject: [PATCH 22/25] SideEffects: Use less cryptic syntax. --- lib/pleroma/web/activity_pub/side_effects.ex | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/pleroma/web/activity_pub/side_effects.ex b/lib/pleroma/web/activity_pub/side_effects.ex index a0f71fd88..5981e7545 100644 --- a/lib/pleroma/web/activity_pub/side_effects.ex +++ b/lib/pleroma/web/activity_pub/side_effects.ex @@ -15,15 +15,17 @@ def handle(object, meta \\ []) # - Add like to object # - Set up notification def handle(%{data: %{"type" => "Like"}} = object, meta) do - Pleroma.Repo.transaction(fn -> - liked_object = Object.get_by_ap_id(object.data["object"]) - Utils.add_like_to_object(object, liked_object) + {:ok, result} = + Pleroma.Repo.transaction(fn -> + liked_object = Object.get_by_ap_id(object.data["object"]) + Utils.add_like_to_object(object, liked_object) - Notification.create_notifications(object) + Notification.create_notifications(object) - {:ok, object, meta} - end) - |> (fn {:ok, res} -> res end).() + {:ok, object, meta} + end) + + result end # Nothing to do From 7d38197894692306c940b55045b91d563e138284 Mon Sep 17 00:00:00 2001 From: lain Date: Thu, 23 Apr 2020 13:33:30 +0200 Subject: [PATCH 23/25] CommonAPI: Don't make repeating announces possible --- lib/pleroma/web/common_api/common_api.ex | 23 +++++++++++-------- lib/pleroma/web/common_api/utils.ex | 18 --------------- test/web/common_api/common_api_test.exs | 18 ++++++++++++--- test/web/common_api/common_api_utils_test.exs | 20 ---------------- 4 files changed, 28 insertions(+), 51 deletions(-) diff --git a/lib/pleroma/web/common_api/common_api.ex b/lib/pleroma/web/common_api/common_api.ex index f50a909aa..d1efe0c36 100644 --- a/lib/pleroma/web/common_api/common_api.ex +++ b/lib/pleroma/web/common_api/common_api.ex @@ -86,8 +86,9 @@ def delete(activity_id, user) do end end - def repeat(id_or_ap_id, user, params \\ %{}) do - with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)}, + def repeat(id, user, params \\ %{}) do + with {_, %Activity{data: %{"type" => "Create"}} = activity} <- + {:find_activity, Activity.get_by_id(id)}, object <- Object.normalize(activity), announce_activity <- Utils.get_existing_announce(user.ap_id, object), public <- public_announce?(object, params) do @@ -102,8 +103,9 @@ def repeat(id_or_ap_id, user, params \\ %{}) do end end - def unrepeat(id_or_ap_id, user) do - with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do + def unrepeat(id, user) do + with {_, %Activity{data: %{"type" => "Create"}} = activity} <- + {:find_activity, Activity.get_by_id(id)} do object = Object.normalize(activity) ActivityPub.unannounce(user, object) else @@ -160,8 +162,9 @@ def favorite_helper(user, id) do end end - def unfavorite(id_or_ap_id, user) do - with {_, %Activity{} = activity} <- {:find_activity, get_by_id_or_ap_id(id_or_ap_id)} do + def unfavorite(id, user) do + with {_, %Activity{data: %{"type" => "Create"}} = activity} <- + {:find_activity, Activity.get_by_id(id)} do object = Object.normalize(activity) ActivityPub.unlike(user, object) else @@ -332,12 +335,12 @@ defp maybe_create_activity_expiration({:ok, activity}, %NaiveDateTime{} = expire defp maybe_create_activity_expiration(result, _), do: result - def pin(id_or_ap_id, %{ap_id: user_ap_id} = user) do + def pin(id, %{ap_id: user_ap_id} = user) do with %Activity{ actor: ^user_ap_id, data: %{"type" => "Create"}, object: %Object{data: %{"type" => object_type}} - } = activity <- get_by_id_or_ap_id(id_or_ap_id), + } = activity <- Activity.get_by_id_with_object(id), true <- object_type in ["Note", "Article", "Question"], true <- Visibility.is_public?(activity), {:ok, _user} <- User.add_pinnned_activity(user, activity) do @@ -348,8 +351,8 @@ def pin(id_or_ap_id, %{ap_id: user_ap_id} = user) do end end - def unpin(id_or_ap_id, user) do - with %Activity{} = activity <- get_by_id_or_ap_id(id_or_ap_id), + def unpin(id, user) do + with %Activity{data: %{"type" => "Create"}} = activity <- Activity.get_by_id(id), {:ok, _user} <- User.remove_pinnned_activity(user, activity) do {:ok, activity} else diff --git a/lib/pleroma/web/common_api/utils.ex b/lib/pleroma/web/common_api/utils.ex index 7eec5aa09..945e63e22 100644 --- a/lib/pleroma/web/common_api/utils.ex +++ b/lib/pleroma/web/common_api/utils.ex @@ -22,24 +22,6 @@ defmodule Pleroma.Web.CommonAPI.Utils do require Logger require Pleroma.Constants - # This is a hack for twidere. - def get_by_id_or_ap_id(id) do - activity = - with true <- FlakeId.flake_id?(id), - %Activity{} = activity <- Activity.get_by_id_with_object(id) do - activity - else - _ -> Activity.get_create_by_object_ap_id_with_object(id) - end - - activity && - if activity.data["type"] == "Create" do - activity - else - Activity.get_create_by_object_ap_id_with_object(activity.data["object"]) - end - end - def attachments_from_ids(%{"media_ids" => ids, "descriptions" => desc} = _) do attachments_from_ids_descs(ids, desc) end diff --git a/test/web/common_api/common_api_test.exs b/test/web/common_api/common_api_test.exs index 68a29108a..e87193c83 100644 --- a/test/web/common_api/common_api_test.exs +++ b/test/web/common_api/common_api_test.exs @@ -283,6 +283,16 @@ test "repeating a status" do {:ok, %Activity{}, _} = CommonAPI.repeat(activity.id, user) end + test "can't repeat a repeat" do + user = insert(:user) + other_user = insert(:user) + {:ok, activity} = CommonAPI.post(other_user, %{"status" => "cofe"}) + + {:ok, %Activity{} = announce, _} = CommonAPI.repeat(activity.id, other_user) + + refute match?({:ok, %Activity{}, _}, CommonAPI.repeat(announce.id, user)) + end + test "repeating a status privately" do user = insert(:user) other_user = insert(:user) @@ -312,8 +322,8 @@ test "retweeting a status twice returns the status" do other_user = insert(:user) {:ok, activity} = CommonAPI.post(other_user, %{"status" => "cofe"}) - {:ok, %Activity{} = activity, object} = CommonAPI.repeat(activity.id, user) - {:ok, ^activity, ^object} = CommonAPI.repeat(activity.id, user) + {:ok, %Activity{} = announce, object} = CommonAPI.repeat(activity.id, user) + {:ok, ^announce, ^object} = CommonAPI.repeat(activity.id, user) end test "favoriting a status twice returns ok, but without the like activity" do @@ -387,7 +397,9 @@ test "unpin status", %{user: user, activity: activity} do user = refresh_record(user) - assert {:ok, ^activity} = CommonAPI.unpin(activity.id, user) + id = activity.id + + assert match?({:ok, %{id: ^id}}, CommonAPI.unpin(activity.id, user)) user = refresh_record(user) diff --git a/test/web/common_api/common_api_utils_test.exs b/test/web/common_api/common_api_utils_test.exs index b21445fe9..18a3b3b87 100644 --- a/test/web/common_api/common_api_utils_test.exs +++ b/test/web/common_api/common_api_utils_test.exs @@ -335,26 +335,6 @@ test "for direct posts, a reply" do end end - describe "get_by_id_or_ap_id/1" do - test "get activity by id" do - activity = insert(:note_activity) - %Pleroma.Activity{} = note = Utils.get_by_id_or_ap_id(activity.id) - assert note.id == activity.id - end - - test "get activity by ap_id" do - activity = insert(:note_activity) - %Pleroma.Activity{} = note = Utils.get_by_id_or_ap_id(activity.data["object"]) - assert note.id == activity.id - end - - test "get activity by object when type isn't `Create` " do - activity = insert(:like_activity) - %Pleroma.Activity{} = like = Utils.get_by_id_or_ap_id(activity.id) - assert like.data["object"] == activity.data["object"] - end - end - describe "to_master_date/1" do test "removes microseconds from date (NaiveDateTime)" do assert Utils.to_masto_date(~N[2015-01-23 23:50:07.123]) == "2015-01-23T23:50:07.000Z" From bbf8554c975ea1ba9b5c809a7891ec0fb4a8e537 Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 24 Apr 2020 13:48:13 +0200 Subject: [PATCH 24/25] ActivitPub: Remove `like` function. We don't need another way to build likes. --- lib/pleroma/web/activity_pub/activity_pub.ex | 30 -------- .../activity_pub/activity_pub_controller.ex | 7 +- test/web/activity_pub/activity_pub_test.exs | 77 ++----------------- test/web/activity_pub/utils_test.exs | 5 +- 4 files changed, 15 insertions(+), 104 deletions(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex index 4a133498e..c67b3335d 100644 --- a/lib/pleroma/web/activity_pub/activity_pub.ex +++ b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -398,36 +398,6 @@ defp do_unreact_with_emoji(user, reaction_id, options) do end end - # TODO: This is weird, maybe we shouldn't check here if we can make the activity. - @spec like(User.t(), Object.t(), String.t() | nil, boolean()) :: - {:ok, Activity.t(), Object.t()} | {:error, any()} - def like(user, object, activity_id \\ nil, local \\ true) do - with {:ok, result} <- Repo.transaction(fn -> do_like(user, object, activity_id, local) end) do - result - end - end - - defp do_like( - %User{ap_id: ap_id} = user, - %Object{data: %{"id" => _}} = object, - activity_id, - local - ) do - with nil <- get_existing_like(ap_id, object), - like_data <- make_like_data(user, object, activity_id), - {:ok, activity} <- insert(like_data, local), - {:ok, object} <- add_like_to_object(activity, object), - :ok <- maybe_federate(activity) do - {:ok, activity, object} - else - %Activity{} = activity -> - {:ok, activity, object} - - {:error, error} -> - Repo.rollback(error) - end - end - @spec unlike(User.t(), Object.t(), String.t() | nil, boolean()) :: {:ok, Activity.t(), Activity.t(), Object.t()} | {:ok, Object.t()} | {:error, any()} def unlike(%User{} = actor, %Object{} = object, activity_id \\ nil, local \\ true) do diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 8b9eb4a2c..325a714b4 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -12,6 +12,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do alias Pleroma.Plugs.EnsureAuthenticatedPlug alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub + alias Pleroma.Web.ActivityPub.Builder + alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.InternalFetchActor alias Pleroma.Web.ActivityPub.ObjectView alias Pleroma.Web.ActivityPub.Relay @@ -421,7 +423,10 @@ defp handle_user_activity(%User{} = user, %{"type" => "Delete"} = params) do defp handle_user_activity(%User{} = user, %{"type" => "Like"} = params) do with %Object{} = object <- Object.normalize(params["object"]), - {:ok, activity, _object} <- ActivityPub.like(user, object) do + {_, {:ok, like_object, meta}} <- {:build_object, Builder.like(user, object)}, + {_, {:ok, %Activity{} = activity, _meta}} <- + {:common_pipeline, + Pipeline.common_pipeline(like_object, Keyword.put(meta, :local, true))} do {:ok, activity} else _ -> {:error, dgettext("errors", "Can't like object")} diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs index 6410df49b..53176917e 100644 --- a/test/web/activity_pub/activity_pub_test.exs +++ b/test/web/activity_pub/activity_pub_test.exs @@ -994,72 +994,6 @@ test "reverts emoji unreact on error" do end end - describe "like an object" do - test_with_mock "sends an activity to federation", Federator, [:passthrough], [] do - Config.put([:instance, :federating], true) - note_activity = insert(:note_activity) - assert object_activity = Object.normalize(note_activity) - - user = insert(:user) - - {:ok, like_activity, _object} = ActivityPub.like(user, object_activity) - assert called(Federator.publish(like_activity)) - end - - test "returns exist activity if object already liked" do - note_activity = insert(:note_activity) - assert object_activity = Object.normalize(note_activity) - - user = insert(:user) - - {:ok, like_activity, _object} = ActivityPub.like(user, object_activity) - - {:ok, like_activity_exist, _object} = ActivityPub.like(user, object_activity) - assert like_activity == like_activity_exist - end - - test "reverts like activity on error" do - note_activity = insert(:note_activity) - object = Object.normalize(note_activity) - user = insert(:user) - - with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do - assert {:error, :reverted} = ActivityPub.like(user, object) - end - - assert Repo.aggregate(Activity, :count, :id) == 1 - assert Repo.get(Object, object.id) == object - end - - test "adds a like activity to the db" do - note_activity = insert(:note_activity) - assert object = Object.normalize(note_activity) - - user = insert(:user) - user_two = insert(:user) - - {:ok, like_activity, object} = ActivityPub.like(user, object) - - assert like_activity.data["actor"] == user.ap_id - assert like_activity.data["type"] == "Like" - assert like_activity.data["object"] == object.data["id"] - assert like_activity.data["to"] == [User.ap_followers(user), note_activity.data["actor"]] - assert like_activity.data["context"] == object.data["context"] - assert object.data["like_count"] == 1 - assert object.data["likes"] == [user.ap_id] - - # Just return the original activity if the user already liked it. - {:ok, same_like_activity, object} = ActivityPub.like(user, object) - - assert like_activity == same_like_activity - assert object.data["likes"] == [user.ap_id] - assert object.data["like_count"] == 1 - - {:ok, _like_activity, object} = ActivityPub.like(user_two, object) - assert object.data["like_count"] == 2 - end - end - describe "unliking" do test_with_mock "sends an activity to federation", Federator, [:passthrough], [] do Config.put([:instance, :federating], true) @@ -1071,7 +1005,8 @@ test "adds a like activity to the db" do {:ok, object} = ActivityPub.unlike(user, object) refute called(Federator.publish()) - {:ok, _like_activity, object} = ActivityPub.like(user, object) + {:ok, _like_activity} = CommonAPI.favorite(user, note_activity.id) + object = Object.get_by_id(object.id) assert object.data["like_count"] == 1 {:ok, unlike_activity, _, object} = ActivityPub.unlike(user, object) @@ -1082,10 +1017,10 @@ test "adds a like activity to the db" do test "reverts unliking on error" do note_activity = insert(:note_activity) - object = Object.normalize(note_activity) user = insert(:user) - {:ok, like_activity, object} = ActivityPub.like(user, object) + {:ok, like_activity} = CommonAPI.favorite(user, note_activity.id) + object = Object.normalize(note_activity) assert object.data["like_count"] == 1 with_mock(Utils, [:passthrough], maybe_federate: fn _ -> {:error, :reverted} end) do @@ -1106,7 +1041,9 @@ test "unliking a previously liked object" do {:ok, object} = ActivityPub.unlike(user, object) assert object.data["like_count"] == 0 - {:ok, like_activity, object} = ActivityPub.like(user, object) + {:ok, like_activity} = CommonAPI.favorite(user, note_activity.id) + + object = Object.get_by_id(object.id) assert object.data["like_count"] == 1 {:ok, unlike_activity, _, object} = ActivityPub.unlike(user, object) diff --git a/test/web/activity_pub/utils_test.exs b/test/web/activity_pub/utils_test.exs index e913a5148..b0bfed917 100644 --- a/test/web/activity_pub/utils_test.exs +++ b/test/web/activity_pub/utils_test.exs @@ -224,8 +224,7 @@ test "fetches only Create activities" do object = Object.normalize(activity) {:ok, [vote], object} = CommonAPI.vote(other_user, object, [0]) - vote_object = Object.normalize(vote) - {:ok, _activity, _object} = ActivityPub.like(user, vote_object) + {:ok, _activity} = CommonAPI.favorite(user, activity.id) [fetched_vote] = Utils.get_existing_votes(other_user.ap_id, object) assert fetched_vote.id == vote.id end @@ -346,7 +345,7 @@ test "fetches existing like" do user = insert(:user) refute Utils.get_existing_like(user.ap_id, object) - {:ok, like_activity, _object} = ActivityPub.like(user, object) + {:ok, like_activity} = CommonAPI.favorite(user, note_activity.id) assert ^like_activity = Utils.get_existing_like(user.ap_id, object) end From 1df6af2a4c93257f94e2780e4317cfcf9bef7adb Mon Sep 17 00:00:00 2001 From: lain Date: Fri, 24 Apr 2020 13:59:48 +0200 Subject: [PATCH 25/25] Credo fixes. --- lib/pleroma/web/activity_pub/activity_pub_controller.ex | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 325a714b4..d625530ec 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -13,9 +13,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.Builder - alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.InternalFetchActor alias Pleroma.Web.ActivityPub.ObjectView + alias Pleroma.Web.ActivityPub.Pipeline alias Pleroma.Web.ActivityPub.Relay alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.ActivityPub.UserView