From: Ivan Tashkinov <ivantashkinov@gmail.com>
Date: Tue, 17 Sep 2019 19:19:39 +0000 (+0300)
Subject: [#1234] Defined admin OAuth scopes, refined other scopes. Added tests.
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=76068873dbf9da191dd2487158ca88df198b811a;p=akkoma

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

diff --git a/lib/pleroma/web/admin_api/admin_api_controller.ex b/lib/pleroma/web/admin_api/admin_api_controller.ex
index 0a508d40e..fa69a23d9 100644
--- a/lib/pleroma/web/admin_api/admin_api_controller.ex
+++ b/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,
-           :relay_follow,
-           :relay_unfollow,
-           :revoke_invite,
-           :email_invite,
-           :config_update
+           :set_activation_status
          ]
   )
 
+  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 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
     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)
diff --git a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex b/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
index c5632bb5e..d7a83a2f5 100644
--- a/lib/pleroma/web/mastodon_api/controllers/mastodon_api_controller.ex
+++ b/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
diff --git a/test/plugs/oauth_scopes_plug_test.exs b/test/plugs/oauth_scopes_plug_test.exs
index 9b0a2e702..3b895a6e4 100644
--- a/test/plugs/oauth_scopes_plug_test.exs
+++ b/test/plugs/oauth_scopes_plug_test.exs
@@ -6,23 +6,47 @@ 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
-    conn =
-      conn
-      |> assign(:user, insert(:user))
-      |> OAuthScopesPlug.call(%{scopes: ["read"]})
+  setup_with_mocks([{EnsurePublicOrAuthenticatedPlug, [], [call: fn conn, _ -> conn end]}]) do
+    :ok
+  end
 
-    refute conn.halted
-    assert conn.assigns[:user]
+  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))
+        |> OAuthScopesPlug.call(%{scopes: ["read"]})
+
+      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", %{
-    conn: conn
-  } do
+  test "if `token.scopes` fulfills specified 'any of' conditions, " <>
+         "proceeds with no op",
+       %{conn: conn} do
     token = insert(:oauth_token, scopes: ["read", "write"]) |> Repo.preload(:user)
 
     conn =
@@ -35,9 +59,9 @@ defmodule Pleroma.Plugs.OAuthScopesPlugTest do
     assert conn.assigns[:user]
   end
 
-  test "proceeds with no op if `token.scopes` fulfill specified 'all of' conditions", %{
-    conn: conn
-  } do
+  test "if `token.scopes` fulfills specified 'all of' conditions, " <>
+         "proceeds with no op",
+       %{conn: conn} do
     token = insert(:oauth_token, scopes: ["scope1", "scope2", "scope3"]) |> Repo.preload(:user)
 
     conn =
@@ -50,82 +74,112 @@ defmodule Pleroma.Plugs.OAuthScopesPlugTest do
     assert conn.assigns[:user]
   end
 
-  test "proceeds with cleared `assigns[:user]` if `token.scopes` doesn't fulfill specified 'any of' conditions " <>
-         "and `fallback: :proceed_unauthenticated` option is specified",
-       %{conn: conn} do
-    token = insert(:oauth_token, scopes: ["read", "write"]) |> Repo.preload(:user)
+  describe "with `fallback: :proceed_unauthenticated` option, " do
+    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)
 
-    conn =
-      conn
-      |> assign(:user, token.user)
-      |> assign(:token, token)
-      |> OAuthScopesPlug.call(%{scopes: ["follow"], fallback: :proceed_unauthenticated})
+      conn =
+        conn
+        |> assign(:user, token.user)
+        |> assign(:token, token)
+        |> OAuthScopesPlug.call(%{scopes: ["follow"], fallback: :proceed_unauthenticated})
 
-    refute conn.halted
-    refute conn.assigns[:user]
-  end
+      refute conn.halted
+      refute conn.assigns[:user]
 
-  test "proceeds with cleared `assigns[:user]` if `token.scopes` doesn't fulfill specified 'all of' conditions " <>
-         "and `fallback: :proceed_unauthenticated` option is specified",
-       %{conn: conn} do
-    token = insert(:oauth_token, scopes: ["read", "write"]) |> Repo.preload(:user)
+      assert called(EnsurePublicOrAuthenticatedPlug.call(conn, :_))
+    end
 
-    conn =
-      conn
-      |> assign(:user, token.user)
-      |> assign(:token, token)
-      |> OAuthScopesPlug.call(%{
-        scopes: ["read", "follow"],
-        op: :&,
-        fallback: :proceed_unauthenticated
-      })
+    test "if `token.scopes` doesn't fulfill specified 'all of' conditions, " <>
+           "clears `assigns[:user] and calls EnsurePublicOrAuthenticatedPlug",
+         %{conn: conn} do
+      token = insert(:oauth_token, scopes: ["read", "write"]) |> Repo.preload(:user)
 
-    refute conn.halted
-    refute conn.assigns[:user]
-  end
+      conn =
+        conn
+        |> assign(:user, token.user)
+        |> assign(:token, token)
+        |> OAuthScopesPlug.call(%{
+          scopes: ["read", "follow"],
+          op: :&,
+          fallback: :proceed_unauthenticated
+        })
 
-  test "returns 403 and halts " <>
-         "in case of no :fallback option and `token.scopes` not fulfilling specified 'any of' conditions",
-       %{conn: conn} do
-    token = insert(:oauth_token, scopes: ["read", "write"])
-    any_of_scopes = ["follow"]
+      refute conn.halted
+      refute conn.assigns[:user]
 
-    conn =
-      conn
-      |> assign(:token, token)
-      |> OAuthScopesPlug.call(%{scopes: any_of_scopes})
+      assert called(EnsurePublicOrAuthenticatedPlug.call(conn, :_))
+    end
 
-    assert conn.halted
-    assert 403 == conn.status
+    test "with :skip_instance_privacy_check option, " <>
+           "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)
 
-    expected_error = "Insufficient permissions: #{Enum.join(any_of_scopes, ", ")}."
-    assert Jason.encode!(%{error: expected_error}) == conn.resp_body
+      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
 
-  test "returns 403 and halts " <>
-         "in case of no :fallback option and `token.scopes` not fulfilling specified 'all of' conditions",
-       %{conn: conn} do
-    token = insert(:oauth_token, scopes: ["read", "write"])
-    all_of_scopes = ["write", "follow"]
+  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"]
 
-    conn =
-      conn
-      |> assign(:token, token)
-      |> OAuthScopesPlug.call(%{scopes: all_of_scopes, op: :&})
+      conn =
+        conn
+        |> assign(:token, token)
+        |> OAuthScopesPlug.call(%{scopes: any_of_scopes})
+
+      assert conn.halted
+      assert 403 == conn.status
+
+      expected_error = "Insufficient permissions: #{Enum.join(any_of_scopes, ", ")}."
+      assert Jason.encode!(%{error: expected_error}) == conn.resp_body
+    end
 
-    assert conn.halted
-    assert 403 == conn.status
+    test "if `token.scopes` does not fulfill specified 'all of' conditions, " <>
+           "returns 403 and halts",
+         %{conn: conn} do
+      token = insert(:oauth_token, scopes: ["read", "write"])
+      all_of_scopes = ["write", "follow"]
 
-    expected_error =
-      "Insufficient permissions: #{Enum.join(all_of_scopes -- token.scopes, ", ")}."
+      conn =
+        conn
+        |> assign(:token, token)
+        |> OAuthScopesPlug.call(%{scopes: all_of_scopes, op: :&})
 
-    assert Jason.encode!(%{error: expected_error}) == conn.resp_body
+      assert conn.halted
+      assert 403 == conn.status
+
+      expected_error =
+        "Insufficient permissions: #{Enum.join(all_of_scopes -- token.scopes, ", ")}."
+
+      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", %{
-      conn: conn
-    } do
+    test "if `token.scopes` fulfills specified 'any of' conditions, " <>
+           "proceeds with no op",
+         %{conn: conn} do
       token = insert(:oauth_token, scopes: ["read", "write"]) |> Repo.preload(:user)
 
       conn =
@@ -138,9 +192,9 @@ defmodule Pleroma.Plugs.OAuthScopesPlugTest do
       assert conn.assigns[:user]
     end
 
-    test "proceeds with no op if `token.scopes` fulfill specified 'all of' conditions", %{
-      conn: conn
-    } do
+    test "if `token.scopes` fulfills specified 'all of' conditions, " <>
+           "proceeds with no op",
+         %{conn: conn} do
       token = insert(:oauth_token, scopes: ["scope1", "scope2", "scope3"]) |> Repo.preload(:user)
 
       conn =
@@ -153,4 +207,21 @@ defmodule Pleroma.Plugs.OAuthScopesPlugTest do
       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