No OneTemporary
Actions

Authored By

Unknown

Size

38 KB

Referenced Files

None

Subscribers

None

View Options

	diff --git a/changelog.d/actor-containment.security b/changelog.d/actor-containment.security
	new file mode 100644
	index 000000000..58dbca4bc
	--- /dev/null
	+++ b/changelog.d/actor-containment.security
	@@ -0,0 +1 @@
	+Reject remote actors whose inbox, shared inbox, followers, following or featured collection, or key id is on another host
	diff --git a/changelog.d/fetch-id-containment.security b/changelog.d/fetch-id-containment.security
	new file mode 100644
	index 000000000..aee5eca95
	--- /dev/null
	+++ b/changelog.d/fetch-id-containment.security
	@@ -0,0 +1 @@
	+Only accept fetched objects whose id is the URL they were fetched from; for other ids, refetch once from the canonical id on the same host
	diff --git a/changelog.d/get-actor-crash.fix b/changelog.d/get-actor-crash.fix
	new file mode 100644
	index 000000000..591ac3ba2
	--- /dev/null
	+++ b/changelog.d/get-actor-crash.fix
	@@ -0,0 +1 @@
	+Reject fetched objects whose author has no id instead of crashing
	diff --git a/changelog.d/refetch-containment.security b/changelog.d/refetch-containment.security
	new file mode 100644
	index 000000000..bfd118ab1
	--- /dev/null
	+++ b/changelog.d/refetch-containment.security
	@@ -0,0 +1 @@
	+Do not let a refetched object (such as a poll) change its id or author
	diff --git a/changelog.d/user-fetch-display-url.fix b/changelog.d/user-fetch-display-url.fix
	new file mode 100644
	index 000000000..fcde4ecf4
	--- /dev/null
	+++ b/changelog.d/user-fetch-display-url.fix
	@@ -0,0 +1 @@
	+Update the known user instead of failing when an actor is fetched from a display URL
	diff --git a/lib/pleroma/object/containment.ex b/lib/pleroma/object/containment.ex
	index c4cc1f014..531100cbb 100644
	--- a/lib/pleroma/object/containment.ex
	+++ b/lib/pleroma/object/containment.ex
	@@ -44,6 +44,9 @@ defmodule Pleroma.Object.Containment do
	id
	end

	+ # remote data can carry any shape here; callers treat nil as "no actor"
	+ def get_actor(_), do: nil
	+
	def get_object(%{"object" => id}) when is_binary(id) do
	id
	end
	@@ -108,8 +111,46 @@ defmodule Pleroma.Object.Containment do

	def contain_origin_from_id(_id, _data), do: :error

	+ @doc """
	+ Checks that a fetched document's id is the URL it was fetched from
	+ (after redirects).
	+
	+ Fragments, a trailing slash, the case of the host and differences
	+ that our URL encoding removes before a request are ignored.
	+ Documents without an id never match.
	+
	+ Based on Akkoma's `contain_id_to_fetch/2`.
	+ """
	+ def contain_id_to_fetch(url, %{"id" => id}) when is_binary(url) and is_binary(id) do
	+ if normalize_uri(url) == normalize_uri(id), do: :ok, else: :error
	+ end
	+
	+ def contain_id_to_fetch(_url, _data), do: :error
	+
	+ defp normalize_uri(uri) do
	+ uri = uri \|> Pleroma.Utils.URIEncoding.encode_url() \|> URI.parse()
	+
	+ %URI{
	+ uri
	+ \| fragment: nil,
	+ host: uri.host && String.downcase(uri.host, :ascii),
	+ path: uri.path && String.replace_suffix(uri.path, "/", "")
	+ }
	+ \|> URI.to_string()
	+ end
	+
	def contain_child(%{"object" => %{"id" => id, "attributedTo" => _} = object}),
	do: contain_origin(id, object)

	def contain_child(_), do: :ok
	+
	+ @doc "Checks that two URIs are on the same host. URIs without a host never match."
	+ def same_origin(id, other_id) when is_binary(id) and is_binary(other_id) do
	+ case {URI.parse(id), URI.parse(other_id)} do
	+ {%URI{host: host}, %URI{host: host}} when is_binary(host) and host != "" -> :ok
	+ _ -> :error
	+ end
	+ end
	+
	+ def same_origin(_id, _other_id), do: :error
	end
	diff --git a/lib/pleroma/object/fetcher.ex b/lib/pleroma/object/fetcher.ex
	index 084d44d5e..706ee317c 100644
	--- a/lib/pleroma/object/fetcher.ex
	+++ b/lib/pleroma/object/fetcher.ex
	@@ -11,6 +11,7 @@ defmodule Pleroma.Object.Fetcher do
	alias Pleroma.Web.ActivityPub.InternalFetchActor
	alias Pleroma.Web.ActivityPub.MRF
	alias Pleroma.Web.ActivityPub.ObjectValidator
	+ alias Pleroma.Web.ActivityPub.ObjectValidators.CommonFixes
	alias Pleroma.Web.ActivityPub.Pipeline
	alias Pleroma.Web.ActivityPub.Transmogrifier
	alias Pleroma.Web.Federator
	@@ -46,9 +47,14 @@ defmodule Pleroma.Object.Fetcher do
	end
	end

	- def refetch_object(%Object{data: %{"id" => id}} = object) do
	+ # The stored id is canonical, so the refetch must not redirect, and the
	+ # remote must not swap in another object or author.
	+ # Based on Akkoma 9061d148b and fee57eb37.
	+ def refetch_object(%Object{data: %{"id" => id} = data} = object) do
	with {:local, false} <- {:local, Object.local?(object)},
	- {:ok, new_data} <- fetch_and_contain_remote_object_from_id(id),
	+ {:ok, new_data} <- fetch_and_contain(id, true),
	+ {:id, true} <- {:id, new_data["id"] == id},
	+ {:actor, true} <- {:actor, actor_of(new_data) == actor_of(data)},
	{:ok, object} <- reinject_object(object, new_data) do
	{:ok, object}
	else
	@@ -57,6 +63,8 @@ defmodule Pleroma.Object.Fetcher do
	end
	end

	+ defp actor_of(data), do: CommonFixes.fix_actor(data)["actor"]
	+
	@typep fetcher_errors ::
	:error \| :reject \| :allowed_depth \| :fetch \| :containment \| :transmogrifier

	@@ -140,17 +148,39 @@ defmodule Pleroma.Object.Fetcher do
	def fetch_and_contain_remote_object_from_id(%{"id" => id}),
	do: fetch_and_contain_remote_object_from_id(id)

	- def fetch_and_contain_remote_object_from_id(id) when is_binary(id) do
	+ def fetch_and_contain_remote_object_from_id(id) when is_binary(id),
	+ do: fetch_and_contain(id, false)
	+
	+ def fetch_and_contain_remote_object_from_id(_id),
	+ do: {:error, "id must be a string"}
	+
	+ # The fetched document's id must be the URL it came from. Display URLs
	+ # (e.g. Mastodon's /@user/123) serve the object under another id, so
	+ # if the id differs we refetch once from that id, which has to be on the
	+ # same host, must not redirect, and must then match exactly.
	+ # Based on Akkoma 8684964c5 and 940792f8b.
	+ defp fetch_and_contain(id, canonical?) do
	Logger.debug("Fetching object #{id} via AP")

	with {:scheme, true} <- {:scheme, String.starts_with?(id, "http")},
	{_, true} <- {:mrf, MRF.id_filter(id)},
	{_, :ok} <- {:local_fetch, Containment.contain_local_fetch(id)},
	- {:ok, body} <- get_object(id),
	+ {:ok, final_url, body} <- get_object(id),
	+ {_, true} <- {:redirect, not canonical? or same_url?(final_url, id)},
	{:ok, data} <- safe_json_decode(body),
	- :ok <- Containment.contain_origin_from_id(id, data) do
	+ :ok <- Containment.contain_origin_from_id(id, data),
	+ {_, _, :ok} <- {:id, data, Containment.contain_id_to_fetch(final_url, data)} do
	{:ok, data}
	else
	+ {:id, %{"id" => canonical_id}, :error} when is_binary(canonical_id) and not canonical? ->
	+ fetch_and_contain(canonical_id, true)
	+
	+ {:id, _, :error} ->
	+ {:error, :id_mismatch}
	+
	+ {:redirect, false} ->
	+ {:error, :canonical_id_redirect}
	+
	{:scheme, _} ->
	{:error, "Unsupported URI scheme"}

	@@ -168,8 +198,7 @@ defmodule Pleroma.Object.Fetcher do
	end
	end

	- def fetch_and_contain_remote_object_from_id(_id),
	- do: {:error, "id must be a string"}
	+ defp same_url?(url, other), do: Containment.contain_id_to_fetch(url, %{"id" => other}) == :ok

	defp check_crossdomain_redirect(final_host, _original_url) when is_nil(final_host) do
	{:cross_domain_redirect, false}
	@@ -195,7 +224,7 @@ defmodule Pleroma.Object.Fetcher do
	with {:cross_domain_redirect, false} <- check_crossdomain_redirect(remote_host, id),
	{_, content_type} <- List.keyfind(headers, "content-type", 0),
	{:ok, _media_type} <- verify_content_type(content_type) do
	- {:ok, body}
	+ {:ok, if(final_url in [nil, ""], do: id, else: final_url), body}
	else
	{:cross_domain_redirect, true} ->
	{:error, {:cross_domain_redirect, true}}
	@@ -209,7 +238,7 @@ defmodule Pleroma.Object.Fetcher do
	case List.keyfind(headers, "content-type", 0) do
	{_, content_type} ->
	case verify_content_type(content_type) do
	- {:ok, _} -> {:ok, body}
	+ {:ok, _} -> {:ok, id, body}
	error -> error
	end

	diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex
	index 1279e0cb4..b997e7831 100644
	--- a/lib/pleroma/web/activity_pub/activity_pub.ex
	+++ b/lib/pleroma/web/activity_pub/activity_pub.ex
	@@ -21,6 +21,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
	alias Pleroma.Upload
	alias Pleroma.User
	alias Pleroma.Web.ActivityPub.MRF
	+ alias Pleroma.Web.ActivityPub.ObjectValidators.UserValidator
	alias Pleroma.Web.ActivityPub.Transmogrifier
	alias Pleroma.Web.Streamer
	alias Pleroma.Web.WebFinger
	@@ -1924,6 +1925,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do

	def user_data_from_user_object(data, additional \\ []) do
	with {:ok, data} <- MRF.filter(data),
	+ {:ok, data, _} <- UserValidator.validate(data, []),
	{:ok, data} <- object_to_user_data(data, additional) do
	{:ok, data}
	else
	@@ -2053,11 +2055,12 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do
	def enqueue_pin_fetches(_), do: nil

	def make_user_from_ap_id(ap_id, additional \\ []) do
	- user = User.get_cached_by_ap_id(ap_id)
	-
	with {:ok, data} <- fetch_and_prepare_user_from_ap_id(ap_id, additional) do
	enqueue_pin_fetches(data)

	+ # the fetched actor's id differs from ap_id when it was fetched from a display URL
	+ user = User.get_cached_by_ap_id(data.ap_id)
	+
	if user do
	user
	\|> User.remote_user_changeset(data)
	diff --git a/lib/pleroma/web/activity_pub/object_validator.ex b/lib/pleroma/web/activity_pub/object_validator.ex
	index 0254d6dc1..468298a4d 100644
	--- a/lib/pleroma/web/activity_pub/object_validator.ex
	+++ b/lib/pleroma/web/activity_pub/object_validator.ex
	@@ -11,7 +11,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do

	@behaviour Pleroma.Web.ActivityPub.ObjectValidator.Validating

	- import Pleroma.Constants, only: [activity_types: 0, object_types: 0]
	+ import Pleroma.Constants, only: [activity_types: 0, actor_types: 0, object_types: 0]

	alias Pleroma.Activity
	alias Pleroma.EctoType.ActivityPub.ObjectValidators
	@@ -37,6 +37,7 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
	alias Pleroma.Web.ActivityPub.ObjectValidators.QuestionValidator
	alias Pleroma.Web.ActivityPub.ObjectValidators.UndoValidator
	alias Pleroma.Web.ActivityPub.ObjectValidators.UpdateValidator
	+ alias Pleroma.Web.ActivityPub.ObjectValidators.UserValidator

	@impl true
	def validate(object, meta)
	@@ -226,7 +227,8 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do
	end

	def validate(%{"type" => type} = object, meta) when type == "Update" do
	- with {:ok, object} <-
	+ with :ok <- validate_updated_actor(object, meta),
	+ {:ok, object} <-
	object
	\|> UpdateValidator.cast_and_validate(meta)
	\|> Ecto.Changeset.apply_action(:insert) do
	@@ -247,6 +249,19 @@ defmodule Pleroma.Web.ActivityPub.ObjectValidator do

	def validate(o, m), do: {:error, {:validator_not_set, {o, m}}}

	+ defp validate_updated_actor(%{"object" => %{"type" => type} = actor}, meta)
	+ when type in actor_types() do
	+ with {_, false} <- {:local, Access.get(meta, :local, false)},
	+ {:ok, _, _} <- UserValidator.validate(actor, meta) do
	+ :ok
	+ else
	+ {:local, true} -> :ok
	+ error -> error
	+ end
	+ end
	+
	+ defp validate_updated_actor(_, _), do: :ok
	+
	def cast_and_apply_and_stringify_with_history(object) do
	do_separate_with_history(object, fn object ->
	with {:ok, object_data} <- cast_and_apply(object),
	diff --git a/lib/pleroma/web/activity_pub/object_validators/user_validator.ex b/lib/pleroma/web/activity_pub/object_validators/user_validator.ex
	new file mode 100644
	index 000000000..a368ccc16
	--- /dev/null
	+++ b/lib/pleroma/web/activity_pub/object_validators/user_validator.ex
	@@ -0,0 +1,53 @@
	+# Pleroma: A lightweight social networking server
	+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
	+# SPDX-License-Identifier: AGPL-3.0-only
	+
	+defmodule Pleroma.Web.ActivityPub.ObjectValidators.UserValidator do
	+ @moduledoc """
	+ Checks that a remote actor document only points at its own host.
	+
	+ We deliver to the inbox and shared inbox, use the collections for
	+ addressing and visibility checks, and map signature key ids back to
	+ the actor. If an actor could point any of these at another host, it
	+ could redirect our signed deliveries or claim another actor's
	+ collections. Optional fields are only checked when present; null and
	+ empty strings count as missing.
	+
	+ Based on Akkoma's UserValidator.
	+ """
	+
	+ @behaviour Pleroma.Web.ActivityPub.ObjectValidator.Validating
	+
	+ alias Pleroma.Object.Containment
	+
	+ @collections ["inbox", "followers", "following", "featured"]
	+
	+ @impl true
	+ def validate(%{"id" => id} = data, meta) when is_binary(id) do
	+ fields =
	+ Enum.map(@collections, &{&1, data[&1]}) ++
	+ [
	+ {"sharedInbox", get_in_map(data, ["endpoints", "sharedInbox"])},
	+ {"publicKey", get_in_map(data, ["publicKey", "id"])}
	+ ]
	+
	+ case Enum.find(fields, fn {_, value} -> not same_origin?(id, value) end) do
	+ nil -> {:ok, data, meta}
	+ {field, _} -> {:error, "#{field} is not on the same host as the actor"}
	+ end
	+ end
	+
	+ def validate(_data, _meta), do: {:error, "Actor has no id"}
	+
	+ defp same_origin?(_id, value) when value in [nil, ""], do: true
	+ defp same_origin?(id, %{"id" => value}), do: same_origin?(id, value)
	+ defp same_origin?(id, value), do: Containment.same_origin(id, value) == :ok
	+
	+ defp get_in_map(data, [key \| rest]) do
	+ case data do
	+ %{^key => value} when rest == [] -> value
	+ %{^key => value} -> get_in_map(value, rest)
	+ _ -> nil
	+ end
	+ end
	+end
	diff --git a/test/fixtures/bridgy/actor.json b/test/fixtures/bridgy/actor.json
	index 5b2d8982b..b4e859a82 100644
	--- a/test/fixtures/bridgy/actor.json
	+++ b/test/fixtures/bridgy/actor.json
	@@ -70,7 +70,7 @@
	"preferredUsername": "jk.nipponalba.scot",
	"summary": "",
	"publicKey": {
	- "id": "jk.nipponalba.scot",
	+ "id": "https://fed.brid.gy/jk.nipponalba.scot#key",
	"publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIGfMA0GCSqGSIb3DQEBAQUAA4GNADCBiQKBgQDdarxwzxnNbJ2hneWOYHkYJowk\npyigQtxlUd0VjgSQHwxU9kWqfbrHBVADyTtcqi/4dAzQd3UnCI1TPNnn4LPZY9PW\noiWd3Zl1/EfLFxO7LU9GS7fcSLQkyj5JNhSlN3I8QPudZbybrgRDVZYooDe1D+52\n5KLGqC2ajrIVOiDRTQIDAQAB\n-----END PUBLIC KEY-----"
	},
	"inbox": "https://fed.brid.gy/jk.nipponalba.scot/inbox",
	diff --git a/test/fixtures/tesla_mock/relay@mastdon.example.org.json b/test/fixtures/tesla_mock/relay@mastdon.example.org.json
	index c1fab7d3b..21dd405c8 100644
	--- a/test/fixtures/tesla_mock/relay@mastdon.example.org.json
	+++ b/test/fixtures/tesla_mock/relay@mastdon.example.org.json
	@@ -11,7 +11,7 @@
	"toot": "http://joinmastodon.org/ns#",
	"Emoji": "toot:Emoji"
	}],
	- "id": "http://mastodon.example.org/users/admin",
	+ "id": "http://mastodon.example.org/users/relay",
	"type": "Application",
	"invisible": true,
	"following": "http://mastodon.example.org/users/admin/following",
	@@ -24,8 +24,8 @@
	"url": "http://mastodon.example.org/@admin",
	"manuallyApprovesFollowers": false,
	"publicKey": {
	- "id": "http://mastodon.example.org/users/admin#main-key",
	- "owner": "http://mastodon.example.org/users/admin",
	+ "id": "http://mastodon.example.org/users/relay#main-key",
	+ "owner": "http://mastodon.example.org/users/relay",
	"publicKeyPem": "-----BEGIN PUBLIC KEY-----\nMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAtc4Tir+3ADhSNF6VKrtW\nOU32T01w7V0yshmQei38YyiVwVvFu8XOP6ACchkdxbJ+C9mZud8qWaRJKVbFTMUG\nNX4+6Q+FobyuKrwN7CEwhDALZtaN2IPbaPd6uG1B7QhWorrY+yFa8f2TBM3BxnUy\nI4T+bMIZIEYG7KtljCBoQXuTQmGtuffO0UwJksidg2ffCF5Q+K//JfQagJ3UzrR+\nZXbKMJdAw4bCVJYs4Z5EhHYBwQWiXCyMGTd7BGlmMkY6Av7ZqHKC/owp3/0EWDNz\nNqF09Wcpr3y3e8nA10X40MJqp/wR+1xtxp+YGbq/Cj5hZGBG7etFOmIpVBrDOhry\nBwIDAQAB\n-----END PUBLIC KEY-----\n"
	},
	"attachment": [{
	diff --git a/test/pleroma/object/containment_test.exs b/test/pleroma/object/containment_test.exs
	index 5560dfce5..2fdc83605 100644
	--- a/test/pleroma/object/containment_test.exs
	+++ b/test/pleroma/object/containment_test.exs
	@@ -127,4 +127,56 @@ defmodule Pleroma.Object.ContainmentTest do
	:ok = Containment.contain_child(data)
	end
	end
	+
	+ describe "get_actor/1" do
	+ test "returns nil for actor shapes without an id" do
	+ for actor <- [%{"type" => "Person"}, %{"id" => 1}, 1, true] do
	+ assert nil == Containment.get_actor(%{"actor" => actor})
	+ assert nil == Containment.get_actor(%{"actor" => nil, "attributedTo" => actor})
	+ end
	+
	+ assert nil == Containment.get_actor(%{})
	+ end
	+
	+ test "contain_origin/2 rejects actor shapes without an id" do
	+ assert :error ==
	+ Containment.contain_origin("https://a.test/o/1", %{
	+ "attributedTo" => %{"type" => "Person"}
	+ })
	+ end
	+ end
	+
	+ describe "contain_id_to_fetch/2" do
	+ test "matches the exact id" do
	+ assert :ok =
	+ Containment.contain_id_to_fetch("https://a.test/o/1", %{
	+ "id" => "https://a.test/o/1"
	+ })
	+ end
	+
	+ test "ignores fragments, a trailing slash and the host case" do
	+ assert :ok =
	+ Containment.contain_id_to_fetch("https://A.test/o/1/", %{
	+ "id" => "https://a.test/o/1#x"
	+ })
	+ end
	+
	+ test "rejects another id on the same host" do
	+ assert :error =
	+ Containment.contain_id_to_fetch("https://a.test/o/1", %{
	+ "id" => "https://a.test/o/2"
	+ })
	+ end
	+
	+ test "rejects a different query" do
	+ assert :error =
	+ Containment.contain_id_to_fetch("https://a.test/o?page=1", %{
	+ "id" => "https://a.test/o?page=2"
	+ })
	+ end
	+
	+ test "rejects documents without an id" do
	+ assert :error = Containment.contain_id_to_fetch("https://a.test/o/1", %{})
	+ end
	+ end
	end
	diff --git a/test/pleroma/object/fetcher_test.exs b/test/pleroma/object/fetcher_test.exs
	index 9afa34fa2..61a6fb517 100644
	--- a/test/pleroma/object/fetcher_test.exs
	+++ b/test/pleroma/object/fetcher_test.exs
	@@ -48,6 +48,8 @@ defmodule Pleroma.Object.FetcherTest do
	%{method: :get, url: "https://social.sakamoto.gq/notice/9wTkLEnuq47B25EehM"} ->
	%Tesla.Env{
	status: 200,
	+ # Pleroma redirects /notice/:id to the object id
	+ url: "https://social.sakamoto.gq/objects/f20f2497-66d9-4a52-a2e1-1be2a39c32c1",
	body: File.read!("test/fixtures/fetch_mocks/9wTkLEnuq47B25EehM.json"),
	headers: HttpRequestMock.activitypub_object_headers()
	}
	@@ -463,6 +465,78 @@ defmodule Pleroma.Object.FetcherTest do
	%{object1: local_object1, object2: local_object2}
	end

	+ test "it rejects a refetched object whose actor changed", %{object1: object1} do
	+ insert(:user, ap_id: "https://mastodon.social/users/someone_else")
	+ {:ok, o} = Object.create(object1)
	+
	+ mock(fn %{method: :get, url: "https://mastodon.social/1"} ->
	+ %Tesla.Env{
	+ status: 200,
	+ headers: [{"content-type", "application/activity+json"}],
	+ body:
	+ object1
	+ \|> Map.merge(%{
	+ "actor" => "https://mastodon.social/users/someone_else",
	+ "attributedTo" => "https://mastodon.social/users/someone_else",
	+ "content" => "hijacked"
	+ })
	+ \|> Jason.encode!()
	+ }
	+ end)
	+
	+ assert {:error, _} = Fetcher.refetch_object(o)
	+ assert Object.get_by_ap_id("https://mastodon.social/1").data["content"] == "test 1"
	+ end
	+
	+ test "it rejects a refetched object whose author has no id", %{object1: object1} do
	+ {:ok, o} = Object.create(object1)
	+
	+ mock(fn %{method: :get, url: "https://mastodon.social/1"} ->
	+ %Tesla.Env{
	+ status: 200,
	+ headers: [{"content-type", "application/activity+json"}],
	+ body:
	+ object1
	+ \|> Map.delete("actor")
	+ \|> Map.put("attributedTo", %{"type" => "Person"})
	+ \|> Jason.encode!()
	+ }
	+ end)
	+
	+ assert {:error, _} = Fetcher.refetch_object(o)
	+ end
	+
	+ test "it rejects a refetched object whose id changed", %{object1: object1} do
	+ {:ok, o} = Object.create(object1)
	+
	+ mock(fn %{method: :get, url: "https://mastodon.social/1"} ->
	+ %Tesla.Env{
	+ status: 200,
	+ url: "https://mastodon.social/2",
	+ headers: [{"content-type", "application/activity+json"}],
	+ body: object1 \|> Map.put("id", "https://mastodon.social/2") \|> Jason.encode!()
	+ }
	+ end)
	+
	+ assert {:error, _} = Fetcher.refetch_object(o)
	+ end
	+
	+ test "it rejects a refetched object whose id differs only by a trailing slash", %{
	+ object1: object1
	+ } do
	+ {:ok, o} = Object.create(object1)
	+
	+ mock(fn %{method: :get, url: "https://mastodon.social/1"} ->
	+ %Tesla.Env{
	+ status: 200,
	+ headers: [{"content-type", "application/activity+json"}],
	+ body: object1 \|> Map.put("id", "https://mastodon.social/1/") \|> Jason.encode!()
	+ }
	+ end)
	+
	+ assert {:error, _} = Fetcher.refetch_object(o)
	+ end
	+
	test "it keeps formerRepresentations if remote does not have this attr", %{object1: object1} do
	full_object1 =
	object1
	@@ -642,7 +716,23 @@ defmodule Pleroma.Object.FetcherTest do
	})
	}

	- # No redirect - same domain
	+ # Same-domain redirect to the object id
	+ %{method: :get, url: "https://original.test/@actor/redirect"} ->
	+ %Tesla.Env{
	+ status: 200,
	+ url: "https://original.test/objects/redirect-target",
	+ headers: [{"content-type", "application/activity+json"}],
	+ body:
	+ Jason.encode!(%{
	+ "id" => "https://original.test/objects/redirect-target",
	+ "type" => "Note",
	+ "content" => "This has a same-domain redirect",
	+ "actor" => "https://original.test/users/actor",
	+ "attributedTo" => "https://original.test/users/actor"
	+ })
	+ }
	+
	+ # Same-domain redirect to a document claiming the original URL as id
	%{method: :get, url: "https://original.test/objects/same-domain-redirect"} ->
	%Tesla.Env{
	status: 200,
	@@ -695,12 +785,19 @@ defmodule Pleroma.Object.FetcherTest do
	test "it accepts objects with same-domain redirects" do
	assert {:ok, data} =
	Fetcher.fetch_and_contain_remote_object_from_id(
	- "https://original.test/objects/same-domain-redirect"
	+ "https://original.test/@actor/redirect"
	)

	assert data["content"] == "This has a same-domain redirect"
	end

	+ test "it rejects redirect targets that claim the original URL as id" do
	+ assert {:error, _} =
	+ Fetcher.fetch_and_contain_remote_object_from_id(
	+ "https://original.test/objects/same-domain-redirect"
	+ )
	+ end
	+
	test "it handles responses without URL field (common in tests)" do
	assert {:ok, data} =
	Fetcher.fetch_and_contain_remote_object_from_id(
	@@ -711,6 +808,150 @@ defmodule Pleroma.Object.FetcherTest do
	end
	end

	+ describe "id containment" do
	+ setup do
	+ note = fn id, content ->
	+ %Tesla.Env{
	+ status: 200,
	+ headers: [{"content-type", "application/activity+json"}],
	+ body:
	+ Jason.encode!(%{
	+ "id" => id,
	+ "type" => "Note",
	+ "content" => content,
	+ "actor" => "https://host.test/users/actor",
	+ "attributedTo" => "https://host.test/users/actor"
	+ })
	+ }
	+ end
	+
	+ mock(fn
	+ # display URL serving the canonical object
	+ %{method: :get, url: "https://host.test/@actor/1"} ->
	+ note.("https://host.test/objects/1", "display")
	+
	+ %{method: :get, url: "https://host.test/objects/1"} ->
	+ note.("https://host.test/objects/1", "canonical")
	+
	+ # uploaded file claiming to be another object on the same host
	+ %{method: :get, url: "https://host.test/media/evil.json"} ->
	+ note.("https://host.test/objects/2", "spoofed")
	+
	+ %{method: :get, url: "https://host.test/objects/2"} ->
	+ note.("https://host.test/objects/2", "real")
	+
	+ # canonical id does not serve itself
	+ %{method: :get, url: "https://host.test/media/loop.json"} ->
	+ note.("https://host.test/objects/3", "loop")
	+
	+ %{method: :get, url: "https://host.test/objects/3"} ->
	+ note.("https://host.test/objects/4", "other")
	+
	+ # canonical id redirects
	+ %{method: :get, url: "https://host.test/media/redirect.json"} ->
	+ note.("https://host.test/objects/5", "redirect")
	+
	+ %{method: :get, url: "https://host.test/objects/5"} ->
	+ %{note.("https://host.test/objects/5", "moved") \| url: "https://host.test/media/x"}
	+
	+ %{method: :get, url: "https://host.test/objects/slash"} ->
	+ note.("https://host.test/objects/slash/", "slash")
	+
	+ %{method: :get, url: "https://host.test/objects/no-id"} ->
	+ note.(nil, "no id")
	+ end)
	+
	+ :ok
	+ end
	+
	+ test "it refetches display URLs from the canonical id" do
	+ assert {:ok, %{"id" => "https://host.test/objects/1", "content" => "canonical"}} =
	+ Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/@actor/1")
	+ end
	+
	+ test "it does not trust a document claiming another id on the same host" do
	+ assert {:ok, %{"id" => "https://host.test/objects/2", "content" => "real"}} =
	+ Fetcher.fetch_and_contain_remote_object_from_id(
	+ "https://host.test/media/evil.json"
	+ )
	+ end
	+
	+ test "it refetches at most once" do
	+ assert {:error, :id_mismatch} =
	+ Fetcher.fetch_and_contain_remote_object_from_id(
	+ "https://host.test/media/loop.json"
	+ )
	+ end
	+
	+ test "it does not follow redirects when refetching from the canonical id" do
	+ assert {:error, _} =
	+ Fetcher.fetch_and_contain_remote_object_from_id(
	+ "https://host.test/media/redirect.json"
	+ )
	+ end
	+
	+ test "it ignores a trailing slash" do
	+ assert {:ok, %{"content" => "slash"}} =
	+ Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/objects/slash")
	+ end
	+
	+ test "it accepts ids that only differ by URL encoding" do
	+ clear_config(:test_url_encoding, true)
	+
	+ mock(fn %{method: :get, url: url} ->
	+ # the encoder sends uppercase escapes
	+ assert url in [
	+ "https://host.test/users/%E3%81%82",
	+ "https://host.test/notes/%E3%81%82"
	+ ]
	+
	+ id =
	+ if url =~ "users",
	+ do: "https://host.test/users/あ",
	+ else: "https://host.test/notes/%e3%81%82"
	+
	+ %Tesla.Env{
	+ status: 200,
	+ url: url,
	+ headers: [{"content-type", "application/activity+json"}],
	+ body: Jason.encode!(%{"id" => id, "type" => "Note"})
	+ }
	+ end)
	+
	+ assert {:ok, _} =
	+ Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/users/あ")
	+
	+ assert {:ok, _} =
	+ Fetcher.fetch_and_contain_remote_object_from_id(
	+ "https://host.test/notes/%e3%81%82"
	+ )
	+ end
	+
	+ test "it rejects objects whose author has no id" do
	+ mock(fn %{method: :get, url: "https://host.test/objects/anon" = url} ->
	+ %Tesla.Env{
	+ status: 200,
	+ headers: [{"content-type", "application/activity+json"}],
	+ body:
	+ Jason.encode!(%{
	+ "id" => url,
	+ "type" => "Note",
	+ "content" => "anon",
	+ "attributedTo" => %{"type" => "Person", "name" => "nobody"}
	+ })
	+ }
	+ end)
	+
	+ assert {:containment, :error} =
	+ Fetcher.fetch_object_from_id("https://host.test/objects/anon")
	+ end
	+
	+ test "it rejects documents without an id" do
	+ assert {:error, _} =
	+ Fetcher.fetch_and_contain_remote_object_from_id("https://host.test/objects/no-id")
	+ end
	+ end
	+
	describe "fetch with history" do
	setup do
	object2 = %{
	diff --git a/test/pleroma/web/activity_pub/activity_pub_test.exs b/test/pleroma/web/activity_pub/activity_pub_test.exs
	index ed6db558b..c118cd8f1 100644
	--- a/test/pleroma/web/activity_pub/activity_pub_test.exs
	+++ b/test/pleroma/web/activity_pub/activity_pub_test.exs
	@@ -244,6 +244,58 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
	}
	end

	+ test "rejects actors whose inbox is on another host" do
	+ ap_id = "https://example.com/users/evil"
	+
	+ user_data =
	+ "test/fixtures/users_mock/user.json"
	+ \|> File.read!()
	+ \|> String.replace("{{nickname}}", "evil")
	+ \|> Jason.decode!()
	+ \|> Map.put("inbox", "https://victim.example/inbox")
	+ \|> Map.put("featured", nil)
	+ \|> Jason.encode!()
	+
	+ Tesla.Mock.mock(fn
	+ %{method: :get, url: ^ap_id} ->
	+ %Tesla.Env{
	+ status: 200,
	+ body: user_data,
	+ headers: [{"content-type", "application/activity+json"}]
	+ }
	+ end)
	+
	+ assert {:error, _} = ActivityPub.make_user_from_ap_id(ap_id)
	+ refute User.get_by_ap_id(ap_id)
	+ end
	+
	+ test "updates the known user when fetched from a display URL" do
	+ ap_id = "https://example.com/users/lain"
	+ display_url = "https://example.com/@lain"
	+ user = insert(:user, local: false, ap_id: ap_id, nickname: "lain@example.com")
	+
	+ user_data =
	+ "test/fixtures/users_mock/user.json"
	+ \|> File.read!()
	+ \|> String.replace("{{nickname}}", "lain")
	+ \|> Jason.decode!()
	+ \|> Map.merge(%{"featured" => nil, "name" => "new name"})
	+ \|> Jason.encode!()
	+
	+ Tesla.Mock.mock(fn %{method: :get, url: url} when url in [ap_id, display_url] ->
	+ %Tesla.Env{
	+ status: 200,
	+ body: user_data,
	+ headers: [{"content-type", "application/activity+json"}]
	+ }
	+ end)
	+
	+ assert {:ok, %User{id: id, name: "new name"}} =
	+ ActivityPub.make_user_from_ap_id(display_url)
	+
	+ assert id == user.id
	+ end
	+
	test "fetches user featured collection" do
	ap_id = "https://example.com/users/lain"

	@@ -516,7 +568,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
	end

	test "fetches user birthday information from misskey" do
	- user_id = "https://misskey.io/@mkljczk"
	+ user_id = "https://misskey.io/users/8dhi2ne167"

	Tesla.Mock.mock(fn
	%{
	@@ -2033,7 +2085,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
	json(
	%{
	"@context" => "https://www.w3.org/ns/activitystreams",
	- "id" => "https://remote.org/users/masto_hidden_counters/followers"
	+ "id" => "https://remote.org/users/masto_hidden_counters/following"
	},
	headers: HttpRequestMock.activitypub_object_headers()
	)
	@@ -2045,7 +2097,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do
	json(
	%{
	"@context" => "https://www.w3.org/ns/activitystreams",
	- "id" => "https://remote.org/users/masto_hidden_counters/following"
	+ "id" => "https://remote.org/users/masto_hidden_counters/followers"
	},
	headers: HttpRequestMock.activitypub_object_headers()
	)
	diff --git a/test/pleroma/web/activity_pub/object_validators/user_validator_test.exs b/test/pleroma/web/activity_pub/object_validators/user_validator_test.exs
	new file mode 100644
	index 000000000..fbc89462e
	--- /dev/null
	+++ b/test/pleroma/web/activity_pub/object_validators/user_validator_test.exs
	@@ -0,0 +1,75 @@
	+# Pleroma: A lightweight social networking server
	+# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
	+# SPDX-License-Identifier: AGPL-3.0-only
	+
	+defmodule Pleroma.Web.ActivityPub.ObjectValidators.UserValidatorTest do
	+ use ExUnit.Case, async: true
	+
	+ alias Pleroma.Web.ActivityPub.ObjectValidators.UserValidator
	+
	+ defp actor do
	+ "test/fixtures/users_mock/user.json"
	+ \|> File.read!()
	+ \|> String.replace("{{nickname}}", "lain")
	+ \|> Jason.decode!()
	+ end
	+
	+ test "accepts an actor whose endpoints and collections are on its own host" do
	+ data = actor()
	+ assert {:ok, ^data, []} = UserValidator.validate(data, [])
	+ end
	+
	+ test "accepts an actor without optional fields" do
	+ data = Map.drop(actor(), ["endpoints", "featured", "followers", "following", "publicKey"])
	+ assert {:ok, _, _} = UserValidator.validate(data, [])
	+ end
	+
	+ test "accepts all actor fixtures" do
	+ for path <- Path.wildcard("test/fixtures/*/.json"),
	+ {:ok, %{"inbox" => _, "id" => id} = data} when is_binary(id) <-
	+ [Jason.decode(File.read!(path))] do
	+ assert {{:ok, _, _}, ^path} = {UserValidator.validate(data, []), path}
	+ end
	+ end
	+
	+ test "treats empty and null fields as missing" do
	+ data =
	+ actor()
	+ \|> Map.merge(%{"inbox" => "", "followers" => nil, "following" => "", "featured" => ""})
	+ \|> put_in(["endpoints", "sharedInbox"], "")
	+ \|> put_in(["publicKey", "id"], "")
	+
	+ assert {:ok, _, _} = UserValidator.validate(data, [])
	+ end
	+
	+ for field <- ["inbox", "followers", "following", "featured"] do
	+ test "rejects an actor whose #{field} is on another host" do
	+ data = Map.put(actor(), unquote(field), "https://victim.example/users/lain/x")
	+ assert {:error, _} = UserValidator.validate(data, [])
	+ end
	+ end
	+
	+ test "rejects an actor whose sharedInbox is on another host" do
	+ data = put_in(actor(), ["endpoints", "sharedInbox"], "https://victim.example/inbox")
	+ assert {:error, _} = UserValidator.validate(data, [])
	+ end
	+
	+ test "rejects an actor whose collection is an object on another host" do
	+ data = Map.put(actor(), "followers", %{"id" => "https://victim.example/users/v/followers"})
	+ assert {:error, _} = UserValidator.validate(data, [])
	+ end
	+
	+ test "rejects an actor whose key id is on another host" do
	+ data = put_in(actor(), ["publicKey", "id"], "https://victim.example/users/lain#main-key")
	+ assert {:error, _} = UserValidator.validate(data, [])
	+ end
	+
	+ test "rejects an actor whose key id has no host" do
	+ data = put_in(actor(), ["publicKey", "id"], "lain")
	+ assert {:error, _} = UserValidator.validate(data, [])
	+ end
	+
	+ test "rejects an actor without an id" do
	+ assert {:error, _} = UserValidator.validate(Map.delete(actor(), "id"), [])
	+ end
	+end
	diff --git a/test/pleroma/web/activity_pub/transmogrifier/user_update_handling_test.exs b/test/pleroma/web/activity_pub/transmogrifier/user_update_handling_test.exs
	index cf995e143..b39bc8780 100644
	--- a/test/pleroma/web/activity_pub/transmogrifier/user_update_handling_test.exs
	+++ b/test/pleroma/web/activity_pub/transmogrifier/user_update_handling_test.exs
	@@ -11,8 +11,11 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do

	import Pleroma.Factory

	+ # matches the host of the actor in mastodon-update.json
	+ @gargron "http://mastodon.example.org/users/gargron"
	+
	test "it works for incoming update activities" do
	- user = insert(:user, local: false)
	+ user = insert(:user, local: false, ap_id: @gargron)

	update_data = File.read!("test/fixtures/mastodon-update.json") \|> Jason.decode!()

	@@ -50,8 +53,28 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do
	assert user.bio == "<p>Some bio</p>"
	end

	+ test "it rejects updates that move the shared inbox to another host" do
	+ user = insert(:user, local: false, ap_id: @gargron)
	+
	+ update_data = File.read!("test/fixtures/mastodon-update.json") \|> Jason.decode!()
	+
	+ object =
	+ update_data["object"]
	+ \|> Map.put("actor", user.ap_id)
	+ \|> Map.put("id", user.ap_id)
	+ \|> put_in(["endpoints", "sharedInbox"], "https://victim.example/inbox")
	+
	+ update_data =
	+ update_data
	+ \|> Map.put("actor", user.ap_id)
	+ \|> Map.put("object", object)
	+
	+ assert {:error, _} = Transmogrifier.handle_incoming(update_data)
	+ assert User.get_cached_by_ap_id(user.ap_id).shared_inbox == user.shared_inbox
	+ end
	+
	test "it selects an actor type after an extension type" do
	- user = insert(:user, local: false)
	+ user = insert(:user, local: false, ap_id: @gargron)
	update_data = File.read!("test/fixtures/mastodon-update.json") \|> Jason.decode!()

	object =
	@@ -70,7 +93,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do
	end

	test "it works with alsoKnownAs" do
	- %{ap_id: actor} = insert(:user, local: false)
	+ %{ap_id: actor} = insert(:user, local: false, ap_id: @gargron)

	assert User.get_cached_by_ap_id(actor).also_known_as == []

	@@ -97,7 +120,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do
	end

	test "it works with custom profile fields" do
	- user = insert(:user, local: false)
	+ user = insert(:user, local: false, ap_id: @gargron)

	assert user.fields == []

	@@ -155,7 +178,7 @@ defmodule Pleroma.Web.ActivityPub.Transmogrifier.UserUpdateHandlingTest do
	end

	test "it works for incoming update activities which lock the account" do
	- user = insert(:user, local: false)
	+ user = insert(:user, local: false, ap_id: @gargron)

	update_data = File.read!("test/fixtures/mastodon-update.json") \|> Jason.decode!()

	diff --git a/test/support/http_request_mock.ex b/test/support/http_request_mock.ex
	index 82affe7ac..94be59536 100644
	--- a/test/support/http_request_mock.ex
	+++ b/test/support/http_request_mock.ex
	@@ -577,6 +577,16 @@ defmodule HttpRequestMock do
	}}
	end

	+ # the same status via its canonical id
	+ def get("http://mastodon.example.org/users/admin/statuses/99541947525187367", _, _, _) do
	+ {:ok,
	+ %Tesla.Env{
	+ status: 200,
	+ body: File.read!("test/fixtures/mastodon-note-object.json"),
	+ headers: activitypub_object_headers()
	+ }}
	+ end
	+
	def get("http://mastodon.example.org/@admin/99541947525187368", _, _, _) do
	{:ok,
	%Tesla.Env{

File Metadata

Mime Type: text/x-diff
Expires: Fri, Oct 30, 12:19 AM (1 d, 15 h)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 1798971
Default Alt Text: (38 KB)

No OneTemporaryActions

View Options

File Metadata

Event Timeline

No OneTemporary
Actions