diff --git a/changelog.d/specific-oauth-errors.change b/changelog.d/specific-oauth-errors.change new file mode 100644 index 0000000000..300c9ba3a6 --- /dev/null +++ b/changelog.d/specific-oauth-errors.change @@ -0,0 +1 @@ +Instead of saying "Invalid login/password", be more specific with oauth errors when the client_id is not found or the redirect uri is wrong \ No newline at end of file diff --git a/lib/pleroma/web/o_auth/o_auth_controller.ex b/lib/pleroma/web/o_auth/o_auth_controller.ex index 0b3de54816..4ab7f45df7 100644 --- a/lib/pleroma/web/o_auth/o_auth_controller.ex +++ b/lib/pleroma/web/o_auth/o_auth_controller.ex @@ -245,6 +245,36 @@ defmodule Pleroma.Web.OAuth.OAuthController do |> authorize(params) end + defp handle_create_authorization_error( + %Plug.Conn{} = conn, + {:unknown_app, _}, + %{"authorization" => %{"client_id" => client_id}} = params + ) do + conn + |> put_flash( + :error, + dgettext("errors", "Unknown OAuth app client_id: \"%{client_id}\"", %{client_id: client_id}) + ) + |> put_status(:unauthorized) + |> authorize(params) + end + + defp handle_create_authorization_error( + %Plug.Conn{} = conn, + {:wrong_redirect_uri, _}, + %{"authorization" => %{"redirect_uri" => redirect_uri}} = params + ) do + conn + |> put_flash( + :error, + dgettext("errors", "Redirect URI not requested by the app: \"%{redirect_uri}\"", %{ + redirect_uri: redirect_uri + }) + ) + |> put_status(:unauthorized) + |> authorize(params) + end + defp handle_create_authorization_error(%Plug.Conn{} = conn, error, %{"authorization" => _}) do Authenticator.handle_error(conn, error) end @@ -572,8 +602,9 @@ defmodule Pleroma.Web.OAuth.OAuthController do ) do with {_, {:ok, %User{} = user}} <- {:get_user, (user && {:ok, user}) || Authenticator.get_user(conn)}, - %App{} = app <- Repo.get_by(App, client_id: client_id), - true <- redirect_uri in String.split(app.redirect_uris), + {:unknown_app, %App{} = app} <- {:unknown_app, Repo.get_by(App, client_id: client_id)}, + {:wrong_redirect_uri, true} <- + {:wrong_redirect_uri, redirect_uri in String.split(app.redirect_uris)}, requested_scopes <- Scopes.fetch_scopes(auth_attrs, app.scopes), {:ok, auth} <- do_create_authorization(user, app, requested_scopes) do {:ok, auth, user} diff --git a/test/pleroma/web/o_auth/o_auth_controller_test.exs b/test/pleroma/web/o_auth/o_auth_controller_test.exs index 260442771f..da561635bb 100644 --- a/test/pleroma/web/o_auth/o_auth_controller_test.exs +++ b/test/pleroma/web/o_auth/o_auth_controller_test.exs @@ -271,7 +271,9 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do } ) - assert html_response(conn, 401) + result = html_response(conn, 401) + # Error message + assert result =~ "Redirect URI not requested by the app" end test "with invalid params, POST /oauth/register?op=register renders registration_details page", @@ -709,6 +711,29 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do assert result =~ "Invalid Username/Password" end + test "returns 401 for a non-existent app", %{conn: conn} do + user = insert(:user) + app = insert(:oauth_app) + redirect_uri = OAuthController.default_redirect_uri(app) + + result = + conn + |> post("/oauth/authorize", %{ + "authorization" => %{ + "name" => user.nickname, + "password" => "test", + "client_id" => "wrong", + "redirect_uri" => redirect_uri, + "state" => "statepassed", + "scope" => Enum.join(app.scopes, " ") + } + }) + |> html_response(:unauthorized) + + # Error message + assert result =~ "Unknown OAuth app client_id" + end + test "returns 401 for missing scopes" do user = insert(:user, is_admin: false) app = insert(:oauth_app, scopes: ["read", "write", "admin"])