sam/pleroma
sam/pleroma
1
0
Fork 0
Code Issues Pull requests Projects Releases Wiki Activity

Merge branch 'issue/1280' into 'develop'

Browse source 
[#1280] updated error messages for authentication process

Closes #1280

See merge request pleroma/pleroma!2111
This commit is contained in:
feld 2020-01-26 17:33:12 +00:00
commit f4335ccd7a
5 changed files with 149 additions and 73 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
lib/pleroma/plugs/user_enabled_plug.ex
View file

@ -11,11 +11,9 @@ def init(options) 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

24
lib/pleroma/user.ex
View file

@ -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 @@ def unquote(:"#{outgoing_relation_target}_ap_ids")(user, restrict_deactivated? \
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 @@ def visible_for?(%User{invisible: true}, _), do: false
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

103
lib/pleroma/web/oauth/oauth_controller.ex
View file

@ -167,17 +167,37 @@ defp handle_create_authorization_error(
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 @@ def token_exchange(
) 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 @@ def token_exchange(%Plug.Conn{} = conn, %{"grant_type" => "client_credentials"}
# 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 @@ defp do_create_authorization(
%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

36
test/user_test.exs
View file

@ -1286,23 +1286,35 @@ test "User.delete() plugs any possible zombie objects" do
end
end
test "auth_active?/1 works correctly" do
Pleroma.Config.put([:instance, :account_activation_required], true)
describe "account_status/1" do
clear_config([:instance, :account_activation_required])
local_user = insert(:user, local: true, confirmation_pending: true)
confirmed_user = insert(:user, local: true, confirmation_pending: false)
remote_user = insert(:user, local: false)
test "return confirmation_pending for unconfirm user" do
Pleroma.Config.put([:instance, :account_activation_required], true)
user = insert(:user, confirmation_pending: true)
assert User.account_status(user) == :confirmation_pending
end
refute User.auth_active?(local_user)
assert User.auth_active?(confirmed_user)
assert User.auth_active?(remote_user)
test "return active for confirmed user" do
Pleroma.Config.put([:instance, :account_activation_required], true)
user = insert(:user, confirmation_pending: false)
assert User.account_status(user) == :active
end
# also shows unactive for deactivated users
test "return active for remote user" do
user = insert(:user, local: false)
assert User.account_status(user) == :active
end
deactivated_but_confirmed =
insert(:user, local: true, confirmation_pending: false, deactivated: true)
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
refute User.auth_active?(deactivated_but_confirmed)
test "returns :deactivated for deactivated user" do
user = insert(:user, local: true, confirmation_pending: false, deactivated: true)
assert User.account_status(user) == :deactivated
end
end
describe "superuser?/1" do

51
test/web/oauth/oauth_controller_test.exs
View file

@ -819,7 +819,7 @@ test "rejects token exchange for valid credentials belonging to unconfirmed user
|> User.confirmation_changeset(need_confirmation: true)
|> User.update_and_set_cache()
refute Pleroma.User.auth_active?(user)
refute Pleroma.User.account_status(user) == :active
app = insert(:oauth_app)
@ -849,7 +849,7 @@ test "rejects token exchange for valid credentials belonging to deactivated user
app = insert(:oauth_app)
conn =
resp =
build_conn()
|> post("/oauth/token", %{
"grant_type" => "password",
@ -858,10 +858,12 @@ test "rejects token exchange for valid credentials belonging to deactivated user
"client_id" => app.client_id,
"client_secret" => app.client_secret
})
|> json_response(403)
assert resp = json_response(conn, 403)
assert %{"error" => _} = resp
refute Map.has_key?(resp, "access_token")
assert resp == %{
"error" => "Your account is currently disabled",
"identifier" => "account_is_disabled"
}
end
test "rejects token exchange for user with password_reset_pending set to true" do
@ -875,7 +877,7 @@ test "rejects token exchange for user with password_reset_pending set to true" d
app = insert(:oauth_app, scopes: ["read", "write"])
conn =
resp =
build_conn()
|> post("/oauth/token", %{
"grant_type" => "password",
@ -884,12 +886,41 @@ test "rejects token exchange for user with password_reset_pending set to true" d
"client_id" => app.client_id,
"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"
assert resp["identifier"] == "password_reset_required"
refute Map.has_key?(resp, "access_token")
test "rejects token exchange for user with confirmation_pending set to true" do
Pleroma.Config.put([:instance, :account_activation_required], true)
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
test "rejects an invalid authorization code" do