forked from AkkomaGang/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
This commit is contained in:
parent
e2320f870e
commit
b8be8192fb
3 changed files with 110 additions and 42 deletions
|
@ -211,11 +211,11 @@ defp handle_create_authorization_error(
|
|||
{: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 @@ 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)},
|
||||
{: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 @@ defp build_and_response_mfa_token(user, auth) 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
|
||||
|
|
|
@ -56,12 +56,20 @@ def to_string(scopes), do: Enum.join(scopes, " ")
|
|||
@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}
|
||||
|
|
|
@ -693,30 +693,25 @@ 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- 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)
|
||||
|
||||
non_admin = insert(:user, is_admin: false)
|
||||
admin = insert(:user, is_admin: true)
|
||||
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
|
||||
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,
|
||||
"name" => admin.nickname,
|
||||
"password" => "test",
|
||||
"client_id" => app.client_id,
|
||||
"redirect_uri" => redirect_uri,
|
||||
"scope" => requested_scopes,
|
||||
"scope" => scopes_subset,
|
||||
"state" => "statepassed"
|
||||
}
|
||||
}
|
||||
|
@ -730,8 +725,44 @@ test "redirects with oauth authorization, " <>
|
|||
assert %{"state" => "statepassed", "code" => code} = query
|
||||
auth = Repo.get_by(Authorization, token: code)
|
||||
assert auth
|
||||
assert auth.scopes == expected_scopes
|
||||
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)
|
||||
scopes_subset = ["read:subscope", "write"]
|
||||
|
||||
# In case scope param is missing, expecting _all_ app-supported scopes to be granted
|
||||
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
|
||||
|
||||
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 "authorize from cookie" do
|
||||
|
@ -739,6 +770,7 @@ test "authorize from cookie" 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 @@ test "returns 401 for wrong credentials", %{conn: conn} 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 @@ test "returns 401 for missing scopes" 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 @@ test "returns 401 for scopes beyond app scopes hierarchy", %{conn: conn} 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
|
||||
|
||||
|
|
Loading…
Reference in a new issue