commit: 02cdedbf9fdf27d8dca78caf75f6413cd2566e3e
parent: 5999780e8247b8f26b270ac3285f960057fbc9ea
Author: Haelwenn <contact+git.pleroma.social@hacktivis.me>
Date: Sat, 13 Jul 2019 10:22:19 +0000
Merge branch 'fix/ap-hide-follows' into 'develop'
ActivityPub Controller: Change how hiding follows/followers is represented
See merge request pleroma/pleroma!1406
Diffstat:
6 files changed, 160 insertions(+), 32 deletions(-)
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -8,6 +8,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- **Breaking:** Configuration: A setting to explicitly disable the mailer was added, defaulting to true, if you are using a mailer add `config :pleroma, Pleroma.Emails.Mailer, enabled: true` to your config
- Configuration: OpenGraph and TwitterCard providers enabled by default
- Configuration: Filter.AnonymizeFilename added ability to retain file extension with custom text
+- Federation: Return 403 errors when trying to request pages from a user's follower/following collections if they have `hide_followers`/`hide_follows` set
- NodeInfo: Return `skipThreadContainment` in `metadata` for the `skip_thread_containment` option
### Fixed
@@ -16,6 +17,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
- Mastodon API: Handling of search timeouts (`/api/v1/search` and `/api/v2/search`)
- Mastodon API: Embedded relationships not being properly rendered in the Account entity of Status entity
- Mastodon API: Add `account_id`, `type`, `offset`, and `limit` to search API (`/api/v1/search` and `/api/v2/search`)
+- ActivityPub C2S: follower/following collection pages being inaccessible even when authentifucated if `hide_followers`/ `hide_follows` was set
### Added
- MRF: Support for priming the mediaproxy cache (`Pleroma.Web.ActivityPub.MRF.MediaProxyWarmingPolicy`)
diff --git a/lib/pleroma/web/activity_pub/activity_pub_controller.ex b/lib/pleroma/web/activity_pub/activity_pub_controller.ex
@@ -103,43 +103,57 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
end
end
- def following(conn, %{"nickname" => nickname, "page" => page}) do
+ def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "page" => page}) do
with %User{} = user <- User.get_cached_by_nickname(nickname),
- {:ok, user} <- User.ensure_keys_present(user) do
+ {user, for_user} <- ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user),
+ {:show_follows, true} <-
+ {:show_follows, (for_user && for_user == user) || !user.info.hide_follows} do
{page, _} = Integer.parse(page)
conn
|> put_resp_header("content-type", "application/activity+json")
- |> json(UserView.render("following.json", %{user: user, page: page}))
+ |> json(UserView.render("following.json", %{user: user, page: page, for: for_user}))
+ else
+ {:show_follows, _} ->
+ conn
+ |> put_resp_header("content-type", "application/activity+json")
+ |> send_resp(403, "")
end
end
- def following(conn, %{"nickname" => nickname}) do
+ def following(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) do
with %User{} = user <- User.get_cached_by_nickname(nickname),
- {:ok, user} <- User.ensure_keys_present(user) do
+ {user, for_user} <- ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user) do
conn
|> put_resp_header("content-type", "application/activity+json")
- |> json(UserView.render("following.json", %{user: user}))
+ |> json(UserView.render("following.json", %{user: user, for: for_user}))
end
end
- def followers(conn, %{"nickname" => nickname, "page" => page}) do
+ def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname, "page" => page}) do
with %User{} = user <- User.get_cached_by_nickname(nickname),
- {:ok, user} <- User.ensure_keys_present(user) do
+ {user, for_user} <- ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user),
+ {:show_followers, true} <-
+ {:show_followers, (for_user && for_user == user) || !user.info.hide_followers} do
{page, _} = Integer.parse(page)
conn
|> put_resp_header("content-type", "application/activity+json")
- |> json(UserView.render("followers.json", %{user: user, page: page}))
+ |> json(UserView.render("followers.json", %{user: user, page: page, for: for_user}))
+ else
+ {:show_followers, _} ->
+ conn
+ |> put_resp_header("content-type", "application/activity+json")
+ |> send_resp(403, "")
end
end
- def followers(conn, %{"nickname" => nickname}) do
+ def followers(%{assigns: %{user: for_user}} = conn, %{"nickname" => nickname}) do
with %User{} = user <- User.get_cached_by_nickname(nickname),
- {:ok, user} <- User.ensure_keys_present(user) do
+ {user, for_user} <- ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user) do
conn
|> put_resp_header("content-type", "application/activity+json")
- |> json(UserView.render("followers.json", %{user: user}))
+ |> json(UserView.render("followers.json", %{user: user, for: for_user}))
end
end
@@ -325,4 +339,17 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubController do
conn
end
+
+ defp ensure_user_keys_present_and_maybe_refresh_for_user(user, for_user) do
+ {:ok, new_user} = User.ensure_keys_present(user)
+
+ for_user =
+ if new_user != user and match?(%User{}, for_user) do
+ User.get_cached_by_nickname(for_user.nickname)
+ else
+ for_user
+ end
+
+ {new_user, for_user}
+ end
end
diff --git a/lib/pleroma/web/activity_pub/views/user_view.ex b/lib/pleroma/web/activity_pub/views/user_view.ex
@@ -98,29 +98,31 @@ defmodule Pleroma.Web.ActivityPub.UserView do
|> Map.merge(Utils.make_json_ld_header())
end
- def render("following.json", %{user: user, page: page}) do
+ def render("following.json", %{user: user, page: page} = opts) do
+ showing = (opts[:for] && opts[:for] == user) || !user.info.hide_follows
query = User.get_friends_query(user)
query = from(user in query, select: [:ap_id])
following = Repo.all(query)
total =
- if !user.info.hide_follows do
+ if showing do
length(following)
else
0
end
- collection(following, "#{user.ap_id}/following", page, !user.info.hide_follows, total)
+ collection(following, "#{user.ap_id}/following", page, showing, total)
|> Map.merge(Utils.make_json_ld_header())
end
- def render("following.json", %{user: user}) do
+ def render("following.json", %{user: user} = opts) do
+ showing = (opts[:for] && opts[:for] == user) || !user.info.hide_follows
query = User.get_friends_query(user)
query = from(user in query, select: [:ap_id])
following = Repo.all(query)
total =
- if !user.info.hide_follows do
+ if showing do
length(following)
else
0
@@ -130,34 +132,43 @@ defmodule Pleroma.Web.ActivityPub.UserView do
"id" => "#{user.ap_id}/following",
"type" => "OrderedCollection",
"totalItems" => total,
- "first" => collection(following, "#{user.ap_id}/following", 1, !user.info.hide_follows)
+ "first" =>
+ if showing do
+ collection(following, "#{user.ap_id}/following", 1, !user.info.hide_follows)
+ else
+ "#{user.ap_id}/following?page=1"
+ end
}
|> Map.merge(Utils.make_json_ld_header())
end
- def render("followers.json", %{user: user, page: page}) do
+ def render("followers.json", %{user: user, page: page} = opts) do
+ showing = (opts[:for] && opts[:for] == user) || !user.info.hide_followers
+
query = User.get_followers_query(user)
query = from(user in query, select: [:ap_id])
followers = Repo.all(query)
total =
- if !user.info.hide_followers do
+ if showing do
length(followers)
else
0
end
- collection(followers, "#{user.ap_id}/followers", page, !user.info.hide_followers, total)
+ collection(followers, "#{user.ap_id}/followers", page, showing, total)
|> Map.merge(Utils.make_json_ld_header())
end
- def render("followers.json", %{user: user}) do
+ def render("followers.json", %{user: user} = opts) do
+ showing = (opts[:for] && opts[:for] == user) || !user.info.hide_followers
+
query = User.get_followers_query(user)
query = from(user in query, select: [:ap_id])
followers = Repo.all(query)
total =
- if !user.info.hide_followers do
+ if showing do
length(followers)
else
0
@@ -168,7 +179,11 @@ defmodule Pleroma.Web.ActivityPub.UserView do
"type" => "OrderedCollection",
"totalItems" => total,
"first" =>
- collection(followers, "#{user.ap_id}/followers", 1, !user.info.hide_followers, total)
+ if showing do
+ collection(followers, "#{user.ap_id}/followers", 1, showing, total)
+ else
+ "#{user.ap_id}/followers?page=1"
+ end
}
|> Map.merge(Utils.make_json_ld_header())
end
diff --git a/lib/pleroma/web/router.ex b/lib/pleroma/web/router.ex
@@ -623,8 +623,6 @@ defmodule Pleroma.Web.Router do
# XXX: not really ostatus
pipe_through(:ostatus)
- get("/users/:nickname/followers", ActivityPubController, :followers)
- get("/users/:nickname/following", ActivityPubController, :following)
get("/users/:nickname/outbox", ActivityPubController, :outbox)
get("/objects/:uuid/likes", ActivityPubController, :object_likes)
end
@@ -656,6 +654,12 @@ defmodule Pleroma.Web.Router do
pipe_through(:oauth_write)
post("/users/:nickname/outbox", ActivityPubController, :update_outbox)
end
+
+ scope [] do
+ pipe_through(:oauth_read_or_public)
+ get("/users/:nickname/followers", ActivityPubController, :followers)
+ get("/users/:nickname/following", ActivityPubController, :following)
+ end
end
scope "/relay", Pleroma.Web.ActivityPub do
diff --git a/test/web/activity_pub/activity_pub_controller_test.exs b/test/web/activity_pub/activity_pub_controller_test.exs
@@ -12,6 +12,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
alias Pleroma.Web.ActivityPub.ObjectView
alias Pleroma.Web.ActivityPub.UserView
alias Pleroma.Web.ActivityPub.Utils
+ alias Pleroma.Web.CommonAPI
setup_all do
Tesla.Mock.mock_global(fn env -> apply(HttpRequestMock, :request, [env]) end)
@@ -551,7 +552,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
assert result["first"]["orderedItems"] == [user.ap_id]
end
- test "it returns returns empty if the user has 'hide_followers' set", %{conn: conn} do
+ test "it returns returns a uri if the user has 'hide_followers' set", %{conn: conn} do
user = insert(:user)
user_two = insert(:user, %{info: %{hide_followers: true}})
User.follow(user, user_two)
@@ -561,8 +562,35 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
|> get("/users/#{user_two.nickname}/followers")
|> json_response(200)
- assert result["first"]["orderedItems"] == []
- assert result["totalItems"] == 0
+ 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",
+ %{conn: conn} do
+ user = insert(:user, %{info: %{hide_followers: true}})
+
+ result =
+ conn
+ |> get("/users/#{user.nickname}/followers?page=1")
+
+ assert result.status == 403
+ assert result.resp_body == ""
+ end
+
+ test "it renders the page, if the user has 'hide_followers' set and the request is authenticated with the same user",
+ %{conn: conn} do
+ user = insert(:user, %{info: %{hide_followers: true}})
+ other_user = insert(:user)
+ {:ok, _other_user, user, _activity} = CommonAPI.follow(other_user, user)
+
+ result =
+ conn
+ |> assign(:user, user)
+ |> get("/users/#{user.nickname}/followers?page=1")
+ |> json_response(200)
+
+ assert result["totalItems"] == 1
+ assert result["orderedItems"] == [other_user.ap_id]
end
test "it works for more than 10 users", %{conn: conn} do
@@ -606,7 +634,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
assert result["first"]["orderedItems"] == [user_two.ap_id]
end
- test "it returns returns empty if the user has 'hide_follows' set", %{conn: conn} do
+ test "it returns a uri if the user has 'hide_follows' set", %{conn: conn} do
user = insert(:user, %{info: %{hide_follows: true}})
user_two = insert(:user)
User.follow(user, user_two)
@@ -616,8 +644,35 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubControllerTest do
|> get("/users/#{user.nickname}/following")
|> json_response(200)
- assert result["first"]["orderedItems"] == []
- assert result["totalItems"] == 0
+ 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",
+ %{conn: conn} do
+ user = insert(:user, %{info: %{hide_follows: true}})
+
+ result =
+ conn
+ |> get("/users/#{user.nickname}/following?page=1")
+
+ assert result.status == 403
+ assert result.resp_body == ""
+ end
+
+ test "it renders the page, if the user has 'hide_follows' set and the request is authenticated with the same user",
+ %{conn: conn} do
+ user = insert(:user, %{info: %{hide_follows: true}})
+ other_user = insert(:user)
+ {:ok, user, _other_user, _activity} = CommonAPI.follow(user, other_user)
+
+ result =
+ conn
+ |> assign(:user, user)
+ |> get("/users/#{user.nickname}/following?page=1")
+ |> json_response(200)
+
+ assert result["totalItems"] == 1
+ assert result["orderedItems"] == [other_user.ap_id]
end
test "it works for more than 10 users", %{conn: conn} do
diff --git a/test/web/activity_pub/views/user_view_test.exs b/test/web/activity_pub/views/user_view_test.exs
@@ -8,6 +8,7 @@ defmodule Pleroma.Web.ActivityPub.UserViewTest do
alias Pleroma.User
alias Pleroma.Web.ActivityPub.UserView
+ alias Pleroma.Web.CommonAPI
test "Renders a user, including the public key" do
user = insert(:user)
@@ -82,4 +83,28 @@ defmodule Pleroma.Web.ActivityPub.UserViewTest do
refute result["endpoints"]["oauthTokenEndpoint"]
end
end
+
+ describe "followers" do
+ test "sets totalItems to zero when followers are hidden" do
+ user = insert(:user)
+ other_user = insert(:user)
+ {:ok, _other_user, user, _activity} = CommonAPI.follow(other_user, user)
+ assert %{"totalItems" => 1} = UserView.render("followers.json", %{user: user})
+ info = Map.put(user.info, :hide_followers, true)
+ user = Map.put(user, :info, info)
+ assert %{"totalItems" => 0} = UserView.render("followers.json", %{user: user})
+ end
+ end
+
+ describe "following" do
+ test "sets totalItems to zero when follows are hidden" do
+ user = insert(:user)
+ other_user = insert(:user)
+ {:ok, user, _other_user, _activity} = CommonAPI.follow(user, other_user)
+ assert %{"totalItems" => 1} = UserView.render("following.json", %{user: user})
+ info = Map.put(user.info, :hide_follows, true)
+ user = Map.put(user, :info, info)
+ assert %{"totalItems" => 0} = UserView.render("following.json", %{user: user})
+ end
+ end
end
