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
2024-11-18 19:14:03 +00:00 · 2019-04-24 20:01:42 +03:00 · 2019-04-24 20:01:42 +03:00 · 4baea6e6d9
parent 030a7876b4
commit 4baea6e6d9
9 changed files with 219 additions and 105 deletions
--- 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
--- 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
--- 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}

--- 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),
--- 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
--- 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("<br>", "\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("<br>", "\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"
--- 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,
--- 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)

--- 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" => [],