From 0cf1d4fcd0c15594f663101061670a4555132840 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Sat, 22 Feb 2020 19:48:41 +0300 Subject: [PATCH 01/14] [#1560] Restricted AP- & OStatus-related routes for non-federating instances. --- lib/pleroma/plugs/static_fe_plug.ex | 5 +- .../activity_pub/activity_pub_controller.ex | 2 +- lib/pleroma/web/ostatus/ostatus_controller.ex | 2 + .../controllers/remote_follow_controller.ex | 2 + .../controllers/util_controller.ex | 2 + test/web/activity_pub/publisher_test.exs | 4 + test/web/feed/user_controller_test.exs | 145 +++++++++++------- .../static_fe/static_fe_controller_test.exs | 119 ++++---------- .../remote_follow_controller_test.exs | 6 + test/web/twitter_api/util_controller_test.exs | 37 +++-- 10 files changed, 166 insertions(+), 158 deletions(-) diff --git a/lib/pleroma/plugs/static_fe_plug.ex b/lib/pleroma/plugs/static_fe_plug.ex index b3fb3c582..7d69e661c 100644 --- a/lib/pleroma/plugs/static_fe_plug.ex +++ b/lib/pleroma/plugs/static_fe_plug.ex @@ -21,6 +21,9 @@ def call(conn, _) do defp enabled?, do: Pleroma.Config.get([:static_fe, :enabled], false) defp accepts_html?(conn) do - conn |> get_req_header("accept") |> List.first() |> String.contains?("text/html") + conn + |> get_req_header("accept") + |> List.first() + |> String.contains?("text/html") end end diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 5059e3984..aee574262 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -30,7 +30,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do when action in [:activity, :object] ) - plug(Pleroma.Web.FederatingPlug when action in [:inbox, :relay]) + plug(Pleroma.Web.FederatingPlug) plug(:set_requester_reachable when action in [:inbox]) plug(:relay_active? when action in [:relay]) diff --git a/lib/pleroma/web/ostatus/ostatus_controller.ex b/lib/pleroma/web/ostatus/ostatus_controller.ex index 01ec7941e..630cd0006 100644 --- a/lib/pleroma/web/ostatus/ostatus_controller.ex +++ b/lib/pleroma/web/ostatus/ostatus_controller.ex @@ -16,6 +16,8 @@ defmodule Pleroma.Web.OStatus.OStatusController do alias Pleroma.Web.Metadata.PlayerView alias Pleroma.Web.Router + plug(Pleroma.Web.FederatingPlug) + plug( RateLimiter, [name: :ap_routes, params: ["uuid"]] when action in [:object, :activity] diff --git a/lib/pleroma/web/twitter_api/controllers/remote_follow_controller.ex b/lib/pleroma/web/twitter_api/controllers/remote_follow_controller.ex index fbf31c7eb..89da760da 100644 --- a/lib/pleroma/web/twitter_api/controllers/remote_follow_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/remote_follow_controller.ex @@ -16,6 +16,8 @@ defmodule Pleroma.Web.TwitterAPI.RemoteFollowController do @status_types ["Article", "Event", "Note", "Video", "Page", "Question"] + plug(Pleroma.Web.FederatingPlug) + # Note: follower can submit the form (with password auth) not being signed in (having no token) plug( OAuthScopesPlug, diff --git a/lib/pleroma/web/twitter_api/controllers/util_controller.ex b/lib/pleroma/web/twitter_api/controllers/util_controller.ex index f08b9d28c..0a77978e3 100644 --- a/lib/pleroma/web/twitter_api/controllers/util_controller.ex +++ b/lib/pleroma/web/twitter_api/controllers/util_controller.ex @@ -17,6 +17,8 @@ defmodule Pleroma.Web.TwitterAPI.UtilController do alias Pleroma.Web.CommonAPI alias Pleroma.Web.WebFinger + plug(Pleroma.Web.FederatingPlug when action == :remote_subscribe) + plug( OAuthScopesPlug, %{scopes: ["follow", "write:follows"]} diff --git a/test/web/activity_pub/publisher_test.exs b/test/web/activity_pub/publisher_test.exs index 015af19ab..c8eed68b6 100644 --- a/test/web/activity_pub/publisher_test.exs +++ b/test/web/activity_pub/publisher_test.exs @@ -23,6 +23,10 @@ defmodule Pleroma.Web.ActivityPub.PublisherTest do :ok end + clear_config_all([:instance, :federating]) do + Pleroma.Config.put([:instance, :federating], true) + end + describe "gather_webfinger_links/1" do test "it returns links" do user = insert(:user) diff --git a/test/web/feed/user_controller_test.exs b/test/web/feed/user_controller_test.exs index 41cc9e07e..fceb2ed43 100644 --- a/test/web/feed/user_controller_test.exs +++ b/test/web/feed/user_controller_test.exs @@ -8,66 +8,78 @@ defmodule Pleroma.Web.Feed.UserControllerTest do import Pleroma.Factory import SweetXml + alias Pleroma.Config alias Pleroma.Object alias Pleroma.User - clear_config([:feed]) - - test "gets a feed", %{conn: conn} do - Pleroma.Config.put( - [:feed, :post_title], - %{max_length: 10, omission: "..."} - ) - - activity = insert(:note_activity) - - note = - insert(:note, - data: %{ - "content" => "This is :moominmamma: note ", - "attachment" => [ - %{ - "url" => [%{"mediaType" => "image/png", "href" => "https://pleroma.gov/image.png"}] - } - ], - "inReplyTo" => activity.data["id"] - } - ) - - note_activity = insert(:note_activity, note: note) - user = User.get_cached_by_ap_id(note_activity.data["actor"]) - - note2 = - insert(:note, - user: user, - data: %{"content" => "42 This is :moominmamma: note ", "inReplyTo" => activity.data["id"]} - ) - - _note_activity2 = insert(:note_activity, note: note2) - object = Object.normalize(note_activity) - - resp = - conn - |> put_req_header("content-type", "application/atom+xml") - |> get(user_feed_path(conn, :feed, user.nickname)) - |> response(200) - - activity_titles = - resp - |> SweetXml.parse() - |> SweetXml.xpath(~x"//entry/title/text()"l) - - assert activity_titles == ['42 This...', 'This is...'] - assert resp =~ object.data["content"] + clear_config_all([:instance, :federating]) do + Config.put([:instance, :federating], true) end - test "returns 404 for a missing feed", %{conn: conn} do - conn = - conn - |> put_req_header("content-type", "application/atom+xml") - |> get(user_feed_path(conn, :feed, "nonexisting")) + describe "feed" do + clear_config([:feed]) - assert response(conn, 404) + test "gets a feed", %{conn: conn} do + Config.put( + [:feed, :post_title], + %{max_length: 10, omission: "..."} + ) + + activity = insert(:note_activity) + + note = + insert(:note, + data: %{ + "content" => "This is :moominmamma: note ", + "attachment" => [ + %{ + "url" => [ + %{"mediaType" => "image/png", "href" => "https://pleroma.gov/image.png"} + ] + } + ], + "inReplyTo" => activity.data["id"] + } + ) + + note_activity = insert(:note_activity, note: note) + user = User.get_cached_by_ap_id(note_activity.data["actor"]) + + note2 = + insert(:note, + user: user, + data: %{ + "content" => "42 This is :moominmamma: note ", + "inReplyTo" => activity.data["id"] + } + ) + + _note_activity2 = insert(:note_activity, note: note2) + object = Object.normalize(note_activity) + + resp = + conn + |> put_req_header("content-type", "application/atom+xml") + |> get(user_feed_path(conn, :feed, user.nickname)) + |> response(200) + + activity_titles = + resp + |> SweetXml.parse() + |> SweetXml.xpath(~x"//entry/title/text()"l) + + assert activity_titles == ['42 This...', 'This is...'] + assert resp =~ object.data["content"] + end + + test "returns 404 for a missing feed", %{conn: conn} do + conn = + conn + |> put_req_header("content-type", "application/atom+xml") + |> get(user_feed_path(conn, :feed, "nonexisting")) + + assert response(conn, 404) + end end describe "feed_redirect" do @@ -248,4 +260,29 @@ test "html format. it returns error when user not found", %{conn: conn} do assert response == %{"error" => "Not found"} end end + + describe "feed_redirect (depending on federation enabled state)" do + setup %{conn: conn} do + user = insert(:user) + conn = put_req_header(conn, "accept", "application/json") + + %{conn: conn, user: user} + end + + clear_config([:instance, :federating]) + + test "renders if instance is federating", %{conn: conn, user: user} do + Config.put([:instance, :federating], true) + + conn = get(conn, "/users/#{user.nickname}") + assert json_response(conn, 200) + end + + test "renders 404 if instance is NOT federating", %{conn: conn, user: user} do + Config.put([:instance, :federating], false) + + conn = get(conn, "/users/#{user.nickname}") + assert json_response(conn, 404) + end + end end diff --git a/test/web/static_fe/static_fe_controller_test.exs b/test/web/static_fe/static_fe_controller_test.exs index 2ce8f9fa3..11facab99 100644 --- a/test/web/static_fe/static_fe_controller_test.exs +++ b/test/web/static_fe/static_fe_controller_test.exs @@ -1,56 +1,42 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do use Pleroma.Web.ConnCase + alias Pleroma.Activity + alias Pleroma.Config alias Pleroma.Web.ActivityPub.Transmogrifier alias Pleroma.Web.CommonAPI import Pleroma.Factory clear_config_all([:static_fe, :enabled]) do - Pleroma.Config.put([:static_fe, :enabled], true) + Config.put([:static_fe, :enabled], true) end - describe "user profile page" do - test "just the profile as HTML", %{conn: conn} do - user = insert(:user) + setup %{conn: conn} do + conn = put_req_header(conn, "accept", "text/html") + user = insert(:user) - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/users/#{user.nickname}") + %{conn: conn, user: user} + end + + describe "user profile html" do + test "just the profile as HTML", %{conn: conn, user: user} do + conn = get(conn, "/users/#{user.nickname}") assert html_response(conn, 200) =~ user.nickname end - test "renders json unless there's an html accept header", %{conn: conn} do - user = insert(:user) - - conn = - conn - |> put_req_header("accept", "application/json") - |> get("/users/#{user.nickname}") - - assert json_response(conn, 200) - end - test "404 when user not found", %{conn: conn} do - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/users/limpopo") + conn = get(conn, "/users/limpopo") assert html_response(conn, 404) =~ "not found" end - test "profile does not include private messages", %{conn: conn} do - user = insert(:user) + test "profile does not include private messages", %{conn: conn, user: user} do CommonAPI.post(user, %{"status" => "public"}) CommonAPI.post(user, %{"status" => "private", "visibility" => "private"}) - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/users/#{user.nickname}") + conn = get(conn, "/users/#{user.nickname}") html = html_response(conn, 200) @@ -58,14 +44,10 @@ test "profile does not include private messages", %{conn: conn} do refute html =~ ">private<" end - test "pagination", %{conn: conn} do - user = insert(:user) + test "pagination", %{conn: conn, user: user} do Enum.map(1..30, fn i -> CommonAPI.post(user, %{"status" => "test#{i}"}) end) - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/users/#{user.nickname}") + conn = get(conn, "/users/#{user.nickname}") html = html_response(conn, 200) @@ -75,15 +57,11 @@ test "pagination", %{conn: conn} do refute html =~ ">test1<" end - test "pagination, page 2", %{conn: conn} do - user = insert(:user) + test "pagination, page 2", %{conn: conn, user: user} do activities = Enum.map(1..30, fn i -> CommonAPI.post(user, %{"status" => "test#{i}"}) end) {:ok, a11} = Enum.at(activities, 11) - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/users/#{user.nickname}?max_id=#{a11.id}") + conn = get(conn, "/users/#{user.nickname}?max_id=#{a11.id}") html = html_response(conn, 200) @@ -94,15 +72,11 @@ test "pagination, page 2", %{conn: conn} do end end - describe "notice rendering" do - test "single notice page", %{conn: conn} do - user = insert(:user) + describe "notice html" do + test "single notice page", %{conn: conn, user: user} do {:ok, activity} = CommonAPI.post(user, %{"status" => "testing a thing!"}) - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/notice/#{activity.id}") + conn = get(conn, "/notice/#{activity.id}") html = html_response(conn, 200) assert html =~ "
" @@ -110,8 +84,7 @@ test "single notice page", %{conn: conn} do assert html =~ "testing a thing!" end - test "shows the whole thread", %{conn: conn} do - user = insert(:user) + test "shows the whole thread", %{conn: conn, user: user} do {:ok, activity} = CommonAPI.post(user, %{"status" => "space: the final frontier"}) CommonAPI.post(user, %{ @@ -119,70 +92,47 @@ test "shows the whole thread", %{conn: conn} do "in_reply_to_status_id" => activity.id }) - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/notice/#{activity.id}") + conn = get(conn, "/notice/#{activity.id}") html = html_response(conn, 200) assert html =~ "the final frontier" assert html =~ "voyages" end - test "redirect by AP object ID", %{conn: conn} do - user = insert(:user) - + test "redirect by AP object ID", %{conn: conn, user: user} do {:ok, %Activity{data: %{"object" => object_url}}} = CommonAPI.post(user, %{"status" => "beam me up"}) - conn = - conn - |> put_req_header("accept", "text/html") - |> get(URI.parse(object_url).path) + conn = get(conn, URI.parse(object_url).path) assert html_response(conn, 302) =~ "redirected" end - test "redirect by activity ID", %{conn: conn} do - user = insert(:user) - + test "redirect by activity ID", %{conn: conn, user: user} do {:ok, %Activity{data: %{"id" => id}}} = CommonAPI.post(user, %{"status" => "I'm a doctor, not a devops!"}) - conn = - conn - |> put_req_header("accept", "text/html") - |> get(URI.parse(id).path) + conn = get(conn, URI.parse(id).path) assert html_response(conn, 302) =~ "redirected" end test "404 when notice not found", %{conn: conn} do - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/notice/88c9c317") + conn = get(conn, "/notice/88c9c317") assert html_response(conn, 404) =~ "not found" end - test "404 for private status", %{conn: conn} do - user = insert(:user) - + test "404 for private status", %{conn: conn, user: user} do {:ok, activity} = CommonAPI.post(user, %{"status" => "don't show me!", "visibility" => "private"}) - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/notice/#{activity.id}") + conn = get(conn, "/notice/#{activity.id}") assert html_response(conn, 404) =~ "not found" end - test "302 for remote cached status", %{conn: conn} do - user = insert(:user) - + test "302 for remote cached status", %{conn: conn, user: user} do message = %{ "@context" => "https://www.w3.org/ns/activitystreams", "to" => user.follower_address, @@ -199,10 +149,7 @@ test "302 for remote cached status", %{conn: conn} do assert {:ok, activity} = Transmogrifier.handle_incoming(message) - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/notice/#{activity.id}") + conn = get(conn, "/notice/#{activity.id}") assert html_response(conn, 302) =~ "redirected" end diff --git a/test/web/twitter_api/remote_follow_controller_test.exs b/test/web/twitter_api/remote_follow_controller_test.exs index 80a42989d..73062f18f 100644 --- a/test/web/twitter_api/remote_follow_controller_test.exs +++ b/test/web/twitter_api/remote_follow_controller_test.exs @@ -5,8 +5,10 @@ defmodule Pleroma.Web.TwitterAPI.RemoteFollowControllerTest do use Pleroma.Web.ConnCase + alias Pleroma.Config alias Pleroma.User alias Pleroma.Web.CommonAPI + import ExUnit.CaptureLog import Pleroma.Factory @@ -15,6 +17,10 @@ defmodule Pleroma.Web.TwitterAPI.RemoteFollowControllerTest do :ok end + clear_config_all([:instance, :federating]) do + Config.put([:instance, :federating], true) + end + clear_config([:instance]) clear_config([:frontend_configurations, :pleroma_fe]) clear_config([:user, :deny_follow_blocked]) diff --git a/test/web/twitter_api/util_controller_test.exs b/test/web/twitter_api/util_controller_test.exs index 56633ffce..992cc44a5 100644 --- a/test/web/twitter_api/util_controller_test.exs +++ b/test/web/twitter_api/util_controller_test.exs @@ -6,6 +6,7 @@ defmodule Pleroma.Web.TwitterAPI.UtilControllerTest do use Pleroma.Web.ConnCase use Oban.Testing, repo: Pleroma.Repo + alias Pleroma.Config alias Pleroma.Tests.ObanHelpers alias Pleroma.User @@ -178,7 +179,7 @@ test "it updates notification privacy option", %{user: user, conn: conn} do describe "GET /api/statusnet/config" do test "it returns config in xml format", %{conn: conn} do - instance = Pleroma.Config.get(:instance) + instance = Config.get(:instance) response = conn @@ -195,12 +196,12 @@ test "it returns config in xml format", %{conn: conn} do end test "it returns config in json format", %{conn: conn} do - instance = Pleroma.Config.get(:instance) - Pleroma.Config.put([:instance, :managed_config], true) - Pleroma.Config.put([:instance, :registrations_open], false) - Pleroma.Config.put([:instance, :invites_enabled], true) - Pleroma.Config.put([:instance, :public], false) - Pleroma.Config.put([:frontend_configurations, :pleroma_fe], %{theme: "asuka-hospital"}) + instance = Config.get(:instance) + Config.put([:instance, :managed_config], true) + Config.put([:instance, :registrations_open], false) + Config.put([:instance, :invites_enabled], true) + Config.put([:instance, :public], false) + Config.put([:frontend_configurations, :pleroma_fe], %{theme: "asuka-hospital"}) response = conn @@ -234,7 +235,7 @@ test "it returns config in json format", %{conn: conn} do end test "returns the state of safe_dm_mentions flag", %{conn: conn} do - Pleroma.Config.put([:instance, :safe_dm_mentions], true) + Config.put([:instance, :safe_dm_mentions], true) response = conn @@ -243,7 +244,7 @@ test "returns the state of safe_dm_mentions flag", %{conn: conn} do assert response["site"]["safeDMMentionsEnabled"] == "1" - Pleroma.Config.put([:instance, :safe_dm_mentions], false) + Config.put([:instance, :safe_dm_mentions], false) response = conn @@ -254,8 +255,8 @@ test "returns the state of safe_dm_mentions flag", %{conn: conn} do end test "it returns the managed config", %{conn: conn} do - Pleroma.Config.put([:instance, :managed_config], false) - Pleroma.Config.put([:frontend_configurations, :pleroma_fe], %{theme: "asuka-hospital"}) + Config.put([:instance, :managed_config], false) + Config.put([:frontend_configurations, :pleroma_fe], %{theme: "asuka-hospital"}) response = conn @@ -264,7 +265,7 @@ test "it returns the managed config", %{conn: conn} do refute response["site"]["pleromafe"] - Pleroma.Config.put([:instance, :managed_config], true) + Config.put([:instance, :managed_config], true) response = conn @@ -287,7 +288,7 @@ test "returns everything in :pleroma, :frontend_configurations", %{conn: conn} d } ] - Pleroma.Config.put(:frontend_configurations, config) + Config.put(:frontend_configurations, config) response = conn @@ -320,7 +321,7 @@ test "returns json with custom emoji with tags", %{conn: conn} do clear_config([:instance, :healthcheck]) test "returns 503 when healthcheck disabled", %{conn: conn} do - Pleroma.Config.put([:instance, :healthcheck], false) + Config.put([:instance, :healthcheck], false) response = conn @@ -331,7 +332,7 @@ test "returns 503 when healthcheck disabled", %{conn: conn} do end test "returns 200 when healthcheck enabled and all ok", %{conn: conn} do - Pleroma.Config.put([:instance, :healthcheck], true) + Config.put([:instance, :healthcheck], true) with_mock Pleroma.Healthcheck, system_info: fn -> %Pleroma.Healthcheck{healthy: true} end do @@ -351,7 +352,7 @@ test "returns 200 when healthcheck enabled and all ok", %{conn: conn} do end test "returns 503 when healthcheck enabled and health is false", %{conn: conn} do - Pleroma.Config.put([:instance, :healthcheck], true) + Config.put([:instance, :healthcheck], true) with_mock Pleroma.Healthcheck, system_info: fn -> %Pleroma.Healthcheck{healthy: false} end do @@ -426,6 +427,10 @@ test "it returns version in json format", %{conn: conn} do end describe "POST /main/ostatus - remote_subscribe/2" do + clear_config([:instance, :federating]) do + Config.put([:instance, :federating], true) + end + test "renders subscribe form", %{conn: conn} do user = insert(:user) From b4367125e9afc92ac27ff12552775f8e765140f1 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Mon, 2 Mar 2020 21:43:18 +0300 Subject: [PATCH 02/14] [#1560] Added tests for non-federating instance bahaviour to ActivityPubControllerTest. --- docs/API/differences_in_mastoapi_responses.md | 2 +- docs/clients.md | 2 +- test/plugs/oauth_plug_test.exs | 2 +- .../activity_pub_controller_test.exs | 91 ++++++++++++++++++- 4 files changed, 90 insertions(+), 7 deletions(-) diff --git a/docs/API/differences_in_mastoapi_responses.md b/docs/API/differences_in_mastoapi_responses.md index 06de90f71..476a4a2bf 100644 --- a/docs/API/differences_in_mastoapi_responses.md +++ b/docs/API/differences_in_mastoapi_responses.md @@ -180,7 +180,7 @@ Post here request with grant_type=refresh_token to obtain new access token. Retu ## Account Registration `POST /api/v1/accounts` -Has theses additionnal parameters (which are the same as in Pleroma-API): +Has theses additional parameters (which are the same as in Pleroma-API): * `fullname`: optional * `bio`: optional * `captcha_solution`: optional, contains provider-specific captcha solution, diff --git a/docs/clients.md b/docs/clients.md index 8ac9ad3de..1eae0f0c6 100644 --- a/docs/clients.md +++ b/docs/clients.md @@ -1,5 +1,5 @@ # Pleroma Clients -Note: Additionnal clients may be working but theses are officially supporting Pleroma. +Note: Additional clients may be working but theses are officially supporting Pleroma. Feel free to contact us to be added to this list! ## Desktop diff --git a/test/plugs/oauth_plug_test.exs b/test/plugs/oauth_plug_test.exs index dea11cdb0..0eef27c1f 100644 --- a/test/plugs/oauth_plug_test.exs +++ b/test/plugs/oauth_plug_test.exs @@ -38,7 +38,7 @@ test "with valid token(downcase), it assigns the user", %{conn: conn} = opts do assert conn.assigns[:user] == opts[:user] end - test "with valid token(downcase) in url parameters, it assings the user", opts do + test "with valid token(downcase) in url parameters, it assigns the user", opts do conn = :get |> build_conn("/?access_token=#{opts[:token]}") diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs index ba2ce1dd9..af0417406 100644 --- a/test/web/activity_pub/activity_pub_controller_test.exs +++ b/test/web/activity_pub/activity_pub_controller_test.exs @@ -25,9 +25,9 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do :ok end - clear_config_all([:instance, :federating], - do: Pleroma.Config.put([:instance, :federating], true) - ) + clear_config_all([:instance, :federating]) do + Pleroma.Config.put([:instance, :federating], true) + end describe "/relay" do clear_config([:instance, :allow_relay]) @@ -1008,7 +1008,7 @@ test "it tracks a signed activity fetch when the json is cached", %{conn: conn} end end - describe "Additionnal ActivityPub C2S endpoints" do + describe "Additional ActivityPub C2S endpoints" do test "/api/ap/whoami", %{conn: conn} do user = insert(:user) @@ -1047,4 +1047,87 @@ test "uploadMedia", %{conn: conn} do assert object["actor"] == user.ap_id end end + + describe "when instance is not federating," do + clear_config([:instance, :federating]) do + Pleroma.Config.put([:instance, :federating], false) + end + + test "returns 404 for GET routes", %{conn: conn} do + user = insert(:user) + conn = put_req_header(conn, "accept", "application/json") + + get_uris = [ + "/users/#{user.nickname}", + "/users/#{user.nickname}/outbox", + "/users/#{user.nickname}/inbox?page=true", + "/users/#{user.nickname}/followers", + "/users/#{user.nickname}/following", + "/internal/fetch", + "/relay", + "/relay/following", + "/relay/followers", + "/api/ap/whoami" + ] + + for get_uri <- get_uris do + conn + |> get(get_uri) + |> json_response(404) + + conn + |> assign(:user, user) + |> get(get_uri) + |> json_response(404) + end + end + + test "returns 404 for activity-related POST routes", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:valid_signature, true) + |> put_req_header("content-type", "application/activity+json") + + post_activity_data = + "test/fixtures/mastodon-post-activity.json" + |> File.read!() + |> Poison.decode!() + + post_activity_uris = [ + "/inbox", + "/relay/inbox", + "/users/#{user.nickname}/inbox", + "/users/#{user.nickname}/outbox" + ] + + for post_activity_uri <- post_activity_uris do + conn + |> post(post_activity_uri, post_activity_data) + |> json_response(404) + + conn + |> assign(:user, user) + |> post(post_activity_uri, post_activity_data) + |> json_response(404) + end + end + + test "returns 404 for media upload attempt", %{conn: conn} do + user = insert(:user) + desc = "Description of the image" + + image = %Plug.Upload{ + content_type: "image/jpg", + path: Path.absname("test/fixtures/image.jpg"), + filename: "an_image.jpg" + } + + conn + |> assign(:user, user) + |> post("/api/ap/upload_media", %{"file" => image, "description" => desc}) + |> json_response(404) + end + end end From bd8624d649643c5a14bb24d8b2f2aed0454fb50d Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Mon, 2 Mar 2020 22:02:21 +0300 Subject: [PATCH 03/14] [#1560] Added tests for non-federating instance bahaviour to OStatusControllerTest. --- test/web/ostatus/ostatus_controller_test.exs | 29 ++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/test/web/ostatus/ostatus_controller_test.exs b/test/web/ostatus/ostatus_controller_test.exs index 50235dfef..2b7bc662d 100644 --- a/test/web/ostatus/ostatus_controller_test.exs +++ b/test/web/ostatus/ostatus_controller_test.exs @@ -277,4 +277,33 @@ test "404s when attachment isn't audio or video", %{conn: conn} do |> response(404) end end + + describe "when instance is not federating," do + clear_config([:instance, :federating]) do + Pleroma.Config.put([:instance, :federating], false) + end + + test "returns 404 for GET routes", %{conn: conn} do + conn = put_req_header(conn, "accept", "application/json") + + note_activity = insert(:note_activity, local: true) + [_, activity_uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["id"])) + + object = Object.normalize(note_activity) + [_, object_uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, object.data["id"])) + + get_uris = [ + "/activities/#{activity_uuid}", + "/objects/#{object_uuid}", + "/notice/#{note_activity.id}", + "/notice/#{note_activity.id}/embed_player" + ] + + for get_uri <- get_uris do + conn + |> get(get_uri) + |> json_response(404) + end + end + end end From b6fc98d9cd3a32b39606c65cb4f298d280e2537c Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Tue, 3 Mar 2020 22:22:02 +0300 Subject: [PATCH 04/14] [#1560] ActivityPubController federation state restrictions adjustments. Adjusted tests. --- lib/pleroma/plugs/federating_plug.ex | 4 +- .../activity_pub/activity_pub_controller.ex | 44 ++++++++++++++----- lib/pleroma/web/router.ex | 3 ++ .../activity_pub_controller_test.exs | 29 ++---------- 4 files changed, 41 insertions(+), 39 deletions(-) diff --git a/lib/pleroma/plugs/federating_plug.ex b/lib/pleroma/plugs/federating_plug.ex index 4dc4e9279..4c5aca3e9 100644 --- a/lib/pleroma/plugs/federating_plug.ex +++ b/lib/pleroma/plugs/federating_plug.ex @@ -10,7 +10,7 @@ def init(options) do end def call(conn, _opts) do - if Pleroma.Config.get([:instance, :federating]) do + if federating?() do conn else conn @@ -20,4 +20,6 @@ def call(conn, _opts) do |> halt() end end + + def federating?, do: Pleroma.Config.get([:instance, :federating]) end diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index aee574262..e1984f88f 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -18,19 +18,31 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do alias Pleroma.Web.ActivityPub.UserView alias Pleroma.Web.ActivityPub.Utils alias Pleroma.Web.ActivityPub.Visibility + alias Pleroma.Web.FederatingPlug alias Pleroma.Web.Federator require Logger action_fallback(:errors) + # Note: some of the following actions (like :update_inbox) may be server-to-server as well + @client_to_server_actions [ + :whoami, + :read_inbox, + :update_outbox, + :upload_media, + :followers, + :following + ] + + plug(FederatingPlug when action not in @client_to_server_actions) + plug( Pleroma.Plugs.Cache, [query_params: false, tracking_fun: &__MODULE__.track_object_fetch/2] when action in [:activity, :object] ) - plug(Pleroma.Web.FederatingPlug) plug(:set_requester_reachable when action in [:inbox]) plug(:relay_active? when action in [:relay]) @@ -255,8 +267,16 @@ def inbox(%{assigns: %{valid_signature: true}} = conn, params) do json(conn, "ok") end - # only accept relayed Creates - def inbox(conn, %{"type" => "Create"} = params) do + # POST /relay/inbox -or- POST /internal/fetch/inbox + def inbox(conn, params) do + if params["type"] == "Create" && FederatingPlug.federating?() do + post_inbox_relayed_create(conn, params) + else + post_inbox_fallback(conn, params) + end + end + + defp post_inbox_relayed_create(conn, params) do Logger.debug( "Signature missing or not from author, relayed Create message, fetching object from source" ) @@ -266,7 +286,7 @@ def inbox(conn, %{"type" => "Create"} = params) do json(conn, "ok") end - def inbox(conn, params) do + defp post_inbox_fallback(conn, params) do headers = Enum.into(conn.req_headers, %{}) if String.contains?(headers["signature"], params["actor"]) do @@ -314,7 +334,7 @@ def whoami(%{assigns: %{user: %User{} = user}} = conn, _params) do def whoami(_conn, _params), do: {:error, :not_found} def read_inbox( - %{assigns: %{user: %{nickname: nickname} = user}} = conn, + %{assigns: %{user: %User{nickname: nickname} = user}} = conn, %{"nickname" => nickname, "page" => page?} = params ) when page? in [true, "true"] do @@ -337,7 +357,7 @@ def read_inbox( }) end - def read_inbox(%{assigns: %{user: %{nickname: nickname} = user}} = conn, %{ + def read_inbox(%{assigns: %{user: %User{nickname: nickname} = user}} = conn, %{ "nickname" => nickname }) do with {:ok, user} <- User.ensure_keys_present(user) do @@ -356,7 +376,7 @@ def read_inbox(%{assigns: %{user: nil}} = conn, %{"nickname" => nickname}) do |> json(err) end - def read_inbox(%{assigns: %{user: %{nickname: as_nickname}}} = conn, %{ + def read_inbox(%{assigns: %{user: %User{nickname: as_nickname}}} = conn, %{ "nickname" => nickname }) do err = @@ -370,7 +390,7 @@ def read_inbox(%{assigns: %{user: %{nickname: as_nickname}}} = conn, %{ |> json(err) end - def handle_user_activity(user, %{"type" => "Create"} = params) do + def handle_user_activity(%User{} = user, %{"type" => "Create"} = params) do object = params["object"] |> Map.merge(Map.take(params, ["to", "cc"])) @@ -386,7 +406,7 @@ def handle_user_activity(user, %{"type" => "Create"} = params) do }) end - def handle_user_activity(user, %{"type" => "Delete"} = params) do + def handle_user_activity(%User{} = user, %{"type" => "Delete"} = params) do with %Object{} = object <- Object.normalize(params["object"]), true <- user.is_moderator || user.ap_id == object.data["actor"], {:ok, delete} <- ActivityPub.delete(object) do @@ -396,7 +416,7 @@ def handle_user_activity(user, %{"type" => "Delete"} = params) do end end - def handle_user_activity(user, %{"type" => "Like"} = params) do + def 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, activity} @@ -434,7 +454,7 @@ def update_outbox( end end - def update_outbox(%{assigns: %{user: user}} = conn, %{"nickname" => nickname} = _) do + def update_outbox(%{assigns: %{user: %User{} = user}} = conn, %{"nickname" => nickname}) do err = dgettext("errors", "can't update outbox of %{nickname} as %{as_nickname}", nickname: nickname, @@ -492,7 +512,7 @@ defp ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user) do - HTTP Code: 201 Created - HTTP Body: ActivityPub object to be inserted into another's `attachment` field """ - def upload_media(%{assigns: %{user: user}} = conn, %{"file" => file} = data) do + def upload_media(%{assigns: %{user: %User{} = user}} = conn, %{"file" => file} = data) do with {:ok, object} <- ActivityPub.upload( file, diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 980242c68..5f3a06caa 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -541,6 +541,7 @@ 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) @@ -554,6 +555,7 @@ defmodule Pleroma.Web.Router do get("/users/:nickname/outbox", ActivityPubController, :outbox) end + # Client to Server (C2S) AP interactions pipeline :activitypub_client do plug(:accepts, ["activity+json", "json"]) plug(:fetch_session) @@ -568,6 +570,7 @@ defmodule Pleroma.Web.Router do plug(Pleroma.Plugs.EnsureUserKeyPlug) end + # Note: propagate _any_ updates to `@client_to_server_actions` in `ActivityPubController` scope "/", Pleroma.Web.ActivityPub do pipe_through([:activitypub_client]) diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs index af0417406..b853474d4 100644 --- a/test/web/activity_pub/activity_pub_controller_test.exs +++ b/test/web/activity_pub/activity_pub_controller_test.exs @@ -775,7 +775,7 @@ test "it returns the followers in a collection", %{conn: conn} do assert result["first"]["orderedItems"] == [user.ap_id] end - test "it returns returns a uri if the user has 'hide_followers' set", %{conn: conn} do + test "it returns a uri if the user has 'hide_followers' set", %{conn: conn} do user = insert(:user) user_two = insert(:user, hide_followers: true) User.follow(user, user_two) @@ -1060,14 +1060,8 @@ test "returns 404 for GET routes", %{conn: conn} do get_uris = [ "/users/#{user.nickname}", "/users/#{user.nickname}/outbox", - "/users/#{user.nickname}/inbox?page=true", - "/users/#{user.nickname}/followers", - "/users/#{user.nickname}/following", "/internal/fetch", - "/relay", - "/relay/following", - "/relay/followers", - "/api/ap/whoami" + "/relay" ] for get_uri <- get_uris do @@ -1098,8 +1092,7 @@ test "returns 404 for activity-related POST routes", %{conn: conn} do post_activity_uris = [ "/inbox", "/relay/inbox", - "/users/#{user.nickname}/inbox", - "/users/#{user.nickname}/outbox" + "/users/#{user.nickname}/inbox" ] for post_activity_uri <- post_activity_uris do @@ -1113,21 +1106,5 @@ test "returns 404 for activity-related POST routes", %{conn: conn} do |> json_response(404) end end - - test "returns 404 for media upload attempt", %{conn: conn} do - user = insert(:user) - desc = "Description of the image" - - image = %Plug.Upload{ - content_type: "image/jpg", - path: Path.absname("test/fixtures/image.jpg"), - filename: "an_image.jpg" - } - - conn - |> assign(:user, user) - |> post("/api/ap/upload_media", %{"file" => image, "description" => desc}) - |> json_response(404) - end end end From 40765875d41f181b4ac54a772b4c61d6afc0bc34 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Thu, 5 Mar 2020 21:19:21 +0300 Subject: [PATCH 05/14] [#1560] Misc. improvements in ActivityPubController federation state restrictions. --- lib/pleroma/plugs/federating_plug.ex | 14 +++++++---- .../activity_pub/activity_pub_controller.ex | 25 +++++++++++++------ .../activity_pub_controller_test.exs | 9 ++++--- 3 files changed, 32 insertions(+), 16 deletions(-) diff --git a/lib/pleroma/plugs/federating_plug.ex b/lib/pleroma/plugs/federating_plug.ex index 4c5aca3e9..456c1bfb9 100644 --- a/lib/pleroma/plugs/federating_plug.ex +++ b/lib/pleroma/plugs/federating_plug.ex @@ -13,13 +13,17 @@ def call(conn, _opts) do if federating?() do conn else - conn - |> put_status(404) - |> Phoenix.Controller.put_view(Pleroma.Web.ErrorView) - |> Phoenix.Controller.render("404.json") - |> halt() + fail(conn) end end def federating?, do: Pleroma.Config.get([:instance, :federating]) + + def fail(conn) do + conn + |> put_status(404) + |> Phoenix.Controller.put_view(Pleroma.Web.ErrorView) + |> Phoenix.Controller.render("404.json") + |> halt() + end end diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index e1984f88f..9beaaf8c9 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -29,6 +29,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do @client_to_server_actions [ :whoami, :read_inbox, + :outbox, :update_outbox, :upload_media, :followers, @@ -140,10 +141,14 @@ defp set_cache_ttl_for(conn, entity) do # GET /relay/following def following(%{assigns: %{relay: true}} = conn, _params) do - conn - |> put_resp_content_type("application/activity+json") - |> put_view(UserView) - |> render("following.json", %{user: Relay.get_actor()}) + if FederatingPlug.federating?() do + conn + |> put_resp_content_type("application/activity+json") + |> put_view(UserView) + |> render("following.json", %{user: Relay.get_actor()}) + else + FederatingPlug.fail(conn) + end end def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "page" => page}) do @@ -177,10 +182,14 @@ def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) d # GET /relay/followers def followers(%{assigns: %{relay: true}} = conn, _params) do - conn - |> put_resp_content_type("application/activity+json") - |> put_view(UserView) - |> render("followers.json", %{user: Relay.get_actor()}) + if FederatingPlug.federating?() do + conn + |> put_resp_content_type("application/activity+json") + |> put_view(UserView) + |> render("followers.json", %{user: Relay.get_actor()}) + else + FederatingPlug.fail(conn) + end end def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "page" => page}) do diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs index b853474d4..9c922e991 100644 --- a/test/web/activity_pub/activity_pub_controller_test.exs +++ b/test/web/activity_pub/activity_pub_controller_test.exs @@ -577,7 +577,7 @@ test "it removes all follower collections but actor's", %{conn: conn} do end end - describe "/users/:nickname/outbox" do + describe "GET /users/:nickname/outbox" do test "it will not bomb when there is no activity", %{conn: conn} do user = insert(:user) @@ -614,7 +614,9 @@ test "it returns an announce activity in a collection", %{conn: conn} do assert response(conn, 200) =~ announce_activity.data["object"] end + end + describe "POST /users/:nickname/outbox" do test "it rejects posts from other users", %{conn: conn} do data = File.read!("test/fixtures/activitypub-client-post-activity.json") |> Poison.decode!() user = insert(:user) @@ -1059,9 +1061,10 @@ test "returns 404 for GET routes", %{conn: conn} do get_uris = [ "/users/#{user.nickname}", - "/users/#{user.nickname}/outbox", "/internal/fetch", - "/relay" + "/relay", + "/relay/following", + "/relay/followers" ] for get_uri <- get_uris do From 5fc92deef37dcc4db476520d89dd79e616356e63 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Mon, 9 Mar 2020 20:51:44 +0300 Subject: [PATCH 06/14] [#1560] Ensured authentication or enabled federation for federation-related routes. New tests + tests refactoring. --- .../plugs/ensure_authenticated_plug.ex | 19 +- lib/pleroma/plugs/federating_plug.ex | 2 +- .../activity_pub/activity_pub_controller.ex | 76 ++--- lib/pleroma/web/feed/user_controller.ex | 7 +- lib/pleroma/web/ostatus/ostatus_controller.ex | 10 +- lib/pleroma/web/router.ex | 5 +- test/plugs/ensure_authenticated_plug_test.exs | 66 +++- test/support/conn_case.ex | 19 ++ .../activity_pub_controller_test.exs | 308 ++++++++++++------ test/web/feed/user_controller_test.exs | 197 ++--------- .../media_proxy_controller_test.exs | 3 +- test/web/ostatus/ostatus_controller_test.exs | 110 +++---- 12 files changed, 418 insertions(+), 404 deletions(-) diff --git a/lib/pleroma/plugs/ensure_authenticated_plug.ex b/lib/pleroma/plugs/ensure_authenticated_plug.ex index 6f9b840a9..054d2297f 100644 --- a/lib/pleroma/plugs/ensure_authenticated_plug.ex +++ b/lib/pleroma/plugs/ensure_authenticated_plug.ex @@ -15,9 +15,24 @@ def call(%{assigns: %{user: %User{}}} = conn, _) do conn end - def call(conn, _) do + def call(conn, options) do + perform = + cond do + options[:if_func] -> options[:if_func].() + options[:unless_func] -> !options[:unless_func].() + true -> true + end + + if perform do + fail(conn) + else + conn + end + end + + def fail(conn) do conn |> render_error(:forbidden, "Invalid credentials.") - |> halt + |> halt() end end diff --git a/lib/pleroma/plugs/federating_plug.ex b/lib/pleroma/plugs/federating_plug.ex index c6d622ce4..7d947339f 100644 --- a/lib/pleroma/plugs/federating_plug.ex +++ b/lib/pleroma/plugs/federating_plug.ex @@ -19,7 +19,7 @@ def call(conn, _opts) do def federating?, do: Pleroma.Config.get([:instance, :federating]) - def fail(conn) do + defp fail(conn) do conn |> put_status(404) |> Phoenix.Controller.put_view(Pleroma.Web.ErrorView) diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex index 525e61360..8b9eb4a2c 100644 --- a/lib/pleroma/web/activity_pub/activity_pub_controller.ex +++ b/lib/pleroma/web/activity_pub/activity_pub_controller.ex @@ -9,6 +9,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do alias Pleroma.Delivery alias Pleroma.Object alias Pleroma.Object.Fetcher + alias Pleroma.Plugs.EnsureAuthenticatedPlug alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub alias Pleroma.Web.ActivityPub.InternalFetchActor @@ -25,18 +26,19 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do action_fallback(:errors) - # Note: some of the following actions (like :update_inbox) may be server-to-server as well - @client_to_server_actions [ - :whoami, - :read_inbox, - :outbox, - :update_outbox, - :upload_media, - :followers, - :following - ] + @federating_only_actions [:internal_fetch, :relay, :relay_following, :relay_followers] - plug(FederatingPlug when action not in @client_to_server_actions) + plug(FederatingPlug when action in @federating_only_actions) + + plug( + EnsureAuthenticatedPlug, + [unless_func: &FederatingPlug.federating?/0] when action not in @federating_only_actions + ) + + plug( + EnsureAuthenticatedPlug + when action in [:read_inbox, :update_outbox, :whoami, :upload_media, :following, :followers] + ) plug( Pleroma.Plugs.Cache, @@ -47,7 +49,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do plug(:set_requester_reachable when action in [:inbox]) plug(:relay_active? when action in [:relay]) - def relay_active?(conn, _) do + defp relay_active?(conn, _) do if Pleroma.Config.get([:instance, :allow_relay]) do conn else @@ -140,14 +142,12 @@ defp set_cache_ttl_for(conn, entity) do end # GET /relay/following - def following(%{assigns: %{relay: true}} = conn, _params) do - if FederatingPlug.federating?() do + def relay_following(conn, _params) do + with %{halted: false} = conn <- FederatingPlug.call(conn, []) do conn |> put_resp_content_type("application/activity+json") |> put_view(UserView) |> render("following.json", %{user: Relay.get_actor()}) - else - FederatingPlug.fail(conn) end end @@ -181,14 +181,12 @@ def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) d end # GET /relay/followers - def followers(%{assigns: %{relay: true}} = conn, _params) do - if FederatingPlug.federating?() do + def relay_followers(conn, _params) do + with %{halted: false} = conn <- FederatingPlug.call(conn, []) do conn |> put_resp_content_type("application/activity+json") |> put_view(UserView) |> render("followers.json", %{user: Relay.get_actor()}) - else - FederatingPlug.fail(conn) end end @@ -221,13 +219,16 @@ def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) d end end - def outbox(conn, %{"nickname" => nickname, "page" => page?} = params) + def outbox( + %{assigns: %{user: for_user}} = conn, + %{"nickname" => nickname, "page" => page?} = params + ) when page? in [true, "true"] do with %User{} = user <- User.get_cached_by_nickname(nickname), {:ok, user} <- User.ensure_keys_present(user) do activities = if params["max_id"] do - ActivityPub.fetch_user_activities(user, nil, %{ + ActivityPub.fetch_user_activities(user, for_user, %{ "max_id" => params["max_id"], # This is a hack because postgres generates inefficient queries when filtering by # 'Answer', poll votes will be hidden by the visibility filter in this case anyway @@ -235,7 +236,7 @@ def outbox(conn, %{"nickname" => nickname, "page" => page?} = params) "limit" => 10 }) else - ActivityPub.fetch_user_activities(user, nil, %{ + ActivityPub.fetch_user_activities(user, for_user, %{ "limit" => 10, "include_poll_votes" => true }) @@ -298,7 +299,8 @@ defp post_inbox_relayed_create(conn, params) do defp post_inbox_fallback(conn, params) do headers = Enum.into(conn.req_headers, %{}) - if String.contains?(headers["signature"], params["actor"]) do + if headers["signature"] && params["actor"] && + String.contains?(headers["signature"], params["actor"]) do Logger.debug( "Signature validation error for: #{params["actor"]}, make sure you are forwarding the HTTP Host header!" ) @@ -306,7 +308,9 @@ defp post_inbox_fallback(conn, params) do Logger.debug(inspect(conn.req_headers)) end - json(conn, dgettext("errors", "error")) + conn + |> put_status(:bad_request) + |> json(dgettext("errors", "error")) end defp represent_service_actor(%User{} = user, conn) do @@ -340,8 +344,6 @@ def whoami(%{assigns: %{user: %User{} = user}} = conn, _params) do |> render("user.json", %{user: user}) end - def whoami(_conn, _params), do: {:error, :not_found} - def read_inbox( %{assigns: %{user: %User{nickname: nickname} = user}} = conn, %{"nickname" => nickname, "page" => page?} = params @@ -377,14 +379,6 @@ def read_inbox(%{assigns: %{user: %User{nickname: nickname} = user}} = conn, %{ end end - def read_inbox(%{assigns: %{user: nil}} = conn, %{"nickname" => nickname}) do - err = dgettext("errors", "can't read inbox of %{nickname}", nickname: nickname) - - conn - |> put_status(:forbidden) - |> json(err) - end - def read_inbox(%{assigns: %{user: %User{nickname: as_nickname}}} = conn, %{ "nickname" => nickname }) do @@ -399,7 +393,7 @@ def read_inbox(%{assigns: %{user: %User{nickname: as_nickname}}} = conn, %{ |> json(err) end - def handle_user_activity(%User{} = user, %{"type" => "Create"} = params) do + defp handle_user_activity(%User{} = user, %{"type" => "Create"} = params) do object = params["object"] |> Map.merge(Map.take(params, ["to", "cc"])) @@ -415,7 +409,7 @@ def handle_user_activity(%User{} = user, %{"type" => "Create"} = params) do }) end - def handle_user_activity(%User{} = user, %{"type" => "Delete"} = params) do + defp handle_user_activity(%User{} = user, %{"type" => "Delete"} = params) do with %Object{} = object <- Object.normalize(params["object"]), true <- user.is_moderator || user.ap_id == object.data["actor"], {:ok, delete} <- ActivityPub.delete(object) do @@ -425,7 +419,7 @@ def handle_user_activity(%User{} = user, %{"type" => "Delete"} = params) do end end - def handle_user_activity(%User{} = user, %{"type" => "Like"} = 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, activity} @@ -434,7 +428,7 @@ def handle_user_activity(%User{} = user, %{"type" => "Like"} = params) do end end - def handle_user_activity(_, _) do + defp handle_user_activity(_, _) do {:error, dgettext("errors", "Unhandled activity type")} end @@ -475,13 +469,13 @@ def update_outbox(%{assigns: %{user: %User{} = user}} = conn, %{"nickname" => ni |> json(err) end - def errors(conn, {:error, :not_found}) do + defp errors(conn, {:error, :not_found}) do conn |> put_status(:not_found) |> json(dgettext("errors", "Not found")) end - def errors(conn, _e) do + defp errors(conn, _e) do conn |> put_status(:internal_server_error) |> json(dgettext("errors", "error")) diff --git a/lib/pleroma/web/feed/user_controller.ex b/lib/pleroma/web/feed/user_controller.ex index 59aabb549..9ba602d9f 100644 --- a/lib/pleroma/web/feed/user_controller.ex +++ b/lib/pleroma/web/feed/user_controller.ex @@ -25,7 +25,12 @@ def feed_redirect(%{assigns: %{format: "html"}} = conn, %{"nickname" => nickname def feed_redirect(%{assigns: %{format: format}} = conn, _params) when format in ["json", "activity+json"] do - ActivityPubController.call(conn, :user) + with %{halted: false} = conn <- + Pleroma.Plugs.EnsureAuthenticatedPlug.call(conn, + unless_func: &Pleroma.Web.FederatingPlug.federating?/0 + ) do + ActivityPubController.call(conn, :user) + end end def feed_redirect(conn, %{"nickname" => nickname}) do diff --git a/lib/pleroma/web/ostatus/ostatus_controller.ex b/lib/pleroma/web/ostatus/ostatus_controller.ex index e3f42b5c4..6fd3cfce5 100644 --- a/lib/pleroma/web/ostatus/ostatus_controller.ex +++ b/lib/pleroma/web/ostatus/ostatus_controller.ex @@ -16,7 +16,9 @@ defmodule Pleroma.Web.OStatus.OStatusController do alias Pleroma.Web.Metadata.PlayerView alias Pleroma.Web.Router - plug(Pleroma.Web.FederatingPlug) + plug(Pleroma.Plugs.EnsureAuthenticatedPlug, + unless_func: &Pleroma.Web.FederatingPlug.federating?/0 + ) plug( RateLimiter, @@ -137,13 +139,13 @@ def notice_player(conn, %{"id" => id}) do end end - def errors(conn, {:error, :not_found}) do + defp errors(conn, {:error, :not_found}) do render_error(conn, :not_found, "Not found") end - def errors(conn, {:fetch_user, nil}), do: errors(conn, {:error, :not_found}) + defp errors(conn, {:fetch_user, nil}), do: errors(conn, {:error, :not_found}) - def errors(conn, _) do + defp errors(conn, _) do render_error(conn, :internal_server_error, "Something went wrong") end end diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex index 5f3a06caa..e4e3ee704 100644 --- a/lib/pleroma/web/router.ex +++ b/lib/pleroma/web/router.ex @@ -570,7 +570,6 @@ defmodule Pleroma.Web.Router do plug(Pleroma.Plugs.EnsureUserKeyPlug) end - # Note: propagate _any_ updates to `@client_to_server_actions` in `ActivityPubController` scope "/", Pleroma.Web.ActivityPub do pipe_through([:activitypub_client]) @@ -600,8 +599,8 @@ defmodule Pleroma.Web.Router do post("/inbox", ActivityPubController, :inbox) end - get("/following", ActivityPubController, :following, assigns: %{relay: true}) - get("/followers", ActivityPubController, :followers, assigns: %{relay: true}) + get("/following", ActivityPubController, :relay_following) + get("/followers", ActivityPubController, :relay_followers) end scope "/internal/fetch", Pleroma.Web.ActivityPub do diff --git a/test/plugs/ensure_authenticated_plug_test.exs b/test/plugs/ensure_authenticated_plug_test.exs index 18be5edd0..7f3559b83 100644 --- a/test/plugs/ensure_authenticated_plug_test.exs +++ b/test/plugs/ensure_authenticated_plug_test.exs @@ -8,24 +8,62 @@ defmodule Pleroma.Plugs.EnsureAuthenticatedPlugTest do alias Pleroma.Plugs.EnsureAuthenticatedPlug alias Pleroma.User - test "it halts if no user is assigned", %{conn: conn} do - conn = - conn - |> EnsureAuthenticatedPlug.call(%{}) + describe "without :if_func / :unless_func options" do + test "it halts if user is NOT assigned", %{conn: conn} do + conn = EnsureAuthenticatedPlug.call(conn, %{}) - assert conn.status == 403 - assert conn.halted == true + assert conn.status == 403 + assert conn.halted == true + end + + test "it continues if a user is assigned", %{conn: conn} do + conn = assign(conn, :user, %User{}) + ret_conn = EnsureAuthenticatedPlug.call(conn, %{}) + + assert ret_conn == conn + end end - test "it continues if a user is assigned", %{conn: conn} do - conn = - conn - |> assign(:user, %User{}) + describe "with :if_func / :unless_func options" do + setup do + %{ + true_fn: fn -> true end, + false_fn: fn -> false end + } + end - ret_conn = - conn - |> EnsureAuthenticatedPlug.call(%{}) + test "it continues if a user is assigned", %{conn: conn, true_fn: true_fn, false_fn: false_fn} do + conn = assign(conn, :user, %User{}) + assert EnsureAuthenticatedPlug.call(conn, if_func: true_fn) == conn + assert EnsureAuthenticatedPlug.call(conn, if_func: false_fn) == conn + assert EnsureAuthenticatedPlug.call(conn, unless_func: true_fn) == conn + assert EnsureAuthenticatedPlug.call(conn, unless_func: false_fn) == conn + end - assert ret_conn == conn + test "it continues if a user is NOT assigned but :if_func evaluates to `false`", + %{conn: conn, false_fn: false_fn} do + assert EnsureAuthenticatedPlug.call(conn, if_func: false_fn) == conn + end + + test "it continues if a user is NOT assigned but :unless_func evaluates to `true`", + %{conn: conn, true_fn: true_fn} do + assert EnsureAuthenticatedPlug.call(conn, unless_func: true_fn) == conn + end + + test "it halts if a user is NOT assigned and :if_func evaluates to `true`", + %{conn: conn, true_fn: true_fn} do + conn = EnsureAuthenticatedPlug.call(conn, if_func: true_fn) + + assert conn.status == 403 + assert conn.halted == true + end + + test "it halts if a user is NOT assigned and :unless_func evaluates to `false`", + %{conn: conn, false_fn: false_fn} do + conn = EnsureAuthenticatedPlug.call(conn, unless_func: false_fn) + + assert conn.status == 403 + assert conn.halted == true + end end end diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index 0f2e81f9e..d6595f971 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -48,6 +48,25 @@ defp oauth_access(scopes, opts \\ []) do %{user: user, token: token, conn: conn} end + + defp ensure_federating_or_authenticated(conn, url, user) do + Pleroma.Config.put([:instance, :federating], false) + + conn + |> get(url) + |> response(403) + + conn + |> assign(:user, user) + |> get(url) + |> response(200) + + Pleroma.Config.put([:instance, :federating], true) + + conn + |> get(url) + |> response(200) + end end end diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs index 04800b7ea..a939d0beb 100644 --- a/test/web/activity_pub/activity_pub_controller_test.exs +++ b/test/web/activity_pub/activity_pub_controller_test.exs @@ -8,6 +8,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do import Pleroma.Factory alias Pleroma.Activity + alias Pleroma.Config alias Pleroma.Delivery alias Pleroma.Instances alias Pleroma.Object @@ -25,8 +26,8 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do :ok end - clear_config_all([:instance, :federating]) do - Pleroma.Config.put([:instance, :federating], true) + clear_config([:instance, :federating]) do + Config.put([:instance, :federating], true) end describe "/relay" do @@ -42,12 +43,21 @@ test "with the relay active, it returns the relay user", %{conn: conn} do end test "with the relay disabled, it returns 404", %{conn: conn} do - Pleroma.Config.put([:instance, :allow_relay], false) + Config.put([:instance, :allow_relay], false) conn |> get(activity_pub_path(conn, :relay)) |> json_response(404) - |> assert + end + + test "on non-federating instance, it returns 404", %{conn: conn} do + Config.put([:instance, :federating], false) + user = insert(:user) + + conn + |> assign(:user, user) + |> get(activity_pub_path(conn, :relay)) + |> json_response(404) end end @@ -60,6 +70,16 @@ test "it returns the internal fetch user", %{conn: conn} do assert res["id"] =~ "/fetch" end + + test "on non-federating instance, it returns 404", %{conn: conn} do + Config.put([:instance, :federating], false) + user = insert(:user) + + conn + |> assign(:user, user) + |> get(activity_pub_path(conn, :internal_fetch)) + |> json_response(404) + end end describe "/users/:nickname" do @@ -123,9 +143,34 @@ test "it returns 404 for remote users", %{ assert json_response(conn, 404) end + + test "it returns error when user is not found", %{conn: conn} do + response = + conn + |> put_req_header("accept", "application/json") + |> get("/users/jimm") + |> json_response(404) + + assert response == "Not found" + end + + test "it requires authentication if instance is NOT federating", %{ + conn: conn + } do + user = insert(:user) + + conn = + put_req_header( + conn, + "accept", + "application/ld+json; profile=\"https://www.w3.org/ns/activitystreams\"" + ) + + ensure_federating_or_authenticated(conn, "/users/#{user.nickname}.json", user) + end end - describe "/object/:uuid" do + describe "/objects/:uuid" do test "it returns a json representation of the object with accept application/json", %{ conn: conn } do @@ -236,6 +281,18 @@ test "cached purged after object deletion", %{conn: conn} do assert "Not found" == json_response(conn2, :not_found) end + + test "it requires authentication if instance is NOT federating", %{ + conn: conn + } do + user = insert(:user) + note = insert(:note) + uuid = String.split(note.data["id"], "/") |> List.last() + + conn = put_req_header(conn, "accept", "application/activity+json") + + ensure_federating_or_authenticated(conn, "/objects/#{uuid}", user) + end end describe "/activities/:uuid" do @@ -307,6 +364,18 @@ test "cached purged after activity deletion", %{conn: conn} do assert "Not found" == json_response(conn2, :not_found) end + + test "it requires authentication if instance is NOT federating", %{ + conn: conn + } do + user = insert(:user) + activity = insert(:note_activity) + uuid = String.split(activity.data["id"], "/") |> List.last() + + conn = put_req_header(conn, "accept", "application/activity+json") + + ensure_federating_or_authenticated(conn, "/activities/#{uuid}", user) + end end describe "/inbox" do @@ -341,6 +410,34 @@ test "it clears `unreachable` federation status of the sender", %{conn: conn} do assert "ok" == json_response(conn, 200) assert Instances.reachable?(sender_url) end + + test "without valid signature, " <> + "it only accepts Create activities and requires enabled federation", + %{conn: conn} do + data = File.read!("test/fixtures/mastodon-post-activity.json") |> Poison.decode!() + non_create_data = File.read!("test/fixtures/mastodon-announce.json") |> Poison.decode!() + + conn = put_req_header(conn, "content-type", "application/activity+json") + + Config.put([:instance, :federating], false) + + conn + |> post("/inbox", data) + |> json_response(403) + + conn + |> post("/inbox", non_create_data) + |> json_response(403) + + Config.put([:instance, :federating], true) + + ret_conn = post(conn, "/inbox", data) + assert "ok" == json_response(ret_conn, 200) + + conn + |> post("/inbox", non_create_data) + |> json_response(400) + end end describe "/users/:nickname/inbox" do @@ -479,22 +576,11 @@ test "it accepts messages from actors that are followed by the user", %{ test "it rejects reads from other users", %{conn: conn} do user = insert(:user) - otheruser = insert(:user) - - conn = - conn - |> assign(:user, otheruser) - |> put_req_header("accept", "application/activity+json") - |> get("/users/#{user.nickname}/inbox") - - assert json_response(conn, 403) - end - - test "it doesn't crash without an authenticated user", %{conn: conn} do - user = insert(:user) + other_user = insert(:user) conn = conn + |> assign(:user, other_user) |> put_req_header("accept", "application/activity+json") |> get("/users/#{user.nickname}/inbox") @@ -575,14 +661,30 @@ test "it removes all follower collections but actor's", %{conn: conn} do refute recipient.follower_address in activity.data["cc"] refute recipient.follower_address in activity.data["to"] end + + test "it requires authentication", %{conn: conn} do + user = insert(:user) + conn = put_req_header(conn, "accept", "application/activity+json") + + ret_conn = get(conn, "/users/#{user.nickname}/inbox") + assert json_response(ret_conn, 403) + + ret_conn = + conn + |> assign(:user, user) + |> get("/users/#{user.nickname}/inbox") + + assert json_response(ret_conn, 200) + end end describe "GET /users/:nickname/outbox" do - test "it will not bomb when there is no activity", %{conn: conn} do + test "it returns 200 even if there're no activities", %{conn: conn} do user = insert(:user) conn = conn + |> assign(:user, user) |> put_req_header("accept", "application/activity+json") |> get("/users/#{user.nickname}/outbox") @@ -597,6 +699,7 @@ test "it returns a note activity in a collection", %{conn: conn} do conn = conn + |> assign(:user, user) |> put_req_header("accept", "application/activity+json") |> get("/users/#{user.nickname}/outbox?page=true") @@ -609,26 +712,38 @@ test "it returns an announce activity in a collection", %{conn: conn} do conn = conn + |> assign(:user, user) |> put_req_header("accept", "application/activity+json") |> get("/users/#{user.nickname}/outbox?page=true") assert response(conn, 200) =~ announce_activity.data["object"] end + + test "it requires authentication if instance is NOT federating", %{ + conn: conn + } do + user = insert(:user) + conn = put_req_header(conn, "accept", "application/activity+json") + + ensure_federating_or_authenticated(conn, "/users/#{user.nickname}/outbox", user) + end end describe "POST /users/:nickname/outbox" do - test "it rejects posts from other users", %{conn: conn} do + test "it rejects posts from other users / unauuthenticated users", %{conn: conn} do data = File.read!("test/fixtures/activitypub-client-post-activity.json") |> Poison.decode!() user = insert(:user) - otheruser = insert(:user) + other_user = insert(:user) + conn = put_req_header(conn, "content-type", "application/activity+json") - conn = - conn - |> assign(:user, otheruser) - |> put_req_header("content-type", "application/activity+json") - |> post("/users/#{user.nickname}/outbox", data) + conn + |> post("/users/#{user.nickname}/outbox", data) + |> json_response(403) - assert json_response(conn, 403) + conn + |> assign(:user, other_user) + |> post("/users/#{user.nickname}/outbox", data) + |> json_response(403) end test "it inserts an incoming create activity into the database", %{conn: conn} do @@ -743,24 +858,42 @@ test "it returns relay followers", %{conn: conn} do result = conn - |> assign(:relay, true) |> get("/relay/followers") |> json_response(200) assert result["first"]["orderedItems"] == [user.ap_id] end + + test "on non-federating instance, it returns 404", %{conn: conn} do + Config.put([:instance, :federating], false) + user = insert(:user) + + conn + |> assign(:user, user) + |> get("/relay/followers") + |> json_response(404) + end end describe "/relay/following" do test "it returns relay following", %{conn: conn} do result = conn - |> assign(:relay, true) |> get("/relay/following") |> json_response(200) assert result["first"]["orderedItems"] == [] end + + test "on non-federating instance, it returns 404", %{conn: conn} do + Config.put([:instance, :federating], false) + user = insert(:user) + + conn + |> assign(:user, user) + |> get("/relay/following") + |> json_response(404) + end end describe "/users/:nickname/followers" do @@ -771,6 +904,7 @@ test "it returns the followers in a collection", %{conn: conn} do result = conn + |> assign(:user, user_two) |> get("/users/#{user_two.nickname}/followers") |> json_response(200) @@ -784,19 +918,22 @@ test "it returns a uri if the user has 'hide_followers' set", %{conn: conn} do result = conn + |> assign(:user, user) |> get("/users/#{user_two.nickname}/followers") |> json_response(200) assert is_binary(result["first"]) end - test "it returns a 403 error on pages, if the user has 'hide_followers' set and the request is not authenticated", + test "it returns a 403 error on pages, if the user has 'hide_followers' set and the request is from another user", %{conn: conn} do - user = insert(:user, hide_followers: true) + user = insert(:user) + other_user = insert(:user, hide_followers: true) result = conn - |> get("/users/#{user.nickname}/followers?page=1") + |> assign(:user, user) + |> get("/users/#{other_user.nickname}/followers?page=1") assert result.status == 403 assert result.resp_body == "" @@ -828,6 +965,7 @@ test "it works for more than 10 users", %{conn: conn} do result = conn + |> assign(:user, user) |> get("/users/#{user.nickname}/followers") |> json_response(200) @@ -837,12 +975,21 @@ test "it works for more than 10 users", %{conn: conn} do result = conn + |> assign(:user, user) |> get("/users/#{user.nickname}/followers?page=2") |> json_response(200) assert length(result["orderedItems"]) == 5 assert result["totalItems"] == 15 end + + test "returns 403 if requester is not logged in", %{conn: conn} do + user = insert(:user) + + conn + |> get("/users/#{user.nickname}/followers") + |> json_response(403) + end end describe "/users/:nickname/following" do @@ -853,6 +1000,7 @@ test "it returns the following in a collection", %{conn: conn} do result = conn + |> assign(:user, user) |> get("/users/#{user.nickname}/following") |> json_response(200) @@ -860,25 +1008,28 @@ test "it returns the following in a collection", %{conn: conn} do end test "it returns a uri if the user has 'hide_follows' set", %{conn: conn} do - user = insert(:user, hide_follows: true) - user_two = insert(:user) + user = insert(:user) + user_two = insert(:user, hide_follows: true) User.follow(user, user_two) result = conn - |> get("/users/#{user.nickname}/following") + |> assign(:user, user) + |> get("/users/#{user_two.nickname}/following") |> json_response(200) assert is_binary(result["first"]) end - test "it returns a 403 error on pages, if the user has 'hide_follows' set and the request is not authenticated", + test "it returns a 403 error on pages, if the user has 'hide_follows' set and the request is from another user", %{conn: conn} do - user = insert(:user, hide_follows: true) + user = insert(:user) + user_two = insert(:user, hide_follows: true) result = conn - |> get("/users/#{user.nickname}/following?page=1") + |> assign(:user, user) + |> get("/users/#{user_two.nickname}/following?page=1") assert result.status == 403 assert result.resp_body == "" @@ -911,6 +1062,7 @@ test "it works for more than 10 users", %{conn: conn} do result = conn + |> assign(:user, user) |> get("/users/#{user.nickname}/following") |> json_response(200) @@ -920,12 +1072,21 @@ test "it works for more than 10 users", %{conn: conn} do result = conn + |> assign(:user, user) |> get("/users/#{user.nickname}/following?page=2") |> json_response(200) assert length(result["orderedItems"]) == 5 assert result["totalItems"] == 15 end + + test "returns 403 if requester is not logged in", %{conn: conn} do + user = insert(:user) + + conn + |> get("/users/#{user.nickname}/following") + |> json_response(403) + end end describe "delivery tracking" do @@ -1011,7 +1172,7 @@ test "it tracks a signed activity fetch when the json is cached", %{conn: conn} end describe "Additional ActivityPub C2S endpoints" do - test "/api/ap/whoami", %{conn: conn} do + test "GET /api/ap/whoami", %{conn: conn} do user = insert(:user) conn = @@ -1022,12 +1183,16 @@ test "/api/ap/whoami", %{conn: conn} do user = User.get_cached_by_id(user.id) assert UserView.render("user.json", %{user: user}) == json_response(conn, 200) + + conn + |> get("/api/ap/whoami") + |> json_response(403) end clear_config([:media_proxy]) clear_config([Pleroma.Upload]) - test "uploadMedia", %{conn: conn} do + test "POST /api/ap/upload_media", %{conn: conn} do user = insert(:user) desc = "Description of the image" @@ -1047,67 +1212,10 @@ test "uploadMedia", %{conn: conn} do assert object["name"] == desc assert object["type"] == "Document" assert object["actor"] == user.ap_id - end - end - describe "when instance is not federating," do - clear_config([:instance, :federating]) do - Pleroma.Config.put([:instance, :federating], false) - end - - test "returns 404 for GET routes", %{conn: conn} do - user = insert(:user) - conn = put_req_header(conn, "accept", "application/json") - - get_uris = [ - "/users/#{user.nickname}", - "/internal/fetch", - "/relay", - "/relay/following", - "/relay/followers" - ] - - for get_uri <- get_uris do - conn - |> get(get_uri) - |> json_response(404) - - conn - |> assign(:user, user) - |> get(get_uri) - |> json_response(404) - end - end - - test "returns 404 for activity-related POST routes", %{conn: conn} do - user = insert(:user) - - conn = - conn - |> assign(:valid_signature, true) - |> put_req_header("content-type", "application/activity+json") - - post_activity_data = - "test/fixtures/mastodon-post-activity.json" - |> File.read!() - |> Poison.decode!() - - post_activity_uris = [ - "/inbox", - "/relay/inbox", - "/users/#{user.nickname}/inbox" - ] - - for post_activity_uri <- post_activity_uris do - conn - |> post(post_activity_uri, post_activity_data) - |> json_response(404) - - conn - |> assign(:user, user) - |> post(post_activity_uri, post_activity_data) - |> json_response(404) - end + conn + |> post("/api/ap/upload_media", %{"file" => image, "description" => desc}) + |> json_response(403) end end end diff --git a/test/web/feed/user_controller_test.exs b/test/web/feed/user_controller_test.exs index 00712ab5a..00c50f003 100644 --- a/test/web/feed/user_controller_test.exs +++ b/test/web/feed/user_controller_test.exs @@ -12,7 +12,7 @@ defmodule Pleroma.Web.Feed.UserControllerTest do alias Pleroma.Object alias Pleroma.User - clear_config_all([:instance, :federating]) do + clear_config([:instance, :federating]) do Config.put([:instance, :federating], true) end @@ -82,160 +82,9 @@ test "returns 404 for a missing feed", %{conn: conn} do end end + # Note: see ActivityPubControllerTest for JSON format tests describe "feed_redirect" do - test "undefined format. it redirects to feed", %{conn: conn} do - note_activity = insert(:note_activity) - user = User.get_cached_by_ap_id(note_activity.data["actor"]) - - response = - conn - |> put_req_header("accept", "application/xml") - |> get("/users/#{user.nickname}") - |> response(302) - - assert response == - "You are being redirected." - end - - test "undefined format. it returns error when user not found", %{conn: conn} do - response = - conn - |> put_req_header("accept", "application/xml") - |> get(user_feed_path(conn, :feed, "jimm")) - |> response(404) - - assert response == ~S({"error":"Not found"}) - end - - test "activity+json format. it redirects on actual feed of user", %{conn: conn} do - note_activity = insert(:note_activity) - user = User.get_cached_by_ap_id(note_activity.data["actor"]) - - response = - conn - |> put_req_header("accept", "application/activity+json") - |> get("/users/#{user.nickname}") - |> json_response(200) - - assert response["endpoints"] == %{ - "oauthAuthorizationEndpoint" => "#{Pleroma.Web.base_url()}/oauth/authorize", - "oauthRegistrationEndpoint" => "#{Pleroma.Web.base_url()}/api/v1/apps", - "oauthTokenEndpoint" => "#{Pleroma.Web.base_url()}/oauth/token", - "sharedInbox" => "#{Pleroma.Web.base_url()}/inbox", - "uploadMedia" => "#{Pleroma.Web.base_url()}/api/ap/upload_media" - } - - assert response["@context"] == [ - "https://www.w3.org/ns/activitystreams", - "http://localhost:4001/schemas/litepub-0.1.jsonld", - %{"@language" => "und"} - ] - - assert Map.take(response, [ - "followers", - "following", - "id", - "inbox", - "manuallyApprovesFollowers", - "name", - "outbox", - "preferredUsername", - "summary", - "tag", - "type", - "url" - ]) == %{ - "followers" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}/followers", - "following" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}/following", - "id" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}", - "inbox" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}/inbox", - "manuallyApprovesFollowers" => false, - "name" => user.name, - "outbox" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}/outbox", - "preferredUsername" => user.nickname, - "summary" => user.bio, - "tag" => [], - "type" => "Person", - "url" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}" - } - end - - test "activity+json format. it returns error whe use not found", %{conn: conn} do - response = - conn - |> put_req_header("accept", "application/activity+json") - |> get("/users/jimm") - |> json_response(404) - - assert response == "Not found" - end - - test "json format. it redirects on actual feed of user", %{conn: conn} do - note_activity = insert(:note_activity) - user = User.get_cached_by_ap_id(note_activity.data["actor"]) - - response = - conn - |> put_req_header("accept", "application/json") - |> get("/users/#{user.nickname}") - |> json_response(200) - - assert response["endpoints"] == %{ - "oauthAuthorizationEndpoint" => "#{Pleroma.Web.base_url()}/oauth/authorize", - "oauthRegistrationEndpoint" => "#{Pleroma.Web.base_url()}/api/v1/apps", - "oauthTokenEndpoint" => "#{Pleroma.Web.base_url()}/oauth/token", - "sharedInbox" => "#{Pleroma.Web.base_url()}/inbox", - "uploadMedia" => "#{Pleroma.Web.base_url()}/api/ap/upload_media" - } - - assert response["@context"] == [ - "https://www.w3.org/ns/activitystreams", - "http://localhost:4001/schemas/litepub-0.1.jsonld", - %{"@language" => "und"} - ] - - assert Map.take(response, [ - "followers", - "following", - "id", - "inbox", - "manuallyApprovesFollowers", - "name", - "outbox", - "preferredUsername", - "summary", - "tag", - "type", - "url" - ]) == %{ - "followers" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}/followers", - "following" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}/following", - "id" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}", - "inbox" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}/inbox", - "manuallyApprovesFollowers" => false, - "name" => user.name, - "outbox" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}/outbox", - "preferredUsername" => user.nickname, - "summary" => user.bio, - "tag" => [], - "type" => "Person", - "url" => "#{Pleroma.Web.base_url()}/users/#{user.nickname}" - } - end - - test "json format. it returns error whe use not found", %{conn: conn} do - response = - conn - |> put_req_header("accept", "application/json") - |> get("/users/jimm") - |> json_response(404) - - assert response == "Not found" - end - - test "html format. it redirects on actual feed of user", %{conn: conn} do + test "with html format, it redirects to user feed", %{conn: conn} do note_activity = insert(:note_activity) user = User.get_cached_by_ap_id(note_activity.data["actor"]) @@ -251,7 +100,7 @@ test "html format. it redirects on actual feed of user", %{conn: conn} do ).resp_body end - test "html format. it returns error when user not found", %{conn: conn} do + test "with html format, it returns error when user is not found", %{conn: conn} do response = conn |> get("/users/jimm") @@ -259,30 +108,30 @@ test "html format. it returns error when user not found", %{conn: conn} do assert response == %{"error" => "Not found"} end - end - describe "feed_redirect (depending on federation enabled state)" do - setup %{conn: conn} do - user = insert(:user) - conn = put_req_header(conn, "accept", "application/json") + test "with non-html / non-json format, it redirects to user feed in atom format", %{ + conn: conn + } do + note_activity = insert(:note_activity) + user = User.get_cached_by_ap_id(note_activity.data["actor"]) - %{conn: conn, user: user} + conn = + conn + |> put_req_header("accept", "application/xml") + |> get("/users/#{user.nickname}") + + assert conn.status == 302 + assert redirected_to(conn) == "#{Pleroma.Web.base_url()}/users/#{user.nickname}/feed.atom" end - clear_config([:instance, :federating]) + test "with non-html / non-json format, it returns error when user is not found", %{conn: conn} do + response = + conn + |> put_req_header("accept", "application/xml") + |> get(user_feed_path(conn, :feed, "jimm")) + |> response(404) - test "renders if instance is federating", %{conn: conn, user: user} do - Config.put([:instance, :federating], true) - - conn = get(conn, "/users/#{user.nickname}") - assert json_response(conn, 200) - end - - test "renders 404 if instance is NOT federating", %{conn: conn, user: user} do - Config.put([:instance, :federating], false) - - conn = get(conn, "/users/#{user.nickname}") - assert json_response(conn, 404) + assert response == ~S({"error":"Not found"}) end end end diff --git a/test/web/media_proxy/media_proxy_controller_test.exs b/test/web/media_proxy/media_proxy_controller_test.exs index f035dfeee..7ac7e4af1 100644 --- a/test/web/media_proxy/media_proxy_controller_test.exs +++ b/test/web/media_proxy/media_proxy_controller_test.exs @@ -52,9 +52,8 @@ test "redirects on valid url when filename invalidated", %{conn: conn} do url = Pleroma.Web.MediaProxy.encode_url("https://google.fn/test.png") invalid_url = String.replace(url, "test.png", "test-file.png") response = get(conn, invalid_url) - html = "You are being redirected." assert response.status == 302 - assert response.resp_body == html + assert redirected_to(response) == url end test "it performs ReverseProxy.call when signature valid", %{conn: conn} do diff --git a/test/web/ostatus/ostatus_controller_test.exs b/test/web/ostatus/ostatus_controller_test.exs index 725ab1785..3b84358e4 100644 --- a/test/web/ostatus/ostatus_controller_test.exs +++ b/test/web/ostatus/ostatus_controller_test.exs @@ -7,6 +7,7 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do import Pleroma.Factory + alias Pleroma.Config alias Pleroma.Object alias Pleroma.User alias Pleroma.Web.CommonAPI @@ -16,22 +17,24 @@ defmodule Pleroma.Web.OStatus.OStatusControllerTest do :ok end - clear_config_all([:instance, :federating]) do - Pleroma.Config.put([:instance, :federating], true) + clear_config([:instance, :federating]) do + Config.put([:instance, :federating], true) end - describe "GET object/2" do + # Note: see ActivityPubControllerTest for JSON format tests + describe "GET /objects/:uuid (text/html)" do + setup %{conn: conn} do + conn = put_req_header(conn, "accept", "text/html") + %{conn: conn} + end + test "redirects to /notice/id for html format", %{conn: conn} do note_activity = insert(:note_activity) object = Object.normalize(note_activity) [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, object.data["id"])) url = "/objects/#{uuid}" - conn = - conn - |> put_req_header("accept", "text/html") - |> get(url) - + conn = get(conn, url) assert redirected_to(conn) == "/notice/#{note_activity.id}" end @@ -45,23 +48,25 @@ test "404s on private objects", %{conn: conn} do |> response(404) end - test "404s on nonexisting objects", %{conn: conn} do + test "404s on non-existing objects", %{conn: conn} do conn |> get("/objects/123") |> response(404) end end - describe "GET activity/2" do + # Note: see ActivityPubControllerTest for JSON format tests + describe "GET /activities/:uuid (text/html)" do + setup %{conn: conn} do + conn = put_req_header(conn, "accept", "text/html") + %{conn: conn} + end + test "redirects to /notice/id for html format", %{conn: conn} do note_activity = insert(:note_activity) [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["id"])) - conn = - conn - |> put_req_header("accept", "text/html") - |> get("/activities/#{uuid}") - + conn = get(conn, "/activities/#{uuid}") assert redirected_to(conn) == "/notice/#{note_activity.id}" end @@ -79,19 +84,6 @@ test "404s on nonexistent activities", %{conn: conn} do |> get("/activities/123") |> response(404) end - - test "gets an activity in AS2 format", %{conn: conn} do - note_activity = insert(:note_activity) - [_, uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["id"])) - url = "/activities/#{uuid}" - - conn = - conn - |> put_req_header("accept", "application/activity+json") - |> get(url) - - assert json_response(conn, 200) - end end describe "GET notice/2" do @@ -170,7 +162,7 @@ test "404s a private notice", %{conn: conn} do assert response(conn, 404) end - test "404s a nonexisting notice", %{conn: conn} do + test "404s a non-existing notice", %{conn: conn} do url = "/notice/123" conn = @@ -179,10 +171,21 @@ test "404s a nonexisting notice", %{conn: conn} do assert response(conn, 404) end + + test "it requires authentication if instance is NOT federating", %{ + conn: conn + } do + user = insert(:user) + note_activity = insert(:note_activity) + + conn = put_req_header(conn, "accept", "text/html") + + ensure_federating_or_authenticated(conn, "/notice/#{note_activity.id}", user) + end end describe "GET /notice/:id/embed_player" do - test "render embed player", %{conn: conn} do + setup do note_activity = insert(:note_activity) object = Pleroma.Object.normalize(note_activity) @@ -204,9 +207,11 @@ test "render embed player", %{conn: conn} do |> Ecto.Changeset.change(data: object_data) |> Pleroma.Repo.update() - conn = - conn - |> get("/notice/#{note_activity.id}/embed_player") + %{note_activity: note_activity} + end + + test "renders embed player", %{conn: conn, note_activity: note_activity} do + conn = get(conn, "/notice/#{note_activity.id}/embed_player") assert Plug.Conn.get_resp_header(conn, "x-frame-options") == ["ALLOW"] @@ -272,38 +277,19 @@ test "404s when attachment isn't audio or video", %{conn: conn} do |> Ecto.Changeset.change(data: object_data) |> Pleroma.Repo.update() - assert conn - |> get("/notice/#{note_activity.id}/embed_player") - |> response(404) - end - end - - describe "when instance is not federating," do - clear_config([:instance, :federating]) do - Pleroma.Config.put([:instance, :federating], false) + conn + |> get("/notice/#{note_activity.id}/embed_player") + |> response(404) end - test "returns 404 for GET routes", %{conn: conn} do - conn = put_req_header(conn, "accept", "application/json") + test "it requires authentication if instance is NOT federating", %{ + conn: conn, + note_activity: note_activity + } do + user = insert(:user) + conn = put_req_header(conn, "accept", "text/html") - note_activity = insert(:note_activity, local: true) - [_, activity_uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, note_activity.data["id"])) - - object = Object.normalize(note_activity) - [_, object_uuid] = hd(Regex.scan(~r/.+\/([\w-]+)$/, object.data["id"])) - - get_uris = [ - "/activities/#{activity_uuid}", - "/objects/#{object_uuid}", - "/notice/#{note_activity.id}", - "/notice/#{note_activity.id}/embed_player" - ] - - for get_uri <- get_uris do - conn - |> get(get_uri) - |> json_response(404) - end + ensure_federating_or_authenticated(conn, "/notice/#{note_activity.id}/embed_player", user) end end end From 5b696a8ac1b5a06e60c2143cf88e014b28e14702 Mon Sep 17 00:00:00 2001 From: Ivan Tashkinov Date: Wed, 11 Mar 2020 14:05:56 +0300 Subject: [PATCH 07/14] [#1560] Enforced authentication for non-federating instances in StaticFEController. --- .../web/static_fe/static_fe_controller.ex | 20 +++++++++++-------- test/support/conn_case.ex | 9 +++++++-- .../static_fe/static_fe_controller_test.exs | 14 +++++++++++++ 3 files changed, 33 insertions(+), 10 deletions(-) diff --git a/lib/pleroma/web/static_fe/static_fe_controller.ex b/lib/pleroma/web/static_fe/static_fe_controller.ex index 5ac75f1c4..5027d5c23 100644 --- a/lib/pleroma/web/static_fe/static_fe_controller.ex +++ b/lib/pleroma/web/static_fe/static_fe_controller.ex @@ -17,6 +17,10 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do plug(:put_view, Pleroma.Web.StaticFE.StaticFEView) plug(:assign_id) + plug(Pleroma.Plugs.EnsureAuthenticatedPlug, + unless_func: &Pleroma.Web.FederatingPlug.federating?/0 + ) + @page_keys ["max_id", "min_id", "limit", "since_id", "order"] defp get_title(%Object{data: %{"name" => name}}) when is_binary(name), @@ -33,7 +37,7 @@ defp not_found(conn, message) do |> render("error.html", %{message: message, meta: ""}) end - def get_counts(%Activity{} = activity) do + defp get_counts(%Activity{} = activity) do %Object{data: data} = Object.normalize(activity) %{ @@ -43,9 +47,9 @@ def get_counts(%Activity{} = activity) do } end - def represent(%Activity{} = activity), do: represent(activity, false) + defp represent(%Activity{} = activity), do: represent(activity, false) - def represent(%Activity{object: %Object{data: data}} = activity, selected) do + defp represent(%Activity{object: %Object{data: data}} = activity, selected) do {:ok, user} = User.get_or_fetch(activity.object.data["actor"]) link = @@ -147,17 +151,17 @@ def show(%{assigns: %{activity_id: _}} = conn, _params) do end end - def assign_id(%{path_info: ["notice", notice_id]} = conn, _opts), + defp assign_id(%{path_info: ["notice", notice_id]} = conn, _opts), do: assign(conn, :notice_id, notice_id) - def assign_id(%{path_info: ["users", user_id]} = conn, _opts), + defp assign_id(%{path_info: ["users", user_id]} = conn, _opts), do: assign(conn, :username_or_id, user_id) - def assign_id(%{path_info: ["objects", object_id]} = conn, _opts), + defp assign_id(%{path_info: ["objects", object_id]} = conn, _opts), do: assign(conn, :object_id, object_id) - def assign_id(%{path_info: ["activities", activity_id]} = conn, _opts), + defp assign_id(%{path_info: ["activities", activity_id]} = conn, _opts), do: assign(conn, :activity_id, activity_id) - def assign_id(conn, _opts), do: conn + defp assign_id(conn, _opts), do: conn end diff --git a/test/support/conn_case.ex b/test/support/conn_case.ex index d6595f971..064874201 100644 --- a/test/support/conn_case.ex +++ b/test/support/conn_case.ex @@ -26,6 +26,8 @@ defmodule Pleroma.Web.ConnCase do use Pleroma.Tests.Helpers import Pleroma.Web.Router.Helpers + alias Pleroma.Config + # The default endpoint for testing @endpoint Pleroma.Web.Endpoint @@ -50,7 +52,10 @@ defp oauth_access(scopes, opts \\ []) do end defp ensure_federating_or_authenticated(conn, url, user) do - Pleroma.Config.put([:instance, :federating], false) + initial_setting = Config.get([:instance, :federating]) + on_exit(fn -> Config.put([:instance, :federating], initial_setting) end) + + Config.put([:instance, :federating], false) conn |> get(url) @@ -61,7 +66,7 @@ defp ensure_federating_or_authenticated(conn, url, user) do |> get(url) |> response(200) - Pleroma.Config.put([:instance, :federating], true) + Config.put([:instance, :federating], true) conn |> get(url) diff --git a/test/web/static_fe/static_fe_controller_test.exs b/test/web/static_fe/static_fe_controller_test.exs index 11facab99..a072cc78f 100644 --- a/test/web/static_fe/static_fe_controller_test.exs +++ b/test/web/static_fe/static_fe_controller_test.exs @@ -12,6 +12,10 @@ defmodule Pleroma.Web.StaticFE.StaticFEControllerTest do Config.put([:static_fe, :enabled], true) end + clear_config([:instance, :federating]) do + Config.put([:instance, :federating], true) + end + setup %{conn: conn} do conn = put_req_header(conn, "accept", "text/html") user = insert(:user) @@ -70,6 +74,10 @@ test "pagination, page 2", %{conn: conn, user: user} do refute html =~ ">test20<" refute html =~ ">test29<" end + + test "it requires authentication if instance is NOT federating", %{conn: conn, user: user} do + ensure_federating_or_authenticated(conn, "/users/#{user.nickname}", user) + end end describe "notice html" do @@ -153,5 +161,11 @@ test "302 for remote cached status", %{conn: conn, user: user} do assert html_response(conn, 302) =~ "redirected" end + + test "it requires authentication if instance is NOT federating", %{conn: conn, user: user} do + {:ok, activity} = CommonAPI.post(user, %{"status" => "testing a thing!"}) + + ensure_federating_or_authenticated(conn, "/notice/#{activity.id}", user) + end end end From dca21cd1d6fc0720ed70cce50389a30f8a16952f Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Sun, 15 Mar 2020 17:07:08 +0100 Subject: [PATCH 08/14] test/earmark_renderer_test.exs: Rename from test/earmark_renderer_test.ex Wasn't in the test suite otherwise --- test/{earmark_renderer_test.ex => earmark_renderer_test.exs} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/{earmark_renderer_test.ex => earmark_renderer_test.exs} (100%) diff --git a/test/earmark_renderer_test.ex b/test/earmark_renderer_test.exs similarity index 100% rename from test/earmark_renderer_test.ex rename to test/earmark_renderer_test.exs From 0ac6e296549f43e553bdd2350050efcf95d3b6fa Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Sun, 15 Mar 2020 15:45:57 +0100 Subject: [PATCH 09/14] static_fe: Sanitize HTML in posts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Note: Seems to have different sanitization with TwitterCard generator giving the following: --- lib/pleroma/web/static_fe/static_fe_controller.ex | 9 ++++++++- test/web/static_fe/static_fe_controller_test.exs | 13 +++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/lib/pleroma/web/static_fe/static_fe_controller.ex b/lib/pleroma/web/static_fe/static_fe_controller.ex index 5027d5c23..0b77f949c 100644 --- a/lib/pleroma/web/static_fe/static_fe_controller.ex +++ b/lib/pleroma/web/static_fe/static_fe_controller.ex @@ -58,10 +58,17 @@ defp represent(%Activity{object: %Object{data: data}} = activity, selected) do _ -> data["url"] || data["external_url"] || data["id"] end + content = + if data["content"] do + Pleroma.HTML.filter_tags(data["content"]) + else + nil + end + %{ user: user, title: get_title(activity.object), - content: data["content"] || nil, + content: content, attachment: data["attachment"], link: link, published: data["published"], diff --git a/test/web/static_fe/static_fe_controller_test.exs b/test/web/static_fe/static_fe_controller_test.exs index a072cc78f..c3d2ae3b4 100644 --- a/test/web/static_fe/static_fe_controller_test.exs +++ b/test/web/static_fe/static_fe_controller_test.exs @@ -92,6 +92,19 @@ test "single notice page", %{conn: conn, user: user} do assert html =~ "testing a thing!" end + test "filters HTML tags", %{conn: conn} do + user = insert(:user) + {:ok, activity} = CommonAPI.post(user, %{"status" => ""}) + + conn = + conn + |> put_req_header("accept", "text/html") + |> get("/notice/#{activity.id}") + + html = html_response(conn, 200) + assert html =~ ~s[<script>alert('xss')</script>] + end + test "shows the whole thread", %{conn: conn, user: user} do {:ok, activity} = CommonAPI.post(user, %{"status" => "space: the final frontier"}) From acb016397ea82a6365f5ba30ae27d2094cd8ab7e Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Sun, 15 Mar 2020 15:58:26 +0100 Subject: [PATCH 10/14] mix.lock: [minor] last hash appended --- mix.lock | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mix.lock b/mix.lock index 1b4fbc927..62e14924a 100644 --- a/mix.lock +++ b/mix.lock @@ -4,10 +4,10 @@ "base62": {:hex, :base62, "1.2.1", "4866763e08555a7b3917064e9eef9194c41667276c51b59de2bc42c6ea65f806", [:mix], [{:custom_base, "~> 0.2.1", [hex: :custom_base, repo: "hexpm", optional: false]}], "hexpm", "3b29948de2013d3f93aa898c884a9dff847e7aec75d9d6d8c1dc4c61c2716c42"}, "base64url": {:hex, :base64url, "0.0.1", "36a90125f5948e3afd7be97662a1504b934dd5dac78451ca6e9abf85a10286be", [:rebar], [], "hexpm"}, "bbcode": {:git, "https://git.pleroma.social/pleroma/elixir-libraries/bbcode.git", "f2d267675e9a7e1ad1ea9beb4cc23382762b66c2", [ref: "v0.2.0"]}, - "bbcode_pleroma": {:hex, :bbcode_pleroma, "0.2.0", "d36f5bca6e2f62261c45be30fa9b92725c0655ad45c99025cb1c3e28e25803ef", [:mix], [{:nimble_parsec, "~> 0.5", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm"}, + "bbcode_pleroma": {:hex, :bbcode_pleroma, "0.2.0", "d36f5bca6e2f62261c45be30fa9b92725c0655ad45c99025cb1c3e28e25803ef", [:mix], [{:nimble_parsec, "~> 0.5", [hex: :nimble_parsec, repo: "hexpm", optional: false]}], "hexpm", "19851074419a5fedb4ef49e1f01b30df504bb5dbb6d6adfc135238063bebd1c3"}, "benchee": {:hex, :benchee, "1.0.1", "66b211f9bfd84bd97e6d1beaddf8fc2312aaabe192f776e8931cb0c16f53a521", [:mix], [{:deep_merge, "~> 1.0", [hex: :deep_merge, repo: "hexpm", optional: false]}], "hexpm", "3ad58ae787e9c7c94dd7ceda3b587ec2c64604563e049b2a0e8baafae832addb"}, "bunt": {:hex, :bunt, "0.2.0", "951c6e801e8b1d2cbe58ebbd3e616a869061ddadcc4863d0a2182541acae9a38", [:mix], [], "hexpm", "7af5c7e09fe1d40f76c8e4f9dd2be7cebd83909f31fee7cd0e9eadc567da8353"}, - "cachex": {:hex, :cachex, "3.2.0", "a596476c781b0646e6cb5cd9751af2e2974c3e0d5498a8cab71807618b74fe2f", [:mix], [{:eternal, "~> 1.2", [hex: :eternal, repo: "hexpm", optional: false]}, {:jumper, "~> 1.0", [hex: :jumper, repo: "hexpm", optional: false]}, {:sleeplocks, "~> 1.1", [hex: :sleeplocks, repo: "hexpm", optional: false]}, {:unsafe, "~> 1.0", [hex: :unsafe, repo: "hexpm", optional: false]}], "hexpm"}, + "cachex": {:hex, :cachex, "3.2.0", "a596476c781b0646e6cb5cd9751af2e2974c3e0d5498a8cab71807618b74fe2f", [:mix], [{:eternal, "~> 1.2", [hex: :eternal, repo: "hexpm", optional: false]}, {:jumper, "~> 1.0", [hex: :jumper, repo: "hexpm", optional: false]}, {:sleeplocks, "~> 1.1", [hex: :sleeplocks, repo: "hexpm", optional: false]}, {:unsafe, "~> 1.0", [hex: :unsafe, repo: "hexpm", optional: false]}], "hexpm", "aef93694067a43697ae0531727e097754a9e992a1e7946296f5969d6dd9ac986"}, "calendar": {:hex, :calendar, "0.17.6", "ec291cb2e4ba499c2e8c0ef5f4ace974e2f9d02ae9e807e711a9b0c7850b9aee", [:mix], [{:tzdata, "~> 0.5.20 or ~> 0.1.201603 or ~> 1.0", [hex: :tzdata, repo: "hexpm", optional: false]}], "hexpm", "738d0e17a93c2ccfe4ddc707bdc8e672e9074c8569498483feb1c4530fb91b2b"}, "captcha": {:git, "https://git.pleroma.social/pleroma/elixir-libraries/elixir-captcha.git", "e0f16822d578866e186a0974d65ad58cddc1e2ab", [ref: "e0f16822d578866e186a0974d65ad58cddc1e2ab"]}, "certifi": {:hex, :certifi, "2.5.1", "867ce347f7c7d78563450a18a6a28a8090331e77fa02380b4a21962a65d36ee5", [:rebar3], [{:parse_trans, "~>3.3", [hex: :parse_trans, repo: "hexpm", optional: false]}], "hexpm", "805abd97539caf89ec6d4732c91e62ba9da0cda51ac462380bbd28ee697a8c42"}, From 8176ca9e4026d2de4fa0ab385b8c3f77e7f81daf Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Sun, 15 Mar 2020 17:00:54 +0100 Subject: [PATCH 11/14] static_fe: Sanitize HTML in users --- lib/pleroma/user.ex | 24 +++++++++++++++++++ .../web/activity_pub/views/user_view.ex | 7 +----- .../web/admin_api/views/account_view.ex | 4 ++-- .../web/mastodon_api/views/account_view.ex | 19 ++++----------- .../web/static_fe/static_fe_controller.ex | 4 ++-- 5 files changed, 33 insertions(+), 25 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index db510d957..911dde6e2 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -16,6 +16,7 @@ defmodule Pleroma.User do alias Pleroma.Conversation.Participation alias Pleroma.Delivery alias Pleroma.FollowingRelationship + alias Pleroma.HTML alias Pleroma.Keys alias Pleroma.Notification alias Pleroma.Object @@ -2032,4 +2033,27 @@ def set_invisible(user, invisible) do |> validate_required([:invisible]) |> update_and_set_cache() end + + def sanitize_html(%User{} = user) do + sanitize_html(user, nil) + end + + # User data that mastodon isn't filtering (treated as plaintext): + # - field name + # - display name + def sanitize_html(%User{} = user, filter) do + fields = + user + |> User.fields() + |> Enum.map(fn %{"name" => name, "value" => value} -> + %{ + "name" => name, + "value" => HTML.filter_tags(value, Pleroma.HTML.Scrubber.LinksOnly) + } + end) + + user + |> Map.put(:bio, HTML.filter_tags(user.bio, filter)) + |> Map.put(:fields, fields) + end end diff --git a/lib/pleroma/web/activity_pub/views/user_view.ex b/lib/pleroma/web/activity_pub/views/user_view.ex index c0358b678..bc21ac6c7 100644 --- a/lib/pleroma/web/activity_pub/views/user_view.ex +++ b/lib/pleroma/web/activity_pub/views/user_view.ex @@ -73,6 +73,7 @@ def render("user.json", %{user: user}) do {:ok, _, public_key} = Keys.keys_from_pem(user.keys) public_key = :public_key.pem_entry_encode(:SubjectPublicKeyInfo, public_key) public_key = :public_key.pem_encode([public_key]) + user = User.sanitize_html(user) endpoints = render("endpoints.json", %{user: user}) @@ -81,12 +82,6 @@ def render("user.json", %{user: user}) do fields = user |> User.fields() - |> Enum.map(fn %{"name" => name, "value" => value} -> - %{ - "name" => Pleroma.HTML.strip_tags(name), - "value" => Pleroma.HTML.filter_tags(value, Pleroma.HTML.Scrubber.LinksOnly) - } - end) |> Enum.map(&Map.put(&1, "type", "PropertyValue")) %{ diff --git a/lib/pleroma/web/admin_api/views/account_view.ex b/lib/pleroma/web/admin_api/views/account_view.ex index 619390ef4..1e03849de 100644 --- a/lib/pleroma/web/admin_api/views/account_view.ex +++ b/lib/pleroma/web/admin_api/views/account_view.ex @@ -5,7 +5,6 @@ defmodule Pleroma.Web.AdminAPI.AccountView do use Pleroma.Web, :view - alias Pleroma.HTML alias Pleroma.User alias Pleroma.Web.AdminAPI.AccountView alias Pleroma.Web.MediaProxy @@ -26,7 +25,8 @@ def render("index.json", %{users: users}) do def render("show.json", %{user: user}) do avatar = User.avatar_url(user) |> MediaProxy.url() - display_name = HTML.strip_tags(user.name || user.nickname) + display_name = Pleroma.HTML.strip_tags(user.name || user.nickname) + user = User.sanitize_html(user, FastSanitize.Sanitizer.StripTags) %{ "id" => user.id, diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index 6dc191250..341dc2c91 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -5,7 +5,6 @@ defmodule Pleroma.Web.MastodonAPI.AccountView do use Pleroma.Web, :view - alias Pleroma.HTML alias Pleroma.User alias Pleroma.Web.CommonAPI.Utils alias Pleroma.Web.MastodonAPI.AccountView @@ -67,6 +66,7 @@ def render("relationships.json", %{user: user, targets: targets}) do end defp do_render("show.json", %{user: user} = opts) do + user = User.sanitize_html(user, User.html_filter_policy(opts[:for])) display_name = user.name || user.nickname image = User.avatar_url(user) |> MediaProxy.url() @@ -100,17 +100,6 @@ defp do_render("show.json", %{user: user} = opts) do } end) - fields = - user - |> User.fields() - |> Enum.map(fn %{"name" => name, "value" => value} -> - %{ - "name" => name, - "value" => Pleroma.HTML.filter_tags(value, Pleroma.HTML.Scrubber.LinksOnly) - } - end) - - bio = HTML.filter_tags(user.bio, User.html_filter_policy(opts[:for])) relationship = render("relationship.json", %{user: opts[:for], target: user}) %{ @@ -123,17 +112,17 @@ defp do_render("show.json", %{user: user} = opts) do followers_count: followers_count, following_count: following_count, statuses_count: user.note_count, - note: bio || "", + note: user.bio || "", url: User.profile_url(user), avatar: image, avatar_static: image, header: header, header_static: header, emojis: emojis, - fields: fields, + fields: user.fields, bot: bot, source: %{ - note: HTML.strip_tags((user.bio || "") |> String.replace("
", "\n")), + note: Pleroma.HTML.strip_tags((user.bio || "") |> String.replace("
", "\n")), sensitive: false, fields: user.raw_fields, pleroma: %{ diff --git a/lib/pleroma/web/static_fe/static_fe_controller.ex b/lib/pleroma/web/static_fe/static_fe_controller.ex index 0b77f949c..7f9464268 100644 --- a/lib/pleroma/web/static_fe/static_fe_controller.ex +++ b/lib/pleroma/web/static_fe/static_fe_controller.ex @@ -66,7 +66,7 @@ defp represent(%Activity{object: %Object{data: data}} = activity, selected) do end %{ - user: user, + user: User.sanitize_html(user), title: get_title(activity.object), content: content, attachment: data["attachment"], @@ -120,7 +120,7 @@ def show(%{assigns: %{username_or_id: username_or_id}} = conn, params) do next_page_id = List.last(timeline) && List.last(timeline).id render(conn, "profile.html", %{ - user: user, + user: User.sanitize_html(user), timeline: timeline, prev_page_id: prev_page_id, next_page_id: next_page_id, From ba90a6d3e5c1f0a69044ec68748aeed870a085c0 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Mon, 16 Mar 2020 14:01:35 +0300 Subject: [PATCH 12/14] removing from descriptions.exs deprecated settings --- config/benchmark.exs | 2 -- config/description.exs | 32 -------------------------------- 2 files changed, 34 deletions(-) diff --git a/config/benchmark.exs b/config/benchmark.exs index 84c6782a2..ff59395cf 100644 --- a/config/benchmark.exs +++ b/config/benchmark.exs @@ -61,8 +61,6 @@ config :web_push_encryption, :http_client, Pleroma.Web.WebPushHttpClientMock -config :pleroma_job_queue, disabled: true - config :pleroma, Pleroma.ScheduledActivity, daily_user_limit: 2, total_user_limit: 3, diff --git a/config/description.exs b/config/description.exs index c0e403b2e..732c76734 100644 --- a/config/description.exs +++ b/config/description.exs @@ -1780,25 +1780,6 @@ } ] }, - %{ - group: :pleroma_job_queue, - key: :queues, - type: :group, - description: "[Deprecated] Replaced with `Oban`/`:queues` (keeping the same format)" - }, - %{ - group: :pleroma, - key: Pleroma.Web.Federator.RetryQueue, - type: :group, - description: "[Deprecated] See `Oban` and `:workers` sections for configuration notes", - children: [ - %{ - key: :max_retries, - type: :integer, - description: "[Deprecated] Replaced as `Oban`/`:queues`/`:outgoing_federation` value" - } - ] - }, %{ group: :pleroma, key: Oban, @@ -2577,19 +2558,6 @@ } ] }, - %{ - group: :tesla, - type: :group, - description: "Tesla settings", - children: [ - %{ - key: :adapter, - type: :module, - description: "Tesla adapter", - suggestions: [Tesla.Adapter.Hackney] - } - ] - }, %{ group: :pleroma, key: :chat, From dc2ec84c0fe41e8af3ee5b961fa86c66c483e5b4 Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Mon, 16 Mar 2020 14:19:36 +0300 Subject: [PATCH 13/14] warnings fix --- test/plugs/rate_limiter_test.exs | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/test/plugs/rate_limiter_test.exs b/test/plugs/rate_limiter_test.exs index 81e2009c8..c6e494c13 100644 --- a/test/plugs/rate_limiter_test.exs +++ b/test/plugs/rate_limiter_test.exs @@ -51,7 +51,7 @@ test "it restricts based on config values" do Config.put([:rate_limit, limiter_name], {scale, limit}) plug_opts = RateLimiter.init(name: limiter_name) - conn = conn(:get, "/") + conn = build_conn(:get, "/") for i <- 1..5 do conn = RateLimiter.call(conn, plug_opts) @@ -65,7 +65,7 @@ test "it restricts based on config values" do Process.sleep(50) - conn = conn(:get, "/") + conn = build_conn(:get, "/") conn = RateLimiter.call(conn, plug_opts) assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts) @@ -85,7 +85,7 @@ test "`bucket_name` option overrides default bucket name" do base_bucket_name = "#{limiter_name}:group1" plug_opts = RateLimiter.init(name: limiter_name, bucket_name: base_bucket_name) - conn = conn(:get, "/") + conn = build_conn(:get, "/") RateLimiter.call(conn, plug_opts) assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, plug_opts) @@ -99,9 +99,9 @@ test "`params` option allows different queries to be tracked independently" do plug_opts = RateLimiter.init(name: limiter_name, params: ["id"]) - conn = conn(:get, "/?id=1") + conn = build_conn(:get, "/?id=1") conn = Plug.Conn.fetch_query_params(conn) - conn_2 = conn(:get, "/?id=2") + conn_2 = build_conn(:get, "/?id=2") RateLimiter.call(conn, plug_opts) assert {1, 4} = RateLimiter.inspect_bucket(conn, limiter_name, plug_opts) @@ -120,9 +120,9 @@ test "it supports combination of options modifying bucket name" do id = "100" - conn = conn(:get, "/?id=#{id}") + conn = build_conn(:get, "/?id=#{id}") conn = Plug.Conn.fetch_query_params(conn) - conn_2 = conn(:get, "/?id=#{101}") + conn_2 = build_conn(:get, "/?id=#{101}") RateLimiter.call(conn, plug_opts) assert {1, 4} = RateLimiter.inspect_bucket(conn, base_bucket_name, plug_opts) @@ -138,8 +138,8 @@ test "are restricted based on remote IP" do plug_opts = RateLimiter.init(name: limiter_name) - conn = %{conn(:get, "/") | remote_ip: {127, 0, 0, 2}} - conn_2 = %{conn(:get, "/") | remote_ip: {127, 0, 0, 3}} + conn = %{build_conn(:get, "/") | remote_ip: {127, 0, 0, 2}} + conn_2 = %{build_conn(:get, "/") | remote_ip: {127, 0, 0, 3}} for i <- 1..5 do conn = RateLimiter.call(conn, plug_opts) @@ -179,7 +179,7 @@ test "can have limits separate from unauthenticated connections" do plug_opts = RateLimiter.init(name: limiter_name) user = insert(:user) - conn = conn(:get, "/") |> assign(:user, user) + conn = build_conn(:get, "/") |> assign(:user, user) for i <- 1..5 do conn = RateLimiter.call(conn, plug_opts) @@ -201,10 +201,10 @@ test "different users are counted independently" do plug_opts = RateLimiter.init(name: limiter_name) user = insert(:user) - conn = conn(:get, "/") |> assign(:user, user) + conn = build_conn(:get, "/") |> assign(:user, user) user_2 = insert(:user) - conn_2 = conn(:get, "/") |> assign(:user, user_2) + conn_2 = build_conn(:get, "/") |> assign(:user, user_2) for i <- 1..5 do conn = RateLimiter.call(conn, plug_opts) @@ -230,8 +230,8 @@ test "doesn't crash due to a race condition when multiple requests are made at t opts = RateLimiter.init(name: limiter_name) - conn = conn(:get, "/") - conn_2 = conn(:get, "/") + conn = build_conn(:get, "/") + conn_2 = build_conn(:get, "/") %Task{pid: pid1} = task1 = From 7829de6da4b2782a4ae2124ff05787d500bbb990 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Mon, 16 Mar 2020 17:26:28 +0300 Subject: [PATCH 14/14] gitlab: create templates for bug reports and release MRs --- .gitlab/issue_templates/Bug.md | 20 ++++++++++++++++++++ .gitlab/merge_request_templates/Release.md | 5 +++++ 2 files changed, 25 insertions(+) create mode 100644 .gitlab/issue_templates/Bug.md create mode 100644 .gitlab/merge_request_templates/Release.md diff --git a/.gitlab/issue_templates/Bug.md b/.gitlab/issue_templates/Bug.md new file mode 100644 index 000000000..66fbc510e --- /dev/null +++ b/.gitlab/issue_templates/Bug.md @@ -0,0 +1,20 @@ + + +### Environment + +* Installation type: + - [ ] OTP + - [ ] From source +* Pleroma version (could be found in the "Version" tab of settings in Pleroma-FE): +* Elixir version (`elixir -v` for from source installations, N/A for OTP): +* Operating system: +* PostgreSQL version (`postgres -V`): + + +### Bug description diff --git a/.gitlab/merge_request_templates/Release.md b/.gitlab/merge_request_templates/Release.md new file mode 100644 index 000000000..237f74e00 --- /dev/null +++ b/.gitlab/merge_request_templates/Release.md @@ -0,0 +1,5 @@ +### Release checklist +* [ ] Bump version in `mix.exs` +* [ ] Compile a changelog +* [ ] Create an MR with an announcement to pleroma.social +* [ ] Tag the release