updated error messages for authentication process

This commit is contained in:
Maksim Pechnikov 2020-01-17 14:55:36 +03:00
parent 52a0bf62f5
commit 108a39c876
5 changed files with 149 additions and 73 deletions

View file

@ -11,11 +11,9 @@ defmodule Pleroma.Plugs.UserEnabledPlug do
end end
def call(%{assigns: %{user: %User{} = user}} = conn, _) do def call(%{assigns: %{user: %User{} = user}} = conn, _) do
if User.auth_active?(user) do case User.account_status(user) do
conn :active -> conn
else _ -> assign(conn, :user, nil)
conn
|> assign(:user, nil)
end end
end end

View file

@ -12,6 +12,7 @@ defmodule Pleroma.User do
alias Comeonin.Pbkdf2 alias Comeonin.Pbkdf2
alias Ecto.Multi alias Ecto.Multi
alias Pleroma.Activity alias Pleroma.Activity
alias Pleroma.Config
alias Pleroma.Conversation.Participation alias Pleroma.Conversation.Participation
alias Pleroma.Delivery alias Pleroma.Delivery
alias Pleroma.FollowingRelationship alias Pleroma.FollowingRelationship
@ -35,7 +36,7 @@ defmodule Pleroma.User do
require Logger require Logger
@type t :: %__MODULE__{} @type t :: %__MODULE__{}
@type account_status :: :active | :deactivated | :password_reset_pending | :confirmation_pending
@primary_key {:id, FlakeId.Ecto.CompatType, autogenerate: true} @primary_key {:id, FlakeId.Ecto.CompatType, autogenerate: true}
# credo:disable-for-next-line Credo.Check.Readability.MaxLineLength # credo:disable-for-next-line Credo.Check.Readability.MaxLineLength
@ -216,14 +217,21 @@ defmodule Pleroma.User do
end end
end end
@doc "Returns if the user should be allowed to authenticate" @doc "Returns status account"
def auth_active?(%User{deactivated: true}), do: false @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}), def account_status(%User{confirmation_pending: true}) do
do: !Pleroma.Config.get([:instance, :account_activation_required]) 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, for_user \\ nil)
def visible_for?(%User{invisible: true}, _), do: false 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{id: user_id}, %User{id: for_id}) when user_id == for_id, do: true
def visible_for?(%User{} = user, for_user) do def visible_for?(%User{} = user, for_user) do
auth_active?(user) || superuser?(for_user) account_status(user) == :active || superuser?(for_user)
end end
def visible_for?(_, _), do: false 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_admin: true}), do: true
def superuser?(%User{local: true, is_moderator: true}), do: true def superuser?(%User{local: true, is_moderator: true}), do: true
def superuser?(_), do: false def superuser?(_), do: false
@spec invisible?(User.t()) :: boolean()
def invisible?(%User{invisible: true}), do: true def invisible?(%User{invisible: true}), do: true
def invisible?(_), do: false def invisible?(_), do: false

View file

@ -167,17 +167,37 @@ defmodule Pleroma.Web.OAuth.OAuthController do
defp handle_create_authorization_error( defp handle_create_authorization_error(
%Plug.Conn{} = conn, %Plug.Conn{} = conn,
{:auth_active, false}, {:account_status, :confirmation_pending},
%{"authorization" => _} = params %{"authorization" => _} = params
) do ) do
# Per https://github.com/tootsuite/mastodon/blob/
# 51e154f5e87968d6bb115e053689767ab33e80cd/app/controllers/api/base_controller.rb#L76
conn conn
|> put_flash(:error, dgettext("errors", "Your login is missing a confirmed e-mail address")) |> put_flash(:error, dgettext("errors", "Your login is missing a confirmed e-mail address"))
|> put_status(:forbidden) |> put_status(:forbidden)
|> authorize(params) |> authorize(params)
end 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 defp handle_create_authorization_error(%Plug.Conn{} = conn, error, %{"authorization" => _}) do
Authenticator.handle_error(conn, error) Authenticator.handle_error(conn, error)
end end
@ -218,46 +238,14 @@ defmodule Pleroma.Web.OAuth.OAuthController do
) do ) do
with {:ok, %User{} = user} <- Authenticator.get_user(conn), with {:ok, %User{} = user} <- Authenticator.get_user(conn),
{:ok, app} <- Token.Utils.fetch_app(conn), {:ok, app} <- Token.Utils.fetch_app(conn),
{:auth_active, true} <- {:auth_active, User.auth_active?(user)}, {:account_status, :active} <- {:account_status, User.account_status(user)},
{:user_active, true} <- {:user_active, !user.deactivated},
{:password_reset_pending, false} <-
{:password_reset_pending, user.password_reset_pending},
{:ok, scopes} <- validate_scopes(app, params), {:ok, scopes} <- validate_scopes(app, params),
{:ok, auth} <- Authorization.create_authorization(app, user, scopes), {:ok, auth} <- Authorization.create_authorization(app, user, scopes),
{:ok, token} <- Token.exchange_token(app, auth) do {:ok, token} <- Token.exchange_token(app, auth) do
json(conn, Token.Response.build(user, token)) json(conn, Token.Response.build(user, token))
else else
{:auth_active, false} -> error ->
# Per https://github.com/tootsuite/mastodon/blob/ handle_token_exchange_error(conn, error)
# 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)
end end
end end
@ -286,6 +274,43 @@ defmodule Pleroma.Web.OAuth.OAuthController do
# Bad request # Bad request
def token_exchange(%Plug.Conn{} = conn, params), do: bad_request(conn, params) 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 def token_revoke(%Plug.Conn{} = conn, %{"token" => _token} = params) do
with {:ok, app} <- Token.Utils.fetch_app(conn), with {:ok, app} <- Token.Utils.fetch_app(conn),
{:ok, _token} <- RevokeToken.revoke(app, params) do {: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), %App{} = app <- Repo.get_by(App, client_id: client_id),
true <- redirect_uri in String.split(app.redirect_uris), true <- redirect_uri in String.split(app.redirect_uris),
{:ok, scopes} <- validate_scopes(app, auth_attrs), {: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) Authorization.create_authorization(app, user, scopes)
end end
end end

View file

@ -1286,23 +1286,35 @@ defmodule Pleroma.UserTest do
end end
end end
test "auth_active?/1 works correctly" do describe "account_status/1" do
clear_config([:instance, :account_activation_required])
test "return confirmation_pending for unconfirm user" do
Pleroma.Config.put([:instance, :account_activation_required], true) Pleroma.Config.put([:instance, :account_activation_required], true)
user = insert(:user, confirmation_pending: true)
assert User.account_status(user) == :confirmation_pending
end
local_user = insert(:user, local: true, confirmation_pending: true) test "return active for confirmed user" do
confirmed_user = insert(:user, local: true, confirmation_pending: false) Pleroma.Config.put([:instance, :account_activation_required], true)
remote_user = insert(:user, local: false) user = insert(:user, confirmation_pending: false)
assert User.account_status(user) == :active
end
refute User.auth_active?(local_user) test "return active for remote user" do
assert User.auth_active?(confirmed_user) user = insert(:user, local: false)
assert User.auth_active?(remote_user) assert User.account_status(user) == :active
end
# also shows unactive for deactivated users test "returns :password_reset_pending for user with reset password" do
user = insert(:user, password_reset_pending: true)
assert User.account_status(user) == :password_reset_pending
end
deactivated_but_confirmed = test "returns :deactivated for deactivated user" do
insert(:user, local: true, confirmation_pending: false, deactivated: true) user = insert(:user, local: true, confirmation_pending: false, deactivated: true)
assert User.account_status(user) == :deactivated
refute User.auth_active?(deactivated_but_confirmed) end
end end
describe "superuser?/1" do describe "superuser?/1" do

View file

@ -819,7 +819,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
|> User.confirmation_changeset(need_confirmation: true) |> User.confirmation_changeset(need_confirmation: true)
|> User.update_and_set_cache() |> User.update_and_set_cache()
refute Pleroma.User.auth_active?(user) refute Pleroma.User.account_status(user) == :active
app = insert(:oauth_app) app = insert(:oauth_app)
@ -849,7 +849,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
app = insert(:oauth_app) app = insert(:oauth_app)
conn = resp =
build_conn() build_conn()
|> post("/oauth/token", %{ |> post("/oauth/token", %{
"grant_type" => "password", "grant_type" => "password",
@ -858,10 +858,12 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
"client_id" => app.client_id, "client_id" => app.client_id,
"client_secret" => app.client_secret "client_secret" => app.client_secret
}) })
|> json_response(403)
assert resp = json_response(conn, 403) assert resp == %{
assert %{"error" => _} = resp "error" => "Your account is currently disabled",
refute Map.has_key?(resp, "access_token") "identifier" => "account_is_disabled"
}
end end
test "rejects token exchange for user with password_reset_pending set to true" do test "rejects token exchange for user with password_reset_pending set to true" do
@ -875,7 +877,7 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
app = insert(:oauth_app, scopes: ["read", "write"]) app = insert(:oauth_app, scopes: ["read", "write"])
conn = resp =
build_conn() build_conn()
|> post("/oauth/token", %{ |> post("/oauth/token", %{
"grant_type" => "password", "grant_type" => "password",
@ -884,12 +886,41 @@ defmodule Pleroma.Web.OAuth.OAuthControllerTest do
"client_id" => app.client_id, "client_id" => app.client_id,
"client_secret" => app.client_secret "client_secret" => app.client_secret
}) })
|> json_response(403)
assert resp = json_response(conn, 403) assert resp == %{
"error" => "Password reset is required",
"identifier" => "password_reset_required"
}
end
assert resp["error"] == "Password reset is required" test "rejects token exchange for user with confirmation_pending set to true" do
assert resp["identifier"] == "password_reset_required" Pleroma.Config.put([:instance, :account_activation_required], true)
refute Map.has_key?(resp, "access_token") password = "testpassword"
user =
insert(:user,
password_hash: Comeonin.Pbkdf2.hashpwsalt(password),
confirmation_pending: true
)
app = insert(:oauth_app, scopes: ["read", "write"])
resp =
build_conn()
|> post("/oauth/token", %{
"grant_type" => "password",
"username" => user.nickname,
"password" => password,
"client_id" => app.client_id,
"client_secret" => app.client_secret
})
|> json_response(403)
assert resp == %{
"error" => "Your login is missing a confirmed e-mail address",
"identifier" => "missing_confirmed_email"
}
end end
test "rejects an invalid authorization code" do test "rejects an invalid authorization code" do