logo

pleroma

My custom branche(s) on git.pleroma.social/pleroma/pleroma
commit: 96f7b1b140979249cf28bc742b7749f1bee7ea9c
parent: 1199cf3a788334cd3fdb968d9f736e43c1401da1
Author: lain <lain@soykaf.club>
Date:   Mon, 18 May 2020 10:57:59 +0000

Merge branch 'show-media-endpoint-fixes' into 'develop'

MediaController OAuth scope assignments fix

See merge request pleroma/pleroma!2541

Diffstat:

Mlib/pleroma/object.ex15++++++++++-----
Mlib/pleroma/web/mastodon_api/controllers/fallback_controller.ex4++++
Mlib/pleroma/web/mastodon_api/controllers/media_controller.ex12+++++++-----
Mtest/web/mastodon_api/controllers/media_controller_test.exs30++++++++++++++++++++++++------
4 files changed, 45 insertions(+), 16 deletions(-)

diff --git a/lib/pleroma/object.ex b/lib/pleroma/object.ex @@ -138,12 +138,17 @@ defmodule Pleroma.Object do def normalize(_, _, _), do: nil - # Owned objects can only be mutated by their owner - def authorize_mutation(%Object{data: %{"actor" => actor}}, %User{ap_id: ap_id}), - do: actor == ap_id + # Owned objects can only be accessed by their owner + def authorize_access(%Object{data: %{"actor" => actor}}, %User{ap_id: ap_id}) do + if actor == ap_id do + :ok + else + {:error, :forbidden} + end + end - # Legacy objects can be mutated by anybody - def authorize_mutation(%Object{}, %User{}), do: true + # Legacy objects can be accessed by anybody + def authorize_access(%Object{}, %User{}), do: :ok @spec get_cached_by_ap_id(String.t()) :: Object.t() | nil def get_cached_by_ap_id(ap_id) do diff --git a/lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex b/lib/pleroma/web/mastodon_api/controllers/fallback_controller.ex @@ -20,6 +20,10 @@ defmodule Pleroma.Web.MastodonAPI.FallbackController do render_error(conn, :not_found, "Record not found") end + def call(conn, {:error, :forbidden}) do + render_error(conn, :forbidden, "Access denied") + end + def call(conn, {:error, error_message}) do conn |> put_status(:bad_request) diff --git a/lib/pleroma/web/mastodon_api/controllers/media_controller.ex b/lib/pleroma/web/mastodon_api/controllers/media_controller.ex @@ -14,7 +14,8 @@ defmodule Pleroma.Web.MastodonAPI.MediaController do plug(Pleroma.Web.ApiSpec.CastAndValidate) plug(:put_view, Pleroma.Web.MastodonAPI.StatusView) - plug(OAuthScopesPlug, %{scopes: ["write:media"]}) + plug(OAuthScopesPlug, %{scopes: ["read:media"]} when action == :show) + plug(OAuthScopesPlug, %{scopes: ["write:media"]} when action != :show) defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.MediaOperation @@ -55,7 +56,7 @@ 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), - true <- Object.authorize_mutation(object, user), + :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) @@ -66,13 +67,14 @@ defmodule Pleroma.Web.MastodonAPI.MediaController do def update(conn, data), do: show(conn, data) @doc "GET /api/v1/media/:id" - def show(conn, %{id: id}) do - with %Object{data: data, id: object_id} <- Object.get_by_id(id) do + def show(%{assigns: %{user: user}} = conn, %{id: id}) do + with %Object{data: data, id: object_id} = object <- Object.get_by_id(id), + :ok <- Object.authorize_access(object, user) do attachment_data = Map.put(data, "id", object_id) render(conn, "attachment.json", %{attachment: attachment_data}) end end - def get_media(_conn, _data), do: {:error, :bad_request} + def show(_conn, _data), do: {:error, :bad_request} end diff --git a/test/web/mastodon_api/controllers/media_controller_test.exs b/test/web/mastodon_api/controllers/media_controller_test.exs @@ -9,9 +9,9 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do alias Pleroma.User alias Pleroma.Web.ActivityPub.ActivityPub - setup do: oauth_access(["write:media"]) - describe "Upload media" do + setup do: oauth_access(["write:media"]) + setup do image = %Plug.Upload{ content_type: "image/jpg", @@ -42,7 +42,7 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do assert object.data["actor"] == User.ap_id(conn.assigns[:user]) end - test "/api/v2/media", %{conn: conn, image: image} do + test "/api/v2/media", %{conn: conn, user: user, image: image} do desc = "Description of the image" response = @@ -53,6 +53,8 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do assert media_id = response["id"] + %{conn: conn} = oauth_access(["read:media"], user: user) + media = conn |> get("/api/v1/media/#{media_id}") @@ -61,12 +63,15 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do assert media["type"] == "image" assert media["description"] == desc assert media["id"] + object = Object.get_by_id(media["id"]) - assert object.data["actor"] == User.ap_id(conn.assigns[:user]) + assert object.data["actor"] == user.ap_id end end describe "Update media description" do + setup do: oauth_access(["write:media"]) + setup %{user: actor} do file = %Plug.Upload{ content_type: "image/jpg", @@ -96,7 +101,9 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do end end - describe "Get media by id" do + describe "Get media by id (/api/v1/media/:id)" do + setup do: oauth_access(["read:media"]) + setup %{user: actor} do file = %Plug.Upload{ content_type: "image/jpg", @@ -114,7 +121,7 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do [object: object] end - test "/api/v1/media/:id", %{conn: conn, object: object} do + test "it returns media object when requested by owner", %{conn: conn, object: object} do media = conn |> get("/api/v1/media/#{object.id}") @@ -124,5 +131,16 @@ defmodule Pleroma.Web.MastodonAPI.MediaControllerTest do assert media["type"] == "image" assert media["id"] 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"]) + + assert object.data["actor"] == user.ap_id + refute user.id == other_user.id + + conn + |> get("/api/v1/media/#{object.id}") + |> json_response(403) + end end end