From 52d8183787db739fc10dd3e95141e274d4fb2bcc Mon Sep 17 00:00:00 2001 From: FloatingGhost Date: Sat, 17 Dec 2022 23:14:49 +0000 Subject: [PATCH] drop admin scopes on create app instead of rejecting --- CHANGELOG.md | 1 + config/description.exs | 3 +-- lib/pleroma/web/o_auth/o_auth_controller.ex | 1 + lib/pleroma/web/o_auth/scopes.ex | 11 +++++++++++ test/pleroma/web/o_auth/o_auth_controller_test.exs | 6 +++--- 5 files changed, 17 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6dcbc4b14..5019fc2f2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ### Changed - Return HTTP error 413 when uploading an avatar or banner that's above the configured upload limit instead of a 500. - Non-admin users now cannot register `admin` scope tokens (not security-critical, they didn't work before, but you _could_ create them) + - Admin scopes will be dropped on create - Rich media will now backoff for 20 minutes after a failure ### Upgrade notes diff --git a/config/description.exs b/config/description.exs index eb61c7218..c43e25468 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2664,8 +2664,7 @@ %{ key: :pool_size, type: :integer, - description: - "Number of concurrent outbound HTTP requests to allow. Default 50.", + description: "Number of concurrent outbound HTTP requests to allow. Default 50.", suggestions: [50] }, %{ diff --git a/lib/pleroma/web/o_auth/o_auth_controller.ex b/lib/pleroma/web/o_auth/o_auth_controller.ex index 3943ca449..277df1c46 100644 --- a/lib/pleroma/web/o_auth/o_auth_controller.ex +++ b/lib/pleroma/web/o_auth/o_auth_controller.ex @@ -605,6 +605,7 @@ defp do_create_authorization( 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)}, + requested_scopes <- Scopes.filter_admin_scopes(requested_scopes, user), {:ok, scopes} <- validate_scopes(user, app, requested_scopes), {:ok, auth} <- Authorization.create_authorization(app, user, scopes) do {:ok, auth} diff --git a/lib/pleroma/web/o_auth/scopes.ex b/lib/pleroma/web/o_auth/scopes.ex index 7fe04b912..ccd8d4665 100644 --- a/lib/pleroma/web/o_auth/scopes.ex +++ b/lib/pleroma/web/o_auth/scopes.ex @@ -69,6 +69,17 @@ def validate(scopes, app_scopes, %Pleroma.User{is_admin: is_admin}) do end end + @spec filter_admin_scopes([String.t()], Pleroma.User.t()) :: [String.t()] + @doc """ + Remove admin scopes for non-admins + """ + def filter_admin_scopes(scopes, %Pleroma.User{is_admin: true}), do: scopes + + def filter_admin_scopes(scopes, _user) do + drop_scopes = OAuthScopesPlug.filter_descendants(scopes, ["admin"]) + Enum.reject(scopes, fn scope -> Enum.member?(drop_scopes, scope) end) + end + defp validate_scopes_are_supported(scopes, app_scopes) do case OAuthScopesPlug.filter_descendants(scopes, app_scopes) do ^scopes -> {:ok, 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 d3cc0acb2..bc2d929e5 100644 --- a/test/pleroma/web/o_auth/o_auth_controller_test.exs +++ b/test/pleroma/web/o_auth/o_auth_controller_test.exs @@ -693,7 +693,7 @@ test "with existing authentication and OOB `redirect_uri`, redirects to app with describe "POST /oauth/authorize" do test "redirects with oauth authorization, " <> - "granting requested app-supported scopes to both admin users" do + "granting requested app-supported scopes to admin users" do app_scopes = ["read", "write", "admin", "secret_scope"] app = insert(:oauth_app, scopes: app_scopes) redirect_uri = OAuthController.default_redirect_uri(app) @@ -735,7 +735,7 @@ test "redirects with oauth authorization, " <> redirect_uri = OAuthController.default_redirect_uri(app) non_admin = insert(:user, is_admin: false) - scopes_subset = ["read:subscope", "write"] + scopes_subset = ["read:subscope", "write", "admin", "admin:metrics"] # In case scope param is missing, expecting _all_ app-supported scopes to be granted conn = @@ -762,7 +762,7 @@ test "redirects with oauth authorization, " <> assert %{"state" => "statepassed", "code" => code} = query auth = Repo.get_by(Authorization, token: code) assert auth - assert auth.scopes == scopes_subset + assert auth.scopes == ["read:subscope", "write"] end test "authorize from cookie" do