No OneTemporary
Actions

Authored By

Unknown

Size

15 KB

Referenced Files

None

Subscribers

None

View Options

	diff --git a/changelog.d/oauth-app.fix b/changelog.d/oauth-app.fix
	new file mode 100644
	index 000000000..eb917462f
	--- /dev/null
	+++ 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
	index 844673ae0..4677ac40a 100644
	--- a/lib/pleroma/web/mastodon_api/controllers/app_controller.ex
	+++ b/lib/pleroma/web/mastodon_api/controllers/app_controller.ex
	@@ -1,57 +1,56 @@
	# Pleroma: A lightweight social networking server
	# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
	# SPDX-License-Identifier: AGPL-3.0-only

	defmodule Pleroma.Web.MastodonAPI.AppController do
	@moduledoc """
	Controller for supporting app-related actions.
	If authentication is an option, app tokens (user-unbound) must be supported.
	"""

	use Pleroma.Web, :controller

	alias Pleroma.Maps
	alias Pleroma.Repo
	alias Pleroma.User
	alias Pleroma.Web.OAuth.App
	alias Pleroma.Web.OAuth.Scopes
	alias Pleroma.Web.OAuth.Token

	action_fallback(Pleroma.Web.MastodonAPI.FallbackController)

	plug(:skip_auth when action in [:create, :verify_credentials])

	plug(Pleroma.Web.ApiSpec.CastAndValidate)

	defdelegate open_api_operation(action), to: Pleroma.Web.ApiSpec.AppOperation

	@doc "POST /api/v1/apps"
	def create(%{body_params: params} = conn, _params) do
	scopes = Scopes.fetch_scopes(params, ["read"])
	user_id = get_user_id(conn)

	app_attrs =
	params
	\|> Map.take([:client_name, :redirect_uris, :website])
	\|> 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

	defp get_user_id(%{assigns: %{user: %User{id: user_id}}}), do: user_id
	defp get_user_id(_conn), do: nil

	@doc """
	GET /api/v1/apps/verify_credentials
	Gets compact non-secret representation of the app. Supports app tokens and user tokens.
	"""
	def verify_credentials(%{assigns: %{token: %Token{} = token}} = conn, _) do
	with %{app: %App{} = app} <- Repo.preload(token, :app) do
	render(conn, "compact_non_secret.json", app: app)
	end
	end
	end
	diff --git a/lib/pleroma/web/o_auth/app.ex b/lib/pleroma/web/o_auth/app.ex
	index d1bf6dd18..889850c73 100644
	--- a/lib/pleroma/web/o_auth/app.ex
	+++ b/lib/pleroma/web/o_auth/app.ex
	@@ -1,158 +1,150 @@
	# Pleroma: A lightweight social networking server
	# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
	# SPDX-License-Identifier: AGPL-3.0-only

	defmodule Pleroma.Web.OAuth.App do
	use Ecto.Schema
	import Ecto.Changeset
	import Ecto.Query
	alias Pleroma.Repo
	alias Pleroma.User

	@type t :: %__MODULE__{}

	schema "apps" do
	field(:client_name, :string)
	field(:redirect_uris, :string)
	field(:scopes, {:array, :string}, default: [])
	field(:website, :string)
	field(:client_id, :string)
	field(:client_secret, :string)
	field(:trusted, :boolean, default: false)

	belongs_to(:user, User, type: FlakeId.Ecto.CompatType)

	has_many(:oauth_authorizations, Pleroma.Web.OAuth.Authorization, on_delete: :delete_all)
	has_many(:oauth_tokens, Pleroma.Web.OAuth.Token, on_delete: :delete_all)

	timestamps()
	end

	@spec changeset(t(), map()) :: Ecto.Changeset.t()
	def changeset(struct, params) do
	cast(struct, params, [:client_name, :redirect_uris, :scopes, :website, :trusted, :user_id])
	end

	@spec register_changeset(t(), map()) :: Ecto.Changeset.t()
	def register_changeset(struct, params \\ %{}) do
	changeset =
	struct
	\|> changeset(params)
	\|> validate_required([:client_name, :redirect_uris, :scopes])

	if changeset.valid? do
	changeset
	\|> put_change(
	:client_id,
	:crypto.strong_rand_bytes(32) \|> Base.url_encode64(padding: false)
	)
	\|> put_change(
	:client_secret,
	:crypto.strong_rand_bytes(32) \|> Base.url_encode64(padding: false)
	)
	else
	changeset
	end
	end

	@spec create(map()) :: {:ok, t()} \| {:error, Ecto.Changeset.t()}
	def create(params) do
	%__MODULE__{}
	\|> register_changeset(params)
	\|> Repo.insert()
	end

	@spec update(pos_integer(), map()) :: {:ok, t()} \| {:error, Ecto.Changeset.t()} \| nil
	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.
	- 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__)

	query =
	if params[:client_name] do
	from(a in query, where: a.client_name == ^params[:client_name])
	else
	query
	end

	query =
	if params[:client_id] do
	from(a in query, where: a.client_id == ^params[:client_id])
	else
	query
	end

	query =
	if Map.has_key?(params, :trusted) do
	from(a in query, where: a.trusted == ^params[:trusted])
	else
	query
	end

	query =
	from(u in query,
	limit: ^params[:page_size],
	offset: ^((params[:page] - 1) * params[:page_size])
	)

	count = Repo.aggregate(__MODULE__, :count, :id)

	{:ok, Repo.all(query), count}
	end

	@spec get_user_apps(User.t()) :: {:ok, [t()], non_neg_integer()}
	def get_user_apps(%User{id: user_id}) do
	from(a in __MODULE__, where: a.user_id == ^user_id)
	\|> Repo.all()
	end

	@spec destroy(pos_integer()) :: {:ok, t()} \| {:error, Ecto.Changeset.t()}
	def destroy(id) do
	with %__MODULE__{} = app <- Repo.get(__MODULE__, id) do
	Repo.delete(app)
	end
	end

	@spec errors(Ecto.Changeset.t()) :: map()
	def errors(changeset) do
	Enum.reduce(changeset.errors, %{}, fn
	{:client_name, {error, _}}, acc ->
	Map.put(acc, :name, error)

	{key, {error, _}}, acc ->
	Map.put(acc, key, error)
	end)
	end
	end
	diff --git a/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs b/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs
	index bc9d4048c..df28f2010 100644
	--- a/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs
	+++ b/test/pleroma/web/mastodon_api/controllers/app_controller_test.exs
	@@ -1,92 +1,202 @@
	# Pleroma: A lightweight social networking server
	# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
	# SPDX-License-Identifier: AGPL-3.0-only

	defmodule Pleroma.Web.MastodonAPI.AppControllerTest do
	use Pleroma.Web.ConnCase, async: true

	alias Pleroma.Repo
	alias Pleroma.Web.OAuth.App
	alias Pleroma.Web.Push

	import Pleroma.Factory

	test "apps/verify_credentials", %{conn: conn} do
	user_bound_token = insert(:oauth_token)
	app_bound_token = insert(:oauth_token, user: nil)
	refute app_bound_token.user

	for token <- [app_bound_token, user_bound_token] do
	conn =
	conn
	\|> put_req_header("authorization", "Bearer #{token.token}")
	\|> get("/api/v1/apps/verify_credentials")

	app = Repo.preload(token, :app).app

	expected = %{
	"name" => app.client_name,
	"website" => app.website,
	"vapid_key" => Push.vapid_config() \|> Keyword.get(:public_key)
	}

	assert expected == json_response_and_validate_schema(conn, 200)
	end
	end

	test "creates an oauth app", %{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)
	assert app.user_id == nil
	end

	test "creates an oauth app with a user", %{conn: conn} do
	user = insert(:user)
	app_attrs = build(:oauth_app)

	conn =
	conn
	\|> put_req_header("content-type", "application/json")
	\|> assign(:user, user)
	\|> 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)
	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
	index 96a67de6b..423b660ea 100644
	--- a/test/pleroma/web/o_auth/app_test.exs
	+++ b/test/pleroma/web/o_auth/app_test.exs
	@@ -1,56 +1,59 @@
	# Pleroma: A lightweight social networking server
	# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
	# SPDX-License-Identifier: AGPL-3.0-only

	defmodule Pleroma.Web.OAuth.AppTest do
	use Pleroma.DataCase, async: true

	alias Pleroma.Web.OAuth.App
	import Pleroma.Factory

	describe "get_or_make/2" 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

	test "has unique client_id" do
	insert(:oauth_app, client_name: "", redirect_uris: "", client_id: "boop")

	error =
	catch_error(insert(:oauth_app, client_name: "", redirect_uris: "", client_id: "boop"))

	assert %Ecto.ConstraintError{} = error
	assert error.constraint == "apps_client_id_index"
	assert error.type == :unique
	end
	end

	test "get_user_apps/1" do
	user = insert(:user)

	apps = [
	insert(:oauth_app, user_id: user.id),
	insert(:oauth_app, user_id: user.id),
	insert(:oauth_app, user_id: user.id)
	]

	assert Enum.sort(App.get_user_apps(user)) == Enum.sort(apps)
	end
	end

File Metadata

Mime Type: text/x-diff
Expires: Fri, Oct 30, 5:55 AM (1 d, 18 m)
Storage Engine: blob
Storage Format: Raw Data
Storage Handle: 1799621
Default Alt Text: (15 KB)

No OneTemporaryActions

View Options

File Metadata

Event Timeline

No OneTemporary
Actions