commit: 9077d0925b61b0ba23c0ee5b41627dc91266d52d
parent 61e4be396f741cf45a2861e026b1e5fb3d185d99
Author: feld <feld@feld.me>
Date: Sun, 1 Sep 2024 18:24:06 +0000
Merge branch 'oauth-app-spam' into 'develop'
Fix OAuth app spam
See merge request pleroma/pleroma!4244
Diffstat:
5 files changed, 129 insertions(+), 24 deletions(-)
diff --git a/changelog.d/oauth-app.fix b/changelog.d/oauth-app.fix
@@ -0,0 +1 @@
+Prevent OAuth App flow from creating duplicate entries
diff --git a/lib/pleroma/web/mastodon_api/controllers/app_controller.ex b/lib/pleroma/web/mastodon_api/controllers/app_controller.ex
@@ -36,8 +36,7 @@ defmodule Pleroma.Web.MastodonAPI.AppController do
|> Map.put(:scopes, scopes)
|> Maps.put_if_present(:user_id, user_id)
- with cs <- App.register_changeset(%App{}, app_attrs),
- {:ok, app} <- Repo.insert(cs) do
+ with {:ok, app} <- App.get_or_make(app_attrs) do
render(conn, "show.json", app: app)
end
end
diff --git a/lib/pleroma/web/o_auth/app.ex b/lib/pleroma/web/o_auth/app.ex
@@ -67,35 +67,27 @@ defmodule Pleroma.Web.OAuth.App 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.
- And updates the scopes if need.
+ Gets app by attrs or create new with attrs.
+ Updates the attrs if needed.
"""
- @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)
+ @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]))
else
_e ->
%__MODULE__{}
- |> register_changeset(Map.put(attrs, :scopes, scopes))
+ |> register_changeset(attrs)
|> 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__)
diff --git a/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs
@@ -89,4 +89,114 @@ defmodule Pleroma.Web.MastodonAPI.AppControllerTest 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
diff --git a/test/pleroma/web/o_auth/app_test.exs b/test/pleroma/web/o_auth/app_test.exs
@@ -12,20 +12,23 @@ 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: "."}
- {:ok, %App{} = app} = App.get_or_make(attrs, ["write"])
+ attrs = %{client_name: "Mastodon-Local", redirect_uris: ".", scopes: ["write"]}
+ {:ok, %App{} = app} = App.get_or_make(attrs)
assert app.scopes == ["write"]
end
test "gets exist app and updates scopes" 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, ["read", "write", "follow", "push"])
+ 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"]})
+
assert exist_app.id == app.id
assert exist_app.scopes == ["read", "write", "follow", "push"]
end
