Enforcement of OAuth scopes check for authenticated API endpoints, :skip_plug plug...
authorIvan Tashkinov <ivantashkinov@gmail.com>
Mon, 6 Apr 2020 07:20:44 +0000 (10:20 +0300)
committerIvan Tashkinov <ivantashkinov@gmail.com>
Mon, 6 Apr 2020 07:20:44 +0000 (10:20 +0300)
14 files changed:
lib/pleroma/plugs/auth_expected_plug.ex [new file with mode: 0644]
lib/pleroma/plugs/oauth_scopes_plug.ex
lib/pleroma/plugs/plug_helper.ex [new file with mode: 0644]
lib/pleroma/web/masto_fe_controller.ex
lib/pleroma/web/mastodon_api/controllers/account_controller.ex
lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
lib/pleroma/web/mastodon_api/controllers/suggestion_controller.ex
lib/pleroma/web/oauth/oauth_controller.ex
lib/pleroma/web/pleroma_api/controllers/pleroma_api_controller.ex
lib/pleroma/web/router.ex
lib/pleroma/web/twitter_api/twitter_api_controller.ex
lib/pleroma/web/web.ex
test/web/mastodon_api/controllers/suggestion_controller_test.exs
test/web/pleroma_api/controllers/pleroma_api_controller_test.exs

diff --git a/lib/pleroma/plugs/auth_expected_plug.ex b/lib/pleroma/plugs/auth_expected_plug.ex
new file mode 100644 (file)
index 0000000..9e4a4be
--- /dev/null
@@ -0,0 +1,13 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Plugs.AuthExpectedPlug do
+  import Plug.Conn
+
+  def init(options), do: options
+
+  def call(conn, _) do
+    put_private(conn, :auth_expected, true)
+  end
+end
index 38df074adfdd3cc607c476fa1b3f8ec238c9a4d9..b09e1bb4dd118686fc7aa908525ea098c7495ef8 100644 (file)
@@ -8,12 +8,15 @@ defmodule Pleroma.Plugs.OAuthScopesPlug do
 
   alias Pleroma.Config
   alias Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug
+  alias Pleroma.Plugs.PlugHelper
 
   @behaviour Plug
 
   def init(%{scopes: _} = options), do: options
 
   def call(%Plug.Conn{assigns: assigns} = conn, %{scopes: scopes} = options) do
+    conn = PlugHelper.append_to_called_plugs(conn, __MODULE__)
+
     op = options[:op] || :|
     token = assigns[:token]
 
diff --git a/lib/pleroma/plugs/plug_helper.ex b/lib/pleroma/plugs/plug_helper.ex
new file mode 100644 (file)
index 0000000..4f83e94
--- /dev/null
@@ -0,0 +1,38 @@
+# Pleroma: A lightweight social networking server
+# Copyright © 2017-2020 Pleroma Authors <https://pleroma.social/>
+# SPDX-License-Identifier: AGPL-3.0-only
+
+defmodule Pleroma.Plugs.PlugHelper do
+  @moduledoc "Pleroma Plug helper"
+
+  def append_to_called_plugs(conn, plug_module) do
+    append_to_private_list(conn, :called_plugs, plug_module)
+  end
+
+  def append_to_skipped_plugs(conn, plug_module) do
+    append_to_private_list(conn, :skipped_plugs, plug_module)
+  end
+
+  def plug_called?(conn, plug_module) do
+    contained_in_private_list?(conn, :called_plugs, plug_module)
+  end
+
+  def plug_skipped?(conn, plug_module) do
+    contained_in_private_list?(conn, :skipped_plugs, plug_module)
+  end
+
+  def plug_called_or_skipped?(conn, plug_module) do
+    plug_called?(conn, plug_module) || plug_skipped?(conn, plug_module)
+  end
+
+  defp append_to_private_list(conn, private_variable, value) do
+    list = conn.private[private_variable] || []
+    modified_list = Enum.uniq(list ++ [value])
+    Plug.Conn.put_private(conn, private_variable, modified_list)
+  end
+
+  defp contained_in_private_list?(conn, private_variable, value) do
+    list = conn.private[private_variable] || []
+    value in list
+  end
+end
index 43649ad2611dacc509c6f571c7a89cde9240f6e9..557cde328f4cdd47d646d1b2f21769d34a66e8a4 100644 (file)
@@ -17,7 +17,7 @@ defmodule Pleroma.Web.MastoFEController do
     when action == :index
   )
 
-  plug(Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug when action != :index)
+  plug(Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug when action not in [:index, :manifest])
 
   @doc "GET /web/*path"
   def index(%{assigns: %{user: user, token: token}} = conn, _params)
index 21bc3d5a549d5fde33a633c00f1147df3968deb4..bd6853d121dc18626b4a8543835823c824d0c687 100644 (file)
@@ -15,10 +15,13 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
   alias Pleroma.Web.CommonAPI
   alias Pleroma.Web.MastodonAPI.ListView
   alias Pleroma.Web.MastodonAPI.MastodonAPI
+  alias Pleroma.Web.MastodonAPI.MastodonAPIController
   alias Pleroma.Web.MastodonAPI.StatusView
   alias Pleroma.Web.OAuth.Token
   alias Pleroma.Web.TwitterAPI.TwitterAPI
 
+  plug(:skip_plug, OAuthScopesPlug when action == :identity_proofs)
+
   plug(
     OAuthScopesPlug,
     %{fallback: :proceed_unauthenticated, scopes: ["read:accounts"]}
@@ -369,6 +372,8 @@ defmodule Pleroma.Web.MastodonAPI.AccountController do
   end
 
   @doc "GET /api/v1/endorsements"
-  def endorsements(conn, params),
-    do: Pleroma.Web.MastodonAPI.MastodonAPIController.empty_array(conn, params)
+  def endorsements(conn, params), do: MastodonAPIController.empty_array(conn, params)
+
+  @doc "GET /api/v1/identity_proofs"
+  def identity_proofs(conn, params), do: MastodonAPIController.empty_array(conn, params)
 end
index 14075307de35b0f154a9ec1ea723823f3353e241..ac8c18f24272892d387406aa30d3d8f24cd87a3d 100644 (file)
@@ -3,21 +3,31 @@
 # SPDX-License-Identifier: AGPL-3.0-only
 
 defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do
+  @moduledoc """
+  Contains stubs for unimplemented Mastodon API endpoints.
+
+  Note: instead of routing directly to this controller's action,
+    it's preferable to define an action in relevant (non-generic) controller,
+    set up OAuth rules for it and call this controller's function from it.
+  """
+
   use Pleroma.Web, :controller
 
   require Logger
 
+  plug(:skip_plug, Pleroma.Plugs.OAuthScopesPlug when action in [:empty_array, :empty_object])
+
+  plug(Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug)
+
   action_fallback(Pleroma.Web.MastodonAPI.FallbackController)
 
-  # Stubs for unimplemented mastodon api
-  #
   def empty_array(conn, _) do
-    Logger.debug("Unimplemented, returning an empty array")
+    Logger.debug("Unimplemented, returning an empty array (list)")
     json(conn, [])
   end
 
   def empty_object(conn, _) do
-    Logger.debug("Unimplemented, returning an empty object")
+    Logger.debug("Unimplemented, returning an empty object (map)")
     json(conn, %{})
   end
 end
index 0cdc7bd8d9e1316b66ccbdfe3925be03dcc879ad..c93a439696223e619263c03a94260988f4fd36c6 100644 (file)
@@ -5,10 +5,13 @@
 defmodule Pleroma.Web.MastodonAPI.SuggestionController do
   use Pleroma.Web, :controller
 
+  alias Pleroma.Plugs.OAuthScopesPlug
+
   require Logger
 
+  plug(OAuthScopesPlug, %{scopes: ["read"]} when action == :index)
+
   @doc "GET /api/v1/suggestions"
-  def index(conn, _) do
-    json(conn, [])
-  end
+  def index(conn, params),
+    do: Pleroma.Web.MastodonAPI.MastodonAPIController.empty_array(conn, params)
 end
index 46688db7e8358cf85e4a78765812addffab8f76a..0121cd661001211b45f509f0bce2344f6f9f0800 100644 (file)
@@ -27,6 +27,8 @@ defmodule Pleroma.Web.OAuth.OAuthController do
   plug(:fetch_flash)
   plug(RateLimiter, [name: :authentication] when action == :create_authorization)
 
+  plug(:skip_plug, Pleroma.Plugs.OAuthScopesPlug)
+
   action_fallback(Pleroma.Web.OAuth.FallbackController)
 
   @oob_token_redirect_uri "urn:ietf:wg:oauth:2.0:oob"
index dae7f0f2f7aff92eab595b1f80e1bf332937f1b2..75f61b675a15db1eee3d3110190278aa45b83540 100644 (file)
@@ -34,7 +34,7 @@ defmodule Pleroma.Web.PleromaAPI.PleromaAPIController do
 
   plug(
     OAuthScopesPlug,
-    %{scopes: ["write:conversations"]} when action == :update_conversation
+    %{scopes: ["write:conversations"]} when action in [:update_conversation, :read_conversations]
   )
 
   plug(OAuthScopesPlug, %{scopes: ["write:notifications"]} when action == :read_notification)
index 5a09027391f6ab01b62553e192c8b1a1dbf627b8..3d57073d0f533f356ea703d7c00210abb4889ee2 100644 (file)
@@ -34,6 +34,7 @@ defmodule Pleroma.Web.Router do
   pipeline :authenticated_api do
     plug(:accepts, ["json"])
     plug(:fetch_session)
+    plug(Pleroma.Plugs.AuthExpectedPlug)
     plug(Pleroma.Plugs.OAuthPlug)
     plug(Pleroma.Plugs.BasicAuthDecoderPlug)
     plug(Pleroma.Plugs.UserFetcherPlug)
@@ -333,7 +334,7 @@ defmodule Pleroma.Web.Router do
     get("/accounts/relationships", AccountController, :relationships)
 
     get("/accounts/:id/lists", AccountController, :lists)
-    get("/accounts/:id/identity_proofs", MastodonAPIController, :empty_array)
+    get("/accounts/:id/identity_proofs", AccountController, :identity_proofs)
 
     get("/follow_requests", FollowRequestController, :index)
     get("/blocks", AccountController, :blocks)
index 0229aea975bba9bd2ca7adb23796bc00c4f57d2f..31adc28174bf1c7af9855c82f2907290d4f681c0 100644 (file)
@@ -15,6 +15,8 @@ defmodule Pleroma.Web.TwitterAPI.Controller do
 
   plug(OAuthScopesPlug, %{scopes: ["write:notifications"]} when action == :notifications_read)
 
+  plug(:skip_plug, OAuthScopesPlug when action in [:oauth_tokens, :revoke_token])
+
   plug(Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug)
 
   action_fallback(:errors)
index cf3ac1287b364bf9552cc970bf47da6eead76b7d..1af29ce788bcdadffb18871f372e1e76c256c252 100644 (file)
@@ -29,11 +29,34 @@ defmodule Pleroma.Web do
       import Pleroma.Web.Router.Helpers
       import Pleroma.Web.TranslationHelpers
 
+      alias Pleroma.Plugs.PlugHelper
+
       plug(:set_put_layout)
 
       defp set_put_layout(conn, _) do
         put_layout(conn, Pleroma.Config.get(:app_layout, "app.html"))
       end
+
+      # Marks a plug as intentionally skipped
+      #   (states that the plug is not called for a good reason, not by a mistake)
+      defp skip_plug(conn, plug_module) do
+        PlugHelper.append_to_skipped_plugs(conn, plug_module)
+      end
+
+      # Here we can apply before-action hooks (e.g. verify whether auth checks were preformed)
+      defp action(conn, params) do
+        if conn.private[:auth_expected] &&
+             not PlugHelper.plug_called_or_skipped?(conn, Pleroma.Plugs.OAuthScopesPlug) do
+          conn
+          |> render_error(
+            :forbidden,
+            "Security violation: OAuth scopes check was neither handled nor explicitly skipped."
+          )
+          |> halt()
+        else
+          super(conn, params)
+        end
+      end
     end
   end
 
index c697a39f8bf3b1fa6387204f286367b7e07e5831..8d0e70db86750cf99165a44c903afd25e6331b8c 100644 (file)
@@ -7,34 +7,8 @@ defmodule Pleroma.Web.MastodonAPI.SuggestionControllerTest do
 
   alias Pleroma.Config
 
-  import Pleroma.Factory
-  import Tesla.Mock
-
   setup do: oauth_access(["read"])
 
-  setup %{user: user} do
-    other_user = insert(:user)
-    host = Config.get([Pleroma.Web.Endpoint, :url, :host])
-    url500 = "http://test500?#{host}&#{user.nickname}"
-    url200 = "http://test200?#{host}&#{user.nickname}"
-
-    mock(fn
-      %{method: :get, url: ^url500} ->
-        %Tesla.Env{status: 500, body: "bad request"}
-
-      %{method: :get, url: ^url200} ->
-        %Tesla.Env{
-          status: 200,
-          body:
-            ~s([{"acct":"yj455","avatar":"https://social.heldscal.la/avatar/201.jpeg","avatar_static":"https://social.heldscal.la/avatar/s/201.jpeg"}, {"acct":"#{
-              other_user.ap_id
-            }","avatar":"https://social.heldscal.la/avatar/202.jpeg","avatar_static":"https://social.heldscal.la/avatar/s/202.jpeg"}])
-        }
-    end)
-
-    [other_user: other_user]
-  end
-
   test "returns empty result", %{conn: conn} do
     res =
       conn
index 32250f06f6332380f785ebe10f038f78c7fe1373..8f0cbe9b25e669378541597a442aca4ed1d0e927 100644 (file)
@@ -203,7 +203,7 @@ defmodule Pleroma.Web.PleromaAPI.PleromaAPIControllerTest do
 
   test "POST /api/v1/pleroma/conversations/read" do
     user = insert(:user)
-    %{user: other_user, conn: conn} = oauth_access(["write:notifications"])
+    %{user: other_user, conn: conn} = oauth_access(["write:conversations"])
 
     {:ok, _activity} =
       CommonAPI.post(user, %{"status" => "Hi @#{other_user.nickname}", "visibility" => "direct"})