[#1234] Defined admin OAuth scopes, refined other scopes. Added tests.

lib/pleroma/web/admin_api/admin_api_controller.ex

@@ -24,38 +24,20 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
 
   require Logger
 
-  plug(OAuthScopesPlug, %{scopes: ["read:statuses"]} when action == :list_user_statuses)
-
   plug(
     OAuthScopesPlug,
-    %{scopes: ["write:statuses"]} when action in [:status_update, :status_delete]
+    %{scopes: ["admin:read:accounts", "read:accounts"]}
+    when action in [:list_users, :user_show, :right_get, :invites]
   )
 
   plug(
     OAuthScopesPlug,
-    %{scopes: ["read"]}
+    %{scopes: ["admin:write", "write:accounts"]}
     when action in [
-           :list_reports,
-           :report_show,
-           :right_get,
            :get_invite_token,
-           :invites,
+           :revoke_invite,
+           :email_invite,
            :get_password_reset,
-           :list_users,
-           :user_show,
-           :config_show,
-           :migrate_to_db,
-           :migrate_from_db,
-           :list_log
-         ]
-  )
-
-  plug(
-    OAuthScopesPlug,
-    %{scopes: ["write"]}
-    when action in [
-           :report_update_state,
-           :report_respond,
            :user_follow,
            :user_unfollow,
            :user_delete,
@@ -65,15 +47,44 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
            :untag_users,
            :right_add,
            :right_delete,
-           :set_activation_status,
+           :set_activation_status
-           :relay_follow,
-           :relay_unfollow,
-           :revoke_invite,
-           :email_invite,
-           :config_update
          ]
   )
 
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["admin:read:reports", "read:reports"]} when action in [:list_reports, :report_show]
+  )
+
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["admin:write:reports", "write:reports"]}
+    when action in [:report_update_state, :report_respond]
+  )
+
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["admin:read:statuses", "read:statuses"]} when action == :list_user_statuses
+  )
+
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["admin:write:statuses", "write:statuses"]}
+    when action in [:status_update, :status_delete]
+  )
+
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["admin:read", "read"]}
+    when action in [:config_show, :migrate_to_db, :migrate_from_db, :list_log]
+  )
+
+  plug(
+    OAuthScopesPlug,
+    %{scopes: ["admin:write", "write"]}
+    when action in [:relay_follow, :relay_unfollow, :config_update]
+  )
+
   @users_page_size 50
 
   action_fallback(:errors)
@@ -451,7 +462,7 @@ def email_invite(%{assigns: %{user: user}} = conn, %{"email" => email} = params)
     end
   end
 
-  @doc "Get a account registeration invite token (base64 string)"
+  @doc "Get a account registration invite token (base64 string)"
   def get_invite_token(conn, params) do
     options = params["invite"] || %{}
     {:ok, invite} = UserInviteToken.create_invite(options)

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

@@ -53,13 +53,13 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do
   require Logger
   require Pleroma.Constants
 
-  plug(Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug when action != :index)
-
   @unauthenticated_access %{fallback: :proceed_unauthenticated, scopes: []}
 
+  # Note: :index action handles attempt of unauthenticated access to private instance with redirect
   plug(
     OAuthScopesPlug,
-    %{scopes: ["read"], skip_instance_privacy_check: true} when action == :index
+    Map.merge(@unauthenticated_access, %{scopes: ["read"], skip_instance_privacy_check: true})
+    when action == :index
   )
 
   plug(
@@ -220,6 +220,23 @@ defmodule Pleroma.Web.MastodonAPI.MastodonAPIController do
     %{scopes: ["write:bookmarks"]} when action in [:bookmark_status, :unbookmark_status]
   )
 
+  # An extra safety measure for possible actions not guarded by OAuth permissions specification
+  plug(
+    Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug
+    when action not in [
+           :account_register,
+           :create_app,
+           :index,
+           :login,
+           :logout,
+           :password_reset,
+           :account_confirmation_resend,
+           :masto_instance,
+           :peers,
+           :custom_emojis
+         ]
+  )
+
   @rate_limited_relations_actions ~w(follow unfollow)a
 
   @rate_limited_status_actions ~w(reblog_status unreblog_status fav_status unfav_status

test/plugs/oauth_scopes_plug_test.exs

@@ -6,11 +6,32 @@ defmodule Pleroma.Plugs.OAuthScopesPlugTest do
   use Pleroma.Web.ConnCase, async: true
 
   alias Pleroma.Plugs.OAuthScopesPlug
+  alias Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug
   alias Pleroma.Repo
 
+  import Mock
   import Pleroma.Factory
 
-  test "proceeds with no op if `assigns[:token]` is nil", %{conn: conn} do
+  setup_with_mocks([{EnsurePublicOrAuthenticatedPlug, [], [call: fn conn, _ -> conn end]}]) do
+    :ok
+  end
+
+  describe "when `assigns[:token]` is nil, " do
+    test "with :skip_instance_privacy_check option, proceeds with no op", %{conn: conn} do
+      conn =
+        conn
+        |> assign(:user, insert(:user))
+        |> OAuthScopesPlug.call(%{scopes: ["read"], skip_instance_privacy_check: true})
+
+      refute conn.halted
+      assert conn.assigns[:user]
+
+      refute called(EnsurePublicOrAuthenticatedPlug.call(conn, :_))
+    end
+
+    test "without :skip_instance_privacy_check option, calls EnsurePublicOrAuthenticatedPlug", %{
+      conn: conn
+    } do
       conn =
         conn
         |> assign(:user, insert(:user))
@@ -18,11 +39,14 @@ test "proceeds with no op if `assigns[:token]` is nil", %{conn: conn} do
 
       refute conn.halted
       assert conn.assigns[:user]
+
+      assert called(EnsurePublicOrAuthenticatedPlug.call(conn, :_))
+    end
   end
 
-  test "proceeds with no op if `token.scopes` fulfill specified 'any of' conditions", %{
+  test "if `token.scopes` fulfills specified 'any of' conditions, " <>
-    conn: conn
+         "proceeds with no op",
-  } do
+       %{conn: conn} do
     token = insert(:oauth_token, scopes: ["read", "write"]) |> Repo.preload(:user)
 
     conn =
@@ -35,9 +59,9 @@ test "proceeds with no op if `token.scopes` fulfill specified 'any of' condition
     assert conn.assigns[:user]
   end
 
-  test "proceeds with no op if `token.scopes` fulfill specified 'all of' conditions", %{
+  test "if `token.scopes` fulfills specified 'all of' conditions, " <>
-    conn: conn
+         "proceeds with no op",
-  } do
+       %{conn: conn} do
     token = insert(:oauth_token, scopes: ["scope1", "scope2", "scope3"]) |> Repo.preload(:user)
 
     conn =
@@ -50,8 +74,9 @@ test "proceeds with no op if `token.scopes` fulfill specified 'all of' condition
     assert conn.assigns[:user]
   end
 
-  test "proceeds with cleared `assigns[:user]` if `token.scopes` doesn't fulfill specified 'any of' conditions " <>
+  describe "with `fallback: :proceed_unauthenticated` option, " do
-         "and `fallback: :proceed_unauthenticated` option is specified",
+    test "if `token.scopes` doesn't fulfill specified 'any of' conditions, " <>
+           "clears `assigns[:user]` and calls EnsurePublicOrAuthenticatedPlug",
          %{conn: conn} do
       token = insert(:oauth_token, scopes: ["read", "write"]) |> Repo.preload(:user)
 
@@ -63,10 +88,12 @@ test "proceeds with cleared `assigns[:user]` if `token.scopes` doesn't fulfill s
 
       refute conn.halted
       refute conn.assigns[:user]
+
+      assert called(EnsurePublicOrAuthenticatedPlug.call(conn, :_))
     end
 
-  test "proceeds with cleared `assigns[:user]` if `token.scopes` doesn't fulfill specified 'all of' conditions " <>
+    test "if `token.scopes` doesn't fulfill specified 'all of' conditions, " <>
-         "and `fallback: :proceed_unauthenticated` option is specified",
+           "clears `assigns[:user] and calls EnsurePublicOrAuthenticatedPlug",
          %{conn: conn} do
       token = insert(:oauth_token, scopes: ["read", "write"]) |> Repo.preload(:user)
 
@@ -82,10 +109,36 @@ test "proceeds with cleared `assigns[:user]` if `token.scopes` doesn't fulfill s
 
       refute conn.halted
       refute conn.assigns[:user]
+
+      assert called(EnsurePublicOrAuthenticatedPlug.call(conn, :_))
     end
 
-  test "returns 403 and halts " <>
+    test "with :skip_instance_privacy_check option, " <>
-         "in case of no :fallback option and `token.scopes` not fulfilling specified 'any of' conditions",
+           "if `token.scopes` doesn't fulfill specified conditions, " <>
+           "clears `assigns[:user]` and does not call EnsurePublicOrAuthenticatedPlug",
+         %{conn: conn} do
+      token = insert(:oauth_token, scopes: ["read:statuses", "write"]) |> Repo.preload(:user)
+
+      conn =
+        conn
+        |> assign(:user, token.user)
+        |> assign(:token, token)
+        |> OAuthScopesPlug.call(%{
+          scopes: ["read"],
+          fallback: :proceed_unauthenticated,
+          skip_instance_privacy_check: true
+        })
+
+      refute conn.halted
+      refute conn.assigns[:user]
+
+      refute called(EnsurePublicOrAuthenticatedPlug.call(conn, :_))
+    end
+  end
+
+  describe "without :fallback option, " do
+    test "if `token.scopes` does not fulfill specified 'any of' conditions, " <>
+           "returns 403 and halts",
          %{conn: conn} do
       token = insert(:oauth_token, scopes: ["read", "write"])
       any_of_scopes = ["follow"]
@@ -102,8 +155,8 @@ test "returns 403 and halts " <>
       assert Jason.encode!(%{error: expected_error}) == conn.resp_body
     end
 
-  test "returns 403 and halts " <>
+    test "if `token.scopes` does not fulfill specified 'all of' conditions, " <>
-         "in case of no :fallback option and `token.scopes` not fulfilling specified 'all of' conditions",
+           "returns 403 and halts",
          %{conn: conn} do
       token = insert(:oauth_token, scopes: ["read", "write"])
       all_of_scopes = ["write", "follow"]
@@ -121,11 +174,12 @@ test "returns 403 and halts " <>
 
       assert Jason.encode!(%{error: expected_error}) == conn.resp_body
     end
+  end
 
   describe "with hierarchical scopes, " do
-    test "proceeds with no op if `token.scopes` fulfill specified 'any of' conditions", %{
+    test "if `token.scopes` fulfills specified 'any of' conditions, " <>
-      conn: conn
+           "proceeds with no op",
-    } do
+         %{conn: conn} do
       token = insert(:oauth_token, scopes: ["read", "write"]) |> Repo.preload(:user)
 
       conn =
@@ -138,9 +192,9 @@ test "proceeds with no op if `token.scopes` fulfill specified 'any of' condition
       assert conn.assigns[:user]
     end
 
-    test "proceeds with no op if `token.scopes` fulfill specified 'all of' conditions", %{
+    test "if `token.scopes` fulfills specified 'all of' conditions, " <>
-      conn: conn
+           "proceeds with no op",
-    } do
+         %{conn: conn} do
       token = insert(:oauth_token, scopes: ["scope1", "scope2", "scope3"]) |> Repo.preload(:user)
 
       conn =
@@ -153,4 +207,21 @@ test "proceeds with no op if `token.scopes` fulfill specified 'all of' condition
       assert conn.assigns[:user]
     end
   end
+
+  describe "filter_descendants/2" do
+    test "filters scopes which directly match or are ancestors of supported scopes" do
+      f = fn scopes, supported_scopes ->
+        OAuthScopesPlug.filter_descendants(scopes, supported_scopes)
+      end
+
+      assert f.(["read", "follow"], ["write", "read"]) == ["read"]
+
+      assert f.(["read", "write:something", "follow"], ["write", "read"]) ==
+               ["read", "write:something"]
+
+      assert f.(["admin:read"], ["write", "read"]) == []
+
+      assert f.(["admin:read"], ["write", "admin"]) == ["admin:read"]
+    end
+  end
 end