From: FloatingGhost <hannah@coffee-and-dreams.uk>
Date: Fri, 16 Dec 2022 03:25:14 +0000 (+0000)
Subject: do not allow non-admins to register tokens with admin scopes
X-Git-Url: https://git.squeep.com/?a=commitdiff_plain;h=b8be8192fbfe8c27b457094e8d64ccb1c3a29f7f;p=akkoma

do not allow non-admins to register tokens with admin scopes

this didn't actually _do_ anything in the past,
the users would be prevented from accessing the resource,
but they shouldn't be able to even create them
---

diff --git a/lib/pleroma/web/o_auth/o_auth_controller.ex b/lib/pleroma/web/o_auth/o_auth_controller.ex
index 8f32e7219..3943ca449 100644
--- a/lib/pleroma/web/o_auth/o_auth_controller.ex
+++ b/lib/pleroma/web/o_auth/o_auth_controller.ex
@@ -211,11 +211,11 @@ defmodule Pleroma.Web.OAuth.OAuthController do
          {:error, scopes_issue},
          %{"authorization" => _} = params
        )
-       when scopes_issue in [:unsupported_scopes, :missing_scopes] do
+       when scopes_issue in [:unsupported_scopes, :missing_scopes, :user_is_not_an_admin] do
     # Per https://github.com/tootsuite/mastodon/blob/
     #   51e154f5e87968d6bb115e053689767ab33e80cd/app/controllers/api/base_controller.rb#L39
     conn
-    |> put_flash(:error, dgettext("errors", "This action is outside the authorized scopes"))
+    |> put_flash(:error, dgettext("errors", "This action is outside of authorized scopes"))
     |> put_status(:unauthorized)
     |> authorize(params)
   end
@@ -605,7 +605,7 @@ defmodule Pleroma.Web.OAuth.OAuthController do
   defp do_create_authorization(%User{} = user, %App{} = app, requested_scopes)
        when is_list(requested_scopes) do
     with {:account_status, :active} <- {:account_status, User.account_status(user)},
-         {:ok, scopes} <- validate_scopes(app, requested_scopes),
+         {:ok, scopes} <- validate_scopes(user, app, requested_scopes),
          {:ok, auth} <- Authorization.create_authorization(app, user, scopes) do
       {:ok, auth}
     end
@@ -637,15 +637,16 @@ defmodule Pleroma.Web.OAuth.OAuthController do
     end
   end
 
-  @spec validate_scopes(App.t(), map() | list()) ::
+  @spec validate_scopes(User.t(), App.t(), map() | list()) ::
           {:ok, list()} | {:error, :missing_scopes | :unsupported_scopes}
-  defp validate_scopes(%App{} = app, params) when is_map(params) do
+  defp validate_scopes(%User{} = user, %App{} = app, params) when is_map(params) do
     requested_scopes = Scopes.fetch_scopes(params, app.scopes)
-    validate_scopes(app, requested_scopes)
+    validate_scopes(user, app, requested_scopes)
   end
 
-  defp validate_scopes(%App{} = app, requested_scopes) when is_list(requested_scopes) do
-    Scopes.validate(requested_scopes, app.scopes)
+  defp validate_scopes(%User{} = user, %App{} = app, requested_scopes)
+       when is_list(requested_scopes) do
+    Scopes.validate(requested_scopes, app.scopes, user)
   end
 
   def default_redirect_uri(%App{} = app) do
diff --git a/lib/pleroma/web/o_auth/scopes.ex b/lib/pleroma/web/o_auth/scopes.ex
index ada43eae9..7fe04b912 100644
--- a/lib/pleroma/web/o_auth/scopes.ex
+++ b/lib/pleroma/web/o_auth/scopes.ex
@@ -56,12 +56,20 @@ defmodule Pleroma.Web.OAuth.Scopes do
   @doc """
   Validates scopes.
   """
-  @spec validate(list() | nil, list()) ::
-          {:ok, list()} | {:error, :missing_scopes | :unsupported_scopes}
-  def validate(blank_scopes, _app_scopes) when blank_scopes in [nil, []],
+  @spec validate(list() | nil, list(), Pleroma.User.t()) ::
+          {:ok, list()} | {:error, :missing_scopes | :unsupported_scopes, :user_is_not_an_admin}
+  def validate(blank_scopes, _app_scopes, _user) when blank_scopes in [nil, []],
     do: {:error, :missing_scopes}
 
-  def validate(scopes, app_scopes) do
+  def validate(scopes, app_scopes, %Pleroma.User{is_admin: is_admin}) do
+    if !is_admin && contains_admin_scopes?(scopes) do
+      {:error, :user_is_not_an_admin}
+    else
+      validate_scopes_are_supported(scopes, app_scopes)
+    end
+  end
+
+  defp validate_scopes_are_supported(scopes, app_scopes) do
     case OAuthScopesPlug.filter_descendants(scopes, app_scopes) do
       ^scopes -> {:ok, scopes}
       _ -> {:error, :unsupported_scopes}
diff --git a/test/pleroma/web/o_auth/o_auth_controller_test.exs b/test/pleroma/web/o_auth/o_auth_controller_test.exs
index 5a1258ec3..19a7dea60 100644
--- a/test/pleroma/web/o_auth/o_auth_controller_test.exs
+++ b/test/pleroma/web/o_auth/o_auth_controller_test.exs
@@ -693,45 +693,76 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
 
   describe "POST /oauth/authorize" do
     test "redirects with oauth authorization, " <>
-           "granting requested app-supported scopes to both admin- and non-admin users" do
+           "granting requested app-supported scopes to both admin users" do
       app_scopes = ["read", "write", "admin", "secret_scope"]
       app = insert(:oauth_app, scopes: app_scopes)
       redirect_uri = OAuthController.default_redirect_uri(app)
+      scopes_subset = ["read:subscope", "write", "admin"]
+      admin = insert(:user, is_admin: true)
+
+      # In case scope param is missing, expecting _all_ app-supported scopes to be granted
+      conn =
+        post(
+          build_conn(),
+          "/oauth/authorize",
+          %{
+            "authorization" => %{
+              "name" => admin.nickname,
+              "password" => "test",
+              "client_id" => app.client_id,
+              "redirect_uri" => redirect_uri,
+              "scope" => scopes_subset,
+              "state" => "statepassed"
+            }
+          }
+        )
+
+      target = redirected_to(conn)
+      assert target =~ redirect_uri
+
+      query = URI.parse(target).query |> URI.query_decoder() |> Map.new()
+
+      assert %{"state" => "statepassed", "code" => code} = query
+      auth = Repo.get_by(Authorization, token: code)
+      assert auth
+      assert auth.scopes == scopes_subset
+    end
+
+    test "redirects with oauth authorization, " <>
+           "granting requested app-supported scopes for non-admin users" do
+      app_scopes = ["read", "write", "secret_scope", "admin"]
+      app = insert(:oauth_app, scopes: app_scopes)
+      redirect_uri = OAuthController.default_redirect_uri(app)
 
       non_admin = insert(:user, is_admin: false)
-      admin = insert(:user, is_admin: true)
-      scopes_subset = ["read:subscope", "write", "admin"]
+      scopes_subset = ["read:subscope", "write"]
 
       # In case scope param is missing, expecting _all_ app-supported scopes to be granted
-      for user <- [non_admin, admin],
-          {requested_scopes, expected_scopes} <-
-            %{scopes_subset => scopes_subset, nil: app_scopes} do
-        conn =
-          post(
-            build_conn(),
-            "/oauth/authorize",
-            %{
-              "authorization" => %{
-                "name" => user.nickname,
-                "password" => "test",
-                "client_id" => app.client_id,
-                "redirect_uri" => redirect_uri,
-                "scope" => requested_scopes,
-                "state" => "statepassed"
-              }
+      conn =
+        post(
+          build_conn(),
+          "/oauth/authorize",
+          %{
+            "authorization" => %{
+              "name" => non_admin.nickname,
+              "password" => "test",
+              "client_id" => app.client_id,
+              "redirect_uri" => redirect_uri,
+              "scope" => scopes_subset,
+              "state" => "statepassed"
             }
-          )
+          }
+        )
 
-        target = redirected_to(conn)
-        assert target =~ redirect_uri
+      target = redirected_to(conn)
+      assert target =~ redirect_uri
 
-        query = URI.parse(target).query |> URI.query_decoder() |> Map.new()
+      query = URI.parse(target).query |> URI.query_decoder() |> Map.new()
 
-        assert %{"state" => "statepassed", "code" => code} = query
-        auth = Repo.get_by(Authorization, token: code)
-        assert auth
-        assert auth.scopes == expected_scopes
-      end
+      assert %{"state" => "statepassed", "code" => code} = query
+      auth = Repo.get_by(Authorization, token: code)
+      assert auth
+      assert auth.scopes == scopes_subset
     end
 
     test "authorize from cookie" do
@@ -739,6 +770,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
       app = insert(:oauth_app)
       oauth_token = insert(:oauth_token, user: user, app: app)
       redirect_uri = OAuthController.default_redirect_uri(app)
+      IO.inspect(app)
 
       conn =
         build_conn()
@@ -831,6 +863,33 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
       assert result =~ "Invalid Username/Password"
     end
 
+    test "returns 401 when attempting to use an admin scope with a non-admin", %{conn: conn} do
+      user = insert(:user)
+      app = insert(:oauth_app, scopes: ["admin"])
+      redirect_uri = OAuthController.default_redirect_uri(app)
+
+      result =
+        conn
+        |> post("/oauth/authorize", %{
+          "authorization" => %{
+            "name" => user.nickname,
+            "password" => "test",
+            "client_id" => app.client_id,
+            "redirect_uri" => redirect_uri,
+            "state" => "statepassed",
+            "scope" => Enum.join(app.scopes, " ")
+          }
+        })
+        |> html_response(:unauthorized)
+
+      # Keep the details
+      assert result =~ app.client_id
+      assert result =~ redirect_uri
+
+      # Error message
+      assert result =~ "outside of authorized scopes"
+    end
+
     test "returns 401 for missing scopes" do
       user = insert(:user, is_admin: false)
       app = insert(:oauth_app, scopes: ["read", "write", "admin"])
@@ -855,7 +914,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
       assert result =~ redirect_uri
 
       # Error message
-      assert result =~ "This action is outside the authorized scopes"
+      assert result =~ "This action is outside of authorized scopes"
     end
 
     test "returns 401 for scopes beyond app scopes hierarchy", %{conn: conn} do
@@ -882,7 +941,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
       assert result =~ redirect_uri
 
       # Error message
-      assert result =~ "This action is outside the authorized scopes"
+      assert result =~ "This action is outside of authorized scopes"
     end
   end