forked from AkkomaGang/akkoma
[#1427] Graceful clearance of OAuth admin scopes for non-admin users (no error raised).
PleromaFE and other clients may safely request admin scope(s): if user isn't an admin, request is successful but only non-admin scopes from request are granted.
This commit is contained in:
parent
79532a7f7c
commit
81b05340e9
2 changed files with 57 additions and 44 deletions
|
@ -79,7 +79,9 @@ defp authorize_admin_scopes(scopes, app_scopes, %User{} = user) do
|
|||
if user.is_admin || !contains_admin_scopes?(scopes) || !contains_admin_scopes?(app_scopes) do
|
||||
{:ok, scopes}
|
||||
else
|
||||
{:error, :unsupported_scopes}
|
||||
# Gracefully dropping admin scopes from requested scopes if user isn't an admin (not raising)
|
||||
scopes = scopes -- OAuthScopesPlug.filter_descendants(scopes, ["admin"])
|
||||
validate(scopes, app_scopes, user)
|
||||
end
|
||||
end
|
||||
|
||||
|
|
|
@ -567,33 +567,41 @@ test "with existing authentication and OOB `redirect_uri`, redirects to app with
|
|||
end
|
||||
|
||||
describe "POST /oauth/authorize" do
|
||||
test "redirects with oauth authorization" do
|
||||
user = insert(:user)
|
||||
app = insert(:oauth_app, scopes: ["read", "write", "follow"])
|
||||
test "redirects with oauth authorization, " <>
|
||||
"keeping only non-admin scopes for non-admin user" do
|
||||
app = insert(:oauth_app, scopes: ["read", "write", "admin"])
|
||||
redirect_uri = OAuthController.default_redirect_uri(app)
|
||||
|
||||
conn =
|
||||
build_conn()
|
||||
|> post("/oauth/authorize", %{
|
||||
"authorization" => %{
|
||||
"name" => user.nickname,
|
||||
"password" => "test",
|
||||
"client_id" => app.client_id,
|
||||
"redirect_uri" => redirect_uri,
|
||||
"scope" => "read:subscope write",
|
||||
"state" => "statepassed"
|
||||
}
|
||||
})
|
||||
non_admin = insert(:user, is_admin: false)
|
||||
admin = insert(:user, is_admin: true)
|
||||
|
||||
target = redirected_to(conn)
|
||||
assert target =~ redirect_uri
|
||||
for {user, expected_scopes} <- %{
|
||||
non_admin => ["read:subscope", "write"],
|
||||
admin => ["read:subscope", "write", "admin"]
|
||||
} do
|
||||
conn =
|
||||
build_conn()
|
||||
|> post("/oauth/authorize", %{
|
||||
"authorization" => %{
|
||||
"name" => user.nickname,
|
||||
"password" => "test",
|
||||
"client_id" => app.client_id,
|
||||
"redirect_uri" => redirect_uri,
|
||||
"scope" => "read:subscope write admin",
|
||||
"state" => "statepassed"
|
||||
}
|
||||
})
|
||||
|
||||
query = URI.parse(target).query |> URI.query_decoder() |> Map.new()
|
||||
target = redirected_to(conn)
|
||||
assert target =~ redirect_uri
|
||||
|
||||
assert %{"state" => "statepassed", "code" => code} = query
|
||||
auth = Repo.get_by(Authorization, token: code)
|
||||
assert auth
|
||||
assert auth.scopes == ["read:subscope", "write"]
|
||||
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
|
||||
end
|
||||
|
||||
test "returns 401 for wrong credentials", %{conn: conn} do
|
||||
|
@ -623,31 +631,34 @@ test "returns 401 for wrong credentials", %{conn: conn} do
|
|||
assert result =~ "Invalid Username/Password"
|
||||
end
|
||||
|
||||
test "returns 401 for missing scopes", %{conn: conn} do
|
||||
user = insert(:user)
|
||||
app = insert(:oauth_app)
|
||||
test "returns 401 for missing scopes " <>
|
||||
"(including all admin-only scopes for non-admin user)" do
|
||||
user = insert(:user, is_admin: false)
|
||||
app = insert(:oauth_app, scopes: ["read", "write", "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" => ""
|
||||
}
|
||||
})
|
||||
|> html_response(:unauthorized)
|
||||
for scope_param <- ["", "admin:read admin:write"] do
|
||||
result =
|
||||
build_conn()
|
||||
|> post("/oauth/authorize", %{
|
||||
"authorization" => %{
|
||||
"name" => user.nickname,
|
||||
"password" => "test",
|
||||
"client_id" => app.client_id,
|
||||
"redirect_uri" => redirect_uri,
|
||||
"state" => "statepassed",
|
||||
"scope" => scope_param
|
||||
}
|
||||
})
|
||||
|> html_response(:unauthorized)
|
||||
|
||||
# Keep the details
|
||||
assert result =~ app.client_id
|
||||
assert result =~ redirect_uri
|
||||
# Keep the details
|
||||
assert result =~ app.client_id
|
||||
assert result =~ redirect_uri
|
||||
|
||||
# Error message
|
||||
assert result =~ "This action is outside the authorized scopes"
|
||||
# Error message
|
||||
assert result =~ "This action is outside the authorized scopes"
|
||||
end
|
||||
end
|
||||
|
||||
test "returns 401 for scopes beyond app scopes hierarchy", %{conn: conn} do
|
||||
|
|
Loading…
Reference in a new issue