Merge remote-tracking branch 'oneric-sec/media-owner' into develop

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 
 ## Unreleased
 
+## Fixed
+- Issue allowing non-owners to use media objects in posts
+- Issue allowing use of non-media objects as attachments and crashing timeline rendering
+
 ## 2024.04
 
 ## Added
@@ -37,6 +41,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
 - Issue leading to Mastodon bot accounts being rejected
 - Scope misdetection of remote posts resulting from not recognising
   JSON-LD-compacted forms of public scope; affected e.g. federation with bovine
+- Ratelimits encountered when fetching objects are now respected; 429 responses will cause a backoff when we get one.
 
 ## Removed
 - ActivityPub Client-To-Server write API endpoints have been disabled;

lib/pleroma/application.ex

@@ -180,7 +180,8 @@ defmodule Pleroma.Application do
       build_cachex("instances", default_ttl: :timer.hours(24), ttl_interval: 1000, limit: 2500),
       build_cachex("request_signatures", default_ttl: :timer.hours(24 * 30), limit: 3000),
       build_cachex("rel_me", default_ttl: :timer.hours(24 * 30), limit: 300),
-      build_cachex("host_meta", default_ttl: :timer.minutes(120), limit: 5000)
+      build_cachex("host_meta", default_ttl: :timer.minutes(120), limit: 5000),
+      build_cachex("http_backoff", default_ttl: :timer.hours(24 * 30), limit: 10000)
     ]
   end
 

lib/pleroma/constants.ex

@@ -64,4 +64,7 @@ defmodule Pleroma.Constants do
       "Service"
     ]
   )
+
+  # Internally used as top-level types for media attachments and user images
+  const(attachment_types, do: ["Document", "Image"])
 end

lib/pleroma/http/backoff.ex

@@ -0,0 +1,121 @@
+defmodule Pleroma.HTTP.Backoff do
+  alias Pleroma.HTTP
+  require Logger
+
+  @cachex Pleroma.Config.get([:cachex, :provider], Cachex)
+  @backoff_cache :http_backoff_cache
+
+  # attempt to parse a timestamp from a header
+  # returns nil if it can't parse the timestamp
+  @spec timestamp_or_nil(binary) :: DateTime.t() | nil
+  defp timestamp_or_nil(header) do
+    case DateTime.from_iso8601(header) do
+      {:ok, stamp, _} ->
+        stamp
+
+      _ ->
+        nil
+    end
+  end
+
+  # attempt to parse the x-ratelimit-reset header from the headers
+  @spec x_ratelimit_reset(headers :: list) :: DateTime.t() | nil
+  defp x_ratelimit_reset(headers) do
+    with {_header, value} <- List.keyfind(headers, "x-ratelimit-reset", 0),
+         true <- is_binary(value) do
+      timestamp_or_nil(value)
+    else
+      _ ->
+        nil
+    end
+  end
+
+  # attempt to parse the Retry-After header from the headers
+  # this can be either a timestamp _or_ a number of seconds to wait!
+  # we'll return a datetime if we can parse it, or nil if we can't
+  @spec retry_after(headers :: list) :: DateTime.t() | nil
+  defp retry_after(headers) do
+    with {_header, value} <- List.keyfind(headers, "retry-after", 0),
+         true <- is_binary(value) do
+      # first, see if it's an integer
+      case Integer.parse(value) do
+        {seconds, ""} ->
+          Logger.debug("Parsed Retry-After header: #{seconds} seconds")
+          DateTime.utc_now() |> Timex.shift(seconds: seconds)
+
+        _ ->
+          # if it's not an integer, try to parse it as a timestamp
+          timestamp_or_nil(value)
+      end
+    else
+      _ ->
+        nil
+    end
+  end
+
+  # given a set of headers, will attempt to find the next backoff timestamp
+  # if it can't find one, it will default to 5 minutes from now
+  @spec next_backoff_timestamp(%{headers: list}) :: DateTime.t()
+  defp next_backoff_timestamp(%{headers: headers}) when is_list(headers) do
+    default_5_minute_backoff =
+      DateTime.utc_now()
+      |> Timex.shift(seconds: 5 * 60)
+
+    backoff =
+      [&x_ratelimit_reset/1, &retry_after/1]
+      |> Enum.map(& &1.(headers))
+      |> Enum.find(&(&1 != nil))
+
+    if is_nil(backoff) do
+      Logger.debug("No backoff headers found, defaulting to 5 minutes from now")
+      default_5_minute_backoff
+    else
+      Logger.debug("Found backoff header, will back off until: #{backoff}")
+      backoff
+    end
+  end
+
+  defp next_backoff_timestamp(_), do: DateTime.utc_now() |> Timex.shift(seconds: 5 * 60)
+
+  # utility function to check the HTTP response for potential backoff headers
+  # will check if we get a 429 or 503 response, and if we do, will back off for a bit
+  @spec check_backoff({:ok | :error, HTTP.Env.t()}, binary()) ::
+          {:ok | :error, HTTP.Env.t()} | {:error, :ratelimit}
+  defp check_backoff({:ok, env}, host) do
+    case env.status do
+      status when status in [429, 503] ->
+        Logger.error("Rate limited on #{host}! Backing off...")
+        timestamp = next_backoff_timestamp(env)
+        ttl = Timex.diff(timestamp, DateTime.utc_now(), :seconds)
+        # we will cache the host for 5 minutes
+        @cachex.put(@backoff_cache, host, true, ttl: ttl)
+        {:error, :ratelimit}
+
+      _ ->
+        {:ok, env}
+    end
+  end
+
+  defp check_backoff(env, _), do: env
+
+  @doc """
+  this acts as a single throughput for all GET requests
+  we will check if the host is in the cache, and if it is, we will automatically fail the request
+  this ensures that we don't hammer the server with requests, and instead wait for the backoff to expire
+  this is a very simple implementation, and can be improved upon!
+  """
+  @spec get(binary, list, list) :: {:ok | :error, HTTP.Env.t()} | {:error, :ratelimit}
+  def get(url, headers \\ [], options \\ []) do
+    %{host: host} = URI.parse(url)
+
+    case @cachex.get(@backoff_cache, host) do
+      {:ok, nil} ->
+        url
+        |> HTTP.get(headers, options)
+        |> check_backoff(host)
+
+      _ ->
+        {:error, :ratelimit}
+    end
+  end
+end

lib/pleroma/object/fetcher.ex

@@ -354,7 +354,7 @@ defmodule Pleroma.Object.Fetcher do
 
     with {:ok, %{body: body, status: code, headers: headers, url: final_url}}
          when code in 200..299 <-
-           HTTP.get(id, headers),
+           HTTP.Backoff.get(id, headers),
          remote_host <-
            URI.parse(final_url).host,
          {:cross_domain_redirect, false} <-

lib/pleroma/scheduled_activity.ex

@@ -28,7 +28,7 @@ defmodule Pleroma.ScheduledActivity do
     timestamps()
   end
 
-  def changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
+  defp changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
     scheduled_activity
     |> cast(attrs, [:scheduled_at, :params])
     |> validate_required([:scheduled_at, :params])
@@ -40,26 +40,36 @@ defmodule Pleroma.ScheduledActivity do
          %{changes: %{params: %{"media_ids" => media_ids} = params}} = changeset
        )
        when is_list(media_ids) do
-    media_attachments = Utils.attachments_from_ids(%{media_ids: media_ids})
+    user = User.get_by_id(changeset.data.user_id)
 
-    params =
+    case Utils.attachments_from_ids(user, %{media_ids: media_ids}) do
-      params
+      media_attachments when is_list(media_attachments) ->
-      |> Map.put("media_attachments", media_attachments)
+        params =
-      |> Map.put("media_ids", media_ids)
+          params
+          |> Map.put("media_attachments", media_attachments)
+          |> Map.put("media_ids", media_ids)
 
-    put_change(changeset, :params, params)
+        put_change(changeset, :params, params)
+
+      {:error, _} = e ->
+        e
+
+      e ->
+        {:error, e}
+    end
   end
 
   defp with_media_attachments(changeset), do: changeset
 
-  def update_changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
+  defp update_changeset(%ScheduledActivity{} = scheduled_activity, attrs) do
+    # note: should this ever allow swapping media attachments, make sure ownership is checked
     scheduled_activity
     |> cast(attrs, [:scheduled_at])
     |> validate_required([:scheduled_at])
     |> validate_scheduled_at()
   end
 
-  def validate_scheduled_at(changeset) do
+  defp validate_scheduled_at(changeset) do
     validate_change(changeset, :scheduled_at, fn _, scheduled_at ->
       cond do
         not far_enough?(scheduled_at) ->
@@ -77,7 +87,7 @@ defmodule Pleroma.ScheduledActivity do
     end)
   end
 
-  def exceeds_daily_user_limit?(user_id, scheduled_at) do
+  defp exceeds_daily_user_limit?(user_id, scheduled_at) do
     ScheduledActivity
     |> where(user_id: ^user_id)
     |> where([sa], type(sa.scheduled_at, :date) == type(^scheduled_at, :date))
@@ -86,7 +96,7 @@ defmodule Pleroma.ScheduledActivity do
     |> Kernel.>=(Config.get([ScheduledActivity, :daily_user_limit]))
   end
 
-  def exceeds_total_user_limit?(user_id) do
+  defp exceeds_total_user_limit?(user_id) do
     ScheduledActivity
     |> where(user_id: ^user_id)
     |> select([sa], count(sa.id))
@@ -108,20 +118,29 @@ defmodule Pleroma.ScheduledActivity do
     diff > @min_offset
   end
 
-  def new(%User{} = user, attrs) do
+  defp new(%User{} = user, attrs) do
     changeset(%ScheduledActivity{user_id: user.id}, attrs)
   end
 
   @doc """
   Creates ScheduledActivity and add to queue to perform at scheduled_at date
   """
-  @spec create(User.t(), map()) :: {:ok, ScheduledActivity.t()} | {:error, Ecto.Changeset.t()}
+  @spec create(User.t(), map()) :: {:ok, ScheduledActivity.t()} | {:error, any()}
   def create(%User{} = user, attrs) do
-    Multi.new()
+    case new(user, attrs) do
-    |> Multi.insert(:scheduled_activity, new(user, attrs))
+      %Ecto.Changeset{} = sched_data ->
-    |> maybe_add_jobs(Config.get([ScheduledActivity, :enabled]))
+        Multi.new()
-    |> Repo.transaction()
+        |> Multi.insert(:scheduled_activity, sched_data)
-    |> transaction_response
+        |> maybe_add_jobs(Config.get([ScheduledActivity, :enabled]))
+        |> Repo.transaction()
+        |> transaction_response
+
+      {:error, _} = e ->
+        e
+
+      e ->
+        {:error, e}
+    end
   end
 
   defp maybe_add_jobs(multi, true) do
@@ -187,17 +206,7 @@ defmodule Pleroma.ScheduledActivity do
     |> where(user_id: ^user.id)
   end
 
-  def due_activities(offset \\ 0) do
+  defp job_query(scheduled_activity_id) do
-    naive_datetime =
-      NaiveDateTime.utc_now()
-      |> NaiveDateTime.add(offset, :millisecond)
-
-    ScheduledActivity
-    |> where([sa], sa.scheduled_at < ^naive_datetime)
-    |> Repo.all()
-  end
-
-  def job_query(scheduled_activity_id) do
     from(j in Oban.Job,
       where: j.queue == "scheduled_activities",
       where: fragment("args ->> 'activity_id' = ?::text", ^to_string(scheduled_activity_id))

lib/pleroma/upload.ex

@@ -13,7 +13,6 @@ defmodule Pleroma.Upload do
   * `:uploader`: override uploader
   * `:filters`: override filters
   * `:size_limit`: override size limit
-  * `:activity_type`: override activity type
 
   The `%Pleroma.Upload{}` struct: all documented fields are meant to be overwritten in filters:
 
@@ -48,7 +47,6 @@ defmodule Pleroma.Upload do
   @type option ::
           {:type, :avatar | :banner | :background}
           | {:description, String.t()}
-          | {:activity_type, String.t()}
           | {:size_limit, nil | non_neg_integer()}
           | {:uploader, module()}
           | {:filters, [module()]}
@@ -143,7 +141,7 @@ defmodule Pleroma.Upload do
       end
 
     %{
-      activity_type: Keyword.get(opts, :activity_type, activity_type),
+      activity_type: activity_type,
       size_limit: Keyword.get(opts, :size_limit, size_limit),
       uploader: Keyword.get(opts, :uploader, Pleroma.Config.get([__MODULE__, :uploader])),
       filters:

lib/pleroma/web/common_api/activity_draft.ex

@@ -41,7 +41,7 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do
             preview?: false,
             changes: %{}
 
-  def new(user, params) do
+  defp new(user, params) do
     %__MODULE__{user: user}
     |> put_params(params)
   end
@@ -92,9 +92,14 @@ defmodule Pleroma.Web.CommonAPI.ActivityDraft do
     end
   end
 
-  defp attachments(%{params: params} = draft) do
+  defp attachments(%{params: params, user: user} = draft) do
-    attachments = Utils.attachments_from_ids(params)
+    case Utils.attachments_from_ids(user, params) do
-    %__MODULE__{draft | attachments: attachments}
+      attachments when is_list(attachments) ->
+        %__MODULE__{draft | attachments: attachments}
+
+      {:error, reason} ->
+        add_error(draft, reason)
+    end
   end
 
   defp in_reply_to(%{params: %{in_reply_to_status_id: ""}} = draft), do: draft

lib/pleroma/web/common_api/utils.ex

@@ -22,43 +22,31 @@ defmodule Pleroma.Web.CommonAPI.Utils do
   require Logger
   require Pleroma.Constants
 
-  def attachments_from_ids(%{media_ids: ids, descriptions: desc}) do
+  def attachments_from_ids(user, %{media_ids: ids}) do
-    attachments_from_ids_descs(ids, desc)
+    attachments_from_ids(user, ids, [])
   end
 
-  def attachments_from_ids(%{media_ids: ids}) do
+  def attachments_from_ids(_, _), do: []
-    attachments_from_ids_no_descs(ids)
+
+  defp attachments_from_ids(_user, [], acc), do: Enum.reverse(acc)
+
+  defp attachments_from_ids(user, [media_id | ids], acc) do
+    with {_, %Object{} = object} <- {:get, get_attachment(media_id)},
+         :ok <- Object.authorize_access(object, user) do
+      attachments_from_ids(user, ids, [object.data | acc])
+    else
+      {:get, _} -> attachments_from_ids(user, ids, acc)
+      {:error, reason} -> {:error, reason}
+    end
   end
 
-  def attachments_from_ids(_), do: []
+  def get_attachment(media_id) do
-
+    with %Object{} = object <- Repo.get(Object, media_id),
-  def attachments_from_ids_no_descs([]), do: []
+         true <- object.data["type"] in Pleroma.Constants.attachment_types() do
-
+      object
-  def attachments_from_ids_no_descs(ids) do
+    else
-    Enum.map(ids, fn media_id ->
+      _ -> nil
-      case get_attachment(media_id) do
+    end
-        %Object{data: data} -> data
-        _ -> nil
-      end
-    end)
-    |> Enum.reject(&is_nil/1)
-  end
-
-  def attachments_from_ids_descs([], _), do: []
-
-  def attachments_from_ids_descs(ids, descs_str) do
-    {_, descs} = Jason.decode(descs_str)
-
-    Enum.map(ids, fn media_id ->
-      with %Object{data: data} <- get_attachment(media_id) do
-        Map.put(data, "name", descs[media_id])
-      end
-    end)
-    |> Enum.reject(&is_nil/1)
-  end
-
-  defp get_attachment(media_id) do
-    Repo.get(Object, media_id)
   end
 
   @spec get_to_and_cc(ActivityDraft.t()) :: {list(String.t()), list(String.t())}

lib/pleroma/web/mastodon_api/controllers/media_controller.ex

@@ -8,6 +8,7 @@ defmodule Pleroma.Web.MastodonAPI.MediaController do
   alias Pleroma.Object
   alias Pleroma.User
   alias Pleroma.Web.ActivityPub.ActivityPub
+  alias Pleroma.Web.CommonAPI.Utils
   alias Pleroma.Web.Plugs.OAuthScopesPlug
 
   action_fallback(Pleroma.Web.MastodonAPI.FallbackController)
@@ -55,12 +56,15 @@ defmodule Pleroma.Web.MastodonAPI.MediaController do
 
   @doc "PUT /api/v1/media/:id"
   def update(%{assigns: %{user: user}, body_params: %{description: description}} = conn, %{id: id}) do
-    with %Object{} = object <- Object.get_by_id(id),
+    with {_, %Object{} = object} <- {:get, Utils.get_attachment(id)},
          :ok <- Object.authorize_access(object, user),
          {:ok, %Object{data: data}} <- Object.update_data(object, %{"name" => description}) do
       attachment_data = Map.put(data, "id", object.id)
 
       render(conn, "attachment.json", %{attachment: attachment_data})
+    else
+      {:get, _} -> {:error, :not_found}
+      e -> e
     end
   end
 
@@ -68,11 +72,14 @@ defmodule Pleroma.Web.MastodonAPI.MediaController do
 
   @doc "GET /api/v1/media/:id"
   def show(%{assigns: %{user: user}} = conn, %{id: id}) do
-    with %Object{data: data, id: object_id} = object <- Object.get_by_id(id),
+    with {_, %Object{data: data, id: object_id} = object} <- {:get, Utils.get_attachment(id)},
          :ok <- Object.authorize_access(object, user) do
       attachment_data = Map.put(data, "id", object_id)
 
       render(conn, "attachment.json", %{attachment: attachment_data})
+    else
+      {:get, _} -> {:error, :not_found}
+      e -> e
     end
   end
 

lib/pleroma/web/mastodon_api/controllers/status_controller.ex

@@ -87,7 +87,7 @@ defmodule Pleroma.Web.MastodonAPI.StatusController do
     %{scopes: ["write:bookmarks"]} when action in [:bookmark, :unbookmark]
   )
 
-  @rate_limited_status_actions ~w(reblog unreblog favourite unfavourite create delete)a
+  @rate_limited_status_actions ~w(reblog unreblog favourite unfavourite create delete update)a
 
   plug(
     RateLimiter,

lib/pleroma/web/web_finger.ex

@@ -169,7 +169,8 @@ defmodule Pleroma.Web.WebFinger do
     # WebFinger is restricted to HTTPS - https://tools.ietf.org/html/rfc7033#section-9.1
     meta_url = "https://#{domain}/.well-known/host-meta"
 
-    with {:ok, %{status: status, body: body}} when status in 200..299 <- HTTP.get(meta_url) do
+    with {:ok, %{status: status, body: body}} when status in 200..299 <-
+           HTTP.Backoff.get(meta_url) do
       get_template_from_xml(body)
     else
       error ->
@@ -209,7 +210,7 @@ defmodule Pleroma.Web.WebFinger do
 
     with address when is_binary(address) <- get_address_from_domain(domain, account),
          {:ok, %{status: status, body: body, headers: headers}} when status in 200..299 <-
-           HTTP.get(
+           HTTP.Backoff.get(
              address,
              [{"accept", "application/xrd+xml,application/jrd+json"}]
            ) do

test/pleroma/http/backoff_test.exs

@@ -0,0 +1,105 @@
+defmodule Pleroma.HTTP.BackoffTest do
+  @backoff_cache :http_backoff_cache
+  use Pleroma.DataCase, async: false
+  alias Pleroma.HTTP.Backoff
+
+  defp within_tolerance?(ttl, expected) do
+    ttl > expected - 10 and ttl < expected + 10
+  end
+
+  describe "get/3" do
+    test "should return {:ok, env} when not rate limited" do
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://akkoma.dev/api/v1/instance"} ->
+          {:ok, %Tesla.Env{status: 200, body: "ok"}}
+      end)
+
+      assert {:ok, env} = Backoff.get("https://akkoma.dev/api/v1/instance")
+      assert env.status == 200
+    end
+
+    test "should return {:error, env} when rate limited" do
+      # Shove a value into the cache to simulate a rate limit
+      Cachex.put(@backoff_cache, "akkoma.dev", true)
+      assert {:error, :ratelimit} = Backoff.get("https://akkoma.dev/api/v1/instance")
+    end
+
+    test "should insert a value into the cache when rate limited" do
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok, %Tesla.Env{status: 429, body: "Rate limited"}}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+    end
+
+    test "should insert a value into the cache when rate limited with a 503 response" do
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok, %Tesla.Env{status: 503, body: "Rate limited"}}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+    end
+
+    test "should parse the value of x-ratelimit-reset, if present" do
+      ten_minutes_from_now =
+        DateTime.utc_now() |> Timex.shift(minutes: 10) |> DateTime.to_iso8601()
+
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok,
+           %Tesla.Env{
+             status: 429,
+             body: "Rate limited",
+             headers: [{"x-ratelimit-reset", ten_minutes_from_now}]
+           }}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+      {:ok, ttl} = Cachex.ttl(@backoff_cache, "ratelimited.dev")
+      assert within_tolerance?(ttl, 600)
+    end
+
+    test "should parse the value of retry-after when it's a timestamp" do
+      ten_minutes_from_now =
+        DateTime.utc_now() |> Timex.shift(minutes: 10) |> DateTime.to_iso8601()
+
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok,
+           %Tesla.Env{
+             status: 429,
+             body: "Rate limited",
+             headers: [{"retry-after", ten_minutes_from_now}]
+           }}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+      {:ok, ttl} = Cachex.ttl(@backoff_cache, "ratelimited.dev")
+      assert within_tolerance?(ttl, 600)
+    end
+
+    test "should parse the value of retry-after when it's a number of seconds" do
+      Tesla.Mock.mock_global(fn
+        %Tesla.Env{url: "https://ratelimited.dev/api/v1/instance"} ->
+          {:ok,
+           %Tesla.Env{
+             status: 429,
+             body: "Rate limited",
+             headers: [{"retry-after", "600"}]
+           }}
+      end)
+
+      assert {:error, :ratelimit} = Backoff.get("https://ratelimited.dev/api/v1/instance")
+      assert {:ok, true} = Cachex.get(@backoff_cache, "ratelimited.dev")
+      # assert that the value is 10 minutes from now
+      {:ok, ttl} = Cachex.ttl(@backoff_cache, "ratelimited.dev")
+      assert within_tolerance?(ttl, 600)
+    end
+  end
+end

test/pleroma/web/common_api/utils_test.exs

@@ -590,41 +590,22 @@ defmodule Pleroma.Web.CommonAPI.UtilsTest do
     end
   end
 
-  describe "attachments_from_ids_descs/2" do
-    test "returns [] when attachment ids is empty" do
-      assert Utils.attachments_from_ids_descs([], "{}") == []
-    end
-
-    test "returns list attachments with desc" do
-      object = insert(:note)
-      desc = Jason.encode!(%{object.id => "test-desc"})
-
-      assert Utils.attachments_from_ids_descs(["#{object.id}", "34"], desc) == [
-               Map.merge(object.data, %{"name" => "test-desc"})
-             ]
-    end
-  end
-
   describe "attachments_from_ids/1" do
-    test "returns attachments with descs" do
+    test "returns attachments without descs" do
-      object = insert(:note)
+      user = insert(:user)
-      desc = Jason.encode!(%{object.id => "test-desc"})
+      object = insert(:attachment, user: user)
-
+      assert Utils.attachments_from_ids(user, %{media_ids: ["#{object.id}"]}) == [object.data]
-      assert Utils.attachments_from_ids(%{
-               media_ids: ["#{object.id}"],
-               descriptions: desc
-             }) == [
-               Map.merge(object.data, %{"name" => "test-desc"})
-             ]
     end
 
-    test "returns attachments without descs" do
+    test "returns [] when passed non-media object ids" do
-      object = insert(:note)
+      user = insert(:user)
-      assert Utils.attachments_from_ids(%{media_ids: ["#{object.id}"]}) == [object.data]
+      object = insert(:note, user: user)
+      assert Utils.attachments_from_ids(user, %{media_ids: ["#{object.id}"]}) == []
     end
 
     test "returns [] when not pass media_ids" do
-      assert Utils.attachments_from_ids(%{}) == []
+      user = insert(:user)
+      assert Utils.attachments_from_ids(user, %{}) == []
     end
   end
 

test/pleroma/web/mastodon_api/controllers/media_controller_test.exs

@@ -6,6 +6,7 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do
   use Pleroma.Web.ConnCase, async: false
 
   import ExUnit.CaptureLog
+  import Pleroma.Factory
 
   alias Pleroma.Object
   alias Pleroma.User
@@ -174,6 +175,18 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do
       assert media["description"] == "test-media"
       assert refresh_record(object).data["name"] == "test-media"
     end
+
+    test "won't update non-media", %{conn: conn, user: user} do
+      object = insert(:note, user: user)
+
+      response =
+        conn
+        |> put_req_header("content-type", "multipart/form-data")
+        |> put("/api/v1/media/#{object.id}", %{"description" => "test-media"})
+        |> json_response(404)
+
+      assert response == %{"error" => "Record not found"}
+    end
   end
 
   describe "Get media by id (/api/v1/media/:id)" do
@@ -207,6 +220,17 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do
       assert media["id"]
     end
 
+    test "it returns 404 when requesting non-media object", %{conn: conn, user: user} do
+      object = insert(:note, user: user)
+
+      response =
+        conn
+        |> get("/api/v1/media/#{object.id}")
+        |> json_response(404)
+
+      assert response == %{"error" => "Record not found"}
+    end
+
     test "it returns 403 if media object requested by non-owner", %{object: object, user: user} do
       %{conn: conn, user: other_user} = oauth_access(["read:media"])
 

test/pleroma/web/mastodon_api/controllers/status_controller_test.exs

@@ -220,6 +220,28 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
       assert json_response_and_validate_schema(conn, 200)
     end
 
+    test "refuses to post non-owned media", %{conn: conn} do
+      other_user = insert(:user)
+
+      file = %Plug.Upload{
+        content_type: "image/jpeg",
+        path: Path.absname("test/fixtures/image.jpg"),
+        filename: "an_image.jpg"
+      }
+
+      {:ok, upload} = ActivityPub.upload(file, actor: other_user.ap_id)
+
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> post("/api/v1/statuses", %{
+          "status" => "mew",
+          "media_ids" => [to_string(upload.id)]
+        })
+
+      assert json_response(conn, 422) == %{"error" => "forbidden"}
+    end
+
     test "posting a status with an invalid language", %{conn: conn} do
       conn =
         conn
@@ -569,6 +591,29 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
       assert %{"type" => "image"} = media_attachment
     end
 
+    test "refuses to schedule post with non-owned media", %{conn: conn} do
+      other_user = insert(:user)
+
+      file = %Plug.Upload{
+        content_type: "image/jpeg",
+        path: Path.absname("test/fixtures/image.jpg"),
+        filename: "an_image.jpg"
+      }
+
+      {:ok, upload} = ActivityPub.upload(file, actor: other_user.ap_id)
+
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> post("/api/v1/statuses", %{
+          "status" => "mew",
+          "scheduled_at" => DateTime.add(DateTime.utc_now(), 6, :minute),
+          "media_ids" => [to_string(upload.id)]
+        })
+
+      assert json_response(conn, 403) == %{"error" => "Access denied"}
+    end
+
     test "skips the scheduling and creates the activity if scheduled_at is earlier than 5 minutes from now",
          %{conn: conn} do
       scheduled_at =
@@ -2406,6 +2451,25 @@ defmodule Pleroma.Web.MastodonAPI.StatusControllerTest do
       assert [%{"id" => ^attachment_id}] = response["media_attachments"]
     end
 
+    test "it does not update to non-owned attachments", %{conn: conn, user: user} do
+      other_user = insert(:user)
+      attachment = insert(:attachment, user: other_user)
+      attachment_id = to_string(attachment.id)
+
+      {:ok, activity} = CommonAPI.post(user, %{status: "mew mew #abc", spoiler_text: "#def"})
+
+      conn =
+        conn
+        |> put_req_header("content-type", "application/json")
+        |> put("/api/v1/statuses/#{activity.id}", %{
+          "status" => "mew mew #abc",
+          "spoiler_text" => "#def",
+          "media_ids" => [attachment_id]
+        })
+
+      assert json_response(conn, 400) == %{"error" => "internal_server_error"}
+    end
+
     test "it does not update visibility", %{conn: conn, user: user} do
       {:ok, activity} =
         CommonAPI.post(user, %{

test/pleroma/web/mastodon_api/views/scheduled_activity_view_test.exs

@@ -47,8 +47,7 @@ defmodule Pleroma.Web.MastodonAPI.ScheduledActivityViewTest do
     expected = %{
       id: to_string(scheduled_activity.id),
       media_attachments:
-        %{media_ids: [upload.id]}
+        Utils.attachments_from_ids(user, %{media_ids: [upload.id]})
-        |> Utils.attachments_from_ids()
         |> Enum.map(&StatusView.render("attachment.json", %{attachment: &1})),
       params: %{
         in_reply_to_id: to_string(activity.id),