[#1427] Fixes / improvements of admin scopes support. Added tests.

lib/pleroma/plugs/oauth_scopes_plug.ex

@@ -6,6 +6,7 @@ defmodule Pleroma.Plugs.OAuthScopesPlug do
   import Plug.Conn
   import Pleroma.Web.Gettext
 
+  alias Pleroma.Config
   alias Pleroma.Plugs.EnsurePublicOrAuthenticatedPlug
 
   @behaviour Plug
@@ -15,6 +16,14 @@ def init(%{scopes: _} = options), do: options
   def call(%Plug.Conn{assigns: assigns} = conn, %{scopes: scopes} = options) do
     op = options[:op] || :|
     token = assigns[:token]
+
+    scopes =
+      if options[:admin] do
+        Config.oauth_admin_scopes(scopes)
+      else
+        scopes
+      end
+
     matched_scopes = token && filter_descendants(scopes, token.scopes)
 
     cond do

lib/pleroma/plugs/user_is_admin_plug.ex

@@ -12,31 +12,23 @@ def init(options) do
     options
   end
 
-  unless Pleroma.Config.enforce_oauth_admin_scope_usage?() do
-    # To do: once AdminFE makes use of "admin" scope, disable the following func definition
-    #   (fail on no admin scope(s) in token even if `is_admin` is true)
-    def call(%Plug.Conn{assigns: %{user: %Pleroma.User{is_admin: true}}} = conn, _) do
-      conn
+  def call(%Plug.Conn{assigns: assigns} = conn, _) do
+    token = assigns[:token]
+    user = assigns[:user]
+
+    cond do
+      token && OAuth.Scopes.contains_admin_scopes?(token.scopes) ->
+        # Note: checking for _any_ admin scope presence, not necessarily fitting requested action.
+        #   Thus, controller must explicitly invoke OAuthScopesPlug to verify scope requirements.
+        conn
+
+      user && user.is_admin && !Pleroma.Config.enforce_oauth_admin_scope_usage?() ->
+        conn
+
+      true ->
+        conn
+        |> render_error(:forbidden, "User is not an admin or OAuth admin scope is not granted.")
+        |> halt()
     end
   end
-
-  def call(%Plug.Conn{assigns: %{token: %OAuth.Token{scopes: oauth_scopes} = _token}} = conn, _) do
-    if OAuth.Scopes.contains_admin_scopes?(oauth_scopes) do
-      # Note: checking for _any_ admin scope presence, not necessarily fitting requested action.
-      #   Thus, controller must explicitly invoke OAuthScopesPlug to verify scope requirements.
-      conn
-    else
-      fail(conn)
-    end
-  end
-
-  def call(conn, _) do
-    fail(conn)
-  end
-
-  defp fail(conn) do
-    conn
-    |> render_error(:forbidden, "User is not an admin or OAuth admin scope is not granted.")
-    |> halt()
-  end
 end

lib/pleroma/web/admin_api/admin_api_controller.ex

@@ -30,13 +30,13 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
 
   plug(
     OAuthScopesPlug,
-    %{scopes: Pleroma.Config.oauth_admin_scopes("read:accounts")}
+    %{scopes: ["read:accounts"], admin: true}
     when action in [:list_users, :user_show, :right_get, :invites]
   )
 
   plug(
     OAuthScopesPlug,
-    %{scopes: Pleroma.Config.oauth_admin_scopes("write:accounts")}
+    %{scopes: ["write:accounts"], admin: true}
     when action in [
            :get_invite_token,
            :revoke_invite,
@@ -58,37 +58,37 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIController do
 
   plug(
     OAuthScopesPlug,
-    %{scopes: Pleroma.Config.oauth_admin_scopes("read:reports")}
+    %{scopes: ["read:reports"], admin: true}
     when action in [:list_reports, :report_show]
   )
 
   plug(
     OAuthScopesPlug,
-    %{scopes: Pleroma.Config.oauth_admin_scopes("write:reports")}
+    %{scopes: ["write:reports"], admin: true}
     when action in [:report_update_state, :report_respond]
   )
 
   plug(
     OAuthScopesPlug,
-    %{scopes: Pleroma.Config.oauth_admin_scopes("read:statuses")}
+    %{scopes: ["read:statuses"], admin: true}
     when action == :list_user_statuses
   )
 
   plug(
     OAuthScopesPlug,
-    %{scopes: Pleroma.Config.oauth_admin_scopes("write:statuses")}
+    %{scopes: ["write:statuses"], admin: true}
     when action in [:status_update, :status_delete]
   )
 
   plug(
     OAuthScopesPlug,
-    %{scopes: Pleroma.Config.oauth_admin_scopes("read")}
+    %{scopes: ["read"], admin: true}
     when action in [:config_show, :migrate_to_db, :migrate_from_db, :list_log]
   )
 
   plug(
     OAuthScopesPlug,
-    %{scopes: Pleroma.Config.oauth_admin_scopes("write")}
+    %{scopes: ["write"], admin: true}
     when action in [:relay_follow, :relay_unfollow, :config_update]
   )
 

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

@@ -7,7 +7,7 @@ defmodule Pleroma.Web.PleromaAPI.EmojiAPIController do
 
   plug(
     OAuthScopesPlug,
-    %{scopes: Pleroma.Config.oauth_admin_scopes("write")}
+    %{scopes: ["write"], admin: true}
     when action in [
            :create,
            :delete,

test/plugs/user_is_admin_plug_test.exs

@@ -8,36 +8,96 @@ defmodule Pleroma.Plugs.UserIsAdminPlugTest do
   alias Pleroma.Plugs.UserIsAdminPlug
   import Pleroma.Factory
 
-  test "accepts a user that is admin" do
-    user = insert(:user, is_admin: true)
+  describe "unless [:auth, :enforce_oauth_admin_scope_usage]," do
+    clear_config([:auth, :enforce_oauth_admin_scope_usage]) do
+      Pleroma.Config.put([:auth, :enforce_oauth_admin_scope_usage], false)
+    end
 
-    conn =
-      build_conn()
-      |> assign(:user, user)
+    test "accepts a user that is admin" do
+      user = insert(:user, is_admin: true)
 
-    ret_conn =
-      conn
-      |> UserIsAdminPlug.call(%{})
+      conn = assign(build_conn(), :user, user)
 
-    assert conn == ret_conn
+      ret_conn = UserIsAdminPlug.call(conn, %{})
+
+      assert conn == ret_conn
+    end
+
+    test "denies a user that isn't admin" do
+      user = insert(:user)
+
+      conn =
+        build_conn()
+        |> assign(:user, user)
+        |> UserIsAdminPlug.call(%{})
+
+      assert conn.status == 403
+    end
+
+    test "denies when a user isn't set" do
+      conn = UserIsAdminPlug.call(build_conn(), %{})
+
+      assert conn.status == 403
+    end
   end
 
-  test "denies a user that isn't admin" do
-    user = insert(:user)
+  describe "with [:auth, :enforce_oauth_admin_scope_usage]," do
+    clear_config([:auth, :enforce_oauth_admin_scope_usage]) do
+      Pleroma.Config.put([:auth, :enforce_oauth_admin_scope_usage], true)
+    end
 
-    conn =
-      build_conn()
-      |> assign(:user, user)
-      |> UserIsAdminPlug.call(%{})
+    setup do
+      admin_user = insert(:user, is_admin: true)
+      non_admin_user = insert(:user, is_admin: false)
+      blank_user = nil
 
-    assert conn.status == 403
-  end
+      {:ok, %{users: [admin_user, non_admin_user, blank_user]}}
+    end
 
-  test "denies when a user isn't set" do
-    conn =
-      build_conn()
-      |> UserIsAdminPlug.call(%{})
+    # Note: in real-life scenarios only users with is_admin flag can possess admin-scoped tokens;
+    #   however, the following test stresses out that is_admin flag is not checked if we got token
+    test "if token has any of admin scopes, accepts users regardless of is_admin flag",
+         %{users: users} do
+      for user <- users do
+        token = insert(:oauth_token, user: user, scopes: ["admin:something"])
 
-    assert conn.status == 403
+        conn =
+          build_conn()
+          |> assign(:user, user)
+          |> assign(:token, token)
+          |> UserIsAdminPlug.call(%{})
+
+        ret_conn = UserIsAdminPlug.call(conn, %{})
+
+        assert conn == ret_conn
+      end
+    end
+
+    test "if token lacks admin scopes, denies users regardless of is_admin flag",
+         %{users: users} do
+      for user <- users do
+        token = insert(:oauth_token, user: user)
+
+        conn =
+          build_conn()
+          |> assign(:user, user)
+          |> assign(:token, token)
+          |> UserIsAdminPlug.call(%{})
+
+        assert conn.status == 403
+      end
+    end
+
+    test "if token is missing, denies users regardless of is_admin flag", %{users: users} do
+      for user <- users do
+        conn =
+          build_conn()
+          |> assign(:user, user)
+          |> assign(:token, nil)
+          |> UserIsAdminPlug.call(%{})
+
+        assert conn.status == 403
+      end
+    end
   end
 end

test/web/admin_api/admin_api_controller_test.exs

@@ -24,6 +24,49 @@ defmodule Pleroma.Web.AdminAPI.AdminAPIControllerTest do
     :ok
   end
 
+  clear_config([:auth, :enforce_oauth_admin_scope_usage]) do
+    Pleroma.Config.put([:auth, :enforce_oauth_admin_scope_usage], false)
+  end
+
+  describe "with [:auth, :enforce_oauth_admin_scope_usage]," do
+    clear_config([:auth, :enforce_oauth_admin_scope_usage]) do
+      Pleroma.Config.put([:auth, :enforce_oauth_admin_scope_usage], true)
+    end
+
+    test "GET /api/pleroma/admin/users/:nickname requires admin:read:accounts or broader scope" do
+      user = insert(:user)
+      admin = insert(:user, is_admin: true)
+
+      good_token1 = insert(:oauth_token, user: admin, scopes: ["admin"])
+      good_token2 = insert(:oauth_token, user: admin, scopes: ["admin:read"])
+      good_token3 = insert(:oauth_token, user: admin, scopes: ["admin:read:accounts"])
+
+      bad_token1 = insert(:oauth_token, user: admin, scopes: ["read:accounts"])
+      bad_token2 = insert(:oauth_token, user: admin, scopes: ["admin:read:accounts:partial"])
+      bad_token3 = nil
+
+      for good_token <- [good_token1, good_token2, good_token3] do
+        conn =
+          build_conn()
+          |> assign(:user, admin)
+          |> assign(:token, good_token)
+          |> get("/api/pleroma/admin/users/#{user.nickname}")
+
+        assert json_response(conn, 200)
+      end
+
+      for bad_token <- [bad_token1, bad_token2, bad_token3] do
+        conn =
+          build_conn()
+          |> assign(:user, admin)
+          |> assign(:token, bad_token)
+          |> get("/api/pleroma/admin/users/#{user.nickname}")
+
+        assert json_response(conn, :forbidden)
+      end
+    end
+  end
+
   describe "DELETE /api/pleroma/admin/users" do
     test "single user" do
       admin = insert(:user, is_admin: true)
@@ -97,7 +140,7 @@ test "Create" do
       assert ["lain", "lain2"] -- Enum.map(log_entry.data["subjects"], & &1["nickname"]) == []
     end
 
-    test "Cannot create user with exisiting email" do
+    test "Cannot create user with existing email" do
       admin = insert(:user, is_admin: true)
       user = insert(:user)
 
@@ -128,7 +171,7 @@ test "Cannot create user with exisiting email" do
              ]
     end
 
-    test "Cannot create user with exisiting nickname" do
+    test "Cannot create user with existing nickname" do
       admin = insert(:user, is_admin: true)
       user = insert(:user)