Merge remote-tracking branch 'origin/develop' into fork

This commit is contained in:
marcin mikołajczak 2024-09-06 09:11:48 +02:00
commit 6c4d0e10c0
11 changed files with 125 additions and 130 deletions

View file

@ -0,0 +1 @@
Add a rate limiter to the OAuth App creation endpoint and ensure registered apps are assigned to users.

View file

@ -1 +0,0 @@
Prevent OAuth App flow from creating duplicate entries

View file

@ -610,7 +610,8 @@
crontab: [
{"0 0 * * 0", Pleroma.Workers.Cron.CheckDomainsResolveWorker},
{"0 0 * * 0", Pleroma.Workers.Cron.DigestEmailsWorker},
{"0 0 * * *", Pleroma.Workers.Cron.NewUsersDigestWorker}
{"0 0 * * *", Pleroma.Workers.Cron.NewUsersDigestWorker},
{"*/10 * * * *", Pleroma.Workers.Cron.AppCleanupWorker}
]
config :pleroma, Pleroma.Formatter,
@ -735,6 +736,7 @@
timeline: {500, 3},
search: [{1000, 10}, {1000, 30}],
app_account_creation: {1_800_000, 25},
oauth_app_creation: {900_000, 5},
relations_actions: {10_000, 10},
relation_id_action: {60_000, 2},
statuses_actions: {10_000, 15},

View file

@ -19,6 +19,8 @@ defmodule Pleroma.Web.MastodonAPI.AppController do
action_fallback(Pleroma.Web.MastodonAPI.FallbackController)
plug(Pleroma.Web.Plugs.RateLimiter, [name: :oauth_app_creation] when action == :create)
plug(:skip_auth when action in [:create, :verify_credentials])
plug(Pleroma.Web.ApiSpec.CastAndValidate)
@ -36,7 +38,8 @@ def create(%{body_params: params} = conn, _params) do
|> Map.put(:scopes, scopes)
|> Maps.put_if_present(:user_id, user_id)
with {:ok, app} <- App.get_or_make(app_attrs) do
with cs <- App.register_changeset(%App{}, app_attrs),
{:ok, app} <- Repo.insert(cs) do
render(conn, "show.json", app: app)
end
end

View file

@ -8,6 +8,7 @@ defmodule Pleroma.Web.OAuth.App do
import Ecto.Query
alias Pleroma.Repo
alias Pleroma.User
alias Pleroma.Web.OAuth.Token
@type t :: %__MODULE__{}
@ -67,27 +68,35 @@ def update(id, params) do
with %__MODULE__{} = app <- Repo.get(__MODULE__, id) do
app
|> changeset(params)
|> validate_required([:scopes])
|> Repo.update()
end
end
@doc """
Gets app by attrs or create new with attrs.
Updates the attrs if needed.
And updates the scopes if need.
"""
@spec get_or_make(map()) :: {:ok, t()} | {:error, Ecto.Changeset.t()}
def get_or_make(attrs) do
with %__MODULE__{} = app <- Repo.get_by(__MODULE__, client_name: attrs.client_name) do
__MODULE__.update(app.id, Map.take(attrs, [:scopes, :website]))
@spec get_or_make(map(), list(String.t())) :: {:ok, t()} | {:error, Ecto.Changeset.t()}
def get_or_make(attrs, scopes) do
with %__MODULE__{} = app <- Repo.get_by(__MODULE__, attrs) do
update_scopes(app, scopes)
else
_e ->
%__MODULE__{}
|> register_changeset(attrs)
|> register_changeset(Map.put(attrs, :scopes, scopes))
|> Repo.insert()
end
end
defp update_scopes(%__MODULE__{} = app, []), do: {:ok, app}
defp update_scopes(%__MODULE__{scopes: scopes} = app, scopes), do: {:ok, app}
defp update_scopes(%__MODULE__{} = app, scopes) do
app
|> change(%{scopes: scopes})
|> Repo.update()
end
@spec search(map()) :: {:ok, [t()], non_neg_integer()}
def search(params) do
query = from(a in __MODULE__)
@ -147,4 +156,29 @@ def errors(changeset) do
Map.put(acc, key, error)
end)
end
@spec maybe_update_owner(Token.t()) :: :ok
def maybe_update_owner(%Token{app_id: app_id, user_id: user_id}) when not is_nil(user_id) do
__MODULE__.update(app_id, %{user_id: user_id})
:ok
end
def maybe_update_owner(_), do: :ok
@spec remove_orphans(pos_integer()) :: :ok
def remove_orphans(limit \\ 100) do
fifteen_mins_ago = DateTime.add(DateTime.utc_now(), -900, :second)
Repo.transaction(fn ->
from(a in __MODULE__,
where: is_nil(a.user_id) and a.inserted_at < ^fifteen_mins_ago,
limit: ^limit
)
|> Repo.all()
|> Enum.each(&Repo.delete(&1))
end)
:ok
end
end

View file

@ -318,6 +318,8 @@ def token_exchange(%Plug.Conn{} = conn, %{"grant_type" => "client_credentials"}
def token_exchange(%Plug.Conn{} = conn, params), do: bad_request(conn, params)
def after_token_exchange(%Plug.Conn{} = conn, %{token: token} = view_params) do
App.maybe_update_owner(token)
conn
|> AuthHelper.put_session_token(token.token)
|> json(OAuthView.render("token.json", view_params))

View file

@ -0,0 +1,21 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Workers.Cron.AppCleanupWorker do
@moduledoc """
Cleans up registered apps that were never associated with a user.
"""
use Oban.Worker, queue: "background"
alias Pleroma.Web.OAuth.App
@impl true
def perform(_job) do
App.remove_orphans()
end
@impl true
def timeout(_job), do: :timer.seconds(30)
end

View file

@ -0,0 +1,21 @@
defmodule Pleroma.Repo.Migrations.AssignAppUser do
use Ecto.Migration
alias Pleroma.Repo
alias Pleroma.Web.OAuth.App
alias Pleroma.Web.OAuth.Token
def up do
Repo.all(Token)
|> Enum.group_by(fn x -> Map.get(x, :app_id) end)
|> Enum.each(fn {_app_id, tokens} ->
token =
Enum.filter(tokens, fn x -> not is_nil(x.user_id) end)
|> List.first()
App.maybe_update_owner(token)
end)
end
def down, do: :ok
end

View file

@ -89,114 +89,4 @@ test "creates an oauth app with a user", %{conn: conn} do
assert expected == json_response_and_validate_schema(conn, 200)
assert app.user_id == user.id
end
test "creates an oauth app without a user", %{conn: conn} do
app_attrs = build(:oauth_app)
conn =
conn
|> put_req_header("content-type", "application/json")
|> post("/api/v1/apps", %{
client_name: app_attrs.client_name,
redirect_uris: app_attrs.redirect_uris
})
[app] = Repo.all(App)
expected = %{
"name" => app.client_name,
"website" => app.website,
"client_id" => app.client_id,
"client_secret" => app.client_secret,
"id" => app.id |> to_string(),
"redirect_uri" => app.redirect_uris,
"vapid_key" => Push.vapid_config() |> Keyword.get(:public_key)
}
assert expected == json_response_and_validate_schema(conn, 200)
end
test "does not duplicate apps with the same client name", %{conn: conn} do
client_name = "BleromaSE"
redirect_uris = "https://bleroma.app/oauth-callback"
for _i <- 1..3 do
conn
|> put_req_header("content-type", "application/json")
|> post("/api/v1/apps", %{
client_name: client_name,
redirect_uris: redirect_uris
})
|> json_response_and_validate_schema(200)
end
apps = Repo.all(App)
assert length(apps) == 1
assert List.first(apps).client_name == client_name
assert List.first(apps).redirect_uris == redirect_uris
end
test "app scopes can be updated", %{conn: conn} do
client_name = "BleromaSE"
redirect_uris = "https://bleroma.app/oauth-callback"
website = "https://bleromase.com"
scopes = "read write"
conn
|> put_req_header("content-type", "application/json")
|> post("/api/v1/apps", %{
client_name: client_name,
redirect_uris: redirect_uris,
website: website,
scopes: scopes
})
|> json_response_and_validate_schema(200)
assert List.first(Repo.all(App)).scopes == String.split(scopes, " ")
updated_scopes = "read write push"
conn
|> put_req_header("content-type", "application/json")
|> post("/api/v1/apps", %{
client_name: client_name,
redirect_uris: redirect_uris,
website: website,
scopes: updated_scopes
})
|> json_response_and_validate_schema(200)
assert List.first(Repo.all(App)).scopes == String.split(updated_scopes, " ")
end
test "app website URL can be updated", %{conn: conn} do
client_name = "BleromaSE"
redirect_uris = "https://bleroma.app/oauth-callback"
website = "https://bleromase.com"
conn
|> put_req_header("content-type", "application/json")
|> post("/api/v1/apps", %{
client_name: client_name,
redirect_uris: redirect_uris,
website: website
})
|> json_response_and_validate_schema(200)
assert List.first(Repo.all(App)).website == website
updated_website = "https://bleromase2ultimateedition.com"
conn
|> put_req_header("content-type", "application/json")
|> post("/api/v1/apps", %{
client_name: client_name,
redirect_uris: redirect_uris,
website: updated_website
})
|> json_response_and_validate_schema(200)
assert List.first(Repo.all(App)).website == updated_website
end
end

View file

@ -12,23 +12,20 @@ defmodule Pleroma.Web.OAuth.AppTest do
test "gets exist app" do
attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
app = insert(:oauth_app, Map.merge(attrs, %{scopes: ["read", "write"]}))
{:ok, %App{} = exist_app} = App.get_or_make(attrs)
{:ok, %App{} = exist_app} = App.get_or_make(attrs, [])
assert exist_app == app
end
test "make app" do
attrs = %{client_name: "Mastodon-Local", redirect_uris: ".", scopes: ["write"]}
{:ok, %App{} = app} = App.get_or_make(attrs)
attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
{:ok, %App{} = app} = App.get_or_make(attrs, ["write"])
assert app.scopes == ["write"]
end
test "gets exist app and updates scopes" do
attrs = %{client_name: "Mastodon-Local", redirect_uris: ".", scopes: ["read", "write"]}
app = insert(:oauth_app, attrs)
{:ok, %App{} = exist_app} =
App.get_or_make(%{attrs | scopes: ["read", "write", "follow", "push"]})
attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
app = insert(:oauth_app, Map.merge(attrs, %{scopes: ["read", "write"]}))
{:ok, %App{} = exist_app} = App.get_or_make(attrs, ["read", "write", "follow", "push"])
assert exist_app.id == app.id
assert exist_app.scopes == ["read", "write", "follow", "push"]
end
@ -56,4 +53,21 @@ test "get_user_apps/1" do
assert Enum.sort(App.get_user_apps(user)) == Enum.sort(apps)
end
test "removes orphaned apps" do
attrs = %{client_name: "Mastodon-Local", redirect_uris: "."}
{:ok, %App{} = old_app} = App.get_or_make(attrs, ["write"])
attrs = %{client_name: "PleromaFE", redirect_uris: "."}
{:ok, %App{} = app} = App.get_or_make(attrs, ["write"])
# backdate the old app so it's within the threshold for being cleaned up
{:ok, _} =
"UPDATE apps SET inserted_at = now() - interval '1 hour' WHERE id = #{old_app.id}"
|> Pleroma.Repo.query()
App.remove_orphans()
assert [app] == Pleroma.Repo.all(App)
end
end

View file

@ -12,6 +12,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
alias Pleroma.MFA.TOTP
alias Pleroma.Repo
alias Pleroma.User
alias Pleroma.Web.OAuth.App
alias Pleroma.Web.OAuth.Authorization
alias Pleroma.Web.OAuth.OAuthController
alias Pleroma.Web.OAuth.Token
@ -770,6 +771,9 @@ test "issues a token for an all-body request" do
{:ok, auth} = Authorization.create_authorization(app, user, ["write"])
# Verify app has no associated user yet
assert %Pleroma.Web.OAuth.App{user_id: nil} = Repo.get_by(App, %{id: app.id})
conn =
build_conn()
|> post("/oauth/token", %{
@ -786,6 +790,10 @@ test "issues a token for an all-body request" do
assert token
assert token.scopes == auth.scopes
assert user.ap_id == ap_id
# Verify app has an associated user now
user_id = user.id
assert %Pleroma.Web.OAuth.App{user_id: ^user_id} = Repo.get_by(App, %{id: app.id})
end
test "issues a token for `password` grant_type with valid credentials, with full permissions by default" do