Merge branch 'issue/1280' into 'develop'

[#1280] updated error messages for authentication process

Closes #1280

See merge request pleroma/pleroma!2111

3 files changed, 84 insertions, 51 deletions

diff --git a/lib/pleroma/plugs/user_enabled_plug.ex b/lib/pleroma/plugs/user_enabled_plug.ex
index 8d102ee5b..7b304eebc 100644
--- a/lib/pleroma/plugs/user_enabled_plug.ex
+++ b/lib/pleroma/plugs/user_enabled_plug.ex
@@ -11,11 +11,9 @@ defmodule Pleroma.Plugs.UserEnabledPlug do
   end
 
   def call(%{assigns: %{user: %User{} = user}} = conn, _) do
-    if User.auth_active?(user) do
-      conn
-    else
-      conn
-      |> assign(:user, nil)
+    case User.account_status(user) do
+      :active -> conn
+      _ -> assign(conn, :user, nil)
     end
   end
 
diff --git a/lib/pleroma/user.ex b/lib/pleroma/user.ex
index 430f04ae9..3899c34c2 100644
--- a/lib/pleroma/user.ex
+++ b/lib/pleroma/user.ex
@@ -12,6 +12,7 @@ defmodule Pleroma.User do
   alias Comeonin.Pbkdf2
   alias Ecto.Multi
   alias Pleroma.Activity
+  alias Pleroma.Config
   alias Pleroma.Conversation.Participation
   alias Pleroma.Delivery
   alias Pleroma.FollowingRelationship
@@ -35,7 +36,7 @@ defmodule Pleroma.User do
   require Logger
 
   @type t :: %__MODULE__{}
-
+  @type account_status :: :active | :deactivated | :password_reset_pending | :confirmation_pending
   @primary_key {:id, FlakeId.Ecto.CompatType, autogenerate: true}
 
   # credo:disable-for-next-line Credo.Check.Readability.MaxLineLength
@@ -216,14 +217,21 @@ defmodule Pleroma.User do
     end
   end
 
-  @doc "Returns if the user should be allowed to authenticate"
-  def auth_active?(%User{deactivated: true}), do: false
+  @doc "Returns status account"
+  @spec account_status(User.t()) :: account_status()
+  def account_status(%User{deactivated: true}), do: :deactivated
+  def account_status(%User{password_reset_pending: true}), do: :password_reset_pending
 
-  def auth_active?(%User{confirmation_pending: true}),
-    do: !Pleroma.Config.get([:instance, :account_activation_required])
+  def account_status(%User{confirmation_pending: true}) do
+    case Config.get([:instance, :account_activation_required]) do
+      true -> :confirmation_pending
+      _ -> :active
+    end
+  end
 
-  def auth_active?(%User{}), do: true
+  def account_status(%User{}), do: :active
 
+  @spec visible_for?(User.t(), User.t() | nil) :: boolean()
   def visible_for?(user, for_user \\ nil)
 
   def visible_for?(%User{invisible: true}, _), do: false
@@ -231,15 +239,17 @@ defmodule Pleroma.User do
   def visible_for?(%User{id: user_id}, %User{id: for_id}) when user_id == for_id, do: true
 
   def visible_for?(%User{} = user, for_user) do
-    auth_active?(user) || superuser?(for_user)
+    account_status(user) == :active || superuser?(for_user)
   end
 
   def visible_for?(_, _), do: false
 
+  @spec superuser?(User.t()) :: boolean()
   def superuser?(%User{local: true, is_admin: true}), do: true
   def superuser?(%User{local: true, is_moderator: true}), do: true
   def superuser?(_), do: false
 
+  @spec invisible?(User.t()) :: boolean()
   def invisible?(%User{invisible: true}), do: true
   def invisible?(_), do: false
 
diff --git a/lib/pleroma/web/oauth/oauth_controller.ex b/lib/pleroma/web/oauth/oauth_controller.ex
index 5292aedf2..528f08574 100644
--- a/lib/pleroma/web/oauth/oauth_controller.ex
+++ b/lib/pleroma/web/oauth/oauth_controller.ex
@@ -167,17 +167,37 @@ defmodule Pleroma.Web.OAuth.OAuthController do
 
   defp handle_create_authorization_error(
          %Plug.Conn{} = conn,
-         {:auth_active, false},
+         {:account_status, :confirmation_pending},
          %{"authorization" => _} = params
        ) do
-    # Per https://github.com/tootsuite/mastodon/blob/
-    #   51e154f5e87968d6bb115e053689767ab33e80cd/app/controllers/api/base_controller.rb#L76
     conn
     |> put_flash(:error, dgettext("errors", "Your login is missing a confirmed e-mail address"))
     |> put_status(:forbidden)
     |> authorize(params)
   end
 
+  defp handle_create_authorization_error(
+         %Plug.Conn{} = conn,
+         {:account_status, :password_reset_pending},
+         %{"authorization" => _} = params
+       ) do
+    conn
+    |> put_flash(:error, dgettext("errors", "Password reset is required"))
+    |> put_status(:forbidden)
+    |> authorize(params)
+  end
+
+  defp handle_create_authorization_error(
+         %Plug.Conn{} = conn,
+         {:account_status, :deactivated},
+         %{"authorization" => _} = params
+       ) do
+    conn
+    |> put_flash(:error, dgettext("errors", "Your account is currently disabled"))
+    |> put_status(:forbidden)
+    |> authorize(params)
+  end
+
   defp handle_create_authorization_error(%Plug.Conn{} = conn, error, %{"authorization" => _}) do
     Authenticator.handle_error(conn, error)
   end
@@ -218,46 +238,14 @@ defmodule Pleroma.Web.OAuth.OAuthController do
       ) do
     with {:ok, %User{} = user} <- Authenticator.get_user(conn),
          {:ok, app} <- Token.Utils.fetch_app(conn),
-         {:auth_active, true} <- {:auth_active, User.auth_active?(user)},
-         {:user_active, true} <- {:user_active, !user.deactivated},
-         {:password_reset_pending, false} <-
-           {:password_reset_pending, user.password_reset_pending},
+         {:account_status, :active} <- {:account_status, User.account_status(user)},
          {:ok, scopes} <- validate_scopes(app, params),
          {:ok, auth} <- Authorization.create_authorization(app, user, scopes),
          {:ok, token} <- Token.exchange_token(app, auth) do
       json(conn, Token.Response.build(user, token))
     else
-      {:auth_active, false} ->
-        # Per https://github.com/tootsuite/mastodon/blob/
-        #   51e154f5e87968d6bb115e053689767ab33e80cd/app/controllers/api/base_controller.rb#L76
-        render_error(
-          conn,
-          :forbidden,
-          "Your login is missing a confirmed e-mail address",
-          %{},
-          "missing_confirmed_email"
-        )
-
-      {:user_active, false} ->
-        render_error(
-          conn,
-          :forbidden,
-          "Your account is currently disabled",
-          %{},
-          "account_is_disabled"
-        )
-
-      {:password_reset_pending, true} ->
-        render_error(
-          conn,
-          :forbidden,
-          "Password reset is required",
-          %{},
-          "password_reset_required"
-        )
-
-      _error ->
-        render_invalid_credentials_error(conn)
+      error ->
+        handle_token_exchange_error(conn, error)
     end
   end
 
@@ -286,6 +274,43 @@ defmodule Pleroma.Web.OAuth.OAuthController do
   # Bad request
   def token_exchange(%Plug.Conn{} = conn, params), do: bad_request(conn, params)
 
+  defp handle_token_exchange_error(%Plug.Conn{} = conn, {:account_status, :deactivated}) do
+    render_error(
+      conn,
+      :forbidden,
+      "Your account is currently disabled",
+      %{},
+      "account_is_disabled"
+    )
+  end
+
+  defp handle_token_exchange_error(
+         %Plug.Conn{} = conn,
+         {:account_status, :password_reset_pending}
+       ) do
+    render_error(
+      conn,
+      :forbidden,
+      "Password reset is required",
+      %{},
+      "password_reset_required"
+    )
+  end
+
+  defp handle_token_exchange_error(%Plug.Conn{} = conn, {:account_status, :confirmation_pending}) do
+    render_error(
+      conn,
+      :forbidden,
+      "Your login is missing a confirmed e-mail address",
+      %{},
+      "missing_confirmed_email"
+    )
+  end
+
+  defp handle_token_exchange_error(%Plug.Conn{} = conn, _error) do
+    render_invalid_credentials_error(conn)
+  end
+
   def token_revoke(%Plug.Conn{} = conn, %{"token" => _token} = params) do
     with {:ok, app} <- Token.Utils.fetch_app(conn),
          {:ok, _token} <- RevokeToken.revoke(app, params) do
@@ -472,7 +497,7 @@ defmodule Pleroma.Web.OAuth.OAuthController do
          %App{} = app <- Repo.get_by(App, client_id: client_id),
          true <- redirect_uri in String.split(app.redirect_uris),
          {:ok, scopes} <- validate_scopes(app, auth_attrs),
-         {:auth_active, true} <- {:auth_active, User.auth_active?(user)} do
+         {:account_status, :active} <- {:account_status, User.account_status(user)} do
       Authorization.create_authorization(app, user, scopes)
     end
   end