Be more specific with OAuth authorization errors

Say more specifically if the app id is wrong, if the redirect_uri is
wrong, or if the user's credentials are actually wrong. Previously it
always said that the credentials were wrong.

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

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}

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"])