Merge branch 'revert-9077d092' into 'develop'

Revert "Merge branch 'oauth-app-spam' into 'develop'"

See merge request pleroma/pleroma!4249

changelog.d/oauth-app.fix

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

lib/pleroma/web/mastodon_api/controllers/app_controller.ex

@@ -36,7 +36,8 @@ defmodule Pleroma.Web.MastodonAPI.AppController 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

lib/pleroma/web/o_auth/app.ex

@@ -67,27 +67,35 @@ 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.
-  Updates the attrs if needed.
+  Gets app by attrs or create new  with attrs.
+  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__)

test/pleroma/web/mastodon_api/controllers/app_controller_test.exs

@@ -89,114 +89,4 @@ 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

test/pleroma/web/o_auth/app_test.exs

@@ -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