From 320ca7b11e163d059a3f181e2d6eb5ea300f5b55 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Tue, 19 Jun 2018 00:36:40 +0000 Subject: [PATCH 1/5] user: when processing a block in User.block(), ensure all follow relationships are broken this is needed for activitypub conformance ref #213 --- lib/pleroma/user.ex | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index b27397e13..bfa5d78a4 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -505,12 +505,25 @@ def search(query, resolve) do Repo.all(q) end - def block(user, %{ap_id: ap_id}) do - blocks = user.info["blocks"] || [] - new_blocks = Enum.uniq([ap_id | blocks]) - new_info = Map.put(user.info, "blocks", new_blocks) + def block(blocker, %User{ap_id: ap_id} = blocked) do + # sever any follow relationships to prevent leaks per activitypub (Pleroma issue #213) + blocker = + if following?(blocker, blocked) do + {:ok, blocker, _} = unfollow(blocker, blocked) + blocker + else + blocker + end - cs = User.info_changeset(user, %{info: new_info}) + if following?(blocked, blocker) do + unfollow(blocked, blocker) + end + + blocks = blocker.info["blocks"] || [] + new_blocks = Enum.uniq([ap_id | blocks]) + new_info = Map.put(blocker.info, "blocks", new_blocks) + + cs = User.info_changeset(blocker, %{info: new_info}) update_and_set_cache(cs) end From e9ed7eb963d03a67ee12b30c6d5697e62bf3e0a1 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Tue, 19 Jun 2018 00:45:24 +0000 Subject: [PATCH 2/5] tests: prove that the block code is properly destroying follow relationships --- test/user_test.exs | 55 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/test/user_test.exs b/test/user_test.exs index 200352981..352a16687 100644 --- a/test/user_test.exs +++ b/test/user_test.exs @@ -359,6 +359,61 @@ test "it unblocks users" do refute User.blocks?(user, blocked_user) end + + test "blocks tear down cyclical follow relationships" do + blocker = insert(:user) + blocked = insert(:user) + + {:ok, blocker} = User.follow(blocker, blocked) + {:ok, blocked} = User.follow(blocked, blocker) + + assert User.following?(blocker, blocked) + assert User.following?(blocked, blocker) + + {:ok, blocker} = User.block(blocker, blocked) + blocked = Repo.get(User, blocked.id) + + assert User.blocks?(blocker, blocked) + + refute User.following?(blocker, blocked) + refute User.following?(blocked, blocker) + end + + test "blocks tear down blocker->blocked follow relationships" do + blocker = insert(:user) + blocked = insert(:user) + + {:ok, blocker} = User.follow(blocker, blocked) + + assert User.following?(blocker, blocked) + refute User.following?(blocked, blocker) + + {:ok, blocker} = User.block(blocker, blocked) + blocked = Repo.get(User, blocked.id) + + assert User.blocks?(blocker, blocked) + + refute User.following?(blocker, blocked) + refute User.following?(blocked, blocker) + end + + test "blocks tear down blocked->blocker follow relationships" do + blocker = insert(:user) + blocked = insert(:user) + + {:ok, blocked} = User.follow(blocked, blocker) + + refute User.following?(blocker, blocked) + assert User.following?(blocked, blocker) + + {:ok, blocker} = User.block(blocker, blocked) + blocked = Repo.get(User, blocked.id) + + assert User.blocks?(blocker, blocked) + + refute User.following?(blocker, blocked) + refute User.following?(blocked, blocker) + end end describe "domain blocking" do From 3707a7fa4288c5bdc4c1031728d8a10165c975b4 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Tue, 19 Jun 2018 00:57:57 +0000 Subject: [PATCH 3/5] tests: transmogrifier: ensure incoming blocks have the same effect as local blocks --- test/web/activity_pub/transmogrifier_test.exs | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/test/web/activity_pub/transmogrifier_test.exs b/test/web/activity_pub/transmogrifier_test.exs index 7e771b9f8..1e135d65f 100644 --- a/test/web/activity_pub/transmogrifier_test.exs +++ b/test/web/activity_pub/transmogrifier_test.exs @@ -382,6 +382,37 @@ test "it works for incoming blocks" do assert User.blocks?(blocker, user) end + test "incoming blocks successfully tear down any follow relationship" do + blocker = insert(:user) + blocked = insert(:user) + + data = + File.read!("test/fixtures/mastodon-block-activity.json") + |> Poison.decode!() + |> Map.put("object", blocked.ap_id) + |> Map.put("actor", blocker.ap_id) + + {:ok, blocker} = User.follow(blocker, blocked) + {:ok, blocked} = User.follow(blocked, blocker) + + assert User.following?(blocker, blocked) + assert User.following?(blocked, blocker) + + {:ok, %Activity{data: data, local: false}} = Transmogrifier.handle_incoming(data) + + assert data["type"] == "Block" + assert data["object"] == blocked.ap_id + assert data["actor"] == blocker.ap_id + + blocker = User.get_by_ap_id(data["actor"]) + blocked = User.get_by_ap_id(data["object"]) + + assert User.blocks?(blocker, blocked) + + refute User.following?(blocker, blocked) + refute User.following?(blocked, blocker) + end + test "it works for incoming unblocks with an existing block" do user = insert(:user) From 056305dfa745f679ce0082c08abb06725432dc5d Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Tue, 19 Jun 2018 08:31:06 +0000 Subject: [PATCH 4/5] user: add helper function to fetch a user given only an ap_id (fix tests) --- lib/pleroma/user.ex | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex index bfa5d78a4..aba8742a0 100644 --- a/lib/pleroma/user.ex +++ b/lib/pleroma/user.ex @@ -527,6 +527,11 @@ def block(blocker, %User{ap_id: ap_id} = blocked) do update_and_set_cache(cs) end + # helper to handle the block given only an actor's AP id + def block(blocker, %{ap_id: ap_id}) do + block(blocker, User.get_by_ap_id(ap_id)) + end + def unblock(user, %{ap_id: ap_id}) do blocks = user.info["blocks"] || [] new_blocks = List.delete(blocks, ap_id) From 590e8d555557c7f375c15bcbb00ea46cf2dcc4b9 Mon Sep 17 00:00:00 2001 From: William Pitcock Date: Tue, 19 Jun 2018 08:53:54 +0000 Subject: [PATCH 5/5] tests: fix a completely bogus mastodon api test --- test/web/mastodon_api/account_view_test.exs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/web/mastodon_api/account_view_test.exs b/test/web/mastodon_api/account_view_test.exs index 597690bf7..f7b8d7438 100644 --- a/test/web/mastodon_api/account_view_test.exs +++ b/test/web/mastodon_api/account_view_test.exs @@ -60,7 +60,7 @@ test "represent a relationship" do expected = %{ id: to_string(other_user.id), - following: true, + following: false, followed_by: false, blocking: true, muting: false,