Fixed OAuth restrictions for :api routes. Made auth info dropped for :api routes if OAuth check was neither performed nor explicitly skipped.

docs/dev.md

@@ -4,29 +4,19 @@ This document contains notes and guidelines for Pleroma developers.
 
 ## OAuth token-based authentication & authorization
 
-* Pleroma supports hierarchical OAuth scopes, just like Mastodon but with added granularity of admin scopes.
-  For a reference, see [Mastodon OAuth scopes](https://docs.joinmastodon.org/api/oauth-scopes/).
+* Pleroma supports hierarchical OAuth scopes, just like Mastodon but with added granularity of admin scopes. For a reference, see [Mastodon OAuth scopes](https://docs.joinmastodon.org/api/oauth-scopes/).
 
-* It is important to either define OAuth scope restrictions or explicitly mark OAuth scope check as skipped, for every 
-    controller action. To define scopes, call `plug(Pleroma.Plugs.OAuthScopesPlug, %{scopes: [...]})`. To explicitly set 
-    OAuth scopes check skipped, call `plug(:skip_plug, Pleroma.Plugs.OAuthScopesPlug <when ...>)`.
+* It is important to either define OAuth scope restrictions or explicitly mark OAuth scope check as skipped, for every controller action. To define scopes, call `plug(Pleroma.Plugs.OAuthScopesPlug, %{scopes: [...]})`. To explicitly set OAuth scopes check skipped, call `plug(:skip_plug, Pleroma.Plugs.OAuthScopesPlug <when ...>)`.
 
-* In controllers, `use Pleroma.Web, :controller` will result in `action/2` (see `Pleroma.Web.controller/0` for definition)
-    be called prior to actual controller action, and it'll perform security / privacy checks before passing control to
-    actual controller action. For routes with `:authenticated_api` pipeline, authentication & authorization are expected,
-    thus `OAuthScopesPlug` will be run unless explicitly skipped (also `EnsureAuthenticatedPlug` will be executed
-    immediately before action even if there was an early run to give an early error, since `OAuthScopesPlug` supports
-    `:proceed_unauthenticated` option, and other plugs may support similar options as well). For `:api` pipeline routes,
-    `EnsurePublicOrAuthenticatedPlug` will be called to ensure that the instance is not private or user is authenticated
-    (unless explicitly skipped). Such automated checks help to prevent human errors and result in higher security / privacy
-    for users.
+* In controllers, `use Pleroma.Web, :controller` will result in `action/2` (see `Pleroma.Web.controller/0` for definition) be called prior to actual controller action, and it'll perform security / privacy checks before passing control to actual controller action.
+
+  For routes with `:authenticated_api` pipeline, authentication & authorization are expected, thus `OAuthScopesPlug` will be run unless explicitly skipped (also `EnsureAuthenticatedPlug` will be executed immediately before action even if there was an early run to give an early error, since `OAuthScopesPlug` supports `:proceed_unauthenticated` option, and other plugs may support similar options as well).
+
+  For `:api` pipeline routes, it'll be verified whether `OAuthScopesPlug` was called or explicitly skipped, and if it was not then auth information will be dropped for request. Then `EnsurePublicOrAuthenticatedPlug` will be called to ensure that either the instance is not private or user is authenticated (unless explicitly skipped). Such automated checks help to prevent human errors and result in higher security / privacy for users.
 
 ## [HTTP Basic Authentication](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Authorization)
 
-* With HTTP Basic Auth, OAuth scopes check is _not_ performed for any action (since password is provided during the auth,
-    requester is able to obtain a token with full permissions anyways). `Pleroma.Plugs.AuthenticationPlug` and
-    `Pleroma.Plugs.LegacyAuthenticationPlug` both call `Pleroma.Plugs.OAuthScopesPlug.skip_plug(conn)` when password
-    is provided.
+* With HTTP Basic Auth, OAuth scopes check is _not_ performed for any action (since password is provided during the auth, requester is able to obtain a token with full permissions anyways). `Pleroma.Plugs.AuthenticationPlug` and `Pleroma.Plugs.LegacyAuthenticationPlug` both call `Pleroma.Plugs.OAuthScopesPlug.skip_plug(conn)` when password is provided.
 
 ## Auth-related configuration, OAuth consumer mode etc.
 

lib/pleroma/plugs/oauth_scopes_plug.ex

@@ -28,9 +28,7 @@ def perform(%Plug.Conn{assigns: assigns} = conn, %{scopes: scopes} = options) do
         conn
 
       options[:fallback] == :proceed_unauthenticated ->
-        conn
-        |> assign(:user, nil)
-        |> assign(:token, nil)
+        drop_auth_info(conn)
 
       true ->
         missing_scopes = scopes -- matched_scopes
@@ -46,6 +44,15 @@ def perform(%Plug.Conn{assigns: assigns} = conn, %{scopes: scopes} = options) do
     end
   end
 
+  @doc "Drops authentication info from connection"
+  def drop_auth_info(conn) do
+    # To simplify debugging, setting a private variable on `conn` if auth info is dropped
+    conn
+    |> put_private(:authentication_ignored, true)
+    |> assign(:user, nil)
+    |> assign(:token, nil)
+  end
+
   @doc "Filters descendants of supported scopes"
   def filter_descendants(scopes, supported_scopes) do
     Enum.filter(

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

@@ -37,7 +37,7 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
   plug(
     OAuthScopesPlug,
     %{fallback: :proceed_unauthenticated, scopes: ["read:accounts"]}
-    when action in [:show, :endorsements]
+    when action in [:show, :followers, :following, :endorsements]
   )
 
   plug(
@@ -49,7 +49,7 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
   plug(
     OAuthScopesPlug,
     %{scopes: ["read:accounts"]}
-    when action in [:endorsements, :verify_credentials, :followers, :following]
+    when action in [:endorsements, :verify_credentials]
   )
 
   plug(OAuthScopesPlug, %{scopes: ["write:accounts"]} when action == :update_credentials)

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

@@ -5,6 +5,7 @@
 defmodule Pleroma.Web.MastodonAPI.AppController do
   use Pleroma.Web, :controller
 
+  alias Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug
   alias Pleroma.Plugs.OAuthScopesPlug
   alias Pleroma.Repo
   alias Pleroma.Web.OAuth.App
@@ -13,7 +14,14 @@ defmodule Pleroma.Web.MastodonAPI.AppController do
 
   action_fallback(Pleroma.Web.MastodonAPI.FallbackController)
 
+  plug(
+    :skip_plug,
+    [OAuthScopesPlug, EnsurePublicOrAuthenticatedPlug]
+    when action == :create
+  )
+
   plug(OAuthScopesPlug, %{scopes: ["read"]} when action == :verify_credentials)
+
   plug(OpenApiSpex.Plug.CastAndValidate)
 
   @local_mastodon_name "Mastodon-Local"

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

@@ -7,6 +7,12 @@ defmodule Pleroma.Web.MastodonAPI.CustomEmojiController do
 
   plug(OpenApiSpex.Plug.CastAndValidate)
 
+  plug(
+    :skip_plug,
+    [Pleroma.Plugs.OAuthScopesPlug, Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug]
+    when action == :index
+  )
+
   defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.CustomEmojiOperation
 
   def index(conn, _params) do

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

@@ -5,6 +5,12 @@
 defmodule Pleroma.Web.MastodonAPI.InstanceController do
   use Pleroma.Web, :controller
 
+  plug(
+    :skip_plug,
+    [Pleroma.Plugs.OAuthScopesPlug, Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug]
+    when action in [:show, :peers]
+  )
+
   @doc "GET /api/v1/instance"
   def show(conn, _params) do
     render(conn, "show.json")

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

@@ -15,7 +15,11 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do
 
   require Logger
 
-  plug(:skip_plug, Pleroma.Plugs.OAuthScopesPlug when action in [:empty_array, :empty_object])
+  plug(
+    :skip_plug,
+    [Pleroma.Plugs.OAuthScopesPlug, Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug]
+    when action in [:empty_array, :empty_object]
+  )
 
   action_fallback(Pleroma.Web.MastodonAPI.FallbackController)
 

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

@@ -21,6 +21,8 @@ defmodule Pleroma.Web.MastodonAPI.SearchController do
   # Note: Mastodon doesn't allow unauthenticated access (requires read:accounts / read:search)
   plug(OAuthScopesPlug, %{scopes: ["read:search"], fallback: :proceed_unauthenticated})
 
+  # Note: on private instances auth is required (EnsurePublicOrAuthenticatedPlug is not skipped)
+
   plug(RateLimiter, [name: :search] when action in [:search, :search2, :account_search])
 
   def account_search(%{assigns: %{user: user}} = conn, %{"q" => query} = params) do

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

@@ -9,6 +9,7 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
     only: [add_link_headers: 2, add_link_headers: 3, truthy_param?: 1, skip_relationships?: 1]
 
   alias Pleroma.Pagination
+  alias Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug
   alias Pleroma.Plugs.OAuthScopesPlug
   alias Pleroma.Plugs.RateLimiter
   alias Pleroma.User
@@ -26,7 +27,13 @@ defmodule Pleroma.Web.MastodonAPI.TimelineController do
   plug(OAuthScopesPlug, %{scopes: ["read:statuses"]} when action in [:home, :direct])
   plug(OAuthScopesPlug, %{scopes: ["read:lists"]} when action == :list)
 
-  plug(:skip_plug, Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug when action == :public)
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["read:statuses"], fallback: :proceed_unauthenticated}
+    when action in [:public, :hashtag]
+  )
+
+  plug(:skip_plug, EnsurePublicOrAuthenticatedPlug when action in [:public, :hashtag])
 
   plug(:put_view, Pleroma.Web.MastodonAPI.StatusView)
 
@@ -93,7 +100,9 @@ def public(%{assigns: %{user: user}} = conn, params) do
 
     restrict? = Pleroma.Config.get([:restrict_unauthenticated, :timelines, cfg_key])
 
-    if not (restrict? and is_nil(user)) do
+    if restrict? and is_nil(user) do
+      render_error(conn, :unauthorized, "authorization required for timeline view")
+    else
       activities =
         params
         |> Map.put("type", ["Create", "Announce"])
@@ -110,12 +119,10 @@ def public(%{assigns: %{user: user}} = conn, params) do
         as: :activity,
         skip_relationships: skip_relationships?(params)
       )
-    else
-      render_error(conn, :unauthorized, "authorization required for timeline view")
     end
   end
 
-  def hashtag_fetching(params, user, local_only) do
+  defp hashtag_fetching(params, user, local_only) do
     tags =
       [params["tag"], params["any"]]
       |> List.flatten()

lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex

@@ -26,6 +26,12 @@ defmodule Pleroma.Web.PleromaAPI.PleromaAPIController do
     when action in [:conversation, :conversation_statuses]
   )
 
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["read:statuses"], fallback: :proceed_unauthenticated}
+    when action == :emoji_reactions_by
+  )
+
   plug(
     OAuthScopesPlug,
     %{scopes: ["write:statuses"]}

lib/pleroma/web/pleroma_api/controllers/scrobble_controller.ex

@@ -13,7 +13,11 @@ defmodule Pleroma.Web.PleromaAPI.ScrobbleController do
   alias Pleroma.Web.CommonAPI
   alias Pleroma.Web.MastodonAPI.StatusView
 
-  plug(OAuthScopesPlug, %{scopes: ["read"]} when action == :user_scrobbles)
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["read"], fallback: :proceed_unauthenticated} when action == :user_scrobbles
+  )
+
   plug(OAuthScopesPlug, %{scopes: ["write"]} when action != :user_scrobbles)
 
   def new_scrobble(%{assigns: %{user: user}} = conn, %{"title" => _} = params) do

lib/pleroma/web/router.ex

@@ -312,14 +312,10 @@ defmodule Pleroma.Web.Router do
       post("/scrobble", ScrobbleController, :new_scrobble)
     end
 
-    scope [] do
-      pipe_through(:api)
-      get("/accounts/:id/favourites", AccountController, :favourites)
-    end
-
     scope [] do
       pipe_through(:authenticated_api)
 
+      get("/accounts/:id/favourites", AccountController, :favourites)
       post("/accounts/:id/subscribe", AccountController, :subscribe)
       post("/accounts/:id/unsubscribe", AccountController, :unsubscribe)
     end
@@ -353,6 +349,8 @@ defmodule Pleroma.Web.Router do
     post("/accounts/:id/mute", AccountController, :mute)
     post("/accounts/:id/unmute", AccountController, :unmute)
 
+    get("/apps/verify_credentials", AppController, :verify_credentials)
+
     get("/conversations", ConversationController, :index)
     post("/conversations/:id/read", ConversationController, :mark_as_read)
 
@@ -431,6 +429,7 @@ defmodule Pleroma.Web.Router do
 
     get("/timelines/home", TimelineController, :home)
     get("/timelines/direct", TimelineController, :direct)
+    get("/timelines/list/:list_id", TimelineController, :list)
   end
 
   scope "/api/web", Pleroma.Web do
@@ -442,15 +441,24 @@ defmodule Pleroma.Web.Router do
   scope "/api/v1", Pleroma.Web.MastodonAPI do
     pipe_through(:api)
 
-    post("/accounts", AccountController, :create)
     get("/accounts/search", SearchController, :account_search)
+    get("/search", SearchController, :search)
+
+    get("/accounts/:id/statuses", AccountController, :statuses)
+    get("/accounts/:id/followers", AccountController, :followers)
+    get("/accounts/:id/following", AccountController, :following)
+    get("/accounts/:id", AccountController, :show)
+
+    post("/accounts", AccountController, :create)
 
     get("/instance", InstanceController, :show)
     get("/instance/peers", InstanceController, :peers)
 
     post("/apps", AppController, :create)
-    get("/apps/verify_credentials", AppController, :verify_credentials)
 
+    get("/statuses", StatusController, :index)
+    get("/statuses/:id", StatusController, :show)
+    get("/statuses/:id/context", StatusController, :context)
     get("/statuses/:id/card", StatusController, :card)
     get("/statuses/:id/favourited_by", StatusController, :favourited_by)
     get("/statuses/:id/reblogged_by", StatusController, :reblogged_by)
@@ -461,20 +469,8 @@ defmodule Pleroma.Web.Router do
 
     get("/timelines/public", TimelineController, :public)
     get("/timelines/tag/:tag", TimelineController, :hashtag)
-    get("/timelines/list/:list_id", TimelineController, :list)
-
-    get("/statuses", StatusController, :index)
-    get("/statuses/:id", StatusController, :show)
-    get("/statuses/:id/context", StatusController, :context)
 
     get("/polls/:id", PollController, :show)
-
-    get("/accounts/:id/statuses", AccountController, :statuses)
-    get("/accounts/:id/followers", AccountController, :followers)
-    get("/accounts/:id/following", AccountController, :following)
-    get("/accounts/:id", AccountController, :show)
-
-    get("/search", SearchController, :search)
   end
 
   scope "/api/v2", Pleroma.Web.MastodonAPI do

lib/pleroma/web/twitter_api/twitter_api_controller.ex

@@ -18,7 +18,7 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
     %{scopes: ["write:notifications"]} when action == :mark_notifications_as_read
   )
 
-  plug(:skip_plug, OAuthScopesPlug when action in [:oauth_tokens, :revoke_token])
+  plug(:skip_plug, OAuthScopesPlug when action in [:confirm_email, :oauth_tokens, :revoke_token])
 
   action_fallback(:errors)
 
@@ -47,13 +47,13 @@ def revoke_token(%{assigns: %{user: user}} = conn, %{"id" => id} = _params) do
     json_reply(conn, 201, "")
   end
 
-  def errors(conn, {:param_cast, _}) do
+  defp errors(conn, {:param_cast, _}) do
     conn
     |> put_status(400)
     |> json("Invalid parameters")
   end
 
-  def errors(conn, _) do
+  defp errors(conn, _) do
     conn
     |> put_status(500)
     |> json("Something went wrong")

lib/pleroma/web/web.ex

@@ -67,13 +67,25 @@ defp skip_plug(conn, plug_modules) do
 
       # Executed just before actual controller action, invokes before-action hooks (callbacks)
       defp action(conn, params) do
-        with %Plug.Conn{halted: false} <- maybe_perform_public_or_authenticated_check(conn),
+        with %Plug.Conn{halted: false} <- maybe_drop_authentication_if_oauth_check_ignored(conn),
+             %Plug.Conn{halted: false} <- maybe_perform_public_or_authenticated_check(conn),
              %Plug.Conn{halted: false} <- maybe_perform_authenticated_check(conn),
              %Plug.Conn{halted: false} <- maybe_halt_on_missing_oauth_scopes_check(conn) do
           super(conn, params)
         end
       end
 
+      # For non-authenticated API actions, drops auth info if OAuth scopes check was ignored
+      #   (neither performed nor explicitly skipped)
+      defp maybe_drop_authentication_if_oauth_check_ignored(conn) do
+        if PlugHelper.plug_called?(conn, ExpectPublicOrAuthenticatedCheckPlug) and
+             not PlugHelper.plug_called_or_skipped?(conn, OAuthScopesPlug) do
+          OAuthScopesPlug.drop_auth_info(conn)
+        else
+          conn
+        end
+      end
+
       # Ensures instance is public -or- user is authenticated if such check was scheduled
       defp maybe_perform_public_or_authenticated_check(conn) do
         if PlugHelper.plug_called?(conn, ExpectPublicOrAuthenticatedCheckPlug) do