drop admin scopes on create app instead of rejecting

This commit is contained in:
FloatingGhost 2022-12-17 23:14:49 +00:00
parent dcac8adb3d
commit 52d8183787
5 changed files with 17 additions and 5 deletions

View file

@ -18,6 +18,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/).
### Changed ### Changed
- Return HTTP error 413 when uploading an avatar or banner that's above the configured upload limit instead of a 500. - 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) - 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 - Rich media will now backoff for 20 minutes after a failure
### Upgrade notes ### Upgrade notes

View file

@ -2664,8 +2664,7 @@
%{ %{
key: :pool_size, key: :pool_size,
type: :integer, type: :integer,
description: description: "Number of concurrent outbound HTTP requests to allow. Default 50.",
"Number of concurrent outbound HTTP requests to allow. Default 50.",
suggestions: [50] suggestions: [50]
}, },
%{ %{

View file

@ -605,6 +605,7 @@ defp do_create_authorization(
defp do_create_authorization(%User{} = user, %App{} = app, requested_scopes) defp do_create_authorization(%User{} = user, %App{} = app, requested_scopes)
when is_list(requested_scopes) do when is_list(requested_scopes) do
with {:account_status, :active} <- {:account_status, User.account_status(user)}, 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, scopes} <- validate_scopes(user, app, requested_scopes),
{:ok, auth} <- Authorization.create_authorization(app, user, scopes) do {:ok, auth} <- Authorization.create_authorization(app, user, scopes) do
{:ok, auth} {:ok, auth}

View file

@ -69,6 +69,17 @@ def validate(scopes, app_scopes, %Pleroma.User{is_admin: is_admin}) do
end end
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 defp validate_scopes_are_supported(scopes, app_scopes) do
case OAuthScopesPlug.filter_descendants(scopes, app_scopes) do case OAuthScopesPlug.filter_descendants(scopes, app_scopes) do
^scopes -> {:ok, scopes} ^scopes -> {:ok, scopes}

View file

@ -693,7 +693,7 @@ test "with existing authentication and OOB `redirect_uri`, redirects to app with
describe "POST /oauth/authorize" do describe "POST /oauth/authorize" do
test "redirects with oauth authorization, " <> 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_scopes = ["read", "write", "admin", "secret_scope"]
app = insert(:oauth_app, scopes: app_scopes) app = insert(:oauth_app, scopes: app_scopes)
redirect_uri = OAuthController.default_redirect_uri(app) redirect_uri = OAuthController.default_redirect_uri(app)
@ -735,7 +735,7 @@ test "redirects with oauth authorization, " <>
redirect_uri = OAuthController.default_redirect_uri(app) redirect_uri = OAuthController.default_redirect_uri(app)
non_admin = insert(:user, is_admin: false) 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 # In case scope param is missing, expecting _all_ app-supported scopes to be granted
conn = conn =
@ -762,7 +762,7 @@ test "redirects with oauth authorization, " <>
assert %{"state" => "statepassed", "code" => code} = query assert %{"state" => "statepassed", "code" => code} = query
auth = Repo.get_by(Authorization, token: code) auth = Repo.get_by(Authorization, token: code)
assert auth assert auth
assert auth.scopes == scopes_subset assert auth.scopes == ["read:subscope", "write"]
end end
test "authorize from cookie" do test "authorize from cookie" do