From 4baea6e6d9efa619402a031a84f74787653df2b5 Mon Sep 17 00:00:00 2001 From: rinpatch Date: Wed, 24 Apr 2019 20:01:42 +0300 Subject: [PATCH] Fix leaking private configuration parameters in Mastodon and Twitter APIs, and add new configuration parameters to Mastodon API This patch: - Fixes `rights` in twitterapi ignoring `show_role` - Fixes exposing default scope of the user to anyone in Mastodon API - Extends Mastodon API to be able to show and set `no_rich_text`, `default_scope`, `hide_follows`, `hide_followers`, `hide_favorites` (requested by the FE in #674) Sorry in advance for 500 line one commit diff, I should have split it up to separate MRs --- CHANGELOG.md | 7 +- docs/api/differences_in_mastoapi_responses.md | 17 ++- lib/pleroma/user/info.ex | 8 -- .../mastodon_api/mastodon_api_controller.ex | 14 ++- .../web/mastodon_api/views/account_view.ex | 55 ++++++--- .../web/twitter_api/views/user_view.ex | 110 ++++++++++-------- test/web/mastodon_api/account_view_test.exs | 20 +++- .../mastodon_api_controller_test.exs | 60 ++++++++++ test/web/twitter_api/views/user_view_test.exs | 33 ++---- 9 files changed, 219 insertions(+), 105 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 70381f382..51ba239b6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,7 +44,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: Add `languages` and `registrations` to `/api/v1/instance` - Mastodon API: Provide plaintext versions of cw/content in the Status entity - Mastodon API: Add `pleroma.conversation_id`, `pleroma.in_reply_to_account_acct` fields to the Status entity -- Mastodon API: Add `pleroma.tags`, `pleroma.relationship{}`, `pleroma.is_moderator`, `pleroma.is_admin`, `pleroma.confirmation_pending` fields to the User entity +- Mastodon API: Add `pleroma.tags`, `pleroma.relationship{}`, `pleroma.is_moderator`, `pleroma.is_admin`, `pleroma.confirmation_pending`, `pleroma.hide_followers`, `pleroma.hide_follows`, `pleroma.hide_favorites` fields to the User entity +- Mastodon API: Add `pleroma.show_role`, `pleroma.no_rich_text` fields to the User entity (when the user is requesting themselves) +- Mastodon API: Add support for updating `no_rich_text`, `hide_followers`, `hide_follows`, `hide_favorites`, `show_role` in `PATCH /api/v1/update_credentials` - Mastodon API: Add `pleroma.is_seen` to the Notification entity - Mastodon API: Add `pleroma.local` to the Status entity - Mastodon API: Add `preview` parameter to `POST /api/v1/statuses` @@ -72,12 +74,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - MediaProxy: S3 link encoding - Rich Media: Reject any data which cannot be explicitly encoded into JSON - Pleroma API: Importing follows from Mastodon 2.8+ +- Twitter API: Exposing default scope, `no_rich_text` of the user to anyone +- Twitter API: Returning the `role` object in user entity despite `show_role = false` - Mastodon API: `/api/v1/favourites` serving only public activities - Mastodon API: Reblogs having `in_reply_to_id` - `null` even when they are replies - Mastodon API: Streaming API broadcasting wrong activity id - Mastodon API: 500 errors when requesting a card for a private conversation - Mastodon API: Handling of `reblogs` in `/api/v1/accounts/:id/follow` - Mastodon API: Correct `reblogged`, `favourited`, and `bookmarked` values in the reblog status JSON +- Mastodon API: Exposing default scope of the user to anyone ## [0.9.9999] - 2019-04-05 ### Security diff --git a/docs/api/differences_in_mastoapi_responses.md b/docs/api/differences_in_mastoapi_responses.md index 3bb1bd41f..7f05527fb 100644 --- a/docs/api/differences_in_mastoapi_responses.md +++ b/docs/api/differences_in_mastoapi_responses.md @@ -38,9 +38,12 @@ Has these additional fields under the `pleroma` object: - `tags`: Lists an array of tags for the user - `relationship{}`: Includes fields as documented for Mastodon API https://docs.joinmastodon.org/api/entities/#relationship -- `is_moderator`: boolean, true if user is a moderator -- `is_admin`: boolean, true if user is an admin +- `is_moderator`: boolean, nullable, true if user is a moderator +- `is_admin`: boolean, nullable, true if user is an admin - `confirmation_pending`: boolean, true if a new user account is waiting on email confirmation to be activated +- `hide_followers`: boolean, true when the user has follower hiding enabled +- `hide_follows`: boolean, true when the user has follow hiding enabled +- `show_role`: boolean, nullable (only shown when the user is requesting themselves), true when the user wants his role (e.g admin, moderator) to be shown ## Account Search @@ -60,3 +63,13 @@ Additional parameters can be added to the JSON body/Form data: - `preview`: boolean, if set to `true` the post won't be actually posted, but the status entitiy would still be rendered back. This could be useful for previewing rich text/custom emoji, for example. - `content_type`: string, contain the MIME type of the status, it is transformed into HTML by the backend. You can get the list of the supported MIME types with the nodeinfo endpoint. + +## PATCH `/api/v1/update_credentials` + +Additional parameters can be added to the JSON body/Form data: + +- `no_rich_text` - if true, html tags are stripped from all statuses requested from the API +- `hide_followers` - if true, user's followers will be hidden +- `hide_follows` - if true, user's follows will be hidden +- `hide_favorites` - if true, user's favorites timeline will be hidden +- `show_role` - if true, user's role (e.g admin, moderator) will be exposed to anyone in the API diff --git a/lib/pleroma/user/info.ex b/lib/pleroma/user/info.ex index 7f22a45b5..a3658d57f 100644 --- a/lib/pleroma/user/info.ex +++ b/lib/pleroma/user/info.ex @@ -227,14 +227,6 @@ def confirmation_changeset(info, params) do cast(info, params, [:confirmation_pending, :confirmation_token]) end - def mastodon_profile_update(info, params) do - info - |> cast(params, [ - :locked, - :banner - ]) - end - def mastodon_settings_update(info, settings) do params = %{settings: settings} diff --git a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex index 0ba8d9eea..1379baacf 100644 --- a/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex +++ b/lib/pleroma/web/mastodon_api/mastodon_api_controller.ex @@ -35,7 +35,7 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do alias Pleroma.Web.OAuth.Authorization alias Pleroma.Web.OAuth.Token - import Pleroma.Web.ControllerHelper, only: [oauth_scopes: 2] + alias Pleroma.Web.ControllerHelper import Ecto.Query require Logger @@ -46,7 +46,7 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do action_fallback(:errors) def create_app(conn, params) do - scopes = oauth_scopes(params, ["read"]) + scopes = ControllerHelper.oauth_scopes(params, ["read"]) app_attrs = params @@ -96,8 +96,12 @@ def update_credentials(%{assigns: %{user: user}} = conn, params) do end) info_params = - %{} - |> add_if_present(params, "locked", :locked, fn value -> {:ok, value == "true"} end) + [:no_rich_text, :locked, :hide_followers, :hide_follows, :hide_favorites, :show_role] + |> Enum.reduce(%{}, fn key, acc -> + add_if_present(acc, params, to_string(key), key, fn value -> + {:ok, ControllerHelper.truthy_param?(value)} + end) + end) |> add_if_present(params, "header", :banner, fn value -> with %Plug.Upload{} <- value, {:ok, object} <- ActivityPub.upload(value, type: :banner) do @@ -107,7 +111,7 @@ def update_credentials(%{assigns: %{user: user}} = conn, params) do end end) - info_cng = User.Info.mastodon_profile_update(user.info, info_params) + info_cng = User.Info.profile_update(user.info, info_params) with changeset <- User.update_changeset(user, user_params), changeset <- Ecto.Changeset.put_embed(changeset, :info, info_cng), diff --git a/lib/pleroma/web/mastodon_api/views/account_view.ex b/lib/pleroma/web/mastodon_api/views/account_view.ex index d87fdb15d..6e6f0ba93 100644 --- a/lib/pleroma/web/mastodon_api/views/account_view.ex +++ b/lib/pleroma/web/mastodon_api/views/account_view.ex @@ -113,21 +113,22 @@ defp do_render("account.json", %{user: user} = opts) do bot: bot, source: %{ note: "", - privacy: user_info.default_scope, sensitive: false }, # Pleroma extension - pleroma: - %{ - confirmation_pending: user_info.confirmation_pending, - tags: user.tags, - is_moderator: user.info.is_moderator, - is_admin: user.info.is_admin, - relationship: relationship - } - |> with_notification_settings(user, opts[:for]) + pleroma: %{ + confirmation_pending: user_info.confirmation_pending, + tags: user.tags, + hide_followers: user.info.hide_followers, + hide_follows: user.info.hide_follows, + hide_favorites: user.info.hide_favorites, + relationship: relationship + } } + |> maybe_put_role(user, opts[:for]) + |> maybe_put_settings(user, opts[:for], user_info) + |> maybe_put_notification_settings(user, opts[:for]) end defp username_from_nickname(string) when is_binary(string) do @@ -136,9 +137,37 @@ defp username_from_nickname(string) when is_binary(string) do defp username_from_nickname(_), do: nil - defp with_notification_settings(data, %User{id: user_id} = user, %User{id: user_id}) do - Map.put(data, :notification_settings, user.info.notification_settings) + defp maybe_put_settings( + data, + %User{id: user_id} = user, + %User{id: user_id}, + user_info + ) do + data + |> Kernel.put_in([:source, :privacy], user_info.default_scope) + |> Kernel.put_in([:pleroma, :show_role], user.info.show_role) + |> Kernel.put_in([:pleroma, :no_rich_text], user.info.no_rich_text) end - defp with_notification_settings(data, _, _), do: data + defp maybe_put_settings(data, _, _, _), do: data + + defp maybe_put_role(data, %User{info: %{show_role: true}} = user, _) do + data + |> Kernel.put_in([:pleroma, :is_admin], user.info.is_admin) + |> Kernel.put_in([:pleroma, :is_moderator], user.info.is_moderator) + end + + defp maybe_put_role(data, %User{id: user_id} = user, %User{id: user_id}) do + data + |> Kernel.put_in([:pleroma, :is_admin], user.info.is_admin) + |> Kernel.put_in([:pleroma, :is_moderator], user.info.is_moderator) + end + + defp maybe_put_role(data, _, _), do: data + + defp maybe_put_notification_settings(data, %User{id: user_id} = user, %User{id: user_id}) do + Kernel.put_in(data, [:pleroma, :notification_settings], user.info.notification_settings) + end + + defp maybe_put_notification_settings(data, _, _), do: data end diff --git a/lib/pleroma/web/twitter_api/views/user_view.ex b/lib/pleroma/web/twitter_api/views/user_view.ex index 0791ed760..39b3f21c0 100644 --- a/lib/pleroma/web/twitter_api/views/user_view.ex +++ b/lib/pleroma/web/twitter_api/views/user_view.ex @@ -74,52 +74,48 @@ defp do_render("user.json", %{user: user = %User{}} = assigns) do |> Enum.filter(fn %{"type" => t} -> t == "PropertyValue" end) |> Enum.map(fn fields -> Map.take(fields, ["name", "value"]) end) - data = %{ - "created_at" => user.inserted_at |> Utils.format_naive_asctime(), - "description" => HTML.strip_tags((user.bio || "") |> String.replace("
", "\n")), - "description_html" => HTML.filter_tags(user.bio, User.html_filter_policy(for_user)), - "favourites_count" => 0, - "followers_count" => user_info[:follower_count], - "following" => following, - "follows_you" => follows_you, - "statusnet_blocking" => statusnet_blocking, - "friends_count" => user_info[:following_count], - "id" => user.id, - "name" => user.name || user.nickname, - "name_html" => - if(user.name, - do: HTML.strip_tags(user.name) |> Formatter.emojify(emoji), - else: user.nickname - ), - "profile_image_url" => image, - "profile_image_url_https" => image, - "profile_image_url_profile_size" => image, - "profile_image_url_original" => image, - "rights" => %{ - "delete_others_notice" => !!user.info.is_moderator, - "admin" => !!user.info.is_admin - }, - "screen_name" => user.nickname, - "statuses_count" => user_info[:note_count], - "statusnet_profile_url" => user.ap_id, - "cover_photo" => User.banner_url(user) |> MediaProxy.url(), - "background_image" => image_url(user.info.background) |> MediaProxy.url(), - "is_local" => user.local, - "locked" => user.info.locked, - "default_scope" => user.info.default_scope, - "no_rich_text" => user.info.no_rich_text, - "hide_followers" => user.info.hide_followers, - "hide_follows" => user.info.hide_follows, - "fields" => fields, + data = + %{ + "created_at" => user.inserted_at |> Utils.format_naive_asctime(), + "description" => HTML.strip_tags((user.bio || "") |> String.replace("
", "\n")), + "description_html" => HTML.filter_tags(user.bio, User.html_filter_policy(for_user)), + "favourites_count" => 0, + "followers_count" => user_info[:follower_count], + "following" => following, + "follows_you" => follows_you, + "statusnet_blocking" => statusnet_blocking, + "friends_count" => user_info[:following_count], + "id" => user.id, + "name" => user.name || user.nickname, + "name_html" => + if(user.name, + do: HTML.strip_tags(user.name) |> Formatter.emojify(emoji), + else: user.nickname + ), + "profile_image_url" => image, + "profile_image_url_https" => image, + "profile_image_url_profile_size" => image, + "profile_image_url_original" => image, + "screen_name" => user.nickname, + "statuses_count" => user_info[:note_count], + "statusnet_profile_url" => user.ap_id, + "cover_photo" => User.banner_url(user) |> MediaProxy.url(), + "background_image" => image_url(user.info.background) |> MediaProxy.url(), + "is_local" => user.local, + "locked" => user.info.locked, + "hide_followers" => user.info.hide_followers, + "hide_follows" => user.info.hide_follows, + "fields" => fields, - # Pleroma extension - "pleroma" => - %{ - "confirmation_pending" => user_info.confirmation_pending, - "tags" => user.tags - } - |> maybe_with_activation_status(user, for_user) - } + # Pleroma extension + "pleroma" => + %{ + "confirmation_pending" => user_info.confirmation_pending, + "tags" => user.tags + } + |> maybe_with_activation_status(user, for_user) + } + |> maybe_with_user_settings(user, for_user) data = if(user.info.is_admin || user.info.is_moderator, @@ -141,15 +137,35 @@ defp maybe_with_activation_status(data, user, %User{info: %{is_admin: true}}) do defp maybe_with_activation_status(data, _, _), do: data defp maybe_with_role(data, %User{id: id} = user, %User{id: id}) do - Map.merge(data, %{"role" => role(user), "show_role" => user.info.show_role}) + Map.merge(data, %{ + "role" => role(user), + "show_role" => user.info.show_role, + "rights" => %{ + "delete_others_notice" => !!user.info.is_moderator, + "admin" => !!user.info.is_admin + } + }) end defp maybe_with_role(data, %User{info: %{show_role: true}} = user, _user) do - Map.merge(data, %{"role" => role(user)}) + Map.merge(data, %{ + "role" => role(user), + "rights" => %{ + "delete_others_notice" => !!user.info.is_moderator, + "admin" => !!user.info.is_admin + } + }) end defp maybe_with_role(data, _, _), do: data + defp maybe_with_user_settings(data, %User{info: info, id: id} = _user, %User{id: id}) do + data + |> Kernel.put_in(["default_scope"], info.default_scope) + |> Kernel.put_in(["no_rich_text"], info.no_rich_text) + end + + defp maybe_with_user_settings(data, _, _), do: data defp role(%User{info: %{:is_admin => true}}), do: "admin" defp role(%User{info: %{:is_moderator => true}}), do: "moderator" defp role(_), do: "member" diff --git a/test/web/mastodon_api/account_view_test.exs b/test/web/mastodon_api/account_view_test.exs index 0730201bd..db870f1d1 100644 --- a/test/web/mastodon_api/account_view_test.exs +++ b/test/web/mastodon_api/account_view_test.exs @@ -56,7 +56,6 @@ test "Represent a user account" do bot: false, source: %{ note: "", - privacy: "public", sensitive: false }, pleroma: %{ @@ -64,6 +63,9 @@ test "Represent a user account" do tags: [], is_admin: false, is_moderator: false, + hide_favorites: true, + hide_followers: false, + hide_follows: false, relationship: %{} } } @@ -81,8 +83,12 @@ test "Represent the user account for the account owner" do "follows" => true } - assert %{pleroma: %{notification_settings: ^notification_settings}} = - AccountView.render("account.json", %{user: user, for: user}) + privacy = user.info.default_scope + + assert %{ + pleroma: %{notification_settings: ^notification_settings}, + source: %{privacy: ^privacy} + } = AccountView.render("account.json", %{user: user, for: user}) end test "Represent a Service(bot) account" do @@ -114,7 +120,6 @@ test "Represent a Service(bot) account" do bot: true, source: %{ note: "", - privacy: "public", sensitive: false }, pleroma: %{ @@ -122,6 +127,9 @@ test "Represent a Service(bot) account" do tags: [], is_admin: false, is_moderator: false, + hide_favorites: true, + hide_followers: false, + hide_follows: false, relationship: %{} } } @@ -200,7 +208,6 @@ test "represent an embedded relationship" do bot: true, source: %{ note: "", - privacy: "public", sensitive: false }, pleroma: %{ @@ -208,6 +215,9 @@ test "represent an embedded relationship" do tags: [], is_admin: false, is_moderator: false, + hide_favorites: true, + hide_followers: false, + hide_follows: false, relationship: %{ id: to_string(user.id), following: false, diff --git a/test/web/mastodon_api/mastodon_api_controller_test.exs b/test/web/mastodon_api/mastodon_api_controller_test.exs index a22944088..0c52dd3e3 100644 --- a/test/web/mastodon_api/mastodon_api_controller_test.exs +++ b/test/web/mastodon_api/mastodon_api_controller_test.exs @@ -2214,6 +2214,66 @@ test "updates the user's locking status", %{conn: conn} do assert user["locked"] == true end + test "updates the user's hide_followers status", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:user, user) + |> patch("/api/v1/accounts/update_credentials", %{hide_followers: "true"}) + + assert user = json_response(conn, 200) + assert user["pleroma"]["hide_followers"] == true + end + + test "updates the user's hide_follows status", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:user, user) + |> patch("/api/v1/accounts/update_credentials", %{hide_follows: "true"}) + + assert user = json_response(conn, 200) + assert user["pleroma"]["hide_follows"] == true + end + + test "updates the user's hide_favorites status", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:user, user) + |> patch("/api/v1/accounts/update_credentials", %{hide_favorites: "true"}) + + assert user = json_response(conn, 200) + assert user["pleroma"]["hide_favorites"] == true + end + + test "updates the user's show_role status", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:user, user) + |> patch("/api/v1/accounts/update_credentials", %{show_role: "false"}) + + assert user = json_response(conn, 200) + assert user["pleroma"]["show_role"] == false + end + + test "updates the user's no_rich_text status", %{conn: conn} do + user = insert(:user) + + conn = + conn + |> assign(:user, user) + |> patch("/api/v1/accounts/update_credentials", %{no_rich_text: "true"}) + + assert user = json_response(conn, 200) + assert user["pleroma"]["show_role"] == true + end + test "updates the user's name", %{conn: conn} do user = insert(:user) diff --git a/test/web/twitter_api/views/user_view_test.exs b/test/web/twitter_api/views/user_view_test.exs index 36b461992..2f9b2af01 100644 --- a/test/web/twitter_api/views/user_view_test.exs +++ b/test/web/twitter_api/views/user_view_test.exs @@ -89,17 +89,11 @@ test "A user" do "following" => false, "follows_you" => false, "statusnet_blocking" => false, - "rights" => %{ - "delete_others_notice" => false, - "admin" => false - }, "statusnet_profile_url" => user.ap_id, "cover_photo" => banner, "background_image" => nil, "is_local" => true, "locked" => false, - "default_scope" => "public", - "no_rich_text" => false, "hide_follows" => false, "hide_followers" => false, "fields" => [], @@ -112,6 +106,15 @@ test "A user" do assert represented == UserView.render("show.json", %{user: user}) end + test "User exposes settings for themselves and only for themselves", %{user: user} do + as_user = UserView.render("show.json", %{user: user, for: user}) + assert as_user["default_scope"] == user.info.default_scope + assert as_user["no_rich_text"] == user.info.no_rich_text + as_stranger = UserView.render("show.json", %{user: user}) + refute as_stranger["default_scope"] + refute as_stranger["no_rich_text"] + end + test "A user for a given other follower", %{user: user} do follower = insert(:user, %{following: [User.ap_followers(user)]}) {:ok, user} = User.update_follower_count(user) @@ -137,17 +140,11 @@ test "A user for a given other follower", %{user: user} do "following" => true, "follows_you" => false, "statusnet_blocking" => false, - "rights" => %{ - "delete_others_notice" => false, - "admin" => false - }, "statusnet_profile_url" => user.ap_id, "cover_photo" => banner, "background_image" => nil, "is_local" => true, "locked" => false, - "default_scope" => "public", - "no_rich_text" => false, "hide_follows" => false, "hide_followers" => false, "fields" => [], @@ -186,17 +183,11 @@ test "A user that follows you", %{user: user} do "following" => false, "follows_you" => true, "statusnet_blocking" => false, - "rights" => %{ - "delete_others_notice" => false, - "admin" => false - }, "statusnet_profile_url" => follower.ap_id, "cover_photo" => banner, "background_image" => nil, "is_local" => true, "locked" => false, - "default_scope" => "public", - "no_rich_text" => false, "hide_follows" => false, "hide_followers" => false, "fields" => [], @@ -272,17 +263,11 @@ test "A blocked user for the blocker" do "following" => false, "follows_you" => false, "statusnet_blocking" => true, - "rights" => %{ - "delete_others_notice" => false, - "admin" => false - }, "statusnet_profile_url" => user.ap_id, "cover_photo" => banner, "background_image" => nil, "is_local" => true, "locked" => false, - "default_scope" => "public", - "no_rich_text" => false, "hide_follows" => false, "hide_followers" => false, "fields" => [],